Add BIP21 Unified QR Code Support

slanesuke commented 3 months ago

Based on #182 This pull request introduces a payment submodule unifiedqr.rs that handles BIP21 URIs with support for BOLT11 invoices. Its goal is to facilitate the creation and payment of Unified QR codes with an additional lightning parameter.

Important features

Receive Method : Generates a BIP21 URI string with an on the address and BOLT11 invoice.
Send Method: Parses a URI string, attempts to pay a BOLT12 offer, BOLT11 invoice, then falls back to the on-chain address if the offer and invoice fail.
Tests: The parse_uri parses the URIs and verifies we're pulling the right values. The first URI was created from the receive method, but the others were created from zeus and muun. The unified_qr_send_and_receive test verifies the functionality of generating and making Unified QR payments. This includes handling BOLT12 offers, verifying payment types, and ensuring the correct payments between nodes. The generate_bip21_uri test creates a URI to visualize.

I'd appreciate some feedback on how things look as well as any critiques on the structure of the module or any errors I made

slanesuke commented 3 months ago

@tnull I read uppercasing characters in URIs make the qr code easier to scan. Should I format the URI so all characters are uppercase? Or maybe just the address and invoice? As of right now the URIs come out in this format with bitcoin in all caps: BITCOIN:TB1Q78S2PPDUEFLXEWDW5UAXHLMUED6UZJHXFP7DED?amount=0.01&message=Test%20message&lightning=lntb10m1pnxreqkdq523jhxapqd4jhxumpvajsnp4q2tev92txgkgjr0673w08dk033xwz8u2ks2thvpm5tw4kd2xn7apgpp5xefd0dw6da4r9ajfrs6dctf3xwd6agnfvat52s0vf627cnnzr5fssp5kmgq67kzm4rfmkpd2kmjzz60aw476zhsu4d46h5wjmfcqa28w3ls9qyysgqcqpcxqrraqqz74svlwzjpqna2tgq0497jfx0d28pm0fklp5rngdxgj6vqpkqyj8w6h2e7ck3mlsvd0jecnmjpz86zkr0yj2getjp6rzhc9ced72hcqucgzc3

tnull commented 3 months ago

@tnull I read uppercasing characters in URIs make the qr code easier to scan. Should I format the URI so all characters are uppercase? Or maybe just the address and invoice? As of right now the URIs come out in this format with bitcoin in all caps: BITCOIN:TB1Q78S2PPDUEFLXEWDW5UAXHLMUED6UZJHXFP7DED?amount=0.01&message=Test%20message&lightning=lntb10m1pnxreqkdq523jhxapqd4jhxumpvajsnp4q2tev92txgkgjr0673w08dk033xwz8u2ks2thvpm5tw4kd2xn7apgpp5xefd0dw6da4r9ajfrs6dctf3xwd6agnfvat52s0vf627cnnzr5fssp5kmgq67kzm4rfmkpd2kmjzz60aw476zhsu4d46h5wjmfcqa28w3ls9qyysgqcqpcxqrraqqz74svlwzjpqna2tgq0497jfx0d28pm0fklp5rngdxgj6vqpkqyj8w6h2e7ck3mlsvd0jecnmjpz86zkr0yj2getjp6rzhc9ced72hcqucgzc3

Yes, for the on-chain part bip21/rust-bitcoin already do the right thing if you use the {:#} formatter (see https://github.com/Kixunil/bip21/blob/88ac4516ccf19ccbf747a874f9725accac08fe17/src/ser.rs#L122-L143 / https://github.com/rust-bitcoin/rust-bitcoin/blob/45433095182e103430c33f07c9746f71e1b4fbcf/bitcoin/src/address/mod.rs#L146-L182), but for the rest we probably want to do it manually for now. Eventually we'll need to test that none of that interferes with wallet compatibility too much.

slanesuke commented 2 months ago

@tnull I added the ability to pay an offer in the send method. And since we aren't adding an offer when creating the URI I just concatenated an offer to an existing URI in the integration tests and it worked fine! Also, I had to determine whether the value after the lighting parameter was an invoice or an offer based on the prefix (lnbt, lntb, lno, etc) so thats a bit quirky but let me know what you think!

slanesuke commented 2 months ago

After some research, a BIP21 URI containing both an invoice and an offer should scan fine as a QR code. According to ISO/IEC 18004, which is the international standard for QR codes, a static QR code can store up to 3 KB of data (4,296 alphanumeric characters). This should easily accommodate our URI size.

So, our BIP21 QR code would be scannable by most modern smartphones, including older iPhones and Androids, if its printed with fair resolution and within a minimum physical dimension of 2.5x2.5cm (which is larger than all wallets I've checked). Plus, error correction levels in QR codes ensure reliability, even if the QR code is damaged

slanesuke commented 2 months ago

@tnull ready for review!

tnull commented 2 months ago

Btw, feel free to rebase on main since we just fixed the broken CI (mod flaky jobs, of course ..)

slanesuke commented 1 month ago

rebased

slanesuke commented 1 month ago

One last nit, but otherwise should be good to go.

Should I squash down to one commit?

lightningdevkit / ldk-node

Add BIP21 Unified QR Code Support #302