From 176b1627c2bba7f1af3500da52534dc7d4bc815a Mon Sep 17 00:00:00 2001 From: David Terpay <35130517+davidterpay@users.noreply.github.com> Date: Tue, 25 Jun 2024 10:48:18 -0400 Subject: [PATCH 1/2] chore: Default Process Proposal (#543) * init * nits (cherry picked from commit 79d7ef7c3309d74c6f66f8a9692a9a051edf83c0) # Conflicts: # abci/abci.go --- abci/abci.go | 48 ++++++++++++++++++++++++++++++++++++++++++--- abci/abci_test.go | 12 ++++++++---- abci/utils_test.go | 3 ++- tests/app/README.md | 2 ++ tests/app/app.go | 11 ++++++++++- 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/abci/abci.go b/abci/abci.go index 8b2c9554..38f2ea4e 100644 --- a/abci/abci.go +++ b/abci/abci.go @@ -7,6 +7,7 @@ import ( abci "github.com/cometbft/cometbft/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/skip-mev/block-sdk/v2/block" "github.com/skip-mev/block-sdk/v2/block/proposals" "github.com/skip-mev/block-sdk/v2/block/utils" @@ -16,28 +17,65 @@ type ( // ProposalHandler is a wrapper around the ABCI++ PrepareProposal and ProcessProposal // handlers. ProposalHandler struct { +<<<<<<< HEAD logger log.Logger txDecoder sdk.TxDecoder txEncoder sdk.TxEncoder prepareLanesHandler block.PrepareLanesHandler mempool block.Mempool +======= + logger log.Logger + txDecoder sdk.TxDecoder + txEncoder sdk.TxEncoder + mempool block.Mempool + useCustomProcessProposal bool +>>>>>>> 79d7ef7 (chore: Default Process Proposal (#543)) } ) -// NewProposalHandler returns a new ABCI++ proposal handler. This proposal handler will -// iteratively call each of the lanes in the chain to prepare and process the proposal. -func NewProposalHandler( +// NewDefaultProposalHandler returns a new ABCI++ proposal handler. This proposal handler will +// iteratively call each of the lanes in the chain to prepare and process the proposal. This +// will not use custom process proposal logic. +func NewDefaultProposalHandler( logger log.Logger, txDecoder sdk.TxDecoder, txEncoder sdk.TxEncoder, mempool block.Mempool, ) *ProposalHandler { return &ProposalHandler{ +<<<<<<< HEAD logger: logger, txDecoder: txDecoder, txEncoder: txEncoder, prepareLanesHandler: ChainPrepareLanes(mempool.Registry()), mempool: mempool, +======= + logger: logger, + txDecoder: txDecoder, + txEncoder: txEncoder, + mempool: mempool, + useCustomProcessProposal: false, + } +} + +// New returns a new ABCI++ proposal handler with the ability to use custom process proposal logic. +// +// NOTE: It is highly recommended to use the default proposal handler unless you have a specific +// use case that requires custom process proposal logic. +func New( + logger log.Logger, + txDecoder sdk.TxDecoder, + txEncoder sdk.TxEncoder, + mempool block.Mempool, + useCustomProcessProposal bool, +) *ProposalHandler { + return &ProposalHandler{ + logger: logger, + txDecoder: txDecoder, + txEncoder: txEncoder, + mempool: mempool, + useCustomProcessProposal: useCustomProcessProposal, +>>>>>>> 79d7ef7 (chore: Default Process Proposal (#543)) } } @@ -110,6 +148,10 @@ func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { // verify all transactions in the proposal that belong to the lane and pass any remaining transactions // to the next lane in the chain. func (h *ProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { + if !h.useCustomProcessProposal { + return baseapp.NoOpProcessProposal() + } + return func(ctx sdk.Context, req *abci.RequestProcessProposal) (resp *abci.ResponseProcessProposal, err error) { if req.Height <= 1 { return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil diff --git a/abci/abci_test.go b/abci/abci_test.go index 879503c9..88c825a6 100644 --- a/abci/abci_test.go +++ b/abci/abci_test.go @@ -498,11 +498,12 @@ func (s *ProposalsTestSuite) TestPrepareProposalEdgeCases() { ) s.Require().NoError(err) - proposalHandler := abci.NewProposalHandler( + proposalHandler := abci.New( log.NewNopLogger(), s.encodingConfig.TxConfig.TxDecoder(), s.encodingConfig.TxConfig.TxEncoder(), mempool, + true, ).PrepareProposalHandler() maxTxBytes := s.ctx.ConsensusParams().Block.MaxBytes @@ -545,11 +546,12 @@ func (s *ProposalsTestSuite) TestPrepareProposalEdgeCases() { ) s.Require().NoError(err) - proposalHandler := abci.NewProposalHandler( + proposalHandler := abci.New( log.NewNopLogger(), s.encodingConfig.TxConfig.TxDecoder(), s.encodingConfig.TxConfig.TxEncoder(), mempool, + true, ).PrepareProposalHandler() maxTxBytes := s.ctx.ConsensusParams().Block.MaxBytes @@ -594,11 +596,12 @@ func (s *ProposalsTestSuite) TestPrepareProposalEdgeCases() { ) s.Require().NoError(err) - proposalHandler := abci.NewProposalHandler( + proposalHandler := abci.New( log.NewNopLogger(), s.encodingConfig.TxConfig.TxDecoder(), s.encodingConfig.TxConfig.TxEncoder(), mempool, + true, ).PrepareProposalHandler() maxTxBytes := s.ctx.ConsensusParams().Block.MaxBytes @@ -643,11 +646,12 @@ func (s *ProposalsTestSuite) TestPrepareProposalEdgeCases() { ) s.Require().NoError(err) - proposalHandler := abci.NewProposalHandler( + proposalHandler := abci.New( log.NewNopLogger(), s.encodingConfig.TxConfig.TxDecoder(), s.encodingConfig.TxConfig.TxEncoder(), mempool, + true, ).PrepareProposalHandler() maxTxBytes := s.ctx.ConsensusParams().Block.MaxBytes diff --git a/abci/utils_test.go b/abci/utils_test.go index eff8dcd6..daf58a99 100644 --- a/abci/utils_test.go +++ b/abci/utils_test.go @@ -148,11 +148,12 @@ func (s *ProposalsTestSuite) setUpProposalHandlers(lanes []block.Lane) *abci.Pro mempool, err := block.NewLanedMempool(log.NewNopLogger(), lanes) s.Require().NoError(err) - return abci.NewProposalHandler( + return abci.New( log.NewNopLogger(), s.encodingConfig.TxConfig.TxDecoder(), s.encodingConfig.TxConfig.TxEncoder(), mempool, + true, ) } diff --git a/tests/app/README.md b/tests/app/README.md index 2a2eaa80..dc554ff0 100644 --- a/tests/app/README.md +++ b/tests/app/README.md @@ -19,6 +19,8 @@ There are fix critical steps to building a test app that uses the Block SDK: 5. Setting the antehandlers - used for transaction validation - for each lane. 6. Setting the proposal handlers - used for block creation and verification - for the application to utilize the Block SDK's Prepare and Process Proposal handlers. +**IMPORTANT NOTE:** It is recommended that applications use the **`NewDefaultProposalHandler`** when constructing their Prepare and Process Proposal handlers. This is because the priority nonce mempool - the underlying storage device of transactions - has non-deterministic ordering of transactions on chains with multiple fees. The use of the `NewDefaultProposalHandler` ensures that the ordering of transactions on proposal construction follows the ordering logic of the lanes, which is deterministic, and that process proposal optimistically assumes that the ordering of transactions is correct. + ### 1. Signer Extractor The signer extractor is responsible for extracting signers and relevant information about who is signing the transaction. We recommend using the default implementation provided by the Block SDK. diff --git a/tests/app/app.go b/tests/app/app.go index 38f66714..1df310c7 100644 --- a/tests/app/app.go +++ b/tests/app/app.go @@ -304,11 +304,20 @@ func New( // Step 6: Create the proposal handler and set it on the app. Now the application // will build and verify proposals using the Block SDK! - proposalHandler := abci.NewProposalHandler( + // + // NOTE: It is recommended to use the default proposal handler by constructing + // using the NewDefaultProposalHandler function. This will use the correct prepare logic + // for the lanes, but the process logic will be a no-op. To read more about the default + // proposal handler, see the documentation in readme.md in this directory. + // + // If you want to customize the prepare and process logic, you can construct the proposal + // handler using the New function and setting the useProcess flag to true. + proposalHandler := abci.New( // use NewDefaultProposalHandler instead for default behavior (RECOMMENDED) app.Logger(), app.TxConfig().TxDecoder(), app.TxConfig().TxEncoder(), mempool, + true, ) app.App.SetPrepareProposal(proposalHandler.PrepareProposalHandler()) app.App.SetProcessProposal(proposalHandler.ProcessProposalHandler()) From 6268d799086515eda0afc190799015b9392bf310 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 25 Jun 2024 12:06:06 -0400 Subject: [PATCH 2/2] nit test case --- abci/abci.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/abci/abci.go b/abci/abci.go index 38f2ea4e..ef223296 100644 --- a/abci/abci.go +++ b/abci/abci.go @@ -17,19 +17,11 @@ type ( // ProposalHandler is a wrapper around the ABCI++ PrepareProposal and ProcessProposal // handlers. ProposalHandler struct { -<<<<<<< HEAD - logger log.Logger - txDecoder sdk.TxDecoder - txEncoder sdk.TxEncoder - prepareLanesHandler block.PrepareLanesHandler - mempool block.Mempool -======= logger log.Logger txDecoder sdk.TxDecoder txEncoder sdk.TxEncoder mempool block.Mempool useCustomProcessProposal bool ->>>>>>> 79d7ef7 (chore: Default Process Proposal (#543)) } ) @@ -43,13 +35,6 @@ func NewDefaultProposalHandler( mempool block.Mempool, ) *ProposalHandler { return &ProposalHandler{ -<<<<<<< HEAD - logger: logger, - txDecoder: txDecoder, - txEncoder: txEncoder, - prepareLanesHandler: ChainPrepareLanes(mempool.Registry()), - mempool: mempool, -======= logger: logger, txDecoder: txDecoder, txEncoder: txEncoder, @@ -75,7 +60,6 @@ func New( txEncoder: txEncoder, mempool: mempool, useCustomProcessProposal: useCustomProcessProposal, ->>>>>>> 79d7ef7 (chore: Default Process Proposal (#543)) } } @@ -113,7 +97,8 @@ func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { proposal := proposals.NewProposal(h.logger, req.MaxTxBytes, maxGasLimit) // Fill the proposal with transactions from each lane. - finalProposal, err := h.prepareLanesHandler(ctx, proposal) + prepareLanesHandler := ChainPrepareLanes(h.mempool.Registry()) + finalProposal, err := prepareLanesHandler(ctx, proposal) if err != nil { h.logger.Error("failed to prepare proposal", "err", err) return &abci.ResponsePrepareProposal{Txs: make([][]byte, 0)}, err