-
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
Update block's blob_kzg_commitments
size limit to MAX_BLOB_COMMITMENTS_PER_BLOCK (4096)
#3338
Changes from all commits
17dac8c
5e43f43
1aad9b5
9f530a7
a75292b
8ccc257
4458645
feb1968
a05b689
f0a4281
e9cc8dc
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 |
---|---|---|
|
@@ -117,6 +117,11 @@ Deneb introduces new global topics for blob sidecars. | |
|
||
The *type* of the payload of this topic changes to the (modified) `SignedBeaconBlock` found in deneb. | ||
|
||
New validation: | ||
|
||
- _[REJECT]_ The length of KZG commitments is less than or equal to the limitation defined in Consensus Layer -- | ||
i.e. validate that `len(body.signed_beacon_block.message.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK` | ||
Comment on lines
+120
to
+123
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. @djrtwo I added the p2p validation here. Feel free to open a post-merge PR to revise it. 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. yep, makes sense!. definitely need the p2p limit given the type isn't protecting us from dos |
||
|
||
###### `blob_sidecar_{subnet_id}` | ||
|
||
This topic is used to propagate signed blob sidecars, where each blob index maps to some `subnet_id`. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
from eth2spec.test.context import ( | ||
single_phase, | ||
spec_test, | ||
with_deneb_and_later, | ||
) | ||
|
||
|
||
@with_deneb_and_later | ||
@spec_test | ||
@single_phase | ||
def test_length(spec): | ||
assert spec.MAX_BLOBS_PER_BLOCK < spec.MAX_BLOB_COMMITMENTS_PER_BLOCK |
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.
Not sure I would put this reference to execution-layer constraints here.
Definitely don't need a reference to "builder", and there is no definition of EL in the consensus-layer specs at this point. If we want to reference that layer use
ExecutionEngine