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)`.

diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs
index 0de791c555adb2cf77bfdbef80d45292e7a0983d..97e8ae2430638db4978954ce5fdcbd25ef738b7d 100644 (file)
--- 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<OffersContext>,
                _responder: Option<Responder>,
-       ) -> ResponseInstruction<OffersMessage> {
-               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<Responder>,
-       ) -> ResponseInstruction<ReleaseHeldHtlc> {
+       ) -> 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<Vec<u8>>,
                responder: Option<Responder>,
-       ) -> ResponseInstruction<Self::CustomMessage> {
+       ) -> 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<R: io::Read>(

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 167ab0ac8fd8a16475abc99890939df39e372e6f..b6ae70836baa55820fde2a5fa54a4668efc5b2b0 100644 (file)
--- 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<OffersContext>, responder: Option<Responder>,
-       ) -> ResponseInstruction<OffersMessage> {
+       ) -> 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<Responder>
-       ) -> ResponseInstruction<ReleaseHeldHtlc> {
-               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 6b03b36545779f33a146a786b5e2a35de39bf196..bde5454b49c8d02ae2acd5b745ed6df41dda3dda 100644 (file)
--- 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<OffersContext>, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
-               ResponseInstruction::NoResponse
+       fn handle_message(&self, _message: OffersMessage, _context: Option<OffersContext>, _responder: Option<Responder>) -> Option<(OffersMessage, ResponseInstruction)> {
+               None
        }
 }
 impl AsyncPaymentsMessageHandler for IgnoringMessageHandler {
        fn held_htlc_available(
                &self, _message: HeldHtlcAvailable, _responder: Option<Responder>,
-       ) -> ResponseInstruction<ReleaseHeldHtlc> {
-               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<Vec<u8>>, _responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
+       fn handle_custom_message(&self, _message: Self::CustomMessage, _context: Option<Vec<u8>>, _responder: Option<Responder>) -> 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 4a08010191526f3db9b78d3b18cf53b10d00007d..dcc528305663f5d79f8440a1e884ba22d9e3da6a 100644 (file)
--- 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<Responder>,
-       ) -> ResponseInstruction<ReleaseHeldHtlc>;
+       ) -> 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 508e7982b408abde548f08da02a85597d1236385..01b90314ba111816b1f4f9be1344a546384dfe59 100644 (file)
--- 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<OffersContext>, _responder: Option<Responder>) -> ResponseInstruction<OffersMessage> {
-               ResponseInstruction::NoResponse
+       fn handle_message(&self, _message: OffersMessage, _context: Option<OffersContext>, _responder: Option<Responder>) -> Option<(OffersMessage, ResponseInstruction)> {
+               None
        }
 }
 
@@ -86,8 +86,8 @@ struct TestAsyncPaymentsMessageHandler {}
 impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {
        fn held_htlc_available(
                &self, _message: HeldHtlcAvailable, _responder: Option<Responder>,
-       ) -> ResponseInstruction<ReleaseHeldHtlc> {
-               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<Vec<u8>>, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
+       fn handle_custom_message(&self, msg: Self::CustomMessage, context: Option<Vec<u8>>, responder: Option<Responder>) -> 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<R: io::Read>(&self, message_type: u64, buffer: &mut R) -> Result<Option<Self::CustomMessage>, 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 89aa4dbce682b9d9c759ca8fe892835d250feba4..3d83e58a8d74e983bd0e5e944ec5b3a2a76b0a28 100644 (file)
--- 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<T: OnionMessageContents>(self, response: T) -> ResponseInstruction<T> {
-               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<T: OnionMessageContents>(self, response: T, context: MessageContext) -> ResponseInstruction<T> {
-               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<T: OnionMessageContents> {
-       message: T,
-       reply_path: BlindedMessagePath,
-}
-
 /// `ResponseInstruction` represents instructions for responding to received messages.
-pub enum ResponseInstruction<T: OnionMessageContents> {
+pub enum ResponseInstruction {
        /// Indicates that a response should be sent including a reply path for
        /// the recipient to respond back.
-       WithReplyPath(OnionMessageResponse<T>, 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<T>),
-       /// 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<Vec<u8>>, responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage>;
+       fn handle_custom_message(
+               &self, message: Self::CustomMessage, context: Option<Vec<u8>>, responder: Option<Responder>
+       ) -> 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<T: OnionMessageContents>(
-               &self, response: ResponseInstruction<T>
+               &self, response: T, instructions: ResponseInstruction,
        ) -> Result<Option<SendSuccess>, 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 28f6d7d6fe7439e2d2dc1cb69c5e9accb983ff09..1042080f8bea6f7fece5d795952c21db2e7cf67b 100644 (file)
--- 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<OffersContext>, responder: Option<Responder>,
-       ) -> ResponseInstruction<OffersMessage>;
+       ) -> Option<(OffersMessage, ResponseInstruction)>;
 
        /// Releases any [`OffersMessage`]s that need to be sent.
        ///