Update monitor with preimage after channel close

If the channel is hitting the chain right as we receive a preimage,
previous to this commit the relevant ChannelMonitor would never
learn of this preimage.

Author: valentinewallace

lightning/src/chain/channelmonitor.rs

@@ -64,14 +64,32 @@ pub struct ChannelMonitorUpdate {
 	pub(crate) updates: Vec<ChannelMonitorUpdateStep>,
 	/// The sequence number of this update. Updates *must* be replayed in-order according to this
 	/// sequence number (and updates may panic if they are not). The update_id values are strictly
-	/// increasing and increase by one for each new update.
+	/// increasing and increase by one for each new update, with one exception specified below.
 	///
 	/// This sequence number is also used to track up to which points updates which returned
 	/// ChannelMonitorUpdateErr::TemporaryFailure have been applied to all copies of a given
 	/// ChannelMonitor when ChannelManager::channel_monitor_updated is called.
+	///
+	/// The only instance where update_id values are not strictly increasing is the case where: (1)
+	/// the channel has been force closed and (2) we receive a preimage from a forward link that
+	/// allows us to spend an HTLC output on this channel's (the backward link's) broadcasted
+	/// commitment transaction. In this case, we allow the `ChannelManager` to send a
+	/// `ChannelMonitorUpdate` with an update_id of [`CLOSED_CHANNEL_UPDATE_ID`], with the update
+	/// providing said payment preimage. No other update types are allowed after force-close.
+	///
+	/// [`CLOSED_CHANNEL_UPDATE_ID`]: constant.CLOSED_CHANNEL_UPDATE_ID.html
 	pub update_id: u64,
 }
 
+/// If:
+///    (1) a channel has been force closed and
+///    (2) we receive a preimage from a forward link that allows us to spend an HTLC output on
+///        this channel's (the backward link's) broadcasted commitment transaction
+/// then we allow the `ChannelManager` to send a `ChannelMonitorUpdate` with this update ID,
+/// with the update providing said payment preimage. No other update types are allowed after
+/// force-close.
+pub const CLOSED_CHANNEL_UPDATE_ID: u64 = std::u64::MAX;
+
 impl Writeable for ChannelMonitorUpdate {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
 		self.update_id.write(w)?;
@@ -1166,7 +1184,18 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		where B::Target: BroadcasterInterface,
 					L::Target: Logger,
 	{
-		if self.latest_update_id + 1 != updates.update_id {
+		// ChannelMonitor updates may be applied after force close if we receive a
+		// preimage for a broadcasted commitment transaction HTLC output that we'd
+		// like to claim on-chain. If this is the case, we no longer have guaranteed
+		// access to the monitor's update ID, so we use a sentinel value instead.
+		if updates.update_id == CLOSED_CHANNEL_UPDATE_ID {
+			match updates.updates[0] {
+				ChannelMonitorUpdateStep::PaymentPreimage { .. } => {},
+				_ => panic!("Attempted to apply post-force-close ChannelMonitorUpdate that wasn't providing a payment preimage"),
+			}
+			assert_eq!(updates.updates.len(), 1);
+		}
+		if updates.update_id != CLOSED_CHANNEL_UPDATE_ID && self.latest_update_id + 1 != updates.update_id {
 			panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!");
 		}
 		for update in updates.updates.iter() {

lightning/src/ln/channelmanager.rs

@@ -37,7 +37,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, CLOSED_CHANNEL_UPDATE_ID};
 use chain::transaction::{OutPoint, TransactionData};
 use ln::channel::{Channel, ChannelError};
 use ln::features::{InitFeatures, NodeFeatures};
@@ -2152,12 +2152,22 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 				});
 			},
 			HTLCSource::PreviousHopData(hop_data) => {
+				let prev_outpoint = hop_data.outpoint;
 				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: CLOSED_CHANNEL_UPDATE_ID,
+							updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
+								payment_preimage: payment_preimage.clone(),
+							}],
+						};
+						// We update the ChannelMonitor on the backward link, after
+						// receiving an offchain preimage event from the forward link (the
+						// event being update_fulfill_htlc).
+						if let Err(e) = self.chain_monitor.update_channel(prev_outpoint, preimage_update) {
+							log_error!(self.logger, "Critical error: failed to update channel monitor with preimage: {:?}", e);
+						}
 						Ok(())
 					},
 					Err(Some(res)) => Err(res),