move the bolt12 invoice inside HTLCSource::OutboundRoute

Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: #3714
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

Author: vincenzopalazzo

fuzz/src/process_onion_failure.rs

@@ -114,8 +114,13 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	let path = Path { hops, blinded_tail };
 
-	let htlc_source =
-		HTLCSource::OutboundRoute { path, session_priv, first_hop_htlc_msat: 0, payment_id };
+	let htlc_source = HTLCSource::OutboundRoute {
+		path,
+		session_priv,
+		first_hop_htlc_msat: 0,
+		payment_id,
+		bolt12_invoice: None,
+	};
 
 	let failure_len = get_u16!();
 	let failure_data = get_slice!(failure_len);

lightning/src/ln/channel.rs

@@ -11836,6 +11836,7 @@ mod tests {
 				session_priv: SecretKey::from_slice(&<Vec<u8>>::from_hex("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
 				first_hop_htlc_msat: 548,
 				payment_id: PaymentId([42; 32]),
+				bolt12_invoice: None,
 			},
 			skimmed_fee_msat: None,
 			blinding_point: None,
@@ -12214,6 +12215,7 @@ mod tests {
 			session_priv: test_utils::privkey(42),
 			first_hop_htlc_msat: 0,
 			payment_id: PaymentId([42; 32]),
+			bolt12_invoice: None,
 		};
 		let dummy_outbound_output = OutboundHTLCOutput {
 			htlc_id: 0,

lightning/src/ln/channelmanager.rs

@@ -34,7 +34,7 @@ use bitcoin::{secp256k1, Sequence};
 #[cfg(splicing)]
 use bitcoin::{TxIn, Weight};
 
-use crate::events::FundingInfo;
+use crate::events::{FundingInfo, PaidBolt12Invoice};
 use crate::blinded_path::message::{AsyncPaymentsContext, MessageContext, OffersContext};
 use crate::blinded_path::NodeIdLookUp;
 use crate::blinded_path::message::{BlindedMessagePath, MessageForwardNode};
@@ -668,6 +668,8 @@ mod fuzzy_channelmanager {
 			/// doing a double-pass on route when we get a failure back
 			first_hop_htlc_msat: u64,
 			payment_id: PaymentId,
+			// TODO(vincenzopalazzo): add the documentation for this field
+			bolt12_invoice: Option<PaidBolt12Invoice>,
 		},
 	}
 
@@ -705,7 +707,8 @@ impl core::hash::Hash for HTLCSource {
 				0u8.hash(hasher);
 				prev_hop_data.hash(hasher);
 			},
-			HTLCSource::OutboundRoute { path, session_priv, payment_id, first_hop_htlc_msat } => {
+			// FIXME(vincenzopalazzo): we can ignore the bolt12_invoice here?
+			HTLCSource::OutboundRoute { path, session_priv, payment_id, first_hop_htlc_msat, .. } => {
 				1u8.hash(hasher);
 				path.hash(hasher);
 				session_priv[..].hash(hasher);
@@ -723,6 +726,7 @@ impl HTLCSource {
 			session_priv: SecretKey::from_slice(&[1; 32]).unwrap(),
 			first_hop_htlc_msat: 0,
 			payment_id: PaymentId([2; 32]),
+			bolt12_invoice: None,
 		}
 	}
 
@@ -4629,14 +4633,14 @@ where
 		let _lck = self.total_consistency_lock.read().unwrap();
 		self.send_payment_along_path(SendAlongPathArgs {
 			path, payment_hash, recipient_onion: &recipient_onion, total_value,
-			cur_height, payment_id, keysend_preimage, invoice_request: None, session_priv_bytes
+			cur_height, payment_id, keysend_preimage, invoice_request: None, bolt12_invoice: None, session_priv_bytes
 		})
 	}
 
 	fn send_payment_along_path(&self, args: SendAlongPathArgs) -> Result<(), APIError> {
 		let SendAlongPathArgs {
 			path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage,
-			invoice_request, session_priv_bytes
+			invoice_request, bolt12_invoice, session_priv_bytes
 		} = args;
 		// The top-level caller should hold the total_consistency_lock read lock.
 		debug_assert!(self.total_consistency_lock.try_write().is_err());
@@ -4686,6 +4690,7 @@ where
 								session_priv: session_priv.clone(),
 								first_hop_htlc_msat: htlc_msat,
 								payment_id,
+								bolt12_invoice: bolt12_invoice.cloned(),
 							}, onion_packet, None, &self.fee_estimator, &&logger);
 						match break_channel_entry!(self, peer_state, send_res, chan_entry) {
 							Some(monitor_update) => {
@@ -7447,15 +7452,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		next_channel_outpoint: OutPoint, next_channel_id: ChannelId, next_user_channel_id: Option<u128>,
 	) {
 		match source {
-			HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
+			HTLCSource::OutboundRoute { session_priv, payment_id, path, bolt12_invoice, .. } => {
 				debug_assert!(self.background_events_processed_since_startup.load(Ordering::Acquire),
 					"We don't support claim_htlc claims during startup - monitors may not be available yet");
 				debug_assert_eq!(next_channel_counterparty_node_id, path.hops[0].pubkey);
 				let ev_completion_action = EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
 					channel_funding_outpoint: next_channel_outpoint, channel_id: next_channel_id,
 					counterparty_node_id: path.hops[0].pubkey,
 				};
-				self.pending_outbound_payments.claim_htlc(payment_id, payment_preimage,
+				self.pending_outbound_payments.claim_htlc(payment_id, payment_preimage, bolt12_invoice,
 					session_priv, path, from_onchain, ev_completion_action, &self.pending_events,
 					&self.logger);
 			},
@@ -13131,13 +13136,15 @@ impl Readable for HTLCSource {
 				let mut payment_id = None;
 				let mut payment_params: Option<PaymentParameters> = None;
 				let mut blinded_tail: Option<BlindedTail> = None;
+				let mut bolt12_invoice: Option<PaidBolt12Invoice> = None;
 				read_tlv_fields!(reader, {
 					(0, session_priv, required),
 					(1, payment_id, option),
 					(2, first_hop_htlc_msat, required),
 					(4, path_hops, required_vec),
 					(5, payment_params, (option: ReadableArgs, 0)),
 					(6, blinded_tail, option),
+					(7, bolt12_invoice, option),
 				});
 				if payment_id.is_none() {
 					// For backwards compat, if there was no payment_id written, use the session_priv bytes
@@ -13160,6 +13167,7 @@ impl Readable for HTLCSource {
 					first_hop_htlc_msat,
 					path,
 					payment_id: payment_id.unwrap(),
+					bolt12_invoice,
 				})
 			}
 			1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)),
@@ -13171,7 +13179,7 @@ impl Readable for HTLCSource {
 impl Writeable for HTLCSource {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), crate::io::Error> {
 		match self {
-			HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id } => {
+			HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, bolt12_invoice } => {
 				0u8.write(writer)?;
 				let payment_id_opt = Some(payment_id);
 				write_tlv_fields!(writer, {
@@ -13182,6 +13190,7 @@ impl Writeable for HTLCSource {
 					(4, path.hops, required_vec),
 					(5, None::<PaymentParameters>, option), // payment_params in LDK versions prior to 0.0.115
 					(6, path.blinded_tail, option),
+					(7, bolt12_invoice, option),
 				 });
 			}
 			HTLCSource::PreviousHopData(ref field) => {
@@ -14368,7 +14377,7 @@ where
 									} else { true }
 								});
 							},
-							HTLCSource::OutboundRoute { payment_id, session_priv, path, .. } => {
+							HTLCSource::OutboundRoute { payment_id, session_priv, path, bolt12_invoice, .. } => {
 								if let Some(preimage) = preimage_opt {
 									let pending_events = Mutex::new(pending_events_read);
 									// Note that we set `from_onchain` to "false" here,
@@ -14385,7 +14394,7 @@ where
 											channel_id: monitor.channel_id(),
 											counterparty_node_id: path.hops[0].pubkey,
 										};
-									pending_outbounds.claim_htlc(payment_id, preimage, session_priv,
+									pending_outbounds.claim_htlc(payment_id, preimage, bolt12_invoice, session_priv,
 										path, false, compl_action, &pending_events, &&logger);
 									pending_events_read = pending_events.into_inner().unwrap();
 								}

lightning/src/ln/onion_utils.rs

@@ -3233,6 +3233,7 @@ mod tests {
 			session_priv: get_test_session_key(),
 			first_hop_htlc_msat: 0,
 			payment_id: PaymentId([1; 32]),
+			bolt12_invoice: None,
 		};
 
 		process_onion_failure(&ctx_full, &logger, &htlc_source, onion_error)
@@ -3360,6 +3361,7 @@ mod tests {
 				session_priv,
 				first_hop_htlc_msat: dummy_amt_msat,
 				payment_id: PaymentId([1; 32]),
+				bolt12_invoice: None,
 			};
 
 			{
@@ -3547,6 +3549,7 @@ mod tests {
 			session_priv: session_key,
 			first_hop_htlc_msat: 0,
 			payment_id: PaymentId([1; 32]),
+			bolt12_invoice: None,
 		};
 
 		// Iterate over all possible failure positions and check that the cases that can be attributed are.
@@ -3655,6 +3658,7 @@ mod tests {
 			session_priv: get_test_session_key(),
 			first_hop_htlc_msat: 0,
 			payment_id: PaymentId([1; 32]),
+			bolt12_invoice: None,
 		};
 
 		let decrypted_failure = process_onion_failure(&ctx_full, &logger, &htlc_source, packet);

lightning/src/ln/outbound_payment.rs

@@ -163,6 +163,7 @@ impl PendingOutboundPayment {
 			_ => None,
 		}
 	}
+
 	fn increment_attempts(&mut self) {
 		if let PendingOutboundPayment::Retryable { attempts, .. } = self {
 			attempts.count += 1;
@@ -797,6 +798,7 @@ pub(super) struct SendAlongPathArgs<'a> {
 	pub payment_id: PaymentId,
 	pub keysend_preimage: &'a Option<PaymentPreimage>,
 	pub invoice_request: Option<&'a InvoiceRequest>,
+	pub bolt12_invoice: Option<&'a PaidBolt12Invoice>,
 	pub session_priv_bytes: [u8; 32],
 }
 
@@ -1042,7 +1044,8 @@ impl OutboundPayments {
 			hash_map::Entry::Occupied(entry) => match entry.get() {
 				PendingOutboundPayment::InvoiceReceived { .. } => {
 					let (retryable_payment, onion_session_privs) = Self::create_pending_payment(
-						payment_hash, recipient_onion.clone(), keysend_preimage, None, Some(bolt12_invoice), &route,
+						// FIXME: remove the bolt12_invoice here! we need to clean up this part!
+						payment_hash, recipient_onion.clone(), keysend_preimage, None, Some(bolt12_invoice.clone()), &route,
 						Some(retry_strategy), payment_params, entropy_source, best_block_height,
 					);
 					*entry.into_mut() = retryable_payment;
@@ -1053,7 +1056,8 @@ impl OutboundPayments {
 						invoice_request
 					} else { unreachable!() };
 					let (retryable_payment, onion_session_privs) = Self::create_pending_payment(
-						payment_hash, recipient_onion.clone(), keysend_preimage, Some(invreq), Some(bolt12_invoice), &route,
+						// FIXME: We do not need anymore the bolt12_invoice here! we need to clean up this part!
+						payment_hash, recipient_onion.clone(), keysend_preimage, Some(invreq), Some(bolt12_invoice.clone()), &route,
 						Some(retry_strategy), payment_params, entropy_source, best_block_height
 					);
 					outbounds.insert(payment_id, retryable_payment);
@@ -1066,7 +1070,7 @@ impl OutboundPayments {
 		core::mem::drop(outbounds);
 
 		let result = self.pay_route_internal(
-			&route, payment_hash, &recipient_onion, keysend_preimage, invoice_request, payment_id,
+			&route, payment_hash, &recipient_onion, keysend_preimage, invoice_request, Some(bolt12_invoice), payment_id,
 			Some(route_params.final_value_msat), &onion_session_privs, node_signer, best_block_height,
 			&send_payment_along_path
 		);
@@ -1359,7 +1363,7 @@ impl OutboundPayments {
 			})?;
 
 		let res = self.pay_route_internal(&route, payment_hash, &recipient_onion,
-			keysend_preimage, None, payment_id, None, &onion_session_privs, node_signer,
+			keysend_preimage, None, None, payment_id, None, &onion_session_privs, node_signer,
 			best_block_height, &send_payment_along_path);
 		log_info!(logger, "Sending payment with id {} and hash {} returned {:?}",
 			payment_id, payment_hash, res);
@@ -1437,7 +1441,7 @@ impl OutboundPayments {
 				}
 			}
 		}
-		let (total_msat, recipient_onion, keysend_preimage, onion_session_privs, invoice_request) = {
+		let (total_msat, recipient_onion, keysend_preimage, onion_session_privs, invoice_request, bolt12_invoice) = {
 			let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 			match outbounds.entry(payment_id) {
 				hash_map::Entry::Occupied(mut payment) => {
@@ -1479,8 +1483,9 @@ impl OutboundPayments {
 							}
 
 							payment.get_mut().increment_attempts();
+							let bolt12_invoice = payment.get().bolt12_invoice();
 
-							(total_msat, recipient_onion, keysend_preimage, onion_session_privs, invoice_request)
+							(total_msat, recipient_onion, keysend_preimage, onion_session_privs, invoice_request, bolt12_invoice.cloned())
 						},
 						PendingOutboundPayment::Legacy { .. } => {
 							log_error!(logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102");
@@ -1520,7 +1525,7 @@ impl OutboundPayments {
 			}
 		};
 		let res = self.pay_route_internal(&route, payment_hash, &recipient_onion, keysend_preimage,
-			invoice_request.as_ref(), payment_id, Some(total_msat), &onion_session_privs, node_signer,
+			invoice_request.as_ref(), bolt12_invoice, payment_id, Some(total_msat), &onion_session_privs, node_signer,
 			best_block_height, &send_payment_along_path);
 		log_info!(logger, "Result retrying payment id {}: {:?}", &payment_id, res);
 		if let Err(e) = res {
@@ -1673,7 +1678,7 @@ impl OutboundPayments {
 
 		let recipient_onion_fields = RecipientOnionFields::spontaneous_empty();
 		match self.pay_route_internal(&route, payment_hash, &recipient_onion_fields,
-			None, None, payment_id, None, &onion_session_privs, node_signer, best_block_height,
+			None, None, None, payment_id, None, &onion_session_privs, node_signer, best_block_height,
 			&send_payment_along_path
 		) {
 			Ok(()) => Ok((payment_hash, payment_id)),
@@ -1865,7 +1870,7 @@ impl OutboundPayments {
 
 	fn pay_route_internal<NS: Deref, F>(
 		&self, route: &Route, payment_hash: PaymentHash, recipient_onion: &RecipientOnionFields,
-		keysend_preimage: Option<PaymentPreimage>, invoice_request: Option<&InvoiceRequest>,
+		keysend_preimage: Option<PaymentPreimage>, invoice_request: Option<&InvoiceRequest>, bolt12_invoice: Option<PaidBolt12Invoice>,
 		payment_id: PaymentId, recv_value_msat: Option<u64>, onion_session_privs: &Vec<[u8; 32]>,
 		node_signer: &NS, best_block_height: u32, send_payment_along_path: &F
 	) -> Result<(), PaymentSendFailure>
@@ -1921,6 +1926,7 @@ impl OutboundPayments {
 			let path_res = send_payment_along_path(SendAlongPathArgs {
 				path: &path, payment_hash: &payment_hash, recipient_onion, total_value,
 				cur_height, payment_id, keysend_preimage: &keysend_preimage, invoice_request,
+				bolt12_invoice: bolt12_invoice.as_ref(),
 				session_priv_bytes: *session_priv_bytes
 			});
 			results.push(path_res);
@@ -1987,7 +1993,7 @@ impl OutboundPayments {
 		F: Fn(SendAlongPathArgs) -> Result<(), APIError>,
 	{
 		self.pay_route_internal(route, payment_hash, &recipient_onion,
-			keysend_preimage, None, payment_id, recv_value_msat, &onion_session_privs,
+			keysend_preimage, None, None, payment_id, recv_value_msat, &onion_session_privs,
 			node_signer, best_block_height, &send_payment_along_path)
 			.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
 	}
@@ -2008,8 +2014,8 @@ impl OutboundPayments {
 	}
 
 	pub(super) fn claim_htlc<L: Deref>(
-		&self, payment_id: PaymentId, payment_preimage: PaymentPreimage, session_priv: SecretKey,
-		path: Path, from_onchain: bool, ev_completion_action: EventCompletionAction,
+		&self, payment_id: PaymentId, payment_preimage: PaymentPreimage, bolt12_invoice: Option<PaidBolt12Invoice>,
+		session_priv: SecretKey, path: Path, from_onchain: bool, ev_completion_action: EventCompletionAction,
 		pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
 		logger: &L,
 	) where L::Target: Logger {
@@ -2029,7 +2035,7 @@ impl OutboundPayments {
 					payment_hash,
 					amount_msat,
 					fee_paid_msat,
-					bolt12_invoice: payment.get().bolt12_invoice().cloned(),
+					bolt12_invoice: bolt12_invoice.clone(),
 				}, Some(ev_completion_action.clone())));
 				payment.get_mut().mark_fulfilled();
 			}
@@ -2061,6 +2067,7 @@ impl OutboundPayments {
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 		let mut pending_events = pending_events.lock().unwrap();
 		for source in sources {
+			// TODO(vincenzopalazzo): This should contain the paid bolt12 invoice.
 			if let HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } = source {
 				let mut session_priv_bytes = [0; 32];
 				session_priv_bytes.copy_from_slice(&session_priv[..]);