From 99913cdce00ca0ebb381c117b99cdec4c314f47d Mon Sep 17 00:00:00 2001 From: testinginprod <98415576+testinginprod@users.noreply.github.com> Date: Thu, 15 Jun 2023 16:12:42 +0200 Subject: [PATCH] refactor(distribution)!: use collections for validator accumulated commission state (#16571) Co-authored-by: unknown unknown --- CHANGELOG.md | 2 + .../distribution/keeper/grpc_query_test.go | 2 +- .../distribution/keeper/msg_server_test.go | 5 +- x/distribution/keeper/allocation.go | 6 +- x/distribution/keeper/allocation_test.go | 26 ++++---- x/distribution/keeper/delegation_test.go | 18 +++--- x/distribution/keeper/genesis.go | 21 ++++--- x/distribution/keeper/grpc_query.go | 9 +-- x/distribution/keeper/hooks.go | 9 ++- x/distribution/keeper/keeper.go | 31 +++++++--- x/distribution/keeper/keeper_test.go | 4 +- x/distribution/keeper/store.go | 61 ------------------- x/distribution/keeper/validator.go | 2 +- x/distribution/migrations/v2/store_test.go | 2 +- x/distribution/simulation/decoder_test.go | 3 - x/distribution/simulation/operations.go | 6 +- x/distribution/simulation/operations_test.go | 4 +- x/distribution/types/keys.go | 11 +--- 18 files changed, 85 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c5cf711c7..6128a5b23c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/authz) [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`. * (x/distribution) [#16483](https://github.com/cosmos/cosmos-sdk/pull/16483) use collections for `DelegatorStartingInfo` state management: * remove `Keeper`: `IterateDelegatorStartingInfo`, `GetDelegatorStartingInfo`, `SetDelegatorStartingInfo`, `DeleteDelegatorStartingInfo`, `HasDelegatorStartingInfo` +* (x/distribution) [#16571](https://github.com/cosmos/cosmos-sdk/pull/16571) use collections for `ValidatorAccumulatedCommission` state management: + * remove `Keeper`: `IterateValidatorAccumulatedCommission`, `GetValidatorAccumulatedCommission`, `SetValidatorAccumulatedCommission`, `DeleteValidatorAccumulatedCommission` ### Bug Fixes diff --git a/tests/integration/distribution/keeper/grpc_query_test.go b/tests/integration/distribution/keeper/grpc_query_test.go index 868f13eac3..9e6a19fbe1 100644 --- a/tests/integration/distribution/keeper/grpc_query_test.go +++ b/tests/integration/distribution/keeper/grpc_query_test.go @@ -173,7 +173,7 @@ func TestGRPCValidatorCommission(t *testing.T) { tstaking.CreateValidator(f.valAddr, valConsPk0, sdk.NewInt(initialStake), true) commission := sdk.DecCoins{sdk.DecCoin{Denom: "token1", Amount: math.LegacyNewDec(4)}, {Denom: "token2", Amount: math.LegacyNewDec(2)}} - assert.NilError(t, f.distrKeeper.SetValidatorAccumulatedCommission(f.sdkCtx, f.valAddr, types.ValidatorAccumulatedCommission{Commission: commission})) + assert.NilError(t, f.distrKeeper.ValidatorsAccumulatedCommission.Set(f.sdkCtx, f.valAddr, types.ValidatorAccumulatedCommission{Commission: commission})) testCases := []struct { name string diff --git a/tests/integration/distribution/keeper/msg_server_test.go b/tests/integration/distribution/keeper/msg_server_test.go index 0df1d2f77c..02e0d470af 100644 --- a/tests/integration/distribution/keeper/msg_server_test.go +++ b/tests/integration/distribution/keeper/msg_server_test.go @@ -481,7 +481,7 @@ func TestMsgWithdrawValidatorCommission(t *testing.T) { require.NoError(t, err) // set commission - err = f.distrKeeper.SetValidatorAccumulatedCommission(f.sdkCtx, f.valAddr, distrtypes.ValidatorAccumulatedCommission{Commission: valCommission}) + err = f.distrKeeper.ValidatorsAccumulatedCommission.Set(f.sdkCtx, f.valAddr, distrtypes.ValidatorAccumulatedCommission{Commission: valCommission}) require.NoError(t, err) testCases := []struct { @@ -542,7 +542,8 @@ func TestMsgWithdrawValidatorCommission(t *testing.T) { ), balance) // check remainder - remainder, _ := f.distrKeeper.GetValidatorAccumulatedCommission(f.sdkCtx, f.valAddr) + remainder, err := f.distrKeeper.ValidatorsAccumulatedCommission.Get(f.sdkCtx, f.valAddr) + require.NoError(t, err) assert.DeepEqual(t, sdk.DecCoins{ sdk.NewDecCoinFromDec("mytoken", math.LegacyNewDec(1).Quo(math.LegacyNewDec(4))), sdk.NewDecCoinFromDec("stake", math.LegacyNewDec(1).Quo(math.LegacyNewDec(2))), diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index 5c654c48cb..93bde40152 100644 --- a/x/distribution/keeper/allocation.go +++ b/x/distribution/keeper/allocation.go @@ -95,13 +95,13 @@ func (k Keeper) AllocateTokensToValidator(ctx context.Context, val stakingtypes. sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()), ), ) - currentCommission, err := k.GetValidatorAccumulatedCommission(ctx, val.GetOperator()) - if err != nil { + currentCommission, err := k.ValidatorsAccumulatedCommission.Get(ctx, val.GetOperator()) + if err != nil && !errors.Is(err, collections.ErrNotFound) { return err } currentCommission.Commission = currentCommission.Commission.Add(commission...) - err = k.SetValidatorAccumulatedCommission(ctx, val.GetOperator(), currentCommission) + err = k.ValidatorsAccumulatedCommission.Set(ctx, val.GetOperator(), currentCommission) if err != nil { return err } diff --git a/x/distribution/keeper/allocation_test.go b/x/distribution/keeper/allocation_test.go index dfd426ee0e..94052aa846 100644 --- a/x/distribution/keeper/allocation_test.go +++ b/x/distribution/keeper/allocation_test.go @@ -66,7 +66,7 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) { {Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(5)}, } - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, val.GetOperator()) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, val.GetOperator()) require.NoError(t, err) require.Equal(t, expected, valCommission.Commission) @@ -142,13 +142,11 @@ func TestAllocateTokensToManyValidators(t *testing.T) { require.NoError(t, err) require.True(t, feePool.CommunityPool.IsZero()) - val0Commission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr0) - require.NoError(t, err) - require.True(t, val0Commission.Commission.IsZero()) + _, err = distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr0) + require.ErrorIs(t, err, collections.ErrNotFound) - val1Commission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr1) - require.NoError(t, err) - require.True(t, val1Commission.Commission.IsZero()) + _, err = distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr1) + require.ErrorIs(t, err, collections.ErrNotFound) _, err = distrKeeper.ValidatorCurrentRewards.Get(ctx, valAddr0) require.ErrorIs(t, err, collections.ErrNotFound) // require no rewards @@ -186,12 +184,12 @@ func TestAllocateTokensToManyValidators(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(2)}}, feePool.CommunityPool) // 50% commission for first proposer, (0.5 * 98%) * 100 / 2 = 23.25 - val0Commission, err = distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr0) + val0Commission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr0) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDecWithPrec(2450, 2)}}, val0Commission.Commission) // zero commission for second proposer - val1Commission, err = distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr1) + val1Commission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr1) require.NoError(t, err) require.True(t, val1Commission.Commission.IsZero()) @@ -283,13 +281,11 @@ func TestAllocateTokensTruncation(t *testing.T) { require.NoError(t, err) require.True(t, feePool.CommunityPool.IsZero()) - val0Commission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr0) - require.NoError(t, err) - require.True(t, val0Commission.Commission.IsZero()) + _, err = distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr0) + require.ErrorIs(t, err, collections.ErrNotFound) - val1Commission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr1) - require.NoError(t, err) - require.True(t, val1Commission.Commission.IsZero()) + _, err = distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr1) + require.ErrorIs(t, err, collections.ErrNotFound) _, err = distrKeeper.ValidatorCurrentRewards.Get(ctx, valAddr0) require.ErrorIs(t, err, collections.ErrNotFound) // require no rewards diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index e1e1680244..125723658c 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -100,7 +100,7 @@ func TestCalculateRewardsBasic(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial / 2)}}, rewards) // commission should be the other half - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial / 2)}}, valCommission.Commission) } @@ -199,7 +199,7 @@ func TestCalculateRewardsAfterSlash(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDecFromInt(initial.QuoRaw(2))}}, rewards) // commission should be the other half - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDecFromInt(initial.QuoRaw(2))}}, valCommission.Commission) @@ -319,7 +319,7 @@ func TestCalculateRewardsAfterManySlashes(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDecFromInt(initial)}}, rewards) // commission should be the other half - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDecFromInt(initial)}}, valCommission.Commission) @@ -415,7 +415,7 @@ func TestCalculateRewardsMultiDelegator(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial * 1 / 4)}}, rewards) // commission should be equal to initial (50% twice) - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial)}}, valCommission.Commission) } @@ -600,7 +600,7 @@ func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial}}, rewards) // commission should be the other half - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial}}, valCommission.Commission) } @@ -732,7 +732,7 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial.QuoInt64(3)}}, rewards) // commission should be equal to initial (twice 50% commission, unaffected by slashing) - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial}}, valCommission.Commission) } @@ -860,7 +860,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.True(t, rewards.IsZero()) // commission should be zero - valCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.True(t, valCommission.Commission.IsZero()) @@ -894,7 +894,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial / 4)}}, rewards) // commission should be half initial - valCommission, err = distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err = distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial / 2)}}, valCommission.Commission) @@ -928,7 +928,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial / 2)}}, rewards) // commission should be zero - valCommission, err = distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + valCommission, err = distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) require.True(t, valCommission.Commission.IsZero()) } diff --git a/x/distribution/keeper/genesis.go b/x/distribution/keeper/genesis.go index 32055e30d6..dc3a5c4f2b 100644 --- a/x/distribution/keeper/genesis.go +++ b/x/distribution/keeper/genesis.go @@ -64,7 +64,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data types.GenesisState) { if err != nil { panic(err) } - err = k.SetValidatorAccumulatedCommission(ctx, valAddr, acc.Accumulated) + err = k.ValidatorsAccumulatedCommission.Set(ctx, valAddr, acc.Accumulated) if err != nil { panic(err) } @@ -175,15 +175,16 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { ) acc := make([]types.ValidatorAccumulatedCommissionRecord, 0) - k.IterateValidatorAccumulatedCommissions(ctx, - func(addr sdk.ValAddress, commission types.ValidatorAccumulatedCommission) (stop bool) { - acc = append(acc, types.ValidatorAccumulatedCommissionRecord{ - ValidatorAddress: addr.String(), - Accumulated: commission, - }) - return false - }, - ) + err = k.ValidatorsAccumulatedCommission.Walk(ctx, nil, func(addr sdk.ValAddress, commission types.ValidatorAccumulatedCommission) (stop bool, err error) { + acc = append(acc, types.ValidatorAccumulatedCommissionRecord{ + ValidatorAddress: addr.String(), + Accumulated: commission, + }) + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } his := make([]types.ValidatorHistoricalRewardsRecord, 0) k.IterateValidatorHistoricalRewards(ctx, diff --git a/x/distribution/keeper/grpc_query.go b/x/distribution/keeper/grpc_query.go index c4f69aded6..309cf2a37d 100644 --- a/x/distribution/keeper/grpc_query.go +++ b/x/distribution/keeper/grpc_query.go @@ -3,6 +3,7 @@ package keeper import ( "context" + "cosmossdk.io/collections" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -77,8 +78,8 @@ func (k Querier) ValidatorDistributionInfo(c context.Context, req *types.QueryVa } // validator's commission - validatorCommission, err := k.GetValidatorAccumulatedCommission(ctx, valAdr) - if err != nil { + validatorCommission, err := k.ValidatorsAccumulatedCommission.Get(ctx, valAdr) + if err != nil && !errors.IsOf(err, collections.ErrNotFound) { return nil, err } @@ -140,8 +141,8 @@ func (k Querier) ValidatorCommission(c context.Context, req *types.QueryValidato if validator == nil { return nil, errors.Wrapf(types.ErrNoValidatorExists, valAdr.String()) } - commission, err := k.GetValidatorAccumulatedCommission(ctx, valAdr) - if err != nil { + commission, err := k.ValidatorsAccumulatedCommission.Get(ctx, valAdr) + if err != nil && !errors.IsOf(err, collections.ErrNotFound) { return nil, err } diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index b35477ce18..b18b299459 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -1,6 +1,9 @@ package keeper import ( + "errors" + + "cosmossdk.io/collections" sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" @@ -35,8 +38,8 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr } // force-withdraw commission - valCommission, err := h.k.GetValidatorAccumulatedCommission(ctx, valAddr) - if err != nil { + valCommission, err := h.k.ValidatorsAccumulatedCommission.Get(ctx, valAddr) + if err != nil && !errors.Is(err, collections.ErrNotFound) { return err } @@ -96,7 +99,7 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr } // remove commission record - err = h.k.DeleteValidatorAccumulatedCommission(ctx, valAddr) + err = h.k.ValidatorsAccumulatedCommission.Remove(ctx, valAddr) if err != nil { return err } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index f579095f95..dfa84ea8fc 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -9,6 +9,7 @@ import ( "cosmossdk.io/core/store" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" + "github.com/cockroachdb/errors" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -27,12 +28,13 @@ type Keeper struct { // should be the x/gov module account. authority string - Schema collections.Schema - Params collections.Item[types.Params] - FeePool collections.Item[types.FeePool] - DelegatorsWithdrawAddress collections.Map[sdk.AccAddress, sdk.AccAddress] - ValidatorCurrentRewards collections.Map[sdk.ValAddress, types.ValidatorCurrentRewards] - DelegatorStartingInfo collections.Map[collections.Pair[sdk.ValAddress, sdk.AccAddress], types.DelegatorStartingInfo] + Schema collections.Schema + Params collections.Item[types.Params] + FeePool collections.Item[types.FeePool] + DelegatorsWithdrawAddress collections.Map[sdk.AccAddress, sdk.AccAddress] + ValidatorCurrentRewards collections.Map[sdk.ValAddress, types.ValidatorCurrentRewards] + DelegatorStartingInfo collections.Map[collections.Pair[sdk.ValAddress, sdk.AccAddress], types.DelegatorStartingInfo] + ValidatorsAccumulatedCommission collections.Map[sdk.ValAddress, types.ValidatorAccumulatedCommission] feeCollectorName string // name of the FeeCollector ModuleAccount } @@ -80,6 +82,13 @@ func NewKeeper( collections.PairKeyCodec(sdk.ValAddressKey, sdk.LengthPrefixedAddressKey(sdk.AccAddressKey)), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility codec.CollValue[types.DelegatorStartingInfo](cdc), ), + ValidatorsAccumulatedCommission: collections.NewMap( + sb, + types.ValidatorAccumulatedCommissionPrefix, + "validators_accumulated_commission", + sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + codec.CollValue[types.ValidatorAccumulatedCommission](cdc), + ), } schema, err := sb.Build() @@ -157,8 +166,8 @@ func (k Keeper) WithdrawDelegationRewards(ctx context.Context, delAddr sdk.AccAd // withdraw validator commission func (k Keeper) WithdrawValidatorCommission(ctx context.Context, valAddr sdk.ValAddress) (sdk.Coins, error) { // fetch validator accumulated commission - accumCommission, err := k.GetValidatorAccumulatedCommission(ctx, valAddr) - if err != nil { + accumCommission, err := k.ValidatorsAccumulatedCommission.Get(ctx, valAddr) + if err != nil && !errors.Is(err, collections.ErrNotFound) { return nil, err } @@ -167,8 +176,10 @@ func (k Keeper) WithdrawValidatorCommission(ctx context.Context, valAddr sdk.Val } commission, remainder := accumCommission.Commission.TruncateDecimal() - k.SetValidatorAccumulatedCommission(ctx, valAddr, types.ValidatorAccumulatedCommission{Commission: remainder}) // leave remainder to withdraw later - + err = k.ValidatorsAccumulatedCommission.Set(ctx, valAddr, types.ValidatorAccumulatedCommission{Commission: remainder}) // leave remainder to withdraw later + if err != nil { + return nil, err + } // update outstanding outstanding, err := k.GetValidatorOutstandingRewards(ctx, valAddr) if err != nil { diff --git a/x/distribution/keeper/keeper_test.go b/x/distribution/keeper/keeper_test.go index 21c2ed67ca..81dfa2a5f5 100644 --- a/x/distribution/keeper/keeper_test.go +++ b/x/distribution/keeper/keeper_test.go @@ -109,7 +109,7 @@ func TestWithdrawValidatorCommission(t *testing.T) { require.NoError(t, distrKeeper.SetValidatorOutstandingRewards(ctx, valAddr, types.ValidatorOutstandingRewards{Rewards: valCommission})) // set commission - require.NoError(t, distrKeeper.SetValidatorAccumulatedCommission(ctx, valAddr, types.ValidatorAccumulatedCommission{Commission: valCommission})) + require.NoError(t, distrKeeper.ValidatorsAccumulatedCommission.Set(ctx, valAddr, types.ValidatorAccumulatedCommission{Commission: valCommission})) // withdraw commission coins := sdk.NewCoins(sdk.NewCoin("mytoken", math.NewInt(1)), sdk.NewCoin("stake", math.NewInt(1))) @@ -120,7 +120,7 @@ func TestWithdrawValidatorCommission(t *testing.T) { require.NoError(t, err) // check remainder - remainderValCommission, err := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddr) + remainderValCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valAddr) require.NoError(t, err) remainder := remainderValCommission.Commission require.Equal(t, sdk.DecCoins{ diff --git a/x/distribution/keeper/store.go b/x/distribution/keeper/store.go index bc2e41773f..757830c1b7 100644 --- a/x/distribution/keeper/store.go +++ b/x/distribution/keeper/store.go @@ -129,67 +129,6 @@ func (k Keeper) GetValidatorHistoricalReferenceCount(ctx context.Context) (count return } -// get accumulated commission for a validator -func (k Keeper) GetValidatorAccumulatedCommission(ctx context.Context, val sdk.ValAddress) (commission types.ValidatorAccumulatedCommission, err error) { - store := k.storeService.OpenKVStore(ctx) - b, err := store.Get(types.GetValidatorAccumulatedCommissionKey(val)) - if err != nil { - return types.ValidatorAccumulatedCommission{}, err - } - - if b == nil { - return types.ValidatorAccumulatedCommission{}, nil - } - - err = k.cdc.Unmarshal(b, &commission) - if err != nil { - return types.ValidatorAccumulatedCommission{}, err - } - return -} - -// set accumulated commission for a validator -func (k Keeper) SetValidatorAccumulatedCommission(ctx context.Context, val sdk.ValAddress, commission types.ValidatorAccumulatedCommission) error { - var ( - bz []byte - err error - ) - - store := k.storeService.OpenKVStore(ctx) - if commission.Commission.IsZero() { - bz, err = k.cdc.Marshal(&types.ValidatorAccumulatedCommission{}) - } else { - bz, err = k.cdc.Marshal(&commission) - } - - if err != nil { - return err - } - - return store.Set(types.GetValidatorAccumulatedCommissionKey(val), bz) -} - -// delete accumulated commission for a validator -func (k Keeper) DeleteValidatorAccumulatedCommission(ctx context.Context, val sdk.ValAddress) error { - store := k.storeService.OpenKVStore(ctx) - return store.Delete(types.GetValidatorAccumulatedCommissionKey(val)) -} - -// iterate over accumulated commissions -func (k Keeper) IterateValidatorAccumulatedCommissions(ctx context.Context, handler func(val sdk.ValAddress, commission types.ValidatorAccumulatedCommission) (stop bool)) { - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - iter := storetypes.KVStorePrefixIterator(store, types.ValidatorAccumulatedCommissionPrefix) - defer iter.Close() - for ; iter.Valid(); iter.Next() { - var commission types.ValidatorAccumulatedCommission - k.cdc.MustUnmarshal(iter.Value(), &commission) - addr := types.GetValidatorAccumulatedCommissionAddress(iter.Key()) - if handler(addr, commission) { - break - } - } -} - // get validator outstanding rewards func (k Keeper) GetValidatorOutstandingRewards(ctx context.Context, val sdk.ValAddress) (rewards types.ValidatorOutstandingRewards, err error) { store := k.storeService.OpenKVStore(ctx) diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index 7354d8a7c7..eab8aeebab 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -29,7 +29,7 @@ func (k Keeper) initializeValidator(ctx context.Context, val stakingtypes.Valida } // set accumulated commission - err = k.SetValidatorAccumulatedCommission(ctx, val.GetOperator(), types.InitialValidatorAccumulatedCommission()) + err = k.ValidatorsAccumulatedCommission.Set(ctx, val.GetOperator(), types.InitialValidatorAccumulatedCommission()) if err != nil { return err } diff --git a/x/distribution/migrations/v2/store_test.go b/x/distribution/migrations/v2/store_test.go index 6c226e6aa3..9b8a081f5a 100644 --- a/x/distribution/migrations/v2/store_test.go +++ b/x/distribution/migrations/v2/store_test.go @@ -73,7 +73,7 @@ func TestStoreMigration(t *testing.T) { { "ValidatorAccumulatedCommission", v1.GetValidatorAccumulatedCommissionKey(valAddr), - types.GetValidatorAccumulatedCommissionKey(valAddr), + append(types.ValidatorAccumulatedCommissionPrefix, address.MustLengthPrefix(valAddr.Bytes())...), }, { "ValidatorSlashEvent", diff --git a/x/distribution/simulation/decoder_test.go b/x/distribution/simulation/decoder_test.go index 3054d2e7c7..6fb4d797ae 100644 --- a/x/distribution/simulation/decoder_test.go +++ b/x/distribution/simulation/decoder_test.go @@ -32,7 +32,6 @@ func TestDecodeDistributionStore(t *testing.T) { feePool := types.InitialFeePool() feePool.CommunityPool = decCoins outstanding := types.ValidatorOutstandingRewards{Rewards: decCoins} - commission := types.ValidatorAccumulatedCommission{Commission: decCoins} historicalRewards := types.NewValidatorHistoricalRewards(decCoins, 100) slashEvent := types.NewValidatorSlashEvent(10, math.LegacyOneDec()) @@ -42,7 +41,6 @@ func TestDecodeDistributionStore(t *testing.T) { {Key: types.ProposerKey, Value: consAddr1.Bytes()}, {Key: types.GetValidatorOutstandingRewardsKey(valAddr1), Value: cdc.MustMarshal(&outstanding)}, {Key: types.GetValidatorHistoricalRewardsKey(valAddr1, 100), Value: cdc.MustMarshal(&historicalRewards)}, - {Key: types.GetValidatorAccumulatedCommissionKey(valAddr1), Value: cdc.MustMarshal(&commission)}, {Key: types.GetValidatorSlashEventKeyPrefix(valAddr1, 13), Value: cdc.MustMarshal(&slashEvent)}, {Key: []byte{0x99}, Value: []byte{0x99}}, }, @@ -56,7 +54,6 @@ func TestDecodeDistributionStore(t *testing.T) { {"Proposer", fmt.Sprintf("%v\n%v", consAddr1, consAddr1)}, {"ValidatorOutstandingRewards", fmt.Sprintf("%v\n%v", outstanding, outstanding)}, {"ValidatorHistoricalRewards", fmt.Sprintf("%v\n%v", historicalRewards, historicalRewards)}, - {"ValidatorAccumulatedCommission", fmt.Sprintf("%v\n%v", commission, commission)}, {"ValidatorSlashEvent", fmt.Sprintf("%v\n%v", slashEvent, slashEvent)}, {"other", ""}, } diff --git a/x/distribution/simulation/operations.go b/x/distribution/simulation/operations.go index 8de9fe1ce5..29f12ecc31 100644 --- a/x/distribution/simulation/operations.go +++ b/x/distribution/simulation/operations.go @@ -4,6 +4,7 @@ import ( "fmt" "math/rand" + "cosmossdk.io/collections" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" @@ -13,6 +14,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/distribution/keeper" "github.com/cosmos/cosmos-sdk/x/distribution/types" "github.com/cosmos/cosmos-sdk/x/simulation" + "github.com/pkg/errors" ) // Simulation operation weights constants @@ -171,8 +173,8 @@ func SimulateMsgWithdrawValidatorCommission(txConfig client.TxConfig, ak types.A return simtypes.NoOpMsg(types.ModuleName, msgType, "random validator is not ok"), nil, nil } - commission, err := k.GetValidatorAccumulatedCommission(ctx, validator.GetOperator()) - if err != nil { + commission, err := k.ValidatorsAccumulatedCommission.Get(ctx, validator.GetOperator()) + if err != nil && !errors.Is(err, collections.ErrNotFound) { return simtypes.NoOpMsg(types.ModuleName, msgType, "error getting validator commission"), nil, err } diff --git a/x/distribution/simulation/operations_test.go b/x/distribution/simulation/operations_test.go index b3293e278a..843c0e9078 100644 --- a/x/distribution/simulation/operations_test.go +++ b/x/distribution/simulation/operations_test.go @@ -172,8 +172,8 @@ func (suite *SimTestSuite) testSimulateMsgWithdrawValidatorCommission(tokenName suite.distrKeeper.SetValidatorOutstandingRewards(suite.ctx, suite.genesisVals[0].GetOperator(), types.ValidatorOutstandingRewards{Rewards: valCommission}) // setup validator accumulated commission - suite.distrKeeper.SetValidatorAccumulatedCommission(suite.ctx, validator0.GetOperator(), types.ValidatorAccumulatedCommission{Commission: valCommission}) - suite.distrKeeper.SetValidatorAccumulatedCommission(suite.ctx, suite.genesisVals[0].GetOperator(), types.ValidatorAccumulatedCommission{Commission: valCommission}) + suite.Require().NoError(suite.distrKeeper.ValidatorsAccumulatedCommission.Set(suite.ctx, validator0.GetOperator(), types.ValidatorAccumulatedCommission{Commission: valCommission})) + suite.Require().NoError(suite.distrKeeper.ValidatorsAccumulatedCommission.Set(suite.ctx, suite.genesisVals[0].GetOperator(), types.ValidatorAccumulatedCommission{Commission: valCommission})) suite.app.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: suite.app.LastBlockHeight() + 1, diff --git a/x/distribution/types/keys.go b/x/distribution/types/keys.go index 17dc32b281..ea6c76f0db 100644 --- a/x/distribution/types/keys.go +++ b/x/distribution/types/keys.go @@ -50,9 +50,9 @@ var ( DelegatorWithdrawAddrPrefix = collections.NewPrefix(3) // key for delegator withdraw address DelegatorStartingInfoPrefix = collections.NewPrefix(4) // key for delegator starting info - ValidatorHistoricalRewardsPrefix = collections.NewPrefix(5) // key for historical validators rewards / stake - ValidatorCurrentRewardsPrefix = []byte{0x06} // key for current validator rewards - ValidatorAccumulatedCommissionPrefix = []byte{0x07} // key for accumulated validator commission + ValidatorHistoricalRewardsPrefix = []byte{0x05} // key for historical validators rewards / stake + ValidatorCurrentRewardsPrefix = collections.NewPrefix(6) // key for current validator rewards + ValidatorAccumulatedCommissionPrefix = collections.NewPrefix(7) // key for accumulated validator commission ValidatorSlashEventPrefix = []byte{0x08} // key for validator slash fraction ParamsKey = collections.NewPrefix(9) // key for distribution module params @@ -130,11 +130,6 @@ func GetValidatorHistoricalRewardsKey(v sdk.ValAddress, k uint64) []byte { return append(append(ValidatorHistoricalRewardsPrefix, address.MustLengthPrefix(v.Bytes())...), b...) } -// GetValidatorAccumulatedCommissionKey creates the key for a validator's current commission. -func GetValidatorAccumulatedCommissionKey(v sdk.ValAddress) []byte { - return append(ValidatorAccumulatedCommissionPrefix, address.MustLengthPrefix(v.Bytes())...) -} - // GetValidatorSlashEventPrefix creates the prefix key for a validator's slash fractions. func GetValidatorSlashEventPrefix(v sdk.ValAddress) []byte { return append(ValidatorSlashEventPrefix, address.MustLengthPrefix(v.Bytes())...)