1/3 Use MessageSendInstructions instead of PendingOnionMessage

Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the first step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `CustomMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

By unifying the response type with the outbound message queue type,
this also fixes #3178.

Author: TheBlueMatt

lightning/src/ln/peer_handler.rs

@@ -30,7 +30,7 @@ use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor, NextNoiseStep, Mes
 use crate::ln::wire;
 use crate::ln::wire::{Encode, Type};
 use crate::onion_message::async_payments::{AsyncPaymentsMessageHandler, HeldHtlcAvailable, ReleaseHeldHtlc};
-use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage, Responder, MessageSendInstructions};
+use crate::onion_message::messenger::{CustomOnionMessageHandler, Responder, MessageSendInstructions};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeId, NodeAlias};
@@ -165,7 +165,7 @@ impl CustomOnionMessageHandler for IgnoringMessageHandler {
 	fn read_custom_message<R: io::Read>(&self, _msg_type: u64, _buffer: &mut R) -> Result<Option<Infallible>, msgs::DecodeError> where Self: Sized {
 		Ok(None)
 	}
-	fn release_pending_custom_messages(&self) -> Vec<PendingOnionMessage<Infallible>> {
+	fn release_pending_custom_messages(&self) -> Vec<(Infallible, MessageSendInstructions)> {
 		vec![]
 	}
 }

lightning/src/onion_message/functional_tests.rs

@@ -20,7 +20,7 @@ use crate::sign::{NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
 use super::async_payments::{AsyncPaymentsMessageHandler, HeldHtlcAvailable, ReleaseHeldHtlc};
-use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, Responder, MessageSendInstructions, SendError, SendSuccess};
+use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, Responder, MessageSendInstructions, SendError, SendSuccess};
 use super::offers::{OffersMessage, OffersMessageHandler};
 use super::packet::{OnionMessageContents, Packet};
 
@@ -211,7 +211,7 @@ impl CustomOnionMessageHandler for TestCustomMessageHandler {
 			_ => Ok(None),
 		}
 	}
-	fn release_pending_custom_messages(&self) -> Vec<PendingOnionMessage<Self::CustomMessage>> {
+	fn release_pending_custom_messages(&self) -> Vec<(Self::CustomMessage, MessageSendInstructions)> {
 		vec![]
 	}
 }

lightning/src/onion_message/messenger.rs

@@ -812,15 +812,7 @@ pub trait CustomOnionMessageHandler {
 	///
 	/// Typically, this is used for messages initiating a message flow rather than in response to
 	/// another message. The latter should use the return value of [`Self::handle_custom_message`].
-	#[cfg(not(c_bindings))]
-	fn release_pending_custom_messages(&self) -> Vec<PendingOnionMessage<Self::CustomMessage>>;
-
-	/// Releases any [`Self::CustomMessage`]s that need to be sent.
-	///
-	/// Typically, this is used for messages initiating a message flow rather than in response to
-	/// another message. The latter should use the return value of [`Self::handle_custom_message`].
-	#[cfg(c_bindings)]
-	fn release_pending_custom_messages(&self) -> Vec<(Self::CustomMessage, Destination, Option<BlindedMessagePath>)>;
+	fn release_pending_custom_messages(&self) -> Vec<(Self::CustomMessage, MessageSendInstructions)>;
 }
 
 /// A processed incoming onion message, containing either a Forward (another onion message)
@@ -1170,6 +1162,33 @@ where
 		)
 	}
 
+	fn handle_onion_message_send<T: OnionMessageContents>(
+		&self, response_message: T, response: MessageSendInstructions, log_suffix: fmt::Arguments,
+	) -> Result<Option<SendSuccess>, SendError> {
+		let (destination, context) = match response {
+			MessageSendInstructions::WithReplyPath { destination, context } => (destination, Some(context)),
+			MessageSendInstructions::WithoutReplyPath { destination } => (destination, None),
+		};
+
+		let reply_path = if let Some(context) = context {
+			match self.create_blinded_path(context) {
+				Ok(reply_path) => Some(reply_path),
+				Err(err) => {
+					log_trace!(
+						self.logger,
+						"Failed to create reply path {}: {:?}",
+						log_suffix, err
+					);
+					return Err(err);
+				}
+			}
+		} else { None };
+
+		self.find_path_and_enqueue_onion_message(
+			response_message, destination, reply_path, log_suffix,
+		).map(|result| Some(result))
+	}
+
 	fn find_path_and_enqueue_onion_message<T: OnionMessageContents>(
 		&self, contents: T, destination: Destination, reply_path: Option<BlindedMessagePath>,
 		log_suffix: fmt::Arguments
@@ -1322,35 +1341,16 @@ where
 	/// generating the response asynchronously. Subsequently, when the response is prepared and
 	/// ready for sending, that task can invoke this method to enqueue the response for delivery.
 	pub fn handle_onion_message_response<T: OnionMessageContents>(
-		&self, response_message: T, response: MessageSendInstructions,
+		&self, response_message: T, instructions: MessageSendInstructions,
 	) -> Result<Option<SendSuccess>, SendError> {
-		let (destination, context) = match response {
-			MessageSendInstructions::WithReplyPath { destination, context } => (destination, Some(context)),
-			MessageSendInstructions::WithoutReplyPath { destination } => (destination, None),
-		};
-
 		let message_type = response_message.msg_type();
-		let reply_path = if let Some(context) = context {
-			match self.create_blinded_path(context) {
-				Ok(reply_path) => Some(reply_path),
-				Err(err) => {
-					log_trace!(
-						self.logger,
-						"Failed to create reply path when responding with {} to an onion message: {:?}",
-						message_type, err
-					);
-					return Err(err);
-				}
-			}
-		} else { None };
-
-		self.find_path_and_enqueue_onion_message(
-			response_message, destination, reply_path,
+		self.handle_onion_message_send(
+			response_message, instructions,
 			format_args!(
 				"when responding with {} to an onion message",
 				message_type,
 			)
-		).map(|result| Some(result))
+		)
 	}
 
 	#[cfg(test)]
@@ -1730,13 +1730,9 @@ where
 		}
 
 		// Enqueue any initiating `CustomMessage`s to send.
-		for message in self.custom_handler.release_pending_custom_messages() {
-			#[cfg(not(c_bindings))]
-			let PendingOnionMessage { contents, destination, reply_path } = message;
-			#[cfg(c_bindings)]
-			let (contents, destination, reply_path) = message;
-			let _ = self.find_path_and_enqueue_onion_message(
-				contents, destination, reply_path, format_args!("when sending CustomMessage")
+		for (message, instructions) in self.custom_handler.release_pending_custom_messages() {
+			let _ = self.handle_onion_message_send(
+				message, instructions, format_args!("when sending CustomMessage")
 			);
 		}