Skip to content

fix(test-benchmark): clamp nested frame depth to budget and assert receipt status - #3585

Open
zexoverz wants to merge 1 commit into
ethereum:forks/amsterdamfrom
zexoverz:3547
Open

zexoverz wants to merge 1 commit into
ethereum:forks/amsterdamfrom
zexoverz:3547

Conversation

@zexoverz

Copy link
Copy Markdown

Description

This is @spencer-tb's fix from his review on #3349, cherry-picked as he suggested in the issue, authorship kept. Each tx now carries the deepest depth its budget can fund and the fill asserts every receipt is status 1, so an OOG mid descent fails the fill instead of passing silently.

Filled test_nested_frame_memory on Amsterdam at 100M. Before the change, the 64K/256 case fills with receipt status 0 if you add the assertion. After it, all nine cases pass. At mem 0 and 8K every tx still descends the full 256 frames. At 64K the 16,777,216 tx cap only funds 175 frames, so the clamp takes 256 down to 175 there, which is the point of the change rather than a gap in it.

Related Issues or PRs

Closes #3547

Checklist

  • Ran fast static checks to avoid CI fails, see Code Standards & Verifying Changes: just static
  • PR title has the form <type>(<area>): <title>, where <type> and <area> come from an appropriate C-<type>, respectively A-<area>, label. The title should match the target squash commit message.

@spencer-tb spencer-tb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! cc-ing @LouisTsai-Csie for merge, maybe it was missed or there was a reason to not include it :)

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.44%. Comparing base (d4ca3c9) to head (09d62bf).
⚠️ Report is 12 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #3585   +/-   ##
================================================
  Coverage            94.44%   94.44%           
================================================
  Files                  624      624           
  Lines                36927    36929    +2     
  Branches              3326     3326           
================================================
+ Hits                 34874    34876    +2     
  Misses                1450     1450           
  Partials               603      603           
Flag Coverage Δ
unittests 94.44% <ø> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

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.

test_memory_* benchmark refactor

2 participants