Skip to content

feat(tests): execute blob tests client agnostic #1720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spencer-tb
Copy link
Contributor

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

🗒️ Description

Updates the peerdas blob tests for execute hive mode to be client agnostic with respect to max blobs per tx. Additionally adds some cleanup to tests/osaka/eip7594_peerdas/test_get_blobs.py.

Geth now passes the test_get_blobs[...single_blob_max_txs] test!

🔗 Related Issues

ethereum/go-ethereum#31837

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.

@spencer-tb spencer-tb added scope:tests Scope: Changes EL client test cases in `./tests` type:feat type: Feature scope:execute Scope: Changes to the execute command fork:osaka Osaka hardfork labels Jun 8, 2025
@spencer-tb spencer-tb force-pushed the hive-execute-blobs-geth-fix branch from a9a328d to 725a77b Compare June 8, 2025 12:34
@spencer-tb spencer-tb self-assigned this Jun 8, 2025
@spencer-tb spencer-tb marked this pull request as ready for review June 8, 2025 12:57
@spencer-tb spencer-tb force-pushed the hive-execute-blobs-geth-fix branch from e134511 to 12a87c3 Compare June 8, 2025 12:58
This runs at test execution time when we have access to both the fork and client type.
"""
requested_id = request.param
all_test_cases = generate_full_blob_tests(fork, client_type)
Copy link
Member

@danceratopz danceratopz Jun 8, 2025

Choose a reason for hiding this comment

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

I think this will unnecessarily generate the blobs for all tests in the setup of every test case. Applying scope="module" to the fixture will avoid this.

Copy link
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Two small comments, thanks!

@pytest.mark.parametrize_by_fork(
"txs_blobs",
generate_full_blob_tests,
@pytest.fixture
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@pytest.fixture
@pytest.fixture(scope="module")

"""Get max blobs per tx considering both fork and client."""
# https://github.com/ethereum/go-ethereum/issues/31792
# https://github.com/ethereum/go-ethereum/pull/31837
if client_type and "go-ethereum" in client_type.name and fork >= Osaka:
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, could it ever be geth? In the client YAML, yes. But I'm not sure where the client type comes from, perhaps it's fixed by Hive and doesn't come from the YAML (e.g., client field or nametag field)? I can check later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fork:osaka Osaka hardfork scope:execute Scope: Changes to the execute command scope:tests Scope: Changes EL client test cases in `./tests` type:feat type: Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants