From ad084a41e83dd0acaacb2b55b6f3f68860edfd5f Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 14 Jul 2023 10:33:26 -0400 Subject: [PATCH 1/4] handled rounding precision case in tokenize and redeem --- x/staking/keeper/msg_server.go | 51 +++++++++++++++++----------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index e7f1fb334464..0db31ec4c7b2 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -646,11 +646,6 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS return nil, types.ErrOnlyBondDenomAllowdForTokenize } - delegationAmount := sdk.NewDecFromInt(validator.Tokens).Mul(delegation.GetShares()).Quo(validator.DelegatorShares) - if sdk.NewDecFromInt(msg.Amount.Amount).GT(delegationAmount) { - return nil, types.ErrNotEnoughDelegationShares - } - acc := k.authKeeper.GetAccount(ctx, delegatorAddress) if acc != nil { acc, ok := acc.(vesting.VestingAccount) @@ -696,29 +691,32 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS Validator: msg.ValidatorAddress, } - shareToken := sdk.NewCoin(record.GetShareTokenDenom(), msg.Amount.Amount) - - err = k.bankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.Coins{shareToken}) + // note: this returnAmount can be slightly off from the original delegation amount if there + // is a decimal to int precision error + returnAmount, err := k.Unbond(ctx, delegatorAddress, valAddr, shares) if err != nil { return nil, err } - err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, delegatorAddress, sdk.Coins{shareToken}) - if err != nil { - return nil, err + if validator.IsBonded() { + k.bondedTokensToNotBonded(ctx, returnAmount) } - returnAmount, err := k.Unbond(ctx, delegatorAddress, valAddr, shares) + // Note: UndelegateCoinsFromModuleToAccount is internally calling TrackUndelegation for vesting account + returnCoin := sdk.NewCoin(k.BondDenom(ctx), returnAmount) + err = k.bankKeeper.UndelegateCoinsFromModuleToAccount(ctx, types.NotBondedPoolName, delegatorAddress, sdk.Coins{returnCoin}) if err != nil { return nil, err } - if validator.IsBonded() { - k.bondedTokensToNotBonded(ctx, returnAmount) + shareToken := sdk.NewCoin(record.GetShareTokenDenom(), msg.Amount.Amount) + + err = k.bankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.Coins{shareToken}) + if err != nil { + return nil, err } - // Note: UndelegateCoinsFromModuleToAccount is internally calling TrackUndelegation for vesting account - err = k.bankKeeper.UndelegateCoinsFromModuleToAccount(ctx, types.NotBondedPoolName, delegatorAddress, sdk.Coins{msg.Amount}) + err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, delegatorAddress, sdk.Coins{shareToken}) if err != nil { return nil, err } @@ -729,7 +727,7 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS return nil, err } // send coins to module account - err = k.bankKeeper.SendCoins(ctx, delegatorAddress, record.GetModuleAddress(), sdk.Coins{msg.Amount}) + err = k.bankKeeper.SendCoins(ctx, delegatorAddress, record.GetModuleAddress(), sdk.Coins{returnCoin}) if err != nil { return nil, err } @@ -741,7 +739,7 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS } // delegate from module account - _, err = k.Keeper.Delegate(ctx, record.GetModuleAddress(), msg.Amount.Amount, types.Unbonded, validator, true) + _, err = k.Keeper.Delegate(ctx, record.GetModuleAddress(), returnAmount, types.Unbonded, validator, true) if err != nil { return nil, err } @@ -771,12 +769,13 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe return nil, err } - balance := k.bankKeeper.GetBalance(ctx, delegatorAddress, msg.Amount.Denom) - if balance.Amount.LT(msg.Amount.Amount) { + shareToken := msg.Amount + balance := k.bankKeeper.GetBalance(ctx, delegatorAddress, shareToken.Denom) + if balance.Amount.LT(shareToken.Amount) { return nil, types.ErrNotEnoughBalance } - record, err := k.GetTokenizeShareRecordByDenom(ctx, msg.Amount.Denom) + record, err := k.GetTokenizeShareRecordByDenom(ctx, shareToken.Denom) if err != nil { return nil, err } @@ -797,8 +796,8 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe if !found { return nil, types.ErrNoUnbondingDelegation } - shareDenomSupply := k.bankKeeper.GetSupply(ctx, msg.Amount.Denom) - shares := delegation.Shares.Mul(sdk.NewDecFromInt(msg.Amount.Amount)).QuoInt(shareDenomSupply.Amount) + shareDenomSupply := k.bankKeeper.GetSupply(ctx, shareToken.Denom) + shares := delegation.Shares.Mul(sdk.NewDecFromInt(shareToken.Amount)).QuoInt(shareDenomSupply.Amount) tokens := validator.TokensFromShares(shares).TruncateInt() // If this redemption is NOT from a liquid staking provider, decrement the total liquid staked @@ -837,11 +836,11 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe } // send share tokens to NotBondedPool and burn - err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, delegatorAddress, types.NotBondedPoolName, sdk.Coins{msg.Amount}) + err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, delegatorAddress, types.NotBondedPoolName, sdk.Coins{shareToken}) if err != nil { return nil, err } - err = k.bankKeeper.BurnCoins(ctx, types.NotBondedPoolName, sdk.Coins{msg.Amount}) + err = k.bankKeeper.BurnCoins(ctx, types.NotBondedPoolName, sdk.Coins{shareToken}) if err != nil { return nil, err } @@ -871,7 +870,7 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe types.EventTypeRedeemShares, sdk.NewAttribute(types.AttributeKeyDelegator, msg.DelegatorAddress), sdk.NewAttribute(types.AttributeKeyValidator, validator.OperatorAddress), - sdk.NewAttribute(types.AttributeKeyAmount, msg.Amount.String()), + sdk.NewAttribute(types.AttributeKeyAmount, shareToken.String()), ), ) From da7f8b7746e17e3eb951ba4feffb6560aefbca91 Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 14 Jul 2023 10:35:39 -0400 Subject: [PATCH 2/4] updated lsm token to map 1:1 with shares --- x/staking/keeper/msg_server.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 0db31ec4c7b2..51b0591dff55 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -709,7 +709,14 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS return nil, err } - shareToken := sdk.NewCoin(record.GetShareTokenDenom(), msg.Amount.Amount) + // Re-calculate the shares in case there was rounding precision during the undelegation + newShares, err := validator.SharesFromTokens(returnAmount) + if err != nil { + return nil, err + } + + // The share tokens returned maps 1:1 with shares + shareToken := sdk.NewCoin(record.GetShareTokenDenom(), newShares.TruncateInt()) err = k.bankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.Coins{shareToken}) if err != nil { @@ -790,14 +797,18 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe return nil, types.ErrNoValidatorFound } - // calculate the ratio between shares and redeem amount - // moduleAccountTotalDelegation * redeemAmount / totalIssue delegation, found := k.GetDelegation(ctx, record.GetModuleAddress(), valAddr) if !found { return nil, types.ErrNoUnbondingDelegation } - shareDenomSupply := k.bankKeeper.GetSupply(ctx, shareToken.Denom) - shares := delegation.Shares.Mul(sdk.NewDecFromInt(shareToken.Amount)).QuoInt(shareDenomSupply.Amount) + + // 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 := shareToken.Amount.ToDec() + if shareToken.Amount.Equal(delegation.Shares.TruncateInt()) { + shares = delegation.Shares + } tokens := validator.TokensFromShares(shares).TruncateInt() // If this redemption is NOT from a liquid staking provider, decrement the total liquid staked From f5fd65bd7c7754338364e2d86f8b9b8ef633a21c Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 14 Jul 2023 10:35:49 -0400 Subject: [PATCH 3/4] added unit tests around the precision error cases --- x/staking/keeper/msg_server_test.go | 226 +++++++++++++++++++++++++++- 1 file changed, 225 insertions(+), 1 deletion(-) diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 70908dd55a2f..ad9f14c0bccf 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -496,7 +496,6 @@ func TestTokenizeSharesAndRedeemTokens(t *testing.T) { slashedTokens = sdk.NewDecFromInt(val1.Tokens).Mul(tc.slashFactor).TruncateInt() val1, _ := app.StakingKeeper.GetValidator(ctx, addrVal1) - redeemedShares = delegation.Shares.Mul(sdk.NewDecFromInt(tc.redeemAmount)).QuoInt(shareToken.Amount).TruncateInt() redeemedTokens = val1.TokensFromShares(sdk.NewDecFromInt(redeemedShares)).TruncateInt() } @@ -590,6 +589,231 @@ func TestTokenizeSharesAndRedeemTokens(t *testing.T) { } } +// Helper function to setup a delegator and validator for the Tokenize/Redeem conversion tests +func setupTestTokenizeAndRedeemConversion( + t *testing.T, + app *simapp.SimApp, + ctx sdk.Context, +) (delAddress sdk.AccAddress, valAddress sdk.ValAddress) { + addresses := simapp.AddTestAddrs(app, ctx, 2, sdk.NewInt(1_000_000)) + pubKeys := simapp.CreateTestPubKeys(1) + + delegatorAddress := addresses[0] + validatorAddress := sdk.ValAddress(addresses[1]) + + validator := teststaking.NewValidator(t, validatorAddress, pubKeys[0]) + validator.DelegatorShares = sdk.NewDec(1_000_000) + validator.Tokens = sdk.NewInt(1_000_000) + validator.TotalLiquidShares = sdk.NewDec(0) + validator.Status = types.Bonded + + app.StakingKeeper.SetValidator(ctx, validator) + app.StakingKeeper.SetValidatorByConsAddr(ctx, validator) + + return delegatorAddress, validatorAddress +} + +// Simulate a slash by decrementing the validator's tokens +// We'll do this in a way such that the exchange rate is not an even integer +// and the shares associated with a delegation will have a long decimal +func simulateSlashWithImprecision(t *testing.T, app *simapp.SimApp, ctx sdk.Context, valAddress sdk.ValAddress) { + validator, found := app.StakingKeeper.GetValidator(ctx, valAddress) + require.True(t, found) + + slashMagnitude := sdk.MustNewDecFromStr("0.1111111111") + slashTokens := validator.Tokens.ToDec().Mul(slashMagnitude).TruncateInt() + validator.Tokens = validator.Tokens.Sub(slashTokens) + + app.StakingKeeper.SetValidator(ctx, validator) +} + +// Tests the conversion from tokenization and redemption from the following scenario: +// Slash -> Delegate -> Tokenize -> Redeem +// Note, in this example, there 2 tokens are lost during the decimal to int conversion +// during the unbonding step within tokenization and redemption +func TestTokenizeAndRedeemConversion_SlashBeforeDelegation(t *testing.T) { + _, app, ctx := createTestInput() + msgServer := keeper.NewMsgServerImpl(app.StakingKeeper) + + delegatorAddress, validatorAddress := setupTestTokenizeAndRedeemConversion(t, app, ctx) + + // slash the validator + simulateSlashWithImprecision(t, app, ctx, validatorAddress) + validator, found := app.StakingKeeper.GetValidator(ctx, validatorAddress) + require.True(t, found) + + // Delegate and confirm the delegation record was created + delegateAmount := sdk.NewInt(1000) + delegateCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegateAmount) + _, err := msgServer.Delegate(sdk.WrapSDKContext(ctx), &types.MsgDelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAddress.String(), + Amount: delegateCoin, + }) + require.NoError(t, err, "no error expected when delegating") + + delegation, found := app.StakingKeeper.GetDelegation(ctx, delegatorAddress, validatorAddress) + require.True(t, found, "delegation should have been found") + + // Tokenize the full delegation amount + _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAddress.String(), + Amount: delegateCoin, + TokenizedShareOwner: delegatorAddress.String(), + }) + require.NoError(t, err, "no error expected when tokenizing") + + // Confirm the number of shareTokens equals the number of shares truncated + // Note: 1 token is lost during unbonding due to rounding + shareDenom := validatorAddress.String() + "/1" + shareToken := app.BankKeeper.GetBalance(ctx, delegatorAddress, shareDenom) + expectedShareTokens := delegation.Shares.TruncateInt().Int64() - 1 // 1 token was lost during unbonding + require.Equal(t, expectedShareTokens, shareToken.Amount.Int64(), "share token amount") + + // Redeem the share tokens + _, err = msgServer.RedeemTokensForShares(sdk.WrapSDKContext(ctx), &types.MsgRedeemTokensForShares{ + DelegatorAddress: delegatorAddress.String(), + Amount: shareToken, + }) + require.NoError(t, err, "no error expected when redeeming") + + // Confirm (almost) the full delegation was recovered - minus the 2 tokens from the precision error + // (1 occurs during tokenization, and 1 occurs during redemption) + newDelegation, found := app.StakingKeeper.GetDelegation(ctx, delegatorAddress, validatorAddress) + require.True(t, found) + + endDelegationTokens := validator.TokensFromShares(newDelegation.Shares).TruncateInt().Int64() + expectedDelegationTokens := delegateAmount.Int64() - 2 + require.Equal(t, expectedDelegationTokens, endDelegationTokens, "final delegation tokens") +} + +// Tests the conversion from tokenization and redemption from the following scenario: +// Delegate -> Slash -> Tokenize -> Redeem +// Note, in this example, there 1 token lost during the decimal to int conversion +// during the unbonding step within tokenization +func TestTokenizeAndRedeemConversion_SlashBeforeTokenization(t *testing.T) { + _, app, ctx := createTestInput() + msgServer := keeper.NewMsgServerImpl(app.StakingKeeper) + + delegatorAddress, validatorAddress := setupTestTokenizeAndRedeemConversion(t, app, ctx) + + // Delegate and confirm the delegation record was created + delegateAmount := sdk.NewInt(1000) + delegateCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegateAmount) + _, err := msgServer.Delegate(sdk.WrapSDKContext(ctx), &types.MsgDelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAddress.String(), + Amount: delegateCoin, + }) + require.NoError(t, err, "no error expected when delegating") + + _, found := app.StakingKeeper.GetDelegation(ctx, delegatorAddress, validatorAddress) + require.True(t, found, "delegation should have been found") + + // slash the validator + simulateSlashWithImprecision(t, app, ctx, validatorAddress) + validator, found := app.StakingKeeper.GetValidator(ctx, validatorAddress) + require.True(t, found) + + // Tokenize the new amount after the slash + delegationAmountAfterSlash := validator.TokensFromShares(delegateAmount.ToDec()).TruncateInt() + tokenizationCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegationAmountAfterSlash) + + _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAddress.String(), + Amount: tokenizationCoin, + TokenizedShareOwner: delegatorAddress.String(), + }) + require.NoError(t, err, "no error expected when tokenizing") + + // The number of share tokens should line up with the **new** number of shares associated + // with the original delegated amount + // Note: 1 token is lost during unbonding due to rounding + shareDenom := validatorAddress.String() + "/1" + shareToken := app.BankKeeper.GetBalance(ctx, delegatorAddress, shareDenom) + expectedShareTokens, err := validator.SharesFromTokens(tokenizationCoin.Amount) + require.Equal(t, expectedShareTokens.TruncateInt().Int64()-1, shareToken.Amount.Int64(), "share token amount") + + // // Redeem the share tokens + _, err = msgServer.RedeemTokensForShares(sdk.WrapSDKContext(ctx), &types.MsgRedeemTokensForShares{ + DelegatorAddress: delegatorAddress.String(), + Amount: shareToken, + }) + require.NoError(t, err, "no error expected when redeeming") + + // Confirm the full tokenization amount was recovered - minus the 1 token from the precision error + newDelegation, found := app.StakingKeeper.GetDelegation(ctx, delegatorAddress, validatorAddress) + require.True(t, found) + + endDelegationTokens := validator.TokensFromShares(newDelegation.Shares).TruncateInt().Int64() + expectedDelegationTokens := delegationAmountAfterSlash.Int64() - 1 + require.Equal(t, expectedDelegationTokens, endDelegationTokens, "final delegation tokens") +} + +// Tests the conversion from tokenization and redemption from the following scenario: +// Delegate -> Tokenize -> Slash -> Redeem +// Note, in this example, there 1 token lost during the decimal to int conversion +// during the unbonding step within redemption +func TestTokenizeAndRedeemConversion_SlashBeforeRedemptino(t *testing.T) { + _, app, ctx := createTestInput() + msgServer := keeper.NewMsgServerImpl(app.StakingKeeper) + + delegatorAddress, validatorAddress := setupTestTokenizeAndRedeemConversion(t, app, ctx) + + // Delegate and confirm the delegation record was created + delegateAmount := sdk.NewInt(1000) + delegateCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegateAmount) + _, err := msgServer.Delegate(sdk.WrapSDKContext(ctx), &types.MsgDelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAddress.String(), + Amount: delegateCoin, + }) + require.NoError(t, err, "no error expected when delegating") + + _, found := app.StakingKeeper.GetDelegation(ctx, delegatorAddress, validatorAddress) + require.True(t, found, "delegation should have been found") + + // Tokenize the full delegation amount + _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAddress.String(), + Amount: delegateCoin, + TokenizedShareOwner: delegatorAddress.String(), + }) + require.NoError(t, err, "no error expected when tokenizing") + + // The number of share tokens should line up 1:1 with the number of issued shares + // Since the validator has not been slashed, the shares also line up 1;1 + // with the original delegation amount + shareDenom := validatorAddress.String() + "/1" + shareToken := app.BankKeeper.GetBalance(ctx, delegatorAddress, shareDenom) + expectedShareTokens := delegateAmount + require.Equal(t, expectedShareTokens.Int64(), shareToken.Amount.Int64(), "share token amount") + + // slash the validator + simulateSlashWithImprecision(t, app, ctx, validatorAddress) + validator, found := app.StakingKeeper.GetValidator(ctx, validatorAddress) + require.True(t, found) + + // Redeem the share tokens + _, err = msgServer.RedeemTokensForShares(sdk.WrapSDKContext(ctx), &types.MsgRedeemTokensForShares{ + DelegatorAddress: delegatorAddress.String(), + Amount: shareToken, + }) + require.NoError(t, err, "no error expected when redeeming") + + // Confirm the original delegation, minus the slash, was recovered + // There's an additional 1 token lost from precision error during unbonding + delegationAmountAfterSlash := validator.TokensFromShares(delegateAmount.ToDec()).TruncateInt().Int64() + newDelegation, found := app.StakingKeeper.GetDelegation(ctx, delegatorAddress, validatorAddress) + require.True(t, found) + + endDelegationTokens := validator.TokensFromShares(newDelegation.Shares).TruncateInt().Int64() + require.Equal(t, delegationAmountAfterSlash-1, endDelegationTokens, "final delegation tokens") +} + func TestTransferTokenizeShareRecord(t *testing.T) { _, app, ctx := createTestInput() From 0d5a1807aa4648a3749b64b1ad6c9d389085eb56 Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 14 Jul 2023 12:03:17 -0400 Subject: [PATCH 4/4] added lsm share token description to ADR --- docs/architecture/adr-061-liquid-staking.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/architecture/adr-061-liquid-staking.md b/docs/architecture/adr-061-liquid-staking.md index 5b99beff890b..41eb38a46e37 100644 --- a/docs/architecture/adr-061-liquid-staking.md +++ b/docs/architecture/adr-061-liquid-staking.md @@ -84,6 +84,10 @@ A user would be able to visit any liquid staking provider that has integrated wi Technically speaking, this is accomplished by using something called an “LSM share.” Using the liquid staking module, a user can tokenize their staked tokens and turn it into LSM shares. LSM shares can be redeemed for underlying staked tokens and are transferable. After staked tokens are tokenized they can be immediately transferred to a liquid staking provider in exchange for liquid staking tokens - without having to wait for the unbonding period. +## LSM share token +When tokenizing a delegation, the returned token has a denom of the format `{validatorAddress}/{recordId}`, where `recordId` is a monotonically increasing number that increments every tokenization. As a result, two successive tokenizations to the same validator will yield different denom's. +Additionally, the share tokens returned will map 1:1 with the number of shares of the underlying delegation (e.g. if the delegation of X shares is tokenized, X share tokens be returned). This reduces ambiguity with respect to the value of the token if a slash occurs after tokenization. + ## Toggling the ability to tokenize shares Currently LSM facilitates the immediate conversion of staked assets into liquid staked tokens (referred to as "tokenization"). Despite the many benefits that come with this capability, it does inadvertently negate a protective measure available via traditional staking, where a user can stake their tokens to render them illiquid in the event that their wallet is compromised (the attacker would first need to unbond, then transfer out the tokens).