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

implement fallback from built-in scrypt to scryptsy/scrypt packages #2952

Merged
merged 5 commits into from
Jul 18, 2019

Conversation

michaelsbradleyjr
Copy link
Contributor

This commit introduces an additional fallback behavior and console warning in the case that Node's built-in scrypt experiences memory exhaustion, as was discovered in the 1.x test suite with the static tests (see #2938). A large n parameter is the culprit, but that's not the case when using the 3rd party (deprecated) scrypt package. The scryptsy package can handle a large n, but performance does suffer considerably.

This commit introduces an additional fallback behavior and console warning in
the case that Node's built-in scrypt experiences memory exhaustion, as was
discovered in the 1.x test suite with the static tests (see web3#2938). A large `n`
parameter is the culprit, but that's not the case when using the 3rd
party (deprecated) scrypt package. The `scryptsy` package can handle a large
`n`, but performance does suffer considerably.
@coveralls
Copy link

coveralls commented Jul 17, 2019

Coverage Status

Coverage decreased (-0.3%) to 95.3% when pulling ae9dd29 on michaelsbradleyjr:refactor/scrypt into 20689fe on ethereum:2.x.

@nivida nivida added 2.x 2.0 related issues Bug Addressing a bug In Progress Currently being worked on labels Jul 17, 2019
@nivida nivida removed the In Progress Currently being worked on label Jul 17, 2019
@michaelsbradleyjr
Copy link
Contributor Author

See also: ethereumjs/ethereumjs-wallet#91 (comment).

@nivida nivida merged commit 04bbf4a into web3:2.x Jul 18, 2019
@nivida nivida mentioned this pull request Aug 5, 2019
11 tasks
@Astrantia
Copy link

Astrantia commented Aug 13, 2019

@michaelsbradleyjr @nivida This PR has resulted into this compilation warning

[ warn ]  ./node_modules/web3-eth-accounts/dist/web3-eth-accounts.umd.js
Module not found: Can't resolve 'scrypt' in 'C:/App/node_modules/web3-eth-accounts/dist'

Can this PR be reverted, or this bug fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x 2.0 related issues Bug Addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants