From 4d79e01f3ab90e541986ae6b49281509f06f168d Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 27 Feb 2024 18:33:11 +0100 Subject: [PATCH 1/9] save LSM vesting test draft --- 1 | 0 go.mod | 2 +- runtime/app.go | 6 +- .../staking/keeper/msg_server_test.go | 60 +++++++++++++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 1 diff --git a/1 b/1 new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/go.mod b/go.mod index 81e8b4124607..5052df8b6ed3 100644 --- a/go.mod +++ b/go.mod @@ -64,9 +64,9 @@ require ( google.golang.org/genproto/googleapis/api v0.0.0-20231212172506-995d672761c0 google.golang.org/grpc v1.60.1 google.golang.org/protobuf v1.32.0 + gopkg.in/yaml.v2 v2.4.0 gotest.tools/v3 v3.5.1 pgregory.net/rapid v1.1.0 - gopkg.in/yaml.v2 v2.4.0 sigs.k8s.io/yaml v1.3.0 ) diff --git a/runtime/app.go b/runtime/app.go index fa733fc4ce53..767a5f7b4c77 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -121,9 +121,9 @@ func (a *App) Load(loadLatest bool) error { a.SetEndBlocker(a.EndBlocker) } - if len(a.config.OrderMigrations) != 0 { - a.ModuleManager.SetOrderMigrations(a.config.OrderMigrations...) - } + // if len(a.config.OrderMigrations) != 0 { + // a.ModuleManager.SetOrderMigrations(a.config.OrderMigrations...) + // } if loadLatest { if err := a.LoadLatestVersion(); err != nil { diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index 535ecfcc9190..3c2887a28305 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -1583,6 +1583,66 @@ func TestICADelegateUndelegate(t *testing.T) { require.Equal(t, sdk.ZeroDec(), validator.LiquidShares, "validator liquid shares after undelegation") } +func TestTokenizeVestingDelegation(t *testing.T) { + _, app, ctx := createTestInput(t) + var ( + bankKeeper = app.BankKeeper + accountKeeper = app.AccountKeeper + stakingKeeper = app.StakingKeeper + ) + + addrs := simtestutil.AddTestAddrs(bankKeeper, stakingKeeper, ctx, 1, stakingKeeper.TokensFromConsensusPower(ctx, 10000)) + addrAcc1 := addrs[0] + addrVal1 := sdk.ValAddress(addrAcc1) + + vestingAmount := math.NewInt(100_000) + originalVesting := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, vestingAmount)) + startTime := ctx.BlockTime().Unix() + endTime := startTime + 86400 + + // create vesting account + pubkey := secp256k1.GenPrivKey().PubKey() + baseAcc := authtypes.NewBaseAccount(addrAcc1, pubkey, 0, 0) + delayedVestingAccount := vestingtypes.NewContinuousVestingAccount( + baseAcc, + originalVesting, + startTime, + endTime, + ) + accountKeeper.SetAccount(ctx, delayedVestingAccount) + + pubKeys := simtestutil.CreateTestPubKeys(1) + pk1 := pubKeys[0] + + // Create Validators and Delegation + val1 := testutil.NewValidator(t, addrVal1, pk1) + val1.Status = stakingtypes.Bonded + app.StakingKeeper.SetValidator(ctx, val1) + app.StakingKeeper.SetValidatorByPowerIndex(ctx, val1) + err := app.StakingKeeper.SetValidatorByConsAddr(ctx, val1) + require.NoError(t, err) + + delAmount := math.NewInt(100_0000) + // Delegate from both the main delegator as well as a random account so there is a + // non-zero delegation after redemption + err = delegateCoinsFromAccount(ctx, *stakingKeeper, addrAcc1, delAmount, val1) + require.NoError(t, err) + + // apply TM updates + applyValidatorSetUpdates(t, ctx, stakingKeeper, -1) + + // // Create ICA module account + // icaAccountAddress := createICAAccount(ctx, accountKeeper) + + // // Fund module account + // delegationCoin := sdk.NewCoin(stakingKeeper.BondDenom(ctx), tc.delegationAmount) + // err := bankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(delegationCoin)) + // require.NoError(t, err) + // err = bankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, icaAccountAddress, sdk.NewCoins(delegationCoin)) + // require.NoError(t, err) + +} + // Helper function to create 32-length account // Used to mock an liquid staking provider's ICA account func createICAAccount(ctx sdk.Context, ak accountkeeper.AccountKeeper) sdk.AccAddress { From ac71ede9e74d4bbb9891740f27dcae0d17e72610 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Wed, 28 Feb 2024 19:17:32 +0100 Subject: [PATCH 2/9] finish integration test --- .../staking/keeper/msg_server_test.go | 84 ++++++++++++++----- 1 file changed, 65 insertions(+), 19 deletions(-) diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index 3c2887a28305..b37264270156 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -1583,7 +1583,7 @@ func TestICADelegateUndelegate(t *testing.T) { require.Equal(t, sdk.ZeroDec(), validator.LiquidShares, "validator liquid shares after undelegation") } -func TestTokenizeVestingDelegation(t *testing.T) { +func TestTokenizeVestedDelegation(t *testing.T) { _, app, ctx := createTestInput(t) var ( bankKeeper = app.BankKeeper @@ -1595,21 +1595,20 @@ func TestTokenizeVestingDelegation(t *testing.T) { addrAcc1 := addrs[0] addrVal1 := sdk.ValAddress(addrAcc1) - vestingAmount := math.NewInt(100_000) - originalVesting := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, vestingAmount)) - startTime := ctx.BlockTime().Unix() - endTime := startTime + 86400 + originalVesting := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000))) + startTime := time.Now().Unix() + endTime := time.Now().Add(24 * time.Hour).Unix() // create vesting account pubkey := secp256k1.GenPrivKey().PubKey() baseAcc := authtypes.NewBaseAccount(addrAcc1, pubkey, 0, 0) - delayedVestingAccount := vestingtypes.NewContinuousVestingAccount( + continuousVestingAccount := vestingtypes.NewContinuousVestingAccount( baseAcc, originalVesting, startTime, endTime, ) - accountKeeper.SetAccount(ctx, delayedVestingAccount) + accountKeeper.SetAccount(ctx, continuousVestingAccount) pubKeys := simtestutil.CreateTestPubKeys(1) pk1 := pubKeys[0] @@ -1622,25 +1621,72 @@ func TestTokenizeVestingDelegation(t *testing.T) { err := app.StakingKeeper.SetValidatorByConsAddr(ctx, val1) require.NoError(t, err) - delAmount := math.NewInt(100_0000) - // Delegate from both the main delegator as well as a random account so there is a - // non-zero delegation after redemption - err = delegateCoinsFromAccount(ctx, *stakingKeeper, addrAcc1, delAmount, val1) + // Delegate all the vesting coins + originalVestingAmount := originalVesting.AmountOf(sdk.DefaultBondDenom) + err = delegateCoinsFromAccount(ctx, *stakingKeeper, addrAcc1, originalVestingAmount, val1) require.NoError(t, err) // apply TM updates applyValidatorSetUpdates(t, ctx, stakingKeeper, -1) - // // Create ICA module account - // icaAccountAddress := createICAAccount(ctx, accountKeeper) + _, found := stakingKeeper.GetDelegation(ctx, addrAcc1, addrVal1) + require.True(t, found) + + // check vesting account data + acc := accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) + require.Equal(t, originalVesting, acc.GetVestingCoins(ctx.BlockTime())) + require.Empty(t, acc.GetVestedCoins(ctx.BlockTime())) + require.Equal(t, originalVesting, acc.GetDelegatedVesting()) + require.Empty(t, acc.GetDelegatedFree()) + + msgServer := keeper.NewMsgServerImpl(stakingKeeper) + + // vest tokens during half the vesting period + ctx = ctx.WithBlockTime(time.Now().Add(12 * time.Hour)) - // // Fund module account - // delegationCoin := sdk.NewCoin(stakingKeeper.BondDenom(ctx), tc.delegationAmount) - // err := bankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(delegationCoin)) - // require.NoError(t, err) - // err = bankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, icaAccountAddress, sdk.NewCoins(delegationCoin)) - // require.NoError(t, err) + // check vesting account data + acc = accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) + require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestingCoins(ctx.BlockTime())) + require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestedCoins(ctx.BlockTime())) + require.Equal(t, originalVesting, acc.GetDelegatedVesting()) + require.Empty(t, acc.GetDelegatedFree()) + + // expect to fail when tokenizing the original vesting amount + _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ + DelegatorAddress: addrAcc1.String(), + ValidatorAddress: addrVal1.String(), + Amount: originalVesting[0], + TokenizedShareOwner: addrAcc1.String(), + }) + + require.Error(t, err) + + // tokenize the vested coins should succeed + _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ + DelegatorAddress: addrAcc1.String(), + ValidatorAddress: addrVal1.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: originalVestingAmount.Quo(math.NewInt(2))}, + TokenizedShareOwner: addrAcc1.String(), + }) + require.NoError(t, err) + + shareDenom := addrVal1.String() + "/1" + + // redeem the tokens + _, err = msgServer.RedeemTokensForShares(sdk.WrapSDKContext(ctx), + &types.MsgRedeemTokensForShares{ + DelegatorAddress: addrAcc1.String(), + Amount: sdk.Coin{Denom: shareDenom, Amount: originalVestingAmount.Quo(math.NewInt(2))}, + }, + ) + require.NoError(t, err) + // check that vesting account delegations correspond to the vesting and vested amount + acc = accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) + require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestingCoins(ctx.BlockTime())) + require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestedCoins(ctx.BlockTime())) + require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetDelegatedVesting()) + require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetDelegatedFree()) } // Helper function to create 32-length account From d21d8f905e58d9d1c5d2147c15fa3da6306568a7 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Fri, 1 Mar 2024 10:42:15 +0100 Subject: [PATCH 3/9] add UT --- .../staking/keeper/msg_server_test.go | 26 +++-- x/staking/keeper/liquid_stake.go | 30 ++++++ x/staking/keeper/liquid_stake_test.go | 102 ++++++++++++++++++ x/staking/keeper/msg_server.go | 4 +- 4 files changed, 149 insertions(+), 13 deletions(-) diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index b37264270156..57bcb4da56ac 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -1583,7 +1583,7 @@ func TestICADelegateUndelegate(t *testing.T) { require.Equal(t, sdk.ZeroDec(), validator.LiquidShares, "validator liquid shares after undelegation") } -func TestTokenizeVestedDelegation(t *testing.T) { +func TestTokenizeAndRedeemVestedDelegation(t *testing.T) { _, app, ctx := createTestInput(t) var ( bankKeeper = app.BankKeeper @@ -1595,11 +1595,12 @@ func TestTokenizeVestedDelegation(t *testing.T) { addrAcc1 := addrs[0] addrVal1 := sdk.ValAddress(addrAcc1) + // Original vesting mount (OV) originalVesting := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000))) startTime := time.Now().Unix() endTime := time.Now().Add(24 * time.Hour).Unix() - // create vesting account + // Create vesting account pubkey := secp256k1.GenPrivKey().PubKey() baseAcc := authtypes.NewBaseAccount(addrAcc1, pubkey, 0, 0) continuousVestingAccount := vestingtypes.NewContinuousVestingAccount( @@ -1626,13 +1627,14 @@ func TestTokenizeVestedDelegation(t *testing.T) { err = delegateCoinsFromAccount(ctx, *stakingKeeper, addrAcc1, originalVestingAmount, val1) require.NoError(t, err) - // apply TM updates + // Apply TM updates applyValidatorSetUpdates(t, ctx, stakingKeeper, -1) _, found := stakingKeeper.GetDelegation(ctx, addrAcc1, addrVal1) require.True(t, found) - // check vesting account data + // Check vesting account data + // V=100, V'=0, DV=100, DF=0 acc := accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) require.Equal(t, originalVesting, acc.GetVestingCoins(ctx.BlockTime())) require.Empty(t, acc.GetVestedCoins(ctx.BlockTime())) @@ -1641,27 +1643,28 @@ func TestTokenizeVestedDelegation(t *testing.T) { msgServer := keeper.NewMsgServerImpl(stakingKeeper) - // vest tokens during half the vesting period + // Vest coins during a half of the vesting period ctx = ctx.WithBlockTime(time.Now().Add(12 * time.Hour)) - // check vesting account data + // Check vesting account data + // V=50, V'=50, DV=100, DF=0 acc = accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestingCoins(ctx.BlockTime())) require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestedCoins(ctx.BlockTime())) require.Equal(t, originalVesting, acc.GetDelegatedVesting()) require.Empty(t, acc.GetDelegatedFree()) - // expect to fail when tokenizing the original vesting amount + // Expect to fail when tokenizing the all the delegated coins + // since only a half is vested _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ DelegatorAddress: addrAcc1.String(), ValidatorAddress: addrVal1.String(), Amount: originalVesting[0], TokenizedShareOwner: addrAcc1.String(), }) - require.Error(t, err) - // tokenize the vested coins should succeed + // Tokenize the delegated vested coins _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ DelegatorAddress: addrAcc1.String(), ValidatorAddress: addrVal1.String(), @@ -1672,7 +1675,7 @@ func TestTokenizeVestedDelegation(t *testing.T) { shareDenom := addrVal1.String() + "/1" - // redeem the tokens + // Redeem the tokens _, err = msgServer.RedeemTokensForShares(sdk.WrapSDKContext(ctx), &types.MsgRedeemTokensForShares{ DelegatorAddress: addrAcc1.String(), @@ -1681,7 +1684,8 @@ func TestTokenizeVestedDelegation(t *testing.T) { ) require.NoError(t, err) - // check that vesting account delegations correspond to the vesting and vested amount + // After the redeeming the tokens the vesting delegations should be balanced + // V=50, V'=50, DV=100, DF=50 acc = accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestingCoins(ctx.BlockTime())) require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestedCoins(ctx.BlockTime())) diff --git a/x/staking/keeper/liquid_stake.go b/x/staking/keeper/liquid_stake.go index 2a52d373f0e9..f7e55d0206ce 100644 --- a/x/staking/keeper/liquid_stake.go +++ b/x/staking/keeper/liquid_stake.go @@ -5,6 +5,7 @@ import ( "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" + vesting "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/cosmos/cosmos-sdk/x/staking/types" ) @@ -441,3 +442,32 @@ func (k Keeper) RefreshTotalLiquidStaked(ctx sdk.Context) error { return nil } + +// CheckVestedDelegationInVestingAccount verifies whether the provided vesting account +// holds a vested delegation for an equal or greater amount of the specified coin +// at the given block time. +// +// Note that this function enables a specific use-case in the LSM module for tokenizing vested delegations. +// See https://github.com/cosmos/gaia/issues/2877 for more details. +func CheckVestedDelegationInVestingAccount( + account vesting.VestingAccount, + blockTime time.Time, + coin sdk.Coin, +) bool { + // Get the vesting coins at the current block time + vestingAmount := account.GetVestingCoins(blockTime).AmountOf(coin.Denom) + + // Note that the "DelegatedVesting" and "DelegatedFree" values + // were computed during the last delegation or undelegation operation + delVestingAmount := account.GetDelegatedVesting().AmountOf(coin.Denom) + delVested := account.GetDelegatedFree() + + // x represents the new vested delegated coins + x := sdk.MinInt(vestingAmount.Sub(delVestingAmount), math.ZeroInt()) + + if !x.IsZero() { + delVested = delVested.Add(sdk.Coin{Denom: coin.Denom, Amount: x.Abs()}) + } + + return delVested.AmountOf(coin.Denom).GTE(coin.Amount) +} diff --git a/x/staking/keeper/liquid_stake_test.go b/x/staking/keeper/liquid_stake_test.go index a4870c8419db..34d0d8bfa397 100644 --- a/x/staking/keeper/liquid_stake_test.go +++ b/x/staking/keeper/liquid_stake_test.go @@ -1,14 +1,19 @@ package keeper_test import ( + "testing" "time" + "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" + "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/stretchr/testify/require" ) // Tests Set/Get TotalLiquidStakedTokens @@ -808,3 +813,100 @@ func (s *KeeperTestSuite) TestDelegatorIsLiquidStaker() { require.False(keeper.DelegatorIsLiquidStaker(baseAccountAddress), "base account") require.True(keeper.DelegatorIsLiquidStaker(icaAccountAddress), "ICA module account") } + +func TestCheckVestedDelegationInVestingAccount(t *testing.T) { + + var ( + vestingAcct *vestingtypes.ContinuousVestingAccount + startTime = time.Now() + endTime = startTime.Add(24 * time.Hour) + originalVesting = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000))) + ) + + testCases := []struct { + name string + setupAcct func() + blockTime time.Time + delegation sdk.Coin + expRes bool + }{ + { + name: "vesting account has zero delegations", + setupAcct: func() {}, + blockTime: endTime, + delegation: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), + expRes: false, + }, + { + name: "vested delegations exist but for a different coin", + setupAcct: func() { + vestingAcct.DelegatedFree = sdk.NewCoins(sdk.NewCoin("uatom", math.NewInt(100_000))) + }, + blockTime: endTime, + delegation: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), + expRes: false, + }, + { + name: "all delegations are vesting", + setupAcct: func() { + vestingAcct.DelegatedVesting = vestingAcct.OriginalVesting + }, + blockTime: startTime, + delegation: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), + expRes: false, + }, + { + name: "not enough vested coins", + setupAcct: func() { + vestingAcct.DelegatedFree = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(80_000))) + }, + blockTime: endTime, + delegation: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000)), + expRes: false, + }, + { + name: "account is vested and have vested delegations", + setupAcct: func() { + vestingAcct.DelegatedFree = vestingAcct.OriginalVesting + }, + blockTime: endTime, + delegation: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000)), + expRes: true, + }, + { + name: "vesting account partially vested and have vesting and vested delegations", + setupAcct: func() { + // In this use-case 25k in "DelegatedVesting" are vested + vestingAcct.DelegatedFree = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(50_000))) + vestingAcct.DelegatedVesting = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(50_000))) + }, + blockTime: startTime.Add(18 * time.Hour), // vest 3/4 vesting period + delegation: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(75_000)), + + expRes: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + baseAcc := authtypes.NewBaseAccount(sdk.AccAddress([]byte("addr")), secp256k1.GenPrivKey().PubKey(), 0, 0) + vestingAcct = vestingtypes.NewContinuousVestingAccount( + baseAcc, + originalVesting, + startTime.Unix(), + endTime.Unix(), + ) + + tc.setupAcct() + + require.Equal( + t, + tc.expRes, keeper.CheckVestedDelegationInVestingAccount( + vestingAcct, + tc.blockTime, + tc.delegation, + ), + ) + }) + } +} diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 0eac2ace6123..a88af57ec750 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -702,8 +702,8 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS // the tokenize share amount and execute further tokenize share process // tokenize share is reducing unlocked tokens delegation from the vesting account and further process // is not causing issues - delFree := acc.GetDelegatedFree().AmountOf(msg.Amount.Denom) - if delFree.LT(msg.Amount.Amount) { + + if !CheckVestedDelegationInVestingAccount(acc, ctx.BlockTime(), msg.Amount) { return nil, types.ErrExceedingFreeVestingDelegations } } From 9eeb34f98f6c32a3b5483807bd18dc5f56801ee5 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Fri, 1 Mar 2024 11:14:17 +0100 Subject: [PATCH 4/9] nits --- runtime/app.go | 6 +-- .../staking/keeper/msg_server_test.go | 33 ++++++------ x/staking/keeper/liquid_stake.go | 8 +-- x/staking/keeper/liquid_stake_test.go | 53 +++++++++---------- 4 files changed, 52 insertions(+), 48 deletions(-) diff --git a/runtime/app.go b/runtime/app.go index 767a5f7b4c77..fa733fc4ce53 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -121,9 +121,9 @@ func (a *App) Load(loadLatest bool) error { a.SetEndBlocker(a.EndBlocker) } - // if len(a.config.OrderMigrations) != 0 { - // a.ModuleManager.SetOrderMigrations(a.config.OrderMigrations...) - // } + if len(a.config.OrderMigrations) != 0 { + a.ModuleManager.SetOrderMigrations(a.config.OrderMigrations...) + } if loadLatest { if err := a.LoadLatestVersion(); err != nil { diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index 57bcb4da56ac..97afe815ce71 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -1597,8 +1597,8 @@ func TestTokenizeAndRedeemVestedDelegation(t *testing.T) { // Original vesting mount (OV) originalVesting := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000))) - startTime := time.Now().Unix() - endTime := time.Now().Add(24 * time.Hour).Unix() + startTime := time.Now() + endTime := time.Now().Add(24 * time.Hour) // Create vesting account pubkey := secp256k1.GenPrivKey().PubKey() @@ -1606,8 +1606,8 @@ func TestTokenizeAndRedeemVestedDelegation(t *testing.T) { continuousVestingAccount := vestingtypes.NewContinuousVestingAccount( baseAcc, originalVesting, - startTime, - endTime, + startTime.Unix(), + endTime.Unix(), ) accountKeeper.SetAccount(ctx, continuousVestingAccount) @@ -1643,19 +1643,22 @@ func TestTokenizeAndRedeemVestedDelegation(t *testing.T) { msgServer := keeper.NewMsgServerImpl(stakingKeeper) - // Vest coins during a half of the vesting period - ctx = ctx.WithBlockTime(time.Now().Add(12 * time.Hour)) + // Vest half the original vesting coins + ctx = ctx.WithBlockTime(startTime.Add(time.Duration(endTime.Sub(startTime).Hours() / float64(2)))) + + // expect that half of the orignal vesting coins are vested + expVestedCoins := originalVesting.QuoInt(math.NewInt(2)) // Check vesting account data // V=50, V'=50, DV=100, DF=0 acc = accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) - require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestingCoins(ctx.BlockTime())) - require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestedCoins(ctx.BlockTime())) + require.Equal(t, expVestedCoins, acc.GetVestingCoins(ctx.BlockTime())) + require.Equal(t, expVestedCoins, acc.GetVestedCoins(ctx.BlockTime())) require.Equal(t, originalVesting, acc.GetDelegatedVesting()) require.Empty(t, acc.GetDelegatedFree()) - // Expect to fail when tokenizing the all the delegated coins - // since only a half is vested + // Expect that tokenizing all the delegated coins fails + // since only the half are vested _, err = msgServer.TokenizeShares(sdk.WrapSDKContext(ctx), &types.MsgTokenizeShares{ DelegatorAddress: addrAcc1.String(), ValidatorAddress: addrVal1.String(), @@ -1684,13 +1687,13 @@ func TestTokenizeAndRedeemVestedDelegation(t *testing.T) { ) require.NoError(t, err) - // After the redeeming the tokens the vesting delegations should be balanced + // After the redemption of the tokens, the vesting delegations should be evenly distributed // V=50, V'=50, DV=100, DF=50 acc = accountKeeper.GetAccount(ctx, addrAcc1).(*vestingtypes.ContinuousVestingAccount) - require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestingCoins(ctx.BlockTime())) - require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetVestedCoins(ctx.BlockTime())) - require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetDelegatedVesting()) - require.Equal(t, originalVesting.QuoInt(math.NewInt(2)), acc.GetDelegatedFree()) + require.Equal(t, expVestedCoins, acc.GetVestingCoins(ctx.BlockTime())) + require.Equal(t, expVestedCoins, acc.GetVestedCoins(ctx.BlockTime())) + require.Equal(t, expVestedCoins, acc.GetDelegatedVesting()) + require.Equal(t, expVestedCoins, acc.GetDelegatedFree()) } // Helper function to create 32-length account diff --git a/x/staking/keeper/liquid_stake.go b/x/staking/keeper/liquid_stake.go index f7e55d0206ce..b7f04d443586 100644 --- a/x/staking/keeper/liquid_stake.go +++ b/x/staking/keeper/liquid_stake.go @@ -447,8 +447,8 @@ func (k Keeper) RefreshTotalLiquidStaked(ctx sdk.Context) error { // holds a vested delegation for an equal or greater amount of the specified coin // at the given block time. // -// Note that this function enables a specific use-case in the LSM module for tokenizing vested delegations. -// See https://github.com/cosmos/gaia/issues/2877 for more details. +// Note that this function facilitates a specific use-case in the LSM module for tokenizing vested delegations. +// For more details, see https://github.com/cosmos/gaia/issues/2877. func CheckVestedDelegationInVestingAccount( account vesting.VestingAccount, blockTime time.Time, @@ -462,12 +462,14 @@ func CheckVestedDelegationInVestingAccount( delVestingAmount := account.GetDelegatedVesting().AmountOf(coin.Denom) delVested := account.GetDelegatedFree() - // x represents the new vested delegated coins + // Calculate the new vested delegated coins x := sdk.MinInt(vestingAmount.Sub(delVestingAmount), math.ZeroInt()) + // Add the newly vested delegated coins to the existing delegated vested amount if !x.IsZero() { delVested = delVested.Add(sdk.Coin{Denom: coin.Denom, Amount: x.Abs()}) } + // Check if the total delegated vested amount is greater than or equal to the specified coin amount return delVested.AmountOf(coin.Denom).GTE(coin.Amount) } diff --git a/x/staking/keeper/liquid_stake_test.go b/x/staking/keeper/liquid_stake_test.go index 34d0d8bfa397..f31bebc0fe15 100644 --- a/x/staking/keeper/liquid_stake_test.go +++ b/x/staking/keeper/liquid_stake_test.go @@ -824,64 +824,63 @@ func TestCheckVestedDelegationInVestingAccount(t *testing.T) { ) testCases := []struct { - name string - setupAcct func() - blockTime time.Time - delegation sdk.Coin - expRes bool + name string + setupAcct func() + blockTime time.Time + coinRequired sdk.Coin + expRes bool }{ { - name: "vesting account has zero delegations", - setupAcct: func() {}, - blockTime: endTime, - delegation: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), - expRes: false, + name: "vesting account has zero delegations", + setupAcct: func() {}, + blockTime: endTime, + coinRequired: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), + expRes: false, }, { name: "vested delegations exist but for a different coin", setupAcct: func() { vestingAcct.DelegatedFree = sdk.NewCoins(sdk.NewCoin("uatom", math.NewInt(100_000))) }, - blockTime: endTime, - delegation: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), - expRes: false, + blockTime: endTime, + coinRequired: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), + expRes: false, }, { name: "all delegations are vesting", setupAcct: func() { vestingAcct.DelegatedVesting = vestingAcct.OriginalVesting }, - blockTime: startTime, - delegation: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), - expRes: false, + blockTime: startTime, + coinRequired: sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt()), + expRes: false, }, { - name: "not enough vested coins", + name: "not enough vested coin", setupAcct: func() { vestingAcct.DelegatedFree = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(80_000))) }, - blockTime: endTime, - delegation: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000)), - expRes: false, + blockTime: endTime, + coinRequired: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000)), + expRes: false, }, { name: "account is vested and have vested delegations", setupAcct: func() { vestingAcct.DelegatedFree = vestingAcct.OriginalVesting }, - blockTime: endTime, - delegation: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000)), - expRes: true, + blockTime: endTime, + coinRequired: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100_000)), + expRes: true, }, { name: "vesting account partially vested and have vesting and vested delegations", setupAcct: func() { - // In this use-case 25k in "DelegatedVesting" are vested vestingAcct.DelegatedFree = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(50_000))) vestingAcct.DelegatedVesting = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(50_000))) }, - blockTime: startTime.Add(18 * time.Hour), // vest 3/4 vesting period - delegation: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(75_000)), + blockTime: startTime.Add(18 * time.Hour), // vest 3/4 vesting period + coinRequired: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(75_000)), expRes: true, }, @@ -904,7 +903,7 @@ func TestCheckVestedDelegationInVestingAccount(t *testing.T) { tc.expRes, keeper.CheckVestedDelegationInVestingAccount( vestingAcct, tc.blockTime, - tc.delegation, + tc.coinRequired, ), ) }) From f72d359920e767041c8847cb8d733ace1f61d63f Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 4 Mar 2024 08:46:12 +0100 Subject: [PATCH 5/9] Update x/staking/keeper/liquid_stake.go Co-authored-by: Aleksandr Bezobchuk --- x/staking/keeper/liquid_stake.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/x/staking/keeper/liquid_stake.go b/x/staking/keeper/liquid_stake.go index b7f04d443586..95bd4e234fe5 100644 --- a/x/staking/keeper/liquid_stake.go +++ b/x/staking/keeper/liquid_stake.go @@ -449,11 +449,7 @@ func (k Keeper) RefreshTotalLiquidStaked(ctx sdk.Context) error { // // Note that this function facilitates a specific use-case in the LSM module for tokenizing vested delegations. // For more details, see https://github.com/cosmos/gaia/issues/2877. -func CheckVestedDelegationInVestingAccount( - account vesting.VestingAccount, - blockTime time.Time, - coin sdk.Coin, -) bool { +func CheckVestedDelegationInVestingAccount(account vesting.VestingAccount, blockTime time.Time, coin sdk.Coin) bool { // Get the vesting coins at the current block time vestingAmount := account.GetVestingCoins(blockTime).AmountOf(coin.Denom) From cd5b806132b1a6271c43c4d92efed0ac6af4d454 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 4 Mar 2024 09:09:59 +0100 Subject: [PATCH 6/9] nits --- 1 | 0 x/staking/keeper/liquid_stake.go | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 1 diff --git a/1 b/1 deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/x/staking/keeper/liquid_stake.go b/x/staking/keeper/liquid_stake.go index 95bd4e234fe5..2d49761630bd 100644 --- a/x/staking/keeper/liquid_stake.go +++ b/x/staking/keeper/liquid_stake.go @@ -463,7 +463,7 @@ func CheckVestedDelegationInVestingAccount(account vesting.VestingAccount, block // Add the newly vested delegated coins to the existing delegated vested amount if !x.IsZero() { - delVested = delVested.Add(sdk.Coin{Denom: coin.Denom, Amount: x.Abs()}) + delVested = delVested.Add(sdk.NewCoin(coin.Denom, x.Abs())) } // Check if the total delegated vested amount is greater than or equal to the specified coin amount From 987a86dae5f363a744059a4de2e6a9c8dc1e08e0 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 4 Mar 2024 13:01:49 +0100 Subject: [PATCH 7/9] clean go.mod --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index a26603aa8a83..eda543583cd0 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,6 @@ require ( gopkg.in/yaml.v2 v2.4.0 gotest.tools/v3 v3.5.1 pgregory.net/rapid v1.1.0 - gopkg.in/yaml.v2 v2.4.0 sigs.k8s.io/yaml v1.4.0 ) From 7cfaabca7e9510b619275a235aa3bdbb881ed40e Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 4 Mar 2024 13:32:18 +0100 Subject: [PATCH 8/9] address formatting error --- tests/integration/staking/keeper/msg_server_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index 97afe815ce71..4c7336c2f52c 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -1644,7 +1644,8 @@ func TestTokenizeAndRedeemVestedDelegation(t *testing.T) { msgServer := keeper.NewMsgServerImpl(stakingKeeper) // Vest half the original vesting coins - ctx = ctx.WithBlockTime(startTime.Add(time.Duration(endTime.Sub(startTime).Hours() / float64(2)))) + vestHalfTime := startTime.Add(time.Duration(float64(endTime.Sub(startTime).Nanoseconds()) / float64(2))) + ctx = ctx.WithBlockTime(vestHalfTime) // expect that half of the orignal vesting coins are vested expVestedCoins := originalVesting.QuoInt(math.NewInt(2)) From 5b42259ed28f102aa6c94456e4df7c20efced871 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Fri, 8 Mar 2024 09:16:08 +0100 Subject: [PATCH 9/9] update Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e25178196652..477f79cebec5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -376,6 +376,7 @@ during the [Oak Security audit of SDK 0.47](https://github.com/oak-security/audi ### State Machine Breaking +* (x/staking) [#19614](https://github.com/cosmos/cosmos-sdk/pull/19614) Facilitate the tokenization of vested delegation in the LSM module. * (baseapp, x/auth/posthandler) [#13940](https://github.com/cosmos/cosmos-sdk/pull/13940) Update `PostHandler` to receive the `runTx` success boolean. * (store) [#14378](https://github.com/cosmos/cosmos-sdk/pull/14378) The `CacheKV` store is thread-safe again, which includes improved iteration and deletion logic. Iteration is on a strictly isolated view now, which is breaking from previous behavior. * (x/bank) [#14538](https://github.com/cosmos/cosmos-sdk/pull/14538) Validate denom in bank balances GRPC queries.