feat(BB): Decoding Txs before ProcessLane + IgnoreList (#170)

This commit is contained in:
David Terpay 2023-06-08 16:23:20 -04:00 committed by GitHub
parent 38f2ae20f6
commit 3d505edf31
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 113 additions and 82 deletions

View File

@ -11,6 +11,7 @@ import (
"github.com/skip-mev/pob/blockbuster"
"github.com/skip-mev/pob/blockbuster/abci"
"github.com/skip-mev/pob/blockbuster/lanes/auction"
"github.com/skip-mev/pob/blockbuster/utils"
)
const (
@ -39,8 +40,8 @@ type (
VerifyTx(ctx sdk.Context, tx sdk.Tx) error
// ProcessLaneBasic is utilized to verify the rest of the proposal according to
// the preferences of the top of block lane. This is used to verify that no
ProcessLaneBasic(txs [][]byte) error
// the preferences of the top of block lane.
ProcessLaneBasic(txs []sdk.Tx) error
}
// ProposalHandler contains the functionality and handlers required to\
@ -127,15 +128,22 @@ func (h *ProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler {
return cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}
}
// Decode the transactions in the proposal.
decodedTxs, err := utils.GetDecodedTxs(h.txDecoder, proposal[NumInjectedTxs:])
if err != nil {
h.logger.Error("failed to decode transactions", "err", err)
return cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}
}
// Do a basic check of the rest of the proposal to make sure no auction transactions
// are included.
if err := h.tobLane.ProcessLaneBasic(proposal[NumInjectedTxs:]); err != nil {
if err := h.tobLane.ProcessLaneBasic(decodedTxs); err != nil {
h.logger.Error("failed to process proposal", "err", err)
return cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}
}
// Verify that the rest of the proposal is valid according to each lane's verification logic.
if _, err = h.processLanesHandler(ctx, proposal[auctionInfo.NumTxs:]); err != nil {
if _, err = h.processLanesHandler(ctx, decodedTxs[auctionInfo.NumTxs:]); err != nil {
h.logger.Error("failed to process proposal", "err", err)
return cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}
}

View File

@ -14,15 +14,17 @@ type (
// handlers.
ProposalHandler struct {
logger log.Logger
txDecoder sdk.TxDecoder
prepareLanesHandler blockbuster.PrepareLanesHandler
processLanesHandler blockbuster.ProcessLanesHandler
}
)
// NewProposalHandler returns a new abci++ proposal handler.
func NewProposalHandler(logger log.Logger, mempool blockbuster.Mempool) *ProposalHandler {
func NewProposalHandler(logger log.Logger, txDecoder sdk.TxDecoder, mempool blockbuster.Mempool) *ProposalHandler {
return &ProposalHandler{
logger: logger,
txDecoder: txDecoder,
prepareLanesHandler: ChainPrepareLanes(mempool.Registry()...),
processLanesHandler: ChainProcessLanes(mempool.Registry()...),
}
@ -67,8 +69,15 @@ func (h *ProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler {
}
}()
// Decode the transactions from the proposal.
decodedTxs, err := utils.GetDecodedTxs(h.txDecoder, req.Txs)
if err != nil {
h.logger.Error("failed to decode transactions", "err", err)
return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}
}
// Verify the proposal using the verification logic from each lane.
if _, err := h.processLanesHandler(ctx, req.Txs); err != nil {
if _, err := h.processLanesHandler(ctx, decodedTxs); err != nil {
h.logger.Error("failed to validate the proposal", "err", err)
return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}
}
@ -163,7 +172,7 @@ func ChainProcessLanes(chain ...blockbuster.Lane) blockbuster.ProcessLanesHandle
chain = append(chain, terminator.Terminator{})
}
return func(ctx sdk.Context, proposalTxs [][]byte) (sdk.Context, error) {
return func(ctx sdk.Context, proposalTxs []sdk.Tx) (sdk.Context, error) {
// Short circuit if there are no transactions to process.
if len(proposalTxs) == 0 {
return ctx, nil

View File

@ -143,7 +143,7 @@ func (suite *ABCITestSuite) SetupTest() {
suite.builderDecorator = ante.NewBuilderDecorator(suite.builderKeeper, suite.encodingConfig.TxConfig.TxEncoder(), suite.tobLane, suite.mempool)
// Proposal handler set up
suite.proposalHandler = abci.NewProposalHandler(log.NewNopLogger(), suite.mempool)
suite.proposalHandler = abci.NewProposalHandler(log.NewNopLogger(), suite.encodingConfig.TxConfig.TxDecoder(), suite.mempool)
}
func (suite *ABCITestSuite) anteHandler(ctx sdk.Context, tx sdk.Tx, _ bool) (sdk.Context, error) {
@ -693,7 +693,7 @@ func (suite *ABCITestSuite) TestPrepareProposal() {
}
// Create a new proposal handler
suite.proposalHandler = abci.NewProposalHandler(suite.logger, suite.mempool)
suite.proposalHandler = abci.NewProposalHandler(suite.logger, suite.encodingConfig.TxConfig.TxDecoder(), suite.mempool)
handler := suite.proposalHandler.PrepareProposalHandler()
res := handler(suite.ctx, abcitypes.RequestPrepareProposal{
MaxTxBytes: maxTxBytes,

View File

@ -34,7 +34,7 @@ type (
// ProcessLanesHandler wraps all of the lanes Process functions into a single chained
// function. You can think of it like an AnteHandler, but for processing proposals in the
// context of lanes instead of modules.
ProcessLanesHandler func(ctx sdk.Context, proposalTxs [][]byte) (sdk.Context, error)
ProcessLanesHandler func(ctx sdk.Context, txs []sdk.Tx) (sdk.Context, error)
// BaseLaneConfig defines the basic functionality needed for a lane.
BaseLaneConfig struct {
@ -48,6 +48,14 @@ type (
// on the number of transactions that can be included in the block for this
// lane (up to maxTxBytes as provided by the request). This is useful for the default lane.
MaxBlockSpace sdk.Dec
// IgnoreList defines the list of lanes to ignore when processing transactions. This
// is useful for when you want lanes to exist after the default lane. For example,
// say there are two lanes: default and free. The free lane should be processed after
// the default lane. In this case, the free lane should be added to the ignore list
// of the default lane. Otherwise, the transactions that belong to the free lane
// will be processed by the default lane.
IgnoreList []Lane
}
// Lane defines an interface used for block construction
@ -74,12 +82,12 @@ type (
// ProcessLaneBasic validates that transactions belonging to this lane are not misplaced
// in the block proposal.
ProcessLaneBasic(txs [][]byte) error
ProcessLaneBasic(txs []sdk.Tx) error
// ProcessLane verifies this lane's portion of a proposed block. It inputs the transactions
// that may belong to this lane and a function to call the next lane in the chain. The next
// lane in the chain will be called with the updated context and filtered down transactions.
ProcessLane(ctx sdk.Context, proposalTxs [][]byte, next ProcessLanesHandler) (sdk.Context, error)
ProcessLane(ctx sdk.Context, proposalTxs []sdk.Tx, next ProcessLanesHandler) (sdk.Context, error)
// SetAnteHandler sets the lane's antehandler.
SetAnteHandler(antehander sdk.AnteHandler)

View File

@ -123,26 +123,23 @@ selectBidTxLoop:
// ProcessLane will ensure that block proposals that include transactions from
// the top-of-block auction lane are valid.
func (l *TOBLane) ProcessLane(ctx sdk.Context, proposalTxs [][]byte, next blockbuster.ProcessLanesHandler) (sdk.Context, error) {
tx, err := l.Cfg.TxDecoder(proposalTxs[0])
if err != nil {
return ctx, fmt.Errorf("failed to decode tx in lane %s: %w", l.Name(), err)
func (l *TOBLane) ProcessLane(ctx sdk.Context, txs []sdk.Tx, next blockbuster.ProcessLanesHandler) (sdk.Context, error) {
bidTx := txs[0]
if !l.Match(bidTx) {
return next(ctx, txs)
}
if !l.Match(tx) {
return next(ctx, proposalTxs)
}
bidInfo, err := l.GetAuctionBidInfo(tx)
bidInfo, err := l.GetAuctionBidInfo(bidTx)
if err != nil {
return ctx, fmt.Errorf("failed to get bid info for lane %s: %w", l.Name(), err)
}
if err := l.VerifyTx(ctx, tx); err != nil {
if err := l.VerifyTx(ctx, bidTx); err != nil {
return ctx, fmt.Errorf("invalid bid tx: %w", err)
}
return next(ctx, proposalTxs[len(bidInfo.Transactions)+1:])
return next(ctx, txs[len(bidInfo.Transactions)+1:])
}
// ProcessLaneBasic ensures that if a bid transaction is present in a proposal,
@ -150,20 +147,12 @@ func (l *TOBLane) ProcessLane(ctx sdk.Context, proposalTxs [][]byte, next blockb
// - all of the bundled transactions are included after the bid transaction in the order
// they were included in the bid transaction.
// - there are no other bid transactions in the proposal
func (l *TOBLane) ProcessLaneBasic(txs [][]byte) error {
tx, err := l.Cfg.TxDecoder(txs[0])
if err != nil {
return fmt.Errorf("failed to decode tx in lane %s: %w", l.Name(), err)
}
func (l *TOBLane) ProcessLaneBasic(txs []sdk.Tx) error {
bidTx := txs[0]
// If there is a bid transaction, it must be the first transaction in the block proposal.
if !l.Match(tx) {
for _, txBz := range txs[1:] {
tx, err := l.Cfg.TxDecoder(txBz)
if err != nil {
return fmt.Errorf("failed to decode tx in lane %s: %w", l.Name(), err)
}
if !l.Match(bidTx) {
for _, tx := range txs[1:] {
if l.Match(tx) {
return fmt.Errorf("misplaced bid transactions in lane %s", l.Name())
}
@ -172,7 +161,7 @@ func (l *TOBLane) ProcessLaneBasic(txs [][]byte) error {
return nil
}
bidInfo, err := l.GetAuctionBidInfo(tx)
bidInfo, err := l.GetAuctionBidInfo(bidTx)
if err != nil {
return fmt.Errorf("failed to get bid info for lane %s: %w", l.Name(), err)
}
@ -182,17 +171,12 @@ func (l *TOBLane) ProcessLaneBasic(txs [][]byte) error {
}
// Ensure that the order of transactions in the bundle is preserved.
for i, bundleTxBz := range txs[1 : len(bidInfo.Transactions)+1] {
tx, err := l.WrapBundleTransaction(bundleTxBz)
if err != nil {
return fmt.Errorf("failed to decode bundled tx in lane %s: %w", l.Name(), err)
}
if l.Match(tx) {
for i, bundleTx := range txs[1 : len(bidInfo.Transactions)+1] {
if l.Match(bundleTx) {
return fmt.Errorf("multiple bid transactions in lane %s", l.Name())
}
txBz, err := l.Cfg.TxEncoder(tx)
txBz, err := l.Cfg.TxEncoder(bundleTx)
if err != nil {
return fmt.Errorf("failed to encode bundled tx in lane %s: %w", l.Name(), err)
}
@ -203,12 +187,7 @@ func (l *TOBLane) ProcessLaneBasic(txs [][]byte) error {
}
// Ensure that there are no more bid transactions in the block proposal.
for _, txBz := range txs[len(bidInfo.Transactions)+1:] {
tx, err := l.Cfg.TxDecoder(txBz)
if err != nil {
return fmt.Errorf("failed to decode tx in lane %s: %w", l.Name(), err)
}
for _, tx := range txs[len(bidInfo.Transactions)+1:] {
if l.Match(tx) {
return fmt.Errorf("multiple bid transactions in lane %s", l.Name())
}

View File

@ -66,19 +66,14 @@ func (l *DefaultLane) PrepareLane(
}
// ProcessLane verifies the default lane's portion of a block proposal.
func (l *DefaultLane) ProcessLane(ctx sdk.Context, proposalTxs [][]byte, next blockbuster.ProcessLanesHandler) (sdk.Context, error) {
for index, tx := range proposalTxs {
tx, err := l.Cfg.TxDecoder(tx)
if err != nil {
return ctx, fmt.Errorf("failed to decode tx: %w", err)
}
func (l *DefaultLane) ProcessLane(ctx sdk.Context, txs []sdk.Tx, next blockbuster.ProcessLanesHandler) (sdk.Context, error) {
for index, tx := range txs {
if l.Match(tx) {
if err := l.VerifyTx(ctx, tx); err != nil {
return ctx, fmt.Errorf("failed to verify tx: %w", err)
}
} else {
return next(ctx, proposalTxs[index:])
return next(ctx, txs[index:])
}
}
@ -88,16 +83,11 @@ func (l *DefaultLane) ProcessLane(ctx sdk.Context, proposalTxs [][]byte, next bl
// ProcessLaneBasic does basic validation on the block proposal to ensure that
// transactions that belong to this lane are not misplaced in the block proposal.
func (l *DefaultLane) ProcessLaneBasic(txs [][]byte) error {
func (l *DefaultLane) ProcessLaneBasic(txs []sdk.Tx) error {
seenOtherLaneTx := false
lastSeenIndex := 0
for _, txBz := range txs {
tx, err := l.Cfg.TxDecoder(txBz)
if err != nil {
return fmt.Errorf("failed to decode tx in lane %s: %w", l.Name(), err)
}
for _, tx := range txs {
if l.Match(tx) {
if seenOtherLaneTx {
return fmt.Errorf("the %s lane contains a transaction that belongs to another lane", l.Name())

View File

@ -36,9 +36,15 @@ func NewDefaultLane(cfg blockbuster.BaseLaneConfig) *DefaultLane {
}
// Match returns true if the transaction belongs to this lane. Since
// this is the default lane, it always returns true. This means that
// any transaction can be included in this lane.
func (l *DefaultLane) Match(sdk.Tx) bool {
// this is the default lane, it always returns true except for transactions
// that belong to lanes in the ignore list.
func (l *DefaultLane) Match(tx sdk.Tx) bool {
for _, lane := range l.Cfg.IgnoreList {
if lane.Match(tx) {
return false
}
}
return true
}
@ -61,3 +67,8 @@ func (l *DefaultLane) SetAnteHandler(anteHandler sdk.AnteHandler) {
func (l *DefaultLane) GetMaxBlockSpace() sdk.Dec {
return l.Cfg.MaxBlockSpace
}
// GetIgnoreList returns the lane's ignore list.
func (l *DefaultLane) GetIgnoreList() []blockbuster.Lane {
return l.Cfg.IgnoreList
}

View File

@ -40,7 +40,7 @@ func (t Terminator) PrepareLane(_ sdk.Context, proposal *blockbuster.Proposal, _
}
// ProcessLane is a no-op
func (t Terminator) ProcessLane(ctx sdk.Context, _ [][]byte, _ blockbuster.ProcessLanesHandler) (sdk.Context, error) {
func (t Terminator) ProcessLane(ctx sdk.Context, _ []sdk.Tx, _ blockbuster.ProcessLanesHandler) (sdk.Context, error) {
return ctx, nil
}
@ -85,7 +85,7 @@ func (t Terminator) Select(context.Context, [][]byte) sdkmempool.Iterator {
}
// ValidateLaneBasic is a no-op
func (t Terminator) ProcessLaneBasic([][]byte) error {
func (t Terminator) ProcessLaneBasic([]sdk.Tx) error {
return nil
}

View File

@ -24,6 +24,21 @@ func GetTxHashStr(txEncoder sdk.TxEncoder, tx sdk.Tx) ([]byte, string, error) {
return txBz, txHashStr, nil
}
// GetDecodedTxs returns the decoded transactions from the given bytes.
func GetDecodedTxs(txDecoder sdk.TxDecoder, txs [][]byte) ([]sdk.Tx, error) {
var decodedTxs []sdk.Tx
for _, txBz := range txs {
tx, err := txDecoder(txBz)
if err != nil {
return nil, fmt.Errorf("failed to decode transaction: %w", err)
}
decodedTxs = append(decodedTxs, tx)
}
return decodedTxs, nil
}
// RemoveTxsFromLane removes the transactions from the given lane's mempool.
func RemoveTxsFromLane(txs map[sdk.Tx]struct{}, mempool sdkmempool.Mempool) error {
for tx := range txs {

View File

@ -295,7 +295,17 @@ func New(
)
// Default lane accepts all other transactions.
defaultLane := base.NewDefaultLane(config)
defaultConfig := blockbuster.BaseLaneConfig{
Logger: app.Logger(),
TxEncoder: app.txConfig.TxEncoder(),
TxDecoder: app.txConfig.TxDecoder(),
MaxBlockSpace: sdk.ZeroDec(),
IgnoreList: []blockbuster.Lane{
tobLane,
freeLane,
},
}
defaultLane := base.NewDefaultLane(defaultConfig)
lanes := []blockbuster.Lane{
tobLane,
freeLane,
@ -333,6 +343,7 @@ func New(
// Set the proposal handlers on the BaseApp along with the custom antehandler.
proposalHandlers := abci.NewProposalHandler(
app.Logger(),
app.txConfig.TxDecoder(),
mempool,
)
app.App.SetPrepareProposal(proposalHandlers.PrepareProposalHandler())

View File

@ -364,9 +364,6 @@ func (s *IntegrationTestSuite) TestMultipleBids() {
bundle2[i] = s.createMsgSendTx(accounts[1], accounts[0].Address.String(), defaultSendAmount, uint64(i), 1000)
}
// Wait for a block to ensure all transactions are included in the same block
s.waitForABlock()
// Create a bid transaction that includes the bundle and is valid
bid := reserveFee
height := s.queryCurrentHeight()
@ -376,6 +373,9 @@ func (s *IntegrationTestSuite) TestMultipleBids() {
bid2 := reserveFee.Add(sdk.NewCoin(app.BondDenom, sdk.NewInt(10)))
bidTx2 := s.createAuctionBidTx(accounts[3], bid2, bundle2, 0, height+5)
// Wait for a block to ensure all transactions are included in the same block
s.waitForABlock()
// Broadcast the transactions to different validators
s.broadcastTx(bidTx, 0)
s.broadcastTx(bidTx2, 1)
@ -1066,14 +1066,6 @@ func (s *IntegrationTestSuite) TestFreeLane() {
s.waitForABlock()
height := s.queryCurrentHeight()
// Ensure that the transaction was executed
balanceAfterFreeTx := s.queryBalanceOf(accounts[0].Address.String(), app.BondDenom)
s.Require().True(balanceAfterFreeTx.Add(defaultStakeAmount).IsGTE(balanceBeforeFreeTx))
// The balance must be strictly less than to account for fees
balanceAfterNormalTx := s.queryBalanceOf(accounts[1].Address.String(), app.BondDenom)
s.Require().True(balanceAfterNormalTx.IsLT((balanceBeforeNormalTx.Sub(defaultSendAmount))))
hashes := s.normalTxsToTxHashes([][]byte{freeTx, normalTx})
expectedExecution := map[string]bool{
hashes[0]: true,
@ -1082,6 +1074,14 @@ func (s *IntegrationTestSuite) TestFreeLane() {
// Ensure that the block was built correctly
s.verifyBlock(height, hashes, expectedExecution)
// Ensure that the transaction was executed
balanceAfterFreeTx := s.queryBalanceOf(accounts[0].Address.String(), app.BondDenom)
s.Require().True(balanceAfterFreeTx.Add(defaultStakeAmount).IsGTE(balanceBeforeFreeTx))
// The balance must be strictly less than to account for fees
balanceAfterNormalTx := s.queryBalanceOf(accounts[1].Address.String(), app.BondDenom)
s.Require().True(balanceAfterNormalTx.IsLT((balanceBeforeNormalTx.Sub(defaultSendAmount))))
},
},
{