fixup: remove bounded_map and use vec for tracking requests

Author: martinsaposnic

lightning-liquidity/src/lsps5/client.rs

@@ -21,7 +21,6 @@ use crate::lsps5::msgs::{
 use crate::message_queue::MessageQueue;
 use crate::prelude::{new_hash_map, HashMap};
 use crate::sync::{Arc, Mutex, RwLock};
-use crate::utils::bounded_map::BoundedMap;
 use crate::utils::generate_request_id;
 
 use super::msgs::{LSPS5AppName, LSPS5Error, LSPS5WebhookUrl};
@@ -32,28 +31,68 @@ use lightning::ln::msgs::{ErrorAction, LightningError};
 use lightning::sign::EntropySource;
 use lightning::util::logger::Level;
 
+use alloc::collections::VecDeque;
 use alloc::string::String;
 
 use core::ops::Deref;
 
+impl PartialEq<LSPSRequestId> for (LSPSRequestId, (LSPS5AppName, LSPS5WebhookUrl)) {
+	fn eq(&self, other: &LSPSRequestId) -> bool {
+		&self.0 == other
+	}
+}
+
+impl PartialEq<LSPSRequestId> for (LSPSRequestId, LSPS5AppName) {
+	fn eq(&self, other: &LSPSRequestId) -> bool {
+		&self.0 == other
+	}
+}
+
 #[derive(Debug, Clone, Copy, Default)]
 /// Configuration for the LSPS5 client
 pub struct LSPS5ClientConfig {}
 
 struct PeerState {
-	pending_set_webhook_requests: BoundedMap<LSPSRequestId, (LSPS5AppName, LSPS5WebhookUrl)>,
-	pending_list_webhooks_requests: BoundedMap<LSPSRequestId, ()>,
-	pending_remove_webhook_requests: BoundedMap<LSPSRequestId, LSPS5AppName>,
+	pending_set_webhook_requests: VecDeque<(LSPSRequestId, (LSPS5AppName, LSPS5WebhookUrl))>,
+	pending_list_webhooks_requests: VecDeque<LSPSRequestId>,
+	pending_remove_webhook_requests: VecDeque<(LSPSRequestId, LSPS5AppName)>,
 }
 
 const MAX_PENDING_REQUESTS: usize = 5;
 
 impl PeerState {
 	fn new() -> Self {
 		Self {
-			pending_set_webhook_requests: BoundedMap::new(MAX_PENDING_REQUESTS),
-			pending_list_webhooks_requests: BoundedMap::new(MAX_PENDING_REQUESTS),
-			pending_remove_webhook_requests: BoundedMap::new(MAX_PENDING_REQUESTS),
+			pending_set_webhook_requests: VecDeque::with_capacity(MAX_PENDING_REQUESTS),
+			pending_list_webhooks_requests: VecDeque::with_capacity(MAX_PENDING_REQUESTS),
+			pending_remove_webhook_requests: VecDeque::with_capacity(MAX_PENDING_REQUESTS),
+		}
+	}
+
+	fn add_request<T, F>(&mut self, item: T, queue_selector: F)
+	where
+		F: FnOnce(&mut Self) -> &mut VecDeque<T>,
+	{
+		let queue = queue_selector(self);
+		if queue.len() == MAX_PENDING_REQUESTS {
+			queue.pop_front();
+		}
+		queue.push_back(item);
+	}
+
+	fn find_and_remove_request<T, F>(
+		&mut self, queue_selector: F, request_id: &LSPSRequestId,
+	) -> Option<T>
+	where
+		F: FnOnce(&mut Self) -> &mut VecDeque<T>,
+		T: Clone,
+		for<'a> &'a T: PartialEq<&'a LSPSRequestId>,
+	{
+		let queue = queue_selector(self);
+		if let Some(pos) = queue.iter().position(|item| item == request_id) {
+			queue.remove(pos)
+		} else {
+			None
 		}
 	}
 
@@ -162,9 +201,10 @@ where
 		let request_id = generate_request_id(&self.entropy_source);
 
 		self.with_peer_state(counterparty_node_id, |peer_state| {
-			peer_state
-				.pending_set_webhook_requests
-				.insert(request_id.clone(), (app_name.clone(), lsps_webhook_url.clone()));
+			peer_state.add_request(
+				(request_id.clone(), (app_name.clone(), lsps_webhook_url.clone())),
+				|s| &mut s.pending_set_webhook_requests,
+			);
 		});
 
 		let request =
@@ -196,7 +236,7 @@ where
 		let request_id = generate_request_id(&self.entropy_source);
 
 		self.with_peer_state(counterparty_node_id, |peer_state| {
-			peer_state.pending_list_webhooks_requests.insert(request_id.clone(), ());
+			peer_state.add_request(request_id.clone(), |s| &mut s.pending_list_webhooks_requests);
 		});
 
 		let request = LSPS5Request::ListWebhooks(ListWebhooksRequest {});
@@ -233,7 +273,9 @@ where
 		let request_id = generate_request_id(&self.entropy_source);
 
 		self.with_peer_state(counterparty_node_id, |peer_state| {
-			peer_state.pending_remove_webhook_requests.insert(request_id.clone(), app_name.clone());
+			peer_state.add_request((request_id.clone(), app_name.clone()), |s| {
+				&mut s.pending_remove_webhook_requests
+			});
 		});
 
 		let request = LSPS5Request::RemoveWebhook(RemoveWebhookRequest { app_name });
@@ -264,8 +306,8 @@ where
 		});
 		let event_queue_notifier = self.pending_events.notifier();
 		let handle_response = |peer_state: &mut PeerState| {
-			if let Some((app_name, webhook_url)) =
-				peer_state.pending_set_webhook_requests.remove(&request_id)
+			if let Some((_, (app_name, webhook_url))) = peer_state
+				.find_and_remove_request(|s| &mut s.pending_set_webhook_requests, &request_id)
 			{
 				match &response {
 					LSPS5Response::SetWebhook(r) => {
@@ -297,7 +339,9 @@ where
 						});
 					},
 				}
-			} else if peer_state.pending_list_webhooks_requests.remove(&request_id).is_some() {
+			} else if let Some(_) = peer_state
+				.find_and_remove_request(|s| &mut s.pending_list_webhooks_requests, &request_id)
+			{
 				match &response {
 					LSPS5Response::ListWebhooks(r) => {
 						event_queue_notifier.enqueue(LSPS5ClientEvent::WebhooksListed {
@@ -315,8 +359,8 @@ where
 						});
 					},
 				}
-			} else if let Some(app_name) =
-				peer_state.pending_remove_webhook_requests.remove(&request_id)
+			} else if let Some((_, app_name)) = peer_state
+				.find_and_remove_request(|s| &mut s.pending_remove_webhook_requests, &request_id)
 			{
 				match &response {
 					LSPS5Response::RemoveWebhook(_) => {
@@ -449,10 +493,16 @@ mod tests {
 			let outer_state_lock = client.per_peer_state.read().unwrap();
 
 			let peer_1_state = outer_state_lock.get(&peer_1).unwrap().lock().unwrap();
-			assert!(peer_1_state.pending_set_webhook_requests.contains_key(&req_id_1));
+			assert!(peer_1_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &req_id_1));
 
 			let peer_2_state = outer_state_lock.get(&peer_2).unwrap().lock().unwrap();
-			assert!(peer_2_state.pending_set_webhook_requests.contains_key(&req_id_2));
+			assert!(peer_2_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &req_id_2));
 		}
 	}
 
@@ -471,17 +521,21 @@ mod tests {
 		{
 			let outer_state_lock = client.per_peer_state.read().unwrap();
 			let peer_state = outer_state_lock.get(&peer).unwrap().lock().unwrap();
-			assert_eq!(
-				peer_state.pending_set_webhook_requests.get(&set_req_id).unwrap(),
-				&(lsps5_app_name.clone(), lsps5_webhook_url)
-			);
+			let set_request = peer_state
+				.pending_set_webhook_requests
+				.iter()
+				.find(|(id, _)| id == &set_req_id)
+				.unwrap();
+			assert_eq!(&set_request.1, &(lsps5_app_name.clone(), lsps5_webhook_url));
 
-			assert!(peer_state.pending_list_webhooks_requests.contains_key(&list_req_id));
+			assert!(peer_state.pending_list_webhooks_requests.contains(&list_req_id));
 
-			assert_eq!(
-				peer_state.pending_remove_webhook_requests.get(&remove_req_id).unwrap(),
-				&lsps5_app_name
-			);
+			let remove_request = peer_state
+				.pending_remove_webhook_requests
+				.iter()
+				.find(|(id, _)| id == &remove_req_id)
+				.unwrap();
+			assert_eq!(&remove_request.1, &lsps5_app_name);
 		}
 	}
 
@@ -523,7 +577,7 @@ mod tests {
 			let outer_state_lock = client.per_peer_state.read().unwrap();
 			let peer_state = outer_state_lock.get(&peer).unwrap().lock().unwrap();
 			for req_id in &request_ids {
-				assert!(peer_state.pending_set_webhook_requests.contains_key(req_id));
+				assert!(peer_state.pending_set_webhook_requests.iter().any(|(id, _)| id == req_id));
 			}
 			assert_eq!(peer_state.pending_set_webhook_requests.len(), MAX_PENDING_REQUESTS);
 		}
@@ -537,13 +591,19 @@ mod tests {
 			let peer_state = outer_state_lock.get(&peer).unwrap().lock().unwrap();
 			assert_eq!(peer_state.pending_set_webhook_requests.len(), MAX_PENDING_REQUESTS);
 
-			assert!(!peer_state.pending_set_webhook_requests.contains_key(&request_ids[0]));
+			assert!(!peer_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &request_ids[0]));
 
 			for req_id in &request_ids[1..] {
-				assert!(peer_state.pending_set_webhook_requests.contains_key(req_id));
+				assert!(peer_state.pending_set_webhook_requests.iter().any(|(id, _)| id == req_id));
 			}
 
-			assert!(peer_state.pending_set_webhook_requests.contains_key(&new_req_id));
+			assert!(peer_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &new_req_id));
 		}
 	}
 
@@ -561,8 +621,11 @@ mod tests {
 			let state = client.per_peer_state.read().unwrap();
 			assert!(state.contains_key(&peer));
 			let peer_state = state.get(&peer).unwrap().lock().unwrap();
-			assert!(peer_state.pending_set_webhook_requests.contains_key(&set_webhook_req_id));
-			assert!(peer_state.pending_list_webhooks_requests.contains_key(&list_webhooks_req_id));
+			assert!(peer_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &set_webhook_req_id));
+			assert!(peer_state.pending_list_webhooks_requests.contains(&list_webhooks_req_id));
 		}
 
 		let set_webhook_response = LSPS5Response::SetWebhook(SetWebhookResponse {
@@ -579,8 +642,11 @@ mod tests {
 			let state = client.per_peer_state.read().unwrap();
 			assert!(state.contains_key(&peer));
 			let peer_state = state.get(&peer).unwrap().lock().unwrap();
-			assert!(!peer_state.pending_set_webhook_requests.contains_key(&set_webhook_req_id));
-			assert!(peer_state.pending_list_webhooks_requests.contains_key(&list_webhooks_req_id));
+			assert!(!peer_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &set_webhook_req_id));
+			assert!(peer_state.pending_list_webhooks_requests.contains(&list_webhooks_req_id));
 		}
 
 		let list_webhooks_response =
@@ -607,7 +673,10 @@ mod tests {
 			let state = client.per_peer_state.read().unwrap();
 			assert!(state.contains_key(&peer));
 			let peer_state = state.get(&peer).unwrap().lock().unwrap();
-			assert!(peer_state.pending_set_webhook_requests.contains_key(&new_req_id));
+			assert!(peer_state
+				.pending_set_webhook_requests
+				.iter()
+				.any(|(id, _)| id == &new_req_id));
 		}
 	}
 }