Skip to content

Commit 309591a

Browse files
tnulltnull
committed
Simplify message handling logic
Now that we have the `MessageQueueNotifierGuard`, we can be sure that we always dropped locks before notifying. Hence, we can save *a lot* of error-prone boilerplate that we used to ensure we'd only enqueue if we dropped locks.
1 parent 107de27 commit 309591a

File tree

3 files changed

+266
-380
lines changed

3 files changed

+266
-380
lines changed

‎lightning-liquidity/src/lsps1/client.rs

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -202,27 +202,19 @@ where
202202
) -> LSPSRequestId {
203203
let mut message_queue_notifier = self.pending_messages.notifier();
204204

205-
let (request_id, request_msg) = {
206-
let mut outer_state_lock = self.per_peer_state.write().unwrap();
207-
let inner_state_lock = outer_state_lock
208-
.entry(*counterparty_node_id)
209-
.or_insert(Mutex::new(PeerState::default()));
210-
let mut peer_state_lock = inner_state_lock.lock().unwrap();
211-
212-
let request_id = crate::utils::generate_request_id(&self.entropy_source);
213-
let request = LSPS1Request::CreateOrder(LSPS1CreateOrderRequest {
214-
order,
215-
refund_onchain_address,
216-
});
217-
let msg = LSPS1Message::Request(request_id.clone(), request).into();
218-
peer_state_lock.pending_create_order_requests.insert(request_id.clone());
205+
let mut outer_state_lock = self.per_peer_state.write().unwrap();
206+
let inner_state_lock = outer_state_lock
207+
.entry(*counterparty_node_id)
208+
.or_insert(Mutex::new(PeerState::default()));
209+
let mut peer_state_lock = inner_state_lock.lock().unwrap();
219210

220-
(request_id, Some(msg))
221-
};
211+
let request_id = crate::utils::generate_request_id(&self.entropy_source);
212+
let request =
213+
LSPS1Request::CreateOrder(LSPS1CreateOrderRequest { order, refund_onchain_address });
214+
let msg = LSPS1Message::Request(request_id.clone(), request).into();
215+
peer_state_lock.pending_create_order_requests.insert(request_id.clone());
222216

223-
if let Some(msg) = request_msg {
224-
message_queue_notifier.enqueue(&counterparty_node_id, msg);
225-
}
217+
message_queue_notifier.enqueue(&counterparty_node_id, msg);
226218

227219
request_id
228220
}
@@ -328,26 +320,19 @@ where
328320
) -> LSPSRequestId {
329321
let mut message_queue_notifier = self.pending_messages.notifier();
330322

331-
let (request_id, request_msg) = {
332-
let mut outer_state_lock = self.per_peer_state.write().unwrap();
333-
let inner_state_lock = outer_state_lock
334-
.entry(*counterparty_node_id)
335-
.or_insert(Mutex::new(PeerState::default()));
336-
let mut peer_state_lock = inner_state_lock.lock().unwrap();
337-
338-
let request_id = crate::utils::generate_request_id(&self.entropy_source);
339-
peer_state_lock.pending_get_order_requests.insert(request_id.clone());
323+
let mut outer_state_lock = self.per_peer_state.write().unwrap();
324+
let inner_state_lock = outer_state_lock
325+
.entry(*counterparty_node_id)
326+
.or_insert(Mutex::new(PeerState::default()));
327+
let mut peer_state_lock = inner_state_lock.lock().unwrap();
340328

341-
let request =
342-
LSPS1Request::GetOrder(LSPS1GetOrderRequest { order_id: order_id.clone() });
343-
let msg = LSPS1Message::Request(request_id.clone(), request).into();
329+
let request_id = crate::utils::generate_request_id(&self.entropy_source);
330+
peer_state_lock.pending_get_order_requests.insert(request_id.clone());
344331

345-
(request_id, Some(msg))
346-
};
332+
let request = LSPS1Request::GetOrder(LSPS1GetOrderRequest { order_id: order_id.clone() });
333+
let msg = LSPS1Message::Request(request_id.clone(), request).into();
347334

348-
if let Some(msg) = request_msg {
349-
message_queue_notifier.enqueue(&counterparty_node_id, msg);
350-
}
335+
message_queue_notifier.enqueue(&counterparty_node_id, msg);
351336

352337
request_id
353338
}

‎lightning-liquidity/src/lsps1/service.rs

Lines changed: 66 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -255,66 +255,46 @@ where
255255
payment: LSPS1PaymentInfo, created_at: LSPSDateTime,
256256
) -> Result<(), APIError> {
257257
let mut message_queue_notifier = self.pending_messages.notifier();
258-
let (result, response) = {
259-
let outer_state_lock = self.per_peer_state.read().unwrap();
260-
261-
match outer_state_lock.get(counterparty_node_id) {
262-
Some(inner_state_lock) => {
263-
let mut peer_state_lock = inner_state_lock.lock().unwrap();
264-
265-
match peer_state_lock.pending_requests.remove(&request_id) {
266-
Some(LSPS1Request::CreateOrder(params)) => {
267-
let order_id = self.generate_order_id();
268-
let channel = OutboundCRChannel::new(
269-
params.order.clone(),
270-
created_at.clone(),
271-
order_id.clone(),
272-
payment.clone(),
273-
);
274-
275-
peer_state_lock.insert_outbound_channel(order_id.clone(), channel);
276-
277-
let response = LSPS1Response::CreateOrder(LSPS1CreateOrderResponse {
278-
order: params.order,
279-
order_id,
280-
order_state: LSPS1OrderState::Created,
281-
created_at,
282-
payment,
283-
channel: None,
284-
});
285-
286-
(Ok(()), Some(response))
287-
},
288-
289-
_ => (
290-
Err(APIError::APIMisuseError {
291-
err: format!(
292-
"No pending buy request for request_id: {:?}",
293-
request_id
294-
),
295-
}),
296-
None,
297-
),
298-
}
299-
},
300-
None => (
301-
Err(APIError::APIMisuseError {
302-
err: format!(
303-
"No state for the counterparty exists: {:?}",
304-
counterparty_node_id
305-
),
306-
}),
307-
None,
308-
),
309-
}
310-
};
311258

312-
if let Some(response) = response {
313-
let msg = LSPS1Message::Response(request_id, response).into();
314-
message_queue_notifier.enqueue(counterparty_node_id, msg);
315-
}
259+
let outer_state_lock = self.per_peer_state.read().unwrap();
260+
match outer_state_lock.get(counterparty_node_id) {
261+
Some(inner_state_lock) => {
262+
let mut peer_state_lock = inner_state_lock.lock().unwrap();
263+
264+
match peer_state_lock.pending_requests.remove(&request_id) {
265+
Some(LSPS1Request::CreateOrder(params)) => {
266+
let order_id = self.generate_order_id();
267+
let channel = OutboundCRChannel::new(
268+
params.order.clone(),
269+
created_at.clone(),
270+
order_id.clone(),
271+
payment.clone(),
272+
);
316273

317-
result
274+
peer_state_lock.insert_outbound_channel(order_id.clone(), channel);
275+
276+
let response = LSPS1Response::CreateOrder(LSPS1CreateOrderResponse {
277+
order: params.order,
278+
order_id,
279+
order_state: LSPS1OrderState::Created,
280+
created_at,
281+
payment,
282+
channel: None,
283+
});
284+
let msg = LSPS1Message::Response(request_id, response).into();
285+
message_queue_notifier.enqueue(counterparty_node_id, msg);
286+
Ok(())
287+
},
288+
289+
_ => Err(APIError::APIMisuseError {
290+
err: format!("No pending buy request for request_id: {:?}", request_id),
291+
}),
292+
}
293+
},
294+
None => Err(APIError::APIMisuseError {
295+
err: format!("No state for the counterparty exists: {:?}", counterparty_node_id),
296+
}),
297+
}
318298
}
319299

320300
fn handle_get_order_request(
@@ -383,54 +363,38 @@ where
383363
) -> Result<(), APIError> {
384364
let mut message_queue_notifier = self.pending_messages.notifier();
385365

386-
let (result, response) = {
387-
let outer_state_lock = self.per_peer_state.read().unwrap();
388-
389-
match outer_state_lock.get(&counterparty_node_id) {
390-
Some(inner_state_lock) => {
391-
let mut peer_state_lock = inner_state_lock.lock().unwrap();
366+
let outer_state_lock = self.per_peer_state.read().unwrap();
392367

393-
if let Some(outbound_channel) =
394-
peer_state_lock.outbound_channels_by_order_id.get_mut(&order_id)
395-
{
396-
let config = &outbound_channel.config;
368+
match outer_state_lock.get(&counterparty_node_id) {
369+
Some(inner_state_lock) => {
370+
let mut peer_state_lock = inner_state_lock.lock().unwrap();
397371

398-
let response = LSPS1Response::GetOrder(LSPS1CreateOrderResponse {
399-
order_id,
400-
order: config.order.clone(),
401-
order_state,
402-
created_at: config.created_at.clone(),
403-
payment: config.payment.clone(),
404-
channel,
405-
});
406-
(Ok(()), Some(response))
407-
} else {
408-
(
409-
Err(APIError::APIMisuseError {
410-
err: format!("Channel with order_id {} not found", order_id.0),
411-
}),
412-
None,
413-
)
414-
}
415-
},
416-
None => (
372+
if let Some(outbound_channel) =
373+
peer_state_lock.outbound_channels_by_order_id.get_mut(&order_id)
374+
{
375+
let config = &outbound_channel.config;
376+
377+
let response = LSPS1Response::GetOrder(LSPS1CreateOrderResponse {
378+
order_id,
379+
order: config.order.clone(),
380+
order_state,
381+
created_at: config.created_at.clone(),
382+
payment: config.payment.clone(),
383+
channel,
384+
});
385+
let msg = LSPS1Message::Response(request_id, response).into();
386+
message_queue_notifier.enqueue(&counterparty_node_id, msg);
387+
Ok(())
388+
} else {
417389
Err(APIError::APIMisuseError {
418-
err: format!(
419-
"No existing state with counterparty {}",
420-
counterparty_node_id
421-
),
422-
}),
423-
None,
424-
),
425-
}
426-
};
427-
428-
if let Some(response) = response {
429-
let msg = LSPS1Message::Response(request_id, response).into();
430-
message_queue_notifier.enqueue(&counterparty_node_id, msg);
390+
err: format!("Channel with order_id {} not found", order_id.0),
391+
})
392+
}
393+
},
394+
None => Err(APIError::APIMisuseError {
395+
err: format!("No existing state with counterparty {}", counterparty_node_id),
396+
}),
431397
}
432-
433-
result
434398
}
435399

436400
fn generate_order_id(&self) -> LSPS1OrderId {

0 commit comments

Comments
 (0)