-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
EIP-1716 Hardfork Meta: Petersburg (a.k.a. "Constantinople Fix") #1716
Closed
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
3511a6a
Merge pull request #3 from ethereum/master
5chdn 9332c17
Merge branch 'master' of https://github.com/ethereum/EIPs
5chdn 0ad123a
Merge branch 'master' of https://github.com/ethereum/EIPs
5chdn b831928
add hardfork meta for peters fork
5chdn 16d41e8
assign eip number
5chdn 9f3518f
fix Ropsten block number
5chdn f88222b
use consistent naming
5chdn 5f07fc2
format the block numbers
5chdn e256123
add precendence
5chdn 4e67ad9
Update eip-1716.md
5chdn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| --- | ||
| eip: 1716 | ||
| title: "Hardfork Meta: Petersburg" | ||
| author: Afri Schoedon (@5chdn) | ||
| type: Meta | ||
| status: Draft | ||
| created: 2019-01-21 | ||
| requires: 1013, 1283 | ||
| --- | ||
|
|
||
| ## Abstract | ||
|
|
||
| This meta-EIP specifies the changes included in the Ethereum hardfork that removes [EIP-1283](./eip-1283.md) from [Constantinople](./eip-1013.md). | ||
|
|
||
| ## Specification | ||
|
|
||
| - Codename: Petersburg | ||
| - Aliases: St. Petersfork, Peter's Fork, Constantinople Fix | ||
| - Activation: | ||
| - `Block >= 7_280_000` on the Ethereum mainnet | ||
| - `Block >= 4_939_394` on the Ropsten testnet | ||
| - `Block >= 10_255_201` on the Kovan testnet | ||
| - `Block >= 9_999_999` on the Rinkeby testnet | ||
| - `Block >= 0` on the Görli testnet | ||
| - Removed EIPs: | ||
| - [EIP 1283](./eip-1283.md): Net gas metering for SSTORE without dirty maps | ||
|
|
||
| If `Petersburg` and `Constantinople` are applied at the same block, `Petersburg` takes precedence: with the net effect of EIP-1283 being _disabled_. | ||
|
|
||
| If `Petersburg` is defined with an earlier block number than `Constantinople`, then there is _no immediate effect_ from the `Petersburg` fork. However, when `Constantinople` is later activated, EIP-1283 should be _disabled_. | ||
|
|
||
| ## References | ||
|
|
||
| The list above includes the EIPs that had to be removed from Constantinople due to a [potential reentrancy attack vector](https://medium.com/chainsecurity/constantinople-enables-new-reentrancy-attack-ace4088297d9). Removing this was agreed upon at the [All-Core-Devs call #53 in January 2019](https://github.com/ethereum/pm/issues/70). | ||
|
|
||
| ## Copyright | ||
|
|
||
| Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/). | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Potential clarifications:
PetersburgandConstantinopleare applied at the same block,Petersburgtakes precedence: with the net effect of EIP 1283 being disabled.Petersburgis defined with an earlier block number thanConstantinople, then there is no immediate effect from thePetersburgfork. However, whenConstantinopleis later activated, EIP-1283 should be disabled.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.
Isn't it easier to state Petersburg can only be applied after Constantinople, with the one exception of activating at the same time, then Petersburg takes preference?
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 with @axic It will better if we can set Petersburg block number >= Constantinople
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 makes it unnecessary complicated. The clarification Martin proposed clearly states how a client should behave if Petersburg < Constantinople.
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 beg to differ. With allowing Petersburg before Constantinople the client must track a flag, while my proposal would allow the client to just reject starting after parsing the block numbers in the configuration.
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.
Well, the flag needs to be tracked regardless. With the proposed solution, we don't have to add an extra constraint when validating the configuration. It also makes it easier to manually/semi-manually create configurations, since you can set the
petersburgblock to0and don't worry about moving that one around if you move constantinople.Also, geth has
override.constantinopleflag, which would continue to work nicely (and I'd recommend Parity to implement it too). If we change like you propose, the user would have to override both.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.
Wouldn't it be better to specify it that Petersburg activates EIP 145, 1014, 1052, 1234 and disables 1283?
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.
No, Constantinople enables EIP 145, 1014, 1052, 1234, and 1283; Petersfork disables 1283. I don't see why we should have two hardforks for the same thing.