From b13f5e999db6a11f918398ca26f8908869cf4ab7 Mon Sep 17 00:00:00 2001 From: David Terpay <35130517+davidterpay@users.noreply.github.com> Date: Mon, 24 Jul 2023 18:04:15 +0200 Subject: [PATCH] feat(v2): Validate basic on the VEs (#200) --- abci/abci_test.go | 3 ++ abci/proposal_auction.go | 6 ++++ abci/proposals.go | 44 +++++++++++++++------------ abci/proposals_test.go | 2 ++ abci/types.go | 16 ++++++++++ abci/vote_extensions.go | 59 +++++++++++++++++++++++++++++++++++- abci/vote_extensions_test.go | 8 ++++- go.mod | 4 +-- go.sum | 3 +- 9 files changed, 120 insertions(+), 25 deletions(-) diff --git a/abci/abci_test.go b/abci/abci_test.go index fb057c4..5805280 100644 --- a/abci/abci_test.go +++ b/abci/abci_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + sdklogger "cosmossdk.io/log" comettypes "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/libs/log" storetypes "github.com/cosmos/cosmos-sdk/store/types" @@ -131,8 +132,10 @@ func (suite *ABCITestSuite) SetupTest() { suite.logger, suite.encodingConfig.TxConfig.TxEncoder(), suite.encodingConfig.TxConfig.TxDecoder(), + abci.NoOpValidateVoteExtensionsFn(), ) suite.voteExtensionHandler = abci.NewVoteExtensionHandler( + sdklogger.NewTestLogger(suite.T()), suite.tobLane, suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder(), diff --git a/abci/proposal_auction.go b/abci/proposal_auction.go index 9517a9a..db74a48 100644 --- a/abci/proposal_auction.go +++ b/abci/proposal_auction.go @@ -86,6 +86,12 @@ func (h *ProposalHandler) VerifyTOB(ctx sdk.Context, proposalTxs [][]byte) (*Auc return nil, fmt.Errorf("failed to unmarshal last commit info from auction info: %w", err) } + // verify that the included vote extensions are valid in accordance with the + // the preferences of the application + if err := h.validateVoteExtensionsFn(ctx, ctx.BlockHeight(), lastCommitInfo); err != nil { + return nil, fmt.Errorf("failed to validate vote extensions: %w", err) + } + // Build the top of block proposal from the auction info. expectedTOB := h.BuildTOB(ctx, lastCommitInfo, auctionInfo.MaxTxBytes) diff --git a/abci/proposals.go b/abci/proposals.go index 4716b41..136eeda 100644 --- a/abci/proposals.go +++ b/abci/proposals.go @@ -1,9 +1,6 @@ package abci import ( - "errors" - "fmt" - cometabci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/libs/log" sdk "github.com/cosmos/cosmos-sdk/types" @@ -57,12 +54,24 @@ type ( // ProposalHandler contains the functionality and handlers required to\ // process, validate and build blocks. ProposalHandler struct { + logger log.Logger + txEncoder sdk.TxEncoder + txDecoder sdk.TxDecoder + + // prepareLanesHandler is responsible for preparing the proposal by selecting + // transactions from each lane according to each lane's selection logic. prepareLanesHandler blockbuster.PrepareLanesHandler + + // processLanesHandler is responsible for verifying that the proposal is valid + // according to each lane's verification logic. processLanesHandler blockbuster.ProcessLanesHandler - tobLane TOBLaneProposal - logger log.Logger - txEncoder sdk.TxEncoder - txDecoder sdk.TxDecoder + + // tobLane is the top of block lane which is utilized to verify transactions that + // should be included in the top of block. + tobLane TOBLaneProposal + + // validateVoteExtensionsFn is the function responsible for validating vote extensions. + validateVoteExtensionsFn ValidateVoteExtensionsFn } ) @@ -74,14 +83,16 @@ func NewProposalHandler( logger log.Logger, txEncoder sdk.TxEncoder, txDecoder sdk.TxDecoder, + validateVeFN ValidateVoteExtensionsFn, ) *ProposalHandler { return &ProposalHandler{ - prepareLanesHandler: abci.ChainPrepareLanes(lanes...), - processLanesHandler: abci.ChainProcessLanes(lanes...), - tobLane: tobLane, - logger: logger, - txEncoder: txEncoder, - txDecoder: txDecoder, + prepareLanesHandler: abci.ChainPrepareLanes(lanes...), + processLanesHandler: abci.ChainProcessLanes(lanes...), + tobLane: tobLane, + logger: logger, + txEncoder: txEncoder, + txDecoder: txDecoder, + validateVoteExtensionsFn: validateVeFN, } } @@ -165,10 +176,3 @@ func (h *ProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { return cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_ACCEPT} } } - -// RemoveTx removes a transaction from the application-side mempool. -func (h *ProposalHandler) RemoveTx(tx sdk.Tx) { - if err := h.tobLane.Remove(tx); err != nil && !errors.Is(err, sdkmempool.ErrTxNotFound) { - panic(fmt.Errorf("failed to remove invalid transaction from the mempool: %w", err)) - } -} diff --git a/abci/proposals_test.go b/abci/proposals_test.go index b059ea6..7d116bf 100644 --- a/abci/proposals_test.go +++ b/abci/proposals_test.go @@ -299,6 +299,7 @@ func (suite *ABCITestSuite) TestPrepareProposal() { suite.logger, suite.encodingConfig.TxConfig.TxEncoder(), suite.encodingConfig.TxConfig.TxDecoder(), + abci.NoOpValidateVoteExtensionsFn(), ) handler := suite.proposalHandler.PrepareProposalHandler() req := suite.createPrepareProposalRequest(maxTxBytes) @@ -765,6 +766,7 @@ func (suite *ABCITestSuite) TestProcessProposal() { suite.tobLane, log.NewNopLogger(), suite.encodingConfig.TxConfig.TxEncoder(), suite.encodingConfig.TxConfig.TxDecoder(), + abci.NoOpValidateVoteExtensionsFn(), ) tc.createTxs() diff --git a/abci/types.go b/abci/types.go index 2b8961e..a7097a7 100644 --- a/abci/types.go +++ b/abci/types.go @@ -7,6 +7,7 @@ against. package abci import ( + cometabci "github.com/cometbft/cometbft/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -56,3 +57,18 @@ type ( // pre-commit vote extension. VerifyVoteExtensionHandler func(sdk.Context, *RequestVerifyVoteExtension) (*ResponseVerifyVoteExtension, error) ) + +// ValidateVoteExtensionsFn defines the function for validating vote extensions. This +// function is not explicitly used to validate the oracle data but rather that +// the signed vote extensions included in the proposal are valid and provide +// a supermajority of vote extensions for the current block. This method is +// expected to be used in ProcessProposal, the expected ctx is the ProcessProposalState's ctx. +type ValidateVoteExtensionsFn func(ctx sdk.Context, currentHeight int64, extendedCommitInfo cometabci.ExtendedCommitInfo) error + +// NoOpValidateVoteExtensionsFn returns a ValidateVoteExtensionsFn that does nothing. This should NOT +// be used in production. +func NoOpValidateVoteExtensionsFn() ValidateVoteExtensionsFn { + return func(_ sdk.Context, _ int64, _ cometabci.ExtendedCommitInfo) error { + return nil + } +} diff --git a/abci/vote_extensions.go b/abci/vote_extensions.go index 4107a49..b8c7c13 100644 --- a/abci/vote_extensions.go +++ b/abci/vote_extensions.go @@ -4,6 +4,7 @@ import ( "crypto/sha256" "encoding/hex" + "cosmossdk.io/log" sdk "github.com/cosmos/cosmos-sdk/types" sdkmempool "github.com/cosmos/cosmos-sdk/types/mempool" "github.com/skip-mev/pob/blockbuster/lanes/auction" @@ -28,6 +29,10 @@ type ( // VoteExtensionHandler contains the functionality and handlers required to // process, validate and build vote extensions. VoteExtensionHandler struct { + logger log.Logger + + // tobLane is the top of block lane which is used to extract the top bidding + // auction transaction from the local mempool. tobLane TOBLaneVE // txDecoder is used to decode the top bidding auction transaction @@ -47,8 +52,9 @@ type ( // NewVoteExtensionHandler returns an VoteExtensionHandler that contains the functionality and handlers // required to inject, process, and validate vote extensions. -func NewVoteExtensionHandler(lane TOBLaneVE, txDecoder sdk.TxDecoder, txEncoder sdk.TxEncoder) *VoteExtensionHandler { +func NewVoteExtensionHandler(logger log.Logger, lane TOBLaneVE, txDecoder sdk.TxDecoder, txEncoder sdk.TxEncoder) *VoteExtensionHandler { return &VoteExtensionHandler{ + logger: logger, tobLane: lane, txDecoder: txDecoder, txEncoder: txEncoder, @@ -74,11 +80,25 @@ func (h *VoteExtensionHandler) ExtendVoteHandler() ExtendVoteHandler { cacheCtx, _ := ctx.CacheContext() if err := h.tobLane.VerifyTx(cacheCtx, bidTx); err == nil { + hash := sha256.Sum256(bidBz) + hashStr := hex.EncodeToString(hash[:]) + + h.logger.Info( + "extending vote with auction transaction", + "tx_hash", hashStr, + "height", ctx.BlockHeight(), + ) + return &ResponseExtendVote{VoteExtension: bidBz}, nil } } } + h.logger.Info( + "extending vote with no auction transaction", + "height", ctx.BlockHeight(), + ) + return &ResponseExtendVote{VoteExtension: []byte{}}, nil } } @@ -90,6 +110,11 @@ func (h *VoteExtensionHandler) VerifyVoteExtensionHandler() VerifyVoteExtensionH return func(ctx sdk.Context, req *RequestVerifyVoteExtension) (*ResponseVerifyVoteExtension, error) { txBz := req.VoteExtension if len(txBz) == 0 { + h.logger.Info( + "verifyed vote extension with no auction transaction", + "height", ctx.BlockHeight(), + ) + return &ResponseVerifyVoteExtension{Status: ResponseVerifyVoteExtension_ACCEPT}, nil } @@ -102,27 +127,59 @@ func (h *VoteExtensionHandler) VerifyVoteExtensionHandler() VerifyVoteExtensionH // Short circuit if we have already verified this vote extension if err, ok := h.cache[hash]; ok { if err != nil { + h.logger.Info( + "rejected vote extension", + "tx_hash", hash, + "height", ctx.BlockHeight(), + ) + return &ResponseVerifyVoteExtension{Status: ResponseVerifyVoteExtension_REJECT}, err } + h.logger.Info( + "verified vote extension", + "tx_hash", hash, + "height", ctx.BlockHeight(), + ) + return &ResponseVerifyVoteExtension{Status: ResponseVerifyVoteExtension_ACCEPT}, nil } // Decode the vote extension which should be a valid auction transaction bidTx, err := h.txDecoder(txBz) if err != nil { + h.logger.Info( + "rejected vote extension", + "tx_hash", hash, + "height", ctx.BlockHeight(), + "err", err, + ) + h.cache[hash] = err return &ResponseVerifyVoteExtension{Status: ResponseVerifyVoteExtension_REJECT}, err } // Verify the auction transaction and cache the result if err = h.tobLane.VerifyTx(ctx, bidTx); err != nil { + h.logger.Info( + "rejected vote extension", + "tx_hash", hash, + "height", ctx.BlockHeight(), + "err", err, + ) + h.cache[hash] = err return &ResponseVerifyVoteExtension{Status: ResponseVerifyVoteExtension_REJECT}, err } h.cache[hash] = nil + h.logger.Info( + "verified vote extension", + "tx_hash", hash, + "height", ctx.BlockHeight(), + ) + return &ResponseVerifyVoteExtension{Status: ResponseVerifyVoteExtension_ACCEPT}, nil } } diff --git a/abci/vote_extensions_test.go b/abci/vote_extensions_test.go index 9aa47aa..2833df9 100644 --- a/abci/vote_extensions_test.go +++ b/abci/vote_extensions_test.go @@ -1,6 +1,7 @@ package abci_test import ( + "cosmossdk.io/log" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/skip-mev/pob/abci" testutils "github.com/skip-mev/pob/testutils" @@ -123,7 +124,12 @@ func (suite *ABCITestSuite) TestExtendVoteExtensionHandler() { suite.Require().NoError(err) // Reset the handler with the new mempool - suite.voteExtensionHandler = abci.NewVoteExtensionHandler(suite.tobLane, suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder()) + suite.voteExtensionHandler = abci.NewVoteExtensionHandler( + log.NewTestLogger(suite.T()), + suite.tobLane, + suite.encodingConfig.TxConfig.TxDecoder(), + suite.encodingConfig.TxConfig.TxEncoder(), + ) handler := suite.voteExtensionHandler.ExtendVoteHandler() resp, err := handler(suite.ctx, nil) diff --git a/go.mod b/go.mod index d867134..b7ef99e 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( cosmossdk.io/core v0.6.1 cosmossdk.io/depinject v1.0.0-alpha.3 cosmossdk.io/errors v1.0.0 + cosmossdk.io/log v1.1.1-0.20230704160919-88f2c830b0ca cosmossdk.io/math v1.0.1 github.com/cometbft/cometbft v0.37.2 github.com/cometbft/cometbft-db v0.8.0 @@ -34,7 +35,6 @@ require ( cloud.google.com/go/compute/metadata v0.2.3 // indirect cloud.google.com/go/iam v1.1.0 // indirect cloud.google.com/go/storage v1.30.1 // indirect - cosmossdk.io/log v1.1.1-0.20230704160919-88f2c830b0ca // indirect cosmossdk.io/tools/rosetta v0.2.1 // indirect filippo.io/edwards25519 v1.0.0 // indirect github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect @@ -156,7 +156,7 @@ require ( github.com/tendermint/go-amino v0.16.0 // indirect github.com/tidwall/btree v1.6.0 // indirect github.com/ulikunitz/xz v0.5.11 // indirect - github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect + github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v1.2.0 // indirect github.com/zondax/hid v0.9.1 // indirect diff --git a/go.sum b/go.sum index a1c0918..fa38fa6 100644 --- a/go.sum +++ b/go.sum @@ -974,8 +974,9 @@ github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijb github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYppBueQtXaqoE= github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df/go.mod h1:JP3t17pCcGlemwknint6hfoeCVQrEMVwxRLRjXpq+BU= -github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= +github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo= +github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=