v3.0.0

[fanatid]

Continuance of #55

---

API changes:

• remove async functions
• add compressed flag to functions that gives public key as result
• add options (noncefn, nonce function data) to .sign
• rename secret key to private key

bitcoin/secp256k1 updated to c18b869

JavaScript implementation:

• remove all dependencies except bn.js
• better performance (see benchmark results below)
• minified size -- 112kb, minified and gziped -- 27kb (instead 518kb and 134kb for v2.0.7)

---

v3.0.0-alpha.0 (dc2a0ef) benchmark (with improved bn.js):

$ node benchmark/benchmark.js 
Set seed: 509fe4299a45e24309b803996f0c0a157e63194e297cc89753fdbeff9a8eec75
100% (1000/1000), 3.1s elapsed, eta 0.0s
Create 1000 fixtures
++++++++++++++++++++++++++++++++++++++++++++++++++
Benchmarking: publicKeyCreate
--------------------------------------------------
bindings x 14,021 ops/sec ±0.32% (89 runs sampled)
secp256k1js x 844 ops/sec ±1.30% (87 runs sampled)
elliptic x 841 ops/sec ±1.05% (88 runs sampled)
ecdsa x 25.39 ops/sec ±0.54% (44 runs sampled)
==================================================
Benchmarking: sign
--------------------------------------------------
bindings x 8,165 ops/sec ±0.31% (90 runs sampled)
secp256k1js x 631 ops/sec ±0.99% (86 runs sampled)
elliptic x 609 ops/sec ±1.29% (86 runs sampled)
ecdsa x 24.56 ops/sec ±0.93% (43 runs sampled)
==================================================
Benchmarking: verify
--------------------------------------------------
bindings x 5,349 ops/sec ±0.21% (91 runs sampled)
secp256k1js x 181 ops/sec ±2.18% (77 runs sampled)
elliptic x 139 ops/sec ±1.93% (73 runs sampled)
ecdsa x 8.59 ops/sec ±0.63% (25 runs sampled)
==================================================

v2.0.7 (9df019c benchmark:

$ node benchmark/benchmark.js 
Benchmarking: sign
--------------------------------------------------
bindings x 8,161 ops/sec ±0.26% (87 runs sampled)
secp256k1js (elliptic) x 494 ops/sec ±7.21% (85 runs sampled)
ecdsa x 24.83 ops/sec ±1.12% (43 runs sampled)
==================================================
Benchmarking: verify
--------------------------------------------------
bindings x 5,457 ops/sec ±0.08% (91 runs sampled)
secp256k1js (elliptic) x 105 ops/sec ±22.72% (71 runs sampled)
ecdsa x 8.38 ops/sec ±1.39% (25 runs sampled)
==================================================

---

Now version is tagged as v3.0.0-alpha.0, when bitcoin-core/secp256k1#352 (ecdh interface) and indutny/bn.js#94 (improve performance) will be resolved -- v3.0.0 can be released.

Special thanks to @indutny for awesome libraries: bn.js, elliptic and hash.js!

[indutny]

My goodness! The numbers look awesome!

Any chance these changes could be contributed back to the elliptic?

[fanatid]

@indutny elliptic performance almost indistinguishable from secp256k1-node js
.verify here is better only because mul-and-add working without endomorphism
some number for elliptic with removed endo functions (bn.js with indutny/bn.js#94):

Benchmarking: verify
--------------------------------------------------
secp256k1js x 183 ops/sec ±1.20% (79 runs sampled)
elliptic x 180 ops/sec ±0.94% (78 runs sampled)

[indutny]

@fanatid what's the point of embedding it with changes then?

[fanatid]

There are two goal: code size (it's very important for browsers) and easiest code review (only secp256k1, nothing more).

[wanderer]

@fanatid this looks great. The only thing I'm not convinced of is embedding elliptic and hashjs. I don't know of code size gains can justify it. The downside is future maintainability. Its will be hard to track changes and bug fixes in hashjs and elliptic manually.

Maybe there is middle ground here. We could breakup hash.js into minimodules, sha256.js , ripemd160, ect.. and do the same to elliptic. Then just include the modules that we need. @indutny what do you think about further modularizing hash.js and elliptic?

[indutny]

I'm fine with modularizing things.

[fanatid]

I'm also not happy with "embedding", but for me, 2 points that I mentioned above is reasonable. I think we don't need modularizing elliptic or hash.js, we can just include elliptic version by default and in that moment give users ability using version without dependencies with next construction: require('secp256k1-node/js').
In any case if we want to use elliptic in this project, elliptic should support options for sign (noncefn and data).
@wanderer I don't think that this version or elliptic (only secp256k1 was checked) contains bugs (can't say for endomorphism, because don't check deeply, but think all fine here). I'm mostly concerned about bn.js.

[fanatid]

I mark this PR as WIP. Please let me time to make PR to elliptic and write code for using elliptic in this project (not think that will be fast, because I will busy in nearest time).

[axic]

@fanatid wouldn't it make sense applying the non-breaking changes from this patch set to 2.x?

[fanatid]

@axic I'm already made this in #60

[axic]

@fanatid I think there a couple more which are non breaking, for example 5061632, 24f9d2a, some of 13e63ca, etc.

[fanatid]

It's look reasonable, but my main goal right now is finish v3.0.0
I don't mind if somebody move non-conflict commits to separate PR for v2.x

[fanatid]

Elliptic was added as fallback, waiting indutny/elliptic#71, indutny/bn.js#97, indutny/bn.js#98, indutny/bn.js#99, indutny/bn.js#100

[fanatid]

@wanderer @Kagami What do you think should be by default? js.js or elliptic ?

[wanderer]

I don't really care that much. I prefer using elliptic because it may be more up to date. But I happy as long as it's an option.

[Kagami]

I prefer using elliptic because it may be more up to date

Agreed. Upstream version IMO should be prioritized.

[axic]

@fanatid in the elliptic fallback code you can use BN.toBuffer() now, given the other PR is merged that will also be one less copy

[fanatid]

@wanderer should be fixed now, it was bug related with BN, carry in multiplication on number: 8d26e3a#diff-f0b4a946089e134cf321d6e452ff27a9L319
I will add this test vector and previous later.

[fanatid]

Use toBuffer now, thanks @axic !

[axic]

@fanatid is there a way more of your code to be merged back into bn/elliptic?

When creating the benchmarks, did you just use stock elliptic or an updated version relying on a newer bn.js release? As the current elliptic release uses an ancient bn.js version.

[fanatid]

Oh, I even didn't know that elliptic uses bn.js^4.0.0. Looks that I should return to using toArray :(
I didn't think that we should merge this while elliptic uses ancient bn.js, @indutny thoughts?

About benchmarks: stock elliptic.

[wanderer]

@fanatid I don't think the version is the problem. All the tests passing including pure js. The only problem is with in the browser

[wanderer]

ah i think the problem is here https://github.com/indutny/bn.js/blob/master/package.json#L34

[wanderer]

PR indutny/bn.js/pull/115 fixes the browser problems

[wanderer]

@fanatid what do you think about rolling back to 8d26e3a and releasing? It doesn't look like browserify problem will get solved fast.

[fanatid]

@wanderer I don't want release with elliptic that uses bn.js^4.0.0, it shouldn't take much time for update bn.js in elliptic, I'll try today.
Agree about 8d26e3a, but look at indutny/bn.js#114, we can use toArrayLike(Buffer instead toBuffer.

EDIT: I was totally confused in packages versions. @wanderer let's wait toArrayLike merge and make release :)

[fanatid]

Switching to elliptic, at least embedded version doesn't has tests for edge cases

[wanderer]

@fanatid ok sounds good

[fanatid]

@wanderer updated!

[axic]

@fanatid not only toArrayLike was added, but toBuffer should be fixed too in the same bn.js release

[fanatid]

@axic I tried, but got assertion error from bn.js

[wanderer]

lgtm, awesome work @fanatid !

[fanatid]

Thanks to all 🍰

[axic]

@axic I tried, but got assertion error from bn.js

Hmm, that shouldn't happen. Let me check.

In any case toArrayLike became a public API so it is safe to use :)