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, u64) -> Result<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, u64) -> Result<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, u64) -> Result<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, u64) -> Result<BlindedMessagePath, ()>,
 	{
 		debug_assert!(
 			self.holder_commitment_point.next_transaction_number() <= INITIAL_COMMITMENT_NUMBER - 2
@@ -8973,14 +9003,32 @@ 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 {
+					// TODO: how to handle the errors here
+					let held_htlc = htlc.state.should_hold_htlc();
+					if held_htlc {
+						let scid = match self.funding.short_channel_id {
+							Some(scid) => scid,
+							None => {
+								continue;
+							},
+						};
+						match path_for_release_htlc(scid, htlc.htlc_id) {
+							Ok(path) => release_htlc_message_paths.push((htlc.htlc_id, path)),
+							Err(()) => continue,
+						}
+					}
+				}
+
 				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 +9176,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, u64) -> Result<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 +9285,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");
@@ -16433,6 +16483,7 @@ mod tests {
 			chain_hash,
 			&config,
 			0,
+			|_, _| Err(())
 		);
 
 		// Receive funding_signed, but the channel will be configured to hold sending channel_ready and
@@ -16447,6 +16498,7 @@ mod tests {
 			chain_hash,
 			&config,
 			0,
+			|_, _| Err(())
 		);
 		// 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,7 +3430,8 @@ macro_rules! handle_monitor_update_completion {
 		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);
+			$self.best_block.read().unwrap().height,
+			|scid, htlc_id| $self.path_for_release_held_htlc(scid, htlc_id));
 		let counterparty_node_id = $chan.context.get_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
@@ -11102,6 +11103,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(),
+							|scid, htlc_id| self.path_for_release_held_htlc(scid, htlc_id)
 						);
 						let responses = try_channel_entry!(self, peer_state, res, chan_entry);
 						let mut channel_update = None;
@@ -11567,7 +11569,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		let unblock_chan = |chan: &mut Channel<SP>, pending_msg_events: &mut Vec<MessageSendEvent>| -> Option<ShutdownResult> {
 			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, |scid, htlc_id| self.path_for_release_held_htlc(scid, htlc_id)
+			) {
 				if let Some(msg) = msgs.open_channel {
 					pending_msg_events.push(MessageSendEvent::SendOpenChannel {
 						node_id,