fix(inspector): forward EIP-7708 transfer logs - #3796
0xalpharush wants to merge 3 commits into
Conversation
Merging this PR will degrade performance by 12.74%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | snailtracer-inspect |
187.4 ms | 214.7 ms | -12.74% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing 0xalpharush:agent/eip7708-inspector-logs (0977207) with glam-devnet-7 (3b2393b)
Footnotes
-
1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩
825a01f to
0977207
Compare
| if (opcode::LOG0..=opcode::LOG4).contains(&opcode) { | ||
| inspect_log(interpreter, context, &mut inspector); | ||
| } else { | ||
| inspect_eip7708_transfer_logs(context, &mut inspector, logs_i); |
There was a problem hiding this comment.
We could remove the previous lines LOG0..= and just use inspect_eip7708_transfer_logs to report every log.
| next_action | ||
| } | ||
|
|
||
| #[inline] |
There was a problem hiding this comment.
| #[inline] | |
| #[inline(never)] |
| #[inline] | ||
| fn is_eip7708_transfer_log(log: &Log) -> bool { | ||
| log.address == ETH_TRANSFER_LOG_ADDRESS | ||
| && log.data.topics().first() == Some(Ð_TRANSFER_LOG_TOPIC) | ||
| } | ||
|
|
There was a problem hiding this comment.
And remove this if we want to use it for every log
| #[inline] | |
| fn is_eip7708_transfer_log(log: &Log) -> bool { | |
| log.address == ETH_TRANSFER_LOG_ADDRESS | |
| && log.data.topics().first() == Some(Ð_TRANSFER_LOG_TOPIC) | |
| } |
* fix(inspector): forward EIP-7708 transfer logs to the inspector Backport of the inspector fix from #3796: journal logs emitted outside LOG* opcodes (EIP-7708 value-transfer logs from calls, tx value transfers and selfdestructs) were never handed to Inspector::log. Co-authored-by: 0xalpharush <0xalpharush@protonmail.com> * refactor(inspector): report all step logs through a single hook Replace the LOG*-only inspect_log with inspect_step_logs, which forwards every log the instruction journaled since the pre-step index via log_full. This covers LOG* and the EIP-7708 transfer logs of value-moving instructions, and drops the stale-logs().last() guard in favour of exact index math. The frame-init paths keep the filtered, interpreter-less variant. * refactor(inspector): merge the log-forwarding helpers into one cold fn inspect_step_logs, inspect_step_logs_inner, inspect_eip7708_transfer_logs and is_eip7708_transfer_log collapse into inspect_logs, which forwards the logs journaled since logs_i via log_full when an interpreter is available and log otherwise. It is always cold: each call site keeps only the cheap journal-length check inline. The 7708 address/topic filter is dropped now that every journaled log is forwarded, which also flattens the branch chain in inspect_frame_init. --------- Co-authored-by: 0xalpharush <0xalpharush@protonmail.com>
|
Superseed with #3816 |
2 participants
Summary
Forward journal-created EIP-7708 ETH transfer logs through
Inspector::logfor frame initialization and SELFDESTRUCT transfers.Also fix SELFDESTRUCT inspector callbacks so they use the relevant journal entry and report the correct value:
This branch is retargeted onto
glam-devnet-7/ #3795.Sequencing
This PR currently carries the stale-SELFDESTRUCT journal fix that is also isolated in #3797, because the Amsterdam preserved-value fix depends on inspecting the correct SELFDESTRUCT journal entry. If #3797 lands first, this branch should be rebased and the duplicate stale-journal commit dropped, leaving only the EIP-7708 log forwarding and Amsterdam preserved-value fixes. If this combined PR lands first, #3797 can be closed as a subset.
Impact
Receipts already contained the EIP-7708 transfer logs, but revm inspector consumers could miss them because the inspector handler only forwarded LOG opcode and precompile logs. Execution and state were already correct; tracer output was incomplete.
SELFDESTRUCT inspector consumers could also observe misleading callback data in selfdestruct-to-self cases: stale journal entries could be reused, and Amsterdam preserved-balance cases could report
0even when the account retained value.Root Cause
The inspector handler needed to forward journal-created EIP-7708 ETH transfer logs after frame initialization and SELFDESTRUCT transfers.
For SELFDESTRUCT, the inspector path also needed to inspect the most recent relevant
AccountDestroyedjournal entry for the target account. Amsterdam/EIP-8246 selfdestruct-to-self preserves the account balance and recordshad_balance = 0for revert bookkeeping, so the callback value must come from the preserved account balance instead of that journal field.Testing
cargo +nightly test -p revm-inspector eip7708on the feat(amsterdam): glamsterdam devnet-7 alignment (EIP-2780 runtime gas phase, fixtures v7.0.0) #3795 basecargo +nightly test -p revm-inspector selfdestruct_to_selfon the feat(amsterdam): glamsterdam devnet-7 alignment (EIP-2780 runtime gas phase, fixtures v7.0.0) #3795 basestructured_compare_amsterdam/crash-97865b0ccb6eca1d7c1c70cbe3229bbdb91d8ce6passes in evm2-fuzzers when pinned to this branch at09772073