Use existing height timer to retry untractable packages
authorWilmer Paulino <wilmer@wilmerpaulino.com>
Sat, 15 Apr 2023 00:02:16 +0000 (17:02 -0700)
committerWilmer Paulino <wilmer@wilmerpaulino.com>
Wed, 19 Apr 2023 23:49:35 +0000 (16:49 -0700)
Untractable packages are those which cannot have their fees updated once
signed, hence why they weren't retried. There's no harm in retrying
these packages by simply re-broadcasting them though, as the fee market
could have spontaneously spiked when we first broadcast it, leading to
our transaction not propagating throughout node mempools unless
broadcast manually.

lightning/src/chain/onchaintx.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/monitor_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/reorg_tests.rs
lightning/src/util/test_utils.rs

index f690d3664273b94e8ee9fe8d6d4c9f16d44ece0e..297b80421d656073985456bf81e28b46424e40da 100644 (file)
@@ -583,7 +583,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
                                None => return None,
                        };
                        if !cached_request.requires_external_funding() {
-                               return Some((None, 0, OnchainClaim::Tx(tx)));
+                               return Some((new_timer, 0, OnchainClaim::Tx(tx)));
                        }
                        #[cfg(anchors)]
                        return inputs.find_map(|input| match input {
index 8747927df8b467fc88cd17c62f64935fc3e1b245..6d96877625ccb4c74d1b41c0d56ee1ab5c46c1d0 100644 (file)
@@ -137,6 +137,20 @@ pub enum ConnectStyle {
 }
 
 impl ConnectStyle {
+       pub fn skips_blocks(&self) -> bool {
+               match self {
+                       ConnectStyle::BestBlockFirst => false,
+                       ConnectStyle::BestBlockFirstSkippingBlocks => true,
+                       ConnectStyle::BestBlockFirstReorgsOnlyTip => true,
+                       ConnectStyle::TransactionsFirst => false,
+                       ConnectStyle::TransactionsFirstSkippingBlocks => true,
+                       ConnectStyle::TransactionsDuplicativelyFirstSkippingBlocks => true,
+                       ConnectStyle::HighlyRedundantTransactionsFirstSkippingBlocks => true,
+                       ConnectStyle::TransactionsFirstReorgsOnlyTip => true,
+                       ConnectStyle::FullBlockViaListen => false,
+               }
+       }
+
        fn random_style() -> ConnectStyle {
                #[cfg(feature = "std")] {
                        use core::hash::{BuildHasher, Hasher};
@@ -164,12 +178,7 @@ impl ConnectStyle {
 }
 
 pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) -> BlockHash {
-       let skip_intermediaries = match *node.connect_style.borrow() {
-               ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks|
-                       ConnectStyle::TransactionsDuplicativelyFirstSkippingBlocks|ConnectStyle::HighlyRedundantTransactionsFirstSkippingBlocks|
-                       ConnectStyle::BestBlockFirstReorgsOnlyTip|ConnectStyle::TransactionsFirstReorgsOnlyTip => true,
-               _ => false,
-       };
+       let skip_intermediaries = node.connect_style.borrow().skips_blocks();
 
        let height = node.best_block_info().1 + 1;
        let mut block = Block {
@@ -2535,6 +2544,8 @@ pub enum HTLCType { NONE, TIMEOUT, SUCCESS }
 /// also fail.
 pub fn test_txn_broadcast<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, chan: &(msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction), commitment_tx: Option<Transaction>, has_htlc_tx: HTLCType) -> Vec<Transaction>  {
        let mut node_txn = node.tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let mut txn_seen = HashSet::new();
+       node_txn.retain(|tx| txn_seen.insert(tx.txid()));
        assert!(node_txn.len() >= if commitment_tx.is_some() { 0 } else { 1 } + if has_htlc_tx == HTLCType::NONE { 0 } else { 1 });
 
        let mut res = Vec::with_capacity(2);
@@ -2598,22 +2609,23 @@ pub fn test_revoked_htlc_claim_txn_broadcast<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>
 
 pub fn check_preimage_claim<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, prev_txn: &Vec<Transaction>) -> Vec<Transaction>  {
        let mut node_txn = node.tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let mut txn_seen = HashSet::new();
+       node_txn.retain(|tx| txn_seen.insert(tx.txid()));
 
-       assert!(node_txn.len() >= 1);
-       assert_eq!(node_txn[0].input.len(), 1);
        let mut found_prev = false;
-
-       for tx in prev_txn {
-               if node_txn[0].input[0].previous_output.txid == tx.txid() {
-                       check_spends!(node_txn[0], tx);
-                       let mut iter = node_txn[0].input[0].witness.iter();
-                       iter.next().expect("expected 3 witness items");
-                       iter.next().expect("expected 3 witness items");
-                       assert!(iter.next().expect("expected 3 witness items").len() > 106); // must spend an htlc output
-                       assert_eq!(tx.input.len(), 1); // must spend a commitment tx
-
-                       found_prev = true;
-                       break;
+       for prev_tx in prev_txn {
+               for tx in &*node_txn {
+                       if tx.input[0].previous_output.txid == prev_tx.txid() {
+                               check_spends!(tx, prev_tx);
+                               let mut iter = tx.input[0].witness.iter();
+                               iter.next().expect("expected 3 witness items");
+                               iter.next().expect("expected 3 witness items");
+                               assert!(iter.next().expect("expected 3 witness items").len() > 106); // must spend an htlc output
+                               assert_eq!(tx.input.len(), 1); // must spend a commitment tx
+
+                               found_prev = true;
+                               break;
+                       }
                }
        }
        assert!(found_prev);
index c07775c8785bfd24adbd1e1639ec37649845b283..f2b05b6e1ede4fa05d1fd8eee35679f82e0c88e6 100644 (file)
@@ -2390,8 +2390,8 @@ fn channel_monitor_network_test() {
 }
 
 #[test]
-fn test_justice_tx() {
-       // Test justice txn built on revoked HTLC-Success tx, against both sides
+fn test_justice_tx_htlc_timeout() {
+       // Test justice txn built on revoked HTLC-Timeout tx, against both sides
        let mut alice_config = UserConfig::default();
        alice_config.channel_handshake_config.announced_channel = true;
        alice_config.channel_handshake_limits.force_announced_channel_preference = false;
@@ -2407,7 +2407,6 @@ fn test_justice_tx() {
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &user_cfgs);
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-       *nodes[0].connect_style.borrow_mut() = ConnectStyle::FullBlockViaListen;
        // Create some new channels:
        let chan_5 = create_announced_chan_between_nodes(&nodes, 0, 1);
 
@@ -2431,7 +2430,6 @@ fn test_justice_tx() {
                        let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                        assert_eq!(node_txn.len(), 1); // ChannelMonitor: penalty 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]);
                        node_txn.swap_remove(0);
                }
@@ -2450,17 +2448,30 @@ fn test_justice_tx() {
                test_revoked_htlc_claim_txn_broadcast(&nodes[1], node_txn[1].clone(), revoked_local_txn[0].clone());
        }
        get_announce_close_broadcast_events(&nodes, 0, 1);
-
        assert_eq!(nodes[0].node.list_channels().len(), 0);
        assert_eq!(nodes[1].node.list_channels().len(), 0);
+}
 
-       // We test justice_tx build by A on B's revoked HTLC-Success tx
+#[test]
+fn test_justice_tx_htlc_success() {
+       // Test justice txn built on revoked HTLC-Success tx, against both sides
+       let mut alice_config = UserConfig::default();
+       alice_config.channel_handshake_config.announced_channel = true;
+       alice_config.channel_handshake_limits.force_announced_channel_preference = false;
+       alice_config.channel_handshake_config.our_to_self_delay = 6 * 24 * 5;
+       let mut bob_config = UserConfig::default();
+       bob_config.channel_handshake_config.announced_channel = true;
+       bob_config.channel_handshake_limits.force_announced_channel_preference = false;
+       bob_config.channel_handshake_config.our_to_self_delay = 6 * 24 * 3;
+       let user_cfgs = [Some(alice_config), Some(bob_config)];
+       let mut chanmon_cfgs = create_chanmon_cfgs(2);
+       chanmon_cfgs[0].keys_manager.disable_revocation_policy_check = true;
+       chanmon_cfgs[1].keys_manager.disable_revocation_policy_check = true;
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &user_cfgs);
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        // Create some new channels:
        let chan_6 = create_announced_chan_between_nodes(&nodes, 0, 1);
-       {
-               let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               node_txn.clear();
-       }
 
        // A pending HTLC which will be revoked:
        let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
@@ -2638,8 +2649,7 @@ fn claim_htlc_outputs_single_tx() {
                connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
                assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
 
-               let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-               assert_eq!(node_txn.len(), 7);
+               let mut node_txn = nodes[1].tx_broadcaster.txn_broadcast();
 
                // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration
                assert_eq!(node_txn[0].input.len(), 1);
@@ -2649,19 +2659,22 @@ fn claim_htlc_outputs_single_tx() {
                assert_eq!(witness_script.len(), OFFERED_HTLC_SCRIPT_WEIGHT); //Spending an offered htlc output
                check_spends!(node_txn[1], node_txn[0]);
 
-               // Justice transactions are indices 2-3-4
+               // Filter out any non justice transactions.
+               node_txn.retain(|tx| tx.input[0].previous_output.txid == revoked_local_txn[0].txid());
+               assert!(node_txn.len() > 3);
+
+               assert_eq!(node_txn[0].input.len(), 1);
+               assert_eq!(node_txn[1].input.len(), 1);
                assert_eq!(node_txn[2].input.len(), 1);
-               assert_eq!(node_txn[3].input.len(), 1);
-               assert_eq!(node_txn[4].input.len(), 1);
 
+               check_spends!(node_txn[0], revoked_local_txn[0]);
+               check_spends!(node_txn[1], revoked_local_txn[0]);
                check_spends!(node_txn[2], revoked_local_txn[0]);
-               check_spends!(node_txn[3], revoked_local_txn[0]);
-               check_spends!(node_txn[4], revoked_local_txn[0]);
 
                let mut witness_lens = BTreeSet::new();
+               witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
+               witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
                witness_lens.insert(node_txn[2].input[0].witness.last().unwrap().len());
-               witness_lens.insert(node_txn[3].input[0].witness.last().unwrap().len());
-               witness_lens.insert(node_txn[4].input[0].witness.last().unwrap().len());
                assert_eq!(witness_lens.len(), 3);
                assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
                assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
@@ -2669,9 +2682,9 @@ fn claim_htlc_outputs_single_tx() {
 
                // Finally, mine the penalty transactions and check that we get an HTLC failure after
                // ANTI_REORG_DELAY confirmations.
+               mine_transaction(&nodes[1], &node_txn[0]);
+               mine_transaction(&nodes[1], &node_txn[1]);
                mine_transaction(&nodes[1], &node_txn[2]);
-               mine_transaction(&nodes[1], &node_txn[3]);
-               mine_transaction(&nodes[1], &node_txn[4]);
                connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
                expect_payment_failed!(nodes[1], payment_hash_2, false);
        }
@@ -2970,25 +2983,20 @@ fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) {
 
        // Broadcast timeout transaction by B on received output from C's commitment tx on B's chain
        // Verify that B's ChannelManager is able to detect that HTLC is timeout by its own tx and react backward in consequence
-       connect_blocks(&nodes[1], 200 - nodes[2].best_block_info().1);
        mine_transaction(&nodes[1], &commitment_tx[0]);
-       check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
-       let timeout_tx;
-       {
-               let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 3); // 2 (local commitment tx + HTLC-timeout), 1 timeout tx
-
-               check_spends!(node_txn[2], commitment_tx[0]);
-               assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-
-               check_spends!(node_txn[0], chan_2.3);
-               check_spends!(node_txn[1], node_txn[0]);
-               assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), 71);
-               assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-
-               timeout_tx = node_txn[2].clone();
-               node_txn.clear();
-       }
+       check_closed_event(&nodes[1], 1, ClosureReason::CommitmentTxConfirmed, false);
+       connect_blocks(&nodes[1], 200 - nodes[2].best_block_info().1);
+       let timeout_tx = {
+               let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
+               if nodes[1].connect_style.borrow().skips_blocks() {
+                       assert_eq!(txn.len(), 1);
+               } else {
+                       assert_eq!(txn.len(), 2); // Extra rebroadcast of timeout transaction
+               }
+               check_spends!(txn[0], commitment_tx[0]);
+               assert_eq!(txn[0].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               txn.remove(0)
+       };
 
        mine_transaction(&nodes[1], &timeout_tx);
        check_added_monitors!(nodes[1], 1);
@@ -7312,17 +7320,21 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
        connect_blocks(&nodes[1], 49); // Confirm blocks until the HTLC expires (note CLTV was explicitly 50 above)
 
-       let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-       assert_eq!(revoked_htlc_txn.len(), 2);
+       let revoked_htlc_txn = {
+               let txn = nodes[1].tx_broadcaster.unique_txn_broadcast();
+               assert_eq!(txn.len(), 2);
 
-       assert_eq!(revoked_htlc_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-       assert_eq!(revoked_htlc_txn[0].input.len(), 1);
-       check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
+               assert_eq!(txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               assert_eq!(txn[0].input.len(), 1);
+               check_spends!(txn[0], revoked_local_txn[0]);
+
+               assert_eq!(txn[1].input.len(), 1);
+               assert_eq!(txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+               assert_eq!(txn[1].output.len(), 1);
+               check_spends!(txn[1], revoked_local_txn[0]);
 
-       assert_eq!(revoked_htlc_txn[1].input.len(), 1);
-       assert_eq!(revoked_htlc_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-       assert_eq!(revoked_htlc_txn[1].output.len(), 1);
-       check_spends!(revoked_htlc_txn[1], revoked_local_txn[0]);
+               txn
+       };
 
        // Broadcast set of revoked txn on A
        let hash_128 = connect_blocks(&nodes[0], 40);
@@ -8494,11 +8506,11 @@ fn test_concurrent_monitor_claim() {
        watchtower_alice.chain_monitor.block_connected(&block, CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
        // Watchtower Alice should have broadcast a commitment/HTLC-timeout
-       {
-               let mut txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let alice_state = {
+               let mut txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcast();
                assert_eq!(txn.len(), 2);
-               txn.clear();
-       }
+               txn.remove(0)
+       };
 
        // Copy ChainMonitor to simulate watchtower Bob and make it receive a commitment update first.
        let chain_source = test_utils::TestChainSource::new(Network::Testnet);
@@ -8549,19 +8561,21 @@ fn test_concurrent_monitor_claim() {
        // Watchtower Bob should have broadcast a commitment/HTLC-timeout
        let bob_state_y;
        {
-               let mut txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+               let mut txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcast();
                assert_eq!(txn.len(), 2);
-               bob_state_y = txn[0].clone();
-               txn.clear();
+               bob_state_y = txn.remove(0);
        };
 
        // We confirm Bob's state Y on Alice, she should broadcast a HTLC-timeout
        let header = BlockHeader { version: 0x20000000, prev_blockhash: BlockHash::all_zeros(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 };
        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();
-               assert_eq!(htlc_txn.len(), 1);
+               let htlc_txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcast();
+               assert_eq!(htlc_txn.len(), 2);
                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);
        }
 }
 
index c589d57405b0d52c8b6ebfbe59343d457ae0ce46..8631c9288056b4c4f86c33c7ab23a6bcd9b73c22 100644 (file)
@@ -30,9 +30,7 @@ use crate::ln::msgs::ChannelMessageHandler;
 use crate::util::config::UserConfig;
 #[cfg(anchors)]
 use crate::util::crypto::sign;
-#[cfg(anchors)]
 use crate::util::ser::Writeable;
-#[cfg(anchors)]
 use crate::util::test_utils;
 
 #[cfg(anchors)]
@@ -1324,20 +1322,29 @@ fn test_revoked_counterparty_htlc_tx_balances() {
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
-       let revoked_htlc_success_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-
-       assert_eq!(revoked_htlc_success_txn.len(), 1);
-       assert_eq!(revoked_htlc_success_txn[0].input.len(), 1);
-       assert_eq!(revoked_htlc_success_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-       check_spends!(revoked_htlc_success_txn[0], revoked_local_txn[0]);
+       let revoked_htlc_success = {
+               let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
+               assert_eq!(txn.len(), 1);
+               assert_eq!(txn[0].input.len(), 1);
+               assert_eq!(txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               check_spends!(txn[0], revoked_local_txn[0]);
+               txn.pop().unwrap()
+       };
 
        connect_blocks(&nodes[1], TEST_FINAL_CLTV);
-       let revoked_htlc_timeout_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-       assert_eq!(revoked_htlc_timeout_txn.len(), 1);
-       check_spends!(revoked_htlc_timeout_txn[0], revoked_local_txn[0]);
-       assert_ne!(revoked_htlc_success_txn[0].input[0].previous_output, revoked_htlc_timeout_txn[0].input[0].previous_output);
-       assert_eq!(revoked_htlc_success_txn[0].lock_time.0, 0);
-       assert_ne!(revoked_htlc_timeout_txn[0].lock_time.0, 0);
+       let revoked_htlc_timeout = {
+               let mut txn = nodes[1].tx_broadcaster.unique_txn_broadcast();
+               assert_eq!(txn.len(), 2);
+               if txn[0].input[0].previous_output == revoked_htlc_success.input[0].previous_output {
+                       txn.remove(1)
+               } else {
+                       txn.remove(0)
+               }
+       };
+       check_spends!(revoked_htlc_timeout, revoked_local_txn[0]);
+       assert_ne!(revoked_htlc_success.input[0].previous_output, revoked_htlc_timeout.input[0].previous_output);
+       assert_eq!(revoked_htlc_success.lock_time.0, 0);
+       assert_ne!(revoked_htlc_timeout.lock_time.0, 0);
 
        // A will generate justice tx from B's revoked commitment/HTLC tx
        mine_transaction(&nodes[0], &revoked_local_txn[0]);
@@ -1369,10 +1376,10 @@ fn test_revoked_counterparty_htlc_tx_balances() {
        assert_eq!(as_balances,
                sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));
 
-       mine_transaction(&nodes[0], &revoked_htlc_success_txn[0]);
+       mine_transaction(&nodes[0], &revoked_htlc_success);
        let as_htlc_claim_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(as_htlc_claim_tx.len(), 2);
-       check_spends!(as_htlc_claim_tx[0], revoked_htlc_success_txn[0]);
+       check_spends!(as_htlc_claim_tx[0], revoked_htlc_success);
        check_spends!(as_htlc_claim_tx[1], revoked_local_txn[0]); // A has to generate a new claim for the remaining revoked
                                                                  // outputs (which no longer includes the spent HTLC output)
 
@@ -1381,7 +1388,7 @@ fn test_revoked_counterparty_htlc_tx_balances() {
 
        assert_eq!(as_htlc_claim_tx[0].output.len(), 1);
        fuzzy_assert_eq(as_htlc_claim_tx[0].output[0].value,
-               3_000 - chan_feerate * (revoked_htlc_success_txn[0].weight() + as_htlc_claim_tx[0].weight()) as u64 / 1000);
+               3_000 - chan_feerate * (revoked_htlc_success.weight() + as_htlc_claim_tx[0].weight()) as u64 / 1000);
 
        mine_transaction(&nodes[0], &as_htlc_claim_tx[0]);
        assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations {
@@ -1423,7 +1430,7 @@ fn test_revoked_counterparty_htlc_tx_balances() {
                }]),
                sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));
 
-       connect_blocks(&nodes[0], revoked_htlc_timeout_txn[0].lock_time.0 - nodes[0].best_block_info().1);
+       connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.0 - nodes[0].best_block_info().1);
        expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
                [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
        // As time goes on A may split its revocation claim transaction into multiple.
@@ -1440,11 +1447,11 @@ fn test_revoked_counterparty_htlc_tx_balances() {
                check_spends!(tx, revoked_local_txn[0]);
        }
 
-       mine_transaction(&nodes[0], &revoked_htlc_timeout_txn[0]);
+       mine_transaction(&nodes[0], &revoked_htlc_timeout);
        let as_second_htlc_claim_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(as_second_htlc_claim_tx.len(), 2);
 
-       check_spends!(as_second_htlc_claim_tx[0], revoked_htlc_timeout_txn[0]);
+       check_spends!(as_second_htlc_claim_tx[0], revoked_htlc_timeout);
        check_spends!(as_second_htlc_claim_tx[1], revoked_local_txn[0]);
 
        // Connect blocks to finalize the HTLC resolution with the HTLC-Timeout transaction. In a
@@ -1695,6 +1702,65 @@ fn test_revoked_counterparty_aggregated_claims() {
        assert!(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances().is_empty());
 }
 
+fn do_test_restored_packages_retry() {
+       // Tests that we'll retry packages that were previously timelocked after we've restored them.
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       // Open a channel, lock in an HTLC, and immediately broadcast the commitment transaction. This
+       // ensures that the HTLC timeout package is held until we reach its expiration height.
+       let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000);
+       route_payment(&nodes[0], &[&nodes[1]], 10_000_000);
+
+       nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id()).unwrap();
+       check_added_monitors(&nodes[0], 1);
+       check_closed_broadcast(&nodes[0], 1, true);
+       check_closed_event(&nodes[0], 1, ClosureReason::HolderForceClosed, false);
+
+       let commitment_tx = {
+               let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
+               assert_eq!(txn.len(), 1);
+               assert_eq!(txn[0].output.len(), 3);
+               check_spends!(txn[0], funding_tx);
+               txn.pop().unwrap()
+       };
+
+       mine_transaction(&nodes[0], &commitment_tx);
+
+       // Connect blocks until the HTLC's expiration is met, expecting a transaction broadcast.
+       connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1);
+       let htlc_timeout_tx = {
+               let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
+               assert_eq!(txn.len(), 1);
+               check_spends!(txn[0], commitment_tx);
+               txn.pop().unwrap()
+       };
+
+       // Connecting more blocks should result in the HTLC transactions being rebroadcast.
+       connect_blocks(&nodes[0], 6);
+       {
+               let txn = nodes[0].tx_broadcaster.txn_broadcast();
+               if !nodes[0].connect_style.borrow().skips_blocks() {
+                       assert_eq!(txn.len(), 6);
+               } else {
+                       assert!(txn.len() < 6);
+               }
+               for tx in txn {
+                       assert_eq!(tx.input.len(), htlc_timeout_tx.input.len());
+                       assert_eq!(tx.output.len(), htlc_timeout_tx.output.len());
+                       assert_eq!(tx.input[0].previous_output, htlc_timeout_tx.input[0].previous_output);
+                       assert_eq!(tx.output[0], htlc_timeout_tx.output[0]);
+               }
+       }
+}
+
+#[test]
+fn test_restored_packages_retry() {
+       do_test_restored_packages_retry();
+}
+
 #[cfg(anchors)]
 #[test]
 fn test_yield_anchors_events() {
index 183f520a50933f4bb41baf10e0a92b641107a676..65b0fc00628a6a5237591f9bcc58647a5575948a 100644 (file)
@@ -406,9 +406,11 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
        mine_transaction(&nodes[0], &bs_htlc_claim_txn[0]);
        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(), 2);
-       let (first_htlc_timeout_tx, second_htlc_timeout_tx) = (&as_htlc_timeout_txn[0], &as_htlc_timeout_txn[1]);
+       let (first_htlc_timeout_tx, second_htlc_timeout_tx) = {
+               let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast();
+               assert_eq!(txn.len(), 2);
+               (txn.remove(0), txn.remove(0))
+       };
        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 {
index 6d89268cdc34357808178ec5a99d2330dcff61dd..14b06457fc3f66b497ba86c09ca8409fb00dcd0e 100644 (file)
@@ -9,7 +9,7 @@
 
 //! Further functional tests which test blockchain reorganizations.
 
-use crate::chain::channelmonitor::ANTI_REORG_DELAY;
+use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS};
 use crate::chain::transaction::OutPoint;
 use crate::chain::Confirm;
 use crate::events::{Event, MessageSendEventsProvider, ClosureReason, HTLCDestination};
@@ -467,19 +467,21 @@ fn test_set_outpoints_partial_claiming() {
        }
 
        // Connect blocks on node B
-       connect_blocks(&nodes[1], 135);
+       connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        check_closed_broadcast!(nodes[1], true);
        check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
        check_added_monitors!(nodes[1], 1);
        // Verify node B broadcast 2 HTLC-timeout txn
        let partial_claim_tx = {
-               let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+               let mut node_txn = nodes[1].tx_broadcaster.unique_txn_broadcast();
                assert_eq!(node_txn.len(), 3);
+               check_spends!(node_txn[0], chan.3);
                check_spends!(node_txn[1], node_txn[0]);
                check_spends!(node_txn[2], node_txn[0]);
                assert_eq!(node_txn[1].input.len(), 1);
                assert_eq!(node_txn[2].input.len(), 1);
-               node_txn[1].clone()
+               assert_ne!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output);
+               node_txn.remove(1)
        };
 
        // Broadcast partial claim on node A, should regenerate a claiming tx with HTLC dropped
index a34cb0cf323a337edc757579c761d1363948212b..46a7bb340070618b85e69f860148b0319f14cea4 100644 (file)
@@ -311,6 +311,17 @@ impl TestBroadcaster {
        pub fn new(blocks: Arc<Mutex<Vec<(Block, u32)>>>) -> TestBroadcaster {
                TestBroadcaster { txn_broadcasted: Mutex::new(Vec::new()), blocks }
        }
+
+       pub fn txn_broadcast(&self) -> Vec<Transaction> {
+               self.txn_broadcasted.lock().unwrap().split_off(0)
+       }
+
+       pub fn unique_txn_broadcast(&self) -> Vec<Transaction> {
+               let mut txn = self.txn_broadcasted.lock().unwrap().split_off(0);
+               let mut seen = HashSet::new();
+               txn.retain(|tx| seen.insert(tx.txid()));
+               txn
+       }
 }
 
 impl chaininterface::BroadcasterInterface for TestBroadcaster {