Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: replace bignumber.js with bn.js #406

Closed
wants to merge 1 commit into from
Closed

WIP: replace bignumber.js with bn.js #406

wants to merge 1 commit into from

Conversation

ryanxcharles
Copy link
Contributor

This is part of an experiment to replace bignumber.js+cryptojs with @indutny's bn.js+elliptic. Hopefully this will result in faster cryptography. I have run some trials on the speed of the mocha browser tests after this change:

Trial #1

fedor’s bignum library:
42.27s in firefox
37.14s in chrome

bignumber.js:
43.03s in firefox
39.09s in chrome

Trial #2

fedor’s bignum library:
42.38s in firefox
36.96s in chrome

bignumber.js:
43.32s in firefox
43.87s in chrome

...so the tests are not any faster or slower. However, I have not made any direct speed comparisons between the bignum libraries, and I have not yet changed the elliptic curve code. The slowest operation is point multiplication, so that will be the ultimate test.

This PR is labeled WIP because, although all tests pass and it all works as far as I can tell, it touches a whole bunch of code, so I want to review some more and write more tests before I consider it finished.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling 2fff555 on ryanxcharles:feature/bnjs into e485613 on bitpay:master.

@indutny
Copy link

indutny commented Jul 4, 2014

@ryanxcharles hm... so you just ran a test suite, right? Are you sure that it is the slowest point?

It appears to me that elliptic.js is one of the fastest libraries right now: http://jsperf.com/jsbn-vs-sjcl-ecc/16

@ryanxcharles
Copy link
Contributor Author

Yes, I just ran a test suite. No, I am not sure that is the slowest point, just the point that was easiest to test.

@ryanxcharles
Copy link
Contributor Author

I'm closing this PR in favor of a much more expansive PR, where I replace all bitcore crypto with @indutny's bn.js, elliptic, and hash.js: #409

micahriggan pushed a commit to micahriggan/bitcore that referenced this pull request Dec 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants