From 4932f115d4dd1331a08b54ec78cd6c8b9daead61 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 8 Mar 2023 14:19:51 -0500 Subject: [PATCH] feat: Create ABCI++ Verfication Methods (#15298) --- baseapp/baseapp.go | 177 ++++++++++++++-------- docs/docs/building-apps/02-app-mempool.md | 6 +- simapp/app.go | 34 +++-- simapp/app_v2.go | 41 +++-- 4 files changed, 169 insertions(+), 89 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index cff9075464..80fd8578d7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -1,12 +1,17 @@ package baseapp import ( - "errors" "fmt" "sort" "strings" + errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" + "cosmossdk.io/store" + storemetrics "cosmossdk.io/store/metrics" + "cosmossdk.io/store/snapshots" + storetypes "cosmossdk.io/store/types" + "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/crypto/tmhash" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" @@ -14,18 +19,23 @@ import ( "github.com/cosmos/gogoproto/proto" "golang.org/x/exp/maps" - errorsmod "cosmossdk.io/errors" - "cosmossdk.io/store" - storemetrics "cosmossdk.io/store/metrics" - "cosmossdk.io/store/snapshots" - storetypes "cosmossdk.io/store/types" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/mempool" ) +type ( + // Enum mode for app.runTx + runTxMode uint8 + + // StoreLoader defines a customizable function to control how we load the CommitMultiStore + // from disk. This is useful for state migration, when loading a datastore written with + // an older version of the software. In particular, if a module changed the substore key name + // (or removed a substore) between two versions of the software. + StoreLoader func(ms storetypes.CommitMultiStore) error +) + const ( runTxModeCheck runTxMode = iota // Check a transaction runTxModeReCheck // Recheck a (pending) transaction after a commit @@ -37,17 +47,6 @@ const ( var _ abci.Application = (*BaseApp)(nil) -type ( - // Enum mode for app.runTx - runTxMode uint8 - - // StoreLoader defines a customizable function to control how we load the CommitMultiStore - // from disk. This is useful for state migration, when loading a datastore written with - // an older version of the software. In particular, if a module changed the substore key name - // (or removed a substore) between two versions of the software. - StoreLoader func(ms storetypes.CommitMultiStore) error -) - // BaseApp reflects the ABCI application implementation. type BaseApp struct { //nolint: maligned // initialized on creation @@ -175,12 +174,14 @@ func NewBaseApp( app.SetMempool(mempool.NoOpMempool{}) } - if app.processProposal == nil { - app.SetProcessProposal(app.DefaultProcessProposal()) - } + abciProposalHandler := NewDefaultProposalHandler(app.mempool, app) if app.prepareProposal == nil { - app.SetPrepareProposal(app.DefaultPrepareProposal()) + app.SetPrepareProposal(abciProposalHandler.PrepareProposalHandler()) + } + + if app.processProposal == nil { + app.SetProcessProposal(abciProposalHandler.ProcessProposalHandler()) } if app.interBlockCache != nil { @@ -856,7 +857,69 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { return sdk.Events{msgEvent}.AppendEvents(events) } -// DefaultPrepareProposal returns the default implementation for processing an +// PrepareProposalVerifyTx performs transaction verification when a proposer is +// creating a block proposal during PrepareProposal. Any state committed to the +// PrepareProposal state internally will be discarded. will be +// returned if the transaction cannot be encoded. will be returned if +// the transaction is valid, otherwise will be returned. +func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { + bz, err := app.txEncoder(tx) + if err != nil { + return nil, err + } + + _, _, _, _, err = app.runTx(runTxPrepareProposal, bz) //nolint:dogsled + if err != nil { + return nil, err + } + + return bz, nil +} + +// ProcessProposalVerifyTx performs transaction verification when receiving a +// block proposal during ProcessProposal. Any state committed to the +// ProcessProposal state internally will be discarded. will be +// returned if the transaction cannot be decoded. will be returned if +// the transaction is valid, otherwise will be returned. +func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) { + tx, err := app.txDecoder(txBz) + if err != nil { + return nil, err + } + + _, _, _, _, err = app.runTx(runTxProcessProposal, txBz) //nolint:dogsled + if err != nil { + return nil, err + } + + return tx, nil +} + +type ( + // ProposalTxVerifier defines the interface that is implemented by BaseApp, + // that any custom ABCI PrepareProposal and ProcessProposal handler can use + // to verify a transaction. + ProposalTxVerifier interface { + PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) + ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) + } + + // DefaultProposalHandler defines the default ABCI PrepareProposal and + // ProcessProposal handlers. + DefaultProposalHandler struct { + mempool mempool.Mempool + txVerifier ProposalTxVerifier + } +) + +func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier) DefaultProposalHandler { + return DefaultProposalHandler{ + mempool: mp, + txVerifier: txVerifier, + } +} + +// PrepareProposalHandler returns the default implementation for processing an // ABCI proposal. The application's mempool is enumerated and all valid // transactions are added to the proposal. Transactions are valid if they: // @@ -876,74 +939,68 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { // - If no mempool is set or if the mempool is a no-op mempool, the transactions // requested from CometBFT will simply be returned, which, by default, are in // FIFO order. -func (app *BaseApp) DefaultPrepareProposal() sdk.PrepareProposalHandler { +func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { return func(ctx sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { // If the mempool is nil or a no-op mempool, we simply return the transactions // requested from CometBFT, which, by default, should be in FIFO order. - _, isNoOp := app.mempool.(mempool.NoOpMempool) - if app.mempool == nil || isNoOp { + _, isNoOp := h.mempool.(mempool.NoOpMempool) + if h.mempool == nil || isNoOp { return abci.ResponsePrepareProposal{Txs: req.Txs} } var ( - txsBytes [][]byte - byteCount int64 + selectedTxs [][]byte + totalTxBytes int64 ) - iterator := app.mempool.Select(ctx, req.Txs) + iterator := h.mempool.Select(ctx, req.Txs) + for iterator != nil { memTx := iterator.Tx() - bz, err := app.txEncoder(memTx) + // NOTE: Since transaction verification was already executed in CheckTx, + // which calls mempool.Insert, in theory everything in the pool should be + // valid. But some mempool implementations may insert invalid txs, so we + // check again. + bz, err := h.txVerifier.PrepareProposalVerifyTx(memTx) if err != nil { - panic(err) - } - - txSize := int64(len(bz)) - - // NOTE: Since runTx was already executed in CheckTx, which calls - // mempool.Insert, ideally everything in the pool should be valid. But - // some mempool implementations may insert invalid txs, so we check again. - _, _, _, _, err = app.runTx(runTxPrepareProposal, bz) - if err != nil { - err := app.mempool.Remove(memTx) + err := h.mempool.Remove(memTx) if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { panic(err) } - - iterator = iterator.Next() - continue - } else if byteCount += txSize; byteCount <= req.MaxTxBytes { - txsBytes = append(txsBytes, bz) } else { - break + txSize := int64(len(bz)) + if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes { + selectedTxs = append(selectedTxs, bz) + } else { + // We've reached capacity per req.MaxTxBytes so we cannot select any + // more transactions. + break + } } iterator = iterator.Next() } - return abci.ResponsePrepareProposal{Txs: txsBytes} + return abci.ResponsePrepareProposal{Txs: selectedTxs} } } -// DefaultProcessProposal returns the default implementation for processing an ABCI proposal. -// Every transaction in the proposal must pass 2 conditions: +// ProcessProposalHandler returns the default implementation for processing an +// ABCI proposal. Every transaction in the proposal must pass 2 conditions: // // 1. The transaction bytes must decode to a valid transaction. // 2. The transaction must be valid (i.e. pass runTx, AnteHandler only) // -// If any transaction fails to pass either condition, the proposal is rejected. Note that step (2) is identical to the -// validation step performed in DefaultPrepareProposal. It is very important that the same validation logic is used -// in both steps, and applications must ensure that this is the case in non-default handlers. -func (app *BaseApp) DefaultProcessProposal() sdk.ProcessProposalHandler { +// If any transaction fails to pass either condition, the proposal is rejected. +// Note that step (2) is identical to the validation step performed in +// DefaultPrepareProposal. It is very important that the same validation logic +// is used in both steps, and applications must ensure that this is the case in +// non-default handlers. +func (h DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { return func(ctx sdk.Context, req abci.RequestProcessProposal) abci.ResponseProcessProposal { for _, txBytes := range req.Txs { - _, err := app.txDecoder(txBytes) - if err != nil { - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} - } - - _, _, _, _, err = app.runTx(runTxProcessProposal, txBytes) + _, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) if err != nil { return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} } diff --git a/docs/docs/building-apps/02-app-mempool.md b/docs/docs/building-apps/02-app-mempool.md index f365fa0748..84699a6640 100644 --- a/docs/docs/building-apps/02-app-mempool.md +++ b/docs/docs/building-apps/02-app-mempool.md @@ -54,7 +54,8 @@ favor of a custom implementation in [`app.go`](./01-app-go-v2.md): ```go prepareOpt := func(app *baseapp.BaseApp) { - app.SetPrepareProposal(app.DefaultPrepareProposal()) + abciPropHandler := baseapp.NewDefaultProposalHandler(mempool, app) + app.SetPrepareProposal(abciPropHandler.PrepareProposalHandler()) } baseAppOptions = append(baseAppOptions, prepareOpt) @@ -83,7 +84,8 @@ Like `PrepareProposal` this implementation is the default and can be modified by ```go processOpt := func(app *baseapp.BaseApp) { - app.SetProcessProposal(app.DefaultProcessProposal()) + abciPropHandler := baseapp.NewDefaultProposalHandler(mempool, app) + app.SetProcessProposal(abciPropHandler.ProcessProposalHandler()) } baseAppOptions = append(baseAppOptions, processOpt) diff --git a/simapp/app.go b/simapp/app.go index befebfbeb6..d28a40c9f4 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -225,21 +225,31 @@ func NewSimApp( interfaceRegistry := encodingConfig.InterfaceRegistry txConfig := encodingConfig.TxConfig - // Below we could construct and set an application specific mempool and ABCI 1.0 Prepare and Process Proposal - // handlers. These defaults are already set in the SDK's BaseApp, this shows an example of how to override + // 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 // them. // - // nonceMempool := mempool.NewSenderNonceMempool() - // mempoolOpt := baseapp.SetMempool(nonceMempool) - // prepareOpt := func(app *baseapp.BaseApp) { - // app.SetPrepareProposal(app.DefaultPrepareProposal()) - // } - // processOpt := func(app *baseapp.BaseApp) { - // app.SetProcessProposal(app.DefaultProcessProposal()) - // } + // Example: // - // Further down we'd set the options in the AppBuilder like below. - // baseAppOptions = append(baseAppOptions, mempoolOpt, prepareOpt, processOpt) + // bApp := baseapp.NewBaseApp(...) + // nonceMempool := mempool.NewSenderNonceMempool() + // abciPropHandler := NewDefaultProposalHandler(nonceMempool, bApp) + // + // bApp.SetMempool(nonceMempool) + // bApp.SetPrepareProposal(abciPropHandler.PrepareProposalHandler()) + // bApp.SetProcessProposal(abciPropHandler.ProcessProposalHandler()) + // + // Alternatively, you can construct BaseApp options, append those to + // baseAppOptions and pass them to NewBaseApp. + // + // Example: + // + // prepareOpt = func(app *baseapp.BaseApp) { + // abciPropHandler := baseapp.NewDefaultProposalHandler(nonceMempool, app) + // app.SetPrepareProposal(abciPropHandler.PrepareProposalHandler()) + // } + // baseAppOptions = append(baseAppOptions, prepareOpt) bApp := baseapp.NewBaseApp(appName, logger, db, txConfig.TxDecoder(), baseAppOptions...) bApp.SetCommitMultiStoreTracer(traceStore) diff --git a/simapp/app_v2.go b/simapp/app_v2.go index 059f073482..6f92be3fe0 100644 --- a/simapp/app_v2.go +++ b/simapp/app_v2.go @@ -167,21 +167,6 @@ func NewSimApp( var ( app = &SimApp{} appBuilder *runtime.AppBuilder - // Below we could construct and set an application specific mempool and ABCI 1.0 Prepare and Process Proposal - // handlers. These defaults are already set in the SDK's BaseApp, this shows an example of how to override - // them. - // - // nonceMempool = mempool.NewSenderNonceMempool() - // mempoolOpt = baseapp.SetMempool(nonceMempool) - // prepareOpt = func(app *baseapp.BaseApp) { - // app.SetPrepareProposal(app.DefaultPrepareProposal()) - // } - // processOpt = func(app *baseapp.BaseApp) { - // app.SetProcessProposal(app.DefaultProcessProposal()) - // } - // - // Further down we'd set the options in the AppBuilder like below. - // baseAppOptions = append(baseAppOptions, mempoolOpt, prepareOpt, processOpt) // merge the AppConfig and other configuration in one config appConfig = depinject.Configs( @@ -244,6 +229,32 @@ func NewSimApp( panic(err) } + // 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 + // them. + // + // Example: + // + // app.App = appBuilder.Build(...) + // nonceMempool := mempool.NewSenderNonceMempool() + // abciPropHandler := NewDefaultProposalHandler(nonceMempool, app.App.BaseApp) + // + // app.App.BaseApp.SetMempool(nonceMempool) + // app.App.BaseApp.SetPrepareProposal(abciPropHandler.PrepareProposalHandler()) + // app.App.BaseApp.SetProcessProposal(abciPropHandler.ProcessProposalHandler()) + // + // Alternatively, you can construct BaseApp options, append those to + // baseAppOptions and pass them to the appBuilder. + // + // Example: + // + // prepareOpt = func(app *baseapp.BaseApp) { + // abciPropHandler := baseapp.NewDefaultProposalHandler(nonceMempool, app) + // app.SetPrepareProposal(abciPropHandler.PrepareProposalHandler()) + // } + // baseAppOptions = append(baseAppOptions, prepareOpt) + app.App = appBuilder.Build(logger, db, traceStore, baseAppOptions...) if err := app.App.BaseApp.SetStreamingService(appOpts, app.appCodec, app.kvStoreKeys()); err != nil {