Deduplicate HTLC preimage events from channelmonitor.

This avoids calling get_update_fulfill_htlc_and_commit twice for
the same HTLC if we have to rescan a block.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -1140,6 +1140,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
 	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
 	/// Ok(_) if debug assertions are turned on and preconditions are met.
+	///
+	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
+	/// but then have a reorg which settles on an HTLC-failure on chain.
 	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 		// Either ChannelFunded got set (which means it won't be unset) or there is no way any
 		// caller thought we could have something claimed (cause we wouldn't have accepted in an
@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 						} else {
 							log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 						}
+						debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 						return Ok((None, None));
 					},
 					_ => {
@@ -1202,6 +1206,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 						if htlc_id_arg == htlc_id {
 							// Make sure we don't leave latest_monitor_update_id incremented here:
 							self.latest_monitor_update_id -= 1;
+							debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 							return Ok((None, None));
 						}
 					},
@@ -1210,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 							log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 							// TODO: We may actually be able to switch to a fulfill here, though its
 							// rare enough it may not be worth the complexity burden.
+							debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 							return Ok((None, Some(monitor_update)));
 						}
 					},
@@ -1263,6 +1269,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
 	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
 	/// Ok(_) if debug assertions are turned on and preconditions are met.
+	///
+	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
+	/// but then have a reorg which settles on an HTLC-failure on chain.
 	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 		if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 			panic!("Was asked to fail an HTLC when channel was not in an operational state");
@@ -1279,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				match htlc.state {
 					InboundHTLCState::Committed => {},
 					InboundHTLCState::LocalRemoved(_) => {
+						debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 						return Ok(None);
 					},
 					_ => {
@@ -1299,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				match pending_update {
 					&HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 						if htlc_id_arg == htlc_id {
+							debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 							return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 						}
 					},
 					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 						if htlc_id_arg == htlc_id {
+							debug_assert!(false, "Tried to fail an HTLC that was already failed");
 							return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 						}
 					},

lightning/src/ln/channelmonitor.rs

@@ -2278,19 +2278,23 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			if let Some((source, payment_hash)) = payment_data {
 				let mut payment_preimage = PaymentPreimage([0; 32]);
 				if accepted_preimage_claim {
-					payment_preimage.0.copy_from_slice(&input.witness[3]);
-					self.pending_htlcs_updated.push(HTLCUpdate {
-						source,
-						payment_preimage: Some(payment_preimage),
-						payment_hash
-					});
+					if !self.pending_htlcs_updated.iter().any(|update| update.source == source) {
+						payment_preimage.0.copy_from_slice(&input.witness[3]);
+						self.pending_htlcs_updated.push(HTLCUpdate {
+							source,
+							payment_preimage: Some(payment_preimage),
+							payment_hash
+						});
+					}
 				} else if offered_preimage_claim {
-					payment_preimage.0.copy_from_slice(&input.witness[1]);
-					self.pending_htlcs_updated.push(HTLCUpdate {
-						source,
-						payment_preimage: Some(payment_preimage),
-						payment_hash
-					});
+					if !self.pending_htlcs_updated.iter().any(|update| update.source == source) {
+						payment_preimage.0.copy_from_slice(&input.witness[1]);
+						self.pending_htlcs_updated.push(HTLCUpdate {
+							source,
+							payment_preimage: Some(payment_preimage),
+							payment_hash
+						});
+					}
 				} else {
 					log_info!(self, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height{})", log_bytes!(payment_hash.0), height + ANTI_REORG_DELAY - 1);
 					match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) {