Merge pull request #461 from ariard/2020-remove-duplicata
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 12 Feb 2020 17:38:21 +0000 (17:38 +0000)
committerGitHub <noreply@github.com>
Wed, 12 Feb 2020 17:38:21 +0000 (17:38 +0000)
Remove some duplicata of broadcast txn from ChannelMonitor

lightning/src/ln/channelmonitor.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/test_utils.rs

index 51446ccc57162d80fb1a1f56472959ddaa97d803..7d843506e10d4810726bdc3ec20f415d006275cd 100644 (file)
@@ -1799,11 +1799,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                        let mut inputs_info = Vec::new();
 
                                        macro_rules! sign_input {
-                                               ($sighash_parts: expr, $input: expr, $amount: expr, $preimage: expr) => {
+                                               ($sighash_parts: expr, $input: expr, $amount: expr, $preimage: expr, $idx: expr) => {
                                                        {
                                                                let (sig, redeemscript, htlc_key) = match self.key_storage {
                                                                        Storage::Local { ref htlc_base_key, .. } => {
-                                                                               let htlc = &per_commitment_option.unwrap()[$input.sequence as usize].0;
+                                                                               let htlc = &per_commitment_option.unwrap()[$idx as usize].0;
                                                                                let redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
                                                                                let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
                                                                                let htlc_key = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key));
@@ -1832,19 +1832,19 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                        }
                                                        if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
                                                                if htlc.offered {
-                                                                       let input = TxIn {
+                                                                       let mut input = TxIn {
                                                                                previous_output: BitcoinOutPoint {
                                                                                        txid: commitment_txid,
                                                                                        vout: transaction_output_index,
                                                                                },
                                                                                script_sig: Script::new(),
-                                                                               sequence: idx as u32, // reset to 0xfffffffd in sign_input
+                                                                               sequence: 0xff_ff_ff_fd,
                                                                                witness: Vec::new(),
                                                                        };
                                                                        if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
                                                                                inputs.push(input);
                                                                                inputs_desc.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC });
-                                                                               inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value, htlc.cltv_expiry));
+                                                                               inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value, htlc.cltv_expiry, idx));
                                                                                total_value += tx.output[transaction_output_index as usize].value;
                                                                        } else {
                                                                                let mut single_htlc_tx = Transaction {
@@ -1861,7 +1861,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                                let mut used_feerate;
                                                                                if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, used_feerate) {
                                                                                        let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
-                                                                                       let (redeemscript, htlc_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
+                                                                                       let (redeemscript, htlc_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec(), idx);
                                                                                        assert!(predicted_weight >= single_htlc_tx.get_weight());
                                                                                        spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
                                                                                                outpoint: BitcoinOutPoint { txid: single_htlc_tx.txid(), vout: 0 },
@@ -1892,7 +1892,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                                vout: transaction_output_index,
                                                                        },
                                                                        script_sig: Script::new(),
-                                                                       sequence: idx as u32,
+                                                                       sequence: 0xff_ff_ff_fd,
                                                                        witness: Vec::new(),
                                                                };
                                                                let mut timeout_tx = Transaction {
@@ -1909,7 +1909,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                let mut used_feerate;
                                                                if subtract_high_prio_fee!(self, fee_estimator, timeout_tx.output[0].value, predicted_weight, used_feerate) {
                                                                        let sighash_parts = bip143::SighashComponents::new(&timeout_tx);
-                                                                       let (redeemscript, htlc_key) = sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0]);
+                                                                       let (redeemscript, htlc_key) = sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0], idx);
                                                                        assert!(predicted_weight >= timeout_tx.get_weight());
                                                                        //TODO: track SpendableOutputDescriptor
                                                                        log_trace!(self, "Outpoint {}:{} is being being claimed, if it doesn't succeed, a bumped claiming txn is going to be broadcast at height {}", timeout_tx.input[0].previous_output.txid, timeout_tx.input[0].previous_output.vout, height_timer);
@@ -1961,7 +1961,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                        let height_timer = Self::get_height_timer(height, soonest_timelock);
                                        let spend_txid = spend_tx.txid();
                                        for (input, info) in spend_tx.input.iter_mut().zip(inputs_info.iter()) {
-                                               let (redeemscript, htlc_key) = sign_input!(sighash_parts, input, info.1, (info.0).0.to_vec());
+                                               let (redeemscript, htlc_key) = sign_input!(sighash_parts, input, info.1, (info.0).0.to_vec(), info.3);
                                                log_trace!(self, "Outpoint {}:{} is being being claimed, if it doesn't succeed, a bumped claiming txn is going to be broadcast at height {}", input.previous_output.txid, input.previous_output.vout, height_timer);
                                                per_input_material.insert(input.previous_output, InputMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*(info.0)), amount: info.1, locktime: 0});
                                                match self.claimable_outpoints.entry(input.previous_output) {
@@ -2908,7 +2908,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                for per_outp_material in cached_claim_datas.per_input_material.values() {
                        match per_outp_material {
                                &InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
-                                       log_trace!(self, "Is HLTC ? {}", is_htlc);
                                        inputs_witnesses_weight += Self::get_witnesses_weight(if !is_htlc { &[InputDescriptors::RevokedOutput] } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::OfferedHTLC) { &[InputDescriptors::RevokedOfferedHTLC] } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::AcceptedHTLC) { &[InputDescriptors::RevokedReceivedHTLC] } else { unreachable!() });
                                        amt += *amount;
                                },
index 472e50aae27b1cedfeccccd880b1cd2958099d4a..1ae8ca1e0058cbb59a006017fc30f9aa4bb75849 100644 (file)
@@ -35,6 +35,7 @@ use std::cell::RefCell;
 use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use std::mem;
+use std::collections::HashSet;
 
 pub const CHAN_CONFIRM_DEPTH: u32 = 100;
 pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b>, chain: &chaininterface::ChainWatchInterfaceUtil, tx: &Transaction, chan_id: u32) {
@@ -857,7 +858,7 @@ pub fn create_node_cfgs(node_count: usize) -> Vec<NodeCfg> {
                let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i)));
                let fee_estimator = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
                let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, logger.clone() as Arc<Logger>));
-               let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
+               let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashSet::new())});
                let mut seed = [0; 32];
                rng.fill_bytes(&mut seed);
                let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&seed, Network::Testnet, logger.clone() as Arc<Logger>));
index 5c16e72748198917d93ae7bd956106f94df641f3..9b1907cbb776e4b048482ba21328947b4f68b497 100644 (file)
@@ -1378,11 +1378,9 @@ fn test_duplicate_htlc_different_direction_onchain() {
        // Check we only broadcast 1 timeout tx
        let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
        let htlc_pair = if claim_txn[0].output[0].value == 800_000 / 1000 { (claim_txn[0].clone(), claim_txn[1].clone()) } else { (claim_txn[1].clone(), claim_txn[0].clone()) };
-       assert_eq!(claim_txn.len(), 7);
+       assert_eq!(claim_txn.len(), 5);
        check_spends!(claim_txn[2], chan_1.3);
        check_spends!(claim_txn[3], claim_txn[2]);
-       assert_eq!(claim_txn[0], claim_txn[5]);
-       assert_eq!(claim_txn[1], claim_txn[6]);
        assert_eq!(htlc_pair.0.input.len(), 1);
        assert_eq!(htlc_pair.0.input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
        check_spends!(htlc_pair.0, remote_txn[0].clone());
@@ -1999,8 +1997,7 @@ fn test_justice_tx() {
                nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
                {
                        let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-                       assert_eq!(node_txn.len(), 3);
-                       assert_eq!(node_txn.pop().unwrap(), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
+                       assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty tx, ChannelManager: local commitment tx
                        assert_eq!(node_txn[0].input.len(), 2); // We should claim the revoked output and the HTLC output
 
                        check_spends!(node_txn[0], revoked_local_txn[0].clone());
@@ -2043,8 +2040,7 @@ fn test_justice_tx() {
                nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
                {
                        let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-                       assert_eq!(node_txn.len(), 3);
-                       assert_eq!(node_txn.pop().unwrap(), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
+                       assert_eq!(node_txn.len(), 2); //ChannelMonitor: penalty tx, ChannelManager: local commitment tx
                        assert_eq!(node_txn[0].input.len(), 1); // We claim the received HTLC output
 
                        check_spends!(node_txn[0], revoked_local_txn[0].clone());
@@ -2083,9 +2079,7 @@ fn revoked_output_claim() {
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-       assert_eq!(node_txn.len(), 3); // nodes[1] will broadcast justice tx twice, and its own local state once
-
-       assert_eq!(node_txn[0], node_txn[2]);
+       assert_eq!(node_txn.len(), 2); // ChannelMonitor: justice tx against revoked to_local output, ChannelManager: local commitment tx
 
        check_spends!(node_txn[0], revoked_local_txn[0].clone());
        check_spends!(node_txn[1], chan_1.3.clone());
@@ -2140,13 +2134,11 @@ fn claim_htlc_outputs_shared_tx() {
                }
 
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 4);
+               assert_eq!(node_txn.len(), 3); // ChannelMonitor: penalty tx, ChannelManager: local commitment + HTLC-timeout
 
                assert_eq!(node_txn[0].input.len(), 3); // Claim the revoked output + both revoked HTLC outputs
                check_spends!(node_txn[0], revoked_local_txn[0].clone());
 
-               assert_eq!(node_txn[0], node_txn[3]); // justice tx is duplicated due to block re-scanning
-
                let mut witness_lens = BTreeSet::new();
                witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
                witness_lens.insert(node_txn[0].input[1].witness.last().unwrap().len());
@@ -2210,15 +2202,28 @@ fn claim_htlc_outputs_single_tx() {
                }
 
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 29); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + 5 * (1 local commitment tx + 1 htlc timeout tx)
-
-               assert_eq!(node_txn[0], node_txn[7]);
-               assert_eq!(node_txn[1], node_txn[8]);
-               assert_eq!(node_txn[2], node_txn[9]);
-               assert_eq!(node_txn[3], node_txn[10]);
-               assert_eq!(node_txn[4], node_txn[11]);
-               assert_eq!(node_txn[3], node_txn[5]); //local commitment tx + htlc timeout tx broadcasted by ChannelManger
+               assert_eq!(node_txn.len(), 26);
+               // 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 * 7 (7), after one increase, bumps on HTLC aren't generated not being substantial anymore
+               // ChannelMonitor: local commitment + local HTLC-timeout (14)
+
+
+               assert_eq!(node_txn[3], node_txn[5]);
+               assert_eq!(node_txn[3], node_txn[7]);
+               assert_eq!(node_txn[3], node_txn[9]);
+               assert_eq!(node_txn[3], node_txn[14]);
+               assert_eq!(node_txn[3], node_txn[17]);
+               assert_eq!(node_txn[3], node_txn[20]);
+               assert_eq!(node_txn[3], node_txn[23]);
+
                assert_eq!(node_txn[4], node_txn[6]);
+               assert_eq!(node_txn[4], node_txn[8]);
+               assert_eq!(node_txn[4], node_txn[10]);
+               assert_eq!(node_txn[4], node_txn[15]);
+               assert_eq!(node_txn[4], node_txn[18]);
+               assert_eq!(node_txn[4], node_txn[21]);
+               assert_eq!(node_txn[4], node_txn[24]);
 
                assert_eq!(node_txn[0].input.len(), 1);
                assert_eq!(node_txn[1].input.len(), 1);
@@ -2350,13 +2355,16 @@ fn test_htlc_on_chain_success() {
        };
        macro_rules! check_tx_local_broadcast {
                ($node: expr, $htlc_offered: expr, $commitment_tx: expr, $chan_tx: expr) => { {
-                       // ChannelManager : 3 (commitment tx, 2*HTLC-Timeout tx), ChannelMonitor : 2 (timeout tx) * 2 (block-rescan)
                        let mut node_txn = $node.tx_broadcaster.txn_broadcasted.lock().unwrap();
-                       assert_eq!(node_txn.len(), 7);
-                       assert_eq!(node_txn[0], node_txn[5]);
-                       assert_eq!(node_txn[1], node_txn[6]);
+                       assert_eq!(node_txn.len(), if $htlc_offered { 7 } else { 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)
                        check_spends!(node_txn[0], $commitment_tx.clone());
                        check_spends!(node_txn[1], $commitment_tx.clone());
+                       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 {
@@ -2394,9 +2402,8 @@ fn test_htlc_on_chain_success() {
        check_spends!(commitment_tx[0], chan_1.3.clone());
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![commitment_tx[0].clone()]}, 1);
        check_closed_broadcast!(nodes[1], false);
-       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx + 2*HTLC-Success), ChannelMonitor : 1 (HTLC-Success) * 2 (block-rescan)
-       assert_eq!(node_txn.len(), 5);
-       assert_eq!(node_txn[0], node_txn[4]);
+       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx + HTLC-Sucess * 2), ChannelMonitor : 1 (HTLC-Success)
+       assert_eq!(node_txn.len(), 4);
        check_spends!(node_txn[0], commitment_tx[0].clone());
        assert_eq!(node_txn[0].input.len(), 2);
        assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
@@ -2490,10 +2497,11 @@ 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(), 8); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 6 (HTLC-Timeout tx, commitment tx, timeout tx) * 2 (block-rescan)
-               assert_eq!(node_txn[0], node_txn[5]);
-               assert_eq!(node_txn[1], node_txn[6]);
-               assert_eq!(node_txn[2], node_txn[7]);
+               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[1], node_txn[3]);
+               assert_eq!(node_txn[1], node_txn[5]);
+               assert_eq!(node_txn[2], node_txn[4]);
+               assert_eq!(node_txn[2], node_txn[6]);
                check_spends!(node_txn[0], commitment_tx[0].clone());
                assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
                check_spends!(node_txn[1], chan_2.3.clone());
@@ -2536,9 +2544,8 @@ fn test_htlc_on_chain_timeout() {
 
        nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![commitment_tx[0].clone()]}, 200);
        check_closed_broadcast!(nodes[0], false);
-       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 2 (timeout tx) * 2 block-rescan
-       assert_eq!(node_txn.len(), 4);
-       assert_eq!(node_txn[0], node_txn[3]);
+       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 1 timeout tx
+       assert_eq!(node_txn.len(), 3);
        check_spends!(node_txn[0], commitment_tx[0].clone());
        assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
        check_spends!(node_txn[1], chan_1.3.clone());
@@ -3956,10 +3963,9 @@ fn test_static_spendable_outputs_preimage_tx() {
        }
 
        // Check B's monitor was able to send back output descriptor event for preimage tx on A's commitment tx
-       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); // ChannelManager : 2 (local commitment tx + HTLC-Success), ChannelMonitor: 2 (1 preimage tx)
-       assert_eq!(node_txn.len(), 4);
+       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); // ChannelManager : 2 (local commitment tx + HTLC-Success), ChannelMonitor: preimage tx
+       assert_eq!(node_txn.len(), 3);
        check_spends!(node_txn[0], commitment_tx[0].clone());
-       assert_eq!(node_txn[0], node_txn[3]);
        assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
 eprintln!("{:?}", node_txn[1]);
        check_spends!(node_txn[1], chan_1.3.clone());
@@ -3991,9 +3997,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
        check_closed_broadcast!(nodes[1], false);
 
-       let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-       assert_eq!(node_txn.len(), 3);
-       assert_eq!(node_txn.pop().unwrap(), node_txn[0]);
+       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+       assert_eq!(node_txn.len(), 2);
        assert_eq!(node_txn[0].input.len(), 2);
        check_spends!(node_txn[0], revoked_local_txn[0].clone());
 
@@ -4037,16 +4042,16 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
        check_closed_broadcast!(nodes[1], false);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-       assert_eq!(node_txn.len(), 5);
-       assert_eq!(node_txn[3].input.len(), 1);
-       check_spends!(node_txn[3], revoked_htlc_txn[0].clone());
+       assert_eq!(node_txn.len(), 4 ); // ChannelMonitor: justice tx on revoked commitment, justice tx on revoked HTLC-timeout, adjusted justice tx, ChannelManager: local commitment tx
+       assert_eq!(node_txn[2].input.len(), 1);
+       check_spends!(node_txn[2], revoked_htlc_txn[0].clone());
 
        // Check B's ChannelMonitor was able to generate the right spendable output descriptor
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
        assert_eq!(spend_txn.len(), 3);
        assert_eq!(spend_txn[0], spend_txn[1]);
        check_spends!(spend_txn[0], node_txn[0].clone());
-       check_spends!(spend_txn[2], node_txn[3].clone());
+       check_spends!(spend_txn[2], node_txn[2].clone());
 }
 
 #[test]
@@ -4082,9 +4087,9 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
        check_closed_broadcast!(nodes[0], false);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-       assert_eq!(node_txn.len(), 4);
-       assert_eq!(node_txn[3].input.len(), 1);
-       check_spends!(node_txn[3], revoked_htlc_txn[0].clone());
+       assert_eq!(node_txn.len(), 3); // ChannelMonitor: justice tx on revoked commitment, justice tx on revoked HTLC-success, ChannelManager: local commitment tx
+       assert_eq!(node_txn[2].input.len(), 1);
+       check_spends!(node_txn[2], revoked_htlc_txn[0].clone());
 
        // Check A's ChannelMonitor was able to generate the right spendable output descriptor
        let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -4092,8 +4097,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
        assert_eq!(spend_txn[0], spend_txn[2]);
        assert_eq!(spend_txn[1], spend_txn[3]);
        check_spends!(spend_txn[0], revoked_local_txn[0].clone()); // spending to_remote output from revoked local tx
-       check_spends!(spend_txn[1], node_txn[2].clone()); // spending justice tx output from revoked local tx htlc received output
-       check_spends!(spend_txn[4], node_txn[3].clone()); // spending justice tx output on htlc success tx
+       check_spends!(spend_txn[1], node_txn[0].clone()); // spending justice tx output from revoked local tx htlc received output
+       check_spends!(spend_txn[4], node_txn[2].clone()); // spending justice tx output on htlc success tx
 }
 
 #[test]
@@ -4149,8 +4154,8 @@ fn test_onchain_to_onchain_claim() {
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![c_txn[1].clone(), c_txn[2].clone()]}, 1);
        {
                let mut b_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(b_txn.len(), 4);
-               assert_eq!(b_txn[0], b_txn[3]);
+               // ChannelMonitor: claim tx, ChannelManager: local commitment tx + HTLC-timeout tx
+               assert_eq!(b_txn.len(), 3);
                check_spends!(b_txn[1], chan_2.3); // B local commitment tx, issued by ChannelManager
                check_spends!(b_txn[2], b_txn[1].clone()); // HTLC-Timeout on B local commitment tx, issued by ChannelManager
                assert_eq!(b_txn[2].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
@@ -4182,14 +4187,14 @@ fn test_onchain_to_onchain_claim() {
        let commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get_mut(&chan_1.2).unwrap().channel_monitor().get_latest_local_commitment_txn();
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![commitment_tx[0].clone()]}, 1);
        let b_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-       assert_eq!(b_txn.len(), 4);
-       check_spends!(b_txn[1], chan_1.3); // Local commitment tx, issued by ChannelManager
-       check_spends!(b_txn[2], b_txn[1]); // HTLC-Success tx, as a part of the local txn rebroadcast by ChannelManager in the force close
-       assert_eq!(b_txn[0], b_txn[3]); // HTLC-Success tx, issued by ChannelMonitor, * 2 due to block rescan
+       // ChannelMonitor: HTLC-Success tx, ChannelManager: local commitment tx + HTLC-Success tx
+       assert_eq!(b_txn.len(), 3);
+       check_spends!(b_txn[1], chan_1.3);
+       check_spends!(b_txn[2], b_txn[1].clone());
        check_spends!(b_txn[0], commitment_tx[0].clone());
        assert_eq!(b_txn[0].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
        assert!(b_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
-       assert_eq!(b_txn[2].lock_time, 0); // Success tx
+       assert_eq!(b_txn[0].lock_time, 0); // Success tx
 
        check_closed_broadcast!(nodes[1], false);
 }
@@ -4220,14 +4225,15 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
        let htlc_timeout_tx;
        { // Extract one of the two HTLC-Timeout transaction
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 7);
-               assert_eq!(node_txn[0], node_txn[5]);
-               assert_eq!(node_txn[1], node_txn[6]);
+               // ChannelMonitor: timeout tx * 2, ChannelManager: local commitment tx + HTLC-timeout * 2
+               assert_eq!(node_txn.len(), 5);
                check_spends!(node_txn[0], commitment_txn[0].clone());
                assert_eq!(node_txn[0].input.len(), 1);
                check_spends!(node_txn[1], commitment_txn[0].clone());
                assert_eq!(node_txn[1].input.len(), 1);
                assert_ne!(node_txn[0].input[0], node_txn[1].input[0]);
+               assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
                check_spends!(node_txn[2], chan_2.3.clone());
                check_spends!(node_txn[3], node_txn[2].clone());
                check_spends!(node_txn[4], node_txn[2].clone());
@@ -6373,7 +6379,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)));
-       let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
+       let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashSet::new())});
        let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
        monitor = test_utils::TestChannelMonitor::new(chain_monitor.clone(), tx_broadcaster.clone(), logger.clone(), feeest.clone());
        node_state_0 = {
@@ -6614,8 +6620,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() {
        let feerate_1;
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 4); // justice tx (broadcasted from ChannelMonitor) * 2 (block-reparsing) + local commitment tx + local HTLC-timeout (broadcasted from ChannelManager)
-               assert_eq!(node_txn[0], node_txn[3]);
+               assert_eq!(node_txn.len(), 3); // justice tx (broadcasted from ChannelMonitor) + local commitment tx + local HTLC-timeout (broadcasted from ChannelManager)
                assert_eq!(node_txn[0].input.len(), 3); // Penalty txn claims to_local, offered_htlc and received_htlc outputs
                assert_eq!(node_txn[0].output.len(), 1);
                check_spends!(node_txn[0], revoked_txn[0].clone());
@@ -6733,21 +6738,21 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        let feerate_2;
        {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 9); // 3 penalty txn on revoked commitment tx * 2 (block-rescan) + A commitment tx + 2 penalty tnx on revoked HTLC txn
+               assert_eq!(node_txn.len(), 6); // 3 penalty txn on revoked commitment tx + A commitment tx + 2 penalty tnx on revoked HTLC txn
                // Verify claim tx are spending revoked HTLC txn
-               assert_eq!(node_txn[7].input.len(), 1);
-               assert_eq!(node_txn[7].output.len(), 1);
-               check_spends!(node_txn[7], revoked_htlc_txn[0].clone());
-               first = node_txn[7].txid();
-               assert_eq!(node_txn[8].input.len(), 1);
-               assert_eq!(node_txn[8].output.len(), 1);
-               check_spends!(node_txn[8], revoked_htlc_txn[1].clone());
-               second = node_txn[8].txid();
+               assert_eq!(node_txn[4].input.len(), 1);
+               assert_eq!(node_txn[4].output.len(), 1);
+               check_spends!(node_txn[4], revoked_htlc_txn[0].clone());
+               first = node_txn[4].txid();
+               assert_eq!(node_txn[5].input.len(), 1);
+               assert_eq!(node_txn[5].output.len(), 1);
+               check_spends!(node_txn[5], revoked_htlc_txn[1].clone());
+               second = node_txn[5].txid();
                // Store both feerates for later comparison
-               let fee_1 = revoked_htlc_txn[0].output[0].value - node_txn[7].output[0].value;
-               feerate_1 = fee_1 * 1000 / node_txn[7].get_weight() as u64;
-               let fee_2 = revoked_htlc_txn[1].output[0].value - node_txn[8].output[0].value;
-               feerate_2 = fee_2 * 1000 / node_txn[8].get_weight() as u64;
+               let fee_1 = revoked_htlc_txn[0].output[0].value - node_txn[4].output[0].value;
+               feerate_1 = fee_1 * 1000 / node_txn[4].get_weight() as u64;
+               let fee_2 = revoked_htlc_txn[1].output[0].value - node_txn[5].output[0].value;
+               feerate_2 = fee_2 * 1000 / node_txn[5].get_weight() as u64;
                node_txn.clear();
        }
 
@@ -6862,9 +6867,7 @@ fn test_bump_penalty_txn_on_remote_commitment() {
        let feerate_preimage;
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 7); // 2 * claim tx (broadcasted from ChannelMonitor) * 2 (block-reparsing) + local commitment tx + local HTLC-timeout + HTLC-success (broadcasted from ChannelManager)
-               assert_eq!(node_txn[0], node_txn[5]);
-               assert_eq!(node_txn[1], node_txn[6]);
+               assert_eq!(node_txn.len(), 5); // 2 * claim tx (broadcasted from ChannelMonitor) + local commitment tx + local HTLC-timeout + local HTLC-success (broadcasted from ChannelManager)
                assert_eq!(node_txn[0].input.len(), 1);
                assert_eq!(node_txn[1].input.len(), 1);
                check_spends!(node_txn[0], remote_txn[0].clone());
@@ -6976,8 +6979,8 @@ fn test_set_outpoints_partial_claiming() {
        // Verify node A broadcast tx claiming both HTLCs
        {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 5);
-               assert_eq!(node_txn[0], node_txn[4]);
+               // ChannelMonitor: claim tx, ChannelManager: local commitment tx + HTLC-Success*2
+               assert_eq!(node_txn.len(), 4);
                check_spends!(node_txn[0], remote_txn[0].clone());
                check_spends!(node_txn[1], chan.3.clone());
                check_spends!(node_txn[2], node_txn[1]);
@@ -7087,7 +7090,7 @@ fn test_bump_txn_sanitize_tracking_maps() {
        check_closed_broadcast!(nodes[0], false);
        let penalty_txn = {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 7);
+               assert_eq!(node_txn.len(), 4); //ChannelMonitor: justice txn * 3, ChannelManager: local commitment tx
                check_spends!(node_txn[0], revoked_local_txn[0].clone());
                check_spends!(node_txn[1], revoked_local_txn[0].clone());
                check_spends!(node_txn[2], revoked_local_txn[0].clone());
index cd2064a4ea330cacc824a6dc453c371f96acaf61..b343ac35379f17195e785d70f6f6dea9dab5829e 100644 (file)
@@ -23,7 +23,7 @@ use secp256k1::{SecretKey, PublicKey};
 use std::time::{SystemTime, UNIX_EPOCH};
 use std::sync::{Arc,Mutex};
 use std::{mem};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 pub struct TestVecWriter(pub Vec<u8>);
 impl Writer for TestVecWriter {
@@ -81,9 +81,19 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 
 pub struct TestBroadcaster {
        pub txn_broadcasted: Mutex<Vec<Transaction>>,
+       pub broadcasted_txn: Mutex<HashSet<Sha256dHash>> // Temporary field while refactoring out tx duplication
 }
 impl chaininterface::BroadcasterInterface for TestBroadcaster {
        fn broadcast_transaction(&self, tx: &Transaction) {
+               {
+                       if let Some(_) = self.broadcasted_txn.lock().unwrap().get(&tx.txid()) {
+                               // If commitment tx, HTLC-timeout or HTLC-Success, duplicate broadcast are still ok
+                               if tx.input[0].sequence == 0xfffffffd {
+                                       return;
+                               }
+                       }
+               }
+               self.broadcasted_txn.lock().unwrap().insert(tx.txid());
                self.txn_broadcasted.lock().unwrap().push(tx.clone());
        }
 }