refactor(simapp): remove public module basic manager (cosmos#15958)

UPGRADING.md

@@ -1,6 +1,7 @@
 # Upgrading Cosmos SDK
 
 This guide provides instructions for upgrading to specific versions of Cosmos SDK.
+Note, always read the **SimApp** section for more information on application wiring updates.
 
 ## [Unreleased]
 
@@ -61,12 +62,14 @@ The `gogoproto.goproto_stringer = false` annotation has been removed from most p
 
 ### SimApp
 
+<!-- TODO(@julienrbrt) collapse this section in 3 parts, general, app v1 and app v2 changes, now it is a bit confusing -->
+
 #### Module Assertions
 
 Previously, all modules were required to be set in `OrderBeginBlockers`, `OrderEndBlockers` and `OrderInitGenesis / OrderExportGenesis` in `app.go` / `app_config.go`.
 This is no longer the case, the assertion has been loosened to only require modules implementing, respectively, the `module.BeginBlockAppModule`, `module.EndBlockAppModule` and `module.HasGenesis` interfaces.
 
-### Modules Keepers
+#### Modules Keepers
 
 The following modules `NewKeeper` function now take a `KVStoreService` instead of a `StoreKey`:
 
@@ -78,7 +81,7 @@ The following modules `NewKeeper` function now take a `KVStoreService` instead o
 * `x/feegrant`
 * `x/nft`
 
-When not using depinject, the `runtime.NewKVStoreService` method can be used to create a `KVStoreService` from a `StoreKey`:
+User manually wiring their chain need to use the `runtime.NewKVStoreService` method to create a `KVStoreService` from a `StoreKey`:
 
 ```diff
 app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(
@@ -89,19 +92,21 @@ app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(
 )
 ```
 
-The following modules `NewKeeper` function now also take a `log.Logger`:
-
-* `x/bank`
-
 The following modules' `Keeper` methods now take in a `context.Context` instead of `sdk.Context`. Any module that has an interfaces for them (like "expected keepers") will need to update and re-generate mocks if needed:
 
 * `x/authz`
 * `x/bank`
 * `x/distribution`
 
-### depinject
+**Users using depinject do not need any changes, this is automatically done for them.**
+
+#### Logger
+
+The following modules `NewKeeper` function now take a `log.Logger`:
+
+* `x/bank`
 
-For `depinject` users, now the logger must be supplied through the main `depinject.Inject` function instead of passing it to `appBuilder.Build`.
+`depinject` users must now supply the logger through the main `depinject.Supply` function instead of passing it to `appBuilder.Build`.
 
 ```diff
 appConfig = depinject.Configs(
@@ -118,6 +123,31 @@ appConfig = depinject.Configs(
 + app.App = appBuilder.Build(db, traceStore, baseAppOptions...)
 ```
 
+User manually wiring their chain need to add the logger argument when creating the keeper.
+
+#### Module Basics
+
+Previously, the `ModuleBasics` was a global variable that was used to register all modules's `AppModuleBasic` implementation.
+The global variable has been removed and the basic module manager can be now created from the module manager.
+
+This is automatically done for depinject users, however for supplying different app module implementation, pass them via `depinject.Supply` in the main `AppConfig` (`app_config.go`):
+
+```go
+depinject.Supply(
+			// supply custom module basics
+			map[string]module.AppModuleBasic{
+				genutiltypes.ModuleName: genutil.NewAppModuleBasic(genutiltypes.DefaultMessageValidator),
+				govtypes.ModuleName: gov.NewAppModuleBasic(
+					[]govclient.ProposalHandler{
+						paramsclient.ProposalHandler,
+					},
+				),
+			},
+		)
+```
+
+Users manually wiring their chain need to use the new `module.NewBasicManagerFromManager` function, after the module manager creation, and pass a `map[string]module.AppModuleBasic` as argument for optionally overridding some module's `AppModuleBasic`.
+
 ### Packages
 
 #### Store

core/appmodule/module.go

@@ -45,7 +45,7 @@ type HasPrepareCheckState interface {
 	PrepareCheckState(context.Context) error
 }
 
-// HasPreommit is an extension interface that contains information about the AppModule and Precommit.
+// HasPrecommit is an extension interface that contains information about the AppModule and Precommit.
 type HasPrecommit interface {
 	AppModule
 	Precommit(context.Context) error

runtime/builder.go

@@ -24,11 +24,7 @@ func (a *AppBuilder) DefaultGenesis() map[string]json.RawMessage {
 }
 
 // Build builds an *App instance.
-func (a *AppBuilder) Build(
-	db dbm.DB,
-	traceStore io.Writer,
-	baseAppOptions ...func(*baseapp.BaseApp),
-) *App {
+func (a *AppBuilder) Build(db dbm.DB, traceStore io.Writer, baseAppOptions ...func(*baseapp.BaseApp)) *App {
 	for _, option := range a.app.baseAppOptions {
 		baseAppOptions = append(baseAppOptions, option)
 	}
@@ -42,8 +38,7 @@ func (a *AppBuilder) Build(
 
 	a.app.BaseApp = bApp
 	a.app.configurator = module.NewConfigurator(a.app.cdc, a.app.MsgServiceRouter(), a.app.GRPCQueryRouter())
-	err := a.app.ModuleManager.RegisterServices(a.app.configurator)
-	if err != nil {
+	if err := a.app.ModuleManager.RegisterServices(a.app.configurator); err != nil {
 		panic(err)
 	}
 

runtime/module.go

@@ -67,6 +67,7 @@ func init() {
 			ProvideMemoryStoreService,
 			ProvideTransientStoreService,
 			ProvideEventService,
+			ProvideBasicManager,
 		),
 		appmodule.Invoke(SetupAppBuilder),
 	)
@@ -91,8 +92,7 @@ func ProvideApp() (
 	protoTypes := protoregistry.GlobalTypes
 
 	// At startup, check that all proto annotations are correct.
-	err = msgservice.ValidateProtoAnnotations(protoFiles)
-	if err != nil {
+	if err := msgservice.ValidateProtoAnnotations(protoFiles); err != nil {
 		// Once we switch to using protoreflect-based antehandlers, we might
 		// want to panic here instead of logging a warning.
 		_, _ = fmt.Fprintln(os.Stderr, err.Error())
@@ -109,8 +109,7 @@ func ProvideApp() (
 
 	// validate the signing context to make sure that messages are properly configured
 	// with cosmos.msg.v1.signer
-	err = interfaceRegistry.SigningContext().Validate()
-	if err != nil {
+	if err := interfaceRegistry.SigningContext().Validate(); err != nil {
 		return nil, nil, nil, nil, nil, nil, nil, nil, nil, err
 	}
 
@@ -137,25 +136,33 @@ func ProvideApp() (
 type AppInputs struct {
 	depinject.In
 
-	AppConfig         *appv1alpha1.Config
-	Config            *runtimev1alpha1.Module
-	AppBuilder        *AppBuilder
-	Modules           map[string]appmodule.AppModule
-	BaseAppOptions    []BaseAppOption
-	InterfaceRegistry codectypes.InterfaceRegistry
-	LegacyAmino       *codec.LegacyAmino
-	Logger            log.Logger
+	AppConfig          *appv1alpha1.Config
+	Config             *runtimev1alpha1.Module
+	AppBuilder         *AppBuilder
+	Modules            map[string]appmodule.AppModule
+	CustomModuleBasics map[string]module.AppModuleBasic `optional:"true"`
+	BaseAppOptions     []BaseAppOption
+	InterfaceRegistry  codectypes.InterfaceRegistry
+	LegacyAmino        *codec.LegacyAmino
+	Logger             log.Logger
 }
 
 func SetupAppBuilder(inputs AppInputs) {
 	app := inputs.AppBuilder.app
 	app.baseAppOptions = inputs.BaseAppOptions
 	app.config = inputs.Config
-	app.ModuleManager = module.NewManagerFromMap(inputs.Modules)
 	app.appConfig = inputs.AppConfig
 	app.logger = inputs.Logger
+	app.ModuleManager = module.NewManagerFromMap(inputs.Modules)
 
 	for name, mod := range inputs.Modules {
+		if customBasicMod, ok := inputs.CustomModuleBasics[name]; ok {
+			app.basicManager[name] = customBasicMod
+			customBasicMod.RegisterInterfaces(inputs.InterfaceRegistry)
+			customBasicMod.RegisterLegacyAminoCodec(inputs.LegacyAmino)
+			continue
+		}
+
 		if basicMod, ok := mod.(module.AppModuleBasic); ok {
 			app.basicManager[name] = basicMod
 			basicMod.RegisterInterfaces(inputs.InterfaceRegistry)
@@ -229,6 +236,10 @@ func ProvideEventService() event.Service {
 	return EventService{}
 }
 
+func ProvideBasicManager(app *AppBuilder) module.BasicManager {
+	return app.app.basicManager
+}
+
 // globalAccAddressCodec is a temporary address codec that we will use until we
 // can populate it with the correct bech32 prefixes without depending on the global.
 type globalAccAddressCodec struct{}

simapp/app.go

@@ -111,34 +111,6 @@ var (
 	// DefaultNodeHome default home directories for the application daemon
 	DefaultNodeHome string
 
-	// ModuleBasics defines the module BasicManager is in charge of setting up basic,
-	// non-dependant module elements, such as codec registration
-	// and genesis verification.
-	ModuleBasics = module.NewBasicManager(
-		auth.AppModuleBasic{},
-		genutil.NewAppModuleBasic(genutiltypes.DefaultMessageValidator),
-		bank.AppModuleBasic{},
-		staking.AppModuleBasic{},
-		mint.AppModuleBasic{},
-		distr.AppModuleBasic{},
-		gov.NewAppModuleBasic(
-			[]govclient.ProposalHandler{
-				paramsclient.ProposalHandler,
-			},
-		),
-		params.AppModuleBasic{},
-		crisis.AppModuleBasic{},
-		slashing.AppModuleBasic{},
-		feegrantmodule.AppModuleBasic{},
-		upgrade.AppModuleBasic{},
-		evidence.AppModuleBasic{},
-		authzmodule.AppModuleBasic{},
-		groupmodule.AppModuleBasic{},
-		vesting.AppModuleBasic{},
-		nftmodule.AppModuleBasic{},
-		consensus.AppModuleBasic{},
-	)
-
 	// module account permissions
 	maccPerms = map[string][]string{
 		authtypes.FeeCollectorName:     nil,
@@ -189,7 +161,8 @@ type SimApp struct {
 	ConsensusParamsKeeper consensusparamkeeper.Keeper
 
 	// the module manager
-	ModuleManager *module.Manager
+	ModuleManager      *module.Manager
+	BasicModuleManager module.BasicManager
 
 	// simulation manager
 	sm *module.SimulationManager
@@ -216,13 +189,16 @@ func NewSimApp(
 	appOpts servertypes.AppOptions,
 	baseAppOptions ...func(*baseapp.BaseApp),
 ) *SimApp {
-	encodingConfig := makeEncodingConfig()
+	encodingConfig := simappparams.MakeTestEncodingConfig()
 
 	appCodec := encodingConfig.Codec
 	legacyAmino := encodingConfig.Amino
 	interfaceRegistry := encodingConfig.InterfaceRegistry
 	txConfig := encodingConfig.TxConfig
 
+	std.RegisterLegacyAminoCodec(legacyAmino)
+	std.RegisterInterfaces(interfaceRegistry)
+
 	// Below we could construct and set an application specific mempool and
 	// ABCI 1.0 PrepareProposal and ProcessProposal handlers. These defaults are
 	// already set in the SDK's BaseApp, this shows an example of how to override
@@ -404,6 +380,23 @@ func NewSimApp(
 		consensus.NewAppModule(appCodec, app.ConsensusParamsKeeper),
 	)
 
+	// BasicModuleManager defines the module BasicManager is in charge of setting up basic,
+	// non-dependant module elements, such as codec registration and genesis verification.
+	// By default it is composed of all the module from the module manager.
+	// Additionally, app module basics can be overwritten by passing them as argument.
+	app.BasicModuleManager = module.NewBasicManagerFromManager(
+		app.ModuleManager,
+		map[string]module.AppModuleBasic{
+			genutiltypes.ModuleName: genutil.NewAppModuleBasic(genutiltypes.DefaultMessageValidator),
+			govtypes.ModuleName: gov.NewAppModuleBasic(
+				[]govclient.ProposalHandler{
+					paramsclient.ProposalHandler,
+				},
+			),
+		})
+	app.BasicModuleManager.RegisterLegacyAminoCodec(encodingConfig.Amino)
+	app.BasicModuleManager.RegisterInterfaces(encodingConfig.InterfaceRegistry)
+
 	// During begin block slashing happens after distr.BeginBlocker so that
 	// there is nothing left over in the validator fee pool, so as to keep the
 	// CanWithdrawInvariant invariant.
@@ -634,7 +627,7 @@ func (app *SimApp) AutoCliOpts() autocli.AppOptions {
 
 // DefaultGenesis returns a default genesis from the registered AppModuleBasic's.
 func (a *SimApp) DefaultGenesis() map[string]json.RawMessage {
-	return ModuleBasics.DefaultGenesis(a.appCodec)
+	return a.BasicModuleManager.DefaultGenesis(a.appCodec)
 }
 
 // GetKey returns the KVStoreKey for the provided store key.
@@ -681,7 +674,7 @@ func (app *SimApp) RegisterAPIRoutes(apiSvr *api.Server, apiConfig config.APICon
 	nodeservice.RegisterGRPCGatewayRoutes(clientCtx, apiSvr.GRPCGatewayRouter)
 
 	// Register grpc-gateway routes for all modules.
-	ModuleBasics.RegisterGRPCGatewayRoutes(clientCtx, apiSvr.GRPCGatewayRouter)
+	app.BasicModuleManager.RegisterGRPCGatewayRoutes(clientCtx, apiSvr.GRPCGatewayRouter)
 
 	// register swagger API from root so that other applications can override easily
 	if err := server.RegisterSwaggerAPI(apiSvr.ClientCtx, apiSvr.Router, apiConfig.Swagger); err != nil {
@@ -748,12 +741,3 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 
 	return paramsKeeper
 }
-
-func makeEncodingConfig() simappparams.EncodingConfig {
-	encodingConfig := simappparams.MakeTestEncodingConfig()
-	std.RegisterLegacyAminoCodec(encodingConfig.Amino)
-	std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
-	ModuleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
-	ModuleBasics.RegisterInterfaces(encodingConfig.InterfaceRegistry)
-	return encodingConfig
-}