Skip to content

Commit

Permalink
fix: Gas limit check on PrepareProposal (#277)
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidterpay
davidterpay committed Dec 28, 2023
1 parent 79d930b commit 684faeb
Show file tree
Hide file tree
Showing 3 changed files with 310 additions and 5 deletions.
52 changes: 47 additions & 5 deletions abci/abci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,32 @@ func NewProposalHandler(
func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler {
return func(ctx sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal {
var (
selectedTxs [][]byte
totalTxBytes int64
selectedTxs [][]byte
totalTxBytes int64
totalGasLimit int64
)

bidTxIterator := h.mempool.AuctionBidSelect(ctx)
txsToRemove := make(map[sdk.Tx]struct{}, 0)
seenTxs := make(map[string]struct{}, 0)

maxGasLimit := ctx.ConsensusParams().Block.MaxGas

// Attempt to select the highest bid transaction that is valid and whose
// bundled transactions are valid.
selectBidTxLoop:
for ; bidTxIterator != nil; bidTxIterator = bidTxIterator.Next() {
cacheCtx, write := ctx.CacheContext()
tmpBidTx := bidTxIterator.Tx()

// Retrieve the gas limit of the transaction
feeTx, ok := tmpBidTx.(sdk.FeeTx)
if !ok {
txsToRemove[tmpBidTx] = struct{}{}
continue selectBidTxLoop
}
gasLimit := feeTx.GetGas()

bidTxBz, err := h.PrepareProposalVerifyTx(cacheCtx, tmpBidTx)
if err != nil {
txsToRemove[tmpBidTx] = struct{}{}
Expand Down Expand Up @@ -103,6 +114,15 @@ func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler {
continue selectBidTxLoop
}

feeTx, ok := refTx.(sdk.FeeTx)
if !ok {
// Malformed bundled transaction, so we remove the bid transaction
// and try the next top bid.
txsToRemove[tmpBidTx] = struct{}{}
continue selectBidTxLoop
}
gasLimit += feeTx.GetGas()

txBz, err := h.PrepareProposalVerifyTx(cacheCtx, refTx)
if err != nil {
// Invalid bundled transaction, so we remove the bid transaction
Expand All @@ -114,6 +134,15 @@ func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler {
sdkTxBytes[index] = txBz
}

if maxGasLimit > 0 && int64(gasLimit) > maxGasLimit {
// The gas limit of the bundled transactions exceeds the maximum gas
// limit of the block, so we remove the bid transaction and try the
// next top bid.
txsToRemove[tmpBidTx] = struct{}{}
continue selectBidTxLoop
}
totalGasLimit = int64(gasLimit)

// At this point, both the bid transaction itself and all the bundled
// transactions are valid. So we select the bid transaction along with
// all the bundled transactions. We also mark these transactions as seen and
Expand Down Expand Up @@ -177,13 +206,26 @@ func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler {
}

txSize := int64(len(txBz))
if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes {
selectedTxs = append(selectedTxs, txBz)
} else {
if totalTxBytes += txSize; totalTxBytes > req.MaxTxBytes {
// We've reached capacity per req.MaxTxBytes so we cannot select any
// more transactions.
break selectTxLoop
}

// Gas Limit check
feeTx, ok := memTx.(sdk.FeeTx)
if !ok {
txsToRemove[memTx] = struct{}{}
continue selectTxLoop
}
gasLimit := feeTx.GetGas()
if totalGasLimit += int64(gasLimit); maxGasLimit > 0 && totalGasLimit > maxGasLimit {
// We've reached capacity per maxGasLimit so we cannot select any more
// transactions.
break selectTxLoop
}

selectedTxs = append(selectedTxs, txBz)
}

// Remove all invalid transactions from the mempool.
Expand Down
162 changes: 162 additions & 0 deletions abci/abci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
"encoding/hex"
"fmt"
"math/rand"
"os"
"testing"
"time"

abcitypes "github.com/cometbft/cometbft/abci/types"
"github.com/cometbft/cometbft/libs/log"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -69,6 +71,13 @@ func (suite *ABCITestSuite) SetupTest() {
testCtx := testutil.DefaultContextWithDB(suite.T(), suite.key, storetypes.NewTransientStoreKey("transient_test"))
suite.ctx = testCtx.Ctx.WithBlockHeight(1)

cp := tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxGas: int64(1000000000000000000),
},
}
suite.ctx = suite.ctx.WithConsensusParams(&cp)

// Mempool set up
suite.config = mempool.NewDefaultAuctionFactory(suite.encodingConfig.TxConfig.TxDecoder())
suite.mempool = mempool.NewAuctionMempool(suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder(), 0, suite.config)
Expand Down Expand Up @@ -768,3 +777,156 @@ func (suite *ABCITestSuite) isTopBidValid() bool {
_, err := suite.anteHandler(suite.ctx, iterator.Tx(), true)
return err == nil
}

func (suite *ABCITestSuite) TestPrepareProposalLimits() {
suite.Run("rejects a single tx that is over the limit", func() {
suite.SetupTest() // reset
gasLimit := uint64(1000)
tx1, err := testutils.CreateTxWithLimit(suite.encodingConfig.TxConfig, suite.accounts[0], 0, 1000, testutils.CreateRandomMsgs(suite.accounts[0].Address, 1), gasLimit)
suite.Require().NoError(err)

mempool := mempool.NewAuctionMempool(suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder(), 0, suite.config)
err = mempool.Insert(suite.ctx, tx1)
suite.Require().NoError(err)

suite.proposalHandler = abci.NewProposalHandler(
mempool,
log.NewTMLogger(os.Stdout),
suite.anteHandler,
suite.encodingConfig.TxConfig.TxEncoder(),
suite.encodingConfig.TxConfig.TxDecoder(),
)

// Update the consensus params to have a lower gas limit
cp := tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxGas: int64(1000) - 1,
},
}
ctx := suite.ctx.WithConsensusParams(&cp)

res := suite.proposalHandler.PrepareProposalHandler()(ctx, abcitypes.RequestPrepareProposal{
MaxTxBytes: 1000000000000000000,
})
suite.Require().Len(res.Txs, 0)
})

suite.Run("accepts a single tx that is at the limit", func() {
suite.SetupTest() // reset
gasLimit := uint64(1000)
tx1, err := testutils.CreateTxWithLimit(suite.encodingConfig.TxConfig, suite.accounts[0], 0, 1000, testutils.CreateRandomMsgs(suite.accounts[0].Address, 1), gasLimit)
suite.Require().NoError(err)

mempool := mempool.NewAuctionMempool(suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder(), 0, suite.config)
err = mempool.Insert(suite.ctx, tx1)
suite.Require().NoError(err)

suite.proposalHandler = abci.NewProposalHandler(
mempool,
log.NewTMLogger(os.Stdout),
suite.anteHandler,
suite.encodingConfig.TxConfig.TxEncoder(),
suite.encodingConfig.TxConfig.TxDecoder(),
)

// Update the consensus params to have a lower gas limit
cp := tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxGas: int64(1000),
},
}
ctx := suite.ctx.WithConsensusParams(&cp)

req := abcitypes.RequestPrepareProposal{
MaxTxBytes: 1000000000000000000,
}
res := suite.proposalHandler.PrepareProposalHandler()(ctx, req)

suite.Require().Len(res.Txs, 1)

decodedTx, err := suite.encodingConfig.TxConfig.TxEncoder()(tx1)
suite.Require().NoError(err)
suite.Require().Equal(res.Txs[0], decodedTx)
})

suite.Run("rejects a bid that is over the limit", func() {
suite.SetupTest() // reset
gasLimit := uint64(1000)
tx1, err := testutils.CreateAuctionTxWithSignersAndLimit(
suite.encodingConfig.TxConfig,
suite.accounts[0],
sdk.NewCoin("foo", sdk.NewInt(1000000000000000000)),
0,
1000,
suite.accounts[0:10],
gasLimit,
)
suite.Require().NoError(err)

mempool := mempool.NewAuctionMempool(suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder(), 0, suite.config)
err = mempool.Insert(suite.ctx, tx1)
suite.Require().NoError(err)

suite.proposalHandler = abci.NewProposalHandler(
mempool,
log.NewTMLogger(os.Stdout),
suite.anteHandler,
suite.encodingConfig.TxConfig.TxEncoder(),
suite.encodingConfig.TxConfig.TxDecoder(),
)

// Update the consensus params to have a lower gas limit
cp := tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxGas: int64(1000) - 1,
},
}
ctx := suite.ctx.WithConsensusParams(&cp)

res := suite.proposalHandler.PrepareProposalHandler()(ctx, abcitypes.RequestPrepareProposal{
MaxTxBytes: 1000000000000000000,
})
suite.Require().Len(res.Txs, 0)
})

suite.Run("rejects a tx that is too large but accepts a smaller one", func() {
suite.SetupTest() // reset
gasLimit := uint64(1000)
tx1, err := testutils.CreateTxWithLimit(suite.encodingConfig.TxConfig, suite.accounts[0], 1, 1000, testutils.CreateRandomMsgs(suite.accounts[0].Address, 1), gasLimit)
suite.Require().NoError(err)

tx2, err := testutils.CreateTxWithLimit(suite.encodingConfig.TxConfig, suite.accounts[0], 0, 1000, testutils.CreateRandomMsgs(suite.accounts[0].Address, 1), gasLimit-1)
suite.Require().NoError(err)

mempool := mempool.NewAuctionMempool(suite.encodingConfig.TxConfig.TxDecoder(), suite.encodingConfig.TxConfig.TxEncoder(), 0, suite.config)
err = mempool.Insert(suite.ctx, tx1)
suite.Require().NoError(err)
err = mempool.Insert(suite.ctx, tx2)
suite.Require().NoError(err)

suite.proposalHandler = abci.NewProposalHandler(
mempool,
log.NewTMLogger(os.Stdout),
suite.anteHandler,
suite.encodingConfig.TxConfig.TxEncoder(),
suite.encodingConfig.TxConfig.TxDecoder(),
)

// Update the consensus params to have a lower gas limit
cp := tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxGas: int64(1000) - 1,
},
}
ctx := suite.ctx.WithConsensusParams(&cp)

res := suite.proposalHandler.PrepareProposalHandler()(ctx, abcitypes.RequestPrepareProposal{
MaxTxBytes: 1000000000000000000,
})
suite.Require().Len(res.Txs, 1)

decodedTx, err := suite.encodingConfig.TxConfig.TxEncoder()(tx2)
suite.Require().NoError(err)
suite.Require().Equal(res.Txs[0], decodedTx)
})
}
Loading

0 comments on commit 684faeb

Please sign in to comment.