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)`.
lightningdevkit · Aug 21, 2024 · 9782aec · 9782aec
1 parent bbfa15e
commit 9782aec
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 56 deletions.
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -10749,33 +10749,33 @@ 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;
 
 		match message {
 			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 {
 					Some(nonce) => match invoice_request.verify_using_recipient_data(
 						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,
 					},
 				};
 
@@ -10859,7 +10859,7 @@ where
 			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)),
 					None => {
 						log_trace!(logger, "No reply path to send error: {:?}", error);
-						ResponseInstruction::NoResponse
+						None
 					},
 				}
 			},
@@ -10909,7 +10909,7 @@ where
 								InvoiceError::from_string("Static invoices not yet supported".to_string())
 						))
 					},
-					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
@@ -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
@@ -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
@@ -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);
 
@@ -193,7 +193,7 @@ impl CustomOnionMessageHandler for TestCustomMessageHandler {
 				responder.respond_with_reply_path(response, MessageContext::Custom(context.unwrap_or_else(Vec::new)))
 			},
 			Some(responder) => responder.respond(response),
-			None => ResponseInstruction::NoResponse,
+			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
@@ -365,30 +365,24 @@ 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<T: OnionMessageContents>(self, response: T) -> Option<(T, ResponseInstruction)> {
+		Some((response, 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<T: OnionMessageContents>(self, response: T, context: MessageContext) -> Option<(T, ResponseInstruction)> {
+		Some((response, 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,
-}
-
+/*#[cfg(not(c_bindings))]
 /// `ResponseInstruction` represents instructions for responding to received messages.
 pub enum ResponseInstruction<T: OnionMessageContents> {
 	/// Indicates that a response should be sent including a reply path for
@@ -401,6 +395,26 @@ pub enum ResponseInstruction<T: OnionMessageContents> {
 	NoResponse,
 }
 
+#[cfg(c_bindings)]*/
+/// `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.
+	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 {
+		/// The path over which we'll send our reply.
+		send_path: BlindedMessagePath,
+	}
+}
+
 /// An [`OnionMessage`] for [`OnionMessenger`] to send.
 ///
 /// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
@@ -799,7 +813,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.
@@ -1320,15 +1336,14 @@ where
 	/// 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_message: T, response: 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 response {
+			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_message.msg_type();
 		let reply_path = if let Some(context) = context {
 			match self.create_blinded_path(context) {
 				Ok(reply_path) => Some(reply_path),
@@ -1344,7 +1359,7 @@ where
 		} else { None };
 
 		self.find_path_and_enqueue_onion_message(
-			response.message, Destination::BlindedPath(response.reply_path), reply_path,
+			response_message, Destination::BlindedPath(response_path), reply_path,
 			format_args!(
 				"when responding with {} to an onion message",
 				message_type,
@@ -1564,14 +1579,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 +1606,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
@@ -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.
 	///