Merge pull request #3757 from TheBlueMatt/2025-04-0.1.3-backports

Backports for 0.1.3

Author: TheBlueMatt

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/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;
@@ -530,20 +531,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)
@@ -558,13 +556,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.
@@ -583,8 +602,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

@@ -12148,6 +12148,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();
+}