fixup: return error when rejecting due to DOS so it does not silently fail and the client knows what's going on

Author: martinsaposnic

lightning-liquidity/src/lsps5/client.rs

@@ -185,6 +185,9 @@ where
 	/// Also ensure the URL is valid, has HTTPS protocol, its length does not exceed [`MAX_WEBHOOK_URL_LENGTH`]
 	/// and that the URL points to a public host.
 	///
+	/// Your request may fail if you recently opened a channel or started an LSPS1 / LSPS2 flow.
+	/// Please retry shortly.
+	///
 	/// [`MAX_WEBHOOK_URL_LENGTH`]: super::msgs::MAX_WEBHOOK_URL_LENGTH
 	/// [`MAX_APP_NAME_LENGTH`]: super::msgs::MAX_APP_NAME_LENGTH
 	/// [`WebhookRegistered`]: super::event::LSPS5ClientEvent::WebhookRegistered

lightning-liquidity/src/lsps5/msgs.rs

@@ -53,6 +53,8 @@ pub const LSPS5_UNKNOWN_ERROR_CODE: i32 = 1000;
 pub const LSPS5_SERIALIZATION_ERROR_CODE: i32 = 1001;
 /// A notification was sent too frequently.
 pub const LSPS5_SLOW_DOWN_ERROR_CODE: i32 = 1002;
+/// A request was rejected because the client has no prior activity with the LSP (no open channel and no active LSPS1 or LSPS2 flow). The client should first open a channel
+pub const LSPS5_NO_PRIOR_ACTIVITY_ERROR_CODE: i32 = 1003;
 
 pub(crate) const LSPS5_SET_WEBHOOK_METHOD_NAME: &str = "lsps5.set_webhook";
 pub(crate) const LSPS5_LIST_WEBHOOKS_METHOD_NAME: &str = "lsps5.list_webhooks";
@@ -113,6 +115,10 @@ pub enum LSPS5ProtocolError {
 	///
 	/// [`NOTIFICATION_COOLDOWN_TIME`]: super::service::NOTIFICATION_COOLDOWN_TIME
 	SlowDownError,
+
+	/// Request rejected because the client has no prior activity with the LSP (no open channel and no active LSPS1 or LSPS2 flow). The client should first open a channel
+	/// or initiate an LSPS1/LSPS2 interaction before retrying.
+	NoPriorActivityError,
 }
 
 impl LSPS5ProtocolError {
@@ -129,6 +135,7 @@ impl LSPS5ProtocolError {
 			LSPS5ProtocolError::UnknownError => LSPS5_UNKNOWN_ERROR_CODE,
 			LSPS5ProtocolError::SerializationError => LSPS5_SERIALIZATION_ERROR_CODE,
 			LSPS5ProtocolError::SlowDownError => LSPS5_SLOW_DOWN_ERROR_CODE,
+			LSPS5ProtocolError::NoPriorActivityError => LSPS5_NO_PRIOR_ACTIVITY_ERROR_CODE,
 		}
 	}
 	/// The error message for the LSPS5 protocol error.
@@ -145,6 +152,9 @@ impl LSPS5ProtocolError {
 				"Error serializing LSPS5 webhook notification"
 			},
 			LSPS5ProtocolError::SlowDownError => "Notification sent too frequently",
+			LSPS5ProtocolError::NoPriorActivityError => {
+				"Request rejected due to no prior activity with the LSP"
+			},
 		}
 	}
 }
@@ -249,6 +259,9 @@ impl From<LSPSResponseError> for LSPS5ProtocolError {
 			LSPS5_UNSUPPORTED_PROTOCOL_ERROR_CODE => LSPS5ProtocolError::UnsupportedProtocol,
 			LSPS5_TOO_MANY_WEBHOOKS_ERROR_CODE => LSPS5ProtocolError::TooManyWebhooks,
 			LSPS5_APP_NAME_NOT_FOUND_ERROR_CODE => LSPS5ProtocolError::AppNameNotFound,
+			LSPS5_SERIALIZATION_ERROR_CODE => LSPS5ProtocolError::SerializationError,
+			LSPS5_SLOW_DOWN_ERROR_CODE => LSPS5ProtocolError::SlowDownError,
+			LSPS5_NO_PRIOR_ACTIVITY_ERROR_CODE => LSPS5ProtocolError::NoPriorActivityError,
 			_ => LSPS5ProtocolError::UnknownError,
 		}
 	}

lightning-liquidity/src/lsps5/service.rs

@@ -149,14 +149,32 @@ where
 		}
 	}
 
-	/// Returns whether a request from the given client should be accepted.
-	///
-	/// Prior activity includes an existing open channel, an active LSPS1 flow,
-	/// or an LSPS2 flow that has an opening or open JIT channel.
-	pub(crate) fn can_accept_request(
+	/// Enforces the prior-activity requirement for state-allocating LSPS5 requests (e.g.
+	/// `lsps5.set_webhook`), rejecting and replying with `NoPriorActivityError` if not met.
+	pub(crate) fn enforce_prior_activity_or_reject(
 		&self, client_id: &PublicKey, lsps2_has_active_requests: bool, lsps1_has_activity: bool,
-	) -> bool {
-		self.client_has_open_channel(client_id) || lsps2_has_active_requests || lsps1_has_activity
+		request_id: LSPSRequestId,
+	) -> Result<(), LightningError> {
+		let can_accept = self.client_has_open_channel(client_id)
+			|| lsps2_has_active_requests
+			|| lsps1_has_activity;
+
+		let mut message_queue_notifier = self.pending_messages.notifier();
+		if !can_accept {
+			let error = LSPS5ProtocolError::NoPriorActivityError;
+			let msg = LSPS5Message::Response(
+				request_id,
+				LSPS5Response::SetWebhookError(error.clone().into()),
+			)
+			.into();
+			message_queue_notifier.enqueue(&client_id, msg);
+			return Err(LightningError {
+				err: error.message().into(),
+				action: ErrorAction::IgnoreAndLog(Level::Info),
+			});
+		} else {
+			Ok(())
+		}
 	}
 
 	fn check_prune_stale_webhooks<'a>(

lightning-liquidity/src/manager.rs

@@ -568,7 +568,7 @@ where
 			LSPSMessage::LSPS5(msg @ LSPS5Message::Request(..)) => {
 				match &self.lsps5_service_handler {
 					Some(lsps5_service_handler) => {
-						if let LSPS5Message::Request(_, ref req) = msg {
+						if let LSPS5Message::Request(ref req_id, ref req) = msg {
 							if req.is_state_allocating() {
 								let lsps2_has_active_requests = self
 									.lsps2_service_handler
@@ -582,19 +582,12 @@ where
 								#[cfg(not(lsps1_service))]
 								let lsps1_has_active_requests = false;
 
-								if !lsps5_service_handler.can_accept_request(
+								lsps5_service_handler.enforce_prior_activity_or_reject(
 									sender_node_id,
 									lsps2_has_active_requests,
 									lsps1_has_active_requests,
-								) {
-									return Err(LightningError {
-										err: format!(
-											"Rejecting LSPS5 request from {:?} without prior activity (requires open channel or active LSPS1 or LSPS2 flow)",
-											sender_node_id
-										),
-										action: ErrorAction::IgnoreAndLog(Level::Debug),
-									});
-								}
+									req_id.clone(),
+								)?
 							}
 						}
 

lightning-liquidity/tests/lsps5_integration_tests.rs

@@ -1241,11 +1241,23 @@ macro_rules! assert_lsps5_reject {
 			.expect("Request should send");
 		let request = get_lsps_message!($client_node, $service_node_id);
 
-		let result =
+		let service_result =
 			$service_node.liquidity_manager.handle_custom_message(request, $client_node_id);
-		assert!(result.is_err(), "Service should reject request without prior interaction");
-
-		assert!($service_node.liquidity_manager.get_and_clear_pending_msg().is_empty());
+		assert!(service_result.is_err(), "Service should reject request without prior interaction");
+
+		let req = get_lsps_message!($service_node, $client_node_id);
+		$client_node.liquidity_manager.handle_custom_message(req, $service_node_id).unwrap();
+		let event = $client_node.liquidity_manager.next_event().unwrap();
+		match event {
+			LiquidityEvent::LSPS5Client(LSPS5ClientEvent::WebhookRegistrationFailed {
+				error,
+				..
+			}) => {
+				let error_to_check = LSPS5ProtocolError::NoPriorActivityError;
+				assert_eq!(error, error_to_check.into());
+			},
+			_ => panic!("Expected WebhookRegistrationFailed event, got {:?}", event),
+		}
 	}};
 }
 
@@ -1269,7 +1281,11 @@ macro_rules! assert_lsps5_accept {
 			.liquidity_manager
 			.handle_custom_message(response, $service_node_id)
 			.expect("Client should handle response");
-		let _ = $client_node.liquidity_manager.next_event().unwrap();
+		let event = $client_node.liquidity_manager.next_event().unwrap();
+		match event {
+			LiquidityEvent::LSPS5Client(LSPS5ClientEvent::WebhookRegistered { .. }) => {},
+			_ => panic!("Expected WebhookRegistered event, got {:?}", event),
+		}
 	}};
 }