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

Allow zero chain id in EIP155 signing process #9792

Merged
merged 3 commits into from
Oct 26, 2018
Merged

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Oct 22, 2018

EIP155 actually didn't specify chainid 0 is invalid, and it can be valid -- 35 means refined v to be 0, and 36 means refined v to be 1. This PR fixes that.

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 22, 2018
@sorpaas sorpaas added this to the 2.2 milestone Oct 22, 2018
@ascjones
Copy link
Contributor

Tests are failing

@sorpaas
Copy link
Collaborator Author

sorpaas commented Oct 22, 2018

We need to either:

  • Disable those tests.
  • Or force EIP155 being disabled when network id is zero.

@sorpaas sorpaas closed this Oct 23, 2018
@sorpaas sorpaas deleted the sp-eip155-zero-chainid branch October 23, 2018 15:52
@sorpaas sorpaas restored the sp-eip155-zero-chainid branch October 26, 2018 11:41
@sorpaas
Copy link
Collaborator Author

sorpaas commented Oct 26, 2018

Reopening this for a while as I'm really confused why those tests fail.

@sorpaas sorpaas reopened this Oct 26, 2018
@sorpaas sorpaas added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 26, 2018
@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 26, 2018
@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. B1-patch-beta 🕷🕷 B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 26, 2018
@sorpaas sorpaas merged commit 7036ab2 into master Oct 26, 2018
@sorpaas sorpaas deleted the sp-eip155-zero-chainid branch October 26, 2018 14:46
ordian pushed a commit that referenced this pull request Oct 26, 2018
* Allow zero chain id in EIP155 signing process

* Rename test

* Fix test failure
ordian pushed a commit that referenced this pull request Oct 26, 2018
* Allow zero chain id in EIP155 signing process

* Rename test

* Fix test failure
5chdn added a commit that referenced this pull request Oct 28, 2018
* version: bump parity beta to 2.1.4

* ethcore: bump ropsten forkblock checkpoint (#9775)

* ethcore: handle vm exception when estimating gas (#9615)

* removed "rustup" & added new runner tag (#9731)

* removed "rustup" & added new runner tag

* exchanged tag "rust-windows" with "windows"

* revert windows tag change

* sync: retry different peer after empty subchain heads response (#9753)

* If no subchain heads then try a different peer

* Add log when useless chain head

* Restrict ChainHead useless peer to ancient blocks

* sync: replace `limit_reorg` with `block_set` condition

* update jsonrpc-core to a1b2bb742ce16d1168669ffb13ffe856e8131228

* Allow zero chain id in EIP155 signing process (#9792)

* Allow zero chain id in EIP155 signing process

* Rename test

* Fix test failure

* Insert dev account before unlocking (#9813)
5chdn added a commit that referenced this pull request Oct 28, 2018
* version: bump parity stable to 2.0.9

* ethcore: bump ropsten forkblock checkpoint (#9775)

* ethcore: handle vm exception when estimating gas (#9615)

* update jsonrpc-core to a1b2bb742ce16d1168669ffb13ffe856e8131228

* removed "rustup" & added new runner tag (#9731)

* removed "rustup" & added new runner tag

* exchanged tag "rust-windows" with "windows"

* revert windows tag change

* Allow zero chain id in EIP155 signing process (#9792)

* Allow zero chain id in EIP155 signing process

* Rename test

* Fix test failure

* Insert dev account before unlocking (#9813)
This was referenced Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants