Include release_held_htlc blinded paths in RAA

As part of supporting sending payments as an often-offline sender, the sender
needs to send held_htlc_available onion messages such that the reply path to
the message terminates at their always-online channel counterparty that is
holding the HTLC. That way when the recipient responds with release_held_htlc,
the sender's counterparty will receive that message.

Here the counterparty starts including said reply paths in the revoke_and_ack
message destined for the sender, so the sender can use these paths in
subsequent held_htlc_available messages.

We put the paths in the RAA to ensure the sender receives the blinded paths,
because failure to deliver the paths means the HTLC will timeout/fail.

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -28,6 +28,7 @@ use bitcoin::{secp256k1, sighash, TxIn};
 #[cfg(splicing)]
 use bitcoin::{FeeRate, Sequence};
 
+use crate::blinded_path::message::BlindedMessagePath;
 use crate::chain::chaininterface::{
 	fee_for_weight, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator,
 };
@@ -282,6 +283,29 @@ impl InboundHTLCState {
 			_ => None,
 		}
 	}
+
+	/// Whether we need to hold onto this HTLC until receipt of a corresponding [`ReleaseHeldHtlc`]
+	/// onion message.
+	///
+	///[`ReleaseHeldHtlc`]: crate::onion_message::async_payments::ReleaseHeldHtlc
+	fn should_hold_htlc(&self) -> bool {
+		match self {
+			InboundHTLCState::RemoteAnnounced(res)
+			| InboundHTLCState::AwaitingRemoteRevokeToAnnounce(res)
+			| InboundHTLCState::AwaitingAnnouncedRemoteRevoke(res) => match res {
+				InboundHTLCResolution::Resolved { pending_htlc_status } => {
+					match pending_htlc_status {
+						PendingHTLCStatus::Forward(info) => info.routing.should_hold_htlc(),
+						_ => false,
+					}
+				},
+				InboundHTLCResolution::Pending { update_add_htlc } => {
+					update_add_htlc.hold_htlc.is_some()
+				},
+			},
+			InboundHTLCState::Committed | InboundHTLCState::LocalRemoved(_) => false,
+		}
+	}
 }
 
 struct InboundHTLCOutput {
@@ -1602,12 +1626,12 @@ where
 	}
 
 	#[rustfmt::skip]
-	pub fn signer_maybe_unblocked<L: Deref>(
-		&mut self, chain_hash: ChainHash, logger: &L,
-	) -> Option<SignerResumeUpdates> where L::Target: Logger {
+	pub fn signer_maybe_unblocked<L: Deref, CBP>(
+		&mut self, chain_hash: ChainHash, logger: &L, path_for_release_htlc: CBP
+	) -> Option<SignerResumeUpdates> where L::Target: Logger, CBP: Fn(u64) -> BlindedMessagePath {
 		match &mut self.phase {
 			ChannelPhase::Undefined => unreachable!(),
-			ChannelPhase::Funded(chan) => Some(chan.signer_maybe_unblocked(logger)),
+			ChannelPhase::Funded(chan) => Some(chan.signer_maybe_unblocked(logger, path_for_release_htlc)),
 			ChannelPhase::UnfundedOutboundV1(chan) => {
 				let (open_channel, funding_created) = chan.signer_maybe_unblocked(chain_hash, logger);
 				Some(SignerResumeUpdates {
@@ -8707,13 +8731,14 @@ where
 	/// successfully and we should restore normal operation. Returns messages which should be sent
 	/// to the remote side.
 	#[rustfmt::skip]
-	pub fn monitor_updating_restored<L: Deref, NS: Deref>(
+	pub fn monitor_updating_restored<L: Deref, NS: Deref, CBP>(
 		&mut self, logger: &L, node_signer: &NS, chain_hash: ChainHash,
-		user_config: &UserConfig, best_block_height: u32
+		user_config: &UserConfig, best_block_height: u32, path_for_release_htlc: CBP
 	) -> MonitorRestoreUpdates
 	where
 		L::Target: Logger,
-		NS::Target: NodeSigner
+		NS::Target: NodeSigner,
+		CBP: Fn(u64) -> BlindedMessagePath
 	{
 		assert!(self.context.channel_state.is_monitor_update_in_progress());
 		self.context.channel_state.clear_monitor_update_in_progress();
@@ -8770,7 +8795,7 @@ where
 		}
 
 		let mut raa = if self.context.monitor_pending_revoke_and_ack {
-			self.get_last_revoke_and_ack(logger)
+			self.get_last_revoke_and_ack(path_for_release_htlc, logger)
 		} else { None };
 		let mut commitment_update = if self.context.monitor_pending_commitment_signed {
 			self.get_last_commitment_update_for_send(logger).ok()
@@ -8859,7 +8884,9 @@ where
 	/// Indicates that the signer may have some signatures for us, so we should retry if we're
 	/// blocked.
 	#[rustfmt::skip]
-	pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger {
+	pub fn signer_maybe_unblocked<L: Deref, CBP>(
+		&mut self, logger: &L, path_for_release_htlc: CBP
+	) -> SignerResumeUpdates where L::Target: Logger, CBP: Fn(u64) -> BlindedMessagePath {
 		if !self.holder_commitment_point.can_advance() {
 			log_trace!(logger, "Attempting to update holder per-commitment point...");
 			self.holder_commitment_point.try_resolve_pending(&self.context.holder_signer, &self.context.secp_ctx, logger);
@@ -8887,7 +8914,7 @@ where
 		} else { None };
 		let mut revoke_and_ack = if self.context.signer_pending_revoke_and_ack {
 			log_trace!(logger, "Attempting to generate pending revoke and ack...");
-			self.get_last_revoke_and_ack(logger)
+			self.get_last_revoke_and_ack(path_for_release_htlc, logger)
 		} else { None };
 
 		if self.context.resend_order == RAACommitmentOrder::CommitmentFirst
@@ -8958,9 +8985,12 @@ where
 		}
 	}
 
-	fn get_last_revoke_and_ack<L: Deref>(&mut self, logger: &L) -> Option<msgs::RevokeAndACK>
+	fn get_last_revoke_and_ack<CBP, L: Deref>(
+		&mut self, path_for_release_htlc: CBP, logger: &L,
+	) -> Option<msgs::RevokeAndACK>
 	where
 		L::Target: Logger,
+		CBP: Fn(u64) -> BlindedMessagePath,
 	{
 		debug_assert!(
 			self.holder_commitment_point.next_transaction_number() <= INITIAL_COMMITMENT_NUMBER - 2
@@ -8973,14 +9003,22 @@ where
 			.ok();
 		if let Some(per_commitment_secret) = per_commitment_secret {
 			if self.holder_commitment_point.can_advance() {
+				let mut release_htlc_message_paths = Vec::new();
+				for htlc in &self.context.pending_inbound_htlcs {
+					if htlc.state.should_hold_htlc() {
+						let path = path_for_release_htlc(htlc.htlc_id);
+						release_htlc_message_paths.push((htlc.htlc_id, path));
+					}
+				}
+
 				self.context.signer_pending_revoke_and_ack = false;
 				return Some(msgs::RevokeAndACK {
 					channel_id: self.context.channel_id,
 					per_commitment_secret,
 					next_per_commitment_point: self.holder_commitment_point.next_point(),
 					#[cfg(taproot)]
 					next_local_nonce: None,
-					release_htlc_message_paths: Vec::new(),
+					release_htlc_message_paths,
 				});
 			}
 		}
@@ -9128,13 +9166,15 @@ where
 	/// May panic if some calls other than message-handling calls (which will all Err immediately)
 	/// have been called between remove_uncommitted_htlcs_and_mark_paused and this call.
 	#[rustfmt::skip]
-	pub fn channel_reestablish<L: Deref, NS: Deref>(
+	pub fn channel_reestablish<L: Deref, NS: Deref, CBP>(
 		&mut self, msg: &msgs::ChannelReestablish, logger: &L, node_signer: &NS,
-		chain_hash: ChainHash, user_config: &UserConfig, best_block: &BestBlock
+		chain_hash: ChainHash, user_config: &UserConfig, best_block: &BestBlock,
+		path_for_release_htlc: CBP,
 	) -> Result<ReestablishResponses, ChannelError>
 	where
 		L::Target: Logger,
-		NS::Target: NodeSigner
+		NS::Target: NodeSigner,
+		CBP: Fn(u64) -> BlindedMessagePath
 	{
 		if !self.context.channel_state.is_peer_disconnected() {
 			// While BOLT 2 doesn't indicate explicitly we should error this channel here, it
@@ -9235,7 +9275,7 @@ where
 				self.context.monitor_pending_revoke_and_ack = true;
 				None
 			} else {
-				self.get_last_revoke_and_ack(logger)
+				self.get_last_revoke_and_ack(path_for_release_htlc, logger)
 			}
 		} else {
 			debug_assert!(false, "All values should have been handled in the four cases above");
@@ -16490,6 +16530,7 @@ mod tests {
 			chain_hash,
 			&config,
 			0,
+			|_| unreachable!()
 		);
 
 		// Receive funding_signed, but the channel will be configured to hold sending channel_ready and
@@ -16504,6 +16545,7 @@ mod tests {
 			chain_hash,
 			&config,
 			0,
+			|_| unreachable!()
 		);
 		// Our channel_ready shouldn't be sent yet, even with trust_own_funding_0conf set,
 		// as the funding transaction depends on all channels in the batch becoming ready.

lightning/src/ln/channelmanager.rs

@@ -377,7 +377,7 @@ impl PendingHTLCRouting {
 
 	/// Whether this HTLC should be held by our node until we receive a corresponding
 	/// [`ReleaseHeldHtlc`] onion message.
-	fn should_hold_htlc(&self) -> bool {
+	pub(super) fn should_hold_htlc(&self) -> bool {
 		match self {
 			Self::Forward { hold_htlc: Some(()), .. } => true,
 			_ => false,
@@ -3430,18 +3430,20 @@ macro_rules! emit_initial_channel_ready_event {
 /// set for this channel is empty!
 macro_rules! handle_monitor_update_completion {
 	($self: ident, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr) => { {
+		let channel_id = $chan.context.channel_id();
+		let counterparty_node_id = $chan.context.get_counterparty_node_id();
 		#[cfg(debug_assertions)]
 		{
 			let in_flight_updates =
-				$peer_state.in_flight_monitor_updates.get(&$chan.context.channel_id());
+				$peer_state.in_flight_monitor_updates.get(&channel_id);
 			assert!(in_flight_updates.map(|(_, updates)| updates.is_empty()).unwrap_or(true));
 			assert_eq!($chan.blocked_monitor_updates_pending(), 0);
 		}
 		let logger = WithChannelContext::from(&$self.logger, &$chan.context, None);
 		let mut updates = $chan.monitor_updating_restored(&&logger,
 			&$self.node_signer, $self.chain_hash, &*$self.config.read().unwrap(),
-			$self.best_block.read().unwrap().height);
-		let counterparty_node_id = $chan.context.get_counterparty_node_id();
+			$self.best_block.read().unwrap().height,
+			|htlc_id| $self.path_for_release_held_htlc(htlc_id, &channel_id, &counterparty_node_id));
 		let channel_update = if updates.channel_ready.is_some() && $chan.context.is_usable() {
 			// We only send a channel_update in the case where we are just now sending a
 			// channel_ready and the channel is in a usable state. We may re-send a
@@ -3457,7 +3459,7 @@ macro_rules! handle_monitor_update_completion {
 		} else { None };
 
 		let update_actions = $peer_state.monitor_update_blocked_actions
-			.remove(&$chan.context.channel_id()).unwrap_or(Vec::new());
+			.remove(&channel_id).unwrap_or(Vec::new());
 
 		let (htlc_forwards, decode_update_add_htlcs) = $self.handle_channel_resumption(
 			&mut $peer_state.pending_msg_events, $chan, updates.raa,
@@ -3468,7 +3470,6 @@ macro_rules! handle_monitor_update_completion {
 			$peer_state.pending_msg_events.push(upd);
 		}
 
-		let channel_id = $chan.context.channel_id();
 		let unbroadcasted_batch_funding_txid = $chan.context.unbroadcasted_batch_funding_txid(&$chan.funding);
 		core::mem::drop($peer_state_lock);
 		core::mem::drop($per_peer_state_lock);
@@ -11138,6 +11139,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 							self.chain_hash,
 							&self.config.read().unwrap(),
 							&*self.best_block.read().unwrap(),
+							|htlc_id| self.path_for_release_held_htlc(htlc_id, &msg.channel_id, counterparty_node_id)
 						);
 						let responses = try_channel_entry!(self, peer_state, res, chan_entry);
 						let mut channel_update = None;
@@ -11601,9 +11603,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 
 		// Returns whether we should remove this channel as it's just been closed.
 		let unblock_chan = |chan: &mut Channel<SP>, pending_msg_events: &mut Vec<MessageSendEvent>| -> Option<ShutdownResult> {
+			let channel_id = chan.context().channel_id();
 			let logger = WithChannelContext::from(&self.logger, &chan.context(), None);
 			let node_id = chan.context().get_counterparty_node_id();
-			if let Some(msgs) = chan.signer_maybe_unblocked(self.chain_hash, &&logger) {
+			if let Some(msgs) = chan.signer_maybe_unblocked(
+				self.chain_hash, &&logger,
+				|htlc_id| self.path_for_release_held_htlc(htlc_id, &channel_id, &node_id)
+			) {
 				if let Some(msg) = msgs.open_channel {
 					pending_msg_events.push(MessageSendEvent::SendOpenChannel {
 						node_id,
@@ -11624,7 +11630,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				}
 				let cu_msg = msgs.commitment_update.map(|updates| MessageSendEvent::UpdateHTLCs {
 					node_id,
-					channel_id: chan.context().channel_id(),
+					channel_id,
 					updates,
 				});
 				let raa_msg = msgs.revoke_and_ack.map(|msg| MessageSendEvent::SendRevokeAndACK {