lightning-browser-extension
lightning-browser-extension copied to clipboard
Add ability to decode signet
Describe the changes you have made in this PR
This PR begins the process of properly decoding signet payments with Alby as mentioned by #3213. The issue is Alby doesn't seem to have signet enabled anywhere that I could find in the app, and I'm not familiar enough with the codebase to know where to conditionally retrieve the network. The bolt11 library Alby is using to decode invoices takes an optional second parameter that allows specification of a custom network, i.e.:
const signet = {
bech32: "tbs",
pubKeyHash: 0x6f,
scriptHash: 0xc4,
validWitnessVersions: [0],
};
const paymentRequestDetails = lightningPayReq.decode(paymentRequest, signet);
Right now, the changes aren't very DRY and make payments only work with signet, so if someone more knowledgeable could show me how to grab the network dynamically and place the signet constant in the appropriate file, I would be happy to finish implementing.
Link this PR to an issue [optional]
Fixes #3213
Type of change
feat: New feature (non-breaking change which adds functionality)
Screenshots of the changes [optional]
Note that this is the signet Mutiny app
https://github.com/getAlby/lightning-browser-extension/assets/38222767/809f6042-fae3-458e-8d05-73075dfbb055
How has this been tested?
Checklist
- [x] Self-review of changed code
- [x] Manual testing
- [ ] Added automated tests where applicable
- [ ] Update Docs & Guides
- For UI-related changes
- [ ] Darkmode
- [ ] Responsive layout
@thebrandonlucas awesome, thanks for looking into this!
Actually, ideally the bolt11 package would actually support mutinynet. It already supports testnet/mainnet so it could also do this check internally. What do you think?
@rolznz So the library actually does support signets (see here), but only if an extra parameter is manually provided with the details of the custom network, as shown above. There are many different roads we could take with this to varying levels of complexity:
- Fork the bolt11 library (since it seems unmaintained), and modify the code to parse signet by default, then install this package from that fork or see if we can get it merged.
- For each current call to
lightningPayReq.decode(), we check if the network is equal tosignet. Not very DRY. - Create a wrapper function that checks if the network being used for the current account is
signet, if it is, parse with the custom parameters for mutinynet, if it isn't, parse without them. - Create a wrapper function that just attempts to parse, if it receives the
Unknown coin bech32 prefixerror, try to parse it again withmutinynetparameters. If it did receive an error but it wasn'tUnknown coin bech32 prefix, then it just re-throws.
I've gone with Option (4) as the DRYest solution that required the least refactoring, although it seems a bit suboptimal to be unnecessarily throwing/catching/rethrowing an error. I'm open to suggestions, but the way around this is for me to have a way to tell what the network is (preferably outside of a React component).
@rolznz I've updated the code and taking it out of draft. Let me know if there is anything else I need to do/fix.
Hi @thebrandonlucas
After looking at your changes, your first suggestion (forking the bolt11 library and using that until the PR gets merged) seems like the best option to me, as adding mutinynet there would be a very minor change and more consistent (it's the same checks as it already does for e.g. regtest). Then we just change the dependency in our package.json.
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
| Package | New capabilities | Transitives | Size | Publisher |
|---|---|---|---|---|
| npm/[email protected] | None | +3 |
1.97 MB | bslucas |
| npm/[email protected] | None | +3 |
98.1 kB | doowb |
🚮 Removed packages: npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎
This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.
After looking at your changes, your first suggestion (forking the bolt11 library and using that until the PR gets merged) seems like the best option to me, as adding mutinynet there would be a very minor change and more consistent (it's the same checks as it already does for e.g. regtest). Then we just change the dependency in our package.json.
@rolznz Done. I modified the bolt11 library to bolt11-signet, opened a PR to the original bolt11 library just in case that gets merged, and updated Alby's package to use it. Signet should now parse out-of-the-box now without any additional changes. Let me know if I need to do anything else. Cheers!
I tested this with https://signet-app.mutinywallet.com/ to create an invoice and NWC-next using mutinynet configuration connected to the Alby Extension to pay it - looks good!
@rolznz updated!
would be nice to get it into the official package. until then let's fix the version
@bumi Agree 100%. If you want to bump here perhaps we can get it in sooner: https://github.com/bitcoinjs/bolt11/pull/73.
Committed your suggestion in the meantime.
@thebrandonlucas thanks for making the PR there! -and here obviously :)
@thebrandonlucas thanks for making the PR there! -and here obviously :)
@bumi happy to! let me know if I can do anything else here
Thanks @thebrandonlucas !