fixups addressing comments. list of changes:

- get rid of dos_protection_enforcer. logic will go inside lsps5/service
- improve naming conventions so it's clearer
- do not log on level info for possible spam
- make the dos protections enabled by default

Author: martinsaposnic

lightning-liquidity/src/dos_protection_enforcer.rs

@@ -58,7 +58,6 @@ mod prelude {
 	pub(crate) use lightning::util::hash_tables::*;
 }
 
-pub mod dos_protection_enforcer;
 pub mod events;
 pub mod lsps0;
 pub mod lsps1;

lightning-liquidity/src/lib.rs

@@ -108,8 +108,8 @@ struct ForwardPaymentAction(ChannelId, FeePayment);
 struct ForwardHTLCsAction(ChannelId, Vec<InterceptedHTLC>);
 
 /// The different states a requested JIT channel can be in.
-#[derive(Debug)]
-enum OutboundJITChannelState {
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub(crate) enum OutboundJITChannelState {
 	/// The JIT channel SCID was created after a buy request, and we are awaiting an initial payment
 	/// of sufficient size to open the channel.
 	PendingInitialPayment { payment_queue: PaymentQueue },
@@ -134,6 +134,30 @@ enum OutboundJITChannelState {
 	PaymentForwarded { channel_id: ChannelId },
 }
 
+impl OutboundJITChannelState {
+	fn ord_index(&self) -> u8 {
+		match self {
+			OutboundJITChannelState::PendingInitialPayment { .. } => 0,
+			OutboundJITChannelState::PendingChannelOpen { .. } => 1,
+			OutboundJITChannelState::PendingPaymentForward { .. } => 2,
+			OutboundJITChannelState::PendingPayment { .. } => 3,
+			OutboundJITChannelState::PaymentForwarded { .. } => 4,
+		}
+	}
+}
+
+impl PartialOrd for OutboundJITChannelState {
+	fn partial_cmp(&self, other: &Self) -> Option<CmpOrdering> {
+		Some(self.cmp(other))
+	}
+}
+
+impl Ord for OutboundJITChannelState {
+	fn cmp(&self, other: &Self) -> core::cmp::Ordering {
+		self.ord_index().cmp(&other.ord_index())
+	}
+}
+
 impl OutboundJITChannelState {
 	fn new() -> Self {
 		OutboundJITChannelState::PendingInitialPayment { payment_queue: PaymentQueue::new() }
@@ -427,10 +451,6 @@ impl OutboundJITChannel {
 		matches!(self.state, OutboundJITChannelState::PendingInitialPayment { .. })
 	}
 
-	fn is_pending_channel_open(&self) -> bool {
-		matches!(self.state, OutboundJITChannelState::PendingChannelOpen { .. })
-	}
-
 	fn is_prunable(&self) -> bool {
 		// We deem an OutboundJITChannel prunable if our offer expired and we haven't intercepted
 		// any HTLCs initiating the flow yet.
@@ -576,18 +596,15 @@ where
 		&self.config
 	}
 
-	pub(crate) fn has_pending_channel_open_request(
+	pub(crate) fn highest_state_for_peer(
 		&self, counterparty_node_id: &PublicKey,
-	) -> bool {
+	) -> Option<OutboundJITChannelState> {
 		let outer_state_lock = self.per_peer_state.read().unwrap();
 		if let Some(inner_state_lock) = outer_state_lock.get(counterparty_node_id) {
 			let peer_state = inner_state_lock.lock().unwrap();
-			peer_state
-				.outbound_channels_by_intercept_scid
-				.values()
-				.any(|c| c.is_pending_channel_open())
+			peer_state.outbound_channels_by_intercept_scid.values().map(|c| c.state.clone()).max()
 		} else {
-			false
+			None
 		}
 	}
 
@@ -1924,4 +1941,55 @@ mod tests {
 			);
 		}
 	}
+
+	#[test]
+	fn highest_state_for_peer_orders() {
+		let opening_fee_params = LSPS2OpeningFeeParams {
+			min_fee_msat: 0,
+			proportional: 0,
+			valid_until: LSPSDateTime::from_str("1970-01-01T00:00:00Z").unwrap(),
+			min_lifetime: 0,
+			max_client_to_self_delay: 0,
+			min_payment_size_msat: 0,
+			max_payment_size_msat: 0,
+			promise: String::new(),
+		};
+
+		let mut map = new_hash_map();
+		map.insert(
+			0,
+			OutboundJITChannel {
+				state: OutboundJITChannelState::PendingInitialPayment {
+					payment_queue: PaymentQueue::new(),
+				},
+				user_channel_id: 0,
+				opening_fee_params: opening_fee_params.clone(),
+				payment_size_msat: None,
+			},
+		);
+		map.insert(
+			1,
+			OutboundJITChannel {
+				state: OutboundJITChannelState::PendingChannelOpen {
+					payment_queue: PaymentQueue::new(),
+					opening_fee_msat: 0,
+				},
+				user_channel_id: 1,
+				opening_fee_params: opening_fee_params.clone(),
+				payment_size_msat: None,
+			},
+		);
+		map.insert(
+			2,
+			OutboundJITChannel {
+				state: OutboundJITChannelState::PaymentForwarded { channel_id: ChannelId([0; 32]) },
+				user_channel_id: 2,
+				opening_fee_params,
+				payment_size_msat: None,
+			},
+		);
+
+		let max_state = map.values().map(|c| c.state.clone()).max().unwrap();
+		assert!(matches!(max_state, OutboundJITChannelState::PaymentForwarded { .. }));
+	}
 }

lightning-liquidity/src/lsps2/service.rs

@@ -22,6 +22,7 @@ use crate::prelude::*;
 use crate::sync::{Arc, Mutex};
 use crate::utils::time::TimeProvider;
 
+use crate::lsps2::service::OutboundJITChannelState;
 use bitcoin::secp256k1::PublicKey;
 
 use lightning::ln::channelmanager::AChannelManager;
@@ -64,8 +65,6 @@ struct StoredWebhook {
 pub struct LSPS5ServiceConfig {
 	/// Maximum number of webhooks allowed per client.
 	pub max_webhooks_per_client: u32,
-	/// Require an existing channel or active LSPS1/LSPS2 flow before accepting requests.
-	pub enforce_dos_protections: bool,
 }
 
 /// Default maximum number of webhooks allowed per client.
@@ -76,10 +75,7 @@ pub const DEFAULT_NOTIFICATION_COOLDOWN_HOURS: Duration = Duration::from_secs(60
 // Default configuration for LSPS5 service.
 impl Default for LSPS5ServiceConfig {
 	fn default() -> Self {
-		Self {
-			max_webhooks_per_client: DEFAULT_MAX_WEBHOOKS_PER_CLIENT,
-			enforce_dos_protections: true,
-		}
+		Self { max_webhooks_per_client: DEFAULT_MAX_WEBHOOKS_PER_CLIENT }
 	}
 }
 
@@ -155,9 +151,26 @@ where
 		}
 	}
 
-	/// Returns a reference to the used config.
-	pub fn config(&self) -> &LSPS5ServiceConfig {
-		&self.config
+	/// 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 progressed to at least
+	/// [`OutboundJITChannelState::PendingChannelOpen`].
+	pub(crate) fn can_accept_request(
+		&self, client_id: &PublicKey, lsps2_max_state: Option<OutboundJITChannelState>,
+		lsps1_has_activity: bool,
+	) -> bool {
+		self.client_has_open_channel(client_id)
+			|| lsps1_has_activity
+			|| lsps2_max_state.map_or(false, |s| {
+				matches!(
+					s,
+					OutboundJITChannelState::PendingChannelOpen { .. }
+						| OutboundJITChannelState::PendingPaymentForward { .. }
+						| OutboundJITChannelState::PendingPayment { .. }
+						| OutboundJITChannelState::PaymentForwarded { .. }
+				)
+			})
 	}
 
 	fn check_prune_stale_webhooks(&self) {

lightning-liquidity/src/lsps5/service.rs

@@ -1,7 +1,6 @@
 use alloc::string::ToString;
 use alloc::vec::Vec;
 
-use crate::dos_protection_enforcer::DosProtectionEnforcer;
 use crate::events::{EventQueue, LiquidityEvent};
 use crate::lsps0::client::LSPS0ClientHandler;
 use crate::lsps0::msgs::LSPS0Message;
@@ -484,18 +483,6 @@ where
 		self.pending_events.get_and_clear_pending_events()
 	}
 
-	fn peer_is_engaged(&self, peer: &PublicKey) -> bool {
-		let lsps5_engaged =
-			self.lsps5_service_handler.as_ref().map_or(false, |h| h.is_engaged(peer));
-		let lsps2_engaged =
-			self.lsps2_service_handler.as_ref().map_or(false, |h| h.is_engaged(peer));
-		#[cfg(lsps1_service)]
-		let lsps1_engaged = self.lsps1_service_handler.as_ref().map_or(false, |h| h.is_engaged(peer));
-		#[cfg(not(lsps1_service))]
-		let lsps1_engaged = false;
-		lsps5_engaged || lsps2_engaged || lsps1_engaged
-	}
-
 	fn handle_lsps_message(
 		&self, msg: LSPSMessage, sender_node_id: &PublicKey,
 	) -> Result<(), lightning::ln::msgs::LightningError> {
@@ -572,17 +559,32 @@ where
 			LSPSMessage::LSPS5(msg @ LSPS5Message::Request(..)) => {
 				match &self.lsps5_service_handler {
 					Some(lsps5_service_handler) => {
-						if lsps5_service_handler.config().enforce_dos_protections {
-							if !self.peer_is_engaged(sender_node_id) {
-								return Err(LightningError {
+						let lsps2_max_state = self
+							.lsps2_service_handler
+							.as_ref()
+							.and_then(|h| h.highest_state_for_peer(sender_node_id));
+						#[cfg(lsps1_service)]
+						let lsps1_has_active_requests = self
+							.lsps1_service_handler
+							.as_ref()
+							.map_or(false, |h| h.has_active_requests(sender_node_id));
+						#[cfg(not(lsps1_service))]
+						let lsps1_has_active_requests = false;
+
+						if !lsps5_service_handler.can_accept_request(
+							sender_node_id,
+							lsps2_max_state,
+							lsps1_has_active_requests,
+						) {
+							return Err(LightningError {
                                     err: format!(
-                                        "Rejecting LSPS5 request from {:?} without existing engagement",
+                                        "Rejecting LSPS5 request from {:?} without prior activity (requires open channel or active LSPS1 or LSPS2 flow)",
                                         sender_node_id
                                     ),
-                                    action: ErrorAction::IgnoreAndLog(Level::Info),
+                                    action: ErrorAction::IgnoreAndLog(Level::Debug),
                                 });
-							}
 						}
+
 						lsps5_service_handler.handle_message(msg, sender_node_id)?;
 					},
 					None => {