Test coverage for transaction_unconfirmed

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -103,7 +103,7 @@ fn test_monitor_and_persister_update_fail() {
 		// 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, 200); 200])),
+		blocks: Arc::new(Mutex::new(vec![(genesis_block(Network::Testnet), 200); 200])),
 	};
 	let chain_mon = {
 		let monitor = nodes[0].chain_monitor.chain_monitor.get_monitor(outpoint).unwrap();

lightning/src/ln/functional_test_utils.rs

@@ -77,28 +77,60 @@ pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &T
 }
 
 /// The possible ways we may notify a ChannelManager of a new block
-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub enum ConnectStyle {
-	/// Calls best_block_updated first, detecting transactions in the block only after receiving the
-	/// header and height information.
+	/// Calls `best_block_updated` first, detecting transactions in the block only after receiving
+	/// the header and height information.
 	BestBlockFirst,
-	/// The same as BestBlockFirst, however when we have multiple blocks to connect, we only
-	/// make a single best_block_updated call.
+	/// The same as `BestBlockFirst`, however when we have multiple blocks to connect, we only
+	/// make a single `best_block_updated` call.
 	BestBlockFirstSkippingBlocks,
-	/// Calls transactions_confirmed first, detecting transactions in the block before updating the
-	/// header and height information.
+	/// The same as `BestBlockFirst` when connecting blocks. During disconnection only
+	/// `transaction_unconfirmed` is called.
+	BestBlockFirstReorgsOnlyTip,
+	/// Calls `transactions_confirmed` first, detecting transactions in the block before updating
+	/// the header and height information.
 	TransactionsFirst,
-	/// The same as TransactionsFirst, however when we have multiple blocks to connect, we only
-	/// make a single best_block_updated call.
+	/// The same as `TransactionsFirst`, however when we have multiple blocks to connect, we only
+	/// make a single `best_block_updated` call.
 	TransactionsFirstSkippingBlocks,
-	/// Provides the full block via the chain::Listen interface. In the current code this is
-	/// equivalent to TransactionsFirst with some additional assertions.
+	/// The same as `TransactionsFirst` when connecting blocks. During disconnection only
+	/// `transaction_unconfirmed` is called.
+	TransactionsFirstReorgsOnlyTip,
+	/// Provides the full block via the `chain::Listen` interface. In the current code this is
+	/// equivalent to `TransactionsFirst` with some additional assertions.
 	FullBlockViaListen,
 }
 
+impl ConnectStyle {
+	fn random_style() -> ConnectStyle {
+		#[cfg(feature = "std")] {
+			use core::hash::{BuildHasher, Hasher};
+			// Get a random value using the only std API to do so - the DefaultHasher
+			let rand_val = std::collections::hash_map::RandomState::new().build_hasher().finish();
+			let res = match rand_val % 7 {
+				0 => ConnectStyle::BestBlockFirst,
+				1 => ConnectStyle::BestBlockFirstSkippingBlocks,
+				2 => ConnectStyle::BestBlockFirstReorgsOnlyTip,
+				3 => ConnectStyle::TransactionsFirst,
+				4 => ConnectStyle::TransactionsFirstSkippingBlocks,
+				5 => ConnectStyle::TransactionsFirstReorgsOnlyTip,
+				6 => ConnectStyle::FullBlockViaListen,
+				_ => unreachable!(),
+			};
+			eprintln!("Using Block Connection Style: {:?}", res);
+			res
+		}
+		#[cfg(not(feature = "std"))] {
+			ConnectStyle::FullBlockViaListen
+		}
+	}
+}
+
 pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) -> BlockHash {
 	let skip_intermediaries = match *node.connect_style.borrow() {
-		ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks => true,
+		ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks|
+			ConnectStyle::BestBlockFirstReorgsOnlyTip|ConnectStyle::TransactionsFirstReorgsOnlyTip => true,
 		_ => false,
 	};
 
@@ -109,18 +141,20 @@ pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) ->
 	};
 	assert!(depth >= 1);
 	for i in 1..depth {
-		do_connect_block(node, &block, skip_intermediaries);
+		let prev_blockhash = block.header.block_hash();
+		do_connect_block(node, block, skip_intermediaries);
 		block = Block {
-			header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.block_hash(), merkle_root: Default::default(), time: height + i, bits: 42, nonce: 42 },
+			header: BlockHeader { version: 0x20000000, prev_blockhash, merkle_root: Default::default(), time: height + i, bits: 42, nonce: 42 },
 			txdata: vec![],
 		};
 	}
-	connect_block(node, &block);
-	block.header.block_hash()
+	let hash = block.header.block_hash();
+	do_connect_block(node, block, false);
+	hash
 }
 
 pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block) {
-	do_connect_block(node, block, false);
+	do_connect_block(node, block.clone(), false);
 }
 
 fn call_claimable_balances<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>) {
@@ -130,20 +164,23 @@ fn call_claimable_balances<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>) {
 	}
 }
 
-fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, skip_intermediaries: bool) {
+fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, skip_intermediaries: bool) {
 	call_claimable_balances(node);
 	let height = node.best_block_info().1 + 1;
+	#[cfg(feature = "std")] {
+		eprintln!("Connecting block using Block Connection Style: {:?}", *node.connect_style.borrow());
+	}
 	if !skip_intermediaries {
 		let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
 		match *node.connect_style.borrow() {
-			ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstSkippingBlocks => {
+			ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::BestBlockFirstReorgsOnlyTip => {
 				node.chain_monitor.chain_monitor.best_block_updated(&block.header, height);
 				call_claimable_balances(node);
 				node.chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height);
 				node.node.best_block_updated(&block.header, height);
 				node.node.transactions_confirmed(&block.header, &txdata, height);
 			},
-			ConnectStyle::TransactionsFirst|ConnectStyle::TransactionsFirstSkippingBlocks => {
+			ConnectStyle::TransactionsFirst|ConnectStyle::TransactionsFirstSkippingBlocks|ConnectStyle::TransactionsFirstReorgsOnlyTip => {
 				node.chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height);
 				call_claimable_balances(node);
 				node.chain_monitor.chain_monitor.best_block_updated(&block.header, height);
@@ -158,30 +195,39 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, s
 	}
 	call_claimable_balances(node);
 	node.node.test_process_background_events();
-	node.blocks.lock().unwrap().push((block.header, height));
+	node.blocks.lock().unwrap().push((block, height));
 }
 
 pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) {
 	call_claimable_balances(node);
+	#[cfg(feature = "std")] {
+		eprintln!("Disconnecting {} blocks using Block Connection Style: {:?}", count, *node.connect_style.borrow());
+	}
 	for i in 0..count {
-		let orig_header = node.blocks.lock().unwrap().pop().unwrap();
-		assert!(orig_header.1 > 0); // Cannot disconnect genesis
-		let prev_header = node.blocks.lock().unwrap().last().unwrap().clone();
+		let orig = node.blocks.lock().unwrap().pop().unwrap();
+		assert!(orig.1 > 0); // Cannot disconnect genesis
+		let prev = node.blocks.lock().unwrap().last().unwrap().clone();
 
 		match *node.connect_style.borrow() {
 			ConnectStyle::FullBlockViaListen => {
-				node.chain_monitor.chain_monitor.block_disconnected(&orig_header.0, orig_header.1);
-				Listen::block_disconnected(node.node, &orig_header.0, orig_header.1);
+				node.chain_monitor.chain_monitor.block_disconnected(&orig.0.header, orig.1);
+				Listen::block_disconnected(node.node, &orig.0.header, orig.1);
 			},
 			ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks => {
 				if i == count - 1 {
-					node.chain_monitor.chain_monitor.best_block_updated(&prev_header.0, prev_header.1);
-					node.node.best_block_updated(&prev_header.0, prev_header.1);
+					node.chain_monitor.chain_monitor.best_block_updated(&prev.0.header, prev.1);
+					node.node.best_block_updated(&prev.0.header, prev.1);
+				}
+			},
+			ConnectStyle::BestBlockFirstReorgsOnlyTip|ConnectStyle::TransactionsFirstReorgsOnlyTip => {
+				for tx in orig.0.txdata {
+					node.chain_monitor.chain_monitor.transaction_unconfirmed(&tx.txid());
+					node.node.transaction_unconfirmed(&tx.txid());
 				}
 			},
 			_ => {
-				node.chain_monitor.chain_monitor.best_block_updated(&prev_header.0, prev_header.1);
-				node.node.best_block_updated(&prev_header.0, prev_header.1);
+				node.chain_monitor.chain_monitor.best_block_updated(&prev.0.header, prev.1);
+				node.node.best_block_updated(&prev.0.header, prev.1);
 			},
 		}
 		call_claimable_balances(node);
@@ -227,7 +273,7 @@ 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: Arc<Mutex<Vec<(BlockHeader, u32)>>>,
+	pub blocks: Arc<Mutex<Vec<(Block, u32)>>>,
 	pub connect_style: Rc<RefCell<ConnectStyle>>,
 }
 impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
@@ -238,7 +284,7 @@ impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
 		self.blocks.lock().unwrap().last().map(|(a, b)| (a.block_hash(), *b)).unwrap()
 	}
 	pub fn get_block_header(&self, height: u32) -> BlockHeader {
-		self.blocks.lock().unwrap()[height as usize].0
+		self.blocks.lock().unwrap()[height as usize].0.header
 	}
 }
 
@@ -1815,7 +1861,7 @@ pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
 	for i in 0..node_count {
 		let tx_broadcaster = test_utils::TestBroadcaster {
 			txn_broadcasted: Mutex::new(Vec::new()),
-			blocks: Arc::new(Mutex::new(vec![(genesis_block(Network::Testnet).header, 0)])),
+			blocks: Arc::new(Mutex::new(vec![(genesis_block(Network::Testnet), 0)])),
 		};
 		let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 		let chain_source = test_utils::TestChainSource::new(Network::Testnet);
@@ -1889,7 +1935,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
 	let mut nodes = Vec::new();
 	let chan_count = Rc::new(RefCell::new(0));
 	let payment_count = Rc::new(RefCell::new(0));
-	let connect_style = Rc::new(RefCell::new(ConnectStyle::FullBlockViaListen));
+	let connect_style = Rc::new(RefCell::new(ConnectStyle::random_style()));
 
 	for i in 0..node_count {
 		let net_graph_msg_handler = NetGraphMsgHandler::new(cfgs[i].network_graph, None, cfgs[i].logger);

lightning/src/ln/functional_tests.rs

@@ -1280,23 +1280,41 @@ fn test_duplicate_htlc_different_direction_onchain() {
 	check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed);
 	connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
 
-	// Check we only broadcast 1 timeout tx
 	let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
 	assert_eq!(claim_txn.len(), 8);
-	assert_eq!(claim_txn[1], claim_txn[4]);
-	assert_eq!(claim_txn[2], claim_txn[5]);
-	check_spends!(claim_txn[1], chan_1.3);
-	check_spends!(claim_txn[2], claim_txn[1]);
-	check_spends!(claim_txn[7], claim_txn[1]);
+
+	check_spends!(claim_txn[0], remote_txn[0]); // Immediate HTLC claim with preimage
+
+	check_spends!(claim_txn[1], chan_1.3); // Alternative commitment tx
+	check_spends!(claim_txn[2], claim_txn[1]); // HTLC spend in alternative commitment tx
+
+	let bump_tx = if claim_txn[1] == claim_txn[4] {
+		assert_eq!(claim_txn[1], claim_txn[4]);
+		assert_eq!(claim_txn[2], claim_txn[5]);
+
+		check_spends!(claim_txn[7], claim_txn[1]); // HTLC timeout on alternative commitment tx
+
+		check_spends!(claim_txn[3], remote_txn[0]); // HTLC timeout on broadcasted commitment tx
+		&claim_txn[3]
+	} else {
+		assert_eq!(claim_txn[1], claim_txn[3]);
+		assert_eq!(claim_txn[2], claim_txn[4]);
+
+		check_spends!(claim_txn[5], claim_txn[1]); // HTLC timeout on alternative commitment tx
+
+		check_spends!(claim_txn[7], remote_txn[0]); // HTLC timeout on broadcasted commitment tx
+
+		&claim_txn[7]
+	};
 
 	assert_eq!(claim_txn[0].input.len(), 1);
-	assert_eq!(claim_txn[3].input.len(), 1);
-	assert_eq!(claim_txn[0].input[0].previous_output, claim_txn[3].input[0].previous_output);
+	assert_eq!(bump_tx.input.len(), 1);
+	assert_eq!(claim_txn[0].input[0].previous_output, bump_tx.input[0].previous_output);
 
 	assert_eq!(claim_txn[0].input.len(), 1);
 	assert_eq!(claim_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
-	check_spends!(claim_txn[0], remote_txn[0]);
 	assert_eq!(remote_txn[0].output[claim_txn[0].input[0].previous_output.vout as usize].value, 800);
+
 	assert_eq!(claim_txn[6].input.len(), 1);
 	assert_eq!(claim_txn[6].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
 	check_spends!(claim_txn[6], remote_txn[0]);
@@ -2351,7 +2369,8 @@ fn test_justice_tx() {
 	chanmon_cfgs[1].keys_manager.disable_revocation_policy_check = true;
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &user_cfgs);
-	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	*nodes[0].connect_style.borrow_mut() = ConnectStyle::FullBlockViaListen;
 	// Create some new channels:
 	let chan_5 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
 
@@ -2583,11 +2602,7 @@ fn claim_htlc_outputs_single_tx() {
 		expect_payment_failed!(nodes[1], payment_hash_2, true);
 
 		let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-		assert_eq!(node_txn.len(), 9);
-		// ChannelMonitor: justice tx revoked offered htlc, justice tx revoked received htlc, justice tx revoked to_local (3)
-		// ChannelManager: local commmitment + local HTLC-timeout (2)
-		// ChannelMonitor: bumped justice tx, after one increase, bumps on HTLC aren't generated not being substantial anymore, bump on revoked to_local isn't generated due to more room for expiration (2)
-		// ChannelMonitor: local commitment + local HTLC-timeout (2)
+		assert!(node_txn.len() == 9 || node_txn.len() == 10);
 
 		// Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration
 		assert_eq!(node_txn[0].input.len(), 1);
@@ -5283,21 +5298,30 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 	let htlc_timeout_tx;
 	{ // Extract one of the two HTLC-Timeout transaction
 		let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-		// ChannelMonitor: timeout tx * 3, ChannelManager: local commitment tx
-		assert_eq!(node_txn.len(), 4);
+		// ChannelMonitor: timeout tx * 2-or-3, ChannelManager: local commitment tx
+		assert!(node_txn.len() == 4 || node_txn.len() == 3);
 		check_spends!(node_txn[0], chan_2.3);
 
 		check_spends!(node_txn[1], commitment_txn[0]);
 		assert_eq!(node_txn[1].input.len(), 1);
-		check_spends!(node_txn[2], commitment_txn[0]);
-		assert_eq!(node_txn[2].input.len(), 1);
-		assert_eq!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output);
-		check_spends!(node_txn[3], commitment_txn[0]);
-		assert_ne!(node_txn[1].input[0].previous_output, node_txn[3].input[0].previous_output);
+
+		if node_txn.len() > 3 {
+			check_spends!(node_txn[2], commitment_txn[0]);
+			assert_eq!(node_txn[2].input.len(), 1);
+			assert_eq!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output);
+
+			check_spends!(node_txn[3], commitment_txn[0]);
+			assert_ne!(node_txn[1].input[0].previous_output, node_txn[3].input[0].previous_output);
+		} else {
+			check_spends!(node_txn[2], commitment_txn[0]);
+			assert_ne!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output);
+		}
 
 		assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
 		assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
-		assert_eq!(node_txn[3].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+		if node_txn.len() > 3 {
+			assert_eq!(node_txn[3].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+		}
 		htlc_timeout_tx = node_txn[1].clone();
 	}
 
@@ -7957,13 +7981,24 @@ fn test_bump_penalty_txn_on_remote_commitment() {
 		assert_eq!(node_txn[6].input.len(), 1);
 		check_spends!(node_txn[0], remote_txn[0]);
 		check_spends!(node_txn[6], remote_txn[0]);
-		assert_eq!(node_txn[0].input[0].previous_output, node_txn[3].input[0].previous_output);
-		preimage_bump = node_txn[3].clone();
 
 		check_spends!(node_txn[1], chan.3);
 		check_spends!(node_txn[2], node_txn[1]);
-		assert_eq!(node_txn[1], node_txn[4]);
-		assert_eq!(node_txn[2], node_txn[5]);
+
+		if node_txn[0].input[0].previous_output == node_txn[3].input[0].previous_output {
+			preimage_bump = node_txn[3].clone();
+			check_spends!(node_txn[3], remote_txn[0]);
+
+			assert_eq!(node_txn[1], node_txn[4]);
+			assert_eq!(node_txn[2], node_txn[5]);
+		} else {
+			preimage_bump = node_txn[7].clone();
+			check_spends!(node_txn[7], remote_txn[0]);
+			assert_eq!(node_txn[0].input[0].previous_output, node_txn[7].input[0].previous_output);
+
+			assert_eq!(node_txn[1], node_txn[3]);
+			assert_eq!(node_txn[2], node_txn[4]);
+		}
 
 		timeout = node_txn[6].txid();
 		let index = node_txn[6].input[0].previous_output.vout;
@@ -8711,10 +8746,11 @@ 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 };
+	let block = Block { header, txdata: vec![] };
 	// 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);
+	chanmon_cfgs[0].tx_broadcaster.blocks.lock().unwrap().resize(200, (block.clone(), 0));
+	watchtower.chain_monitor.block_connected(&block, 200);
 
 	// Try to update ChannelMonitor
 	assert!(nodes[1].node.claim_funds(preimage));
@@ -8772,10 +8808,11 @@ 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 };
+	let block = Block { header, txdata: vec![] };
 	// 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);
+	chanmon_cfgs[0].tx_broadcaster.blocks.lock().unwrap().resize((CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS) as usize, (block.clone(), 0));
+	watchtower_alice.chain_monitor.block_connected(&block, CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
 	// Watchtower Alice should have broadcast a commitment/HTLC-timeout
 	{