Skip to content

vm: externalize header field validation#4262

Open
ScottyPoi wants to merge 4 commits intomasterfrom
header-fields
Open

vm: externalize header field validation#4262
ScottyPoi wants to merge 4 commits intomasterfrom
header-fields

Conversation

@ScottyPoi
Copy link
Contributor

This PR externalizes the header field validation conditionals from vm.runBlock into a new fuction: validateHeaderFields exported from validateHeaderFields.ts.

Also modifies the error reporting to include all mismatches. Previously runBlock would fail at the first header field mismatch. Often this meant a invalid receiptsRoot error thrown, when in reality, multiple header fields were probably mismatched.

The new function will check all of the header fields, and report the full list of mismatched fields.

Copilot AI review requested due to automatic review settings March 12, 2026 19:45
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 41.17647% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.31%. Comparing base (186f20d) to head (c07704a).

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 87.33% <ø> (ø)
blockchain 88.85% <ø> (ø)
common 93.44% <ø> (ø)
evm 61.28% <ø> (ø)
mpt 89.99% <ø> (+0.33%) ⬆️
statemanager 78.10% <ø> (ø)
static 91.35% <ø> (ø)
tx 88.01% <ø> (ø)
util 80.81% <ø> (ø)
vm 55.86% <41.17%> (+0.25%) ⬆️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

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 refactors block header field validation out of vm.runBlock into a new validateHeaderFields helper, with the intended behavior change of collecting and reporting all header mismatches instead of failing fast on the first mismatch.

Changes:

  • Added validateHeaderFields.ts to perform header validation and aggregate mismatches into a single error.
  • Updated runBlock to call validateHeaderFields during non-generated-field execution to centralize validation logic.

Reviewed changes

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

File Description
packages/vm/src/validateHeaderFields.ts Introduces the new header validation helper and aggregated error reporting.
packages/vm/src/runBlock.ts Replaces inline header validation with a call to validateHeaderFields.

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

@github-actions
Copy link

github-actions bot commented Mar 12, 2026

📦 Bundle Size Analysis

Package Size (min+gzip) Δ
binarytree 17.5 KB ⚪ ±0%
block 43.7 KB ⚪ ±0%
blockchain 69.8 KB ⚪ ±0%
common 25.4 KB ⚪ ±0%
devp2p 17.7 KB ⚪ ±0%
e2store 87.2 KB ⚪ ±0%
ethash 61.6 KB ⚪ ±0%
evm 62.4 KB ⚪ ±0%
genesis 272.2 KB ⚪ ±0%
mpt 21.9 KB ⚪ ±0%
rlp 1.7 KB ⚪ ±0%
statemanager 41.3 KB ⚪ ±0%
testdata 43.8 KB ⚪ ±0%
tx 20.8 KB ⚪ ±0%
util 13.1 KB ⚪ ±0%
vm 154.7 KB 🔴 +0.1 KB (+0.06%)
wallet 15.0 KB ⚪ ±0%

Values are minified+gzipped bundles of each package entry. Workspace deps are bundled; external deps are excluded.

Generated by bundle-size workflow

@holgerd77
Copy link
Member

Can you give a quick reasoning why you are doing this?

@ScottyPoi
Copy link
Contributor Author

Can you give a quick reasoning why you are doing this?

2 main reasons:

  1. pulling header-field validation out of vm.runBlock keeps the block execution path smaller and easier to follow
  2. reporting all header mismatches at once instead of stopping at the first one makes bad blocks and failing fixtures faster to diagnose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants