Skip to content

Conversation

LouisTsai-Csie
Copy link
Collaborator

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

πŸ—’οΈ Description

Add missing cases for eip7825, and update the checklist items.

πŸ”— Related Issues or PRs

Issue #1793

βœ… 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 8, 2025
@LouisTsai-Csie LouisTsai-Csie added fork:osaka Osaka hardfork scope:checklists Scope: ethereum_test_checklists package type:test Type: Add/refactor fw unit tests; no fw or el client test case changes labels Sep 8, 2025
@LouisTsai-Csie LouisTsai-Csie changed the title Eip7825 checklist refactor(tests): add checklist marker for eip7825 Sep 8, 2025
@LouisTsai-Csie LouisTsai-Csie marked this pull request as ready for review September 8, 2025 07:27
Copy link
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.

Thanks. Small comment on the test_maximum_gas_refund test. I think its correct as is personally. It fills on EELS and passes on geth too.

I still plan to add the blob tx test here for 7825. I'll do that next week.

The code coverage part looks incomplete so just adding that here:
Screenshot 2025-09-12 at 15 02 27

@LouisTsai-Csie
Copy link
Collaborator Author

I update some checklist items, could you run this command again after the blob tx case is implemented.

uv run fill -v tests/osaka/eip7825_transaction_gas_limit_cap/test_tx_gas_limit.py --fork Osaka --clean --cov

Our coverage is 99% and i hope the missing 1% is the blob scenario! We should wait for full coverage before merging.

Screenshot 2025-09-12 at 11 00 50β€―PM

@spencer-tb
Copy link
Contributor

I update some checklist items, could you run this command again after the blob tx case is implemented.

uv run fill -v tests/osaka/eip7825_transaction_gas_limit_cap/test_tx_gas_limit.py --fork Osaka --clean --cov

Our coverage is 99% and i hope the missing 1% is the blob scenario! We should wait for full coverage before merging.
Screenshot 2025-09-12 at 11 00 50β€―PM

Ahh we have the blob tx in: test_transaction_gas_limit_cap().

@spencer-tb
Copy link
Contributor

Could you update the reference spec?

ref_spec_7825 = ReferenceSpec("EIPS/eip-7825.md", "1ed95cbac750539c2aac67c8cbbcc2d77974231c")

@spencer-tb
Copy link
Contributor

Looks like we can remove this line, given the EIP starts from Osaka too:
Screenshot 2025-09-15 at 14 24 29

@LouisTsai-Csie
Copy link
Collaborator Author

Looks like we can remove this line, given the EIP starts from Osaka too:

I am not sure, but it seems the test is valid from Prague? I think this line should be covered, this coverage report seems to be outdated, it does not update to our latest changes, but not sure how to trigger the coverage report again

@pytest.mark.parametrize_by_fork("tx_gas_limit,error", tx_gas_limit_cap_tests)
@pytest.mark.with_all_tx_types
@pytest.mark.valid_from("Prague")
def test_transaction_gas_limit_cap(
    state_test: StateTestFiller,
    pre: Alloc,
    fork: Fork,
    tx_gas_limit: int,
    error: TransactionException | None,
    tx_type: int,
):

@spencer-tb spencer-tb merged commit df81019 into ethereum:main Sep 16, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fork:osaka Osaka hardfork scope:checklists Scope: ethereum_test_checklists package type:test Type: Add/refactor fw unit tests; no fw or el client test case changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants