Skip to content

refactor(tests): add checklist marker for eip7883#2098

Merged
spencer-tb merged 11 commits intoethereum:mainfrom
LouisTsai-Csie:eip7883-checklist
Sep 16, 2025
Merged

refactor(tests): add checklist marker for eip7883#2098
spencer-tb merged 11 commits intoethereum:mainfrom
LouisTsai-Csie:eip7883-checklist

Conversation

@LouisTsai-Csie
Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie commented Sep 3, 2025

🗒️ Description

EIP-7883 updates the gas calculation formula. This PR marks the appropriate checklist item for the eip.

This category aligns with the Gas Cost Changes section, but since test coverage in the previous fork was incomplete, I’ve included it under New Precompiles for review.

New Precompiles - Link

  • Call contexts
    • CALL
    • DELEGATECALL
    • STATICCALL
    • CALLCODE
    • Transaction Entry-point
    • Initcode call
    • Precompile as Set-code Delegated Address -> used in test_set_code_txs.py.
  • Inputs
  • Value Transfer
    • Minimum Fee Precompile: No value transfer is required
    • [x]No-Fee Precompile
  • Out-of-bounds checks
    • precompile/test/out_of_bounds/max
    • precompile/test/out_of_bounds/max_plus_one
  • Input Lengths
    • Zero-length Input
    • Static Required Input Length
    • Dynamic Required Input Length
  • Gas usage
    • Constant Gas Cost: The gas cost is dynamic
    • Variable Gas Cost
    • Excessive Gas
  • Fork transition
    • Pre-fork Block Call
    • Cold/Warm Precompile Address State: Note Modexp is not a new one, so it is warm even before fork activation

Gas Cost Changes - Link

  • Gas Usage:
    • gas_cost_changes/test/gas_updates_measurement
  • Out-of-gas
  • Fork transition

Missing Test:

  • precompile/test/gas_usage/dynamic/exact
  • precompile/test/gas_usage/dynamic/oog
  • gas_cost_changes/test/out_of_gas

🔗 Related Issues or PRs

Issue ethereum/execution-specs#1564

✅ 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).
  • 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.

@LouisTsai-Csie LouisTsai-Csie self-assigned this Sep 3, 2025
@LouisTsai-Csie LouisTsai-Csie added fork:osaka Osaka hardfork scope:tests Scope: Changes EL client test cases in `./tests` type:refactor Type: Refactor labels Sep 3, 2025
@LouisTsai-Csie LouisTsai-Csie marked this pull request as draft September 3, 2025 14:11
@LouisTsai-Csie LouisTsai-Csie marked this pull request as ready for review September 4, 2025 08:22
@LouisTsai-Csie LouisTsai-Csie marked this pull request as draft September 4, 2025 15:41
@LouisTsai-Csie
Copy link
Collaborator Author

I am not very sure about intention for gas_cost_changes/test/out_of_gas.

For the Inputs section in eip7883, i would like to update it later after a separate PR for eip7823 refactoring.

Copy link
Collaborator

@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! Really glad we are checking the precompile result too, and nice gas used parameterization!

@spencer-tb
Copy link
Collaborator

spencer-tb commented Sep 15, 2025

Could we also overwrite the test id for the transition test:

@pytest.mark.parametrize(
    "modexp_input,modexp_expected,gas_old,gas_new",
    [
        pytest.param(Spec.modexp_input, Spec.modexp_expected, 200, 1200),
    ],
    ids=[""],
)

Otherwise we get a huge test id :)

Ohh, and ethereumjs mentioned it was a pain to parse test_modexp_invalid_inputs so we should do the same there:

@pytest.mark.parametrize(
    "modexp_expected,call_succeeds",
    [
        pytest.param(bytes(), False),
    ],
    ids=[""],
)

@spencer-tb spencer-tb merged commit 2302246 into ethereum:main Sep 16, 2025
15 checks passed
kclowes pushed a commit to kclowes/execution-spec-tests that referenced this pull request Oct 20, 2025
* refactor: add checklist marker for eip7883

* doc: update eip checklist template

* refactor: update modexp test and add checklist marker

* chore: update checklist rules

* refactor: enhance modexp checklist descriptions for clarity

* refactor: update coverage table and check gas usage

* fix rebase issue

* chore: remove outdated entry from modexp checklist

* refactor: expand modexp checklist with external coverage entries

* refactor: adjust gas usage assertion for only Osaka fork

* refactor: overwrite id and add comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork:osaka Osaka hardfork scope:tests Scope: Changes EL client test cases in `./tests` type:refactor Type: Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants