refactor(runtime): Audit runtime changes (#21309)

Co-authored-by: Julien Robert <julien@rbrt.fr>

CHANGELOG.md

@@ -75,6 +75,9 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (client) [#19905](https://github.com/cosmos/cosmos-sdk/pull/19905) Add grpc client config to `client.toml`.
 * (genutil) [#19971](https://github.com/cosmos/cosmos-sdk/pull/19971) Allow manually setting the consensus key type in genesis
 * (runtime) [#19953](https://github.com/cosmos/cosmos-sdk/pull/19953) Implement `core/transaction.Service` in runtime.
+* (runtime) [#18475](https://github.com/cosmos/cosmos-sdk/pull/18475) Adds an implementation for `core.branch.Service`.
+* (runtime) [#19004](https://github.com/cosmos/cosmos-sdk/pull/19004) Adds an implementation for `core/header.Service` in runtime.
+* (runtime) [#20238](https://github.com/cosmos/cosmos-sdk/pull/20238) Adds an implementation for `core/comet.Service` in runtime.
 * (tests) [#20013](https://github.com/cosmos/cosmos-sdk/pull/20013) Introduce system tests to run multi node local testnet in CI
 * (crypto/keyring) [#20212](https://github.com/cosmos/cosmos-sdk/pull/20212) Expose the db keyring used in the keystore.
 * (client/tx) [#20870](https://github.com/cosmos/cosmos-sdk/pull/20870) Add `timeout-timestamp` field for tx body defines time based timeout.Add `WithTimeoutTimestamp` to tx factory. Increased gas cost for processing newly added timeout timestamp field in tx body.

runtime/environment.go

@@ -64,7 +64,7 @@ func EnvWithMemStoreService(memStoreService store.MemoryStoreService) EnvOption
 }
 
 // failingMsgRouter is a message router that panics when accessed
-// this is to ensure all fields are set by in environment
+// this is to ensure all fields are set in environment
 type failingMsgRouter struct {
 	baseapp.MessageRouter
 }
@@ -86,7 +86,7 @@ func (failingMsgRouter) HybridHandlerByMsgName(msgName string) func(ctx context.
 }
 
 // failingQueryRouter is a query router that panics when accessed
-// this is to ensure all fields are set by in environment
+// this is to ensure all fields are set in environment
 type failingQueryRouter struct {
 	baseapp.QueryRouter
 }
@@ -112,7 +112,7 @@ func (failingQueryRouter) SetInterfaceRegistry(interfaceRegistry codectypes.Inte
 }
 
 // failingMemStore is a memstore that panics when accessed
-// this is to ensure all fields are set by in environment
+// this is to ensure all fields are set in environment
 type failingMemStore struct {
 	store.MemoryStoreService
 }

runtime/router.go

@@ -15,7 +15,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp"
 )
 
-// NewMsgRouterService implements router.Service.
+// NewMsgRouterService return new implementation of router.Service.
 func NewMsgRouterService(msgRouter baseapp.MessageRouter) router.Service {
 	return &msgRouterService{
 		router: msgRouter,
@@ -75,7 +75,7 @@ func (m *msgRouterService) Invoke(ctx context.Context, msg gogoproto.Message) (g
 	return msgResp, nil
 }
 
-// NewQueryRouterService implements router.Service.
+// NewQueryRouterService return new implementation of router.Service.
 func NewQueryRouterService(queryRouter baseapp.QueryRouter) router.Service {
 	return &queryRouterService{
 		router: queryRouter,

runtime/store.go

@@ -63,34 +63,32 @@ func (failingStoreService) OpenTransientStore(ctx context.Context) store.KVStore
 }
 
 // CoreKVStore is a wrapper of Core/Store kvstore interface
-// Remove after https://github.com/cosmos/cosmos-sdk/issues/14714 is closed
 type coreKVStore struct {
 	kvStore storetypes.KVStore
 }
 
 // NewKVStore returns a wrapper of Core/Store kvstore interface
-// Remove once store migrates to core/store kvstore interface
 func newKVStore(store storetypes.KVStore) store.KVStore {
 	return coreKVStore{kvStore: store}
 }
 
-// Get returns nil iff key doesn't exist. Errors on nil key.
+// Get returns value corresponding to the key. Panics on nil key.
 func (store coreKVStore) Get(key []byte) ([]byte, error) {
 	return store.kvStore.Get(key), nil
 }
 
-// Has checks if a key exists. Errors on nil key.
+// Has checks if a key exists. Panics on nil key.
 func (store coreKVStore) Has(key []byte) (bool, error) {
 	return store.kvStore.Has(key), nil
 }
 
-// Set sets the key. Errors on nil key or value.
+// Set sets the key. Panics on nil key or value.
 func (store coreKVStore) Set(key, value []byte) error {
 	store.kvStore.Set(key, value)
 	return nil
 }
 
-// Delete deletes the key. Errors on nil key.
+// Delete deletes the key. Panics on nil key.
 func (store coreKVStore) Delete(key []byte) error {
 	store.kvStore.Delete(key)
 	return nil

runtime/v2/app.go

@@ -110,11 +110,6 @@ func (a *App[T]) GetStore() Store {
 	return a.db
 }
 
-// GetLogger returns the app logger.
-func (a *App[T]) GetLogger() log.Logger {
-	return a.logger
-}
-
 func (a *App[T]) GetAppManager() *appmanager.AppManager[T] {
 	return a.AppManager
 }

runtime/v2/builder.go

@@ -127,7 +127,7 @@ func (a *AppBuilder[T]) Build(opts ...AppBuilderOption[T]) (*App[T], error) {
 	storeOpts := rootstore.DefaultStoreOptions()
 	if s := a.viper.Sub("store.options"); s != nil {
 		if err := s.Unmarshal(&storeOpts); err != nil {
-			return nil, fmt.Errorf("failed to store options: %w", err)
+			return nil, fmt.Errorf("failed to unmarshal store options: %w", err)
 		}
 	}
 

runtime/v2/manager.go

@@ -217,7 +217,7 @@ func (m *MM[T]) ExportGenesisForModules(
 
 		if module, hasGenesis := mod.(appmodulev2.HasGenesis); hasGenesis {
 			moduleI = module.(ModuleI)
-		} else if module, hasABCIGenesis := mod.(appmodulev2.HasGenesis); hasABCIGenesis {
+		} else if module, hasABCIGenesis := mod.(appmodulev2.HasABCIGenesis); hasABCIGenesis {
 			moduleI = module.(ModuleI)
 		} else {
 			continue
@@ -357,8 +357,56 @@ func (m *MM[T]) TxValidators() func(ctx context.Context, tx T) error {
 	}
 }
 
-// TODO write as descriptive godoc as module manager v1.
-// TODO include feedback from https://github.com/cosmos/cosmos-sdk/issues/15120
+// RunMigrations performs in-place store migrations for all modules. This
+// function MUST be called inside an x/upgrade UpgradeHandler.
+//
+// Recall that in an upgrade handler, the `fromVM` VersionMap is retrieved from
+// x/upgrade's store, and the function needs to return the target VersionMap
+// that will in turn be persisted to the x/upgrade's store. In general,
+// returning RunMigrations should be enough:
+//
+// Example:
+//
+//	app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx context.Context, plan upgradetypes.Plan, fromVM appmodule.VersionMap) (appmodule.VersionMap, error) {
+//	    return app.ModuleManager().RunMigrations(ctx, fromVM)
+//	})
+//
+// Internally, RunMigrations will perform the following steps:
+//   - create an `updatedVM` VersionMap of module with their latest ConsensusVersion
+//   - if module implements `HasConsensusVersion` interface get the consensus version as `toVersion`,
+//     if not `toVersion` is set to 0.
+//   - get `fromVersion` from `fromVM` with module's name.
+//   - if the module's name exists in `fromVM` map, then run in-place store migrations
+//     for that module between `fromVersion` and `toVersion`.
+//   - if the module does not exist in the `fromVM` (which means that it's a new module,
+//     because it was not in the previous x/upgrade's store), then run
+//     `InitGenesis` on that module.
+//
+// - return the `updatedVM` to be persisted in the x/upgrade's store.
+//
+// Migrations are run in an order defined by `mm.config.OrderMigrations`.
+//
+// As an app developer, if you wish to skip running InitGenesis for your new
+// module "foo", you need to manually pass a `fromVM` argument to this function
+// foo's module version set to its latest ConsensusVersion. That way, the diff
+// between the function's `fromVM` and `udpatedVM` will be empty, hence not
+// running anything for foo.
+//
+// Example:
+//
+//	app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx context.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
+//	    // Assume "foo" is a new module.
+//	    // `fromVM` is fetched from existing x/upgrade store. Since foo didn't exist
+//	    // before this upgrade, `v, exists := fromVM["foo"]; exists == false`, and RunMigration will by default
+//	    // run InitGenesis on foo.
+//	    // To skip running foo's InitGenesis, you need set `fromVM`'s foo to its latest
+//	    // consensus version:
+//	    fromVM["foo"] = foo.AppModule{}.ConsensusVersion()
+//
+//	    return app.ModuleManager().RunMigrations(ctx, fromVM)
+//	})
+//
+// Please also refer to https://docs.cosmos.network/main/core/upgrade for more information.
 func (m *MM[T]) RunMigrations(ctx context.Context, fromVM appmodulev2.VersionMap) (appmodulev2.VersionMap, error) {
 	updatedVM := appmodulev2.VersionMap{}
 	for _, moduleName := range m.config.OrderMigrations {
@@ -443,8 +491,8 @@ func (m *MM[T]) RegisterServices(app *App[T]) error {
 func (m *MM[T]) validateConfig() error {
 	if err := m.assertNoForgottenModules("PreBlockers", m.config.PreBlockers, func(moduleName string) bool {
 		module := m.modules[moduleName]
-		_, hasBlock := module.(appmodulev2.HasPreBlocker)
-		return !hasBlock
+		_, hasPreBlock := module.(appmodulev2.HasPreBlocker)
+		return !hasPreBlock
 	}); err != nil {
 		return err
 	}

runtime/v2/migrations.go

@@ -14,7 +14,7 @@ type migrationRegistrar struct {
 	migrations map[string]map[uint64]appmodulev2.MigrationHandler
 }
 
-// newMigrationRegistrar is constructor for registering in-place store migrations for modules.
+// newMigrationRegistrar is a constructor for registering in-place store migrations for modules.
 func newMigrationRegistrar() *migrationRegistrar {
 	return &migrationRegistrar{
 		migrations: make(map[string]map[uint64]appmodulev2.MigrationHandler),

runtime/v2/store.go

@@ -23,7 +23,7 @@ type Store interface {
 	StateLatest() (uint64, store.ReaderMap, error)
 
 	// StateAt returns a readonly view over the provided
-	// state. Must error when the version does not exist.
+	// version. Must error when the version does not exist.
 	StateAt(version uint64) (store.ReaderMap, error)
 
 	// SetInitialVersion sets the initial version of the store.
@@ -52,5 +52,6 @@ type Store interface {
 	// latest version implicitly.
 	LoadLatestVersion() error
 
+	// LastCommitID returns the latest commit ID
 	LastCommitID() (proof.CommitID, error)
 }

runtime/v2/types.go

@@ -17,7 +17,7 @@ const (
 	FlagHome   = "home"
 )
 
-// ValidateProtoAnnotations validates that the proto annotations are correct.
+// validateProtoAnnotations validates that the proto annotations are correct.
 // More specifically, it verifies:
 // - all services named "Msg" have `(cosmos.msg.v1.service) = true`,
 //