Merge pull request #1048 from TheBlueMatt/2021-08-no-net-print

Remove explicit print in lightning-net-tokio, reduce redundant block connection logging

Author: TheBlueMatt

lightning-net-tokio/src/lib.rs

@@ -141,30 +141,23 @@ impl Connection {
 			PeerDisconnected
 		}
 		let disconnect_type = loop {
-			macro_rules! shutdown_socket {
-				($err: expr, $need_disconnect: expr) => { {
-					println!("Disconnecting peer due to {}!", $err);
-					break $need_disconnect;
-				} }
-			}
-
 			let read_paused = {
 				let us_lock = us.lock().unwrap();
 				if us_lock.rl_requested_disconnect {
-					shutdown_socket!("disconnect_socket() call from RL", Disconnect::CloseConnection);
+					break Disconnect::CloseConnection;
 				}
 				us_lock.read_paused
 			};
 			tokio::select! {
 				v = write_avail_receiver.recv() => {
 					assert!(v.is_some()); // We can't have dropped the sending end, its in the us Arc!
-					if let Err(e) = peer_manager.write_buffer_space_avail(&mut our_descriptor) {
-						shutdown_socket!(e, Disconnect::CloseConnection);
+					if let Err(_) = peer_manager.write_buffer_space_avail(&mut our_descriptor) {
+						break Disconnect::CloseConnection;
 					}
 				},
 				_ = read_wake_receiver.recv() => {},
 				read = reader.read(&mut buf), if !read_paused => match read {
-					Ok(0) => shutdown_socket!("Connection closed", Disconnect::PeerDisconnected),
+					Ok(0) => break Disconnect::PeerDisconnected,
 					Ok(len) => {
 						let read_res = peer_manager.read_event(&mut our_descriptor, &buf[0..len]);
 						let mut us_lock = us.lock().unwrap();
@@ -174,10 +167,10 @@ impl Connection {
 									us_lock.read_paused = true;
 								}
 							},
-							Err(e) => shutdown_socket!(e, Disconnect::CloseConnection),
+							Err(_) => break Disconnect::CloseConnection,
 						}
 					},
-					Err(e) => shutdown_socket!(e, Disconnect::PeerDisconnected),
+					Err(_) => break Disconnect::PeerDisconnected,
 				},
 			}
 			peer_manager.process_events();

lightning/src/chain/chainmonitor.rs

@@ -162,6 +162,7 @@ where
 	fn block_connected(&self, block: &Block, height: u32) {
 		let header = &block.header;
 		let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
+		log_debug!(self.logger, "New best block {} at height {} provided via block_connected", header.block_hash(), height);
 		self.process_chain_data(header, &txdata, |monitor, txdata| {
 			monitor.block_connected(
 				header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
@@ -170,6 +171,7 @@ where
 
 	fn block_disconnected(&self, header: &BlockHeader, height: u32) {
 		let monitors = self.monitors.read().unwrap();
+		log_debug!(self.logger, "Latest block {} at height {} removed via block_disconnected", header.block_hash(), height);
 		for monitor in monitors.values() {
 			monitor.block_disconnected(
 				header, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
@@ -187,20 +189,23 @@ where
 	P::Target: channelmonitor::Persist<ChannelSigner>,
 {
 	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+		log_debug!(self.logger, "{} provided transactions confirmed at height {} in block {}", txdata.len(), height, header.block_hash());
 		self.process_chain_data(header, txdata, |monitor, txdata| {
 			monitor.transactions_confirmed(
 				header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
 		});
 	}
 
 	fn transaction_unconfirmed(&self, txid: &Txid) {
+		log_debug!(self.logger, "Transaction {} reorganized out of chain", txid);
 		let monitors = self.monitors.read().unwrap();
 		for monitor in monitors.values() {
 			monitor.transaction_unconfirmed(txid, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
 		}
 	}
 
 	fn best_block_updated(&self, header: &BlockHeader, height: u32) {
+		log_debug!(self.logger, "New best block {} at height {} provided via best_block_updated", header.block_hash(), height);
 		self.process_chain_data(header, &[], |monitor, txdata| {
 			// While in practice there shouldn't be any recursive calls when given empty txdata,
 			// it's still possible if a chain::Filter implementation returns a transaction.

lightning/src/chain/channelmonitor.rs

@@ -1913,7 +1913,6 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 					L::Target: Logger,
 	{
 		let block_hash = header.block_hash();
-		log_trace!(logger, "New best block {} at height {}", block_hash, height);
 		self.best_block = BestBlock::new(block_hash, height);
 
 		self.transactions_confirmed(header, txdata, height, broadcaster, fee_estimator, logger)
@@ -1933,7 +1932,6 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		L::Target: Logger,
 	{
 		let block_hash = header.block_hash();
-		log_trace!(logger, "New best block {} at height {}", block_hash, height);
 
 		if height > self.best_block.height() {
 			self.best_block = BestBlock::new(block_hash, height);
@@ -1971,7 +1969,6 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		}
 
 		let block_hash = header.block_hash();
-		log_trace!(logger, "Block {} at height {} connected with {} txn matched", block_hash, height, txn_matched.len());
 
 		let mut watch_outputs = Vec::new();
 		let mut claimable_outpoints = Vec::new();
@@ -2044,6 +2041,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		F::Target: FeeEstimator,
 		L::Target: Logger,
 	{
+		log_trace!(logger, "Processing {} matched transactions for block at height {}.", txn_matched.len(), conf_height);
 		debug_assert!(self.best_block.height() >= conf_height);
 
 		let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);

lightning/src/ln/peer_handler.rs

@@ -667,6 +667,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
 		match self.do_read_event(peer_descriptor, data) {
 			Ok(res) => Ok(res),
 			Err(e) => {
+				log_trace!(self.logger, "Peer sent invalid data or we decided to disconnect due to a protocol error");
 				self.disconnect_event_internal(peer_descriptor, e.no_connection_possible);
 				Err(e)
 			}
@@ -1344,6 +1345,9 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
 			Some(peer) => {
 				match peer.their_node_id {
 					Some(node_id) => {
+						log_trace!(self.logger,
+							"Handling disconnection of peer {}, with {}future connection to the peer possible.",
+							log_pubkey!(node_id), if no_connection_possible { "no " } else { "" });
 						peers.node_id_to_descriptor.remove(&node_id);
 						self.message_handler.chan_handler.peer_disconnected(&node_id, no_connection_possible);
 					},

lightning/src/ln/script.rs

@@ -86,7 +86,10 @@ impl ShutdownScript {
 		Self(ShutdownScriptImpl::Bolt2(Script::new_v0_wsh(script_hash)))
 	}
 
-	/// Generates a P2WSH script pubkey from the given segwit version and program.
+	/// Generates a witness script pubkey from the given segwit version and program.
+	///
+	/// Note for version-zero witness scripts you must use [`ShutdownScript::new_p2wpkh`] or
+	/// [`ShutdownScript::new_p2wsh`] instead.
 	///
 	/// # Errors
 	///