bitcoinjs / bolt11

A library for encoding and decoding lightning network payment requests as defined in BOLT #11.
MIT License
92 stars 64 forks source link

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

Closed motorina0 closed 2 years ago

motorina0 commented 2 years ago

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: https://github.com/bitcoinjs/bolt11/issues/43 Also tiny-secp256k1 reduces the overall number of dependencies.

junderw commented 2 years ago

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 commented 2 years ago

Both approaches have their merits and their downsides.

Included ECC lib

Advantages

Modular ECC lib

Advantages

Disadvantages

Suggested Solution

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

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 commented 2 years ago

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 commented 2 years ago

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

bumi commented 2 years ago

I don’t think you should close it. (just wanted to share my current experience here)

On 28. Apr 2022, at 06:37, Vlad Stan @.***> wrote:

 Closed #54.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.