fix(l1): align SIGPARAM copy operands with CALLDATACOPY

[edg-l]

EIP-8141 changed the SIGPARAM copy form's operand order so it matches CALLDATACOPY: the three operands below signatureIndex and param are now memOffset, dataOffset, length (ethereum/EIPs@4a9ad32c, "align SIGPARAM copy operand order with CALLDATACOPY"). We still popped them in the old length, dataOffset, memOffset order, so an ARBITRARY-signature verifier copying its own bytes read the wrong three words — consensus-visible, since SIGPARAM 0x04 is the only way to reach those bytes from the EVM.

• OpSigParamHandler pops [mem_offset, data_offset, length].
• Doc comment and docs/eip-8141.md state the operand order top-first, like FRAMEDATACOPY.
• sigparam_copy_code pushes in the new order, and a new test pins it with three distinct operands and a destination past the first word, so a reversed read lands the bytes elsewhere instead of coincidentally passing.

The two existing copy tests use memOffset = 0; they now exercise the new order too.

Note for the frame-tx satellite branches: eip-7906, eip-8250 and eip-8272 all touch OpSigParamHandler and will need this one-line rebase.

[github-actions]

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Stateless (zkEVM) Amsterdam+ EF tests skipped

Where: tooling/ef_tests/blockchain/test_runner.rs — parse_and_execute skips
fixtures with network >= Fork::Amsterdam when running with a stateless backend.
Affects make test-stateless (the vectors_zkevm/ run); make test-levm is
unaffected.

Why: The stateless run uses the tests-zkevm@v0.5.0 bundle, filled against
glamsterdam-devnet v6.1.0, which predeploys the EIP-8282 builder deposit/exit
contracts at the OLD addresses (0x0000884d…d9008282 / 0x000014574a…0f008282).
This client uses the devnet-7 addresses (0x0000bff4…300d8282 /
0x000064d6…800e8282, matching the live tests-glamsterdam-devnet@v7.2.0 bundle
used by make test-levm). Every Amsterdam+ block runs the end-of-block EIP-8282
builder system call; with the new addresses absent from the v0.5.0 bundle, each
stateless Amsterdam+ block fails with
SystemContractCallFailed("System contract: 0x0000…8282 has no code after deployment").
The skip is by fork rather than by test name, since cross-fork directories such as
for_amsterdam/prague/... still execute at the Amsterdam fork.

Removal: Delete the skip_stateless_amsterdam branch in parse_and_execute
once a tests-zkevm bundle filled with the devnet-7 builder predeploy addresses is
released and .fixtures_url_zkevm is bumped to it.

[github-actions]

🤖 Kimi Code Review

The PR correctly fixes the operand order for EIP-8141's SIGPARAM copy operation (param 0x04) to match the CALLDATACOPY convention ([memOffset, dataOffset, length]). This is a consensus-critical fix for developer experience and consistency with existing EVM patterns.

Review:

crates/vm/levm/src/opcode_handlers/frame_tx.rs:478

• Correctness: The destructuring order change from [length, data_offset, mem_offset] to [mem_offset, data_offset, length] correctly aligns with the stack push order in tests and matches CALLDATACOPY semantics. Verified that stack.pop() returns values in LIFO order (top of stack first), so the array destructuring correctly assigns the most recently pushed operand (mem_offset) to the first binding.

test/tests/levm/eip8141_tests.rs:3286-3307

• Testing: The updated test helper sigparam_copy_code correctly reverses the push order to match the new stack layout (pushing length first, mem_offset last before param and index).
• Coverage: The new test sigparam_0x04_reads_operands_in_calldatacopy_order (lines 3426-3442) effectively validates the operand order by using distinct values (mem_offset=0x20, data_offset=1, length=3) and verifying the data lands at the expected memory location with the expected slice offset. This prevents regression.

docs/eip-8141.md

• Documentation: Accurately reflects the new stack layout [signatureIndex, param, memOffset, dataOffset, length] with signatureIndex on top.

No issues found. The fix is minimal, well-tested, and maintains consistency with existing EVM opcode patterns.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Claude Code Review

No other stale operand-order references found — those are just opcode table/gas constant/comment mentions, unrelated to operand order. The fix is fully self-contained and consistent.

Review: fix(l1): align SIGPARAM copy operands with CALLDATACOPY

This is a clean, well-scoped consensus bug fix. Findings:

Correctness

• The handler change (crates/vm/levm/src/opcode_handlers/frame_tx.rs:477) correctly swaps the pop destructuring from [length, data_offset, mem_offset] to [mem_offset, data_offset, length]. I verified this now matches OpCallDataCopyHandler (crates/vm/levm/src/opcode_handlers/environment.rs:195), which pops [dst_offset, src_offset, len] — same top-to-bottom convention (destination offset on top). The fix is internally consistent with the stack-pop convention used elsewhere in the codebase (stack.pop::<N>() returns [top, ..., bottom]).
• The size_offset_to_usize(length, mem_offset) call argument order is preserved correctly and matches the equivalent call in OpCallDataCopyHandler.
• The metadata-pop path ([signature_index, param], signatureIndex on top) is untouched and was already correct — good, this confirms the bug was isolated to the copy-form operand order only.

Test coverage

• The new test sigparam_0x04_reads_operands_in_calldatacopy_order is well designed: it uses three distinct operand values (length=3, data_offset=1, mem_offset=0x20) and a destination offset in the second memory word. This is important — a same-value or offset-0 test could pass "by accident" under either operand order, but this test would fail if any of the three operands were transposed. Good adversarial test design.
• The two pre-existing tests (sigparam_0x04_...) are correctly updated to pass mem_offset=0 explicitly now that the helper takes a fourth parameter, and still exercise the new order since mem_offset == data_offset == 0 previously masked the swap between those two — but length was already distinct from both, so this old test actually should have caught a length/mem_offset transposition before, just not a mem_offset/data_offset one. Worth noting only for context; not a defect in this PR.
• I manually traced the bytecode in sigparam_copy_code (push order: length, data_offset, mem_offset, param, index) against the resulting stack (top-to-bottom: index, param, mem_offset, data_offset, length) and confirmed it matches both the doc's stated order and the handler's new pop order.

Documentation

• docs/eip-8141.md and the doc comment above OpSigParamHandler are updated consistently with the code change and with each other.
• No other stale references to the old operand order were found elsewhere in the codebase (checked vm.rs, opcodes.rs, gas_cost.rs, transaction.rs — only unrelated opcode-table/gas-constant mentions).

Scope

• Minimal diff (17 lines removed, 38 added, mostly test/doc), no unrelated changes. Gas accounting (charged before the context/scheme guards) is untouched by this fix and wasn't affected by the bug.

Minor note (not blocking)

• The PR description flags that satellite branches (eip-7906, eip-8250, eip-8272) touch the same handler and will need a rebase — that's appropriately called out for the author/reviewers rather than handled in this PR, which is the right call for scope containment.

Overall: correct, well-tested, appropriately scoped fix for a real consensus-affecting bug. No changes requested.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

No findings.

The fix in frame_tx.rs now matches the existing CALLDATACOPY/FRAMEDATACOPY operand-pop convention, and the regression added in eip8141_tests.rs uses distinct memOffset, dataOffset, and length values, so it should catch any future stack-order regression in this consensus-sensitive path. I didn’t see new security, gas-accounting, or memory-safety issues in the touched code or docs update at eip-8141.md.

Testing gap: I couldn’t run the levm test target in this sandbox because cargo/rustup tried to write to a read-only toolchain home, so this review is static only.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[greptile-apps]

Greptile Summary

This PR aligns EIP-8141 SIGPARAM copy operands with CALLDATACOPY semantics.

• Rebinds the three copy operands as memOffset, dataOffset, and length.
• Updates opcode and EIP documentation to describe the top-first stack order.
• Revises existing copy fixtures and adds a distinct-operand regression test.

Confidence Score: 5/5

The PR appears safe to merge; the implementation, documentation, and regression coverage consistently apply the corrected SIGPARAM operand order.

The stack implementation consumes values top-first, the revised handler therefore binds the operands as intended, and the new test uses distinct values that expose swapped memory offset, data offset, or length interpretations.

Important Files Changed

Filename	Overview
crates/vm/levm/src/opcode_handlers/frame_tx.rs	Correctly rebinds SIGPARAM copy operands in top-first stack order and updates the handler documentation.
docs/eip-8141.md	Updates the documented copy-form operand order to match the corrected implementation.
test/tests/levm/eip8141_tests.rs	Updates bytecode construction for the corrected order and adds a regression test using distinct offsets and length.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Push["Push length, dataOffset, memOffset, param, signatureIndex"] --> Stack["Stack top: signatureIndex, param, memOffset, dataOffset, length"]
  Stack --> PopMeta["Pop signatureIndex and param"]
  PopMeta --> PopCopy["Pop memOffset, dataOffset, length"]
  PopCopy --> Copy["Copy signature[dataOffset..] to memory[memOffset..]"]

Loading

_{Reviews (1): Last reviewed commit: "fix(l1): align SIGPARAM copy operands wi..." | Re-trigger Greptile}

[github-actions]

Benchmark Results Comparison

No significant difference was registered for any benchmark run.

Detailed Results

Benchmark Results: BubbleSort

Command	Mean [s]	Min [s]	Max [s]	Relative
main_revm_BubbleSort	2.957 ± 0.017	2.932	2.995	1.14 ± 0.01
main_levm_BubbleSort	2.596 ± 0.018	2.580	2.627	1.00
pr_revm_BubbleSort	3.005 ± 0.031	2.976	3.081	1.16 ± 0.01
pr_levm_BubbleSort	2.609 ± 0.047	2.585	2.739	1.00 ± 0.02

Benchmark Results: ERC20Approval

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_ERC20Approval	989.7 ± 10.1	980.6	1012.9	1.03 ± 0.02
main_levm_ERC20Approval	956.2 ± 11.2	947.0	986.5	1.00
pr_revm_ERC20Approval	991.1 ± 13.0	980.6	1019.0	1.04 ± 0.02
pr_levm_ERC20Approval	956.7 ± 14.2	946.0	993.5	1.00 ± 0.02

Benchmark Results: ERC20Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_ERC20Mint	131.8 ± 0.2	131.6	132.2	1.00
main_levm_ERC20Mint	148.4 ± 0.6	147.4	149.1	1.13 ± 0.00
pr_revm_ERC20Mint	134.3 ± 1.9	132.4	138.9	1.02 ± 0.01
pr_levm_ERC20Mint	148.9 ± 1.0	148.1	151.7	1.13 ± 0.01

Benchmark Results: ERC20Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_ERC20Transfer	236.3 ± 4.7	231.9	245.5	1.00
main_levm_ERC20Transfer	244.6 ± 5.1	240.1	257.7	1.03 ± 0.03
pr_revm_ERC20Transfer	239.2 ± 7.9	233.8	260.2	1.01 ± 0.04
pr_levm_ERC20Transfer	242.6 ± 2.2	240.6	247.0	1.03 ± 0.02

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_Factorial	224.3 ± 3.9	220.1	232.2	1.01 ± 0.02
main_levm_Factorial	248.6 ± 1.0	246.7	250.3	1.12 ± 0.01
pr_revm_Factorial	222.5 ± 2.0	220.5	226.8	1.00
pr_levm_Factorial	247.0 ± 1.7	245.2	250.9	1.11 ± 0.01

Benchmark Results: FactorialRecursive

Command	Mean [s]	Min [s]	Max [s]	Relative
main_revm_FactorialRecursive	1.564 ± 0.037	1.500	1.613	1.00
main_levm_FactorialRecursive	8.505 ± 0.036	8.468	8.565	5.44 ± 0.13
pr_revm_FactorialRecursive	1.585 ± 0.029	1.542	1.620	1.01 ± 0.03
pr_levm_FactorialRecursive	8.518 ± 0.041	8.456	8.581	5.44 ± 0.13

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_Fibonacci	197.9 ± 1.4	194.0	198.8	1.00 ± 0.01
main_levm_Fibonacci	219.0 ± 2.8	215.6	224.3	1.11 ± 0.02
pr_revm_Fibonacci	197.5 ± 1.1	196.2	199.7	1.00
pr_levm_Fibonacci	225.0 ± 21.7	215.5	286.4	1.14 ± 0.11

Benchmark Results: FibonacciRecursive

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_FibonacciRecursive	839.3 ± 10.0	827.7	860.2	1.19 ± 0.02
main_levm_FibonacciRecursive	710.3 ± 6.5	701.8	721.6	1.00 ± 0.02
pr_revm_FibonacciRecursive	842.0 ± 8.6	827.5	857.5	1.19 ± 0.02
pr_levm_FibonacciRecursive	707.4 ± 9.3	699.0	730.4	1.00

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_ManyHashes	8.4 ± 0.1	8.4	8.6	1.00 ± 0.01
main_levm_ManyHashes	9.3 ± 0.2	9.1	9.6	1.10 ± 0.02
pr_revm_ManyHashes	8.4 ± 0.1	8.3	8.5	1.00
pr_levm_ManyHashes	9.3 ± 0.1	9.1	9.5	1.11 ± 0.02

Benchmark Results: MstoreBench

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_MstoreBench	259.8 ± 8.9	250.3	276.7	1.36 ± 0.05
main_levm_MstoreBench	194.5 ± 11.5	189.3	227.1	1.02 ± 0.06
pr_revm_MstoreBench	258.6 ± 16.1	250.8	303.8	1.35 ± 0.09
pr_levm_MstoreBench	191.6 ± 2.1	189.0	195.3	1.00

Benchmark Results: Push

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_Push	288.4 ± 1.0	287.0	290.3	1.21 ± 0.01
main_levm_Push	240.1 ± 3.9	237.4	248.5	1.01 ± 0.02
pr_revm_Push	294.4 ± 12.4	285.4	318.6	1.23 ± 0.05
pr_levm_Push	238.7 ± 2.1	236.0	242.3	1.00

Benchmark Results: SstoreBench_no_opt

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
main_revm_SstoreBench_no_opt	165.4 ± 5.8	161.5	181.4	1.62 ± 0.06
main_levm_SstoreBench_no_opt	105.3 ± 6.2	100.9	122.6	1.03 ± 0.06
pr_revm_SstoreBench_no_opt	165.0 ± 2.6	161.7	169.2	1.61 ± 0.03
pr_levm_SstoreBench_no_opt	102.4 ± 1.4	101.0	104.6	1.00

[ElFantasma]

Two doc nits, both non-blocking.