Remove Clone from mempool Storage

zebrad/src/components/mempool.rs

@@ -109,10 +109,10 @@ impl Mempool {
     /// then it shouldn't be downloaded/verified.
     fn should_download_or_verify(&mut self, txid: UnminedTxId) -> Result<(), MempoolError> {
         // Check if the transaction is already in the mempool.
-        if self.storage.clone().contains(&txid) {
+        if self.storage.contains(&txid) {
             return Err(MempoolError::InMempool);
         }
-        if self.storage.clone().contains_rejected(&txid) {
+        if self.storage.contains_rejected(&txid) {
             return Err(MempoolError::Rejected);
         }
         Ok(())
@@ -140,15 +140,15 @@ impl Service<Request> for Mempool {
     fn call(&mut self, req: Request) -> Self::Future {
         match req {
             Request::TransactionIds => {
-                let res = self.storage.clone().tx_ids();
+                let res = self.storage.tx_ids();
                 async move { Ok(Response::TransactionIds(res)) }.boxed()
             }
             Request::TransactionsById(ids) => {
-                let rsp = Ok(self.storage.clone().transactions(ids)).map(Response::Transactions);
+                let rsp = Ok(self.storage.transactions(ids)).map(Response::Transactions);
                 async move { rsp }.boxed()
             }
             Request::RejectedTransactionIds(ids) => {
-                let rsp = Ok(self.storage.clone().rejected_transactions(ids))
+                let rsp = Ok(self.storage.rejected_transactions(ids))
                     .map(Response::RejectedTransactionIds);
                 async move { rsp }.boxed()
             }

zebrad/src/components/mempool/storage.rs

@@ -31,7 +31,7 @@ pub enum State {
     Excess,
 }
 
-#[derive(Clone, Default)]
+#[derive(Default)]
 pub struct Storage {
     /// The set of verified transactions in the mempool. This is a
     /// cache of size [`MEMPOOL_SIZE`].
@@ -87,31 +87,32 @@ impl Storage {
     /// Returns `true` if a [`UnminedTx`] matching an [`UnminedTxId`] is in
     /// the mempool.
     #[allow(dead_code)]
-    pub fn contains(self, txid: &UnminedTxId) -> bool {
+    pub fn contains(&self, txid: &UnminedTxId) -> bool {
         self.verified.iter().any(|tx| &tx.id == txid)
     }
 
     /// Returns the set of [`UnminedTxId`]s in the mempool.
-    pub fn tx_ids(self) -> Vec<UnminedTxId> {
+    pub fn tx_ids(&self) -> Vec<UnminedTxId> {
         self.verified.iter().map(|tx| tx.id).collect()
     }
 
     /// Returns the set of [`Transaction`]s matching ids in the mempool.
-    pub fn transactions(self, tx_ids: HashSet<UnminedTxId>) -> Vec<UnminedTx> {
+    pub fn transactions(&self, tx_ids: HashSet<UnminedTxId>) -> Vec<UnminedTx> {
         self.verified
-            .into_iter()
+            .iter()
             .filter(|tx| tx_ids.contains(&tx.id))
+            .cloned()
             .collect()
     }
 
     /// Returns `true` if a [`UnminedTx`] matching an [`UnminedTxId`] is in
     /// the mempool rejected list.
-    pub fn contains_rejected(self, txid: &UnminedTxId) -> bool {
+    pub fn contains_rejected(&self, txid: &UnminedTxId) -> bool {
         self.rejected.contains_key(txid)
     }
 
     /// Returns the set of [`UnminedTxId`]s matching ids in the rejected list.
-    pub fn rejected_transactions(self, tx_ids: HashSet<UnminedTxId>) -> Vec<UnminedTxId> {
+    pub fn rejected_transactions(&self, tx_ids: HashSet<UnminedTxId>) -> Vec<UnminedTxId> {
         tx_ids
             .into_iter()
             .filter(|tx| self.rejected.contains_key(tx))

zebrad/src/components/mempool/storage/tests.rs

@@ -36,7 +36,7 @@ fn mempool_storage_basic_for_network(network: Network) -> Result<()> {
 
     // Make sure the last MEMPOOL_SIZE transactions we sent are in the verified
     for tx in unmined_transactions.iter().rev().take(MEMPOOL_SIZE) {
-        assert!(storage.clone().contains(&tx.id));
+        assert!(storage.contains(&tx.id));
     }
 
     // Anything greater should not be in the verified
@@ -44,7 +44,7 @@ fn mempool_storage_basic_for_network(network: Network) -> Result<()> {
         .iter()
         .take(unmined_transactions.len() - MEMPOOL_SIZE)
     {
-        assert!(!storage.clone().contains(&tx.id));
+        assert!(!storage.contains(&tx.id));
     }
 
     // Query all the ids we have for rejected, get back `total - MEMPOOL_SIZE`
@@ -55,18 +55,13 @@ fn mempool_storage_basic_for_network(network: Network) -> Result<()> {
         .map(|tx| tx.id)
         .collect();
     // Convert response to a `HashSet` as we need a fixed order to compare.
-    let rejected_response: HashSet<UnminedTxId> = storage
+    let rejected_response: HashSet<UnminedTxId> =
-        .clone()
+        storage.rejected_transactions(all_ids).into_iter().collect();
-        .rejected_transactions(all_ids)
-        .into_iter()
-        .collect();
 
     assert_eq!(rejected_response, rejected_ids);
 
     // Use `contains_rejected` to make sure the first id stored is now rejected
-    assert!(storage
+    assert!(storage.contains_rejected(&unmined_transactions[0].id));
-        .clone()
-        .contains_rejected(&unmined_transactions[0].id));
     // Use `contains_rejected` to make sure the last id stored is not rejected
     assert!(!storage.contains_rejected(&unmined_transactions[unmined_transactions.len() - 1].id));