Merge branch 'main' into tip/handlers_continuation

CHANGELOG.md

@@ -48,6 +48,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (client/keys) [#21829](https://github.com/cosmos/cosmos-sdk/pull/21829) Add support for importing hex key using standard input.
 * (x/validate) [#21822](https://github.com/cosmos/cosmos-sdk/pull/21822) New module solely responsible for providing ante/post handlers and tx validators for v2. It can be extended by the app developer to provide extra tx validators.
     * In comparison to x/auth/tx/config, there is no app config to skip ante/post handlers, as overwriting them in baseapp or not injecting the x/validate module has the same effect.
+* (baeapp) [#21979](https://github.com/cosmos/cosmos-sdk/pull/21979) Create CheckTxHandler to allow extending the logic of CheckTx.
 
 ### Improvements
 

baseapp/abci.go

@@ -367,18 +367,27 @@ func (app *BaseApp) CheckTx(req *abci.CheckTxRequest) (*abci.CheckTxResponse, er
 		return nil, fmt.Errorf("unknown RequestCheckTx type: %s", req.Type)
 	}
 
-	gInfo, result, anteEvents, err := app.runTx(mode, req.Tx)
-	if err != nil {
-		return responseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace), nil
+	if app.checkTxHandler == nil {
+		gInfo, result, anteEvents, err := app.runTx(mode, req.Tx, nil)
+		if err != nil {
+			return responseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace), nil
+		}
+
+		return &abci.CheckTxResponse{
+			GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
+			GasUsed:   int64(gInfo.GasUsed),   // TODO: Should type accept unsigned ints?
+			Log:       result.Log,
+			Data:      result.Data,
+			Events:    sdk.MarkEventsToIndex(result.Events, app.indexEvents),
+		}, nil
 	}
 
-	return &abci.CheckTxResponse{
-		GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
-		GasUsed:   int64(gInfo.GasUsed),   // TODO: Should type accept unsigned ints?
-		Log:       result.Log,
-		Data:      result.Data,
-		Events:    sdk.MarkEventsToIndex(result.Events, app.indexEvents),
-	}, nil
+	// Create wrapper to avoid users overriding the execution mode
+	runTx := func(txBytes []byte, tx sdk.Tx) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) {
+		return app.runTx(mode, txBytes, tx)
+	}
+
+	return app.checkTxHandler(runTx, req)
 }
 
 // PrepareProposal implements the PrepareProposal ABCI method and returns a

baseapp/baseapp.go

@@ -91,6 +91,7 @@ type BaseApp struct {
 	prepareCheckStater sdk.PrepareCheckStater         // logic to run during commit using the checkState
 	precommiter        sdk.Precommiter                // logic to run during commit using the deliverState
 	versionModifier    server.VersionModifier         // interface to get and set the app version
+	checkTxHandler     sdk.CheckTxHandler
 
 	addrPeerFilter sdk.PeerFilter // filter peers by address and port
 	idPeerFilter   sdk.PeerFilter // filter peers by node ID
@@ -688,7 +689,6 @@ func (app *BaseApp) getContextForTx(mode execMode, txBytes []byte) sdk.Context {
 // a branched multi-store.
 func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context, storetypes.CacheMultiStore) {
 	ms := ctx.MultiStore()
-	// TODO: https://github.com/cosmos/cosmos-sdk/issues/2824
 	msCache := ms.CacheMultiStore()
 	if msCache.TracingEnabled() {
 		msCache = msCache.SetTracingContext(
@@ -761,7 +761,7 @@ func (app *BaseApp) deliverTx(tx []byte) *abci.ExecTxResult {
 		telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted")
 	}()
 
-	gInfo, result, anteEvents, err := app.runTx(execModeFinalize, tx)
+	gInfo, result, anteEvents, err := app.runTx(execModeFinalize, tx, nil)
 	if err != nil {
 		resultStr = "failed"
 		resp = responseExecTxResultWithEvents(
@@ -822,7 +822,9 @@ type HasNestedMsgs interface {
 // Note, gas execution info is always returned. A reference to a Result is
 // returned if the tx does not run out of gas and if all the messages are valid
 // and execute successfully. An error is returned otherwise.
-func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) {
+// both txbytes and the decoded tx are passed to runTx to avoid the state machine encoding the tx and decoding the transaction twice
+// passing the decoded tx to runTX is optional, it will be decoded if the tx is nil
+func (app *BaseApp) runTx(mode execMode, txBytes []byte, tx sdk.Tx) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) {
 	// NOTE: GasWanted should be returned by the AnteHandler. GasUsed is
 	// determined by the GasMeter. We need access to the context to get the gas
 	// meter, so we initialize upfront.
@@ -870,9 +872,12 @@ func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, res
 		defer consumeBlockGas()
 	}
 
-	tx, err := app.txDecoder(txBytes)
-	if err != nil {
-		return sdk.GasInfo{GasUsed: 0, GasWanted: 0}, nil, nil, sdkerrors.ErrTxDecode.Wrap(err.Error())
+	// if the transaction is not decoded, decode it here
+	if tx == nil {
+		tx, err = app.txDecoder(txBytes)
+		if err != nil {
+			return sdk.GasInfo{GasUsed: 0, GasWanted: 0}, nil, nil, sdkerrors.ErrTxDecode.Wrap(err.Error())
+		}
 	}
 
 	msgs := tx.GetMsgs()
@@ -1160,7 +1165,7 @@ func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) {
 		return nil, err
 	}
 
-	_, _, _, err = app.runTx(execModePrepareProposal, bz)
+	_, _, _, err = app.runTx(execModePrepareProposal, bz, tx)
 	if err != nil {
 		return nil, err
 	}
@@ -1179,7 +1184,7 @@ func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) {
 		return nil, err
 	}
 
-	_, _, _, err = app.runTx(execModeProcessProposal, txBz)
+	_, _, _, err = app.runTx(execModeProcessProposal, txBz, tx)
 	if err != nil {
 		return nil, err
 	}

baseapp/options.go

@@ -367,6 +367,15 @@ func (app *BaseApp) SetPrepareProposal(handler sdk.PrepareProposalHandler) {
 	app.prepareProposal = handler
 }
 
+// SetCheckTx sets the checkTx function for the BaseApp.
+func (app *BaseApp) SetCheckTxHandler(handler sdk.CheckTxHandler) {
+	if app.sealed {
+		panic("SetCheckTx() on sealed BaseApp")
+	}
+
+	app.checkTxHandler = handler
+}
+
 func (app *BaseApp) SetExtendVoteHandler(handler sdk.ExtendVoteHandler) {
 	if app.sealed {
 		panic("SetExtendVoteHandler() on sealed BaseApp")

baseapp/test_helpers.go

@@ -19,13 +19,13 @@ func (app *BaseApp) SimCheck(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, *
 		return sdk.GasInfo{}, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err)
 	}
 
-	gasInfo, result, _, err := app.runTx(execModeCheck, bz)
+	gasInfo, result, _, err := app.runTx(execModeCheck, bz, tx)
 	return gasInfo, result, err
 }
 
 // Simulate executes a tx in simulate mode to get result and gas info.
 func (app *BaseApp) Simulate(txBytes []byte) (sdk.GasInfo, *sdk.Result, error) {
-	gasInfo, result, _, err := app.runTx(execModeSimulate, txBytes)
+	gasInfo, result, _, err := app.runTx(execModeSimulate, txBytes, nil)
 	return gasInfo, result, err
 }
 
@@ -36,7 +36,7 @@ func (app *BaseApp) SimDeliver(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo,
 		return sdk.GasInfo{}, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err)
 	}
 
-	gasInfo, result, _, err := app.runTx(execModeFinalize, bz)
+	gasInfo, result, _, err := app.runTx(execModeFinalize, bz, tx)
 	return gasInfo, result, err
 }
 

docs/build/abci/04-checktx.md

@@ -0,0 +1,50 @@
+# CheckTx
+
+CheckTx is called by the `BaseApp` when comet receives a transaction from a client, over the p2p network or RPC. The CheckTx method is responsible for validating the transaction and returning an error if the transaction is invalid. 
+
+```mermaid
+graph TD
+    subgraph SDK[Cosmos SDK]
+        B[Baseapp]
+        A[AnteHandlers]
+        B <-->|Validate TX| A
+    end
+    C[CometBFT] <-->|CheckTx|SDK
+    U((User)) -->|Submit TX| C
+    N[P2P] -->|Receive TX| C
+```
+
+```go reference
+https://github.com/cosmos/cosmos-sdk/blob/31c604762a434c7b676b6a89897ecbd7c4653a23/baseapp/abci.go#L350-L390
+```
+
+## CheckTx Handler
+
+`CheckTxHandler` allows users to extend the logic of `CheckTx`. `CheckTxHandler` is called by pasding context and the transaction bytes received through ABCI. It is required that the handler returns deterministic results given the same transaction bytes. 
+
+:::note
+we return the raw decoded transaction here to avoid decoding it twice.
+:::
+
+```go
+type CheckTxHandler func(ctx sdk.Context, tx []byte) (Tx, error)
+```
+
+Setting a custom `CheckTxHandler` is optional. It can be done from your app.go file:
+
+```go
+func NewSimApp(
+	logger log.Logger,
+	db corestore.KVStoreWithBatch,
+	traceStore io.Writer,
+	loadLatest bool,
+	appOpts servertypes.AppOptions,
+	baseAppOptions ...func(*baseapp.BaseApp),
+) *SimApp {
+  ...
+  // Create ChecktxHandler
+  checktxHandler := abci.NewCustomCheckTxHandler(...)
+  app.SetCheckTxHandler(checktxHandler)
+  ...
+}
+```

docs/build/building-apps/02-app-mempool.md

@@ -22,7 +22,7 @@ Notably it introduces the `PrepareProposal` and `ProcessProposal` steps of ABCI+
 
 ## Mempool
 
-+ Before we delve into `PrepareProposal` and `ProcessProposal`, let's first walk through the mempool concepts.
+* Before we delve into `PrepareProposal` and `ProcessProposal`, let's first walk through the mempool concepts.
 
 There are countless designs that an application developer can write for a mempool, the SDK opted to provide only simple mempool implementations.
 Namely, the SDK provides the following mempools:

docs/build/building-apps/06-system-tests.md

@@ -0,0 +1,58 @@
+---
+sidebar_position: 1
+---
+
+# System Tests
+
+System tests provide a framework to write and execute black box tests against a running chain. This adds another level
+of confidence on top of unit, integration, and simulations tests, ensuring that business-critical scenarios
+(like double signing prevention) or scenarios that can't be tested otherwise (like a chain upgrade) are covered.
+
+## Vanilla Go for Flow Control
+
+System tests are vanilla Go tests that interact with the compiled chain binary. The `test runner` component starts a
+local testnet of 4 nodes (by default) and provides convenient helper methods for accessing the
+`system under test (SUT)`.
+A `CLI wrapper` makes it easy to access keys, submit transactions, or execute operations. Together, these components
+enable the replication and validation of complex business scenarios.
+
+Here's an example of a double signing test, where a new node is added with the same key as the first validator:
+[double signing test example](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/tests/systemtests/fraud_test.go)
+
+The [getting started tutorial](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/tests/systemtests/getting_started.md)
+contains a step-by-step guide to building and running your first system test. It covers setting chain state via genesis
+or
+transactions and validation via transaction response or queries.
+
+## Design Principles and Guidelines
+
+System tests are slower compared to unit or integration tests as they interact with a running chain. Therefore, certain
+principles can guide their usage:
+
+- **Perspective:** Tests should mimic a human interacting with the chain from the outside. Initial states can be set via
+  genesis or transactions to support a test scenario.
+- **Roles:** The user can have multiple roles such as validator, delegator, granter, or group admin.
+- **Focus:** Tests should concentrate on happy paths or business-critical workflows. Unit and integration tests are
+  better suited for more fine-grained testing.
+- **Workflows:** Test workflows and scenarios, not individual units. Given the high setup costs, it is reasonable to
+  combine multiple steps and assertions in a single test method.
+- **Genesis Mods:** Genesis modifications can incur additional time costs for resetting dirty states. Reuse existing
+  accounts (node0..n) whenever possible.
+- **Framework:** Continuously improve the framework for better readability and reusability.
+
+## Errors and Debugging
+
+All output is logged to `systemtests/testnet/node{0..n}.out`. Usually, `node0.out` is very noisy as it receives the CLI
+connections. Prefer any other node's log to find stack traces or error messages.
+
+Using system tests for state setup during debugging has become very handy:
+
+- Start the test with one node only and verbose output:
+
+  ```sh
+  go test -v -tags=system_test ./ --run TestAccountCreation --verbose --nodes-count=1
+  ```
+
+- Copy the CLI command for the transaction and modify the test to stop before the command
+- Start the node with `--home=<project-home>/tests/systemtests/testnet/node0/<binary-name>/` in debug mode
+- Execute CLI command from shell and enter breakpoints

docs/build/building-modules/16-testing.md

@@ -86,38 +86,26 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/tests/integration/bank
 
 ## Simulations
 
-Simulations uses as well a minimal application, built with [`depinject`](../packages/01-depinject.md):
+Simulations fuzz tests for deterministic message execution. They use a minimal application, built with [`depinject`](../packages/01-depinject.md):
 
 :::note
-You can as well use the `AppConfig` `configurator` for creating an `AppConfig` [inline](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/slashing/app_test.go#L54-L62). There is no difference between those two ways, use whichever you prefer.
+Simulations have been refactored to message factories
 :::
 
-Following is an example for `x/gov/` simulations:
+An example for `x/bank/` simulations:
 
 ```go reference
-https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/gov/simulation/operations_test.go#L406-L430
+https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/bank/simulation/msg_factory.go#L13-L20
 ```
 
-```go reference
-https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/gov/simulation/operations_test.go#L90-L132
-```
-
-## End-to-end Tests
+## System Tests
 
-End-to-end tests are at the top of the [test pyramid](https://martinfowler.com/articles/practical-test-pyramid.html).
-They must test the whole application flow, from the user perspective (for instance, CLI tests). They are located under [`/tests/e2e`](https://github.com/cosmos/cosmos-sdk/tree/main/tests/e2e).
+System tests are at the top of the [test pyramid](https://martinfowler.com/articles/practical-test-pyramid.html).
+They test the whole application flow as black box, from the user perspective. They are located under [`/tests/systemtests`](https://github.com/cosmos/cosmos-sdk/tree/main/tests/systemtests).
 
-<!-- @julienrbrt: makes more sense to use an app wired app to have 0 simapp dependencies -->
-For that, the SDK is using `simapp` but you should use your own application (`appd`).
-Here are some examples:
+For that, the SDK is using the `simapp` binary, but you should use your own binary.
+More details about system test can be found in [building-apps](https://docs.cosmos.network/main/build/building-apps/06-system-tests.md)
 
-* SDK E2E tests: <https://github.com/cosmos/cosmos-sdk/tree/main/tests/e2e>.
-* Cosmos Hub E2E tests: <https://github.com/cosmos/gaia/tree/main/tests/e2e>.
-* Osmosis E2E tests: <https://github.com/osmosis-labs/osmosis/tree/main/tests/e2e>.
-
-:::note warning
-The SDK is in the process of creating its E2E tests, as defined in [ADR-59](https://docs.cosmos.network/main/build/architecture/adr-059-test-scopes). This page will eventually be updated with better examples.
-:::
 
 ## Learn More
 

types/abci.go

@@ -23,6 +23,11 @@ type ProcessProposalHandler func(Context, *abci.ProcessProposalRequest) (*abci.P
 // PrepareProposalHandler defines a function type alias for preparing a proposal
 type PrepareProposalHandler func(Context, *abci.PrepareProposalRequest) (*abci.PrepareProposalResponse, error)
 
+// CheckTxHandler defines a function type alias for executing logic before transactions are executed.
+// `RunTx` is a function type alias for executing logic before transactions are executed.
+// The passed in runtx does not override antehandlers, the execution mode is not passed into runtx to avoid overriding the execution mode.
+type CheckTxHandler func(func(txBytes []byte, tx Tx) (gInfo GasInfo, result *Result, anteEvents []abci.Event, err error), *abci.CheckTxRequest) (*abci.CheckTxResponse, error)
+
 // ExtendVoteHandler defines a function type alias for extending a pre-commit vote.
 type ExtendVoteHandler func(Context, *abci.ExtendVoteRequest) (*abci.ExtendVoteResponse, error)