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: Hardfork Meta Backfill - Berlin to Shapella #8005

Merged
merged 17 commits into from
Dec 8, 2023

Conversation

timbeiko
Copy link
Contributor

@timbeiko timbeiko commented Dec 1, 2023

On ACDC#123 we agreed to bring back Hardfork Meta EIPs for Dencun. This EIP "backfills" the forks for which we did not have Meta EIPs by linking out to specifications for each upgrade. Having this merged before the Dencun Hardfork Meta will keep the numbering of Hardfork Meta EIPs chronically aligned with mainnet deployment times.

Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-meta labels Dec 1, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Dec 1, 2023

✅ All reviewers have approved.

@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels Dec 1, 2023
@eth-bot eth-bot changed the title Hardfork Meta Backfill Add EIP: Hardfork Meta Backfill, from Berlin to Shapella Dec 1, 2023
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Dec 1, 2023
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@@ -0,0 +1,91 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
---
---
eip: 7568

Assigning next sequential EIP/ERC/RIP number.
Numbering changed to sequential from 7500 and is no longer the PR number.

Please also update the filename.

Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Dec 1, 2023
@eth-bot eth-bot changed the title Add EIP: Hardfork Meta Backfill, from Berlin to Shapella Add EIP: Hardfork Meta Backfill - Berlin to Shapella Dec 1, 2023
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Dec 1, 2023
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@github-actions github-actions bot added w-ci Waiting on CI to pass and removed w-ci Waiting on CI to pass labels Dec 1, 2023
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Dec 1, 2023
Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
@lightclient
Copy link
Member

Btw I kind of liked how all the eips were explicitly listed in the dencun meta eip. Any thoughts on doing that for this?

g11tech
g11tech previously approved these changes Dec 7, 2023
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

@timbeiko
Copy link
Contributor Author

timbeiko commented Dec 7, 2023

@lightclient interesting idea! Some of the CL upgrades don't have EIPs, but do you think it's worth doing for everything that does?

@lightclient
Copy link
Member

I can go either way. I suppose the execution-specs has the list to all the EIPs anyways. I'll leave it up to you to decide and then I will merge.

@poojaranjan
Copy link
Contributor

Getting an error on the discussion-to link, thus sharing thoughts here.

  1. The bot may stop this proposal from merging given it has EIP-2070: Hardfork Meta: Berlin, EIP-6122: Forkid checks based on timestamps, EIP-6953: Network Upgrade Activation Triggers in "requires" and the EIP is in Stagnant status and Review status respectively.

For Berlin we can perhaps resurrect EIP-2070.

  • bring it to Review status
  • Update to include EIPs deployed with the upgrade
  • Fast forward status promotion to Last Call & Final without delay.
  1. EIP-6122: Forkid checks based on timestamps
  • Historically, only Core EIPs were part of Meta EIP specification. Is there any specific need to necessarily include Networking EIP in "requires" for this Meta EIP?
    • If so, we must promote EIP-6122 to Last Call and Final sooner.
  1. EIP-6953: Network Upgrade Activation Triggers
  • Same as above.
  • If this "Informational" EIP must be added as "requires", let's promote 6122 sooner. Because EIP-6122 needs to be promoted before we make any change to the status of EIP-6953.

I'd like to see EIP-2387 in "requires" to provide the information or prior upgrade that is needed to get to the following network upgrades.

Signed-off-by: Tim Beiko <9390255+timbeiko@users.noreply.github.com>
Copy link

github-actions bot commented Dec 7, 2023

The commit 0862c0d (as a parent of 58187a7) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Dec 7, 2023
@timbeiko
Copy link
Contributor Author

timbeiko commented Dec 7, 2023

Thanks for your comments, @poojaranjan @lightclient! My last commit addresses them as follows:

I suppose the execution-specs has the list to all the EIPs anyways. I'll leave it up to you to decide and then I will merge.

Instead of listing everything, I made it explicit that the linked specs themselves link to the specific changes in forks. Didn't use "EIPs" because there are none one the first CL forks 😅 !


Getting an error on the discussion-to link, thus sharing thoughts here.

Fixed the link!

The bot may stop this proposal from merging given it has EIP-2070: Hardfork Meta: Berlin, EIP-6122: Forkid checks based on timestamps, EIP-6953: Network Upgrade Activation Triggers in "requires" and the EIP is in Stagnant status and Review status respectively.

Good catch! I've opened #8028 to move 2070 to Withdrawn instead of Review. I don't think it should be Final given the "final" spec was moved to the execution-specs repo. That said, if @axic disagrees, I'm happy to have it moved through Last Call & Final ASAP.

EIP-6122: Forkid checks based on timestamps & EIP-6953: Network Upgrade Activation Triggers

I agree it's a bit unusual for these to be included even though they are not Core EIPs themselves. I included them because they both significantly alter the behavior of the network (or describe that alteration), and I'd rather lean towards having them and providing readers a full picture vs. potentially omitting something important. Similarly, 2982 is not a Core EIP but provides important context.

EIPS/eip-7568.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Dec 7, 2023
@poojaranjan
Copy link
Contributor

Thanks for addressing earlier comments, @timbeiko.

Even though this EIP includes missing Meta EIP history, IMHO, it should include EIP-2387: Hardfork Meta: Muir Glacier in "Requires" to include upgrade Meta before Berlin upgrade.

Including prior upgrade Meta EIP in "Requires" had been in practice (eg EIP-1679 is added to EIP-2387) to connect the dots. It will be nice to see it continued.

status: Draft
type: Meta
created: 2023-12-01
requires: 2070, 2387, 2982, 6122, 6953
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@poojaranjan oops, didn't mention it, but it does!

@eth-bot eth-bot enabled auto-merge (squash) December 8, 2023 16:28
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 0249f04 into ethereum:master Dec 8, 2023
10 checks passed
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-meta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants