Merge pull request #730 from TheBlueMatt/2020-09-no-rescan

Drop last bits of rescan as its too complicated to be worth having

Author: TheBlueMatt

lightning/src/chain/chainmonitor.rs

@@ -80,32 +80,27 @@ impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref> ChainMonit
 	/// [`ChannelMonitor::block_connected`] for details. Any HTLCs that were resolved on chain will
 	/// be returned by [`chain::Watch::release_pending_monitor_events`].
 	///
-	/// Calls back to [`chain::Filter`] if any monitor indicated new outputs to watch, returning
-	/// `true` if so. Subsequent calls must not exclude any transactions matching the new outputs
-	/// nor any in-block descendants of such transactions. It is not necessary to re-fetch the block
-	/// to obtain updated `txdata`.
+	/// Calls back to [`chain::Filter`] if any monitor indicated new outputs to watch. Subsequent
+	/// calls must not exclude any transactions matching the new outputs nor any in-block
+	/// descendants of such transactions. It is not necessary to re-fetch the block to obtain
+	/// updated `txdata`.
 	///
 	/// [`ChannelMonitor::block_connected`]: ../channelmonitor/struct.ChannelMonitor.html#method.block_connected
 	/// [`chain::Watch::release_pending_monitor_events`]: ../trait.Watch.html#tymethod.release_pending_monitor_events
 	/// [`chain::Filter`]: ../trait.Filter.html
-	pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) -> bool {
-		let mut has_new_outputs_to_watch = false;
-		{
-			let mut monitors = self.monitors.lock().unwrap();
-			for monitor in monitors.values_mut() {
-				let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
-				has_new_outputs_to_watch |= !txn_outputs.is_empty();
-
-				if let Some(ref chain_source) = self.chain_source {
-					for (txid, outputs) in txn_outputs.drain(..) {
-						for (idx, output) in outputs.iter().enumerate() {
-							chain_source.register_output(&OutPoint { txid, index: idx as u16 }, &output.script_pubkey);
-						}
+	pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+		let mut monitors = self.monitors.lock().unwrap();
+		for monitor in monitors.values_mut() {
+			let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
+
+			if let Some(ref chain_source) = self.chain_source {
+				for (txid, outputs) in txn_outputs.drain(..) {
+					for (idx, output) in outputs.iter().enumerate() {
+						chain_source.register_output(&OutPoint { txid, index: idx as u16 }, &output.script_pubkey);
 					}
 				}
 			}
 		}
-		has_new_outputs_to_watch
 	}
 
 	/// Dispatches to per-channel monitors, which are responsible for updating their on-chain view

lightning/src/ln/functional_test_utils.rs

@@ -81,7 +81,7 @@ pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32, he
 
 pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, height: u32) {
 	let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
-	while node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height) {}
+	node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height);
 	node.node.block_connected(&block.header, &txdata, height);
 }
 

lightning/src/ln/functional_tests.rs

@@ -4852,8 +4852,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
 	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1, 1, true, header.block_hash());
 
 	let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
-	assert_eq!(spend_txn.len(), 2);
-	assert_eq!(spend_txn[0], spend_txn[1]);
+	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
 }
 
@@ -4885,10 +4884,9 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
 	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1, 1, true, header.block_hash());
 
 	let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
-	assert_eq!(spend_txn.len(), 3);
-	assert_eq!(spend_txn[0], spend_txn[1]); // to_remote output on revoked remote commitment_tx
-	check_spends!(spend_txn[0], revoked_local_txn[0]);
-	check_spends!(spend_txn[2], node_txn[0]);
+	assert_eq!(spend_txn.len(), 2);
+	check_spends!(spend_txn[0], revoked_local_txn[0]); // to_remote output on revoked remote commitment_tx
+	check_spends!(spend_txn[1], node_txn[0]);
 }
 
 #[test]
@@ -4983,8 +4981,8 @@ fn test_static_spendable_outputs_timeout_tx() {
 	expect_payment_failed!(nodes[1], our_payment_hash, true);
 
 	let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
-	assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote (*2), timeout_tx.output (*1)
-	check_spends!(spend_txn[2], node_txn[0].clone());
+	assert_eq!(spend_txn.len(), 2); // SpendableOutput: remote_commitment_tx.to_remote, timeout_tx.output
+	check_spends!(spend_txn[1], node_txn[0]);
 }
 
 #[test]
@@ -5159,12 +5157,11 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
 
 	// Check A's ChannelMonitor was able to generate the right spendable output descriptor
 	let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000);
-	assert_eq!(spend_txn.len(), 3); // Duplicated SpendableOutput due to block rescan after revoked htlc output tracking
-	assert_eq!(spend_txn[0], spend_txn[1]);
+	assert_eq!(spend_txn.len(), 2);
 	assert_eq!(spend_txn[0].input.len(), 1);
 	check_spends!(spend_txn[0], revoked_local_txn[0]); // spending to_remote output from revoked local tx
 	assert_ne!(spend_txn[0].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output);
-	check_spends!(spend_txn[2], node_txn[1]); // spending justice tx output on the htlc success tx
+	check_spends!(spend_txn[1], node_txn[1]); // spending justice tx output on the htlc success tx
 }
 
 #[test]
@@ -5228,7 +5225,7 @@ fn test_onchain_to_onchain_claim() {
 		assert_eq!(b_txn[2].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
 		assert!(b_txn[2].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
 		assert_ne!(b_txn[2].lock_time, 0); // Timeout tx
-		check_spends!(b_txn[0], c_txn[1]); // timeout tx on C remote commitment tx, issued by ChannelMonitor, * 2 due to block rescan
+		check_spends!(b_txn[0], c_txn[1]); // timeout tx on C remote commitment tx, issued by ChannelMonitor
 		assert_eq!(b_txn[0].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
 		assert!(b_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
 		assert_ne!(b_txn[2].lock_time, 0); // Timeout tx
@@ -5726,10 +5723,9 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 
 	// 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);
-	assert_eq!(spend_txn[0], spend_txn[1]);
+	assert_eq!(spend_txn.len(), 2);
 	check_spends!(spend_txn[0], local_txn[0]);
-	check_spends!(spend_txn[2], htlc_timeout);
+	check_spends!(spend_txn[1], htlc_timeout);
 }
 
 #[test]
@@ -5797,10 +5793,9 @@ fn test_key_derivation_params() {
 	// 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);
-	assert_eq!(spend_txn[0], spend_txn[1]);
+	assert_eq!(spend_txn.len(), 2);
 	check_spends!(spend_txn[0], local_txn_1[0]);
-	check_spends!(spend_txn[2], htlc_timeout);
+	check_spends!(spend_txn[1], htlc_timeout);
 }
 
 #[test]