Skip to content
This repository was archived by the owner on Apr 3, 2019. It is now read-only.

secp256k1-node instead elliptic #41

Open
fanatid opened this issue Feb 2, 2016 · 5 comments
Open

secp256k1-node instead elliptic #41

fanatid opened this issue Feb 2, 2016 · 5 comments

Comments

@fanatid
Copy link
Contributor

fanatid commented Feb 2, 2016

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: bitpay/bitcore#1285 bitpay/bitcore#1327

@braydonf
Copy link
Contributor

braydonf commented Feb 2, 2016

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

@fanatid
Copy link
Contributor Author

fanatid commented Feb 2, 2016

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
Copy link
Contributor

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 braydonf added this to the 1.0.0 milestone Sep 12, 2016
@braydonf
Copy link
Contributor

braydonf commented Sep 13, 2016

@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
Copy link
Contributor

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

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

No branches or pull requests

2 participants