From eb2c43c517f66ef5c1ced0ff85ed0e0fbcbf1cdf Mon Sep 17 00:00:00 2001 From: javiersuweijie Date: Mon, 25 Mar 2024 11:41:22 +0800 Subject: [PATCH] fix: claim accured rewards first before adding delegation --- x/alliance/keeper/delegation.go | 6 ++ x/alliance/keeper/tests/delegation_test.go | 38 ++++++++- x/alliance/keeper/tests/reward_test.go | 91 ++++++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) diff --git a/x/alliance/keeper/delegation.go b/x/alliance/keeper/delegation.go index a563ace7..09d9ba06 100644 --- a/x/alliance/keeper/delegation.go +++ b/x/alliance/keeper/delegation.go @@ -38,6 +38,12 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, validator type if err != nil { return nil, err } + } else { + // Claim validator rewards first to ensure that we distribute accrued rewards to delegators before the new delegation + _, err = k.ClaimValidatorRewards(ctx, validator) + if err != nil { + return nil, err + } } // Create or update a delegation diff --git a/x/alliance/keeper/tests/delegation_test.go b/x/alliance/keeper/tests/delegation_test.go index 6bfa38e6..2a2271c7 100644 --- a/x/alliance/keeper/tests/delegation_test.go +++ b/x/alliance/keeper/tests/delegation_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "cosmossdk.io/math" test_helpers "github.com/terra-money/alliance/app" "github.com/terra-money/alliance/x/alliance" @@ -599,6 +601,7 @@ func TestSuccessfulUndelegation(t *testing.T) { }, }) delegations := app.StakingKeeper.GetAllDelegations(ctx) + queryServer := keeper.NewQueryServerImpl(app.AllianceKeeper) require.Len(t, delegations, 1) unbondingTime := app.StakingKeeper.UnbondingTime(ctx) @@ -616,6 +619,12 @@ func TestSuccessfulUndelegation(t *testing.T) { err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, delAddr, sdk.NewCoins(sdk.NewCoin(AllianceDenom, sdk.NewInt(2000_000)))) require.NoError(t, err) + // Mint reward tokens + err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin("reward", math.NewInt(2000_000)))) + require.NoError(t, err) + err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, authtypes.FeeCollectorName, sdk.NewCoins(sdk.NewCoin("reward", math.NewInt(2000_000)))) + require.NoError(t, err) + // Delegate to a validator _, err = app.AllianceKeeper.Delegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, sdk.NewInt(1000_000))) require.NoError(t, err) @@ -639,6 +648,19 @@ func TestSuccessfulUndelegation(t *testing.T) { Shares: sdk.NewDec(2), }, d) + err = app.AllianceKeeper.AddAssetsToRewardPool(ctx, app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName), val, sdk.NewCoins(sdk.NewCoin("reward", math.NewInt(1000_000)))) + require.NoError(t, err) + + rewards, err := queryServer.AllianceDelegationRewards(ctx, &types.QueryAllianceDelegationRewardsRequest{ + DelegatorAddr: delAddr.String(), + ValidatorAddr: valAddr.String(), + Denom: AllianceDenom, + Pagination: nil, + }) + require.NoError(t, err) + require.Equal(t, 1, len(rewards.Rewards)) + rewardAmount := rewards.Rewards[0].Amount + // Immediately undelegate from the validator _, err = app.AllianceKeeper.Undelegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, sdk.NewInt(250_000))) require.NoError(t, err) @@ -646,8 +668,11 @@ func TestSuccessfulUndelegation(t *testing.T) { _, err = app.AllianceKeeper.Undelegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, sdk.NewInt(250_000))) require.NoError(t, err) + // Check that rewards sent out + rewardBalance := app.BankKeeper.GetBalance(ctx, delAddr, "reward") + require.Equal(t, rewardAmount, rewardBalance.Amount) + // Query unbondings directly from the entry point - queryServer := keeper.NewQueryServerImpl(app.AllianceKeeper) res, err := queryServer.AllianceUnbondingsByDelegator(ctx, &types.QueryAllianceUnbondingsByDelegatorRequest{ DelegatorAddr: delAddr.String(), }) @@ -724,6 +749,17 @@ func TestSuccessfulUndelegation(t *testing.T) { // Completing again should not process anymore undelegations err = app.AllianceKeeper.CompleteUnbondings(ctx) require.NoError(t, err) + + q := keeper.NewQueryServerImpl(app.AllianceKeeper) + rewardRes, err := q.AllianceDelegationRewards(ctx, &types.QueryAllianceDelegationRewardsRequest{ + DelegatorAddr: delAddr.String(), + ValidatorAddr: valAddr.String(), + Denom: AllianceDenom, + Pagination: nil, + }) + + require.NoError(t, err) + require.Equal(t, 0, len(rewardRes.Rewards)) } func TestUndelegationWithoutDelegation(t *testing.T) { diff --git a/x/alliance/keeper/tests/reward_test.go b/x/alliance/keeper/tests/reward_test.go index e656c385..6268cfbc 100644 --- a/x/alliance/keeper/tests/reward_test.go +++ b/x/alliance/keeper/tests/reward_test.go @@ -1319,3 +1319,94 @@ func TestMigratedRewards(t *testing.T) { require.NoError(t, err) require.Equal(t, sdk.NewInt(0), rewards4.AmountOf(bondDenom)) } + +func TestRewardsDelegateBeforeValidatorClaim(t *testing.T) { + var err error + app, ctx := createTestContext(t) + ctx = ctx.WithBlockHeight(1) + app.AllianceKeeper.InitGenesis(ctx, &types.GenesisState{ + Params: types.DefaultParams(), + Assets: []types.AllianceAsset{ + types.NewAllianceAsset(AllianceDenom, sdk.NewDec(2), sdk.NewDec(0), sdk.NewDec(5), sdk.NewDec(0), ctx.BlockTime()), + types.NewAllianceAsset(AllianceDenomTwo, sdk.NewDec(8), sdk.NewDec(2), sdk.NewDec(12), sdk.NewDec(0), ctx.BlockTime()), + }, + }) + + // Set tax and rewards to be zero for easier calculation + distParams := app.DistrKeeper.GetParams(ctx) + distParams.CommunityTax = sdk.ZeroDec() + + err = app.DistrKeeper.SetParams(ctx, distParams) + require.NoError(t, err) + + // Accounts + // mintPoolAddr := app.AccountKeeper.GetModuleAddress(minttypes.ModuleName) + bondDenom := app.StakingKeeper.BondDenom(ctx) + require.NoError(t, err) + addrs := test_helpers.AddTestAddrsIncremental(app, ctx, 4, sdk.NewCoins( + sdk.NewCoin(AllianceDenom, sdk.NewInt(10_000_000)), + sdk.NewCoin(AllianceDenomTwo, sdk.NewInt(10_000_000)), + )) + + pks := test_helpers.CreateTestPubKeys(2) + + // Creating two validators with 0% commission + valAddr1 := sdk.ValAddress(addrs[0]) + _val1 := teststaking.NewValidator(t, valAddr1, pks[0]) + _val1.Commission = stakingtypes.Commission{ + CommissionRates: stakingtypes.CommissionRates{ + Rate: sdk.NewDec(0), + MaxRate: sdk.NewDec(0), + MaxChangeRate: sdk.NewDec(0), + }, + UpdateTime: time.Now(), + } + test_helpers.RegisterNewValidator(t, app, ctx, _val1) + val1, _ := app.AllianceKeeper.GetAllianceValidator(ctx, valAddr1) + + user1 := addrs[2] + user2 := addrs[3] + + // Mint bond denom + err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin(bondDenom, sdk.NewInt(40_000_000)))) + require.NoError(t, err) + + // New delegations + _, err = app.AllianceKeeper.Delegate(ctx, user2, val1, sdk.NewCoin(AllianceDenom, sdk.NewInt(1000_000))) + require.NoError(t, err) + assets := app.AllianceKeeper.GetAllAssets(ctx) + err = app.AllianceKeeper.RebalanceBondTokenWeights(ctx, assets) + require.NoError(t, err) + + // Transfer to rewards to fee pool to be distributed + err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, authtypes.FeeCollectorName, sdk.NewCoins(sdk.NewCoin(bondDenom, sdk.NewInt(10_000_000)))) + require.NoError(t, err) + + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // Distribute in the next begin block + // At the next begin block, tokens will be distributed from the fee pool + cons1, _ := val1.GetConsAddr() + var votingPower int64 = 100 + app.DistrKeeper.AllocateTokens(ctx, votingPower, []abcitypes.VoteInfo{ + { + Validator: abcitypes.Validator{ + Address: cons1, + Power: 100, + }, + SignedLastBlock: true, + }, + }) + require.NoError(t, err) + + // New delegations + _, err = app.AllianceKeeper.Delegate(ctx, user1, val1, sdk.NewCoin(AllianceDenom, sdk.NewInt(1000_000))) + require.NoError(t, err) + + assets = app.AllianceKeeper.GetAllAssets(ctx) + err = app.AllianceKeeper.RebalanceBondTokenWeights(ctx, assets) + require.NoError(t, err) + + rewards1, err := app.AllianceKeeper.ClaimDelegationRewards(ctx, user1, val1, AllianceDenom) + require.NoError(t, err) + require.Equal(t, sdk.NewInt(0), rewards1.AmountOf(bondDenom)) +}