bitpay / bitcore-lib

A pure and powerful JavaScript Bitcoin library
https://bitcore.io/
Other
613 stars 1.03k forks source link

secp256k1-node instead elliptic #41

Open fanatid opened 8 years ago

fanatid commented 8 years ago

secp256k1-node uses bindings to bitcoin/secp256k1 in node.js and elliptic (right now) in browser as fallback, available with same interface. Since v3.0.0 was released, bitcore-lib could use this package for secp256k1 operations. Related: https://github.com/bitpay/bitcore/issues/1285 https://github.com/bitpay/bitcore/issues/1327

braydonf commented 8 years ago

Great news on the release of 3.0.0. Could be a huge improvement for performance.

fanatid commented 8 years ago

In node.js yes, much much faster. In browser I think it will be on 20-30% better. The problem is that this PR will be very large, requires lot of resources (majority for tests) and break API.

braydonf commented 8 years ago

I took a look at this to see what it would require to use secp256k1-node as an optional dependency in a way that would keep the existing API.

Started with using secp256k1-node only for signing and ran into a few issues with tests that are expecting the k value to be available. I was able to get the r and s values. I also noticed that the default signature is compact instead of DER, so signatureExport/Import was needed. Maybe the tests need to be update there for that case (tests covering an area that isn't needed).

I also took a look at using secp256k1-node for verifying only and ran into a few issues with existing tests failing, however there were only a few, and could likely be resolved.

braydonf commented 8 years ago

@fanatid Here is a minimal change set to add functionality to sign transactions with libsecp256k1: https://github.com/bitpay/bitcore-lib/pull/100/files

braydonf commented 8 years ago

Tests around deterministic k, and bindings for public key recovery are the only tests that needed to be skipped.