Handle double-HTLC-claims without failing the backwards channel
[rust-lightning] / lightning / src / ln / channel.rs
index 92a9ab6ab9ad4cd19836ddd8e66ecfc66c5104de..6d95525773445363361f5c6b5ba0d9f5da499d9f 100644 (file)
@@ -26,9 +26,10 @@ use ln::{PaymentPreimage, PaymentHash};
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
-use ln::channelmanager::{BestBlock, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor};
 use ln::chan_utils;
+use chain::BestBlock;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use chain::transaction::{OutPoint, TransactionData};
@@ -443,6 +444,15 @@ pub(super) struct Channel<Signer: Sign> {
        ///
        /// See-also <https://github.com/lightningnetwork/lnd/issues/4006>
        pub workaround_lnd_bug_4006: Option<msgs::FundingLocked>,
+
+       #[cfg(any(test, feature = "fuzztarget"))]
+       // When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the
+       // corresponding HTLC on the inbound path. If, then, the outbound path channel is
+       // disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack
+       // messages), they may re-broadcast their update_fulfill_htlc, causing a duplicate claim. This
+       // is fine, but as a sanity check in our failure to generate the second claim, we check here
+       // that the original was a claim, and that we aren't now trying to fulfill a failed HTLC.
+       historical_inbound_htlc_fulfills: HashSet<u64>,
 }
 
 #[cfg(any(test, feature = "fuzztarget"))]
@@ -644,6 +654,9 @@ impl<Signer: Sign> Channel<Signer> {
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
+
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       historical_inbound_htlc_fulfills: HashSet::new(),
                })
        }
 
@@ -889,6 +902,9 @@ impl<Signer: Sign> Channel<Signer> {
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
+
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       historical_inbound_htlc_fulfills: HashSet::new(),
                };
 
                Ok(chan)
@@ -1248,8 +1264,8 @@ impl<Signer: Sign> Channel<Signer> {
                                                if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
                                                } else {
                                                        log_warn!(logger, "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 failed");
                                                }
-                                               debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
                                                return Ok((None, None));
                                        },
                                        _ => {
@@ -1262,7 +1278,11 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                if pending_idx == core::usize::MAX {
-                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       // If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and
+                       // this is simply a duplicate claim, not previously failed and we lost funds.
+                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                       return Ok((None, None));
                }
 
                // Now update local state:
@@ -1284,7 +1304,8 @@ impl<Signer: Sign> Channel<Signer> {
                                                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");
+                                                       #[cfg(any(test, feature = "fuzztarget"))]
+                                                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
                                                        return Ok((None, None));
                                                }
                                        },
@@ -1304,8 +1325,12 @@ impl<Signer: Sign> Channel<Signer> {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
                                payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
                        return Ok((None, Some(monitor_update)));
                }
+               #[cfg(any(test, feature = "fuzztarget"))]
+               self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
 
                {
                        let htlc = &mut self.pending_inbound_htlcs[pending_idx];
@@ -1365,8 +1390,11 @@ impl<Signer: Sign> Channel<Signer> {
                        if htlc.htlc_id == htlc_id_arg {
                                match htlc.state {
                                        InboundHTLCState::Committed => {},
-                                       InboundHTLCState::LocalRemoved(_) => {
-                                               debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
+                                       InboundHTLCState::LocalRemoved(ref reason) => {
+                                               if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+                                               } else {
+                                                       debug_assert!(false, "Tried to fail an HTLC that was already failed");
+                                               }
                                                return Ok(None);
                                        },
                                        _ => {
@@ -1378,7 +1406,11 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                if pending_idx == core::usize::MAX {
-                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       // If we failed to find an HTLC to fail, make sure it was previously fulfilled and this
+                       // is simply a duplicate fail, not previously failed and we failed-back too early.
+                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                       return Ok(None);
                }
 
                // Now update local state:
@@ -1387,8 +1419,9 @@ impl<Signer: Sign> Channel<Signer> {
                                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".to_owned()));
+                                                       #[cfg(any(test, feature = "fuzztarget"))]
+                                                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                                                       return Ok(None);
                                                }
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
@@ -1784,8 +1817,22 @@ impl<Signer: Sign> Channel<Signer> {
        /// corner case properly.
        pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) {
                // Note that we have to handle overflow due to the above case.
-               (cmp::max(self.channel_value_satoshis as i64 * 1000 - self.value_to_self_msat as i64 - self.get_inbound_pending_htlc_stats().1 as i64, 0) as u64,
-               cmp::max(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64)
+               (
+                       cmp::max(self.channel_value_satoshis as i64 * 1000
+                               - self.value_to_self_msat as i64
+                               - self.get_inbound_pending_htlc_stats().1 as i64
+                               - Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000,
+                       0) as u64,
+                       cmp::max(self.value_to_self_msat as i64
+                               - self.get_outbound_pending_htlc_stats().1 as i64
+                               - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000,
+                       0) as u64
+               )
+       }
+
+       pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option<u64>) {
+               (Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
+               self.counterparty_selected_channel_reserve_satoshis)
        }
 
        // Get the fee cost of a commitment tx with a given number of HTLC outputs.
@@ -2438,7 +2485,14 @@ impl<Signer: Sign> Channel<Signer> {
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
                                                match self.get_update_fail_htlc(htlc_id, err_packet.clone(), logger) {
-                                                       Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()),
+                                                       Ok(update_fail_msg_option) => {
+                                                               // If an HTLC failure was previously added to the holding cell (via
+                                                               // `get_update_fail_htlc`) then generating the fail message itself
+                                                               // must not fail - we should never end up in a state where we
+                                                               // double-fail an HTLC or fail-then-claim an HTLC as it indicates
+                                                               // we didn't wait for a full revocation before failing.
+                                                               update_fail_htlcs.push(update_fail_msg_option.unwrap())
+                                                       },
                                                        Err(e) => {
                                                                if let ChannelError::Ignore(_) = e {}
                                                                else {
@@ -3338,6 +3392,10 @@ impl<Signer: Sign> Channel<Signer> {
                self.channel_id
        }
 
+       pub fn minimum_depth(&self) -> Option<u32> {
+               self.minimum_depth
+       }
+
        /// Gets the "user_id" value passed into the construction of this channel. It has no special
        /// meaning and exists only to allow users to have a persistent identifier of a channel.
        pub fn get_user_id(&self) -> u64 {
@@ -3365,7 +3423,7 @@ impl<Signer: Sign> Channel<Signer> {
                &self.channel_transaction_parameters.holder_pubkeys
        }
 
-       fn get_counterparty_selected_contest_delay(&self) -> Option<u16> {
+       pub fn get_counterparty_selected_contest_delay(&self) -> Option<u16> {
                self.channel_transaction_parameters.counterparty_parameters
                        .as_ref().map(|params| params.selected_contest_delay)
        }
@@ -4671,6 +4729,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
                self.channel_update_status.write(writer)?;
 
+               #[cfg(any(test, feature = "fuzztarget"))]
+               (self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?;
+               #[cfg(any(test, feature = "fuzztarget"))]
+               for htlc in self.historical_inbound_htlc_fulfills.iter() {
+                       htlc.write(writer)?;
+               }
+
                write_tlv_fields!(writer, {
                        (0, self.announcement_sigs, option),
                        // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -4863,6 +4928,16 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
                let channel_update_status = Readable::read(reader)?;
 
+               #[cfg(any(test, feature = "fuzztarget"))]
+               let mut historical_inbound_htlc_fulfills = HashSet::new();
+               #[cfg(any(test, feature = "fuzztarget"))]
+               {
+                       let htlc_fulfills_len: u64 = Readable::read(reader)?;
+                       for _ in 0..htlc_fulfills_len {
+                               assert!(historical_inbound_htlc_fulfills.insert(Readable::read(reader)?));
+                       }
+               }
+
                let mut announcement_sigs = None;
                read_tlv_fields!(reader, {
                        (0, announcement_sigs, option),
@@ -4954,6 +5029,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
+
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       historical_inbound_htlc_fulfills,
                })
        }
 }
@@ -4970,13 +5048,14 @@ mod tests {
        use bitcoin::hashes::hex::FromHex;
        use hex;
        use ln::{PaymentPreimage, PaymentHash};
-       use ln::channelmanager::{BestBlock, HTLCSource};
+       use ln::channelmanager::HTLCSource;
        use ln::channel::{Channel,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys};
        use ln::channel::MAX_FUNDING_SATOSHIS;
        use ln::features::InitFeatures;
        use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate};
        use ln::chan_utils;
        use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT};
+       use chain::BestBlock;
        use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
        use chain::keysinterface::{InMemorySigner, KeysInterface, BaseSign};
        use chain::transaction::OutPoint;