Skip to content

feat: Remove multistaking token proposal - #153

Merged
hieuvubk merged 11 commits into
mainfrom
hieu/remove-ms-token
Sep 18, 2025
Merged

feat: Remove multistaking token proposal#153
hieuvubk merged 11 commits into
mainfrom
hieu/remove-ms-token

Conversation

@hieuvubk

Copy link
Copy Markdown
Contributor
  • Add a proposal for removing multistaking token
  • How it affect to current validators and delegations of that token:
    • Get all delegations of multistaking token will remove
    • Force undelegate all delegation and remove bond denom, 
    • Since token was removed and all delegation was removed from validators, they will be jailed immediately.
    • After unbonding period, locked token will be transfered back to user. Validator will be removed from store.

@jiujiteiro jiujiteiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. can you please update docs in /spec/02_gov.md?

Comment on lines +112 to +115
_, found := k.GetBondWeight(ctx, p.Denom)
if !found {
return fmt.Errorf("Error MultiStakingCoin %s not found", p.Denom) //nolint:stylecheck
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If bond weight is 0 before this happens, would this work or throw an error here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don’t accept zero weight anymore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should still be able to handle that scenario though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont get it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what u think we should do with zero weight then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anw the scenario that weight is zero can not happen since we block it in AddMultiStakingCoinProposal and UpdateBondWeightProposal also.
There must be 2 state of a denom, non-zero or be removed.

Comment thread x/multi-staking/keeper/proposal.go Outdated
return fmt.Errorf("Error MultiStakingCoin %s not found", p.Denom) //nolint:stylecheck
}

msgServer := NewMsgServerImpl(k)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make more sense to me to make this function, call an internal keeper.Undelegate function, than a keeper method creating a MsgServer instance that calls the keeper internally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can not call keeper.Undelegate func since Undelegate is msgServer implementation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is to take the logic within MsgServer.Undelegate and create a new Kepper.Undelegate method that does everything there. And call that method from within MsgServer.Undelegate instead

@hieuvubk hieuvubk Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it break msg_server.go a lot since Undelegate, Delegate,... they all are MsgServer funcs for now. I can wrap the for loop for 1 func like ForceUndelegateFromDenom or st

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an alternative implementation. Take a look to see what you think

Comment thread x/multi-staking/keeper/proposal.go
@hieuvubk
hieuvubk merged commit 7a9feb0 into main Sep 18, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants