Actually remember the order state in `ChannelOrder`
What changed, and why it matters
This commit fixes a bookkeeping bug in the LSPS1 (liquidity service) module. Previously, when a service updated an order's status (e.g., paid, channel opened), it did not actually store the new state or channel details in its internal record. The response was sent with the new state, but the internal `ChannelOrder` was left stale. This could cause the service to report inconsistent order information later, for example when a client later queries the order. The patch now stores and updates the order state and channel details correctly.
Treat as a correctness fix with minor security relevance. Review callers of `update_order_status` and `send_payment_details` to ensure order state transitions are validated (e.g., preventing invalid state changes). Consider adding tests that query an order after updating its state to confirm persistence. No immediate emergency response is indicated.
Security signals we found
State inconsistency between protocol response and internal record
Previously ignored update parameters could lead to stale order data being served on subsequent queries
Fix removes hardcoded placeholder state in response
Adds UnknownOrderId error handling for missing orders
Evidence from the diff
In lightning-liquidity/src/lsps1/peer_state.rs, ChannelOrder gains order_state: LSPS1OrderState and channel_details: Option<LSPS1ChannelInfo>, and new_order initializes them. A new update_order method mutates these fields by order_id, returning PeerStateError::UnknownOrderId if missing. In service.rs, send_payment_details now returns the stored order fields in LSPS1CreateOrderResponse instead of hardcoding order_state: Created and channel: None. update_order_status now calls peer_state_lock.update_order(...) to persist the state and channel details before echoing them back. A prior FIXME about remembering the order state is resolved.
Changed components
lightning-liquidity/src/lsps1/peer_state.rslightning-liquidity/src/lsps1/service.rsLSPS1 service handler order state trackingInspect captured patch +50 / −28
diff --git a/lightning-liquidity/src/lsps1/peer_state.rs b/lightning-liquidity/src/lsps1/peer_state.rs
index 8f7c5a9..a3d2000 100644
--- a/lightning-liquidity/src/lsps1/peer_state.rs
+++ b/lightning-liquidity/src/lsps1/peer_state.rs
@@ -9,7 +9,10 @@
//! Contains peer state objects that are used by `LSPS1ServiceHandler`.
-use super::msgs::{LSPS1OrderId, LSPS1OrderParams, LSPS1PaymentInfo, LSPS1Request};
+use super::msgs::{
+ LSPS1ChannelInfo, LSPS1OrderId, LSPS1OrderParams, LSPS1OrderState, LSPS1PaymentInfo,
+ LSPS1Request,
+};
use crate::lsps0::ser::{LSPSDateTime, LSPSRequestId};
use crate::prelude::HashMap;
@@ -26,13 +29,31 @@ impl PeerState {
pub(super) fn new_order(
&mut self, order_id: LSPS1OrderId, order_params: LSPS1OrderParams,
created_at: LSPSDateTime, payment_details: LSPS1PaymentInfo,
- ) {
- let channel_order = ChannelOrder { order_params, created_at, payment_details };
- self.outbound_channels_by_order_id.insert(order_id, channel_order);
+ ) -> ChannelOrder {
+ let order_state = LSPS1OrderState::Created;
+ let channel_details = None;
+ let channel_order = ChannelOrder {
+ order_params,
+ order_state,
+ created_at,
+ payment_details,
+ channel_details,
+ };
+ self.outbound_channels_by_order_id.insert(order_id, channel_order.clone());
+ channel_order
}
- pub(super) fn get_order<'a>(&'a self, order_id: &LSPS1OrderId) -> Option<&'a ChannelOrder> {
- self.outbound_channels_by_order_id.get(order_id)
+ pub(super) fn update_order<'a>(
+ &'a mut self, order_id: &LSPS1OrderId, order_state: LSPS1OrderState,
+ channel_details: Option<LSPS1ChannelInfo>,
+ ) -> Result<&'a ChannelOrder, PeerStateError> {
+ let order = self
+ .outbound_channels_by_order_id
+ .get_mut(order_id)
+ .ok_or(PeerStateError::UnknownOrderId)?;
+ order.order_state = order_state;
+ order.channel_details = channel_details;
+ Ok(order)
}
pub(super) fn register_request(
@@ -60,6 +81,7 @@ impl PeerState {
pub(super) enum PeerStateError {
UnknownRequestId,
DuplicateRequestId,
+ UnknownOrderId,
}
impl fmt::Display for PeerStateError {
@@ -67,12 +89,16 @@ impl fmt::Display for PeerStateError {
match self {
Self::UnknownRequestId => write!(f, "unknown request id"),
Self::DuplicateRequestId => write!(f, "duplicate request id"),
+ Self::UnknownOrderId => write!(f, "unknown order id"),
}
}
}
+#[derive(Debug, Clone)]
pub(super) struct ChannelOrder {
pub(super) order_params: LSPS1OrderParams,
+ pub(super) order_state: LSPS1OrderState,
pub(super) created_at: LSPSDateTime,
pub(super) payment_details: LSPS1PaymentInfo,
+ pub(super) channel_details: Option<LSPS1ChannelInfo>,
}
diff --git a/lightning-liquidity/src/lsps1/service.rs b/lightning-liquidity/src/lsps1/service.rs
index a75db34..52d9715 100644
--- a/lightning-liquidity/src/lsps1/service.rs
+++ b/lightning-liquidity/src/lsps1/service.rs
@@ -181,7 +181,7 @@ where
/// [`LSPS1ServiceEvent::RequestForPaymentDetails`]: crate::lsps1::event::LSPS1ServiceEvent::RequestForPaymentDetails
pub fn send_payment_details(
&self, request_id: LSPSRequestId, counterparty_node_id: &PublicKey,
- payment: LSPS1PaymentInfo, created_at: LSPSDateTime,
+ payment_details: LSPS1PaymentInfo, created_at: LSPSDateTime,
) -> Result<(), APIError> {
let mut message_queue_notifier = self.pending_messages.notifier();
@@ -198,23 +198,21 @@ where
match request {
LSPS1Request::CreateOrder(params) => {
let order_id = self.generate_order_id();
- peer_state_lock.new_order(
+ let order = peer_state_lock.new_order(
order_id.clone(),
- params.order.clone(),
+ params.order,
created_at,
- payment.clone(),
+ payment_details,
);
let response = LSPS1Response::CreateOrder(LSPS1CreateOrderResponse {
- order: params.order,
+ order: order.order_params,
order_id,
- // TODO, we need to set this in the peer/channel state, and send the
- // set value here:
- order_state: LSPS1OrderState::Created,
- created_at,
- payment,
- channel: None,
+ order_state: order.order_state,
+ created_at: order.created_at,
+ payment: order.payment_details,
+ channel: order.channel_details,
});
let msg = LSPS1Message::Response(request_id, response).into();
message_queue_notifier.enqueue(counterparty_node_id, msg);
@@ -284,7 +282,7 @@ where
/// [`LSPS1ServiceEvent::CheckPaymentConfirmation`]: crate::lsps1::event::LSPS1ServiceEvent::CheckPaymentConfirmation
pub fn update_order_status(
&self, request_id: LSPSRequestId, counterparty_node_id: PublicKey, order_id: LSPS1OrderId,
- order_state: LSPS1OrderState, channel: Option<LSPS1ChannelInfo>,
+ order_state: LSPS1OrderState, channel_details: Option<LSPS1ChannelInfo>,
) -> Result<(), APIError> {
let mut message_queue_notifier = self.pending_messages.notifier();
@@ -292,22 +290,20 @@ where
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
- let peer_state_lock = inner_state_lock.lock().unwrap();
- let order =
- peer_state_lock.get_order(&order_id).ok_or(APIError::APIMisuseError {
- err: format!("Channel with order_id {} not found", order_id.0),
- })?;
-
- // FIXME: we need to actually remember the order state (and eventually persist it)
- // here.
+ let mut peer_state_lock = inner_state_lock.lock().unwrap();
+ let order = peer_state_lock
+ .update_order(&order_id, order_state, channel_details)
+ .map_err(|e| APIError::APIMisuseError {
+ err: format!("Failed to update order: {:?}", e),
+ })?;
let response = LSPS1Response::GetOrder(LSPS1CreateOrderResponse {
order_id,
order: order.order_params.clone(),
- order_state,
+ order_state: order.order_state.clone(),
created_at: order.created_at.clone(),
payment: order.payment_details.clone(),
- channel,
+ channel: order.channel_details.clone(),
});
let msg = LSPS1Message::Response(request_id, response).into();
message_queue_notifier.enqueue(&counterparty_node_id, msg);
Why this scored 26/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.