test: poa cover staking hook error propagation in MsgServer remove

x/poa/keeper/common_test.go

@@ -22,6 +22,7 @@ import (
 const (
 	accountAddressPrefix = "ethm"
 	bip44CoinType        = 60
+	poaAuthority         = "ethm1wunfhl05vc8r8xxnnp8gt62wa54r6y52pg03zq"
 )
 
 func setupSdkConfig() {
@@ -82,7 +83,7 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety
 		msr,
 		bankKeeper,
 		stakingKeeper,
-		"ethm1wunfhl05vc8r8xxnnp8gt62wa54r6y52pg03zq",
+		poaAuthority,
 	)
 	poaKeeper.SetParams(ctx, types.DefaultParams())
 	types.RegisterMsgServer(msr, NewMsgServerImpl(*poaKeeper))

x/poa/keeper/keeper_test.go

@@ -14,42 +14,6 @@ import (
 	"github.com/xrplevm/node/v10/x/poa/types"
 )
 
-func poaKeeperTestSetup(t *testing.T) (*Keeper, sdk.Context) {
-	stakingExpectations := func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) {
-		stakingHooks := testutil.NewMockStakingHooks(gomock.NewController(t))
-		stakingHooks.EXPECT().BeforeValidatorModified(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-		stakingHooks.EXPECT().BeforeValidatorSlashed(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-
-		stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{
-			BondDenom:     "XRP",
-			MaxValidators: 32,
-		}, nil).AnyTimes()
-		stakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return(stakingtypes.Validator{Tokens: math.NewInt(0)}, nil).AnyTimes()
-		stakingKeeper.EXPECT().GetAllDelegatorDelegations(ctx, gomock.Any()).Return([]stakingtypes.Delegation{}, nil).AnyTimes()
-		stakingKeeper.EXPECT().GetUnbondingDelegationsFromValidator(ctx, gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil).AnyTimes()
-		stakingKeeper.EXPECT().SlashUnbondingDelegation(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(math.ZeroInt(), nil).AnyTimes()
-		stakingKeeper.EXPECT().RemoveDelegation(ctx, gomock.Any()).Return(nil).AnyTimes()
-		stakingKeeper.EXPECT().RemoveValidatorTokensAndShares(ctx, gomock.Any(), gomock.Any()).Return(stakingtypes.Validator{Tokens: math.NewInt(0), Status: stakingtypes.Bonded}, math.ZeroInt(), nil).AnyTimes()
-		stakingKeeper.EXPECT().RemoveValidatorTokens(ctx, gomock.Any(), gomock.Any()).Return(stakingtypes.Validator{Tokens: math.NewInt(0), Status: stakingtypes.Bonded}, nil).AnyTimes()
-		stakingKeeper.EXPECT().BondDenom(ctx).Return("XRP", nil).AnyTimes()
-		stakingKeeper.EXPECT().Unbond(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(math.ZeroInt(), nil).AnyTimes()
-		stakingKeeper.EXPECT().Hooks().Return(stakingHooks).AnyTimes()
-		stakingKeeper.EXPECT().GetAllValidators(ctx).Return([]stakingtypes.Validator{}, nil).AnyTimes()
-	}
-
-	bankExpectations := func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper) {
-		bankKeeper.EXPECT().GetBalance(ctx, gomock.Any(), gomock.Any()).Return(sdk.Coin{
-			Amount: math.NewInt(0),
-		}).AnyTimes()
-		bankKeeper.EXPECT().MintCoins(ctx, gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-		bankKeeper.EXPECT().SendCoinsFromModuleToAccount(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-		bankKeeper.EXPECT().BurnCoins(ctx, stakingtypes.BondedPoolName, gomock.Any()).Return(nil).AnyTimes()
-		bankKeeper.EXPECT().SendCoinsFromAccountToModule(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-	}
-
-	return setupPoaKeeper(t, stakingExpectations, bankExpectations)
-}
-
 // Define here Keeper methods to be unit tested
 func TestKeeper_ExecuteAddValidator(t *testing.T) {
 	ctrl := gomock.NewController(t)

x/poa/keeper/msg_server_add_validator_test.go

@@ -4,7 +4,9 @@ import (
 	"errors"
 	"testing"
 
+	"cosmossdk.io/math"
 	types1 "github.com/cosmos/cosmos-sdk/codec/types"
+	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"
@@ -14,40 +16,64 @@ import (
 )
 
 func TestMsgServer_AddValidator(t *testing.T) {
-	poaKeeper, ctx := poaKeeperTestSetup(t)
-
 	ctrl := gomock.NewController(t)
 	pubKey := testutil.NewMockPubKey(ctrl)
 	msgPubKey, _ := types1.NewAnyWithValue(pubKey)
-	msgServer := NewMsgServerImpl(*poaKeeper)
 
 	tt := []struct {
 		name             string
 		authority        string
 		validatorAddress string
+		stakingMocks     func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper)
+		bankMocks        func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper)
 		expectedErr      error
 	}{
 		{
 			name:             "should fail - invalid authority address",
 			authority:        "invalidauthority",
 			validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp",
+			stakingMocks:     func(_ sdk.Context, _ *testutil.MockStakingKeeper) {},
+			bankMocks:        func(_ sdk.Context, _ *testutil.MockBankKeeper) {},
 			expectedErr:      govtypes.ErrInvalidSigner,
 		},
 		{
 			name:             "should fail - invalid validator address",
-			authority:        poaKeeper.GetAuthority(),
+			authority:        poaAuthority,
 			validatorAddress: "invalidvalidatoraddress",
+			stakingMocks:     func(_ sdk.Context, _ *testutil.MockStakingKeeper) {},
+			bankMocks:        func(_ sdk.Context, _ *testutil.MockBankKeeper) {},
 			expectedErr:      errors.New("decoding bech32 failed"),
 		},
 		{
 			name:             "should pass",
-			authority:        poaKeeper.GetAuthority(),
+			authority:        poaAuthority,
 			validatorAddress: "ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp",
+			stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) {
+				stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{
+					BondDenom:     "BND",
+					MaxValidators: 2,
+				}, nil)
+				stakingKeeper.EXPECT().GetAllValidators(ctx).Return([]stakingtypes.Validator{}, nil)
+				stakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return(stakingtypes.Validator{Tokens: math.NewInt(0)}, nil)
+				stakingKeeper.EXPECT().GetAllDelegatorDelegations(ctx, gomock.Any()).Return([]stakingtypes.Delegation{}, nil)
+				stakingKeeper.EXPECT().GetUnbondingDelegationsFromValidator(ctx, gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil)
+			},
+			bankMocks: func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper) {
+				bankKeeper.EXPECT().GetBalance(ctx, gomock.Any(), gomock.Any()).Return(sdk.Coin{
+					Denom:  "BND",
+					Amount: math.NewInt(0),
+				})
+				bankKeeper.EXPECT().MintCoins(ctx, gomock.Any(), gomock.Any()).Return(nil)
+				bankKeeper.EXPECT().SendCoinsFromModuleToAccount(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
+			},
 		},
 	}
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
+			poaKeeper, ctx := setupPoaKeeper(t, tc.stakingMocks, tc.bankMocks)
+			msgServer := NewMsgServerImpl(*poaKeeper)
+
 			msg := &types.MsgAddValidator{
 				Authority:        tc.authority,
 				ValidatorAddress: tc.validatorAddress,

x/poa/keeper/msg_server_remove_validator_test.go

@@ -1,46 +1,145 @@
 package keeper
 
 import (
+	"bytes"
 	"errors"
 	"testing"
 
+	"cosmossdk.io/log"
+	"cosmossdk.io/math"
+	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"
 )
 
 func TestMsgServer_RemoveValidator(t *testing.T) {
-	poaKeeper, ctx := poaKeeperTestSetup(t)
-
-	msgServer := NewMsgServerImpl(*poaKeeper)
+	ctrl := gomock.NewController(t)
 
 	tt := []struct {
 		name             string
 		authority        string
 		validatorAddress string
+		stakingMocks     func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper)
+		bankMocks        func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper)
 		expectedErr      error
+		expectedLog      string
 	}{
 		{
 			name:             "should fail - invalid authority address",
 			authority:        "invalidauthority",
 			validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu",
+			stakingMocks:     func(_ sdk.Context, _ *testutil.MockStakingKeeper) {},
+			bankMocks:        func(_ sdk.Context, _ *testutil.MockBankKeeper) {},
 			expectedErr:      govtypes.ErrInvalidSigner,
 		},
 		{
 			name:             "should fail - invalid validator address",
-			authority:        poaKeeper.GetAuthority(),
+			authority:        poaAuthority,
 			validatorAddress: "invalidvalidatoraddress",
+			stakingMocks:     func(_ sdk.Context, _ *testutil.MockStakingKeeper) {},
+			bankMocks:        func(_ sdk.Context, _ *testutil.MockBankKeeper) {},
 			expectedErr:      errors.New("decoding bech32 failed"),
 		},
 		{
 			name:             "should pass",
-			authority:        poaKeeper.GetAuthority(),
+			authority:        poaAuthority,
+			validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu",
+			stakingMocks: func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) {
+				stakingKeeper.EXPECT().GetParams(ctx).Return(stakingtypes.Params{
+					BondDenom: "BND",
+				}, nil)
+				stakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return(stakingtypes.Validator{
+					Tokens: math.NewInt(0),
+				}, nil)
+				stakingKeeper.EXPECT().GetUnbondingDelegationsFromValidator(ctx, gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil)
+
+				hooks := testutil.NewMockStakingHooks(ctrl)
+				hooks.EXPECT().BeforeValidatorModified(ctx, gomock.Any()).Return(nil)
+				stakingKeeper.EXPECT().Hooks().Return(hooks)
+
+				stakingKeeper.EXPECT().RemoveValidatorTokens(ctx, gomock.Any(), gomock.Any()).Return(
+					stakingtypes.Validator{Status: stakingtypes.Bonded}, nil,
+				)
+				stakingKeeper.EXPECT().Unbond(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(math.ZeroInt(), nil)
+			},
+			bankMocks: func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper) {
+				bankKeeper.EXPECT().BurnCoins(ctx, stakingtypes.BondedPoolName, gomock.Any()).Return(nil)
+			},
+		},
+		{
+			name:             "should pass - BeforeValidatorModified hook error is swallowed and logged",
+			authority:        poaAuthority,
+			validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu",
+			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.
+				stakingKeeper.EXPECT().GetParams(gomock.Any()).Return(stakingtypes.Params{
+					BondDenom: "BND",
+				}, nil)
+				stakingKeeper.EXPECT().GetValidator(gomock.Any(), gomock.Any()).Return(stakingtypes.Validator{
+					Tokens: math.NewInt(0),
+				}, nil)
+				stakingKeeper.EXPECT().GetUnbondingDelegationsFromValidator(gomock.Any(), gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil)
+
+				hooks := testutil.NewMockStakingHooks(ctrl)
+				hooks.EXPECT().BeforeValidatorModified(gomock.Any(), gomock.Any()).Return(errors.New("hook failure"))
+				stakingKeeper.EXPECT().Hooks().Return(hooks)
+
+				stakingKeeper.EXPECT().RemoveValidatorTokens(gomock.Any(), gomock.Any(), gomock.Any()).Return(
+					stakingtypes.Validator{Status: stakingtypes.Bonded}, nil,
+				)
+				stakingKeeper.EXPECT().Unbond(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(math.ZeroInt(), nil)
+			},
+			bankMocks: func(_ sdk.Context, bankKeeper *testutil.MockBankKeeper) {
+				bankKeeper.EXPECT().BurnCoins(gomock.Any(), stakingtypes.BondedPoolName, gomock.Any()).Return(nil)
+			},
+			expectedLog: "failed to call before validator modified hook",
+		},
+		{
+			name:             "should pass - BeforeValidatorSlashed hook error is swallowed and logged",
+			authority:        poaAuthority,
 			validatorAddress: "ethmvaloper1a0pd5cyew47pvgf7rd7axxy3humv9ev0urudmu",
+			stakingMocks: func(_ sdk.Context, stakingKeeper *testutil.MockStakingKeeper) {
+				stakingKeeper.EXPECT().GetParams(gomock.Any()).Return(stakingtypes.Params{
+					BondDenom: "BND",
+				}, nil)
+				stakingKeeper.EXPECT().GetValidator(gomock.Any(), gomock.Any()).Return(stakingtypes.Validator{
+					Tokens: sdk.DefaultPowerReduction,
+				}, nil)
+				stakingKeeper.EXPECT().GetUnbondingDelegationsFromValidator(gomock.Any(), gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil)
+
+				hooks := testutil.NewMockStakingHooks(ctrl)
+				hooks.EXPECT().BeforeValidatorModified(gomock.Any(), gomock.Any()).Return(nil)
+				hooks.EXPECT().BeforeValidatorSlashed(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("hook failure"))
+				stakingKeeper.EXPECT().Hooks().Return(hooks).Times(2)
+
+				stakingKeeper.EXPECT().RemoveValidatorTokens(gomock.Any(), gomock.Any(), gomock.Any()).Return(
+					stakingtypes.Validator{Status: stakingtypes.Bonded}, nil,
+				)
+				stakingKeeper.EXPECT().Unbond(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(math.ZeroInt(), nil)
+			},
+			bankMocks: func(_ sdk.Context, bankKeeper *testutil.MockBankKeeper) {
+				bankKeeper.EXPECT().BurnCoins(gomock.Any(), stakingtypes.BondedPoolName, gomock.Any()).Return(nil)
+			},
+			expectedLog: "failed to call before validator slashed hook",
 		},
 	}
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
+			poaKeeper, ctx := setupPoaKeeper(t, tc.stakingMocks, tc.bankMocks)
+
+			var logBuf bytes.Buffer
+			if tc.expectedLog != "" {
+				ctx = ctx.WithLogger(log.NewLogger(&logBuf, log.OutputJSONOption()))
+			}
+
+			msgServer := NewMsgServerImpl(*poaKeeper)
+
 			msg := &types.MsgRemoveValidator{
 				Authority:        tc.authority,
 				ValidatorAddress: tc.validatorAddress,
@@ -53,6 +152,9 @@ func TestMsgServer_RemoveValidator(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 			}
+			if tc.expectedLog != "" {
+				require.Contains(t, logBuf.String(), tc.expectedLog)
+			}
 		})
 	}
 }