Remove duplicata for local commitment+HTLC txn

Previously, we would regenerate this class of txn twice due to
block-rescan triggered by new watching outputs registered.

This commmit doesn't change behavior, it only tweaks TestBroadcaster
to ensure we modify cleanly tests anticipating next commit
refactor.

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 95358208e9de283f2b78837e74cd78875d773a8e..2e071c2b78baf31851f85132f0117891ec50a0b7 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1038,7 +1038,7 @@ pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
        let mut chan_mon_cfgs = Vec::new();
        for _ in 0..node_count {
-               let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())};
+               let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashMap::new())};
                let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
                chan_mon_cfgs.push(TestChanMonCfg{ tx_broadcaster, fee_estimator });
        }

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 8c0f05723bf521fa4904815fb80649f26a24284b..62cae026911acbfd1210a0a195e4b13b34a87b89 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -2335,30 +2335,13 @@ fn claim_htlc_outputs_single_tx() {
                }
 
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 21);
+               assert_eq!(node_txn.len(), 9);
                // ChannelMonitor: justice tx revoked offered htlc, justice tx revoked received htlc, justice tx revoked to_local (3)
                // ChannelManager: local commmitment + local HTLC-timeout (2)
-               // ChannelMonitor: bumped justice tx (4), after one increase, bumps on HTLC aren't generated not being substantial anymore
-               // ChannelMonito r: local commitment + local HTLC-timeout (14)
-
-               assert_eq!(node_txn[0], node_txn[5]);
-               assert_eq!(node_txn[0], node_txn[7]);
-               assert_eq!(node_txn[0], node_txn[9]);
-               assert_eq!(node_txn[0], node_txn[13]);
-               assert_eq!(node_txn[0], node_txn[15]);
-               assert_eq!(node_txn[0], node_txn[17]);
-               assert_eq!(node_txn[0], node_txn[19]);
-
-               assert_eq!(node_txn[1], node_txn[6]);
-               assert_eq!(node_txn[1], node_txn[8]);
-               assert_eq!(node_txn[1], node_txn[10]);
-               assert_eq!(node_txn[1], node_txn[14]);
-               assert_eq!(node_txn[1], node_txn[16]);
-               assert_eq!(node_txn[1], node_txn[18]);
-               assert_eq!(node_txn[1], node_txn[20]);
-
-
-               // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration and present 8 times (rebroadcast at every block from 200 to 206)
+               // ChannelMonitor: bumped justice tx, after one increase, bumps on HTLC aren't generated not being substantial anymore, bump on revoked to_local isn't generated due to more room for expiration (2)
+               // ChannelMonitor: local commitment + local HTLC-timeout (2)
+
+               // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration
                assert_eq!(node_txn[0].input.len(), 1);
                check_spends!(node_txn[0], chan_1.3);
                assert_eq!(node_txn[1].input.len(), 1);
@@ -2438,12 +2421,10 @@ fn test_htlc_on_chain_success() {
        nodes[2].block_notifier.block_connected(&Block { header, txdata: vec![commitment_tx[0].clone()]}, 1);
        check_closed_broadcast!(nodes[2], false);
        check_added_monitors!(nodes[2], 1);
-       let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx, 2*htlc-success tx), ChannelMonitor : 4 (2*2 * HTLC-Success tx)
-       assert_eq!(node_txn.len(), 7);
+       let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx, 2*htlc-success tx), ChannelMonitor : 2 (2 * HTLC-Success tx)
+       assert_eq!(node_txn.len(), 5);
        assert_eq!(node_txn[0], node_txn[3]);
        assert_eq!(node_txn[1], node_txn[4]);
-       assert_eq!(node_txn[0], node_txn[5]);
-       assert_eq!(node_txn[1], node_txn[6]);
        assert_eq!(node_txn[2], commitment_tx[0]);
        check_spends!(node_txn[0], commitment_tx[0]);
        check_spends!(node_txn[1], commitment_tx[0]);
@@ -2488,15 +2469,11 @@ fn test_htlc_on_chain_success() {
        macro_rules! check_tx_local_broadcast {
                ($node: expr, $htlc_offered: expr, $commitment_tx: expr, $chan_tx: expr) => { {
                        let mut node_txn = $node.tx_broadcaster.txn_broadcasted.lock().unwrap();
-                       assert_eq!(node_txn.len(), if $htlc_offered { 7 } else { 5 });
+                       assert_eq!(node_txn.len(), 5);
                        // Node[1]: ChannelManager: 3 (commitment tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 (timeout tx)
-                       // Node[0]: ChannelManager: 3 (commtiemtn tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 HTLC-timeout * 2 (block-rescan)
+                       // Node[0]: ChannelManager: 3 (commtiemtn tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 HTLC-timeout
                        check_spends!(node_txn[0], $commitment_tx);
                        check_spends!(node_txn[1], $commitment_tx);
-                       if $htlc_offered {
-                               assert_eq!(node_txn[0], node_txn[5]);
-                               assert_eq!(node_txn[1], node_txn[6]);
-                       }
                        assert_ne!(node_txn[0].lock_time, 0);
                        assert_ne!(node_txn[1].lock_time, 0);
                        if $htlc_offered {
@@ -2633,11 +2610,9 @@ fn test_htlc_on_chain_timeout() {
        let timeout_tx;
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 7); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : (local commitment tx + HTLC-timeout) * 2 (block-rescan), timeout tx
+               assert_eq!(node_txn.len(), 5); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 2 (local commitment tx + HTLC-timeout), 1 timeout tx
                assert_eq!(node_txn[0], node_txn[3]);
-               assert_eq!(node_txn[0], node_txn[5]);
                assert_eq!(node_txn[1], node_txn[4]);
-               assert_eq!(node_txn[1], node_txn[6]);
 
                check_spends!(node_txn[2], commitment_tx[0]);
                assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
@@ -4504,9 +4479,8 @@ fn test_onchain_to_onchain_claim() {
        check_added_monitors!(nodes[2], 1);
 
        let c_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Success tx), ChannelMonitor : 1 (HTLC-Success tx)
-       assert_eq!(c_txn.len(), 4);
+       assert_eq!(c_txn.len(), 3);
        assert_eq!(c_txn[0], c_txn[2]);
-       assert_eq!(c_txn[0], c_txn[3]);
        assert_eq!(commitment_tx[0], c_txn[1]);
        check_spends!(c_txn[1], chan_2.3);
        check_spends!(c_txn[2], c_txn[1]);
@@ -4622,11 +4596,11 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
                _ => panic!("Unexepected event"),
        }
        let htlc_success_txn: Vec<_> = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
-       assert_eq!(htlc_success_txn.len(), 7);
+       assert_eq!(htlc_success_txn.len(), 5); // ChannelMonitor: HTLC-Success txn (*2 due to 2-HTLC outputs), ChannelManager: local commitment tx + HTLC-Success txn (*2 due to 2-HTLC outputs)
        check_spends!(htlc_success_txn[2], chan_2.3);
        check_spends!(htlc_success_txn[3], htlc_success_txn[2]);
        check_spends!(htlc_success_txn[4], htlc_success_txn[2]);
-       assert_eq!(htlc_success_txn[0], htlc_success_txn[5]);
+       assert_eq!(htlc_success_txn[0], htlc_success_txn[3]);
        assert_eq!(htlc_success_txn[0].input.len(), 1);
        assert_eq!(htlc_success_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
        assert_eq!(htlc_success_txn[1], htlc_success_txn[4]);
@@ -6819,7 +6793,7 @@ fn test_data_loss_protect() {
        let logger: Arc<Logger> = Arc::new(test_utils::TestLogger::with_id(format!("node {}", 0)));
        let mut chan_monitor = <(Sha256dHash, ChannelMonitor<EnforcingChannelKeys>)>::read(&mut ::std::io::Cursor::new(previous_chan_monitor_state.0), Arc::clone(&logger)).unwrap().1;
        let chain_monitor = Arc::new(ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger)));
-       tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())};
+       tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashMap::new())};
        fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
        keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::clone(&logger));
        monitor = test_utils::TestChannelMonitor::new(chain_monitor.clone(), &tx_broadcaster, logger.clone(), &fee_estimator);

diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 13cbde4aa1fe35c24702d277a48b310c77d08490..618cef0c2080f3b7fd9f34cdb6f0701a35ba6909 100644 (file)
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -110,9 +110,24 @@ impl<'a> channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChanne
 
 pub struct TestBroadcaster {
        pub txn_broadcasted: Mutex<Vec<Transaction>>,
+       pub broadcasted_txn: Mutex<HashMap<Sha256dHash, u8>> // Temporary field while refactoring out tx duplication
 }
 impl chaininterface::BroadcasterInterface for TestBroadcaster {
        fn broadcast_transaction(&self, tx: &Transaction) {
+               let mut already = false;
+               {
+                       if let Some(counter) = self.broadcasted_txn.lock().unwrap().get_mut(&tx.txid()) {
+                               match counter {
+                                       0 => { *counter = 1; already = true }, // We still authorize at least 2 duplicata for a given TXID to account ChannelManager/ChannelMonitor broadcast
+                                       1 => return,
+                                       _ => panic!()
+                               }
+                       }
+               }
+               if !already {
+                       self.broadcasted_txn.lock().unwrap().insert(tx.txid(), 0);
+               }
+               print!("\nFRESH BROADCAST {}\n\n", tx.txid());
                self.txn_broadcasted.lock().unwrap().push(tx.clone());
        }
 }