Avoid commitment broadcast upon detected funding spend
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 4d109294cf061e4a5a5f296ca839b1d5c778e716..da30f86149b7b918065de56dfa821267c563f721 100644 (file)
@@ -22,6 +22,7 @@
 
 use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::transaction::{TxOut,Transaction};
+use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
 
@@ -58,8 +59,13 @@ use core::convert::TryInto;
 use core::ops::Deref;
 use sync::Mutex;
 
-/// An update generated by the underlying Channel itself which contains some new information the
-/// ChannelMonitor should be made aware of.
+/// An update generated by the underlying channel itself which contains some new information the
+/// [`ChannelMonitor`] should be made aware of.
+///
+/// Because this represents only a small number of updates to the underlying state, it is generally
+/// much smaller than a full [`ChannelMonitor`]. However, for large single commitment transaction
+/// updates (e.g. ones during which there are hundreds of HTLCs pending on the commitment
+/// transaction), a single update may reach upwards of 1 MiB in serialized size.
 #[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 #[must_use]
@@ -369,6 +375,8 @@ enum OnchainEvent {
                /// transaction which appeared on chain.
                commitment_tx_output_idx: Option<u32>,
        },
+       /// An output waiting on [`ANTI_REORG_DELAY`] confirmations before we hand the user the
+       /// [`SpendableOutputDescriptor`].
        MaturingOutput {
                descriptor: SpendableOutputDescriptor,
        },
@@ -380,6 +388,9 @@ enum OnchainEvent {
                on_local_output_csv: Option<u16>,
                /// If the funding spend transaction was a known remote commitment transaction, we track
                /// the output index and amount of the counterparty's `to_self` output here.
+               ///
+               /// This allows us to generate a [`Balance::CounterpartyRevokedOutputClaimable`] for the
+               /// counterparty output.
                commitment_tx_to_counterparty_output: CommitmentTxCounterpartyOutputInfo,
        },
        /// A spend of a commitment transaction HTLC output, set in the cases where *no* `HTLCUpdate`
@@ -390,6 +401,9 @@ enum OnchainEvent {
        ///  * an inbound HTLC is claimed by us (with a preimage).
        ///  * a revoked-state HTLC transaction was broadcasted, which was claimed by the revocation
        ///    signature.
+       ///  * a revoked-state HTLC transaction was broadcasted, which was claimed by an
+       ///    HTLC-Success/HTLC-Failure transaction (and is still claimable with a revocation
+       ///    signature).
        HTLCSpendConfirmation {
                commitment_tx_output_idx: u32,
                /// If the claim was made by either party with a preimage, this is filled in
@@ -569,28 +583,84 @@ pub enum Balance {
        /// HTLCs which we sent to our counterparty which are claimable after a timeout (less on-chain
        /// fees) if the counterparty does not know the preimage for the HTLCs. These are somewhat
        /// likely to be claimed by our counterparty before we do.
-       MaybeClaimableHTLCAwaitingTimeout {
-               /// The amount available to claim, in satoshis, excluding the on-chain fees which will be
-               /// required to do so.
+       MaybeTimeoutClaimableHTLC {
+               /// The amount potentially available to claim, in satoshis, excluding the on-chain fees
+               /// which will be required to do so.
                claimable_amount_satoshis: u64,
                /// The height at which we will be able to claim the balance if our counterparty has not
                /// done so.
                claimable_height: u32,
        },
+       /// HTLCs which we received from our counterparty which are claimable with a preimage which we
+       /// do not currently have. This will only be claimable if we receive the preimage from the node
+       /// to which we forwarded this HTLC before the timeout.
+       MaybePreimageClaimableHTLC {
+               /// The amount potentially available to claim, in satoshis, excluding the on-chain fees
+               /// which will be required to do so.
+               claimable_amount_satoshis: u64,
+               /// The height at which our counterparty will be able to claim the balance if we have not
+               /// yet received the preimage and claimed it ourselves.
+               expiry_height: u32,
+       },
+       /// The channel has been closed, and our counterparty broadcasted a revoked commitment
+       /// transaction.
+       ///
+       /// Thus, we're able to claim all outputs in the commitment transaction, one of which has the
+       /// following amount.
+       CounterpartyRevokedOutputClaimable {
+               /// The amount, in satoshis, of the output which we can claim.
+               ///
+               /// Note that for outputs from HTLC balances this may be excluding some on-chain fees that
+               /// were already spent.
+               claimable_amount_satoshis: u64,
+       },
 }
 
 /// An HTLC which has been irrevocably resolved on-chain, and has reached ANTI_REORG_DELAY.
 #[derive(PartialEq)]
 struct IrrevocablyResolvedHTLC {
-       commitment_tx_output_idx: u32,
+       commitment_tx_output_idx: Option<u32>,
+       /// The txid of the transaction which resolved the HTLC, this may be a commitment (if the HTLC
+       /// was not present in the confirmed commitment transaction), HTLC-Success, or HTLC-Timeout
+       /// transaction.
+       resolving_txid: Option<Txid>, // Added as optional, but always filled in, in 0.0.110
        /// Only set if the HTLC claim was ours using a payment preimage
        payment_preimage: Option<PaymentPreimage>,
 }
 
-impl_writeable_tlv_based!(IrrevocablyResolvedHTLC, {
-       (0, commitment_tx_output_idx, required),
-       (2, payment_preimage, option),
-});
+// In LDK versions prior to 0.0.111 commitment_tx_output_idx was not Option-al and
+// IrrevocablyResolvedHTLC objects only existed for non-dust HTLCs. This was a bug, but to maintain
+// backwards compatibility we must ensure we always write out a commitment_tx_output_idx field,
+// using `u32::max_value()` as a sentinal to indicate the HTLC was dust.
+impl Writeable for IrrevocablyResolvedHTLC {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               let mapped_commitment_tx_output_idx = self.commitment_tx_output_idx.unwrap_or(u32::max_value());
+               write_tlv_fields!(writer, {
+                       (0, mapped_commitment_tx_output_idx, required),
+                       (1, self.resolving_txid, option),
+                       (2, self.payment_preimage, option),
+               });
+               Ok(())
+       }
+}
+
+impl Readable for IrrevocablyResolvedHTLC {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               let mut mapped_commitment_tx_output_idx = 0;
+               let mut resolving_txid = None;
+               let mut payment_preimage = None;
+               read_tlv_fields!(reader, {
+                       (0, mapped_commitment_tx_output_idx, required),
+                       (1, resolving_txid, option),
+                       (2, payment_preimage, option),
+               });
+               Ok(Self {
+                       commitment_tx_output_idx: if mapped_commitment_tx_output_idx == u32::max_value() { None } else { Some(mapped_commitment_tx_output_idx) },
+                       resolving_txid,
+                       payment_preimage,
+               })
+       }
+}
 
 /// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
 /// on-chain transactions to ensure no loss of funds occurs.
@@ -723,7 +793,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
@@ -1401,7 +1474,155 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        pub fn current_best_block(&self) -> BestBlock {
                self.inner.lock().unwrap().best_block.clone()
        }
+}
+
+impl<Signer: Sign> 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,
+               counterparty_revoked_commitment: bool, confirmed_txid: Option<Txid>)
+       -> Option<Balance> {
+               let htlc_commitment_tx_output_idx =
+                       if let Some(v) = htlc.transaction_output_index { v } else { return None; };
+
+               let mut htlc_spend_txid_opt = None;
+               let mut holder_timeout_spend_pending = None;
+               let mut htlc_spend_pending = None;
+               let mut holder_delayed_output_pending = None;
+               for event in self.onchain_events_awaiting_threshold_conf.iter() {
+                       match event.event {
+                               OnchainEvent::HTLCUpdate { commitment_tx_output_idx, htlc_value_satoshis, .. }
+                               if commitment_tx_output_idx == Some(htlc_commitment_tx_output_idx) => {
+                                       debug_assert!(htlc_spend_txid_opt.is_none());
+                                       htlc_spend_txid_opt = event.transaction.as_ref().map(|tx| tx.txid());
+                                       debug_assert!(holder_timeout_spend_pending.is_none());
+                                       debug_assert_eq!(htlc_value_satoshis.unwrap(), htlc.amount_msat / 1000);
+                                       holder_timeout_spend_pending = Some(event.confirmation_threshold());
+                               },
+                               OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. }
+                               if commitment_tx_output_idx == htlc_commitment_tx_output_idx => {
+                                       debug_assert!(htlc_spend_txid_opt.is_none());
+                                       htlc_spend_txid_opt = event.transaction.as_ref().map(|tx| tx.txid());
+                                       debug_assert!(htlc_spend_pending.is_none());
+                                       htlc_spend_pending = Some((event.confirmation_threshold(), preimage.is_some()));
+                               },
+                               OnchainEvent::MaturingOutput {
+                                       descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(ref descriptor) }
+                               if descriptor.outpoint.index as u32 == htlc_commitment_tx_output_idx => {
+                                       debug_assert!(holder_delayed_output_pending.is_none());
+                                       holder_delayed_output_pending = Some(event.confirmation_threshold());
+                               },
+                               _ => {},
+                       }
+               }
+               let htlc_resolved = self.htlcs_resolved_on_chain.iter()
+                       .find(|v| if v.commitment_tx_output_idx == Some(htlc_commitment_tx_output_idx) {
+                               debug_assert!(htlc_spend_txid_opt.is_none());
+                               htlc_spend_txid_opt = v.resolving_txid;
+                               true
+                       } else { false });
+               debug_assert!(holder_timeout_spend_pending.is_some() as u8 + htlc_spend_pending.is_some() as u8 + htlc_resolved.is_some() as u8 <= 1);
+
+               let htlc_output_to_spend =
+                       if let Some(txid) = htlc_spend_txid_opt {
+                               debug_assert!(
+                                       self.onchain_tx_handler.channel_transaction_parameters.opt_anchors.is_none(),
+                                       "This code needs updating for anchors");
+                               BitcoinOutPoint::new(txid, 0)
+                       } else {
+                               BitcoinOutPoint::new(confirmed_txid.unwrap(), htlc_commitment_tx_output_idx)
+                       };
+               let htlc_output_spend_pending = self.onchain_tx_handler.is_output_spend_pending(&htlc_output_to_spend);
+
+               if let Some(conf_thresh) = holder_delayed_output_pending {
+                       debug_assert!(holder_commitment);
+                       return Some(Balance::ClaimableAwaitingConfirmations {
+                               claimable_amount_satoshis: htlc.amount_msat / 1000,
+                               confirmation_height: conf_thresh,
+                       });
+               } else if htlc_resolved.is_some() && !htlc_output_spend_pending {
+                       // Funding transaction spends should be fully confirmed by the time any
+                       // HTLC transactions are resolved, unless we're talking about a holder
+                       // commitment tx, whose resolution is delayed until the CSV timeout is
+                       // reached, even though HTLCs may be resolved after only
+                       // ANTI_REORG_DELAY confirmations.
+                       debug_assert!(holder_commitment || self.funding_spend_confirmed.is_some());
+               } else if counterparty_revoked_commitment {
+                       let htlc_output_claim_pending = self.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
+                               if let OnchainEvent::MaturingOutput {
+                                       descriptor: SpendableOutputDescriptor::StaticOutput { .. }
+                               } = &event.event {
+                                       if event.transaction.as_ref().map(|tx| tx.input.iter().any(|inp| {
+                                               if let Some(htlc_spend_txid) = htlc_spend_txid_opt {
+                                                       Some(tx.txid()) == htlc_spend_txid_opt ||
+                                                               inp.previous_output.txid == htlc_spend_txid
+                                               } else {
+                                                       Some(inp.previous_output.txid) == confirmed_txid &&
+                                                               inp.previous_output.vout == htlc_commitment_tx_output_idx
+                                               }
+                                       })).unwrap_or(false) {
+                                               Some(())
+                                       } else { None }
+                               } else { None }
+                       });
+                       if htlc_output_claim_pending.is_some() {
+                               // We already push `Balance`s onto the `res` list for every
+                               // `StaticOutput` in a `MaturingOutput` in the revoked
+                               // counterparty commitment transaction case generally, so don't
+                               // need to do so again here.
+                       } else {
+                               debug_assert!(holder_timeout_spend_pending.is_none(),
+                                       "HTLCUpdate OnchainEvents should never appear for preimage claims");
+                               debug_assert!(!htlc.offered || htlc_spend_pending.is_none() || !htlc_spend_pending.unwrap().1,
+                                       "We don't (currently) generate preimage claims against revoked outputs, where did you get one?!");
+                               return Some(Balance::CounterpartyRevokedOutputClaimable {
+                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
+                               });
+                       }
+               } else if htlc.offered == holder_commitment {
+                       // If the payment was outbound, check if there's an HTLCUpdate
+                       // indicating we have spent this HTLC with a timeout, claiming it back
+                       // and awaiting confirmations on it.
+                       if let Some(conf_thresh) = holder_timeout_spend_pending {
+                               return Some(Balance::ClaimableAwaitingConfirmations {
+                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
+                                       confirmation_height: conf_thresh,
+                               });
+                       } else {
+                               return Some(Balance::MaybeTimeoutClaimableHTLC {
+                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
+                                       claimable_height: htlc.cltv_expiry,
+                               });
+                       }
+               } else if self.payment_preimages.get(&htlc.payment_hash).is_some() {
+                       // Otherwise (the payment was inbound), only expose it as claimable if
+                       // we know the preimage.
+                       // Note that if there is a pending claim, but it did not use the
+                       // preimage, we lost funds to our counterparty! We will then continue
+                       // to show it as ContentiousClaimable until ANTI_REORG_DELAY.
+                       debug_assert!(holder_timeout_spend_pending.is_none());
+                       if let Some((conf_thresh, true)) = htlc_spend_pending {
+                               return Some(Balance::ClaimableAwaitingConfirmations {
+                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
+                                       confirmation_height: conf_thresh,
+                               });
+                       } else {
+                               return Some(Balance::ContentiousClaimable {
+                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
+                                       timeout_height: htlc.cltv_expiry,
+                               });
+                       }
+               } else if htlc_resolved.is_none() {
+                       return Some(Balance::MaybePreimageClaimableHTLC {
+                               claimable_amount_satoshis: htlc.amount_msat / 1000,
+                               expiry_height: htlc.cltv_expiry,
+                       });
+               }
+               None
+       }
+}
 
+impl<Signer: Sign> 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).
@@ -1411,9 +1632,9 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        /// balance, or until our counterparty has claimed the balance and accrued several
        /// confirmations on the claim transaction.
        ///
-       /// Note that the balances available when you or your counterparty have broadcasted revoked
-       /// state(s) may not be fully captured here.
-       // TODO, fix that ^
+       /// Note that for `ChannelMonitors` which track a channel which went on-chain with versions of
+       /// 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
        /// may be returned here and their meanings.
@@ -1422,9 +1643,13 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                let us = self.inner.lock().unwrap();
 
                let mut confirmed_txid = us.funding_spend_confirmed;
+               let mut confirmed_counterparty_output = us.confirmed_commitment_tx_counterparty_output;
                let mut pending_commitment_tx_conf_thresh = None;
                let funding_spend_pending = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
-                       if let OnchainEvent::FundingSpendConfirmation { .. } = event.event {
+                       if let OnchainEvent::FundingSpendConfirmation { commitment_tx_to_counterparty_output, .. } =
+                               event.event
+                       {
+                               confirmed_counterparty_output = commitment_tx_to_counterparty_output;
                                Some((event.txid, event.confirmation_threshold()))
                        } else { None }
                });
@@ -1436,71 +1661,12 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                }
 
                macro_rules! walk_htlcs {
-                       ($holder_commitment: expr, $htlc_iter: expr) => {
+                       ($holder_commitment: expr, $counterparty_revoked_commitment: expr, $htlc_iter: expr) => {
                                for htlc in $htlc_iter {
-                                       if let Some(htlc_commitment_tx_output_idx) = htlc.transaction_output_index {
-                                               if let Some(conf_thresh) = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
-                                                       if let OnchainEvent::MaturingOutput { descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) } = &event.event {
-                                                               if descriptor.outpoint.index as u32 == htlc_commitment_tx_output_idx { Some(event.confirmation_threshold()) } else { None }
-                                                       } else { None }
-                                               }) {
-                                                       debug_assert!($holder_commitment);
-                                                       res.push(Balance::ClaimableAwaitingConfirmations {
-                                                               claimable_amount_satoshis: htlc.amount_msat / 1000,
-                                                               confirmation_height: conf_thresh,
-                                                       });
-                                               } else if us.htlcs_resolved_on_chain.iter().any(|v| v.commitment_tx_output_idx == htlc_commitment_tx_output_idx) {
-                                                       // Funding transaction spends should be fully confirmed by the time any
-                                                       // HTLC transactions are resolved, unless we're talking about a holder
-                                                       // commitment tx, whose resolution is delayed until the CSV timeout is
-                                                       // reached, even though HTLCs may be resolved after only
-                                                       // ANTI_REORG_DELAY confirmations.
-                                                       debug_assert!($holder_commitment || us.funding_spend_confirmed.is_some());
-                                               } else if htlc.offered == $holder_commitment {
-                                                       // If the payment was outbound, check if there's an HTLCUpdate
-                                                       // indicating we have spent this HTLC with a timeout, claiming it back
-                                                       // and awaiting confirmations on it.
-                                                       let htlc_update_pending = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
-                                                               if let OnchainEvent::HTLCUpdate { commitment_tx_output_idx: Some(commitment_tx_output_idx), .. } = event.event {
-                                                                       if commitment_tx_output_idx == htlc_commitment_tx_output_idx {
-                                                                               Some(event.confirmation_threshold()) } else { None }
-                                                               } else { None }
-                                                       });
-                                                       if let Some(conf_thresh) = htlc_update_pending {
-                                                               res.push(Balance::ClaimableAwaitingConfirmations {
-                                                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
-                                                                       confirmation_height: conf_thresh,
-                                                               });
-                                                       } else {
-                                                               res.push(Balance::MaybeClaimableHTLCAwaitingTimeout {
-                                                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
-                                                                       claimable_height: htlc.cltv_expiry,
-                                                               });
-                                                       }
-                                               } else if us.payment_preimages.get(&htlc.payment_hash).is_some() {
-                                                       // Otherwise (the payment was inbound), only expose it as claimable if
-                                                       // we know the preimage.
-                                                       // Note that if there is a pending claim, but it did not use the
-                                                       // preimage, we lost funds to our counterparty! We will then continue
-                                                       // to show it as ContentiousClaimable until ANTI_REORG_DELAY.
-                                                       let htlc_spend_pending = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
-                                                               if let OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } = event.event {
-                                                                       if commitment_tx_output_idx == htlc_commitment_tx_output_idx {
-                                                                               Some((event.confirmation_threshold(), preimage.is_some()))
-                                                                       } else { None }
-                                                               } else { None }
-                                                       });
-                                                       if let Some((conf_thresh, true)) = htlc_spend_pending {
-                                                               res.push(Balance::ClaimableAwaitingConfirmations {
-                                                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
-                                                                       confirmation_height: conf_thresh,
-                                                               });
-                                                       } else {
-                                                               res.push(Balance::ContentiousClaimable {
-                                                                       claimable_amount_satoshis: htlc.amount_msat / 1000,
-                                                                       timeout_height: htlc.cltv_expiry,
-                                                               });
-                                                       }
+                                       if htlc.transaction_output_index.is_some() {
+
+                                               if let Some(bal) = us.get_htlc_balance(htlc, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid) {
+                                                       res.push(bal);
                                                }
                                        }
                                }
@@ -1509,8 +1675,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
                if let Some(txid) = confirmed_txid {
                        let mut found_commitment_tx = false;
-                       if Some(txid) == us.current_counterparty_commitment_txid || Some(txid) == us.prev_counterparty_commitment_txid {
-                               walk_htlcs!(false, us.counterparty_claimable_outpoints.get(&txid).unwrap().iter().map(|(a, _)| a));
+                       if let Some(counterparty_tx_htlcs) = us.counterparty_claimable_outpoints.get(&txid) {
+                               // First look for the to_remote output back to us.
                                if let Some(conf_thresh) = pending_commitment_tx_conf_thresh {
                                        if let Some(value) = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
                                                if let OnchainEvent::MaturingOutput {
@@ -1529,9 +1695,50 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                                                // confirmation with the same height or have never met our dust amount.
                                        }
                                }
+                               if Some(txid) == us.current_counterparty_commitment_txid || Some(txid) == us.prev_counterparty_commitment_txid {
+                                       walk_htlcs!(false, false, counterparty_tx_htlcs.iter().map(|(a, _)| a));
+                               } else {
+                                       walk_htlcs!(false, true, counterparty_tx_htlcs.iter().map(|(a, _)| a));
+                                       // The counterparty broadcasted a revoked state!
+                                       // Look for any StaticOutputs first, generating claimable balances for those.
+                                       // If any match the confirmed counterparty revoked to_self output, skip
+                                       // generating a CounterpartyRevokedOutputClaimable.
+                                       let mut spent_counterparty_output = false;
+                                       for event in us.onchain_events_awaiting_threshold_conf.iter() {
+                                               if let OnchainEvent::MaturingOutput {
+                                                       descriptor: SpendableOutputDescriptor::StaticOutput { output, .. }
+                                               } = &event.event {
+                                                       res.push(Balance::ClaimableAwaitingConfirmations {
+                                                               claimable_amount_satoshis: output.value,
+                                                               confirmation_height: event.confirmation_threshold(),
+                                                       });
+                                                       if let Some(confirmed_to_self_idx) = confirmed_counterparty_output.map(|(idx, _)| idx) {
+                                                               if event.transaction.as_ref().map(|tx|
+                                                                       tx.input.iter().any(|inp| inp.previous_output.vout == confirmed_to_self_idx)
+                                                               ).unwrap_or(false) {
+                                                                       spent_counterparty_output = true;
+                                                               }
+                                                       }
+                                               }
+                                       }
+
+                                       if spent_counterparty_output {
+                                       } else if let Some((confirmed_to_self_idx, amt)) = confirmed_counterparty_output {
+                                               let output_spendable = us.onchain_tx_handler
+                                                       .is_output_spend_pending(&BitcoinOutPoint::new(txid, confirmed_to_self_idx));
+                                               if output_spendable {
+                                                       res.push(Balance::CounterpartyRevokedOutputClaimable {
+                                                               claimable_amount_satoshis: amt,
+                                                       });
+                                               }
+                                       } else {
+                                               // Counterparty output is missing, either it was broadcasted on a
+                                               // previous version of LDK or the counterparty hadn't met dust.
+                                       }
+                               }
                                found_commitment_tx = true;
                        } else if txid == us.current_holder_commitment_tx.txid {
-                               walk_htlcs!(true, us.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, _)| a));
+                               walk_htlcs!(true, false, us.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, _)| a));
                                if let Some(conf_thresh) = pending_commitment_tx_conf_thresh {
                                        res.push(Balance::ClaimableAwaitingConfirmations {
                                                claimable_amount_satoshis: us.current_holder_commitment_tx.to_self_value_sat,
@@ -1541,7 +1748,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                                found_commitment_tx = true;
                        } else if let Some(prev_commitment) = &us.prev_holder_signed_commitment_tx {
                                if txid == prev_commitment.txid {
-                                       walk_htlcs!(true, prev_commitment.htlc_outputs.iter().map(|(a, _, _)| a));
+                                       walk_htlcs!(true, false, prev_commitment.htlc_outputs.iter().map(|(a, _, _)| a));
                                        if let Some(conf_thresh) = pending_commitment_tx_conf_thresh {
                                                res.push(Balance::ClaimableAwaitingConfirmations {
                                                        claimable_amount_satoshis: prev_commitment.to_self_value_sat,
@@ -1562,19 +1769,24 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                                        });
                                }
                        }
-                       // TODO: Add logic to provide claimable balances for counterparty broadcasting revoked
-                       // outputs.
                } else {
                        let mut claimable_inbound_htlc_value_sat = 0;
                        for (htlc, _, _) in us.current_holder_commitment_tx.htlc_outputs.iter() {
                                if htlc.transaction_output_index.is_none() { continue; }
                                if htlc.offered {
-                                       res.push(Balance::MaybeClaimableHTLCAwaitingTimeout {
+                                       res.push(Balance::MaybeTimeoutClaimableHTLC {
                                                claimable_amount_satoshis: htlc.amount_msat / 1000,
                                                claimable_height: htlc.cltv_expiry,
                                        });
                                } else if us.payment_preimages.get(&htlc.payment_hash).is_some() {
                                        claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000;
+                               } else {
+                                       // As long as the HTLC is still in our latest commitment state, treat
+                                       // it as potentially claimable, even if it has long-since expired.
+                                       res.push(Balance::MaybePreimageClaimableHTLC {
+                                               claimable_amount_satoshis: htlc.amount_msat / 1000,
+                                               expiry_height: htlc.cltv_expiry,
+                                       });
                                }
                        }
                        res.push(Balance::ClaimableOnChannelClose {
@@ -1594,7 +1806,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                macro_rules! walk_htlcs {
                        ($holder_commitment: expr, $htlc_iter: expr) => {
                                for (htlc, source) in $htlc_iter {
-                                       if us.htlcs_resolved_on_chain.iter().any(|v| Some(v.commitment_tx_output_idx) == htlc.transaction_output_index) {
+                                       if us.htlcs_resolved_on_chain.iter().any(|v| v.commitment_tx_output_idx == htlc.transaction_output_index) {
                                                // We should assert that funding_spend_confirmed is_some() here, but we
                                                // have some unit tests which violate HTLC transaction CSVs entirely and
                                                // would fail.
@@ -2721,9 +2933,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                source: source.clone(),
                                                htlc_value_satoshis,
                                        }));
-                                       if let Some(idx) = commitment_tx_output_idx {
-                                               self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { commitment_tx_output_idx: idx, payment_preimage: None });
-                                       }
+                                       self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
+                                               commitment_tx_output_idx, resolving_txid: Some(entry.txid),
+                                               payment_preimage: None,
+                                       });
                                },
                                OnchainEvent::MaturingOutput { descriptor } => {
                                        log_debug!(logger, "Descriptor {} has got enough confirmations to be passed upstream", log_spendable!(descriptor));
@@ -2732,7 +2945,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        });
                                },
                                OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } => {
-                                       self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { commitment_tx_output_idx, payment_preimage: preimage });
+                                       self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
+                                               commitment_tx_output_idx: Some(commitment_tx_output_idx), resolving_txid: Some(entry.txid),
+                                               payment_preimage: preimage,
+                                       });
                                },
                                OnchainEvent::FundingSpendConfirmation { commitment_tx_to_counterparty_output, .. } => {
                                        self.funding_spend_confirmed = Some(entry.txid);
@@ -2855,6 +3071,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
@@ -2965,7 +3191,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                log_error!(logger, "Input spending {} ({}:{}) in {} resolves {} HTLC with payment hash {} with {}!",
                                                        $tx_info, input.previous_output.txid, input.previous_output.vout, tx.txid(),
                                                        if outbound_htlc { "outbound" } else { "inbound" }, log_bytes!($htlc.payment_hash.0),
-                                                       if revocation_sig_claim { "revocation sig" } else { "preimage claim after we'd passed the HTLC resolution back" });
+                                                       if revocation_sig_claim { "revocation sig" } else { "preimage claim after we'd passed the HTLC resolution back. We can likely claim the HTLC output with a revocation claim" });
                                        } else {
                                                log_info!(logger, "Input spending {} ({}:{}) in {} resolves {} HTLC with payment hash {} with {}",
                                                        $tx_info, input.previous_output.txid, input.previous_output.vout, tx.txid(),
@@ -3012,29 +3238,20 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        if payment_data.is_none() {
                                                                log_claim!($tx_info, $holder_tx, htlc_output, false);
                                                                let outbound_htlc = $holder_tx == htlc_output.offered;
-                                                               if !outbound_htlc || revocation_sig_claim {
-                                                                       self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
-                                                                               txid: tx.txid(), height, transaction: Some(tx.clone()),
-                                                                               event: OnchainEvent::HTLCSpendConfirmation {
-                                                                                       commitment_tx_output_idx: input.previous_output.vout,
-                                                                                       preimage: if accepted_preimage_claim || offered_preimage_claim {
-                                                                                               Some(payment_preimage) } else { None },
-                                                                                       // If this is a payment to us (!outbound_htlc, above),
-                                                                                       // wait for the CSV delay before dropping the HTLC from
-                                                                                       // claimable balance if the claim was an HTLC-Success
-                                                                                       // transaction.
-                                                                                       on_to_local_output_csv: if accepted_preimage_claim {
-                                                                                               Some(self.on_holder_tx_csv) } else { None },
-                                                                               },
-                                                                       });
-                                                               } else {
-                                                                       // Outbound claims should always have payment_data, unless
-                                                                       // we've already failed the HTLC as the commitment transaction
-                                                                       // which was broadcasted was revoked. In that case, we should
-                                                                       // spend the HTLC output here immediately, and expose that fact
-                                                                       // as a Balance, something which we do not yet do.
-                                                                       // TODO: Track the above as claimable!
-                                                               }
+                                                               self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
+                                                                       txid: tx.txid(), height, transaction: Some(tx.clone()),
+                                                                       event: OnchainEvent::HTLCSpendConfirmation {
+                                                                               commitment_tx_output_idx: input.previous_output.vout,
+                                                                               preimage: if accepted_preimage_claim || offered_preimage_claim {
+                                                                                       Some(payment_preimage) } else { None },
+                                                                               // If this is a payment to us (ie !outbound_htlc), wait for
+                                                                               // the CSV delay before dropping the HTLC from claimable
+                                                                               // balance if the claim was an HTLC-Success transaction (ie
+                                                                               // accepted_preimage_claim).
+                                                                               on_to_local_output_csv: if accepted_preimage_claim && !outbound_htlc {
+                                                                                       Some(self.on_holder_tx_csv) } else { None },
+                                                                       },
+                                                               });
                                                                continue 'outer_loop;
                                                        }
                                                }