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

Update EIP-4844: Increase Blob Throughput #7154

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

adietrichs
Copy link
Member

@adietrichs adietrichs commented Jun 8, 2023

Increases the throughput to a target of 3 blobs (0.375MB) and limit of 6 blobs (0.75MB) as decided on ethereum/pm#786.

Also adjusts DATA_GASPRICE_UPDATE_FRACTION to keep targeting a max change rate for the blob gas price of 12.5% per block (see EIP section for math).

@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Jun 8, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Jun 8, 2023

✅ All reviewers have approved.

EIPS/eip-4844.md Outdated Show resolved Hide resolved
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.

looks good, I think we should move ahead w/ this change on 4844 devnets and can always dial it back down if needed

Comment on lines +48 to +49
| `MAX_DATA_GAS_PER_BLOCK` | `786432` |
| `TARGET_DATA_GAS_PER_BLOCK` | `393216` |
Copy link
Member

@ralexstokes ralexstokes Jun 8, 2023

Choose a reason for hiding this comment

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

checking math:
2^18 works out to 2 blobs (4096 * 32 bytes per blob given trusted setup size)
2^19 works out to 4 blobs (max is double target)

if we increase target by 50% (to move from 2 to 3), then we in fact get the numbers proposed

Copy link
Contributor

Choose a reason for hiding this comment

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

consider 3 * 2**18 and 3 * 2**17 for legibility?

Copy link
Member

Choose a reason for hiding this comment

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

could add in the third column (so after the decimal representations), I do like having the multiplier * base factor representation

Copy link
Member Author

Choose a reason for hiding this comment

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

the problem is that this depends on the specific context, e.g. not obvious why it always should be base 2. or also, might be better in this specific case to always use factor * 2**17 (so that factor would be the number of blobs)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you go with partially calculated values, here another option (clearer imho):
3 * DATA_GAS_PER_BLOB
6 * DATA_GAS_PER_BLOB

EIPS/eip-4844.md Show resolved Hide resolved
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
@adietrichs adietrichs marked this pull request as ready for review June 8, 2023 15:31
@adietrichs adietrichs requested a review from eth-bot as a code owner June 8, 2023 15:31
@eth-bot eth-bot enabled auto-merge (squash) June 8, 2023 15:50
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal s-review This EIP is in Review t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants