fix(mempool): remove TODOs and other minor changes

base_layer/core/src/mempool/config.rs

@@ -66,9 +66,6 @@ impl Default for MempoolServiceConfig {
 
 #[cfg(test)]
 mod test {
-    // TODO: Use new Config api - seems that you need to use the builder each time you want to change a value which
-    //       isn't great, there must be a better way.
-    #![allow(deprecated)]
     use config::Config;
     use tari_common::DefaultConfigLoader;
 
@@ -77,11 +74,12 @@ mod test {
 
     #[test]
     pub fn test_mempool_config() {
-        let mut config = Config::builder().build().unwrap();
+        let config = Config::builder()
+            .set_override("mempool.unconfirmed_pool.storage_capacity", 3)
+            .unwrap()
+            .build()
+            .unwrap();
 
-        config
-            .set("mempool.unconfirmed_pool.storage_capacity", 3)
-            .expect("Could not set ''");
         let my_config = MempoolConfig::load_from(&config).expect("Could not load configuration");
         // [ ] mempool.mainnet, [X]  mempool = 3, [X] Default
         assert_eq!(my_config.unconfirmed_pool.storage_capacity, 3);
@@ -92,13 +90,15 @@ mod test {
             ReorgPoolConfig::default().expiry_height
         );
 
-        config
-            .set("mainnet.mempool.unconfirmed_pool.storage_capacity", 20)
-            .expect("Could not set ''");
+        let config = Config::builder()
+            .add_source(config)
+            .set_override("mainnet.mempool.unconfirmed_pool.storage_capacity", 20)
+            .unwrap()
+            .set_override("mempool.override_from", "mainnet")
+            .unwrap()
+            .build()
+            .unwrap();
 
-        config
-            .set("mempool.override_from", "mainnet")
-            .expect("Could not set 'override_from'");
         // use_network = mainnet
         let my_config = MempoolConfig::load_from(&config).expect("Could not load configuration");
         // [ ] mempool.mainnet, [X]  mempool = 3, [X] Default

base_layer/core/src/mempool/mempool_storage.rs

@@ -243,7 +243,6 @@ impl MempoolStorage {
     }
 
     /// Returns all unconfirmed transaction stored in the Mempool, except the transactions stored in the ReOrgPool.
-    // TODO: Investigate returning an iterator rather than a large vector of transactions
     pub fn snapshot(&self) -> Vec<Arc<Transaction>> {
         self.unconfirmed_pool.snapshot()
     }

base_layer/core/src/mempool/reorg_pool/reorg_pool.rs

@@ -77,16 +77,16 @@ impl ReorgPool {
     /// Insert a new transaction into the ReorgPool. Published transactions will be discarded once they are
     /// `config.expiry_height` blocks old.
     fn insert(&mut self, height: u64, tx: Arc<Transaction>) {
-        let excess_hex = tx
-            .first_kernel_excess_sig()
-            .map(|s| s.get_signature().to_hex())
-            .unwrap_or_else(|| "no kernel!".to_string());
         if tx
             .body
             .kernels()
             .iter()
             .all(|k| self.txs_by_signature.contains_key(k.excess_sig.get_signature()))
         {
+            let excess_hex = tx
+                .first_kernel_excess_sig()
+                .map(|s| s.get_signature().to_hex())
+                .unwrap_or_else(|| "no kernel!".to_string());
             debug!(
                 target: LOG_TARGET,
                 "Transaction {} already found in reorg pool", excess_hex
@@ -287,13 +287,6 @@ impl ReorgPool {
             None => return,
         };
 
-        // let heights_to_remove = self
-        //     .txs_by_height
-        //     .keys()
-        //     .filter(|h| **h <= height)
-        //     .copied()
-        //     .collect::<Vec<_>>();
-        // for height in heights_to_remove {
         if let Some(tx_ids) = self.txs_by_height.remove(&height) {
             debug!(
                 target: LOG_TARGET,
@@ -338,7 +331,7 @@ impl ReorgPool {
         shrink_hashmap(&mut self.txs_by_signature);
         shrink_hashmap(&mut self.txs_by_height);
 
-        if old - new > 0 {
+        if old > new {
             debug!(
                 target: LOG_TARGET,
                 "Shrunk reorg mempool memory usage ({}/{}) ~{}%",

base_layer/core/src/mempool/rpc/service.rs

@@ -48,11 +48,9 @@ impl MempoolRpcService {
     }
 }
 
-// TODO: Logging the error and returning a general error to the requester is a common requirement. Figure out a clean
-//       way to provide this functionality.
 fn to_internal_error<T: std::error::Error>(err: T) -> RpcStatus {
     error!(target: LOG_TARGET, "Internal error: {}", err);
-    RpcStatus::general(&err.to_string())
+    RpcStatus::general_default()
 }
 
 #[tari_comms::async_trait]

base_layer/core/src/mempool/service/inbound_handlers.rs

@@ -45,13 +45,16 @@ pub const LOG_TARGET: &str = "c::mp::service::inbound_handlers";
 #[derive(Clone)]
 pub struct MempoolInboundHandlers {
     mempool: Mempool,
-    outbound_nmi: OutboundMempoolServiceInterface,
+    outbound_service: OutboundMempoolServiceInterface,
 }
 
 impl MempoolInboundHandlers {
     /// Construct the MempoolInboundHandlers.
-    pub fn new(mempool: Mempool, outbound_nmi: OutboundMempoolServiceInterface) -> Self {
-        Self { mempool, outbound_nmi }
+    pub fn new(mempool: Mempool, outbound_service: OutboundMempoolServiceInterface) -> Self {
+        Self {
+            mempool,
+            outbound_service,
+        }
     }
 
     /// Handle inbound Mempool service requests from remote nodes and local services.
@@ -139,7 +142,7 @@ impl MempoolInboundHandlers {
                         target: LOG_TARGET,
                         "Propagate transaction ({}) to network.", kernel_excess_sig,
                     );
-                    self.outbound_nmi
+                    self.outbound_service
                         .propagate_tx(tx, source_peer.into_iter().collect())
                         .await?;
                 }

base_layer/core/src/mempool/service/service.rs

@@ -131,7 +131,6 @@ impl MempoolService {
     }
 
     async fn handle_request(&mut self, request: MempoolRequest) -> Result<MempoolResponse, MempoolServiceError> {
-        // TODO: Move db calls into MempoolService
         self.inbound_handlers.handle_request(request).await
     }