Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix: enforce eip-2 imposed limits and secp256k1 upper bound for private keys #2944

Merged
merged 1 commit into from
May 3, 2022

Conversation

davidmurdoch
Copy link
Member

@davidmurdoch davidmurdoch commented Apr 20, 2022

Sending transactions from an impersonated account with a "large" account number, like fffffffffffffffffffffffffffffffffffffffe, would result in the error "The nonce generation function failed, or the private key was invalid" due to the way we fake transaction signing in ganache. Previously we would take the account number plus the first 12 bytes of the account number, fffffffffffffffffffffffffffffffffffffffe + ffffffffffffffffffffffff, and would use that as a fake private key. This results in an invalid key, as secp256k1, the elliptic curve used in Ethereum cryptography, has an effective maximum private key value of 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364140n (AKA secp256k1_n - 1, or the total number of non-trivial points on the curve). This fixes #2586.

While implementing this fix it was discovered that we were not rejecting transactions with too-high s-values; i.e., s-values that are greater than (secp256k1_n - 1) / 2. This restriction was added way back in Ethereum's first hardfork, homestead, as part of EIP-2 in order to remove the possibility of "malleable" transactions. While somewhat unrelated to the core reason for this fix, it has been added as part of this PR. This fixes #2600.

@davidmurdoch davidmurdoch changed the title fix: enforce eip-2 fix: enforce eip-2 imposed limits and secp256k1 upper bound for private keys Apr 21, 2022
@davidmurdoch davidmurdoch force-pushed the develop branch 5 times, most recently from 892c68f to 0e9642f Compare April 22, 2022 19:52
@davidmurdoch davidmurdoch force-pushed the fix/eip-2 branch 2 times, most recently from 35a2c11 to 057a22c Compare April 28, 2022 20:38
@davidmurdoch davidmurdoch marked this pull request as ready for review April 29, 2022 21:25
@MicaiahReid
Copy link
Contributor

Reviewed! This also needs a description that is release ready.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants