Migrates ECKey to stricter API #139
Merged
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This pull request migrates the
ECKeyAPI to be stricter and force the user to be explicit about their intent instead of detecting their intent from the parameters.This is done through the following three new functions:
ECKey.fromBuffer,ECKey.fromWIF, andECKey.makeRandomThe default constructor
ECKey()accepts aBigIntegerand compression flag, and is what the other 3 constructors use to instantiate anECKey.A point of discussion is the change in behaviour by the constructor when dealing with keys outside of the
secp256k1keyspace.Previously, any key imported into
ECKeywould be automatically constrained/wrapped:This pull request removes that implicit behaviour and instead asserts that the input is already constrained to be within the key space.
This removes an unnecessary
modoperation, while also stopping confusion from users who might think that0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFis a valid private key.This behaviour still exists in
ECKey.makeRandomas it must constrain the randomly generated key to within these bounds.Although not necessary, I have maintained
toHex/fromHexconvenience wrappers, these exist until we can simplify down the test fixtures enough that there will not be a huge amount ofnew Buffer(..., 'hex')andtoBuffer().toString('hex')modifications just to maintain compatibility.This pull request also removes the incorrect behaviour of
ECKey.toBytes()which would append a0x01compression flag to the raw private key.This could lead to invalid private keys being imported when moving around between APIs (any sane implementation would fail, but you'd be surprised).
The
0x01compression flag is a feature of the Wallet Import Format (WIF) only, and has been isolated as such.This is why some of the
expectedPrivateKey's intest/hdwallet.jshave been altered (0x01removed).Conforms with #106 and partially resolves #113.