From 73b875431885ee63ca703fb910d2668e810610e6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 29 Nov 2023 23:11:02 +0000 Subject: [PATCH 1/3] Flesh out docs on `PendingHTLCRouting` Now that `PendingHTLCRouting` is public, its docs should be meaningful to developers not working directly on LDK, and thus needs substantially more information than it previously had. This adds much of that information. --- lightning/src/ln/channelmanager.rs | 65 +++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a38422852..2f7db2278 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -109,50 +109,79 @@ use crate::ln::script::ShutdownScript; // Alternatively, we can fill an outbound HTLC with a HTLCSource::OutboundRoute indicating this is // our payment, which we can use to decode errors or inform the user that the payment was sent. -/// Routing info for an inbound HTLC onion. +/// Information about where a received HTLC('s onion) has indicated the HTLC should go. #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug pub enum PendingHTLCRouting { - /// A forwarded HTLC. + /// An HTLC which should be forwarded on to another node. Forward { - /// BOLT 4 onion packet. + /// The onion which should be included in the forwarded HTLC, telling the next hop what to + /// do with the HTLC. onion_packet: msgs::OnionPacket, - /// The SCID from the onion that we should forward to. This could be a real SCID or a fake one - /// generated using `get_fake_scid` from the scid_utils::fake_scid module. + /// The short channel ID of the channel which we were instructed to forward this HTLC to. + /// + /// This could be a real on-chain SCID, an SCID alias, or some other SCID which has meaning + /// to the receiving node, such as one returned from + /// [`ChannelManager::get_intercept_scid`] or [`ChannelManager::get_phantom_scid`]. short_channel_id: u64, // This should be NonZero eventually when we bump MSRV /// Set if this HTLC is being forwarded within a blinded path. blinded: Option, }, - /// An HTLC paid to an invoice (supposedly) generated by us. - /// At this point, we have not checked that the invoice being paid was actually generated by us, - /// but rather it's claiming to pay an invoice of ours. + /// The onion indicates that this is a payment for an invoice (supposedly) generated by us. + /// + /// Note that at this point, we have not checked that the invoice being paid was actually + /// generated by us, but rather it's claiming to pay an invoice of ours. Receive { - /// Payment secret and total msat received. + /// Information about the amount the sender intended to pay and (potential) proof that this + /// is a payment for an invoice we generated. This proof of payment is is also used for + /// linking MPP parts of a larger payment. payment_data: msgs::FinalOnionHopData, - /// See [`RecipientOnionFields::payment_metadata`] for more info. + /// Additional data which we (allegedly) instructed the sender to include in the onion. + /// + /// For HTLCs received by LDK, this will ultimately be exposed in + /// [`Event::PaymentClaimable::onion_fields`] as + /// [`RecipientOnionFields::payment_metadata`]. payment_metadata: Option>, /// CLTV expiry of the received HTLC. + /// /// Used to track when we should expire pending HTLCs that go unclaimed. incoming_cltv_expiry: u32, - /// Shared secret derived using a phantom node secret key. If this field is Some, the - /// payment was sent to a phantom node (one hop beyond the current node), but can be - /// settled by this node. + /// If the onion had forwarding instructions to one of our phantom node SCIDs, this will + /// provide the onion shared secret used to decrypt the next level of forwarding + /// instructions. phantom_shared_secret: Option<[u8; 32]>, - /// See [`RecipientOnionFields::custom_tlvs`] for more info. + /// Custom TLVs which were set by the sender. + /// + /// For HTLCs received by LDK, this will ultimately be exposed in + /// [`Event::PaymentClaimable::onion_fields`] as + /// [`RecipientOnionFields::custom_tlvs`]. custom_tlvs: Vec<(u64, Vec)>, }, - /// Incoming keysend (sender provided the preimage in a TLV). + /// The onion indicates that this is for payment to us but which contains the preimage for + /// claiming included, and is unrelated to any invoice we'd previously generated (aka a + /// "keysend" or "spontaneous" payment). ReceiveKeysend { - /// This was added in 0.0.116 and will break deserialization on downgrades. + /// Information about the amount the sender intended to pay and possibly a token to + /// associate MPP parts of a larger payment. + /// + /// This will only be filled in if receiving MPP keysend payments is enabled, and it being + /// present will cause deserialization to fail on versions of LDK prior to 0.0.116. payment_data: Option, /// Preimage for this onion payment. This preimage is provided by the sender and will be /// used to settle the spontaneous payment. payment_preimage: PaymentPreimage, - /// See [`RecipientOnionFields::payment_metadata`] for more info. + /// Additional data which we (allegedly) instructed the sender to include in the onion. + /// + /// For HTLCs received by LDK, this will ultimately bubble back up as + /// [`RecipientOnionFields::payment_metadata`]. payment_metadata: Option>, /// CLTV expiry of the received HTLC. + /// /// Used to track when we should expire pending HTLCs that go unclaimed. incoming_cltv_expiry: u32, - /// See [`RecipientOnionFields::custom_tlvs`] for more info. + /// Custom TLVs which were set by the sender. + /// + /// For HTLCs received by LDK, these will ultimately bubble back up as + /// [`RecipientOnionFields::custom_tlvs`]. custom_tlvs: Vec<(u64, Vec)>, }, } From c7a89598fe28aa3f68cc73817752a6cdb6be2534 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 29 Nov 2023 23:23:34 +0000 Subject: [PATCH 2/3] Flesh out docs on `PendingHTLCInfo` Now that `PendingHTLCInfo` is public, its docs should be meaningful to developers not working directly on LDK, and thus needs substantially more information than it previously had. This adds much of that information. --- lightning/src/ln/channelmanager.rs | 42 +++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2f7db2278..796f823e5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -208,25 +208,47 @@ impl PendingHTLCRouting { } } -/// Full details of an incoming HTLC, including routing info. +/// Information about an incoming HTLC, including the [`PendingHTLCRouting`] describing where it +/// should go next. #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug pub struct PendingHTLCInfo { /// Further routing details based on whether the HTLC is being forwarded or received. pub routing: PendingHTLCRouting, - /// Shared secret from the previous hop. - /// Used encrypt failure packets in the event that the HTLC needs to be failed backwards. + /// The onion shared secret we build with the sender used to decrypt the onion. + /// + /// This is later used to encrypt failure packets in the event that the HTLC is failed. pub incoming_shared_secret: [u8; 32], /// Hash of the payment preimage, to lock the payment until the receiver releases the preimage. pub payment_hash: PaymentHash, - /// Amount offered by this HTLC. - pub incoming_amt_msat: Option, // Added in 0.0.113 - /// Sender intended amount to forward or receive (actual amount received - /// may overshoot this in either case) + /// Amount received in the incoming HTLC. + /// + /// This field was added in LDK 0.0.113 and will be `None` for objects written by prior + /// versions. + pub incoming_amt_msat: Option, + /// The amount the sender indicated should be forwarded on to the next hop or amount the sender + /// intended for us to receive for received payments. + /// + /// If the received amount is less than this for received payments, an intermediary hop has + /// attempted to steal some of our funds and we should fail the HTLC (the sender should retry + /// it along another path). + /// + /// Because nodes can take less than their required fees, and because senders may wish to + /// improve their own privacy, this amount may be less than [`Self::incoming_amt_msat`] for + /// received payments. In such cases, recipients must handle this HTLC as if it had received + /// [`Self::outgoing_amt_msat`]. pub outgoing_amt_msat: u64, - /// Outgoing timelock expiration blockheight. + /// The CLTV the sender has indicated we should set on the forwarded HTLC (or has indicated + /// should have been set on the received HTLC for received payments). pub outgoing_cltv_value: u32, - /// The fee being skimmed off the top of this HTLC. If this is a forward, it'll be the fee we are - /// skimming. If we're receiving this HTLC, it's the fee that our counterparty skimmed. + /// The fee taken for this HTLC in addition to the standard protocol HTLC fees. + /// + /// If this is a payment for forwarding, this is the fee we are taking before forwarding the + /// HTLC. + /// + /// If this is a received payment, this is the fee that our counterparty took. + /// + /// This is used to allow LSPs to take fees as a part of payments, without the sender having to + /// shoulder them. pub skimmed_fee_msat: Option, } From 63ee03f695e4077adbbe18dbb5624f0bbb0206f5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 29 Nov 2023 23:25:04 +0000 Subject: [PATCH 3/3] Allow users to accept skimmed fees in calling `peel_payment_onion` LSP users who wish to use `peel_payment_onion` to understand if they'd accept an HTLC prior to receit should be able to check the skimmed fees just like they would for full payment receipt. Thus, we need to expose the fee-skimming acceptance bool to `peel_payment_onion`, which we do here, in addition to some doc cleanups. --- lightning/src/ln/onion_payment.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index ca15a37c0..06a9ae07e 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -1,6 +1,7 @@ -//! Utilities for channelmanager.rs +//! Utilities to decode payment onions and do contextless validation of incoming payments. //! -//! Includes a public [`peel_payment_onion`] function for use by external projects or libraries. +//! Primarily features [`peel_payment_onion`], which allows the decoding of an onion statelessly +//! and can be used to predict whether we'd accept a payment. use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; @@ -225,7 +226,9 @@ pub(super) fn create_recv_pending_htlc_info( }) } -/// Peel one layer off an incoming onion, returning [`PendingHTLCInfo`] (either Forward or Receive). +/// Peel one layer off an incoming onion, returning a [`PendingHTLCInfo`] that contains information +/// about the intended next-hop for the HTLC. +/// /// This does all the relevant context-free checks that LDK requires for payment relay or /// acceptance. If the payment is to be received, and the amount matches the expected amount for /// a given invoice, this indicates the [`msgs::UpdateAddHTLC`], once fully committed in the @@ -234,7 +237,7 @@ pub(super) fn create_recv_pending_htlc_info( /// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable pub fn peel_payment_onion( msg: &msgs::UpdateAddHTLC, node_signer: &NS, logger: &L, secp_ctx: &Secp256k1, - cur_height: u32, accept_mpp_keysend: bool, + cur_height: u32, accept_mpp_keysend: bool, allow_skimmed_fees: bool, ) -> Result where NS::Target: NodeSigner, @@ -273,6 +276,10 @@ where err_data: Vec::new(), }); } + + // TODO: If this is potentially a phantom payment we should decode the phantom payment + // onion here and check it. + create_fwd_pending_htlc_info( msg, next_hop_data, next_hop_hmac, new_packet_bytes, shared_secret, Some(next_packet_pubkey) @@ -281,7 +288,7 @@ where onion_utils::Hop::Receive(received_data) => { create_recv_pending_htlc_info( received_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, - None, false, msg.skimmed_fee_msat, cur_height, accept_mpp_keysend, + None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height, accept_mpp_keysend, )? } }) @@ -477,7 +484,7 @@ mod tests { let msg = make_update_add_msg(amount_msat, cltv_expiry, payment_hash, onion); let logger = test_utils::TestLogger::with_id("bob".to_string()); - let peeled = peel_payment_onion(&msg, &&bob, &&logger, &secp_ctx, cur_height, true) + let peeled = peel_payment_onion(&msg, &&bob, &&logger, &secp_ctx, cur_height, true, false) .map_err(|e| e.msg).unwrap(); let next_onion = match peeled.routing { @@ -488,7 +495,7 @@ mod tests { }; let msg2 = make_update_add_msg(amount_msat, cltv_expiry, payment_hash, next_onion); - let peeled2 = peel_payment_onion(&msg2, &&charlie, &&logger, &secp_ctx, cur_height, true) + let peeled2 = peel_payment_onion(&msg2, &&charlie, &&logger, &secp_ctx, cur_height, true, false) .map_err(|e| e.msg).unwrap(); match peeled2.routing {