address review comments from shesek part II

Author: jamesdorfman

src/new_index/mempool.rs

@@ -15,6 +15,7 @@ use crate::chain::{deserialize, Network, OutPoint, Transaction, TxOut, Txid};
 use crate::config::Config;
 use crate::daemon::Daemon;
 use crate::errors::*;
+use crate::error_chain::ChainedError;
 use crate::metrics::{GaugeVec, HistogramOpts, HistogramVec, MetricOpts, Metrics};
 use crate::new_index::{
     compute_script_hash, schema::FullHash, ChainQuery, FundingInfo, GetAmountVal, ScriptStats,
@@ -280,73 +281,6 @@ impl Mempool {
         return HashSet::from_iter(self.txstore.keys().cloned());
     }
 
-    pub fn download_new_mempool_txs(daemon: &Daemon, old_txids: &HashSet<Txid>, new_txids: &HashSet<Txid>) -> Vec<Transaction> {
-        let txids: Vec<&Txid> = (*new_txids).difference(old_txids).collect();
-        let tranactions = match daemon.gettransactions(&txids) {
-            Ok(txs) => txs,
-            Err(err) => {
-                warn!("failed to get {} transactions: {}", txids.len(), err); // e.g. new block or RBF
-                vec![] // return an empty vector if there's an error
-            }
-        };
-
-        return tranactions;
-    }
-
-    pub fn update_quick(&mut self, to_add: &Vec<Transaction>, to_remove: &HashSet<&Txid>) -> Result<()> {
-        let _timer = self.latency.with_label_values(&["update_quick"]).start_timer();
-
-        // Add new transactions
-        self.add(to_add.clone());
-        // Remove missing transactions
-        self.remove(to_remove.clone());
-
-        self.count
-            .with_label_values(&["txs"])
-            .set(self.txstore.len() as f64);
-
-        // Update cached backlog stats (if expired)
-        if self.backlog_stats.1.elapsed() > Duration::from_secs(BACKLOG_STATS_TTL) {
-            self.update_backlog_stats();
-        }
-
-        Ok(())
-    }
-
-    pub fn update(&mut self, daemon: &Daemon) -> Result<()> {
-        let _timer = self.latency.with_label_values(&["update"]).start_timer();
-        let new_txids = daemon
-            .getmempooltxids()
-            .chain_err(|| "failed to update mempool from daemon")?;
-        let old_txids = HashSet::from_iter(self.txstore.keys().cloned());
-        let to_remove: HashSet<&Txid> = old_txids.difference(&new_txids).collect();
-
-        // Download and add new transactions from bitcoind's mempool
-        let txids: Vec<&Txid> = new_txids.difference(&old_txids).collect();
-        let to_add = match daemon.gettransactions(&txids) {
-            Ok(txs) => txs,
-            Err(err) => {
-                warn!("failed to get {} transactions: {}", txids.len(), err); // e.g. new block or RBF
-                return Ok(()); // keep the mempool until next update()
-            }
-        };
-        // Add new transactions
-        self.add(to_add);
-        // Remove missing transactions
-        self.remove(to_remove);
-
-        self.count
-            .with_label_values(&["txs"])
-            .set(self.txstore.len() as f64);
-
-        // Update cached backlog stats (if expired)
-        if self.backlog_stats.1.elapsed() > Duration::from_secs(BACKLOG_STATS_TTL) {
-            self.update_backlog_stats();
-        }
-
-        Ok(())
-    }
-
     pub fn update_backlog_stats(&mut self) {
         let _timer = self
             .latency
@@ -563,60 +497,47 @@ impl Mempool {
         old_txids: &HashSet<Txid>,
         new_txids: &HashSet<Txid>
     ) -> Result<Vec<Transaction>>{
-        let t = Instant::now();
-
         let txids: Vec<&Txid> = (*new_txids).difference(old_txids).collect();
-        let transactions = match daemon.gettransactions(&txids) {
-            Ok(txs) => txs,
-            Err(err) => {
-                warn!("failed to get {} transactions: {}", txids.len(), err); // e.g. new block or RBF
-                return Err(err);
-            }
-        };
-
-        log_fn_duration("mempool::download_new_mempool_txs", t.elapsed().as_micros());
+        let transactions = daemon.gettransactions(&txids)
+            .chain_err(|| format!("failed to get {} transactions", txids.len()))?;
         return Ok(transactions);
     }
 
     pub fn update(mempool: &Arc<RwLock<Mempool>>, daemon: &Daemon) -> Result<()> {
-        let t = Instant::now();
-
-        // 1. Determine which transactions are no longer in the daemon's mempool
+        // 1. Determine which transactions are no longer in the daemon's mempool and which ones have newly entered it
         let old_txids = mempool.read().unwrap().old_txids();
         let new_txids = daemon
             .getmempooltxids()
             .chain_err(|| "failed to update mempool from daemon")?;
-        let old_mempool_txs: HashSet<&Txid> = old_txids.difference(&new_txids).collect();
-        log_fn_duration("mempool::paratial_tx_fetch", t.elapsed().as_micros());
+        let to_remove: HashSet<&Txid> = old_txids.difference(&new_txids).collect();
 
-        // 2. Download new transactions from the daemon's mempool
-        let new_mempool_txs = match Mempool::download_new_mempool_txs(&daemon, &old_txids, &new_txids) {
+        // 2. Download the new transactions from the daemon's mempool
+        let to_add = match Mempool::download_new_mempool_txs(&daemon, &old_txids, &new_txids) {
             Ok(txs) => txs,
-            Err(_) => {
-                warn!("Failed to get new mempool txs, skipping mempool update");
+            Err(err) => {
+                warn!("Unable to get new mempool txs, skipping mempool update: {}", err.display_chain());
                 return Ok(());
             }
         };
 
         // 3. Update local mempool to match daemon's state
         {
-            let mut mempool_guard = mempool.write().unwrap();
+            let mut mempool = mempool.write().unwrap();
             // Add new transactions
-            mempool_guard.add(new_mempool_txs.clone());
+            mempool.add(to_add);
             // Remove missing transactions
-            mempool_guard.remove(old_mempool_txs.clone());
+            mempool.remove(to_remove);
 
-            mempool_guard.count
+            mempool.count
                 .with_label_values(&["txs"])
-                .set(mempool_guard.txstore.len() as f64);
+                .set(mempool.txstore.len() as f64);
 
             // Update cached backlog stats (if expired)
-            if mempool_guard.backlog_stats.1.elapsed() > Duration::from_secs(BACKLOG_STATS_TTL) {
-                mempool_guard.update_backlog_stats();
+            if mempool.backlog_stats.1.elapsed() > Duration::from_secs(BACKLOG_STATS_TTL) {
+                mempool.update_backlog_stats();
             }
         }
 
-        log_fn_duration("mempool::update", t.elapsed().as_micros());
         Ok(())
     }
 }