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

EIP-155 update with Vitalik's new test vectors #3166

Merged
merged 5 commits into from
Nov 4, 2016
Merged

Conversation

gavofyork
Copy link
Contributor

No description provided.

@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. B0-patch labels Nov 4, 2016
@gavofyork gavofyork changed the title Vitalik's new test vectors. EIP-155 update with Vitalik's new test vectors Nov 4, 2016
@gavofyork gavofyork added the P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible. label Nov 4, 2016
@arkpar
Copy link
Collaborator

arkpar commented Nov 4, 2016

A few state and transaction test failing in travis build

@arkpar arkpar added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 4, 2016
@@ -302,13 +302,13 @@ impl SignedTransaction {
}

/// 0 if `v` would have been 27 under "Electrum" notation, 1 if 28 or 4 if invalid.
pub fn standard_v(&self) -> u8 { match self.v { 0 => 4, v => (v - 1) & 1, } }
pub fn standard_v(&self) -> u8 { match self.v { v if v == 27 || v == 28 || v > 36 => (v - 37) % 2, _ => 4 } }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like v - 37 will overflow. Should just return 0 if v == 27 or 28

@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Nov 4, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 4, 2016
@arkpar arkpar merged commit 0f16942 into master Nov 4, 2016
arkpar pushed a commit that referenced this pull request Nov 4, 2016
* Vitalik's new test vectors.

* Update to latest EIP155 spec.

* Fix txs.

* Another fix.
arkpar pushed a commit that referenced this pull request Nov 4, 2016
* Vitalik's new test vectors.

* Update to latest EIP155 spec.

* Fix txs.

* Another fix.
@arkpar arkpar deleted the new_tx_test_vectors branch November 4, 2016 14:25
arkpar added a commit that referenced this pull request Nov 4, 2016
* Vitalik's new test vectors.

* Update to latest EIP155 spec.

* Fix txs.

* Another fix.
arkpar added a commit that referenced this pull request Nov 4, 2016
* Vitalik's new test vectors.

* Update to latest EIP155 spec.

* Fix txs.

* Another fix.
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. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants