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
Prepare the LSPS2 client for dedicated state and storage modules. Co-Authored-By: HAL 9000
Track negotiated parameters as expiring leases so intercept identifiers cannot be reused across payments. Co-Authored-By: HAL 9000
Load cached leases in parallel at startup and remove entries that are expired or too close to expiry for safe reuse. Co-Authored-By: HAL 9000
Remove unusable leases hourly so persisted cache data cannot grow without bound. Co-Authored-By: HAL 9000
Persist negotiated parameters before making them available and consume them before constructing existing BOLT11 invoices. Co-Authored-By: HAL 9000
Move LSPS2 fee policy out of individual BOLT11 receive calls and into node configuration. A shared total limit gives every receive flow one policy across all configured liquidity sources. 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
Reuse one cached-or-negotiated lease path across invoice formats. Keep BOLT11 selection and fee-limit behavior unchanged. Co-Authored-By: HAL 9000
Reserve each cached lease while selecting it so concurrent receive requests cannot advertise the same intercept SCID. Restore the lease when durable removal fails because no invoice has used it yet. 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
Keep MPP enabled while probing ordinary payment paths, including for variable-amount offers. Only apply the fixed-versus-variable policy when an LSPS2 JIT path is included. Co-Authored-By: HAL 9000
Validate the LSP-provided CLTV delta before storing a negotiated lease and exclude incompatible restored leases from selection. This prevents BOLT12 from consuming a lease before rejecting it and avoids truncating the value in BOLT11 route hints. 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
Track the hashes of signed JIT invoice requests while their response futures are pending. This prevents a replay from negotiating another single-use lease without persisting replay state beyond the short-lived request flow. 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
Permit existing inbound BOLT12 records to accept an unchanged hash when failing a claimable payment. The remaining assertion still prevents changing an established hash. 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
Register payment-lease refills as cancellable node tasks so shutdown can reject late work and abort active negotiations. Keep only a weak runtime reference in the liquidity client to avoid extending its lifetime. 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
Retry transient LSPS2 request failures for foreground acquisition and background cache refills. Preserve immediate errors for fee limits and unavailable liquidity sources. Co-Authored-By: HAL 9000
Try the next eligible provider when a selected LSP rejects or times out during the buy request. Skip remaining fee-menu entries from a failed provider while preserving bounded whole-round retries. 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 exclusively owned and persist only leases intended for reuse. 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
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
Use the cache-specific validity threshold and pass the requested lifetime through the lease-selection tests after the validity policies were separated. Co-Authored-By: HAL 9000
Pass the available LSP set through the pool-isolation test after lease selection gained availability filtering. Co-Authored-By: HAL 9000
Addressed most of the points now. @jkczyz please let me know what you think. |
7b456df to
03260f1
Compare
|
Rebased to address minor conflicts. |
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: