From: Wilmer Paulino Date: Sat, 15 Apr 2023 00:02:16 +0000 (-0700) Subject: Use existing height timer to retry untractable packages X-Git-Tag: v0.0.115~13^2~1 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=4828817f3f8425a0cf1874fc5000cae5ab95a9e8;p=rust-lightning Use existing height timer to retry untractable packages 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. --- diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index f690d3664..297b80421 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -583,7 +583,7 @@ impl OnchainTxHandler 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 { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 8747927df..6d9687762 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -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, has_htlc_tx: HTLCType) -> Vec { 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) -> Vec { 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); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index c07775c87..f2b05b6e1 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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); } } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index c589d5740..8631c9288 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -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() { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 183f520a5..65b0fc006 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -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 { diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 6d89268cd..14b06457f 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -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 diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index a34cb0cf3..46a7bb340 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -311,6 +311,17 @@ impl TestBroadcaster { pub fn new(blocks: Arc>>) -> TestBroadcaster { TestBroadcaster { txn_broadcasted: Mutex::new(Vec::new()), blocks } } + + pub fn txn_broadcast(&self) -> Vec { + self.txn_broadcasted.lock().unwrap().split_off(0) + } + + pub fn unique_txn_broadcast(&self) -> Vec { + 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 {