From e640951f9532a0b90099e04bca03db37e115ee1c Mon Sep 17 00:00:00 2001 From: shaavan Date: Mon, 25 Mar 2024 14:12:47 +0530 Subject: [PATCH 1/2] Introduce ResponseInstructions for OnionMessage Handling - Currently, handle_message (or handle_custom_message) only exposes the generated response for an OnionMessage, lacking the necessary reply_path for asynchronous responses. - This commit introduces a new flow for OnionMessage handling. - Instead of solely taking the message as input, handle_message now accepts an Optional `responder`, returning a `ResponseInstruction` containing both the response and the reply_path. - `ResponseInstruction` utilizes different enum variants to indicate how `handle_onion_message_response` should handle the response. - This enhancement enables exposing the reply_path alongside the response and allows for more complex response mechanisms, such as responding with an added reply_path. - The commit introduces the foundational framework (structs & enums) for this new flow. --- lightning/src/onion_message/messenger.rs | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 1d7a730fa36..647239743c3 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -246,6 +246,51 @@ 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(self, response: T) -> ResponseInstruction { + 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. +#[allow(unused)] +pub struct OnionMessageResponse { + message: T, + reply_path: BlindedPath, + path_id: Option<[u8; 32]>, +} + +/// `ResponseInstruction` represents instructions for responding to received messages. +pub enum ResponseInstruction { + /// Indicates that a response should be sent without including a reply path + /// for the recipient to respond back. + WithoutReplyPath(OnionMessageResponse), + /// 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 From 15d016ac520077b01604f2ede57be83d65e08e59 Mon Sep 17 00:00:00 2001 From: shaavan Date: Thu, 18 Apr 2024 17:01:52 +0530 Subject: [PATCH 2/2] Implement ResponseInstruction Usage in OnionMessage Handling This commit integrates the newly introduced ResponseInstruction structs and enums into the codebase for handling OnionMessage responses. --- fuzz/src/onion_message.rs | 20 +++++--- lightning/src/ln/channelmanager.rs | 36 +++++++++----- lightning/src/ln/peer_handler.rs | 11 +++-- .../src/onion_message/functional_tests.rs | 22 ++++++--- lightning/src/onion_message/messenger.rs | 47 +++++++++---------- lightning/src/onion_message/offers.rs | 10 +++- lightning/src/onion_message/packet.rs | 9 ++++ 7 files changed, 100 insertions(+), 55 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 91fcb9bf2d4..19ba4cb6aac 100644 --- a/fuzz/src/onion_message.rs +++ b/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 { - None + fn handle_message(&self, _message: OffersMessage, _responder: Option) -> ResponseInstruction { + 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 { - Some(TestCustomMessage {}) + fn handle_custom_message(&self, message: Self::CustomMessage, responder: Option) -> ResponseInstruction { + match responder { + Some(responder) => responder.respond(message), + None => ResponseInstruction::NoResponse + } } fn read_custom_message(&self, _message_type: u64, buffer: &mut R) -> Result, 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 = "\ diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 51555c7a0ac..f33df2bd797 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -64,7 +64,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; @@ -75,6 +75,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, @@ -10332,23 +10333,27 @@ where R::Target: Router, L::Target: Logger, { - fn handle_message(&self, message: OffersMessage) -> Option { + fn handle_message(&self, message: OffersMessage, responder: Option) -> ResponseInstruction { 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::::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())); }, }; @@ -10359,7 +10364,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())); }, }; @@ -10373,7 +10378,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())); }, }; @@ -10418,8 +10423,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) => { @@ -10439,14 +10444,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; }, } } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 9ce23086145..0413cc0ed68 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/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 { None } @@ -134,12 +135,15 @@ impl OnionMessageHandler for IgnoringMessageHandler { InitFeatures::empty() } } + impl OffersMessageHandler for IgnoringMessageHandler { - fn handle_message(&self, _msg: OffersMessage) -> Option { None } + fn handle_message(&self, _message: OffersMessage, _responder: Option) -> ResponseInstruction { + ResponseInstruction::NoResponse + } } impl CustomOnionMessageHandler for IgnoringMessageHandler { type CustomMessage = Infallible; - fn handle_custom_message(&self, _msg: Infallible) -> Option { + fn handle_custom_message(&self, _message: Self::CustomMessage, _responder: Option) -> ResponseInstruction { // 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 { diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index acf34a3a8c8..34cc0c39593 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/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 { - None + fn handle_message(&self, _message: OffersMessage, _responder: Option) -> ResponseInstruction { + 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 { + fn handle_custom_message(&self, msg: Self::CustomMessage, responder: Option) -> ResponseInstruction { 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(&self, message_type: u64, buffer: &mut R) -> Result, 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 { diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 647239743c3..e87229d66ad 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/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); @@ -275,7 +276,6 @@ impl Responder { } /// This struct contains the information needed to reply to a received message. -#[allow(unused)] pub struct OnionMessageResponse { message: T, reply_path: BlindedPath, @@ -591,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; + fn handle_custom_message(&self, message: Self::CustomMessage, responder: Option) -> ResponseInstruction; /// Read a custom message of type `message_type` from `buffer`, returning `Ok(None)` if the /// message type is unknown. @@ -978,19 +978,18 @@ where } fn handle_onion_message_response( - &self, response: Option, reply_path: Option, log_suffix: fmt::Arguments + &self, response: ResponseInstruction ) { - 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 + ) + ); } } @@ -1074,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); }, } }, diff --git a/lightning/src/onion_message/offers.rs b/lightning/src/onion_message/offers.rs index faf539f8682..42c6914157e 100644 --- a/lightning/src/onion_message/offers.rs +++ b/lightning/src/onion_message/offers.rs @@ -19,6 +19,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; @@ -39,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(&self, message: OffersMessage) -> Option; + fn handle_message(&self, message: OffersMessage, responder: Option) -> ResponseInstruction; /// Releases any [`OffersMessage`]s that need to be sent. /// @@ -117,6 +118,13 @@ impl OnionMessageContents for OffersMessage { OffersMessage::InvoiceError(_) => INVOICE_ERROR_TLV_TYPE, } } + fn msg_type(&self) -> &'static str { + match &self { + OffersMessage::InvoiceRequest(_) => "Invoice Request", + OffersMessage::Invoice(_) => "Invoice", + OffersMessage::InvoiceError(_) => "Invoice Error", + } + } } impl Writeable for OffersMessage { diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index 510f0ea025a..bbf04ca3ef4 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -135,6 +135,12 @@ impl OnionMessageContents for ParsedOnionMessageContent &ParsedOnionMessageContents::Custom(ref msg) => msg.tlv_type(), } } + fn msg_type(&self) -> &'static str { + match self { + ParsedOnionMessageContents::Offers(ref msg) => msg.msg_type(), + ParsedOnionMessageContents::Custom(ref msg) => msg.msg_type(), + } + } } impl Writeable for ParsedOnionMessageContents { @@ -150,6 +156,9 @@ impl Writeable for ParsedOnionMessageContents { pub trait OnionMessageContents: Writeable + core::fmt::Debug { /// Returns the TLV type identifying the message contents. MUST be >= 64. fn tlv_type(&self) -> u64; + + /// Returns the message type + fn msg_type(&self) -> &'static str; } /// Forward control TLVs in their blinded and unblinded form.