-
Notifications
You must be signed in to change notification settings - Fork 137
feat(execute,tests): implement blob_transaction_test
execute spec
#1644
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
Conversation
d0a6be6
to
fe9f546
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty amazing! Minor comments below!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whilst looking at the hive simulator Dockerfile, I realised that -m get_blobs
is not very descriptive and not particularly consistent with our current naming. A small change, but an important one 🙂
To be more consistent, I quite like blob_transaction_test
. Even if we don't fill
these tests, I think the naming's ok? And matches expectations, following on from our regular transaction_test
format?
4de6545
to
bc1cc3f
Compare
blob_transaction_test
execute spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Amazing addition!
I would suggest updating the changelog to reflect the execute spec name change to blob_transaction_test
.
…version refactor(tests): Blob type returns NetworkWrappedTransaction refactor(types): Move Blob to types refactor(types): NetworkWrappedTransaction uses Blob construct refactor(types): Return correct BlobAndProof
Update src/ethereum_test_rpc/types.py Co-authored-by: danceratopz <[email protected]> fix(rpc): BlobAndProofV1 and V2
bc1cc3f
to
7dcda11
Compare
Update exception message in src/ethereum_test_execution/get_blobs.py Co-authored-by: danceratopz <[email protected]> Update description src/ethereum_test_execution/get_blobs.py Co-authored-by: danceratopz <[email protected]> Apply suggestions from code review to src/ethereum_test_execution/get_blobs.py Co-authored-by: danceratopz <[email protected]> Update docs/CHANGELOG.md for ethereum_test_execution Co-authored-by: danceratopz <[email protected]> fix ethereum_test_execution comment fix ethereum_test_execution tox fix(execution): Use BlobAndProofV1 and BlobAndProofV2 fix(execution): Rename blob_transaction_test fix(sepcs): Rename blob_transaction_test Update docs/CHANGELOG.md Co-authored-by: danceratopz <[email protected]>
This reverts commit da371d2c45fdf448345168cb5ef2655b7ed05c90. feat(tests): Update 7594 test to include cell proofs
08e9a60
to
e52feca
Compare
🗒️ Description
Adds
get_blobs
marked tests to execute that send blobs to a running client and verify the blob commitments usingengine_getBlobsVX
endpoint.The tests can only be run using execute and cannot be used to generate fixtures using
fill
, contrary to the rest of the specs which normally can do both operations.Test
tests/osaka/eip7594_peerdas/test_get_blobs.py
is added to make a sanity check of the blob endpoints for clients, but should be considered incomplete and a follow up PR should increase coverage by adding meaningful tests that contain proper kzg cell proofs.Note: The PR contains commits separated by functionality and could be merged using "Rebase and Merge".
🔗 Related Issues
None
✅ Checklist