From d0c5d1ee52b99127733ca2fda1fe5bcf733c7fa7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 18 Sep 2022 12:11:39 +0300 Subject: [PATCH 1/8] updates --- x/distribution/keeper/delegation.go | 26 +++++++++++++++++++++----- x/distribution/keeper/keeper.go | 17 ----------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 9ffc14bab622..35c333ca9681 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -4,6 +4,7 @@ import ( "fmt" "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -163,13 +164,13 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali ) } - // truncate coins, return remainder to community pool - coins, remainder := rewards.TruncateDecimal() + // truncate finalRewards, 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 +191,20 @@ 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() + // 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/keeper.go b/x/distribution/keeper/keeper.go index 31de65efa37f..35368c01ff22 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 From 246351d8022e3cf7fd84768f53fdbd7d3e143e7e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 18 Sep 2022 12:12:51 +0300 Subject: [PATCH 2/8] updates --- x/distribution/keeper/delegation.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 35c333ca9681..3ea4343e2637 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -6,7 +6,6 @@ import ( "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/distribution/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) From 92e12a86a5965c28ee0b985d9e2583b6d895d084 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 18 Sep 2022 12:13:18 +0300 Subject: [PATCH 3/8] updates --- x/distribution/keeper/delegation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 3ea4343e2637..85453cd4c5b6 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -163,7 +163,7 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali ) } - // truncate finalRewards, return remainder to community pool + // truncate reward dec coins, return remainder to community pool finalRewards, remainder := rewards.TruncateDecimal() // add coins to user account From 6aac5a6fb51e28a614ff8b2b4f26c73a886ed64b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 18 Sep 2022 12:14:51 +0300 Subject: [PATCH 4/8] updates --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3e737a3d307..0c5ab329583d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,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. From b9274c1cd4cf63f4e002b3f7472465fbfcec141a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 18 Sep 2022 21:07:10 +0300 Subject: [PATCH 5/8] updates --- testutil/sims/app_helpers.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/testutil/sims/app_helpers.go b/testutil/sims/app_helpers.go index fb030e49f812..c5dc0c9d3aa6 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" @@ -162,6 +163,10 @@ func SetupWithConfiguration(appConfig depinject.Config, startupConfig StartupCon return nil, fmt.Errorf("failed to marshal default genesis state: %w", err) } + if err := sdk.RegisterDenom(sdk.DefaultBondDenom, sdk.NewDecWithPrec(1, 6)); err != nil { + return nil, err + } + // init chain will set the validator set and initialize the genesis accounts app.InitChain( abci.RequestInitChain{ @@ -186,8 +191,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 +235,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) From 700a169e91e57666676fbedbb511d4c07a20dce5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 18 Sep 2022 21:07:57 +0300 Subject: [PATCH 6/8] updates --- x/distribution/keeper/delegation.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 85453cd4c5b6..47617b245e6f 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -192,6 +192,10 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali 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())} From f362c2944f6f9851712388a2a20d08a69b6352b4 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 19 Sep 2022 10:56:56 +0300 Subject: [PATCH 7/8] updates --- testutil/sims/app_helpers.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/testutil/sims/app_helpers.go b/testutil/sims/app_helpers.go index c5dc0c9d3aa6..08aa4597e15a 100644 --- a/testutil/sims/app_helpers.go +++ b/testutil/sims/app_helpers.go @@ -163,10 +163,6 @@ func SetupWithConfiguration(appConfig depinject.Config, startupConfig StartupCon return nil, fmt.Errorf("failed to marshal default genesis state: %w", err) } - if err := sdk.RegisterDenom(sdk.DefaultBondDenom, sdk.NewDecWithPrec(1, 6)); err != nil { - return nil, err - } - // init chain will set the validator set and initialize the genesis accounts app.InitChain( abci.RequestInitChain{ From 83614f6c3dcfc78c8507496ae4c50c5daffcb568 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 19 Sep 2022 11:46:53 +0300 Subject: [PATCH 8/8] updates --- .../distribution/keeper/delegation_test.go | 14 +++++--------- x/distribution/keeper/delegation_test.go | 15 ++++++--------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/tests/integration/distribution/keeper/delegation_test.go b/tests/integration/distribution/keeper/delegation_test.go index 14a3504ca621..0bb991158114 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/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index af8eeb0d3c13..7750beb57110 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 } }