RFC for PoC of bolt12 contacts formally known as a BLIP 42 #4210
Conversation
…et, invreq_payer_offer, invreq_payer_bip_353_name Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Implements BIP 353 human-readable contact addresses and bLIP 42 contact secret derivation for mutual authentication in Lightning Network payments. The implementation supports both offers with issuer_signing_pubkey and offers using blinded paths for privacy-preserving contact management. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
…for offer Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Implements a public accessor method to retrieve the payer_offer field from invoice requests. This completes the interface for accessing the invreq_payer_offer experimental TLV field that was added in commit 61799bf. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Implements automatic injection of the payer's offer into invoice requests to support BLIP-42 contact management. This allows recipients to identify which specific offer is being paid, enabling better contact tracking and payment relationship management. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
…ment This commit implements the infrastructure to expose BLIP-42 contact information through the PaymentSent event, allowing applications to manage contact relationships when BOLT12 offer payments complete.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
@TheBlueMatt the status of this PR is kind doing everything that the specs specifieds, but I am taking some opinionate decision like on the offer to inject inside the invoice_request (that I am calling in my mental model
|
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
A handful of high-level comments. Apologies if there's something I missed cause I don't recall exactly how all the bLIP 42 stuff was supposed to work.
| }; | ||
|
|
||
| let contacts = match contacts { | ||
| None => ContactSecrets::new(self.entropy_source.get_secure_random_bytes()), |
There was a problem hiding this comment.
My recollection of the intended UX is that the wallet would have a "save recipient to contacts" checkbox and only if that is checked would we include contact information and refund offer. If our refund offer had perfect privacy, maybe it'd make sense to include it always, but as-is including it is a somewhat annoying privacy leak as the sender.
| /// [`Event::PaymentFailed`]). | ||
| pub retry_strategy: Retry, | ||
| /// Contact secrets to include in the invoice request for BLIP-42 contact management. | ||
| /// If provided, these secrets will be used to establish a contact relationship with the recipient. |
There was a problem hiding this comment.
This needs to be substantially more filled-out, including information about intended UX and UI integration logic, how/when to derive secrets, etc.
| let builder = builder.contact_secrets(contacts.clone()); | ||
| // Create a minimal offer for BLIP-42 contact exchange (just node_id, no description/paths) | ||
| // TODO: Create a better minimal offer with a single blinded path hop for privacy, | ||
| // while keeping the size small enough to fit in the onion packet. |
There was a problem hiding this comment.
Luckily the onion packet is 64K so we're not too worried about the size. Indeed, as you note in your questions, we should probably try to figure out if the dev probably wants us to use an async offer (based on async availability/config) and use one of those if we can.
There was a problem hiding this comment.
I will add a complete answer later but my message was refering to the current CI error https://github.com/lightningdevkit/rust-lightning/actions/runs/19137908502/job/54694666815?pr=4210#step:9:6040 that probably an offer with two blinded path is too big?
node 0 ERROR [lightning::ln::outbound_payment:1486] Can't construct an onion packet without exceeding 1300-byte onion hop_data length for payment with id 0101010101010101010101010101010101010101010101010101010101010101 and hash 74b87079b704f2162403361478f1bca041556cd1103e9882b394d5e4bb379c41
node 0 TRACE [lightning::ln::channelmanager:15385] Failed paying invoice: OnionPacketSizeExceeded
node 0 TRACE [lightning::onion_message::messenger:1538] Constructing onion message when responding with Invoice Error to an onion message: InvoiceError { erroneous_field: None, message: UntrustedString("OnionPacketSizeExceeded") }
node 0 TRACE [lightning::onion_message::messenger:1575] Buffered onion message when responding with Invoice Error to an onion message
thread 'ln::offers_tests::rejects_keysend_to_non_static_invoice_path' (4319) panicked at lightning/src/ln/functional_test_utils.rs:1283:9:
assertion `left == right` failed: expected 1 monitors to be added, not 0
left: 0
right: 1
async offer (based on async availability/config)
Oh good point will look into it next!
| // Create a minimal offer for BLIP-42 contact exchange (just node_id, no description/paths) | ||
| // TODO: Create a better minimal offer with a single blinded path hop for privacy, | ||
| // while keeping the size small enough to fit in the onion packet. | ||
| let payer_offer = self.create_offer_builder()?.build()?; |
There was a problem hiding this comment.
If we do keep always passing an offer instead of doing it conditionally, we should probably consider including (encrypted) data in the offer so that we can tie return payments back to the offer we paid (incl, eg, the BIP 353 we used to look up the name to begin with).
| let invoice = PaidBolt12Invoice::Bolt12Invoice(invoice.clone()); | ||
| self.send_payment_for_bolt12_invoice_internal( | ||
| payment_id, payment_hash, None, None, invoice, route_params, retry_strategy, false, router, | ||
| payment_id, payment_hash, None, invoice_request.as_ref(), invoice, route_params, retry_strategy, false, router, |
There was a problem hiding this comment.
This isn't correct - we'll as a side-effect of this include the full invoice request in the payment onion, which we should only be doing in async payments. It also doesn't seem worth storing the full InvoiceRequest in PendingOutboundPayment::InvoiceReceived just to extract a few fields out of it. Tho I also don't really see a reason why we need to include the payer_offer in Event::PaymentSent at all. I can see why we might want to include the contact secret (so that it can be stored with the sent payment info, I guess?), but if we move to requiring explicit contact secrets and not auto-generating them it might not be worth it either.
| /// | ||
| /// Since the second case should be very infrequent, it's more likely that the remote node | ||
| /// is malicious and we shouldn't store them in our contacts list. | ||
| pub fn verify(&self, offer: &Offer) -> bool { |
There was a problem hiding this comment.
nit: verify methods should never return a bool, they should Result<(), ()>.
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub struct ContactSecrets { | ||
| primary_secret: [u8; 32], | ||
| additional_remote_secrets: Vec<[u8; 32]>, |
There was a problem hiding this comment.
Should this be a single Option? I'm a bit unclear on what UX would result in there being more than one.
There was a problem hiding this comment.
We should have an additional one for when the country party will not remember us, but use somehow the same offer, so we will use this vector to store additional secrets that the other side sends to us. IIRC this should be the logic
| } | ||
| } | ||
|
|
||
| /// We derive our contact secret deterministically based on our offer and our contact's offer. |
There was a problem hiding this comment.
I'm a bit unclear on this usecase here - using our_private_key (I guess it implies the node_id?) means that it won't match what we put in offers (we should never be reusing the node_id as an offer identity, IMO), but in general we should be using a fresh identity for every issued offer, so its not clear to me how often this will work. Also, we don't currently use it, just expose it as a freestanding function. If we want to support this derivation IMO we should integrate it somehow into the API.
| /// - our contact added us without using the contact_secret we initially sent them | ||
| /// - our contact is using a different wallet from the one(s) we have already stored |
There was a problem hiding this comment.
This doesn't tell me, a wallet developer, what to do here - am I supposed to have some kind of UI that says "attribute this payment to an outbound payment" (hopefully not no one is gonna build that)? Or is there some way of automatically detecting that (maybe based on validated 353?), in which case we should automate that somehow, presumably?
| // FIXME: this can be simply a function call? | ||
| impl UnverifiedContactAddress { | ||
| /// Creates a new [`UnverifiedContactAddress`]. | ||
| pub fn new(address: ContactAddress, expected_offer_signing_key: PublicKey) -> Self { |
There was a problem hiding this comment.
There's a lot of stuff here that could really use examples discussing how to use it in an overall flow but also actual sample code. From reading this API its really not clear to me when/how I'd use this - the type doesn't exist outside of a freestanding struct.
3 participants
With this PR I am proposing the first implementation for the BLIP 42 implementation that allow to send a "contact" with a contact secret for verification when making an invoice request during a
pay_for_offer.The current implementation is injecting the BLIP 42 information by default and there is no way to op out to this feature (and IMHO would be could to have a way to disable it).
In addition this RFC it is just a way to collect the first comments on API and design choice that I made and to collect feedback on how I manage stuff on the ldk internal stuff, probably there is some more simple way of doing the same thing.
However, this PR has already two problem: