Avoid commitment broadcast upon detected funding spend
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 573f144e5c04b0e6ba6342515d32efc05b31a4e8..da30f86149b7b918065de56dfa821267c563f721 100644 (file)
@@ -583,14 +583,25 @@ 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.
        ///
@@ -608,7 +619,7 @@ pub enum Balance {
 /// 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.
@@ -617,11 +628,39 @@ struct IrrevocablyResolvedHTLC {
        payment_preimage: Option<PaymentPreimage>,
 }
 
-impl_writeable_tlv_based!(IrrevocablyResolvedHTLC, {
-       (0, commitment_tx_output_idx, required),
-       (1, resolving_txid, option),
-       (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.
@@ -754,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
@@ -1474,7 +1516,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        }
                }
                let htlc_resolved = self.htlcs_resolved_on_chain.iter()
-                       .find(|v| if v.commitment_tx_output_idx == htlc_commitment_tx_output_idx {
+                       .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
@@ -1547,7 +1589,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        confirmation_height: conf_thresh,
                                });
                        } else {
-                               return Some(Balance::MaybeClaimableHTLCAwaitingTimeout {
+                               return Some(Balance::MaybeTimeoutClaimableHTLC {
                                        claimable_amount_satoshis: htlc.amount_msat / 1000,
                                        claimable_height: htlc.cltv_expiry,
                                });
@@ -1570,6 +1612,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        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
        }
@@ -1586,7 +1633,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
@@ -1727,12 +1774,19 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                        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 {
@@ -1752,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.
@@ -2879,12 +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, resolving_txid: Some(entry.txid),
-                                                       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));
@@ -2894,7 +2946,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                },
                                OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } => {
                                        self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
-                                               commitment_tx_output_idx, resolving_txid: Some(entry.txid),
+                                               commitment_tx_output_idx: Some(commitment_tx_output_idx), resolving_txid: Some(entry.txid),
                                                payment_preimage: preimage,
                                        });
                                },
@@ -3019,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