search

bitcoinjs / tiny-secp256k1

A tiny secp256k1 native/JS wrapper
MIT License
92 stars 55 forks source link

Split off native code #26

Closed dcousens closed 5 years ago

dcousens commented 6 years ago

Into a new module tiny-secp256k1-native, and have it as an optional dependency for this module. (with automatic pull-in if it exists) This should resolve a lot of the issues users are facing upstream in compiling it and/or isolating it from their projects.

And it supports better modularity, at the expense of merely defaulting to the JS implementation.

Thoughts? Breaking change.

junderw commented 6 years ago

I am not too sure... 99% of issues seem to be people not properly configuring their environments.

Could there be a way where we do this, but any time they NPM install it and it doesn't pull in the native log a warning:

ie.

WARNING: Could not load native bindings for tiny-secp256k1-native. Will use native JavaScript bindings instead. PLEASE BE AWARE OF THE IMPLICATIONS THIS HAS ON YOUR PROJECT.

If that would show up in the terminal when installing, I would concept ACK.

dcousens commented 6 years ago

@junderw I suppose we can simply do a post-install check to show that warning?

sapmathias commented 5 years ago

In my corporate build infrastructure git submodules are not allowed during build phase. How can I get around this with the current version?

I am actually using bitcoinjs-lib module, but tiny-secp256k1 module is failing when it hits gyp rebuild because of the git submodule.

dcousens commented 5 years ago

At first, maybe rename ecurve (defunct name anyway) to js so we can do:

... require('tiny-secp256k1/js')

For those that want only Javascript bindings (and document that fact as part of the API). And then tiny-secp256k1/native for native only.

A good first step no? Non breaking change IMHO as it was undocumented.

junderw commented 5 years ago

sounds good.

junderw commented 5 years ago

Done in #40