From cd93a41bd690373933d42e9b78e4b16c75be9f93 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 21 Aug 2024 15:32:14 +0000 Subject: [PATCH 1/9] Remove message type bound on `ResponseInstruction` Our onion message handlers generally have one or more methods which return a `ResponseInstruction` parameterized with the expected message type (enum) of the message handler. Sadly, that restriction is impossible to represent in our bindings - our bindings concretize all LDK structs, enums, and traits into a single concrete instance with generics set to our concrete trait instances (which hold a jump table). This prevents us from having multiple instances of `ResponseInstruction` structs for different message types. Our bindings do, however, support different parameterizations of standard enums, including `Option`s and tuples. In order to support bindings for the onion message handlers, we are thus forced into std types bound by expected message types, which we do here by making `ResponseInstruction` contain only the instructions and generally using it as a two-tuple of `(message, ResponseInstruction)`. --- fuzz/src/onion_message.rs | 20 ++--- lightning/src/ln/channelmanager.rs | 44 +++++----- lightning/src/ln/peer_handler.rs | 10 +-- lightning/src/onion_message/async_payments.rs | 2 +- .../src/onion_message/functional_tests.rs | 25 +++--- lightning/src/onion_message/messenger.rs | 80 ++++++++++--------- lightning/src/onion_message/offers.rs | 2 +- 7 files changed, 97 insertions(+), 86 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 0de791c555a..97e8ae24306 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -109,8 +109,8 @@ impl OffersMessageHandler for TestOffersMessageHandler { fn handle_message( &self, _message: OffersMessage, _context: Option, _responder: Option, - ) -> ResponseInstruction { - ResponseInstruction::NoResponse + ) -> Option<(OffersMessage, ResponseInstruction)> { + None } } @@ -119,13 +119,15 @@ struct TestAsyncPaymentsMessageHandler {} impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler { fn held_htlc_available( &self, message: HeldHtlcAvailable, responder: Option, - ) -> ResponseInstruction { + ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { let responder = match responder { Some(resp) => resp, - None => return ResponseInstruction::NoResponse, + None => return None, }; - responder - .respond(ReleaseHeldHtlc { payment_release_secret: message.payment_release_secret }) + Some(( + ReleaseHeldHtlc { payment_release_secret: message.payment_release_secret }, + responder.respond(), + )) } fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {} } @@ -158,10 +160,10 @@ impl CustomOnionMessageHandler for TestCustomMessageHandler { fn handle_custom_message( &self, message: Self::CustomMessage, _context: Option>, responder: Option, - ) -> ResponseInstruction { + ) -> Option<(Self::CustomMessage, ResponseInstruction)> { match responder { - Some(responder) => responder.respond(message), - None => ResponseInstruction::NoResponse, + Some(responder) => Some((message, responder.respond())), + None => None, } } fn read_custom_message( diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 167ab0ac8fd..b6ae70836ba 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10749,7 +10749,7 @@ where { fn handle_message( &self, message: OffersMessage, context: Option, responder: Option, - ) -> ResponseInstruction { + ) -> Option<(OffersMessage, ResponseInstruction)> { let secp_ctx = &self.secp_ctx; let expanded_key = &self.inbound_payment_key; @@ -10757,13 +10757,13 @@ where OffersMessage::InvoiceRequest(invoice_request) => { let responder = match responder { Some(responder) => responder, - None => return ResponseInstruction::NoResponse, + None => return None, }; let nonce = match context { None if invoice_request.metadata().is_some() => None, Some(OffersContext::InvoiceRequest { nonce }) => Some(nonce), - _ => return ResponseInstruction::NoResponse, + _ => return None, }; let invoice_request = match nonce { @@ -10771,11 +10771,11 @@ where nonce, expanded_key, secp_ctx, ) { Ok(invoice_request) => invoice_request, - Err(()) => return ResponseInstruction::NoResponse, + Err(()) => return None, }, None => match invoice_request.verify_using_metadata(expanded_key, secp_ctx) { Ok(invoice_request) => invoice_request, - Err(()) => return ResponseInstruction::NoResponse, + Err(()) => return None, }, }; @@ -10783,7 +10783,7 @@ where &invoice_request.inner ) { Ok(amount_msats) => amount_msats, - Err(error) => return responder.respond(OffersMessage::InvoiceError(error.into())), + Err(error) => return Some((OffersMessage::InvoiceError(error.into()), responder.respond())), }; let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; @@ -10793,7 +10793,7 @@ where Ok((payment_hash, payment_secret)) => (payment_hash, payment_secret), Err(()) => { let error = Bolt12SemanticError::InvalidAmount; - return responder.respond(OffersMessage::InvoiceError(error.into())); + return Some((OffersMessage::InvoiceError(error.into()), responder.respond())); }, }; @@ -10807,7 +10807,7 @@ where Ok(payment_paths) => payment_paths, Err(()) => { let error = Bolt12SemanticError::MissingPaths; - return responder.respond(OffersMessage::InvoiceError(error.into())); + return Some((OffersMessage::InvoiceError(error.into()), responder.respond())); }, }; @@ -10852,14 +10852,14 @@ where }; match response { - Ok(invoice) => responder.respond(OffersMessage::Invoice(invoice)), - Err(error) => responder.respond(OffersMessage::InvoiceError(error.into())), + Ok(invoice) => Some((OffersMessage::Invoice(invoice), responder.respond())), + Err(error) => Some((OffersMessage::InvoiceError(error.into()), responder.respond())), } }, OffersMessage::Invoice(invoice) => { let payment_id = match self.verify_bolt12_invoice(&invoice, context.as_ref()) { Ok(payment_id) => payment_id, - Err(()) => return ResponseInstruction::NoResponse, + Err(()) => return None, }; let logger = WithContext::from( @@ -10871,7 +10871,7 @@ where payment_id, invoice, context, responder, }; self.pending_events.lock().unwrap().push_back((event, None)); - return ResponseInstruction::NoResponse; + return None; } let error = match self.send_payment_for_verified_bolt12_invoice( @@ -10890,14 +10890,14 @@ where }, Err(Bolt12PaymentError::UnexpectedInvoice) | Err(Bolt12PaymentError::DuplicateInvoice) - | Ok(()) => return ResponseInstruction::NoResponse, + | Ok(()) => return None, }; match responder { - Some(responder) => responder.respond(OffersMessage::InvoiceError(error)), + Some(responder) => Some((OffersMessage::InvoiceError(error), responder.respond())), None => { log_trace!(logger, "No reply path to send error: {:?}", error); - ResponseInstruction::NoResponse + None }, } }, @@ -10905,11 +10905,11 @@ where OffersMessage::StaticInvoice(_invoice) => { match responder { Some(responder) => { - responder.respond(OffersMessage::InvoiceError( - InvoiceError::from_string("Static invoices not yet supported".to_string()) - )) + return Some((OffersMessage::InvoiceError( + InvoiceError::from_string("Static invoices not yet supported".to_string()) + ), responder.respond())); }, - None => return ResponseInstruction::NoResponse, + None => return None, } }, OffersMessage::InvoiceError(invoice_error) => { @@ -10932,7 +10932,7 @@ where _ => {}, } - ResponseInstruction::NoResponse + None }, } } @@ -10956,8 +10956,8 @@ where { fn held_htlc_available( &self, _message: HeldHtlcAvailable, _responder: Option - ) -> ResponseInstruction { - ResponseInstruction::NoResponse + ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { + None } fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {} diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 6b03b365457..bde5454b49c 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -144,21 +144,21 @@ impl OnionMessageHandler for IgnoringMessageHandler { } impl OffersMessageHandler for IgnoringMessageHandler { - fn handle_message(&self, _message: OffersMessage, _context: Option, _responder: Option) -> ResponseInstruction { - ResponseInstruction::NoResponse + fn handle_message(&self, _message: OffersMessage, _context: Option, _responder: Option) -> Option<(OffersMessage, ResponseInstruction)> { + None } } impl AsyncPaymentsMessageHandler for IgnoringMessageHandler { fn held_htlc_available( &self, _message: HeldHtlcAvailable, _responder: Option, - ) -> ResponseInstruction { - ResponseInstruction::NoResponse + ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { + None } fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {} } impl CustomOnionMessageHandler for IgnoringMessageHandler { type CustomMessage = Infallible; - fn handle_custom_message(&self, _message: Self::CustomMessage, _context: Option>, _responder: Option) -> ResponseInstruction { + fn handle_custom_message(&self, _message: Self::CustomMessage, _context: Option>, _responder: Option) -> Option<(Infallible, ResponseInstruction)> { // Since we always return `None` in the read the handle method should never be called. unreachable!(); } diff --git a/lightning/src/onion_message/async_payments.rs b/lightning/src/onion_message/async_payments.rs index 4a080101915..dcc52830566 100644 --- a/lightning/src/onion_message/async_payments.rs +++ b/lightning/src/onion_message/async_payments.rs @@ -30,7 +30,7 @@ pub trait AsyncPaymentsMessageHandler { /// the held funds. fn held_htlc_available( &self, message: HeldHtlcAvailable, responder: Option, - ) -> ResponseInstruction; + ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)>; /// Handle a [`ReleaseHeldHtlc`] message. If authentication of the message succeeds, an HTLC /// should be released to the corresponding payee. diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 508e7982b40..01b90314ba1 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -76,8 +76,8 @@ impl Drop for MessengerNode { struct TestOffersMessageHandler {} impl OffersMessageHandler for TestOffersMessageHandler { - fn handle_message(&self, _message: OffersMessage, _context: Option, _responder: Option) -> ResponseInstruction { - ResponseInstruction::NoResponse + fn handle_message(&self, _message: OffersMessage, _context: Option, _responder: Option) -> Option<(OffersMessage, ResponseInstruction)> { + None } } @@ -86,8 +86,8 @@ struct TestAsyncPaymentsMessageHandler {} impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler { fn held_htlc_available( &self, _message: HeldHtlcAvailable, _responder: Option, - ) -> ResponseInstruction { - ResponseInstruction::NoResponse + ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { + None } fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {} } @@ -174,7 +174,7 @@ impl Drop for TestCustomMessageHandler { impl CustomOnionMessageHandler for TestCustomMessageHandler { type CustomMessage = TestCustomMessage; - fn handle_custom_message(&self, msg: Self::CustomMessage, context: Option>, responder: Option) -> ResponseInstruction { + fn handle_custom_message(&self, msg: Self::CustomMessage, context: Option>, responder: Option) -> Option<(Self::CustomMessage, ResponseInstruction)> { let expectation = self.get_next_expectation(); assert_eq!(msg, expectation.expect); @@ -190,10 +190,10 @@ impl CustomOnionMessageHandler for TestCustomMessageHandler { match responder { Some(responder) if expectation.include_reply_path => { - responder.respond_with_reply_path(response, MessageContext::Custom(context.unwrap_or_else(Vec::new))) + Some((response, responder.respond_with_reply_path(MessageContext::Custom(context.unwrap_or_else(Vec::new))))) }, - Some(responder) => responder.respond(response), - None => ResponseInstruction::NoResponse, + Some(responder) => Some((response, responder.respond())), + None => None } } fn read_custom_message(&self, message_type: u64, buffer: &mut R) -> Result, DecodeError> where Self: Sized { @@ -444,8 +444,9 @@ fn async_response_over_one_blinded_hop() { let response_instruction = nodes[0].custom_message_handler.handle_custom_message(message, None, responder); // 6. Simulate Alice asynchronously responding back to Bob with a response. + let (msg, instructions) = response_instruction.unwrap(); assert_eq!( - nodes[0].messenger.handle_onion_message_response(response_instruction), + nodes[0].messenger.handle_onion_message_response(msg, instructions), Ok(Some(SendSuccess::Buffered)), ); @@ -477,8 +478,9 @@ fn async_response_with_reply_path_succeeds() { alice.custom_message_handler.expect_message_and_response(message.clone()); let response_instruction = alice.custom_message_handler.handle_custom_message(message, None, Some(responder)); + let (msg, instructions) = response_instruction.unwrap(); assert_eq!( - alice.messenger.handle_onion_message_response(response_instruction), + alice.messenger.handle_onion_message_response(msg, instructions), Ok(Some(SendSuccess::Buffered)), ); @@ -516,8 +518,9 @@ fn async_response_with_reply_path_fails() { alice.custom_message_handler.expect_message_and_response(message.clone()); let response_instruction = alice.custom_message_handler.handle_custom_message(message, None, Some(responder)); + let (msg, instructions) = response_instruction.unwrap(); assert_eq!( - alice.messenger.handle_onion_message_response(response_instruction), + alice.messenger.handle_onion_message_response(msg, instructions), Err(SendError::PathNotFound), ); } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 89aa4dbce68..3d83e58a8d7 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -365,40 +365,40 @@ impl Responder { /// Creates a [`ResponseInstruction::WithoutReplyPath`] for a given response. /// /// Use when the recipient doesn't need to send back a reply to us. - pub fn respond(self, response: T) -> ResponseInstruction { - ResponseInstruction::WithoutReplyPath(OnionMessageResponse { - message: response, - reply_path: self.reply_path, - }) + pub fn respond(self) -> ResponseInstruction { + ResponseInstruction::WithoutReplyPath { + send_path: self.reply_path, + } } /// Creates a [`ResponseInstruction::WithReplyPath`] for a given response. /// /// Use when the recipient needs to send back a reply to us. - pub fn respond_with_reply_path(self, response: T, context: MessageContext) -> ResponseInstruction { - ResponseInstruction::WithReplyPath(OnionMessageResponse { - message: response, - reply_path: self.reply_path, - }, context) + pub fn respond_with_reply_path(self, context: MessageContext) -> ResponseInstruction { + ResponseInstruction::WithReplyPath { + send_path: self.reply_path, + context: context, + } } } -/// This struct contains the information needed to reply to a received message. -pub struct OnionMessageResponse { - message: T, - reply_path: BlindedMessagePath, -} - /// `ResponseInstruction` represents instructions for responding to received messages. -pub enum ResponseInstruction { +pub enum ResponseInstruction { /// Indicates that a response should be sent including a reply path for /// the recipient to respond back. - WithReplyPath(OnionMessageResponse, MessageContext), + WithReplyPath { + /// The path over which we'll send our reply. + send_path: BlindedMessagePath, + /// The context to include in the reply path we'll give the recipient so they can respond + /// to us. + context: MessageContext, + }, /// 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, + WithoutReplyPath { + /// The path over which we'll send our reply. + send_path: BlindedMessagePath, + } } /// An [`OnionMessage`] for [`OnionMessenger`] to send. @@ -799,7 +799,9 @@ 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, message: Self::CustomMessage, context: Option>, responder: Option) -> ResponseInstruction; + fn handle_custom_message( + &self, message: Self::CustomMessage, context: Option>, responder: Option + ) -> Option<(Self::CustomMessage, ResponseInstruction)>; /// Read a custom message of type `message_type` from `buffer`, returning `Ok(None)` if the /// message type is unknown. @@ -1314,21 +1316,19 @@ where /// enqueueing any response for sending. /// /// This function is useful for asynchronous handling of [`OnionMessage`]s. - /// Handlers have the option to return [`ResponseInstruction::NoResponse`], indicating that - /// no immediate response should be sent. Then, they can transfer the associated [`Responder`] - /// to another task responsible for 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. + /// Handlers have the option to return `None`, indicating that no immediate response should be + /// sent. Then, they can transfer the associated [`Responder`] to another task responsible for + /// 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( - &self, response: ResponseInstruction + &self, response: T, instructions: ResponseInstruction, ) -> Result, SendError> { - let (response, context) = match response { - ResponseInstruction::WithReplyPath(response, context) => (response, Some(context)), - ResponseInstruction::WithoutReplyPath(response) => (response, None), - ResponseInstruction::NoResponse => return Ok(None), + let (response_path, context) = match instructions { + ResponseInstruction::WithReplyPath { send_path, context } => (send_path, Some(context)), + ResponseInstruction::WithoutReplyPath { send_path } => (send_path, None), }; - let message_type = response.message.msg_type(); + let message_type = response.msg_type(); let reply_path = if let Some(context) = context { match self.create_blinded_path(context) { Ok(reply_path) => Some(reply_path), @@ -1344,7 +1344,7 @@ where } else { None }; self.find_path_and_enqueue_onion_message( - response.message, Destination::BlindedPath(response.reply_path), reply_path, + response, Destination::BlindedPath(response_path), reply_path, format_args!( "when responding with {} to an onion message", message_type, @@ -1564,14 +1564,18 @@ where } }; let response_instructions = self.offers_handler.handle_message(msg, context, responder); - let _ = self.handle_onion_message_response(response_instructions); + if let Some((msg, instructions)) = response_instructions { + let _ = self.handle_onion_message_response(msg, instructions); + } }, #[cfg(async_payments)] ParsedOnionMessageContents::AsyncPayments(AsyncPaymentsMessage::HeldHtlcAvailable(msg)) => { let response_instructions = self.async_payments_handler.held_htlc_available( msg, responder ); - let _ = self.handle_onion_message_response(response_instructions); + if let Some((msg, instructions)) = response_instructions { + let _ = self.handle_onion_message_response(msg, instructions); + } }, #[cfg(async_payments)] ParsedOnionMessageContents::AsyncPayments(AsyncPaymentsMessage::ReleaseHeldHtlc(msg)) => { @@ -1587,7 +1591,9 @@ where } }; let response_instructions = self.custom_handler.handle_custom_message(msg, context, responder); - let _ = self.handle_onion_message_response(response_instructions); + if let Some((msg, instructions)) = response_instructions { + let _ = self.handle_onion_message_response(msg, instructions); + } }, } }, diff --git a/lightning/src/onion_message/offers.rs b/lightning/src/onion_message/offers.rs index 28f6d7d6fe7..1042080f8be 100644 --- a/lightning/src/onion_message/offers.rs +++ b/lightning/src/onion_message/offers.rs @@ -47,7 +47,7 @@ pub trait OffersMessageHandler { /// [`OnionMessenger`]: crate::onion_message::messenger::OnionMessenger fn handle_message( &self, message: OffersMessage, context: Option, responder: Option, - ) -> ResponseInstruction; + ) -> Option<(OffersMessage, ResponseInstruction)>; /// Releases any [`OffersMessage`]s that need to be sent. /// From 2695331686f41f1ba75de0692fa1bf2ae0792e3a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 01:35:18 +0000 Subject: [PATCH 2/9] Pull the guts of `ResponseInstruction` into a new enum In the coming commits we'll use the `ResponseInstruction` enum's contents for all messages, allowing message senders to rely on the in-`OnionMessegner` reply path selection logic. In order to do so and avoid users confusing the new `MessageSendInstructions` for `ResponseInstruction`, we leave `ResponseInstruction` as a now-unconstructible struct which wraps a `MessageSendInstructions`. --- lightning/src/onion_message/messenger.rs | 54 ++++++++++++++++-------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 3d83e58a8d7..8df16599eb8 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -342,8 +342,8 @@ impl OnionMessageRecipient { } -/// The `Responder` struct creates an appropriate [`ResponseInstruction`] -/// for responding to a message. +/// The `Responder` struct creates an appropriate [`ResponseInstruction`] for responding to a +/// message. #[derive(Clone, Debug, Eq, PartialEq)] pub struct Responder { /// The path along which a response can be sent. @@ -362,41 +362,59 @@ impl Responder { } } - /// Creates a [`ResponseInstruction::WithoutReplyPath`] for a given response. + /// Creates a [`ResponseInstruction`] for responding without including a reply path. /// /// Use when the recipient doesn't need to send back a reply to us. pub fn respond(self) -> ResponseInstruction { - ResponseInstruction::WithoutReplyPath { + ResponseInstruction { send_path: self.reply_path, + context: None, } } - /// Creates a [`ResponseInstruction::WithReplyPath`] for a given response. + /// Creates a [`ResponseInstruction`] for responding including a reply path. /// /// Use when the recipient needs to send back a reply to us. pub fn respond_with_reply_path(self, context: MessageContext) -> ResponseInstruction { - ResponseInstruction::WithReplyPath { + ResponseInstruction { send_path: self.reply_path, - context: context, + context: Some(context), } } } -/// `ResponseInstruction` represents instructions for responding to received messages. -pub enum ResponseInstruction { - /// Indicates that a response should be sent including a reply path for - /// the recipient to respond back. +/// Instructions for how and where to send the response to an onion message. +pub struct ResponseInstruction { + send_path: BlindedMessagePath, + context: Option, +} + +impl ResponseInstruction { + fn into_instructions(self) -> MessageSendInstructions { + let send_path = self.send_path; + match self.context { + Some(context) => MessageSendInstructions::WithReplyPath { send_path, context }, + None => MessageSendInstructions::WithoutReplyPath { send_path }, + } + } +} + +/// Instructions for how and where to send a message. +#[derive(Clone)] +pub enum MessageSendInstructions { + /// Indicates that a message should be sent including a reply path for the recipient to + /// respond. WithReplyPath { - /// The path over which we'll send our reply. + /// The destination where we need to send our message. send_path: BlindedMessagePath, /// The context to include in the reply path we'll give the recipient so they can respond /// to us. context: MessageContext, }, - /// Indicates that a response should be sent without including a reply path - /// for the recipient to respond back. + /// Indicates that a message should be sent without including a reply path, preventing the + /// recipient from responding. WithoutReplyPath { - /// The path over which we'll send our reply. + /// The destination where we need to send our message. send_path: BlindedMessagePath, } } @@ -1323,9 +1341,9 @@ where pub fn handle_onion_message_response( &self, response: T, instructions: ResponseInstruction, ) -> Result, SendError> { - let (response_path, context) = match instructions { - ResponseInstruction::WithReplyPath { send_path, context } => (send_path, Some(context)), - ResponseInstruction::WithoutReplyPath { send_path } => (send_path, None), + let (response_path, context) = match instructions.into_instructions() { + MessageSendInstructions::WithReplyPath { send_path, context } => (send_path, Some(context)), + MessageSendInstructions::WithoutReplyPath { send_path } => (send_path, None), }; let message_type = response.msg_type(); From 4784bbf7d97d126bd52396b37f2afb20ee674ec8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 21 Aug 2024 16:54:31 +0000 Subject: [PATCH 3/9] Make `ResponseInstruction` `Clone`able --- lightning/src/onion_message/messenger.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 8df16599eb8..7689aed0c27 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -384,6 +384,7 @@ impl Responder { } /// Instructions for how and where to send the response to an onion message. +#[derive(Clone)] pub struct ResponseInstruction { send_path: BlindedMessagePath, context: Option, From b396f0afd1975b48ba4e08f9a87edc0ea53b0c91 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 18:43:33 +0000 Subject: [PATCH 4/9] Give `MessageSendInstructions` any `Destination`, not only Blinded In the next commit we'll use `MessageSendInstructions` for all messages, so it needs the ability to take any `Destination`, not only a `Destination::Blinded`. --- lightning/src/onion_message/messenger.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 7689aed0c27..2bcdbffba55 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -392,10 +392,10 @@ pub struct ResponseInstruction { impl ResponseInstruction { fn into_instructions(self) -> MessageSendInstructions { - let send_path = self.send_path; + let destination = Destination::BlindedPath(self.send_path); match self.context { - Some(context) => MessageSendInstructions::WithReplyPath { send_path, context }, - None => MessageSendInstructions::WithoutReplyPath { send_path }, + Some(context) => MessageSendInstructions::WithReplyPath { destination, context }, + None => MessageSendInstructions::WithoutReplyPath { destination }, } } } @@ -407,7 +407,7 @@ pub enum MessageSendInstructions { /// respond. WithReplyPath { /// The destination where we need to send our message. - send_path: BlindedMessagePath, + destination: Destination, /// The context to include in the reply path we'll give the recipient so they can respond /// to us. context: MessageContext, @@ -416,7 +416,7 @@ pub enum MessageSendInstructions { /// recipient from responding. WithoutReplyPath { /// The destination where we need to send our message. - send_path: BlindedMessagePath, + destination: Destination, } } @@ -1342,9 +1342,9 @@ where pub fn handle_onion_message_response( &self, response: T, instructions: ResponseInstruction, ) -> Result, SendError> { - let (response_path, context) = match instructions.into_instructions() { - MessageSendInstructions::WithReplyPath { send_path, context } => (send_path, Some(context)), - MessageSendInstructions::WithoutReplyPath { send_path } => (send_path, None), + let (destination, context) = match instructions.into_instructions() { + MessageSendInstructions::WithReplyPath { destination, context } => (destination, Some(context)), + MessageSendInstructions::WithoutReplyPath { destination } => (destination, None), }; let message_type = response.msg_type(); @@ -1363,7 +1363,7 @@ where } else { None }; self.find_path_and_enqueue_onion_message( - response, Destination::BlindedPath(response_path), reply_path, + response, destination, reply_path, format_args!( "when responding with {} to an onion message", message_type, From 90361c18bf709fa2d2f148ff375351fda7528c98 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 01:41:27 +0000 Subject: [PATCH 5/9] 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. --- fuzz/src/onion_message.rs | 6 +- lightning/src/ln/peer_handler.rs | 4 +- .../src/onion_message/functional_tests.rs | 4 +- lightning/src/onion_message/messenger.rs | 72 +++++++++---------- 4 files changed, 41 insertions(+), 45 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 97e8ae24306..33458436c66 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -16,8 +16,8 @@ use lightning::onion_message::async_payments::{ AsyncPaymentsMessageHandler, HeldHtlcAvailable, ReleaseHeldHtlc, }; use lightning::onion_message::messenger::{ - CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, - PendingOnionMessage, Responder, ResponseInstruction, + CustomOnionMessageHandler, Destination, MessageRouter, MessageSendInstructions, + OnionMessagePath, OnionMessenger, Responder, ResponseInstruction, }; use lightning::onion_message::offers::{OffersMessage, OffersMessageHandler}; use lightning::onion_message::packet::OnionMessageContents; @@ -173,7 +173,7 @@ impl CustomOnionMessageHandler for TestCustomMessageHandler { buffer.read_to_limit(&mut buf, u64::MAX)?; return Ok(Some(TestCustomMessage {})); } - fn release_pending_custom_messages(&self) -> Vec> { + fn release_pending_custom_messages(&self) -> Vec<(TestCustomMessage, MessageSendInstructions)> { vec![] } } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index bde5454b49c..b4fc9252a3f 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/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, ResponseInstruction}; +use crate::onion_message::messenger::{CustomOnionMessageHandler, Responder, ResponseInstruction, 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(&self, _msg_type: u64, _buffer: &mut R) -> Result, msgs::DecodeError> where Self: Sized { Ok(None) } - fn release_pending_custom_messages(&self) -> Vec> { + fn release_pending_custom_messages(&self) -> Vec<(Infallible, MessageSendInstructions)> { vec![] } } diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 01b90314ba1..8dcbb809996 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/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, ResponseInstruction, SendError, SendSuccess}; +use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, Responder, ResponseInstruction, 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> { + fn release_pending_custom_messages(&self) -> Vec<(Self::CustomMessage, MessageSendInstructions)> { vec![] } } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 2bcdbffba55..cf885aaee63 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -830,15 +830,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>; - - /// 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)>; + fn release_pending_custom_messages(&self) -> Vec<(Self::CustomMessage, MessageSendInstructions)>; } /// A processed incoming onion message, containing either a Forward (another onion message) @@ -1188,6 +1180,33 @@ where ) } + fn send_onion_message_internal( + &self, message: T, instructions: MessageSendInstructions, log_suffix: fmt::Arguments, + ) -> Result, SendError> { + let (destination, context) = match instructions { + 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( + message, destination, reply_path, log_suffix, + ).map(|result| Some(result)) + } + fn find_path_and_enqueue_onion_message( &self, contents: T, destination: Destination, reply_path: Option, log_suffix: fmt::Arguments @@ -1342,33 +1361,14 @@ where pub fn handle_onion_message_response( &self, response: T, instructions: ResponseInstruction, ) -> Result, SendError> { - let (destination, context) = match instructions.into_instructions() { - MessageSendInstructions::WithReplyPath { destination, context } => (destination, Some(context)), - MessageSendInstructions::WithoutReplyPath { destination } => (destination, None), - }; - let message_type = response.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, destination, reply_path, + self.send_onion_message_internal( + response, instructions.into_instructions(), format_args!( "when responding with {} to an onion message", message_type, ) - ).map(|result| Some(result)) + ) } #[cfg(test)] @@ -1748,13 +1748,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.send_onion_message_internal( + message, instructions, format_args!("when sending CustomMessage") ); } From eecaf1487990f132ce4ca7b750eabb43a7207525 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 01:42:55 +0000 Subject: [PATCH 6/9] 2/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 next step towards untangling that, moving from `PendingOnionMessage` to `MessageSendInstructions` for the outbound message queue in `OffersMessageHandler`. Better, we can also drop the `c_bindings`-specific message queue variant, unifying the APIs. Because `ChannelManager` needs to actually control the reply path set in individual messages, however, we have to halfway this patch, adding a new `MessageSendInstructions` variant that allows specifying the `reply_path` explicitly. Still, because other message handlers are moving this way, its nice to be consistent. --- lightning/src/ln/channelmanager.rs | 56 +++++++++--------- lightning/src/ln/offers_tests.rs | 72 ++++++++++-------------- lightning/src/onion_message/messenger.rs | 53 +++++++++-------- lightning/src/onion_message/offers.rs | 14 +---- 4 files changed, 88 insertions(+), 107 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b6ae70836ba..b215880c986 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -71,7 +71,7 @@ use crate::offers::parse::Bolt12SemanticError; use crate::offers::refund::{Refund, RefundBuilder}; use crate::offers::signer; use crate::onion_message::async_payments::{AsyncPaymentsMessage, HeldHtlcAvailable, ReleaseHeldHtlc, AsyncPaymentsMessageHandler}; -use crate::onion_message::messenger::{new_pending_onion_message, Destination, MessageRouter, PendingOnionMessage, Responder, ResponseInstruction}; +use crate::onion_message::messenger::{Destination, MessageRouter, PendingOnionMessage, Responder, ResponseInstruction, MessageSendInstructions}; use crate::onion_message::offers::{OffersMessage, OffersMessageHandler}; use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider}; use crate::sign::ecdsa::EcdsaChannelSigner; @@ -2277,9 +2277,9 @@ where needs_persist_flag: AtomicBool, #[cfg(not(any(test, feature = "_test_utils")))] - pending_offers_messages: Mutex>>, + pending_offers_messages: Mutex>, #[cfg(any(test, feature = "_test_utils"))] - pub(crate) pending_offers_messages: Mutex>>, + pub(crate) pending_offers_messages: Mutex>, /// Tracks the message events that are to be broadcasted when we are connected to some peer. pending_broadcast_messages: Mutex>, @@ -9068,21 +9068,21 @@ where .flat_map(|reply_path| offer.paths().iter().map(move |path| (path, reply_path))) .take(OFFERS_MESSAGE_REQUEST_LIMIT) .for_each(|(path, reply_path)| { - let message = new_pending_onion_message( - OffersMessage::InvoiceRequest(invoice_request.clone()), - Destination::BlindedPath(path.clone()), - Some(reply_path.clone()), - ); - pending_offers_messages.push(message); + let instructions = MessageSendInstructions::WithSpecifiedReplyPath { + destination: Destination::BlindedPath(path.clone()), + reply_path: reply_path.clone(), + }; + let message = OffersMessage::InvoiceRequest(invoice_request.clone()); + pending_offers_messages.push((message, instructions)); }); } else if let Some(signing_pubkey) = offer.signing_pubkey() { for reply_path in reply_paths { - let message = new_pending_onion_message( - OffersMessage::InvoiceRequest(invoice_request.clone()), - Destination::Node(signing_pubkey), - Some(reply_path), - ); - pending_offers_messages.push(message); + let instructions = MessageSendInstructions::WithSpecifiedReplyPath { + destination: Destination::Node(signing_pubkey), + reply_path, + }; + let message = OffersMessage::InvoiceRequest(invoice_request.clone()); + pending_offers_messages.push((message, instructions)); } } else { debug_assert!(false); @@ -9162,12 +9162,12 @@ where let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap(); if refund.paths().is_empty() { for reply_path in reply_paths { - let message = new_pending_onion_message( - OffersMessage::Invoice(invoice.clone()), - Destination::Node(refund.payer_id()), - Some(reply_path), - ); - pending_offers_messages.push(message); + let instructions = MessageSendInstructions::WithSpecifiedReplyPath { + destination: Destination::Node(refund.payer_id()), + reply_path, + }; + let message = OffersMessage::Invoice(invoice.clone()); + pending_offers_messages.push((message, instructions)); } } else { reply_paths @@ -9175,12 +9175,12 @@ where .flat_map(|reply_path| refund.paths().iter().map(move |path| (path, reply_path))) .take(OFFERS_MESSAGE_REQUEST_LIMIT) .for_each(|(path, reply_path)| { - let message = new_pending_onion_message( - OffersMessage::Invoice(invoice.clone()), - Destination::BlindedPath(path.clone()), - Some(reply_path.clone()), - ); - pending_offers_messages.push(message); + let instructions = MessageSendInstructions::WithSpecifiedReplyPath { + destination: Destination::BlindedPath(path.clone()), + reply_path: reply_path.clone(), + }; + let message = OffersMessage::Invoice(invoice.clone()); + pending_offers_messages.push((message, instructions)); }); } @@ -10937,7 +10937,7 @@ where } } - fn release_pending_messages(&self) -> Vec> { + fn release_pending_messages(&self) -> Vec<(OffersMessage, MessageSendInstructions)> { core::mem::take(&mut self.pending_offers_messages.lock().unwrap()) } } diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index b1c09fd54f8..8bbf9a0ad7d 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -59,7 +59,7 @@ use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields}; use crate::offers::nonce::Nonce; use crate::offers::parse::Bolt12SemanticError; -use crate::onion_message::messenger::{Destination, PeeledOnion, new_pending_onion_message}; +use crate::onion_message::messenger::{Destination, PeeledOnion, MessageSendInstructions}; use crate::onion_message::offers::OffersMessage; use crate::onion_message::packet::ParsedOnionMessageContents; use crate::routing::gossip::{NodeAlias, NodeId}; @@ -1313,13 +1313,10 @@ fn fails_authentication_when_handling_invoice_request() { expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); connect_peers(david, alice); - #[cfg(not(c_bindings))] { - david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = - Destination::Node(alice_id); - } - #[cfg(c_bindings)] { - david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = - Destination::Node(alice_id); + match &mut david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 { + MessageSendInstructions::WithSpecifiedReplyPath { destination, .. } => + *destination = Destination::Node(alice_id), + _ => panic!(), } let onion_message = david.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); @@ -1341,13 +1338,10 @@ fn fails_authentication_when_handling_invoice_request() { .unwrap(); expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); - #[cfg(not(c_bindings))] { - david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = - Destination::BlindedPath(invalid_path); - } - #[cfg(c_bindings)] { - david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = - Destination::BlindedPath(invalid_path); + match &mut david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 { + MessageSendInstructions::WithSpecifiedReplyPath { destination, .. } => + *destination = Destination::BlindedPath(invalid_path), + _ => panic!(), } connect_peers(david, bob); @@ -1427,11 +1421,9 @@ fn fails_authentication_when_handling_invoice_for_offer() { let mut pending_offers_messages = david.node.pending_offers_messages.lock().unwrap(); let pending_invoice_request = pending_offers_messages.pop().unwrap(); pending_offers_messages.clear(); - #[cfg(not(c_bindings))] { - pending_invoice_request.reply_path - } - #[cfg(c_bindings)] { - pending_invoice_request.2 + match pending_invoice_request.1 { + MessageSendInstructions::WithSpecifiedReplyPath { reply_path, .. } => reply_path, + _ => panic!(), } }; @@ -1445,11 +1437,10 @@ fn fails_authentication_when_handling_invoice_for_offer() { { let mut pending_offers_messages = david.node.pending_offers_messages.lock().unwrap(); let mut pending_invoice_request = pending_offers_messages.first_mut().unwrap(); - #[cfg(not(c_bindings))] { - pending_invoice_request.reply_path = invalid_reply_path; - } - #[cfg(c_bindings)] { - pending_invoice_request.2 = invalid_reply_path; + match &mut pending_invoice_request.1 { + MessageSendInstructions::WithSpecifiedReplyPath { reply_path, .. } => + *reply_path = invalid_reply_path, + _ => panic!(), } } @@ -1531,13 +1522,10 @@ fn fails_authentication_when_handling_invoice_for_refund() { let expected_invoice = alice.node.request_refund_payment(&refund).unwrap(); connect_peers(david, alice); - #[cfg(not(c_bindings))] { - alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = - Destination::Node(david_id); - } - #[cfg(c_bindings)] { - alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = - Destination::Node(david_id); + match &mut alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 { + MessageSendInstructions::WithSpecifiedReplyPath { destination, .. } => + *destination = Destination::Node(david_id), + _ => panic!(), } let onion_message = alice.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); @@ -1565,13 +1553,10 @@ fn fails_authentication_when_handling_invoice_for_refund() { let expected_invoice = alice.node.request_refund_payment(&refund).unwrap(); - #[cfg(not(c_bindings))] { - alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = - Destination::BlindedPath(invalid_path); - } - #[cfg(c_bindings)] { - alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = - Destination::BlindedPath(invalid_path); + match &mut alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 { + MessageSendInstructions::WithSpecifiedReplyPath { destination, .. } => + *destination = Destination::BlindedPath(invalid_path), + _ => panic!(), } connect_peers(alice, charlie); @@ -2155,10 +2140,11 @@ fn fails_paying_invoice_with_unknown_required_features() { .build_and_sign(&secp_ctx).unwrap(); // Enqueue an onion message containing the new invoice. - let pending_message = new_pending_onion_message( - OffersMessage::Invoice(invoice), Destination::BlindedPath(reply_path), None - ); - alice.node.pending_offers_messages.lock().unwrap().push(pending_message); + let instructions = MessageSendInstructions::WithoutReplyPath { + destination: Destination::BlindedPath(reply_path), + }; + let message = OffersMessage::Invoice(invoice); + alice.node.pending_offers_messages.lock().unwrap().push((message, instructions)); let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap(); charlie.onion_messenger.handle_onion_message(&alice_id, &onion_message); diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index cf885aaee63..ba2fe982d29 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -403,6 +403,14 @@ impl ResponseInstruction { /// Instructions for how and where to send a message. #[derive(Clone)] pub enum MessageSendInstructions { + /// Indicates that a message should be sent including the provided reply path for the recipient + /// to respond. + WithSpecifiedReplyPath { + /// The destination where we need to send our message. + destination: Destination, + /// The reply path which should be included in the message. + reply_path: BlindedMessagePath, + }, /// Indicates that a message should be sent including a reply path for the recipient to /// respond. WithReplyPath { @@ -1183,24 +1191,25 @@ where fn send_onion_message_internal( &self, message: T, instructions: MessageSendInstructions, log_suffix: fmt::Arguments, ) -> Result, SendError> { - let (destination, context) = match instructions { - 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); + let (destination, reply_path) = match instructions { + MessageSendInstructions::WithSpecifiedReplyPath { destination, reply_path } => + (destination, Some(reply_path)), + MessageSendInstructions::WithReplyPath { destination, context } => { + match self.create_blinded_path(context) { + Ok(reply_path) => (destination, Some(reply_path)), + Err(err) => { + log_trace!( + self.logger, + "Failed to create reply path {}: {:?}", + log_suffix, err + ); + return Err(err); + } } - } - } else { None }; + }, + MessageSendInstructions::WithoutReplyPath { destination } => + (destination, None), + }; self.find_path_and_enqueue_onion_message( message, destination, reply_path, log_suffix, @@ -1737,13 +1746,9 @@ where // node, and then enqueue the message for sending to the first peer in the full path. fn next_onion_message_for_peer(&self, peer_node_id: PublicKey) -> Option { // Enqueue any initiating `OffersMessage`s to send. - for message in self.offers_handler.release_pending_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 OffersMessage") + for (message, instructions) in self.offers_handler.release_pending_messages() { + let _ = self.send_onion_message_internal( + message, instructions, format_args!("when sending OffersMessage") ); } diff --git a/lightning/src/onion_message/offers.rs b/lightning/src/onion_message/offers.rs index 1042080f8be..1b3274ae148 100644 --- a/lightning/src/onion_message/offers.rs +++ b/lightning/src/onion_message/offers.rs @@ -22,9 +22,7 @@ use crate::offers::static_invoice::StaticInvoice; 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; +use crate::onion_message::messenger::{ResponseInstruction, Responder, MessageSendInstructions}; use crate::prelude::*; @@ -53,15 +51,7 @@ pub trait OffersMessageHandler { /// /// Typically, this is used for messages initiating a payment flow rather than in response to /// another message. The latter should use the return value of [`Self::handle_message`]. - #[cfg(not(c_bindings))] - fn release_pending_messages(&self) -> Vec> { vec![] } - - /// Releases any [`OffersMessage`]s that need to be sent. - /// - /// Typically, this is used for messages initiating a payment flow rather than in response to - /// another message. The latter should use the return value of [`Self::handle_message`]. - #[cfg(c_bindings)] - fn release_pending_messages(&self) -> Vec<(OffersMessage, crate::onion_message::messenger::Destination, Option)> { vec![] } + fn release_pending_messages(&self) -> Vec<(OffersMessage, MessageSendInstructions)> { vec![] } } /// Possible BOLT 12 Offers messages sent and received via an [`OnionMessage`]. From 29990674ea96f6439481cc8d7730bb8dc5defd54 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 21 Aug 2024 19:10:46 +0000 Subject: [PATCH 7/9] 3/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 next step towards untangling that, moving from `PendingOnionMessage` to `MessageSendInstructions` for the outbound message queue in `AsyncPaymentsMessageHandler`. Better, we can also drop the `c_bindings`-specific message queue variant, unifying the APIs. Here we also drop `PendingOnionMessage` entirely. --- lightning/src/ln/channelmanager.rs | 4 +-- lightning/src/onion_message/async_payments.rs | 22 ++----------- lightning/src/onion_message/messenger.rs | 32 ------------------- 3 files changed, 4 insertions(+), 54 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b215880c986..855af0d8903 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -71,7 +71,7 @@ use crate::offers::parse::Bolt12SemanticError; use crate::offers::refund::{Refund, RefundBuilder}; use crate::offers::signer; use crate::onion_message::async_payments::{AsyncPaymentsMessage, HeldHtlcAvailable, ReleaseHeldHtlc, AsyncPaymentsMessageHandler}; -use crate::onion_message::messenger::{Destination, MessageRouter, PendingOnionMessage, Responder, ResponseInstruction, MessageSendInstructions}; +use crate::onion_message::messenger::{Destination, MessageRouter, Responder, ResponseInstruction, MessageSendInstructions}; use crate::onion_message::offers::{OffersMessage, OffersMessageHandler}; use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider}; use crate::sign::ecdsa::EcdsaChannelSigner; @@ -10962,7 +10962,7 @@ where fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {} - fn release_pending_messages(&self) -> Vec> { + fn release_pending_messages(&self) -> Vec<(AsyncPaymentsMessage, MessageSendInstructions)> { Vec::new() } } diff --git a/lightning/src/onion_message/async_payments.rs b/lightning/src/onion_message/async_payments.rs index dcc52830566..37e4a534180 100644 --- a/lightning/src/onion_message/async_payments.rs +++ b/lightning/src/onion_message/async_payments.rs @@ -11,9 +11,7 @@ use crate::io; use crate::ln::msgs::DecodeError; -#[cfg(not(c_bindings))] -use crate::onion_message::messenger::PendingOnionMessage; -use crate::onion_message::messenger::{Responder, ResponseInstruction}; +use crate::onion_message::messenger::{MessageSendInstructions, Responder, ResponseInstruction}; use crate::onion_message::packet::OnionMessageContents; use crate::prelude::*; use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer}; @@ -40,23 +38,7 @@ pub trait AsyncPaymentsMessageHandler { /// /// Typically, this is used for messages initiating an async payment flow rather than in response /// to another message. - #[cfg(not(c_bindings))] - fn release_pending_messages(&self) -> Vec> { - vec![] - } - - /// Release any [`AsyncPaymentsMessage`]s that need to be sent. - /// - /// Typically, this is used for messages initiating a payment flow rather than in response to - /// another message. - #[cfg(c_bindings)] - fn release_pending_messages( - &self, - ) -> Vec<( - AsyncPaymentsMessage, - crate::onion_message::messenger::Destination, - Option, - )> { + fn release_pending_messages(&self) -> Vec<(AsyncPaymentsMessage, MessageSendInstructions)> { vec![] } } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index ba2fe982d29..fd5108d5ad7 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -428,38 +428,6 @@ pub enum MessageSendInstructions { } } -/// An [`OnionMessage`] for [`OnionMessenger`] to send. -/// -/// These are obtained when released from [`OnionMessenger`]'s handlers after which they are -/// enqueued for sending. -#[cfg(not(c_bindings))] -pub struct PendingOnionMessage { - /// The message contents to send in an [`OnionMessage`]. - pub contents: T, - - /// The destination of the message. - pub destination: Destination, - - /// A reply path to include in the [`OnionMessage`] for a response. - pub reply_path: Option, -} - -#[cfg(c_bindings)] -/// An [`OnionMessage`] for [`OnionMessenger`] to send. -/// -/// These are obtained when released from [`OnionMessenger`]'s handlers after which they are -/// enqueued for sending. -pub type PendingOnionMessage = (T, Destination, Option); - -pub(crate) fn new_pending_onion_message( - contents: T, destination: Destination, reply_path: Option -) -> PendingOnionMessage { - #[cfg(not(c_bindings))] - return PendingOnionMessage { contents, destination, reply_path }; - #[cfg(c_bindings)] - return (contents, destination, reply_path); -} - /// A trait defining behavior for routing an [`OnionMessage`]. pub trait MessageRouter { /// Returns a route for sending an [`OnionMessage`] to the given [`Destination`]. From 131ac4f5a86efa3dcfd860c64b2d863478c44900 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 01:47:37 +0000 Subject: [PATCH 8/9] Change `send_onion_message` to take a `MessageSendInstructions` This lets callers include a `reply_path` without doing the path-finding at the callsite, utilizing the built-in path-finding logic in `OnionMessenger` --- .../src/onion_message/functional_tests.rs | 48 ++++++++++++------- lightning/src/onion_message/messenger.rs | 35 +++++--------- 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 8dcbb809996..765c4ba0fa6 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -345,7 +345,8 @@ fn one_unblinded_hop() { let test_msg = TestCustomMessage::Pong; let destination = Destination::Node(nodes[1].node_id); - nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap(); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; + nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap(); nodes[1].custom_message_handler.expect_message(TestCustomMessage::Pong); pass_along_path(&nodes); } @@ -375,7 +376,8 @@ fn one_blinded_hop() { let context = MessageContext::Custom(Vec::new()); let blinded_path = BlindedMessagePath::new(&[], nodes[1].node_id, context, &*nodes[1].entropy_source, &secp_ctx).unwrap(); let destination = Destination::BlindedPath(blinded_path); - nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap(); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; + nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap(); nodes[1].custom_message_handler.expect_message(TestCustomMessage::Pong); pass_along_path(&nodes); } @@ -413,8 +415,9 @@ fn three_blinded_hops() { let context = MessageContext::Custom(Vec::new()); let blinded_path = BlindedMessagePath::new(&intermediate_nodes, nodes[3].node_id, context, &*nodes[3].entropy_source, &secp_ctx).unwrap(); let destination = Destination::BlindedPath(blinded_path); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; - nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap(); + nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap(); nodes[3].custom_message_handler.expect_message(TestCustomMessage::Pong); pass_along_path(&nodes); } @@ -447,7 +450,7 @@ fn async_response_over_one_blinded_hop() { let (msg, instructions) = response_instruction.unwrap(); assert_eq!( nodes[0].messenger.handle_onion_message_response(msg, instructions), - Ok(Some(SendSuccess::Buffered)), + Ok(SendSuccess::Buffered), ); bob.custom_message_handler.expect_message(TestCustomMessage::Pong); @@ -481,7 +484,7 @@ fn async_response_with_reply_path_succeeds() { let (msg, instructions) = response_instruction.unwrap(); assert_eq!( alice.messenger.handle_onion_message_response(msg, instructions), - Ok(Some(SendSuccess::Buffered)), + Ok(SendSuccess::Buffered), ); // Set Bob's expectation and pass the Onion Message along the path. @@ -557,8 +560,9 @@ fn we_are_intro_node() { let context = MessageContext::Custom(Vec::new()); let blinded_path = BlindedMessagePath::new(&intermediate_nodes, nodes[2].node_id, context, &*nodes[2].entropy_source, &secp_ctx).unwrap(); let destination = Destination::BlindedPath(blinded_path); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; - nodes[0].messenger.send_onion_message(test_msg.clone(), destination, None).unwrap(); + nodes[0].messenger.send_onion_message(test_msg.clone(), instructions).unwrap(); nodes[2].custom_message_handler.expect_message(TestCustomMessage::Pong); pass_along_path(&nodes); @@ -567,7 +571,9 @@ fn we_are_intro_node() { let context = MessageContext::Custom(Vec::new()); let blinded_path = BlindedMessagePath::new(&intermediate_nodes, nodes[1].node_id, context, &*nodes[1].entropy_source, &secp_ctx).unwrap(); let destination = Destination::BlindedPath(blinded_path); - nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap(); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; + + nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap(); nodes[1].custom_message_handler.expect_message(TestCustomMessage::Pong); nodes.remove(2); pass_along_path(&nodes); @@ -585,7 +591,9 @@ fn invalid_blinded_path_error() { let mut blinded_path = BlindedMessagePath::new(&intermediate_nodes, nodes[2].node_id, context, &*nodes[2].entropy_source, &secp_ctx).unwrap(); blinded_path.clear_blinded_hops(); let destination = Destination::BlindedPath(blinded_path); - let err = nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap_err(); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; + + let err = nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap_err(); assert_eq!(err, SendError::TooFewBlindedHops); } @@ -629,8 +637,9 @@ fn reply_path() { ]; let context = MessageContext::Custom(Vec::new()); let reply_path = BlindedMessagePath::new(&intermediate_nodes, nodes[0].node_id, context, &*nodes[0].entropy_source, &secp_ctx).unwrap(); + let instructions = MessageSendInstructions::WithSpecifiedReplyPath { destination, reply_path }; - nodes[0].messenger.send_onion_message(test_msg, destination, Some(reply_path)).unwrap(); + nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap(); nodes[3].custom_message_handler.expect_message(TestCustomMessage::Ping); pass_along_path(&nodes); @@ -662,7 +671,9 @@ fn invalid_custom_message_type() { let test_msg = InvalidCustomMessage {}; let destination = Destination::Node(nodes[1].node_id); - let err = nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap_err(); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; + + let err = nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap_err(); assert_eq!(err, SendError::InvalidMessage); } @@ -671,10 +682,12 @@ fn peer_buffer_full() { let nodes = create_nodes(2); let test_msg = TestCustomMessage::Ping; let destination = Destination::Node(nodes[1].node_id); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; + for _ in 0..188 { // Based on MAX_PER_PEER_BUFFER_SIZE in OnionMessenger - nodes[0].messenger.send_onion_message(test_msg.clone(), destination.clone(), None).unwrap(); + nodes[0].messenger.send_onion_message(test_msg.clone(), instructions.clone()).unwrap(); } - let err = nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap_err(); + let err = nodes[0].messenger.send_onion_message(test_msg, instructions.clone()).unwrap_err(); assert_eq!(err, SendError::BufferFull); } @@ -714,9 +727,10 @@ fn requests_peer_connection_for_buffered_messages() { &intermediate_nodes, nodes[2].node_id, context, &*nodes[0].entropy_source, &secp_ctx ).unwrap(); let destination = Destination::BlindedPath(blinded_path); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; // Buffer an onion message for a connected peer - nodes[0].messenger.send_onion_message(message.clone(), destination.clone(), None).unwrap(); + nodes[0].messenger.send_onion_message(message.clone(), instructions.clone()).unwrap(); assert!(release_events(&nodes[0]).is_empty()); assert!(nodes[0].messenger.next_onion_message_for_peer(nodes[1].node_id).is_some()); assert!(nodes[0].messenger.next_onion_message_for_peer(nodes[1].node_id).is_none()); @@ -724,7 +738,7 @@ fn requests_peer_connection_for_buffered_messages() { // Buffer an onion message for a disconnected peer disconnect_peers(&nodes[0], &nodes[1]); assert!(nodes[0].messenger.next_onion_message_for_peer(nodes[1].node_id).is_none()); - nodes[0].messenger.send_onion_message(message, destination, None).unwrap(); + nodes[0].messenger.send_onion_message(message, instructions).unwrap(); // Check that a ConnectionNeeded event for the peer is provided let events = release_events(&nodes[0]); @@ -753,10 +767,11 @@ fn drops_buffered_messages_waiting_for_peer_connection() { &intermediate_nodes, nodes[2].node_id, context, &*nodes[0].entropy_source, &secp_ctx ).unwrap(); let destination = Destination::BlindedPath(blinded_path); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; // Buffer an onion message for a disconnected peer disconnect_peers(&nodes[0], &nodes[1]); - nodes[0].messenger.send_onion_message(message, destination, None).unwrap(); + nodes[0].messenger.send_onion_message(message, instructions).unwrap(); // Release the event so the timer can start ticking let events = release_events(&nodes[0]); @@ -804,10 +819,11 @@ fn intercept_offline_peer_oms() { &intermediate_nodes, nodes[2].node_id, context, &*nodes[2].entropy_source, &secp_ctx ).unwrap(); let destination = Destination::BlindedPath(blinded_path); + let instructions = MessageSendInstructions::WithoutReplyPath { destination }; // Disconnect the peers to ensure we intercept the OM. disconnect_peers(&nodes[1], &nodes[2]); - nodes[0].messenger.send_onion_message(message, destination, None).unwrap(); + nodes[0].messenger.send_onion_message(message, instructions).unwrap(); let mut final_node_vec = nodes.split_off(2); pass_along_path(&nodes); diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index fd5108d5ad7..d9e36f41325 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -151,7 +151,7 @@ for OnionMessenger where /// # use lightning::blinded_path::message::{BlindedMessagePath, ForwardNode, MessageContext}; /// # use lightning::sign::{EntropySource, KeysManager}; /// # use lightning::ln::peer_handler::IgnoringMessageHandler; -/// # use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath, OnionMessenger}; +/// # use lightning::onion_message::messenger::{Destination, MessageRouter, MessageSendInstructions, OnionMessagePath, OnionMessenger}; /// # use lightning::onion_message::packet::OnionMessageContents; /// # use lightning::util::logger::{Logger, Record}; /// # use lightning::util::ser::{Writeable, Writer}; @@ -218,9 +218,9 @@ for OnionMessenger where /// } /// // Send a custom onion message to a node id. /// let destination = Destination::Node(destination_node_id); -/// let reply_path = None; +/// let instructions = MessageSendInstructions::WithoutReplyPath { destination }; /// # let message = YourCustomMessage {}; -/// onion_messenger.send_onion_message(message, destination, reply_path); +/// onion_messenger.send_onion_message(message, instructions); /// /// // Create a blinded path to yourself, for someone to send an onion message to. /// # let your_node_id = hop_node_id1; @@ -233,9 +233,9 @@ for OnionMessenger where /// /// // Send a custom onion message to a blinded path. /// let destination = Destination::BlindedPath(blinded_path); -/// let reply_path = None; +/// let instructions = MessageSendInstructions::WithoutReplyPath { destination }; /// # let message = YourCustomMessage {}; -/// onion_messenger.send_onion_message(message, destination, reply_path); +/// onion_messenger.send_onion_message(message, instructions); /// ``` /// /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest @@ -1145,20 +1145,16 @@ where self.offers_handler = offers_handler; } - /// Sends an [`OnionMessage`] with the given `contents` to `destination`. - /// - /// See [`OnionMessenger`] for example usage. + /// Sends an [`OnionMessage`] based on its [`MessageSendInstructions`]. pub fn send_onion_message( - &self, contents: T, destination: Destination, reply_path: Option + &self, contents: T, instructions: MessageSendInstructions, ) -> Result { - self.find_path_and_enqueue_onion_message( - contents, destination, reply_path, format_args!("") - ) + self.send_onion_message_internal(contents, instructions, format_args!("")) } fn send_onion_message_internal( - &self, message: T, instructions: MessageSendInstructions, log_suffix: fmt::Arguments, - ) -> Result, SendError> { + &self, contents: T, instructions: MessageSendInstructions, log_suffix: fmt::Arguments, + ) -> Result { let (destination, reply_path) = match instructions { MessageSendInstructions::WithSpecifiedReplyPath { destination, reply_path } => (destination, Some(reply_path)), @@ -1179,15 +1175,6 @@ where (destination, None), }; - self.find_path_and_enqueue_onion_message( - message, destination, reply_path, log_suffix, - ).map(|result| Some(result)) - } - - fn find_path_and_enqueue_onion_message( - &self, contents: T, destination: Destination, reply_path: Option, - log_suffix: fmt::Arguments - ) -> Result { let mut logger = WithContext::from(&self.logger, None, None, None); let result = self.find_path(destination).and_then(|path| { let first_hop = path.intermediate_nodes.get(0).map(|p| *p); @@ -1337,7 +1324,7 @@ where /// ready for sending, that task can invoke this method to enqueue the response for delivery. pub fn handle_onion_message_response( &self, response: T, instructions: ResponseInstruction, - ) -> Result, SendError> { + ) -> Result { let message_type = response.msg_type(); self.send_onion_message_internal( response, instructions.into_instructions(), From 47b527a656b3d965a63f7871eebb98ebdc9638f0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 01:31:07 +0000 Subject: [PATCH 9/9] Add a `MessageSendInstructions::ForReply` In order to allow onion message handlers to reply asynchronously without introducing a circular dependency graph, the message handlers need to be able to send replies described by `MessageSendInstructions`. This allows them to send replies via the normal message queuing (i.e. without making a function call to `OnionMessenger`). Here we enable that by adding a `MessageSendInstructions::ForReply` variant which holds `ReplyInstruction`s. Fixes #3178 --- lightning/src/onion_message/messenger.rs | 29 +++++++++++++++--------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index d9e36f41325..7d215e365be 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -367,7 +367,7 @@ impl Responder { /// Use when the recipient doesn't need to send back a reply to us. pub fn respond(self) -> ResponseInstruction { ResponseInstruction { - send_path: self.reply_path, + destination: Destination::BlindedPath(self.reply_path), context: None, } } @@ -377,7 +377,7 @@ impl Responder { /// Use when the recipient needs to send back a reply to us. pub fn respond_with_reply_path(self, context: MessageContext) -> ResponseInstruction { ResponseInstruction { - send_path: self.reply_path, + destination: Destination::BlindedPath(self.reply_path), context: Some(context), } } @@ -386,17 +386,16 @@ impl Responder { /// Instructions for how and where to send the response to an onion message. #[derive(Clone)] pub struct ResponseInstruction { - send_path: BlindedMessagePath, + /// The destination in a response is always a [`Destination::BlindedPath`] but using a + /// [`Destination`] rather than an explicit [`BlindedMessagePath`] simplifies the logic in + /// [`OnionMessenger::send_onion_message_internal`] somewhat. + destination: Destination, context: Option, } impl ResponseInstruction { fn into_instructions(self) -> MessageSendInstructions { - let destination = Destination::BlindedPath(self.send_path); - match self.context { - Some(context) => MessageSendInstructions::WithReplyPath { destination, context }, - None => MessageSendInstructions::WithoutReplyPath { destination }, - } + MessageSendInstructions::ForReply { instructions: self } } } @@ -425,7 +424,12 @@ pub enum MessageSendInstructions { WithoutReplyPath { /// The destination where we need to send our message. destination: Destination, - } + }, + /// Indicates that a message is being sent as a reply to a received message. + ForReply { + /// The instructions provided by the [`Responder`]. + instructions: ResponseInstruction, + }, } /// A trait defining behavior for routing an [`OnionMessage`]. @@ -1158,7 +1162,9 @@ where let (destination, reply_path) = match instructions { MessageSendInstructions::WithSpecifiedReplyPath { destination, reply_path } => (destination, Some(reply_path)), - MessageSendInstructions::WithReplyPath { destination, context } => { + MessageSendInstructions::WithReplyPath { destination, context } + |MessageSendInstructions::ForReply { instructions: ResponseInstruction { destination, context: Some(context) } } => + { match self.create_blinded_path(context) { Ok(reply_path) => (destination, Some(reply_path)), Err(err) => { @@ -1171,7 +1177,8 @@ where } } }, - MessageSendInstructions::WithoutReplyPath { destination } => + MessageSendInstructions::WithoutReplyPath { destination } + |MessageSendInstructions::ForReply { instructions: ResponseInstruction { destination, context: None } } => (destination, None), };