[IF-FINDING-004]: Unnecessary token burning on validator removal

app/app.go

@@ -444,7 +444,6 @@ func New(
 		stakingtypes.NewMultiStakingHooks(
 			app.DistrKeeper.Hooks(),
 			app.SlashingKeeper.Hooks(),
-			app.PoaKeeper.Hooks(),
 		),
 	)
 	// register the staking hooks

x/poa/keeper/keeper.go

@@ -197,7 +197,7 @@ func (k Keeper) ExecuteAddValidator(ctx sdk.Context, msg *types.MsgAddValidator)
 }
 
 func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) error {
-	accAddress, err := sdk.AccAddressFromBech32(validatorAddress)
+	valAddress, err := sdk.ValAddressFromBech32(validatorAddress)
 	if err != nil {
 		return err
 	}
@@ -207,35 +207,11 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string)
 	}
 	denom := params.BondDenom
 
-	// Check if address has some balance in bank and withdraw in case of having
-	balance := k.bk.GetBalance(ctx, accAddress, denom)
-	if balance.IsPositive() {
-		coins := sdk.NewCoins(balance)
-		err = k.bk.SendCoinsFromAccountToModule(ctx, accAddress, types.ModuleName, coins)
-		if err != nil {
-			// Fail hard if we can't send coins to the module account
-			return err
-		}
-
-		err = k.bk.BurnCoins(ctx, types.ModuleName, coins)
-		if err != nil {
-			// Fail hard if we can't burn coins
-			return err
-		}
-	}
-
 	// If address also has a validator, we need to check additional conditions
-	valAddress := sdk.ValAddress(accAddress)
 	validator, err := k.sk.GetValidator(ctx, valAddress)
 	if err != nil {
 		ctx.Logger().Warn("Error getting validator", "error", err)
-		if balance.IsZero() {
-			// Address has no balance in bank and is not a validator either
-			// NOTE: Since delegations are not enabled in this version, we don't need to check for them
-			return types.ErrAddressHasNoTokens
-		}
-		// Validator does not exist, but we already took its balance from bank, we can safely return
-		return nil
+		return types.ErrAddressIsNotAValidator
 	}
 
 	if err := k.sk.Hooks().BeforeValidatorModified(ctx, valAddress); err != nil {
@@ -297,7 +273,6 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string)
 			sdk.NewAttribute(types.AttributeValidator, validatorAddress),
 			sdk.NewAttribute(types.AttributeHeight, fmt.Sprintf("%d", ctx.BlockHeight())),
 			sdk.NewAttribute(types.AttributeStakingTokens, fmt.Sprintf("%d", validator.Tokens)),
-			sdk.NewAttribute(types.AttributeBankTokens, balance.String()),
 		),
 	)
 

x/poa/keeper/keeper_test.go

@@ -76,6 +76,6 @@ func TestPoAKeeper_ExecuteAddValidator(t *testing.T) {
 func TestPoAKeeper_ExecuteRemoveValidator(t *testing.T) {
 	keeper, ctx := poaKeeperTestSetup(t)
 
-	err := keeper.ExecuteRemoveValidator(ctx, "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp")
+	err := keeper.ExecuteRemoveValidator(ctx, "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu")
 	require.NoError(t, err)
 }

x/poa/keeper/msg_server_remove_validator_test.go

@@ -14,7 +14,7 @@ func TestMsgServer_RemoveValidator(t *testing.T) {
 
 	msg := &types.MsgRemoveValidator{
 		Authority:        poaKeeper.GetAuthority(),
-		ValidatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp",
+		ValidatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu",
 	}
 
 	_, err := msgServer.RemoveValidator(ctx, msg)

x/poa/types/errors.go

@@ -8,12 +8,11 @@ import (
 
 // x/poa module sentinel errors
 var (
-	ErrAddressHasBankTokens      = sdkerrors.Register(ModuleName, 2, "address already has bank tokens")
-	ErrAddressHasNoTokens        = sdkerrors.Register(ModuleName, 3, "address has not tokens")
-	ErrAddressHasBondedTokens    = sdkerrors.Register(ModuleName, 4, "address already has bonded tokens")
-	ErrAddressHasUnbondingTokens = sdkerrors.Register(ModuleName, 5, "address already has unbonding tokens")
-	ErrAddressHasUnbondedTokens  = sdkerrors.Register(ModuleName, 6, "address already has unbonded tokens")
-	ErrAddressHasDelegatedTokens = sdkerrors.Register(ModuleName, 7, "address already has delegated tokens")
-	ErrInvalidValidatorStatus    = sdkerrors.Register(ModuleName, 8, "invalid validator status")
-	ErrMaxValidatorsReached      = sdkerrors.Register(ModuleName, 9, "maximum number of validators reached")
+	ErrAddressHasBankTokens      = sdkerrors.Register(ModuleName, 1, "address already has bank tokens")
+	ErrAddressHasBondedTokens    = sdkerrors.Register(ModuleName, 2, "address already has bonded tokens")
+	ErrAddressHasUnbondingTokens = sdkerrors.Register(ModuleName, 3, "address already has unbonding tokens")
+	ErrAddressHasDelegatedTokens = sdkerrors.Register(ModuleName, 4, "address already has delegated tokens")
+	ErrInvalidValidatorStatus    = sdkerrors.Register(ModuleName, 5, "invalid validator status")
+	ErrAddressIsNotAValidator    = sdkerrors.Register(ModuleName, 6, "address is not a validator")
+	ErrMaxValidatorsReached      = sdkerrors.Register(ModuleName, 7, "maximum number of validators reached")
 )