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

Implement eth/64 (EIP-2364) and drop support for eth/62#11472

Merged
dvdplm merged 2 commits into
openethereum:masterfrom
vorot93:eip-2364
Feb 19, 2020
Merged

Implement eth/64 (EIP-2364) and drop support for eth/62#11472
dvdplm merged 2 commits into
openethereum:masterfrom
vorot93:eip-2364

Conversation

@vorot93
Copy link
Copy Markdown

@vorot93 vorot93 commented Feb 9, 2020 •
edited
Loading

This PR adds support for Ethereum protocol version 64 (eth/64) described in EIP-2364 and drops support for eth/62.

Note that there is a special case for Rinkeby test network since it uses a Clique consensus and has a non-zero Homestead transition block - an edge case that falls outside parity's spec format. Actually fixing this will require breaking json spec format and thus falls outside the scope of this PR.

@parity-cla-bot
Copy link
Copy Markdown

parity-cla-bot commented Feb 9, 2020

It looks like @vorot93 signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

niklasad1
niklasad1 reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/handler.rs
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/mod.rs
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/mod.rs Outdated
@ordian ordian mentioned this pull request Feb 10, 2020
Merged
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/mod.rs Outdated
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/handler.rs Outdated
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/handler.rs Outdated
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/handler.rs Outdated
ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/api.rs Outdated
@vorot93 vorot93 force-pushed the eip-2364 branch 2 times, most recently from 30a6f03 to 2b1c29f Compare February 10, 2020 19:16
@vorot93 vorot93 marked this pull request as ready for review February 10, 2020 19:20
@vorot93
Copy link
Copy Markdown
Author

vorot93 commented Feb 10, 2020

I've switched the tests to eth/64 and they pass.

@vorot93 vorot93 requested review from niklasad1 and ordian February 10, 2020 19:21
@vorot93 vorot93 changed the title Implement EIP-2364 (eth/64) Implement eth/64 (EIP-2364) and drop support for eth/62 Feb 10, 2020
dvdplm
dvdplm reviewed Feb 10, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
@dvdplm
Copy link
Copy Markdown
Collaborator

dvdplm commented Feb 10, 2020

@vorot93 Other than reviewing the code, what's a good way to test this out?

ordian
ordian reviewed Feb 10, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

The networking part looks good to me, but the downcasting thing feels like a layer violation. Need some time to think about a better approach.

dvdplm
dvdplm reviewed Feb 11, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
dvdplm
dvdplm reviewed Feb 11, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/mod.rs Outdated
Comment thread ethcore/sync/src/chain/mod.rs Outdated
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
dvdplm
dvdplm reviewed Feb 11, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
ordian
ordian reviewed Feb 11, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Some comments about EIP-2124 changes.

Comment thread util/EIP-2124/src/lib.rs Outdated
Comment thread util/EIP-2124/src/lib.rs Outdated
Comment thread util/EIP-2124/src/lib.rs Outdated
ordian
ordian reviewed Feb 13, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
@vorot93 vorot93 force-pushed the eip-2364 branch 3 times, most recently from b6aadbe to 2d801eb Compare February 15, 2020 01:11
@ordian ordian mentioned this pull request Feb 18, 2020
Open
ordian
ordian reviewed Feb 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Looks much better now, just need to figure out the dependencies.

Comment thread ethcore/engines/ethash/src/lib.rs Outdated
Comment thread ethcore/sync/Cargo.toml Outdated
@vorot93 vorot93 requested review from dvdplm and ordian February 18, 2020 21:06
ordian
ordian reviewed Feb 18, 2020
View reviewed changes
Comment thread ethcore/sync/Cargo.toml Outdated
Comment thread ethcore/sync/src/chain/fork_filter.rs Outdated
ordian
ordian reviewed Feb 18, 2020
View reviewed changes
Comment thread ethcore/sync/src/chain/fork_filter.rs
ordian
ordian approved these changes Feb 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!

@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust. A9-buythatmanabeer 🍻 Pull request is reviewed well and worth buying the author a beer. A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Feb 18, 2020
@vorot93
Copy link
Copy Markdown
Author

vorot93 commented Feb 19, 2020

Rebased and squashed

dvdplm
dvdplm approved these changes Feb 19, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Nice, this is great. :)

@dvdplm dvdplm merged commit 06df521 into openethereum:master Feb 19, 2020
@vorot93 vorot93 deleted the eip-2364 branch February 19, 2020 13:51
ordian added a commit that referenced this pull request Mar 6, 2020
@ordian
Merge branch 'master' into ao-sled
4508b58 
* master: (27 commits)
  Faster kill_garbage (#11514)
  [EngineSigner]: don't sign message with only zeroes (#11524)
  fix compilation warnings (#11522)
  [ethcore cleanup]: various unrelated fixes from `#11493` (#11507)
  Add benchmark for transaction execution (#11509)
  Add Smart Contract License v1.0
  Misc fixes (#11510)
  [dependencies]: unify `rustc-hex` (#11506)
  Activate on-chain randomness in POA Sokol (#11505)
  Grab bag of cleanup (#11504)
  Implement eth/64 (EIP-2364) and drop support for eth/62 (#11472)
  [dependencies]: remove `util/macros` (#11501)
  OpenEthereum bootnodes are added (#11499)
  [ci benches]: use `all-features` (#11496)
  [verification]: make test-build compile standalone (#11495)
  complete null-signatures removal (#11491)
  Include the seal when populating the header for a new block (#11475)
  fix compilation warnings (#11492)
  cargo update -p cmake (#11490)
  update to published rlp-derive (#11489)
  ...
@adria0 adria0 mentioned this pull request Aug 20, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@niklasad1 niklasad1 Awaiting requested review from niklasad1

2 more reviewers

@dvdplm dvdplm dvdplm approved these changes

@ordian ordian ordian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. A9-buythatmanabeer 🍻 Pull request is reviewed well and worth buying the author a beer. M4-core ⛓ Core client code / Rust.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vorot93 @parity-cla-bot @dvdplm @ordian @niklasad1