Skip to content

feat(tests): tighten invalid-path BAL coverage for EIP-2935 and EIP-4788#2716

Merged
spencer-tb merged 2 commits intoethereum:forks/amsterdamfrom
junbyjun1238:tests/amsterdam-invalid-callee-bal
Apr 20, 2026
Merged

feat(tests): tighten invalid-path BAL coverage for EIP-2935 and EIP-4788#2716
spencer-tb merged 2 commits intoethereum:forks/amsterdamfrom
junbyjun1238:tests/amsterdam-invalid-callee-bal

Conversation

@junbyjun1238
Copy link
Copy Markdown
Contributor

@junbyjun1238 junbyjun1238 commented Apr 18, 2026

🗒️ Description

Tighten invalid path BAL assertions and add a missing 4788 invalid calldata test.

Tightening:
Add missing callee side balance_changes=[] assertions for the invalid query + non-zero value paths in the EIP-2935 and EIP-4788 BAL tests.

The caller/query side is already asserted, but HISTORY_STORAGE_ADDRESS / BEACON_ROOTS_ADDRESS had no explicit callee balance expectation in these cases. Because BAL expectations only validate fields that are explicitly set, those callee balance changes were not being checked at all.

New test: test_bal_4788_invalid_calldata_size, symmetric to the existing test_bal_2935_invalid_calldata_size. EIP-4788 requires exactly 32 bytes of calldata; parametrized over calldata_size (0/31/33) * value (0/100) to verify the beacon roots contract reverts before any storage access with no balance transfer.

Affected tests:

  • test_bal_2935_query
  • test_bal_2935_invalid_calldata_size
  • test_bal_4788_query
  • test_bal_4788_invalid_calldata_size (new)

No semantics change for the tightening, this only makes the existing tests stricter. The new test covers a pre-existing coverage gap.

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast static checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    just 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.

@spencer-tb spencer-tb added C-feat Category: an improvement or new feature P-high A-tests Area: Consensus tests. labels Apr 20, 2026
@spencer-tb spencer-tb changed the title tests(amsterdam): add callee BAL assertions for invalid EIP-2935/4788 queries feat(tests): tighten invalid-path BAL coverage for EIP-2935 and EIP-4788 Apr 20, 2026
Copy link
Copy Markdown
Contributor

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

LGTM! I added an extra test here too, I would like to get it in the next release today so merging :D
cc @fselmo for extra eyes, anything else can be a follow up

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.26%. Comparing base (40461a6) to head (a049c35).
⚠️ Report is 4 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #2716   +/-   ##
================================================
  Coverage            86.26%   86.26%           
================================================
  Files                  599      599           
  Lines                37038    37038           
  Branches              3795     3795           
================================================
  Hits                 31949    31949           
  Misses                4525     4525           
  Partials               564      564           
Flag Coverage Δ
unittests 86.26% <ø> (ø)

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.

@spencer-tb spencer-tb merged commit 0345cf1 into ethereum:forks/amsterdam Apr 20, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-tests Area: Consensus tests. C-feat Category: an improvement or new feature P-high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants