Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Create hardfork meta config flags #9973

Closed
5chdn opened this issue Nov 27, 2018 · 4 comments
Closed

Create hardfork meta config flags #9973

5chdn opened this issue Nov 27, 2018 · 4 comments
Labels
F8-enhancement 🎊 An additional feature request. M2-config 📂 Chain specifications and node configurations. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Milestone

Comments

@5chdn
Copy link
Contributor

5chdn commented Nov 27, 2018

For:

  • byzantiumTransition
  • constantinopleTransition

To simplify the configuration. Duplicate or conflicting transitions should throw a meaningful error, i.e., both constantinopleTransition and eip1283Transition specified.

Stand-alone flags like eip1283Transition should be still available for networks that don't want all of the transitions, i.e., there is no EIP-1234 on Kovan.

@5chdn 5chdn added F8-enhancement 🎊 An additional feature request. P5-sometimesoon 🌲 Issue is worth doing soon. M4-core ⛓ Core client code / Rust. M2-config 📂 Chain specifications and node configurations. labels Nov 27, 2018
@5chdn 5chdn added this to the 2.3 milestone Nov 27, 2018
@5chdn
Copy link
Contributor Author

5chdn commented Nov 27, 2018

Related to #9972

@sorpaas
Copy link
Collaborator

sorpaas commented Nov 27, 2018

(Repost from another discussion.)

From a pure engineering perspective though, it can break guarantees that a certain transition only means one thing. This is not black and white and we have already broken it before, but I would hope we make the chances as small as possible. Giving a hypothetical example that what grouped transition might break:

  1. When Metropolis hard fork comes out, it's planned to include EIP-A, EIP-B, and EIP-C. We then activate those three via grouped transition metropolisTransition on Kovan and some other custom PoA networks.
  2. It's then decided that we want to remove EIP-B from the hard fork, and add an additional EIP-D. Apparently we'll need to change metropolisTransition to use this new meaning, but at this time, if any custom chain forgets to update its config, it will break (even though it passes chainspec tests).

My suggestion for dealing with chainspec would be:

  1. Add deny_unknown_fields to deserializer! In this way, a newer chainspec cannot be accidentally used in an older parity-ethereum version. If any transition is deprecated/removed, we also ensure that an older chainspec that contains those won't accidentally be loaded on newer parity-ethereum version.
  2. Create a "reduced" format that doesn't have eipXXXTransitionfields, but byzantiumTransition, constantinopleTransition, and a config generator that generates chainspec from the "reduced" format.

Geth's approach on chainspec is just to always group mainnet hard fork changes together. So you won't find individual EIP numbers there. This certainly makes configuration easier (and the engineering cost is the same as us). However, the apparent drawback is that you cannot support other networks.

@ghost
Copy link

ghost commented Dec 25, 2018

Comment from @karalabe

ethereum/go-ethereum#17675 (comment)

While in general that's not a bad idea, within Geth we kind of always focused on the Ethereum use case and tried to avoid making everything too abstract/flexible. There have been multiple consensus issues in Parity caused by EIPs getting accidentally activated due to their flexible architecture, as well as configuring a Parity node is a nightmare that can lately only be done via custom tooling. Our code mostly assumes that Ethereum hard forks come in progression one after the other and this keeps everything a lot simpler. I of course understand that this puts a bit more burden on teams creating Ethereum forks, but at the end of the day why should the go-ethereum maintainers pay the price of running derivatives based on our code? :)

😂

@5chdn 5chdn modified the milestones: 2.3, 2.4 Jan 10, 2019
@5chdn 5chdn modified the milestones: 2.4, 2.5 Feb 21, 2019
@soc1c soc1c modified the milestones: 2.5, 2.6 Apr 2, 2019
@ordian ordian modified the milestones: 2.6, 2.7 Jul 12, 2019
@adria0
Copy link

adria0 commented Jul 27, 2020

Closing issue due to its stale state.

@adria0 adria0 closed this as completed Jul 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F8-enhancement 🎊 An additional feature request. M2-config 📂 Chain specifications and node configurations. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

5 participants