From d4546d3b2e260f46907fa4c54c11dc8388d1f4d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Luque?= Date: Mon, 15 Jun 2026 13:22:30 +0200 Subject: [PATCH 1/2] refactor(poa)!: parse validator address as AccAddress --- tests/integration/poa_test.go | 8 +- x/poa/keeper/keeper.go | 16 +++- x/poa/keeper/keeper_test.go | 77 ++++++++++++++++--- .../msg_server_remove_validator_test.go | 10 +-- 4 files changed, 90 insertions(+), 21 deletions(-) diff --git a/tests/integration/poa_test.go b/tests/integration/poa_test.go index a7b7f7d..25ad821 100644 --- a/tests/integration/poa_test.go +++ b/tests/integration/poa_test.go @@ -609,7 +609,7 @@ func (s *TestSuite) TestRemoveValidator_UnexistingValidator() { }{ { name: "remove unexisting validator - random address - with balance", - valAddress: randomValAddr.String(), + valAddress: randomAccs[0].Address.String(), expectedError: poatypes.ErrAddressIsNotAValidator, beforeRun: func() { _, err := s.Network().GetStakingClient().Validator( @@ -758,7 +758,7 @@ func (s *TestSuite) TestRemoveValidator_ExistingValidator_StatusBonded() { authority := sdktypes.AccAddress(address.Module("gov")) msg := poatypes.NewMsgRemoveValidator( authority.String(), - valAddr.String(), + sdktypes.AccAddress(valAddr).String(), ) proposal, err := utils.SubmitAndAwaitProposalResolution(s.factory, s.Network(), s.keyring.GetKeys(), "test", msg) @@ -871,7 +871,7 @@ func (s *TestSuite) TestRemoveValidator_ExistingValidator_Jailed() { authority := sdktypes.AccAddress(address.Module("gov")) msg := poatypes.NewMsgRemoveValidator( authority.String(), - valAddr.String(), + sdktypes.AccAddress(valAddr).String(), ) proposal, err := utils.SubmitAndAwaitProposalResolution(s.factory, s.Network(), s.keyring.GetKeys(), "test", msg) @@ -1014,7 +1014,7 @@ func (s *TestSuite) TestRemoveValidator_ExistingValidator_Tombstoned() { authority := sdktypes.AccAddress(address.Module("gov")) msg := poatypes.NewMsgRemoveValidator( authority.String(), - valAddr.String(), + sdktypes.AccAddress(valAddr).String(), ) proposal, err := utils.SubmitAndAwaitProposalResolution(s.factory, s.Network(), s.keyring.GetKeys(), "test", msg) diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index 5a6af7b..16a7099 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -82,10 +82,20 @@ func (k Keeper) GetAuthority() string { return k.authority } +// parseValidatorAddress parses a validator's account address (the cosmos.AddressString +// format declared by both MsgAddValidator and MsgRemoveValidator) and derives the +// matching validator operator address from it. +func parseValidatorAddress(validatorAddress string) (sdk.AccAddress, sdk.ValAddress, error) { + accAddress, err := sdk.AccAddressFromBech32(validatorAddress) + if err != nil { + return nil, nil, err + } + return accAddress, sdk.ValAddress(accAddress), nil +} + func (k Keeper) ExecuteAddValidator(ctx sdk.Context, msg *types.MsgAddValidator) error { // Check if the new validator already has staking power in the bank account - accAddress, err := sdk.AccAddressFromBech32(msg.ValidatorAddress) - valAddress := sdk.ValAddress(accAddress) + accAddress, valAddress, err := parseValidatorAddress(msg.ValidatorAddress) if err != nil { return err } @@ -207,7 +217,7 @@ func (k Keeper) ExecuteAddValidator(ctx sdk.Context, msg *types.MsgAddValidator) } func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) error { - valAddress, err := sdk.ValAddressFromBech32(validatorAddress) + _, valAddress, err := parseValidatorAddress(validatorAddress) if err != nil { return err } diff --git a/x/poa/keeper/keeper_test.go b/x/poa/keeper/keeper_test.go index d2f340e..40879c1 100644 --- a/x/poa/keeper/keeper_test.go +++ b/x/poa/keeper/keeper_test.go @@ -408,7 +408,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }, { name: "should fail - staking keeper returns error on GetParams", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("staking params error"), stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{}, errors.New("staking params error")) @@ -418,7 +418,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { { name: "should fail - staking keeper returns error on GetValidator", expectedError: types.ErrAddressIsNotAValidator, - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{ BondDenom: "BND", @@ -429,7 +429,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }, { name: "should fail - staking keeper returns error on call GetUnbondingDelegationsFromValidator", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("staking keeper get unbonding delegations from validator error"), stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{ @@ -446,7 +446,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }, { name: "should fail - staking keeper returns error on call SlashUnbondingDelegation", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("staking keeper slash unbonding delegation error"), stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{ @@ -471,7 +471,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }, { name: "should fail - staking keeper returns error on RemoveValidatorTokens call", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("staking keeper remove validator tokens error"), stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{ @@ -498,7 +498,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { //nolint:dupl { name: "should fail - bank keeper returns error on call BurnCoins for status bonded", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("bank keeper burn coins error"), //nolint:dupl stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { @@ -529,7 +529,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { //nolint:dupl { name: "should fail - bank keeper returns error on call BurnCoins for status unbonding/unbonded", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("bank keeper burn coins error"), //nolint:dupl stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { @@ -559,7 +559,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }, { name: "should fail - bank keeper returns error for invalid validator status", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: types.ErrInvalidValidatorStatus, //nolint:dupl stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { @@ -587,7 +587,7 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }, { name: "should fail - staking keeper returns error on call Unbond", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", expectedError: errors.New("staking keeper unbond error"), stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{ @@ -633,3 +633,62 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }) } } + +func TestParseValidatorAddress(t *testing.T) { + setupSdkConfig() + + // accAddr and valAddr are the account and validator-operator encodings of the + // same underlying key (same 20 bytes, different bech32 prefix). + const ( + accAddr = "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp" + valAddr = "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu" + ) + + tt := []struct { + name string + input string + expectedErr string + }{ + { + name: "should pass - account address yields matching account and validator keys", + input: accAddr, + }, + { + name: "should fail - validator operator address is not a valid account address", + input: valAddr, + expectedErr: "invalid Bech32 prefix", + }, + { + name: "should fail - invalid bech32 string", + input: "invalidvalidatoraddress", + expectedErr: "decoding bech32 failed", + }, + { + name: "should fail - empty address", + input: "", + expectedErr: "empty address string is not allowed", + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + acc, val, err := parseValidatorAddress(tc.input) + + if tc.expectedErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErr) + require.Nil(t, acc) + require.Nil(t, val) + return + } + + require.NoError(t, err) + // The account address is returned exactly as provided. + require.Equal(t, accAddr, acc.String()) + // The validator operator address is derived from the same key. + require.Equal(t, valAddr, val.String()) + // Both encodings share the exact same underlying bytes. + require.Equal(t, acc.Bytes(), val.Bytes()) + }) + } +} diff --git a/x/poa/keeper/msg_server_remove_validator_test.go b/x/poa/keeper/msg_server_remove_validator_test.go index 83a1047..ebdbb1b 100644 --- a/x/poa/keeper/msg_server_remove_validator_test.go +++ b/x/poa/keeper/msg_server_remove_validator_test.go @@ -10,10 +10,10 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" "github.com/xrplevm/node/v10/x/poa/testutil" "github.com/xrplevm/node/v10/x/poa/types" + "go.uber.org/mock/gomock" ) func TestMsgServer_RemoveValidator(t *testing.T) { @@ -31,7 +31,7 @@ func TestMsgServer_RemoveValidator(t *testing.T) { { name: "should fail - invalid authority address", authority: "invalidauthority", - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", stakingMocks: func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, bankMocks: func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, expectedErr: govtypes.ErrInvalidSigner, @@ -47,7 +47,7 @@ func TestMsgServer_RemoveValidator(t *testing.T) { { name: "should pass", authority: poaAuthority, - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{ BondDenom: "BND", @@ -73,7 +73,7 @@ func TestMsgServer_RemoveValidator(t *testing.T) { { name: "should pass - BeforeValidatorModified hook error is swallowed and logged", authority: poaAuthority, - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", stakingMocks: func(_ sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { // gomock.Any() for ctx because the test swaps the logger after // setup, producing a different sdk.Context value. @@ -102,7 +102,7 @@ func TestMsgServer_RemoveValidator(t *testing.T) { { name: "should pass - BeforeValidatorSlashed hook error is swallowed and logged", authority: poaAuthority, - validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu", + validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp", stakingMocks: func(_ sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { stakingKeeper.EXPECT().GetParams(gomock.Any()).Return(stakingtypes.Params{ BondDenom: "BND", From 0ee1fa47ee0437e10f5aff51334131d0ba00483c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Luque?= Date: Thu, 18 Jun 2026 10:54:26 +0200 Subject: [PATCH 2/2] feat(poa): add ValidateBasic to validator messages --- tests/integration/poa_test.go | 2 +- x/poa/keeper/keeper.go | 19 ++---- x/poa/keeper/keeper_test.go | 59 ------------------ x/poa/types/message_add_validator.go | 19 ++++++ x/poa/types/message_add_validator_test.go | 64 ++++++++++++++++++++ x/poa/types/message_remove_validator.go | 17 +++++- x/poa/types/message_remove_validator_test.go | 64 ++++++++++++++++++++ 7 files changed, 169 insertions(+), 75 deletions(-) create mode 100644 x/poa/types/message_add_validator_test.go create mode 100644 x/poa/types/message_remove_validator_test.go diff --git a/tests/integration/poa_test.go b/tests/integration/poa_test.go index 25ad821..49b72e0 100644 --- a/tests/integration/poa_test.go +++ b/tests/integration/poa_test.go @@ -609,7 +609,7 @@ func (s *TestSuite) TestRemoveValidator_UnexistingValidator() { }{ { name: "remove unexisting validator - random address - with balance", - valAddress: randomAccs[0].Address.String(), + valAddress: sdktypes.AccAddress(randomValAddr).String(), expectedError: poatypes.ErrAddressIsNotAValidator, beforeRun: func() { _, err := s.Network().GetStakingClient().Validator( diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index 16a7099..3b5bff0 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -82,23 +82,13 @@ func (k Keeper) GetAuthority() string { return k.authority } -// parseValidatorAddress parses a validator's account address (the cosmos.AddressString -// format declared by both MsgAddValidator and MsgRemoveValidator) and derives the -// matching validator operator address from it. -func parseValidatorAddress(validatorAddress string) (sdk.AccAddress, sdk.ValAddress, error) { - accAddress, err := sdk.AccAddressFromBech32(validatorAddress) - if err != nil { - return nil, nil, err - } - return accAddress, sdk.ValAddress(accAddress), nil -} - func (k Keeper) ExecuteAddValidator(ctx sdk.Context, msg *types.MsgAddValidator) error { // Check if the new validator already has staking power in the bank account - accAddress, valAddress, err := parseValidatorAddress(msg.ValidatorAddress) + accAddress, err := sdk.AccAddressFromBech32(msg.ValidatorAddress) if err != nil { return err } + valAddress := sdk.ValAddress(accAddress) params, err := k.sk.GetParams(ctx) if err != nil { return err @@ -217,10 +207,11 @@ func (k Keeper) ExecuteAddValidator(ctx sdk.Context, msg *types.MsgAddValidator) } func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) error { - _, valAddress, err := parseValidatorAddress(validatorAddress) + accAddress, err := sdk.AccAddressFromBech32(validatorAddress) if err != nil { return err } + valAddress := sdk.ValAddress(accAddress) params, err := k.sk.GetParams(ctx) if err != nil { return err @@ -282,7 +273,7 @@ func (k Keeper) ExecuteRemoveValidator(ctx sdk.Context, validatorAddress string) } // Unbond self-delegation so the validator is removed after being unbonded - _, err = k.sk.Unbond(ctx, sdk.AccAddress(valAddress), valAddress, changedVal.DelegatorShares) + _, err = k.sk.Unbond(ctx, accAddress, valAddress, changedVal.DelegatorShares) if err != nil { return err } diff --git a/x/poa/keeper/keeper_test.go b/x/poa/keeper/keeper_test.go index 40879c1..582d96f 100644 --- a/x/poa/keeper/keeper_test.go +++ b/x/poa/keeper/keeper_test.go @@ -633,62 +633,3 @@ func TestKeeper_ExecuteRemoveValidator(t *testing.T) { }) } } - -func TestParseValidatorAddress(t *testing.T) { - setupSdkConfig() - - // accAddr and valAddr are the account and validator-operator encodings of the - // same underlying key (same 20 bytes, different bech32 prefix). - const ( - accAddr = "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp" - valAddr = "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu" - ) - - tt := []struct { - name string - input string - expectedErr string - }{ - { - name: "should pass - account address yields matching account and validator keys", - input: accAddr, - }, - { - name: "should fail - validator operator address is not a valid account address", - input: valAddr, - expectedErr: "invalid Bech32 prefix", - }, - { - name: "should fail - invalid bech32 string", - input: "invalidvalidatoraddress", - expectedErr: "decoding bech32 failed", - }, - { - name: "should fail - empty address", - input: "", - expectedErr: "empty address string is not allowed", - }, - } - - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - acc, val, err := parseValidatorAddress(tc.input) - - if tc.expectedErr != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expectedErr) - require.Nil(t, acc) - require.Nil(t, val) - return - } - - require.NoError(t, err) - // The account address is returned exactly as provided. - require.Equal(t, accAddr, acc.String()) - // The validator operator address is derived from the same key. - require.Equal(t, valAddr, val.String()) - // Both encodings share the exact same underlying bytes. - require.Equal(t, acc.Bytes(), val.Bytes()) - }) - } -} diff --git a/x/poa/types/message_add_validator.go b/x/poa/types/message_add_validator.go index 8306917..1b5ea59 100644 --- a/x/poa/types/message_add_validator.go +++ b/x/poa/types/message_add_validator.go @@ -4,11 +4,13 @@ import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) var ( _ sdk.Msg = &MsgAddValidator{} + _ sdk.HasValidateBasic = &MsgAddValidator{} _ codectypes.UnpackInterfacesMessage = (*MsgAddValidator)(nil) ) @@ -28,6 +30,23 @@ func NewMsgAddValidator(authority string, address string, pubKey cryptotypes.Pub }, nil } +// ValidateBasic performs stateless validation of the message fields. +func (msg *MsgAddValidator) ValidateBasic() error { + if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil { + return sdkerrors.ErrInvalidAddress.Wrapf("invalid authority address: %s", err) + } + if _, err := sdk.AccAddressFromBech32(msg.ValidatorAddress); err != nil { + return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) + } + if msg.Pubkey == nil { + return sdkerrors.ErrInvalidPubKey.Wrap("validator pubkey is required") + } + if _, ok := msg.Pubkey.GetCachedValue().(cryptotypes.PubKey); !ok { + return sdkerrors.ErrInvalidPubKey.Wrapf("expecting cryptotypes.PubKey, got %T", msg.Pubkey.GetCachedValue()) + } + return nil +} + // UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces func (msg *MsgAddValidator) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { var pubKey cryptotypes.PubKey diff --git a/x/poa/types/message_add_validator_test.go b/x/poa/types/message_add_validator_test.go new file mode 100644 index 0000000..6ec617a --- /dev/null +++ b/x/poa/types/message_add_validator_test.go @@ -0,0 +1,64 @@ +package types + +import ( + "testing" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) + +func TestMsgAddValidator_ValidateBasic(t *testing.T) { + validAddr := sdk.AccAddress("12345678901234567890").String() + + validPkAny, err := codectypes.NewAnyWithValue(ed25519.GenPrivKey().PubKey()) + require.NoError(t, err) + + wrongTypeAny, err := codectypes.NewAnyWithValue(&MsgRemoveValidator{}) + require.NoError(t, err) + + tt := []struct { + name string + msg *MsgAddValidator + expectedErr string + }{ + { + name: "should pass - valid message", + msg: &MsgAddValidator{Authority: validAddr, ValidatorAddress: validAddr, Pubkey: validPkAny}, + }, + { + name: "should fail - invalid authority address", + msg: &MsgAddValidator{Authority: "invalid", ValidatorAddress: validAddr, Pubkey: validPkAny}, + expectedErr: "invalid authority address", + }, + { + name: "should fail - invalid validator address", + msg: &MsgAddValidator{Authority: validAddr, ValidatorAddress: "invalid", Pubkey: validPkAny}, + expectedErr: "invalid validator address", + }, + { + name: "should fail - nil pubkey", + msg: &MsgAddValidator{Authority: validAddr, ValidatorAddress: validAddr, Pubkey: nil}, + expectedErr: "validator pubkey is required", + }, + { + name: "should fail - pubkey is not a cryptotypes.PubKey", + msg: &MsgAddValidator{Authority: validAddr, ValidatorAddress: validAddr, Pubkey: wrongTypeAny}, + expectedErr: "expecting cryptotypes.PubKey", + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + err := tc.msg.ValidateBasic() + + if tc.expectedErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErr) + return + } + require.NoError(t, err) + }) + } +} diff --git a/x/poa/types/message_remove_validator.go b/x/poa/types/message_remove_validator.go index 85af3f2..bc7bf8b 100644 --- a/x/poa/types/message_remove_validator.go +++ b/x/poa/types/message_remove_validator.go @@ -2,9 +2,13 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -var _ sdk.Msg = &MsgRemoveValidator{} +var ( + _ sdk.Msg = &MsgRemoveValidator{} + _ sdk.HasValidateBasic = &MsgRemoveValidator{} +) func NewMsgRemoveValidator(authority string, address string) *MsgRemoveValidator { return &MsgRemoveValidator{ @@ -12,3 +16,14 @@ func NewMsgRemoveValidator(authority string, address string) *MsgRemoveValidator ValidatorAddress: address, } } + +// ValidateBasic performs stateless validation of the message fields. +func (msg *MsgRemoveValidator) ValidateBasic() error { + if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil { + return sdkerrors.ErrInvalidAddress.Wrapf("invalid authority address: %s", err) + } + if _, err := sdk.AccAddressFromBech32(msg.ValidatorAddress); err != nil { + return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) + } + return nil +} diff --git a/x/poa/types/message_remove_validator_test.go b/x/poa/types/message_remove_validator_test.go new file mode 100644 index 0000000..2306ce0 --- /dev/null +++ b/x/poa/types/message_remove_validator_test.go @@ -0,0 +1,64 @@ +package types + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) + +func TestMsgRemoveValidator_ValidateBasic(t *testing.T) { + validAddr := sdk.AccAddress("12345678901234567890").String() + + tt := []struct { + name string + authority string + validator string + expectedErr string + }{ + { + name: "should pass - valid authority and validator address", + authority: validAddr, + validator: validAddr, + }, + { + name: "should fail - invalid authority address", + authority: "invalid", + validator: validAddr, + expectedErr: "invalid authority address", + }, + { + name: "should fail - empty authority address", + authority: "", + validator: validAddr, + expectedErr: "invalid authority address", + }, + { + name: "should fail - invalid validator address", + authority: validAddr, + validator: "invalid", + expectedErr: "invalid validator address", + }, + { + name: "should fail - empty validator address", + authority: validAddr, + validator: "", + expectedErr: "invalid validator address", + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + msg := NewMsgRemoveValidator(tc.authority, tc.validator) + + err := msg.ValidateBasic() + + if tc.expectedErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErr) + return + } + require.NoError(t, err) + }) + } +}