fix(evm): charge a frame for resolving its target's delegation - #12786
fix(evm): charge a frame for resolving its target's delegation#12786Marchhill wants to merge 7 commits into
Conversation
A frame whose target carries an EIP-7702 designation followed it for free: the entry charge covered the target's own access but not the access of the designated address that create_evm_from_frame charges alongside it.
|
Claude finished @Marchhill's task in 7m 3s —— View job Review: charge a frame for resolving its target's delegation
Verdict: no Critical/High/Medium findings — looks correct and mergeable. Two Low notes, both posted inline. What I checkedThe charge reproduces
The subtle case is the third row, and the Other things that hold up:
FindingsLow — duplicated access-cost expression ( Low — the new OOG-at-entry branch is untested ( Informational — pre-existing, not introduced here. Not verified
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Cover the frame gas window between the target's own access and the designation's, a designated precompile's warm access, and share the entry-gas delta between the two entry-charge tests.
|
@claude review |
|
Claude finished @Marchhill's task in 6m 26s —— View job Re-review: charge a frame for resolving its target's delegation
Verdict: one new Medium finding — everything from the previous round is resolved. Previous round: both rebuttals hold, tests address the coverage gap
FindingsMedium — the designated account is read before the charge that gates it ( Re-confirmed from the previous round (unchanged code)The charge itself still agrees with Not verified
|
create_evm_from_frame resolves the designation only once its access is charged, so a frame that cannot afford it must leave the designated account untouched and out of the block access list.
EVM Opcode Benchmark DiffAggregated runs: base=1, pr=1 No significant regressions or improvements detected. |
|
@claude review |
|
Claude finished @Marchhill's task in 6m 32s —— View job Re-review: charge a frame for resolving its target's delegation
Verdict: the previous Medium is fixed. No Critical/High/Medium findings — one Low. The Medium from the last round is resolved
So the BAL divergence is gone: a frame that cannot pay the designation access no longer reads the designated account. The Everything else re-confirmed
FindingsLow — the new test pins the gas half of the fix, not the BAL half ( Informational — pre-existing, and the reason the precompile test reads cleanly. A frame targeting a precompile directly never executes it: Not verified
|
The frame gas boundary case asserts status and gas, both already correct before the read was moved behind its charge; the block access list is what the move changed.
|
@claude review |
|
Claude finished @Marchhill's task in 4m 45s —— View job Re-review: charge a frame for resolving its target's delegation
Verdict: no Critical/High/Medium findings. Two Low notes, both posted inline. Mergeable. The last round's Low is addressed
Verified this roundI went a level deeper than the previous rounds on the repository plumbing, since
FindingsLow — Low — the traced-processor setup is now a third verbatim copy ( Not verified
|
The precompile branch asks the repository for nothing, so the explicit read is the only thing recording the designated account. Share the traced-processor setup while adding its third caller.
|
@claude review |
|
Claude finished @Marchhill's task in 5m 7s —— View job Re-review: charge a frame for resolving its target's delegation
Verdict: no Critical/High/Medium findings. Every prior round's finding is resolved. One readability nit, posted inline. Mergeable. The last round's Low is closed
I also checked the assertion is the right expectation for this repo rather than an accident: there is no precompile filtering anywhere in The
|
|
@claude review |
|
Claude finished @Marchhill's task in 2m 26s —— View job Re-review: charge a frame for resolving its target's delegation
Verdict: no Critical/High/Medium findings. Every finding from the previous four rounds is resolved, and the only new commit is a pure renesting. Mergeable.
|
1 participant
Changes
The frame entry charge covered the target's own access and the EIP-8037
NEW_ACCOUNTstate cost, but not the access that resolving a designation performs.create_evm_from_framecharges it alongside the target's:resolve_delegated_code_addressreads the target's code and, when it is a designation, charges warm or cold for the designated address and warms it. A frame targeting a delegated account was therefore following the designation for free.The designated code is also read only after that charge succeeds, mirroring the top-level path:
create_evm_from_frameresolves the designation before it loads the code, so a frame that cannot afford the access must leave the designated account untouched — and out of the block access list. Following the designation now goes through the sameIsPrecompileguard theCALLpath uses, which is required once the code is fetched separately, since the repository would otherwise dispatch a precompile through a designation.Direction: gas moves up for a frame whose target is delegated — by
COLD_ACCOUNT_ACCESSwhen the designated address is cold, orWARM_ACCESSwhen an earlier frame already touched it (a designation pointing at a precompile is warm, since EIP-2929 seeds the precompiles). Every other frame is unaffected. The charge is applied after the target's own, so a frame that cannot afford it fails with its gas limit consumed, matching the order the reference implementation charges in; the designated address is warmed into the frame's tracker so a reverting frame discards it with the rest of its accesses.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Three cases in
FrameTxProcessorTests, all failing before the change:Execute_FrameTargetingDelegatedAccount_PaysTheDelegateAccesscompares a frame targeting a delegated account against one targeting the designated contract directly, parameterised over a contract designation (3000, was 0) and a precompile designation (100, was 0 — pinning the warm disjunct).Execute_FrameGasCoveringOnlyTheTargetAccess_FailsOnTheDelegateAccessgives the frame exactly the target's own cold access, the window the second entry check opens: the frame now fails with its whole limit consumed, where before it succeeded.Execute_FrameGasCoveringOnlyTheTargetAccess_LeavesTheDesignatedAccountOutOfTheBalcovers the read-ordering half of the same case, which status and gas cannot see: the designated account is absent from the block access list, where reading it before the charge recorded it.Nethermind.Evm.Test: 5125 passed, 8 skipped, 0 failed.dotnet format whitespaceclean, and a build with the lint workflow'sNoWarnset reports noIDE/CAwarnings.Also verified against fixtures: with this change, the frame transaction suite's delegated-target entry-charge cases (cold and warm designated address) and the designation-to-precompile case pass, where all three failed before. No regression in the rest of the suite.
Documentation
Requires documentation update
Requires explanation in Release Notes