diff --git a/lightning/src/blinded_path/payment.rs b/lightning/src/blinded_path/payment.rs index 5fd608d6135..35d59b92635 100644 --- a/lightning/src/blinded_path/payment.rs +++ b/lightning/src/blinded_path/payment.rs @@ -111,7 +111,6 @@ impl BlindedPaymentPath { /// Errors if: /// * [`BlindedPayInfo`] calculation results in an integer overflow /// * any unknown features are required in the provided [`ForwardTlvs`] - // TODO: make all payloads the same size with padding + add dummy hops pub fn new( intermediate_nodes: &[PaymentForwardNode], payee_node_id: PublicKey, local_node_receive_key: ReceiveAuthKey, payee_tlvs: ReceiveTlvs, htlc_maximum_msat: u64, @@ -138,10 +137,7 @@ impl BlindedPaymentPath { /// /// This improves privacy by making path-length analysis based on fee and CLTV delta /// values less reliable. - /// - /// TODO: Add end-to-end tests validating fee aggregation, CLTV deltas, and - /// HTLC bounds when dummy hops are present, before exposing this API publicly. - pub(crate) fn new_with_dummy_hops< + pub fn new_with_dummy_hops< ES: EntropySource, T: secp256k1::Signing + secp256k1::Verification, >( @@ -454,6 +450,23 @@ impl ForwardTlvsInfo for TrampolineForwardTlvs { } } +/// Default relay parameters used for dummy hops when there is no nearby +/// forwarding hop to mirror. +/// +/// These values were chosen because they correspond to the most commonly +/// observed forwarding parameters on the public Lightning Network. They also +/// act as upper bounds when deriving relay parameters from nearby forwarding +/// data, ensuring dummy hops remain representative of typical network routing +/// behavior. +const DEFAULT_DUMMY_PAYMENT_RELAY: PaymentRelay = + PaymentRelay { cltv_expiry_delta: 40, fee_proportional_millionths: 1, fee_base_msat: 1_000 }; + +/// Default payment constraints used for dummy hops. +/// +/// These values impose no additional CLTV or HTLC-minimum restrictions. +const DEFAULT_DUMMY_PAYMENT_CONSTRAINTS: PaymentConstraints = + PaymentConstraints { max_cltv_expiry: u32::MAX, htlc_minimum_msat: 0 }; + /// TLVs carried by a dummy hop within a blinded payment path. /// /// Dummy hops do not correspond to real forwarding decisions, but are processed @@ -468,20 +481,54 @@ impl ForwardTlvsInfo for TrampolineForwardTlvs { pub struct DummyTlvs { /// Relay requirements (fees and CLTV delta) that must be satisfied when /// processing this dummy hop. - pub payment_relay: PaymentRelay, + pub(crate) payment_relay: PaymentRelay, /// Constraints that apply to the payment when relaying over this dummy hop. - pub payment_constraints: PaymentConstraints, + pub(crate) payment_constraints: PaymentConstraints, } -impl Default for DummyTlvs { - fn default() -> Self { - let payment_relay = - PaymentRelay { cltv_expiry_delta: 0, fee_proportional_millionths: 0, fee_base_msat: 0 }; +impl DummyTlvs { + /// Constructs a dummy-hop TLV set from explicit relay requirements and + /// payment constraints. + /// + /// This is primarily intended for callers that need full control over the + /// relay parameters encoded into a dummy hop. + pub fn new(payment_relay: PaymentRelay, payment_constraints: PaymentConstraints) -> Self { + Self { payment_relay, payment_constraints } + } + + /// Constructs dummy-hop TLVs by deriving relay parameters from a nearby + /// forwarding hop. + /// + /// Fee and CLTV values are capped to the default dummy-hop relay policy so + /// that the resulting parameters remain representative of common network + /// routing behavior while avoiding unusually large relay requirements. + pub fn from_forward_tlvs(forward_tlvs: &ForwardTlvs) -> Self { + let forward_payment_relay = forward_tlvs.payment_relay; + + let dummy_payment_relay = PaymentRelay { + cltv_expiry_delta: core::cmp::min( + forward_payment_relay.cltv_expiry_delta, + DEFAULT_DUMMY_PAYMENT_RELAY.cltv_expiry_delta, + ), + fee_proportional_millionths: core::cmp::min( + forward_payment_relay.fee_proportional_millionths, + DEFAULT_DUMMY_PAYMENT_RELAY.fee_proportional_millionths, + ), + fee_base_msat: core::cmp::min( + forward_payment_relay.fee_base_msat, + DEFAULT_DUMMY_PAYMENT_RELAY.fee_base_msat, + ), + }; - let payment_constraints = - PaymentConstraints { max_cltv_expiry: u32::MAX, htlc_minimum_msat: 0 }; + Self::new(dummy_payment_relay, DEFAULT_DUMMY_PAYMENT_CONSTRAINTS) + } +} - Self { payment_relay, payment_constraints } +impl Default for DummyTlvs { + /// Returns a dummy-hop TLV set using the default relay policy and payment + /// constraints. + fn default() -> Self { + Self::new(DEFAULT_DUMMY_PAYMENT_RELAY, DEFAULT_DUMMY_PAYMENT_CONSTRAINTS) } } @@ -693,6 +740,79 @@ pub struct Bolt12RefundContext { pub payment_metadata: Option>>, } +/// Common network-wide base fee buckets used when approximating forwarding policies. +const BASE_FEE_BUCKETS: &[u32] = &[ + 0, + 500, + 1_000, + 2_000, + 5_000, + 10_000, + 20_000, + 50_000, + 100_000, + 200_000, + 500_000, + 1_000_000, + 2_000_000, + 5_000_000, + 10_000_000, + 20_000_000, + 50_000_000, + 100_000_000, + u32::MAX, +]; + +/// Common network-wide proportional fee buckets used when approximating forwarding policies. +const PROPORTIONAL_FEE_BUCKETS: &[u32] = &[ + 0, + 1, + 5, + 10, + 20, + 50, + 100, + 200, + 500, + 1_000, + 2_500, + 5_000, + 10_000, + 20_000, + 50_000, + 100_000, + 200_000, + 500_000, + 1_000_000, + u32::MAX, +]; + +/// Common CLTV expiry delta buckets used when approximating forwarding policies. +/// +/// Values outside the supported range are rejected. +const CLTV_EXPIRY_DELTA_BUCKETS: &[u16] = &[40, 80, 144, 216]; + +fn bucket_cltv_expiry_delta(cltv_expiry_delta: u16) -> Result { + ceil_bucket(cltv_expiry_delta, CLTV_EXPIRY_DELTA_BUCKETS) +} + +fn bucket_fee_base_msat(fee_base_msat: u32) -> u32 { + ceil_bucket(fee_base_msat, BASE_FEE_BUCKETS).expect("fee buckets must include an upper bound") +} + +fn bucket_fee_proportional_millionths(fee_proportional_millionths: u32) -> u32 { + ceil_bucket(fee_proportional_millionths, PROPORTIONAL_FEE_BUCKETS) + .expect("fee buckets must include an upper bound") +} + +/// Rounds `value` upward to the nearest bucket. +/// +/// This is used to avoid underfunding blinded forwarding fees while avoiding exposure of unusually +/// specific forwarding policy values. +fn ceil_bucket(value: T, buckets: &[T]) -> Result { + buckets.iter().copied().find(|&bucket| value <= bucket).ok_or(()) +} + impl TryFrom for PaymentRelay { type Error = (); @@ -703,14 +823,10 @@ impl TryFrom for PaymentRelay { cltv_expiry_delta, } = info; - // Avoid exposing esoteric CLTV expiry deltas - let cltv_expiry_delta = match cltv_expiry_delta { - 0..=40 => 40, - 41..=80 => 80, - 81..=144 => 144, - 145..=216 => 216, - _ => return Err(()), - }; + let cltv_expiry_delta = bucket_cltv_expiry_delta(cltv_expiry_delta)?; + let fee_base_msat = bucket_fee_base_msat(fee_base_msat); + let fee_proportional_millionths = + bucket_fee_proportional_millionths(fee_proportional_millionths); Ok(Self { cltv_expiry_delta, fee_proportional_millionths, fee_base_msat }) } @@ -1026,6 +1142,10 @@ pub(super) fn compute_payinfo( ) .ok_or(())?; // If underflow occurs, we cannot send to this hop without exceeding their max } + // `payee_htlc_maximum_msat` limits the final payment plus all dummy-hop fees. For example, a + // 100,000 msat channel maximum and a 1,000 msat dummy-hop fee allow a 99,000 msat final payment. + // Apply the channel maximum here; the loop below subtracts each dummy-hop fee from it. + htlc_maximum_msat = core::cmp::min(payee_htlc_maximum_msat, htlc_maximum_msat); for dummy_tlvs in dummy_tlvs.iter() { cltv_expiry_delta = cltv_expiry_delta.checked_add(dummy_tlvs.payment_relay.cltv_expiry_delta).ok_or(())?; @@ -1035,10 +1155,14 @@ pub(super) fn compute_payinfo( &dummy_tlvs.payment_relay, ) .unwrap_or(1); // If underflow occurs, we definitely reached this node's min + + // Track the amount left after this dummy hop deducts its fee. After all dummy hops, this is + // the largest final amount whose inbound HTLC does not exceed the payee's channel maximum. + htlc_maximum_msat = + amt_to_forward_msat(htlc_maximum_msat, &dummy_tlvs.payment_relay).ok_or(())?; } htlc_minimum_msat = core::cmp::max(payee_tlvs.payment_constraints.htlc_minimum_msat, htlc_minimum_msat); - htlc_maximum_msat = core::cmp::min(payee_htlc_maximum_msat, htlc_maximum_msat); if htlc_maximum_msat < htlc_minimum_msat { return Err(()); diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index ec0ad6ccd9b..aa5171d51b6 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1025,6 +1025,8 @@ pub enum Event { /// /// [`ChannelConfig::accept_underpaying_htlcs`]: crate::util::config::ChannelConfig::accept_underpaying_htlcs counterparty_skimmed_fee_msat: u64, + /// The value, in thousands of a satoshi, that was collected by locally-peeled dummy hops. + dummy_skimmed_fees_msat: u64, /// Information for claiming this received payment, based on whether the purpose of the /// payment is to pay an invoice or to send a spontaneous payment. purpose: PaymentPurpose, @@ -1081,6 +1083,8 @@ pub enum Event { /// The value, in thousandths of a satoshi, that this payment is for. May be greater than the /// invoice amount. amount_msat: u64, + /// The value, in thousands of a satoshi, that was collected by locally-peeled dummy hops. + dummy_skimmed_fees_msat: u64, /// The purpose of the claimed payment, i.e. whether the payment was for an invoice or a /// spontaneous payment. purpose: PaymentPurpose, @@ -2041,6 +2045,7 @@ impl Writeable for Event { ref payment_hash, ref amount_msat, counterparty_skimmed_fee_msat, + dummy_skimmed_fees_msat, ref purpose, ref receiver_node_id, ref receiving_channel_ids, @@ -2087,6 +2092,8 @@ impl Writeable for Event { } else { Some(counterparty_skimmed_fee_msat) }; + let dummy_skimmed_fees_opt = + (dummy_skimmed_fees_msat != 0).then_some(dummy_skimmed_fees_msat); let (receiving_channel_id_legacy, receiving_user_channel_id_legacy) = match receiving_channel_ids.last() { @@ -2113,6 +2120,7 @@ impl Writeable for Event { (11, payment_context, option), (13, payment_id, option), (15, *receiving_channel_ids, optional_vec), + (17, dummy_skimmed_fees_opt, option), }); }, &Event::PaymentSent { @@ -2338,6 +2346,7 @@ impl Writeable for Event { &Event::PaymentClaimed { ref payment_hash, ref amount_msat, + dummy_skimmed_fees_msat, ref purpose, ref receiver_node_id, ref htlcs, @@ -2346,6 +2355,8 @@ impl Writeable for Event { ref payment_id, } => { 19u8.write(writer)?; + let dummy_skimmed_fees_msat_opt = + (dummy_skimmed_fees_msat != 0).then_some(dummy_skimmed_fees_msat); write_tlv_fields!(writer, { (0, payment_hash, required), (1, receiver_node_id, option), @@ -2355,6 +2366,7 @@ impl Writeable for Event { (7, sender_intended_total_msat, option), (9, onion_fields, option), (11, payment_id, option), + (13, dummy_skimmed_fees_msat_opt, option), }); }, &Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => { @@ -2565,6 +2577,7 @@ impl MaybeReadable for Event { let mut payment_secret = None; let mut amount_msat = 0; let mut counterparty_skimmed_fee_msat_opt = None; + let mut dummy_skimmed_fees_msat_opt = None; let mut receiver_node_id = None; let mut _user_payment_id = None::; // Used in 0.0.103 and earlier, no longer written in 0.0.116+. let mut receiving_channel_id_legacy = None; @@ -2589,6 +2602,7 @@ impl MaybeReadable for Event { (11, payment_context, option), (13, payment_id, option), (15, receiving_channel_ids_opt, optional_vec), + (17, dummy_skimmed_fees_msat_opt, option), }); let purpose = match payment_secret { Some(secret) => { @@ -2614,6 +2628,7 @@ impl MaybeReadable for Event { amount_msat, counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt .unwrap_or(0), + dummy_skimmed_fees_msat: dummy_skimmed_fees_msat_opt.unwrap_or(0), purpose, receiving_channel_ids, claim_deadline, @@ -2917,6 +2932,7 @@ impl MaybeReadable for Event { let mut payment_hash = PaymentHash([0; 32]); let mut purpose = UpgradableRequired(None); let mut amount_msat = 0; + let mut dummy_skimmed_fees_msat = None; let mut receiver_node_id = None; let mut htlcs: Option> = Some(vec![]); let mut sender_intended_total_msat: Option = None; @@ -2932,12 +2948,14 @@ impl MaybeReadable for Event { (9, onion_fields, (option: ReadableArgs, sender_intended_total_msat.unwrap_or(amount_msat))), (11, payment_id, option), + (13, dummy_skimmed_fees_msat, option), }); Ok(Some(Event::PaymentClaimed { receiver_node_id, payment_hash, purpose: _init_tlv_based_struct_field!(purpose, upgradable_required), amount_msat, + dummy_skimmed_fees_msat: dummy_skimmed_fees_msat.unwrap_or(0), htlcs: htlcs.unwrap_or_default(), sender_intended_total_msat, onion_fields, diff --git a/lightning/src/ln/async_payments_tests.rs b/lightning/src/ln/async_payments_tests.rs index 6e8f38f847a..9a2e7da4a0c 100644 --- a/lightning/src/ln/async_payments_tests.rs +++ b/lightning/src/ln/async_payments_tests.rs @@ -1507,8 +1507,10 @@ fn amount_doesnt_match_invreq() { let mut allow_priv_chan_fwds_cfg = test_default_channel_config(); allow_priv_chan_fwds_cfg.accept_forwards_to_priv_channels = true; // Make one blinded path's fees slightly higher so they are tried in a deterministic order. + // Use a fee that already matches a blinded relay fee bucket so the test's expected forwarding + // fee matches what blinded forwarding actually uses. let mut higher_fee_chan_cfg = allow_priv_chan_fwds_cfg.clone(); - higher_fee_chan_cfg.channel_config.forwarding_fee_base_msat += 5000; + higher_fee_chan_cfg.channel_config.forwarding_fee_base_msat = 2_000; let node_chanmgrs = create_node_chanmgrs( 4, &node_cfgs, @@ -3137,7 +3139,9 @@ fn held_htlc_timeout() { MIN_CLTV_EXPIRY_DELTA as u32 + TEST_FINAL_CLTV + HTLC_FAIL_BACK_BUFFER - + LATENCY_GRACE_PERIOD_BLOCKS, + + LATENCY_GRACE_PERIOD_BLOCKS + // Each dummy hop adds a 40-block CLTV delta before the held HTLC can time out. + + DEFAULT_PAYMENT_DUMMY_HOPS as u32 * 40, ); sender_lsp.node.process_pending_htlc_forwards(); diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 32c0709ed5c..10f621c5e06 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -258,12 +258,18 @@ fn one_hop_blinded_path_with_dummy_hops() { let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); let path = &[&nodes[1]]; - let args = PassAlongPathArgs::new(&nodes[0], path, amt_msat, payment_hash, ev) + // Conservative aggregation of the two dummy-hop fees overpays the recipient by 1 msat. + let args = PassAlongPathArgs::new(&nodes[0], path, amt_msat + 1, payment_hash, ev) .with_dummy_tlvs(&dummy_tlvs) .with_payment_secret(payment_secret); do_pass_along_path(args); - claim_payment(&nodes[0], &[&nodes[1]], payment_preimage); + // The sender reports the same 1-msat aggregation overpay as part of its total fee. + let expected_route = &[&[&nodes[1]][..]]; + claim_payment_along_route( + ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage) + .with_expected_extra_total_fees_msat(1), + ); } #[test] diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 49392264709..215ae8baa94 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -247,6 +247,8 @@ pub enum PendingHTLCRouting { /// 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, + /// The fee collected by any dummy hops peeled locally before reaching this receive payload. + dummy_skimmed_msats: Option, /// Additional data which we (allegedly) instructed the sender to include in the onion. /// /// For HTLCs received by LDK, this will ultimately be exposed in @@ -288,6 +290,8 @@ pub enum PendingHTLCRouting { /// 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, + /// The fee collected by any dummy hops peeled locally before reaching this receive payload. + dummy_skimmed_msats: 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, @@ -424,6 +428,12 @@ pub struct PendingHTLCInfo { pub incoming_accountable: bool, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct PendingDecodeAddHTLC { + update_add_htlc: msgs::UpdateAddHTLC, + dummy_skimmed_msats: Option, +} + #[derive(Clone, Debug)] // See FundedChannel::revoke_and_ack for why, tl;dr: Rust bug pub(super) enum HTLCFailureMsg { Relay(msgs::UpdateFailHTLC), @@ -579,6 +589,8 @@ impl HasMppPart for MppPart { struct ClaimableHTLC { mpp_part: MppPart, onion_payload: OnionPayload, + /// The fee collected by locally-peeled dummy hops for this HTLC. + dummy_skimmed_fee_msat: Option, /// The extra fee our counterparty skimmed off the top of this HTLC. counterparty_skimmed_fee_msat: Option, } @@ -1228,6 +1240,7 @@ pub(super) enum ChannelReadyOrder { #[derive(Clone, Debug, PartialEq, Eq)] struct ClaimingPayment { amount_msat: u64, + dummy_skimmed_fees_msat: u64, payment_purpose: events::PaymentPurpose, receiver_node_id: PublicKey, htlcs: Vec, @@ -1254,6 +1267,7 @@ impl_ser_tlv_based!(ClaimingPayment, { // onion_fields was added (and always set for new payments) in 0.0.124 (9, onion_fields, (required: ReadableArgs, amount_msat.0.unwrap())), (11, payment_id, option), + (13, dummy_skimmed_fees_msat, (default_value, 0u64)), }); struct ClaimablePayment { @@ -1443,6 +1457,11 @@ impl ClaimablePayments { debug_assert!(durable_preimage_channel.is_some()); ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.mpp_part.value).sum(), + dummy_skimmed_fees_msat: payment + .htlcs + .iter() + .map(|htlc| htlc.dummy_skimmed_fee_msat.unwrap_or(0)) + .sum(), payment_purpose: payment.purpose, receiver_node_id, htlcs, @@ -2883,12 +2902,16 @@ pub struct ChannelManager< /// [`ReleaseHeldHtlc`] onion message from an often-offline recipient pending_intercepted_htlcs: Mutex>, - /// Outbound SCID Alias -> pending `update_add_htlc`s to decode. + /// Outbound SCID Alias -> pending `update_add_htlc` decode state. /// We use the scid alias because regular scids may change if a splice occurs. /// - /// Note that no consistency guarantees are made about the existence of a channel with the - /// `short_channel_id` here, nor the `channel_id` in `UpdateAddHTLC`! - decode_update_add_htlcs: Mutex>>, + /// Each entry retains the original `UpdateAddHTLC` together with any metadata + /// accumulated while peeling dummy hops before final decode. + /// + /// Note that no consistency guarantees are made about the existence of a + /// channel with the `short_channel_id` here, nor the `channel_id` in + /// `UpdateAddHTLC`! + decode_update_add_htlcs: Mutex>>, /// The sets of payments which are claimable or currently being claimed. See /// [`ClaimablePayments`]' individual field docs for more info. @@ -5326,6 +5349,7 @@ impl< &self, msg: &msgs::UpdateAddHTLC, shared_secret: [u8; 32], decoded_hop: onion_utils::Hop, allow_underpay: bool, next_packet_pubkey_opt: Option>, + dummy_skimmed_msats: Option, ) -> Result { match decoded_hop { onion_utils::Hop::Receive { .. } | onion_utils::Hop::BlindedReceive { .. } | @@ -5338,7 +5362,7 @@ impl< let current_height: u32 = self.best_block.read().unwrap().height; create_recv_pending_htlc_info(decoded_hop, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat, - msg.accountable.unwrap_or(false), current_height) + dummy_skimmed_msats, msg.accountable.unwrap_or(false), current_height) }, onion_utils::Hop::Forward { .. } | onion_utils::Hop::BlindedForward { .. } => { create_fwd_pending_htlc_info(msg, decoded_hop, shared_secret, next_packet_pubkey_opt) @@ -7467,10 +7491,11 @@ impl< let mut htlc_forwards = Vec::new(); let mut htlc_fails = Vec::new(); - for update_add_htlc in &update_add_htlcs { + for pending_decode_add in &update_add_htlcs { + let update_add_htlc = &pending_decode_add.update_add_htlc; let (next_hop, next_packet_details_opt) = match decode_incoming_update_add_htlc_onion( - &update_add_htlc, + update_add_htlc, &self.node_signer, &self.logger, &self.secp_ctx, @@ -7495,11 +7520,25 @@ impl< &self.node_signer, &self.secp_ctx, ); + let skimmed_here = update_add_htlc + .amount_msat + .saturating_sub(new_update_add_htlc.amount_msat); + let total_dummy_skimmed_msats = pending_decode_add + .dummy_skimmed_msats + .unwrap_or(0) + .checked_add(skimmed_here) + .expect( + "dummy hop skim cannot overflow as it is bounded by the original HTLC amount", + ); dummy_update_add_htlcs .entry(incoming_scid_alias) .or_insert_with(Vec::new) - .push(new_update_add_htlc); + .push(PendingDecodeAddHTLC { + update_add_htlc: new_update_add_htlc, + dummy_skimmed_msats: (total_dummy_skimmed_msats != 0) + .then_some(total_dummy_skimmed_msats), + }); continue; }, @@ -7595,11 +7634,12 @@ impl< } match self.get_pending_htlc_info( - &update_add_htlc, + update_add_htlc, shared_secret, next_hop, incoming_accept_underpaying_htlcs, next_packet_details_opt.map(|d| d.next_packet_pubkey), + pending_decode_add.dummy_skimmed_msats, ) { Ok(info) => { let pending_add = PendingAddHTLCInfo { @@ -7990,6 +8030,7 @@ impl< Some(phantom_shared_secret), false, None, + None, incoming_accountable, current_height, ); @@ -8423,6 +8464,11 @@ impl< claimable_payment.total_counterparty_skimmed_msat(); let amount_msat: u64 = claimable_payment.htlcs.iter().map(|h| h.mpp_part.value).sum(); + let dummy_skimmed_fees_msat = claimable_payment + .htlcs + .iter() + .map(|htlc| htlc.dummy_skimmed_fee_msat.unwrap_or(0)) + .sum(); let total_sender_intended: u64 = claimable_payment.htlcs.iter().map(|h| h.mpp_part.sender_intended_value).sum(); debug_assert!( @@ -8448,6 +8494,7 @@ impl< .iter() .map(|htlc| htlc.mpp_part.value) .sum(), + dummy_skimmed_fees_msat, counterparty_skimmed_fee_msat: claimable_payment .total_counterparty_skimmed_msat(), receiving_channel_ids: claimable_payment.receiving_channel_ids(), @@ -8498,6 +8545,7 @@ impl< cltv_expiry, onion_payload, payment_data, + dummy_skimmed_msats, payment_context, phantom_shared_secret, mut onion_fields, @@ -8507,6 +8555,7 @@ impl< ) = match routing { PendingHTLCRouting::Receive { payment_data, + dummy_skimmed_msats, payment_metadata, payment_context, incoming_cltv_expiry, @@ -8526,6 +8575,7 @@ impl< incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data }, Some(payment_data), + dummy_skimmed_msats, payment_context, phantom_shared_secret, onion_fields, @@ -8536,6 +8586,7 @@ impl< }, PendingHTLCRouting::ReceiveKeysend { payment_data, + dummy_skimmed_msats, payment_preimage, payment_metadata, incoming_cltv_expiry, @@ -8560,6 +8611,7 @@ impl< incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), payment_data, + dummy_skimmed_msats, payment_context, None, onion_fields, @@ -8599,6 +8651,7 @@ impl< total_value_received: None, }, onion_payload, + dummy_skimmed_fee_msat: dummy_skimmed_msats, counterparty_skimmed_fee_msat: skimmed_fee_msat, }; @@ -10477,6 +10530,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .remove(&payment_hash); if let Some(ClaimingPayment { amount_msat, + dummy_skimmed_fees_msat, payment_purpose: purpose, receiver_node_id, htlcs, @@ -10490,6 +10544,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ payment_hash, purpose, amount_msat, + dummy_skimmed_fees_msat, receiver_node_id: Some(receiver_node_id), htlcs, sender_intended_total_msat, @@ -11035,7 +11090,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ channel_ready: Option, announcement_sigs: Option, mut funding_tx_signed: Option, tx_abort: Option, channel_ready_order: ChannelReadyOrder, - ) -> (Vec, Option<(u64, Vec)>) { + ) -> (Vec, Option<(u64, Vec)>) { let logger = WithChannelContext::from(&self.logger, &channel.context, None); log_trace!(logger, "Handling channel resumption with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures, {} tx_abort, {} splice_locked", if raa.is_some() { "an" } else { "no" }, @@ -11068,7 +11123,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } let mut decode_update_add_htlcs = None; if !pending_update_adds.is_empty() { - decode_update_add_htlcs = Some((outbound_scid_alias, pending_update_adds)); + decode_update_add_htlcs = Some(( + outbound_scid_alias, + pending_update_adds + .into_iter() + .map(|update_add_htlc| PendingDecodeAddHTLC { + update_add_htlc, + dummy_skimmed_msats: None, + }) + .collect(), + )); } if channel.context.is_connected() { @@ -12996,7 +13060,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - fn push_decode_update_add_htlcs(&self, mut update_add_htlcs: (u64, Vec)) { + fn push_decode_update_add_htlcs(&self, mut update_add_htlcs: (u64, Vec)) { let mut decode_update_add_htlcs = self.decode_update_add_htlcs.lock().unwrap(); let src_outbound_scid_alias = update_add_htlcs.0; match decode_update_add_htlcs.entry(src_outbound_scid_alias) { @@ -17636,7 +17700,7 @@ impl< // map's lock but before acquiring the `decode_update_add_htlcs` lock. let mut decode_update_add_htlcs = self.decode_update_add_htlcs.lock().unwrap(); if let Some(htlcs) = decode_update_add_htlcs.get_mut(&prev_outbound_scid_alias) { - for update_add in htlcs.iter_mut() { + for update_add in htlcs.iter_mut().map(|htlc| &mut htlc.update_add_htlc) { if update_add.htlc_id == htlc_id { log_trace!( self.logger, @@ -17897,6 +17961,7 @@ impl_ser_tlv_based_enum!(PendingHTLCRouting, (7, requires_blinded_error, (default_value, false)), (9, payment_context, option), (11, trampoline_shared_secret, option), + (13, dummy_skimmed_msats, option), }, (2, ReceiveKeysend) => { (0, payment_preimage, required), @@ -17908,6 +17973,7 @@ impl_ser_tlv_based_enum!(PendingHTLCRouting, (7, has_recipient_created_payment_secret, (default_value, false)), (9, payment_context, option), (11, invoice_request, option), + (13, dummy_skimmed_msats, option), }, (3, TrampolineForward) => { (0, trampoline_shared_secret, required), @@ -17929,6 +17995,11 @@ impl_ser_tlv_based!(PendingHTLCInfo, { (11, incoming_accountable, (default_value, false)), }); +impl_ser_tlv_based!(PendingDecodeAddHTLC, { + (0, update_add_htlc, required), + (2, dummy_skimmed_msats, option), +}); + impl Writeable for HTLCFailureMsg { #[rustfmt::skip] fn write(&self, writer: &mut W) -> Result<(), io::Error> { @@ -18046,6 +18117,7 @@ fn write_claimable_htlc( (6, htlc.mpp_part.cltv_expiry, required), (8, keysend_preimage, option), (10, htlc.counterparty_skimmed_fee_msat, option), + (12, htlc.dummy_skimmed_fee_msat, option), }); Ok(()) } @@ -18063,6 +18135,7 @@ impl Readable for (ClaimableHTLC, u64) { (6, cltv_expiry, required), (8, keysend_preimage, option), (10, counterparty_skimmed_fee_msat, option), + (12, dummy_skimmed_fee_msat, option), }); let payment_data: Option = payment_data_opt; let value = value_ser.0.unwrap(); @@ -18084,6 +18157,7 @@ impl Readable for (ClaimableHTLC, u64) { total_value_received, cltv_expiry: cltv_expiry.0.unwrap(), }, + dummy_skimmed_fee_msat, onion_payload, counterparty_skimmed_fee_msat, }, total_msat.0.expect("required field"))) @@ -18554,8 +18628,9 @@ impl< (10, legacy_in_flight_monitor_updates, option), (11, self.probing_cookie_secret, required), (13, htlc_onion_fields, optional_vec), - (14, decode_update_add_htlcs_opt, option), + (14, None::<&HashMap>>, option), (15, self.inbound_payment_id_secret, required), + (16, decode_update_add_htlcs_opt, option), (17, in_flight_monitor_updates, option), (19, peer_storage_dir, optional_vec), (21, WithoutLength(&self.flow.writeable_async_receive_offer_cache()), required), @@ -18651,7 +18726,7 @@ pub(super) struct ChannelManagerData { // `Channel{Monitor}` data. forward_htlcs_legacy: HashMap>, pending_intercepted_htlcs_legacy: HashMap, - decode_update_add_htlcs_legacy: HashMap>, + decode_update_add_htlcs_legacy: HashMap>, // The `ChannelManager` version that was written. version: u8, } @@ -18835,6 +18910,7 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger> HashMap<(PublicKey, ChannelId), Vec>, > = None; let mut inbound_payment_id_secret = None; + let mut decode_update_add_htlcs: Option>> = None; let mut peer_storage_dir: Option)>> = None; let mut async_receive_offer_cache: AsyncReceiveOfferCache = AsyncReceiveOfferCache::new(); let mut best_block_previous_blocks = None; @@ -18853,6 +18929,7 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger> (13, amountless_claimable_htlc_onion_fields, optional_vec), (14, decode_update_add_htlcs_legacy, option), (15, inbound_payment_id_secret, option), + (16, decode_update_add_htlcs, option), (17, in_flight_monitor_updates, option), (19, peer_storage_dir, optional_vec), (21, async_receive_offer_cache, (default_value, async_receive_offer_cache)), @@ -18949,6 +19026,22 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger> return Err(DecodeError::InvalidValue); } + let decode_update_add_htlcs_legacy = decode_update_add_htlcs_legacy + .unwrap_or_else(new_hash_map) + .into_iter() + .map(|(scid, htlcs)| { + let htlcs = htlcs + .into_iter() + .map(|update_add_htlc| PendingDecodeAddHTLC { + update_add_htlc, + dummy_skimmed_msats: None, + }) + .collect(); + + (scid, htlcs) + }) + .collect(); + Ok(ChannelManagerData { chain_hash, best_block: BlockLocator { @@ -18971,8 +19064,8 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger> .unwrap_or_else(Vec::new), fake_scid_rand_bytes, probing_cookie_secret, - decode_update_add_htlcs_legacy: decode_update_add_htlcs_legacy - .unwrap_or_else(new_hash_map), + decode_update_add_htlcs_legacy: decode_update_add_htlcs + .unwrap_or(decode_update_add_htlcs_legacy), inbound_payment_id_secret, in_flight_monitor_updates: in_flight_monitor_updates.unwrap_or_default(), peer_storage_dir: peer_storage_dir.unwrap_or_default(), @@ -19144,24 +19237,24 @@ impl< // If the HTLC corresponding to `prev_hop_data` is present in `decode_update_add_htlcs`, remove it // from the map as it is already being stored and processed elsewhere. fn dedup_decode_update_add_htlcs( - decode_update_add_htlcs: &mut HashMap>, + decode_update_add_htlcs: &mut HashMap>, prev_hop_data: &HTLCPreviousHopData, removal_reason: &'static str, logger: &L, ) { match decode_update_add_htlcs.entry(prev_hop_data.prev_outbound_scid_alias) { hash_map::Entry::Occupied(mut update_add_htlcs) => { update_add_htlcs.get_mut().retain(|update_add| { - let matches = update_add.htlc_id == prev_hop_data.htlc_id; + let matches = update_add.update_add_htlc.htlc_id == prev_hop_data.htlc_id; if matches { let logger = WithContext::from( logger, prev_hop_data.counterparty_node_id, - Some(update_add.channel_id), - Some(update_add.payment_hash), + Some(update_add.update_add_htlc.channel_id), + Some(update_add.update_add_htlc.payment_hash), ); log_info!( logger, "Removing pending to-decode HTLC with id {}: {}", - update_add.htlc_id, + update_add.update_add_htlc.htlc_id, removal_reason ); } @@ -19562,7 +19655,7 @@ impl< } // Post-deserialization processing - let mut decode_update_add_htlcs: HashMap> = new_hash_map(); + let mut decode_update_add_htlcs: HashMap> = new_hash_map(); if fake_scid_rand_bytes.is_none() { fake_scid_rand_bytes = Some(args.entropy_source.get_secure_random_bytes()); } @@ -19914,7 +20007,10 @@ impl< decode_update_add_htlcs .entry(scid_alias) .or_insert_with(Vec::new) - .push(update_add_htlc); + .push(PendingDecodeAddHTLC { + update_add_htlc, + dummy_skimmed_msats: None, + }); } for (payment_hash, prev_hop, next_hop) in funded_chan.inbound_forwarded_htlcs() @@ -20798,12 +20894,18 @@ impl< payment.inbound_payment_id(&inbound_payment_id_secret.unwrap()); let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(); let sender_intended_total_msat = payment.onion_fields.total_mpp_amount_msat; + let dummy_skimmed_fees_msat = payment + .htlcs + .iter() + .map(|htlc| htlc.dummy_skimmed_fee_msat.unwrap_or(0)) + .sum(); pending_events.push_back(( events::Event::PaymentClaimed { receiver_node_id, payment_hash, purpose: payment.purpose, amount_msat: claimable_amt_msat, + dummy_skimmed_fees_msat, htlcs, sender_intended_total_msat: Some(sender_intended_total_msat), onion_fields: Some(payment.onion_fields), @@ -20914,8 +21016,8 @@ fn reconcile_pending_htlcs_with_monitor( forward_htlcs_legacy: &mut HashMap>, pending_events_read: &mut VecDeque<(Event, Option)>, pending_intercepted_htlcs_legacy: &mut HashMap, - decode_update_add_htlcs: &mut HashMap>, - decode_update_add_htlcs_legacy: &mut HashMap>, + decode_update_add_htlcs: &mut HashMap>, + decode_update_add_htlcs_legacy: &mut HashMap>, prev_hop_data: HTLCPreviousHopData, logger: &impl Logger, payment_hash: PaymentHash, channel_id: ChannelId, ) { @@ -21850,7 +21952,7 @@ mod tests { if let Err(crate::ln::channelmanager::InboundHTLCErr { reason, .. }) = create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]), sender_intended_amt_msat - extra_fee_msat - 1, 42, None, true, Some(extra_fee_msat), - false, current_height) + None, false, current_height) { assert_eq!(reason, LocalHTLCFailureReason::FinalIncorrectHTLCAmount); } else { panic!(); } @@ -21873,7 +21975,7 @@ mod tests { let current_height: u32 = node[0].node.best_block.read().unwrap().height; assert!(create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]), sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat), - false, current_height).is_ok()); + None, false, current_height).is_ok()); } #[test] @@ -21898,7 +22000,7 @@ mod tests { custom_tlvs: Vec::new(), }, shared_secret: SharedSecret::from_bytes([0; 32]), - }, [0; 32], PaymentHash([0; 32]), 100, TEST_FINAL_CLTV + 1, None, true, None, false, current_height); + }, [0; 32], PaymentHash([0; 32]), 100, TEST_FINAL_CLTV + 1, None, true, None, None, false, current_height); // Should not return an error as this condition: // https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334 diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 82c1c619e82..6f5fe938c1b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3993,6 +3993,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { assert_eq!(claim_event.len(), 1, "{claim_event:?}"); #[allow(unused)] let mut fwd_amt_msat = 0; + let dummy_skimmed_fee_msat; match claim_event[0] { Event::PaymentClaimed { purpose: @@ -4003,6 +4004,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { amount_msat, ref htlcs, ref onion_fields, + dummy_skimmed_fees_msat, .. } => { assert_eq!(preimage, args.payment_preimage); @@ -4011,6 +4013,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, args.custom_tlvs); check_claimed_htlcs_match_route(args.origin_node, args.expected_paths, htlcs); fwd_amt_msat = amount_msat; + dummy_skimmed_fee_msat = dummy_skimmed_fees_msat; }, Event::PaymentClaimed { purpose: @@ -4021,6 +4024,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { amount_msat, ref htlcs, ref onion_fields, + dummy_skimmed_fees_msat, .. } => { assert_eq!(&payment_hash.0, &Sha256::hash(&args.payment_preimage.0)[..]); @@ -4029,6 +4033,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, args.custom_tlvs); check_claimed_htlcs_match_route(args.origin_node, args.expected_paths, htlcs); fwd_amt_msat = amount_msat; + dummy_skimmed_fee_msat = dummy_skimmed_fees_msat; }, _ => panic!(), } @@ -4060,7 +4065,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { } } - pass_claimed_payment_along_route_from_ev(fwd_amt_msat, per_path_msgs, args) + pass_claimed_payment_along_route_from_ev(fwd_amt_msat, per_path_msgs, args) + dummy_skimmed_fee_msat } pub fn pass_claimed_payment_along_route_from_ev( diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 8f073168465..a990f997248 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -758,7 +758,9 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() { route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Aggregating the three dummy-hop fee policies rounds conservatively, resulting in a 22-msat + // overpayment compared to applying each dummy-hop fee individually. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -827,7 +829,8 @@ fn router_modifies_payment_metadata_in_blinded_path() { // Verifies that Alice's `Event::PaymentClaimable` carries the `payment_metadata` injected by // the router (via the `expected_payment_context` equality check inside this helper). - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -910,7 +913,8 @@ fn pays_for_offer_with_payment_metadata_in_invoice_request_context() { // `claim_bolt12_payment` asserts the surfaced `PaymentContext` matches `payment_context` // above, including the embedded `payment_metadata`. - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -966,7 +970,8 @@ fn creates_and_pays_for_refund_using_one_hop_blinded_path() { route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -1021,7 +1026,8 @@ fn pays_for_offer_without_blinded_paths() { route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -1064,7 +1070,8 @@ fn pays_for_refund_without_blinded_paths() { route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -1303,7 +1310,8 @@ fn creates_and_pays_for_offer_with_retry() { } route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } @@ -1382,7 +1390,8 @@ fn pays_bolt12_invoice_asynchronously() { route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(bob, &[alice], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees(bob, &[alice], payment_context, &invoice, Some(22)); expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); assert_eq!( @@ -2462,7 +2471,12 @@ fn rejects_keysend_to_non_static_invoice_path() { _ => panic!() }; - claim_payment(&nodes[0], &[&nodes[1]], payment_preimage); + // Conservative dummy-hop fee aggregation adds 2 msat beyond the fees skimmed by the hops. + let expected_route = &[&[&nodes[1]][..]]; + claim_payment_along_route( + ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage) + .with_expected_extra_total_fees_msat(2), + ); expect_recent_payment!(&nodes[0], RecentPaymentDetails::Fulfilled, payment_id); // Time out the payment from recent payments so we can attempt to pay it again via keysend. @@ -2669,7 +2683,10 @@ fn creates_and_pays_for_phantom_offer() { route_bolt12_payment(&nodes[0], &[recipient], &invoice); expect_recent_payment!(&nodes[0], RecentPaymentDetails::Pending, payment_id); - claim_bolt12_payment(&nodes[0], &[recipient], payment_context, &invoice); + // Conservative dummy-hop fee aggregation adds 22 msat beyond the fees skimmed by the hops. + claim_bolt12_payment_with_extra_fees( + &nodes[0], &[recipient], payment_context, &invoice, Some(22), + ); expect_recent_payment!(&nodes[0], RecentPaymentDetails::Fulfilled, payment_id); assert!(nodes[0].onion_messenger.next_onion_message_for_peer(node_b_id).is_none()); diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index e8ff9788f3c..4f51a67e2e3 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -281,7 +281,8 @@ pub(super) fn create_fwd_pending_htlc_info( pub(super) fn create_recv_pending_htlc_info( hop_data: onion_utils::Hop, shared_secret: [u8; 32], payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool, - counterparty_skimmed_fee_msat: Option, incoming_accountable: bool, current_height: u32 + counterparty_skimmed_fee_msat: Option, dummy_skimmed_msats: Option, + incoming_accountable: bool, current_height: u32 ) -> Result { let ( payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, onion_cltv_expiry, @@ -436,6 +437,7 @@ pub(super) fn create_recv_pending_htlc_info( } PendingHTLCRouting::ReceiveKeysend { payment_data, + dummy_skimmed_msats, payment_preimage, payment_metadata, incoming_cltv_expiry: cltv_expiry, @@ -448,6 +450,7 @@ pub(super) fn create_recv_pending_htlc_info( } else if let Some(data) = payment_data { PendingHTLCRouting::Receive { payment_data: data, + dummy_skimmed_msats, payment_metadata, payment_context, incoming_cltv_expiry: cltv_expiry, @@ -488,6 +491,22 @@ pub(super) fn create_recv_pending_htlc_info( pub fn peel_payment_onion( msg: &msgs::UpdateAddHTLC, node_signer: NS, logger: L, secp_ctx: &Secp256k1, cur_height: u32, allow_skimmed_fees: bool, +) -> Result { + peel_payment_onion_inner( + msg, + node_signer, + logger, + secp_ctx, + cur_height, + allow_skimmed_fees, + 0, + ) +} + +#[rustfmt::skip] +fn peel_payment_onion_inner( + msg: &msgs::UpdateAddHTLC, node_signer: NS, logger: L, secp_ctx: &Secp256k1, + cur_height: u32, allow_skimmed_fees: bool, accumulated_dummy_skim_msats: u64, ) -> Result { let (hop, next_packet_details_opt) = decode_incoming_update_add_htlc_onion(msg, &node_signer, &logger, secp_ctx @@ -549,14 +568,32 @@ pub fn peel_payment_onion secp_ctx ); - peel_payment_onion(&new_update_add_htlc, node_signer, logger, secp_ctx, cur_height, allow_skimmed_fees)? + let accumulated_dummy_skim_msats = accumulated_dummy_skim_msats + .checked_add(msg.amount_msat.saturating_sub(new_update_add_htlc.amount_msat)) + .ok_or(InboundHTLCErr { + msg: "Dummy hop fee accumulation overflowed", + reason: LocalHTLCFailureReason::InvalidOnionBlinding, + err_data: vec![0; 32], + })?; + + peel_payment_onion_inner( + &new_update_add_htlc, + node_signer, + logger, + secp_ctx, + cur_height, + allow_skimmed_fees, + accumulated_dummy_skim_msats, + )? }, _ => { let shared_secret = hop.shared_secret().secret_bytes(); + let dummy_skimmed_msats = (accumulated_dummy_skim_msats != 0).then_some(accumulated_dummy_skim_msats); + create_recv_pending_htlc_info( hop, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None, allow_skimmed_fees, msg.skimmed_fee_msat, - msg.accountable.unwrap_or(false), cur_height, + dummy_skimmed_msats, msg.accountable.unwrap_or(false), cur_height, )? } }) @@ -673,7 +710,7 @@ pub(super) fn decode_incoming_update_add_htlc_onion (amt, cltv), Err(()) => { - return encode_relay_error("Underflow calculating outbound amount or cltv value for blinded forward", + return encode_relay_error("Underflow calculating outbound amount or cltv value for dummy hop", LocalHTLCFailureReason::InvalidOnionBlinding, shared_secret.secret_bytes(), None, &[0; 32]); } }; diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 01889b2ea60..8d6e65ced16 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -183,21 +183,27 @@ where .saturating_add(cltv_expiry_delta), htlc_minimum_msat: details.inbound_htlc_minimum_msat.unwrap_or(0), }; - Some(PaymentForwardNode { - tlvs: ForwardTlvs { - short_channel_id, - payment_relay, - payment_constraints, - next_blinding_override: None, - features: BlindedHopFeatures::empty(), + let forward_tlvs = ForwardTlvs { + short_channel_id, + payment_relay, + payment_constraints, + next_blinding_override: None, + features: BlindedHopFeatures::empty(), + }; + let dummy_tlvs = DummyTlvs::from_forward_tlvs(&forward_tlvs); + + Some(( + PaymentForwardNode { + tlvs: forward_tlvs, + node_id: details.counterparty.node_id, + htlc_maximum_msat: details.inbound_htlc_maximum_msat.unwrap_or(u64::MAX), }, - node_id: details.counterparty.node_id, - htlc_maximum_msat: details.inbound_htlc_maximum_msat.unwrap_or(u64::MAX), - }) + dummy_tlvs, + )) }) - .map(|forward_node| { + .map(|(forward_node, dummy_tlvs)| { BlindedPaymentPath::new_with_dummy_hops( - &[forward_node], recipient, &[DummyTlvs::default(); DEFAULT_PAYMENT_DUMMY_HOPS], + &[forward_node], recipient, &[dummy_tlvs; DEFAULT_PAYMENT_DUMMY_HOPS], local_node_receive_key, tlvs.clone(), u64::MAX, MIN_FINAL_CLTV_EXPIRY_DELTA, &self.entropy_source, secp_ctx ) }) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index b93be6446ce..1017d0984bd 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1173,6 +1173,7 @@ impl_for_vec!(crate::blinded_path::message::BlindedMessagePath); impl_writeable_for_vec!(&crate::routing::router::BlindedTail); impl_readable_for_vec!(crate::routing::router::BlindedTail); impl_for_vec!(crate::routing::router::TrampolineHop); +impl_for_vec_with_element_length_prefix!(crate::ln::channelmanager::PendingDecodeAddHTLC); impl_for_vec_with_element_length_prefix!(crate::ln::msgs::UpdateAddHTLC); impl_writeable_for_vec_with_element_length_prefix!(&crate::ln::msgs::UpdateAddHTLC); impl_for_vec!(u32);