Merge tag 'v0.1.3' into 2025-04-0.1.3-bindings

v0.1.3 - Apr 30, 2025 - "Routing Unicode in 2025"

Bug Fixes
=========

 * `Event::InvoiceReceived` is now only generated once for each `Bolt12Invoice`
   received matching a pending outbound payment. Previously it would be provided
   each time we received an invoice, which may happen many times if the sender
   sends redundant messages to improve success rates (lightningdevkit#3658).
 * LDK's router now more fully saturates paths which are subject to HTLC
   maximum restrictions after the first hop. In some rare cases this can result
   in finding paths when it would previously spuriously decide it cannot find
   enough diverse paths (lightningdevkit#3707, lightningdevkit#3755).

Security
========

0.1.3 fixes a denial-of-service vulnerability which cause a crash of an
LDK-based node if an attacker has access to a valid `Bolt12Offer` which the
LDK-based node created.
 * A malicious payer which requests a BOLT 12 Invoice from an LDK-based node
   (via the `Bolt12InvoiceRequest` message) can cause the panic of the
   LDK-based node due to the way `String::truncate` handles UTF-8 codepoints.
   The codepath can only be reached once the received `Botlt12InvoiceRequest`
   has been authenticated to be based on a valid `Bolt12Offer` which the same
   LDK-based node issued (lightningdevkit#3747, lightningdevkit#3750).

Author: TheBlueMatt

CHANGELOG.md

@@ -1,3 +1,27 @@
+# 0.1.3 - Apr 30, 2025 - "Routing Unicode in 2025"
+
+## Bug Fixes
+ * `Event::InvoiceReceived` is now only generated once for each `Bolt12Invoice`
+   received matching a pending outbound payment. Previously it would be provided
+   each time we received an invoice, which may happen many times if the sender
+   sends redundant messages to improve success rates (#3658).
+ * LDK's router now more fully saturates paths which are subject to HTLC
+   maximum restrictions after the first hop. In some rare cases this can result
+   in finding paths when it would previously spuriously decide it cannot find
+   enough diverse paths (#3707, #3755).
+
+## Security
+0.1.3 fixes a denial-of-service vulnerability which cause a crash of an
+LDK-based node if an attacker has access to a valid `Bolt12Offer` which the
+LDK-based node created.
+ * A malicious payer which requests a BOLT 12 Invoice from an LDK-based node
+   (via the `Bolt12InvoiceRequest` message) can cause the panic of the
+   LDK-based node due to the way `String::truncate` handles UTF-8 codepoints.
+   The codepath can only be reached once the received `Botlt12InvoiceRequest`
+   has been authenticated to be based on a valid `Bolt12Offer` which the same
+   LDK-based node issued (#3747, #3750).
+
+
 # 0.1.2 - Apr 02, 2025 - "Foolishly Edgy Cases"
 
 ## API Updates
@@ -35,6 +59,7 @@
    vulnerable to pinning attacks if they are not yet claimable by our
    counterparty, potentially reducing our exposure to pinning attacks (#3564).
 
+
 # 0.1.1 - Jan 28, 2025 - "Onchain Matters"
 
 ## API Updates
@@ -71,6 +96,7 @@ cause force-closure of unrelated channels.
    when they broadcast the stale commitment (#3556). Thanks to Matt Morehouse for
    reporting this issue.
 
+
 # 0.1 - Jan 15, 2025 - "Human Readable Version Numbers"
 
 The LDK 0.1 release represents an important milestone for the LDK project. While

ci/ci-tests.sh

@@ -21,6 +21,9 @@ PIN_RELEASE_DEPS # pin the release dependencies in our main workspace
 # The addr2line v0.21 crate (a dependency of `backtrace` starting with 0.3.69) relies on rustc 1.65
 [ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p backtrace --precise "0.3.68" --verbose
 
+# The once_cell v1.21.0 crate (a dependency of `proptest`) relies on rustc 1.70
+[ "$RUSTC_MINOR_VERSION" -lt 70 ] && cargo update -p once_cell --precise "1.20.3" --verbose
+
 # proptest 1.3.0 requires rustc 1.64.0
 [ "$RUSTC_MINOR_VERSION" -lt 64 ] && cargo update -p proptest --precise "1.2.0" --verbose
 

fuzz/src/invoice_request_deser.rs

@@ -85,16 +85,26 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
 	let expanded_key = ExpandedKey::new([42; 32]);
 	let entropy_source = Randomness {};
 	let nonce = Nonce::from_entropy_source(&entropy_source);
+
+	let invoice_request_fields =
+		if let Ok(ver) = invoice_request.clone().verify_using_metadata(&expanded_key, secp_ctx) {
+			// Previously we had a panic where we'd truncate the payer note possibly cutting a
+			// Unicode character in two here, so try to fetch fields if we can validate.
+			ver.fields()
+		} else {
+			InvoiceRequestFields {
+				payer_signing_pubkey: invoice_request.payer_signing_pubkey(),
+				quantity: invoice_request.quantity(),
+				payer_note_truncated: invoice_request
+					.payer_note()
+					.map(|s| UntrustedString(s.to_string())),
+				human_readable_name: None,
+			}
+		};
+
 	let payment_context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
 		offer_id: OfferId([42; 32]),
-		invoice_request: InvoiceRequestFields {
-			payer_signing_pubkey: invoice_request.payer_signing_pubkey(),
-			quantity: invoice_request.quantity(),
-			payer_note_truncated: invoice_request
-				.payer_note()
-				.map(|s| UntrustedString(s.to_string())),
-			human_readable_name: None,
-		},
+		invoice_request: invoice_request_fields,
 	});
 	let payee_tlvs = UnauthenticatedReceiveTlvs {
 		payment_secret: PaymentSecret([42; 32]),

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.1.2"
+version = "0.1.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/blinded_path/payment.rs

@@ -30,6 +30,7 @@ use crate::offers::nonce::Nonce;
 use crate::offers::offer::OfferId;
 use crate::routing::gossip::{NodeId, ReadOnlyNetworkGraph};
 use crate::sign::{EntropySource, NodeSigner, Recipient};
+use crate::types::routing::RoutingFees;
 use crate::util::ser::{FixedLengthReader, LengthReadableArgs, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer};
 
 use core::mem;
@@ -529,20 +530,17 @@ pub(crate) fn amt_to_forward_msat(inbound_amt_msat: u64, payment_relay: &Payment
 	u64::try_from(amt_to_forward).ok()
 }
 
-pub(super) fn compute_payinfo(
-	intermediate_nodes: &[PaymentForwardNode], payee_tlvs: &UnauthenticatedReceiveTlvs,
-	payee_htlc_maximum_msat: u64, min_final_cltv_expiry_delta: u16,
-) -> Result<BlindedPayInfo, ()> {
+// Returns (aggregated_base_fee, aggregated_proportional_fee)
+pub(crate) fn compute_aggregated_base_prop_fee<I>(hops_fees: I) -> Result<(u64, u64), ()>
+where
+	I: DoubleEndedIterator<Item = RoutingFees>,
+{
 	let mut curr_base_fee: u64 = 0;
 	let mut curr_prop_mil: u64 = 0;
-	let mut cltv_expiry_delta: u16 = min_final_cltv_expiry_delta;
-	for tlvs in intermediate_nodes.iter().rev().map(|n| &n.tlvs) {
-		// In the future, we'll want to take the intersection of all supported features for the
-		// `BlindedPayInfo`, but there are no features in that context right now.
-		if tlvs.features.requires_unknown_bits_from(&BlindedHopFeatures::empty()) { return Err(()) }
+	for fees in hops_fees.rev() {
+		let next_base_fee = fees.base_msat as u64;
+		let next_prop_mil = fees.proportional_millionths as u64;
 
-		let next_base_fee = tlvs.payment_relay.fee_base_msat as u64;
-		let next_prop_mil = tlvs.payment_relay.fee_proportional_millionths as u64;
 		// Use integer arithmetic to compute `ceil(a/b)` as `(a+b-1)/b`
 		// ((curr_base_fee * (1_000_000 + next_prop_mil)) / 1_000_000) + next_base_fee
 		curr_base_fee = curr_base_fee.checked_mul(1_000_000 + next_prop_mil)
@@ -557,13 +555,34 @@ pub(super) fn compute_payinfo(
 			.map(|f| f / 1_000_000)
 			.and_then(|f| f.checked_sub(1_000_000))
 			.ok_or(())?;
-
-		cltv_expiry_delta = cltv_expiry_delta.checked_add(tlvs.payment_relay.cltv_expiry_delta).ok_or(())?;
 	}
 
+	Ok((curr_base_fee, curr_prop_mil))
+}
+
+pub(super) fn compute_payinfo(
+	intermediate_nodes: &[PaymentForwardNode], payee_tlvs: &UnauthenticatedReceiveTlvs,
+	payee_htlc_maximum_msat: u64, min_final_cltv_expiry_delta: u16,
+) -> Result<BlindedPayInfo, ()> {
+	let (aggregated_base_fee, aggregated_prop_fee) =
+		compute_aggregated_base_prop_fee(intermediate_nodes.iter().map(|node| RoutingFees {
+			base_msat: node.tlvs.payment_relay.fee_base_msat,
+			proportional_millionths: node.tlvs.payment_relay.fee_proportional_millionths,
+		}))?;
+
 	let mut htlc_minimum_msat: u64 = 1;
 	let mut htlc_maximum_msat: u64 = 21_000_000 * 100_000_000 * 1_000; // Total bitcoin supply
+	let mut cltv_expiry_delta: u16 = min_final_cltv_expiry_delta;
 	for node in intermediate_nodes.iter() {
+		// In the future, we'll want to take the intersection of all supported features for the
+		// `BlindedPayInfo`, but there are no features in that context right now.
+		if node.tlvs.features.requires_unknown_bits_from(&BlindedHopFeatures::empty()) {
+			return Err(());
+		}
+
+		cltv_expiry_delta =
+			cltv_expiry_delta.checked_add(node.tlvs.payment_relay.cltv_expiry_delta).ok_or(())?;
+
 		// The min htlc for an intermediate node is that node's min minus the fees charged by all of the
 		// following hops for forwarding that min, since that fee amount will automatically be included
 		// in the amount that this node receives and contribute towards reaching its min.
@@ -582,8 +601,8 @@ pub(super) fn compute_payinfo(
 
 	if htlc_maximum_msat < htlc_minimum_msat { return Err(()) }
 	Ok(BlindedPayInfo {
-		fee_base_msat: u32::try_from(curr_base_fee).map_err(|_| ())?,
-		fee_proportional_millionths: u32::try_from(curr_prop_mil).map_err(|_| ())?,
+		fee_base_msat: u32::try_from(aggregated_base_fee).map_err(|_| ())?,
+		fee_proportional_millionths: u32::try_from(aggregated_prop_fee).map_err(|_| ())?,
 		cltv_expiry_delta,
 		htlc_minimum_msat,
 		htlc_maximum_msat,

lightning/src/ln/channelmanager.rs

@@ -12151,6 +12151,11 @@ where
 				);
 
 				if self.default_configuration.manually_handle_bolt12_invoices {
+					// Update the corresponding entry in `PendingOutboundPayment` for this invoice.
+					// This ensures that event generation remains idempotent in case we receive
+					// the same invoice multiple times.
+					self.pending_outbound_payments.mark_invoice_received(&invoice, payment_id).ok()?;
+
 					let event = Event::InvoiceReceived {
 						payment_id, invoice, context, responder,
 					};

lightning/src/ln/offers_tests.rs

@@ -1185,7 +1185,14 @@ fn pays_bolt12_invoice_asynchronously() {
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
 
-	let (invoice, context) = match get_event!(bob, Event::InvoiceReceived) {
+	// Re-process the same onion message to ensure idempotency — 
+	// we should not generate a duplicate `InvoiceReceived` event.
+	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
+
+	let mut events = bob.node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+
+	let (invoice, context) = match events.pop().unwrap() {
 		Event::InvoiceReceived { payment_id: actual_payment_id, invoice, context, .. } => {
 			assert_eq!(actual_payment_id, payment_id);
 			(invoice, context)

lightning/src/ln/outbound_payment.rs

@@ -73,9 +73,9 @@ pub(crate) enum PendingOutboundPayment {
 		max_total_routing_fee_msat: Option<u64>,
 		retryable_invoice_request: Option<RetryableInvoiceRequest>
 	},
-	// This state will never be persisted to disk because we transition from `AwaitingInvoice` to
-	// `Retryable` atomically within the `ChannelManager::total_consistency_lock`. Useful to avoid
-	// holding the `OutboundPayments::pending_outbound_payments` lock during pathfinding.
+	// Represents the state after the invoice has been received, transitioning from the corresponding
+	// `AwaitingInvoice` state.
+	// Helps avoid holding the `OutboundPayments::pending_outbound_payments` lock during pathfinding.
 	InvoiceReceived {
 		payment_hash: PaymentHash,
 		retry_strategy: Retry,
@@ -833,26 +833,8 @@ impl OutboundPayments {
 		IH: Fn() -> InFlightHtlcs,
 		SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
 	{
-		let payment_hash = invoice.payment_hash();
-		let max_total_routing_fee_msat;
-		let retry_strategy;
-		match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
-			hash_map::Entry::Occupied(entry) => match entry.get() {
-				PendingOutboundPayment::AwaitingInvoice {
-					retry_strategy: retry, max_total_routing_fee_msat: max_total_fee, ..
-				} => {
-					retry_strategy = *retry;
-					max_total_routing_fee_msat = *max_total_fee;
-					*entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
-						payment_hash,
-						retry_strategy: *retry,
-						max_total_routing_fee_msat,
-					};
-				},
-				_ => return Err(Bolt12PaymentError::DuplicateInvoice),
-			},
-			hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
-		}
+		let (payment_hash, retry_strategy, max_total_routing_fee_msat, _) = self
+			.mark_invoice_received_and_get_details(invoice, payment_id)?;
 
 		if invoice.invoice_features().requires_unknown_bits_from(&features) {
 			self.abandon_payment(
@@ -1754,6 +1736,51 @@ impl OutboundPayments {
 		}
 	}
 
+	pub(super) fn mark_invoice_received(
+		&self, invoice: &Bolt12Invoice, payment_id: PaymentId
+	) -> Result<(), Bolt12PaymentError> {
+		self.mark_invoice_received_and_get_details(invoice, payment_id)
+			.and_then(|(_, _, _, is_newly_marked)| {
+				is_newly_marked
+					.then_some(())
+					.ok_or(Bolt12PaymentError::DuplicateInvoice)
+			})
+	}
+
+	fn mark_invoice_received_and_get_details(
+		&self, invoice: &Bolt12Invoice, payment_id: PaymentId
+	) -> Result<(PaymentHash, Retry, Option<u64>, bool), Bolt12PaymentError> {
+		match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
+			hash_map::Entry::Occupied(entry) => match entry.get() {
+				PendingOutboundPayment::AwaitingInvoice {
+					retry_strategy: retry, max_total_routing_fee_msat: max_total_fee, ..
+				} => {
+					let payment_hash = invoice.payment_hash();
+					let retry = *retry;
+					let max_total_fee = *max_total_fee;
+					*entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
+						payment_hash,
+						retry_strategy: retry,
+						max_total_routing_fee_msat: max_total_fee,
+					};
+
+					Ok((payment_hash, retry, max_total_fee, true))
+				},
+				// When manual invoice handling is enabled, the corresponding `PendingOutboundPayment` entry
+				// is already updated at the time the invoice is received. This ensures that `InvoiceReceived`
+				// event generation remains idempotent, even if the same invoice is received again before the
+				// event is handled by the user.
+				PendingOutboundPayment::InvoiceReceived {
+					retry_strategy, max_total_routing_fee_msat, ..
+				} => {
+					Ok((invoice.payment_hash(), *retry_strategy, *max_total_routing_fee_msat, false))
+				},
+				_ => Err(Bolt12PaymentError::DuplicateInvoice),
+			},
+			hash_map::Entry::Vacant(_) => Err(Bolt12PaymentError::UnexpectedInvoice),
+		}
+	}
+
 	fn pay_route_internal<NS: Deref, F>(
 		&self, route: &Route, payment_hash: PaymentHash, recipient_onion: &RecipientOnionFields,
 		keysend_preimage: Option<PaymentPreimage>, invoice_request: Option<&InvoiceRequest>,

lightning/src/ln/payment_tests.rs

@@ -4479,3 +4479,55 @@ fn pay_route_without_params() {
 		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1]]], payment_preimage)
 	);
 }
+
+#[test]
+fn max_out_mpp_path() {
+	// In this setup, the sender is attempting to route an MPP payment split across the two channels
+	// that it has with its LSP, where the LSP has a single large channel to the recipient.
+	//
+	// Previously a user ran into a pathfinding failure here because our router was not sending the
+	// maximum possible value over the first MPP path it found due to overestimating the fees needed
+	// to cover the following hops. Because the path that had just been found was not maxxed out, our
+	// router assumed that we had already found enough paths to cover the full payment amount and that
+	// we were finding additional paths for the purpose of redundant path selection. This caused the
+	// router to mark the recipient's only channel as exhausted, with the intention of choosing more
+	// unique paths in future iterations. In reality, this ended up with the recipient's only channel
+	// being disabled and subsequently failing to find a route entirely.
+	//
+	// The router has since been updated to fully utilize the capacity of any paths it finds in this
+	// situation, preventing the "redundant path selection" behavior from kicking in.
+
+	let mut user_cfg = test_default_channel_config();
+	user_cfg.channel_config.forwarding_fee_base_msat = 0;
+	user_cfg.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
+	let mut lsp_cfg = test_default_channel_config();
+	lsp_cfg.channel_config.forwarding_fee_base_msat = 0;
+	lsp_cfg.channel_config.forwarding_fee_proportional_millionths = 3000;
+	lsp_cfg.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
+
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(
+		3, &node_cfgs, &[Some(user_cfg.clone()), Some(lsp_cfg.clone()), Some(user_cfg.clone())]
+	);
+	let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+	create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 200_000, 0);
+	create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 300_000, 0);
+	create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 600_000, 0);
+
+	let amt_msat = 350_000_000;
+	let invoice_params = crate::ln::channelmanager::Bolt11InvoiceParameters {
+		amount_msats: Some(amt_msat),
+		..Default::default()
+	};
+	let invoice = nodes[2].node.create_bolt11_invoice(invoice_params).unwrap();
+
+	let (hash, onion, params) =
+		crate::ln::bolt11_payment::payment_parameters_from_invoice(&invoice).unwrap();
+	nodes[0].node.send_payment(hash, onion, PaymentId([42; 32]), params, Retry::Attempts(0)).unwrap();
+
+	assert!(nodes[0].node.list_recent_payments().len() == 1);
+	check_added_monitors(&nodes[0], 2); // one monitor update per MPP part
+	nodes[0].node.get_and_clear_pending_msg_events();
+}