Destination in OnionMessenger::send_onion_message
authorJeffrey Czyz <jkczyz@gmail.com>
Wed, 15 Nov 2023 23:26:45 +0000 (17:26 -0600)
committerJeffrey Czyz <jkczyz@gmail.com>
Wed, 6 Dec 2023 14:47:18 +0000 (08:47 -0600)
OnionMessenger::send_onion_message takes an OnionMessagePath. This isn't
very useful as it requires finding a path manually. Instead, have the
method take a Destination and use OnionMessenger's MessageRouter to
construct the path. Later, this will allow for buffering messages where
the first node in the path isn't a direct connection.

fuzz/src/onion_message.rs
lightning/src/onion_message/functional_tests.rs
lightning/src/onion_message/messenger.rs

index 2882dcfb5089ec7bcb863de6cb9b3bb8e773ae05..afa416a4044872fabb84c3eab0f91aecea28133d 100644 (file)
@@ -269,7 +269,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(),
-                                               "Sending onion message when responding to Custom onion message with path_id None: TestCustomMessage".to_string())), Some(&1));
+                                               "Constructing onion message when responding to Custom 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));
                }
 
                let two_unblinded_hops_om = "\
index c43b218df7b16015364966ed07882b79d29db325..482e5ea8cc5525bf40b42a49c32d170902c86448 100644 (file)
@@ -206,7 +206,7 @@ fn one_unblinded_hop() {
                intermediate_nodes: vec![],
                destination: Destination::Node(nodes[1].get_node_pk()),
        };
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[1].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -220,7 +220,7 @@ fn two_unblinded_hops() {
                intermediate_nodes: vec![nodes[1].get_node_pk()],
                destination: Destination::Node(nodes[2].get_node_pk()),
        };
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[2].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -236,7 +236,7 @@ fn one_blinded_hop() {
                intermediate_nodes: vec![],
                destination: Destination::BlindedPath(blinded_path),
        };
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[1].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -253,7 +253,7 @@ fn two_unblinded_two_blinded() {
                destination: Destination::BlindedPath(blinded_path),
        };
 
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[4].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -270,7 +270,7 @@ fn three_blinded_hops() {
                destination: Destination::BlindedPath(blinded_path),
        };
 
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[3].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -287,7 +287,7 @@ fn too_big_packet_error() {
                intermediate_nodes: hops,
                destination: Destination::Node(hop_node_id),
        };
-       let err = nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap_err();
+       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap_err();
        assert_eq!(err, SendError::TooBigPacket);
 }
 
@@ -305,7 +305,7 @@ fn we_are_intro_node() {
                destination: Destination::BlindedPath(blinded_path),
        };
 
-       nodes[0].messenger.send_onion_message(path, test_msg.clone(), None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg.clone(), None).unwrap();
        nodes[2].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 
@@ -315,7 +315,7 @@ fn we_are_intro_node() {
                intermediate_nodes: vec![],
                destination: Destination::BlindedPath(blinded_path),
        };
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[1].custom_message_handler.expect_message(TestCustomMessage::Response);
        nodes.remove(2);
        pass_along_path(&nodes);
@@ -335,7 +335,7 @@ fn invalid_blinded_path_error() {
                intermediate_nodes: vec![],
                destination: Destination::BlindedPath(blinded_path),
        };
-       let err = nodes[0].messenger.send_onion_message(path, test_msg.clone(), None).unwrap_err();
+       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg.clone(), None).unwrap_err();
        assert_eq!(err, SendError::TooFewBlindedHops);
 }
 
@@ -351,7 +351,7 @@ fn reply_path() {
                destination: Destination::Node(nodes[3].get_node_pk()),
        };
        let reply_path = BlindedPath::new_for_message(&[nodes[2].get_node_pk(), nodes[1].get_node_pk(), nodes[0].get_node_pk()], &*nodes[0].keys_manager, &secp_ctx).unwrap();
-       nodes[0].messenger.send_onion_message(path, test_msg.clone(), Some(reply_path)).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg.clone(), Some(reply_path)).unwrap();
        nodes[3].custom_message_handler.expect_message(TestCustomMessage::Request);
        pass_along_path(&nodes);
        // Make sure the last node successfully decoded the reply path.
@@ -367,7 +367,7 @@ fn reply_path() {
        };
        let reply_path = BlindedPath::new_for_message(&[nodes[2].get_node_pk(), nodes[1].get_node_pk(), nodes[0].get_node_pk()], &*nodes[0].keys_manager, &secp_ctx).unwrap();
 
-       nodes[0].messenger.send_onion_message(path, test_msg, Some(reply_path)).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, Some(reply_path)).unwrap();
        nodes[3].custom_message_handler.expect_message(TestCustomMessage::Request);
        pass_along_path(&nodes);
 
@@ -399,7 +399,7 @@ fn invalid_custom_message_type() {
                intermediate_nodes: vec![],
                destination: Destination::Node(nodes[1].get_node_pk()),
        };
-       let err = nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap_err();
+       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap_err();
        assert_eq!(err, SendError::InvalidMessage);
 }
 
@@ -412,9 +412,9 @@ fn peer_buffer_full() {
                destination: Destination::Node(nodes[1].get_node_pk()),
        };
        for _ in 0..188 { // Based on MAX_PER_PEER_BUFFER_SIZE in OnionMessenger
-               nodes[0].messenger.send_onion_message(path.clone(), test_msg.clone(), None).unwrap();
+               nodes[0].messenger.send_onion_message_using_path(path.clone(), test_msg.clone(), None).unwrap();
        }
-       let err = nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap_err();
+       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap_err();
        assert_eq!(err, SendError::BufferFull);
 }
 
@@ -435,7 +435,7 @@ fn many_hops() {
                intermediate_nodes,
                destination: Destination::Node(nodes[num_nodes-1].get_node_pk()),
        };
-       nodes[0].messenger.send_onion_message(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[num_nodes-1].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
index 2c42566e733bd205157d7aeb2b882b0501512837..8d98e284e954b15032232b04e2fe7d4156339b7f 100644 (file)
@@ -76,7 +76,14 @@ use crate::prelude::*;
 /// # struct FakeMessageRouter {}
 /// # impl MessageRouter for FakeMessageRouter {
 /// #     fn find_path(&self, sender: PublicKey, peers: Vec<PublicKey>, destination: Destination) -> Result<OnionMessagePath, ()> {
-/// #         unimplemented!()
+/// #         let secp_ctx = Secp256k1::new();
+/// #         let node_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
+/// #         let hop_node_id1 = PublicKey::from_secret_key(&secp_ctx, &node_secret);
+/// #         let hop_node_id2 = hop_node_id1;
+/// #         Ok(OnionMessagePath {
+/// #             intermediate_nodes: vec![hop_node_id1, hop_node_id2],
+/// #             destination,
+/// #         })
 /// #     }
 /// # }
 /// # let seed = [42u8; 32];
@@ -86,7 +93,7 @@ use crate::prelude::*;
 /// # let node_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
 /// # let secp_ctx = Secp256k1::new();
 /// # let hop_node_id1 = PublicKey::from_secret_key(&secp_ctx, &node_secret);
-/// # let (hop_node_id2, hop_node_id3, hop_node_id4) = (hop_node_id1, hop_node_id1, hop_node_id1);
+/// # let (hop_node_id3, hop_node_id4) = (hop_node_id1, hop_node_id1);
 /// # let destination_node_id = hop_node_id1;
 /// # let message_router = Arc::new(FakeMessageRouter {});
 /// # let custom_message_handler = IgnoringMessageHandler {};
@@ -113,13 +120,10 @@ use crate::prelude::*;
 ///    }
 /// }
 /// // Send a custom onion message to a node id.
-/// let path = OnionMessagePath {
-///    intermediate_nodes: vec![hop_node_id1, hop_node_id2],
-///    destination: Destination::Node(destination_node_id),
-/// };
+/// let destination = Destination::Node(destination_node_id);
 /// let reply_path = None;
 /// # let message = YourCustomMessage {};
-/// onion_messenger.send_onion_message(path, message, reply_path);
+/// onion_messenger.send_onion_message(message, destination, reply_path);
 ///
 /// // Create a blinded path to yourself, for someone to send an onion message to.
 /// # let your_node_id = hop_node_id1;
@@ -127,13 +131,10 @@ use crate::prelude::*;
 /// let blinded_path = BlindedPath::new_for_message(&hops, &keys_manager, &secp_ctx).unwrap();
 ///
 /// // Send a custom onion message to a blinded path.
-/// let path = OnionMessagePath {
-///    intermediate_nodes: vec![hop_node_id1, hop_node_id2],
-///    destination: Destination::BlindedPath(blinded_path),
-/// };
+/// let destination = Destination::BlindedPath(blinded_path);
 /// let reply_path = None;
 /// # let message = YourCustomMessage {};
-/// onion_messenger.send_onion_message(path, message, reply_path);
+/// onion_messenger.send_onion_message(message, destination, reply_path);
 /// ```
 ///
 /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
@@ -304,6 +305,16 @@ impl Destination {
        }
 }
 
+/// Result of successfully [sending an onion message].
+///
+/// [sending an onion message]: OnionMessenger::send_onion_message
+#[derive(Debug, PartialEq, Eq)]
+pub enum SendSuccess {
+       /// The message was buffered and will be sent once it is processed by
+       /// [`OnionMessageHandler::next_onion_message_for_peer`].
+       Buffered,
+}
+
 /// Errors that may occur when [sending an onion message].
 ///
 /// [sending an onion message]: OnionMessenger::send_onion_message
@@ -319,6 +330,8 @@ pub enum SendError {
        TooFewBlindedHops,
        /// Our next-hop peer was offline or does not support onion message forwarding.
        InvalidFirstHop,
+       /// A path from the sender to the destination could not be found by the [`MessageRouter`].
+       PathNotFound,
        /// Onion message contents must have a TLV type >= 64.
        InvalidMessage,
        /// Our next-hop peer's buffer was full or our total outbound buffer was full.
@@ -568,14 +581,63 @@ where
                }
        }
 
-       /// Sends an [`OnionMessage`] with the given `contents` for sending to the destination of
-       /// `path`.
+       /// Sends an [`OnionMessage`] with the given `contents` to `destination`.
        ///
        /// See [`OnionMessenger`] for example usage.
        pub fn send_onion_message<T: OnionMessageContents>(
-               &self, path: OnionMessagePath, contents: T, reply_path: Option<BlindedPath>
-       ) -> Result<(), SendError> {
-               log_trace!(self.logger, "Sending onion message: {:?}", contents);
+               &self, contents: T, destination: Destination, reply_path: Option<BlindedPath>
+       ) -> Result<SendSuccess, SendError> {
+               self.find_path_and_enqueue_onion_message(
+                       contents, destination, reply_path, format_args!("")
+               )
+       }
+
+       fn find_path_and_enqueue_onion_message<T: OnionMessageContents>(
+               &self, contents: T, destination: Destination, reply_path: Option<BlindedPath>,
+               log_suffix: fmt::Arguments
+       ) -> Result<SendSuccess, SendError> {
+               let result = self.find_path(destination)
+                       .and_then(|path| self.enqueue_onion_message(path, contents, reply_path, log_suffix));
+
+               match result.as_ref() {
+                       Err(SendError::GetNodeIdFailed) => {
+                               log_warn!(self.logger, "Unable to retrieve node id {}", log_suffix);
+                       },
+                       Err(SendError::PathNotFound) => {
+                               log_trace!(self.logger, "Failed to find path {}", log_suffix);
+                       },
+                       Err(e) => {
+                               log_trace!(self.logger, "Failed sending onion message {}: {:?}", log_suffix, e);
+                       },
+                       Ok(SendSuccess::Buffered) => {
+                               log_trace!(self.logger, "Buffered onion message {}", log_suffix);
+                       },
+               }
+
+               result
+       }
+
+       fn find_path(&self, destination: Destination) -> Result<OnionMessagePath, SendError> {
+               let sender = self.node_signer
+                       .get_node_id(Recipient::Node)
+                       .map_err(|_| SendError::GetNodeIdFailed)?;
+
+               let peers = self.message_buffers.lock().unwrap()
+                       .iter()
+                       .filter(|(_, buffer)| matches!(buffer, OnionMessageBuffer::ConnectedPeer(_)))
+                       .map(|(node_id, _)| *node_id)
+                       .collect();
+
+               self.message_router
+                       .find_path(sender, peers, destination)
+                       .map_err(|_| SendError::PathNotFound)
+       }
+
+       fn enqueue_onion_message<T: OnionMessageContents>(
+               &self, path: OnionMessagePath, contents: T, reply_path: Option<BlindedPath>,
+               log_suffix: fmt::Arguments
+       ) -> Result<SendSuccess, SendError> {
+               log_trace!(self.logger, "Constructing onion message {}: {:?}", log_suffix, contents);
 
                let (first_node_id, onion_message) = create_onion_message(
                        &self.entropy_source, &self.node_signer, &self.secp_ctx, path, contents, reply_path
@@ -590,18 +652,25 @@ where
                        hash_map::Entry::Vacant(_) => Err(SendError::InvalidFirstHop),
                        hash_map::Entry::Occupied(mut e) => {
                                e.get_mut().enqueue_message(onion_message);
-                               Ok(())
+                               Ok(SendSuccess::Buffered)
                        },
                }
        }
 
+       #[cfg(test)]
+       pub(super) fn send_onion_message_using_path<T: OnionMessageContents>(
+               &self, path: OnionMessagePath, contents: T, reply_path: Option<BlindedPath>
+       ) -> Result<SendSuccess, SendError> {
+               self.enqueue_onion_message(path, contents, reply_path, format_args!(""))
+       }
+
        fn handle_onion_message_response<T: OnionMessageContents>(
                &self, response: Option<T>, reply_path: Option<BlindedPath>, log_suffix: fmt::Arguments
        ) {
                if let Some(response) = response {
                        match reply_path {
                                Some(reply_path) => {
-                                       self.find_path_and_enqueue_onion_message(
+                                       let _ = self.find_path_and_enqueue_onion_message(
                                                response, Destination::BlindedPath(reply_path), None, log_suffix
                                        );
                                },
@@ -612,34 +681,6 @@ where
                }
        }
 
-       fn find_path_and_enqueue_onion_message<T: OnionMessageContents>(
-               &self, contents: T, destination: Destination, reply_path: Option<BlindedPath>,
-               log_suffix: fmt::Arguments
-       ) {
-               let sender = match self.node_signer.get_node_id(Recipient::Node) {
-                       Ok(node_id) => node_id,
-                       Err(_) => {
-                               log_warn!(self.logger, "Unable to retrieve node id {}", log_suffix);
-                               return;
-                       }
-               };
-
-               let peers = self.message_buffers.lock().unwrap().keys().copied().collect();
-               let path = match self.message_router.find_path(sender, peers, destination) {
-                       Ok(path) => path,
-                       Err(()) => {
-                               log_trace!(self.logger, "Failed to find path {}", log_suffix);
-                               return;
-                       },
-               };
-
-               log_trace!(self.logger, "Sending onion message {}: {:?}", log_suffix, contents);
-
-               if let Err(e) = self.send_onion_message(path, contents, reply_path) {
-                       log_trace!(self.logger, "Failed sending onion message {}: {:?}", log_suffix, e);
-               }
-       }
-
        #[cfg(test)]
        pub(super) fn release_pending_msgs(&self) -> HashMap<PublicKey, VecDeque<OnionMessage>> {
                let mut message_buffers = self.message_buffers.lock().unwrap();
@@ -790,7 +831,7 @@ where
                        let PendingOnionMessage { contents, destination, reply_path } = message;
                        #[cfg(c_bindings)]
                        let (contents, destination, reply_path) = message;
-                       self.find_path_and_enqueue_onion_message(
+                       let _ = self.find_path_and_enqueue_onion_message(
                                contents, destination, reply_path, format_args!("when sending OffersMessage")
                        );
                }
@@ -801,7 +842,7 @@ where
                        let PendingOnionMessage { contents, destination, reply_path } = message;
                        #[cfg(c_bindings)]
                        let (contents, destination, reply_path) = message;
-                       self.find_path_and_enqueue_onion_message(
+                       let _ = self.find_path_and_enqueue_onion_message(
                                contents, destination, reply_path, format_args!("when sending CustomMessage")
                        );
                }