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

Author: TheBlueMatt

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>(

lightning/src/ln/channelmanager.rs

@@ -10749,41 +10749,41 @@ 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,
 					},
 				};
 
 				let amount_msats = match InvoiceBuilder::<DerivedSigningPubkey>::amount_msats(
 					&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,26 +10890,26 @@ 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
 					},
 				}
 			},
 			#[cfg(async_payments)]
 			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) {}

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!();
 	}

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.

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),
 	);
 }