From 60605de779f982ee29c34962b0233977460bdb6c Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Mon, 31 Jul 2023 10:41:22 +0200 Subject: [PATCH] refactor(x/distribution)!: migrate to collections (#17115) Co-authored-by: Julien Robert --- CHANGELOG.md | 7 + go.mod | 2 +- go.sum | 4 +- simapp/export.go | 5 +- .../distribution/keeper/grpc_query_test.go | 7 +- .../distribution/keeper/msg_server_test.go | 6 +- types/query/collections_pagination.go | 9 ++ x/distribution/abci.go | 2 +- x/distribution/keeper/delegation.go | 5 +- x/distribution/keeper/delegation_test.go | 32 +++- x/distribution/keeper/genesis.go | 32 +++- x/distribution/keeper/grpc_query.go | 29 ++-- x/distribution/keeper/hooks.go | 5 +- x/distribution/keeper/invariants.go | 28 +++- x/distribution/keeper/keeper.go | 10 ++ x/distribution/keeper/migrations.go | 5 + x/distribution/keeper/store.go | 138 ++---------------- x/distribution/keeper/validator.go | 10 +- x/distribution/migrations/v4/migrate.go | 77 ++++++++++ x/distribution/migrations/v4/migrate_test.go | 50 +++++++ x/distribution/module.go | 4 + x/distribution/types/keys.go | 6 +- 22 files changed, 296 insertions(+), 177 deletions(-) create mode 100644 x/distribution/migrations/v4/migrate.go create mode 100644 x/distribution/migrations/v4/migrate_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index fa0fe324d1..5137b980a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/distribution) [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Use collections for `PreviousProposer` and `ValidatorSlashEvents`: + * remove from `Keeper`: `GetPreviousProposerConsAddr`, `SetPreviousProposerConsAddr`, `GetValidatorHistoricalReferenceCount`, `GetValidatorSlashEvent`, `SetValidatorSlashEvent`. +* (x/slashing) [17063](https://github.com/cosmos/cosmos-sdk/pull/17063) Use collections for `HistoricalInfo`: * (x/feegrant) [16535](https://github.com/cosmos/cosmos-sdk/pull/16535) Use collections for `FeeAllowance`, `FeeAllowanceQueue`. * (x/staking) [17063](https://github.com/cosmos/cosmos-sdk/pull/17063) Use collections for `HistoricalInfo`: * remove `Keeper`: `GetHistoricalInfo`, `SetHistoricalInfo`, @@ -94,6 +97,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (server) [#17177](https://github.com/cosmos/cosmos-sdk/pull/17177) Remove `iavl-lazy-loading` configuration. * (rosetta) [#16276](https://github.com/cosmos/cosmos-sdk/issues/16276) Rosetta migration to standalone repo. +### State Machine Breaking + +* (x/distribution) [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Migrate `PreviousProposer` to collections. + ## [v0.50.0-beta.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-beta.0) - 2023-07-19 ### Features diff --git a/go.mod b/go.mod index a2b68d6203..7776d01bb8 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ module github.com/cosmos/cosmos-sdk require ( cosmossdk.io/api v0.7.0 - cosmossdk.io/collections v0.3.0 + cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 cosmossdk.io/core v0.9.0 cosmossdk.io/depinject v1.0.0-alpha.3 cosmossdk.io/errors v1.0.0 diff --git a/go.sum b/go.sum index a2d83570fc..bb4e5b5bfb 100644 --- a/go.sum +++ b/go.sum @@ -37,8 +37,8 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= cosmossdk.io/api v0.7.0 h1:QsEMIWuv9xWDbF2HZnW4Lpu1/SejCztPu0LQx7t6MN4= cosmossdk.io/api v0.7.0/go.mod h1:kJFAEMLN57y0viszHDPLMmieF0471o5QAwwApa+270M= -cosmossdk.io/collections v0.3.0 h1:v0eEqLBxebAV+t+Ahwf9tSJOu95HVLINwROXx2TTZ08= -cosmossdk.io/collections v0.3.0/go.mod h1:CHE1+niUElL9ikCpevRZcp0yqQ4TU0TrEEGirN0mvIg= +cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 h1:4XhcAIVBXPwCFTT9abOzZZaEadbRaVws8A6UTr2KGIE= +cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7/go.mod h1:+KJND4gZHilxE3meopEl/Uet6IAw3PyiSbgeOrFzAZE= cosmossdk.io/core v0.9.0 h1:30ScAOHDIUOCg1DKAwqkho9wuQJnu7GUrMcg0XLioic= cosmossdk.io/core v0.9.0/go.mod h1:NFgl5r41Q36+RixTvyrfsS6qQ65agCbZ1FTpnN7/G1Y= cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw= diff --git a/simapp/export.go b/simapp/export.go index 6f611a130e..55335c0a45 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -105,7 +105,10 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] } // clear validator slash events - app.DistrKeeper.DeleteAllValidatorSlashEvents(ctx) + err = app.DistrKeeper.ValidatorSlashEvents.Clear(ctx, nil) + if err != nil { + panic(err) + } // clear validator historical rewards err = app.DistrKeeper.ValidatorHistoricalRewards.Clear(ctx, nil) diff --git a/tests/integration/distribution/keeper/grpc_query_test.go b/tests/integration/distribution/keeper/grpc_query_test.go index cebe57eac7..d1c020883e 100644 --- a/tests/integration/distribution/keeper/grpc_query_test.go +++ b/tests/integration/distribution/keeper/grpc_query_test.go @@ -236,7 +236,12 @@ func TestGRPCValidatorSlashes(t *testing.T) { } for i, slash := range slashes { - assert.NilError(t, f.distrKeeper.SetValidatorSlashEvent(f.sdkCtx, f.valAddr, uint64(i+2), 0, slash)) + err := f.distrKeeper.ValidatorSlashEvents.Set( + f.sdkCtx, + collections.Join3(f.valAddr, uint64(i+2), uint64(0)), + slash, + ) + assert.NilError(t, err) } var ( diff --git a/tests/integration/distribution/keeper/msg_server_test.go b/tests/integration/distribution/keeper/msg_server_test.go index dcce82ad19..6c17aec04e 100644 --- a/tests/integration/distribution/keeper/msg_server_test.go +++ b/tests/integration/distribution/keeper/msg_server_test.go @@ -275,8 +275,8 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) { } height := f.app.LastBlockHeight() - _, err = f.distrKeeper.GetPreviousProposerConsAddr(f.sdkCtx) - assert.Error(t, err, "previous proposer not set") + _, err = f.distrKeeper.PreviousProposer.Get(f.sdkCtx) + assert.ErrorIs(t, err, collections.ErrNotFound) for _, tc := range testCases { tc := tc @@ -315,7 +315,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) { assert.DeepEqual(t, rewards, initOutstandingRewards.Sub(curOutstandingRewards.Rewards)) } - prevProposerConsAddr, err := f.distrKeeper.GetPreviousProposerConsAddr(f.sdkCtx) + prevProposerConsAddr, err := f.distrKeeper.PreviousProposer.Get(f.sdkCtx) assert.NilError(t, err) assert.Assert(t, prevProposerConsAddr.Empty() == false) assert.DeepEqual(t, prevProposerConsAddr, valConsAddr) diff --git a/types/query/collections_pagination.go b/types/query/collections_pagination.go index 47ca6d5fbb..d5d662081e 100644 --- a/types/query/collections_pagination.go +++ b/types/query/collections_pagination.go @@ -19,6 +19,15 @@ func WithCollectionPaginationPairPrefix[K1, K2 any](prefix K1) func(o *Collectio } } +// WithCollectionPaginationTriplePrefix applies a prefix to a collection, whose key is a collection.Triple, +// being paginated that needs prefixing. +func WithCollectionPaginationTriplePrefix[K1, K2, K3 any](prefix K1) func(o *CollectionsPaginateOptions[collections.Triple[K1, K2, K3]]) { + return func(o *CollectionsPaginateOptions[collections.Triple[K1, K2, K3]]) { + prefix := collections.TriplePrefix[K1, K2, K3](prefix) + o.Prefix = &prefix + } +} + // CollectionsPaginateOptions provides extra options for pagination in collections. type CollectionsPaginateOptions[K any] struct { // Prefix allows to optionally set a prefix for the pagination. diff --git a/x/distribution/abci.go b/x/distribution/abci.go index 2751cfab83..573881cfe0 100644 --- a/x/distribution/abci.go +++ b/x/distribution/abci.go @@ -30,5 +30,5 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) error { // record the proposer for when we payout on the next block consAddr := sdk.ConsAddress(ctx.BlockHeader().ProposerAddress) - return k.SetPreviousProposerConsAddr(ctx, consAddr) + return k.PreviousProposer.Set(ctx, consAddr) } diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 5b47505745..2f94605d0e 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -120,7 +120,7 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val stakingtypes // for them for the stake sanity check below. endingHeight := uint64(sdkCtx.BlockHeight()) if endingHeight > startingHeight { - k.IterateValidatorSlashEventsBetween(ctx, valAddr, startingHeight, endingHeight, + err = k.IterateValidatorSlashEventsBetween(ctx, valAddr, startingHeight, endingHeight, func(height uint64, event types.ValidatorSlashEvent) (stop bool) { endingPeriod := event.ValidatorPeriod if endingPeriod > startingPeriod { @@ -138,6 +138,9 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val stakingtypes return false }, ) + if err != nil { + return sdk.DecCoins{}, err + } } // A total stake sanity check; Recalculated final stake should be less than or diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index 6155ba7175..dd2c30444f 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -1,12 +1,14 @@ package keeper_test import ( + "errors" "testing" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "cosmossdk.io/collections" "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" @@ -72,13 +74,13 @@ func TestCalculateRewardsBasic(t *testing.T) { ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) // historical count should be 2 (once for validator init, once for delegation init) - require.Equal(t, uint64(2), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 2, getValHistoricalReferenceCount(distrKeeper, ctx)) // end period endingPeriod, _ := distrKeeper.IncrementValidatorPeriod(ctx, val) // historical count should be 2 still - require.Equal(t, uint64(2), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 2, getValHistoricalReferenceCount(distrKeeper, ctx)) // calculate delegation rewards rewards, err := distrKeeper.CalculateDelegationRewards(ctx, val, del, endingPeriod) @@ -108,6 +110,22 @@ func TestCalculateRewardsBasic(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(initial / 2)}}, valCommission.Commission) } +func getValHistoricalReferenceCount(k keeper.Keeper, ctx sdk.Context) int { + count := 0 + err := k.ValidatorHistoricalRewards.Walk( + ctx, nil, func(key collections.Pair[sdk.ValAddress, uint64], rewards disttypes.ValidatorHistoricalRewards) (stop bool, err error) { + count += int(rewards.ReferenceCount) + return false, nil + }, + ) + + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } + + return count +} + func TestCalculateRewardsAfterSlash(t *testing.T) { ctrl := gomock.NewController(t) key := storetypes.NewKVStoreKey(disttypes.StoreKey) @@ -486,7 +504,7 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { require.NoError(t, distrKeeper.AllocateTokensToValidator(ctx, val, tokens)) // historical count should be 2 (initial + latest for delegation) - require.Equal(t, uint64(2), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 2, getValHistoricalReferenceCount(distrKeeper, ctx)) // withdraw rewards (the bank keeper should be called with the right amount of tokens to transfer) expRewards := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, initial.QuoRaw(2))} @@ -495,7 +513,7 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { require.Nil(t, err) // historical count should still be 2 (added one record, cleared one) - require.Equal(t, uint64(2), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 2, getValHistoricalReferenceCount(distrKeeper, ctx)) // withdraw commission (the bank keeper should be called with the right amount of tokens to transfer) expCommission := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, initial.QuoRaw(2))} @@ -808,7 +826,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.NoError(t, distrKeeper.AllocateTokensToValidator(ctx, val, tokens)) // historical count should be 2 (validator init, delegation init) - require.Equal(t, uint64(2), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 2, getValHistoricalReferenceCount(distrKeeper, ctx)) // second delegation _, del2, err := distrtestutil.Delegate( @@ -830,7 +848,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.NoError(t, err) // historical count should be 3 (second delegation init) - require.Equal(t, uint64(3), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 3, getValHistoricalReferenceCount(distrKeeper, ctx)) // next block ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) @@ -851,7 +869,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.NoError(t, err) // historical count should be 3 (validator init + two delegations) - require.Equal(t, uint64(3), distrKeeper.GetValidatorHistoricalReferenceCount(ctx)) + require.Equal(t, 3, getValHistoricalReferenceCount(distrKeeper, ctx)) // validator withdraws commission expCommission := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(initial))} diff --git a/x/distribution/keeper/genesis.go b/x/distribution/keeper/genesis.go index 3c55c1fb9d..cd1922ea3a 100644 --- a/x/distribution/keeper/genesis.go +++ b/x/distribution/keeper/genesis.go @@ -47,7 +47,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data types.GenesisState) { } } - if err = k.SetPreviousProposerConsAddr(ctx, previousProposer); err != nil { + if err = k.PreviousProposer.Set(ctx, previousProposer); err != nil { panic(err) } @@ -112,7 +112,17 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data types.GenesisState) { if err != nil { panic(err) } - err = k.SetValidatorSlashEvent(ctx, valAddr, evt.Height, evt.Period, evt.ValidatorSlashEvent) + + err = k.ValidatorSlashEvents.Set( + ctx, + collections.Join3( + sdk.ValAddress(valAddr), + evt.Height, + evt.Period, + ), + evt.ValidatorSlashEvent, + ) + if err != nil { panic(err) } @@ -160,7 +170,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { panic(err) } - pp, err := k.GetPreviousProposerConsAddr(ctx) + pp, err := k.PreviousProposer.Get(ctx) if err != nil { panic(err) } @@ -234,17 +244,23 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { } slashes := make([]types.ValidatorSlashEventRecord, 0) - k.IterateValidatorSlashEvents(ctx, - func(val sdk.ValAddress, height uint64, event types.ValidatorSlashEvent) (stop bool) { + err = k.ValidatorSlashEvents.Walk( + ctx, + nil, + func(k collections.Triple[sdk.ValAddress, uint64, uint64], event types.ValidatorSlashEvent) (stop bool, err error) { slashes = append(slashes, types.ValidatorSlashEventRecord{ - ValidatorAddress: val.String(), - Height: height, + ValidatorAddress: k.K1().String(), + Height: k.K2(), Period: event.ValidatorPeriod, ValidatorSlashEvent: event, }) - return false + return false, nil }, ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } + return types.NewGenesisState(params, feePool, dwi, pp, outstanding, acc, his, cur, dels, slashes) } diff --git a/x/distribution/keeper/grpc_query.go b/x/distribution/keeper/grpc_query.go index 371713e7fc..4e8cbce9d7 100644 --- a/x/distribution/keeper/grpc_query.go +++ b/x/distribution/keeper/grpc_query.go @@ -8,9 +8,7 @@ import ( "cosmossdk.io/collections" "cosmossdk.io/errors" - "cosmossdk.io/store/prefix" - "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -178,28 +176,21 @@ func (k Querier) ValidatorSlashes(ctx context.Context, req *types.QueryValidator return nil, status.Errorf(codes.InvalidArgument, "invalid validator address") } - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - slashesStore := prefix.NewStore(store, types.GetValidatorSlashEventPrefix(valAddr)) - - events, pageRes, err := query.GenericFilteredPaginate(k.cdc, slashesStore, req.Pagination, func(key []byte, result *types.ValidatorSlashEvent) (*types.ValidatorSlashEvent, error) { - if result.ValidatorPeriod < req.StartingHeight || result.ValidatorPeriod > req.EndingHeight { - return nil, nil + events, pageRes, err := query.CollectionFilteredPaginate(ctx, k.ValidatorSlashEvents, req.Pagination, func(key collections.Triple[sdk.ValAddress, uint64, uint64], ev types.ValidatorSlashEvent) (include bool, err error) { + if ev.ValidatorPeriod < req.StartingHeight || ev.ValidatorPeriod > req.EndingHeight { + return false, nil } - - return result, nil - }, func() *types.ValidatorSlashEvent { - return &types.ValidatorSlashEvent{} - }) + return true, nil + }, func(_ collections.Triple[sdk.ValAddress, uint64, uint64], value types.ValidatorSlashEvent) (types.ValidatorSlashEvent, error) { + return value, nil + }, + query.WithCollectionPaginationTriplePrefix[sdk.ValAddress, uint64, uint64](valAddr), + ) if err != nil { return nil, err } - slashes := []types.ValidatorSlashEvent{} - for _, event := range events { - slashes = append(slashes, *event) - } - - return &types.QueryValidatorSlashesResponse{Slashes: slashes, Pagination: pageRes}, nil + return &types.QueryValidatorSlashesResponse{Slashes: events, Pagination: pageRes}, nil } // DelegationRewards the total rewards accrued by a delegation diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 97b3db9e56..61d5834c92 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -109,7 +109,10 @@ func (h Hooks) AfterValidatorRemoved(ctx context.Context, _ sdk.ConsAddress, val } // clear slashes - h.k.DeleteValidatorSlashEvents(ctx, valAddr) + err = h.k.ValidatorSlashEvents.Clear(ctx, collections.NewPrefixedTripleRange[sdk.ValAddress, uint64, uint64](valAddr)) + if err != nil { + return err + } // clear historical rewards err = h.k.ValidatorHistoricalRewards.Clear(ctx, collections.NewPrefixedPairRange[sdk.ValAddress, uint64](valAddr)) diff --git a/x/distribution/keeper/invariants.go b/x/distribution/keeper/invariants.go index 0b403baa41..5ffd49d068 100644 --- a/x/distribution/keeper/invariants.go +++ b/x/distribution/keeper/invariants.go @@ -143,16 +143,34 @@ func ReferenceCountInvariant(k Keeper) sdk.Invariant { } slashCount := uint64(0) - k.IterateValidatorSlashEvents(ctx, - func(_ sdk.ValAddress, _ uint64, _ types.ValidatorSlashEvent) (stop bool) { + err = k.ValidatorSlashEvents.Walk( + ctx, + nil, + func(k collections.Triple[sdk.ValAddress, uint64, uint64], event types.ValidatorSlashEvent) (stop bool, err error) { slashCount++ - return false - }) + return false, nil + }, + ) + + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } // one record per validator (last tracked period), one record per // delegation (previous period), one record per slash (previous period) expected := valCount + uint64(len(dels)) + slashCount - count := k.GetValidatorHistoricalReferenceCount(ctx) + count := uint64(0) + err = k.ValidatorHistoricalRewards.Walk( + ctx, nil, func(key collections.Pair[sdk.ValAddress, uint64], rewards types.ValidatorHistoricalRewards) (stop bool, err error) { + count += uint64(rewards.ReferenceCount) + return false, nil + }, + ) + + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } + broken := count != expected return sdk.FormatInvariant(types.ModuleName, "reference count", diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index a27ddaceb7..6f3afcd321 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -38,6 +38,8 @@ type Keeper struct { ValidatorsAccumulatedCommission collections.Map[sdk.ValAddress, types.ValidatorAccumulatedCommission] ValidatorOutstandingRewards collections.Map[sdk.ValAddress, types.ValidatorOutstandingRewards] ValidatorHistoricalRewards collections.Map[collections.Pair[sdk.ValAddress, uint64], types.ValidatorHistoricalRewards] + PreviousProposer collections.Item[sdk.ConsAddress] + ValidatorSlashEvents collections.Map[collections.Triple[sdk.ValAddress, uint64, uint64], types.ValidatorSlashEvent] // key is valAddr, height, period feeCollectorName string // name of the FeeCollector ModuleAccount } @@ -107,6 +109,14 @@ func NewKeeper( collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), sdk.LEUint64Key), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility codec.CollValue[types.ValidatorHistoricalRewards](cdc), ), + PreviousProposer: collections.NewItem(sb, types.ProposerKey, "previous_proposer", collcodec.KeyToValueCodec(sdk.ConsAddressKey)), + ValidatorSlashEvents: collections.NewMap( + sb, + types.ValidatorSlashEventPrefix, + "validator_slash_events", + collections.TripleKeyCodec(sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), collections.Uint64Key, collections.Uint64Key), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + codec.CollValue[types.ValidatorSlashEvent](cdc), + ), } schema, err := sb.Build() diff --git a/x/distribution/keeper/migrations.go b/x/distribution/keeper/migrations.go index bfcf8805e3..8e8944cdea 100644 --- a/x/distribution/keeper/migrations.go +++ b/x/distribution/keeper/migrations.go @@ -5,6 +5,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/distribution/exported" v2 "github.com/cosmos/cosmos-sdk/x/distribution/migrations/v2" v3 "github.com/cosmos/cosmos-sdk/x/distribution/migrations/v3" + v4 "github.com/cosmos/cosmos-sdk/x/distribution/migrations/v4" ) // Migrator is a struct for handling in-place store migrations. @@ -30,3 +31,7 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { func (m Migrator) Migrate2to3(ctx sdk.Context) error { return v3.MigrateStore(ctx, m.keeper.storeService, m.legacySubspace, m.keeper.cdc) } + +func (m Migrator) Migrate3to4(ctx sdk.Context) error { + return v4.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc) +} diff --git a/x/distribution/keeper/store.go b/x/distribution/keeper/store.go index 783a14364d..27f74ac16d 100644 --- a/x/distribution/keeper/store.go +++ b/x/distribution/keeper/store.go @@ -3,13 +3,10 @@ package keeper import ( "context" "errors" - - gogotypes "github.com/cosmos/gogoproto/types" + "math" "cosmossdk.io/collections" - storetypes "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" ) @@ -23,130 +20,25 @@ func (k Keeper) GetDelegatorWithdrawAddr(ctx context.Context, delAddr sdk.AccAdd return addr, err } -// GetPreviousProposerConsAddr returns the proposer consensus address for the -// current block. -func (k Keeper) GetPreviousProposerConsAddr(ctx context.Context) (sdk.ConsAddress, error) { - store := k.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.ProposerKey) - if err != nil { - return nil, err - } - - if bz == nil { - return nil, errors.New("previous proposer not set") - } - - addrValue := gogotypes.BytesValue{} - err = k.cdc.Unmarshal(bz, &addrValue) - if err != nil { - return nil, err - } - - return addrValue.GetValue(), nil -} - -// set the proposer public key for this block -func (k Keeper) SetPreviousProposerConsAddr(ctx context.Context, consAddr sdk.ConsAddress) error { - store := k.storeService.OpenKVStore(ctx) - bz := k.cdc.MustMarshal(&gogotypes.BytesValue{Value: consAddr}) - return store.Set(types.ProposerKey, bz) -} - -// historical reference count (used for testcases) -func (k Keeper) GetValidatorHistoricalReferenceCount(ctx context.Context) (count uint64) { - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - iter := storetypes.KVStorePrefixIterator(store, types.ValidatorHistoricalRewardsPrefix) - defer iter.Close() - for ; iter.Valid(); iter.Next() { - var rewards types.ValidatorHistoricalRewards - k.cdc.MustUnmarshal(iter.Value(), &rewards) - count += uint64(rewards.ReferenceCount) - } - return -} - -// get slash event for height -func (k Keeper) GetValidatorSlashEvent(ctx context.Context, val sdk.ValAddress, height, period uint64) (event types.ValidatorSlashEvent, found bool, err error) { - store := k.storeService.OpenKVStore(ctx) - b, err := store.Get(types.GetValidatorSlashEventKey(val, height, period)) - if err != nil { - return types.ValidatorSlashEvent{}, false, err - } - - if b == nil { - return types.ValidatorSlashEvent{}, false, nil - } - - err = k.cdc.Unmarshal(b, &event) - if err != nil { - return types.ValidatorSlashEvent{}, false, err - } - - return event, true, nil -} - -// set slash event for height -func (k Keeper) SetValidatorSlashEvent(ctx context.Context, val sdk.ValAddress, height, period uint64, event types.ValidatorSlashEvent) error { - store := k.storeService.OpenKVStore(ctx) - b, err := k.cdc.Marshal(&event) - if err != nil { - return err - } - - return store.Set(types.GetValidatorSlashEventKey(val, height, period), b) -} - // iterate over slash events between heights, inclusive func (k Keeper) IterateValidatorSlashEventsBetween(ctx context.Context, val sdk.ValAddress, startingHeight, endingHeight uint64, handler func(height uint64, event types.ValidatorSlashEvent) (stop bool), -) { - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - iter := store.Iterator( - types.GetValidatorSlashEventKeyPrefix(val, startingHeight), - types.GetValidatorSlashEventKeyPrefix(val, endingHeight+1), - ) - defer iter.Close() - for ; iter.Valid(); iter.Next() { - var event types.ValidatorSlashEvent - k.cdc.MustUnmarshal(iter.Value(), &event) - _, height := types.GetValidatorSlashEventAddressHeight(iter.Key()) - if handler(height, event) { - break +) error { + rng := new(collections.Range[collections.Triple[sdk.ValAddress, uint64, uint64]]). + StartInclusive(collections.Join3(val, startingHeight, uint64(0))). + EndExclusive(collections.Join3(val, endingHeight+1, uint64(math.MaxUint64))) + + err := k.ValidatorSlashEvents.Walk(ctx, rng, func(k collections.Triple[sdk.ValAddress, uint64, uint64], ev types.ValidatorSlashEvent) (stop bool, err error) { + height := k.K2() + if handler(height, ev) { + return true, nil } - } -} + return false, nil + }) -// iterate over all slash events -func (k Keeper) IterateValidatorSlashEvents(ctx context.Context, handler func(val sdk.ValAddress, height uint64, event types.ValidatorSlashEvent) (stop bool)) { - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - iter := storetypes.KVStorePrefixIterator(store, types.ValidatorSlashEventPrefix) - defer iter.Close() - for ; iter.Valid(); iter.Next() { - var event types.ValidatorSlashEvent - k.cdc.MustUnmarshal(iter.Value(), &event) - val, height := types.GetValidatorSlashEventAddressHeight(iter.Key()) - if handler(val, height, event) { - break - } + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return err } -} -// delete slash events for a particular validator -func (k Keeper) DeleteValidatorSlashEvents(ctx context.Context, val sdk.ValAddress) { - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - iter := storetypes.KVStorePrefixIterator(store, types.GetValidatorSlashEventPrefix(val)) - defer iter.Close() - for ; iter.Valid(); iter.Next() { - store.Delete(iter.Key()) - } -} - -// delete all slash events -func (k Keeper) DeleteAllValidatorSlashEvents(ctx context.Context) { - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - iter := storetypes.KVStorePrefixIterator(store, types.ValidatorSlashEventPrefix) - defer iter.Close() - for ; iter.Valid(); iter.Next() { - store.Delete(iter.Key()) - } + return nil } diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index 08c0a2e1f2..af60ce7b8d 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -167,5 +167,13 @@ func (k Keeper) updateValidatorSlashFraction(ctx context.Context, valAddr sdk.Va slashEvent := types.NewValidatorSlashEvent(newPeriod, fraction) height := uint64(sdkCtx.BlockHeight()) - return k.SetValidatorSlashEvent(ctx, valAddr, height, newPeriod, slashEvent) + return k.ValidatorSlashEvents.Set( + ctx, + collections.Join3[sdk.ValAddress, uint64, uint64]( + valAddr, + height, + newPeriod, + ), + slashEvent, + ) } diff --git a/x/distribution/migrations/v4/migrate.go b/x/distribution/migrations/v4/migrate.go new file mode 100644 index 0000000000..d5abfe43d2 --- /dev/null +++ b/x/distribution/migrations/v4/migrate.go @@ -0,0 +1,77 @@ +package v4 + +import ( + "context" + "errors" + + gogotypes "github.com/cosmos/gogoproto/types" + + "cosmossdk.io/collections" + collcodec "cosmossdk.io/collections/codec" + "cosmossdk.io/core/store" + + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +var ( + OldProposerKey = []byte{0x01} + NewProposerKey = collections.NewPrefix(1) +) + +func MigrateStore(ctx sdk.Context, storeService store.KVStoreService, cdc codec.BinaryCodec) error { + store := storeService.OpenKVStore(ctx) + bz, err := store.Get(OldProposerKey) + if err != nil { + return err + } + + if bz == nil { + // previous proposer not set, nothing to do + return nil + } + + addrValue := gogotypes.BytesValue{} + err = cdc.Unmarshal(bz, &addrValue) + if err != nil { + return err + } + + sb := collections.NewSchemaBuilder(storeService) + prevProposer := collections.NewItem(sb, NewProposerKey, "previous_proposer", collcodec.KeyToValueCodec(sdk.ConsAddressKey)) + _, err = sb.Build() + if err != nil { + return err + } + + return prevProposer.Set(ctx, addrValue.GetValue()) +} + +// GetPreviousProposerConsAddr returns the proposer consensus address for the +// current block. +func GetPreviousProposerConsAddr(ctx context.Context, storeService store.KVStoreService, cdc codec.BinaryCodec) (sdk.ConsAddress, error) { + store := storeService.OpenKVStore(ctx) + bz, err := store.Get(OldProposerKey) + if err != nil { + return nil, err + } + + if bz == nil { + return nil, errors.New("previous proposer not set") + } + + addrValue := gogotypes.BytesValue{} + err = cdc.Unmarshal(bz, &addrValue) + if err != nil { + return nil, err + } + + return addrValue.GetValue(), nil +} + +// set the proposer public key for this block +func SetPreviousProposerConsAddr(ctx context.Context, storeService store.KVStoreService, cdc codec.BinaryCodec, consAddr sdk.ConsAddress) error { + store := storeService.OpenKVStore(ctx) + bz := cdc.MustMarshal(&gogotypes.BytesValue{Value: consAddr}) + return store.Set(OldProposerKey, bz) +} diff --git a/x/distribution/migrations/v4/migrate_test.go b/x/distribution/migrations/v4/migrate_test.go new file mode 100644 index 0000000000..ed5ed0de3a --- /dev/null +++ b/x/distribution/migrations/v4/migrate_test.go @@ -0,0 +1,50 @@ +package v4_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "cosmossdk.io/collections" + collcodec "cosmossdk.io/collections/codec" + storetypes "cosmossdk.io/store/types" + + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/runtime" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + "github.com/cosmos/cosmos-sdk/x/distribution" + v4 "github.com/cosmos/cosmos-sdk/x/distribution/migrations/v4" +) + +func TestMigration(t *testing.T) { + cdc := moduletestutil.MakeTestEncodingConfig(distribution.AppModuleBasic{}).Codec + storeKey := storetypes.NewKVStoreKey("distribution") + storeService := runtime.NewKVStoreService(storeKey) + tKey := storetypes.NewTransientStoreKey("transient_test") + ctx := testutil.DefaultContext(storeKey, tKey) + + addr1 := secp256k1.GenPrivKey().PubKey().Address() + consAddr1 := sdk.ConsAddress(addr1) + + err := v4.SetPreviousProposerConsAddr(ctx, storeService, cdc, consAddr1) + require.NoError(t, err) + + gotAddr, err := v4.GetPreviousProposerConsAddr(ctx, storeService, cdc) + require.NoError(t, err) + require.Equal(t, consAddr1, gotAddr) + + err = v4.MigrateStore(ctx, storeService, cdc) + require.NoError(t, err) + + sb := collections.NewSchemaBuilder(storeService) + prevProposer := collections.NewItem(sb, v4.NewProposerKey, "previous_proposer", collcodec.KeyToValueCodec(sdk.ConsAddressKey)) + _, err = sb.Build() + require.NoError(t, err) + + newAddr, err := prevProposer.Get(ctx) + require.NoError(t, err) + + require.Equal(t, consAddr1, newAddr) +} diff --git a/x/distribution/module.go b/x/distribution/module.go index 3cf9dfdfc4..778fbcf142 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -149,6 +149,10 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { if err := cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3); err != nil { panic(fmt.Sprintf("failed to migrate x/%s from version 2 to 3: %v", types.ModuleName, err)) } + + if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4); err != nil { + panic(fmt.Sprintf("failed to migrate x/%s from version 3 to 4: %v", types.ModuleName, err)) + } } // InitGenesis performs genesis initialization for the distribution module. It returns diff --git a/x/distribution/types/keys.go b/x/distribution/types/keys.go index 87d80c23e1..8c44743f5d 100644 --- a/x/distribution/types/keys.go +++ b/x/distribution/types/keys.go @@ -43,19 +43,19 @@ const ( // // - 0x07: ValidatorCurrentCommission // -// - 0x08: ValidatorSlashEvent +// - 0x08: ValidatorSlashEvent // // - 0x09: Params var ( FeePoolKey = collections.NewPrefix(0) // key for global distribution state - ProposerKey = []byte{0x01} // key for the proposer operator address + ProposerKey = collections.NewPrefix(1) // key for the proposer operator address ValidatorOutstandingRewardsPrefix = collections.NewPrefix(2) // key for outstanding rewards 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 = 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 + ValidatorSlashEventPrefix = collections.NewPrefix(8) // key for validator slash fraction ParamsKey = collections.NewPrefix(9) // key for distribution module params )