Avoid commitment broadcast upon detected funding spend

There's no need to broadcast our local commitment transaction if we've
already seen a confirmed one as it'll be immediately rejected as a
duplicate/conflict.

This will also help prevent dispatching spurious events for bumping
commitment and HTLC transactions through anchor outputs (once
implemented in future work) and the dispatch for said events follows the
same flow as our usual commitment broadcast.

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index e79e50172f9f3f1c5c8a2e98d378f118e6841001..da30f86149b7b918065de56dfa821267c563f721 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -793,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
@@ -3068,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

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 699444a640cf1ad90020d8c6dac78b643c8b865a..fb9ce86234c90b91de765cfe6d714bc08b7fb128 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1267,44 +1267,32 @@ fn test_duplicate_htlc_different_direction_onchain() {
        connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
 
        let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
-       assert_eq!(claim_txn.len(), 8);
+       assert_eq!(claim_txn.len(), 5);
 
        check_spends!(claim_txn[0], remote_txn[0]); // Immediate HTLC claim with preimage
-
        check_spends!(claim_txn[1], chan_1.3); // Alternative commitment tx
        check_spends!(claim_txn[2], claim_txn[1]); // HTLC spend in alternative commitment tx
 
-       let bump_tx = if claim_txn[1] == claim_txn[4] {
-               assert_eq!(claim_txn[1], claim_txn[4]);
-               assert_eq!(claim_txn[2], claim_txn[5]);
-
-               check_spends!(claim_txn[7], claim_txn[1]); // HTLC timeout on alternative commitment tx
-
-               check_spends!(claim_txn[3], remote_txn[0]); // HTLC timeout on broadcasted commitment tx
-               &claim_txn[3]
+       check_spends!(claim_txn[3], remote_txn[0]);
+       check_spends!(claim_txn[4], remote_txn[0]);
+       let preimage_tx = &claim_txn[0];
+       let (preimage_bump_tx, timeout_tx) = if claim_txn[3].input[0].previous_output == preimage_tx.input[0].previous_output {
+               (&claim_txn[3], &claim_txn[4])
        } else {
-               assert_eq!(claim_txn[1], claim_txn[3]);
-               assert_eq!(claim_txn[2], claim_txn[4]);
-
-               check_spends!(claim_txn[5], claim_txn[1]); // HTLC timeout on alternative commitment tx
-
-               check_spends!(claim_txn[7], remote_txn[0]); // HTLC timeout on broadcasted commitment tx
-
-               &claim_txn[7]
+               (&claim_txn[4], &claim_txn[3])
        };
 
-       assert_eq!(claim_txn[0].input.len(), 1);
-       assert_eq!(bump_tx.input.len(), 1);
-       assert_eq!(claim_txn[0].input[0].previous_output, bump_tx.input[0].previous_output);
+       assert_eq!(preimage_tx.input.len(), 1);
+       assert_eq!(preimage_bump_tx.input.len(), 1);
 
-       assert_eq!(claim_txn[0].input.len(), 1);
-       assert_eq!(claim_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
-       assert_eq!(remote_txn[0].output[claim_txn[0].input[0].previous_output.vout as usize].value, 800);
+       assert_eq!(preimage_tx.input.len(), 1);
+       assert_eq!(preimage_tx.input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
+       assert_eq!(remote_txn[0].output[preimage_tx.input[0].previous_output.vout as usize].value, 800);
 
-       assert_eq!(claim_txn[6].input.len(), 1);
-       assert_eq!(claim_txn[6].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
-       check_spends!(claim_txn[6], remote_txn[0]);
-       assert_eq!(remote_txn[0].output[claim_txn[6].input[0].previous_output.vout as usize].value, 900);
+       assert_eq!(timeout_tx.input.len(), 1);
+       assert_eq!(timeout_tx.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
+       check_spends!(timeout_tx, remote_txn[0]);
+       assert_eq!(remote_txn[0].output[timeout_tx.input[0].previous_output.vout as usize].value, 900);
 
        let events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 3);
@@ -8100,45 +8088,40 @@ fn test_bump_penalty_txn_on_remote_commitment() {
        let feerate_preimage;
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               // 9 transactions including:
-               // 1*2 ChannelManager local broadcasts of commitment + HTLC-Success
-               // 1*3 ChannelManager local broadcasts of commitment + HTLC-Success + HTLC-Timeout
-               // 2 * HTLC-Success (one RBF bump we'll check later)
-               // 1 * HTLC-Timeout
-               assert_eq!(node_txn.len(), 8);
+               // 5 transactions including:
+               //   local commitment + HTLC-Success
+               //   preimage and timeout sweeps from remote commitment + preimage sweep bump
+               assert_eq!(node_txn.len(), 5);
                assert_eq!(node_txn[0].input.len(), 1);
-               assert_eq!(node_txn[6].input.len(), 1);
+               assert_eq!(node_txn[3].input.len(), 1);
+               assert_eq!(node_txn[4].input.len(), 1);
                check_spends!(node_txn[0], remote_txn[0]);
-               check_spends!(node_txn[6], remote_txn[0]);
-
-               check_spends!(node_txn[1], chan.3);
-               check_spends!(node_txn[2], node_txn[1]);
-
-               if node_txn[0].input[0].previous_output == node_txn[3].input[0].previous_output {
-                       preimage_bump = node_txn[3].clone();
-                       check_spends!(node_txn[3], remote_txn[0]);
+               check_spends!(node_txn[3], remote_txn[0]);
+               check_spends!(node_txn[4], remote_txn[0]);
 
-                       assert_eq!(node_txn[1], node_txn[4]);
-                       assert_eq!(node_txn[2], node_txn[5]);
-               } else {
-                       preimage_bump = node_txn[7].clone();
-                       check_spends!(node_txn[7], remote_txn[0]);
-                       assert_eq!(node_txn[0].input[0].previous_output, node_txn[7].input[0].previous_output);
-
-                       assert_eq!(node_txn[1], node_txn[3]);
-                       assert_eq!(node_txn[2], node_txn[4]);
-               }
-
-               timeout = node_txn[6].txid();
-               let index = node_txn[6].input[0].previous_output.vout;
-               let fee = remote_txn[0].output[index as usize].value - node_txn[6].output[0].value;
-               feerate_timeout = fee * 1000 / node_txn[6].weight() as u64;
+               check_spends!(node_txn[1], chan.3); // local commitment
+               check_spends!(node_txn[2], node_txn[1]); // local HTLC-Success
 
                preimage = node_txn[0].txid();
                let index = node_txn[0].input[0].previous_output.vout;
                let fee = remote_txn[0].output[index as usize].value - node_txn[0].output[0].value;
                feerate_preimage = fee * 1000 / node_txn[0].weight() as u64;
 
+               let (preimage_bump_tx, timeout_tx) = if node_txn[3].input[0].previous_output == node_txn[0].input[0].previous_output {
+                       (node_txn[3].clone(), node_txn[4].clone())
+               } else {
+                       (node_txn[4].clone(), node_txn[3].clone())
+               };
+
+               preimage_bump = preimage_bump_tx;
+               check_spends!(preimage_bump, remote_txn[0]);
+               assert_eq!(node_txn[0].input[0].previous_output, preimage_bump.input[0].previous_output);
+
+               timeout = timeout_tx.txid();
+               let index = timeout_tx.input[0].previous_output.vout;
+               let fee = remote_txn[0].output[index as usize].value - timeout_tx.output[0].value;
+               feerate_timeout = fee * 1000 / timeout_tx.weight() as u64;
+
                node_txn.clear();
        };
        assert_ne!(feerate_timeout, 0);
@@ -9014,11 +8997,8 @@ fn test_concurrent_monitor_claim() {
        watchtower_alice.chain_monitor.block_connected(&Block { header, txdata: vec![bob_state_y.clone()] }, CHAN_CONFIRM_DEPTH + 2 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
        {
                let htlc_txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               // We broadcast twice the transaction, once due to the HTLC-timeout, once due
-               // the onchain detection of the HTLC output
-               assert_eq!(htlc_txn.len(), 2);
+               assert_eq!(htlc_txn.len(), 1);
                check_spends!(htlc_txn[0], bob_state_y);
-               check_spends!(htlc_txn[1], bob_state_y);
        }
 }
 

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index dcad041f49cd78c2214260ff9a542b41c32b2e27..d2e5583fe5607cdf2712121387dce9bcc235c989 100644 (file)
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -508,13 +508,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
        expect_payment_sent!(nodes[0], payment_preimage_1);
        connect_blocks(&nodes[0], TEST_FINAL_CLTV*4 + 20);
        let as_htlc_timeout_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-       assert_eq!(as_htlc_timeout_txn.len(), 3);
-       let (first_htlc_timeout_tx, second_htlc_timeout_tx) = if as_htlc_timeout_txn[0] == as_commitment_tx {
-               (&as_htlc_timeout_txn[1], &as_htlc_timeout_txn[2])
-       } else {
-               assert_eq!(as_htlc_timeout_txn[2], as_commitment_tx);
-               (&as_htlc_timeout_txn[0], &as_htlc_timeout_txn[1])
-       };
+       assert_eq!(as_htlc_timeout_txn.len(), 2);
+       let (first_htlc_timeout_tx, second_htlc_timeout_tx) = (&as_htlc_timeout_txn[0], &as_htlc_timeout_txn[1]);
        check_spends!(first_htlc_timeout_tx, as_commitment_tx);
        check_spends!(second_htlc_timeout_tx, as_commitment_tx);
        if first_htlc_timeout_tx.input[0].previous_output == bs_htlc_claim_txn[0].input[0].previous_output {