-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Deneb crypto helpers test coverage #3283
Changes from 16 commits
ca8a51f
661cca5
81ab7de
cce82b4
b4c130a
08ba1f6
a5333a1
5e74c51
29b5309
96ad61b
723e8a1
cc284b2
ff7a6c5
2d4bfab
3e281e7
3141806
5977f36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -44,7 +44,9 @@ Note: This API is *unstable*. `get_blobs_and_kzg_commitments` and `get_payload` | |||||
Implementers may also retrieve blobs individually per transaction. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah and I checked, the engine api doesn't currently give you the proofs. so right now, adding the proofs to this would be not in sync with the engine api That said abstracting it to magically show up in the validator spec does seem correct/safe so I'm okay leaving this as is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should change the engine to give the proofs, because they should be part of the Which made me notice, it seems the EIP has not been updated to include this yet. Seems to be included in this PR: ethereum/EIPs#6610 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, let's go to engine api after eip is merged |
||||||
|
||||||
```python | ||||||
def get_blobs_and_kzg_commitments(payload_id: PayloadId) -> Tuple[Sequence[BLSFieldElement], Sequence[KZGCommitment]]: | ||||||
def get_blobs_and_kzg_commitments( | ||||||
payload_id: PayloadId | ||||||
) -> Tuple[Sequence[Blob], Sequence[KZGCommitment], Sequence[KZGProof]]: | ||||||
# pylint: disable=unused-argument | ||||||
... | ||||||
``` | ||||||
|
@@ -66,13 +68,14 @@ use the `payload_id` to retrieve `blobs` and `blob_kzg_commitments` via `get_blo | |||||
```python | ||||||
def validate_blobs_and_kzg_commitments(execution_payload: ExecutionPayload, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about using a similar name of
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again I don't think it's a "batch" because it's just all of them. Batch suggests it's an arbitrary set which it is not. |
||||||
blobs: Sequence[Blob], | ||||||
blob_kzg_commitments: Sequence[KZGCommitment]) -> None: | ||||||
blob_kzg_commitments: Sequence[KZGCommitment], | ||||||
blob_kzg_proofs: Sequence[KZGProof]) -> None: | ||||||
# Optionally sanity-check that the KZG commitments match the versioned hashes in the transactions | ||||||
assert verify_kzg_commitments_against_transactions(execution_payload.transactions, blob_kzg_commitments) | ||||||
|
||||||
# Optionally sanity-check that the KZG commitments match the blobs (as produced by the execution engine) | ||||||
assert len(blob_kzg_commitments) == len(blobs) | ||||||
assert [blob_to_kzg_commitment(blob) == commitment for blob, commitment in zip(blobs, blob_kzg_commitments)] | ||||||
assert len(blob_kzg_commitments) == len(blobs) == len(blob_kzg_proofs) | ||||||
assert verify_blob_kzg_proof_batch(blobs, blob_kzg_commitments, blob_kzg_proofs) | ||||||
``` | ||||||
|
||||||
3. If valid, set `block.body.blob_kzg_commitments = blob_kzg_commitments`. | ||||||
|
@@ -87,7 +90,9 @@ Blobs associated with a block are packaged into sidecar objects for distribution | |||||
|
||||||
Each `sidecar` is obtained from: | ||||||
```python | ||||||
def get_blob_sidecars(block: BeaconBlock, blobs: Sequence[Blob]) -> Sequence[BlobSidecar]: | ||||||
def get_blob_sidecars(block: BeaconBlock, | ||||||
blobs: Sequence[Blob], | ||||||
blob_kzg_proofs: Sequence[KZGProof]) -> Sequence[BlobSidecar]: | ||||||
return [ | ||||||
BlobSidecar( | ||||||
block_root=hash_tree_root(block), | ||||||
|
@@ -96,7 +101,7 @@ def get_blob_sidecars(block: BeaconBlock, blobs: Sequence[Blob]) -> Sequence[Blo | |||||
block_parent_root=block.parent_root, | ||||||
blob=blob, | ||||||
kzg_commitment=block.body.blob_kzg_commitments[index], | ||||||
kzg_proof=compute_blob_kzg_proof(blob, block.body.blob_kzg_commitments[index]), | ||||||
kzg_proof=blob_kzg_proofs[index], | ||||||
) | ||||||
for index, blob in enumerate(blobs) | ||||||
] | ||||||
|
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.
should add description of when to use it