Merge pull request #2208 from wpaulino/monitor-rebroadcast-pending-claims
[rust-lightning] / lightning / src / ln / functional_tests.rs
index fd9bd7a768de96ecee75958179e80ea52370e19d..17f8e1597b7c0b2c34f064656c8ebf3de91e0a50 100644 (file)
@@ -18,10 +18,10 @@ use crate::chain::channelmonitor;
 use crate::chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
 use crate::chain::transaction::OutPoint;
 use crate::chain::keysinterface::{ChannelSigner, EcdsaChannelSigner, EntropySource};
-use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination};
+use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason};
 use crate::ln::{PaymentPreimage, PaymentSecret, PaymentHash};
 use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT};
-use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
+use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA};
 use crate::ln::channel::{Channel, ChannelError};
 use crate::ln::{chan_utils, onion_utils};
 use crate::ln::chan_utils::{OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
@@ -740,7 +740,9 @@ fn test_update_fee_that_funder_cannot_afford() {
        let commit_signed_msg = msgs::CommitmentSigned {
                channel_id: chan.2,
                signature: res.0,
-               htlc_signatures: res.1
+               htlc_signatures: res.1,
+               #[cfg(taproot)]
+               partial_signature_with_nonce: None,
        };
 
        let update_fee = msgs::UpdateFee {
@@ -1105,7 +1107,7 @@ fn holding_cell_htlc_counting() {
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2);
 
        let mut payments = Vec::new();
-       for _ in 0..crate::ln::channel::OUR_MAX_HTLCS {
+       for _ in 0..50 {
                let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000);
                nodes[1].node.send_payment_with_route(&route, payment_hash,
                        RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
@@ -1379,7 +1381,8 @@ fn test_fee_spike_violation_fails_htlc() {
        let cur_height = nodes[1].node.best_block.read().unwrap().height() + 1;
 
        let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
-       let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3460001, &Some(payment_secret), cur_height, &None).unwrap();
+       let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0],
+               3460001, RecipientOnionFields::secret_only(payment_secret), cur_height, &None).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
        let msg = msgs::UpdateAddHTLC {
                channel_id: chan.2,
@@ -1466,7 +1469,9 @@ fn test_fee_spike_violation_fails_htlc() {
        let commit_signed_msg = msgs::CommitmentSigned {
                channel_id: chan.2,
                signature: res.0,
-               htlc_signatures: res.1
+               htlc_signatures: res.1,
+               #[cfg(taproot)]
+               partial_signature_with_nonce: None,
        };
 
        // Send the commitment_signed message to the nodes[1].
@@ -1477,7 +1482,9 @@ fn test_fee_spike_violation_fails_htlc() {
        let raa_msg = msgs::RevokeAndACK {
                channel_id: chan.2,
                per_commitment_secret: local_secret,
-               next_per_commitment_point: next_local_point
+               next_per_commitment_point: next_local_point,
+               #[cfg(taproot)]
+               next_local_nonce: None,
        };
        nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &raa_msg);
 
@@ -1562,7 +1569,8 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
        let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
        let cur_height = nodes[1].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
-       let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 700_000, &Some(payment_secret), cur_height, &None).unwrap();
+       let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0],
+               700_000, RecipientOnionFields::secret_only(payment_secret), cur_height, &None).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
        let msg = msgs::UpdateAddHTLC {
                channel_id: chan.2,
@@ -1735,7 +1743,8 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() {
        let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
        let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route_2.paths[0], &session_priv).unwrap();
-       let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route_2.paths[0], recv_value_2, &None, cur_height, &None).unwrap();
+       let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(
+               &route_2.paths[0], recv_value_2, RecipientOnionFields::spontaneous_empty(), cur_height, &None).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash_1);
        let msg = msgs::UpdateAddHTLC {
                channel_id: chan.2,
@@ -1988,7 +1997,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
        let events = nodes[2].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
        match events[0] {
-               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        assert_eq!(our_payment_hash_21, *payment_hash);
                        assert_eq!(recv_value_21, amount_msat);
                        assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
@@ -2004,7 +2013,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
                _ => panic!("Unexpected event"),
        }
        match events[1] {
-               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        assert_eq!(our_payment_hash_22, *payment_hash);
                        assert_eq!(recv_value_22, amount_msat);
                        assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
@@ -2381,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;
@@ -2398,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);
 
@@ -2422,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);
                }
@@ -2441,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;
@@ -2629,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);
@@ -2640,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
@@ -2660,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);
        }
@@ -2961,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(), 3); // Two extra fee bumps for timeout transaction
+               }
+               txn.iter().for_each(|tx| check_spends!(tx, 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);
@@ -3378,7 +3395,8 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
                let secp_ctx = Secp256k1::new();
                let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
                let current_height = nodes[1].node.best_block.read().unwrap().height() + 1;
-               let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(&route.paths[0], 50_000, &Some(payment_secret), current_height, &None).unwrap();
+               let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(
+                       &route.paths[0], 50_000, RecipientOnionFields::secret_only(payment_secret), current_height, &None).unwrap();
                let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
                let onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
 
@@ -3443,7 +3461,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(node_txn.len(), 3);
-       assert_eq!(node_txn[0], node_txn[1]);
+       assert_eq!(node_txn[0].txid(), node_txn[1].txid());
 
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[1].clone()]});
@@ -3793,7 +3811,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
        let events_2 = nodes[1].node.get_and_clear_pending_events();
        assert_eq!(events_2.len(), 1);
        match events_2[0] {
-               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        assert_eq!(payment_hash_1, *payment_hash);
                        assert_eq!(amount_msat, 1_000_000);
                        assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
@@ -4118,8 +4136,11 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) {
                // indicates there are more HTLCs coming.
                let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
                let payment_id = PaymentId([42; 32]);
-               let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash, Some(payment_secret), payment_id, &route).unwrap();
-               nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[0]).unwrap();
+               let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash,
+                       RecipientOnionFields::secret_only(payment_secret), payment_id, &route).unwrap();
+               nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash,
+                       RecipientOnionFields::secret_only(payment_secret), 200_000, cur_height, payment_id,
+                       &None, session_privs[0]).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -4884,15 +4905,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], &updates.commitment_signed, false);
-
-       let events = nodes[0].node.get_and_clear_pending_events();
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(*payment_preimage, our_payment_preimage);
-                       assert_eq!(*payment_hash, duplicate_payment_hash);
-               }
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent(&nodes[0], our_payment_preimage, None, true);
 }
 
 #[test]
@@ -6252,7 +6265,8 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
        let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
        let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::signing_only(), &route.paths[0], &session_priv).unwrap();
-       let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3999999, &Some(our_payment_secret), cur_height, &None).unwrap();
+       let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(
+               &route.paths[0], 3999999, RecipientOnionFields::secret_only(our_payment_secret), cur_height, &None).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash);
 
        let mut msg = msgs::UpdateAddHTLC {
@@ -6264,11 +6278,11 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
                onion_routing_packet: onion_packet.clone(),
        };
 
-       for i in 0..super::channel::OUR_MAX_HTLCS {
+       for i in 0..50 {
                msg.htlc_id = i as u64;
                nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);
        }
-       msg.htlc_id = (super::channel::OUR_MAX_HTLCS) as u64;
+       msg.htlc_id = (50) as u64;
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);
 
        assert!(nodes[1].node.list_channels().is_empty());
@@ -7094,8 +7108,9 @@ fn test_announce_disable_channels() {
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
 
-       nodes[0].node.timer_tick_occurred(); // Enabled -> DisabledStaged
-       nodes[0].node.timer_tick_occurred(); // DisabledStaged -> Disabled
+       for _ in 0..DISABLE_GOSSIP_TICKS + 1 {
+               nodes[0].node.timer_tick_occurred();
+       }
        let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(msg_events.len(), 3);
        let mut chans_disabled = HashMap::new();
@@ -7135,7 +7150,9 @@ fn test_announce_disable_channels() {
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[2]);
        handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
-       nodes[0].node.timer_tick_occurred();
+       for _ in 0..ENABLE_GOSSIP_TICKS {
+               nodes[0].node.timer_tick_occurred();
+       }
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
        nodes[0].node.timer_tick_occurred();
        let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -7303,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!(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]);
+               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]);
+
+               txn
+       };
 
        // Broadcast set of revoked txn on A
        let hash_128 = connect_blocks(&nodes[0], 40);
@@ -7489,7 +7510,7 @@ fn test_bump_penalty_txn_on_remote_commitment() {
        assert_ne!(feerate_preimage, 0);
 
        // After exhaustion of height timer, new bumped claim txn should have been broadcast, check it
-       connect_blocks(&nodes[1], 15);
+       connect_blocks(&nodes[1], 1);
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 1);
@@ -7556,7 +7577,13 @@ fn test_counterparty_raa_skip_no_crash() {
        }
 
        nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
-               &msgs::RevokeAndACK { channel_id, per_commitment_secret, next_per_commitment_point });
+               &msgs::RevokeAndACK {
+                       channel_id,
+                       per_commitment_secret,
+                       next_per_commitment_point,
+                       #[cfg(taproot)]
+                       next_local_nonce: None,
+               });
        assert_eq!(check_closed_broadcast!(nodes[1], true).unwrap().data, "Received an unexpected revoke_and_ack");
        check_added_monitors!(nodes[1], 1);
        check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "Received an unexpected revoke_and_ack".to_string() });
@@ -8033,8 +8060,10 @@ fn test_onion_value_mpp_set_calculation() {
 
        // Send payment
        let payment_id = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes());
-       let onion_session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash, Some(our_payment_secret), payment_id, &route).unwrap();
-       nodes[0].node.test_send_payment_internal(&route, our_payment_hash, &Some(our_payment_secret), None, payment_id, Some(total_msat), onion_session_privs).unwrap();
+       let onion_session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), payment_id, &route).unwrap();
+       nodes[0].node.test_send_payment_internal(&route, our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), None, payment_id, Some(total_msat), onion_session_privs).unwrap();
        check_added_monitors!(nodes[0], expected_paths.len());
 
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -8053,7 +8082,8 @@ fn test_onion_value_mpp_set_calculation() {
                        let height = nodes[0].best_block_info().1;
                        let session_priv = SecretKey::from_slice(&session_priv).unwrap();
                        let mut onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
-                       let (mut onion_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], 100_000, &Some(our_payment_secret), height + 1, &None).unwrap();
+                       let (mut onion_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], 100_000,
+                               RecipientOnionFields::secret_only(our_payment_secret), height + 1, &None).unwrap();
                        // Edit amt_to_forward to simulate the sender having set
                        // the final amount and the routing node taking less fee
                        onion_payloads[1].amt_to_forward = 99_000;
@@ -8132,8 +8162,10 @@ fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64) {
 
        // Send payment with manually set total_msat
        let payment_id = PaymentId(nodes[src_idx].keys_manager.backing.get_secure_random_bytes());
-       let onion_session_privs = nodes[src_idx].node.test_add_new_pending_payment(our_payment_hash, Some(our_payment_secret), payment_id, &route).unwrap();
-       nodes[src_idx].node.test_send_payment_internal(&route, our_payment_hash, &Some(our_payment_secret), None, payment_id, Some(total_msat), onion_session_privs).unwrap();
+       let onion_session_privs = nodes[src_idx].node.test_add_new_pending_payment(our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), payment_id, &route).unwrap();
+       nodes[src_idx].node.test_send_payment_internal(&route, our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), None, payment_id, Some(total_msat), onion_session_privs).unwrap();
        check_added_monitors!(nodes[src_idx], expected_paths.len());
 
        let mut events = nodes[src_idx].node.get_and_clear_pending_msg_events();
@@ -8474,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);
@@ -8529,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);
        }
 }
 
@@ -9214,7 +9248,7 @@ fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_t
                // We should broadcast an HTLC transaction spending our funding transaction first
                let spending_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
                assert_eq!(spending_txn.len(), 2);
-               assert_eq!(spending_txn[0], node_txn[0]);
+               assert_eq!(spending_txn[0].txid(), node_txn[0].txid());
                check_spends!(spending_txn[1], node_txn[0]);
                // We should also generate a SpendableOutputs event with the to_self output (as its
                // timelock is up).
@@ -9384,9 +9418,12 @@ fn test_inconsistent_mpp_params() {
                // ultimately have, just not right away.
                let mut dup_route = route.clone();
                dup_route.paths.push(route.paths[1].clone());
-               nodes[0].node.test_add_new_pending_payment(our_payment_hash, Some(our_payment_secret), payment_id, &dup_route).unwrap()
+               nodes[0].node.test_add_new_pending_payment(our_payment_hash,
+                       RecipientOnionFields::secret_only(our_payment_secret), payment_id, &dup_route).unwrap()
        };
-       nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None, session_privs[0]).unwrap();
+       nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), 15_000_000, cur_height, payment_id,
+               &None, session_privs[0]).unwrap();
        check_added_monitors!(nodes[0], 1);
 
        {
@@ -9396,7 +9433,8 @@ fn test_inconsistent_mpp_params() {
        }
        assert!(nodes[3].node.get_and_clear_pending_events().is_empty());
 
-       nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash, &Some(our_payment_secret), 14_000_000, cur_height, payment_id, &None, session_privs[1]).unwrap();
+       nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), 14_000_000, cur_height, payment_id, &None, session_privs[1]).unwrap();
        check_added_monitors!(nodes[0], 1);
 
        {
@@ -9442,7 +9480,9 @@ fn test_inconsistent_mpp_params() {
 
        expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());
 
-       nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None, session_privs[2]).unwrap();
+       nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash,
+               RecipientOnionFields::secret_only(our_payment_secret), 15_000_000, cur_height, payment_id,
+               &None, session_privs[2]).unwrap();
        check_added_monitors!(nodes[0], 1);
 
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -9450,26 +9490,7 @@ fn test_inconsistent_mpp_params() {
        pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);
 
        do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 3);
-       match events[0] {
-               Event::PaymentSent { payment_hash, .. } => { // The payment was abandoned earlier, so the fee paid will be None
-                       assert_eq!(payment_hash, our_payment_hash);
-               },
-               _ => panic!("Unexpected event")
-       }
-       match events[1] {
-               Event::PaymentPathSuccessful { payment_hash, .. } => {
-                       assert_eq!(payment_hash.unwrap(), our_payment_hash);
-               },
-               _ => panic!("Unexpected event")
-       }
-       match events[2] {
-               Event::PaymentPathSuccessful { payment_hash, .. } => {
-                       assert_eq!(payment_hash.unwrap(), our_payment_hash);
-               },
-               _ => panic!("Unexpected event")
-       }
+       expect_payment_sent(&nodes[0], our_payment_preimage, Some(None), true);
 }
 
 #[test]
@@ -9577,7 +9598,7 @@ fn test_double_partial_claim() {
        ];
        expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], failed_destinations);
 
-       pass_failed_payment_back(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_hash);
+       pass_failed_payment_back(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_hash, PaymentFailureReason::RecipientRejected);
 
        // nodes[1] now retries one of the two paths...
        nodes[0].node.send_payment_with_route(&route, payment_hash,