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

Comparison with... #1

Open
dcousens opened this issue Nov 27, 2018 · 7 comments
Open

Comparison with... #1

dcousens opened this issue Nov 27, 2018 · 7 comments
Labels
question Further information is requested

Comments

@dcousens
Copy link
Member

dcousens commented Nov 27, 2018

Another day, another scrypt module.

cryptocoinjs/scrypt#1

Now there is

@fanatid @barrysteyn @calvinmetcalf @jprichardson

How did this happen again?

@dcousens dcousens added the question Further information is requested label Nov 27, 2018
@calvinmetcalf
Copy link
Contributor

calvinmetcalf commented Nov 28, 2018 via email

@calvinmetcalf
Copy link
Contributor

so to unpack it a bit since this would be installed by default in browserify and webpack anything with a build step would be right out because that would get triggered every time those packages are installed, this right off the bat eliminates node-scrypt and scrypt from being useful.

scryptsy could be used but it has the progresscallback stuff which doesn't fit into the node api and would be deadweight and doesn't let us use async pbkdf2 (which leverages browser native crypto) since it always uses sync crypto. Lastly scryptsy calls in the whole crypto module which is fine if it's something you're using in node, but in the browser that would include in all the other parts of crypto preventing you from requiring it in on its own, also it could make a circular dependency issue.

@dcousens
Copy link
Member Author

I don't think any of those issues are unresolvable, I think we could have trivially exposed a Node-like interface by default, and then the progressCallback stuff as a separate export?

@calvinmetcalf
Copy link
Contributor

ideally we'd want https://github.com/cryptocoinjs/scryptsy/blob/14fa57ff829479f28de02eb430dcf6474e4fffcb/lib/scrypt.js#L43-L45 available as a separate export without the progress callback stuff

@jprichardson
Copy link
Member

I'm open to any modifications. Creating new libraries that do the same thing makes it harder for people to audit libraries as the audit surface area increases. Increased audit surface area makes it harder to prevent the problems that we saw on Monday (backdoor targetting Copay).

@jprichardson
Copy link
Member

@calvinmetcalf happy to add you to cryptocoinjs if you'd like.

@calvinmetcalf
Copy link
Contributor

sure you can add me, I'll try switching it up when I get a chance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants