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

replace secp256k1": "^4.0.2 with tiny-secp256k1": "^2.2.0 #54

Conversation

motorina0
Copy link
Member

This PR replaces secp256k1": "^4.0.2 with tiny-secp256k1": "^2.2.0
No functional changes made.

This should fix issues as the one reported here: #43
Also tiny-secp256k1 reduces the overall number of dependencies.

@junderw
Copy link
Member

junderw commented Jan 22, 2022

This is a bit grey... but even though WASM is supported in 92.87% of browser share, and all currently supported versions of NodeJS support it...

A large amount of popular tooling (React, Electron, React Native etc.) are still very nascent in WASM support...

bitcoinjs-lib itself made ecc stuff modular for that reason... but tbh I am not even sure if that was the right call.
It does make things more complicated to have everything become some factory that requires injecting the ecc library, most devs would expect npm install bitcoinjs-lib or npm install bolt11, or bip32 etc. will just work.

@motorina0
Copy link
Member Author

Both approaches have their merits and their downsides.

Included ECC lib

Advantages

  • install package and use it, no extra configs

Disadvantages

  • more advanced users might have their own version of the ECC lib that they want to use
  • new version updates of the ECC dependency might result in audit requests that slow down the upgrade to latest version of the lib

Modular ECC lib

Advantages

  • the user of the lib can provide its own ECC library which is already audited and works on its required environments
  • usage of redundant libraries can be avoided (some libs can require secp256k1 - v3, others secp256k1 - v4 and others tiny-secp256 - v2)

Disadvantages

  • a small wrapper must be created in order to match the TinySecp256k1 interface

Suggested Solution

Take the best of both approaches by using a tool like Lerna:

  • use one repository, but publish two packages. One for with the ECC included, one with modular ECC.
  • the bolt11 package can be shipped with tiny-secp256k1 lib included
  • the bolt11-light package can be modular
    • possible suffixes: *-light, *-small, *-xs, *-min, ...

Note that business logic changes only take place in the "light" package (no duplication). While the "full" package is a wrapper that injects the ECC lib and exports the same interface.
The lerna Fixed/Locked mode takes care that the versions are in sync.

Lerna

@bumi
Copy link

bumi commented Apr 27, 2022

I am currently hesitant with the wasm option - just because I had issues in my build pipeline with a dependency that used tiny-secp256k1 (but also did not want to deal with it now, seemed to me a bit too early; secp256k1 works for me)

@motorina0
Copy link
Member Author

Thanks for the feedback. Closing this PR (for now).

@motorina0 motorina0 closed this Apr 28, 2022
@bumi
Copy link

bumi commented Apr 28, 2022 via email

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.

3 participants