Implement comments. Rebase needed

Author: martinsaposnic

lightning-liquidity/Cargo.toml

@@ -47,5 +47,4 @@ check-cfg = [
 	"cfg(c_bindings)",
 	"cfg(backtrace)",
 	"cfg(ldk_bench)",
-	"cfg(time)",
 ]

lightning-liquidity/src/lsps0/ser.rs

@@ -238,14 +238,15 @@ impl LSPSDateTime {
 		now_seconds_since_epoch > datetime_seconds_since_epoch
 	}
 
-	/// Returns the number of seconds since the Unix epoch.
-	pub fn timestamp(&self) -> u64 {
-		self.0.timestamp() as u64
+	/// Returns the time in seconds since the unix epoch.
+	pub fn abs_diff(&self, other: &Self) -> u64 {
+		self.0.timestamp().abs_diff(other.0.timestamp())
 	}
+}
 
-	/// Returns the number of seconds since the Unix epoch.
-	pub fn new_from_duration(duration: Duration) -> Self {
-		Self(DateTime::from_timestamp(duration.as_secs() as i64, duration.subsec_nanos()).unwrap())
+impl From<Duration> for LSPSDateTime {
+	fn from(duration: Duration) -> Self {
+		Self(DateTime::UNIX_EPOCH + duration)
 	}
 }
 

lightning-liquidity/src/lsps5/client.rs

@@ -217,7 +217,7 @@ where
 
 		peer_state_lock.cleanup_expired_responses(
 			self.config.response_max_age_secs,
-			self.time_provider.clone(),
+			Arc::clone(&self.time_provider),
 		);
 
 		Ok(f(&mut *peer_state_lock))
@@ -240,25 +240,15 @@ where
 	pub fn set_webhook(
 		&self, counterparty_node_id: PublicKey, app_name: String, webhook_url: String,
 	) -> Result<LSPSRequestId, LightningError> {
-		let app_name = match LSPS5AppName::new(app_name) {
-			Ok(app_name) => app_name,
-			Err(e) => {
-				return Err(LightningError {
-					err: e.message,
-					action: ErrorAction::IgnoreAndLog(Level::Error),
-				})
-			},
-		};
+		let app_name = LSPS5AppName::new(app_name).map_err(|e| LightningError {
+			err: e.message,
+			action: ErrorAction::IgnoreAndLog(Level::Error),
+		})?;
 
-		let lsps_webhook_url = match LSPS5WebhookUrl::new(webhook_url) {
-			Ok(webhook) => webhook,
-			Err(e) => {
-				return Err(LightningError {
-					err: e.message,
-					action: ErrorAction::IgnoreAndLog(Level::Error),
-				})
-			},
-		};
+		let lsps_webhook_url = LSPS5WebhookUrl::new(webhook_url).map_err(|e| LightningError {
+			err: e.message,
+			action: ErrorAction::IgnoreAndLog(Level::Error),
+		})?;
 
 		let request_id = generate_request_id(&self.entropy_source);
 
@@ -296,15 +286,13 @@ where
 		&self, counterparty_node_id: PublicKey,
 	) -> Result<LSPSRequestId, LightningError> {
 		let request_id = generate_request_id(&self.entropy_source);
+		let now = self.time_provider.duration_since_epoch();
 
 		self.with_peer_state(counterparty_node_id, |peer_state| {
-			peer_state
-				.pending_list_webhooks_requests
-				.insert(request_id.clone(), self.time_provider.duration_since_epoch());
+			peer_state.pending_list_webhooks_requests.insert(request_id.clone(), now);
 		})?;
 
 		let request = LSPS5Request::ListWebhooks(ListWebhooksRequest {});
-
 		let message = LSPS5Message::Request(request_id.clone(), request);
 		self.pending_messages.enqueue(&counterparty_node_id, LSPSMessage::LSPS5(message));
 
@@ -327,27 +315,21 @@ where
 	pub fn remove_webhook(
 		&self, counterparty_node_id: PublicKey, app_name: String,
 	) -> Result<LSPSRequestId, LightningError> {
-		let app_name = match LSPS5AppName::new(app_name) {
-			Ok(app_name) => app_name,
-			Err(e) => {
-				return Err(LightningError {
-					err: e.message,
-					action: ErrorAction::IgnoreAndLog(Level::Error),
-				})
-			},
-		};
+		let app_name = LSPS5AppName::new(app_name).map_err(|e| LightningError {
+			err: e.message,
+			action: ErrorAction::IgnoreAndLog(Level::Error),
+		})?;
 
 		let request_id = generate_request_id(&self.entropy_source);
+		let now = self.time_provider.duration_since_epoch();
 
 		self.with_peer_state(counterparty_node_id, |peer_state| {
-			peer_state.pending_remove_webhook_requests.insert(
-				request_id.clone(),
-				(app_name.clone(), self.time_provider.duration_since_epoch()),
-			);
+			peer_state
+				.pending_remove_webhook_requests
+				.insert(request_id.clone(), (app_name.clone(), now));
 		})?;
 
 		let request = LSPS5Request::RemoveWebhook(RemoveWebhookRequest { app_name });
-
 		let message = LSPS5Message::Request(request_id.clone(), request);
 		self.pending_messages.enqueue(&counterparty_node_id, LSPSMessage::LSPS5(message));
 
@@ -513,39 +495,30 @@ where
 		&self, counterparty_node_id: PublicKey, timestamp: &str, signature: &str,
 		notification: &WebhookNotification,
 	) -> Result<bool, LightningError> {
-		match LSPSDateTime::from_str(timestamp) {
-			Ok(timestamp_dt) => {
-				let now = self.time_provider.duration_since_epoch();
-				let signature_timestamp = timestamp_dt.timestamp();
-				let diff = if now.as_secs() > signature_timestamp {
-					now.as_secs() - signature_timestamp
-				} else {
-					signature_timestamp - now.as_secs()
-				};
+		LSPSDateTime::from_str(timestamp)
+			.map_err(|_| LightningError {
+				err: format!("Invalid timestamp format: {}", timestamp),
+				action: ErrorAction::IgnoreAndLog(Level::Error),
+			})
+			.and_then(|signature_timestamp| {
+				let now = LSPSDateTime::from(self.time_provider.duration_since_epoch());
+				let diff = signature_timestamp.abs_diff(&now);
+
 				if diff > 600 {
 					return Err(LightningError {
 						err: format!("Timestamp too old: {}", timestamp),
 						action: ErrorAction::IgnoreAndLog(Level::Error),
 					});
 				}
-			},
-			Err(_e) => {
-				return Err(LightningError {
-					err: format!("Invalid timestamp format: {}", timestamp),
-					action: ErrorAction::IgnoreAndLog(Level::Error),
-				});
-			},
-		}
 
-		let notification_json = match serde_json::to_string(notification) {
-			Ok(json) => json,
-			Err(e) => {
-				return Err(LightningError {
-					err: format!("Failed to serialize notification: {}", e),
-					action: ErrorAction::IgnoreAndLog(Level::Error),
-				});
-			},
-		};
+				Ok(())
+			})?;
+
+		let notification_json =
+			serde_json::to_string(notification).map_err(|e| LightningError {
+				err: format!("Failed to serialize notification: {}", e),
+				action: ErrorAction::IgnoreAndLog(Level::Error),
+			})?;
 
 		let message = format!(
 			"LSPS5: DO NOT SIGN THIS MESSAGE MANUALLY: LSP: At {} I notify {}",
@@ -572,7 +545,7 @@ where
 		for (stored_sig, _) in recent_signatures.iter() {
 			if stored_sig == signature {
 				return Err(LightningError {
-					err: format!("Replay attack detected: signature has been used before"),
+					err: "Replay attack detected: signature has been used before".to_string(),
 					action: ErrorAction::IgnoreAndLog(Level::Warn),
 				});
 			}
@@ -594,11 +567,10 @@ where
 		let retention_duration =
 			Duration::from_secs(self.config.signature_config.retention_minutes * 60);
 		while let Some((_, time)) = recent_signatures.front() {
-			match now.checked_sub(*time) {
-				Some(duration) if duration > retention_duration => {
-					recent_signatures.pop_front();
-				},
-				_ => break,
+			if now.checked_sub(*time).map_or(false, |duration| duration > retention_duration) {
+				recent_signatures.pop_front();
+			} else {
+				break;
 			}
 		}
 
@@ -627,15 +599,11 @@ where
 		&self, counterparty_node_id: PublicKey, timestamp: &str, signature: &str,
 		notification_json: &str,
 	) -> Result<WebhookNotification, LightningError> {
-		let notification: WebhookNotification = match serde_json::from_str(notification_json) {
-			Ok(n) => n,
-			Err(e) => {
-				return Err(LightningError {
-					err: format!("Failed to parse notification: {}", e),
-					action: ErrorAction::IgnoreAndLog(Level::Error),
-				});
-			},
-		};
+		let notification: WebhookNotification =
+			serde_json::from_str(notification_json).map_err(|e| LightningError {
+				err: format!("Failed to parse notification: {}", e),
+				action: ErrorAction::IgnoreAndLog(Level::Error),
+			})?;
 
 		self.check_signature_exists(signature)?;