From 157a86407aafb54f88b1c86071d3c984e9b595c5 Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 11:41:30 +0100 Subject: [PATCH 1/7] fix: IF-FINDING-001 Invariant checks through crisis module may go unnoticed --- app/app.go | 1 - x/poa/keeper/invariants.go | 115 ------------------------------------- x/poa/keeper/keeper.go | 3 - 3 files changed, 119 deletions(-) delete mode 100644 x/poa/keeper/invariants.go diff --git a/app/app.go b/app/app.go index fd62559..2d0b648 100644 --- a/app/app.go +++ b/app/app.go @@ -459,7 +459,6 @@ func New( app.MsgServiceRouter(), app.BankKeeper, app.StakingKeeper, - app.SlashingKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) diff --git a/x/poa/keeper/invariants.go b/x/poa/keeper/invariants.go deleted file mode 100644 index 6daf0a0..0000000 --- a/x/poa/keeper/invariants.go +++ /dev/null @@ -1,115 +0,0 @@ -package keeper - -import ( - "fmt" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/xrplevm/node/v6/x/poa/types" -) - -// RegisterInvariants registers all module invariants -func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { - ir.RegisterRoute(types.ModuleName, "staking-power", StakingPowerInvariant(k)) - ir.RegisterRoute(types.ModuleName, "self-delegation", SelfDelegationInvariant(k)) - ir.RegisterRoute(types.ModuleName, "keeper-dependencies-params", CheckKeeperDependenciesParamsInvariant(k)) -} - -// StakingPowerInvariant checks that all validators have the same -// staking power as the default power reduction. If not, it returns an invariant error. -func StakingPowerInvariant(k Keeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - var ( - msg string - broken bool - ) - - validators, err := k.sk.GetAllValidators(ctx) - if err != nil { - panic(err) - } - - for _, validator := range validators { - if !validator.Tokens.Equal(sdk.DefaultPowerReduction) && !validator.Tokens.IsZero() { - msg = fmt.Sprintf("excessive staking power for account %s: %s", validator.GetOperator(), validator.Tokens.String()) - broken = true - break - } - } - - return sdk.FormatInvariant( - types.ModuleName, - "staking-power-invariant", - fmt.Sprintf("excessive staking power for account %s", msg), - ), broken - } -} - -// SelfDelegationInvariant checks that all validators have only one self-delegation. -// Each delegation has to match the delegator and validator address. -func SelfDelegationInvariant(k Keeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - var ( - msg string - broken bool - ) - - delegations, err := k.sk.GetAllDelegations(ctx) - if err != nil { - panic(err) - } - for _, delegation := range delegations { - validatorAddress, err := sdk.ValAddressFromBech32(delegation.GetValidatorAddr()) - if err != nil { - panic(err) - } - delegatorAddress, err := sdk.AccAddressFromBech32(delegation.GetDelegatorAddr()) - if err != nil { - panic(err) - } - if !sdk.AccAddress(validatorAddress).Equals(delegatorAddress) { - msg = fmt.Sprintf("validator address %s and delegation address do not match %s", sdk.AccAddress(validatorAddress), delegatorAddress) - broken = true - break - } - } - - return sdk.FormatInvariant( - types.ModuleName, - "self-delegation-invariant", - fmt.Sprintf("invalid validator self-delegation %s", msg), - ), broken - } -} - -// CheckKeeperDependenciesParamsInvariant checks that keeper dependencies params -// are set to the expected values. -// This is to ensure that the keeper dependencies are correctly initialized. -// Slashing params SlashFractionDoubleSign and SlashFractionDowntime should be zero. -func CheckKeeperDependenciesParamsInvariant(k Keeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - var ( - msg string - broken bool - ) - - params, err := k.ck.GetParams(ctx) - if err != nil { - panic(err) - } - - if !(params.SlashFractionDoubleSign.IsZero() && params.SlashFractionDowntime.IsZero()) { - msg = fmt.Sprintf( - "slashing params are not zero: slash_fraction_double_sign %s, slash_fraction_downtime %s", - params.SlashFractionDoubleSign.String(), - params.SlashFractionDowntime.String(), - ) - broken = true - } - - return sdk.FormatInvariant( - types.ModuleName, - "slashing-params-invariant", - fmt.Sprintf("slashing params are not zero: %s", msg), - ), broken - } -} diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index fb07adb..bde50eb 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -28,7 +28,6 @@ type ( router *baseapp.MsgServiceRouter // Msg server router bk types.BankKeeper sk types.StakingKeeper - ck types.SlashingKeeper } ) @@ -38,7 +37,6 @@ func NewKeeper( router *baseapp.MsgServiceRouter, bk types.BankKeeper, sk types.StakingKeeper, - ck types.SlashingKeeper, authority string, ) *Keeper { // set KeyTable if it has not already been set @@ -58,7 +56,6 @@ func NewKeeper( router: router, bk: bk, sk: sk, - ck: ck, } } From a2e7e9866d0cc158704d6c2c206f5bf3c4965aaf Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 11:58:27 +0100 Subject: [PATCH 2/7] fix: IF-FINDING-001 Invariant checks through crisis module may go unnoticed --- x/poa/module.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x/poa/module.go b/x/poa/module.go index d6c64d6..6d73d48 100644 --- a/x/poa/module.go +++ b/x/poa/module.go @@ -127,9 +127,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { } // RegisterInvariants registers the invariants of the module. If an invariant deviates from its predicted value, the InvariantRegistry triggers appropriate logic (most often the chain will be halted) -func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { - keeper.RegisterInvariants(ir, am.keeper) -} +func (am AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} // InitGenesis performs the module's genesis initialization. It returns no validator updates. func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, gs json.RawMessage) { From 2925f5ab258297aaec8bf0e8759afe5c6ff7c563 Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 11:59:58 +0100 Subject: [PATCH 3/7] fix: IF-FINDING-005 Messages MsgDelegate and MsgCancelUnbondingDelegation should be disabled --- app/ante.go | 2 ++ app/app.go | 1 - x/poa/ante/poa.go | 4 ++- x/poa/keeper/hooks.go | 71 ------------------------------------------- 4 files changed, 5 insertions(+), 73 deletions(-) delete mode 100644 x/poa/keeper/hooks.go diff --git a/app/ante.go b/app/ante.go index ee6666a..5a257c8 100644 --- a/app/ante.go +++ b/app/ante.go @@ -33,6 +33,8 @@ func NewAnteHandlerOptionsFromApp(app *App, txConfig client.TxConfig, maxGasWant AuthzDisabledMsgTypes: []string{ sdk.MsgTypeURL(&stakingtypes.MsgUndelegate{}), sdk.MsgTypeURL(&stakingtypes.MsgBeginRedelegate{}), + sdk.MsgTypeURL(&stakingtypes.MsgCancelUnbondingDelegation{}), + sdk.MsgTypeURL(&stakingtypes.MsgDelegate{}), }, } } diff --git a/app/app.go b/app/app.go index 2d0b648..6917742 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 diff --git a/x/poa/ante/poa.go b/x/poa/ante/poa.go index 94fb39c..a0baf76 100644 --- a/x/poa/ante/poa.go +++ b/x/poa/ante/poa.go @@ -17,7 +17,9 @@ func (cbd PoaDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne // loop through all the messages and check if the message type is allowed for _, msg := range tx.GetMsgs() { if sdk.MsgTypeURL(msg) == sdk.MsgTypeURL(&stakingtypes.MsgUndelegate{}) || - sdk.MsgTypeURL(msg) == sdk.MsgTypeURL(&stakingtypes.MsgBeginRedelegate{}) { + sdk.MsgTypeURL(msg) == sdk.MsgTypeURL(&stakingtypes.MsgBeginRedelegate{}) || + sdk.MsgTypeURL(msg) == sdk.MsgTypeURL(&stakingtypes.MsgDelegate{}) || + sdk.MsgTypeURL(msg) == sdk.MsgTypeURL(&stakingtypes.MsgCancelUnbondingDelegation{}) { return ctx, errors.New("tx type not allowed") } } diff --git a/x/poa/keeper/hooks.go b/x/poa/keeper/hooks.go deleted file mode 100644 index 8e9199d..0000000 --- a/x/poa/keeper/hooks.go +++ /dev/null @@ -1,71 +0,0 @@ -package keeper - -import ( - "context" - "errors" - - "cosmossdk.io/math" - - sdk "github.com/cosmos/cosmos-sdk/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" -) - -func (h Hooks) BeforeDelegationCreated(_ context.Context, accAddr sdk.AccAddress, valAddr sdk.ValAddress) error { - accValAddr := sdk.ValAddress(accAddr) - if !accValAddr.Equals(valAddr) { - return errors.New("delegation to other accounts is not allowed") - } - return nil -} - -// Hooks wrapper struct for slashing keeper -type Hooks struct { - k Keeper -} - -var _ stakingtypes.StakingHooks = Hooks{} - -// Return the wrapper struct -func (k Keeper) Hooks() Hooks { - return Hooks{k} -} - -func (h Hooks) AfterValidatorBonded(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) AfterValidatorRemoved(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) AfterValidatorCreated(_ context.Context, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) AfterValidatorBeginUnbonding(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) BeforeValidatorModified(_ context.Context, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) BeforeDelegationSharesModified(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) BeforeDelegationRemoved(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) AfterDelegationModified(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { - return nil -} - -func (h Hooks) AfterUnbondingInitiated(_ context.Context, _ uint64) error { - return nil -} - -func (h Hooks) BeforeValidatorSlashed(_ context.Context, _ sdk.ValAddress, _ math.LegacyDec) error { - return nil -} From 5a0179ed0b01de0bb4c14fce95f3598f4c7a3396 Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 12:18:26 +0100 Subject: [PATCH 4/7] fix: update tests --- x/poa/keeper/common_test.go | 15 +--- x/poa/keeper/hooks_test.go | 1 - x/poa/keeper/invariants_test.go | 136 -------------------------------- x/poa/keeper/keeper_test.go | 4 +- 4 files changed, 4 insertions(+), 152 deletions(-) delete mode 100644 x/poa/keeper/invariants_test.go diff --git a/x/poa/keeper/common_test.go b/x/poa/keeper/common_test.go index d1a8939..107ce1e 100644 --- a/x/poa/keeper/common_test.go +++ b/x/poa/keeper/common_test.go @@ -54,13 +54,6 @@ func getBankKeeperMock(t *testing.T, ctx sdk.Context, setExpectations func(ctx s return bankKeeper } -func getSlashingKeeperMock(t *testing.T, ctx sdk.Context, setExpectations func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper)) *testutil.MockSlashingKeeper { - ctrl := gomock.NewController(t) - slashingKeeper := testutil.NewMockSlashingKeeper(ctrl) - setExpectations(ctx, slashingKeeper) - return slashingKeeper -} - func getCtxMock(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey) sdk.Context { setupSdkConfig() @@ -69,7 +62,7 @@ func getCtxMock(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.Tran return ctx } -func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey, ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper, bankKeeper *testutil.MockBankKeeper, slashingKeeper *testutil.MockSlashingKeeper) *Keeper { +func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey, ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper, bankKeeper *testutil.MockBankKeeper) *Keeper { encCfg := moduletestutil.MakeTestEncodingConfig() types.RegisterInterfaces(encCfg.InterfaceRegistry) @@ -89,7 +82,6 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety msr, bankKeeper, stakingKeeper, - slashingKeeper, "ethm1wunfhl05vc8r8xxnnp8gt62wa54r6y52pg03zq", ) poaKeeper.SetParams(ctx, types.DefaultParams()) @@ -99,14 +91,13 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety return poaKeeper } -func setupPoaKeeper(t *testing.T, setStakingExpectations func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper), setBankExpectations func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper), setSlashingExpectations func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper)) (*Keeper, sdk.Context) { +func setupPoaKeeper(t *testing.T, setStakingExpectations func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper), setBankExpectations func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper)) (*Keeper, sdk.Context) { key := storetypes.NewKVStoreKey(types.StoreKey) tsKey := storetypes.NewTransientStoreKey("test") ctx := getCtxMock(t, key, tsKey) stakingKeeper := getStakingKeeperMock(t, ctx, setStakingExpectations) bankKeeper := getBankKeeperMock(t, ctx, setBankExpectations) - slashingKeeper := getSlashingKeeperMock(t, ctx, setSlashingExpectations) - return getMockedPoAKeeper(t, key, tsKey, ctx, stakingKeeper, bankKeeper, slashingKeeper), ctx + return getMockedPoAKeeper(t, key, tsKey, ctx, stakingKeeper, bankKeeper), ctx } diff --git a/x/poa/keeper/hooks_test.go b/x/poa/keeper/hooks_test.go index 16ce9e1..09f4c29 100644 --- a/x/poa/keeper/hooks_test.go +++ b/x/poa/keeper/hooks_test.go @@ -15,7 +15,6 @@ func TestPoA_Hooks(t *testing.T) { t, func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, ) hooks := keeper.Hooks() diff --git a/x/poa/keeper/invariants_test.go b/x/poa/keeper/invariants_test.go deleted file mode 100644 index a529c68..0000000 --- a/x/poa/keeper/invariants_test.go +++ /dev/null @@ -1,136 +0,0 @@ -package keeper - -import ( - "testing" - - "cosmossdk.io/math" - sdk "github.com/cosmos/cosmos-sdk/types" - slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/stretchr/testify/require" - "github.com/xrplevm/node/v6/x/poa/testutil" -) - -func TestStakingPowerInvariant_Valid(t *testing.T) { - poaKeeper, ctx := setupPoaKeeper( - t, - func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { - stakingKeeper.EXPECT().GetAllValidators(ctx).Return([]stakingtypes.Validator{ - { - Tokens: sdk.DefaultPowerReduction, - }, - { - Tokens: math.ZeroInt(), - }, - }, nil) - }, - func(sdk.Context, *testutil.MockBankKeeper) {}, - func(sdk.Context, *testutil.MockSlashingKeeper) {}, - ) - - invariant := StakingPowerInvariant(*poaKeeper) - msg, broken := invariant(ctx) - require.False(t, broken, msg) -} - -func TestStakingPowerInvariant_Invalid(t *testing.T) { - poaKeeper, ctx := setupPoaKeeper( - t, - func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { - stakingKeeper.EXPECT().GetAllValidators(ctx).Return([]stakingtypes.Validator{ - { - Tokens: sdk.DefaultPowerReduction, - }, - { - Tokens: sdk.DefaultPowerReduction.Add(math.OneInt()), - }, - }, nil) - }, - func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, - ) - - invariant := StakingPowerInvariant(*poaKeeper) - msg, broken := invariant(ctx) - require.True(t, broken, msg) -} - -func TestSelfDelegationInvariant_Valid(t *testing.T) { - poaKeeper, ctx := setupPoaKeeper( - t, - func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { - stakingKeeper.EXPECT().GetAllDelegations(ctx).Return([]stakingtypes.Delegation{ - { - DelegatorAddress: "ethm13ued6aqj3w7jvks4l270dunhue0a9y7tspnpn5", - ValidatorAddress: "ethmvaloper13ued6aqj3w7jvks4l270dunhue0a9y7tl3edtf", - }, - { - DelegatorAddress: "ethm13ued6aqj3w7jvks4l270dunhue0a9y7tspnpn5", - ValidatorAddress: "ethmvaloper13ued6aqj3w7jvks4l270dunhue0a9y7tl3edtf", - }, - }, nil) - }, - func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, - ) - - invariant := SelfDelegationInvariant(*poaKeeper) - msg, broken := invariant(ctx) - require.False(t, broken, msg) -} - -func TestSelfDelegationInvariant_Invalid(t *testing.T) { - poaKeeper, ctx := setupPoaKeeper( - t, - func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper) { - stakingKeeper.EXPECT().GetAllDelegations(ctx).Return([]stakingtypes.Delegation{ - { - DelegatorAddress: "ethm1wunfhl05vc8r8xxnnp8gt62wa54r6y52pg03zq", - ValidatorAddress: "ethmvaloper13ued6aqj3w7jvks4l270dunhue0a9y7tl3edtf", - }, - }, nil) - }, - func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, - ) - - invariant := SelfDelegationInvariant(*poaKeeper) - msg, broken := invariant(ctx) - require.True(t, broken, msg) -} - -func TestCheckKeeperDependenciesParamsInvariant_Valid(t *testing.T) { - poaKeeper, ctx := setupPoaKeeper( - t, - func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, - func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper) { - slashingKeeper.EXPECT().GetParams(ctx).Return(slashingtypes.Params{ - SlashFractionDoubleSign: math.LegacyZeroDec(), - SlashFractionDowntime: math.LegacyZeroDec(), - }, nil) - }, - ) - - invariant := CheckKeeperDependenciesParamsInvariant(*poaKeeper) - msg, broken := invariant(ctx) - require.False(t, broken, msg) -} - -func TestCheckKeeperDependenciesParamsInvariant_Invalid(t *testing.T) { - poaKeeper, ctx := setupPoaKeeper( - t, - func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, - func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper) { - slashingKeeper.EXPECT().GetParams(ctx).Return(slashingtypes.Params{ - SlashFractionDoubleSign: math.LegacyNewDecWithPrec(5, 2), // 0.05 - SlashFractionDowntime: math.LegacyNewDecWithPrec(6, 1), // 0.6 (invalid, should be less than MinSignedPerWindow) - }, nil) - }, - ) - - invariant := CheckKeeperDependenciesParamsInvariant(*poaKeeper) - msg, broken := invariant(ctx) - require.True(t, broken, msg) -} diff --git a/x/poa/keeper/keeper_test.go b/x/poa/keeper/keeper_test.go index da5286f..63b9eb6 100644 --- a/x/poa/keeper/keeper_test.go +++ b/x/poa/keeper/keeper_test.go @@ -44,9 +44,7 @@ func poaKeeperTestSetup(t *testing.T) (*Keeper, sdk.Context) { bankKeeper.EXPECT().SendCoinsFromAccountToModule(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() } - slashingExpectations := func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {} - - return setupPoaKeeper(t, stakingExpectations, bankExpectations, slashingExpectations) + return setupPoaKeeper(t, stakingExpectations, bankExpectations) } // Define here Keeper methods to be unit tested From 9f4734f88cdc52e9602003b8d14836762c9d8ecc Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 12:22:26 +0100 Subject: [PATCH 5/7] fix: rollback changes from IF-FINDING-005 --- app/app.go | 2 +- x/poa/keeper/common_test.go | 15 ++++++++++++--- x/poa/keeper/hooks_test.go | 1 + x/poa/keeper/keeper.go | 3 +++ x/poa/keeper/keeper_test.go | 4 +++- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/app.go b/app/app.go index 2d0b648..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 @@ -459,6 +458,7 @@ func New( app.MsgServiceRouter(), app.BankKeeper, app.StakingKeeper, + app.SlashingKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) diff --git a/x/poa/keeper/common_test.go b/x/poa/keeper/common_test.go index 107ce1e..d1a8939 100644 --- a/x/poa/keeper/common_test.go +++ b/x/poa/keeper/common_test.go @@ -54,6 +54,13 @@ func getBankKeeperMock(t *testing.T, ctx sdk.Context, setExpectations func(ctx s return bankKeeper } +func getSlashingKeeperMock(t *testing.T, ctx sdk.Context, setExpectations func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper)) *testutil.MockSlashingKeeper { + ctrl := gomock.NewController(t) + slashingKeeper := testutil.NewMockSlashingKeeper(ctrl) + setExpectations(ctx, slashingKeeper) + return slashingKeeper +} + func getCtxMock(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey) sdk.Context { setupSdkConfig() @@ -62,7 +69,7 @@ func getCtxMock(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.Tran return ctx } -func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey, ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper, bankKeeper *testutil.MockBankKeeper) *Keeper { +func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey, ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper, bankKeeper *testutil.MockBankKeeper, slashingKeeper *testutil.MockSlashingKeeper) *Keeper { encCfg := moduletestutil.MakeTestEncodingConfig() types.RegisterInterfaces(encCfg.InterfaceRegistry) @@ -82,6 +89,7 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety msr, bankKeeper, stakingKeeper, + slashingKeeper, "ethm1wunfhl05vc8r8xxnnp8gt62wa54r6y52pg03zq", ) poaKeeper.SetParams(ctx, types.DefaultParams()) @@ -91,13 +99,14 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety return poaKeeper } -func setupPoaKeeper(t *testing.T, setStakingExpectations func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper), setBankExpectations func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper)) (*Keeper, sdk.Context) { +func setupPoaKeeper(t *testing.T, setStakingExpectations func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper), setBankExpectations func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper), setSlashingExpectations func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper)) (*Keeper, sdk.Context) { key := storetypes.NewKVStoreKey(types.StoreKey) tsKey := storetypes.NewTransientStoreKey("test") ctx := getCtxMock(t, key, tsKey) stakingKeeper := getStakingKeeperMock(t, ctx, setStakingExpectations) bankKeeper := getBankKeeperMock(t, ctx, setBankExpectations) + slashingKeeper := getSlashingKeeperMock(t, ctx, setSlashingExpectations) - return getMockedPoAKeeper(t, key, tsKey, ctx, stakingKeeper, bankKeeper), ctx + return getMockedPoAKeeper(t, key, tsKey, ctx, stakingKeeper, bankKeeper, slashingKeeper), ctx } diff --git a/x/poa/keeper/hooks_test.go b/x/poa/keeper/hooks_test.go index 09f4c29..16ce9e1 100644 --- a/x/poa/keeper/hooks_test.go +++ b/x/poa/keeper/hooks_test.go @@ -15,6 +15,7 @@ func TestPoA_Hooks(t *testing.T) { t, func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, + func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, ) hooks := keeper.Hooks() diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index bde50eb..fb07adb 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -28,6 +28,7 @@ type ( router *baseapp.MsgServiceRouter // Msg server router bk types.BankKeeper sk types.StakingKeeper + ck types.SlashingKeeper } ) @@ -37,6 +38,7 @@ func NewKeeper( router *baseapp.MsgServiceRouter, bk types.BankKeeper, sk types.StakingKeeper, + ck types.SlashingKeeper, authority string, ) *Keeper { // set KeyTable if it has not already been set @@ -56,6 +58,7 @@ func NewKeeper( router: router, bk: bk, sk: sk, + ck: ck, } } diff --git a/x/poa/keeper/keeper_test.go b/x/poa/keeper/keeper_test.go index 63b9eb6..da5286f 100644 --- a/x/poa/keeper/keeper_test.go +++ b/x/poa/keeper/keeper_test.go @@ -44,7 +44,9 @@ func poaKeeperTestSetup(t *testing.T) (*Keeper, sdk.Context) { bankKeeper.EXPECT().SendCoinsFromAccountToModule(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() } - return setupPoaKeeper(t, stakingExpectations, bankExpectations) + slashingExpectations := func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {} + + return setupPoaKeeper(t, stakingExpectations, bankExpectations, slashingExpectations) } // Define here Keeper methods to be unit tested From 25787305c0f9a861879d324c8f4f4d5e4254e337 Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 12:28:50 +0100 Subject: [PATCH 6/7] fix: rollback changes from IF-FINDING-005 --- app/app.go | 1 - x/poa/keeper/common_test.go | 15 +++------------ x/poa/keeper/hooks_test.go | 1 - x/poa/keeper/keeper.go | 3 --- x/poa/keeper/keeper_test.go | 4 +--- 5 files changed, 4 insertions(+), 20 deletions(-) diff --git a/app/app.go b/app/app.go index 9c937a7..6917742 100644 --- a/app/app.go +++ b/app/app.go @@ -458,7 +458,6 @@ func New( app.MsgServiceRouter(), app.BankKeeper, app.StakingKeeper, - app.SlashingKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) diff --git a/x/poa/keeper/common_test.go b/x/poa/keeper/common_test.go index d1a8939..107ce1e 100644 --- a/x/poa/keeper/common_test.go +++ b/x/poa/keeper/common_test.go @@ -54,13 +54,6 @@ func getBankKeeperMock(t *testing.T, ctx sdk.Context, setExpectations func(ctx s return bankKeeper } -func getSlashingKeeperMock(t *testing.T, ctx sdk.Context, setExpectations func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper)) *testutil.MockSlashingKeeper { - ctrl := gomock.NewController(t) - slashingKeeper := testutil.NewMockSlashingKeeper(ctrl) - setExpectations(ctx, slashingKeeper) - return slashingKeeper -} - func getCtxMock(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey) sdk.Context { setupSdkConfig() @@ -69,7 +62,7 @@ func getCtxMock(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.Tran return ctx } -func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey, ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper, bankKeeper *testutil.MockBankKeeper, slashingKeeper *testutil.MockSlashingKeeper) *Keeper { +func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storetypes.TransientStoreKey, ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper, bankKeeper *testutil.MockBankKeeper) *Keeper { encCfg := moduletestutil.MakeTestEncodingConfig() types.RegisterInterfaces(encCfg.InterfaceRegistry) @@ -89,7 +82,6 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety msr, bankKeeper, stakingKeeper, - slashingKeeper, "ethm1wunfhl05vc8r8xxnnp8gt62wa54r6y52pg03zq", ) poaKeeper.SetParams(ctx, types.DefaultParams()) @@ -99,14 +91,13 @@ func getMockedPoAKeeper(t *testing.T, key *storetypes.KVStoreKey, tsKey *storety return poaKeeper } -func setupPoaKeeper(t *testing.T, setStakingExpectations func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper), setBankExpectations func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper), setSlashingExpectations func(ctx sdk.Context, slashingKeeper *testutil.MockSlashingKeeper)) (*Keeper, sdk.Context) { +func setupPoaKeeper(t *testing.T, setStakingExpectations func(ctx sdk.Context, stakingKeeper *testutil.MockStakingKeeper), setBankExpectations func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper)) (*Keeper, sdk.Context) { key := storetypes.NewKVStoreKey(types.StoreKey) tsKey := storetypes.NewTransientStoreKey("test") ctx := getCtxMock(t, key, tsKey) stakingKeeper := getStakingKeeperMock(t, ctx, setStakingExpectations) bankKeeper := getBankKeeperMock(t, ctx, setBankExpectations) - slashingKeeper := getSlashingKeeperMock(t, ctx, setSlashingExpectations) - return getMockedPoAKeeper(t, key, tsKey, ctx, stakingKeeper, bankKeeper, slashingKeeper), ctx + return getMockedPoAKeeper(t, key, tsKey, ctx, stakingKeeper, bankKeeper), ctx } diff --git a/x/poa/keeper/hooks_test.go b/x/poa/keeper/hooks_test.go index 16ce9e1..09f4c29 100644 --- a/x/poa/keeper/hooks_test.go +++ b/x/poa/keeper/hooks_test.go @@ -15,7 +15,6 @@ func TestPoA_Hooks(t *testing.T) { t, func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, ) hooks := keeper.Hooks() diff --git a/x/poa/keeper/keeper.go b/x/poa/keeper/keeper.go index fb07adb..bde50eb 100644 --- a/x/poa/keeper/keeper.go +++ b/x/poa/keeper/keeper.go @@ -28,7 +28,6 @@ type ( router *baseapp.MsgServiceRouter // Msg server router bk types.BankKeeper sk types.StakingKeeper - ck types.SlashingKeeper } ) @@ -38,7 +37,6 @@ func NewKeeper( router *baseapp.MsgServiceRouter, bk types.BankKeeper, sk types.StakingKeeper, - ck types.SlashingKeeper, authority string, ) *Keeper { // set KeyTable if it has not already been set @@ -58,7 +56,6 @@ func NewKeeper( router: router, bk: bk, sk: sk, - ck: ck, } } diff --git a/x/poa/keeper/keeper_test.go b/x/poa/keeper/keeper_test.go index da5286f..63b9eb6 100644 --- a/x/poa/keeper/keeper_test.go +++ b/x/poa/keeper/keeper_test.go @@ -44,9 +44,7 @@ func poaKeeperTestSetup(t *testing.T) (*Keeper, sdk.Context) { bankKeeper.EXPECT().SendCoinsFromAccountToModule(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() } - slashingExpectations := func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {} - - return setupPoaKeeper(t, stakingExpectations, bankExpectations, slashingExpectations) + return setupPoaKeeper(t, stakingExpectations, bankExpectations) } // Define here Keeper methods to be unit tested From 768c6a4c6ced29063ad44fcdd69be09a377a968f Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 12:30:01 +0100 Subject: [PATCH 7/7] fix: remove hooks_test.go --- x/poa/keeper/hooks_test.go | 45 -------------------------------------- 1 file changed, 45 deletions(-) delete mode 100644 x/poa/keeper/hooks_test.go diff --git a/x/poa/keeper/hooks_test.go b/x/poa/keeper/hooks_test.go deleted file mode 100644 index 16ce9e1..0000000 --- a/x/poa/keeper/hooks_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package keeper - -import ( - "testing" - - "cosmossdk.io/math" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" - "github.com/xrplevm/node/v6/x/poa/testutil" -) - -func TestPoA_Hooks(t *testing.T) { - keeper, ctx := setupPoaKeeper( - t, - func(_ sdk.Context, _ *testutil.MockStakingKeeper) {}, - func(_ sdk.Context, _ *testutil.MockBankKeeper) {}, - func(_ sdk.Context, _ *testutil.MockSlashingKeeper) {}, - ) - - hooks := keeper.Hooks() - - err := hooks.BeforeDelegationCreated(ctx, sdk.AccAddress("ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp"), sdk.ValAddress("ethm1a0pd5cyew47pvgf7rd7axxy3humv9ev0nnkprp")) - require.NoError(t, err) - err = hooks.AfterValidatorBonded(ctx, sdk.ConsAddress(""), sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.AfterValidatorRemoved(ctx, sdk.ConsAddress(""), sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.AfterValidatorCreated(ctx, sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.AfterValidatorBeginUnbonding(ctx, sdk.ConsAddress(""), sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.BeforeValidatorModified(ctx, sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.BeforeDelegationSharesModified(ctx, sdk.AccAddress(""), sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.BeforeDelegationRemoved(ctx, sdk.AccAddress(""), sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.AfterDelegationModified(ctx, sdk.AccAddress(""), sdk.ValAddress("")) - require.NoError(t, err) - err = hooks.AfterUnbondingInitiated(ctx, 0) - require.NoError(t, err) - err = hooks.BeforeValidatorSlashed(ctx, sdk.ValAddress(""), math.LegacyNewDec(0)) - require.NoError(t, err) -}