From 5004996a59ccea68385ab02f5acaf0e3ba79b155 Mon Sep 17 00:00:00 2001 From: GuillemGarciaDev Date: Mon, 17 Mar 2025 17:08:45 +0100 Subject: [PATCH 1/6] refactor(x/poa): remove unnecessary burn coins for unbonded/unbonding validators + validator address check on `ExecuteRemoveValidator` --- x/poa/keeper/keeper.go | 24 +++--------------------- x/poa/keeper/keeper_test.go | 2 +- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index fb07adb..dc12318 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -189,7 +189,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 } @@ -199,25 +199,12 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) } denom := params.BondDenom + accAddress := sdk.AccAddress(valAddress) + // 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) @@ -268,11 +255,6 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) return err } case stakingtypes.Unbonding, stakingtypes.Unbonded: - coins := sdk.NewCoins(sdk.NewCoin(denom, validator.Tokens)) - err = k.bk.BurnCoins(ctx, stakingtypes.NotBondedPoolName, coins) - if err != nil { - return err - } default: return types.ErrInvalidValidatorStatus } diff --git a/x/poa/keeper/keeper_test.go b/x/poa/keeper/keeper_test.go index da5286f..e1b55fa 100644 --- a/x/poa/keeper/keeper_test.go +++ b/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) } From 2d711db2769822678624f3b346d2cb540e13e7d5 Mon Sep 17 00:00:00 2001 From: GuillemGarciaDev Date: Mon, 17 Mar 2025 19:37:01 +0100 Subject: [PATCH 2/6] fix(x/poa): replace account address with validator address --- x/poa/keeper/msg_server_remove_validator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/poa/keeper/msg_server_remove_validator_test.go b/x/poa/keeper/msg_server_remove_validator_test.go index c0e8231..862e371 100644 --- a/x/poa/keeper/msg_server_remove_validator_test.go +++ b/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) From 58209830ee20eb32eb9fd11a5a6fdd2feb7cd638 Mon Sep 17 00:00:00 2001 From: GuillemGarciaDev Date: Tue, 18 Mar 2025 13:18:01 +0100 Subject: [PATCH 3/6] fix(x/poa): restore burn coins to not bonded pool + update error handling for validator not found --- x/poa/keeper/keeper.go | 19 ++++++------------- x/poa/types/errors.go | 1 + 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index dc12318..71422be 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -199,22 +199,11 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) } denom := params.BondDenom - accAddress := sdk.AccAddress(valAddress) - - // Check if address has some balance in bank and withdraw in case of having - balance := k.bk.GetBalance(ctx, accAddress, denom) - // If address also has a validator, we need to check additional conditions 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 { @@ -255,6 +244,11 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) return err } case stakingtypes.Unbonding, stakingtypes.Unbonded: + coins := sdk.NewCoins(sdk.NewCoin(denom, validator.Tokens)) + err = k.bk.BurnCoins(ctx, stakingtypes.NotBondedPoolName, coins) + if err != nil { + return err + } default: return types.ErrInvalidValidatorStatus } @@ -271,7 +265,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()), ), ) diff --git a/x/poa/types/errors.go b/x/poa/types/errors.go index a51b00e..2a869b2 100644 --- a/x/poa/types/errors.go +++ b/x/poa/types/errors.go @@ -15,4 +15,5 @@ var ( 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") + ErrAddressIsNotAValidator = sdkerrors.Register(ModuleName, 9, "address is not a validator") ) From 1198ebf3caa1900b8a9da4fb0d417293d7c3d7cc Mon Sep 17 00:00:00 2001 From: GuillemGarciaDev Date: Tue, 18 Mar 2025 13:34:46 +0100 Subject: [PATCH 4/6] fix(x/poa): remove unused errors --- x/poa/types/errors.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/poa/types/errors.go b/x/poa/types/errors.go index 2a869b2..b479495 100644 --- a/x/poa/types/errors.go +++ b/x/poa/types/errors.go @@ -9,10 +9,8 @@ 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") ErrAddressIsNotAValidator = sdkerrors.Register(ModuleName, 9, "address is not a validator") From 4bee4b0895c6384aaadbe92fe5b89749e2fcbb05 Mon Sep 17 00:00:00 2001 From: GuillemGarciaDev Date: Tue, 18 Mar 2025 13:44:39 +0100 Subject: [PATCH 5/6] fix(app): remove poa hooks --- app/app.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app/app.go b/app/app.go index fd62559..9c937a7 100644 --- a/app/app.go +++ b/app/app.go @@ -444,7 +444,6 @@ func New( stakingtypes.NewMultiStakingHooks( app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks(), - app.PoaKeeper.Hooks(), ), ) // register the staking hooks From 112af0b5489a0e81ddbdeb1447bb27e8f5266ee8 Mon Sep 17 00:00:00 2001 From: GuillemGarciaDev Date: Tue, 18 Mar 2025 13:48:48 +0100 Subject: [PATCH 6/6] fix(x/poa): poa errors code --- x/poa/types/errors.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x/poa/types/errors.go b/x/poa/types/errors.go index b2a06ae..af38842 100644 --- a/x/poa/types/errors.go +++ b/x/poa/types/errors.go @@ -8,11 +8,11 @@ import ( // x/poa module sentinel errors var ( - ErrAddressHasBankTokens = sdkerrors.Register(ModuleName, 2, "address already has bank tokens") - ErrAddressHasBondedTokens = sdkerrors.Register(ModuleName, 4, "address already has bonded tokens") - ErrAddressHasUnbondingTokens = sdkerrors.Register(ModuleName, 5, "address already has unbonding tokens") - ErrAddressHasDelegatedTokens = sdkerrors.Register(ModuleName, 7, "address already has delegated tokens") - ErrInvalidValidatorStatus = sdkerrors.Register(ModuleName, 8, "invalid validator status") - ErrAddressIsNotAValidator = sdkerrors.Register(ModuleName, 9, "address is not a validator") - ErrMaxValidatorsReached = sdkerrors.Register(ModuleName, 10, "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") )