Skip to content

fix: prevent panic in RedeemTokensForShares when too few tokens are redeemed #19076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sainoe merged 7 commits into from
Jan 17, 2024
Merged

fix: prevent panic in RedeemTokensForShares when too few tokens are redeemed #19076

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3e4d748
fix bug in redeem when a 0 amount is returned
sainoe Jan 16, 2024
9cefe22
remove not needed check
sainoe Jan 16, 2024
37f5d9f
nits
sainoe Jan 16, 2024
c32cba0
rm unwanted file
sainoe Jan 16, 2024
74a9512
rm unwanted file
sainoe Jan 16, 2024
dc36624
nit
sainoe Jan 17, 2024
d2c1535
update changes
sainoe Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion x/staking/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,12 +855,18 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe
// Similar to undelegations, if the account is attempting to tokenize the full delegation,
// but there's a precision error due to the decimal to int conversion, round up to the
// full decimal amount before modifying the delegation
shares := sdk.NewDecFromInt(shareToken.Amount)
shareDenomSupply := k.bankKeeper.GetSupply(ctx, shareToken.Denom)
shares := delegation.Shares.Mul(sdk.NewDecFromInt(shareToken.Amount)).QuoInt(shareDenomSupply.Amount)

@sampocs sampocs Jan 17, 2024

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.

Apologies for the confusion, but can you elaborate on why this is inaccurate in the case of a validator having pending rewards or being slashed?

The LSM token is intended to map 1:1 with shares (it is minted based on shares, not tokens), so I was under the assumption that it would naturally capture rewards/slashing.

@sainoe sainoe Jan 17, 2024 •
edited
Loading

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.

The LSM token is intended to map 1:1 with shares (it is minted based on shares, not tokens), so I was under the assumption that it would naturally capture rewards/slashing.

You're 100% right. Sorry, I got it the wrong way. The simulation tests were failing due to these lines, which don't use a 1:1 mapping between LSM tokens and shares.

if shareToken.Amount.Equal(delegation.Shares.TruncateInt()) {
shares = delegation.Shares
}
tokens := validator.TokensFromShares(shares).TruncateInt()

// prevent redemption that returns a 0 amount
if tokens.IsZero() {
return nil, types.ErrTinyRedeemAmount
}

// If this redemption is NOT from a liquid staking provider, decrement the total liquid staked
// If the redemption was from a liquid staking provider, the shares are still considered
// liquid, even in their non-tokenized form (since they are owned by a liquid staking provider)
Expand Down
1 change: 1 addition & 0 deletions x/staking/types/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,5 @@ var (
ErrTokenizeSharesAlreadyDisabledForAccount = sdkerrors.Register(ModuleName, 116, "tokenize shares is already disabled for this account")
ErrValidatorLiquidSharesUnderflow = sdkerrors.Register(ModuleName, 117, "validator liquid shares underflow")
ErrTotalLiquidStakedUnderflow = sdkerrors.Register(ModuleName, 118, "total liquid staked underflow")
ErrTinyRedeemAmount = sdkerrors.Register(ModuleName, 119, "too few tokens to redeem (truncates to zero tokens)")
Comment thread
sainoe marked this conversation as resolved.
Outdated
)