From f2b4ac0bb067fc112018979be1a24009f7368935 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 20 Sep 2022 06:23:21 -0400 Subject: [PATCH] fix: ensure withdraw_rewards events are always emitted on reward withdrawal (#13323) --- CHANGELOG.md | 1 + .../distribution/keeper/delegation_test.go | 14 +++------ testutil/sims/app_helpers.go | 9 ++++-- x/distribution/keeper/delegation.go | 31 +++++++++++++++---- x/distribution/keeper/delegation_test.go | 15 ++++----- x/distribution/keeper/keeper.go | 17 ---------- 6 files changed, 44 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 224d1c8762ffe..bcd05e5b65d57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [#13323](https://github.com/cosmos/cosmos-sdk/pull/13323) Ensure `withdraw_rewards` rewards are emitted from all actions that result in rewards being withdrawn. * [#13214](https://github.com/cosmos/cosmos-sdk/pull/13214) Add `withdraw-proposal` command to group module's CLI transaction commands. * [#13070](https://github.com/cosmos/cosmos-sdk/pull/13070) Migrate from `gogo/protobuf` to `cosmos/gogoproto`. * [#12981](https://github.com/cosmos/cosmos-sdk/pull/12981) Return proper error when parsing telemetry configuration. diff --git a/tests/integration/distribution/keeper/delegation_test.go b/tests/integration/distribution/keeper/delegation_test.go index 14a3504ca6212..0bb9911581146 100644 --- a/tests/integration/distribution/keeper/delegation_test.go +++ b/tests/integration/distribution/keeper/delegation_test.go @@ -804,19 +804,15 @@ func Test100PercentCommissionReward(t *testing.T) { rewards, err := distrKeeper.WithdrawDelegationRewards(ctx, sdk.AccAddress(valAddrs[0]), valAddrs[0]) require.NoError(t, err) - denom, _ := sdk.GetBaseDenom() - zeroRewards := sdk.Coins{ - sdk.Coin{ - Denom: denom, - Amount: math.ZeroInt(), - }, - } + zeroRewards := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, math.ZeroInt())} require.True(t, rewards.IsEqual(zeroRewards)) + events := ctx.EventManager().Events() lastEvent := events[len(events)-1] - hasValue := false + + var hasValue bool for _, attr := range lastEvent.Attributes { - if string(attr.Key) == "amount" && string(attr.Value) == "0" { + if attr.Key == "amount" && attr.Value == "0stake" { hasValue = true } } diff --git a/testutil/sims/app_helpers.go b/testutil/sims/app_helpers.go index fb030e49f8120..08aa4597e15a7 100644 --- a/testutil/sims/app_helpers.go +++ b/testutil/sims/app_helpers.go @@ -14,6 +14,7 @@ import ( "cosmossdk.io/depinject" "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -186,8 +187,11 @@ func SetupWithConfiguration(appConfig depinject.Config, startupConfig StartupCon } // GenesisStateWithValSet returns a new genesis state with the validator set -func GenesisStateWithValSet(codec codec.Codec, genesisState map[string]json.RawMessage, - valSet *tmtypes.ValidatorSet, genAccs []authtypes.GenesisAccount, +func GenesisStateWithValSet( + codec codec.Codec, + genesisState map[string]json.RawMessage, + valSet *tmtypes.ValidatorSet, + genAccs []authtypes.GenesisAccount, balances ...banktypes.Balance, ) (map[string]json.RawMessage, error) { // set genesis accounts @@ -227,6 +231,7 @@ func GenesisStateWithValSet(codec codec.Codec, genesisState map[string]json.RawM delegations = append(delegations, stakingtypes.NewDelegation(genAccs[0].GetAddress(), val.Address.Bytes(), math.LegacyOneDec())) } + // set validators and delegations stakingGenesis := stakingtypes.NewGenesisState(stakingtypes.DefaultParams(), validators, delegations) genesisState[stakingtypes.ModuleName] = codec.MustMarshalJSON(stakingGenesis) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 9ffc14bab622e..47617b245e6fd 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -4,8 +4,8 @@ import ( "fmt" "cosmossdk.io/math" - sdk "github.com/cosmos/cosmos-sdk/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) @@ -163,13 +163,13 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali ) } - // truncate coins, return remainder to community pool - coins, remainder := rewards.TruncateDecimal() + // truncate reward dec coins, return remainder to community pool + finalRewards, remainder := rewards.TruncateDecimal() // add coins to user account - if !coins.IsZero() { + if !finalRewards.IsZero() { withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, del.GetDelegatorAddr()) - err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, coins) + err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, finalRewards) if err != nil { return nil, err } @@ -190,5 +190,24 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali // remove delegator starting info k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) - return coins, nil + if finalRewards.IsZero() { + baseDenom, _ := sdk.GetBaseDenom() + if baseDenom == "" { + baseDenom = sdk.DefaultBondDenom + } + + // Note, we do not call the NewCoins constructor as we do not want the zero + // coin removed. + finalRewards = sdk.Coins{sdk.NewCoin(baseDenom, math.ZeroInt())} + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeWithdrawRewards, + sdk.NewAttribute(sdk.AttributeKeyAmount, finalRewards.String()), + sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()), + ), + ) + + return finalRewards, nil } diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index af8eeb0d3c13d..7750beb571109 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -8,6 +8,7 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" @@ -959,19 +960,15 @@ func Test100PercentCommissionReward(t *testing.T) { rewards, err := distrKeeper.WithdrawDelegationRewards(ctx, sdk.AccAddress(addr), valAddr) require.NoError(t, err) - denom, _ := sdk.GetBaseDenom() - zeroRewards := sdk.Coins{ - sdk.Coin{ - Denom: denom, - Amount: math.ZeroInt(), - }, - } + zeroRewards := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, math.ZeroInt())} require.True(t, rewards.IsEqual(zeroRewards)) + events := ctx.EventManager().Events() lastEvent := events[len(events)-1] - hasValue := false + + var hasValue bool for _, attr := range lastEvent.Attributes { - if string(attr.Key) == "amount" && string(attr.Value) == "0" { + if attr.Key == "amount" && attr.Value == "0stake" { hasValue = true } } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 31de65efa37f5..35368c01ff221 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -3,7 +3,6 @@ package keeper import ( "fmt" - "cosmossdk.io/math" "github.com/tendermint/tendermint/libs/log" "github.com/cosmos/cosmos-sdk/codec" @@ -98,22 +97,6 @@ func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddres return nil, err } - if rewards.IsZero() { - baseDenom, _ := sdk.GetBaseDenom() - rewards = sdk.Coins{sdk.Coin{ - Denom: baseDenom, - Amount: math.ZeroInt(), - }} - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeWithdrawRewards, - sdk.NewAttribute(sdk.AttributeKeyAmount, rewards.String()), - sdk.NewAttribute(types.AttributeKeyValidator, valAddr.String()), - ), - ) - // reinitialize the delegation k.initializeDelegation(ctx, valAddr, delAddr) return rewards, nil