Skip to content

fix(tests): add BAL related mappings to ethrex exception mapper#2193

Merged
fselmo merged 2 commits intoethereum:forks/amsterdamfrom
edg-l:fix/ethrex-gas-used-overflow-mapper-v2
Feb 11, 2026
Merged

fix(tests): add BAL related mappings to ethrex exception mapper#2193
fselmo merged 2 commits intoethereum:forks/amsterdamfrom
edg-l:fix/ethrex-gas-used-overflow-mapper-v2

Conversation

@edg-l
Copy link
Contributor

@edg-l edg-l commented Feb 11, 2026

🗒️ Description

Add BAL related mappings to the ethrex exception mapper

This is needed because some hive tests fail due to this with ethrex when testing BAL/Amsterdam

🔗 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

Put a link to a cute animal picture inside the parenthesis-->

ethrex now returns "Block gas used overflow..." for EIP-7778
pre-refund gas accounting failures (Amsterdam+), distinct from the
existing "Gas allowance exceeded..." transaction-level error.
@edg-l edg-l changed the title fix(tests): add BlockException.GAS_USED_OVERFLOW mapping to ethrex exception mapper fix(tests): add BAL related mapping to ethrex exception mapper Feb 11, 2026
Add mappings for Block Access List (EIP-7928) related exceptions:
- INVALID_BLOCK_ACCESS_LIST
- INVALID_BAL_HASH
- INVALID_BAL_EXTRA_ACCOUNT
- INVALID_BAL_MISSING_ACCOUNT
- INCORRECT_BLOCK_FORMAT

ethrex validates BAL by computing the hash and comparing against the
header, so all BAL corruption types produce the same hash mismatch
error. Additionally, index-out-of-bounds and RLP decode errors are
mapped for INVALID_BLOCK_ACCESS_LIST and INCORRECT_BLOCK_FORMAT.
@edg-l edg-l force-pushed the fix/ethrex-gas-used-overflow-mapper-v2 branch from 1c590f2 to 3a73bfb Compare February 11, 2026 17:26
@edg-l edg-l changed the title fix(tests): add BAL related mapping to ethrex exception mapper fix(tests): add BAL related mappings to ethrex exception mapper Feb 11, 2026
@marioevz marioevz requested a review from fselmo February 11, 2026 21:07
@fselmo
Copy link
Contributor

fselmo commented Feb 11, 2026

@edg-l please provide the branch to test this against. I spent some time trying to figure this out myself but main does not pass tests, there is no bal-devnet-2 branch, and I did find one that seemed promising update-hive-amsterdam but I still get 4 remaining fails with "Invalid block hash. Expected ..." which doesn't seem ideal as this is not BAL specific message coming back from Ethrex and this doesn't match the changes in this PR so I may have the wrong branch.

Anyway, this is difficult to review without more context.

@edg-l
Copy link
Contributor Author

edg-l commented Feb 11, 2026

I'm running this fix on this branch https://github.com/ethereum/execution-specs/tree/devnets/bal/2 using hive on ethrex, i didn't push the branch on ethrex when making the issue, i pushed it now to bal-devnet-2

I'm using the devnets/bal/2 branch because forks/amsterdam seemed to have an issue with the slot_number field, which this fixes seemingly 2e9b948 (in devnets/bal/2)

I'm using

make setup-hive
cd hive
./hive \
    --client-file ../fixtures/hive/clients.yaml \
    --client ethrex \
    --sim ethereum/eels/consume-engine \
    --sim.limit ".*fork_Amsterdam.*" \
    --sim.parallelism 16 \
    --sim.loglevel 3 \
    --sim.buildarg fixtures=http://172.17.0.1:8000/fixtures_bal.tar.gz \
    --sim.buildarg branch=fix/bal2-gas-overflow-mapper \
    --docker.nocache "ethrex|sim"

Note:

i had to change the hive dockerfile to use my fork

# from 
RUN git clone --depth 1 https://github.com/ethereum/execution-specs.git && \
# to
RUN git clone --depth 1 https://github.com/edg-l/execution-specs.git && \

I have to test more, currently on my end 12 tests fail, but this pr reduced the fail count considerably.

@fselmo
Copy link
Contributor

fselmo commented Feb 11, 2026

I'm using the devnets/bal/2 branch because forks/amsterdam seemed to have an issue with the slot_number field, which this fixes seemingly 2e9b948 (in devnets/bal/2)

Yeah, this is the correct branch to use for execution-specs 👍🏼

i didn't push the branch on ethrex when making the issue, i pushed it now to bal-devnet-2

This is what I needed, the ethrex branch that I can test against in hive, thanks! I will check against this.

@edg-l
Copy link
Contributor Author

edg-l commented Feb 11, 2026

Just tested again (commit a18adec04f6a4d9a198b6760e43a27ff47b3e035 on ethrex) and the command I sent produces no failing tests

Feb 12 00:14:53.462 INF API: suite ended suite=0
Feb 12 00:14:54.484 INF simulation ethereum/eels/consume-engine finished suites=1 tests=864 failed=0

Copy link
Contributor

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

Perfect @edg-l, thank you for the extra info.. I've confirmed on my end as well! It's very much appreciated that you contributed this update for Ethrex support :) lgtm 🚀

@fselmo fselmo merged commit ed47360 into ethereum:forks/amsterdam Feb 11, 2026
15 checks passed
@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.07%. Comparing base (a82b1f6) to head (3a73bfb).
⚠️ Report is 6 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #2193   +/-   ##
================================================
  Coverage            86.07%   86.07%           
================================================
  Files                  599      599           
  Lines                39472    39472           
  Branches              3780     3780           
================================================
  Hits                 33977    33977           
  Misses                4862     4862           
  Partials               633      633           
Flag Coverage Δ
unittests 86.07% <ø> (ø)

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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@edg-l
Copy link
Contributor Author

edg-l commented Feb 12, 2026

Perfect @edg-l, thank you for the extra info.. I've confirmed on my end as well! It's very much appreciated that you contributed this update for Ethrex support :) lgtm 🚀

Thanks for the merge! One question though, could this fix be cherry-picked to the devnets/bal/2 or devnets/bal/3 branch? Else it makes it difficult for me to setup hive testing on ethrex and i would need to use a fork of the execution-specs which i would like to avoid. Since forks/amsterdam doesnt have 2e9b948

@fselmo
Copy link
Contributor

fselmo commented Feb 12, 2026

One question though, could this fix be cherry-picked to the devnets/bal/2 or devnets/bal/3 branch? Else it makes it difficult for me to setup hive testing on ethrex and i would need to use a fork of the execution-specs which i would like to avoid. Since forks/amsterdam doesnt have 2e9b948

Not cherry-picked, the branch is instead rebased. We need to set these on more regular automated actions but I will rebase the branch against forks/amsterdam 👍🏼

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