Skip to content

Conversation

spencer-tb
Copy link
Contributor

@spencer-tb spencer-tb commented Sep 8, 2025

πŸ—’οΈ Description

Fixes the tx gas limit cap (from EIP-7825) for ModExp (EIP-7883) tests.

πŸ”— Related Issues or PRs

#2108

βœ… Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).

@spencer-tb spencer-tb added scope:tests Scope: Changes EL client test cases in `./tests` type:chore Type: Chore labels Sep 8, 2025
@marioevz
Copy link
Member

marioevz commented Sep 8, 2025

I added a raise Exception to make the test fail if the gas limit went above the cap, but none of the tests failed, even with #2108 applied on top of these changes, so I might be missing something.
What's the steps to reproduce the issues for which this fix is needed?

@spencer-tb
Copy link
Contributor Author

I added a raise Exception to make the test fail if the gas limit went above the cap, but none of the tests failed, even with #2108 applied on top of these changes, so I might be missing something. What's the steps to reproduce the issues for which this fix is needed?

Sorry yeah. We need to add this case manually:
("00" * 32, "00" * 1024, "01" * 1024, "00" * 1023 + "01", "Z16"),

I thought it was in the PR. Will push it just now.

@spencer-tb spencer-tb force-pushed the modexp-tx-gas-cap-fix branch from 5d10cac to 2d1326e Compare September 9, 2025 13:57
@spencer-tb spencer-tb force-pushed the modexp-tx-gas-cap-fix branch from c8d45a9 to c8c463f Compare September 9, 2025 14:52
Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie left a comment

Choose a reason for hiding this comment

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

Nice! I will try the remaining refactoring in my PR, thanks for this

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

See my comment, I think current solution could be improved because future changes might break tests if we are not careful.

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@marioevz marioevz merged commit 58368d6 into main Sep 10, 2025
15 checks passed
@marioevz marioevz deleted the modexp-tx-gas-cap-fix branch September 10, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:tests Scope: Changes EL client test cases in `./tests` type:chore Type: Chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants