Bolt12 support #1727
Bolt12 support #1727
Conversation
lib/bolt12-info.js
Outdated
| return invoice.startsWith('lni1') || invoice.startsWith('lno1') | ||
| } | ||
|
|
||
| export function bolt12Info (bolt12) { |
There was a problem hiding this comment.
This is the bolt12 equivalent of the bolt11Tags helper.
Similar to the bolt11 counterpart, i've used a light parser just for the few tags we need in the ui, since the lndk parser needs access to the grpc endpoint.
I couldn't find an established pure js bolt12 parser, so i wrote a small tlv parser (@/lib/tlv) following the specs.
lib/lndk.js
Outdated
| } | ||
| } | ||
|
|
||
| const out = { |
There was a problem hiding this comment.
This function converts the output from lndk to an object with a layout compatible with the output from ln-service.
I've followed the doc so it should match the output very closely, but honestly i don't like the fact that ln-service returns so many values that mean the same thing with different types (eg, tokens, mtokens, safe_tokens...), i've implemented this conversion because it makes our code simpler with fewer changes, since we can handle both bolt11 and bolt12 invoices in the same way, but imo we should provide our own abstraction at some point, using only bigint msats and things that make more sense for our codebase.
There was a problem hiding this comment.
I agree, ln-service tries to make the grpc interface nicer, but it isn't as well documented as the grpc interface so it ends up just being confusing most of the time.
lib/lndk.js
Outdated
| response_invoice_timeout: timeout | ||
| }, async (error, response) => { | ||
| if (error) return reject(error) | ||
| const bech32invoice = 'lni1' + bech32b12.encode(Buffer.from(response.invoice_hex_str, 'hex')) |
There was a problem hiding this comment.
Bolt12 invoices are not supposed to be presented to the user, so the spec doesn't define an human readable representation, instead it is left to apps and nodes to handle the representation how they see more fit.
LNDK uses an hex string, while CLN use bech32.
For consistency with bolt11, i've decided to convert them to bech32 like in CLN.
There was a problem hiding this comment.
Tested normal and automated withdrawals and they work well!
But I noticed the followings things I mentioned in comments that I'd like to see discussed before approval:
-
lib/ was meant to be files that the server and client can import. Can we use dynamic imports so we don't need to split bolt files by server and client and can have just one file per bolt?
-
how did you test
max_fee? -
duplicate checks via
ifand schema -
function signature of
parseBolt12Request -
error handling via async callback
-
add similar function for bolt tags like for
payInvoice,parseInvoice,estimateFees? -
Showing BOLT12 invoices to the user: I think ideally they would always only see the BOLT12 offer. That's also what other wallets do. Is that possible in this PR or should we keep it for another PR?
-
mutating the
lndobject instead of passinglndkseparately in context -
LND custom options
I read in the LNDK docs that we need to run LND with custom options:
[protocol]
protocol.custom-message=513
protocol.custom-nodeann=39
protocol.custom-init=39
You've added these options in #1702.
Afaict, this means @huumn needs to configure LND differently in prod. You should have mentioned this since I think this wouldn't have worked as-is in prod. 👀
- other small stuff (see comments)
| export const card = { | ||
| title: 'Bolt12', | ||
| subtitle: 'receive payments to a bolt12 offer', | ||
| image: { src: '/wallets/bolt12.svg' } |
There was a problem hiding this comment.
nitpick: I am not a fan of this (unofficial?) logo. I would prefer to just use text for the protocols but no strong opinion. I let you or @huumn decide.
There was a problem hiding this comment.
The logo is nice. Given we plan to overhaul the UI/UX flow of this, I don't think it matter much.
prisma/migrations/20241212160430_bolt12_attachment/migration.sql
Outdated
Show resolved
Hide resolved
Co-authored-by: ekzyis <ek@stacker.news>
Co-authored-by: ekzyis <ek@stacker.news>
|
needs to be retested because there were a lot of conflicts and dubious merges |
should be ready for review |
Description
This PR adds support to pay, estimate fees and decode bolt12 invoices and a bolt12 wallet attachment.
The codebase uses the bolt11 nomenclature pretty much everywhere. After this PR, some areas will be generalized to also account for the possibility of a bolt12 invoice. However, to avoid weighing this PR down with unrelated, mostly graphical or naming changes, I’ve decided to leave those changes for a follow-up PR.
Closes #1317
Screenshots
Checklist
Are your changes backwards compatible? Please answer below:
yes
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
Did you introduce any new environment variables? If so, call them out explicitly here:
yes
LNDK_SOCKETLNDK_CERTandLNDK_MACAROON: can be the same values used for LND (see https://github.com/lndk-org/lndk/blob/master/docs/cli_commands.md )Testing
How to get a bolt12 offer string:
bash sndev cli eclair channelsshould return at least 1 channel marked as NORMALbash sndev cli eclair tipjarshowofferbash sndev cli eclair usablebalancesTests
Regression tests