Add static invoice creation utils to ChannelManager
#3408
Conversation
| const SECONDS_PER_BLOCK: u32 = 10 * 60; | ||
| let relative_expiry_blocks = relative_expiry_seconds / SECONDS_PER_BLOCK; | ||
| let max_cltv_expiry = core::cmp::max(relative_expiry_blocks, CLTV_FAR_FAR_AWAY) |
There was a problem hiding this comment.
Note for reviewers: I'm not sure this is robust enough with block times being somewhat unreliable. Maybe it should be buffered?
Prior to this patch, we would take() the invoice request stored for AwaitingInvoice outbound payments when retrying sending the invoice request onion message. This doesn't work for async payments because we need to keep the invoice request stored for inclusion in the payment onion. Therefore, clone it instead of take()ing it.
Prior to this fix, we would attempt to mark outbound async payments as abandoned but silently fail because they were in state AwaitingInvoice, which the mark_abandoned utility doesn't currently work for. These payments would eventually be removed by the remove_stale_payments method, but there would be a delay in generating the PaymentFailed event. Move to manually removing the outbound payment entry.
Useful for creating payment paths for static invoices which are typically amount-less.
Will be useful for static invoices' blinded paths, which may have long expiries. Rather than having a default max_cltv_expiry, we now base it on the invoice expiry.
This context is stored in the blinded payment paths we put in static invoices and is useful to authenticate payments over these paths to the recipient. We can't reuse Bolt12OfferContext for this because we don't have access to the invoice request fields at static invoice creation time.
This context is included in static invoice's blinded message paths, provided back to us in HeldHtlcAvailable onion messages for blinded path authentication. In future work, we will check if this context is valid and respond with a ReleaseHeldHtlc message to release the upstream payment if so. We also add creation methods for the hmac used for authenticating the blinded path using the static invoice's corresponding offer id.
We can't use our regular offer creation util for receiving async payments because the recipient can't be relied on to be online to service invoice_requests. Therefore, add a new offer creation util that is parameterized by blinded message paths to another node on the network that *is* always-online and can serve static invoices on behalf of the often-offline recipient. Also add a utility for creating static invoices corresponding to these offers. See new utils' docs and BOLTs PR 1149 for more info.
Since adding support for creating static invoices from ChannelManager, it's easier to test these failure cases that went untested when we added support for paying static invoices.
valentinewallace
force-pushed
the
2024-11-async-receive-offer-utils
branch
from
86cfb96
to
8feb3cb
Compare
| pub(crate) struct RetryableInvoiceRequest { | ||
| pub(crate) invoice_request: InvoiceRequest, | ||
| pub(crate) nonce: Nonce, | ||
| pub(super) needs_retry: bool, | ||
| } |
There was a problem hiding this comment.
Would it be worth not holding on to these for offers that don't support async payments? Can't recall if we can tell from the offer or if it isn't known until the static invoice is received.
There was a problem hiding this comment.
IIRC there is nothing in the offers, just at the invoice request level.
| const DEFAULT_AMT_MSAT: u64 = 100_000_000; | ||
| let amount_msats = amount_msats.unwrap_or(DEFAULT_AMT_MSAT); |
There was a problem hiding this comment.
Could you add a comment explaining this amount?
There was a problem hiding this comment.
It was chosen pretty arbitrarily lol. Input welcome on it. I'm guessing async receivers are mostly doing p2p payments and this amount is about $100 USD right now. I think the main constraint is that we don't want to exceed the recipient's channel capacity, and they may only have 1 channel with their LSP. Another option would be to not filter channels by capacity/{min,max}_htlc if no amount is provided.
| let amount_msat = offer.amount().and_then(|amount| { | ||
| match amount { | ||
| Amount::Bitcoin { amount_msats } => Some(amount_msats), | ||
| Amount::Currency { .. } => None | ||
| } | ||
| }); |
There was a problem hiding this comment.
Is quantity_max allowed for offers paying often-offline nodes? If so, is the amount checked by them in some way? If not, should we prevent building a static invoice from such offers?
There was a problem hiding this comment.
quantity_max is currently allowed. I'm not sure I follow your question though:
If so, is the amount checked by them in some way?
Who is "them"? It looks like the payer checks the amount/quantity when sending an invreq but not sure I'm following.
There was a problem hiding this comment.
Does the recipient check that the amount is sufficient for the requested quantity before releasing the preimage?
| for path in message_paths_to_always_online_node { | ||
| builder = builder.path(path); | ||
| } |
There was a problem hiding this comment.
If the recipient is online, does the InvoiceRequest get forwarded along? If so, how does the recipient authenticate it?
There was a problem hiding this comment.
If the recipient is online, does the InvoiceRequest get forwarded along?
Yep!
how does the recipient authenticate it?
Since the recipient issued the offer themselves, they authenticate it the same way as always-online recipients, i.e. via verify_using_recipient_data, if I'm understanding you.
There was a problem hiding this comment.
Right, so I guess that means the recipient must construct these paths since it needs to include the Nonce used to derive the signing keys.
There was a problem hiding this comment.
Hmm... but the function is creating and returning the Nonce. I think we'll need to pass it in?
There was a problem hiding this comment.
To be clear, we don't include metadata in the offer when we have blinded paths. Can't recall if we do something different when constructing an offer for use with async payments.
There was a problem hiding this comment.
I brought this up during review club. Since payment path contains the offer nonce -- and the sender will include the invoice request with the payment -- we should be able to verify the invoice request is authentic with this nonce. But ISTM we still want the same nonce in the offer's message paths for the normal case when the recipient is online.
Add static invoice creation utilities as part of supporting the async payments BOLTs spec Support async payments in BOLT 12 lightning/bolts#1149.
Take this opportunity to more easily test some code added in Support paying static invoices #3140 that went untested at the time. This is the bulk of the diff.
Address a piece of feedback from Support paying static invoices #3140 regarding
InvoiceRequests being unavailable when the time comes to send the async payment, cc Support paying static invoices #3140 (comment)