Merge pull request #2907 from shaavan/issue2882

Introduce ResponseInstructions for OnionMessage Handling

Author: jkczyz

fuzz/src/onion_message.rs

@@ -16,7 +16,7 @@ use lightning::sign::{Recipient, KeyMaterial, EntropySource, NodeSigner, SignerP
 use lightning::util::test_channel_signer::TestChannelSigner;
 use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, Writeable, Writer};
-use lightning::onion_message::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage};
+use lightning::onion_message::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, Responder, ResponseInstruction};
 use lightning::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use lightning::onion_message::packet::OnionMessageContents;
 
@@ -97,8 +97,8 @@ impl MessageRouter for TestMessageRouter {
 struct TestOffersMessageHandler {}
 
 impl OffersMessageHandler for TestOffersMessageHandler {
-	fn handle_message(&self, _message: OffersMessage) -> Option<OffersMessage> {
-		None
+	fn handle_message(&self, _message: OffersMessage, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
+		ResponseInstruction::NoResponse
 	}
 }
 
@@ -112,6 +112,9 @@ impl OnionMessageContents for TestCustomMessage {
 	fn tlv_type(&self) -> u64 {
 		CUSTOM_MESSAGE_TYPE
 	}
+	fn msg_type(&self) -> &'static str {
+		"Custom Message"
+	}
 }
 
 impl Writeable for TestCustomMessage {
@@ -124,8 +127,11 @@ struct TestCustomMessageHandler {}
 
 impl CustomOnionMessageHandler for TestCustomMessageHandler {
 	type CustomMessage = TestCustomMessage;
-	fn handle_custom_message(&self, _msg: Self::CustomMessage) -> Option<Self::CustomMessage> {
-		Some(TestCustomMessage {})
+	fn handle_custom_message(&self, message: Self::CustomMessage, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
+		match responder {
+			Some(responder) => responder.respond(message),
+			None => ResponseInstruction::NoResponse
+		}
 	}
 	fn read_custom_message<R: io::Read>(&self, _message_type: u64, buffer: &mut R) -> Result<Option<Self::CustomMessage>, msgs::DecodeError> {
 		let mut buf = Vec::new();
@@ -280,9 +286,9 @@ mod tests {
 						"Received an onion message with path_id None and a reply_path: Custom(TestCustomMessage)"
 						.to_string())), Some(&1));
 			assert_eq!(log_entries.get(&("lightning::onion_message::messenger".to_string(),
-						"Constructing onion message when responding to Custom onion message with path_id None: TestCustomMessage".to_string())), Some(&1));
+						"Constructing onion message when responding with Custom Message to an onion message with path_id None: TestCustomMessage".to_string())), Some(&1));
 			assert_eq!(log_entries.get(&("lightning::onion_message::messenger".to_string(),
-						"Buffered onion message when responding to Custom onion message with path_id None".to_string())), Some(&1));
+						"Buffered onion message when responding with Custom Message to an onion message with path_id None".to_string())), Some(&1));
 		}
 
 		let two_unblinded_hops_om = "\

lightning/src/ln/channelmanager.rs

@@ -65,7 +65,7 @@ use crate::offers::invoice_request::{DerivedPayerId, InvoiceRequestBuilder};
 use crate::offers::offer::{Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
-use crate::onion_message::messenger::{Destination, MessageRouter, PendingOnionMessage, new_pending_onion_message};
+use crate::onion_message::messenger::{new_pending_onion_message, Destination, MessageRouter, PendingOnionMessage, Responder, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
 use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
@@ -76,6 +76,7 @@ use crate::util::string::UntrustedString;
 use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::errors::APIError;
+
 #[cfg(not(c_bindings))]
 use {
 	crate::offers::offer::DerivedMetadata,
@@ -10349,23 +10350,27 @@ where
 	R::Target: Router,
 	L::Target: Logger,
 {
-	fn handle_message(&self, message: OffersMessage) -> Option<OffersMessage> {
+	fn handle_message(&self, message: OffersMessage, responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
 		let secp_ctx = &self.secp_ctx;
 		let expanded_key = &self.inbound_payment_key;
 
 		match message {
 			OffersMessage::InvoiceRequest(invoice_request) => {
+				let responder = match responder {
+					Some(responder) => responder,
+					None => return ResponseInstruction::NoResponse,
+				};
 				let amount_msats = match InvoiceBuilder::<DerivedSigningPubkey>::amount_msats(
 					&invoice_request
 				) {
 					Ok(amount_msats) => amount_msats,
-					Err(error) => return Some(OffersMessage::InvoiceError(error.into())),
+					Err(error) => return responder.respond(OffersMessage::InvoiceError(error.into())),
 				};
 				let invoice_request = match invoice_request.verify(expanded_key, secp_ctx) {
 					Ok(invoice_request) => invoice_request,
 					Err(()) => {
 						let error = Bolt12SemanticError::InvalidMetadata;
-						return Some(OffersMessage::InvoiceError(error.into()));
+						return responder.respond(OffersMessage::InvoiceError(error.into()));
 					},
 				};
 
@@ -10376,7 +10381,7 @@ where
 					Ok((payment_hash, payment_secret)) => (payment_hash, payment_secret),
 					Err(()) => {
 						let error = Bolt12SemanticError::InvalidAmount;
-						return Some(OffersMessage::InvoiceError(error.into()));
+						return responder.respond(OffersMessage::InvoiceError(error.into()));
 					},
 				};
 
@@ -10390,7 +10395,7 @@ where
 					Ok(payment_paths) => payment_paths,
 					Err(()) => {
 						let error = Bolt12SemanticError::MissingPaths;
-						return Some(OffersMessage::InvoiceError(error.into()));
+						return responder.respond(OffersMessage::InvoiceError(error.into()));
 					},
 				};
 
@@ -10435,8 +10440,8 @@ where
 				};
 
 				match response {
-					Ok(invoice) => Some(OffersMessage::Invoice(invoice)),
-					Err(error) => Some(OffersMessage::InvoiceError(error.into())),
+					Ok(invoice) => return responder.respond(OffersMessage::Invoice(invoice)),
+					Err(error) => return responder.respond(OffersMessage::InvoiceError(error.into())),
 				}
 			},
 			OffersMessage::Invoice(invoice) => {
@@ -10456,14 +10461,21 @@ where
 						}
 					});
 
-				match response {
-					Ok(()) => None,
-					Err(e) => Some(OffersMessage::InvoiceError(e)),
+				match (responder, response) {
+					(Some(responder), Err(e)) => responder.respond(OffersMessage::InvoiceError(e)),
+					(None, Err(_)) => {
+						log_trace!(
+							self.logger,
+							"A response was generated, but there is no reply_path specified for sending the response."
+						);
+						return ResponseInstruction::NoResponse;
+					}
+					_ => return ResponseInstruction::NoResponse,
 				}
 			},
 			OffersMessage::InvoiceError(invoice_error) => {
 				log_trace!(self.logger, "Received invoice_error: {}", invoice_error);
-				None
+				return 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};
+use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage, Responder, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeId, NodeAlias};
@@ -123,6 +123,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler {
 	}
 	fn processing_queue_high(&self) -> bool { false }
 }
+
 impl OnionMessageHandler for IgnoringMessageHandler {
 	fn handle_onion_message(&self, _their_node_id: &PublicKey, _msg: &msgs::OnionMessage) {}
 	fn next_onion_message_for_peer(&self, _peer_node_id: PublicKey) -> Option<msgs::OnionMessage> { None }
@@ -134,12 +135,15 @@ impl OnionMessageHandler for IgnoringMessageHandler {
 		InitFeatures::empty()
 	}
 }
+
 impl OffersMessageHandler for IgnoringMessageHandler {
-	fn handle_message(&self, _msg: OffersMessage) -> Option<OffersMessage> { None }
+	fn handle_message(&self, _message: OffersMessage, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
+		ResponseInstruction::NoResponse
+	}
 }
 impl CustomOnionMessageHandler for IgnoringMessageHandler {
 	type CustomMessage = Infallible;
-	fn handle_custom_message(&self, _msg: Infallible) -> Option<Infallible> {
+	fn handle_custom_message(&self, _message: Self::CustomMessage, _responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
 		// Since we always return `None` in the read the handle method should never be called.
 		unreachable!();
 	}
@@ -153,6 +157,7 @@ impl CustomOnionMessageHandler for IgnoringMessageHandler {
 
 impl OnionMessageContents for Infallible {
 	fn tlv_type(&self) -> u64 { unreachable!(); }
+	fn msg_type(&self) -> &'static str { unreachable!(); }
 }
 
 impl Deref for IgnoringMessageHandler {

lightning/src/onion_message/functional_tests.rs

@@ -18,7 +18,7 @@ use crate::routing::test_utils::{add_channel, add_or_update_node};
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
-use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError};
+use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, Responder, ResponseInstruction, SendError};
 use super::offers::{OffersMessage, OffersMessageHandler};
 use super::packet::{OnionMessageContents, Packet};
 
@@ -62,8 +62,8 @@ struct MessengerNode {
 struct TestOffersMessageHandler {}
 
 impl OffersMessageHandler for TestOffersMessageHandler {
-	fn handle_message(&self, _message: OffersMessage) -> Option<OffersMessage> {
-		None
+	fn handle_message(&self, _message: OffersMessage, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
+		ResponseInstruction::NoResponse
 	}
 }
 
@@ -85,6 +85,9 @@ impl OnionMessageContents for TestCustomMessage {
 			TestCustomMessage::Response => CUSTOM_RESPONSE_MESSAGE_TYPE,
 		}
 	}
+	fn msg_type(&self) -> &'static str {
+		"Custom Message"
+	}
 }
 
 impl Writeable for TestCustomMessage {
@@ -123,15 +126,19 @@ impl Drop for TestCustomMessageHandler {
 
 impl CustomOnionMessageHandler for TestCustomMessageHandler {
 	type CustomMessage = TestCustomMessage;
-	fn handle_custom_message(&self, msg: Self::CustomMessage) -> Option<Self::CustomMessage> {
+	fn handle_custom_message(&self, msg: Self::CustomMessage, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
 		match self.expected_messages.lock().unwrap().pop_front() {
 			Some(expected_msg) => assert_eq!(expected_msg, msg),
 			None => panic!("Unexpected message: {:?}", msg),
 		}
-
-		match msg {
+		let response_option = match msg {
 			TestCustomMessage::Request => Some(TestCustomMessage::Response),
 			TestCustomMessage::Response => None,
+		};
+		if let (Some(response), Some(responder)) = (response_option, responder) {
+			responder.respond(response)
+		} 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 {
@@ -422,6 +429,9 @@ fn invalid_custom_message_type() {
 			// Onion message contents must have a TLV >= 64.
 			63
 		}
+		fn msg_type(&self) -> &'static str {
+			"Invalid Message"
+		}
 	}
 
 	impl Writeable for InvalidCustomMessage {

lightning/src/onion_message/messenger.rs

@@ -135,6 +135,7 @@ pub(super) const MAX_TIMER_TICKS: usize = 2;
 /// 		# let your_custom_message_type = 42;
 /// 		your_custom_message_type
 /// 	}
+/// 	fn msg_type(&self) -> &'static str { "YourCustomMessageType" }
 /// }
 /// // Send a custom onion message to a node id.
 /// let destination = Destination::Node(destination_node_id);
@@ -246,6 +247,50 @@ impl OnionMessageRecipient {
 	}
 }
 
+
+/// The `Responder` struct creates an appropriate [`ResponseInstruction`]
+/// for responding to a message.
+pub struct Responder {
+	/// The path along which a response can be sent.
+	reply_path: BlindedPath,
+	path_id: Option<[u8; 32]>
+}
+
+impl Responder {
+	/// Creates a new [`Responder`] instance with the provided reply path.
+	fn new(reply_path: BlindedPath, path_id: Option<[u8; 32]>) -> Self {
+		Responder {
+			reply_path,
+			path_id,
+		}
+	}
+
+	/// Creates the appropriate [`ResponseInstruction`] for a given response.
+	pub fn respond<T: OnionMessageContents>(self, response: T) -> ResponseInstruction<T> {
+		ResponseInstruction::WithoutReplyPath(OnionMessageResponse {
+			message: response,
+			reply_path: self.reply_path,
+			path_id: self.path_id,
+		})
+	}
+}
+
+/// This struct contains the information needed to reply to a received message.
+pub struct OnionMessageResponse<T: OnionMessageContents> {
+	message: T,
+	reply_path: BlindedPath,
+	path_id: Option<[u8; 32]>,
+}
+
+/// `ResponseInstruction` represents instructions for responding to received messages.
+pub enum ResponseInstruction<T: OnionMessageContents> {
+	/// Indicates that a response should be sent without including a reply path
+	/// for the recipient to respond back.
+	WithoutReplyPath(OnionMessageResponse<T>),
+	/// Indicates that there's no response to send back.
+	NoResponse,
+}
+
 /// An [`OnionMessage`] for [`OnionMessenger`] to send.
 ///
 /// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
@@ -546,7 +591,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(&self, msg: Self::CustomMessage) -> Option<Self::CustomMessage>;
+	fn handle_custom_message(&self, message: Self::CustomMessage, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage>;
 
 	/// Read a custom message of type `message_type` from `buffer`, returning `Ok(None)` if the
 	/// message type is unknown.
@@ -933,19 +978,18 @@ where
 	}
 
 	fn handle_onion_message_response<T: OnionMessageContents>(
-		&self, response: Option<T>, reply_path: Option<BlindedPath>, log_suffix: fmt::Arguments
+		&self, response: ResponseInstruction<T>
 	) {
-		if let Some(response) = response {
-			match reply_path {
-				Some(reply_path) => {
-					let _ = self.find_path_and_enqueue_onion_message(
-						response, Destination::BlindedPath(reply_path), None, log_suffix
-					);
-				},
-				None => {
-					log_trace!(self.logger, "Missing reply path {}", log_suffix);
-				},
-			}
+		if let ResponseInstruction::WithoutReplyPath(response) = response {
+			let message_type = response.message.msg_type();
+			let _ = self.find_path_and_enqueue_onion_message(
+				response.message, Destination::BlindedPath(response.reply_path), None,
+				format_args!(
+					"when responding with {} to an onion message with path_id {:02x?}",
+					message_type,
+					response.path_id
+				)
+			);
 		}
 	}
 
@@ -1029,22 +1073,18 @@ where
 
 				match message {
 					ParsedOnionMessageContents::Offers(msg) => {
-						let response = self.offers_handler.handle_message(msg);
-						self.handle_onion_message_response(
-							response, reply_path, format_args!(
-								"when responding to Offers onion message with path_id {:02x?}",
-								path_id
-							)
+						let responder = reply_path.map(
+							|reply_path| Responder::new(reply_path, path_id)
 						);
+						let response_instructions = self.offers_handler.handle_message(msg, responder);
+						self.handle_onion_message_response(response_instructions);
 					},
 					ParsedOnionMessageContents::Custom(msg) => {
-						let response = self.custom_handler.handle_custom_message(msg);
-						self.handle_onion_message_response(
-							response, reply_path, format_args!(
-								"when responding to Custom onion message with path_id {:02x?}",
-								path_id
-							)
+						let responder = reply_path.map(
+							|reply_path| Responder::new(reply_path, path_id)
 						);
+						let response_instructions = self.custom_handler.handle_custom_message(msg, responder);
+						self.handle_onion_message_response(response_instructions);
 					},
 				}
 			},