Cancel claims signed by a remote ChannelMonitor when reorging

In `ChannelMonitorImpl::cancel_prev_commitment_claims` we need to
cancel any claims against a removed commitment transaction. We
were checking if `holder_tx_signed` before checking if either the
current or previous holder commitment transaction had pending
claims against it, but (a) there's no need to do this, there's not
a big performance cost to just always trying to remove claims and
(b) we can't actually rely on `holder_tx_signed`.

`holder_tx_signed` being set doesn't necessarily imply that the
`ChannelMonitor` was persisted (i.e. it may simply be lost in a
poorly-timed restart) but we also (somewhat theoretically) allow
for multiple copies of a `ChannelMonitor` to exist, and a different
one could have signed the commitment transaction which was
confirmed (and then unconfirmed).

Thus, we simply remove the additional check here.

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -3915,35 +3915,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				}
 			}
 		}
-		if self.holder_tx_signed {
-			// If we've signed, we may have broadcast either commitment (prev or current), and
-			// attempted to claim from it immediately without waiting for a confirmation.
-			if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+		// Cancel any pending claims for any holder commitments in case they had previously
+		// confirmed or been signed (in which case we will start attempting to claim without
+		// waiting for confirmation).
+		if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+			log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+				self.current_holder_commitment_tx.txid);
+			let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+			for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+				if let Some(vout) = htlc.transaction_output_index {
+					outpoint.vout = vout;
+					self.onchain_tx_handler.abandon_claim(&outpoint);
+				}
+			}
+		}
+		if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+			if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
 				log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-					self.current_holder_commitment_tx.txid);
-				let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
-				for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+					prev_holder_commitment_tx.txid);
+				let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+				for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
 					if let Some(vout) = htlc.transaction_output_index {
 						outpoint.vout = vout;
 						self.onchain_tx_handler.abandon_claim(&outpoint);
 					}
 				}
 			}
-			if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
-				if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
-					log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-						prev_holder_commitment_tx.txid);
-					let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
-					for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
-						if let Some(vout) = htlc.transaction_output_index {
-							outpoint.vout = vout;
-							self.onchain_tx_handler.abandon_claim(&outpoint);
-						}
-					}
-				}
-			}
-		} else {
-			// No previous claim.
 		}
 	}