refactor(x/distribution): audit changes (backport #16785) (#16799)

Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>

tests/integration/distribution/keeper/grpc_query_test.go

@@ -509,7 +509,7 @@ func TestGRPCDelegationRewards(t *testing.T) {
 	delTokens := sdk.TokensFromConsensusPower(2, sdk.DefaultPowerReduction)
 	validator, issuedShares := val.AddTokensFromDel(delTokens)
 	delegation := stakingtypes.NewDelegation(delAddr, f.valAddr, issuedShares)
-	f.stakingKeeper.SetDelegation(f.sdkCtx, delegation)
+	assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delegation))
 	assert.NilError(t, f.distrKeeper.SetDelegatorStartingInfo(f.sdkCtx, validator.GetOperator(), delAddr, types.NewDelegatorStartingInfo(2, math.LegacyNewDec(initialStake), 20)))
 
 	// setup validator rewards

tests/integration/distribution/keeper/msg_server_test.go

@@ -180,7 +180,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) {
 	assert.NilError(t, err)
 	validator.DelegatorShares = math.LegacyNewDec(100)
 	validator.Tokens = sdk.NewInt(1000000)
-	f.stakingKeeper.SetValidator(f.sdkCtx, validator)
+	assert.NilError(t, f.stakingKeeper.SetValidator(f.sdkCtx, validator))
 
 	// set module account coins
 	initTokens := f.stakingKeeper.TokensFromConsensusPower(f.sdkCtx, int64(1000))
@@ -196,7 +196,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) {
 	delTokens := sdk.TokensFromConsensusPower(2, sdk.DefaultPowerReduction)
 	validator, issuedShares := validator.AddTokensFromDel(delTokens)
 	delegation := stakingtypes.NewDelegation(delAddr, validator.GetOperator(), issuedShares)
-	f.stakingKeeper.SetDelegation(f.sdkCtx, delegation)
+	assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delegation))
 	err = f.distrKeeper.SetDelegatorStartingInfo(f.sdkCtx, validator.GetOperator(), delAddr, distrtypes.NewDelegatorStartingInfo(2, math.LegacyOneDec(), 20))
 	require.NoError(t, err)
 	// setup validator rewards
@@ -894,8 +894,8 @@ func TestMsgDepositValidatorRewardsPool(t *testing.T) {
 
 	// mint a non-staking token and send to an account
 	amt := sdk.NewCoins(sdk.NewInt64Coin("foo", 500))
-	f.bankKeeper.MintCoins(f.sdkCtx, distrtypes.ModuleName, amt)
-	f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, distrtypes.ModuleName, addr, amt)
+	require.NoError(t, f.bankKeeper.MintCoins(f.sdkCtx, distrtypes.ModuleName, amt))
+	require.NoError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, distrtypes.ModuleName, addr, amt))
 
 	bondDenom, err := f.stakingKeeper.BondDenom(f.sdkCtx)
 	require.NoError(t, err)

x/distribution/abci.go

@@ -23,11 +23,12 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) error {
 	// TODO this is Tendermint-dependent
 	// ref https://github.com/cosmos/cosmos-sdk/issues/3095
 	if ctx.BlockHeight() > 1 {
-		k.AllocateTokens(ctx, previousTotalPower, ctx.VoteInfos())
+		if err := k.AllocateTokens(ctx, previousTotalPower, ctx.VoteInfos()); err != nil {
+			return err
+		}
 	}
 
 	// record the proposer for when we payout on the next block
 	consAddr := sdk.ConsAddress(ctx.BlockHeader().ProposerAddress)
-	k.SetPreviousProposerConsAddr(ctx, consAddr)
-	return nil
+	return k.SetPreviousProposerConsAddr(ctx, consAddr)
 }

x/distribution/keeper/genesis.go

@@ -44,7 +44,9 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data types.GenesisState) {
 		}
 	}
 
-	k.SetPreviousProposerConsAddr(ctx, previousProposer)
+	if err = k.SetPreviousProposerConsAddr(ctx, previousProposer); err != nil {
+		panic(err)
+	}
 
 	for _, rew := range data.OutstandingRewards {
 		valAddr, err := sdk.ValAddressFromBech32(rew.ValidatorAddress)

x/distribution/keeper/hooks.go

@@ -158,8 +158,7 @@ func (h Hooks) AfterDelegationModified(ctx context.Context, delAddr sdk.AccAddre
 
 // record the slash event
 func (h Hooks) BeforeValidatorSlashed(ctx context.Context, valAddr sdk.ValAddress, fraction sdkmath.LegacyDec) error {
-	h.k.updateValidatorSlashFraction(ctx, valAddr, fraction)
-	return nil
+	return h.k.updateValidatorSlashFraction(ctx, valAddr, fraction)
 }
 
 func (h Hooks) BeforeValidatorModified(_ context.Context, _ sdk.ValAddress) error {

x/distribution/keeper/msg_server.go

@@ -196,7 +196,9 @@ func (k msgServer) DepositValidatorRewardsPool(ctx context.Context, msg *types.M
 	// Allocate tokens from the distribution module to the validator, which are
 	// then distributed to the validator's delegators.
 	reward := sdk.NewDecCoinsFromCoins(msg.Amount...)
-	k.AllocateTokensToValidator(ctx, validator, reward)
+	if err = k.AllocateTokensToValidator(ctx, validator, reward); err != nil {
+		return nil, err
+	}
 
 	logger := k.Logger(ctx)
 	logger.Info(

x/distribution/keeper/store.go

@@ -72,10 +72,10 @@ func (k Keeper) GetPreviousProposerConsAddr(ctx context.Context) (sdk.ConsAddres
 }
 
 // set the proposer public key for this block
-func (k Keeper) SetPreviousProposerConsAddr(ctx context.Context, consAddr sdk.ConsAddress) {
+func (k Keeper) SetPreviousProposerConsAddr(ctx context.Context, consAddr sdk.ConsAddress) error {
 	store := k.storeService.OpenKVStore(ctx)
 	bz := k.cdc.MustMarshal(&gogotypes.BytesValue{Value: consAddr})
-	store.Set(types.ProposerKey, bz)
+	return store.Set(types.ProposerKey, bz)
 }
 
 // get the starting info associated with a delegator

x/distribution/types/msg.go

@@ -21,37 +21,19 @@ func NewMsgSetWithdrawAddress(delAddr, withdrawAddr sdk.AccAddress) *MsgSetWithd
 	}
 }
 
-// Return address that must sign over msg.GetSignBytes()
-func (msg MsgSetWithdrawAddress) GetSigners() []sdk.AccAddress {
-	delegator, _ := sdk.AccAddressFromBech32(msg.DelegatorAddress)
-	return []sdk.AccAddress{delegator}
-}
-
 func NewMsgWithdrawDelegatorReward(delAddr sdk.AccAddress, valAddr sdk.ValAddress) *MsgWithdrawDelegatorReward {
 	return &MsgWithdrawDelegatorReward{
 		DelegatorAddress: delAddr.String(),
 		ValidatorAddress: valAddr.String(),
 	}
 }
 
-// Return address that must sign over msg.GetSignBytes()
-func (msg MsgWithdrawDelegatorReward) GetSigners() []sdk.AccAddress {
-	delegator, _ := sdk.AccAddressFromBech32(msg.DelegatorAddress)
-	return []sdk.AccAddress{delegator}
-}
-
 func NewMsgWithdrawValidatorCommission(valAddr sdk.ValAddress) *MsgWithdrawValidatorCommission {
 	return &MsgWithdrawValidatorCommission{
 		ValidatorAddress: valAddr.String(),
 	}
 }
 
-// Return address that must sign over msg.GetSignBytes()
-func (msg MsgWithdrawValidatorCommission) GetSigners() []sdk.AccAddress {
-	valAddr, _ := sdk.ValAddressFromBech32(msg.ValidatorAddress)
-	return []sdk.AccAddress{sdk.AccAddress(valAddr)}
-}
-
 // NewMsgFundCommunityPool returns a new MsgFundCommunityPool with a sender and
 // a funding amount.
 func NewMsgFundCommunityPool(amount sdk.Coins, depositor sdk.AccAddress) *MsgFundCommunityPool {
@@ -61,27 +43,6 @@ func NewMsgFundCommunityPool(amount sdk.Coins, depositor sdk.AccAddress) *MsgFun
 	}
 }
 
-// GetSigners returns the signer addresses that are expected to sign the result
-// of GetSignBytes.
-func (msg MsgFundCommunityPool) GetSigners() []sdk.AccAddress {
-	depositor, _ := sdk.AccAddressFromBech32(msg.Depositor)
-	return []sdk.AccAddress{depositor}
-}
-
-// GetSigners returns the signer addresses that are expected to sign the result
-// of GetSignBytes.
-func (msg MsgUpdateParams) GetSigners() []sdk.AccAddress {
-	authority, _ := sdk.AccAddressFromBech32(msg.Authority)
-	return []sdk.AccAddress{authority}
-}
-
-// GetSigners returns the signer addresses that are expected to sign the result
-// of GetSignBytes, which is the authority.
-func (msg MsgCommunityPoolSpend) GetSigners() []sdk.AccAddress {
-	authority, _ := sdk.AccAddressFromBech32(msg.Authority)
-	return []sdk.AccAddress{authority}
-}
-
 // NewMsgDepositValidatorRewardsPool returns a new MsgDepositValidatorRewardsPool
 // with a depositor and a funding amount.
 func NewMsgDepositValidatorRewardsPool(depositor sdk.AccAddress, valAddr sdk.ValAddress, amount sdk.Coins) *MsgDepositValidatorRewardsPool {
@@ -91,10 +52,3 @@ func NewMsgDepositValidatorRewardsPool(depositor sdk.AccAddress, valAddr sdk.Val
 		ValidatorAddress: valAddr.String(),
 	}
 }
-
-// GetSigners returns the signer addresses that are expected to sign the result
-// of GetSignBytes, which is the depositor.
-func (msg MsgDepositValidatorRewardsPool) GetSigners() []sdk.AccAddress {
-	depositor, _ := sdk.AccAddressFromBech32(msg.Depositor)
-	return []sdk.AccAddress{depositor}
-}