refactor(x/slashing): audit QA (backport #21477) (#21501)

Co-authored-by: Akhil Kumar P <[email protected]>

CHANGELOG.md

@@ -152,7 +152,6 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (x/gov/testutil) [#17986](https://github.com/cosmos/cosmos-sdk/pull/18036) `MsgDeposit` has been removed because of AutoCLI migration.
 * (x/staking/testutil) [#17986](https://github.com/cosmos/cosmos-sdk/pull/17986) `MsgRedelegateExec`, `MsgUnbondExec` has been removed because of AutoCLI migration.
 * (x/group) [#17937](https://github.com/cosmos/cosmos-sdk/pull/17937) Groups module was moved to its own go.mod `cosmossdk.io/x/group`
-* (x/slashing) [#18115](https://github.com/cosmos/cosmos-sdk/pull/18115) `NewValidatorSigningInfo` takes strings instead of `sdk.AccAddress`
 * (x/gov) [#18197](https://github.com/cosmos/cosmos-sdk/pull/18197) Gov module was moved to its own go.mod `cosmossdk.io/x/gov`
 * (x/distribution) [#18199](https://github.com/cosmos/cosmos-sdk/pull/18199) Distribution module was moved to its own go.mod `cosmossdk.io/x/distribution`
 * (x/slashing) [#18201](https://github.com/cosmos/cosmos-sdk/pull/18201) Slashing module was moved to its own go.mod `cosmossdk.io/x/slashing`

x/slashing/CHANGELOG.md

@@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [#20026](https://github.com/cosmos/cosmos-sdk/pull/20026) Removal of the Address.String() method and related changes:
     * `Migrate` now takes a `ValidatorAddressCodec` as argument.
     * `Migrator` has a new field of `ValidatorAddressCodec` type.
+* [#18115](https://github.com/cosmos/cosmos-sdk/pull/18115) `NewValidatorSigningInfo` takes strings instead of `sdk.AccAddress`.
 * [#16441](https://github.com/cosmos/cosmos-sdk/pull/16441) Params state is migrated to collections. `GetParams` has been removed.
 * [#17023](https://github.com/cosmos/cosmos-sdk/pull/17023) Use collections for `ValidatorSigningInfo`:
     * remove `Keeper`: `SetValidatorSigningInfo`, `GetValidatorSigningInfo`, `IterateValidatorSigningInfos`

x/slashing/README.md

@@ -148,7 +148,7 @@ https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/slashing/proto/cosmo
 
 ### Params
 
-The slashing module stores it's params in state with the prefix of `0x00`,
+The slashing module stores its params in state with the prefix of `0x00`,
 it can be updated with governance or the address with authority.
 
 * Params: `0x00 | ProtocolBuffer(Params)`

x/slashing/keeper/hooks.go

@@ -98,7 +98,7 @@ func (h Hooks) AfterUnbondingInitiated(_ context.Context, _ uint64) error {
 	return nil
 }
 
-// AfterConsensusPubKeyUpdate triggers the functions to rotate the signing-infos also sets address pubkey relation.
+// AfterConsensusPubKeyUpdate handles the rotation of signing info and updates the address-pubkey relation after a consensus key update.
 func (h Hooks) AfterConsensusPubKeyUpdate(ctx context.Context, oldPubKey, newPubKey cryptotypes.PubKey, _ sdk.Coin) error {
 	if err := h.k.performConsensusPubKeyUpdate(ctx, oldPubKey, newPubKey); err != nil {
 		return err

x/slashing/keeper/infractions.go

@@ -13,7 +13,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-// HandleValidatorSignature handles a validator signature, must be called once per validator per block.
+// HandleValidatorSignature handles a validator signature, must be called once per validator for each block.
 func (k Keeper) HandleValidatorSignature(ctx context.Context, addr cryptotypes.Address, power int64, signed comet.BlockIDFlag) error {
 	params, err := k.Params.Get(ctx)
 	if err != nil {
@@ -22,6 +22,7 @@ func (k Keeper) HandleValidatorSignature(ctx context.Context, addr cryptotypes.A
 	return k.HandleValidatorSignatureWithParams(ctx, params, addr, power, signed)
 }
 
+// HandleValidatorSignature handles a validator signature with the provided slashing module params.
 func (k Keeper) HandleValidatorSignatureWithParams(ctx context.Context, params types.Params, addr cryptotypes.Address, power int64, signed comet.BlockIDFlag) error {
 	height := k.HeaderService.HeaderInfo(ctx).Height
 
@@ -38,7 +39,7 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx context.Context, params t
 		return nil
 	}
 
-	// read the cons address again because validator may've rotated it's key
+	// read the cons address again because validator may've rotated its key
 	valConsAddr, err := val.GetConsAddr()
 	if err != nil {
 		return err

x/slashing/keeper/signing_info.go

@@ -83,8 +83,8 @@ func (k Keeper) SetMissedBlockBitmapChunk(ctx context.Context, addr sdk.ConsAddr
 	return k.ValidatorMissedBlockBitmap.Set(ctx, collections.Join(addr.Bytes(), uint64(chunkIndex)), chunk)
 }
 
-// getPreviousConsKey checks if the key rotated, returns the old consKey to get the missed blocks
-// because missed blocks are still pointing to the old key
+// getPreviousConsKey returns the old consensus key if it has rotated,
+// allowing retrieval of missed blocks associated with the old key.
 func (k Keeper) getPreviousConsKey(ctx context.Context, addr sdk.ConsAddress) (sdk.ConsAddress, error) {
 	oldPk, err := k.sk.ValidatorIdentifier(ctx, addr)
 	if err != nil {
@@ -105,8 +105,7 @@ func (k Keeper) getPreviousConsKey(ctx context.Context, addr sdk.ConsAddress) (s
 // IndexOffset modulo SignedBlocksWindow. This index is used to fetch the chunk
 // in the bitmap and the relative bit in that chunk.
 func (k Keeper) GetMissedBlockBitmapValue(ctx context.Context, addr sdk.ConsAddress, index int64) (bool, error) {
-	// check the key rotated, if rotated use the returned consKey to get the missed blocks
-	// because missed blocks are still pointing to the old key
+	// get the old consensus key if it has rotated, allowing retrieval of missed blocks associated with the old key
 	addr, err := k.getPreviousConsKey(ctx, addr)
 	if err != nil {
 		return false, err
@@ -141,8 +140,7 @@ func (k Keeper) GetMissedBlockBitmapValue(ctx context.Context, addr sdk.ConsAddr
 // index is used to fetch the chunk in the bitmap and the relative bit in that
 // chunk.
 func (k Keeper) SetMissedBlockBitmapValue(ctx context.Context, addr sdk.ConsAddress, index int64, missed bool) error {
-	// check the key rotated, if rotated use the returned consKey to get the missed blocks
-	// because missed blocks are still pointing to the old key
+	// get the old consensus key if it has rotated, allowing retrieval of missed blocks associated with the old key
 	addr, err := k.getPreviousConsKey(ctx, addr)
 	if err != nil {
 		return err
@@ -181,8 +179,7 @@ func (k Keeper) SetMissedBlockBitmapValue(ctx context.Context, addr sdk.ConsAddr
 
 // DeleteMissedBlockBitmap removes a validator's missed block bitmap from state.
 func (k Keeper) DeleteMissedBlockBitmap(ctx context.Context, addr sdk.ConsAddress) error {
-	// check the key rotated, if rotated use the returned consKey to delete the missed blocks
-	// because missed blocks are still pointing to the old key
+	// get the old consensus key if it has rotated, allowing retrieval of missed blocks associated with the old key
 	addr, err := k.getPreviousConsKey(ctx, addr)
 	if err != nil {
 		return err
@@ -239,8 +236,8 @@ func (k Keeper) GetValidatorMissedBlocks(ctx context.Context, addr sdk.ConsAddre
 	return missedBlocks, err
 }
 
-// performConsensusPubKeyUpdate updates cons address to its pub key relation
-// Updates signing info, missed blocks (removes old one, and sets new one)
+// performConsensusPubKeyUpdate updates the consensus address-pubkey relation
+// and refreshes the signing info by replacing the old key with the new one.
 func (k Keeper) performConsensusPubKeyUpdate(ctx context.Context, oldPubKey, newPubKey cryptotypes.PubKey) error {
 	// Connect new consensus address with PubKey
 	if err := k.AddrPubkeyRelation.Set(ctx, newPubKey.Address(), newPubKey); err != nil {

x/slashing/keeper/signing_info_test.go

@@ -101,7 +101,7 @@ func (s *KeeperTestSuite) TestValidatorMissedBlockBitmap_SmallWindow() {
 		require.NoError(err)
 		require.Len(missedBlocks, int(params.SignedBlocksWindow)-1)
 
-		// if the validator rotated it's key there will be different consKeys and a mapping will be added in the state.
+		// if the validator rotated its key, there will be different consKeys and a mapping will be added in the state
 		consAddr1 := sdk.ConsAddress("addr1_______________")
 		s.stakingKeeper.EXPECT().ValidatorIdentifier(gomock.Any(), consAddr1).Return(consAddr, nil).AnyTimes()
 
@@ -147,12 +147,12 @@ func (s *KeeperTestSuite) TestPerformConsensusPubKeyUpdate() {
 	require.NoError(err)
 	require.Equal(savedPubKey, pks[1])
 
-	// check validator SigningInfo is set properly to new consensus pubkey
+	// check validator's SigningInfo is set properly with new consensus pubkey
 	signingInfo, err := slashingKeeper.ValidatorSigningInfo.Get(ctx, newConsAddr)
 	require.NoError(err)
 	require.Equal(signingInfo, newInfo)
 
-	// missed blocks maps to old cons key only since there is a identifier added to get the missed blocks using the new cons key.
+	// missed blocks map corresponds only to the old cons key, as there is an identifier added to get the missed blocks using the new cons key
 	missedBlocks, err := slashingKeeper.GetValidatorMissedBlocks(ctx, oldConsAddr)
 	require.NoError(err)
 

x/slashing/module.go

@@ -85,27 +85,27 @@ func (AppModule) RegisterLegacyAminoCodec(cdc legacy.Amino) {
 	types.RegisterLegacyAminoCodec(cdc)
 }
 
-// RegisterInterfaces registers the module's interface types
+// RegisterInterfaces registers the slashing module's interface types
 func (AppModule) RegisterInterfaces(registrar registry.InterfaceRegistrar) {
 	types.RegisterInterfaces(registrar)
 }
 
-// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the slashig module.
+// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the slashing module.
 func (AppModule) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *gwruntime.ServeMux) {
 	if err := types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)); err != nil {
 		panic(err)
 	}
 }
 
-// RegisterServices registers module services.
+// RegisterServices registers slashing module's services.
 func (am AppModule) RegisterServices(registrar grpc.ServiceRegistrar) error {
 	types.RegisterMsgServer(registrar, keeper.NewMsgServerImpl(am.keeper))
 	types.RegisterQueryServer(registrar, keeper.NewQuerier(am.keeper))
 
 	return nil
 }
 
-// RegisterMigrations registers module migrations.
+// RegisterMigrations registers slashing module's migrations.
 func (am AppModule) RegisterMigrations(mr appmodule.MigrationRegistrar) error {
 	m := keeper.NewMigrator(am.keeper, am.stakingKeeper.ValidatorAddressCodec())
 

x/slashing/simulation/proposals.go

@@ -36,7 +36,7 @@ func ProposalMsgs() []simtypes.WeightedProposalMsg {
 // SimulateMsgUpdateParams returns a random MsgUpdateParams
 func SimulateMsgUpdateParams(_ context.Context, r *rand.Rand, _ []simtypes.Account, ac coreaddress.Codec) (sdk.Msg, error) {
 	// use the default gov module account address as authority
-	var authority sdk.AccAddress = address.Module("gov")
+	var authority sdk.AccAddress = address.Module(types.GovModuleName)
 
 	authorityAddr, err := ac.BytesToString(authority)
 	if err != nil {

x/slashing/simulation/proposals_test.go

@@ -40,7 +40,7 @@ func TestProposalMsgs(t *testing.T) {
 	msgUpdateParams, ok := msg.(*types.MsgUpdateParams)
 	assert.Assert(t, ok)
 
-	moduleAddr, err := ac.BytesToString(address.Module("gov"))
+	moduleAddr, err := ac.BytesToString(address.Module(types.GovModuleName))
 	assert.NilError(t, err)
 
 	assert.Equal(t, moduleAddr, msgUpdateParams.Authority)