Skip to content

test: Skip block reason checks for an unrecovered sender - #1632

Merged
chfast merged 1 commit into
masterfrom
test/blockchain-runner-cleanup
Aug 10, 2026
Merged

test: Skip block reason checks for an unrecovered sender#1632
chfast merged 1 commit into
masterfrom
test/blockchain-runner-cleanup

Conversation

@chfast

@chfast chfast commented Aug 10, 2026

Copy link
Copy Markdown
Member

Follow-up to #1623. The unrecovered-sender gap was patched only in the state-root check's names, but the blob-gas check runs before it, so a block whose bad signature the runner cannot see was judged against a rule it does not break. Skip the whole ladder instead, as already done for the unvalidated ommer headers.

No existing fixture reaches that path, so the regression test is a new one: a Cancun block carrying a blob transaction with a broken signature and a blobGasUsed the execution contradicts. Without the fix it reports

the block failed the check for BlockException.INCORRECT_BLOB_GAS_USED|BlockException.BLOB_GAS_USED_ABOVE_LIMIT,
expected TransactionException.INVALID_SIGNATURE_VRS

@chfast
chfast force-pushed the test/blockchain-runner-cleanup branch from e60331c to 7feee83 Compare August 10, 2026 17:14
@chfast chfast changed the title test: Collapse the blockchain header checks into one table test: Skip block reason checks for an unrecovered sender Aug 10, 2026
@chfast
chfast force-pushed the test/blockchain-runner-cleanup branch from 7feee83 to 04e520a Compare August 10, 2026 17:25
@chfast chfast changed the title test: Skip block reason checks for an unrecovered sender test: Fold the unrecovered-sender skip into one guard Aug 10, 2026
@chfast
chfast force-pushed the test/blockchain-runner-cleanup branch from 04e520a to dca6f4e Compare August 10, 2026 17:34
@chfast chfast changed the title test: Fold the unrecovered-sender skip into one guard test: Skip block reason checks for an unrecovered sender Aug 10, 2026
@chfast
chfast force-pushed the test/blockchain-runner-cleanup branch 2 times, most recently from 6c1c8d1 to 991d7e5 Compare August 10, 2026 17:47
@chfast
chfast requested a balanced review from Copilot August 10, 2026 17:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Extends #1623’s invalidity-reason checks to account for transaction senders that cannot be recovered.

Changes:

  • Skips header-reason comparisons for unrecovered senders.
  • Adds a Cancun blob-gas regression fixture and CTest entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
test/blockchaintest/blockchaintest_runner.cpp Adjusts invalidity-reason matching.
test/integration/blockchaintest/unrecovered_sender_blob_gas.json Adds regression fixture.
test/integration/blockchaintest/CMakeLists.txt Registers the regression test.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/integration/blockchaintest/unrecovered_sender_blob_gas.json Outdated
Comment thread test/blockchaintest/blockchaintest_runner.cpp
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.71%. Comparing base (22c6d37) to head (6f26198).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1632   +/-   ##
=======================================
  Coverage   97.70%   97.71%           
=======================================
  Files         171      171           
  Lines       15593    15593           
  Branches     3606     3607    +1     
=======================================
+ Hits        15235    15236    +1     
  Misses        269      269           
+ Partials       89       88    -1     
Flag Coverage Δ
eest-develop 88.61% <75.00%> (ø)
eest-develop-gmp 26.66% <0.00%> (ø)
eest-legacy 17.17% <12.50%> (-0.02%) ⬇️
eest-libsecp256k1 28.86% <0.00%> (ø)
eest-stable 88.61% <75.00%> (ø)
evmone-unittests 93.41% <0.00%> (+0.27%) ⬆️

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

Components Coverage Δ
core 96.08% <ø> (ø)
tooling 91.92% <100.00%> (+0.05%) ⬆️
tests 99.80% <ø> (ø)
Files with missing lines Coverage Δ
test/blockchaintest/blockchaintest_runner.cpp 83.51% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The gap was patched only in the state-root check's names, but the requests and
blob-gas checks run before it, so a block whose bad signature the runner cannot
see was judged against a rule it does not break. Skip those checks too, as for
the unvalidated ommer headers.
@chfast
chfast force-pushed the test/blockchain-runner-cleanup branch from 991d7e5 to 6f26198 Compare August 10, 2026 18:09
@chfast
chfast requested a balanced review from Copilot August 10, 2026 18:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@chfast
chfast merged commit 56808b6 into master Aug 10, 2026
24 checks passed
@chfast
chfast deleted the test/blockchain-runner-cleanup branch August 10, 2026 18:25
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