From 157a86407aafb54f88b1c86071d3c984e9b595c5 Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 11:41:30 +0100 Subject: [PATCH 1/6] 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/6] 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 5a0179ed0b01de0bb4c14fce95f3598f4c7a3396 Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Mon, 17 Mar 2025 12:18:26 +0100 Subject: [PATCH 3/6] 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 4/6] 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 5/6] 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 98c495b993afec9f25d5cdd5cf4a9d6cc74d53ff Mon Sep 17 00:00:00 2001 From: AdriaCarrera Date: Tue, 18 Mar 2025 12:26:56 +0100 Subject: [PATCH 6/6] fix: restore hooks --- app/app.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/app.go b/app/app.go index 6917742..2d0b648 100644 --- a/app/app.go +++ b/app/app.go @@ -444,6 +444,7 @@ func New( stakingtypes.NewMultiStakingHooks( app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks(), + app.PoaKeeper.Hooks(), ), ) // register the staking hooks