Fix channel reserve calculation on the sending side
[rust-lightning] / lightning / src / ln / functional_tests.rs
index 962e69d27c273b8e90b67a9c431f23b0827b2d7a..d0b2bda047b9cb3b835052423b95f41a6fb03d06 100644 (file)
@@ -12,8 +12,7 @@
 //! claim outputs on-chain.
 
 use chain;
-use chain::Listen;
-use chain::Watch;
+use chain::{Confirm, Listen, Watch};
 use chain::channelmonitor;
 use chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
 use chain::transaction::OutPoint;
@@ -23,7 +22,9 @@ use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
-use routing::router::{Route, RouteHop, get_route};
+use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT;
+use routing::router::{Route, RouteHop, RouteHint, RouteHintHop, get_route};
+use routing::network_graph::RoutingFees;
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction};
@@ -52,9 +53,8 @@ use regex;
 
 use prelude::*;
 use alloc::collections::BTreeSet;
-use std::collections::{HashMap, HashSet};
 use core::default::Default;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 
 use ln::functional_test_utils::*;
 use ln::chan_utils::CommitmentTransaction;
@@ -1041,7 +1041,8 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
                nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_1_2nd_shutdown);
                node_0_2nd_shutdown
        } else {
-               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+               let node_0_chan_update = get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+               assert_eq!(node_0_chan_update.contents.flags & 2, 0); // "disabled" flag must not be set as we just reconnected.
                nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_1_2nd_shutdown);
                get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id())
        };
@@ -1361,15 +1362,7 @@ fn holding_cell_htlc_counting() {
        nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_updates.update_fail_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], bs_fail_updates.commitment_signed, false, true);
 
-       let events = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               MessageSendEvent::PaymentFailureNetworkUpdate { update: msgs::HTLCFailChannelUpdate::ChannelUpdateMessage { ref msg }} => {
-                       assert_eq!(msg.contents.short_channel_id, chan_2.0.contents.short_channel_id);
-               },
-               _ => panic!("Unexpected event"),
-       }
-
+       expect_payment_failure_chan_update!(nodes[0], chan_2.0.contents.short_channel_id, false);
        expect_payment_failed!(nodes[0], payment_hash_2, false);
 
        // Now forward all the pending HTLCs and claim them back
@@ -1500,26 +1493,25 @@ fn test_duplicate_htlc_different_direction_onchain() {
 
        // Check we only broadcast 1 timeout tx
        let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
-       assert_eq!(claim_txn.len(), 9);
-       assert_eq!(claim_txn[1], claim_txn[5]);
-       assert_eq!(claim_txn[2], claim_txn[6]);
-       assert_eq!(claim_txn[3], claim_txn[8]);
+       assert_eq!(claim_txn.len(), 8);
+       assert_eq!(claim_txn[1], claim_txn[4]);
+       assert_eq!(claim_txn[2], claim_txn[5]);
        check_spends!(claim_txn[1], chan_1.3);
        check_spends!(claim_txn[2], claim_txn[1]);
-       check_spends!(claim_txn[3], claim_txn[1]);
+       check_spends!(claim_txn[7], claim_txn[1]);
 
        assert_eq!(claim_txn[0].input.len(), 1);
-       assert_eq!(claim_txn[4].input.len(), 1);
-       assert_eq!(claim_txn[0].input[0].previous_output, claim_txn[4].input[0].previous_output);
+       assert_eq!(claim_txn[3].input.len(), 1);
+       assert_eq!(claim_txn[0].input[0].previous_output, claim_txn[3].input[0].previous_output);
 
        assert_eq!(claim_txn[0].input.len(), 1);
        assert_eq!(claim_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
        check_spends!(claim_txn[0], remote_txn[0]);
        assert_eq!(remote_txn[0].output[claim_txn[0].input[0].previous_output.vout as usize].value, 800);
-       assert_eq!(claim_txn[7].input.len(), 1);
-       assert_eq!(claim_txn[7].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
-       check_spends!(claim_txn[7], remote_txn[0]);
-       assert_eq!(remote_txn[0].output[claim_txn[7].input[0].previous_output.vout as usize].value, 900);
+       assert_eq!(claim_txn[6].input.len(), 1);
+       assert_eq!(claim_txn[6].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
+       check_spends!(claim_txn[6], remote_txn[0]);
+       assert_eq!(remote_txn[0].output[claim_txn[6].input[0].previous_output.vout as usize].value, 900);
 
        let events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 3);
@@ -1696,7 +1688,8 @@ fn test_fee_spike_violation_fails_htlc() {
                },
                _ => panic!("Unexpected event"),
        };
-       nodes[1].logger.assert_log("lightning::ln::channel".to_string(), "Attempting to fail HTLC due to fee spike buffer violation".to_string(), 1);
+       nodes[1].logger.assert_log("lightning::ln::channel".to_string(),
+               format!("Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", ::hex::encode(raa_msg.channel_id)), 1);
 
        check_added_monitors!(nodes[1], 2);
 }
@@ -1708,14 +1701,24 @@ fn test_chan_reserve_violation_outbound_htlc_inbound_chan() {
        // sending any above-dust amount would result in a channel reserve violation.
        // In this test we check that we would be prevented from sending an HTLC in
        // this situation.
-       chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 };
-       chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 };
+       let feerate_per_kw = 253;
+       chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(feerate_per_kw) };
+       chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(feerate_per_kw) };
        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);
-       let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
 
-       let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 4843000);
+       let mut push_amt = 100_000_000;
+       push_amt -= feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 * 1000;
+       push_amt -= Channel::<EnforcingSigner>::get_holder_selected_channel_reserve_satoshis(100_000) * 1000;
+
+       let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt, InitFeatures::known(), InitFeatures::known());
+
+       // Sending exactly enough to hit the reserve amount should be accepted
+       let (_, _, _) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000);
+
+       // However one more HTLC should be significantly over the reserve amount and fail.
+       let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 1_000_000);
        unwrap_send_err!(nodes[1].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)), true, APIError::ChannelUnavailable { ref err },
                assert_eq!(err, "Cannot send value that would put counterparty balance under holder-announced channel reserve value"));
        assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
@@ -1730,8 +1733,8 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
        // to channel reserve violation. This close could also happen if the fee went
        // up a more realistic amount, but many HTLCs were outstanding at the time of
        // the update_add_htlc.
-       chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 };
-       chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 };
+       chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(6000) };
+       chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(6000) };
        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);
@@ -1767,7 +1770,11 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
 fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() {
        // Test that if we receive many dust HTLCs over an outbound channel, they don't count when
        // calculating our commitment transaction fee (this was previously broken).
-       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let mut chanmon_cfgs = create_chanmon_cfgs(2);
+       let feerate_per_kw = 253;
+       chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(feerate_per_kw) };
+       chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(feerate_per_kw) };
+
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
@@ -1775,13 +1782,22 @@ fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() {
        // Set nodes[0]'s balance such that they will consider any above-dust received HTLC to be a
        // channel reserve violation (so their balance is channel reserve (1000 sats) + commitment
        // transaction fee with 0 HTLCs (183 sats)).
-       create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 98817000, InitFeatures::known(), InitFeatures::known());
+       let mut push_amt = 100_000_000;
+       push_amt -= feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT) / 1000 * 1000;
+       push_amt -= Channel::<EnforcingSigner>::get_holder_selected_channel_reserve_satoshis(100_000) * 1000;
+       create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, push_amt, InitFeatures::known(), InitFeatures::known());
 
-       let dust_amt = 329000; // Dust amount
+       let dust_amt = crate::ln::channel::MIN_DUST_LIMIT_SATOSHIS * 1000
+               + feerate_per_kw as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000 * 1000 - 1;
        // In the previous code, routing this dust payment would cause nodes[0] to perceive a channel
        // reserve violation even though it's a dust HTLC and therefore shouldn't count towards the
        // commitment transaction fee.
        let (_, _, _) = route_payment(&nodes[1], &[&nodes[0]], dust_amt);
+
+       // One more than the dust amt should fail, however.
+       let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], dust_amt + 1);
+       unwrap_send_err!(nodes[1].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)), true, APIError::ChannelUnavailable { ref err },
+               assert_eq!(err, "Cannot send value that would put counterparty balance under holder-announced channel reserve value"));
 }
 
 #[test]
@@ -1888,11 +1904,12 @@ fn test_inbound_outbound_capacity_is_not_zero() {
        assert_eq!(channels0.len(), 1);
        assert_eq!(channels1.len(), 1);
 
-       assert_eq!(channels0[0].inbound_capacity_msat, 95000000);
-       assert_eq!(channels1[0].outbound_capacity_msat, 95000000);
+       let reserve = Channel::<EnforcingSigner>::get_holder_selected_channel_reserve_satoshis(100000);
+       assert_eq!(channels0[0].inbound_capacity_msat, 95000000 - reserve*1000);
+       assert_eq!(channels1[0].outbound_capacity_msat, 95000000 - reserve*1000);
 
-       assert_eq!(channels0[0].outbound_capacity_msat, 100000 * 1000 - 95000000);
-       assert_eq!(channels1[0].inbound_capacity_msat, 100000 * 1000 - 95000000);
+       assert_eq!(channels0[0].outbound_capacity_msat, 100000 * 1000 - 95000000 - reserve*1000);
+       assert_eq!(channels1[0].inbound_capacity_msat, 100000 * 1000 - 95000000 - reserve*1000);
 }
 
 fn commit_tx_fee_msat(feerate: u32, num_htlcs: u64) -> u64 {
@@ -1903,7 +1920,11 @@ fn commit_tx_fee_msat(feerate: u32, num_htlcs: u64) -> u64 {
 fn test_channel_reserve_holding_cell_htlcs() {
        let chanmon_cfgs = create_chanmon_cfgs(3);
        let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       // When this test was written, the default base fee floated based on the HTLC count.
+       // It is now fixed, so we simply set the fee to the expected value here.
+       let mut config = test_default_channel_config();
+       config.channel_options.forwarding_fee_base_msat = 239;
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config.clone())]);
        let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
        let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 190000, 1001, InitFeatures::known(), InitFeatures::known());
        let chan_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 190000, 1001, InitFeatures::known(), InitFeatures::known());
@@ -1924,7 +1945,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
                }}
        }
 
-       let feemsat = 239; // somehow we know?
+       let feemsat = 239; // set above
        let total_fee_msat = (nodes.len() - 2) as u64 * feemsat;
        let feerate = get_feerate!(nodes[0], chan_1.2);
 
@@ -2319,12 +2340,15 @@ fn channel_monitor_network_test() {
        // One pending HTLC is discarded by the force-close:
        let payment_preimage_1 = route_payment(&nodes[1], &vec!(&nodes[2], &nodes[3])[..], 3000000).0;
 
-       // Simple case of one pending HTLC to HTLC-Timeout
+       // Simple case of one pending HTLC to HTLC-Timeout (note that the HTLC-Timeout is not
+       // broadcasted until we reach the timelock time).
        nodes[1].node.peer_disconnected(&nodes[2].node.get_our_node_id(), true);
        check_closed_broadcast!(nodes[1], false);
        check_added_monitors!(nodes[1], 1);
        {
-               let mut node_txn = test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::TIMEOUT);
+               let mut node_txn = test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::NONE);
+               connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + MIN_CLTV_EXPIRY_DELTA as u32 + 1);
+               test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::TIMEOUT);
                mine_transaction(&nodes[2], &node_txn[0]);
                check_added_monitors!(nodes[2], 1);
                test_txn_broadcast(&nodes[2], &chan_2, None, HTLCType::NONE);
@@ -2360,7 +2384,9 @@ fn channel_monitor_network_test() {
        check_closed_broadcast!(nodes[2], false);
        let node2_commitment_txid;
        {
-               let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::TIMEOUT);
+               let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::NONE);
+               connect_blocks(&nodes[2], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + MIN_CLTV_EXPIRY_DELTA as u32 + 1);
+               test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::TIMEOUT);
                node2_commitment_txid = node_txn[0].txid();
 
                // Claim the payment on nodes[3], giving it knowledge of the preimage
@@ -2622,7 +2648,7 @@ fn claim_htlc_outputs_shared_tx() {
                expect_payment_failed!(nodes[1], payment_hash_2, true);
 
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 3); // ChannelMonitor: penalty tx, ChannelManager: local commitment + HTLC-timeout
+               assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty tx, ChannelManager: local commitment
 
                assert_eq!(node_txn[0].input.len(), 3); // Claim the revoked output + both revoked HTLC outputs
                check_spends!(node_txn[0], revoked_local_txn[0]);
@@ -2639,13 +2665,6 @@ fn claim_htlc_outputs_shared_tx() {
                // Next nodes[1] broadcasts its current local tx state:
                assert_eq!(node_txn[1].input.len(), 1);
                assert_eq!(node_txn[1].input[0].previous_output.txid, chan_1.3.txid()); //Spending funding tx unique txouput, tx broadcasted by ChannelManager
-
-               assert_eq!(node_txn[2].input.len(), 1);
-               let witness_script = node_txn[2].clone().input[0].witness.pop().unwrap();
-               assert_eq!(witness_script.len(), OFFERED_HTLC_SCRIPT_WEIGHT); //Spending an offered htlc output
-               assert_eq!(node_txn[2].input[0].previous_output.txid, node_txn[1].txid());
-               assert_ne!(node_txn[2].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
-               assert_ne!(node_txn[2].input[0].previous_output.txid, node_txn[0].input[1].previous_output.txid);
        }
        get_announce_close_broadcast_events(&nodes, 0, 1);
        assert_eq!(nodes[0].node.list_channels().len(), 0);
@@ -2750,7 +2769,7 @@ fn test_htlc_on_chain_success() {
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
        // Ensure all nodes are at the same height
-       let node_max_height = nodes.iter().map(|node| node.blocks.borrow().len()).max().unwrap() as u32;
+       let node_max_height = nodes.iter().map(|node| node.blocks.lock().unwrap().len()).max().unwrap() as u32;
        connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
        connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
        connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
@@ -2834,34 +2853,26 @@ fn test_htlc_on_chain_success() {
        macro_rules! check_tx_local_broadcast {
                ($node: expr, $htlc_offered: expr, $commitment_tx: expr, $chan_tx: expr) => { {
                        let mut node_txn = $node.tx_broadcaster.txn_broadcasted.lock().unwrap();
-                       assert_eq!(node_txn.len(), 5);
+                       assert_eq!(node_txn.len(), 3);
                        // Node[1]: ChannelManager: 3 (commitment tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 (timeout tx)
                        // Node[0]: ChannelManager: 3 (commtiemtn tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 HTLC-timeout
-                       check_spends!(node_txn[3], $commitment_tx);
-                       check_spends!(node_txn[4], $commitment_tx);
-                       assert_ne!(node_txn[3].lock_time, 0);
-                       assert_ne!(node_txn[4].lock_time, 0);
+                       check_spends!(node_txn[1], $commitment_tx);
+                       check_spends!(node_txn[2], $commitment_tx);
+                       assert_ne!(node_txn[1].lock_time, 0);
+                       assert_ne!(node_txn[2].lock_time, 0);
                        if $htlc_offered {
-                               assert_eq!(node_txn[3].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-                               assert_eq!(node_txn[4].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-                               assert!(node_txn[3].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
-                               assert!(node_txn[4].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
+                               assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+                               assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+                               assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
+                               assert!(node_txn[2].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
                        } else {
-                               assert_eq!(node_txn[3].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-                               assert_eq!(node_txn[4].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-                               assert!(node_txn[3].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
-                               assert!(node_txn[4].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
+                               assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+                               assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+                               assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
+                               assert!(node_txn[2].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
                        }
                        check_spends!(node_txn[0], $chan_tx);
-                       check_spends!(node_txn[1], node_txn[0]);
-                       check_spends!(node_txn[2], node_txn[0]);
                        assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), 71);
-                       assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-                       assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-                       assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
-                       assert!(node_txn[2].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
-                       assert_ne!(node_txn[1].lock_time, 0);
-                       assert_ne!(node_txn[2].lock_time, 0);
                        node_txn.clear();
                } }
        }
@@ -3020,10 +3031,16 @@ fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) {
 
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
        {
-               // B will rebroadcast its own holder commitment transaction here...just because
+               // B may rebroadcast its own holder commitment transaction here, as a safeguard against
+               // some incredibly unlikely partial-eclipse-attack scenarios. That said, because the
+               // original commitment_tx[0] (also spending chan_2.3) has reached ANTI_REORG_DELAY B really
+               // shouldn't broadcast anything here, and in some connect style scenarios we do not.
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-               assert_eq!(node_txn.len(), 1);
-               check_spends!(node_txn[0], chan_2.3);
+               if node_txn.len() == 1 {
+                       check_spends!(node_txn[0], chan_2.3);
+               } else {
+                       assert_eq!(node_txn.len(), 0);
+               }
        }
 
        expect_pending_htlcs_forwardable!(nodes[1]);
@@ -3050,14 +3067,12 @@ fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) {
 
        check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
-       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 1 timeout tx
-       assert_eq!(node_txn.len(), 3);
+       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 1 commitment tx, ChannelMonitor : 1 timeout tx
+       assert_eq!(node_txn.len(), 2);
        check_spends!(node_txn[0], chan_1.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);
-       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[1], commitment_tx[0]);
+       assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
 }
 
 #[test]
@@ -3108,13 +3123,7 @@ fn test_simple_commitment_revoked_fail_backward() {
 
                        nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &update_fail_htlcs[0]);
                        commitment_signed_dance!(nodes[0], nodes[1], commitment_signed, false, true);
-
-                       let events = nodes[0].node.get_and_clear_pending_msg_events();
-                       assert_eq!(events.len(), 1);
-                       match events[0] {
-                               MessageSendEvent::PaymentFailureNetworkUpdate { .. } => {},
-                               _ => panic!("Unexpected event"),
-                       }
+                       expect_payment_failure_chan_update!(nodes[0], chan_2.0.contents.short_channel_id, true);
                        expect_payment_failed!(nodes[0], payment_hash, false);
                },
                _ => panic!("Unexpected event"),
@@ -3436,11 +3445,13 @@ fn test_htlc_ignore_latest_remote_commitment() {
 
        route_payment(&nodes[0], &[&nodes[1]], 10000000);
        nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap();
+       connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-       assert_eq!(node_txn.len(), 2);
+       assert_eq!(node_txn.len(), 3);
+       assert_eq!(node_txn[0], node_txn[1]);
 
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[1].clone()]});
@@ -3450,7 +3461,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
        // Duplicate the connect_block call since this may happen due to other listeners
        // registering new transactions
        header.prev_blockhash = header.block_hash();
-       connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[1].clone()]});
+       connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[2].clone()]});
 }
 
 #[test]
@@ -3617,15 +3628,20 @@ fn test_simple_peer_disconnect() {
        fail_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), payment_hash_6);
 }
 
-fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) {
+fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken_lnd: bool) {
        // Test that we can reconnect when in-flight HTLC updates get dropped
        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);
+
+       let mut as_funding_locked = None;
        if messages_delivered == 0 {
-               create_chan_between_nodes_with_value_a(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
+               let (funding_locked, _, _) = create_chan_between_nodes_with_value_a(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
+               as_funding_locked = Some(funding_locked);
                // nodes[1] doesn't receive the funding_locked message (it'll be re-sent on reconnect)
+               // Note that we store it so that if we're running with `simulate_broken_lnd` we can deliver
+               // it before the channel_reestablish message.
        } else {
                create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        }
@@ -3680,6 +3696,17 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) {
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
        if messages_delivered < 3 {
+               if simulate_broken_lnd {
+                       // lnd has a long-standing bug where they send a funding_locked prior to a
+                       // channel_reestablish if you reconnect prior to funding_locked time.
+                       //
+                       // Here we simulate that behavior, delivering a funding_locked immediately on
+                       // reconnect. Note that we don't bother skipping the now-duplicate funding_locked sent
+                       // in `reconnect_nodes` but we currently don't fail based on that.
+                       //
+                       // See-also <https://github.com/lightningnetwork/lnd/issues/4006>
+                       nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked.as_ref().unwrap().0);
+               }
                // Even if the funding_locked messages get exchanged, as long as nothing further was
                // received on either side, both sides will need to resend them.
                reconnect_nodes(&nodes[0], &nodes[1], (true, true), (0, 1), (0, 0), (0, 0), (0, 0), (false, false));
@@ -3823,17 +3850,18 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) {
 
 #[test]
 fn test_drop_messages_peer_disconnect_a() {
-       do_test_drop_messages_peer_disconnect(0);
-       do_test_drop_messages_peer_disconnect(1);
-       do_test_drop_messages_peer_disconnect(2);
-       do_test_drop_messages_peer_disconnect(3);
+       do_test_drop_messages_peer_disconnect(0, true);
+       do_test_drop_messages_peer_disconnect(0, false);
+       do_test_drop_messages_peer_disconnect(1, false);
+       do_test_drop_messages_peer_disconnect(2, false);
 }
 
 #[test]
 fn test_drop_messages_peer_disconnect_b() {
-       do_test_drop_messages_peer_disconnect(4);
-       do_test_drop_messages_peer_disconnect(5);
-       do_test_drop_messages_peer_disconnect(6);
+       do_test_drop_messages_peer_disconnect(3, false);
+       do_test_drop_messages_peer_disconnect(4, false);
+       do_test_drop_messages_peer_disconnect(5, false);
+       do_test_drop_messages_peer_disconnect(6, false);
 }
 
 #[test]
@@ -4198,7 +4226,7 @@ fn do_test_holding_cell_htlc_add_timeouts(forwarded_htlc: bool) {
        let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
        let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
        create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
-       create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
+       let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
        // Make sure all nodes are at the same starting height
        connect_blocks(&nodes[0], 2*CHAN_CONFIRM_DEPTH + 1 - nodes[0].best_block_info().1);
@@ -4254,14 +4282,7 @@ fn do_test_holding_cell_htlc_add_timeouts(forwarded_htlc: bool) {
                        _ => unreachable!(),
                }
                expect_payment_failed!(nodes[0], second_payment_hash, false);
-               if let &MessageSendEvent::PaymentFailureNetworkUpdate { ref update } = &nodes[0].node.get_and_clear_pending_msg_events()[0] {
-                       match update {
-                               &HTLCFailChannelUpdate::ChannelUpdateMessage { .. } => {},
-                               _ => panic!("Unexpected event"),
-                       }
-               } else {
-                       panic!("Unexpected event");
-               }
+               expect_payment_failure_chan_update!(nodes[0], chan_2.0.contents.short_channel_id, false);
        } else {
                expect_payment_failed!(nodes[1], second_payment_hash, true);
        }
@@ -4371,7 +4392,7 @@ fn test_no_txn_manager_serialize_deserialize() {
        nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
 
        logger = test_utils::TestLogger::new();
-       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
        persister = test_utils::TestPersister::new();
        let keys_manager = &chanmon_cfgs[0].keys_manager;
        new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager);
@@ -4460,19 +4481,23 @@ fn test_dup_htlc_onchain_fails_on_reload() {
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
 
+       // Connect blocks until the CLTV timeout is up so that we get an HTLC-Timeout transaction
+       connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-       assert_eq!(node_txn.len(), 2);
+       assert_eq!(node_txn.len(), 3);
+       assert_eq!(node_txn[0], node_txn[1]);
 
        assert!(nodes[1].node.claim_funds(payment_preimage));
        check_added_monitors!(nodes[1], 1);
 
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[1].clone()]});
+       connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[1].clone(), node_txn[2].clone()]});
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let claim_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 
-       connect_block(&nodes[0], &Block { header, txdata: node_txn});
+       header.prev_blockhash = nodes[0].best_block_hash();
+       connect_block(&nodes[0], &Block { header, txdata: vec![node_txn[1].clone(), node_txn[2].clone()]});
 
        // Serialize out the ChannelMonitor before connecting the on-chain claim transactions. This is
        // fairly normal behavior as ChannelMonitor(s) are often not re-serialized when on-chain events
@@ -4480,7 +4505,7 @@ fn test_dup_htlc_onchain_fails_on_reload() {
        let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
        nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
 
-       header.prev_blockhash = header.block_hash();
+       header.prev_blockhash = nodes[0].best_block_hash();
        let claim_block = Block { header, txdata: claim_txn};
        connect_block(&nodes[0], &claim_block);
        expect_payment_sent!(nodes[0], payment_preimage);
@@ -4523,7 +4548,8 @@ fn test_dup_htlc_onchain_fails_on_reload() {
        // Note that if we re-connect the block which exposed nodes[0] to the payment preimage (but
        // which the current ChannelMonitor has not seen), the ChannelManager's de-duplication of
        // payment events should kick in, leaving us with no pending events here.
-       nodes[0].chain_monitor.chain_monitor.block_connected(&claim_block, nodes[0].blocks.borrow().len() as u32 - 1);
+       let height = nodes[0].blocks.lock().unwrap().len() as u32 - 1;
+       nodes[0].chain_monitor.chain_monitor.block_connected(&claim_block, height);
        assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
 }
 
@@ -4581,7 +4607,7 @@ fn test_manager_serialize_deserialize_events() {
        let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
        nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
 
-       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
        logger = test_utils::TestLogger::new();
        persister = test_utils::TestPersister::new();
        let keys_manager = &chanmon_cfgs[0].keys_manager;
@@ -4669,7 +4695,7 @@ fn test_simple_manager_serialize_deserialize() {
        nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
 
        logger = test_utils::TestLogger::new();
-       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
        persister = test_utils::TestPersister::new();
        let keys_manager = &chanmon_cfgs[0].keys_manager;
        new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager);
@@ -4749,7 +4775,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        }
 
        logger = test_utils::TestLogger::new();
-       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
        persister = test_utils::TestPersister::new();
        let keys_manager = &chanmon_cfgs[0].keys_manager;
        new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager);
@@ -4800,11 +4826,11 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        nodes_0_deserialized = nodes_0_deserialized_tmp;
        assert!(nodes_0_read.is_empty());
 
-       { // Channel close should result in a commitment tx and an HTLC tx
+       { // Channel close should result in a commitment tx
                let txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(txn.len(), 2);
+               assert_eq!(txn.len(), 1);
+               check_spends!(txn[0], funding_tx);
                assert_eq!(txn[0].input[0].previous_output.txid, funding_tx.txid());
-               assert_eq!(txn[1].input[0].previous_output.txid, txn[0].txid());
        }
 
        for monitor in node_0_monitors.drain(..) {
@@ -4836,7 +4862,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 }
 
 macro_rules! check_spendable_outputs {
-       ($node: expr, $der_idx: expr, $keysinterface: expr, $chan_value: expr) => {
+       ($node: expr, $keysinterface: expr) => {
                {
                        let mut events = $node.chain_monitor.chain_monitor.get_and_clear_pending_events();
                        let mut txn = Vec::new();
@@ -4881,11 +4907,13 @@ fn test_claim_sizeable_push_msat() {
        assert_eq!(node_txn[0].output.len(), 2); // We can't force trimming of to_remote output as channel_reserve_satoshis block us to do so at channel opening
 
        mine_transaction(&nodes[1], &node_txn[0]);
-       connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+       connect_blocks(&nodes[1], BREAKDOWN_TIMEOUT as u32 - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
+       assert_eq!(spend_txn[0].input.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
+       assert_eq!(spend_txn[0].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
 }
 
 #[test]
@@ -4912,7 +4940,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
        check_added_monitors!(nodes[1], 1);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -4942,7 +4970,7 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
        mine_transaction(&nodes[1], &node_txn[0]);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 3);
        check_spends!(spend_txn[0], revoked_local_txn[0]); // to_remote output on revoked remote commitment_tx
        check_spends!(spend_txn[1], node_txn[0]);
@@ -4991,7 +5019,7 @@ fn test_static_spendable_outputs_preimage_tx() {
        mine_transaction(&nodes[1], &node_txn[0]);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -5027,21 +5055,20 @@ fn test_static_spendable_outputs_timeout_tx() {
 
        // Check B's monitor was able to send back output descriptor event for timeout tx on A's commitment tx
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-       assert_eq!(node_txn.len(), 3); // ChannelManager : 2 (local commitent tx + HTLC-timeout), ChannelMonitor: timeout tx
+       assert_eq!(node_txn.len(), 2); // ChannelManager : 1 local commitent tx, ChannelMonitor: timeout tx
        check_spends!(node_txn[0], chan_1.3.clone());
-       check_spends!(node_txn[1], node_txn[0]);
-       check_spends!(node_txn[2],  commitment_tx[0].clone());
-       assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+       check_spends!(node_txn[1],  commitment_tx[0].clone());
+       assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
 
-       mine_transaction(&nodes[1], &node_txn[2]);
+       mine_transaction(&nodes[1], &node_txn[1]);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
        expect_payment_failed!(nodes[1], our_payment_hash, true);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote, timeout_tx.output
        check_spends!(spend_txn[0], commitment_tx[0]);
-       check_spends!(spend_txn[1], node_txn[2]);
-       check_spends!(spend_txn[2], node_txn[2], commitment_tx[0]); // All outputs
+       check_spends!(spend_txn[1], node_txn[1]);
+       check_spends!(spend_txn[2], node_txn[1], commitment_tx[0]); // All outputs
 }
 
 #[test]
@@ -5073,7 +5100,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
        mine_transaction(&nodes[1], &node_txn[0]);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -5140,7 +5167,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
        // Check B's ChannelMonitor was able to generate the right spendable output descriptor
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        assert_eq!(spend_txn[0].input.len(), 1);
        check_spends!(spend_txn[0], node_txn[1]);
@@ -5215,7 +5242,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
        // didn't try to generate any new transactions.
 
        // Check A's ChannelMonitor was able to generate the right spendable output descriptor
-       let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
        assert_eq!(spend_txn.len(), 3);
        assert_eq!(spend_txn[0].input.len(), 1);
        check_spends!(spend_txn[0], revoked_local_txn[0]); // spending to_remote output from revoked local tx
@@ -5244,7 +5271,7 @@ fn test_onchain_to_onchain_claim() {
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
        // Ensure all nodes are at the same height
-       let node_max_height = nodes.iter().map(|node| node.blocks.borrow().len()).max().unwrap() as u32;
+       let node_max_height = nodes.iter().map(|node| node.blocks.lock().unwrap().len()).max().unwrap() as u32;
        connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
        connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
        connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
@@ -5285,17 +5312,13 @@ fn test_onchain_to_onchain_claim() {
        connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
        {
                let mut b_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               // ChannelMonitor: claim tx, ChannelManager: local commitment tx + HTLC-timeout tx
-               assert_eq!(b_txn.len(), 3);
+               // ChannelMonitor: claim tx, ChannelManager: local commitment tx
+               assert_eq!(b_txn.len(), 2);
                check_spends!(b_txn[0], chan_2.3); // B local commitment tx, issued by ChannelManager
-               check_spends!(b_txn[1], b_txn[0]); // HTLC-Timeout on B local commitment tx, issued by ChannelManager
-               assert_eq!(b_txn[1].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-               assert!(b_txn[1].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
+               check_spends!(b_txn[1], c_txn[1]); // timeout tx on C remote commitment tx, issued by ChannelMonitor
+               assert_eq!(b_txn[1].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               assert!(b_txn[1].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
                assert_ne!(b_txn[1].lock_time, 0); // Timeout tx
-               check_spends!(b_txn[2], c_txn[1]); // timeout tx on C remote commitment tx, issued by ChannelMonitor
-               assert_eq!(b_txn[2].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-               assert!(b_txn[2].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
-               assert_ne!(b_txn[2].lock_time, 0); // Timeout tx
                b_txn.clear();
        }
        check_added_monitors!(nodes[1], 1);
@@ -5350,14 +5373,19 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
        // we forward one of the payments onwards to D.
        let chanmon_cfgs = create_chanmon_cfgs(4);
        let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+       // When this test was written, the default base fee floated based on the HTLC count.
+       // It is now fixed, so we simply set the fee to the expected value here.
+       let mut config = test_default_channel_config();
+       config.channel_options.forwarding_fee_base_msat = 196;
+       let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs,
+               &[Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone())]);
        let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);
 
        create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
        create_announced_chan_between_nodes(&nodes, 2, 3, InitFeatures::known(), InitFeatures::known());
 
-       let node_max_height = nodes.iter().map(|node| node.blocks.borrow().len()).max().unwrap() as u32;
+       let node_max_height = nodes.iter().map(|node| node.blocks.lock().unwrap().len()).max().unwrap() as u32;
        connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
        connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
        connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
@@ -5385,24 +5413,22 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
        let htlc_timeout_tx;
        { // Extract one of the two HTLC-Timeout transaction
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               // ChannelMonitor: timeout tx * 3, ChannelManager: local commitment tx + HTLC-timeout * 2
-               assert_eq!(node_txn.len(), 6);
+               // ChannelMonitor: timeout tx * 3, ChannelManager: local commitment tx
+               assert_eq!(node_txn.len(), 4);
                check_spends!(node_txn[0], chan_2.3);
-               check_spends!(node_txn[1], node_txn[0]);
-               check_spends!(node_txn[2], node_txn[0]);
 
+               check_spends!(node_txn[1], commitment_txn[0]);
+               assert_eq!(node_txn[1].input.len(), 1);
+               check_spends!(node_txn[2], commitment_txn[0]);
+               assert_eq!(node_txn[2].input.len(), 1);
+               assert_eq!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output);
                check_spends!(node_txn[3], commitment_txn[0]);
-               assert_eq!(node_txn[3].input.len(), 1);
-               check_spends!(node_txn[4], commitment_txn[0]);
-               assert_eq!(node_txn[4].input.len(), 1);
-               assert_eq!(node_txn[3].input[0].previous_output, node_txn[4].input[0].previous_output);
-               check_spends!(node_txn[5], commitment_txn[0]);
-               assert_ne!(node_txn[3].input[0].previous_output, node_txn[5].input[0].previous_output);
+               assert_ne!(node_txn[1].input[0].previous_output, node_txn[3].input[0].previous_output);
 
+               assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
                assert_eq!(node_txn[3].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-               assert_eq!(node_txn[4].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-               assert_eq!(node_txn[5].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-               htlc_timeout_tx = node_txn[3].clone();
+               htlc_timeout_tx = node_txn[1].clone();
        }
 
        nodes[2].node.claim_funds(our_payment_preimage);
@@ -5446,13 +5472,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
        {
                commitment_signed_dance!(nodes[0], nodes[1], &htlc_updates.commitment_signed, false, true);
-               let events = nodes[0].node.get_and_clear_pending_msg_events();
-               assert_eq!(events.len(), 1);
-               match events[0] {
-                       MessageSendEvent::PaymentFailureNetworkUpdate { update: msgs::HTLCFailChannelUpdate::ChannelClosed { .. }  } => {
-                       },
-                       _ => { panic!("Unexpected event"); }
-               }
+               expect_payment_failure_chan_update!(nodes[0], chan_2.0.contents.short_channel_id, true);
        }
        expect_payment_failed!(nodes[0], duplicate_payment_hash, false);
 
@@ -5520,12 +5540,14 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
        };
 
        mine_transaction(&nodes[1], &node_tx);
-       connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+       connect_blocks(&nodes[1], BREAKDOWN_TIMEOUT as u32 - 1);
 
        // Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
-       let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
+       assert_eq!(spend_txn[0].input.len(), 1);
        check_spends!(spend_txn[0], node_tx);
+       assert_eq!(spend_txn[0].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
 }
 
 fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, announce_latest: bool) {
@@ -5543,7 +5565,12 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
        // And test where C fails back to A/B when D announces its latest commitment transaction
        let chanmon_cfgs = create_chanmon_cfgs(6);
        let node_cfgs = create_node_cfgs(6, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(6, &node_cfgs, &[None, None, None, None, None, None]);
+       // When this test was written, the default base fee floated based on the HTLC count.
+       // It is now fixed, so we simply set the fee to the expected value here.
+       let mut config = test_default_channel_config();
+       config.channel_options.forwarding_fee_base_msat = 196;
+       let node_chanmgrs = create_node_chanmgrs(6, &node_cfgs,
+               &[Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone())]);
        let nodes = create_network(6, &node_cfgs, &node_chanmgrs);
        let logger = test_utils::TestLogger::new();
 
@@ -5806,9 +5833,8 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 
        let htlc_timeout = {
                let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 3);
+               assert_eq!(node_txn.len(), 2);
                check_spends!(node_txn[0], chan_1.3);
-               assert_eq!(node_txn[1], node_txn[2]);
                assert_eq!(node_txn[1].input.len(), 1);
                assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
                check_spends!(node_txn[1], local_txn[0]);
@@ -5816,15 +5842,20 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
        };
 
        mine_transaction(&nodes[0], &htlc_timeout);
-       connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+       connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
        expect_payment_failed!(nodes[0], our_payment_hash, true);
 
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
-       let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
        assert_eq!(spend_txn.len(), 3);
        check_spends!(spend_txn[0], local_txn[0]);
+       assert_eq!(spend_txn[1].input.len(), 1);
        check_spends!(spend_txn[1], htlc_timeout);
+       assert_eq!(spend_txn[1].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
+       assert_eq!(spend_txn[2].input.len(), 2);
        check_spends!(spend_txn[2], local_txn[0], htlc_timeout);
+       assert!(spend_txn[2].input[0].sequence == BREAKDOWN_TIMEOUT as u32 ||
+               spend_txn[2].input[1].sequence == BREAKDOWN_TIMEOUT as u32);
 }
 
 #[test]
@@ -5854,6 +5885,12 @@ fn test_key_derivation_params() {
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        assert_ne!(chan_0.3.output[0].script_pubkey, chan_1.3.output[0].script_pubkey);
 
+       // Ensure all nodes are at the same height
+       let node_max_height = nodes.iter().map(|node| node.blocks.lock().unwrap().len()).max().unwrap() as u32;
+       connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
+       connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
+       connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
+
        let (_, our_payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000);
        let local_txn_0 = get_local_commitment_txn!(nodes[0], chan_0.2);
        let local_txn_1 = get_local_commitment_txn!(nodes[0], chan_1.2);
@@ -5872,6 +5909,7 @@ fn test_key_derivation_params() {
 
        // Timeout HTLC on A's chain and so it can generate a HTLC-Timeout tx
        mine_transaction(&nodes[0], &local_txn_1[0]);
+       connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
        check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
@@ -5884,16 +5922,21 @@ fn test_key_derivation_params() {
        };
 
        mine_transaction(&nodes[0], &htlc_timeout);
-       connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+       connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
        expect_payment_failed!(nodes[0], our_payment_hash, true);
 
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
        let new_keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
-       let spend_txn = check_spendable_outputs!(nodes[0], 1, new_keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[0], new_keys_manager);
        assert_eq!(spend_txn.len(), 3);
        check_spends!(spend_txn[0], local_txn_1[0]);
+       assert_eq!(spend_txn[1].input.len(), 1);
        check_spends!(spend_txn[1], htlc_timeout);
+       assert_eq!(spend_txn[1].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
+       assert_eq!(spend_txn[2].input.len(), 2);
        check_spends!(spend_txn[2], local_txn_1[0], htlc_timeout);
+       assert!(spend_txn[2].input[0].sequence == BREAKDOWN_TIMEOUT as u32 ||
+               spend_txn[2].input[1].sequence == BREAKDOWN_TIMEOUT as u32);
 }
 
 #[test]
@@ -5911,14 +5954,14 @@ fn test_static_output_closing_tx() {
        mine_transaction(&nodes[0], &closing_tx);
        connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[0], 2, node_cfgs[0].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], closing_tx);
 
        mine_transaction(&nodes[1], &closing_tx);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
-       let spend_txn = check_spendable_outputs!(nodes[1], 2, node_cfgs[1].keys_manager, 100000);
+       let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], closing_tx);
 }
@@ -6228,8 +6271,9 @@ fn test_fail_holding_cell_htlc_upon_free() {
        // us to surface its failure to the user.
        chan_stat = get_channel_value_stat!(nodes[0], chan.2);
        assert_eq!(chan_stat.holding_cell_outbound_amount_msat, 0);
-       nodes[0].logger.assert_log("lightning::ln::channel".to_string(), "Freeing holding cell with 1 HTLC updates".to_string(), 1);
-       let failure_log = format!("Failed to send HTLC with payment_hash {} due to Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", log_bytes!(our_payment_hash.0), chan_stat.channel_reserve_msat);
+       nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Freeing holding cell with 1 HTLC updates in channel {}", hex::encode(chan.2)), 1);
+       let failure_log = format!("Failed to send HTLC with payment_hash {} due to Cannot send value that would put our balance under counterparty-announced channel reserve value ({}) in channel {}",
+               hex::encode(our_payment_hash.0), chan_stat.channel_reserve_msat, hex::encode(chan.2));
        nodes[0].logger.assert_log("lightning::ln::channel".to_string(), failure_log.to_string(), 1);
 
        // Check that the payment failed to be sent out.
@@ -6308,8 +6352,9 @@ fn test_free_and_fail_holding_cell_htlcs() {
        // to surface its failure to the user. The first payment should succeed.
        chan_stat = get_channel_value_stat!(nodes[0], chan.2);
        assert_eq!(chan_stat.holding_cell_outbound_amount_msat, 0);
-       nodes[0].logger.assert_log("lightning::ln::channel".to_string(), "Freeing holding cell with 2 HTLC updates".to_string(), 1);
-       let failure_log = format!("Failed to send HTLC with payment_hash {} due to Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", log_bytes!(payment_hash_2.0), chan_stat.channel_reserve_msat);
+       nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Freeing holding cell with 2 HTLC updates in channel {}", hex::encode(chan.2)), 1);
+       let failure_log = format!("Failed to send HTLC with payment_hash {} due to Cannot send value that would put our balance under counterparty-announced channel reserve value ({}) in channel {}",
+               hex::encode(payment_hash_2.0), chan_stat.channel_reserve_msat, hex::encode(chan.2));
        nodes[0].logger.assert_log("lightning::ln::channel".to_string(), failure_log.to_string(), 1);
 
        // Check that the second payment failed to be sent out.
@@ -6375,7 +6420,11 @@ fn test_free_and_fail_holding_cell_htlcs() {
 fn test_fail_holding_cell_htlc_upon_free_multihop() {
        let chanmon_cfgs = create_chanmon_cfgs(3);
        let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       // When this test was written, the default base fee floated based on the HTLC count.
+       // It is now fixed, so we simply set the fee to the expected value here.
+       let mut config = test_default_channel_config();
+       config.channel_options.forwarding_fee_base_msat = 196;
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config.clone())]);
        let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
        let chan_0_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
        let chan_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
@@ -6491,20 +6540,8 @@ fn test_fail_holding_cell_htlc_upon_free_multihop() {
                _ => panic!("Unexpected event"),
        };
        nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &raa);
-       let fail_msg_event = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(fail_msg_event.len(), 1);
-       match &fail_msg_event[0] {
-               &MessageSendEvent::PaymentFailureNetworkUpdate { .. } => {},
-               _ => panic!("Unexpected event"),
-       }
-       let failure_event = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(failure_event.len(), 1);
-       match &failure_event[0] {
-               &Event::PaymentFailed { rejected_by_dest, .. } => {
-                       assert!(!rejected_by_dest);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_failure_chan_update!(nodes[0], chan_1_2.0.contents.short_channel_id, false);
+       expect_payment_failed!(nodes[0], our_payment_hash, false);
        check_added_monitors!(nodes[0], 1);
 }
 
@@ -7313,12 +7350,14 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
        if local {
                // We fail dust-HTLC 1 by broadcast of local commitment tx
                mine_transaction(&nodes[0], &as_commitment_tx[0]);
+               connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+               expect_payment_failed!(nodes[0], dust_hash, true);
+
+               connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS - ANTI_REORG_DELAY);
                check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
                timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[1].clone());
-               connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
-               expect_payment_failed!(nodes[0], dust_hash, true);
                assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
                // We fail non-dust-HTLC 2 by broadcast of local HTLC-timeout tx on local commitment tx
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
@@ -7332,7 +7371,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
                check_added_monitors!(nodes[0], 1);
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
                connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
-               timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[2].clone());
+               timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[1].clone());
                if !revoked {
                        expect_payment_failed!(nodes[0], dust_hash, true);
                        assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
@@ -7603,7 +7642,7 @@ fn test_user_configurable_csv_delay() {
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
        // We test config.our_to_self > BREAKDOWN_TIMEOUT is enforced in Channel::new_outbound()
-       if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: 253 }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), 1000000, 1000000, 0, &low_our_to_self_config) {
+       if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), 1000000, 1000000, 0, &low_our_to_self_config) {
                match error {
                        APIError::APIMisuseError { err } => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); },
                        _ => panic!("Unexpected event"),
@@ -7614,7 +7653,7 @@ fn test_user_configurable_csv_delay() {
        nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
        let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
        open_channel.to_self_delay = 200;
-       if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: 253 }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel, 0, &low_our_to_self_config) {
+       if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel, 0, &low_our_to_self_config) {
                match error {
                        ChannelError::Close(err) => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str()));  },
                        _ => panic!("Unexpected event"),
@@ -7640,7 +7679,7 @@ fn test_user_configurable_csv_delay() {
        nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
        let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
        open_channel.to_self_delay = 200;
-       if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: 253 }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel, 0, &high_their_to_self_config) {
+       if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel, 0, &high_their_to_self_config) {
                match error {
                        ChannelError::Close(err) => { assert!(regex::Regex::new(r"They wanted our payments to be delayed by a needlessly long period\. Upper limit: \d+\. Actual: \d+").unwrap().is_match(err.as_str())); },
                        _ => panic!("Unexpected event"),
@@ -7689,8 +7728,8 @@ fn test_data_loss_protect() {
        logger = test_utils::TestLogger::with_id(format!("node {}", 0));
        let mut chain_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut ::std::io::Cursor::new(previous_chain_monitor_state.0), keys_manager).unwrap().1;
        chain_source = test_utils::TestChainSource::new(Network::Testnet);
-       tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())};
-       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+       tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
+       fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
        persister = test_utils::TestPersister::new();
        monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &tx_broadcaster, &logger, &fee_estimator, &persister, keys_manager);
        node_state_0 = {
@@ -7758,7 +7797,7 @@ fn test_data_loss_protect() {
        assert_eq!(node_txn[0].output.len(), 2);
        mine_transaction(&nodes[0], &node_txn[0]);
        connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
-       let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 1000000);
+       let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -7901,6 +7940,168 @@ fn test_announce_disable_channels() {
        }
 }
 
+#[test]
+fn test_priv_forwarding_rejection() {
+       // If we have a private channel with outbound liquidity, and
+       // UserConfig::accept_forwards_to_priv_channels is set to false, we should reject any attempts
+       // to forward through that channel.
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let mut no_announce_cfg = test_default_channel_config();
+       no_announce_cfg.channel_options.announced_channel = false;
+       no_announce_cfg.accept_forwards_to_priv_channels = false;
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg), None]);
+       let persister: test_utils::TestPersister;
+       let new_chain_monitor: test_utils::TestChainMonitor;
+       let nodes_1_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known());
+
+       // Note that the create_*_chan functions in utils requires announcement_signatures, which we do
+       // not send for private channels.
+       nodes[1].node.create_channel(nodes[2].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
+       let open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[2].node.get_our_node_id());
+       nodes[2].node.handle_open_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel);
+       let accept_channel = get_event_msg!(nodes[2], MessageSendEvent::SendAcceptChannel, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_accept_channel(&nodes[2].node.get_our_node_id(), InitFeatures::known(), &accept_channel);
+
+       let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[1], 1_000_000, 42);
+       nodes[1].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
+       nodes[2].node.handle_funding_created(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingCreated, nodes[2].node.get_our_node_id()));
+       check_added_monitors!(nodes[2], 1);
+
+       nodes[1].node.handle_funding_signed(&nodes[2].node.get_our_node_id(), &get_event_msg!(nodes[2], MessageSendEvent::SendFundingSigned, nodes[1].node.get_our_node_id()));
+       check_added_monitors!(nodes[1], 1);
+
+       let conf_height = core::cmp::max(nodes[1].best_block_info().1 + 1, nodes[2].best_block_info().1 + 1);
+       confirm_transaction_at(&nodes[1], &tx, conf_height);
+       connect_blocks(&nodes[1], CHAN_CONFIRM_DEPTH - 1);
+       confirm_transaction_at(&nodes[2], &tx, conf_height);
+       connect_blocks(&nodes[2], CHAN_CONFIRM_DEPTH - 1);
+       let as_funding_locked = get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[2].node.get_our_node_id());
+       nodes[1].node.handle_funding_locked(&nodes[2].node.get_our_node_id(), &get_event_msg!(nodes[2], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()));
+       get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[2].node.get_our_node_id());
+       nodes[2].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &as_funding_locked);
+       get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+
+       assert!(nodes[0].node.list_usable_channels()[0].is_public);
+       assert_eq!(nodes[1].node.list_usable_channels().len(), 2);
+       assert!(!nodes[2].node.list_usable_channels()[0].is_public);
+
+       // We should always be able to forward through nodes[1] as long as its out through a public
+       // channel:
+       send_payment(&nodes[2], &[&nodes[1], &nodes[0]], 10_000);
+
+       // ... however, if we send to nodes[2], we will have to pass the private channel from nodes[1]
+       // to nodes[2], which should be rejected:
+       let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(nodes[2]);
+       let route = get_route(&nodes[0].node.get_our_node_id(),
+               &nodes[0].net_graph_msg_handler.network_graph.read().unwrap(),
+               &nodes[2].node.get_our_node_id(), Some(InvoiceFeatures::known()), None,
+               &[&RouteHint(vec![RouteHintHop {
+                       src_node_id: nodes[1].node.get_our_node_id(),
+                       short_channel_id: nodes[2].node.list_channels()[0].short_channel_id.unwrap(),
+                       fees: RoutingFees { base_msat: 1000, proportional_millionths: 0 },
+                       cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: None,
+               }])], 10_000, TEST_FINAL_CLTV, nodes[0].logger).unwrap();
+
+       nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let payment_event = SendEvent::from_event(nodes[0].node.get_and_clear_pending_msg_events().remove(0));
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+       commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true);
+
+       let htlc_fail_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+       assert!(htlc_fail_updates.update_add_htlcs.is_empty());
+       assert_eq!(htlc_fail_updates.update_fail_htlcs.len(), 1);
+       assert!(htlc_fail_updates.update_fail_malformed_htlcs.is_empty());
+       assert!(htlc_fail_updates.update_fee.is_none());
+
+       nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_fail_updates.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[1], htlc_fail_updates.commitment_signed, true, true);
+       expect_payment_failed!(nodes[0], our_payment_hash, false);
+       expect_payment_failure_chan_update!(nodes[0], nodes[2].node.list_channels()[0].short_channel_id.unwrap(), true);
+
+       // Now disconnect nodes[1] from its peers and restart with accept_forwards_to_priv_channels set
+       // to true. Sadly there is currently no way to change it at runtime.
+
+       nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+       nodes[2].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+
+       let nodes_1_serialized = nodes[1].node.encode();
+       let mut monitor_a_serialized = test_utils::TestVecWriter(Vec::new());
+       let mut monitor_b_serialized = test_utils::TestVecWriter(Vec::new());
+       {
+               let mons = nodes[1].chain_monitor.chain_monitor.monitors.read().unwrap();
+               let mut mon_iter = mons.iter();
+               mon_iter.next().unwrap().1.write(&mut monitor_a_serialized).unwrap();
+               mon_iter.next().unwrap().1.write(&mut monitor_b_serialized).unwrap();
+       }
+
+       persister = test_utils::TestPersister::new();
+       let keys_manager = &chanmon_cfgs[1].keys_manager;
+       new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[1].chain_source), nodes[1].tx_broadcaster.clone(), nodes[1].logger, node_cfgs[1].fee_estimator, &persister, keys_manager);
+       nodes[1].chain_monitor = &new_chain_monitor;
+
+       let mut monitor_a_read = &monitor_a_serialized.0[..];
+       let mut monitor_b_read = &monitor_b_serialized.0[..];
+       let (_, mut monitor_a) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut monitor_a_read, keys_manager).unwrap();
+       let (_, mut monitor_b) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut monitor_b_read, keys_manager).unwrap();
+       assert!(monitor_a_read.is_empty());
+       assert!(monitor_b_read.is_empty());
+
+       no_announce_cfg.accept_forwards_to_priv_channels = true;
+
+       let mut nodes_1_read = &nodes_1_serialized[..];
+       let (_, nodes_1_deserialized_tmp) = {
+               let mut channel_monitors = HashMap::new();
+               channel_monitors.insert(monitor_a.get_funding_txo().0, &mut monitor_a);
+               channel_monitors.insert(monitor_b.get_funding_txo().0, &mut monitor_b);
+               <(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_1_read, ChannelManagerReadArgs {
+                       default_config: no_announce_cfg,
+                       keys_manager,
+                       fee_estimator: node_cfgs[1].fee_estimator,
+                       chain_monitor: nodes[1].chain_monitor,
+                       tx_broadcaster: nodes[1].tx_broadcaster.clone(),
+                       logger: nodes[1].logger,
+                       channel_monitors,
+               }).unwrap()
+       };
+       assert!(nodes_1_read.is_empty());
+       nodes_1_deserialized = nodes_1_deserialized_tmp;
+
+       assert!(nodes[1].chain_monitor.watch_channel(monitor_a.get_funding_txo().0, monitor_a).is_ok());
+       assert!(nodes[1].chain_monitor.watch_channel(monitor_b.get_funding_txo().0, monitor_b).is_ok());
+       check_added_monitors!(nodes[1], 2);
+       nodes[1].node = &nodes_1_deserialized;
+
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() });
+       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+       let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reestablish);
+       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
+       get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+       get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());
+
+       nodes[1].node.peer_connected(&nodes[2].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() });
+       nodes[2].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[2].node.get_our_node_id());
+       let cs_reestablish = get_event_msg!(nodes[2], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+       nodes[2].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
+       nodes[1].node.handle_channel_reestablish(&nodes[2].node.get_our_node_id(), &cs_reestablish);
+       get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[2].node.get_our_node_id());
+       get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+
+       nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], 10_000, our_payment_hash, our_payment_secret);
+       claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], our_payment_preimage);
+}
+
 #[test]
 fn test_bump_penalty_txn_on_revoked_commitment() {
        // In case of penalty txn with too low feerates for getting into mempools, RBF-bump them to be sure
@@ -7947,7 +8148,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() {
        let feerate_1;
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 3); // justice tx (broadcasted from ChannelMonitor) + local commitment tx + local HTLC-timeout (broadcasted from ChannelManager)
+               assert_eq!(node_txn.len(), 2); // justice tx (broadcasted from ChannelMonitor) + local commitment tx
                assert_eq!(node_txn[0].input.len(), 3); // Penalty txn claims to_local, offered_htlc and received_htlc outputs
                assert_eq!(node_txn[0].output.len(), 1);
                check_spends!(node_txn[0], revoked_txn[0]);
@@ -8042,25 +8243,24 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        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();
-       assert_eq!(revoked_htlc_txn.len(), 4);
+       assert_eq!(revoked_htlc_txn.len(), 3);
        check_spends!(revoked_htlc_txn[1], chan.3);
-       check_spends!(revoked_htlc_txn[2], revoked_htlc_txn[1]);
 
        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!(revoked_htlc_txn[3].input.len(), 1);
-       assert_eq!(revoked_htlc_txn[3].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
-       assert_eq!(revoked_htlc_txn[3].output.len(), 1);
-       check_spends!(revoked_htlc_txn[3], revoked_local_txn[0]);
+       assert_eq!(revoked_htlc_txn[2].input.len(), 1);
+       assert_eq!(revoked_htlc_txn[2].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+       assert_eq!(revoked_htlc_txn[2].output.len(), 1);
+       check_spends!(revoked_htlc_txn[2], revoked_local_txn[0]);
 
        // Broadcast set of revoked txn on A
        let hash_128 = connect_blocks(&nodes[0], 40);
        let header_11 = BlockHeader { version: 0x20000000, prev_blockhash: hash_128, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[0], &Block { header: header_11, txdata: vec![revoked_local_txn[0].clone()] });
        let header_129 = BlockHeader { version: 0x20000000, prev_blockhash: header_11.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       connect_block(&nodes[0], &Block { header: header_129, txdata: vec![revoked_htlc_txn[0].clone(), revoked_htlc_txn[3].clone()] });
+       connect_block(&nodes[0], &Block { header: header_129, txdata: vec![revoked_htlc_txn[0].clone(), revoked_htlc_txn[2].clone()] });
        expect_pending_htlcs_forwardable_ignore!(nodes[0]);
        let first;
        let feerate_1;
@@ -8089,7 +8289,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                assert_ne!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output);
 
                assert_eq!(node_txn[0].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output);
-               assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[3].input[0].previous_output);
+               assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[2].input[0].previous_output);
 
                // node_txn[3] is the local commitment tx broadcast just because (and somewhat in case of
                // reorgs, though its not clear its ever worth broadcasting conflicting txn like this when
@@ -8100,11 +8300,11 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                // output, checked above).
                assert_eq!(node_txn[4].input.len(), 2);
                assert_eq!(node_txn[4].output.len(), 1);
-               check_spends!(node_txn[4], revoked_htlc_txn[0], revoked_htlc_txn[3]);
+               check_spends!(node_txn[4], revoked_htlc_txn[0], revoked_htlc_txn[2]);
 
                first = node_txn[4].txid();
                // Store both feerates for later comparison
-               let fee_1 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[3].output[0].value - node_txn[4].output[0].value;
+               let fee_1 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[2].output[0].value - node_txn[4].output[0].value;
                feerate_1 = fee_1 * 1000 / node_txn[4].get_weight() as u64;
                penalty_txn = vec![node_txn[2].clone()];
                node_txn.clear();
@@ -8123,9 +8323,9 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                check_spends!(node_txn[1], revoked_local_txn[0]);
                // Note that these are both bogus - they spend outputs already claimed in block 129:
                if node_txn[0].input[0].previous_output == revoked_htlc_txn[0].input[0].previous_output  {
-                       assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[3].input[0].previous_output);
+                       assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[2].input[0].previous_output);
                } else {
-                       assert_eq!(node_txn[0].input[0].previous_output, revoked_htlc_txn[3].input[0].previous_output);
+                       assert_eq!(node_txn[0].input[0].previous_output, revoked_htlc_txn[2].input[0].previous_output);
                        assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output);
                }
 
@@ -8141,10 +8341,10 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                assert_eq!(node_txn.len(), 1);
 
                assert_eq!(node_txn[0].input.len(), 2);
-               check_spends!(node_txn[0], revoked_htlc_txn[0], revoked_htlc_txn[3]);
+               check_spends!(node_txn[0], revoked_htlc_txn[0], revoked_htlc_txn[2]);
                // Verify bumped tx is different and 25% bump heuristic
                assert_ne!(first, node_txn[0].txid());
-               let fee_2 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[3].output[0].value - node_txn[0].output[0].value;
+               let fee_2 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[2].output[0].value - node_txn[0].output[0].value;
                let feerate_2 = fee_2 * 1000 / node_txn[0].get_weight() as u64;
                assert!(feerate_2 * 100 > feerate_1 * 125);
                let txn = vec![node_txn[0].clone()];
@@ -8209,28 +8409,27 @@ fn test_bump_penalty_txn_on_remote_commitment() {
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                // 9 transactions including:
-               // 2*3 ChannelManager local broadcasts of commitment + HTLC-Success + HTLC-Timeout
+               // 1*2 ChannelManager local broadcasts of commitment + HTLC-Success
+               // 1*3 ChannelManager local broadcasts of commitment + HTLC-Success + HTLC-Timeout
                // 2 * HTLC-Success (one RBF bump we'll check later)
                // 1 * HTLC-Timeout
-               assert_eq!(node_txn.len(), 9);
+               assert_eq!(node_txn.len(), 8);
                assert_eq!(node_txn[0].input.len(), 1);
-               assert_eq!(node_txn[7].input.len(), 1);
+               assert_eq!(node_txn[6].input.len(), 1);
                check_spends!(node_txn[0], remote_txn[0]);
-               check_spends!(node_txn[7], remote_txn[0]);
-               assert_eq!(node_txn[0].input[0].previous_output, node_txn[4].input[0].previous_output);
-               preimage_bump = node_txn[4].clone();
+               check_spends!(node_txn[6], remote_txn[0]);
+               assert_eq!(node_txn[0].input[0].previous_output, node_txn[3].input[0].previous_output);
+               preimage_bump = node_txn[3].clone();
 
                check_spends!(node_txn[1], chan.3);
                check_spends!(node_txn[2], node_txn[1]);
-               check_spends!(node_txn[3], node_txn[1]);
-               assert_eq!(node_txn[1], node_txn[5]);
-               assert_eq!(node_txn[2], node_txn[6]);
-               assert_eq!(node_txn[3], node_txn[8]);
+               assert_eq!(node_txn[1], node_txn[4]);
+               assert_eq!(node_txn[2], node_txn[5]);
 
-               timeout = node_txn[7].txid();
-               let index = node_txn[7].input[0].previous_output.vout;
-               let fee = remote_txn[0].output[index as usize].value - node_txn[7].output[0].value;
-               feerate_timeout = fee * 1000 / node_txn[7].get_weight() as u64;
+               timeout = node_txn[6].txid();
+               let index = node_txn[6].input[0].previous_output.vout;
+               let fee = remote_txn[0].output[index as usize].value - node_txn[6].output[0].value;
+               feerate_timeout = fee * 1000 / node_txn[6].get_weight() as u64;
 
                preimage = node_txn[0].txid();
                let index = node_txn[0].input[0].previous_output.vout;
@@ -8474,13 +8673,16 @@ fn test_secret_timeout() {
        if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
                assert_eq!(err, "Duplicate payment hash");
        } else { panic!(); }
-       let mut block = Block {
-               header: BlockHeader {
-                       version: 0x2000000,
-                       prev_blockhash: nodes[1].blocks.borrow().last().unwrap().0.block_hash(),
-                       merkle_root: Default::default(),
-                       time: nodes[1].blocks.borrow().len() as u32 + 7200, bits: 42, nonce: 42 },
-               txdata: vec![],
+       let mut block = {
+               let node_1_blocks = nodes[1].blocks.lock().unwrap();
+               Block {
+                       header: BlockHeader {
+                               version: 0x2000000,
+                               prev_blockhash: node_1_blocks.last().unwrap().0.block_hash(),
+                               merkle_root: Default::default(),
+                               time: node_1_blocks.len() as u32 + 7200, bits: 42, nonce: 42 },
+                       txdata: vec![],
+               }
        };
        connect_block(&nodes[1], &block);
        if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
@@ -8630,6 +8832,9 @@ fn test_update_err_monitor_lockdown() {
                watchtower
        };
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       // Make the tx_broadcaster aware of enough blocks that it doesn't think we're violating
+       // transaction lock time requirements here.
+       chanmon_cfgs[0].tx_broadcaster.blocks.lock().unwrap().resize(200, (header, 0));
        watchtower.chain_monitor.block_connected(&Block { header, txdata: vec![] }, 200);
 
        // Try to update ChannelMonitor
@@ -8689,6 +8894,9 @@ fn test_concurrent_monitor_claim() {
                watchtower
        };
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       // Make the tx_broadcaster aware of enough blocks that it doesn't think we're violating
+       // transaction lock time requirements here.
+       chanmon_cfgs[0].tx_broadcaster.blocks.lock().unwrap().resize((CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS) as usize, (header, 0));
        watchtower_alice.chain_monitor.block_connected(&Block { header, txdata: vec![] }, CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
        // Watchtower Alice should have broadcast a commitment/HTLC-timeout
@@ -9283,3 +9491,86 @@ fn test_invalid_funding_tx() {
        } else { panic!(); }
        assert_eq!(nodes[1].node.list_channels().len(), 0);
 }
+
+fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_timelock: bool) {
+       // In the first version of the chain::Confirm interface, after a refactor was made to not
+       // broadcast CSV-locked transactions until their CSV lock is up, we wouldn't reliably broadcast
+       // transactions after a `transactions_confirmed` call. Specifically, if the chain, provided via
+       // `best_block_updated` is at height N, and a transaction output which we wish to spend at
+       // height N-1 (due to a CSV to height N-1) is provided at height N, we will not broadcast the
+       // spending transaction until height N+1 (or greater). This was due to the way
+       // `ChannelMonitor::transactions_confirmed` worked, only checking if we should broadcast a
+       // spending transaction at the height the input transaction was confirmed at, not whether we
+       // should broadcast a spending transaction at the current height.
+       // A second, similar, issue involved failing HTLCs backwards - because we only provided the
+       // height at which transactions were confirmed to `OnchainTx::update_claims_view`, it wasn't
+       // aware that the anti-reorg-delay had, in fact, already expired, waiting to fail-backwards
+       // until we learned about an additional block.
+       //
+       // As an additional check, if `test_height_before_timelock` is set, we instead test that we
+       // aren't broadcasting transactions too early (ie not broadcasting them at all).
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+       *nodes[0].connect_style.borrow_mut() = ConnectStyle::BestBlockFirstSkippingBlocks;
+
+       create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+       let (chan_announce, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
+       let (_, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+       nodes[1].node.peer_disconnected(&nodes[2].node.get_our_node_id(), false);
+       nodes[2].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+
+       nodes[1].node.force_close_channel(&channel_id).unwrap();
+       check_closed_broadcast!(nodes[1], true);
+       check_added_monitors!(nodes[1], 1);
+       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+       assert_eq!(node_txn.len(), 1);
+
+       let conf_height = nodes[1].best_block_info().1;
+       if !test_height_before_timelock {
+               connect_blocks(&nodes[1], 24 * 6);
+       }
+       nodes[1].chain_monitor.chain_monitor.transactions_confirmed(
+               &nodes[1].get_block_header(conf_height), &[(0, &node_txn[0])], conf_height);
+       if test_height_before_timelock {
+               // If we confirmed the close transaction, but timelocks have not yet expired, we should not
+               // generate any events or broadcast any transactions
+               assert!(nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());
+               assert!(nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty());
+       } else {
+               // 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]);
+               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).
+               let descriptor_spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
+               assert_eq!(descriptor_spend_txn.len(), 1);
+
+               // If we also discover that the HTLC-Timeout transaction was confirmed some time ago, we
+               // should immediately fail-backwards the HTLC to the previous hop, without waiting for an
+               // additional block built on top of the current chain.
+               nodes[1].chain_monitor.chain_monitor.transactions_confirmed(
+                       &nodes[1].get_block_header(conf_height + 1), &[(0, &spending_txn[1])], conf_height + 1);
+               expect_pending_htlcs_forwardable!(nodes[1]);
+               check_added_monitors!(nodes[1], 1);
+
+               let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               assert!(updates.update_add_htlcs.is_empty());
+               assert!(updates.update_fulfill_htlcs.is_empty());
+               assert_eq!(updates.update_fail_htlcs.len(), 1);
+               assert!(updates.update_fail_malformed_htlcs.is_empty());
+               assert!(updates.update_fee.is_none());
+               nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
+               commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, true, true);
+               expect_payment_failed!(nodes[0], payment_hash, false);
+               expect_payment_failure_chan_update!(nodes[0], chan_announce.contents.short_channel_id, true);
+       }
+}
+#[test]
+fn test_tx_confirmed_skipping_blocks_immediate_broadcast() {
+       do_test_tx_confirmed_skipping_blocks_immediate_broadcast(false);
+       do_test_tx_confirmed_skipping_blocks_immediate_broadcast(true);
+}