refactor(poa)!: parse validator address as AccAddress

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:    sdktypes.AccAddress(randomValAddr).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)

x/poa/keeper/keeper.go

@@ -85,10 +85,10 @@ func (k Keeper) GetAuthority() string {
 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)
 	if err != nil {
 		return err
 	}
+	valAddress := sdk.ValAddress(accAddress)
 	params, err := k.sk.GetParams(ctx)
 	if err != nil {
 		return err
@@ -207,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 := sdk.ValAddressFromBech32(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
@@ -272,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
 	}

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{

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",

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

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)
+		})
+	}
+}

x/poa/types/message_remove_validator.go

@@ -2,13 +2,28 @@ 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{
 		Authority:        authority,
 		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
+}

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)
+		})
+	}
+}