Implement ResponseInstruction Usage in OnionMessage Handling

This commit integrates the newly introduced ResponseInstruction structs
and enums into the codebase for handling OnionMessage responses.

Author: shaavan

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;
 
@@ -96,8 +96,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
 	}
 }
 
@@ -111,6 +111,9 @@ impl OnionMessageContents for TestCustomMessage {
 	fn tlv_type(&self) -> u64 {
 		CUSTOM_MESSAGE_TYPE
 	}
+	fn msg_type() -> &'static str {
+		"Custom"
+	}
 }
 
 impl Writeable for TestCustomMessage {
@@ -123,8 +126,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();

lightning/src/ln/channelmanager.rs

@@ -65,7 +65,7 @@ use crate::offers::merkle::SignError;
 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,
@@ -9450,23 +9451,30 @@ 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()));
 					},
 				};
 
@@ -9477,8 +9485,8 @@ 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()));
+					}
 				};
 
 				let payment_paths = match self.create_blinded_payment_paths(
@@ -9487,8 +9495,8 @@ where
 					Ok(payment_paths) => payment_paths,
 					Err(()) => {
 						let error = Bolt12SemanticError::MissingPaths;
-						return Some(OffersMessage::InvoiceError(error.into()));
-					},
+						return responder.respond(OffersMessage::InvoiceError(error.into()));
+					}
 				};
 
 				#[cfg(not(feature = "std"))]
@@ -9508,8 +9516,8 @@ where
 					let builder: Result<InvoiceBuilder<DerivedSigningPubkey>, _> =
 						builder.map(|b| b.into());
 					match builder.and_then(|b| b.allow_mpp().build_and_sign(secp_ctx)) {
-						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())),
 					}
 				} else {
 					#[cfg(feature = "std")]
@@ -9538,13 +9546,14 @@ where
 							}
 						});
 					match response {
-						Ok(invoice) => Some(invoice),
-						Err(error) => Some(error),
+						Ok(invoice) => return responder.respond(invoice),
+						Err(error) => return responder.respond(error),
 					}
 				}
 			},
+
 			OffersMessage::Invoice(invoice) => {
-				match invoice.verify(expanded_key, secp_ctx) {
+				let response = match invoice.verify(expanded_key, secp_ctx) {
 					Err(()) => {
 						Some(OffersMessage::InvoiceError(InvoiceError::from_string("Unrecognized invoice".to_owned())))
 					},
@@ -9559,11 +9568,24 @@ where
 							None
 						}
 					},
+				};
+
+				match (responder, response) {
+					(Some(responder), Some(response)) => return responder.respond(response),
+					(None, Some(_)) => {
+						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};
@@ -120,6 +120,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 }
@@ -131,12 +132,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!();
 	}
@@ -150,6 +154,7 @@ impl CustomOnionMessageHandler for IgnoringMessageHandler {
 
 impl OnionMessageContents for Infallible {
 	fn tlv_type(&self) -> u64 { unreachable!(); }
+	fn msg_type() -> &'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};
 
@@ -61,8 +61,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
 	}
 }
 
@@ -84,6 +84,9 @@ impl OnionMessageContents for TestCustomMessage {
 			TestCustomMessage::Response => CUSTOM_RESPONSE_MESSAGE_TYPE,
 		}
 	}
+	fn msg_type() -> &'static str {
+		"Custom"
+	}
 }
 
 impl Writeable for TestCustomMessage {
@@ -122,15 +125,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 {
@@ -420,6 +427,9 @@ fn invalid_custom_message_type() {
 			// Onion message contents must have a TLV >= 64.
 			63
 		}
+		fn msg_type() -> &'static str {
+			"Invalid"
+		}
 	}
 
 	impl Writeable for InvalidCustomMessage {

lightning/src/onion_message/messenger.rs

@@ -134,6 +134,7 @@ pub(super) const MAX_TIMER_TICKS: usize = 2;
 /// 		# let your_custom_message_type = 42;
 /// 		your_custom_message_type
 /// 	}
+/// 	fn msg_type() -> &'static str { "YourCustomMessageType" }
 /// }
 /// // Send a custom onion message to a node id.
 /// let destination = Destination::Node(destination_node_id);
@@ -272,7 +273,6 @@ impl Responder {
 }
 
 /// This struct contains the information needed to reply to a received message.
-#[allow(unused)]
 pub struct OnionMessageResponse<T: OnionMessageContents> {
 	message: T,
 	reply_path: BlindedPath,
@@ -557,7 +557,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.
@@ -893,19 +893,17 @@ 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 _ = self.find_path_and_enqueue_onion_message(
+				response.message, Destination::BlindedPath(response.reply_path), None,
+				format_args!(
+					"when responding to {} onion message with path_id {:02x?}",
+					T::msg_type(),
+					response.path_id
+				)
+			);
 		}
 	}
 
@@ -987,22 +985,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);
 					},
 				}
 			},

lightning/src/onion_message/offers.rs

@@ -20,6 +20,7 @@ use crate::offers::parse::Bolt12ParseError;
 use crate::onion_message::packet::OnionMessageContents;
 use crate::util::logger::Logger;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
+use crate::onion_message::messenger::{ResponseInstruction, Responder};
 #[cfg(not(c_bindings))]
 use crate::onion_message::messenger::PendingOnionMessage;
 
@@ -40,7 +41,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(&self, message: OffersMessage) -> Option<OffersMessage>;
+	fn handle_message(&self, message: OffersMessage, responder: Option<Responder>) -> ResponseInstruction<OffersMessage>;
 
 	/// Releases any [`OffersMessage`]s that need to be sent.
 	///
@@ -118,6 +119,9 @@ impl OnionMessageContents for OffersMessage {
 			OffersMessage::InvoiceError(_) => INVOICE_ERROR_TLV_TYPE,
 		}
 	}
+	fn msg_type() -> &'static str {
+		"Offers"
+	}
 }
 
 impl Writeable for OffersMessage {