From 160a65432bef0fa8942908a2fec6eff62e99ce7b Mon Sep 17 00:00:00 2001 From: Ammar Elsabe Date: Wed, 8 Oct 2025 21:34:43 +0400 Subject: [PATCH] fix(x/epochs): di wiring --- CHANGELOG.md | 1 + simapp/app.go | 6 +- simapp/app_di.go | 2 +- x/epochs/README.md | 170 +++++++++++++++++++++++++++++++++++++ x/epochs/depinject.go | 6 +- x/epochs/depinject_test.go | 108 ++++++++++++++++++++++- x/epochs/export_test.go | 7 ++ x/epochs/module.go | 6 +- 8 files changed, 295 insertions(+), 11 deletions(-) create mode 100644 x/epochs/export_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 052adfea9c59..1e199f397314 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/epochs) [#25425](https://github.com/cosmos/cosmos-sdk/pull/25425) Fix `InvokeSetHooks` being called with a nil keeper and `AppModule` containing a copy instead of a pointer (hooks set post creating the `AppModule` like with depinject didn't apply because it's a different instance). * (client, client/rpc, x/auth/tx) [#24551](https://github.com/cosmos/cosmos-sdk/pull/24551) Handle cancellation properly when supplying context to client methods. * (x/authz) [#24638](https://github.com/cosmos/cosmos-sdk/pull/24638) Fixed a minor bug where the grant key was cast as a string and dumped directly into the error message leading to an error string possibly containing invalid UTF-8. * (client, client/rpc, x/auth/tx) [#24551](https://github.com/cosmos/cosmos-sdk/pull/24551) Handle cancellation properly when supplying context to client methods. diff --git a/simapp/app.go b/simapp/app.go index be6cbcee4ac8..3a2e5a80656b 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -150,7 +150,7 @@ type SimApp struct { // supplementary keepers FeeGrantKeeper feegrantkeeper.Keeper AuthzKeeper authzkeeper.Keeper - EpochsKeeper epochskeeper.Keeper + EpochsKeeper *epochskeeper.Keeper ProtocolPoolKeeper protocolpoolkeeper.Keeper // the module manager @@ -448,11 +448,13 @@ func NewSimApp( // If evidence needs to be handled for the app, set routes in router here and seal app.EvidenceKeeper = *evidenceKeeper - app.EpochsKeeper = epochskeeper.NewKeeper( + epochsKeeper := epochskeeper.NewKeeper( runtime.NewKVStoreService(keys[epochstypes.StoreKey]), appCodec, ) + app.EpochsKeeper = &epochsKeeper + app.EpochsKeeper.SetHooks( epochstypes.NewMultiEpochHooks( // insert epoch hooks receivers here diff --git a/simapp/app_di.go b/simapp/app_di.go index 3e8e010d0acb..972edc75fa1a 100644 --- a/simapp/app_di.go +++ b/simapp/app_di.go @@ -76,7 +76,7 @@ type SimApp struct { // supplementary keepers FeeGrantKeeper feegrantkeeper.Keeper AuthzKeeper authzkeeper.Keeper - EpochsKeeper epochskeeper.Keeper + EpochsKeeper *epochskeeper.Keeper ProtocolPoolKeeper protocolpoolkeeper.Keeper // simulation manager diff --git a/x/epochs/README.md b/x/epochs/README.md index d56970668c99..81c3fd02a7c4 100644 --- a/x/epochs/README.md +++ b/x/epochs/README.md @@ -95,6 +95,176 @@ func (k MyModuleKeeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, e } ``` +### Wiring Hooks + +**Manual Wiring:** + +Import the following: + +```go +import ( + // ... + "github.com/cosmos/cosmos-sdk/x/epochs" + epochskeeper "github.com/cosmos/cosmos-sdk/x/epochs/keeper" + epochstypes "github.com/cosmos/cosmos-sdk/x/epochs/types" +) +``` + +Add the epochs keeper to your application struct: + +```go +EpochsKeeper *epochskeeper.Keeper +``` + +Add the store key: + +```go +keys := storetypes.NewKVStoreKeys( + // ... + epochstypes.StoreKey, +) +``` + +Instantiate the keeper: + +```go +epochsKeeper := epochskeeper.NewKeeper( + runtime.NewKVStoreService(keys[epochstypes.StoreKey]), + appCodec, +) + +app.EpochsKeeper = &epochsKeeper +``` + +Set up hooks for the epochs keeper: + +```go +app.EpochsKeeper.SetHooks( + epochstypes.NewMultiEpochHooks( + // insert epoch hooks receivers here + app.SomeOtherModule + ), +) +``` + +Add the epochs module to the module manager: + +```go +app.ModuleManager = module.NewManager( + // ... + epochs.NewAppModule(appCodec, app.EpochsKeeper), +) +``` + +Add entries for SetOrderBeginBlockers and SetOrderInitGenesis: + +```go +app.ModuleManager.SetOrderBeginBlockers( + // ... + epochstypes.ModuleName, +) +``` + +```go +app.ModuleManager.SetOrderInitGenesis( + // ... + epochstypes.ModuleName, +) +``` + +**DI Wiring:** + +First, set up the keeper for the application. + +Import the epochs keeper: + +```go +epochskeeper "github.com/cosmos/cosmos-sdk/x/epochs/keeper" +``` + +Add the keeper to your application struct: + +```go +EpochsKeeper *epochskeeper.Keeper +``` + +Add the keeper to the depinject system: + +```go +depinject.Inject( + appConfig, + &appBuilder, + &app.appCodec, + &app.legacyAmino, + &app.txConfig, + &app.interfaceRegistry, + // ... other modules + &app.EpochsKeeper, // NEW MODULE! +) +``` + +Next, set up configuration for the module. + +Import the following: + +```go +import ( + epochsmodulev1 "cosmossdk.io/api/cosmos/epochs/module/v1" + + _ "github.com/cosmos/cosmos-sdk/x/epochs" // import for side-effects + epochstypes "github.com/cosmos/cosmos-sdk/x/epochs/types" +) +``` + +Add an entry for BeginBlockers and InitGenesis: + +```go +BeginBlockers: []string{ + // ... + epochstypes.ModuleName, +}, +``` + +```go +InitGenesis: []string{ + // ... + epochstypes.ModuleName, +}, +``` + +Add an entry for epochs in the ModuleConfig: + +```go +{ + Name: epochstypes.ModuleName, + Config: appconfig.WrapAny(&epochsmodulev1.Module{}), +}, +``` + +depinject can automatically add your hooks to the epochs `Keeper`. For it do so, specify an output of your module with the type `epochtypes.EpochHooksWrapper`, ie: + +```go +type TestInputs struct { + depinject.In +} + +type TestOutputs struct { + depinject.Out + + Hooks types.EpochHooksWrapper +} + +func DummyProvider(in TestInputs) TestOutputs { + return TestOutputs{ + Hooks: types.EpochHooksWrapper{ + EpochHooks: testEpochHooks{}, + }, + } +} +``` + +for an example see [`depinject_test.go`](https://github.com/cosmos/cosmos-sdk/tree/main/x/epochs/depinject_test.go) + ### Panic isolation If a given epoch hook panics, its state update is reverted, but we keep diff --git a/x/epochs/depinject.go b/x/epochs/depinject.go index 7c4fd2a53f58..3b0d293a600a 100644 --- a/x/epochs/depinject.go +++ b/x/epochs/depinject.go @@ -39,14 +39,14 @@ type ModuleInputs struct { type ModuleOutputs struct { depinject.Out - EpochKeeper keeper.Keeper + EpochKeeper *keeper.Keeper Module appmodule.AppModule } func ProvideModule(in ModuleInputs) ModuleOutputs { k := keeper.NewKeeper(in.StoreService, in.Cdc) - m := NewAppModule(k) - return ModuleOutputs{EpochKeeper: k, Module: m} + m := NewAppModule(&k) + return ModuleOutputs{EpochKeeper: m.keeper, Module: m} } func InvokeSetHooks(keeper *keeper.Keeper, hooks map[string]types.EpochHooksWrapper) error { diff --git a/x/epochs/depinject_test.go b/x/epochs/depinject_test.go index ac67263a0d0a..a43460dc70d1 100644 --- a/x/epochs/depinject_test.go +++ b/x/epochs/depinject_test.go @@ -6,22 +6,41 @@ import ( "github.com/stretchr/testify/require" + appv1alpha1 "cosmossdk.io/api/cosmos/app/v1alpha1" + bankmodulev1 "cosmossdk.io/api/cosmos/bank/module/v1" + modulev1 "cosmossdk.io/api/cosmos/epochs/module/v1" + "cosmossdk.io/core/appmodule" + "cosmossdk.io/core/store" + "cosmossdk.io/depinject" + "cosmossdk.io/depinject/appconfig" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/types/module/testutil" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/cosmos/cosmos-sdk/x/epochs" "github.com/cosmos/cosmos-sdk/x/epochs/keeper" "github.com/cosmos/cosmos-sdk/x/epochs/types" ) +var _ types.EpochHooks = testEpochHooks{} + type testEpochHooks struct{} -func (h testEpochHooks) AfterEpochEnd(ctx context.Context, epochIdentifier string, epochNumber int64) error { +func (h testEpochHooks) AfterEpochEnd( + ctx context.Context, + epochIdentifier string, + epochNumber int64, +) error { return nil } -func (h testEpochHooks) BeforeEpochStart(ctx context.Context, epochIdentifier string, epochNumber int64) error { +func (h testEpochHooks) BeforeEpochStart( + ctx context.Context, + epochIdentifier string, + epochNumber int64, +) error { return nil } @@ -58,3 +77,88 @@ func TestInvokeSetHooks(t *testing.T) { require.Equal(t, hook1, multiHooks[0]) require.Equal(t, hook2, multiHooks[1]) } + +type TestInputs struct { + depinject.In +} + +type TestOutputs struct { + depinject.Out + + Hooks types.EpochHooksWrapper +} + +func DummyProvider(in TestInputs) TestOutputs { + return TestOutputs{ + Hooks: types.EpochHooksWrapper{ + EpochHooks: testEpochHooks{}, + }, + } +} + +func ProvideDeps(depinject.In) struct { + depinject.Out + Cdc codec.Codec + StoreService store.KVStoreService +} { + encCfg := testutil.MakeTestEncodingConfig() + + key := storetypes.NewKVStoreKey(types.StoreKey) + return struct { + depinject.Out + Cdc codec.Codec + StoreService store.KVStoreService + }{ + Cdc: encCfg.Codec, + StoreService: runtime.NewKVStoreService(key), + } +} + +func TestDepinject(t *testing.T) { + /// we just need any module's proto to register the provider here, no specific reason to use bank + appconfig.RegisterModule(&bankmodulev1.Module{}, appconfig.Provide(DummyProvider)) + var appModules map[string]appmodule.AppModule + keeper := new(keeper.Keeper) + require.NoError(t, + depinject.Inject( + depinject.Configs( + appconfig.Compose( + &appv1alpha1.Config{ + Modules: []*appv1alpha1.ModuleConfig{ + { + Name: banktypes.ModuleName, + Config: appconfig.WrapAny(&bankmodulev1.Module{}), + }, + { + Name: types.ModuleName, + Config: appconfig.WrapAny(&modulev1.Module{}), + }, + }, + }, + ), + depinject.Provide(ProvideDeps), + ), + &appModules, + &keeper, + ), + ) + + require.NotNil(t, keeper, "expected keeper to not be nil after depinject") + multihooks, ok := keeper.Hooks().(types.MultiEpochHooks) + require.True(t, ok, "expected keeper to have MultiEpochHooks after depinject") + require.Len(t, multihooks, 1, "expected MultiEpochHooks to have 1 element after depinject") + require.Equal( + t, + types.EpochHooksWrapper{EpochHooks: testEpochHooks{}}, + multihooks[0], + "expected the only hook in MultiEpochHooks to be the test hook", + ) + module, ok := appModules[types.ModuleName].(epochs.AppModule) + require.True(t, ok, "expected depinject to fill map with the epochs AppModule") + require.Equal( + t, + types.MultiEpochHooks{types.EpochHooksWrapper{EpochHooks: testEpochHooks{}}}, + module.Keeper().Hooks(), + ) + require.Same(t, keeper, module.Keeper()) // pointers pointing to the same instance +} diff --git a/x/epochs/export_test.go b/x/epochs/export_test.go new file mode 100644 index 000000000000..cb8a24599bd6 --- /dev/null +++ b/x/epochs/export_test.go @@ -0,0 +1,7 @@ +package epochs + +import "github.com/cosmos/cosmos-sdk/x/epochs/keeper" + +func (am AppModule) Keeper() *keeper.Keeper { + return am.keeper +} diff --git a/x/epochs/module.go b/x/epochs/module.go index a0c90009472a..2f9fc081dca0 100644 --- a/x/epochs/module.go +++ b/x/epochs/module.go @@ -33,11 +33,11 @@ const ConsensusVersion = 1 // AppModule implements the AppModule interface for the epochs module. type AppModule struct { - keeper keeper.Keeper + keeper *keeper.Keeper } // NewAppModule creates a new AppModule object. -func NewAppModule(keeper keeper.Keeper) AppModule { +func NewAppModule(keeper *keeper.Keeper) AppModule { return AppModule{ keeper: keeper, } @@ -66,7 +66,7 @@ func (AppModule) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *gwrunt // RegisterServices registers module services. func (am AppModule) RegisterServices(registrar grpc.ServiceRegistrar) error { - types.RegisterQueryServer(registrar, keeper.NewQuerier(am.keeper)) + types.RegisterQueryServer(registrar, keeper.NewQuerier(*am.keeper)) return nil }