Merge pull request #17 from get10101/fix/chain-monitor-lock-hog

Do not hog `chain_monitor` lock during `periodic_check`

dlc-manager/src/chain_monitor.rs

@@ -2,7 +2,6 @@
 //! transactions of interest in the context of DLC.
 
 use std::collections::HashMap;
-use std::ops::Deref;
 
 use bitcoin::{OutPoint, Transaction, Txid};
 use dlc_messages::ser_impls::{
@@ -12,7 +11,6 @@ use lightning::ln::msgs::DecodeError;
 use lightning::util::ser::{Readable, Writeable, Writer};
 use secp256k1_zkp::EcdsaAdaptorSignature;
 
-use crate::Blockchain;
 
 /// A `ChainMonitor` keeps a list of transaction ids to watch for in the blockchain,
 /// and some associated information used to apply an action when the id is seen.
@@ -135,56 +133,20 @@ impl ChainMonitor {
         self.watched_tx.remove(txid);
     }
 
-    /// Check if any watched transactions have been confirmed.
-    pub(crate) fn check_transactions<B>(&mut self, blockchain: &B)
-    where
-        B: Deref,
-        B::Target: Blockchain,
-    {
-        for (txid, state) in self.watched_tx.iter_mut() {
-            let confirmations = match blockchain.get_transaction_confirmations(txid) {
-                Ok(confirmations) => confirmations,
-                Err(e) => {
-                    log::error!("Failed to get transaction confirmations for {txid}: {e}");
-                    continue;
-                }
-            };
-
-            if confirmations > 0 {
-                let tx = match blockchain.get_transaction(txid) {
-                    Ok(tx) => tx,
-                    Err(e) => {
-                        log::error!("Failed to get transaction for {txid}: {e}");
-                        continue;
-                    }
-                };
-
-                state.confirm(tx.clone());
-            }
-        }
+    pub(crate) fn get_watched_txs(&self) -> Vec<Txid> {
+        self.watched_tx.keys().cloned().collect()
+    }
 
-        for (txo, state) in self.watched_txo.iter_mut() {
-            let (confirmations, txid) = match blockchain.get_txo_confirmations(txo) {
-                Ok(Some((confirmations, txid))) => (confirmations, txid),
-                Ok(None) => continue,
-                Err(e) => {
-                    log::error!("Failed to get transaction confirmations for {txo}: {e}");
-                    continue;
-                }
-            };
+    pub(crate) fn get_watched_txos(&self) -> Vec<OutPoint> {
+        self.watched_txo.keys().cloned().collect()
+    }
 
-            if confirmations > 0 {
-                let tx = match blockchain.get_transaction(&txid) {
-                    Ok(tx) => tx,
-                    Err(e) => {
-                        log::error!("Failed to get transaction for {txid}: {e}");
-                        continue;
-                    }
-                };
+    pub(crate) fn confirm_tx(&mut self, tx: Transaction) {
+        if let Some(state) = self.watched_tx.get_mut(&tx.txid()) { state.confirm(tx) }
+    }
 
-                state.confirm(tx.clone());
-            }
-        }
+    pub(crate) fn confirm_txo(&mut self, txo: &OutPoint, tx: Transaction) {
+        if let Some(state) = self.watched_txo.get_mut(txo) { state.confirm(tx) }
     }
 
     /// Heuristic to figure out if we sent the last settle offer.

dlc-manager/src/manager.rs

@@ -377,11 +377,7 @@ where
     /// Function to call to check the state of the currently executing DLCs and
     /// update them if possible.
     pub fn periodic_check(&self) -> Result<(), Error> {
-        {
-            let mut chain_monitor = self.chain_monitor.lock().unwrap();
-            chain_monitor.check_transactions(&self.blockchain);
-        }
-
+        self.check_transaction_confirmations();
         self.check_signed_contracts()?;
         self.check_confirmed_contracts()?;
         self.check_preclosed_contracts()?;
@@ -390,6 +386,65 @@ where
         Ok(())
     }
 
+    fn check_transaction_confirmations(&self) {
+        let blockchain = &self.blockchain;
+
+        let (txs, txos) = {
+            let chain_monitor = self.chain_monitor.lock().unwrap();
+            let txs = chain_monitor.get_watched_txs();
+            let txos = chain_monitor.get_watched_txos();
+
+            (txs, txos)
+        };
+
+        for txid in txs {
+            let confirmations = match blockchain.get_transaction_confirmations(&txid) {
+                Ok(confirmations) => confirmations,
+                Err(e) => {
+                    log::error!("Failed to get transaction confirmations for {txid}: {e}");
+                    continue;
+                }
+            };
+
+            if confirmations > 0 {
+                let tx = match blockchain.get_transaction(&txid) {
+                    Ok(tx) => tx,
+                    Err(e) => {
+                        log::error!("Failed to get transaction for {txid}: {e}");
+                        continue;
+                    }
+                };
+
+                let mut chain_monitor = self.chain_monitor.lock().unwrap();
+                chain_monitor.confirm_tx(tx);
+            }
+        }
+
+        for txo in txos {
+            let (confirmations, txid) = match blockchain.get_txo_confirmations(&txo) {
+                Ok(Some((confirmations, txid))) => (confirmations, txid),
+                Ok(None) => continue,
+                Err(e) => {
+                    log::error!("Failed to get transaction confirmations for {txo}: {e}");
+                    continue;
+                }
+            };
+
+            if confirmations > 0 {
+                let tx = match blockchain.get_transaction(&txid) {
+                    Ok(tx) => tx,
+                    Err(e) => {
+                        log::error!("Failed to get transaction for {txid}: {e}");
+                        continue;
+                    }
+                };
+
+                let mut chain_monitor = self.chain_monitor.lock().unwrap();
+                chain_monitor.confirm_txo(&txo, tx.clone());
+            }
+        }
+    }
+
     fn on_offer_message(
         &self,
         offered_message: &OfferDlc,

p2pd-oracle-client/src/lib.rs

@@ -18,7 +18,7 @@ extern crate reqwest;
 extern crate secp256k1_zkp;
 extern crate serde;
 
-use chrono::{DateTime, NaiveDateTime, SecondsFormat, Utc};
+use chrono::{DateTime, SecondsFormat, Utc};
 use dlc_manager::error::Error as DlcManagerError;
 use dlc_manager::Oracle;
 use dlc_messages::oracle_msgs::{OracleAnnouncement, OracleAttestation};
@@ -132,10 +132,9 @@ fn parse_event_id(event_id: &str) -> Result<(String, DateTime<Utc>), DlcManagerE
     let timestamp: i64 = timestamp_str
         .parse()
         .map_err(|_| DlcManagerError::OracleError("Invalid timestamp format".to_string()))?;
-    let naive_date_time = NaiveDateTime::from_timestamp_opt(timestamp, 0).ok_or_else(|| {
+    let date_time = DateTime::<Utc>::from_timestamp(timestamp, 0).ok_or_else(|| {
         DlcManagerError::InvalidParameters(format!("Invalid timestamp {} in event id", timestamp))
     })?;
-    let date_time = DateTime::from_naive_utc_and_offset(naive_date_time, Utc);
     Ok((asset_id.to_string(), date_time))
 }