Add BOLT12 LSPS2 JIT receive support - #999
Conversation
|
👋 Thanks for assigning @jkczyz as a reviewer! |
fc05da7 to
badf902
Compare
f83b7ed to
f872ce4
Compare
| Ok(InvreqResponseInstructions::SendStaticInvoice { .. }) | Err(()) => return None, | ||
| }; | ||
|
|
||
| let allow_mpp = invoice_request.amount().is_some(); |
There was a problem hiding this comment.
Shouldn't this always be true for the first (non-LSP) attempt here?
There was a problem hiding this comment.
Ah, yes, it seems for the first build_invoice call we'll want always true. Will add a fixup.
| Some(pending_request) => pending_request, | ||
| None => { | ||
| return Some(( | ||
| OffersMessage::InvoiceError(InvoiceError::from_string( |
There was a problem hiding this comment.
Should we not consider duplicatively returning an existing LSPS2 cached negotiation here? In cases where the LSP over-allocated liquidity and we have enough for a second payment, or where we're getting DoS'd and most of the invreqs will not result in a payment, returning something might actually get us the payment.
FWIW if we do this it might be worth considering a marginally different router design, which might be cleaner - rather than deciding on which lease to use and then adding it to the metadata and using that metadata to tell the router what to inject, give the router a reference to the LSPS2 lease store and have it simply ask for the latest (potentially-unused) lease for a payment, then inject the metadata it wants.
There was a problem hiding this comment.
Hmm, in my mind both of these fall in the category 'further optimizations', i.e., I'd lean towards taking a new look at them in a follow-up rather than trying to land all of it in this initial implementation.
In particular I had considered doing something along the line of what you describe in the second paragraph to avoid transferring all of the parameters via the payment metadata, but for simplicity's sake (though nothing is really simple here) for now opted to keep it in the metadata for now as that makes it at least easier to reason about compared to introducing yet-another side channel that could race if we're not super careful.
There was a problem hiding this comment.
That's fine with me, I only raised it because it would likely imply a refactor of the router, so if we want to do that refactor ideally we should do it now rather than rewriting the code later. The feature itself would be nice but could be a followup.
f872ce4 to
621fd3d
Compare
|
|
||
| let (negotiated_lease, min_total_fee_msat, cheapest_lsp) = | ||
| self.negotiate_fixed_lease(amount_msat, connection_manager).await?; | ||
| let lease = self.consume_lease(&negotiated_lease.id).await?; |
There was a problem hiding this comment.
Won't this re-fetch through valid() and reject the lease we just bought if its validity is under 24h? Likewise in acquire_variable_lease.
| lease, | ||
| move |lease| async move { lease_store.remove(&lease.id).await }, | ||
| |lease| { | ||
| self.lease_state.lock().expect("lock").remove(&lease.id); |
There was a problem hiding this comment.
Can't two concurrent requests both select the same lease? Both callers would get the same single-use intercept SCID.
There was a problem hiding this comment.
Hmm, good catch. It seems this is a regression as originally we had a slightly different design there that would have made this impossible. Will fix.
| let _pending_request = pending_request; | ||
| let response = | ||
| lsps2_client.prepare_invoice_response(jit_request, connection_manager).await; | ||
| let (message, instructions) = match response { | ||
| Ok(JitInvoiceResponse { payment_metadata: jit_metadata, allow_mpp }) => { | ||
| debug_assert_eq!(allow_mpp, jit_request.allow_mpp()); | ||
| let mut merged_metadata = payment_metadata.unwrap_or_default(); | ||
| merged_metadata.extend(jit_metadata); | ||
| match build_invoice( | ||
| flow.as_ref(), | ||
| channel_manager.as_ref(), | ||
| keys_manager.as_ref(), | ||
| router.as_ref(), | ||
| secp_ctx.as_ref(), | ||
| &invoice_request, | ||
| Some(merged_metadata), | ||
| allow_mpp, | ||
| payment_info.as_ref(), | ||
| ) { | ||
| Ok((invoice, context)) => ( | ||
| OffersMessage::Invoice(invoice), | ||
| responder.respond_with_reply_path(context), | ||
| ), | ||
| Err(error) => ( | ||
| OffersMessage::InvoiceError(error.into_invoice_error()), | ||
| responder.respond(), | ||
| ), | ||
| } | ||
| }, |
There was a problem hiding this comment.
A payer may send many invoice requests (for robustness) but only pay one. Won't this cause us to acquire too many leases / is a DoS vector?
There was a problem hiding this comment.
Well, discussed this quite a bit with Matt before. Yes, it's not ideal, but we'll have to make sure to not reuse leases/intercept SCIDs as otherwise subsequent payments will not trigger channel opens. This is unfortunately a limitation of the bLIP-52/LSPS2 spec that we can't really work around. One follow-up we could consider is to add rate-limiting and more user control, e.g., via an event-based flow in LDK Node (also see the TODO mentioned in OP).
| } | ||
| } | ||
|
|
||
| fn build_invoice( |
There was a problem hiding this comment.
Ideally, LDK provides some abstraction so we don't need to repeat much of this logic.
There was a problem hiding this comment.
Yeah, possibly, but at least for v0.3 that ship has sailed I'm afraid.
|
I had Claude review this and it came up with quite a few issues. I already left comments for ones that I was able to verify. But there some others here: https://gist.github.com/jkczyz/3324c2914601a2728605a49294f25bc9. Failing tests in this branch: https://github.com/jkczyz/ldk-node/tree/claude/pr-999-edge-cases-725f70 |
b6c726d to
7d4f5be
Compare
Still have to go through the list fully. Some seem valid, some bogus. |
7d4f5be to
7b456df
Compare
Addressed most of the points now. @jkczyz please let me know what you think. |
7b456df to
03260f1
Compare
|
Rebased to address minor conflicts. |
27c34ef to
f56fa41
Compare
Make room for a dedicated offers message handler without mixing networking concerns into the public BOLT12 payment API. Co-Authored-By: HAL 9000
Persist a bounded LRU of fixed and variable payment requirements so restarts can pre-negotiate useful leases without making offer recognition depend on per-offer state. Prune expired targets to bound durable storage. Co-Authored-By: HAL 9000
Register bounded fixed and variable lease targets when ordinary offers may need just-in-time liquidity. Cache state remains advisory so offers stay valid and cold invoice requests can negotiate on demand. Co-Authored-By: HAL 9000
Move shared payment metadata out of the BOLT11 module. This lets other protocols reuse it without a BOLT11 dependency. Co-Authored-By: HAL 9000
Keep LSPS2 path construction in LDK Node and fall back to exact- amount JIT paths only when ordinary blinded paths are unavailable. This avoids unnecessary channel opens and unsafe MPP splits. Decode node-local single-use lease metadata directly in the router so the internal wrapper needs no pluggable metadata abstraction. Co-Authored-By: HAL 9000
Exercise aggregate MPP capacity and the per-channel non-MPP check so ordinary fallback behavior remains explicit. Co-Authored-By: HAL 9000
Install a node-local handler while preserving ChannelManager behavior. This creates an isolated integration point for asynchronous JIT replies. Co-Authored-By: HAL 9000
Feed every supported chain source into the node-local offers flow so invoice expiry and blinded-path CLTV limits use the current tip. Co-Authored-By: HAL 9000
Install the node-local LSPS2-aware router during node construction so BOLT12 responses can append negotiated JIT paths. Co-Authored-By: HAL 9000
Verify and answer ordinary invoice requests with the node-owned offers flow while retaining ChannelManager handling for all other messages. Co-Authored-By: HAL 9000
Answer verified JIT invoice requests after asynchronously acquiring a single-use LSPS2 lease. Send the result through the onion messenger so negotiation does not depend on the ChannelManager event queue. Co-Authored-By: HAL 9000
Limit concurrently pending JIT invoice requests so an onion-message storm cannot create an unbounded number of lease negotiations. Hold a permit while requests wait for an offer lock or an LSP response. Co-Authored-By: HAL 9000
Keep built-in service parameters usable beyond the client cache safety margin so freshly negotiated leases are not rejected due to timing. Co-Authored-By: HAL 9000
Read fee limits from BOLT12 payment context metadata and reject unsupported withholding. Record accepted fees on inbound offer payments. Co-Authored-By: HAL 9000
Inbound refunds can be rejected before settlement just like inbound offer payments, so updating their terminal status must not assert. Co-Authored-By: HAL 9000
Describe the amount received after an LSP fee and record the new payment-kind field in the pending release notes. Co-Authored-By: HAL 9000
Make same-amount and variable callers wait for one in-flight LSPS2 request, then recheck the shared cache before negotiating. Co-Authored-By: HAL 9000
Keep one usable lease ready after fixed or variable receive flows consume cached parameters. Foreground callers share the refill lock and reuse its result when it completes. Co-Authored-By: HAL 9000
A cached lease must not suppress refill after its LSP is removed from the configured set. Co-Authored-By: HAL 9000
After startup discovery, refill persisted fixed-amount and variable lease cache targets. Reuse valid leases and renegotiate only missing or stale entries so receiving can resume promptly after restart. Co-Authored-By: HAL 9000
Try the next eligible provider when a selected LSP rejects or times out, and retry bounded whole rounds for transient failures. Preserve immediate errors for fee limits and unavailable liquidity sources. Co-Authored-By: HAL 9000
Use resolved BOLT12 amounts to reject variable leases outside the payment range or total fee policy. Keep BOLT11 selection amountless until payment, while recording the exact BOLT12 fee limit. Co-Authored-By: HAL 9000
Treat the one-day margin as a cache policy while allowing freshly negotiated leases to cover only the invoice being built. Keep on-demand leases caller-owned and persist only leases intended for reuse. Co-Authored-By: HAL 9000
Callers need to know that long invoice expiries require an LSP offer whose validity covers the requested lifetime. Co-Authored-By: HAL 9000
Cover fixed and variable BOLT12 offers through a real LSPS2 service. Verify node-ID offer addressing, fee withholding, fresh JIT channels, and the variable-amount single-path behavior. Co-Authored-By: HAL 9000
Extend the order-independent fee-selection scenario through a real BOLT12 payment. Also prove that BOLT11 consumption and BOLT12 response handling share the same replenished lease pool. Co-Authored-By: HAL 9000
Keep the limited-fee multi-LSP case running through the BOLT12 invoice-request path before returning. Co-Authored-By: HAL 9000
Rebuild the receiver from its persisted store and pay the same long-lived offer again. Verify the pending offer survives and the exact cached lease is consumed before its replacement is negotiated. Co-Authored-By: HAL 9000
Pin the direct lease serialization format and ensure fixed-amount and variable-amount caches remain isolated. These focused checks complement the end-to-end BOLT11 and BOLT12 coverage. Co-Authored-By: HAL 9000
4a164f4 to
1b3e716
Compare
4 participants
Based on https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/4819.
This adds BOLT12 receive support over LSPS2 JIT channels using a node-local
OffersMessageHandlerandOffersMessageFlow.Offers retain ordinary blinded message paths and never contain an intercept SCID, allowing them to outlive individualPaymentLeases.Single-use
PaymentLeasesare persisted throughDataStore, while a bounded LRU persists fixed-amount and variableLeaseCacheTargets. Leases are restored and prefilled at startup, replenished after consumption, and periodically pruned when stale or close to expiry. Cache misses negotiate on demand with serialized requests, retries, and multi-LSP failover. BOLT11 uses the same cache.Every BOLT12 offer is eligible for JIT fallback when LSPS2 is configured. We first try ordinary payment paths and omit the JIT path entirely when existing inbound liquidity is sufficient. Otherwise, we consume or negotiate a lease and include a JIT path constrained to the full payment amount. Fixed invoices advertise MPP, variable invoices disable it, and
PaymentMetadataretains the fee policy needed to validate the LSP skim.Potential follow-ups: