Move common message handler methods into a common trait

Instead of having each `*MessageHandler` implement the same
`peer_connected`, `peer_disconnected`, `provided_init_features`,
and `provided_node_features` methods, here we pull them all out
into one common trait and make all the message handlers extend it.

Since `get_and_clear_pending_msg_events` was also common we also
move that into the same trait, dropping the
`MessageSendEventsProvider` trait.

Best reviewed with `--color-moved`

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -43,7 +43,6 @@ use lightning::chain::{
 	chainmonitor, channelmonitor, BestBlock, ChannelMonitorUpdateStatus, Confirm, Watch,
 };
 use lightning::events;
-use lightning::events::MessageSendEventsProvider;
 use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
 use lightning::ln::channel_state::ChannelDetails;
 use lightning::ln::channelmanager::{
@@ -53,7 +52,8 @@ use lightning::ln::channelmanager::{
 use lightning::ln::functional_test_utils::*;
 use lightning::ln::inbound_payment::ExpandedKey;
 use lightning::ln::msgs::{
-	self, ChannelMessageHandler, CommitmentUpdate, DecodeError, Init, UpdateAddHTLC,
+	self, BaseMessageHandler, ChannelMessageHandler, CommitmentUpdate, DecodeError, Init,
+	UpdateAddHTLC,
 };
 use lightning::ln::script::ShutdownScript;
 use lightning::ln::types::ChannelId;

fuzz/src/onion_message.rs

@@ -10,7 +10,7 @@ use lightning::blinded_path::message::{
 };
 use lightning::blinded_path::EmptyNodeIdLookUp;
 use lightning::ln::inbound_payment::ExpandedKey;
-use lightning::ln::msgs::{self, DecodeError, OnionMessageHandler};
+use lightning::ln::msgs::{self, BaseMessageHandler, DecodeError, OnionMessageHandler};
 use lightning::ln::peer_handler::IgnoringMessageHandler;
 use lightning::ln::script::ShutdownScript;
 use lightning::offers::invoice::UnsignedBolt12Invoice;

lightning-background-processor/src/lib.rs

@@ -1070,15 +1070,13 @@ mod tests {
 	use lightning::chain::channelmonitor::ANTI_REORG_DELAY;
 	use lightning::chain::transaction::OutPoint;
 	use lightning::chain::{chainmonitor, BestBlock, Confirm, Filter};
-	use lightning::events::{
-		Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, ReplayEvent,
-	};
+	use lightning::events::{Event, MessageSendEvent, PathFailure, ReplayEvent};
 	use lightning::ln::channelmanager;
 	use lightning::ln::channelmanager::{
 		ChainParameters, PaymentId, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA,
 	};
 	use lightning::ln::functional_test_utils::*;
-	use lightning::ln::msgs::{ChannelMessageHandler, Init};
+	use lightning::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, Init};
 	use lightning::ln::peer_handler::{
 		IgnoringMessageHandler, MessageHandler, PeerManager, SocketDescriptor,
 	};

lightning-dns-resolver/src/lib.rs

@@ -164,7 +164,9 @@ mod test {
 	use lightning::events::{Event, PaymentPurpose};
 	use lightning::ln::channelmanager::{PaymentId, Retry};
 	use lightning::ln::functional_test_utils::*;
-	use lightning::ln::msgs::{ChannelMessageHandler, Init, OnionMessageHandler};
+	use lightning::ln::msgs::{
+		BaseMessageHandler, ChannelMessageHandler, Init, OnionMessageHandler,
+	};
 	use lightning::ln::peer_handler::IgnoringMessageHandler;
 	use lightning::onion_message::dns_resolution::{HumanReadableName, OMNameResolver};
 	use lightning::onion_message::messenger::{

lightning-liquidity/tests/common/mod.rs

@@ -15,7 +15,7 @@ use lightning::chain::{chainmonitor, BestBlock, Confirm};
 use lightning::ln::channelmanager;
 use lightning::ln::channelmanager::ChainParameters;
 use lightning::ln::functional_test_utils::*;
-use lightning::ln::msgs::{ChannelMessageHandler, Init};
+use lightning::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, Init};
 use lightning::ln::peer_handler::{
 	IgnoringMessageHandler, MessageHandler, PeerManager, SocketDescriptor,
 };

lightning-net-tokio/src/lib.rs

@@ -684,12 +684,6 @@ mod tests {
 		) -> Option<NodeAnnouncement> {
 			None
 		}
-		fn peer_connected(
-			&self, _their_node_id: PublicKey, _init_msg: &Init, _inbound: bool,
-		) -> Result<(), ()> {
-			Ok(())
-		}
-		fn peer_disconnected(&self, _their_node_id: PublicKey) {}
 		fn handle_reply_channel_range(
 			&self, _their_node_id: PublicKey, _msg: ReplyChannelRange,
 		) -> Result<(), LightningError> {
@@ -710,12 +704,6 @@ mod tests {
 		) -> Result<(), LightningError> {
 			Ok(())
 		}
-		fn provided_node_features(&self) -> NodeFeatures {
-			NodeFeatures::empty()
-		}
-		fn provided_init_features(&self, _their_node_id: PublicKey) -> InitFeatures {
-			InitFeatures::empty()
-		}
 		fn processing_queue_high(&self) -> bool {
 			false
 		}
@@ -761,35 +749,39 @@ mod tests {
 		fn handle_tx_init_rbf(&self, _their_node_id: PublicKey, _msg: &TxInitRbf) {}
 		fn handle_tx_ack_rbf(&self, _their_node_id: PublicKey, _msg: &TxAckRbf) {}
 		fn handle_tx_abort(&self, _their_node_id: PublicKey, _msg: &TxAbort) {}
+		fn handle_channel_reestablish(&self, _their_node_id: PublicKey, _msg: &ChannelReestablish) {
+		}
+		fn handle_error(&self, _their_node_id: PublicKey, _msg: &ErrorMessage) {}
+		fn get_chain_hashes(&self) -> Option<Vec<ChainHash>> {
+			Some(vec![ChainHash::using_genesis_block(Network::Testnet)])
+		}
+		fn message_received(&self) {}
+	}
+	impl BaseMessageHandler for MsgHandler {
 		fn peer_disconnected(&self, their_node_id: PublicKey) {
 			if their_node_id == self.expected_pubkey {
 				self.disconnected_flag.store(true, Ordering::SeqCst);
-				self.pubkey_disconnected.clone().try_send(()).unwrap();
+				// This method is called twice as we're two message handlers. `try_send` will fail
+				// the second time.
+				let _ = self.pubkey_disconnected.clone().try_send(());
 			}
 		}
 		fn peer_connected(
 			&self, their_node_id: PublicKey, _init_msg: &Init, _inbound: bool,
 		) -> Result<(), ()> {
 			if their_node_id == self.expected_pubkey {
-				self.pubkey_connected.clone().try_send(()).unwrap();
+				// This method is called twice as we're two message handlers. `try_send` will fail
+				// the second time.
+				let _ = self.pubkey_connected.clone().try_send(());
 			}
 			Ok(())
 		}
-		fn handle_channel_reestablish(&self, _their_node_id: PublicKey, _msg: &ChannelReestablish) {
-		}
-		fn handle_error(&self, _their_node_id: PublicKey, _msg: &ErrorMessage) {}
 		fn provided_node_features(&self) -> NodeFeatures {
 			NodeFeatures::empty()
 		}
 		fn provided_init_features(&self, _their_node_id: PublicKey) -> InitFeatures {
 			InitFeatures::empty()
 		}
-		fn get_chain_hashes(&self) -> Option<Vec<ChainHash>> {
-			Some(vec![ChainHash::using_genesis_block(Network::Testnet)])
-		}
-		fn message_received(&self) {}
-	}
-	impl MessageSendEventsProvider for MsgHandler {
 		fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> {
 			let mut ret = Vec::new();
 			mem::swap(&mut *self.msg_events.lock().unwrap(), &mut ret);

lightning-persister/src/fs_store.rs

@@ -501,8 +501,9 @@ mod tests {
 	use lightning::chain::chainmonitor::Persist;
 	use lightning::chain::ChannelMonitorUpdateStatus;
 	use lightning::check_closed_event;
-	use lightning::events::{ClosureReason, MessageSendEventsProvider};
+	use lightning::events::ClosureReason;
 	use lightning::ln::functional_test_utils::*;
+	use lightning::ln::msgs::BaseMessageHandler;
 	use lightning::util::persist::read_channel_monitors;
 	use lightning::util::test_utils;
 

lightning/src/chain/chainmonitor.rs

@@ -928,9 +928,9 @@ mod tests {
 	use crate::{get_htlc_update_msgs, get_revoke_commit_msgs};
 	use crate::chain::{ChannelMonitorUpdateStatus, Watch};
 	use crate::chain::channelmonitor::ANTI_REORG_DELAY;
-	use crate::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider};
+	use crate::events::{ClosureReason, Event, MessageSendEvent};
 	use crate::ln::functional_test_utils::*;
-	use crate::ln::msgs::ChannelMessageHandler;
+	use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler};
 
 	const CHAINSYNC_MONITOR_PARTITION_FACTOR: u32 = 5;
 

lightning/src/events/mod.rs

@@ -2626,13 +2626,6 @@ pub enum MessageSendEvent {
 	},
 }
 
-/// A trait indicating an object may generate message send events
-pub trait MessageSendEventsProvider {
-	/// Gets the list of pending events which were generated by previous actions, clearing the list
-	/// in the process.
-	fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent>;
-}
-
 /// A trait indicating an object may generate events.
 ///
 /// Events are processed by passing an [`EventHandler`] to [`process_pending_events`].

lightning/src/ln/async_payments_tests.rs

@@ -11,15 +11,12 @@ use crate::blinded_path::message::{MessageContext, OffersContext};
 use crate::blinded_path::payment::PaymentContext;
 use crate::blinded_path::payment::{AsyncBolt12OfferContext, BlindedPaymentTlvs};
 use crate::chain::channelmonitor::{HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS};
-use crate::events::{
-	Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PaymentFailureReason,
-};
+use crate::events::{Event, HTLCDestination, MessageSendEvent, PaymentFailureReason};
 use crate::ln::blinded_payment_tests::{fail_blinded_htlc_backwards, get_blinded_route_parameters};
 use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
 use crate::ln::functional_test_utils::*;
 use crate::ln::msgs;
-use crate::ln::msgs::ChannelMessageHandler;
-use crate::ln::msgs::OnionMessageHandler;
+use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, OnionMessageHandler};
 use crate::ln::offers_tests;
 use crate::ln::onion_utils::INVALID_ONION_BLINDING;
 use crate::ln::outbound_payment::PendingOutboundPayment;

lightning/src/ln/async_signer_tests.rs

@@ -20,11 +20,11 @@ use crate::chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
 use crate::chain::ChannelMonitorUpdateStatus;
 use crate::chain::transaction::OutPoint;
 use crate::events::bump_transaction::WalletSource;
-use crate::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider};
+use crate::events::{ClosureReason, Event, MessageSendEvent};
 use crate::ln::chan_utils::ClosingTransaction;
 use crate::ln::channel_state::{ChannelDetails, ChannelShutdownState};
 use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, RecipientOnionFields};
-use crate::ln::msgs::ChannelMessageHandler;
+use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler};
 use crate::ln::{functional_test_utils::*, msgs};
 use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::sign::SignerProvider;

lightning/src/ln/blinded_payment_tests.rs

@@ -14,7 +14,7 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
 use crate::blinded_path;
 use crate::blinded_path::payment::{BlindedPaymentPath, Bolt12RefundContext, PaymentForwardNode, ForwardTlvs, PaymentConstraints, PaymentContext, PaymentRelay, UnauthenticatedReceiveTlvs};
-use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PaymentFailureReason};
+use crate::events::{Event, HTLCDestination, MessageSendEvent, PaymentFailureReason};
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentHash, PaymentSecret};
 use crate::ln::channelmanager;
@@ -23,7 +23,7 @@ use crate::types::features::{BlindedHopFeatures, ChannelFeatures, NodeFeatures};
 use crate::ln::functional_test_utils::*;
 use crate::ln::inbound_payment::ExpandedKey;
 use crate::ln::msgs;
-use crate::ln::msgs::{ChannelMessageHandler, UnsignedGossipMessage};
+use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, UnsignedGossipMessage};
 use crate::ln::onion_payment;
 use crate::ln::onion_utils;
 use crate::ln::onion_utils::INVALID_ONION_BLINDING;

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -17,12 +17,12 @@ use bitcoin::hash_types::BlockHash;
 use bitcoin::network::Network;
 use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor};
 use crate::chain::{ChannelMonitorUpdateStatus, Listen, Watch};
-use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason, HTLCDestination};
+use crate::events::{Event, MessageSendEvent, PaymentPurpose, ClosureReason, HTLCDestination};
 use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, RecipientOnionFields};
 use crate::ln::channel::AnnouncementSigsState;
 use crate::ln::msgs;
 use crate::ln::types::ChannelId;
-use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
+use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, RoutingMessageHandler};
 use crate::util::test_channel_signer::TestChannelSigner;
 use crate::util::ser::{ReadableArgs, Writeable};
 use crate::util::test_utils::TestBroadcaster;