Skip to content

Conversation

raxhvl
Copy link
Member

@raxhvl raxhvl commented Sep 19, 2025

πŸ—’οΈ Description

Adds No-op and no-op adjacent test cases for EIP-7928

πŸ”— Related Issues or PRs

βœ… 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).

@raxhvl

This comment was marked as resolved.

@raxhvl raxhvl added scope:tests Scope: Changes EL client test cases in `./tests` type:test Type: Add/refactor fw unit tests; no fw or el client test case changes labels Sep 19, 2025
@raxhvl raxhvl self-assigned this Sep 19, 2025
@raxhvl

This comment was marked as resolved.

@raxhvl raxhvl marked this pull request as ready for review September 29, 2025 11:03
@raxhvl raxhvl force-pushed the feat/eip-7928/test-noop branch from f3ef32a to 97e7edc Compare October 1, 2025 13:04
@fselmo fselmo force-pushed the feat/eip-7928/test-noop branch 2 times, most recently from 4b4405a to ccd2966 Compare October 3, 2025 17:37
@fselmo fselmo added the type:feat type: Feature label Oct 3, 2025
@fselmo fselmo force-pushed the feat/eip-7928/test-noop branch from 85480d5 to 179c570 Compare October 3, 2025 19:50
Copy link
Collaborator

@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.

Hey @raxhvl, I think this is good to go but can use your eyes as a sanity check before merging. Some things to note:

  • As we talked about, I updated the empty account expectations to use BalAccountExpectation.empty() explicitly here. We raise if ambiguous BalAccountExpectation() is used. I think this explicitness is much nicer for a reader / implementer. I added unit tests for these as well.
  • I removed a duplicate test. It looks like the fully unmutated test was exactly the same as the bal_noop_storage_write test except for checking the storage read, which I added to the latter and removed the former here.
  • I also moved the tests into the main file and added a changelog entry.

Let me know if you think this is good to go as well and we can get this merged πŸ˜„. Thanks for your work on all these πŸ‘ŒπŸΌ

@fselmo fselmo force-pushed the feat/eip-7928/test-noop branch from 179c570 to 5b0d5cb Compare October 3, 2025 21:03
@fselmo fselmo force-pushed the feat/eip-7928/test-noop branch from 5b0d5cb to faed742 Compare October 4, 2025 02:25
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:feat type: Feature 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.

2 participants