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

EIP-1716 Hardfork Meta: Petersburg (a.k.a. "Constantinople Fix") #1716

Closed
wants to merge 10 commits into from

Conversation

5chdn
Copy link
Contributor

@5chdn 5chdn commented Jan 21, 2019

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 from Constantinople.

@5chdn
Copy link
Contributor Author

5chdn commented Jan 21, 2019

also we need to merge #1689 and #1642 to finalized constantinople

- `Block >= 4_939_394` on the Ropsten testnet
- Removed EIPs:
- [EIP 1283](./eip-1283.md): Net gas metering for SSTORE without dirty maps

Copy link
Contributor

Choose a reason for hiding this comment

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

Potential clarifications:

  • 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.

Copy link
Member

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?

Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor

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 petersburg block to 0 and don't worry about moving that one around if you move constantinople.
Also, geth has override.constantinople flag, 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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

@5chdn 5chdn changed the title add hardfork meta for peters fork (constantinople fix) EIP-1716 Hardfork Meta: Petersburg (a.k.a. "Constantinople Fix") Jan 21, 2019
@chfast
Copy link
Contributor

chfast commented Feb 6, 2019

Will that make the "Petersburg" its official name?

@5chdn 5chdn closed this Feb 17, 2019
@5chdn 5chdn deleted the a5-stpetersfork branch February 17, 2019 19:09
@axic
Copy link
Member

axic commented Feb 18, 2019

@5chdn why did you close this? I think this one should be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants