feat: Create ABCI++ Verfication Methods (#15298)

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. <nil, err> will be
+// returned if the transaction cannot be encoded. <bz, nil> will be returned if
+// the transaction is valid, otherwise <bz, err> 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. <nil, err> will be
+// returned if the transaction cannot be decoded. <Tx, nil> will be returned if
+// the transaction is valid, otherwise <Tx, err> 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}
 			}