[peer_handler] Take the peers lock before getting messages to send
[rust-lightning] / lightning / src / ln / functional_tests.rs
index c7a79e32210639fddcd4c3d01ab16e6f311248a3..9a0c06e776cf4db266050b7986d08dc7fab67d2b 100644 (file)
@@ -16,7 +16,7 @@ use chain::Watch;
 use chain::channelmonitor;
 use chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
 use chain::transaction::OutPoint;
-use chain::keysinterface::{Sign, KeysInterface};
+use chain::keysinterface::{KeysInterface, BaseSign};
 use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure, BREAKDOWN_TIMEOUT};
 use ln::channel::{Channel, ChannelError};
@@ -51,7 +51,6 @@ use regex;
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::default::Default;
 use std::sync::Mutex;
-use std::sync::atomic::Ordering;
 
 use ln::functional_test_utils::*;
 use ln::chan_utils::CommitmentTransaction;
@@ -394,8 +393,7 @@ fn test_multi_flight_update_fee() {
        check_added_monitors!(nodes[1], 1);
 }
 
-#[test]
-fn test_1_conf_open() {
+fn do_test_1_conf_open(connect_style: ConnectStyle) {
        // Previously, if the minium_depth config was set to 1, we'd never send a funding_locked. This
        // tests that we properly send one in that case.
        let mut alice_config = UserConfig::default();
@@ -409,7 +407,8 @@ fn test_1_conf_open() {
        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, &[Some(alice_config), Some(bob_config)]);
-       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       *nodes[0].connect_style.borrow_mut() = connect_style;
 
        let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
        mine_transaction(&nodes[1], &tx);
@@ -425,6 +424,12 @@ fn test_1_conf_open() {
                node.net_graph_msg_handler.handle_channel_update(&bs_update).unwrap();
        }
 }
+#[test]
+fn test_1_conf_open() {
+       do_test_1_conf_open(ConnectStyle::BestBlockFirst);
+       do_test_1_conf_open(ConnectStyle::TransactionsFirst);
+       do_test_1_conf_open(ConnectStyle::FullBlockViaListen);
+}
 
 fn do_test_sanity_on_in_flight_opens(steps: u8) {
        // Previously, we had issues deserializing channels when we hadn't connected the first block
@@ -460,7 +465,7 @@ fn do_test_sanity_on_in_flight_opens(steps: u8) {
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
 
        if steps & 0x0f == 3 { return; }
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
        let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
 
@@ -484,14 +489,7 @@ fn do_test_sanity_on_in_flight_opens(steps: u8) {
        }
 
        let events_4 = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
 
        if steps & 0x0f == 6 { return; }
        create_chan_between_nodes_with_value_confirm_first(&nodes[0], &nodes[1], &tx, 2);
@@ -1508,10 +1506,14 @@ fn test_duplicate_htlc_different_direction_onchain() {
        check_spends!(htlc_pair.1, remote_txn[0]);
 
        let events = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(events.len(), 2);
+       assert_eq!(events.len(), 3);
        for e in events {
                match e {
                        MessageSendEvent::BroadcastChannelUpdate { .. } => {},
+                       MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => {
+                               assert_eq!(node_id, nodes[1].node.get_our_node_id());
+                               assert_eq!(msg.data, "Commitment or closing transaction was confirmed on chain.");
+                       },
                        MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                                assert!(update_add_htlcs.is_empty());
                                assert!(update_fail_htlcs.is_empty());
@@ -1582,7 +1584,7 @@ fn test_fee_spike_violation_fails_htlc() {
        let secp_ctx = Secp256k1::new();
        let session_priv = SecretKey::from_slice(&[42; 32]).expect("RNG is bad!");
 
-       let cur_height = nodes[1].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+       let cur_height = nodes[1].node.best_block.read().unwrap().height() + 1;
 
        let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
        let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3460001, &None, cur_height).unwrap();
@@ -1753,7 +1755,7 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
        // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
        let secp_ctx = Secp256k1::new();
        let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
-       let cur_height = nodes[1].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+       let cur_height = nodes[1].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
        let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 1000, &None, cur_height).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
@@ -1876,7 +1878,7 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() {
        // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
        let secp_ctx = Secp256k1::new();
        let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
-       let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+       let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route_2.paths[0], &session_priv).unwrap();
        let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route_2.paths[0], recv_value_2, &None, cur_height).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash_1);
@@ -2334,6 +2336,7 @@ fn channel_monitor_network_test() {
        // Simple case with no pending HTLCs:
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), true);
        check_added_monitors!(nodes[1], 1);
+       check_closed_broadcast!(nodes[1], false);
        {
                let mut node_txn = test_txn_broadcast(&nodes[1], &chan_1, None, HTLCType::NONE);
                assert_eq!(node_txn.len(), 1);
@@ -2341,7 +2344,7 @@ fn channel_monitor_network_test() {
                check_added_monitors!(nodes[0], 1);
                test_txn_broadcast(&nodes[0], &chan_1, None, HTLCType::NONE);
        }
-       get_announce_close_broadcast_events(&nodes, 0, 1);
+       check_closed_broadcast!(nodes[0], true);
        assert_eq!(nodes[0].node.list_channels().len(), 0);
        assert_eq!(nodes[1].node.list_channels().len(), 1);
 
@@ -2350,6 +2353,7 @@ fn channel_monitor_network_test() {
 
        // Simple case of one pending HTLC to HTLC-Timeout
        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);
@@ -2357,7 +2361,7 @@ fn channel_monitor_network_test() {
                check_added_monitors!(nodes[2], 1);
                test_txn_broadcast(&nodes[2], &chan_2, None, HTLCType::NONE);
        }
-       get_announce_close_broadcast_events(&nodes, 1, 2);
+       check_closed_broadcast!(nodes[2], true);
        assert_eq!(nodes[1].node.list_channels().len(), 0);
        assert_eq!(nodes[2].node.list_channels().len(), 1);
 
@@ -2385,6 +2389,7 @@ fn channel_monitor_network_test() {
        // HTLC-Timeout and a nodes[3] claim against it (+ its own announces)
        nodes[2].node.peer_disconnected(&nodes[3].node.get_our_node_id(), true);
        check_added_monitors!(nodes[2], 1);
+       check_closed_broadcast!(nodes[2], false);
        let node2_commitment_txid;
        {
                let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::TIMEOUT);
@@ -2396,7 +2401,7 @@ fn channel_monitor_network_test() {
                check_added_monitors!(nodes[3], 1);
                check_preimage_claim(&nodes[3], &node_txn);
        }
-       get_announce_close_broadcast_events(&nodes, 2, 3);
+       check_closed_broadcast!(nodes[3], true);
        assert_eq!(nodes[2].node.list_channels().len(), 0);
        assert_eq!(nodes[3].node.list_channels().len(), 1);
 
@@ -2412,13 +2417,19 @@ fn channel_monitor_network_test() {
        let (close_chan_update_1, close_chan_update_2) = {
                connect_blocks(&nodes[3], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
                let events = nodes[3].node.get_and_clear_pending_msg_events();
-               assert_eq!(events.len(), 1);
+               assert_eq!(events.len(), 2);
                let close_chan_update_1 = match events[0] {
                        MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
                                msg.clone()
                        },
                        _ => panic!("Unexpected event"),
                };
+               match events[1] {
+                       MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => {
+                               assert_eq!(node_id, nodes[4].node.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
                check_added_monitors!(nodes[3], 1);
 
                // Clear bumped claiming txn spending node 2 commitment tx. Bumped txn are generated after reaching some height timer.
@@ -2438,13 +2449,19 @@ fn channel_monitor_network_test() {
 
                connect_blocks(&nodes[4], TEST_FINAL_CLTV - CLTV_CLAIM_BUFFER + 2);
                let events = nodes[4].node.get_and_clear_pending_msg_events();
-               assert_eq!(events.len(), 1);
+               assert_eq!(events.len(), 2);
                let close_chan_update_2 = match events[0] {
                        MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
                                msg.clone()
                        },
                        _ => panic!("Unexpected event"),
                };
+               match events[1] {
+                       MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => {
+                               assert_eq!(node_id, nodes[3].node.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
                check_added_monitors!(nodes[4], 1);
                test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
 
@@ -2785,7 +2802,7 @@ fn test_htlc_on_chain_success() {
        assert_eq!(updates.update_fulfill_htlcs.len(), 1);
 
        mine_transaction(&nodes[2], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
        let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx, 2*htlc-success tx), ChannelMonitor : 2 (2 * HTLC-Success tx)
        assert_eq!(node_txn.len(), 5);
@@ -2818,12 +2835,17 @@ fn test_htlc_on_chain_success() {
                assert_eq!(added_monitors[1].0.txid, chan_1.3.txid());
                added_monitors.clear();
        }
-       assert_eq!(events.len(), 2);
+       assert_eq!(events.len(), 3);
        match events[0] {
                MessageSendEvent::BroadcastChannelUpdate { .. } => {},
                _ => panic!("Unexpected event"),
        }
        match events[1] {
+               MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
+               _ => panic!("Unexpected event"),
+       }
+
+       match events[2] {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                        assert!(update_add_htlcs.is_empty());
                        assert!(update_fail_htlcs.is_empty());
@@ -2877,7 +2899,7 @@ fn test_htlc_on_chain_success() {
        let commitment_tx = get_local_commitment_txn!(nodes[0], chan_1.2);
        check_spends!(commitment_tx[0], chan_1.3);
        mine_transaction(&nodes[1], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx + HTLC-Sucess * 2), ChannelMonitor : 1 (HTLC-Success)
        assert_eq!(node_txn.len(), 4);
@@ -2897,7 +2919,7 @@ fn test_htlc_on_chain_success() {
        // Verify that A's ChannelManager is able to extract preimage from preimage tx and generate PaymentSent
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
        connect_block(&nodes[0], &Block { header, txdata: vec![commitment_tx[0].clone(), node_txn[0].clone()] });
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
@@ -2918,8 +2940,7 @@ fn test_htlc_on_chain_success() {
        check_tx_local_broadcast!(nodes[0], true, commitment_tx[0], chan_1.3);
 }
 
-#[test]
-fn test_htlc_on_chain_timeout() {
+fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) {
        // Test that in case of a unilateral close onchain, we detect the state of output and
        // timeout the HTLC backward accordingly. So here we test that ChannelManager is
        // broadcasting the right event to other nodes in payment path.
@@ -2931,7 +2952,10 @@ fn test_htlc_on_chain_timeout() {
        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 nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+       *nodes[0].connect_style.borrow_mut() = connect_style;
+       *nodes[1].connect_style.borrow_mut() = connect_style;
+       *nodes[2].connect_style.borrow_mut() = connect_style;
 
        // Create some intial channels
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
@@ -2964,7 +2988,7 @@ fn test_htlc_on_chain_timeout() {
                _ => panic!("Unexpected event"),
        };
        mine_transaction(&nodes[2], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
        let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 1 (commitment tx)
        assert_eq!(node_txn.len(), 1);
@@ -2996,7 +3020,7 @@ fn test_htlc_on_chain_timeout() {
 
        mine_transaction(&nodes[1], &timeout_tx);
        check_added_monitors!(nodes[1], 1);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        {
                // B will rebroadcast a fee-bumped timeout transaction here.
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
@@ -3033,7 +3057,7 @@ fn test_htlc_on_chain_timeout() {
 
        mine_transaction(&nodes[0], &commitment_tx[0]);
 
-       check_closed_broadcast!(nodes[0], false);
+       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);
@@ -3045,6 +3069,13 @@ fn test_htlc_on_chain_timeout() {
        assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
 }
 
+#[test]
+fn test_htlc_on_chain_timeout() {
+       do_test_htlc_on_chain_timeout(ConnectStyle::BestBlockFirstSkippingBlocks);
+       do_test_htlc_on_chain_timeout(ConnectStyle::TransactionsFirstSkippingBlocks);
+       do_test_htlc_on_chain_timeout(ConnectStyle::FullBlockViaListen);
+}
+
 #[test]
 fn test_simple_commitment_revoked_fail_backward() {
        // Test that in case of a revoked commitment tx, we detect the resolution of output by justice tx
@@ -3070,7 +3101,7 @@ fn test_simple_commitment_revoked_fail_backward() {
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
        check_added_monitors!(nodes[1], 1);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
 
        expect_pending_htlcs_forwardable!(nodes[1]);
        check_added_monitors!(nodes[1], 1);
@@ -3241,11 +3272,18 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
        check_added_monitors!(nodes[1], 1);
 
        let events = nodes[1].node.get_and_clear_pending_msg_events();
-       assert_eq!(events.len(), if deliver_bs_raa { 3 } else { 2 });
+       assert_eq!(events.len(), if deliver_bs_raa { 4 } else { 3 });
        match events[if deliver_bs_raa { 1 } else { 0 }] {
                MessageSendEvent::BroadcastChannelUpdate { msg: msgs::ChannelUpdate { .. } } => {},
                _ => panic!("Unexpected event"),
        }
+       match events[if deliver_bs_raa { 2 } else { 1 }] {
+               MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => {
+                       assert_eq!(channel_id, chan_2.2);
+                       assert_eq!(data.as_str(), "Commitment or closing transaction was confirmed on chain.");
+               },
+               _ => panic!("Unexpected event"),
+       }
        if deliver_bs_raa {
                match events[0] {
                        MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, .. } } => {
@@ -3258,7 +3296,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                        _ => panic!("Unexpected event"),
                }
        }
-       match events[if deliver_bs_raa { 2 } else { 1 }] {
+       match events[if deliver_bs_raa { 3 } else { 2 }] {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed, .. } } => {
                        assert!(update_add_htlcs.is_empty());
                        assert_eq!(update_fail_htlcs.len(), 3);
@@ -3370,7 +3408,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
 
                let secp_ctx = Secp256k1::new();
                let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
-               let current_height = nodes[1].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+               let current_height = nodes[1].node.best_block.read().unwrap().height() + 1;
                let net_graph_msg_handler = &nodes[1].net_graph_msg_handler;
                let route = get_route(&nodes[1].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[0].node.get_our_node_id(), None, None, &Vec::new(), 50_000, TEST_FINAL_CLTV, &logger).unwrap();
                let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(&route.paths[0], 50_000, &None, current_height).unwrap();
@@ -3407,7 +3445,7 @@ 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();
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -3415,7 +3453,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
 
        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()]});
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        // Duplicate the connect_block call since this may happen due to other listeners
@@ -3469,7 +3507,7 @@ fn test_force_close_fail_back() {
        // transaction and ensure nodes[1] doesn't fail-backwards (this was originally a bug!).
 
        nodes[2].node.force_close_channel(&payment_event.commitment_msg.channel_id).unwrap();
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
        let tx = {
                let mut node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -3483,7 +3521,7 @@ fn test_force_close_fail_back() {
        mine_transaction(&nodes[1], &tx);
 
        // Note no UpdateHTLCs event here from nodes[1] to nodes[0]!
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        // Now check that if we add the preimage to ChannelMonitor it broadcasts our HTLC-Success..
@@ -4320,7 +4358,7 @@ fn test_manager_serialize_deserialize_events() {
        let nodes_0_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
-       // Start creating a channel, but stop right before broadcasting the event message FundingBroadcastSafe
+       // Start creating a channel, but stop right before broadcasting the funding transaction
        let channel_value = 100000;
        let push_msat = 10001;
        let a_flags = InitFeatures::known();
@@ -4333,7 +4371,7 @@ fn test_manager_serialize_deserialize_events() {
 
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&node_a, channel_value, 42);
 
-       node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(node_a, 0);
 
        node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id()));
@@ -4351,7 +4389,7 @@ fn test_manager_serialize_deserialize_events() {
                assert_eq!(added_monitors[0].0, funding_output);
                added_monitors.clear();
        }
-       // Normally, this is where node_a would check for a FundingBroadcastSafe event, but the test de/serializes first instead
+       // Normally, this is where node_a would broadcast the funding transaction, but the test de/serializes first instead
 
        nodes.push(node_a);
        nodes.push(node_b);
@@ -4395,16 +4433,11 @@ fn test_manager_serialize_deserialize_events() {
        assert!(nodes[0].chain_monitor.watch_channel(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok());
        nodes[0].node = &nodes_0_deserialized;
 
-       // After deserializing, make sure the FundingBroadcastSafe event is still held by the channel manager
+       // After deserializing, make sure the funding_transaction is still held by the channel manager
        let events_4 = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].txid(), funding_output.txid);
 
        // Make sure the channel is functioning as though the de/serialization never happened
        assert_eq!(nodes[0].node.list_channels().len(), 1);
@@ -4658,7 +4691,7 @@ fn test_claim_sizeable_push_msat() {
 
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 99000000, InitFeatures::known(), InitFeatures::known());
        nodes[1].node.force_close_channel(&chan.2).unwrap();
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
        assert_eq!(node_txn.len(), 1);
@@ -4684,7 +4717,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
 
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 99000000, InitFeatures::known(), InitFeatures::known());
        nodes[0].node.force_close_channel(&chan.2).unwrap();
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4693,7 +4726,7 @@ fn test_claim_on_remote_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]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
@@ -4720,7 +4753,7 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
 
        claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4846,7 +4879,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
        claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
 
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4882,7 +4915,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
 
        // A will generate HTLC-Timeout from revoked commitment tx
        mine_transaction(&nodes[0], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4895,7 +4928,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
        // B will generate justice tx from A's revoked commitment/HTLC tx
        let 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![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()] });
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4951,7 +4984,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
 
        // B will generate HTLC-Success from revoked commitment tx
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 
@@ -4967,7 +5000,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
        // A will generate justice tx from B's revoked commitment/HTLC tx
        let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[0], &Block { header, txdata: vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()] });
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -5041,7 +5074,7 @@ fn test_onchain_to_onchain_claim() {
        assert!(updates.update_fail_malformed_htlcs.is_empty());
 
        mine_transaction(&nodes[2], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
 
        let c_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Success tx), ChannelMonitor : 1 (HTLC-Success tx)
@@ -5075,12 +5108,17 @@ fn test_onchain_to_onchain_claim() {
        }
        check_added_monitors!(nodes[1], 1);
        let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(msg_events.len(), 3);
        check_added_monitors!(nodes[1], 1);
        match msg_events[0] {
-               MessageSendEvent::BroadcastChannelUpdate {  .. } => {},
+               MessageSendEvent::BroadcastChannelUpdate { .. } => {},
                _ => panic!("Unexpected event"),
        }
        match msg_events[1] {
+               MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
+               _ => panic!("Unexpected event"),
+       }
+       match msg_events[2] {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                        assert!(update_add_htlcs.is_empty());
                        assert!(update_fail_htlcs.is_empty());
@@ -5103,7 +5141,7 @@ fn test_onchain_to_onchain_claim() {
        assert!(b_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
        assert_eq!(b_txn[0].lock_time, 0); // Success tx
 
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 }
 
@@ -5128,7 +5166,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
        check_spends!(commitment_txn[0], chan_2.3);
 
        mine_transaction(&nodes[1], &commitment_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let htlc_timeout_tx;
@@ -5408,7 +5446,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
                mine_transaction(&nodes[2], &ds_prev_commitment_tx[0]);
        }
        connect_blocks(&nodes[2], ANTI_REORG_DELAY - 1);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        expect_pending_htlcs_forwardable!(nodes[2]);
        check_added_monitors!(nodes[2], 3);
 
@@ -5545,7 +5583,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 
        // Timeout HTLC on A's chain and so it can generate a HTLC-Timeout tx
        mine_transaction(&nodes[0], &local_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let htlc_timeout = {
@@ -5613,7 +5651,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]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let htlc_timeout = {
@@ -5705,7 +5743,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
                block.header.prev_blockhash = block.block_hash();
        }
        test_txn_broadcast(&nodes[1], &chan, None, if use_dust { HTLCType::NONE } else { HTLCType::SUCCESS });
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 }
 
@@ -5737,7 +5775,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
                header.prev_blockhash = header.block_hash();
        }
        test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 }
 
@@ -5785,7 +5823,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
        }
        if !check_revoke_no_close {
                test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
        } else {
                expect_payment_failed!(nodes[0], our_payment_hash, true);
@@ -6474,7 +6512,7 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
        let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
        let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), None, None, &[], 3999999, TEST_FINAL_CLTV, &logger).unwrap();
 
-       let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+       let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::signing_only(), &route.paths[0], &session_priv).unwrap();
        let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3999999, &None, cur_height).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash);
@@ -6967,7 +7005,7 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
                mine_transaction(&nodes[0], &as_prev_commitment_tx[0]);
        }
 
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
@@ -7029,7 +7067,7 @@ 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]);
-               check_closed_broadcast!(nodes[0], false);
+               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()[0].clone());
@@ -7044,7 +7082,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
        } else {
                // We fail dust-HTLC 1 by broadcast of remote commitment tx. If revoked, fail also non-dust HTLC
                mine_transaction(&nodes[0], &bs_commitment_tx[0]);
-               check_closed_broadcast!(nodes[0], false);
+               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()[0].clone());
@@ -7527,7 +7565,7 @@ fn test_check_htlc_underpaying() {
 
 #[test]
 fn test_announce_disable_channels() {
-       // Create 2 channels between A and B. Disconnect B. Call timer_chan_freshness_every_min and check for generated
+       // Create 2 channels between A and B. Disconnect B. Call timer_tick_occurred and check for generated
        // ChannelUpdate. Reconnect B, reestablish and check there is non-generated ChannelUpdate.
 
        let chanmon_cfgs = create_chanmon_cfgs(2);
@@ -7543,8 +7581,8 @@ fn test_announce_disable_channels() {
        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);
 
-       nodes[0].node.timer_chan_freshness_every_min(); // dirty -> stagged
-       nodes[0].node.timer_chan_freshness_every_min(); // staged -> fresh
+       nodes[0].node.timer_tick_occurred(); // dirty -> stagged
+       nodes[0].node.timer_tick_occurred(); // staged -> fresh
        let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(msg_events.len(), 3);
        for e in msg_events {
@@ -7583,7 +7621,7 @@ fn test_announce_disable_channels() {
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[2]);
        handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
-       nodes[0].node.timer_chan_freshness_every_min();
+       nodes[0].node.timer_tick_occurred();
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 }
 
@@ -7719,7 +7757,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        // B will generate both revoked HTLC-timeout/HTLC-preimage txn from revoked commitment tx
        connect_block(&nodes[1], &Block { header, txdata: vec![revoked_local_txn[0].clone()] });
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -7851,7 +7889,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                assert_eq!(node_txn.len(), 0);
                node_txn.clear();
        }
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 }
 
@@ -8025,7 +8063,7 @@ fn test_bump_txn_sanitize_tracking_maps() {
        assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 0);
 
        mine_transaction(&nodes[0], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        let penalty_txn = {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -8347,9 +8385,9 @@ fn test_pre_lockin_no_chan_closed_update() {
        nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_chan_msg);
 
        // Move the first channel through the funding flow...
-       let (temporary_channel_id, _tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
+       let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], 100000, 42);
 
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
 
        let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -8388,7 +8426,7 @@ fn test_htlc_no_detection() {
        // We deliberately connect the local tx twice as this should provoke a failure calling
        // this test before #653 fix.
        chain::Listen::block_connected(&nodes[0].chain_monitor.chain_monitor, &Block { header, txdata: vec![local_txn[0].clone()] }, nodes[0].best_block_info().1 + 1);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let htlc_timeout = {
@@ -8446,7 +8484,7 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain
        let mut force_closing_node = 0; // Alice force-closes
        if !broadcast_alice { force_closing_node = 1; } // Bob force-closes
        nodes[force_closing_node].node.force_close_channel(&chan_ab.2).unwrap();
-       check_closed_broadcast!(nodes[force_closing_node], false);
+       check_closed_broadcast!(nodes[force_closing_node], true);
        check_added_monitors!(nodes[force_closing_node], 1);
        if go_onchain_before_fulfill {
                let txn_to_broadcast = match broadcast_alice {
@@ -8457,7 +8495,7 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain
                connect_block(&nodes[1], &Block { header, txdata: vec![txn_to_broadcast[0].clone()]});
                let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                if broadcast_alice {
-                       check_closed_broadcast!(nodes[1], false);
+                       check_closed_broadcast!(nodes[1], true);
                        check_added_monitors!(nodes[1], 1);
                }
                assert_eq!(bob_txn.len(), 1);
@@ -8536,7 +8574,7 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain
                connect_block(&nodes[1], &Block { header, txdata: vec![txn_to_broadcast[0].clone()]});
                // If Bob was the one to force-close, he will have already passed these checks earlier.
                if broadcast_alice {
-                       check_closed_broadcast!(nodes[1], false);
+                       check_closed_broadcast!(nodes[1], true);
                        check_added_monitors!(nodes[1], 1);
                }
                let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -8631,7 +8669,7 @@ fn test_duplicate_chan_id() {
        // Move the first channel through the funding flow...
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
 
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
 
        let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -8680,7 +8718,7 @@ fn test_duplicate_chan_id() {
                let mut a_channel_lock = nodes[0].node.channel_state.lock().unwrap();
                let mut as_chan = a_channel_lock.by_id.get_mut(&open_chan_2_msg.temporary_channel_id).unwrap();
                let logger = test_utils::TestLogger::new();
-               as_chan.get_outbound_funding_created(funding_outpoint, &&logger).unwrap()
+               as_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap()
        };
        check_added_monitors!(nodes[0], 0);
        nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created);
@@ -8714,14 +8752,9 @@ fn test_duplicate_chan_id() {
        }
 
        let events_4 = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].txid(), funding_output.txid);
 
        let (funding_locked, _) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx);
        let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked);
@@ -8758,6 +8791,7 @@ fn test_error_chans_closed() {
        nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &msgs::ErrorMessage { channel_id: chan_2.2, data: "ERR".to_owned() });
        check_added_monitors!(nodes[0], 1);
        check_closed_broadcast!(nodes[0], false);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0).len(), 1);
        assert_eq!(nodes[0].node.list_usable_channels().len(), 2);
        assert!(nodes[0].node.list_usable_channels()[0].channel_id == chan_1.2 || nodes[0].node.list_usable_channels()[1].channel_id == chan_1.2);
        assert!(nodes[0].node.list_usable_channels()[0].channel_id == chan_3.2 || nodes[0].node.list_usable_channels()[1].channel_id == chan_3.2);