Add EIP-1283 disable transition #10214
Conversation
sorpaas
added
A0-pleasereview 🤓
|
Waiting for confirmation on block numbers from other teams. I proposed blocks |
5chdn
added
B1-patch-beta 🕷🕷
B0-patch-stable 🕷
| @@ -300,6 +302,10 @@ impl From<ethjson::spec::Params> for CommonParams { | |||
| BlockNumber::max_value, | |||
| Into::into, | |||
| ), | |||
| eip1283_disable_transition: p.eip1283_disable_transition.map_or_else( | |||
| BlockNumber::max_value, | |||
There was a problem hiding this comment.
This means that the disable-eip will be by default disabled (in other words: eip1283 will be enabled), so unless otherwise specified, eip-1283 will be present. How will this work out for mainnet clients? Will you include the fork numbers for Constantinople+Fix later on in this PR?
There was a problem hiding this comment.
eip 1283 is disabled by default too (eip1283 activation transition also default to max block size).
There was a problem hiding this comment.
Yes, but if someone sets constantinople without setting constantinople-disable, it will not default to mainnet behaviour.
Might be fine, but I chose to default to 'if nil: use same as Constantinople' for the geth PR.
There was a problem hiding this comment.
abut the json tag name, it applies camelCase to rust field name so it is indeed eip1283DisableTransition in the json.
There was a problem hiding this comment.
There is no way to "set Constantinople" in Parity Ethereum. You either enable EIP-1283 or you don't (default). You either disable it or you don't (default). So a default chain does neither have a 1283-transition nor a 1283-disable-transition.
There was a problem hiding this comment.
You either enable EIP-1283 or you don't (default)
Not quite true. You either
- Define it, as disabled,
- Define it, as enabled,
- Don't define it.
In the case where you have set a blocknumber for EIP-1283, but haven't defined eip1283_disable_transition, it will default use 1283. Which is what I'm talking about.
Just clarifying what I meant, not trying to argue that you should do it the way I proposed...
There was a problem hiding this comment.
I'm not understanding your comment. if you define something, than you are not using defaults.
eip1283Transitionis defaultBlockNumber::max_value, hence disabledeip1283DisableTransitionis defaultBlockNumber::max_value, hence disabled
So if you don't define 1283, there won't be 1283 by default. You only get it if you set eip1283Transition explicitly.
There was a problem hiding this comment.
Right. So if you have a chain spec which your colleague gave you a month ago, which has eip1283Transition defined as (in two wooks), and update to the latest parity client, it will roll out the version with 1283, because eip1283_disable_transition is not defined.
An alternative approach would be to have it at 0 unless otherwise defined.
|
Also, will the spec json tag be |
5chdn
added
A8-looksgood 🦄
5chdn
added
the
B7-releasenotes 📜
* version: bump stable to 2.2.8 * Update for Android cross-compilation. (#10180) * build-unix update * .gitlab-ci update * Update build-unix.sh add android postprocessing * path to android lib libparity.so * fix path to libparity * add android lib to artifacts * Cancel Constantinople HF on POA Core (#10198) * Add EIP-1283 disable transition (#10214) * Enable St-Peters-Fork ("Constantinople Fix") (#10223) * ethcore: disable eip-1283 on kovan block 10255201 * ethcore: disable eip-1283 on ropsten block 4939394 * ethcore: enable st-peters-fork on mainnet block 7280000 * ethcore: fix kovan chain spec * version: update fork blocks * ethcore: disable eip-1283 on sokol block 7026400
* version: bump beta to 2.3.1 * Fix _cannot recursively call into `Core`_ issue (#10144) * Change igd to github:maufl/rust-igd * Run `igd::search_gateway_from_timeout` from own thread * Update for Android cross-compilation. (#10180) * build-unix update * .gitlab-ci update * Update build-unix.sh add android postprocessing * path to android lib libparity.so * fix path to libparity * add android lib to artifacts * Run all `igd` methods in its own thread (#10195) * Cancel Constantinople HF on POA Core (#10198) * Add EIP-1283 disable transition (#10214) * Enable St-Peters-Fork ("Constantinople Fix") (#10223) * ethcore: disable eip-1283 on kovan block 10255201 * ethcore: disable eip-1283 on ropsten block 4939394 * ethcore: enable st-peters-fork on mainnet block 7280000 * ethcore: fix kovan chain spec * version: update fork blocks * ethcore: disable eip-1283 on sokol block 7026400
Add a transition flag that allows disabling EIP-1283 when EIP-1283 is already enabled. Replaces #10191