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 🕷
| 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 |
cheme
left a comment
cheme
left a comment
There was a problem hiding this comment.
LGTM (even if I am really sad we need to disable it). Using first block disable instead of last block enabled in conf seems like the right choice to me (same behavior as enable in applying update).
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
5 participants
Add a transition flag that allows disabling EIP-1283 when EIP-1283 is already enabled. Replaces #10191