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

Add EIP: eth/69 - Drop pre-merge fields #8271

Merged
merged 11 commits into from
Mar 12, 2024

Conversation

MariusVanDerWijden
Copy link
Member

Draft of eth70
Todo:

  • check if we should do 69 or 70
  • Any additional things we can remove?

@eth-bot
Copy link
Collaborator

eth-bot commented Feb 29, 2024

✅ All reviewers have approved.

@eth-bot eth-bot added the e-consensus Waiting on editor consensus label Feb 29, 2024
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-networking w-ci Waiting on CI to pass labels Feb 29, 2024
@jochem-brouwer
Copy link
Member

jochem-brouwer commented Feb 29, 2024

Quick reminder that here eth/68 was updated: ethereum/devp2p@6b259a7

On this issue it was discussed:

ethereum/devp2p#245

See: ethereum/devp2p#245 (comment)

So maybe we want to include this as well in eth/69 or whatever version gets choosen here? (have no strong opinion to include this though)

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Feb 29, 2024
@eth-bot eth-bot added the e-review Waiting on editor to review label Feb 29, 2024
@eth-bot eth-bot changed the title Draft: Eth70 v2 Add EIP: eth/70 - Drop pre-merge fields Feb 29, 2024
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Feb 29, 2024
@MariusVanDerWijden MariusVanDerWijden changed the title Add EIP: eth/70 - Drop pre-merge fields EIP-7642: eth/69 - Drop pre-merge fields Mar 1, 2024
@MariusVanDerWijden
Copy link
Member Author

I've renamed the proposal to eth/69 now after discussing with @smartprogrammer93 as this will likely go in before the proposal that drops history. Thanks again!

@MariusVanDerWijden
Copy link
Member Author

@jochem-brouwer Do you think we should change the protocol to use the previously proposed mechanism? I would prefer leaving it as is and enshrining the "bug"

@eth-bot eth-bot changed the title EIP-7642: eth/69 - Drop pre-merge fields Add EIP: eth/69 - Drop pre-merge fields Mar 1, 2024
Copy link

github-actions bot commented Mar 1, 2024

The commit 127877f (as a parent of b59d95c) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Mar 1, 2024
@jochem-brouwer
Copy link
Member

@jochem-brouwer Do you think we should change the protocol to use the previously proposed mechanism? I would prefer leaving it as is and enshrining the "bug"

I have no strong opinion here, I think just leaving it in as bug is fine :)

@MariusVanDerWijden
Copy link
Member Author

I did some more benchmarks and determined the floor for bandwidth improvements to be at ~95GiB.
I converted the receipts of a node from non-bloom to with bloom (and compressed them with snappy) and the disk usage for receipts goes up from 179.07GiB to 273.28 GiB, so up by 94.21 GiB. The theoretical increase should be 530GB, so the snappy compression is quite good. We can not expect the same level of compression on the wire though, but this gives us a good floor for this improvement in terms of bandwidth

@MariusVanDerWijden MariusVanDerWijden marked this pull request as ready for review March 7, 2024 11:08
EIPS/eip-7642.md Outdated Show resolved Hide resolved
Co-authored-by: Martin HS <martin@swende.se>
@eth-bot eth-bot enabled auto-merge (squash) March 12, 2024 21:35
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...

@eth-bot eth-bot merged commit e064e49 into ethereum:master Mar 12, 2024
11 checks passed
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-new Creates a brand new proposal e-consensus Waiting on editor consensus e-review Waiting on editor to review s-draft This EIP is a Draft t-networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants