Skip to content

Remove EOF#10866

Merged
benaadams merged 6 commits intomasterfrom
remove-eof
Mar 19, 2026
Merged

Remove EOF#10866
benaadams merged 6 commits intomasterfrom
remove-eof

Conversation

@benaadams
Copy link
Copy Markdown
Member

@benaadams benaadams commented Mar 19, 2026

Changes

  • Remove EOF and associated stuffs

Types of changes

What types of changes does your code introduce?

  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring

Testing

Requires testing

  • No - not beyond existing

Copilot AI review requested due to automatic review settings March 19, 2026 08:53

This comment was marked as outdated.

Sign in to view

@benaadams benaadams requested a review from Copilot March 19, 2026 09:19

This comment was marked as outdated.

Sign in to view

@benaadams
fix: RETURNDATACOPY gas policy and benchmark CLZ coverage
3b37dd3 
- Use ConsumeDataCopyGas for RETURNDATACOPY, consistent with other COPY opcodes
- Add CLZ opcode to benchmark StackRequirements
@benaadams
Copy link
Copy Markdown
Member Author

benaadams commented Mar 19, 2026

@claude review this

@benaadams
Copy link
Copy Markdown
Member Author

benaadams commented Mar 19, 2026

@codex review this

@claude
Copy link
Copy Markdown

claude bot commented Mar 19, 2026 •
edited
Loading

Claude finished @benaadams's task in 3m 10s —— View job

PR Review: Remove EOF

Tasks

  • Read AGENTS.md / CLAUDE.md
  • Read key changed files
  • Verify Copilot-flagged issues
  • Check for additional issues
  • Post final review

Summary

Clean, comprehensive removal of EOF (EVM Object Format) support. The PR deletes ~4,780 lines of EOF-specific code across 87 files with minimal additions — a clear net win for codebase complexity.

Copilot Comment Verification

All three Copilot-flagged issues are false positives in the current code:

  1. InstructionExtensions array size (byte.MaxValue vs byte.MaxValue + 1) — The InstructionExtensions class was deleted entirely. GenerateOpCodes in EvmInstructions.cs:29 correctly uses byte.MaxValue + 1 (256 entries), so SELFDESTRUCT (0xFF) is safe.

  2. RETURNDATACOPY using Consume instead of ConsumeDataCopyGas — InstructionReturnDataCopy at EvmInstructions.CodeCopy.cs:138 already correctly calls TGasPolicy.ConsumeDataCopyGas(...). This was addressed in commit 3b37dd3.

  3. CLZ missing from benchmark StackRequirements — CLZ is already at EvmOpcodesBenchmark.cs:421 with the correct (1, 1, 0) signature. Also addressed in 3b37dd3.

Observations

Minor — Missing test cases for Osaka and Amsterdam in InvalidOpcodeTests

InvalidOpcodeTests.cs defines OsakaInstructions (adds CLZ) and AmsterdamInstructions (adds DUPN, SWAPN, EXCHANGE) in _validOpcodes, but the Test method only has [TestCase] attributes up to PragueBlockTimestamp. Neither activation is exercised. The CLZ, DUPN/SWAPN/EXCHANGE valid-opcode coverage is dead test data for now.

Fix this →

Minor — SLOTNUM absent from all fork instruction lists in InvalidOpcodeTests

SLOTNUM (0x4b) is enabled via spec.IsEip7843Enabled in EvmInstructions.cs:136-139 but isn't in any of the fork instruction arrays in InvalidOpcodeTests. This is a pre-existing gap (not introduced by this PR) but worth noting.

Benchmark note — DUPN/SWAPN/EXCHANGE benchmarks hit error path

AllValidLegacyOpcodes includes DUPN, SWAPN, EXCHANGE. Their StackRequirements returns (0, 0, 1), so SetupGenericStack sets depth = 0. The benchmark bytecode begins with JUMPDEST (0x5b) which falls in the disallowed immediate range for EIP-8024 TryDecodeSingle, so these benchmarks effectively time the error-path (StopOrBadInstruction), not actual instruction throughput. Pre-existing quirk inherited from before this PR, not introduced here.

Code Quality

  • EOF removal is clean and thorough — no lingering IsEofEnabled references remain anywhere in the codebase.
  • CodeDepositHandler.cs is correctly simplified to just EIP-3541 (0xEF prefix) validation.
  • TransactionSubstate simplification (raw ReadOnlyMemory<byte> instead of (CodeInfo, Bytes)) is clean.
  • GenerateOpCodes lookup table correctly sized at 256 entries (byte.MaxValue + 1).
  • RETURNDATACOPY implementation correctly moved to EvmInstructions.CodeCopy.cs and uses ConsumeDataCopyGas — consistent with other *COPY opcodes.

Overall: LGTM with the minor test coverage gap noted above.

@github-actions
chore(pgo): update PGO profile (#10867)
e690b22 
chore(pgo): update PGO profile from weekly collection

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@NethermindEth NethermindEth deleted a comment from github-actions bot Mar 19, 2026
@NethermindEth NethermindEth deleted a comment from github-actions bot Mar 19, 2026
@NethermindEth NethermindEth deleted a comment from github-actions bot Mar 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 19, 2026

Block Processing Benchmark Comparison

Run: View workflow run
Base: fca448c9 | Head: e690b223

Method Base (us) PR (us) Delta Base CV PR CV Alloc Base Alloc PR Alloc Delta
AccessList_50 762.9 784.6 +2.9% 1.7% 2.4% 69.7 KB 69.8 KB +0.1%
ContractCall_200 1,825.9 1,854.9 +1.6% 1.2% 0.7% 351.6 KB 351.6 KB +0.0%
ContractDeploy_10 543.7 549.7 +1.1% 3.3% 3.1% 54.2 KB 53.0 KB -2.3%
Eip1559_200 1,810.5 1,900.6 +5.0% 🔼 0.9% 3.9% 334.3 KB 334.8 KB +0.2%
EmptyBlock 26.2 26.7 +2.1% 14.7% 28.2% 6.9 KB 6.9 KB +0.0%
MixedBlock 1,829.4 1,829.2 -0.0% 0.8% 3.2% 341.5 KB 341.3 KB -0.0%
SingleTransfer 99.7 113.1 +13.5% 🔼 32.8% 30.3% 18.3 KB 18.3 KB +0.0%
Transfers_200 1,818.9 1,916.3 +5.4% 🔼 1.8% 3.9% 334.5 KB 334.9 KB +0.1%
Transfers_50 787.6 798.7 +1.4% 2.2% 2.8% 61.1 KB 61.2 KB +0.2%
Detailed statistics
Method Metric Base PR Delta
AccessList_50 Mean 762.9 us 784.6 us +2.9%
AccessList_50 Median 764.7 us 791.6 us +3.5%
AccessList_50 P90 776.6 us 805.8 us +3.8%
AccessList_50 P95 776.6 us 806.6 us +3.9%
AccessList_50 Min 738.2 us 757.4 us +2.6%
AccessList_50 Max 776.6 us 807.4 us +4.0%
AccessList_50 StdDev 13.1 us 18.6 us +41.6%
ContractCall_200 Mean 1,825.9 us 1,854.9 us +1.6%
ContractCall_200 Median 1,827.1 us 1,853.2 us +1.4%
ContractCall_200 P90 1,847.1 us 1,875.4 us +1.5%
ContractCall_200 P95 1,856.4 us 1,875.7 us +1.0%
ContractCall_200 Min 1,793.7 us 1,832.3 us +2.2%
ContractCall_200 Max 1,865.7 us 1,876.0 us +0.6%
ContractCall_200 StdDev 21.7 us 13.3 us -39.0%
ContractDeploy_10 Mean 543.7 us 549.7 us +1.1%
ContractDeploy_10 Median 545.4 us 550.2 us +0.9%
ContractDeploy_10 P90 559.6 us 571.7 us +2.2%
ContractDeploy_10 P95 565.6 us 574.4 us +1.6%
ContractDeploy_10 Min 513.5 us 526.9 us +2.6%
ContractDeploy_10 Max 571.6 us 577.0 us +0.9%
ContractDeploy_10 StdDev 17.8 us 17.2 us -3.4%
Eip1559_200 Mean 1,810.5 us 1,900.6 us +5.0%
Eip1559_200 Median 1,807.7 us 1,947.9 us +7.8%
Eip1559_200 P90 1,826.7 us 1,959.7 us +7.3%
Eip1559_200 P95 1,835.5 us 1,976.5 us +7.7%
Eip1559_200 Min 1,789.4 us 1,791.1 us +0.1%
Eip1559_200 Max 1,844.2 us 1,993.4 us +8.1%
Eip1559_200 StdDev 16.2 us 73.6 us +353.7%
EmptyBlock Mean 26.2 us 26.7 us +2.1%
EmptyBlock Median 28.6 us 28.8 us +0.5%
EmptyBlock P90 29.7 us 35.9 us +20.7%
EmptyBlock P95 30.0 us 36.2 us +20.6%
EmptyBlock Min 21.6 us 16.9 us -22.0%
EmptyBlock Max 30.3 us 36.5 us +20.5%
EmptyBlock StdDev 3.9 us 7.5 us +95.6%
MixedBlock Mean 1,829.4 us 1,829.2 us -0.0%
MixedBlock Median 1,825.9 us 1,816.8 us -0.5%
MixedBlock P90 1,846.4 us 1,902.0 us +3.0%
MixedBlock P95 1,849.1 us 1,924.4 us +4.1%
MixedBlock Min 1,812.3 us 1,759.4 us -2.9%
MixedBlock Max 1,851.7 us 1,946.9 us +5.1%
MixedBlock StdDev 15.2 us 58.5 us +283.6%
SingleTransfer Mean 99.7 us 113.1 us +13.5%
SingleTransfer Median 81.0 us 132.6 us +63.6%
SingleTransfer P90 143.1 us 145.9 us +1.9%
SingleTransfer P95 154.9 us 150.0 us -3.1%
SingleTransfer Min 73.6 us 75.0 us +1.8%
SingleTransfer Max 166.6 us 154.1 us -7.5%
SingleTransfer StdDev 32.7 us 34.3 us +4.8%
Transfers_200 Mean 1,818.9 us 1,916.3 us +5.4%
Transfers_200 Median 1,821.5 us 1,887.6 us +3.6%
Transfers_200 P90 1,854.3 us 2,015.4 us +8.7%
Transfers_200 P95 1,857.7 us 2,015.7 us +8.5%
Transfers_200 Min 1,765.6 us 1,842.2 us +4.3%
Transfers_200 Max 1,861.2 us 2,016.1 us +8.3%
Transfers_200 StdDev 32.1 us 74.1 us +130.4%
Transfers_50 Mean 787.6 us 798.7 us +1.4%
Transfers_50 Median 786.2 us 794.3 us +1.0%
Transfers_50 P90 804.0 us 832.3 us +3.5%
Transfers_50 P95 812.3 us 836.2 us +2.9%
Transfers_50 Min 753.0 us 765.3 us +1.6%
Transfers_50 Max 820.7 us 840.2 us +2.4%
Transfers_50 StdDev 17.7 us 22.4 us +26.8%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 19, 2026

EXPB Benchmark Comparison

Run: View workflow run

superblocks

Scenario: nethermind-halfpath-superblocks-remove-eof-delay0s

Metric PR Master (cached) Delta PR vs Master
AVG (ms) 1475.673500 1475.090800 +0.04%
MEDIAN (ms) 1128.465000 1128.730000 -0.02%
P90 (ms) 2553.52 2545.41 +0.32%
P95 (ms) 2702.41 2697.21 +0.19%
P99 (ms) 3478.53 4092.57 -15.00%
MIN (ms) 764.54 794.35 -3.75%
MAX (ms) 4237.31 4313.63 -1.77%

realblocks

Scenario: nethermind-halfpath-realblocks-remove-eof-delay0s

Metric PR Master (cached) Delta PR vs Master
AVG (ms) 40.964235 41.213190 -0.60%
MEDIAN (ms) 23.370000 24.120000 -3.11%
P90 (ms) 110.92 112.22 -1.16%
P95 (ms) 146.21 147.43 -0.83%
P99 (ms) 212.38 217.44 -2.33%
MIN (ms) 0.58 0.52 +11.54%
MAX (ms) 3721.59 1966.15 +89.28%

@benaadams
test: add Osaka and Amsterdam fork coverage to InvalidOpcodeTests
4d7c722 
Add TestCase attributes for Osaka (CLZ) and Amsterdam (SLOTNUM, SWAPN,
DUPN, EXCHANGE) activations. Add missing SLOTNUM to AmsterdamInstructions.
@benaadams
Merge branch 'remove-eof' of https://github.com/NethermindEth/nethermind
ad826c5 
 into remove-eof
@benaadams benaadams merged commit 2d4d09a into master Mar 19, 2026
423 of 425 checks passed
@benaadams benaadams deleted the remove-eof branch March 19, 2026 12:55
@benaadams
Copy link
Copy Markdown
Member Author

benaadams commented Mar 19, 2026

EOF it was a journey 🫡

image

This was referenced Mar 25, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@LukaszRozmej LukaszRozmej LukaszRozmej approved these changes

@smartprogrammer93 smartprogrammer93 Awaiting requested review from smartprogrammer93 smartprogrammer93 is a code owner

@MarekM25 MarekM25 Awaiting requested review from MarekM25 MarekM25 is a code owner

@flcl42 flcl42 Awaiting requested review from flcl42 flcl42 is a code owner

+1 more reviewer

@Demuirgos Demuirgos Demuirgos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

block-processing-benchmark BREAKING evm performance is good refactoring reproducible-benchmark skip-evm-benchmarks

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@benaadams @LukaszRozmej @Demuirgos