Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(abci): Validators can propose blocks that exceed the gas limit. #17159

Merged
merged 4 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (baseapp) [#17159](https://github.com/cosmos/cosmos-sdk/pull/17159) Validators can propose blocks that exceed the gas limit.
* (x/group) [#17146](https://github.com/cosmos/cosmos-sdk/pull/17146) Rename x/group legacy ORM package's error codespace from "orm" to "legacy_orm", preventing collisions with ORM's error codespace "orm".

### API Breaking Changes
Expand Down
41 changes: 41 additions & 0 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,47 @@ func TestABCI_PrepareProposal_BadEncoding(t *testing.T) {
require.Equal(t, 1, len(resPrepareProposal.Txs))
}

func TestABCI_PrepareProposal_OverGasUnderBytes(t *testing.T) {
pool := mempool.NewSenderNonceMempool()
suite := NewBaseAppSuite(t, baseapp.SetMempool(pool))
baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), NoopCounterServerImpl{})

// set max block gas limit to 99, this will allow 9 txs of 10 gas each.
_, err := suite.baseApp.InitChain(&abci.RequestInitChain{
ConsensusParams: &cmtproto.ConsensusParams{
Block: &cmtproto.BlockParams{MaxGas: 99},
},
})

require.NoError(t, err)
// insert 100 txs, each with a gas limit of 10
_, _, addr := testdata.KeyTestPubAddr()
for i := int64(0); i < 100; i++ {
msg := &baseapptestutil.MsgCounter{Counter: i, FailOnHandler: false, Signer: addr.String()}
msgs := []sdk.Msg{msg}

builder := suite.txConfig.NewTxBuilder()
err = builder.SetMsgs(msgs...)
require.NoError(t, err)
builder.SetMemo("counter=" + strconv.FormatInt(i, 10) + "&failOnAnte=false")
builder.SetGasLimit(10)
setTxSignature(t, builder, uint64(i))

err := pool.Insert(sdk.Context{}, builder.GetTx())
require.NoError(t, err)
}

// ensure we only select transactions that fit within the block gas limit
res, err := suite.baseApp.PrepareProposal(&abci.RequestPrepareProposal{
MaxTxBytes: 1_000_000, // large enough to ignore restriction
Height: 1,
})
require.NoError(t, err)

// Should include 9 transactions
require.Len(t, res.Txs, 9, "invalid number of transactions returned")
}

func TestABCI_PrepareProposal_MaxGas(t *testing.T) {
pool := mempool.NewSenderNonceMempool()
suite := NewBaseAppSuite(t, baseapp.SetMempool(pool))
Expand Down
10 changes: 6 additions & 4 deletions baseapp/abci_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,12 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand
if (txSize + totalTxBytes) < req.MaxTxBytes {
// If there is a max block gas limit, add the tx only if the limit has
// not been met.
if maxBlockGas > 0 && (txGasLimit+totalTxGas) <= uint64(maxBlockGas) {
totalTxGas += txGasLimit
totalTxBytes += txSize
selectedTxs = append(selectedTxs, bz)
if maxBlockGas > 0 {
if (txGasLimit + totalTxGas) <= uint64(maxBlockGas) {
totalTxGas += txGasLimit
totalTxBytes += txSize
selectedTxs = append(selectedTxs, bz)
}
} else {
totalTxBytes += txSize
selectedTxs = append(selectedTxs, bz)
Expand Down