-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Ethereum Network Upgrade Windows #1872
Conversation
* multi-line text * spel check * move naming to it's own section
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.
Looks good! Minor suggestions.
EIPS/eip-network_upgrade_windows.md
Outdated
|
||
A roadmap upgrade typically should not occur in adjacent upgrade windows. Hence | ||
if a roadmap upgrade occurred in April then the July window would not be used | ||
for network upgrades. |
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.
Explain a little further why here
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.
It relates to release cadence, which is still up in the air. See next comment.
EIPS/eip-network_upgrade_windows.md
Outdated
|
||
If a planned roadmap upgrade needs to be rescheduled then strong consideration | ||
should be given to rescheduling the upgrade for the next window in three months | ||
time. |
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.
It seems like this is the reason for doing the above. Explicitly make that link if so
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.
Instead of specifying a 6 or more month cadence I added text stating the cadence should be a multiple of three months.
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.
I'm thinking that it could perhaps be stated more clearly as "The reason for leaving open the 3 month window after the proposed slot is to create space for recovery if things go wrong. For example, if an update is missed in a specific time window, it can be easily rescheduled for the following window without the pain of having to figure out multiple teams schedules in an emergency scenario"
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.
I agree, but I would slowly be moving into specifying how roadmap upgrades scheduling and cadence work, which should should be its own EIP. It is still uncertain if we want 9, 6, or 3 month upgrades. 3 month upgrades would not permit empty slots. at 6 and 9 the impact of a missed upgrade are different, at 9 a slip of one window still allows for an empty window. But these discussions and specifications are better suited for a roadmap cadence EIP.
EIPS/eip-network_upgrade_windows.md
Outdated
This EIP provides neither guidance nor restrictions to the development and | ||
deployment of these emergency hard forks. These upgrades are typically launched | ||
promptly after a solution to the systemic risk is agreed upon between the client | ||
implementors. |
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.
I would suggest guidance that such upgrades perform the minimal amount of work possible to mitigate the issue, but that is also probably outside the scope here.
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.
done
EIPS/eip-network_upgrade_windows.md
Outdated
Critical Network Upgrades have historically been two word phrases. Sometimes the | ||
name applies to the issue at hand, sometimes it do not. Previous Critical | ||
Network upgrade names have been Dao Fork, Tangerine Whistle, Spurious Monkey, | ||
and Constantinople Fix. |
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.
Maybe naming is out of scope? After all, end users only need to know the target version.
I would also push for a more explicit versioning scheme alongside the naming convention, but that is also probably outside the scope here.
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.
Agreed, out of scope. It belongs in a Roadmap Cadence EIP. Deleting section.
EIPS/eip-network_upgrade_windows.md
Outdated
more zeros. | ||
|
||
For testnet deployments network operators are encouraged to choose a block | ||
activation number that is a palindrome in base 10. |
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.
Kind of random, but I could see why it might be useful to make this distinction. Warrants a little further discussion why if this section is kept.
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.
Choosing a block number is part of getting into the launch window. So it felt in-scope.
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.
Yeah, but how people choose block numbers is up to them. It only matters to this proposal that they choose a number and align everyone on that date.
EIPS/eip-network_upgrade_windows.md
Outdated
Knowing when a upgrade is not going to occur gives the businesses a clear time | ||
frame within which to perform internal upgrades free from external changes. It | ||
also provides a timetable for developers and IT professionals to schedule time | ||
off against. |
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.
Word!
EIPS/eip-network_upgrade_windows.md
Outdated
Critical Network Upgrade in response to TheDao. Tangerine Whistle and Spurious | ||
Dragon were critical upgrades in response to the Shanghai Spam Attacks. | ||
Constantinople Fix (as it is termed in the reference tests) was in response to | ||
the EIP-1283 security issues. |
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.
An analysis of the dates when previous upgrades were proposed/performed may be helpful to the dates discussion above.
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.
Thanks for taking this on! Some comments below. Main points are that I believe this EIP should offer guidance to client implementors on when to release compatible clients before a fork, and that we should try to choose "mid-week" block numbers to make sure as many hands are on deck as possible around the time of a fork.
EIPS/eip-1872.md
Outdated
Priority network upgrades should be announced and a block chosen far enough in | ||
advance so major clients implementors can release software with the needed block | ||
number in a timely fashion. It is expected that two to four weeks would be | ||
sufficient. |
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.
I would also add something like "Implementors are expected to release a compatible client at least one week prior to the upgrade 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.
I put it in softly saying that clients need to be released a week out, so 2-4 weeks notice will be needed.
three zeros but switched to palindromic numbers. Rinkeby has always had | ||
palindromic activation blocks. Goerli has yet to perform a network upgrade. | ||
|
||
To continue this pattern network upgrade activation block numbers for mainnet |
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.
I think this section should strive to pick a block number "mid-week" for upgrades, where mid-week is between Tuesday to Thursday UTC, as that should maximize the amount of resources available from all teams if issue with the fork occur. IMO this is even more important with the testnets, where most of the "rocky" forks have occurred (either due to lack of miners for the new PoW chains or consensus issues around PoA implementations).
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.
I said Wednesday noon UTC+0 to maximize the likelyhood of a monkey fighting fork on a monday through friday plane.
EIPS/eip-1872.md
Outdated
process a particular window should be targeted. A block number for the network | ||
upgrade should be chosen four to six weeks prior to the network upgrade window. | ||
Scheduling details such as whether this choice is made prior to or after testnet | ||
deployment are out of scope of this EIP. |
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.
I recommend adding: "Implementors are expected to release compatible clients at least two to four weeks before the block number chosen".
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.
done
* clarified wednesday is the target for upgrades * clarified third week as week with the third wednesday * added a table of windows based on Jan + 3m *
I think this is ready to become a real draft. Can I get an EIP editor to merge? |
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.
This can eventually get merged, but I haven't seen a single client developer in this discussion so far. You can define windows, but if teams are not following the proposal, I don't see this being a 'standard'.
@shemnon is a client developer for PegaSys, but yeah getting a geth or parity team member to comment would be very helpful. |
It was discussed in AllCore Devs 58, full of client developers. Also, I don't see where getting a certain set of people to discuss it is part of the process to get a draft published: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md#eip-editor-responsibilities - It is certainly a part of the process of getting an EIP accepted but to require core developer participation is to create a chicken and egg problem, many developers won't consider an EIP that is sitting in the pull queue. |
It's not. It was a helpful comment. |
Initial draft.
Purpose is to set, in advance, certain calendar windows in which network upgrades or hard forks should be performed within.