Skip to content

Commit

Permalink
chore(auction): Moving bid extraction to from msg handler to ante han…
Browse files Browse the repository at this point in the history
…dler (#135)

* moving bid extraction to antehandler

* lint

* fix

* no zero timeout

* reset integration tests
  • Loading branch information
davidterpay committed Oct 4, 2023
1 parent d495b38 commit 3374203
Show file tree
Hide file tree
Showing 18 changed files with 535 additions and 936 deletions.
10 changes: 8 additions & 2 deletions lanes/mev/check_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ func (handler *CheckTxHandler) CheckTx() CheckTx {
handler.baseApp.Logger().Info(
"invalid bid tx",
"err", err,
"tx", req.Tx,
"height", ctx.BlockHeight(),
"bid_height", bidInfo.Timeout,
"bidder", bidInfo.Bidder,
"bid", bidInfo.Bid,
"removing tx from mempool", true,
)

Expand All @@ -187,7 +190,10 @@ func (handler *CheckTxHandler) CheckTx() CheckTx {

handler.baseApp.Logger().Info(
"valid bid tx",
"tx", req.Tx,
"height", ctx.BlockHeight(),
"bid_height", bidInfo.Timeout,
"bidder", bidInfo.Bidder,
"bid", bidInfo.Bid,
"inserting tx into mempool", true,
)

Expand Down
3 changes: 1 addition & 2 deletions tests/app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

type BSDKHandlerOptions struct {
BaseOptions ante.HandlerOptions
Mempool block.Mempool
MEVLane auctionante.MEVLane
TxDecoder sdk.TxDecoder
TxEncoder sdk.TxEncoder
Expand Down Expand Up @@ -55,7 +54,7 @@ func NewBSDKAnteHandler(options BSDKHandlerOptions) sdk.AnteHandler {
ante.NewSigGasConsumeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SigGasConsumer),
ante.NewSigVerificationDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SignModeHandler),
ante.NewIncrementSequenceDecorator(options.BaseOptions.AccountKeeper),
auctionante.NewAuctionDecorator(options.auctionkeeper, options.TxEncoder, options.MEVLane, options.Mempool),
auctionante.NewAuctionDecorator(options.auctionkeeper, options.TxEncoder, options.MEVLane),
}

return sdk.ChainAnteDecorators(anteDecorators...)
Expand Down
1 change: 0 additions & 1 deletion tests/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ func New(
TxEncoder: app.txConfig.TxEncoder(),
FreeLane: freeLane,
MEVLane: mevLane,
Mempool: mempool,
}
anteHandler := NewBSDKAnteHandler(options)

Expand Down
145 changes: 0 additions & 145 deletions testutils/mocks.go

This file was deleted.

67 changes: 9 additions & 58 deletions x/auction/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,28 @@ package ante

import (
"bytes"
"context"
"fmt"

"cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/skip-mev/block-sdk/x/auction/keeper"
"github.com/skip-mev/block-sdk/x/auction/types"
)

var _ sdk.AnteDecorator = AuctionDecorator{}

type (
// MEVLane is an interface that defines the methods required to interact with the MEV
// lane.
MEVLane interface {
GetAuctionBidInfo(tx sdk.Tx) (*types.BidInfo, error)
GetTopAuctionTx(ctx context.Context) sdk.Tx
}

// Mempool is an interface that defines the methods required to interact with the application-side mempool.
Mempool interface {
Contains(tx sdk.Tx) bool
}

// AuctionDecorator is an AnteDecorator that validates the auction bid and bundled transactions.
AuctionDecorator struct {
auctionkeeper keeper.Keeper
txEncoder sdk.TxEncoder
lane MEVLane
mempool Mempool
auctionkeeper AuctionKeeper
txEncoder sdk.TxEncoder
}
)

func NewAuctionDecorator(ak keeper.Keeper, txEncoder sdk.TxEncoder, lane MEVLane, mempool Mempool) AuctionDecorator {
// NewAuctionDecorator returns a new AuctionDecorator.
func NewAuctionDecorator(ak AuctionKeeper, txEncoder sdk.TxEncoder, lane MEVLane) AuctionDecorator {
return AuctionDecorator{
auctionkeeper: ak,
txEncoder: txEncoder,
lane: lane,
mempool: mempool,
}
}

Expand All @@ -55,25 +37,14 @@ func (ad AuctionDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool,

// Validate the auction bid if one exists.
if bidInfo != nil {
// If comet is re-checking a transaction, we only need to check if the transaction is in the application-side mempool.
if ctx.IsReCheckTx() {
if !ad.mempool.Contains(tx) {
return ctx, fmt.Errorf("transaction not found in application-side mempool")
}
}

// Auction transactions must have a timeout set to a valid block height.
if err := ad.ValidateTimeout(ctx, int64(bidInfo.Timeout)); err != nil {
if err := ValidateTimeout(ctx, int64(bidInfo.Timeout)); err != nil {
return ctx, err
}

// We only need to verify the auction bid relative to the local validator's mempool if the mode
// is checkTx or recheckTx. Otherwise, the ABCI handlers (VerifyVoteExtension, ExtendVoteExtension, etc.)
// will always compare the auction bid to the highest bidding transaction in the mempool leading to
// poor liveness guarantees.
// TODO(nikhil/david): refactor this logic (is this necessary?)
// Only compare the bid to the top bid if necessary.
topBid := sdk.Coin{}
if ctx.IsCheckTx() || ctx.IsReCheckTx() {
if _, ok := nextHeightExecModes[ctx.ExecMode()]; ok {
if topBidTx := ad.lane.GetTopAuctionTx(ctx); topBidTx != nil {
topBidBz, err := ad.txEncoder(topBidTx)
if err != nil {
Expand All @@ -86,6 +57,8 @@ func (ad AuctionDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool,
}

// Compare the bytes to see if the current transaction is the highest bidding transaction.
// If it is the same transaction, we do not need to compare the bids as the bid check will
// fail.
if !bytes.Equal(topBidBz, currentTxBz) {
topBidInfo, err := ad.lane.GetAuctionBidInfo(topBidTx)
if err != nil {
Expand All @@ -104,25 +77,3 @@ func (ad AuctionDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool,

return next(ctx, tx, simulate)
}

// ValidateTimeout validates that the timeout is greater than or equal to the expected block height
// the bid transaction will be executed in.
func (ad AuctionDecorator) ValidateTimeout(ctx sdk.Context, timeout int64) error {
currentBlockHeight := ctx.BlockHeight()

// If the mode is CheckTx or ReCheckTx, we increment the current block height by one to
// account for the fact that the transaction will be executed in the next block.
if ctx.IsCheckTx() || ctx.IsReCheckTx() {
currentBlockHeight++
}

if timeout < currentBlockHeight {
return fmt.Errorf(
"timeout height cannot be less than the current block height (timeout: %d, current block height: %d)",
timeout,
currentBlockHeight,
)
}

return nil
}
Loading

0 comments on commit 3374203

Please sign in to comment.