From ec4a1e8f2aa50ad25a684d50c2c9a29bed77659c Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 26 Apr 2019 19:11:45 -0400 Subject: [PATCH] Merge PR #4207: Add rewards and commission tags in txs --- ...rds-and-commission-to-distribution-tx-tags | 1 + cmd/gaia/app/export.go | 4 +-- docs/spec/distribution/06_tags.md | 12 ++++---- x/distribution/handler.go | 28 +++++++++---------- x/distribution/keeper/delegation.go | 9 +++--- x/distribution/keeper/delegation_test.go | 6 ++-- x/distribution/keeper/hooks.go | 2 +- x/distribution/keeper/invariants.go | 4 +-- x/distribution/keeper/keeper.go | 22 +++++++-------- x/distribution/tags/tags.go | 3 ++ 10 files changed, 48 insertions(+), 43 deletions(-) create mode 100644 .pending/features/sdk/3850-Add-rewards-and-commission-to-distribution-tx-tags diff --git a/.pending/features/sdk/3850-Add-rewards-and-commission-to-distribution-tx-tags b/.pending/features/sdk/3850-Add-rewards-and-commission-to-distribution-tx-tags new file mode 100644 index 0000000000..7a5ebebdb2 --- /dev/null +++ b/.pending/features/sdk/3850-Add-rewards-and-commission-to-distribution-tx-tags @@ -0,0 +1 @@ +#3850 Add `rewards` and `commission` to distribution tx tags. diff --git a/cmd/gaia/app/export.go b/cmd/gaia/app/export.go index 5174c1498a..2cb110bf89 100644 --- a/cmd/gaia/app/export.go +++ b/cmd/gaia/app/export.go @@ -84,14 +84,14 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []st // withdraw all validator commission app.stakingKeeper.IterateValidators(ctx, func(_ int64, val sdk.Validator) (stop bool) { - _ = app.distrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator()) + _, _ = app.distrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator()) return false }) // withdraw all delegator rewards dels := app.stakingKeeper.GetAllDelegations(ctx) for _, delegation := range dels { - _ = app.distrKeeper.WithdrawDelegationRewards(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress) + _, _ = app.distrKeeper.WithdrawDelegationRewards(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress) } // clear validator slash events diff --git a/docs/spec/distribution/06_tags.md b/docs/spec/distribution/06_tags.md index 5e0592f008..12a5687093 100644 --- a/docs/spec/distribution/06_tags.md +++ b/docs/spec/distribution/06_tags.md @@ -18,13 +18,15 @@ The distribution module emits the following events/tags: |--------------------|-----------------------------| | `action` | `withdraw_delegator_reward` | | `category` | `distribution` | +| `rewards` | {rewards} | | `sender` | {delegatorAccountAddress} | | `source-validator` | {srcOperatorAddress} | ### MsgWithdrawValidatorCommission -| Key | Value | -|------------|---------------------------------| -| `action` | `withdraw_validator_commission` | -| `category` | `distribution` | -| `sender` | {srcOperatorAddress} | +| Key | Value | +|--------------|---------------------------------| +| `action` | `withdraw_validator_commission` | +| `category` | `distribution` | +| `commission` | {commission} | +| `sender` | {srcOperatorAddress} | diff --git a/x/distribution/handler.go b/x/distribution/handler.go index 64f824b9f8..a0276bf70b 100644 --- a/x/distribution/handler.go +++ b/x/distribution/handler.go @@ -48,34 +48,32 @@ func handleMsgModifyWithdrawAddress(ctx sdk.Context, msg types.MsgSetWithdrawAdd } func handleMsgWithdrawDelegatorReward(ctx sdk.Context, msg types.MsgWithdrawDelegatorReward, k keeper.Keeper) sdk.Result { - - err := k.WithdrawDelegationRewards(ctx, msg.DelegatorAddress, msg.ValidatorAddress) + rewards, err := k.WithdrawDelegationRewards(ctx, msg.DelegatorAddress, msg.ValidatorAddress) if err != nil { return err.Result() } - resTags := sdk.NewTags( - tags.Category, tags.TxCategory, - tags.Sender, msg.DelegatorAddress.String(), - tags.Validator, msg.ValidatorAddress.String(), - ) return sdk.Result{ - Tags: resTags, + Tags: sdk.NewTags( + tags.Rewards, rewards.String(), + tags.Category, tags.TxCategory, + tags.Sender, msg.DelegatorAddress.String(), + tags.Validator, msg.ValidatorAddress.String(), + ), } } func handleMsgWithdrawValidatorCommission(ctx sdk.Context, msg types.MsgWithdrawValidatorCommission, k keeper.Keeper) sdk.Result { - - err := k.WithdrawValidatorCommission(ctx, msg.ValidatorAddress) + commission, err := k.WithdrawValidatorCommission(ctx, msg.ValidatorAddress) if err != nil { return err.Result() } - resTags := sdk.NewTags( - tags.Category, tags.TxCategory, - tags.Sender, msg.ValidatorAddress.String(), - ) return sdk.Result{ - Tags: resTags, + Tags: sdk.NewTags( + tags.Commission, commission.String(), + tags.Category, tags.TxCategory, + tags.Sender, msg.ValidatorAddress.String(), + ), } } diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 20321e870a..01b3ffaa64 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -115,11 +115,10 @@ func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, d return rewards } -func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, del sdk.Delegation) sdk.Error { - +func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, del sdk.Delegation) (sdk.Coins, sdk.Error) { // check existence of delegator starting info if !k.HasDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) { - return types.ErrNoDelegationDistInfo(k.codespace) + return nil, types.ErrNoDelegationDistInfo(k.codespace) } // end current period and calculate rewards @@ -154,12 +153,12 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, de if !coins.IsZero() { withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, del.GetDelegatorAddr()) if _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { - return err + return nil, err } } // remove delegator starting info k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) - return nil + return coins, nil } diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index d6eb848e7d..75c4c7de4f 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -303,7 +303,8 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) // withdraw rewards - require.Nil(t, k.WithdrawDelegationRewards(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1)) + _, err := k.WithdrawDelegationRewards(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) + require.Nil(t, err) // historical count should still be 2 (added one record, cleared one) require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) @@ -316,7 +317,8 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { ) // withdraw commission - require.Nil(t, k.WithdrawValidatorCommission(ctx, valOpAddr1)) + _, err = k.WithdrawValidatorCommission(ctx, valOpAddr1) + require.Nil(t, err) // assert correct balance exp = balanceTokens.Sub(valTokens).Add(initial) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 022c8d943c..07d7d23195 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -83,7 +83,7 @@ func (h Hooks) BeforeDelegationSharesModified(ctx sdk.Context, delAddr sdk.AccAd del := h.k.stakingKeeper.Delegation(ctx, delAddr, valAddr) // withdraw delegation rewards (which also increments period) - if err := h.k.withdrawDelegationRewards(ctx, val, del); err != nil { + if _, err := h.k.withdrawDelegationRewards(ctx, val, del); err != nil { panic(err) } } diff --git a/x/distribution/keeper/invariants.go b/x/distribution/keeper/invariants.go index 4eb2a54005..907ff02644 100644 --- a/x/distribution/keeper/invariants.go +++ b/x/distribution/keeper/invariants.go @@ -76,12 +76,12 @@ func CanWithdrawInvariant(k Keeper, sk types.StakingKeeper) sdk.Invariant { // iterate over all validators sk.IterateValidators(ctx, func(_ int64, val sdk.Validator) (stop bool) { - _ = k.WithdrawValidatorCommission(ctx, val.GetOperator()) + _, _ = k.WithdrawValidatorCommission(ctx, val.GetOperator()) delegationAddrs, ok := valDelegationAddrs[val.GetOperator().String()] if ok { for _, delAddr := range delegationAddrs { - if err := k.WithdrawDelegationRewards(ctx, delAddr, val.GetOperator()); err != nil { + if _, err := k.WithdrawDelegationRewards(ctx, delAddr, val.GetOperator()); err != nil { panic(err) } } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 1a4c049282..37ce7b859d 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -52,35 +52,35 @@ func (k Keeper) SetWithdrawAddr(ctx sdk.Context, delegatorAddr sdk.AccAddress, w } // withdraw rewards from a delegation -func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) sdk.Error { +func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (sdk.Coins, sdk.Error) { val := k.stakingKeeper.Validator(ctx, valAddr) if val == nil { - return types.ErrNoValidatorDistInfo(k.codespace) + return nil, types.ErrNoValidatorDistInfo(k.codespace) } del := k.stakingKeeper.Delegation(ctx, delAddr, valAddr) if del == nil { - return types.ErrNoDelegationDistInfo(k.codespace) + return nil, types.ErrNoDelegationDistInfo(k.codespace) } // withdraw rewards - if err := k.withdrawDelegationRewards(ctx, val, del); err != nil { - return err + rewards, err := k.withdrawDelegationRewards(ctx, val, del) + if err != nil { + return nil, err } // reinitialize the delegation k.initializeDelegation(ctx, valAddr, delAddr) - return nil + return rewards, nil } // withdraw validator commission -func (k Keeper) WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddress) sdk.Error { - +func (k Keeper) WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddress) (sdk.Coins, sdk.Error) { // fetch validator accumulated commission commission := k.GetValidatorAccumulatedCommission(ctx, valAddr) if commission.IsZero() { - return types.ErrNoValidatorCommission(k.codespace) + return nil, types.ErrNoValidatorCommission(k.codespace) } coins, remainder := commission.TruncateDecimal() @@ -95,9 +95,9 @@ func (k Keeper) WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddr withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, accAddr) if _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { - return err + return nil, err } } - return nil + return coins, nil } diff --git a/x/distribution/tags/tags.go b/x/distribution/tags/tags.go index 8c84cddeae..96dadacd74 100644 --- a/x/distribution/tags/tags.go +++ b/x/distribution/tags/tags.go @@ -4,7 +4,10 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +// Distribution tx tags var ( + Rewards = "rewards" + Commission = "commission" TxCategory = "distribution" Validator = sdk.TagSrcValidator