From 934e829eb5a7db267cffe35ff1a9bdcb3b0abf2a Mon Sep 17 00:00:00 2001
From: Lucas Soriano del Pino <lucas_soriano@fastmail.com>
Date: Wed, 24 Apr 2024 15:27:55 +1000
Subject: [PATCH] Do not hog chain_monitor lock during periodic_check

---
 dlc-manager/src/chain_monitor.rs | 60 ++++++------------------------
 dlc-manager/src/manager.rs       | 64 ++++++++++++++++++++++++++++++--
 2 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/dlc-manager/src/chain_monitor.rs b/dlc-manager/src/chain_monitor.rs
index 85d0d61b..f75bb3c2 100644
--- a/dlc-manager/src/chain_monitor.rs
+++ b/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.
diff --git a/dlc-manager/src/manager.rs b/dlc-manager/src/manager.rs
index 06a31eb1..cc324f27 100644
--- a/dlc-manager/src/manager.rs
+++ b/dlc-manager/src/manager.rs
@@ -377,11 +377,8 @@ 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 +387,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,