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`.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -4303,6 +4303,29 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				},
 			}
 		} 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 offchain preimage event from the forward link (the
+			// event being update_fulfill_htlc).
+			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);
+			}
+			// Note that we do process the completion action here. This totally could be a
+			// duplicate claim, but we have no way of knowing without interrogating the
+			// `ChannelMonitor` we've provided the above update to. Instead, note that `Event`s are
+			// generally always allowed to be duplicative (and it's specifically noted in
+			// `PaymentForwarded`)..
 			self.handle_monitor_update_completion_actions(completion_action(None));
 			return ClaimFundsFromHop::PrevHopForceClosed
 		}
@@ -4396,31 +4419,6 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 							}})
 						} else { 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
-					// update to. Instead, we simply document in `PaymentForwarded` that this
-					// can happen.
-				}
 				mem::drop(channel_state_lock);
 				if let ClaimFundsFromHop::MonitorUpdateFail(pk, err, _) = res {
 					let result: Result<(), _> = Err(err);