refactor(specs): delay get_code calls in CALL-like opcodes after gas charging and stack-depth checks

[jsign]

🗒️ Description

Move the get_code call from each CALL* opcode (call, callcode,delegatecall, staticcall) into generic_call, removing the codeparameter from its signature.

All four callers were doing the identicalget_account(tx_state, code_address).code_hash →get_code(tx_state, code_hash) sequence before passing the result to generic_call, which already receives code_address.

The motivation for this is two-fold:

• Do the get_code call after the charge_gas call for CALL-like opcodes -- this avoids pulling bytecode that isn't used if that OOGs.
• Also avoid pulling the bytecode if we could charge_gas but we would also fail due to a stack depth overflow, which would also not use pulled bytecodes.

🔗 Related Issues or PRs

N/A.

✅ Checklist

• All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:

  uvx tox -e static

• All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
• All: Considered updating the online docs in the ./docs/ directory.
• All: Set appropriate labels for the changes (only maintainers can apply labels).
• Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
• Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
• Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

[jsign]

cc @kevaundray

[kevaundray]

This makes sense to me CC @gurukamath (for thoughts)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.97%. Comparing base (8a0fc69) to head (23c009e).
⚠️ Report is 31 commits behind head on forks/amsterdam.

Additional details and impacted files

@@                 Coverage Diff                 @@
##           forks/amsterdam    #2473      +/-   ##
===================================================
- Coverage            85.97%   85.97%   -0.01%     
===================================================
  Files                  599      599              
  Lines                36916    36909       -7     
  Branches              3771     3771              
===================================================
- Hits                 31738    31731       -7     
  Misses                4560     4560              
  Partials               618      618              

Flag	Coverage Δ
unittests	85.97% <100.00%> (-0.01%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gurukamath]

Is the concern here related to when the account access happens? Or is it when the code access happens?

If it is the latter, maybe we could just delay the code access and pass the code_hash to the generic_call instead of the code? That will enable us to keep the account access closely coupled with its gas charging and warming.

[gurukamath]

Another thing to keep in mind is a potential interaction with EIP-2780 which is in CFI status for amsterdam. In that EIP, the gas charge is closely coupled with whether an account has code or not. So we will need to access the account at the gas charging stage itself.

IMO, this point is not a huge concern right now but if 2780 gets SFI'd, we'll have to move the get_account call, back to where it currently is.

[jochem-brouwer]

Which stack overflow does this refer to? Is this the operand stack or is this the call stack?

[fselmo]

Which stack overflow does this refer to? Is this the operand stack or is this the call stack?

This is the call stack (message depth) check

[fselmo]

lgtm but I'll defer to @gurukamath for a final review since he's assigned himself a review here.

Leave final review to @gurukamath (assignee)

[gurukamath]

lgtm but I'll defer to @gurukamath for a final review since he's assigned himself a review here.

Yeah. Looks good to me too but I'd still want to wait for what might happen with EIP-2780 just because that EIP tightly couples account access gas accounting with actual account access.