Persist HTLCs before action calculation

htlc_intercepted only persisted HTLCs when a liquidity action
(splice/open) was needed. This prevents calculate_htlc_actions
from returning empty actions (i.e. "do nothing, let the timer
handle it") because the HTLC would never make it into the store
and the timer would never see it.

Move the insert before calculate_htlc_actions_for_peer so every
intercepted HTLC is in the store before we decide what to do
with it. execute_htlc_actions already removes the HTLC on
successful forward, so the store entry is short-lived on the
happy path.

The cost is two extra KV store round-trips (S3 write + delete)
per HTLC on the forward-only path, where previously there were
none. This hits every payment since LSPS4 intercepts all of
them. Unfortunately unavoidable: the insert must happen before
action calculation because we don't know yet whether the result
will be "forward now" or "do nothing and defer to the timer."
If we wait until after and the result is "do nothing," we've
already lost the HTLC.

Replace the unconditional unwrap on insert with explicit error
handling. On persistence failure:

- Forward-only: proceed. If the forward succeeds the store
  entry was redundant. If persistence failed and the channel
  becomes unusable between calculate and execute, the HTLC is
  orphaned (not in store for timer retry, not forwarded) until
  LDK's CLTV timeout cleans it up. Requires both S3 failure
  and a channel state change between two adjacent calls.

- Liquidity action needed: fail the HTLC back to sender. The
  splice/open is async and the timer needs the stored HTLC to
  forward once the channel is ready. Without it, we'd spend
  on-chain fees on a splice that never results in a forward.

Author: amackillop

lightning-liquidity/src/lsps4/service.rs

@@ -239,22 +239,58 @@ where
 					);
 				}
 
+				// Always persist before calculating actions. execute_htlc_actions
+				// removes the HTLC on successful forward. For the liquidity path
+				// (splice/open) the HTLC must survive in the store so the timer
+				// can forward it once the new channel is ready.
+				let persisted = match self.htlc_store.insert(htlc.clone()) {
+					Ok(_) => true,
+					Err(e) => {
+						log_error!(
+							self.logger,
+							"[LSPS4] htlc_intercepted: failed to persist HTLC {:?}, \
+							 payment_hash: {}, error: {}",
+							intercept_id,
+							payment_hash,
+							e
+						);
+						false
+					}
+				};
+
 				let actions = self.calculate_htlc_actions_for_peer(
 					counterparty_node_id,
-					vec![htlc.clone()],
+					vec![htlc],
 				);
 
-				if actions.needs_liquidity_action() {
-					self.htlc_store.insert(htlc).unwrap();
-				}
-
 				log_debug!(
 					self.logger,
 					"[LSPS4] htlc_intercepted: calculated actions for peer {}: {:?}",
 					counterparty_node_id,
 					actions
 				);
 
+				// Liquidity actions (splice/open) are async — the timer forwards
+				// the HTLC once the channel is ready. Without persistence the
+				// splice/open fires but the HTLC is never forwarded, wasting
+				// on-chain fees for a payment that times out anyway.
+				if !persisted && actions.needs_liquidity_action() {
+					log_error!(
+						self.logger,
+						"[LSPS4] htlc_intercepted: liquidity action needed but HTLC {:?} \
+						 not persisted, failing back to sender. payment_hash: {}",
+						intercept_id,
+						payment_hash
+					);
+					let _ = self.channel_manager.get_cm().fail_intercepted_htlc(intercept_id);
+					return Ok(());
+				}
+
+				// Forward-only path is fine without persistence — the HTLC is
+				// consumed immediately by forward_intercepted_htlc. When persisted,
+				// the store entry also covers the TOCTOU race where the channel
+				// becomes unusable between calculate and execute; the timer retries
+				// from the store.
 				self.execute_htlc_actions(actions, counterparty_node_id.clone());
 			}
 		} else {