Time out AwatingRemoteRAA outgoing HTLCs when we reach cltv_expiry

In case of committing out-of-time outgoing HTLCs, we force
ourselves to close the channel to avoid remote peer claims on a
non-backed HTLC

Author: Antoine Riard

lightning/src/ln/channel.rs

@@ -18,7 +18,7 @@ use secp256k1;
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
-use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep};
+use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys};
 use ln::chan_utils;
@@ -3154,13 +3154,33 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		self.network_sync == UpdateStatus::DisabledMarked
 	}
 
-	/// Called by channelmanager based on chain blocks being connected.
-	/// Note that we only need to use this to detect funding_signed, anything else is handled by
-	/// the channel_monitor.
-	/// In case of Err, the channel may have been closed, at which point the standard requirements
-	/// apply - no calls may be made except those explicitly stated to be allowed post-shutdown.
+	/// When we receive a new block, we (a) check whether the block contains the funding
+	/// transaction (which would start us counting blocks until we send the funding_signed), and
+	/// (b) check the height of the block against outbound holding cell HTLCs in case we need to
+	/// give up on them prematurely and time them out. Everything else (e.g. commitment
+	/// transaction broadcasts, channel closure detection, HTLC transaction broadcasting, etc) is
+	/// handled by the ChannelMonitor.
+	///
+	/// If we return Err, the channel may have been closed, at which point the standard
+	/// requirements apply - no calls may be made except those explicitly stated to be allowed
+	/// post-shutdown.
 	/// Only returns an ErrorAction of DisconnectPeer, if Err.
-	pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> {
+	///
+	/// May return some HTLCs (and their payment_hash) which have timed out and should be failed
+	/// back.
+	pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
+		let mut timed_out_htlcs = Vec::new();
+		self.holding_cell_htlc_updates.retain(|htlc_update| {
+			match htlc_update {
+				&HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, ref cltv_expiry, .. } => {
+					if *cltv_expiry <= height + HTLC_FAIL_BACK_BUFFER {
+						timed_out_htlcs.push((source.clone(), payment_hash.clone()));
+						false
+					} else { true }
+				},
+				_ => true
+			}
+		});
 		let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
 		if header.bitcoin_hash() != self.last_block_connected {
 			if self.funding_tx_confirmations > 0 {
@@ -3243,19 +3263,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 						if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
 							let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 							let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
-							return Ok(Some(msgs::FundingLocked {
+							return Ok((Some(msgs::FundingLocked {
 								channel_id: self.channel_id,
 								next_per_commitment_point: next_per_commitment_point,
-							}));
+							}), timed_out_htlcs));
 						} else {
 							self.monitor_pending_funding_locked = true;
-							return Ok(None);
+							return Ok((None, timed_out_htlcs));
 						}
 					}
 				}
 			}
 		}
-		Ok(None)
+		Ok((None, timed_out_htlcs))
 	}
 
 	/// Called by channelmanager based on chain blocks being disconnected.

lightning/src/ln/channelmanager.rs

@@ -2962,23 +2962,32 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
 			let short_to_id = &mut channel_state.short_to_id;
 			let pending_msg_events = &mut channel_state.pending_msg_events;
 			channel_state.by_id.retain(|_, channel| {
-				let chan_res = channel.block_connected(header, height, txn_matched, indexes_of_txn_matched);
-				if let Ok(Some(funding_locked)) = chan_res {
-					pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
-						node_id: channel.get_their_node_id(),
-						msg: funding_locked,
-					});
-					if let Some(announcement_sigs) = self.get_announcement_sigs(channel) {
-						log_trace!(self, "Sending funding_locked and announcement_signatures for {}", log_bytes!(channel.channel_id()));
-						pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+				let res = channel.block_connected(header, height, txn_matched, indexes_of_txn_matched);
+				if let Ok((chan_res, mut timed_out_pending_htlcs)) = res {
+					for (source, payment_hash) in timed_out_pending_htlcs.drain(..) {
+						let chan_update = self.get_channel_update(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe
+						timed_out_htlcs.push((source, payment_hash,  HTLCFailReason::Reason {
+							failure_code: 0x1000 | 14, // expiry_too_soon, or at least it is now
+							data: chan_update,
+						}));
+					}
+					if let Some(funding_locked) = chan_res {
+						pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
 							node_id: channel.get_their_node_id(),
-							msg: announcement_sigs,
+							msg: funding_locked,
 						});
-					} else {
-						log_trace!(self, "Sending funding_locked WITHOUT announcement_signatures for {}", log_bytes!(channel.channel_id()));
+						if let Some(announcement_sigs) = self.get_announcement_sigs(channel) {
+							log_trace!(self, "Sending funding_locked and announcement_signatures for {}", log_bytes!(channel.channel_id()));
+							pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+								node_id: channel.get_their_node_id(),
+								msg: announcement_sigs,
+							});
+						} else {
+							log_trace!(self, "Sending funding_locked WITHOUT announcement_signatures for {}", log_bytes!(channel.channel_id()));
+						}
+						short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
 					}
-					short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
-				} else if let Err(e) = chan_res {
+				} else if let Err(e) = res {
 					pending_msg_events.push(events::MessageSendEvent::HandleError {
 						node_id: channel.get_their_node_id(),
 						action: msgs::ErrorAction::SendErrorMessage { msg: e },
@@ -3049,8 +3058,6 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
 		}
 
 		for (source, payment_hash, reason) in timed_out_htlcs.drain(..) {
-			// Call it incorrect_or_unknown_payment_details as the issue, ultimately, is that the
-			// user failed to provide us a preimage within the cltv_expiry time window.
 			self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), source, &payment_hash, reason);
 		}
 		self.latest_block_height.store(height as usize, Ordering::Release);