From 8035b5b52416bae31fc85a4e2c6cc84f6547feba Mon Sep 17 00:00:00 2001 From: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:38:56 +0530 Subject: [PATCH] refactor(x/slashing): migrate `AddrPubkeyRelation` to collections (#17044) --- CHANGELOG.md | 3 ++ .../evidence/keeper/infraction_test.go | 2 +- .../slashing/keeper/keeper_test.go | 6 ++-- x/slashing/keeper/genesis.go | 2 +- x/slashing/keeper/hooks.go | 5 ++- x/slashing/keeper/keeper.go | 35 +++++-------------- x/slashing/keeper/keeper_test.go | 2 +- x/slashing/migrations/v2/store_test.go | 7 +++- x/slashing/simulation/decoder_test.go | 5 --- x/slashing/types/keys.go | 7 +--- 10 files changed, 27 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f86c30892447..3a0fdbe7edf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/slashing) [17044](https://github.com/cosmos/cosmos-sdk/pull/17044) Use collections for `AddrPubkeyRelation`: + * remove from `types`: `AddrPubkeyRelationKey` + * remove from `Keeper`: `AddPubkey` * (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `DelegationKey`: * remove from `types`: `GetDelegationKey`, `GetDelegationsKey` * (x/staking) [#17288](https://github.com/cosmos/cosmos-sdk/pull/17288) Use collections for `UnbondingIndex`: diff --git a/tests/integration/evidence/keeper/infraction_test.go b/tests/integration/evidence/keeper/infraction_test.go index e250bfb38248..3be456076a04 100644 --- a/tests/integration/evidence/keeper/infraction_test.go +++ b/tests/integration/evidence/keeper/infraction_test.go @@ -193,7 +193,7 @@ func TestHandleDoubleSign(t *testing.T) { assert.NilError(t, err) assert.DeepEqual(t, selfDelegation, val.GetBondedTokens()) - assert.NilError(t, f.slashingKeeper.AddPubkey(f.sdkCtx, valpubkey)) + assert.NilError(t, f.slashingKeeper.AddrPubkeyRelation.Set(f.sdkCtx, valpubkey.Address(), valpubkey)) info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(valpubkey.Address()), f.sdkCtx.BlockHeight(), int64(0), time.Unix(0, 0), false, int64(0)) err = f.slashingKeeper.ValidatorSigningInfo.Set(f.sdkCtx, sdk.ConsAddress(valpubkey.Address()), info) diff --git a/tests/integration/slashing/keeper/keeper_test.go b/tests/integration/slashing/keeper/keeper_test.go index fae0ff2f58f6..866b66b30e63 100644 --- a/tests/integration/slashing/keeper/keeper_test.go +++ b/tests/integration/slashing/keeper/keeper_test.go @@ -233,7 +233,7 @@ func TestHandleNewValidator(t *testing.T) { assert.NilError(t, err) f.ctx = f.ctx.WithBlockHeight(signedBlocksWindow + 1) - assert.NilError(t, f.slashingKeeper.AddPubkey(f.ctx, pks[0])) + assert.NilError(t, f.slashingKeeper.AddrPubkeyRelation.Set(f.ctx, pks[0].Address(), pks[0])) info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(valpubkey.Address()), f.ctx.BlockHeight(), int64(0), time.Unix(0, 0), false, int64(0)) assert.NilError(t, f.slashingKeeper.ValidatorSigningInfo.Set(f.ctx, sdk.ConsAddress(valpubkey.Address()), info)) @@ -287,7 +287,7 @@ func TestHandleAlreadyJailed(t *testing.T) { power := int64(100) tstaking := stakingtestutil.NewHelper(t, f.ctx, f.stakingKeeper) - err := f.slashingKeeper.AddPubkey(f.ctx, pks[0]) + err := f.slashingKeeper.AddrPubkeyRelation.Set(f.ctx, pks[0].Address(), pks[0]) assert.NilError(t, err) info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(val.Address()), f.ctx.BlockHeight(), int64(0), time.Unix(0, 0), false, int64(0)) @@ -362,7 +362,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { tstaking := stakingtestutil.NewHelper(t, f.ctx, f.stakingKeeper) valAddr := sdk.ValAddress(addr) - assert.NilError(t, f.slashingKeeper.AddPubkey(f.ctx, pks[0])) + assert.NilError(t, f.slashingKeeper.AddrPubkeyRelation.Set(f.ctx, pks[0].Address(), pks[0])) info := slashingtypes.NewValidatorSigningInfo(consAddr, f.ctx.BlockHeight(), int64(0), time.Unix(0, 0), false, int64(0)) assert.NilError(t, f.slashingKeeper.ValidatorSigningInfo.Set(f.ctx, consAddr, info)) diff --git a/x/slashing/keeper/genesis.go b/x/slashing/keeper/genesis.go index 60ea32eb71e9..71ac3fccae09 100644 --- a/x/slashing/keeper/genesis.go +++ b/x/slashing/keeper/genesis.go @@ -20,7 +20,7 @@ func (keeper Keeper) InitGenesis(ctx sdk.Context, stakingKeeper types.StakingKee panic(err) } - err = keeper.AddPubkey(ctx, consPk) + err = keeper.AddrPubkeyRelation.Set(ctx, consPk.Address(), consPk) if err != nil { panic(err) } diff --git a/x/slashing/keeper/hooks.go b/x/slashing/keeper/hooks.go index 8dadfb6488da..595ffbf78d72 100644 --- a/x/slashing/keeper/hooks.go +++ b/x/slashing/keeper/hooks.go @@ -46,12 +46,11 @@ func (h Hooks) AfterValidatorBonded(ctx context.Context, consAddr sdk.ConsAddres // AfterValidatorRemoved deletes the address-pubkey relation when a validator is removed, func (h Hooks) AfterValidatorRemoved(ctx context.Context, consAddr sdk.ConsAddress, _ sdk.ValAddress) error { - return h.k.deleteAddrPubkeyRelation(ctx, crypto.Address(consAddr)) + return h.k.AddrPubkeyRelation.Remove(ctx, crypto.Address(consAddr)) } // AfterValidatorCreated adds the address-pubkey relation when a validator is created. func (h Hooks) AfterValidatorCreated(ctx context.Context, valAddr sdk.ValAddress) error { - sdkCtx := sdk.UnwrapSDKContext(ctx) validator, err := h.k.sk.Validator(ctx, valAddr) if err != nil { return err @@ -62,7 +61,7 @@ func (h Hooks) AfterValidatorCreated(ctx context.Context, valAddr sdk.ValAddress return err } - return h.k.AddPubkey(sdkCtx, consPk) + return h.k.AddrPubkeyRelation.Set(ctx, consPk.Address(), consPk) } func (h Hooks) AfterValidatorBeginUnbonding(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { diff --git a/x/slashing/keeper/keeper.go b/x/slashing/keeper/keeper.go index 0c1182da9fea..f963d61c38f9 100644 --- a/x/slashing/keeper/keeper.go +++ b/x/slashing/keeper/keeper.go @@ -29,6 +29,7 @@ type Keeper struct { Schema collections.Schema Params collections.Item[types.Params] ValidatorSigningInfo collections.Map[sdk.ConsAddress, types.ValidatorSigningInfo] + AddrPubkeyRelation collections.Map[[]byte, cryptotypes.PubKey] } // NewKeeper creates a slashing keeper @@ -48,6 +49,13 @@ func NewKeeper(cdc codec.BinaryCodec, legacyAmino *codec.LegacyAmino, storeServi sdk.LengthPrefixedAddressKey(sdk.ConsAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility codec.CollValue[types.ValidatorSigningInfo](cdc), ), + AddrPubkeyRelation: collections.NewMap( + sb, + types.AddrPubkeyRelationKeyPrefix, + "addr_pubkey_relation", + sdk.LengthPrefixedBytesKey, // sdk.LengthPrefixedBytesKey is needed to retain state compatibility + codec.CollInterfaceValue[cryptotypes.PubKey](cdc), + ), } schema, err := sb.Build() @@ -69,29 +77,9 @@ func (k Keeper) Logger(ctx context.Context) log.Logger { return sdkCtx.Logger().With("module", "x/"+types.ModuleName) } -// AddPubkey sets a address-pubkey relation -func (k Keeper) AddPubkey(ctx context.Context, pubkey cryptotypes.PubKey) error { - bz, err := k.cdc.MarshalInterface(pubkey) - if err != nil { - return err - } - store := k.storeService.OpenKVStore(ctx) - key := types.AddrPubkeyRelationKey(pubkey.Address()) - return store.Set(key, bz) -} - // GetPubkey returns the pubkey from the adddress-pubkey relation func (k Keeper) GetPubkey(ctx context.Context, a cryptotypes.Address) (cryptotypes.PubKey, error) { - store := k.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.AddrPubkeyRelationKey(a)) - if err != nil { - return nil, err - } - if bz == nil { - return nil, fmt.Errorf("address %s not found", sdk.ConsAddress(a)) - } - var pk cryptotypes.PubKey - return pk, k.cdc.UnmarshalInterface(bz, &pk) + return k.AddrPubkeyRelation.Get(ctx, a) } // Slash attempts to slash a validator. The slash is delegated to the staking @@ -145,8 +133,3 @@ func (k Keeper) Jail(ctx context.Context, consAddr sdk.ConsAddress) error { ) return nil } - -func (k Keeper) deleteAddrPubkeyRelation(ctx context.Context, addr cryptotypes.Address) error { - store := k.storeService.OpenKVStore(ctx) - return store.Delete(types.AddrPubkeyRelationKey(addr)) -} diff --git a/x/slashing/keeper/keeper_test.go b/x/slashing/keeper/keeper_test.go index 65e6eaaa2ed6..e6ea23a78fab 100644 --- a/x/slashing/keeper/keeper_test.go +++ b/x/slashing/keeper/keeper_test.go @@ -74,7 +74,7 @@ func (s *KeeperTestSuite) TestPubkey() { require := s.Require() _, pubKey, addr := testdata.KeyTestPubAddr() - require.NoError(keeper.AddPubkey(ctx, pubKey)) + require.NoError(keeper.AddrPubkeyRelation.Set(ctx, pubKey.Address(), pubKey)) expectedPubKey, err := keeper.GetPubkey(ctx, addr.Bytes()) require.NoError(err) diff --git a/x/slashing/migrations/v2/store_test.go b/x/slashing/migrations/v2/store_test.go index 4b8794c42db6..26fcba9910e9 100644 --- a/x/slashing/migrations/v2/store_test.go +++ b/x/slashing/migrations/v2/store_test.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" v1 "github.com/cosmos/cosmos-sdk/x/slashing/migrations/v1" v2 "github.com/cosmos/cosmos-sdk/x/slashing/migrations/v2" "github.com/cosmos/cosmos-sdk/x/slashing/types" @@ -46,7 +47,7 @@ func TestStoreMigration(t *testing.T) { { "AddrPubkeyRelationKey", v1.AddrPubkeyRelationKey(consAddr), - types.AddrPubkeyRelationKey(consAddr), + addrPubkeyRelationKey(consAddr), }, } @@ -75,3 +76,7 @@ func TestStoreMigration(t *testing.T) { }) } } + +func addrPubkeyRelationKey(addr []byte) []byte { + return append(types.AddrPubkeyRelationKeyPrefix, address.MustLengthPrefix(addr)...) +} diff --git a/x/slashing/simulation/decoder_test.go b/x/slashing/simulation/decoder_test.go index c63dcbbd23cc..dc98a2d1eeda 100644 --- a/x/slashing/simulation/decoder_test.go +++ b/x/slashing/simulation/decoder_test.go @@ -18,7 +18,6 @@ import ( var ( delPk1 = ed25519.GenPrivKey().PubKey() - delAddr1 = sdk.AccAddress(delPk1.Address()) consAddr1 = sdk.ConsAddress(delPk1.Address().Bytes()) ) @@ -29,14 +28,11 @@ func TestDecodeStore(t *testing.T) { info := types.NewValidatorSigningInfo(consAddr1, 0, 1, time.Now().UTC(), false, 0) missed := []byte{1} // we want to display the bytes for simulation diffs - bz, err := cdc.MarshalInterface(delPk1) - require.NoError(t, err) kvPairs := kv.Pairs{ Pairs: []kv.Pair{ {Key: types.ValidatorSigningInfoKey(consAddr1), Value: cdc.MustMarshal(&info)}, {Key: types.ValidatorMissedBlockBitmapKey(consAddr1, 6), Value: missed}, - {Key: types.AddrPubkeyRelationKey(delAddr1), Value: bz}, {Key: []byte{0x99}, Value: []byte{0x99}}, // This test should panic }, } @@ -48,7 +44,6 @@ func TestDecodeStore(t *testing.T) { }{ {"ValidatorSigningInfo", fmt.Sprintf("%v\n%v", info, info), false}, {"ValidatorMissedBlockBitArray", fmt.Sprintf("missedA: %v\nmissedB: %v\n", missed, missed), false}, - {"AddrPubkeyRelation", fmt.Sprintf("PubKeyA: %s\nPubKeyB: %s", delPk1, delPk1), false}, {"other", "", true}, } for i, tt := range tests { diff --git a/x/slashing/types/keys.go b/x/slashing/types/keys.go index 7786bb4949e4..3894777d933b 100644 --- a/x/slashing/types/keys.go +++ b/x/slashing/types/keys.go @@ -52,7 +52,7 @@ var ( ParamsKey = collections.NewPrefix(0) // Prefix for params key ValidatorSigningInfoKeyPrefix = collections.NewPrefix(1) // Prefix for signing info ValidatorMissedBlockBitmapKeyPrefix = []byte{0x02} // Prefix for missed block bitmap - AddrPubkeyRelationKeyPrefix = []byte{0x03} // Prefix for address-pubkey relation + AddrPubkeyRelationKeyPrefix = collections.NewPrefix(3) // Prefix for address-pubkey relation ) // ValidatorSigningInfoKey - stored by *Consensus* address (not operator address) @@ -74,8 +74,3 @@ func ValidatorMissedBlockBitmapKey(v sdk.ConsAddress, chunkIndex int64) []byte { return append(ValidatorMissedBlockBitmapPrefixKey(v), bz...) } - -// AddrPubkeyRelationKey gets pubkey relation key used to get the pubkey from the address -func AddrPubkeyRelationKey(addr []byte) []byte { - return append(AddrPubkeyRelationKeyPrefix, address.MustLengthPrefix(addr)...) -}