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

add script to compute precomputed curves, elliptic.usePrecomputed, tests #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mvayngrib
Copy link

@mvayngrib mvayngrib commented Sep 9, 2019

the goal is to be able to sacrifice bundle size for performance by choosing to use a precomputed curve at runtime:

const elliptic = require('elliptic');
elliptic.usePrecomputed({
  // require only the ones you need
  p256: require('elliptic/lib/elliptic/precomputed/p256'),
});

also, the validation in the PresetCurve is expensive and unnecessary to do at runtime for pre-defined curves (i think but correct me if i'm wrong!)

this PR incorporates #188 , but i can adjust and rebase if that's merged first

this is kind of a big PR, but we noticed (at Exodus) that elliptic.ec('p256') takes ~1s in React Native on an iPhone Xs, and in that environment the tradeoff of a slightly bigger bundle as a result of bundling a precomputed curve is more than offset by the savings in computation time (~700ms savings)

relevant PR in Exodus: ExodusMovement#1

thanks @fanatid for help in figuring out how to do this

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 89.282% when pulling dae93e6 on ExodusMovement:mv/precomputed_1 into 71e4e8e on indutny:master.

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.

2 participants