Track the blocks a node has connected in the TestBroadcaster

Author: TheBlueMatt

lightning-background-processor/src/lib.rs

@@ -238,7 +238,7 @@ mod tests {
 	fn create_nodes(num_nodes: usize, persist_dir: String) -> Vec<Node> {
 		let mut nodes = Vec::new();
 		for i in 0..num_nodes {
-			let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
+			let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
 			let fee_estimator = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
 			let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
 			let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i)));

lightning/src/chain/channelmonitor.rs

@@ -2918,7 +2918,7 @@ mod tests {
 	fn test_prune_preimages() {
 		let secp_ctx = Secp256k1::new();
 		let logger = Arc::new(TestLogger::new());
-		let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
+		let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
 		let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: 253 });
 
 		let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -13,6 +13,7 @@
 //! here. See also the chanmon_fail_consistency fuzz test.
 
 use bitcoin::blockdata::block::{Block, BlockHeader};
+use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::hash_types::BlockHash;
 use bitcoin::network::constants::Network;
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr};
@@ -30,6 +31,7 @@ use util::enforcing_trait_impls::EnforcingSigner;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
 use util::errors::APIError;
 use util::ser::{ReadableArgs, Writeable};
+use util::test_utils::TestBroadcaster;
 
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::Hash;
@@ -39,6 +41,7 @@ use ln::functional_test_utils::*;
 use util::test_utils;
 
 use std::collections::HashMap;
+use std::sync::{Arc, Mutex};
 
 // If persister_fail is true, we have the persister return a PermanentFailure
 // instead of the higher-level ChainMonitor.
@@ -106,6 +109,13 @@ fn test_monitor_and_persister_update_fail() {
 	let chain_source = test_utils::TestChainSource::new(Network::Testnet);
 	let logger = test_utils::TestLogger::with_id(format!("node {}", 0));
 	let persister = test_utils::TestPersister::new();
+	let tx_broadcaster = TestBroadcaster {
+		txn_broadcasted: Mutex::new(Vec::new()),
+		// Because we will connect a block at height 200 below, we need the TestBroadcaster to know
+		// that we are at height 200 so that it doesn't think we're violating the time lock
+		// requirements of transactions broadcasted at that point.
+		blocks: Arc::new(Mutex::new(vec![(genesis_block(Network::Testnet).header, 0); 200])),
+	};
 	let chain_mon = {
 		let monitors = nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap();
 		let monitor = monitors.get(&outpoint).unwrap();
@@ -114,7 +124,7 @@ fn test_monitor_and_persister_update_fail() {
 		let new_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
 			&mut ::std::io::Cursor::new(&w.0), &test_utils::OnlyReadsKeysInterface {}).unwrap().1;
 		assert!(new_monitor == *monitor);
-		let chain_mon = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
+		let chain_mon = test_utils::TestChainMonitor::new(Some(&chain_source), &tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
 		assert!(chain_mon.watch_channel(outpoint, new_monitor).is_ok());
 		chain_mon
 	};

lightning/src/ln/functional_test_utils.rs

@@ -41,7 +41,7 @@ use bitcoin::secp256k1::key::PublicKey;
 
 use core::cell::RefCell;
 use std::rc::Rc;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 use core::mem;
 use std::collections::HashMap;
 
@@ -148,14 +148,14 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, s
 		}
 	}
 	node.node.test_process_background_events();
-	node.blocks.borrow_mut().push((block.header, height));
+	node.blocks.lock().unwrap().push((block.header, height));
 }
 
 pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) {
 	for i in 0..count {
-		let orig_header = node.blocks.borrow_mut().pop().unwrap();
+		let orig_header = node.blocks.lock().unwrap().pop().unwrap();
 		assert!(orig_header.1 > 0); // Cannot disconnect genesis
-		let prev_header = node.blocks.borrow().last().unwrap().clone();
+		let prev_header = node.blocks.lock().unwrap().last().unwrap().clone();
 
 		match *node.connect_style.borrow() {
 			ConnectStyle::FullBlockViaListen => {
@@ -177,7 +177,7 @@ pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32)
 }
 
 pub fn disconnect_all_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>) {
-	let count = node.blocks.borrow_mut().len() as u32 - 1;
+	let count = node.blocks.lock().unwrap().len() as u32 - 1;
 	disconnect_blocks(node, count);
 }
 
@@ -211,15 +211,15 @@ pub struct Node<'a, 'b: 'a, 'c: 'b> {
 	pub network_payment_count: Rc<RefCell<u8>>,
 	pub network_chan_count: Rc<RefCell<u32>>,
 	pub logger: &'c test_utils::TestLogger,
-	pub blocks: RefCell<Vec<(BlockHeader, u32)>>,
+	pub blocks: Arc<Mutex<Vec<(BlockHeader, u32)>>>,
 	pub connect_style: Rc<RefCell<ConnectStyle>>,
 }
 impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
 	pub fn best_block_hash(&self) -> BlockHash {
-		self.blocks.borrow_mut().last().unwrap().0.block_hash()
+		self.blocks.lock().unwrap().last().unwrap().0.block_hash()
 	}
 	pub fn best_block_info(&self) -> (BlockHash, u32) {
-		self.blocks.borrow_mut().last().map(|(a, b)| (a.block_hash(), *b)).unwrap()
+		self.blocks.lock().unwrap().last().map(|(a, b)| (a.block_hash(), *b)).unwrap()
 	}
 }
 
@@ -295,7 +295,8 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 					fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: 253 },
 					chain_monitor: self.chain_monitor,
 					tx_broadcaster: &test_utils::TestBroadcaster {
-						txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone())
+						txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone()),
+						blocks: Arc::new(Mutex::new(self.tx_broadcaster.blocks.lock().unwrap().clone())),
 					},
 					logger: &test_utils::TestLogger::new(),
 					channel_monitors,
@@ -304,7 +305,8 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 
 			let persister = test_utils::TestPersister::new();
 			let broadcaster = test_utils::TestBroadcaster {
-				txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone())
+				txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone()),
+				blocks: Arc::new(Mutex::new(self.tx_broadcaster.blocks.lock().unwrap().clone())),
 			};
 			let chain_source = test_utils::TestChainSource::new(Network::Testnet);
 			let chain_monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &broadcaster, &self.logger, &feeest, &persister, &self.keys_manager);
@@ -1283,7 +1285,10 @@ pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
 	let mut chan_mon_cfgs = Vec::new();
 	for i in 0..node_count {
-		let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())};
+		let tx_broadcaster = test_utils::TestBroadcaster {
+			txn_broadcasted: Mutex::new(Vec::new()),
+			blocks: Arc::new(Mutex::new(vec![(genesis_block(Network::Testnet).header, 0)])),
+		};
 		let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
 		let chain_source = test_utils::TestChainSource::new(Network::Testnet);
 		let logger = test_utils::TestLogger::with_id(format!("node {}", i));
@@ -1344,7 +1349,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
 		                 keys_manager: &cfgs[i].keys_manager, node: &chan_mgrs[i], net_graph_msg_handler,
 		                 node_seed: cfgs[i].node_seed, network_chan_count: chan_count.clone(),
 		                 network_payment_count: payment_count.clone(), logger: cfgs[i].logger,
-		                 blocks: RefCell::new(vec![(genesis_block(Network::Testnet).header, 0)]),
+		                 blocks: Arc::clone(&cfgs[i].tx_broadcaster.blocks),
 		                 connect_style: Rc::clone(&connect_style),
 		})
 	}

lightning/src/ln/functional_tests.rs

@@ -52,7 +52,7 @@ use regex;
 
 use std::collections::{BTreeSet, HashMap, HashSet};
 use core::default::Default;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 
 use ln::functional_test_utils::*;
 use ln::chan_utils::CommitmentTransaction;
@@ -2748,7 +2748,8 @@ fn test_htlc_on_chain_success() {
 	let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
 	// Ensure all nodes are at the same height
-	let node_max_height = std::cmp::max(nodes[0].blocks.borrow().len(), std::cmp::max(nodes[1].blocks.borrow().len(), nodes[2].blocks.borrow().len())) as u32;
+	let node_max_height = std::cmp::max(nodes[0].blocks.lock().unwrap().len(),
+		std::cmp::max(nodes[1].blocks.lock().unwrap().len(), nodes[2].blocks.lock().unwrap().len())) as u32;
 	connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
 	connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
 	connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
@@ -4521,7 +4522,7 @@ fn test_dup_htlc_onchain_fails_on_reload() {
 	// Note that if we re-connect the block which exposed nodes[0] to the payment preimage (but
 	// which the current ChannelMonitor has not seen), the ChannelManager's de-duplication of
 	// payment events should kick in, leaving us with no pending events here.
-	nodes[0].chain_monitor.chain_monitor.block_connected(&claim_block, nodes[0].blocks.borrow().len() as u32 - 1);
+	nodes[0].chain_monitor.chain_monitor.block_connected(&claim_block, nodes[0].blocks.lock().unwrap().len() as u32 - 1);
 	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
 }
 
@@ -5242,7 +5243,8 @@ fn test_onchain_to_onchain_claim() {
 	let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
 	// Ensure all nodes are at the same height
-	let node_max_height = std::cmp::max(nodes[0].blocks.borrow().len(), std::cmp::max(nodes[1].blocks.borrow().len(), nodes[2].blocks.borrow().len())) as u32;
+	let node_max_height = std::cmp::max(nodes[0].blocks.lock().unwrap().len(),
+		std::cmp::max(nodes[1].blocks.lock().unwrap().len(), nodes[2].blocks.lock().unwrap().len())) as u32;
 	connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
 	connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
 	connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
@@ -5355,7 +5357,9 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 	let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 	create_announced_chan_between_nodes(&nodes, 2, 3, InitFeatures::known(), InitFeatures::known());
 
-	let node_max_height = std::cmp::max(std::cmp::max(nodes[0].blocks.borrow().len(), nodes[1].blocks.borrow().len()), std::cmp::max(nodes[2].blocks.borrow().len(), nodes[3].blocks.borrow().len())) as u32;
+	let node_max_height = std::cmp::max(
+		std::cmp::max(nodes[0].blocks.lock().unwrap().len(), nodes[1].blocks.lock().unwrap().len()),
+		std::cmp::max(nodes[2].blocks.lock().unwrap().len(), nodes[3].blocks.lock().unwrap().len())) as u32;
 	connect_blocks(&nodes[0], node_max_height - nodes[0].best_block_info().1);
 	connect_blocks(&nodes[1], node_max_height - nodes[1].best_block_info().1);
 	connect_blocks(&nodes[2], node_max_height - nodes[2].best_block_info().1);
@@ -7687,7 +7691,7 @@ fn test_data_loss_protect() {
 	logger = test_utils::TestLogger::with_id(format!("node {}", 0));
 	let mut chain_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut ::std::io::Cursor::new(previous_chain_monitor_state.0), keys_manager).unwrap().1;
 	chain_source = test_utils::TestChainSource::new(Network::Testnet);
-	tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())};
+	tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
 	fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
 	persister = test_utils::TestPersister::new();
 	monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &tx_broadcaster, &logger, &fee_estimator, &persister, keys_manager);
@@ -8472,14 +8476,18 @@ fn test_secret_timeout() {
 	if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
 		assert_eq!(err, "Duplicate payment hash");
 	} else { panic!(); }
-	let mut block = Block {
-		header: BlockHeader {
-			version: 0x2000000,
-			prev_blockhash: nodes[1].blocks.borrow().last().unwrap().0.block_hash(),
-			merkle_root: Default::default(),
-			time: nodes[1].blocks.borrow().len() as u32 + 7200, bits: 42, nonce: 42 },
-		txdata: vec![],
-	};
+	let mut block;
+	{
+		let node_1_blocks = nodes[1].blocks.lock().unwrap();
+		block = Block {
+			header: BlockHeader {
+				version: 0x2000000,
+				prev_blockhash: node_1_blocks.last().unwrap().0.block_hash(),
+				merkle_root: Default::default(),
+				time: node_1_blocks.len() as u32 + 7200, bits: 42, nonce: 42 },
+			txdata: vec![],
+		};
+	}
 	connect_block(&nodes[1], &block);
 	if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
 		assert_eq!(err, "Duplicate payment hash");
@@ -8628,6 +8636,9 @@ fn test_update_err_monitor_lockdown() {
 		watchtower
 	};
 	let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+	// Make the tx_broadcaster aware of enough blocks that it doesn't think we're violating
+	// transaction lock time requirements here.
+	chanmon_cfgs[0].tx_broadcaster.blocks.lock().unwrap().resize(200, (header, 0));
 	watchtower.chain_monitor.block_connected(&Block { header, txdata: vec![] }, 200);
 
 	// Try to update ChannelMonitor
@@ -8687,6 +8698,9 @@ fn test_concurrent_monitor_claim() {
 		watchtower
 	};
 	let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+	// Make the tx_broadcaster aware of enough blocks that it doesn't think we're violating
+	// transaction lock time requirements here.
+	chanmon_cfgs[0].tx_broadcaster.blocks.lock().unwrap().resize((CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS) as usize, (header, 0));
 	watchtower_alice.chain_monitor.block_connected(&Block { header, txdata: vec![] }, CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
 	// Watchtower Alice should have broadcast a commitment/HTLC-timeout

lightning/src/util/test_utils.rs

@@ -28,6 +28,7 @@ use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::blockdata::transaction::{Transaction, TxOut};
 use bitcoin::blockdata::script::{Builder, Script};
 use bitcoin::blockdata::opcodes;
+use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::network::constants::Network;
 use bitcoin::hash_types::{BlockHash, Txid};
 
@@ -200,6 +201,7 @@ impl<Signer: keysinterface::Sign> channelmonitor::Persist<Signer> for TestPersis
 
 pub struct TestBroadcaster {
 	pub txn_broadcasted: Mutex<Vec<Transaction>>,
+	pub blocks: Arc<Mutex<Vec<(BlockHeader, u32)>>>,
 }
 impl chaininterface::BroadcasterInterface for TestBroadcaster {
 	fn broadcast_transaction(&self, tx: &Transaction) {