Skip to content

evm: Don't check call depth limit in Osaka - #1704

Merged
chfast merged 1 commit into
masterfrom
evm/no_call_depth_check
Sep 11, 2026
Merged

evm: Don't check call depth limit in Osaka#1704
chfast merged 1 commit into
masterfrom
evm/no_call_depth_check

Conversation

@chfast

@chfast chfast commented Sep 11, 2026

Copy link
Copy Markdown
Member

The call depth limit (1024) is not reachable in Osaka because of the transaction gas limit (EIP-7825) and the 63/64 call gas rule. The max call depth reachable is less than 500.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The consensus-sensitive Osaka boundary lacks regression coverage for CALL and CREATE paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Disables unreachable call-depth checks from Osaka onward due to EIP-7825’s transaction gas cap.

Changes:

  • Retains the 1024-depth limit before Osaka.
  • Skips it for CALL and CREATE operations in Osaka and later revisions.
File summaries
File Description
lib/evmone/instructions_calls.cpp Makes call-depth checks revision-dependent.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/evmone/instructions_calls.cpp
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.02%. Comparing base (bf8303f) to head (7a70350).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1704   +/-   ##
=======================================
  Coverage   98.02%   98.02%           
=======================================
  Files         179      179           
  Lines       16347    16357   +10     
  Branches     3765     3765           
=======================================
+ Hits        16024    16034   +10     
  Misses        243      243           
  Partials       80       80           
Flag Coverage Δ
eest-develop 82.78% <100.00%> (ø)
eest-develop-gmp 26.21% <16.66%> (-0.02%) ⬇️
eest-legacy 17.21% <0.00%> (-0.02%) ⬇️
eest-libsecp256k1 28.47% <16.66%> (-0.02%) ⬇️
eest-stable 82.75% <100.00%> (ø)
evmone-unittests 94.32% <100.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
core 96.20% <100.00%> (ø)
tooling 94.06% <ø> (ø)
tests 99.81% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
lib/evmone/instructions_calls.cpp 99.27% <100.00%> (ø)
test/unittests/state_transition_call_test.cpp 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 129 untouched benchmarks


Comparing evm/no_call_depth_check (7a70350) with master (bf8303f)

Open in CodSpeed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Removing a consensus-sensitive execution check warrants human confirmation despite the focused regression test.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread test/unittests/state_transition_call_test.cpp Outdated
@chfast
chfast force-pushed the evm/no_call_depth_check branch 6 times, most recently from 7faee99 to 2c1c0c5 Compare September 11, 2026 14:37
The call depth limit (1024) is not reachable in Osaka because of the transaction
gas limit (EIP-7825) and the 63/64 call gas rule: the cheapest possible
self-recursion runs out of gas at depth 494. The added state transition test
pins that depth, so the premise fails visibly if a reprice ever changes it.

It is the first test to recurse for real rather than enter at a synthetic
msg.depth, and it overflows the 1 MB stack Windows gives the main thread.
Executing nested calls is evmone's own recursion, so the library asks its
consumers for the 8 MB other platforms provide.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Direct EVMC execution can now exceed the VM’s fixed execution-state capacity, and the standalone library misses the stack option.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

lib/evmone/instructions_calls.cpp:235

  • The same out-of-capacity path applies to contract creation: an Osaka frame entered at depth 1024 now forwards a CREATE at depth 1025. Recursive execution then violates VM::get_execution_state()'s 1,025-entry invariant, asserting in debug builds or reallocating and invalidating the still-active outer state in release builds. The transaction gas cap is not enforced by the EVMC VM itself, so this check must remain unless deeper externally initiated executions are made safe across the VM.
    if (state.rev < EVMC_OSAKA && state.msg->depth >= 1024)
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

}

if (state.msg->depth >= 1024)
if (state.rev < EVMC_OSAKA && state.msg->depth >= 1024)
Comment thread lib/evmone/CMakeLists.txt
target_link_libraries(evmone PUBLIC evmc::evmc intx::intx PRIVATE evmone::precompiles)

# Nested EVM calls recurse ~2.6 KB of stack per frame, too much for Windows' 1 MB default.
target_link_options(evmone INTERFACE $<$<CXX_COMPILER_ID:MSVC>:/STACK:8388608>)
@chfast
chfast merged commit 42a296c into master Sep 11, 2026
26 checks passed
@chfast
chfast deleted the evm/no_call_depth_check branch September 11, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants