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.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index b4c0a90cb3964e3cf99ae3d857ad5d4d4e04346b..57ccab1adf2f39a6b9aaa6cd1873e87cc1ffce2c 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -1138,8 +1138,11 @@ 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.
+       /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
+       /// return Ok(_) if debug assertions are turned on or 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)));
                                                }
                                        },
@@ -1261,8 +1267,11 @@ 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.
+       /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
+       /// return Ok(_) if debug assertions are turned on or 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"));
                                                }
                                        },

diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs
index 514a95d27db9339edc8b78989a0477695ae102ab..e3cea97716d35061aa34469c209eec8e094e1cba 100644 (file)
--- a/lightning/src/ln/channelmonitor.rs
+++ b/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) {