From d66b7fa1895a9369739a594b8cdc4aa627fb26f5 Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Fri, 11 Nov 2022 11:57:53 -0600 Subject: [PATCH] feat: Settable PrepareProposal handler (#13831) Co-authored-by: Jeancarlo Barrios --- baseapp/abci.go | 40 +++------------------------- baseapp/abci_v1_test.go | 8 +++--- baseapp/baseapp.go | 59 ++++++++++++++++++++++++++++++++++++++++- baseapp/options.go | 21 +++++++++++---- simapp/app.go | 17 ++++++++++-- types/abci.go | 9 +++++-- 6 files changed, 102 insertions(+), 52 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index def13328aa..a5e1e5f9c5 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -21,7 +21,6 @@ import ( "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/types/mempool" ) // Supported ABCI Query prefixes @@ -249,44 +248,11 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc // Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md // Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - var ( - txsBytes [][]byte - byteCount int64 - ) - ctx := app.getContextForTx(runTxPrepareProposal, []byte{}) - iterator := app.mempool.Select(ctx, req.Txs) - - for iterator != nil { - memTx := iterator.Tx() - - bz, err := app.txEncoder(memTx) - if err != nil { - panic(err) - } - - txSize := int64(len(bz)) - - // NOTE: runTx was already run in CheckTx which calls mempool.Insert so 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) - 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 - } - - iterator = iterator.Next() + if app.prepareProposal == nil { + panic("PrepareProposal method not set") } - - return abci.ResponsePrepareProposal{Txs: txsBytes} + return app.prepareProposal(ctx, req) } // ProcessProposal implements the ProcessProposal ABCI method and returns a diff --git a/baseapp/abci_v1_test.go b/baseapp/abci_v1_test.go index 83cb7c6a75..4bb6d9296a 100644 --- a/baseapp/abci_v1_test.go +++ b/baseapp/abci_v1_test.go @@ -37,6 +37,7 @@ type ABCIv1TestSuite struct { mempool mempool.Mempool txConfig client.TxConfig cdc codec.ProtoCodecMarshaler + options []func(app *baseapp.BaseApp) } func TestABCIv1TestSuite(t *testing.T) { @@ -59,7 +60,8 @@ func (s *ABCIv1TestSuite) SetupTest() { err := depinject.Inject(makeMinimalConfig(), &appBuilder, &cdc, ®istry) require.NoError(t, err) - app := setupBaseApp(t, anteOpt, baseapp.SetMempool(pool)) + s.options = append(s.options, baseapp.SetMempool(pool), anteOpt) + app := setupBaseApp(t, s.options...) baseapptestutil.RegisterInterfaces(registry) app.SetMsgServiceRouter(baseapp.NewMsgServiceRouter()) app.SetInterfaceRegistry(registry) @@ -121,10 +123,6 @@ func (s *ABCIv1TestSuite) TestABCIv1_HappyPath() { Txs: reqProposalTxBytes[:], } - s.baseApp.SetProcessProposal(nil) - require.Panics(t, func() { s.baseApp.ProcessProposal(reqProcessProposal) }) - s.baseApp.SetProcessProposal(s.baseApp.DefaultProcessProposal()) - resProcessProposal := s.baseApp.ProcessProposal(reqProcessProposal) require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 08a8746589..31f61e7199 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -67,6 +67,7 @@ type BaseApp struct { //nolint: maligned initChainer sdk.InitChainer // initialize state with validators and state blob beginBlocker sdk.BeginBlocker // logic to run before any txs processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal + prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal endBlocker sdk.EndBlocker // logic to run after all txs, and to determine valset changes addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID @@ -176,6 +177,10 @@ func NewBaseApp( app.SetProcessProposal(app.DefaultProcessProposal()) } + if app.prepareProposal == nil { + app.SetPrepareProposal(app.DefaultPrepareProposal()) + } + if app.interBlockCache != nil { app.cms.SetInterBlockCache(app.interBlockCache) } @@ -848,13 +853,65 @@ func createEvents(msg sdk.Msg) sdk.Events { return sdk.Events{msgEvent} } +// DefaultPrepareProposal 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: +// +// 1) Successfully encode to bytes. +// 2) Are valid (i.e. pass runTx, AnteHandler only) +// +// Enumeration is halted once RequestPrepareProposal.MaxBytes of transactions is reached or the mempool is exhausted. +// Note that step (2) is identical to the validation step performed in DefaultProcessProposal. 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) DefaultPrepareProposal() sdk.PrepareProposalHandler { + return func(ctx sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { + var ( + txsBytes [][]byte + byteCount int64 + ) + + iterator := app.mempool.Select(ctx, req.Txs) + for iterator != nil { + memTx := iterator.Tx() + + bz, err := app.txEncoder(memTx) + if err != nil { + panic(err) + } + + txSize := int64(len(bz)) + + // NOTE: runTx was already run in CheckTx which calls mempool.Insert so 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) + 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 + } + + iterator = iterator.Next() + } + return abci.ResponsePrepareProposal{Txs: txsBytes} + } +} + // DefaultProcessProposal 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. +// 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 { return func(ctx sdk.Context, req abci.RequestProcessProposal) abci.ResponseProcessProposal { for _, txBytes := range req.Txs { diff --git a/baseapp/options.go b/baseapp/options.go index d655a0e756..f41f4e961a 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -86,11 +86,6 @@ func SetMempool(mempool mempool.Mempool) func(*BaseApp) { return func(app *BaseApp) { app.SetMempool(mempool) } } -// SetProcessProposal sets the ProcessProposal handler. -func SetProcessProposal(proposalHandler sdk.ProcessProposalHandler) func(*BaseApp) { - return func(app *BaseApp) { app.SetProcessProposal(proposalHandler) } -} - func (app *BaseApp) SetName(name string) { if app.sealed { panic("SetName() on sealed BaseApp") @@ -266,9 +261,25 @@ func (app *BaseApp) SetQueryMultiStore(ms sdk.MultiStore) { // SetMempool sets the mempool for the BaseApp and is required for the app to start up. func (app *BaseApp) SetMempool(mempool mempool.Mempool) { + if app.sealed { + panic("SetMempool() on sealed BaseApp") + } app.mempool = mempool } +// SetProcessProposal sets the process proposal function for the BaseApp. func (app *BaseApp) SetProcessProposal(handler sdk.ProcessProposalHandler) { + if app.sealed { + panic("SetProcessProposal() on sealed BaseApp") + } app.processProposal = handler } + +// SetPrepareProposal sets the prepare proposal function for the BaseApp. +func (app *BaseApp) SetPrepareProposal(handler sdk.PrepareProposalHandler) { + if app.sealed { + panic("SetPrepareProposal() on sealed BaseApp") + } + + app.prepareProposal = handler +} diff --git a/simapp/app.go b/simapp/app.go index bb5328e993..45d518a0ac 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -14,7 +14,6 @@ import ( dbm "github.com/tendermint/tm-db" "cosmossdk.io/depinject" - "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" @@ -189,6 +188,21 @@ 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.NewNonceMempool() + //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( @@ -202,7 +216,6 @@ func NewSimApp( // // AUTH // - // For providing a custom function required in auth to generate custom account types // add it below. By default the auth module uses simulation.RandomGenesisAccounts. // diff --git a/types/abci.go b/types/abci.go index e08cbe449f..2679a5afc0 100644 --- a/types/abci.go +++ b/types/abci.go @@ -1,6 +1,8 @@ package types -import abci "github.com/tendermint/tendermint/abci/types" +import ( + abci "github.com/tendermint/tendermint/abci/types" +) // InitChainer initializes application state at genesis type InitChainer func(ctx Context, req abci.RequestInitChain) abci.ResponseInitChain @@ -21,4 +23,7 @@ type EndBlocker func(ctx Context, req abci.RequestEndBlock) abci.ResponseEndBloc type PeerFilter func(info string) abci.ResponseQuery // ProcessProposalHandler defines a function type alias for processing a proposer -type ProcessProposalHandler func(ctx Context, proposal abci.RequestProcessProposal) abci.ResponseProcessProposal +type ProcessProposalHandler func(Context, abci.RequestProcessProposal) abci.ResponseProcessProposal + +// PrepareProposalHandler defines a function type alias for preparing a proposal +type PrepareProposalHandler func(Context, abci.RequestPrepareProposal) abci.ResponsePrepareProposal