Cancel previous commitment claims on newly confirmed commitment

Once a commitment transaction is broadcast/confirms, we may need to
claim some of the HTLCs in it. These claims are sent as requests to the
`OnchainTxHandler`, which will bump their feerate as they remain
unconfirmed. When said commitment transaction becomes unconfirmed
though, and another commitment confirms instead, i.e., a reorg happens,
the `OnchainTxHandler` doesn't have any insight into whether these
claims are still valid or not, so it continues attempting to claim the
HTLCs from the previous commitment (now unconfirmed) forever, along with
the HTLCs from the newly confirmed commitment.

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index ce1ef9128f91efe8327f2f874daca511e7c6e4f1..472c768d32bb93a103178b9bc39be5bdb640eb10 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -3356,6 +3356,58 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                }
        }
 
+       /// Cancels any existing pending claims for a commitment that previously confirmed and has now
+       /// been replaced by another.
+       pub fn cancel_prev_commitment_claims<L: Deref>(
+               &mut self, logger: &L, confirmed_commitment_txid: &Txid
+       ) where L::Target: Logger {
+               for (counterparty_commitment_txid, _) in &self.counterparty_commitment_txn_on_chain {
+                       // Cancel any pending claims for counterparty commitments we've seen confirm.
+                       if counterparty_commitment_txid == confirmed_commitment_txid {
+                               continue;
+                       }
+                       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);
+                               let mut outpoint = BitcoinOutPoint { txid: *counterparty_commitment_txid, vout: 0 };
+                               if let Some(vout) = htlc.transaction_output_index {
+                                       outpoint.vout = vout;
+                                       self.onchain_tx_handler.abandon_claim(&outpoint);
+                               }
+                       }
+               }
+               if self.holder_tx_signed {
+                       // If we've signed, we may have broadcast either commitment (prev or current), and
+                       // attempted to claim from it immediately without waiting for a confirmation.
+                       if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+                               log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+                                       self.current_holder_commitment_tx.txid);
+                               let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+                               for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+                                       if let Some(vout) = htlc.transaction_output_index {
+                                               outpoint.vout = vout;
+                                               self.onchain_tx_handler.abandon_claim(&outpoint);
+                                       }
+                               }
+                       }
+                       if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+                               if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
+                                       log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+                                               prev_holder_commitment_tx.txid);
+                                       let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+                                       for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
+                                               if let Some(vout) = htlc.transaction_output_index {
+                                                       outpoint.vout = vout;
+                                                       self.onchain_tx_handler.abandon_claim(&outpoint);
+                                               }
+                                       }
+                               }
+                       }
+               } else {
+                       // No previous claim.
+               }
+       }
+
        fn get_latest_holder_commitment_txn<L: Deref>(
                &mut self, logger: &WithChannelMonitor<L>,
        ) -> Vec<Transaction> where L::Target: Logger {
@@ -3568,6 +3620,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                        commitment_tx_to_counterparty_output,
                                                },
                                        });
+                                       // Now that we've detected a confirmed commitment transaction, attempt to cancel
+                                       // pending claims for any commitments that were previously confirmed such that
+                                       // we don't continue claiming inputs that no longer exist.
+                                       self.cancel_prev_commitment_claims(&logger, &txid);
                                }
                        }
                        if tx.input.len() >= 1 {

diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index bbed782bb57bd0a2e6da0fc4e9ca65ddadde6d5f..59c98f05ebc4018f5915165d05e7be8facc697b9 100644 (file)
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -676,6 +676,25 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
                None
        }
 
+       pub fn abandon_claim(&mut self, outpoint: &BitcoinOutPoint) {
+               let claim_id = self.claimable_outpoints.get(outpoint).map(|(claim_id, _)| *claim_id)
+                       .or_else(|| {
+                               self.pending_claim_requests.iter()
+                                       .find(|(_, claim)| claim.outpoints().iter().any(|claim_outpoint| *claim_outpoint == outpoint))
+                                       .map(|(claim_id, _)| *claim_id)
+                       });
+               if let Some(claim_id) = claim_id {
+                       if let Some(claim) = self.pending_claim_requests.remove(&claim_id) {
+                               for outpoint in claim.outpoints() {
+                                       self.claimable_outpoints.remove(&outpoint);
+                               }
+                       }
+               } else {
+                       self.locktimed_packages.values_mut().for_each(|claims|
+                               claims.retain(|claim| !claim.outpoints().iter().any(|claim_outpoint| *claim_outpoint == outpoint)));
+               }
+       }
+
        /// Upon channelmonitor.block_connected(..) or upon provision of a preimage on the forward link
        /// for this channel, provide new relevant on-chain transactions and/or new claim requests.
        /// Together with `update_claims_view_from_matched_txn` this used to be named

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index fee3a3b399475d885ee1ac7996f9f246c771eb81..979ef774e7f001650e1dd0521405455ab4cafaa4 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -8568,10 +8568,11 @@ fn test_concurrent_monitor_claim() {
        watchtower_alice.chain_monitor.block_connected(&block, HTLC_TIMEOUT_BROADCAST);
 
        // Watchtower Alice should have broadcast a commitment/HTLC-timeout
-       let alice_state = {
+       {
                let mut txn = alice_broadcaster.txn_broadcast();
                assert_eq!(txn.len(), 2);
-               txn.remove(0)
+               check_spends!(txn[0], chan_1.3);
+               check_spends!(txn[1], txn[0]);
        };
 
        // Copy ChainMonitor to simulate watchtower Bob and make it receive a commitment update first.
@@ -8640,11 +8641,8 @@ fn test_concurrent_monitor_claim() {
        check_added_monitors(&nodes[0], 1);
        {
                let htlc_txn = alice_broadcaster.txn_broadcast();
-               assert_eq!(htlc_txn.len(), 2);
+               assert_eq!(htlc_txn.len(), 1);
                check_spends!(htlc_txn[0], bob_state_y);
-               // Alice doesn't clean up the old HTLC claim since it hasn't seen a conflicting spend for
-               // it. However, she should, because it now has an invalid parent.
-               check_spends!(htlc_txn[1], alice_state);
        }
 }
 

diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index badb78f245a6651be0abffc415565142e4c7bcf5..d65f258e641fe0a1b1be6c5f511ff2aee8b6178e 100644 (file)
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -666,6 +666,9 @@ fn test_htlc_preimage_claim_holder_commitment_after_counterparty_commitment_reor
 
        mine_transaction(&nodes[0], &commitment_tx_b);
        mine_transaction(&nodes[1], &commitment_tx_b);
+       if nodes[1].connect_style.borrow().updates_best_block_first() {
+               let _ = nodes[1].tx_broadcaster.txn_broadcast();
+       }
 
        // Provide the preimage now, such that we only claim from the holder commitment (since it's
        // currently confirmed) and not the counterparty's.