Tell ChannelMonitors about HTLCs fulfilled after channel close

valentinewallace · valentinewallace · commit df942d3b2411 · 2020-10-15T14:20:16.000-04:00
If a channel is about to hit the chain right as we receive a preimage,
prior to this commit the relevant ChannelMonitor would never learn of this
preimage.
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
@@ -40,7 +40,7 @@ use bitcoin::secp256k1;
 use ln::msgs::DecodeError;
 use ln::chan_utils;
 use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, HTLCType};
-use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
+use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash, MonitorUpdateInfo};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::transaction::{OutPoint, TransactionData};
@@ -182,9 +182,10 @@ pub enum MonitorEvent {
 pub struct HTLCUpdate {
 	pub(crate) payment_hash: PaymentHash,
 	pub(crate) payment_preimage: Option<PaymentPreimage>,
-	pub(crate) source: HTLCSource
+	pub(crate) source: HTLCSource,
+	pub(crate) monitor_info: MonitorUpdateInfo
 }
-impl_writeable!(HTLCUpdate, 0, { payment_hash, payment_preimage, source });
+impl_writeable!(HTLCUpdate, 0, { payment_hash, payment_preimage, source, monitor_info });
 
 /// If an HTLC expires within this many blocks, don't try to claim it in a shared transaction,
 /// instead claiming it in its own individual transaction.
@@ -1741,10 +1742,15 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 				match ev {
 					OnchainEvent::HTLCUpdate { htlc_update } => {
 						log_trace!(logger, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!((htlc_update.1).0));
+						let latest_monitor_update_id = self.get_latest_update_id();
 						self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
 							payment_hash: htlc_update.1,
 							payment_preimage: None,
 							source: htlc_update.0,
+							monitor_info: MonitorUpdateInfo {
+								funding_outpoint: self.funding_info.0,
+								latest_monitor_update_id
+							}
 						}));
 					},
 					OnchainEvent::MaturingOutput { descriptor } => {
@@ -1985,10 +1991,15 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 					if !self.pending_monitor_events.iter().any(
 						|update| if let &MonitorEvent::HTLCEvent(ref upd) = update { upd.source == source } else { false }) {
 						payment_preimage.0.copy_from_slice(&input.witness[3]);
+						let latest_monitor_update_id = self.get_latest_update_id();
 						self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
 							source,
 							payment_preimage: Some(payment_preimage),
-							payment_hash
+							payment_hash,
+							monitor_info: MonitorUpdateInfo {
+								funding_outpoint: self.funding_info.0,
+								latest_monitor_update_id
+							}
 						}));
 					}
 				} else if offered_preimage_claim {
@@ -1997,10 +2008,15 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 							upd.source == source
 						} else { false }) {
 						payment_preimage.0.copy_from_slice(&input.witness[1]);
+						let latest_monitor_update_id = self.get_latest_update_id();
 						self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
 							source,
 							payment_preimage: Some(payment_preimage),
-							payment_hash
+							payment_hash,
+							monitor_info: MonitorUpdateInfo {
+								funding_outpoint: self.funding_info.0,
+								latest_monitor_update_id
+							}
 						}));
 					}
 				} else {
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -25,7 +25,7 @@ use bitcoin::secp256k1;
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
-use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT, MonitorUpdateInfo};
 use ln::chan_utils::{CounterpartyCommitmentSecrets, HolderCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, PreCalculatedTxCreationKeys};
 use ln::chan_utils;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
@@ -1937,7 +1937,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find".to_owned()))
 	}
 
-	pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<HTLCSource, ChannelError> {
+	pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, MonitorUpdateInfo), ChannelError> {
 		if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 			return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state".to_owned()));
 		}
@@ -1946,7 +1946,16 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		}
 
 		let payment_hash = PaymentHash(Sha256::hash(&msg.payment_preimage.0[..]).into_inner());
-		self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None).map(|source| source.clone())
+		match self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None).map(|source| source.clone()) {
+			Ok(res) => {
+				let mon_info = MonitorUpdateInfo {
+					funding_outpoint: self.funding_txo.unwrap(),
+					latest_monitor_update_id: self.latest_monitor_update_id
+				};
+				Ok((res, mon_info))
+			},
+			Err(e) => Err(e)
+		}
 	}
 
 	pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -36,7 +36,7 @@ use bitcoin::secp256k1;
 use chain;
 use chain::Watch;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
-use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent};
+use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent};
 use chain::transaction::{OutPoint, TransactionData};
 use ln::channel::{Channel, ChannelError};
 use ln::features::{InitFeatures, NodeFeatures};
@@ -697,6 +697,19 @@ macro_rules! maybe_break_monitor_err {
 	}
 }
 
+/// When claiming a payment after receiving a preimage, there's a rare case
+/// where the channel hits the chain before the `ChannelMonitor` can be updated
+/// with knowledge of the preimage. Thus, monitor information needs to be passed
+/// to the `ChannelManager` on receipt of a preimage so it's capable of updating
+/// the relevant `ChannelMonitor` after the channel's data has been removed from
+/// the `ChannelManager`'s `ChannelHolder`.
+#[derive(Clone, PartialEq)]
+pub(crate) struct MonitorUpdateInfo {
+	pub(crate) funding_outpoint: OutPoint,
+	pub(crate) latest_monitor_update_id: u64
+}
+impl_writeable!(MonitorUpdateInfo, 0, { funding_outpoint, latest_monitor_update_id });
+
 impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<ChanSigner, M, T, K, F, L>
 	where M::Target: chain::Watch<Keys=ChanSigner>,
         T::Target: BroadcasterInterface,
@@ -2124,7 +2137,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		} else { unreachable!(); }
 	}
 
-	fn claim_funds_internal(&self, mut channel_state_lock: MutexGuard<ChannelHolder<ChanSigner>>, source: HTLCSource, payment_preimage: PaymentPreimage) {
+	fn claim_funds_internal(&self, mut channel_state_lock: MutexGuard<ChannelHolder<ChanSigner>>, source: HTLCSource, payment_preimage: PaymentPreimage, monitor_update_info: MonitorUpdateInfo) {
 		match source {
 			HTLCSource::OutboundRoute { .. } => {
 				mem::drop(channel_state_lock);
@@ -2137,9 +2150,15 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 				if let Err((counterparty_node_id, err)) = match self.claim_funds_from_hop(&mut channel_state_lock, hop_data, payment_preimage) {
 					Ok(()) => Ok(()),
 					Err(None) => {
-						// TODO: There is probably a channel monitor somewhere that needs to
-						// learn the preimage as the channel already hit the chain and that's
-						// why it's missing.
+						let preimage_update = ChannelMonitorUpdate {
+							update_id: monitor_update_info.latest_monitor_update_id,
+							updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
+								payment_preimage: payment_preimage.clone(),
+							}],
+						};
+						if let Err(_) = self.chain_monitor.update_channel(monitor_update_info.funding_outpoint, preimage_update) {
+							// TODO(val): figure out what to do here
+						}
 						Ok(())
 					},
 					Err(Some(res)) => Err(res),
@@ -2581,7 +2600,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
 	fn internal_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), MsgHandleErrInternal> {
 		let mut channel_lock = self.channel_state.lock().unwrap();
-		let htlc_source = {
+		let (htlc_source, monitor_info) = {
 			let channel_state = &mut *channel_lock;
 			match channel_state.by_id.entry(msg.channel_id) {
 				hash_map::Entry::Occupied(mut chan) => {
@@ -2593,7 +2612,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
 			}
 		};
-		self.claim_funds_internal(channel_lock, htlc_source, msg.payment_preimage.clone());
+		self.claim_funds_internal(channel_lock, htlc_source, msg.payment_preimage.clone(), monitor_info);
 		Ok(())
 	}
 
@@ -2979,7 +2998,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 					MonitorEvent::HTLCEvent(htlc_update) => {
 						if let Some(preimage) = htlc_update.payment_preimage {
 							log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
-							self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage);
+							self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage, htlc_update.monitor_info);
 						} else {
 							log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
 							self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
