fixup: Clean up LSPS2 service code

- Update docs to focus on behavior not internal states
- Fix channel_open_abandoned to check before removing SCID
- Use mem::take instead of clone in channel_open_failed
- Remove unnecessary HashSet when failing HTLCs
- Improve error messages

Author: martinsaposnic

lightning-liquidity/src/lsps2/service.rs

@@ -36,7 +36,6 @@ use lightning::ln::channelmanager::{AChannelManager, FailureCode, InterceptId};
 use lightning::ln::msgs::{ErrorAction, LightningError};
 use lightning::ln::types::ChannelId;
 use lightning::util::errors::APIError;
-use lightning::util::hash_tables::HashSet;
 use lightning::util::logger::Level;
 
 use lightning_types::payment::PaymentHash;
@@ -989,18 +988,18 @@ where
 	/// Abandons a pending JIT‐open flow for `user_channel_id`, removing all local state.
 	///
 	/// This removes the intercept SCID, any outbound channel state, and associated
-	/// channel‐ID mappings for the specified `user_channel_id`, but only while the JIT
-	/// channel is still in `PendingInitialPayment` or `PendingChannelOpen`.
+	/// channel‐ID mappings for the specified `user_channel_id`, but only while no payment
+	/// has been forwarded yet and no channel has been opened on-chain.
 	///
 	/// Returns an error if:
 	///  - there is no channel matching `user_channel_id`, or
-	///  - the channel has already advanced past `PendingChannelOpen` (e.g. to
-	///    `PendingPaymentForward` or beyond).
+	///  - a payment has already been forwarded or a channel has already been opened
 	///
 	/// Note: this does *not* close or roll back any on‐chain channel which may already
-	/// have been opened. The caller must only invoke this before a channel is actually
-	/// confirmed (or else provide its own on‐chain cleanup) and is responsible for
-	/// managing any pending channel open attempts separately.
+	/// have been opened. The caller must call this before or instead of initiating the channel
+	/// open, as it only affects the local LSPS2 state and doesn't affect any channels that
+	/// might already exist on-chain. Any pending channel open attempts must be managed
+	/// separately.
 	pub fn channel_open_abandoned(
 		&self, counterparty_node_id: &PublicKey, user_channel_id: u128,
 	) -> Result<(), APIError> {
@@ -1013,28 +1012,37 @@ where
 
 		let intercept_scid = peer_state
 			.intercept_scid_by_user_channel_id
-			.remove(&user_channel_id)
+			.get(&user_channel_id)
+			.copied()
 			.ok_or_else(|| APIError::APIMisuseError {
 				err: format!("Could not find a channel with user_channel_id {}", user_channel_id),
 			})?;
 
-		if let Some(jit_channel) =
-			peer_state.outbound_channels_by_intercept_scid.get(&intercept_scid)
-		{
-			if !matches!(
-				jit_channel.state,
-				OutboundJITChannelState::PendingInitialPayment { .. }
-					| OutboundJITChannelState::PendingChannelOpen { .. }
-			) {
-				return Err(APIError::APIMisuseError {
-					err: "Cannot abandon channel open after channel creation or payment forwarding"
-						.to_string(),
-				});
-			}
+		let jit_channel = peer_state
+			.outbound_channels_by_intercept_scid
+			.get(&intercept_scid)
+			.ok_or_else(|| APIError::APIMisuseError {
+				err: format!(
+					"Failed to map intercept_scid {} for user_channel_id {} to a channel.",
+					intercept_scid, user_channel_id,
+				),
+			})?;
+
+		let is_pending = matches!(
+			jit_channel.state,
+			OutboundJITChannelState::PendingInitialPayment { .. }
+				| OutboundJITChannelState::PendingChannelOpen { .. }
+		);
+
+		if !is_pending {
+			return Err(APIError::APIMisuseError {
+				err: "Cannot abandon channel open after channel creation or payment forwarding"
+					.to_string(),
+			});
 		}
 
+		peer_state.intercept_scid_by_user_channel_id.remove(&user_channel_id);
 		peer_state.outbound_channels_by_intercept_scid.remove(&intercept_scid);
-
 		peer_state.intercept_scid_by_channel_id.retain(|_, &mut scid| scid != intercept_scid);
 
 		Ok(())
@@ -1077,33 +1085,22 @@ where
 			),
 		})?;
 
-		if !matches!(jit_channel.state, OutboundJITChannelState::PendingChannelOpen { .. }) {
-			return Err(APIError::APIMisuseError {
-				err: "Channel is not in the PendingChannelOpen state.".to_string(),
-			});
-		}
-
-		let mut payment_queue = match &jit_channel.state {
-			OutboundJITChannelState::PendingChannelOpen { payment_queue, .. } => {
-				payment_queue.clone()
-			},
-			_ => {
+		let mut payment_queue =
+			if let OutboundJITChannelState::PendingChannelOpen { payment_queue, .. } =
+				&mut jit_channel.state
+			{
+				core::mem::take(payment_queue)
+			} else {
 				return Err(APIError::APIMisuseError {
 					err: "Channel is not in the PendingChannelOpen state.".to_string(),
 				});
-			},
-		};
-		let payment_hashes: Vec<_> = payment_queue
-			.clear()
-			.into_iter()
-			.map(|htlc| htlc.payment_hash)
-			.collect::<HashSet<_>>()
-			.into_iter()
-			.collect();
-		for payment_hash in payment_hashes {
-			self.channel_manager
-				.get_cm()
-				.fail_htlc_backwards_with_reason(&payment_hash, FailureCode::TemporaryNodeFailure);
+			};
+
+		for htlc in payment_queue.clear() {
+			self.channel_manager.get_cm().fail_htlc_backwards_with_reason(
+				&htlc.payment_hash,
+				FailureCode::TemporaryNodeFailure,
+			);
 		}
 
 		jit_channel.state = OutboundJITChannelState::PendingInitialPayment { payment_queue };