X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=da30f86149b7b918065de56dfa821267c563f721;hb=62236c70d863bf9456e28a3a1ba784b2e4680a88;hp=863ce4759e6e302e818cf7d45243293627584fd2;hpb=f712eb41572d98af2e234bb05013728138f3f4f5;p=rust-lightning diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 863ce475..da30f861 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -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] @@ -382,6 +388,9 @@ enum OnchainEvent { on_local_output_csv: Option, /// 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` @@ -574,20 +583,43 @@ 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, /// 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. @@ -596,11 +628,39 @@ struct IrrevocablyResolvedHTLC { payment_preimage: Option, } -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(&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(reader: &mut R) -> Result { + 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. @@ -733,7 +793,10 @@ pub(crate) struct ChannelMonitorImpl { // 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, + 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 @@ -1411,7 +1474,155 @@ impl ChannelMonitor { pub fn current_best_block(&self) -> BestBlock { self.inner.lock().unwrap().best_block.clone() } +} +impl ChannelMonitorImpl { + /// 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) + -> Option { + 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 ChannelMonitor { /// 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). @@ -1421,9 +1632,9 @@ impl ChannelMonitor { /// 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. @@ -1432,9 +1643,13 @@ impl ChannelMonitor { 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 } }); @@ -1446,86 +1661,12 @@ impl ChannelMonitor { } 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 { - let mut htlc_update_pending = None; - let mut htlc_spend_pending = None; - let mut delayed_output_pending = None; - for event in us.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_update_pending.is_none()); - debug_assert_eq!(htlc_value_satoshis.unwrap(), htlc.amount_msat / 1000); - htlc_update_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_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!(delayed_output_pending.is_none()); - delayed_output_pending = Some(event.confirmation_threshold()); - }, - _ => {}, - } - } - let htlc_resolved = us.htlcs_resolved_on_chain.iter() - .find(|v| v.commitment_tx_output_idx == htlc_commitment_tx_output_idx); - debug_assert!(htlc_update_pending.is_some() as u8 + htlc_spend_pending.is_some() as u8 + htlc_resolved.is_some() as u8 <= 1); + if htlc.transaction_output_index.is_some() { - if let Some(conf_thresh) = delayed_output_pending { - debug_assert!($holder_commitment); - res.push(Balance::ClaimableAwaitingConfirmations { - claimable_amount_satoshis: htlc.amount_msat / 1000, - confirmation_height: conf_thresh, - }); - } else if htlc_resolved.is_some() { - // 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. - 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. - debug_assert!(htlc_update_pending.is_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 let Some(bal) = us.get_htlc_balance(htlc, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid) { + res.push(bal); } } } @@ -1534,8 +1675,8 @@ impl ChannelMonitor { 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 { @@ -1554,9 +1695,50 @@ impl ChannelMonitor { // 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, @@ -1566,7 +1748,7 @@ impl ChannelMonitor { 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, @@ -1587,19 +1769,24 @@ impl ChannelMonitor { }); } } - // 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 { @@ -1619,7 +1806,7 @@ impl ChannelMonitor { 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. @@ -2746,12 +2933,10 @@ impl ChannelMonitorImpl { 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)); @@ -2761,7 +2946,7 @@ impl ChannelMonitorImpl { }, 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, }); }, @@ -2886,6 +3071,16 @@ impl ChannelMonitorImpl { } fn should_broadcast_holder_commitment_txn(&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