From 7b77a016b58face4a2d8a97edae84f2777094ee3 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 6 Dec 2022 20:46:02 +0000
Subject: [PATCH] Handle closed-chan HTLC claims in `claim_funds_from_hop`

Currently `claim_funds` does all HTLC claims in one `channel_state`
lock, ensuring that we always make claims from channels which are
open. It can thus avoid ever having to generate a
`ChannelMonitorUpdate` containing a preimage for a closed channel,
which we only do in `claim_funds_internal` (for forwarded payments).

In the next commit we'll change the locking of
`claim_funds_from_hop` so that `claim_funds` is no longer under a
single lock but takes a lock for each claim. This allows us to be
more flexible with locks going forward, and ultimately isn't a huge
change - if our counterparty intends to force-close a channel, us
choosing to ignore it by holding the `channel_state` lock for the
duration of the claim isn't going to result in a commitment update,
it will just result in the preimage already being in the
`ChannelMonitor`.
---
 lightning/src/ln/channelmanager.rs | 39 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index c4a4d8b3..95d11ebc 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -4454,7 +4454,26 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					return ClaimFundsFromHop::MonitorUpdateFail(counterparty_node_id, res, None);
 				},
 			}
-		} else { return ClaimFundsFromHop::PrevHopForceClosed }
+		} else {
+			let preimage_update = ChannelMonitorUpdate {
+				update_id: CLOSED_CHANNEL_UPDATE_ID,
+				updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
+					payment_preimage,
+				}],
+			};
+			// We update the ChannelMonitor on the backward link, after
+			// receiving an `update_fulfill_htlc` from the forward link.
+			let update_res = self.chain_monitor.update_channel(prev_hop.outpoint, preimage_update);
+			if update_res != ChannelMonitorUpdateStatus::Completed {
+				// TODO: This needs to be handled somehow - if we receive a monitor update
+				// with a preimage we *must* somehow manage to propagate it to the upstream
+				// channel, or we must have an ability to receive the same event and try
+				// again on restart.
+				log_error!(self.logger, "Critical error: failed to update channel monitor with preimage {:?}: {:?}",
+					payment_preimage, update_res);
+			}
+			return ClaimFundsFromHop::PrevHopForceClosed
+		}
 	}
 
 	fn finalize_claims(&self, mut sources: Vec<HTLCSource>) {
@@ -4535,24 +4554,6 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					_ => None,
 				};
 				if let ClaimFundsFromHop::PrevHopForceClosed = res {
-					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).
-					let update_res = self.chain_monitor.update_channel(prev_outpoint, preimage_update);
-					if update_res != ChannelMonitorUpdateStatus::Completed {
-						// TODO: This needs to be handled somehow - if we receive a monitor update
-						// with a preimage we *must* somehow manage to propagate it to the upstream
-						// channel, or we must have an ability to receive the same event and try
-						// again on restart.
-						log_error!(self.logger, "Critical error: failed to update channel monitor with preimage {:?}: {:?}",
-							payment_preimage, update_res);
-					}
 					// Note that we do *not* set `claimed_htlc` to false here. In fact, this
 					// totally could be a duplicate claim, but we have no way of knowing
 					// without interrogating the `ChannelMonitor` we've provided the above
-- 
2.30.2