feat: merge preFinalizeBlockHook

CHANGELOG.md

@@ -57,7 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
-* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration.
+* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421), [#17713](https://github.com/cosmos/cosmos-sdk/pull/17713) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics and reimplement `PreFinalizeBlockHook` as `PreBlocker`.
 * (baseapp) [#17518](https://github.com/cosmos/cosmos-sdk/pull/17518) Utilizing voting power from vote extensions (CometBFT) instead of the current bonded tokens (x/staking) to determine if a set of vote extensions are valid.
 * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`.
 * (mempool) [#17668](https://github.com/cosmos/cosmos-sdk/pull/17668) Fix: `PriorityNonceIterator.Next()` nil pointer ref for min priority at the end of iteration.

UPGRADING.md

@@ -102,15 +102,21 @@ allows an application to define handlers for these methods via `ExtendVoteHandle
 and `VerifyVoteExtensionHandler` respectively. Please see [here](https://docs.cosmos.network/v0.50/building-apps/vote-extensions)
 for more info.
 
-#### Upgrade
+#### Set PreBlocker
 
 **Users using `depinject` / app v2 do not need any changes, this is abstracted for them.**
 
 ```diff
-+ app.BaseApp.SetMigrationModuleManager(app.ModuleManager)
++ app.SetPreBlocker(app.PreBlocker)
 ```
 
-BaseApp added `SetMigrationModuleManager` for apps to set their ModuleManager which implements `RunMigrationBeginBlock`. This is essential for BaseApp to run `BeginBlock` of upgrade module and inject `ConsensusParams` to context for `beginBlocker` during `beginBlock`.
+```diff
++func (app *SimApp) PreBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (sdk.ResponsePreBlock, error) {
++	return app.ModuleManager.PreBlock(ctx, req)
++}
+```
+
+BaseApp added `SetPreBlocker` for apps. This is essential for BaseApp to run `PreBlock` which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics.
 
 #### Events
 
@@ -337,7 +343,7 @@ The return type of the interface method `TxConfig.SignModeHandler()` has been ch
     * `x/slashing`
     * `x/upgrade`
 
-* BeginBlock and EndBlock have changed their signature, so it is important that any module implementing them are updated accordingly.
+* `BeginBlock` and `EndBlock` have changed their signature, so it is important that any module implementing them are updated accordingly.
 
 ```diff
 - BeginBlock(sdk.Context, abci.RequestBeginBlock)
@@ -349,18 +355,38 @@ The return type of the interface method `TxConfig.SignModeHandler()` has been ch
 + EndBlock(context.Context) error
 ```
 
-In case a module requires to return `abci.ValidatorUpdate` from `EndBlock`, it can use the `HasABCIEndblock` interface instead.
+In case a module requires to return `abci.ValidatorUpdate` from `EndBlock`, it can use the `HasABCIEndBlock` interface instead.
 
 ```diff
 - EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate
 + EndBlock(context.Context) ([]abci.ValidatorUpdate, error)
 ```
 
-`GetSigners()` is no longer required to be implemented on `Msg` types. The SDK will automatically infer the signers from the `Signer` field on the message. The signer field is required on all messages unless using a custom signer function.
+:::tip
+It is possible to ensure that a module implements the correct interfaces by using compiler assertions in your `x/{moduleName}/module.go`:
 
+```go
+var (
+	_ module.AppModuleBasic      = (*AppModule)(nil)
+	_ module.AppModuleSimulation = (*AppModule)(nil)
+	_ module.HasGenesis          = (*AppModule)(nil)
+
+	_ appmodule.AppModule        = (*AppModule)(nil)
+	_ appmodule.HasBeginBlocker  = (*AppModule)(nil)
+	_ appmodule.HasEndBlocker    = (*AppModule)(nil)
+	...
+)
+```
+
+Read more on those interfaces [here](https://docs.cosmos.network/v0.50/building-modules/module-manager#application-module-interfaces).
+
+:::
+
+* `GetSigners()` is no longer required to be implemented on `Msg` types. The SDK will automatically infer the signers from the `Signer` field on the message. The signer field is required on all messages unless using a custom signer function.
 
-To find out more please read the [signer field](./05-protobuf-annotations.md#signer) &  [here](https://github.com/cosmos/cosmos-sdk/blob/7352d0bce8e72121e824297df453eb1059c28da8/docs/docs/build/building-modules/02-messages-and-queries.md#L40)documentation. 
+To find out more please read the [signer field](./05-protobuf-annotations.md#signer) & [here](https://github.com/cosmos/cosmos-sdk/blob/7352d0bce8e72121e824297df453eb1059c28da8/docs/docs/build/building-modules/02-messages-and-queries.md#L40) documentation. 
 <!-- Link to docs once redeployed -->
+
 #### `x/auth`
 
 For ante handler construction via `ante.NewAnteHandler`, the field `ante.HandlerOptions.SignModeHandler` has been updated to `x/tx/signing/HandlerMap` from `x/auth/signing/SignModeHandler`. Callers typically fetch this value from `client.TxConfig.SignModeHandler()` (which is also changed) so this change should be transparent to most users.

baseapp/abci.go

@@ -730,10 +730,8 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons
 			WithHeaderHash(req.Hash)
 	}
 
-	if app.preFinalizeBlockHook != nil {
-		if err := app.preFinalizeBlockHook(app.finalizeBlockState.ctx, req); err != nil {
-			return nil, err
-		}
+	if err := app.preBlock(req); err != nil {
+		return nil, err
 	}
 
 	beginBlock, err := app.beginBlock(req)

baseapp/abci_test.go

@@ -2035,7 +2035,7 @@ func TestABCI_HaltChain(t *testing.T) {
 	}
 }
 
-func TestBaseApp_PreFinalizeBlockHook(t *testing.T) {
+func TestBaseApp_PreBlocker(t *testing.T) {
 	db := dbm.NewMemDB()
 	name := t.Name()
 	logger := log.NewTestLogger(t)
@@ -2045,9 +2045,11 @@ func TestBaseApp_PreFinalizeBlockHook(t *testing.T) {
 	require.NoError(t, err)
 
 	wasHookCalled := false
-	app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error {
+	app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) {
 		wasHookCalled = true
-		return nil
+		return sdk.ResponsePreBlock{
+			ConsensusParamsChanged: true,
+		}, nil
 	})
 	app.Seal()
 
@@ -2060,8 +2062,8 @@ func TestBaseApp_PreFinalizeBlockHook(t *testing.T) {
 	_, err = app.InitChain(&abci.RequestInitChain{})
 	require.NoError(t, err)
 
-	app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error {
-		return errors.New("some error")
+	app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) {
+		return sdk.ResponsePreBlock{}, errors.New("some error")
 	})
 	app.Seal()
 
@@ -2122,7 +2124,7 @@ func TestBaseApp_VoteExtensions(t *testing.T) {
 			return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil
 		})
 
-		app.SetPreFinalizeBlockHook(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) error {
+		app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (sdk.ResponsePreBlock, error) {
 			count := uint64(0)
 			pricesSum := uint64(0)
 			for _, v := range req.Txs {
@@ -2141,7 +2143,9 @@ func TestBaseApp_VoteExtensions(t *testing.T) {
 				ctx.KVStore(capKey1).Set([]byte("avgPrice"), buf)
 			}
 
-			return nil
+			return sdk.ResponsePreBlock{
+				ConsensusParamsChanged: true,
+			}, nil
 		})
 	}
 

baseapp/baseapp.go

@@ -43,12 +43,6 @@ type (
 	StoreLoader func(ms storetypes.CommitMultiStore) error
 )
 
-// MigrationModuleManager is the interface that a migration module manager should implement to handle
-// the execution of migration logic during the beginning of a block.
-type MigrationModuleManager interface {
-	RunMigrationBeginBlock(ctx sdk.Context) (bool, error)
-}
-
 const (
 	execModeCheck           execMode = iota // Check a transaction
 	execModeReCheck                         // Recheck a (pending) transaction after a commit
@@ -80,16 +74,16 @@ type BaseApp struct {
 	anteHandler sdk.AnteHandler // ante handler for fee and auth
 	postHandler sdk.PostHandler // post handler, optional, e.g. for tips
 
-	initChainer          sdk.InitChainer                // ABCI InitChain handler
-	beginBlocker         sdk.BeginBlocker               // (legacy ABCI) BeginBlock handler
-	endBlocker           sdk.EndBlocker                 // (legacy ABCI) EndBlock handler
-	processProposal      sdk.ProcessProposalHandler     // ABCI ProcessProposal handler
-	prepareProposal      sdk.PrepareProposalHandler     // ABCI PrepareProposal
-	extendVote           sdk.ExtendVoteHandler          // ABCI ExtendVote handler
-	verifyVoteExt        sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler
-	prepareCheckStater   sdk.PrepareCheckStater         // logic to run during commit using the checkState
-	precommiter          sdk.Precommiter                // logic to run during commit using the deliverState
-	preFinalizeBlockHook sdk.PreFinalizeBlockHook       // logic to run before FinalizeBlock
+	initChainer        sdk.InitChainer                // ABCI InitChain handler
+	preBlocker         sdk.PreBlocker                 // logic to run before BeginBlocker
+	beginBlocker       sdk.BeginBlocker               // (legacy ABCI) BeginBlock handler
+	endBlocker         sdk.EndBlocker                 // (legacy ABCI) EndBlock handler
+	processProposal    sdk.ProcessProposalHandler     // ABCI ProcessProposal handler
+	prepareProposal    sdk.PrepareProposalHandler     // ABCI PrepareProposal
+	extendVote         sdk.ExtendVoteHandler          // ABCI ExtendVote handler
+	verifyVoteExt      sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler
+	prepareCheckStater sdk.PrepareCheckStater         // logic to run during commit using the checkState
+	precommiter        sdk.Precommiter                // logic to run during commit using the deliverState
 
 	addrPeerFilter sdk.PeerFilter // filter peers by address and port
 	idPeerFilter   sdk.PeerFilter // filter peers by node ID
@@ -98,9 +92,6 @@ type BaseApp struct {
 	// manages snapshots, i.e. dumps of app state at certain intervals
 	snapshotManager *snapshots.Manager
 
-	// manages migrate module
-	migrationModuleManager MigrationModuleManager
-
 	// volatile states:
 	//
 	// - checkState is set on InitChain and reset on Commit
@@ -283,11 +274,6 @@ func (app *BaseApp) SetMsgServiceRouter(msgServiceRouter *MsgServiceRouter) {
 	app.msgServiceRouter = msgServiceRouter
 }
 
-// SetMigrationModuleManager sets the MigrationModuleManager of a BaseApp.
-func (app *BaseApp) SetMigrationModuleManager(migrationModuleManager MigrationModuleManager) {
-	app.migrationModuleManager = migrationModuleManager
-}
-
 // MountStores mounts all IAVL or DB stores to the provided keys in the BaseApp
 // multistore.
 func (app *BaseApp) MountStores(keys ...storetypes.StoreKey) {
@@ -682,29 +668,34 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context
 	return ctx.WithMultiStore(msCache), msCache
 }
 
+func (app *BaseApp) preBlock(req *abci.RequestFinalizeBlock) error {
+	if app.preBlocker != nil {
+		ctx := app.finalizeBlockState.ctx
+		rsp, err := app.preBlocker(ctx, req)
+		if err != nil {
+			return err
+		}
+		// rsp.ConsensusParamsChanged is true from preBlocker means ConsensusParams in store get changed
+		// write the consensus parameters in store to context
+		if rsp.ConsensusParamsChanged {
+			ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx))
+			// GasMeter must be set after we get a context with updated consensus params.
+			gasMeter := app.getBlockGasMeter(ctx)
+			ctx = ctx.WithBlockGasMeter(gasMeter)
+			app.finalizeBlockState.ctx = ctx
+		}
+	}
+	return nil
+}
+
 func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, error) {
 	var (
 		resp sdk.BeginBlock
 		err  error
 	)
 
 	if app.beginBlocker != nil {
-		ctx := app.finalizeBlockState.ctx
-		if app.migrationModuleManager != nil {
-			if success, err := app.migrationModuleManager.RunMigrationBeginBlock(ctx); success {
-				cp := ctx.ConsensusParams()
-				// Manager skips this step if Block is non-nil since upgrade module is expected to set this params
-				// and consensus parameters should not be overwritten.
-				if cp.Block == nil {
-					if cp = app.GetConsensusParams(ctx); cp.Block != nil {
-						ctx = ctx.WithConsensusParams(cp)
-					}
-				}
-			} else if err != nil {
-				return sdk.BeginBlock{}, err
-			}
-		}
-		resp, err = app.beginBlocker(ctx)
+		resp, err = app.beginBlocker(app.finalizeBlockState.ctx)
 		if err != nil {
 			return resp, err
 		}

baseapp/baseapp_test.go

@@ -427,6 +427,9 @@ func TestBaseAppOptionSeal(t *testing.T) {
 	require.Panics(t, func() {
 		suite.baseApp.SetInitChainer(nil)
 	})
+	require.Panics(t, func() {
+		suite.baseApp.SetPreBlocker(nil)
+	})
 	require.Panics(t, func() {
 		suite.baseApp.SetBeginBlocker(nil)
 	})

baseapp/options.go

@@ -176,6 +176,14 @@ func (app *BaseApp) SetInitChainer(initChainer sdk.InitChainer) {
 	app.initChainer = initChainer
 }
 
+func (app *BaseApp) SetPreBlocker(preBlocker sdk.PreBlocker) {
+	if app.sealed {
+		panic("SetPreBlocker() on sealed BaseApp")
+	}
+
+	app.preBlocker = preBlocker
+}
+
 func (app *BaseApp) SetBeginBlocker(beginBlocker sdk.BeginBlocker) {
 	if app.sealed {
 		panic("SetBeginBlocker() on sealed BaseApp")
@@ -208,14 +216,6 @@ func (app *BaseApp) SetPrecommiter(precommiter sdk.Precommiter) {
 	app.precommiter = precommiter
 }
 
-func (app *BaseApp) SetPreFinalizeBlockHook(hook sdk.PreFinalizeBlockHook) {
-	if app.sealed {
-		panic("SetPreFinalizeBlockHook() on sealed BaseApp")
-	}
-
-	app.preFinalizeBlockHook = hook
-}
-
 func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) {
 	if app.sealed {
 		panic("SetAnteHandler() on sealed BaseApp")

client/v2/autocli/flag/builder.go

@@ -30,7 +30,10 @@ type Builder struct {
 
 	// FileResolver specifies how protobuf file descriptors will be resolved. If it is
 	// nil protoregistry.GlobalFiles will be used.
-	FileResolver protodesc.Resolver
+	FileResolver interface {
+		protodesc.Resolver
+		RangeFiles(func(protoreflect.FileDescriptor) bool)
+	}
 
 	messageFlagTypes map[protoreflect.FullName]Type
 	scalarFlagTypes  map[string]Type

client/v2/autocli/query.go

@@ -2,11 +2,13 @@ package autocli
 
 import (
 	"fmt"
+	"io"
+	"time"
 
 	autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
+	"cosmossdk.io/x/tx/signing/aminojson"
 	"github.com/cockroachdb/errors"
 	"github.com/spf13/cobra"
-	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"cosmossdk.io/client/v2/internal/util"
@@ -101,17 +103,16 @@ func (b *Builder) BuildQueryMethodCommand(descriptor protoreflect.MethodDescript
 	serviceDescriptor := descriptor.Parent().(protoreflect.ServiceDescriptor)
 	methodName := fmt.Sprintf("/%s/%s", serviceDescriptor.FullName(), descriptor.Name())
 	outputType := util.ResolveMessageType(b.TypeResolver, descriptor.Output())
-	jsonMarshalOptions := protojson.MarshalOptions{
+	encoderOptions := aminojson.EncoderOptions{
 		Indent:          "  ",
-		UseProtoNames:   true,
-		UseEnumNumbers:  false,
-		EmitUnpopulated: true,
-		Resolver:        b.TypeResolver,
+		DoNotSortFields: true,
+		TypeResolver:    b.TypeResolver,
+		FileResolver:    b.FileResolver,
 	}
 
 	cmd, err := b.buildMethodCommandCommon(descriptor, options, func(cmd *cobra.Command, input protoreflect.Message) error {
-		if noIdent, _ := cmd.Flags().GetBool(flagNoIndent); noIdent {
-			jsonMarshalOptions.Indent = ""
+		if noIndent, _ := cmd.Flags().GetBool(flagNoIndent); noIndent {
+			encoderOptions.Indent = ""
 		}
 
 		clientConn, err := getClientConn(cmd)
@@ -124,7 +125,8 @@ func (b *Builder) BuildQueryMethodCommand(descriptor protoreflect.MethodDescript
 			return err
 		}
 
-		bz, err := jsonMarshalOptions.Marshal(output.Interface())
+		enc := encoder(aminojson.NewEncoder(encoderOptions))
+		bz, err := enc.Marshal(output.Interface())
 		if err != nil {
 			return fmt.Errorf("cannot marshal response %v: %w", output.Interface(), err)
 		}
@@ -143,3 +145,30 @@ func (b *Builder) BuildQueryMethodCommand(descriptor protoreflect.MethodDescript
 
 	return cmd, nil
 }
+
+func encoder(encoder aminojson.Encoder) aminojson.Encoder {
+	return encoder.DefineTypeEncoding("google.protobuf.Duration", func(_ *aminojson.Encoder, msg protoreflect.Message, w io.Writer) error {
+		var (
+			secondsName protoreflect.Name = "seconds"
+			nanosName   protoreflect.Name = "nanos"
+		)
+
+		fields := msg.Descriptor().Fields()
+		secondsField := fields.ByName(secondsName)
+		if secondsField == nil {
+			return fmt.Errorf("expected seconds field")
+		}
+
+		seconds := msg.Get(secondsField).Int()
+
+		nanosField := fields.ByName(nanosName)
+		if nanosField == nil {
+			return fmt.Errorf("expected nanos field")
+		}
+
+		nanos := msg.Get(nanosField).Int()
+
+		_, err := fmt.Fprintf(w, `"%s"`, (time.Duration(seconds)*time.Second + (time.Duration(nanos) * time.Nanosecond)).String())
+		return err
+	})
+}