Merge pull request #2935 from valentinewallace/2024-03-keysend-to-blinded
[rust-lightning] / lightning / src / ln / reload_tests.rs
index 67a48012789c1e30d6e56e519984f21eefb84eee..4422af1e8c70c63a81b93ece1aac38e2553d00a0 100644 (file)
 
 use crate::chain::{ChannelMonitorUpdateStatus, Watch};
 use crate::chain::chaininterface::LowerBoundedFeeEstimator;
-use crate::chain::channelmonitor::ChannelMonitor;
-use crate::chain::keysinterface::EntropySource;
+use crate::chain::channelmonitor::{CLOSED_CHANNEL_UPDATE_ID, ChannelMonitor};
+use crate::sign::EntropySource;
 use crate::chain::transaction::OutPoint;
 use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
 use crate::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RecipientOnionFields};
-use crate::ln::msgs;
+use crate::ln::{msgs, ChannelId};
 use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
-use crate::util::enforcing_trait_impls::EnforcingSigner;
+use crate::util::test_channel_signer::TestChannelSigner;
 use crate::util::test_utils;
 use crate::util::errors::APIError;
 use crate::util::ser::{Writeable, ReadableArgs};
 use crate::util::config::UserConfig;
-use crate::util::string::UntrustedString;
 
 use bitcoin::hash_types::BlockHash;
 
@@ -38,10 +37,11 @@ fn test_funding_peer_disconnect() {
        // Test that we can lock in our funding tx while disconnected
        let chanmon_cfgs = create_chanmon_cfgs(2);
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let persister;
+       let new_chain_monitor;
+
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_0_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let nodes_0_deserialized;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001);
 
@@ -52,7 +52,9 @@ fn test_funding_peer_disconnect() {
        let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
        assert!(events_1.is_empty());
 
-       reconnect_nodes(&nodes[0], &nodes[1], (false, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+       reconnect_args.send_channel_ready.1 = true;
+       reconnect_nodes(reconnect_args);
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
@@ -61,9 +63,13 @@ fn test_funding_peer_disconnect() {
        let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
        assert!(events_2.is_empty());
 
-       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }, true).unwrap();
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init {
+               features: nodes[1].node.init_features(), networks: None, remote_network_address: None
+       }, true).unwrap();
        let as_reestablish = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
-       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }, false).unwrap();
+       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init {
+               features: nodes[0].node.init_features(), networks: None, remote_network_address: None
+       }, false).unwrap();
        let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
 
        // nodes[0] hasn't yet received a channel_ready, so it only sends that on reconnect.
@@ -176,17 +182,18 @@ fn test_funding_peer_disconnect() {
 
        reload_node!(nodes[0], &nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
 
-       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 }
 
 #[test]
 fn test_no_txn_manager_serialize_deserialize() {
        let chanmon_cfgs = create_chanmon_cfgs(2);
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let persister;
+       let new_chain_monitor;
+
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_0_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let nodes_0_deserialized;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
        let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001);
@@ -194,12 +201,16 @@ fn test_no_txn_manager_serialize_deserialize() {
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
 
        let chan_0_monitor_serialized =
-               get_monitor!(nodes[0], OutPoint { txid: tx.txid(), index: 0 }.to_channel_id()).encode();
+               get_monitor!(nodes[0], ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.txid(), index: 0 })).encode();
        reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
 
-       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }, true).unwrap();
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init {
+               features: nodes[1].node.init_features(), networks: None, remote_network_address: None
+       }, true).unwrap();
        let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
-       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }, false).unwrap();
+       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init {
+               features: nodes[0].node.init_features(), networks: None, remote_network_address: None
+       }, false).unwrap();
        let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
@@ -223,10 +234,11 @@ fn test_manager_serialize_deserialize_events() {
        // This test makes sure the events field in ChannelManager survives de/serialization
        let chanmon_cfgs = create_chanmon_cfgs(2);
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let persister;
+       let new_chain_monitor;
+
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_0_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let nodes_0_deserialized;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
        // Start creating a channel, but stop right before broadcasting the funding transaction
@@ -234,7 +246,7 @@ fn test_manager_serialize_deserialize_events() {
        let push_msat = 10001;
        let node_a = nodes.remove(0);
        let node_b = nodes.remove(0);
-       node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
+       node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None, None).unwrap();
        node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id()));
        node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id()));
 
@@ -282,9 +294,13 @@ fn test_manager_serialize_deserialize_events() {
        // Make sure the channel is functioning as though the de/serialization never happened
        assert_eq!(nodes[0].node.list_channels().len(), 1);
 
-       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }, true).unwrap();
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init {
+               features: nodes[1].node.init_features(), networks: None, remote_network_address: None
+       }, true).unwrap();
        let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
-       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }, false).unwrap();
+       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init {
+               features: nodes[0].node.init_features(), networks: None, remote_network_address: None
+       }, false).unwrap();
        let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
@@ -307,22 +323,23 @@ fn test_manager_serialize_deserialize_events() {
 fn test_simple_manager_serialize_deserialize() {
        let chanmon_cfgs = create_chanmon_cfgs(2);
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let persister;
+       let new_chain_monitor;
+
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_0_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let nodes_0_deserialized;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
 
-       let (our_payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
-       let (_, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (our_payment_preimage, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (_, our_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
 
        let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
        reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
 
-       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 
        fail_payment(&nodes[0], &[&nodes[1]], our_payment_hash);
        claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage);
@@ -333,13 +350,15 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        // Test deserializing a ChannelManager with an out-of-date ChannelMonitor
        let chanmon_cfgs = create_chanmon_cfgs(4);
        let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+       let logger;
+       let fee_estimator;
+       let persister;
+       let new_chain_monitor;
+
        let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
-       let logger: test_utils::TestLogger;
-       let fee_estimator: test_utils::TestFeeEstimator;
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_0_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let nodes_0_deserialized;
        let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
        let chan_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2;
        let chan_id_2 = create_announced_chan_between_nodes(&nodes, 2, 0).2;
        let (_, _, channel_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 3);
@@ -351,7 +370,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
                node_0_stale_monitors_serialized.push(writer.0);
        }
 
-       let (our_payment_preimage, _, _) = route_payment(&nodes[2], &[&nodes[0], &nodes[1]], 1000000);
+       let (our_payment_preimage, ..) = route_payment(&nodes[2], &[&nodes[0], &nodes[1]], 1000000);
 
        // Serialize the ChannelManager here, but the monitor we keep up-to-date
        let nodes_0_serialized = nodes[0].node.encode();
@@ -372,14 +391,14 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        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);
+       new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster, &logger, &fee_estimator, &persister, keys_manager);
        nodes[0].chain_monitor = &new_chain_monitor;
 
 
        let mut node_0_stale_monitors = Vec::new();
        for serialized in node_0_stale_monitors_serialized.iter() {
                let mut read = &serialized[..];
-               let (_, monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
+               let (_, monitor) = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
                assert!(read.is_empty());
                node_0_stale_monitors.push(monitor);
        }
@@ -387,7 +406,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        let mut node_0_monitors = Vec::new();
        for serialized in node_0_monitors_serialized.iter() {
                let mut read = &serialized[..];
-               let (_, monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
+               let (_, monitor) = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
                assert!(read.is_empty());
                node_0_monitors.push(monitor);
        }
@@ -402,7 +421,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
                fee_estimator: &fee_estimator,
                router: &nodes[0].router,
                chain_monitor: nodes[0].chain_monitor,
-               tx_broadcaster: nodes[0].tx_broadcaster.clone(),
+               tx_broadcaster: nodes[0].tx_broadcaster,
                logger: &logger,
                channel_monitors: node_0_stale_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect(),
        }) { } else {
@@ -419,7 +438,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
                fee_estimator: &fee_estimator,
                router: nodes[0].router,
                chain_monitor: nodes[0].chain_monitor,
-               tx_broadcaster: nodes[0].tx_broadcaster.clone(),
+               tx_broadcaster: nodes[0].tx_broadcaster,
                logger: &logger,
                channel_monitors: node_0_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect(),
        }).unwrap();
@@ -427,13 +446,14 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        assert!(nodes_0_read.is_empty());
 
        for monitor in node_0_monitors.drain(..) {
-               assert_eq!(nodes[0].chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor),
-                       ChannelMonitorUpdateStatus::Completed);
+               let funding_outpoint = monitor.get_funding_txo().0;
+               assert_eq!(nodes[0].chain_monitor.watch_channel(funding_outpoint, monitor),
+                       Ok(ChannelMonitorUpdateStatus::Completed));
                check_added_monitors!(nodes[0], 1);
        }
        nodes[0].node = &nodes_0_deserialized;
 
-       check_closed_event!(nodes[0], 1, ClosureReason::OutdatedChannelManager);
+       check_closed_event!(nodes[0], 1, ClosureReason::OutdatedChannelManager, [nodes[3].node.get_our_node_id()], 100000);
        { // Channel close should result in a commitment tx
                nodes[0].node.timer_tick_occurred();
                let txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -444,14 +464,18 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        check_added_monitors!(nodes[0], 1);
 
        // nodes[1] and nodes[2] have no lost state with nodes[0]...
-       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
-       reconnect_nodes(&nodes[0], &nodes[2], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[2]));
        //... and we can even still claim the payment!
        claim_payment(&nodes[2], &[&nodes[0], &nodes[1]], our_payment_preimage);
 
-       nodes[3].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }, true).unwrap();
+       nodes[3].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init {
+               features: nodes[0].node.init_features(), networks: None, remote_network_address: None
+       }, true).unwrap();
        let reestablish = get_chan_reestablish_msgs!(nodes[3], nodes[0]).pop().unwrap();
-       nodes[0].node.peer_connected(&nodes[3].node.get_our_node_id(), &msgs::Init { features: nodes[3].node.init_features(), remote_network_address: None }, false).unwrap();
+       nodes[0].node.peer_connected(&nodes[3].node.get_our_node_id(), &msgs::Init {
+               features: nodes[3].node.init_features(), networks: None, remote_network_address: None
+       }, false).unwrap();
        nodes[0].node.handle_channel_reestablish(&nodes[3].node.get_our_node_id(), &reestablish);
        let mut found_err = false;
        for msg_event in nodes[0].node.get_and_clear_pending_msg_events() {
@@ -469,7 +493,11 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        assert!(found_err);
 }
 
-fn do_test_data_loss_protect(reconnect_panicing: bool) {
+#[cfg(feature = "std")]
+fn do_test_data_loss_protect(reconnect_panicing: bool, substantially_old: bool, not_stale: bool) {
+       use crate::routing::router::{RouteParameters, PaymentParameters};
+       use crate::ln::channelmanager::Retry;
+       use crate::util::string::UntrustedString;
        // When we get a data_loss_protect proving we're behind, we immediately panic as the
        // chain::Watch API requirements have been violated (e.g. the user restored from a backup). The
        // panic message informs the user they should force-close without broadcasting, which is tested
@@ -478,11 +506,13 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
        // We broadcast during Drop because chanmon is out of sync with chanmgr, which would cause a panic
        // during signing due to revoked tx
        chanmon_cfgs[0].keys_manager.disable_revocation_policy_check = true;
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
        let persister;
        let new_chain_monitor;
-       let nodes_0_deserialized;
-       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let nodes_0_deserialized;
+
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000);
@@ -491,8 +521,38 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
        let previous_node_state = nodes[0].node.encode();
        let previous_chain_monitor_state = get_monitor!(nodes[0], chan.2).encode();
 
-       send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
-       send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
+       assert!(!substantially_old || !not_stale, "substantially_old and not_stale doesn't make sense");
+       if not_stale || !substantially_old {
+               // Previously, we'd only hit the data_loss_protect assertion if we had a state which
+               // revoked at least two revocations ago, not the latest revocation. Here, we use
+               // `not_stale` to test the boundary condition.
+               let pay_params = PaymentParameters::for_keysend(nodes[1].node.get_our_node_id(), 100, false);
+               let route_params = RouteParameters::from_payment_params_and_value(pay_params, 40000);
+               nodes[0].node.send_spontaneous_payment_with_retry(None, RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), route_params, Retry::Attempts(0)).unwrap();
+               check_added_monitors(&nodes[0], 1);
+               let update_add_commit = SendEvent::from_node(&nodes[0]);
+
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add_commit.msgs[0]);
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &update_add_commit.commitment_msg);
+               check_added_monitors(&nodes[1], 1);
+               let (raa, cs) = get_revoke_commit_msgs(&nodes[1], &nodes[0].node.get_our_node_id());
+
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &raa);
+               check_added_monitors(&nodes[0], 1);
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+               if !not_stale {
+                       nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &cs);
+                       check_added_monitors(&nodes[0], 1);
+                       // A now revokes their original state, at which point reconnect should panic
+                       let raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+                       nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &raa);
+                       check_added_monitors(&nodes[1], 1);
+                       expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+               }
+       } else {
+               send_payment(&nodes[0], &[&nodes[1]], 8000000);
+               send_payment(&nodes[0], &[&nodes[1]], 8000000);
+       }
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
@@ -500,63 +560,112 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
        reload_node!(nodes[0], previous_node_state, &[&previous_chain_monitor_state], persister, new_chain_monitor, nodes_0_deserialized);
 
        if reconnect_panicing {
-               nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }, true).unwrap();
-               nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }, false).unwrap();
+               nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init {
+                       features: nodes[1].node.init_features(), networks: None, remote_network_address: None
+               }, true).unwrap();
+               nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init {
+                       features: nodes[0].node.init_features(), networks: None, remote_network_address: None
+               }, false).unwrap();
 
                let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
 
-               // Check we close channel detecting A is fallen-behind
-               // Check that we sent the warning message when we detected that A has fallen behind,
-               // and give the possibility for A to recover from the warning.
+               // If A has fallen behind substantially, B should send it a message letting it know
+               // that.
                nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
-               let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction".to_owned();
-               assert!(check_warn_msg!(nodes[1], nodes[0].node.get_our_node_id(), chan.2).contains(&warn_msg));
+               let reestablish_msg;
+               if substantially_old {
+                       let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction: 0 (received) vs 4 (expected)".to_owned();
+
+                       let warn_reestablish = nodes[1].node.get_and_clear_pending_msg_events();
+                       assert_eq!(warn_reestablish.len(), 2);
+                       match warn_reestablish[1] {
+                               MessageSendEvent::HandleError { action: ErrorAction::SendWarningMessage { ref msg, .. }, .. } => {
+                                       assert_eq!(msg.data, warn_msg);
+                               },
+                               _ => panic!("Unexpected events: {:?}", warn_reestablish),
+                       }
+                       reestablish_msg = match &warn_reestablish[0] {
+                               MessageSendEvent::SendChannelReestablish { msg, .. } => msg.clone(),
+                               _ => panic!("Unexpected events: {:?}", warn_reestablish),
+                       };
+               } else {
+                       let msgs = nodes[1].node.get_and_clear_pending_msg_events();
+                       assert!(msgs.len() >= 4);
+                       match msgs.last() {
+                               Some(MessageSendEvent::SendChannelUpdate { .. }) => {},
+                               _ => panic!("Unexpected events: {:?}", msgs),
+                       }
+                       assert!(msgs.iter().any(|msg| matches!(msg, MessageSendEvent::SendRevokeAndACK { .. })));
+                       assert!(msgs.iter().any(|msg| matches!(msg, MessageSendEvent::UpdateHTLCs { .. })));
+                       reestablish_msg = match &msgs[0] {
+                               MessageSendEvent::SendChannelReestablish { msg, .. } => msg.clone(),
+                               _ => panic!("Unexpected events: {:?}", msgs),
+                       };
+               }
 
                {
-                       let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
-                       // The node B should not broadcast the transaction to force close the channel!
+                       let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+                       // The node B should never force-close the channel.
                        assert!(node_txn.is_empty());
                }
 
-               let reestablish_0 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
                // Check A panics upon seeing proof it has fallen behind.
-               nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_0[0]);
-               return; // By this point we should have panic'ed!
-       }
+               let reconnect_res = std::panic::catch_unwind(|| {
+                       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_msg);
+               });
+               if not_stale {
+                       assert!(reconnect_res.is_ok());
+                       // At this point A gets confused because B expects a commitment state newer than A
+                       // has sent, but not a newer revocation secret, so A just (correctly) closes.
+                       check_closed_broadcast(&nodes[0], 1, true);
+                       check_added_monitors(&nodes[0], 1);
+                       check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError {
+                               err: "Peer attempted to reestablish channel with a future remote commitment transaction: 2 (received) vs 1 (expected)".to_owned()
+                       }, [nodes[1].node.get_our_node_id()], 1000000);
+               } else {
+                       assert!(reconnect_res.is_err());
+                       // Skip the `Drop` handler for `Node`s as some may be in an invalid (panicked) state.
+                       std::mem::forget(nodes);
+               }
+       } else {
+               assert!(!not_stale, "We only care about the stale case when not testing panicking");
 
-       nodes[0].node.force_close_without_broadcasting_txn(&chan.2, &nodes[1].node.get_our_node_id()).unwrap();
-       check_added_monitors!(nodes[0], 1);
-       check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
-       {
-               let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
-               assert_eq!(node_txn.len(), 0);
-       }
+               nodes[0].node.force_close_without_broadcasting_txn(&chan.2, &nodes[1].node.get_our_node_id()).unwrap();
+               check_added_monitors!(nodes[0], 1);
+               check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 1000000);
+               {
+                       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+                       assert_eq!(node_txn.len(), 0);
+               }
 
-       for msg in nodes[0].node.get_and_clear_pending_msg_events() {
-               if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg {
-               } else if let MessageSendEvent::HandleError { ref action, .. } = msg {
-                       match action {
-                               &ErrorAction::SendErrorMessage { ref msg } => {
-                                       assert_eq!(msg.data, "Channel force-closed");
-                               },
-                               _ => panic!("Unexpected event!"),
+               for msg in nodes[0].node.get_and_clear_pending_msg_events() {
+                       if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg {
+                       } else if let MessageSendEvent::HandleError { ref action, .. } = msg {
+                               match action {
+                                       &ErrorAction::DisconnectPeer { ref msg } => {
+                                               assert_eq!(msg.as_ref().unwrap().data, "Channel force-closed");
+                                       },
+                                       _ => panic!("Unexpected event!"),
+                               }
+                       } else {
+                               panic!("Unexpected event {:?}", msg)
                        }
-               } else {
-                       panic!("Unexpected event {:?}", msg)
                }
-       }
 
-       // after the warning message sent by B, we should not able to
-       // use the channel, or reconnect with success to the channel.
-       assert!(nodes[0].node.list_usable_channels().is_empty());
-       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }, true).unwrap();
-       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }, false).unwrap();
-       let retry_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
-
-       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &retry_reestablish[0]);
-       let mut err_msgs_0 = Vec::with_capacity(1);
-       for msg in nodes[0].node.get_and_clear_pending_msg_events() {
-               if let MessageSendEvent::HandleError { ref action, .. } = msg {
+               // after the warning message sent by B, we should not able to
+               // use the channel, or reconnect with success to the channel.
+               assert!(nodes[0].node.list_usable_channels().is_empty());
+               nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init {
+                       features: nodes[1].node.init_features(), networks: None, remote_network_address: None
+               }, true).unwrap();
+               nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init {
+                       features: nodes[0].node.init_features(), networks: None, remote_network_address: None
+               }, false).unwrap();
+               let retry_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+
+               nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &retry_reestablish[0]);
+               let mut err_msgs_0 = Vec::with_capacity(1);
+               if let MessageSendEvent::HandleError { ref action, .. } = nodes[0].node.get_and_clear_pending_msg_events()[1] {
                        match action {
                                &ErrorAction::SendErrorMessage { ref msg } => {
                                        assert_eq!(msg.data, format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id()));
@@ -567,24 +676,24 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
                } else {
                        panic!("Unexpected event!");
                }
+               assert_eq!(err_msgs_0.len(), 1);
+               nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &err_msgs_0[0]);
+               assert!(nodes[1].node.list_usable_channels().is_empty());
+               check_added_monitors!(nodes[1], 1);
+               check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id())) }
+                       , [nodes[0].node.get_our_node_id()], 1000000);
+               check_closed_broadcast!(nodes[1], false);
        }
-       assert_eq!(err_msgs_0.len(), 1);
-       nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &err_msgs_0[0]);
-       assert!(nodes[1].node.list_usable_channels().is_empty());
-       check_added_monitors!(nodes[1], 1);
-       check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id())) });
-       check_closed_broadcast!(nodes[1], false);
 }
 
 #[test]
-#[should_panic]
-fn test_data_loss_protect_showing_stale_state_panics() {
-       do_test_data_loss_protect(true);
-}
-
-#[test]
-fn test_force_close_without_broadcast() {
-       do_test_data_loss_protect(false);
+#[cfg(feature = "std")]
+fn test_data_loss_protect() {
+       do_test_data_loss_protect(true, false, true);
+       do_test_data_loss_protect(true, true, false);
+       do_test_data_loss_protect(true, false, false);
+       do_test_data_loss_protect(false, true, false);
+       do_test_data_loss_protect(false, false, false);
 }
 
 #[test]
@@ -596,10 +705,10 @@ fn test_forwardable_regen() {
 
        let chanmon_cfgs = create_chanmon_cfgs(3);
        let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let persister;
+       let new_chain_monitor;
        let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_1_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let nodes_1_deserialized;
        let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
        let chan_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2;
        let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2).2;
@@ -642,10 +751,12 @@ fn test_forwardable_regen() {
        let chan_1_monitor_serialized = get_monitor!(nodes[1], chan_id_2).encode();
        reload_node!(nodes[1], nodes[1].node.encode(), &[&chan_0_monitor_serialized, &chan_1_monitor_serialized], persister, new_chain_monitor, nodes_1_deserialized);
 
-       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
        // Note that nodes[1] and nodes[2] resend their channel_ready here since they haven't updated
        // the commitment state.
-       reconnect_nodes(&nodes[1], &nodes[2], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       let mut reconnect_args = ReconnectArgs::new(&nodes[1], &nodes[2]);
+       reconnect_args.send_channel_ready = (true, true);
+       reconnect_nodes(reconnect_args);
 
        assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
 
@@ -681,11 +792,11 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
        // definitely claimed.
        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]);
+       let persister;
+       let new_chain_monitor;
 
-       let persister: test_utils::TestPersister;
-       let new_chain_monitor: test_utils::TestChainMonitor;
-       let nodes_3_deserialized: ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>;
+       let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+       let nodes_3_deserialized;
 
        let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);
 
@@ -712,15 +823,19 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
        assert_eq!(send_events.len(), 2);
        let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut send_events);
        let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut send_events);
-       do_pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, true, false, None);
-       do_pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_2_msgs, true, false, None);
+       do_pass_along_path(PassAlongPathArgs::new(&nodes[0],&[&nodes[1], &nodes[3]], 15_000_000, payment_hash, node_1_msgs)
+               .with_payment_secret(payment_secret)
+               .without_clearing_recipient_events());
+       do_pass_along_path(PassAlongPathArgs::new(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, payment_hash, node_2_msgs)
+               .with_payment_secret(payment_secret)
+               .without_clearing_recipient_events());
 
        // Now that we have an MPP payment pending, get the latest encoded copies of nodes[3]'s
        // monitors and ChannelManager, for use later, if we don't want to persist both monitors.
        let mut original_monitor = test_utils::TestVecWriter(Vec::new());
        if !persist_both_monitors {
-               for outpoint in nodes[3].chain_monitor.chain_monitor.list_monitors() {
-                       if outpoint.to_channel_id() == chan_id_not_persisted {
+               for (outpoint, channel_id) in nodes[3].chain_monitor.chain_monitor.list_monitors() {
+                       if channel_id == chan_id_not_persisted {
                                assert!(original_monitor.0.is_empty());
                                nodes[3].chain_monitor.chain_monitor.get_monitor(outpoint).unwrap().write(&mut original_monitor).unwrap();
                        }
@@ -739,16 +854,16 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
        // crashed in between the two persistence calls - using one old ChannelMonitor and one new one,
        // with the old ChannelManager.
        let mut updated_monitor = test_utils::TestVecWriter(Vec::new());
-       for outpoint in nodes[3].chain_monitor.chain_monitor.list_monitors() {
-               if outpoint.to_channel_id() == chan_id_persisted {
+       for (outpoint, channel_id) in nodes[3].chain_monitor.chain_monitor.list_monitors() {
+               if channel_id == chan_id_persisted {
                        assert!(updated_monitor.0.is_empty());
                        nodes[3].chain_monitor.chain_monitor.get_monitor(outpoint).unwrap().write(&mut updated_monitor).unwrap();
                }
        }
        // If `persist_both_monitors` is set, get the second monitor here as well
        if persist_both_monitors {
-               for outpoint in nodes[3].chain_monitor.chain_monitor.list_monitors() {
-                       if outpoint.to_channel_id() == chan_id_not_persisted {
+               for (outpoint, channel_id) in nodes[3].chain_monitor.chain_monitor.list_monitors() {
+                       if channel_id == chan_id_not_persisted {
                                assert!(original_monitor.0.is_empty());
                                nodes[3].chain_monitor.chain_monitor.get_monitor(outpoint).unwrap().write(&mut original_monitor).unwrap();
                        }
@@ -774,6 +889,9 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
        if let Event::ChannelClosed { reason: ClosureReason::OutdatedChannelManager, .. } = events[1] { } else { panic!(); }
        if persist_both_monitors {
                if let Event::ChannelClosed { reason: ClosureReason::OutdatedChannelManager, .. } = events[2] { } else { panic!(); }
+               check_added_monitors(&nodes[3], 2);
+       } else {
+               check_added_monitors(&nodes[3], 1);
        }
 
        // On restart, we should also get a duplicate PaymentClaimed event as we persisted the
@@ -788,9 +906,13 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
        if !persist_both_monitors {
                // If one of the two channels is still live, reveal the payment preimage over it.
 
-               nodes[3].node.peer_connected(&nodes[2].node.get_our_node_id(), &msgs::Init { features: nodes[2].node.init_features(), remote_network_address: None }, true).unwrap();
+               nodes[3].node.peer_connected(&nodes[2].node.get_our_node_id(), &msgs::Init {
+                       features: nodes[2].node.init_features(), networks: None, remote_network_address: None
+               }, true).unwrap();
                let reestablish_1 = get_chan_reestablish_msgs!(nodes[3], nodes[2]);
-               nodes[2].node.peer_connected(&nodes[3].node.get_our_node_id(), &msgs::Init { features: nodes[3].node.init_features(), remote_network_address: None }, false).unwrap();
+               nodes[2].node.peer_connected(&nodes[3].node.get_our_node_id(), &msgs::Init {
+                       features: nodes[3].node.init_features(), networks: None, remote_network_address: None
+               }, false).unwrap();
                let reestablish_2 = get_chan_reestablish_msgs!(nodes[2], nodes[3]);
 
                nodes[2].node.handle_channel_reestablish(&nodes[3].node.get_our_node_id(), &reestablish_1[0]);
@@ -838,12 +960,12 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
        // This was never an issue, but it may be easy to regress here going forward.
        let chanmon_cfgs = create_chanmon_cfgs(3);
        let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let persister;
+       let new_chain_monitor;
+
        let mut intercept_forwards_config = test_default_channel_config();
        intercept_forwards_config.accept_intercept_htlcs = true;
        let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), None]);
-
-       let persister;
-       let new_chain_monitor;
        let nodes_1_deserialized;
 
        let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
@@ -909,7 +1031,7 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
        assert_eq!(cs_commitment_tx.len(), if claim_htlc { 2 } else { 1 });
 
        check_added_monitors!(nodes[2], 1);
-       check_closed_event!(nodes[2], 1, ClosureReason::HolderForceClosed);
+       check_closed_event!(nodes[2], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
        check_closed_broadcast!(nodes[2], true);
 
        let chan_0_monitor_serialized = get_monitor!(nodes[1], chan_id_1).encode();
@@ -918,7 +1040,7 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
 
        // Note that this checks that this is the only event on nodes[1], implying the
        // `HTLCIntercepted` event has been removed in the `use_intercept` case.
-       check_closed_event!(nodes[1], 1, ClosureReason::OutdatedChannelManager);
+       check_closed_event!(nodes[1], 1, ClosureReason::OutdatedChannelManager, [nodes[2].node.get_our_node_id()], 100000);
 
        if use_intercept {
                // Attempt to forward the HTLC back out over nodes[1]' still-open channel, ensuring we get
@@ -936,7 +1058,7 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
        check_added_monitors!(nodes[1], 1);
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
-       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 
        if use_cs_commitment {
                // If we confirm a commitment transaction that has the HTLC on-chain, nodes[1] should wait
@@ -949,9 +1071,10 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
                        confirm_transaction(&nodes[1], &cs_commitment_tx[1]);
                } else {
                        connect_blocks(&nodes[1], htlc_expiry - nodes[1].best_block_info().1 + 1);
-                       let bs_htlc_timeout_tx = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
-                       assert_eq!(bs_htlc_timeout_tx.len(), 1);
-                       confirm_transaction(&nodes[1], &bs_htlc_timeout_tx[0]);
+                       let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
+                       assert_eq!(txn.len(), if nodes[1].connect_style.borrow().updates_best_block_first() { 2 } else { 1 });
+                       let bs_htlc_timeout_tx = txn.pop().unwrap();
+                       confirm_transaction(&nodes[1], &bs_htlc_timeout_tx);
                }
        } else {
                confirm_transaction(&nodes[1], &bs_commitment_tx[0]);
@@ -1008,10 +1131,10 @@ fn removed_payment_no_manager_persistence() {
        // were left dangling when a channel was force-closed due to a stale ChannelManager.
        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 persister;
        let new_chain_monitor;
+
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
        let nodes_1_deserialized;
 
        let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
@@ -1019,7 +1142,7 @@ fn removed_payment_no_manager_persistence() {
        let chan_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2;
        let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2).2;
 
-       let (_, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+       let (_, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
 
        let node_encoded = nodes[1].node.encode();
 
@@ -1047,11 +1170,14 @@ fn removed_payment_no_manager_persistence() {
                _ => panic!("Unexpected event"),
        }
 
+       nodes[1].node.test_process_background_events();
+       check_added_monitors(&nodes[1], 1);
+
        // Now that the ChannelManager has force-closed the channel which had the HTLC removed, it is
        // now forgotten everywhere. The ChannelManager should have, as a side-effect of reload,
        // learned that the HTLC is gone from the ChannelMonitor and added it to the to-fail-back set.
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
-       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 
        expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], [HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_id_2 }]);
        check_added_monitors!(nodes[1], 1);
@@ -1067,3 +1193,65 @@ fn removed_payment_no_manager_persistence() {
 
        expect_payment_failed!(nodes[0], payment_hash, false);
 }
+
+#[test]
+fn test_reload_partial_funding_batch() {
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let new_persister;
+       let new_chain_monitor;
+
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let new_channel_manager;
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       // Initiate channel opening and create the batch channel funding transaction.
+       let (tx, funding_created_msgs) = create_batch_channel_funding(&nodes[0], &[
+               (&nodes[1], 100_000, 0, 42, None),
+               (&nodes[2], 200_000, 0, 43, None),
+       ]);
+
+       // Go through the funding_created and funding_signed flow with node 1.
+       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msgs[0]);
+       check_added_monitors(&nodes[1], 1);
+       expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());
+
+       // The monitor is persisted when receiving funding_signed.
+       let funding_signed_msg = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed_msg);
+       check_added_monitors(&nodes[0], 1);
+
+       // The transaction should not have been broadcast before all channels are ready.
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcast().len(), 0);
+
+       // Reload the node while a subset of the channels in the funding batch have persisted monitors.
+       let channel_id_1 = ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.txid(), index: 0 });
+       let node_encoded = nodes[0].node.encode();
+       let channel_monitor_1_serialized = get_monitor!(nodes[0], channel_id_1).encode();
+       reload_node!(nodes[0], node_encoded, &[&channel_monitor_1_serialized], new_persister, new_chain_monitor, new_channel_manager);
+
+       // Process monitor events.
+       assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+
+       // The monitor should become closed.
+       check_added_monitors(&nodes[0], 1);
+       {
+               let mut monitor_updates = nodes[0].chain_monitor.monitor_updates.lock().unwrap();
+               let monitor_updates_1 = monitor_updates.get(&channel_id_1).unwrap();
+               assert_eq!(monitor_updates_1.len(), 1);
+               assert_eq!(monitor_updates_1[0].update_id, CLOSED_CHANNEL_UPDATE_ID);
+       }
+
+       // The funding transaction should not have been broadcast, but we broadcast the force-close
+       // transaction as part of closing the monitor.
+       {
+               let broadcasted_txs = nodes[0].tx_broadcaster.txn_broadcast();
+               assert_eq!(broadcasted_txs.len(), 1);
+               assert!(broadcasted_txs[0].txid() != tx.txid());
+               assert_eq!(broadcasted_txs[0].input.len(), 1);
+               assert_eq!(broadcasted_txs[0].input[0].previous_output.txid, tx.txid());
+       }
+
+       // Ensure the channels don't exist anymore.
+       assert!(nodes[0].node.list_channels().is_empty());
+}