Skip to content

test: Set blob gas budget from the protocol max#1555

Merged
chfast merged 1 commit into
masterfrom
test/blob-budget-from-max
May 29, 2026
Merged

test: Set blob gas budget from the protocol max#1555
chfast merged 1 commit into
masterfrom
test/blob-budget-from-max

Conversation

@chfast
Copy link
Copy Markdown
Member

@chfast chfast commented May 29, 2026

So far we were pre-validating the block header blob_gas_used against the protocol max. This was correct, but hid expected per-tx blob gas limit exceeded errors.

Changed behavior: start with the protocol max and validate each tx. Compare the block header value at the end. The block validity outcome is unchanged; only the reported error differs (a per-tx blob gas error instead of a block-level one). Matches EELS and other implementations.

So far we were pre-validating the block header blob_gas_used against the
protocol max. This was correct, but hid expected per-tx blob gas limit
exceeded errors.

Changed behavior: start with the protocol max and validate each tx.
Compare the block header value at the end. The block validity outcome is
unchanged; only the reported error differs (a per-tx blob gas error
instead of a block-level one). Matches EELS and other implementations.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the blockchain test runner’s blob-gas accounting to start from the protocol per-block maximum blob gas budget and decrement per transaction, then compare the resulting blob gas usage against the block header at the end. This keeps block validity outcomes the same while ensuring blob gas limit violations surface as per-transaction errors (matching other clients/spec behavior).

Changes:

  • Initialize apply_block()’s blob gas budget from state::max_blob_gas_per_block(blob_params) instead of preloading from block.blob_gas_used.
  • Remove block-level prevalidation that rejected blocks solely because header.blob_gas_used > protocol_max, so per-tx validation can report the expected error.
  • Update test assertions/filters to compare (blob_gas_limit - blob_gas_left) against the header’s blob_gas_used.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.09%. Comparing base (0cd0dec) to head (ee8a508).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1555   +/-   ##
=======================================
  Coverage   97.09%   97.09%           
=======================================
  Files         162      162           
  Lines       14438    14440    +2     
  Branches     3371     3370    -1     
=======================================
+ Hits        14019    14021    +2     
  Misses        303      303           
  Partials      116      116           
Flag Coverage Δ
eest-develop 91.93% <100.00%> (+<0.01%) ⬆️
eest-develop-gmp 26.52% <0.00%> (-0.01%) ⬇️
eest-legacy 17.52% <88.88%> (+0.02%) ⬆️
eest-libsecp256k1 28.16% <0.00%> (-0.01%) ⬇️
eest-stable 91.87% <100.00%> (+<0.01%) ⬆️
evmone-unittests 92.78% <66.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 96.01% <ø> (ø)
tooling 87.64% <100.00%> (+0.01%) ⬆️
tests 99.79% <ø> (ø)
Files with missing lines Coverage Δ
test/blockchaintest/blockchaintest_runner.cpp 85.71% <100.00%> (+0.11%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chfast chfast merged commit 1c0a9b9 into master May 29, 2026
24 checks passed
@chfast chfast deleted the test/blob-budget-from-max branch May 29, 2026 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants