Implement ResponseInstruction Usage in OnionMessage Handling
authorshaavan <shaavan.github@gmail.com>
Thu, 18 Apr 2024 11:31:52 +0000 (17:01 +0530)
committershaavan <shaavan.github@gmail.com>
Wed, 8 May 2024 12:27:57 +0000 (17:57 +0530)
This commit integrates the newly introduced ResponseInstruction structs
and enums into the codebase for handling OnionMessage responses.

fuzz/src/onion_message.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/peer_handler.rs
lightning/src/onion_message/functional_tests.rs
lightning/src/onion_message/messenger.rs
lightning/src/onion_message/offers.rs
lightning/src/onion_message/packet.rs

index 91fcb9bf2d406484ba1e364cdfc994971781f00e..19ba4cb6aac1a55a6dcc25ee5ece912442d75b17 100644 (file)
@@ -16,7 +16,7 @@ use lightning::sign::{Recipient, KeyMaterial, EntropySource, NodeSigner, SignerP
 use lightning::util::test_channel_signer::TestChannelSigner;
 use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, Writeable, Writer};
-use lightning::onion_message::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage};
+use lightning::onion_message::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, Responder, ResponseInstruction};
 use lightning::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use lightning::onion_message::packet::OnionMessageContents;
 
@@ -97,8 +97,8 @@ impl MessageRouter for TestMessageRouter {
 struct TestOffersMessageHandler {}
 
 impl OffersMessageHandler for TestOffersMessageHandler {
-       fn handle_message(&self, _message: OffersMessage) -> Option<OffersMessage> {
-               None
+       fn handle_message(&self, _message: OffersMessage, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
+               ResponseInstruction::NoResponse
        }
 }
 
@@ -112,6 +112,9 @@ impl OnionMessageContents for TestCustomMessage {
        fn tlv_type(&self) -> u64 {
                CUSTOM_MESSAGE_TYPE
        }
+       fn msg_type(&self) -> &'static str {
+               "Custom Message"
+       }
 }
 
 impl Writeable for TestCustomMessage {
@@ -124,8 +127,11 @@ struct TestCustomMessageHandler {}
 
 impl CustomOnionMessageHandler for TestCustomMessageHandler {
        type CustomMessage = TestCustomMessage;
-       fn handle_custom_message(&self, _msg: Self::CustomMessage) -> Option<Self::CustomMessage> {
-               Some(TestCustomMessage {})
+       fn handle_custom_message(&self, message: Self::CustomMessage, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
+               match responder {
+                       Some(responder) => responder.respond(message),
+                       None => ResponseInstruction::NoResponse
+               }
        }
        fn read_custom_message<R: io::Read>(&self, _message_type: u64, buffer: &mut R) -> Result<Option<Self::CustomMessage>, msgs::DecodeError> {
                let mut buf = Vec::new();
@@ -280,9 +286,9 @@ mod tests {
                                                "Received an onion message with path_id None and a reply_path: Custom(TestCustomMessage)"
                                                .to_string())), Some(&1));
                        assert_eq!(log_entries.get(&("lightning::onion_message::messenger".to_string(),
-                                               "Constructing onion message when responding to Custom onion message with path_id None: TestCustomMessage".to_string())), Some(&1));
+                                               "Constructing onion message when responding with Custom Message to an onion message with path_id None: TestCustomMessage".to_string())), Some(&1));
                        assert_eq!(log_entries.get(&("lightning::onion_message::messenger".to_string(),
-                                               "Buffered onion message when responding to Custom onion message with path_id None".to_string())), Some(&1));
+                                               "Buffered onion message when responding with Custom Message to an onion message with path_id None".to_string())), Some(&1));
                }
 
                let two_unblinded_hops_om = "\
index 51555c7a0ac6e199fe25f6ec17315edd48f2cb9b..f33df2bd797e5eb07453614aa6a206cae78b86f0 100644 (file)
@@ -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<OffersMessage> {
+       fn handle_message(&self, message: OffersMessage, responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
                let secp_ctx = &self.secp_ctx;
                let expanded_key = &self.inbound_payment_key;
 
                match message {
                        OffersMessage::InvoiceRequest(invoice_request) => {
+                               let responder = match responder {
+                                       Some(responder) => responder,
+                                       None => return ResponseInstruction::NoResponse,
+                               };
                                let amount_msats = match InvoiceBuilder::<DerivedSigningPubkey>::amount_msats(
                                        &invoice_request
                                ) {
                                        Ok(amount_msats) => amount_msats,
-                                       Err(error) => return Some(OffersMessage::InvoiceError(error.into())),
+                                       Err(error) => return responder.respond(OffersMessage::InvoiceError(error.into())),
                                };
                                let invoice_request = match invoice_request.verify(expanded_key, secp_ctx) {
                                        Ok(invoice_request) => invoice_request,
                                        Err(()) => {
                                                let error = Bolt12SemanticError::InvalidMetadata;
-                                               return Some(OffersMessage::InvoiceError(error.into()));
+                                               return responder.respond(OffersMessage::InvoiceError(error.into()));
                                        },
                                };
 
@@ -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;
                        },
                }
        }
index 9ce230861456927f53bf4a2b65dd33e800e17e5d..0413cc0ed68925dac0109fc4fb0d1ffcb210b1e2 100644 (file)
@@ -28,7 +28,7 @@ use crate::util::ser::{VecWriter, Writeable, Writer};
 use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor, NextNoiseStep, MessageBuf, MSG_BUF_ALLOC_SIZE};
 use crate::ln::wire;
 use crate::ln::wire::{Encode, Type};
-use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage};
+use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage, Responder, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeId, NodeAlias};
@@ -123,6 +123,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler {
        }
        fn processing_queue_high(&self) -> bool { false }
 }
+
 impl OnionMessageHandler for IgnoringMessageHandler {
        fn handle_onion_message(&self, _their_node_id: &PublicKey, _msg: &msgs::OnionMessage) {}
        fn next_onion_message_for_peer(&self, _peer_node_id: PublicKey) -> Option<msgs::OnionMessage> { None }
@@ -134,12 +135,15 @@ impl OnionMessageHandler for IgnoringMessageHandler {
                InitFeatures::empty()
        }
 }
+
 impl OffersMessageHandler for IgnoringMessageHandler {
-       fn handle_message(&self, _msg: OffersMessage) -> Option<OffersMessage> { None }
+       fn handle_message(&self, _message: OffersMessage, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
+               ResponseInstruction::NoResponse
+       }
 }
 impl CustomOnionMessageHandler for IgnoringMessageHandler {
        type CustomMessage = Infallible;
-       fn handle_custom_message(&self, _msg: Infallible) -> Option<Infallible> {
+       fn handle_custom_message(&self, _message: Self::CustomMessage, _responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
                // Since we always return `None` in the read the handle method should never be called.
                unreachable!();
        }
@@ -153,6 +157,7 @@ impl CustomOnionMessageHandler for IgnoringMessageHandler {
 
 impl OnionMessageContents for Infallible {
        fn tlv_type(&self) -> u64 { unreachable!(); }
+       fn msg_type(&self) -> &'static str { unreachable!(); }
 }
 
 impl Deref for IgnoringMessageHandler {
index acf34a3a8c8a112067bd22cd905b328166398321..34cc0c3959356bc5d693e44a5015b159cc10fd56 100644 (file)
@@ -18,7 +18,7 @@ use crate::routing::test_utils::{add_channel, add_or_update_node};
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
-use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError};
+use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, Responder, ResponseInstruction, SendError};
 use super::offers::{OffersMessage, OffersMessageHandler};
 use super::packet::{OnionMessageContents, Packet};
 
@@ -62,8 +62,8 @@ struct MessengerNode {
 struct TestOffersMessageHandler {}
 
 impl OffersMessageHandler for TestOffersMessageHandler {
-       fn handle_message(&self, _message: OffersMessage) -> Option<OffersMessage> {
-               None
+       fn handle_message(&self, _message: OffersMessage, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
+               ResponseInstruction::NoResponse
        }
 }
 
@@ -85,6 +85,9 @@ impl OnionMessageContents for TestCustomMessage {
                        TestCustomMessage::Response => CUSTOM_RESPONSE_MESSAGE_TYPE,
                }
        }
+       fn msg_type(&self) -> &'static str {
+               "Custom Message"
+       }
 }
 
 impl Writeable for TestCustomMessage {
@@ -123,15 +126,19 @@ impl Drop for TestCustomMessageHandler {
 
 impl CustomOnionMessageHandler for TestCustomMessageHandler {
        type CustomMessage = TestCustomMessage;
-       fn handle_custom_message(&self, msg: Self::CustomMessage) -> Option<Self::CustomMessage> {
+       fn handle_custom_message(&self, msg: Self::CustomMessage, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
                match self.expected_messages.lock().unwrap().pop_front() {
                        Some(expected_msg) => assert_eq!(expected_msg, msg),
                        None => panic!("Unexpected message: {:?}", msg),
                }
-
-               match msg {
+               let response_option = match msg {
                        TestCustomMessage::Request => Some(TestCustomMessage::Response),
                        TestCustomMessage::Response => None,
+               };
+               if let (Some(response), Some(responder)) = (response_option, responder) {
+                       responder.respond(response)
+               } else {
+                       ResponseInstruction::NoResponse
                }
        }
        fn read_custom_message<R: io::Read>(&self, message_type: u64, buffer: &mut R) -> Result<Option<Self::CustomMessage>, DecodeError> where Self: Sized {
@@ -422,6 +429,9 @@ fn invalid_custom_message_type() {
                        // Onion message contents must have a TLV >= 64.
                        63
                }
+               fn msg_type(&self) -> &'static str {
+                       "Invalid Message"
+               }
        }
 
        impl Writeable for InvalidCustomMessage {
index 647239743c3eff0ec4593e47ac332a493bd0f2a8..e87229d66ad9486815c1af690fc0ed6024019ae4 100644 (file)
@@ -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<T: OnionMessageContents> {
        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<Self::CustomMessage>;
+       fn handle_custom_message(&self, message: Self::CustomMessage, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage>;
 
        /// Read a custom message of type `message_type` from `buffer`, returning `Ok(None)` if the
        /// message type is unknown.
@@ -978,19 +978,18 @@ where
        }
 
        fn handle_onion_message_response<T: OnionMessageContents>(
-               &self, response: Option<T>, reply_path: Option<BlindedPath>, log_suffix: fmt::Arguments
+               &self, response: ResponseInstruction<T>
        ) {
-               if let Some(response) = response {
-                       match reply_path {
-                               Some(reply_path) => {
-                                       let _ = self.find_path_and_enqueue_onion_message(
-                                               response, Destination::BlindedPath(reply_path), None, log_suffix
-                                       );
-                               },
-                               None => {
-                                       log_trace!(self.logger, "Missing reply path {}", log_suffix);
-                               },
-                       }
+               if let ResponseInstruction::WithoutReplyPath(response) = response {
+                       let message_type = response.message.msg_type();
+                       let _ = self.find_path_and_enqueue_onion_message(
+                               response.message, Destination::BlindedPath(response.reply_path), None,
+                               format_args!(
+                                       "when responding with {} to an onion message with path_id {:02x?}",
+                                       message_type,
+                                       response.path_id
+                               )
+                       );
                }
        }
 
@@ -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);
                                        },
                                }
                        },
index faf539f868261a97768e6051b22451861db737ca..42c6914157e74f345ed0d2ad354834262653646a 100644 (file)
@@ -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<OffersMessage>;
+       fn handle_message(&self, message: OffersMessage, responder: Option<Responder>) -> ResponseInstruction<OffersMessage>;
 
        /// 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 {
index 510f0ea025a0d615b0f54292d865602ac7e103a6..bbf04ca3ef4baa3f9e27562f073941e254a482d4 100644 (file)
@@ -135,6 +135,12 @@ impl<T: OnionMessageContents> 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<T: OnionMessageContents> Writeable for ParsedOnionMessageContents<T> {
@@ -150,6 +156,9 @@ impl<T: OnionMessageContents> Writeable for ParsedOnionMessageContents<T> {
 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.