Hacker News Reader

Comment by indutny | Hacker News Reader

Hey guys!

Still not considering to use https://github.com/indutny/elliptic for your EC operations? It seems like Bitcore has moved to it, and are quite fine with the results: http://blog.bitpay.com/2014/07/22/bitcore-3000-is-three-time...

View on HN

Lead developer of http://cryptocoinjs.com here. Also maintainer of https://github.com/cryptocoinjs/ecurve (what bitcoinjs-lib) uses for EC operations. I've worked with the lead developers of bitcoinjs-lib on ecurve so I think that I'd be qualified to answer this.

The main reason was that (1-2 months) ago when we were cleaning up ecurve, we had considered using your elliptic library, but we had problems with such simple operations in bn.js (your big integer library that elliptic depends upon) where arithmetic was incorrect for simple operations like -1 + 2 = -3 (don't quote me on that exact one). So at the time, we felt it wasn't battle tested. But they (we?) have every intention of switching to elliptic in the future.

I took a quick look at your implementation of ECDSA and I think it has a bug at line 311 [1]. It looks like I could bypass the check if r or s is negative.

One thing that I don't understand is why big integer libraries developed exclusively for crypto need negative numbers. The library [2] that I contribute to doesn't need them, and it works just fine. Actually I could argue that having only non-negative numbers make it simpler and faster.

[1] https://github.com/cryptocoinjs/ecdsa/blob/master/lib/ecdsa.... [2] https://code.google.com/p/end-to-end/source/browse/javascrip...

Its not really a bug, the operations after it would still be valid (it is almost immediately reduced to the field order), its just that those parameters would not be akin to the SEC paper specification. I agree that the honus isn't on the users to check that though, so I'm probably going to make a pull request to change this.[1]

[1] https://github.com/bitcoinjs/bitcoinjs-lib/pull/250

What might happen if r = s = -n? I think it's pure luck that this doesn't lead to a signature forgery.

You're not wrong.

Thanks for pointing this out, thankfully the implementation already failed on a negative s value, but you're correct in that it wasn't definitive.

I also whole-heartedly agree with your comment about the unnecessary inclusion of a bignum that allows for negative values. The lack of typing in this (and other cases) has lead to several problematic scenarios for users to the point we have littered the code with assertions to enforce whatever we can.

bn.js is just kind of sharpened for a elliptic.js usage. Though, the low footprint of the bn.js means that you could just use it for ECDSA operations, and use your own thing for the rest of the bitcoin stuff.

Thanks for a definite answer!

We're game to make any library changes. I did want to say though, that raw performance is not a high priority for BitcoinJS. We're far more interested in good testing and correctness than in performance (though performance certainly is a good thing).

Which is not to say that your library is incorrect or has bad testing, just to say that the way to our hearts is to convince us that elliptic is better than what we're using, not that it is faster, which it certainly is (and that's awesome). :)

Pull requests, of course, always welcome!

Bitpay does like your work. Will you sometimes contribute to one of the existing projects or prefer that they keep integrating your work? The former is sometimes more efficient, possibly more impactful. I don't think it will be great for community if there are too many JS bitcoin libs: 3 might be ok but IMO it's close to a reasonable practical limit.

Reply on news.ycombinator.com