Minimize reads to counterparty_claimable_outpoints
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index c2d3a43b1f4bd40d65ea041478313a878d49d7ca..7a6551babca943af7aeeb8dcd265cf860bdd0b38 100644 (file)
@@ -34,7 +34,7 @@ use bitcoin::secp256k1;
 use bitcoin::sighash::EcdsaSighashType;
 
 use crate::ln::channel::INITIAL_COMMITMENT_NUMBER;
-use crate::ln::{PaymentHash, PaymentPreimage, ChannelId};
+use crate::ln::types::{PaymentHash, PaymentPreimage, ChannelId};
 use crate::ln::msgs::DecodeError;
 use crate::ln::channel_keys::{DelayedPaymentKey, DelayedPaymentBasepoint, HtlcBasepoint, HtlcKey, RevocationKey, RevocationBasepoint};
 use crate::ln::chan_utils::{self,CommitmentTransaction, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction, TxCreationKeys};
@@ -43,7 +43,7 @@ use crate::chain;
 use crate::chain::{BestBlock, WatchedOutput};
 use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
+use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::EcdsaChannelSigner, SignerProvider, EntropySource};
 use crate::chain::onchaintx::{ClaimEvent, FeerateStrategy, OnchainTxHandler};
 use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
 use crate::chain::Filter;
@@ -774,14 +774,14 @@ impl Readable for IrrevocablyResolvedHTLC {
 /// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
 /// returned block hash and the the current chain and then reconnecting blocks to get to the
 /// best chain) upon deserializing the object!
-pub struct ChannelMonitor<Signer: WriteableEcdsaChannelSigner> {
+pub struct ChannelMonitor<Signer: EcdsaChannelSigner> {
        #[cfg(test)]
        pub(crate) inner: Mutex<ChannelMonitorImpl<Signer>>,
        #[cfg(not(test))]
        pub(super) inner: Mutex<ChannelMonitorImpl<Signer>>,
 }
 
-impl<Signer: WriteableEcdsaChannelSigner> Clone for ChannelMonitor<Signer> where Signer: Clone {
+impl<Signer: EcdsaChannelSigner> Clone for ChannelMonitor<Signer> where Signer: Clone {
        fn clone(&self) -> Self {
                let inner = self.inner.lock().unwrap().clone();
                ChannelMonitor::from_impl(inner)
@@ -789,7 +789,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Clone for ChannelMonitor<Signer> where
 }
 
 #[derive(Clone, PartialEq)]
-pub(crate) struct ChannelMonitorImpl<Signer: WriteableEcdsaChannelSigner> {
+pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
        latest_update_id: u64,
        commitment_transaction_number_obscure_factor: u64,
 
@@ -935,12 +935,15 @@ pub(crate) struct ChannelMonitorImpl<Signer: WriteableEcdsaChannelSigner> {
        /// Ordering of tuple data: (their_per_commitment_point, feerate_per_kw, to_broadcaster_sats,
        /// to_countersignatory_sats)
        initial_counterparty_commitment_info: Option<(PublicKey, u32, u64, u64)>,
+
+       /// The first block height at which we had no remaining claimable balances.
+       balances_empty_height: Option<u32>,
 }
 
 /// Transaction outputs to watch for on-chain spends.
 pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>);
 
-impl<Signer: WriteableEcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> where Signer: PartialEq {
+impl<Signer: EcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> where Signer: PartialEq {
        fn eq(&self, other: &Self) -> bool {
                // We need some kind of total lockorder. Absent a better idea, we sort by position in
                // memory and take locks in that order (assuming that we can't move within memory while a
@@ -952,7 +955,7 @@ impl<Signer: WriteableEcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> w
        }
 }
 
-impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitor<Signer> {
+impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitor<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                self.inner.lock().unwrap().write(writer)
        }
@@ -962,7 +965,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitor<Signer> {
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
-impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
+impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
@@ -1145,6 +1148,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
                        (15, self.counterparty_fulfilled_htlcs, required),
                        (17, self.initial_counterparty_commitment_info, option),
                        (19, self.channel_id, required),
+                       (21, self.balances_empty_height, option),
                });
 
                Ok(())
@@ -1191,31 +1195,33 @@ pub(crate) struct WithChannelMonitor<'a, L: Deref> where L::Target: Logger {
        logger: &'a L,
        peer_id: Option<PublicKey>,
        channel_id: Option<ChannelId>,
+       payment_hash: Option<PaymentHash>,
 }
 
 impl<'a, L: Deref> Logger for WithChannelMonitor<'a, L> where L::Target: Logger {
        fn log(&self, mut record: Record) {
                record.peer_id = self.peer_id;
                record.channel_id = self.channel_id;
+               record.payment_hash = self.payment_hash;
                self.logger.log(record)
        }
 }
 
 impl<'a, L: Deref> WithChannelMonitor<'a, L> where L::Target: Logger {
-       pub(crate) fn from<S: WriteableEcdsaChannelSigner>(logger: &'a L, monitor: &ChannelMonitor<S>) -> Self {
-               Self::from_impl(logger, &*monitor.inner.lock().unwrap())
+       pub(crate) fn from<S: EcdsaChannelSigner>(logger: &'a L, monitor: &ChannelMonitor<S>, payment_hash: Option<PaymentHash>) -> Self {
+               Self::from_impl(logger, &*monitor.inner.lock().unwrap(), payment_hash)
        }
 
-       pub(crate) fn from_impl<S: WriteableEcdsaChannelSigner>(logger: &'a L, monitor_impl: &ChannelMonitorImpl<S>) -> Self {
+       pub(crate) fn from_impl<S: EcdsaChannelSigner>(logger: &'a L, monitor_impl: &ChannelMonitorImpl<S>, payment_hash: Option<PaymentHash>) -> Self {
                let peer_id = monitor_impl.counterparty_node_id;
                let channel_id = Some(monitor_impl.channel_id());
                WithChannelMonitor {
-                       logger, peer_id, channel_id,
+                       logger, peer_id, channel_id, payment_hash,
                }
        }
 }
 
-impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
+impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
        /// For lockorder enforcement purposes, we need to have a single site which constructs the
        /// `inner` mutex, otherwise cases where we lock two monitors at the same time (eg in our
        /// PartialEq implementation) we may decide a lockorder violation has occurred.
@@ -1328,6 +1334,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                        best_block,
                        counterparty_node_id: Some(counterparty_node_id),
                        initial_counterparty_commitment_info: None,
+                       balances_empty_height: None,
                })
        }
 
@@ -1351,7 +1358,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        where L::Target: Logger
        {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.provide_initial_counterparty_commitment_tx(txid,
                        htlc_outputs, commitment_number, their_cur_per_commitment_point, feerate_per_kw,
                        to_broadcaster_value_sat, to_countersignatory_value_sat, &logger);
@@ -1371,7 +1378,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                logger: &L,
        ) where L::Target: Logger {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.provide_latest_counterparty_commitment_tx(
                        txid, htlc_outputs, commitment_number, their_per_commitment_point, &logger)
        }
@@ -1399,7 +1406,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                L::Target: Logger,
        {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, Some(*payment_hash));
                inner.provide_payment_preimage(
                        payment_hash, payment_preimage, broadcaster, fee_estimator, &logger)
        }
@@ -1421,7 +1428,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                L::Target: Logger,
        {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.update_monitor(updates, broadcaster, fee_estimator, &logger)
        }
 
@@ -1456,7 +1463,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                F::Target: chain::Filter, L::Target: Logger,
        {
                let lock = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*lock);
+               let logger = WithChannelMonitor::from_impl(logger, &*lock, None);
                log_trace!(&logger, "Registering funding outpoint {}", &lock.get_funding_txo().0);
                filter.register_tx(&lock.get_funding_txo().0.txid, &lock.get_funding_txo().1);
                for (txid, outputs) in lock.get_outputs_to_watch().iter() {
@@ -1616,7 +1623,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        {
                let mut inner = self.inner.lock().unwrap();
                let fee_estimator = LowerBoundedFeeEstimator::new(&**fee_estimator);
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &fee_estimator, &logger);
        }
 
@@ -1627,7 +1634,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        pub fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
        where L::Target: Logger {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.unsafe_get_latest_holder_commitment_txn(&logger)
        }
 
@@ -1657,7 +1664,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                L::Target: Logger,
        {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.block_connected(
                        header, txdata, height, broadcaster, fee_estimator, &logger)
        }
@@ -1677,7 +1684,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                L::Target: Logger,
        {
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.block_disconnected(
                        header, height, broadcaster, fee_estimator, &logger)
        }
@@ -1705,7 +1712,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        {
                let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.transactions_confirmed(
                        header, txdata, height, broadcaster, &bounded_fee_estimator, &logger)
        }
@@ -1729,7 +1736,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        {
                let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.transaction_unconfirmed(
                        txid, broadcaster, &bounded_fee_estimator, &logger
                );
@@ -1757,7 +1764,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        {
                let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                inner.best_block_updated(
                        header, height, broadcaster, &bounded_fee_estimator, &logger
                )
@@ -1797,7 +1804,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        {
                let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                let current_height = inner.best_block.height;
                inner.onchain_tx_handler.rebroadcast_pending_claims(
                        current_height, FeerateStrategy::HighestOfPreviousOrNew, &broadcaster, &fee_estimator, &logger,
@@ -1816,7 +1823,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        {
                let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                let mut inner = self.inner.lock().unwrap();
-               let logger = WithChannelMonitor::from_impl(logger, &*inner);
+               let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
                let current_height = inner.best_block.height;
                inner.onchain_tx_handler.rebroadcast_pending_claims(
                        current_height, FeerateStrategy::RetryPrevious, &broadcaster, &fee_estimator, &logger,
@@ -1856,6 +1863,55 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                spendable_outputs
        }
 
+       /// Checks if the monitor is fully resolved. Resolved monitor is one that has claimed all of
+       /// its outputs and balances (i.e. [`Self::get_claimable_balances`] returns an empty set).
+       ///
+       /// This function returns true only if [`Self::get_claimable_balances`] has been empty for at least
+       /// 4032 blocks as an additional protection against any bugs resulting in spuriously empty balance sets.
+       pub fn is_fully_resolved<L: Logger>(&self, logger: &L) -> bool {
+               let mut is_all_funds_claimed = self.get_claimable_balances().is_empty();
+               let current_height = self.current_best_block().height;
+               let mut inner = self.inner.lock().unwrap();
+
+               if is_all_funds_claimed {
+                       if !inner.funding_spend_seen {
+                               debug_assert!(false, "We should see funding spend by the time a monitor clears out");
+                               is_all_funds_claimed = false;
+                       }
+               }
+
+               const BLOCKS_THRESHOLD: u32 = 4032; // ~four weeks
+               match (inner.balances_empty_height, is_all_funds_claimed) {
+                       (Some(balances_empty_height), true) => {
+                               // Claimed all funds, check if reached the blocks threshold.
+                               return current_height >= balances_empty_height + BLOCKS_THRESHOLD;
+                       },
+                       (Some(_), false) => {
+                               // previously assumed we claimed all funds, but we have new funds to claim.
+                               // Should not happen in practice.
+                               debug_assert!(false, "Thought we were done claiming funds, but claimable_balances now has entries");
+                               log_error!(logger,
+                                       "WARNING: LDK thought it was done claiming all the available funds in the ChannelMonitor for channel {}, but later decided it had more to claim. This is potentially an important bug in LDK, please report it at https://github.com/lightningdevkit/rust-lightning/issues/new",
+                                       inner.get_funding_txo().0);
+                               inner.balances_empty_height = None;
+                               false
+                       },
+                       (None, true) => {
+                               // Claimed all funds but `balances_empty_height` is None. It is set to the
+                               // current block height.
+                               log_debug!(logger,
+                                       "ChannelMonitor funded at {} is now fully resolved. It will become archivable in {} blocks",
+                                       inner.get_funding_txo().0, BLOCKS_THRESHOLD);
+                               inner.balances_empty_height = Some(current_height);
+                               false
+                       },
+                       (None, false) => {
+                               // Have funds to claim.
+                               false
+                       },
+               }
+       }
+
        #[cfg(test)]
        pub fn get_counterparty_payment_script(&self) -> ScriptBuf {
                self.inner.lock().unwrap().counterparty_payment_script.clone()
@@ -1873,7 +1929,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        }
 }
 
-impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
+impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        /// Helper for get_claimable_balances which does the work for an individual HTLC, generating up
        /// to one `Balance` for the HTLC.
        fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, holder_commitment: bool,
@@ -2052,7 +2108,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        }
 }
 
-impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
+impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
        /// Gets the balances in this channel which are either claimable by us if we were to
        /// force-close the channel now or which are claimable on-chain (possibly awaiting
        /// confirmation).
@@ -2374,8 +2430,8 @@ macro_rules! fail_unbroadcast_htlcs {
                debug_assert_eq!($commitment_tx_confirmed.txid(), $commitment_txid_confirmed);
 
                macro_rules! check_htlc_fails {
-                       ($txid: expr, $commitment_tx: expr) => {
-                               if let Some(ref latest_outpoints) = $self.counterparty_claimable_outpoints.get($txid) {
+                       ($txid: expr, $commitment_tx: expr, $per_commitment_outpoints: expr) => {
+                               if let Some(ref latest_outpoints) = $per_commitment_outpoints {
                                        for &(ref htlc, ref source_option) in latest_outpoints.iter() {
                                                if let &Some(ref source) = source_option {
                                                        // Check if the HTLC is present in the commitment transaction that was
@@ -2435,10 +2491,10 @@ macro_rules! fail_unbroadcast_htlcs {
                        }
                }
                if let Some(ref txid) = $self.current_counterparty_commitment_txid {
-                       check_htlc_fails!(txid, "current");
+                       check_htlc_fails!(txid, "current", $self.counterparty_claimable_outpoints.get(txid));
                }
                if let Some(ref txid) = $self.prev_counterparty_commitment_txid {
-                       check_htlc_fails!(txid, "previous");
+                       check_htlc_fails!(txid, "previous", $self.counterparty_claimable_outpoints.get(txid));
                }
        } }
 }
@@ -2464,7 +2520,7 @@ pub fn deliberately_bogus_accepted_htlc_witness() -> Vec<Vec<u8>> {
        vec![Vec::new(), Vec::new(), Vec::new(), Vec::new(), deliberately_bogus_accepted_htlc_witness_program().into()].into()
 }
 
-impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
+impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
        /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
        /// counterparty commitment transaction's secret, they are de facto pruned (we can use revocation key).
@@ -2707,15 +2763,15 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                // If the channel is force closed, try to claim the output from this preimage.
                // First check if a counterparty commitment transaction has been broadcasted:
                macro_rules! claim_htlcs {
-                       ($commitment_number: expr, $txid: expr) => {
-                               let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None);
+                       ($commitment_number: expr, $txid: expr, $htlcs: expr) => {
+                               let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None, $htlcs);
                                self.onchain_tx_handler.update_claims_view_from_requests(htlc_claim_reqs, self.best_block.height, self.best_block.height, broadcaster, fee_estimator, logger);
                        }
                }
                if let Some(txid) = self.current_counterparty_commitment_txid {
                        if txid == confirmed_spend_txid {
                                if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
-                                       claim_htlcs!(*commitment_number, txid);
+                                       claim_htlcs!(*commitment_number, txid, self.counterparty_claimable_outpoints.get(&txid));
                                } else {
                                        debug_assert!(false);
                                        log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
@@ -2726,7 +2782,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                if let Some(txid) = self.prev_counterparty_commitment_txid {
                        if txid == confirmed_spend_txid {
                                if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
-                                       claim_htlcs!(*commitment_number, txid);
+                                       claim_htlcs!(*commitment_number, txid, self.counterparty_claimable_outpoints.get(&txid));
                                } else {
                                        debug_assert!(false);
                                        log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
@@ -3223,8 +3279,8 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        }
 
                        // Then, try to find revoked htlc outputs
-                       if let Some(ref per_commitment_data) = per_commitment_option {
-                               for (_, &(ref htlc, _)) in per_commitment_data.iter().enumerate() {
+                       if let Some(per_commitment_claimable_data) = per_commitment_option {
+                               for (htlc, _) in per_commitment_claimable_data {
                                        if let Some(transaction_output_index) = htlc.transaction_output_index {
                                                if transaction_output_index as usize >= tx.output.len() ||
                                                                tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
@@ -3248,9 +3304,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                }
                                self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
-                               if let Some(per_commitment_data) = per_commitment_option {
+                               if let Some(per_commitment_claimable_data) = per_commitment_option {
                                        fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, tx, height,
-                                               block_hash, per_commitment_data.iter().map(|(htlc, htlc_source)|
+                                               block_hash, per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
                                                        (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
                                                ), logger);
                                } else {
@@ -3263,7 +3319,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                block_hash, [].iter().map(|reference| *reference), logger);
                                }
                        }
-               } else if let Some(per_commitment_data) = per_commitment_option {
+               } else if let Some(per_commitment_claimable_data) = per_commitment_option {
                        // While this isn't useful yet, there is a potential race where if a counterparty
                        // revokes a state at the same time as the commitment transaction for that state is
                        // confirmed, and the watchtower receives the block before the user, the user could
@@ -3278,12 +3334,11 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
                        log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
                        fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, tx, height, block_hash,
-                               per_commitment_data.iter().map(|(htlc, htlc_source)|
+                               per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
                                        (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
                                ), logger);
-
                        let (htlc_claim_reqs, counterparty_output_info) =
-                               self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx));
+                               self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option);
                        to_counterparty_output_info = counterparty_output_info;
                        for req in htlc_claim_reqs {
                                claimable_outpoints.push(req);
@@ -3294,12 +3349,12 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        }
 
        /// Returns the HTLC claim package templates and the counterparty output info
-       fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>)
+       fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>)
        -> (Vec<PackageTemplate>, CommitmentTxCounterpartyOutputInfo) {
                let mut claimable_outpoints = Vec::new();
                let mut to_counterparty_output_info: CommitmentTxCounterpartyOutputInfo = None;
 
-               let htlc_outputs = match self.counterparty_claimable_outpoints.get(&commitment_txid) {
+               let per_commitment_claimable_data = match per_commitment_option {
                        Some(outputs) => outputs,
                        None => return (claimable_outpoints, to_counterparty_output_info),
                };
@@ -3338,7 +3393,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        }
                }
 
-               for (_, &(ref htlc, _)) in htlc_outputs.iter().enumerate() {
+               for  &(ref htlc, _) in per_commitment_claimable_data.iter() {
                        if let Some(transaction_output_index) = htlc.transaction_output_index {
                                if let Some(transaction) = tx {
                                        if transaction_output_index as usize >= transaction.output.len() ||
@@ -3528,6 +3583,8 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        if counterparty_commitment_txid == confirmed_commitment_txid {
                                continue;
                        }
+                       // If we have generated claims for counterparty_commitment_txid earlier, we can rely on always
+                       // having claim related htlcs for counterparty_commitment_txid in counterparty_claimable_outpoints.
                        for (htlc, _) in self.counterparty_claimable_outpoints.get(counterparty_commitment_txid).unwrap_or(&vec![]) {
                                log_trace!(logger, "Canceling claims for previously confirmed counterparty commitment {}",
                                        counterparty_commitment_txid);
@@ -4163,9 +4220,8 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        }
 
                        macro_rules! check_htlc_valid_counterparty {
-                               ($counterparty_txid: expr, $htlc_output: expr) => {
-                                       if let Some(txid) = $counterparty_txid {
-                                               for &(ref pending_htlc, ref pending_source) in self.counterparty_claimable_outpoints.get(&txid).unwrap() {
+                               ($htlc_output: expr, $per_commitment_data: expr) => {
+                                               for &(ref pending_htlc, ref pending_source) in $per_commitment_data {
                                                        if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat {
                                                                if let &Some(ref source) = pending_source {
                                                                        log_claim!("revoked counterparty commitment tx", false, pending_htlc, true);
@@ -4174,7 +4230,6 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                                }
                                                        }
                                                }
-                                       }
                                }
                        }
 
@@ -4191,9 +4246,13 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                                // resolve the source HTLC with the original sender.
                                                                payment_data = Some(((*source).clone(), htlc_output.payment_hash, htlc_output.amount_msat));
                                                        } else if !$holder_tx {
-                                                               check_htlc_valid_counterparty!(self.current_counterparty_commitment_txid, htlc_output);
+                                                               if let Some(current_counterparty_commitment_txid) = &self.current_counterparty_commitment_txid {
+                                                                       check_htlc_valid_counterparty!(htlc_output, self.counterparty_claimable_outpoints.get(current_counterparty_commitment_txid).unwrap());
+                                                               }
                                                                if payment_data.is_none() {
-                                                                       check_htlc_valid_counterparty!(self.prev_counterparty_commitment_txid, htlc_output);
+                                                                       if let Some(prev_counterparty_commitment_txid) = &self.prev_counterparty_commitment_txid {
+                                                                               check_htlc_valid_counterparty!(htlc_output, self.counterparty_claimable_outpoints.get(prev_counterparty_commitment_txid).unwrap());
+                                                                       }
                                                                }
                                                        }
                                                        if payment_data.is_none() {
@@ -4231,7 +4290,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                }
                        }
                        if let Some(ref htlc_outputs) = self.counterparty_claimable_outpoints.get(&input.previous_output.txid) {
-                               scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()).map(|boxed| &**boxed))),
+                               scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, b.as_ref().map(|boxed| &**boxed))),
                                        "counterparty commitment tx", false);
                        }
 
@@ -4330,6 +4389,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                revocation_pubkey: broadcasted_holder_revokable_script.2,
                                                channel_keys_id: self.channel_keys_id,
                                                channel_value_satoshis: self.channel_value_satoshis,
+                                               channel_transaction_parameters: Some(self.onchain_tx_handler.channel_transaction_parameters.clone()),
                                        }));
                                }
                        }
@@ -4372,7 +4432,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        }
 }
 
-impl<Signer: WriteableEcdsaChannelSigner, T: Deref, F: Deref, L: Deref> chain::Listen for (ChannelMonitor<Signer>, T, F, L)
+impl<Signer: EcdsaChannelSigner, T: Deref, F: Deref, L: Deref> chain::Listen for (ChannelMonitor<Signer>, T, F, L)
 where
        T::Target: BroadcasterInterface,
        F::Target: FeeEstimator,
@@ -4387,7 +4447,7 @@ where
        }
 }
 
-impl<Signer: WriteableEcdsaChannelSigner, M, T: Deref, F: Deref, L: Deref> chain::Confirm for (M, T, F, L)
+impl<Signer: EcdsaChannelSigner, M, T: Deref, F: Deref, L: Deref> chain::Confirm for (M, T, F, L)
 where
        M: Deref<Target = ChannelMonitor<Signer>>,
        T::Target: BroadcasterInterface,
@@ -4632,6 +4692,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                let mut spendable_txids_confirmed = Some(Vec::new());
                let mut counterparty_fulfilled_htlcs = Some(new_hash_map());
                let mut initial_counterparty_commitment_info = None;
+               let mut balances_empty_height = None;
                let mut channel_id = None;
                read_tlv_fields!(reader, {
                        (1, funding_spend_confirmed, option),
@@ -4644,6 +4705,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                        (15, counterparty_fulfilled_htlcs, option),
                        (17, initial_counterparty_commitment_info, option),
                        (19, channel_id, option),
+                       (21, balances_empty_height, option),
                });
 
                // `HolderForceClosedWithInfo` replaced `HolderForceClosed` in v0.0.122. If we have both
@@ -4722,6 +4784,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                        best_block,
                        counterparty_node_id,
                        initial_counterparty_commitment_info,
+                       balances_empty_height,
                })))
        }
 }
@@ -4753,7 +4816,7 @@ mod tests {
        use crate::chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT};
        use crate::chain::transaction::OutPoint;
        use crate::sign::InMemorySigner;
-       use crate::ln::{PaymentPreimage, PaymentHash, ChannelId};
+       use crate::ln::types::{PaymentPreimage, PaymentHash, ChannelId};
        use crate::ln::channel_keys::{DelayedPaymentBasepoint, DelayedPaymentKey, HtlcBasepoint, RevocationBasepoint, RevocationKey};
        use crate::ln::chan_utils::{self,HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
        use crate::ln::channelmanager::{PaymentSendFailure, PaymentId, RecipientOnionFields};
@@ -5202,7 +5265,8 @@ mod tests {
                        best_block, dummy_key, channel_id);
 
                let chan_id = monitor.inner.lock().unwrap().channel_id();
-               let context_logger = WithChannelMonitor::from(&logger, &monitor);
+               let payment_hash = PaymentHash([1; 32]);
+               let context_logger = WithChannelMonitor::from(&logger, &monitor, Some(payment_hash));
                log_error!(context_logger, "This is an error");
                log_warn!(context_logger, "This is an error");
                log_debug!(context_logger, "This is an error");