Merge pull request #1106 from TheBlueMatt/2021-10-no-perm-err-broadcast
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index ed84750590a1c95ac220ce9408c40135b5aca0ec..8f8cbdf448adb80d7fa50ee279c86862c1d0abd6 100644 (file)
@@ -37,7 +37,7 @@ use bitcoin::secp256k1;
 use ln::{PaymentHash, PaymentPreimage};
 use ln::msgs::DecodeError;
 use ln::chan_utils;
-use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction};
 use ln::channelmanager::HTLCSource;
 use chain;
 use chain::{BestBlock, WatchedOutput};
@@ -76,12 +76,14 @@ pub struct ChannelMonitorUpdate {
        /// increasing and increase by one for each new update, with one exception specified below.
        ///
        /// This sequence number is also used to track up to which points updates which returned
-       /// ChannelMonitorUpdateErr::TemporaryFailure have been applied to all copies of a given
+       /// [`ChannelMonitorUpdateStatus::InProgress`] have been applied to all copies of a given
        /// ChannelMonitor when ChannelManager::channel_monitor_updated is called.
        ///
        /// The only instance where update_id values are not strictly increasing is the case where we
        /// allow post-force-close updates with a special update ID of [`CLOSED_CHANNEL_UPDATE_ID`]. See
        /// its docs for more details.
+       ///
+       /// [`ChannelMonitorUpdateStatus::InProgress`]: super::ChannelMonitorUpdateStatus::InProgress
        pub update_id: u64,
 }
 
@@ -132,10 +134,10 @@ pub enum MonitorEvent {
        CommitmentTxConfirmed(OutPoint),
 
        /// Indicates a [`ChannelMonitor`] update has completed. See
-       /// [`ChannelMonitorUpdateErr::TemporaryFailure`] for more information on how this is used.
+       /// [`ChannelMonitorUpdateStatus::InProgress`] for more information on how this is used.
        ///
-       /// [`ChannelMonitorUpdateErr::TemporaryFailure`]: super::ChannelMonitorUpdateErr::TemporaryFailure
-       UpdateCompleted {
+       /// [`ChannelMonitorUpdateStatus::InProgress`]: super::ChannelMonitorUpdateStatus::InProgress
+       Completed {
                /// The funding outpoint of the [`ChannelMonitor`] that was updated
                funding_txo: OutPoint,
                /// The Update ID from [`ChannelMonitorUpdate::update_id`] which was applied or
@@ -147,15 +149,15 @@ pub enum MonitorEvent {
        },
 
        /// Indicates a [`ChannelMonitor`] update has failed. See
-       /// [`ChannelMonitorUpdateErr::PermanentFailure`] for more information on how this is used.
+       /// [`ChannelMonitorUpdateStatus::PermanentFailure`] for more information on how this is used.
        ///
-       /// [`ChannelMonitorUpdateErr::PermanentFailure`]: super::ChannelMonitorUpdateErr::PermanentFailure
+       /// [`ChannelMonitorUpdateStatus::PermanentFailure`]: super::ChannelMonitorUpdateStatus::PermanentFailure
        UpdateFailed(OutPoint),
 }
 impl_writeable_tlv_based_enum_upgradable!(MonitorEvent,
-       // Note that UpdateCompleted and UpdateFailed are currently never serialized to disk as they are
+       // Note that Completed and UpdateFailed are currently never serialized to disk as they are
        // generated only in ChainMonitor
-       (0, UpdateCompleted) => {
+       (0, Completed) => {
                (0, funding_txo, required),
                (2, monitor_update_id, required),
        },
@@ -793,7 +795,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        // of block connection between ChannelMonitors and the ChannelManager.
        funding_spend_seen: bool,
 
+       /// Set to `Some` of the confirmed transaction spending the funding input of the channel after
+       /// reaching `ANTI_REORG_DELAY` confirmations.
        funding_spend_confirmed: Option<Txid>,
+
        confirmed_commitment_tx_counterparty_output: CommitmentTxCounterpartyOutputInfo,
        /// The set of HTLCs which have been either claimed or failed on chain and have reached
        /// the requisite confirmations on the claim/fail transaction (either ANTI_REORG_DELAY or the
@@ -1311,14 +1316,20 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        }
 
        /// Used by ChannelManager deserialization to broadcast the latest holder state if its copy of
-       /// the Channel was out-of-date. You may use it to get a broadcastable holder toxic tx in case of
-       /// fallen-behind, i.e when receiving a channel_reestablish with a proof that our counterparty side knows
-       /// a higher revocation secret than the holder commitment number we are aware of. Broadcasting these
-       /// transactions are UNSAFE, as they allow counterparty side to punish you. Nevertheless you may want to
-       /// broadcast them if counterparty don't close channel with his higher commitment transaction after a
-       /// substantial amount of time (a month or even a year) to get back funds. Best may be to contact
-       /// out-of-band the other node operator to coordinate with him if option is available to you.
-       /// In any-case, choice is up to the user.
+       /// the Channel was out-of-date.
+       ///
+       /// You may also use this to broadcast the latest local commitment transaction, either because
+       /// a monitor update failed with [`ChannelMonitorUpdateStatus::PermanentFailure`] or because we've
+       /// fallen behind (i.e. we've received proof that our counterparty side knows a revocation
+       /// secret we gave them that they shouldn't know).
+       ///
+       /// Broadcasting these transactions in the second case is UNSAFE, as they allow counterparty
+       /// side to punish you. Nevertheless you may want to broadcast them if counterparty doesn't
+       /// close channel with their commitment transaction after a substantial amount of time. Best
+       /// may be to contact the other node operator out-of-band to coordinate other options available
+       /// to you. In any-case, the choice is up to you.
+       ///
+       /// [`ChannelMonitorUpdateStatus::PermanentFailure`]: super::ChannelMonitorUpdateStatus::PermanentFailure
        pub fn get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
        where L::Target: Logger {
                self.inner.lock().unwrap().get_latest_holder_commitment_txn(logger)
@@ -1630,7 +1641,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        /// confirmations on the claim transaction.
        ///
        /// Note that for `ChannelMonitors` which track a channel which went on-chain with versions of
-       /// LDK prior to 0.0.108, balances may not be fully captured if our counterparty broadcasted
+       /// LDK prior to 0.0.111, balances may not be fully captured if our counterparty broadcasted
        /// a revoked state.
        ///
        /// See [`Balance`] for additional details on the types of claimable balances which
@@ -2245,7 +2256,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        if *should_broadcast {
                                                self.broadcast_latest_holder_commitment_txn(broadcaster, logger);
                                        } else if !self.holder_tx_signed {
-                                               log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take");
+                                               log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast");
+                                               log_error!(logger, "    in channel monitor for channel {}!", log_bytes!(self.funding_info.0.to_channel_id()));
+                                               log_error!(logger, "    Read the docs for ChannelMonitor::get_latest_holder_commitment_txn and take manual action!");
                                        } else {
                                                // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
                                                // will still give us a ChannelForceClosed event with !should_broadcast, but we
@@ -2650,6 +2663,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
                let txid = commitment_tx.txid();
                let mut holder_transactions = vec![commitment_tx];
+               // When anchor outputs are present, the HTLC transactions are only valid once the commitment
+               // transaction confirms.
+               if self.onchain_tx_handler.opt_anchors() {
+                       return holder_transactions;
+               }
                for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
                        if let Some(vout) = htlc.0.transaction_output_index {
                                let preimage = if !htlc.0.offered {
@@ -2683,6 +2701,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                let commitment_tx = self.onchain_tx_handler.get_fully_signed_copy_holder_tx(&self.funding_redeemscript);
                let txid = commitment_tx.txid();
                let mut holder_transactions = vec![commitment_tx];
+               // When anchor outputs are present, the HTLC transactions are only final once the commitment
+               // transaction confirms due to the CSV 1 encumberance.
+               if self.onchain_tx_handler.opt_anchors() {
+                       return holder_transactions;
+               }
                for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
                        if let Some(vout) = htlc.0.transaction_output_index {
                                let preimage = if !htlc.0.offered {
@@ -3068,6 +3091,16 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        }
 
        fn should_broadcast_holder_commitment_txn<L: Deref>(&self, logger: &L) -> bool where L::Target: Logger {
+               // There's no need to broadcast our commitment transaction if we've seen one confirmed (even
+               // with 1 confirmation) as it'll be rejected as duplicate/conflicting.
+               if self.funding_spend_confirmed.is_some() ||
+                       self.onchain_events_awaiting_threshold_conf.iter().find(|event| match event.event {
+                               OnchainEvent::FundingSpendConfirmation { .. } => true,
+                               _ => false,
+                       }).is_some()
+               {
+                       return false;
+               }
                // We need to consider all HTLCs which are:
                //  * in any unrevoked counterparty commitment transaction, as they could broadcast said
                //    transactions and we'd end up in a race, or
@@ -3136,25 +3169,17 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        fn is_resolving_htlc_output<L: Deref>(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger {
                'outer_loop: for input in &tx.input {
                        let mut payment_data = None;
-                       let witness_items = input.witness.len();
-                       let htlctype = input.witness.last().map(|w| w.len()).and_then(HTLCType::scriptlen_to_htlctype);
-                       let prev_last_witness_len = input.witness.second_to_last().map(|w| w.len()).unwrap_or(0);
-                       let revocation_sig_claim = (witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && prev_last_witness_len == 33)
-                               || (witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && prev_last_witness_len == 33);
-                       let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC)
-                               && input.witness.second_to_last().unwrap().len() == 32;
+                       let htlc_claim = HTLCClaim::from_witness(&input.witness);
+                       let revocation_sig_claim = htlc_claim == Some(HTLCClaim::Revocation);
+                       let accepted_preimage_claim = htlc_claim == Some(HTLCClaim::AcceptedPreimage);
                        #[cfg(not(fuzzing))]
-                       let accepted_timeout_claim = witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim;
-                       let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) &&
-                               !revocation_sig_claim && input.witness.second_to_last().unwrap().len() == 32;
-
+                       let accepted_timeout_claim = htlc_claim == Some(HTLCClaim::AcceptedTimeout);
+                       let offered_preimage_claim = htlc_claim == Some(HTLCClaim::OfferedPreimage);
                        #[cfg(not(fuzzing))]
-                       let offered_timeout_claim = witness_items == 5 && htlctype == Some(HTLCType::OfferedHTLC);
+                       let offered_timeout_claim = htlc_claim == Some(HTLCClaim::OfferedTimeout);
 
                        let mut payment_preimage = PaymentPreimage([0; 32]);
-                       if accepted_preimage_claim {
-                               payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap());
-                       } else if offered_preimage_claim {
+                       if offered_preimage_claim || accepted_preimage_claim {
                                payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap());
                        }
 
@@ -3752,8 +3777,7 @@ mod tests {
        use ln::{PaymentPreimage, PaymentHash};
        use ln::chan_utils;
        use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
-       use ln::channelmanager::PaymentSendFailure;
-       use ln::features::InitFeatures;
+       use ln::channelmanager::{self, PaymentSendFailure};
        use ln::functional_test_utils::*;
        use ln::script::ShutdownScript;
        use util::errors::APIError;
@@ -3782,9 +3806,9 @@ mod tests {
                let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
                let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
                let channel = create_announced_chan_between_nodes(
-                       &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+                       &nodes, 0, 1, channelmanager::provided_init_features(), channelmanager::provided_init_features());
                create_announced_chan_between_nodes(
-                       &nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
+                       &nodes, 1, 2, channelmanager::provided_init_features(), channelmanager::provided_init_features());
 
                // Rebalance somewhat
                send_payment(&nodes[0], &[&nodes[1]], 10_000_000);