Approach Revamp: Introducing ResponseInstruction.

1. Since usage of closure could have lead to circular referencing, a new approach is introduced here.
2. This commit introduce a new struct called ResponseInstruction. The job of this struct is to contain
the following information:
    1. The knowledge of whether to have a response to the message,
    2. the response of the message itself, and
    3. The other variable needed to forward this response (that is, reply_path, for now).
3. This struct will be used by OnionMessageHandler for doing proper response handling
4. In this commit this struct is used in handle_onion_message_response to appropriately queue the response of responding.

Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>

Author: shaavan

fuzz/src/onion_message.rs

@@ -96,7 +96,9 @@ impl MessageRouter for TestMessageRouter {
 struct TestOffersMessageHandler {}
 
 impl OffersMessageHandler for TestOffersMessageHandler {
-	fn handle_message<R: RespondFunction<OffersMessage>>(&self, _message: ReceivedOnionMessage<R, OffersMessage>) {}
+	fn handle_message(&self, _message: ReceivedOnionMessage<OffersMessage>) -> ResponseInstruction<OffersMessage> {
+		ResponseInstruction::NoResponse
+	}
 }
 
 #[derive(Debug)]
@@ -121,7 +123,7 @@ struct TestCustomMessageHandler {}
 
 impl CustomOnionMessageHandler for TestCustomMessageHandler {
 	type CustomMessage = TestCustomMessage;
-	fn handle_custom_message<R: RespondFunction<Self::CustomMessage>>(&self, message: ReceivedOnionMessage<R, Self::CustomMessage>) {
+	fn handle_custom_message(&self, message: ReceivedOnionMessage<Self::CustomMessage>) -> ResponseInstruction<Self::CustomMessage> {
 		if let ReceivedOnionMessage::WithReplyPath({_, responder}) = message {
 			responder.respond(TestCustomMessage {})
 		}

lightning/src/ln/channelmanager.rs

@@ -64,7 +64,7 @@ use crate::offers::merkle::SignError;
 use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
-use crate::onion_message::messenger::{new_pending_onion_message, Destination, MessageRouter, PendingOnionMessage, ReceivedOnionMessage, RespondFunction};
+use crate::onion_message::messenger::{new_pending_onion_message, Destination, MessageRouter, PendingOnionMessage, ReceivedOnionMessage, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
 use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
@@ -9274,7 +9274,7 @@ where
 	R::Target: Router,
 	L::Target: Logger,
 {
-	fn handle_message<RF: RespondFunction<OffersMessage>>(&self, message: ReceivedOnionMessage<RF, OffersMessage>)
+	fn handle_message(&self, message: ReceivedOnionMessage<OffersMessage>) -> ResponseInstruction<OffersMessage>
 	{
 		let secp_ctx = &self.secp_ctx;
 		let expanded_key = &self.inbound_payment_key;
@@ -9385,8 +9385,12 @@ where
 			};
 
 			if let Some(response) = response_option {
-				responder.respond(response);
+				responder.respond(response)
+			} else {
+				ResponseInstruction::NoResponse
 			}
+		} else {
+			ResponseInstruction::NoResponse
 		}
 	}
 

lightning/src/ln/peer_handler.rs

@@ -28,7 +28,7 @@ use crate::util::ser::{VecWriter, Writeable, Writer};
 use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor, NextNoiseStep, MessageBuf, MSG_BUF_ALLOC_SIZE};
 use crate::ln::wire;
 use crate::ln::wire::{Encode, Type};
-use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage, ReceivedOnionMessage, RespondFunction};
+use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage, ReceivedOnionMessage, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeId, NodeAlias};
@@ -134,11 +134,13 @@ impl OnionMessageHandler for IgnoringMessageHandler {
 }
 
 impl OffersMessageHandler for IgnoringMessageHandler {
-	fn handle_message<R: RespondFunction<OffersMessage>>(&self, _message: ReceivedOnionMessage<R, OffersMessage>) {}
+	fn handle_message(&self, _message: ReceivedOnionMessage<OffersMessage>) -> ResponseInstruction<OffersMessage> {
+		ResponseInstruction::NoResponse
+	}
 }
 impl CustomOnionMessageHandler for IgnoringMessageHandler {
 	type CustomMessage = Infallible;
-	fn handle_custom_message<R: RespondFunction<Self::CustomMessage>>(&self, _message: ReceivedOnionMessage<R, Self::CustomMessage>) {
+	fn handle_custom_message(&self, _message: ReceivedOnionMessage<Self::CustomMessage>) -> ResponseInstruction<Self::CustomMessage> {
 		// Since we always return `None` in the read the handle method should never be called.
 		unreachable!();
 	}

lightning/src/onion_message/functional_tests.rs

@@ -16,7 +16,7 @@ use crate::ln::msgs::{self, DecodeError, OnionMessageHandler, SocketAddress};
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
-use super::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, ReceivedOnionMessage, RespondFunction, SendError};
+use super::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, ReceivedOnionMessage, ResponseInstruction, SendError};
 use super::offers::{OffersMessage, OffersMessageHandler};
 use super::packet::{OnionMessageContents, Packet};
 
@@ -70,7 +70,9 @@ impl MessageRouter for TestMessageRouter {
 struct TestOffersMessageHandler {}
 
 impl OffersMessageHandler for TestOffersMessageHandler {
-	fn handle_message<R: RespondFunction<OffersMessage>>(&self, _message: ReceivedOnionMessage<R, OffersMessage>) {}
+	fn handle_message(&self, _message: ReceivedOnionMessage<OffersMessage>) -> ResponseInstruction<OffersMessage> {
+		ResponseInstruction::NoResponse
+	}
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -132,7 +134,7 @@ impl Drop for TestCustomMessageHandler {
 
 impl CustomOnionMessageHandler for TestCustomMessageHandler {
 	type CustomMessage = TestCustomMessage;
-	fn handle_custom_message<R: RespondFunction<Self::CustomMessage>>(&self, message: ReceivedOnionMessage<R, Self::CustomMessage>) {
+	fn handle_custom_message(&self, message: ReceivedOnionMessage<Self::CustomMessage>) -> ResponseInstruction<Self::CustomMessage> {
 		if let ReceivedOnionMessage::WithReplyPath{responder, message, path_id: _} = message {
 			match self.expected_messages.lock().unwrap().pop_front() {
 				Some(expected_msg) => assert_eq!(expected_msg, message),
@@ -146,7 +148,11 @@ impl CustomOnionMessageHandler for TestCustomMessageHandler {
 
 			if let Some(response) = response_option {
 				responder.respond(response)
+			} else {
+				ResponseInstruction::NoResponse
 			}
+		} else {
+			ResponseInstruction::NoResponse
 		}
 	}
 	fn read_custom_message<R: io::Read>(&self, message_type: u64, buffer: &mut R) -> Result<Option<Self::CustomMessage>, DecodeError> where Self: Sized {

lightning/src/onion_message/messenger.rs

@@ -245,31 +245,28 @@ impl OnionMessageRecipient {
 }
 
 /// A enum to handle received [`OnionMessage`]
-pub enum ReceivedOnionMessage<R, T>
+pub enum ReceivedOnionMessage<T>
 where
-	R: RespondFunction<T>,
 	T: OnionMessageContents
 {
 	WithReplyPath {
 		message: T,
 		path_id: Option<[u8; 32]>,
-		responder: Responder<R, T>,
+		responder: Responder<T>,
 	},
 	WithoutReplyPath {
 		message: T,
 		path_id: Option<[u8; 32]>,
 	},
 }
 
-impl<R, T> ReceivedOnionMessage<R, T>
+impl<T> ReceivedOnionMessage<T>
 where
-	R: RespondFunction<T>,
 	T: OnionMessageContents
 {
-	fn new(messenger_function: R, message: T, reply_path_opt: Option<BlindedPath>, path_id: Option<[u8; 32]> ) -> Self {
+	fn new( message: T, reply_path_opt: Option<BlindedPath>, path_id: Option<[u8; 32]> ) -> Self {
 		if let Some(reply_path) = reply_path_opt {
 			let responder = Responder {
-				messenger_function,
 				reply_path,
 				_phantom: PhantomData
 			};
@@ -287,43 +284,38 @@ where
     }
 }
 
-pub trait RespondFunction<T: OnionMessageContents>{
-	fn respond(self, response: T, reply_path: BlindedPath);
-}
-
-impl<F, T> RespondFunction<T> for F
-where
-	F: Fn(T, BlindedPath),
-	T: OnionMessageContents
-{
-	fn respond(self, response: T, reply_path: BlindedPath) {
-		self(response, reply_path)
-	}
-}
-
 /// A struct handling response to an [`OnionMessage`]
-pub struct Responder<R, T>
+pub struct Responder<T>
 where
-	R: RespondFunction<T>,
 	T: OnionMessageContents
 {
-	messenger_function: R,
 	reply_path: BlindedPath,
+
 	// This phantom Data is used to ensure that we use T in the struct definition
 	// This allow us to ensure at compile time that the received message type and response type will be same
 	_phantom: PhantomData<T>
 }
 
-impl<R, T> Responder<R, T>
+impl<T> Responder<T>
 where
-	R: RespondFunction<T>,
 	T: OnionMessageContents
 {
-    pub fn respond(self, response: T) {
-		self.messenger_function.respond(response, self.reply_path)
+    pub fn respond(self, response: T) -> ResponseInstruction<T>{
+		ResponseInstruction::HaveResponse {
+			response,
+			reply_path: self.reply_path,
+		}
 	}
 }
 
+pub enum ResponseInstruction<T: OnionMessageContents> {
+	HaveResponse {
+		response: T,
+		reply_path: BlindedPath,
+	},
+	NoResponse
+}
+
 /// An [`OnionMessage`] for [`OnionMessenger`] to send.
 ///
 /// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
@@ -581,7 +573,7 @@ pub trait CustomOnionMessageHandler {
 	/// Called with the custom message that was received, returning a response to send, if any.
 	///
 	/// The returned [`Self::CustomMessage`], if any, is enqueued to be sent by [`OnionMessenger`].
-	fn handle_custom_message<R: RespondFunction<Self::CustomMessage>>(&self, message: ReceivedOnionMessage<R, Self::CustomMessage>);
+	fn handle_custom_message(&self, message: ReceivedOnionMessage<Self::CustomMessage>) -> ResponseInstruction<Self::CustomMessage>;
 
 	/// Read a custom message of type `message_type` from `buffer`, returning `Ok(None)` if the
 	/// message type is unknown.
@@ -910,11 +902,13 @@ where
 	}
 
 	fn handle_onion_message_response<T: OnionMessageContents>(
-		&self, response: T, reply_path: BlindedPath, log_suffix: fmt::Arguments
+		&self, response: ResponseInstruction<T>, log_suffix: fmt::Arguments
 	) {
-		let _ = self.find_path_and_enqueue_onion_message(
-			response, Destination::BlindedPath(reply_path), None, log_suffix
-		);
+		if let ResponseInstruction::HaveResponse { response, reply_path } = response {
+			let _ = self.find_path_and_enqueue_onion_message(
+				response, Destination::BlindedPath(reply_path), None, log_suffix
+			);
+		}
 	}
 
 	#[cfg(test)]
@@ -995,30 +989,22 @@ where
 				let message_type = message.msg_type();
 				match message {
 					ParsedOnionMessageContents::Offers(msg) => {
-						let closure = |response, reply_path: BlindedPath| {
-							self.handle_onion_message_response(
-								response, reply_path, format_args!(
-									"when responding to {} onion message with path_id {:02x?}",
-									message_type,
-									path_id
-								)
-							);
-						};
-						let message = ReceivedOnionMessage::new(closure, msg, reply_path, path_id);
-						self.offers_handler.handle_message(message);
+						let received_message = ReceivedOnionMessage::new(msg, reply_path, path_id);
+						let response_instructions = self.offers_handler.handle_message(received_message);
+						self.handle_onion_message_response(response_instructions, format_args!(
+							"when responding to {} onion message with path_id {:02x?}",
+							message_type,
+							path_id
+						))
 					},
 					ParsedOnionMessageContents::Custom(msg) => {
-						let closure = |response, reply_path: BlindedPath| {
-							self.handle_onion_message_response(
-								response, reply_path, format_args!(
-									"when responding to {} onion message with path_id {:02x?}",
-									message_type,
-									path_id
-								)
-							);
-						};
-						let message = ReceivedOnionMessage::new(closure, msg, reply_path, path_id);
-						self.custom_handler.handle_custom_message(message);
+						let message = ReceivedOnionMessage::new(msg, reply_path, path_id);
+						let response_instructions = self.custom_handler.handle_custom_message(message);
+						self.handle_onion_message_response(response_instructions, format_args!(
+							"when responding to {} onion message with path_id {:02x?}",
+							message_type,
+							path_id
+						))
 					},
 				}
 			},

lightning/src/onion_message/offers.rs

@@ -21,7 +21,7 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::util::logger::Logger;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
 #[cfg(not(c_bindings))]
-use crate::onion_message::messenger::{PendingOnionMessage, ReceivedOnionMessage, RespondFunction};
+use crate::onion_message::messenger::{PendingOnionMessage, ReceivedOnionMessage, ResponseInstruction};
 
 use crate::prelude::*;
 
@@ -40,7 +40,7 @@ pub trait OffersMessageHandler {
 	/// The returned [`OffersMessage`], if any, is enqueued to be sent by [`OnionMessenger`].
 	///
 	/// [`OnionMessenger`]: crate::onion_message::messenger::OnionMessenger
-	fn handle_message<R: RespondFunction<OffersMessage>>(&self, message: ReceivedOnionMessage<R, OffersMessage>);
+	fn handle_message(&self, message: ReceivedOnionMessage<OffersMessage>) -> ResponseInstruction<OffersMessage>;
 
 	/// Releases any [`OffersMessage`]s that need to be sent.
 	///