From 2c6b8665515c02b628608dd59fdccd2187c97f0a Mon Sep 17 00:00:00 2001 From: Tyler <48813565+technicallyty@users.noreply.github.com> Date: Mon, 5 Jul 2021 11:28:25 -0700 Subject: [PATCH] fix: WithdrawRewards event emit value when no rewards (#9599) ## Description Rewards values of 0 were omitted from the `WithdrawRewards` event. To provide consistency in event responses, this is caught in the keeper and emitted as a value of `0[baseDenom]`. Closes: #8581 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [x] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- CHANGELOG.md | 1 + x/distribution/keeper/delegation_test.go | 69 ++++++++++++++++++++++++ x/distribution/keeper/keeper.go | 8 +++ 3 files changed, 78 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae5083ee86b8..6ca3bba6e40e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (types) [\#9627](https://github.com/cosmos/cosmos-sdk/pull/9627) Fix nil pointer panic on `NewBigIntFromInt` * (x/genutil) [\#9574](https://github.com/cosmos/cosmos-sdk/pull/9575) Actually use the `gentx` client tx flags (like `--keyring-dir`) +* (x/distribution) [\#9599](https://github.com/cosmos/cosmos-sdk/pull/9599) Withdraw rewards event now includes a value attribute even if there are 0 rewards (due to situations like 100% commission). ## [v0.43.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc0) - 2021-06-25 diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index 7da5fc029684..c86cb2ef948b 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -626,3 +626,72 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // commission should be zero require.True(t, app.DistrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[0]).Commission.IsZero()) } + +func Test100PercentCommissionReward(t *testing.T) { + app := simapp.Setup(false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + tstaking := teststaking.NewHelper(t, ctx, app.StakingKeeper) + addr := simapp.AddTestAddrs(app, ctx, 2, sdk.NewInt(1000000000)) + valAddrs := simapp.ConvertAddrsToValAddrs(addr) + initial := int64(20) + + // set module account coins + distrAcc := app.DistrKeeper.GetDistributionAccount(ctx) + require.NoError(t, testutil.FundModuleAccount(app.BankKeeper, ctx, distrAcc.GetName(), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000))))) + app.AccountKeeper.SetModuleAccount(ctx, distrAcc) + + tokens := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDec(initial))} + + // create validator with 100% commission + tstaking.Commission = stakingtypes.NewCommissionRates(sdk.NewDecWithPrec(10, 1), sdk.NewDecWithPrec(10, 1), sdk.NewDec(0)) + tstaking.CreateValidator(valAddrs[0], valConsPk1, sdk.NewInt(100), true) + app.StakingKeeper.Delegation(ctx, sdk.AccAddress(valAddrs[0]), valAddrs[0]) + + // end block to bond validator + staking.EndBlocker(ctx, app.StakingKeeper) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + + // fetch validator + val := app.StakingKeeper.Validator(ctx, valAddrs[0]) + + // allocate some rewards + app.DistrKeeper.AllocateTokensToValidator(ctx, val, tokens) + + // end block + staking.EndBlocker(ctx, app.StakingKeeper) + + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + + // allocate some more rewards + app.DistrKeeper.AllocateTokensToValidator(ctx, val, tokens) + + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + + // allocate some more rewards + app.DistrKeeper.AllocateTokensToValidator(ctx, val, tokens) + + rewards, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, sdk.AccAddress(valAddrs[0]), valAddrs[0]) + require.NoError(t, err) + + denom, _ := sdk.GetBaseDenom() + zeroRewards := sdk.Coins{ + sdk.Coin{ + Denom: denom, + Amount: sdk.ZeroInt(), + }, + } + require.True(t, rewards.IsEqual(zeroRewards)) + events := ctx.EventManager().Events() + lastEvent := events[len(events)-1] + hasValue := false + for _, attr := range lastEvent.Attributes { + if string(attr.Key) == "amount" && string(attr.Value) == "0" { + hasValue = true + } + } + require.True(t, hasValue) +} diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index b139c35f4a50..97669cae23a0 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -99,6 +99,14 @@ 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: sdk.ZeroInt(), + }} + } + ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeWithdrawRewards,