Add missing forks to fork ID

openethereum / parity-ethereum

The fast, light, and robust client for Ethereum-like networks.

Other

6.82k stars 1.69k forks source link

Add missing forks to fork ID #11747

Closed vorot93 closed 4 years ago

vorot93 commented 4 years ago

Fixes #11744

meowsbits commented 4 years ago

Thank you @vorot93 :pray: . Indeed, far better than the hack. And fast! :racing_car:

GregTheGreek commented 4 years ago

Great work guys!

I understand the issue, but curious @vorot93 what exactly is going on under the hood here?

vorot93 commented 4 years ago

@GregTheGreek Clients using eth/64 protocol and higher send a FORK_ID which is a hash of all fork heights. It is used to aggressively cut off incompatible clients. Unfortunately, not all heights were included for Classic which made OpenEthereum's ForkId incompatible on a network protocol (not consensus) layer.

Fortunately, the clients have not yet removed support for eth/63 protocol, with Parity Ethereum 2.7.2 serving as an eth/63 bridge between Core-Geth and OpenEthereum 3.0. After we mandate FORK_ID any bug in its generation would instantly lead to a complete network partition.

meowsbits commented 4 years ago

not all heights were included for Classic which made OpenEthereum's ForkId incompatible

Which ones weren't included? (And why wasn't the incompatibility happening before?)

The reason I ask is the changes include new fork features as follows, and I don't see any that look as though they were introduced to the Classic config with the latest fork (Phoenix)

                    ethash.params.difficulty_hardfork_transition,
                    ethash.params.bomb_defuse_transition,
                    ethash.params.eip100b_transition,
                    ethash.params.ecip1010_pause_transition,
                    ethash.params.ecip1010_continue_transition,
                    ethash.params.ecip1017_era_rounds,
                    ethash.params.expip2_transition,

https://github.com/openethereum/openethereum/pull/11747/files#diff-9a0d4c3f753d90ed4b2dd57d597ee2c1R433-R439

(Althought I don't know what expip2_transition is...)

GregTheGreek commented 4 years ago

Right on - I understand that side of it, im actually curious at the changs you made and how that solves it

vorot93 commented 4 years ago

@GregTheGreek I just added the transitions that were overlooked, along with a test for Classic. I admit that it should have been caught before, but we only had the test vectors for Foundation, Ropsten, Rinkeby and Goerli to check against.

meowsbits commented 4 years ago

Does the test show that the the ForkID for Classic at and beyond 10500839 is hash=9007bfcc next=0?

https://github.com/openethereum/openethereum/pull/11747/files#diff-99d10428a60fba02d3df00e8212d7ac9R149-R156 do match up to the eligible forks spec'd at https://github.com/ethereumclassic/ECIPs/blob/master/_specs/ecip-1082.md#ethereum-classic-mainnet-etc), is this the essence of the test?

vorot93 commented 4 years ago

Also, don't worry about expip2_transition - it's not defined in any chain specification except Expanse's one, and thus will not affect FORK_ID generation for Classic in any way.

bobsummerwill commented 4 years ago

Thanks for the speedy fix, @vorot93!