From 0da6b8ad68293c154723862950c2d2df5294621f Mon Sep 17 00:00:00 2001 From: Nick DeLuca Date: Tue, 18 Oct 2022 17:26:32 +0000 Subject: [PATCH 1/3] ensure api compatibility with previous behavior when there are zero delegation rewards -- do not return invalid coins --- x/distribution/keeper/delegation.go | 7 +++--- x/distribution/keeper/delegation_test.go | 27 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index c70062c11d3f..69f1dbc28c9d 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -188,6 +188,7 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali // remove delegator starting info k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) + emittedRewards := finalRewards if finalRewards.IsZero() { baseDenom, _ := sdk.GetBaseDenom() if baseDenom == "" { @@ -195,14 +196,14 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali } // Note, we do not call the NewCoins constructor as we do not want the zero - // coin removed. - finalRewards = sdk.Coins{sdk.NewCoin(baseDenom, sdk.ZeroInt())} + // coin removed for event emission + emittedRewards = sdk.Coins{sdk.NewCoin(baseDenom, sdk.ZeroInt())} } ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeWithdrawRewards, - sdk.NewAttribute(sdk.AttributeKeyAmount, finalRewards.String()), + sdk.NewAttribute(sdk.AttributeKeyAmount, emittedRewards.String()), sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()), ), ) diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index 020bb88c08db..3b633395e021 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -332,6 +332,33 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { require.Nil(t, err) } +func TestWithdrawDelegationZeroRewards(t *testing.T) { + app := simapp.Setup(false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + balancePower := int64(1000) + balanceTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, balancePower) + addr := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(1000000000)) + valAddrs := simapp.ConvertAddrsToValAddrs(addr) + tstaking := teststaking.NewHelper(t, ctx, app.StakingKeeper) + + // set module account coins + distrAcc := app.DistrKeeper.GetDistributionAccount(ctx) + require.NoError(t, simapp.FundModuleAccount(app.BankKeeper, ctx, distrAcc.GetName(), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, balanceTokens)))) + app.AccountKeeper.SetModuleAccount(ctx, distrAcc) + + // create validator with 50% commission + power := int64(100) + tstaking.Commission = stakingtypes.NewCommissionRates(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) + _ = tstaking.CreateValidatorWithValPower(valAddrs[0], valConsPk1, power, true) + + // withdraw rewards -- should be 0 + amount, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, sdk.AccAddress(valAddrs[0]), valAddrs[0]) + require.NoError(t, err) + require.True(t, amount.IsZero(), "expected withdraw rewards to be zero") + require.True(t, amount.IsValid(), "expected returned coins to be valid") +} + func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) { app := simapp.Setup(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) From 44c0873794743811dfeea61c440b2458bffe2ac5 Mon Sep 17 00:00:00 2001 From: Nick DeLuca Date: Tue, 18 Oct 2022 17:46:13 +0000 Subject: [PATCH 2/3] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8471832b535b..2ac72fb4b5f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* [#13588](https://github.com/cosmos/cosmos-sdk/pull/13588) Fix regression in distrubtion.WithdrawDelegationRewards when rewards are zero * [#13564](https://github.com/cosmos/cosmos-sdk/pull/13564) - Fix `make proto-gen`. ## v0.45.9 - 2022-10-14 From 12eb91402c7631dd40a793b0cc06183cb51561cf Mon Sep 17 00:00:00 2001 From: Nick DeLuca Date: Tue, 18 Oct 2022 13:03:19 -0700 Subject: [PATCH 3/3] Update x/distribution/keeper/delegation.go Co-authored-by: Aleksandr Bezobchuk --- 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 69f1dbc28c9d..a9aa8b2520ae 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -196,7 +196,7 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali } // Note, we do not call the NewCoins constructor as we do not want the zero - // coin removed for event emission + // coin removed for event emission. emittedRewards = sdk.Coins{sdk.NewCoin(baseDenom, sdk.ZeroInt())} }