Skip to content
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

specially mark EIP4844 changes #3406

Merged
merged 2 commits into from
Jun 9, 2023
Merged

specially mark EIP4844 changes #3406

merged 2 commits into from
Jun 9, 2023

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Jun 7, 2023

We are planning to include additional features (#3182) in the Deneb fork. This PR modifies the previous "[New in Deneb]" comment to "[New in Deneb:EIP4844]", allowing us to differentiate the sources once we integrate these new features.

Usine the notation EIP4844 rather than EIP-4844 because it's the form we use for feature spec constant, and it's shorter.

@hwwhww hwwhww force-pushed the deneb-4844-clean branch from b9f3a6c to aceb083 Compare June 7, 2023 09:46
@hwwhww hwwhww changed the title specially marked EIP4844 changes specially mark EIP4844 changes Jun 7, 2023
@hwwhww hwwhww added the Deneb was called: eip-4844 label Jun 7, 2023
@hwwhww hwwhww force-pushed the deneb-4844-clean branch from aceb083 to a547d47 Compare June 7, 2023 09:48
@@ -41,16 +41,15 @@

## Introduction

This upgrade adds blobs to the beacon chain as part of Deneb. This is an extension of the Capella upgrade.

The blob transactions are packed into the execution payload by the EL/builder with their corresponding blobs being independently transmitted and are limited by `MAX_DATA_GAS_PER_BLOCK // DATA_GAS_PER_BLOB`. However the CL limit is independently defined by `MAX_BLOBS_PER_BLOCK`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this note to next to MAX_BLOBS_PER_BLOCK preset definition.

Comment on lines +44 to +45
Deneb is a consensus-layer upgrade containing a number of features. Including:
* [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844): Shard Blob Transactions scale data-availability of Ethereum in a simple, forwards-compatible manner.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

other features will join this list later

@hwwhww hwwhww added the general:presentation Presentation (as opposed to content) label Jun 7, 2023
@hwwhww hwwhww requested review from djrtwo and ralexstokes June 7, 2023 13:43
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

nice! seems like a reasonable approach imo

left a couple of nits

@@ -336,7 +336,7 @@ def evaluate_polynomial_in_evaluation_form(polynomial: Polynomial,

### KZG

KZG core functions. These are also defined in Deneb execution specs.
KZG core functions. These are also defined in Deneb execution specs for EIP-4844.
Copy link
Contributor

Choose a reason for hiding this comment

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

The top of this doc mentions "required for Deneb specification". I might be explicit up there that it is for 4844 (within Deneb)

@@ -98,6 +98,8 @@ All validator responsibilities remain unchanged other than those noted below.

##### Blob KZG commitments

[New in Deneb:EIP4844]
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we should add more annotations to this doc -- e.g. to "Constructing the SignedBlobSidecars" and the data structures added

@hwwhww hwwhww mentioned this pull request Jun 8, 2023
4 tasks
Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

I don't know if we need this but I can see how it could be helpful to someone trying to digest the new specs

Copy link
Member

@ppopth ppopth left a comment

Choose a reason for hiding this comment

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

I agree with this. This looks good to me :)

@djrtwo djrtwo merged commit 05790d3 into dev Jun 9, 2023
@djrtwo djrtwo deleted the deneb-4844-clean branch June 9, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb was called: eip-4844 general:presentation Presentation (as opposed to content)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants