Merge pull request #936 from TheBlueMatt/2021-05-spendable-event-locktime-delay

Delay DelayedPaymentOutput spendable events until the CSV delay

Author: TheBlueMatt

lightning-background-processor/src/lib.rs

@@ -180,7 +180,7 @@ mod tests {
 	use lightning::chain::keysinterface::{InMemorySigner, KeysInterface, KeysManager};
 	use lightning::chain::transaction::OutPoint;
 	use lightning::get_event_msg;
-	use lightning::ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, SimpleArcChannelManager};
+	use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, BestBlock, ChainParameters, ChannelManager, SimpleArcChannelManager};
 	use lightning::ln::features::InitFeatures;
 	use lightning::ln::msgs::ChannelMessageHandler;
 	use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor};
@@ -303,8 +303,8 @@ mod tests {
 		}}
 	}
 
-	fn confirm_transaction(node: &mut Node, tx: &Transaction) {
-		for i in 1..=ANTI_REORG_DELAY {
+	fn confirm_transaction_depth(node: &mut Node, tx: &Transaction, depth: u32) {
+		for i in 1..=depth {
 			let prev_blockhash = node.best_block.block_hash();
 			let height = node.best_block.height() + 1;
 			let header = BlockHeader { version: 0x20000000, prev_blockhash, merkle_root: Default::default(), time: height, bits: 42, nonce: 42 };
@@ -315,14 +315,17 @@ mod tests {
 					node.node.transactions_confirmed(&header, &txdata, height);
 					node.chain_monitor.transactions_confirmed(&header, &txdata, height);
 				},
-				ANTI_REORG_DELAY => {
+				x if x == depth => {
 					node.node.best_block_updated(&header, height);
 					node.chain_monitor.best_block_updated(&header, height);
 				},
 				_ => {},
 			}
 		}
 	}
+	fn confirm_transaction(node: &mut Node, tx: &Transaction) {
+		confirm_transaction_depth(node, tx, ANTI_REORG_DELAY);
+	}
 
 	#[test]
 	fn test_background_processor() {
@@ -454,7 +457,7 @@ mod tests {
 		// Force close the channel and check that the SpendableOutputs event was handled.
 		nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap();
 		let commitment_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().pop().unwrap();
-		confirm_transaction(&mut nodes[0], &commitment_tx);
+		confirm_transaction_depth(&mut nodes[0], &commitment_tx, BREAKDOWN_TIMEOUT as u32);
 		let event = receiver
 			.recv_timeout(Duration::from_secs(EVENT_DEADLINE))
 			.expect("SpendableOutputs not handled within deadline");

lightning/src/chain/channelmonitor.rs

@@ -362,7 +362,15 @@ struct OnchainEventEntry {
 
 impl OnchainEventEntry {
 	fn confirmation_threshold(&self) -> u32 {
-		self.height + ANTI_REORG_DELAY - 1
+		let mut conf_threshold = self.height + ANTI_REORG_DELAY - 1;
+		if let OnchainEvent::MaturingOutput {
+			descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(ref descriptor)
+		} = self.event {
+			// A CSV'd transaction is confirmable in block (input height) + CSV delay, which means
+			// it's broadcastable when we see the previous block.
+			conf_threshold = cmp::max(conf_threshold, self.height + descriptor.to_self_delay as u32 - 1);
+		}
+		conf_threshold
 	}
 
 	fn has_reached_confirmation_threshold(&self, height: u32) -> bool {

lightning/src/ln/functional_tests.rs

@@ -4875,11 +4875,13 @@ fn test_claim_sizeable_push_msat() {
 	assert_eq!(node_txn[0].output.len(), 2); // We can't force trimming of to_remote output as channel_reserve_satoshis block us to do so at channel opening
 
 	mine_transaction(&nodes[1], &node_txn[0]);
-	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+	connect_blocks(&nodes[1], BREAKDOWN_TIMEOUT as u32 - 1);
 
 	let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
 	assert_eq!(spend_txn.len(), 1);
+	assert_eq!(spend_txn[0].input.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
+	assert_eq!(spend_txn[0].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
 }
 
 #[test]
@@ -5507,12 +5509,14 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
 	};
 
 	mine_transaction(&nodes[1], &node_tx);
-	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+	connect_blocks(&nodes[1], BREAKDOWN_TIMEOUT as u32 - 1);
 
 	// Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
 	let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
 	assert_eq!(spend_txn.len(), 1);
+	assert_eq!(spend_txn[0].input.len(), 1);
 	check_spends!(spend_txn[0], node_tx);
+	assert_eq!(spend_txn[0].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
 }
 
 fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, announce_latest: bool) {
@@ -5802,15 +5806,20 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 	};
 
 	mine_transaction(&nodes[0], &htlc_timeout);
-	connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+	connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
 	expect_payment_failed!(nodes[0], our_payment_hash, true);
 
 	// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
 	let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000);
 	assert_eq!(spend_txn.len(), 3);
 	check_spends!(spend_txn[0], local_txn[0]);
+	assert_eq!(spend_txn[1].input.len(), 1);
 	check_spends!(spend_txn[1], htlc_timeout);
+	assert_eq!(spend_txn[1].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
+	assert_eq!(spend_txn[2].input.len(), 2);
 	check_spends!(spend_txn[2], local_txn[0], htlc_timeout);
+	assert!(spend_txn[2].input[0].sequence == BREAKDOWN_TIMEOUT as u32 ||
+	        spend_txn[2].input[1].sequence == BREAKDOWN_TIMEOUT as u32);
 }
 
 #[test]
@@ -5877,16 +5886,21 @@ fn test_key_derivation_params() {
 	};
 
 	mine_transaction(&nodes[0], &htlc_timeout);
-	connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+	connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
 	expect_payment_failed!(nodes[0], our_payment_hash, true);
 
 	// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
 	let new_keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
 	let spend_txn = check_spendable_outputs!(nodes[0], 1, new_keys_manager, 100000);
 	assert_eq!(spend_txn.len(), 3);
 	check_spends!(spend_txn[0], local_txn_1[0]);
+	assert_eq!(spend_txn[1].input.len(), 1);
 	check_spends!(spend_txn[1], htlc_timeout);
+	assert_eq!(spend_txn[1].input[0].sequence, BREAKDOWN_TIMEOUT as u32);
+	assert_eq!(spend_txn[2].input.len(), 2);
 	check_spends!(spend_txn[2], local_txn_1[0], htlc_timeout);
+	assert!(spend_txn[2].input[0].sequence == BREAKDOWN_TIMEOUT as u32 ||
+	        spend_txn[2].input[1].sequence == BREAKDOWN_TIMEOUT as u32);
 }
 
 #[test]

lightning/src/util/events.rs

@@ -126,7 +126,8 @@ pub enum Event {
 		/// now + 5*time_forwardable).
 		time_forwardable: Duration,
 	},
-	/// Used to indicate that an output was generated on-chain which you should know how to spend.
+	/// Used to indicate that an output which you should know how to spend was confirmed on chain
+	/// and is now spendable.
 	/// Such an output will *not* ever be spent by rust-lightning, and are not at risk of your
 	/// counterparty spending them due to some kind of timeout. Thus, you need to store them
 	/// somewhere and spend them when you create on-chain transactions.