refactor: update priority queue to use PrioritizedTransaction

crates/mempool/src/mempool.rs

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 
-use starknet_api::core::ContractAddress;
-use starknet_api::transaction::TransactionHash;
+use starknet_api::core::{ContractAddress, Nonce};
+use starknet_api::transaction::{Tip, TransactionHash};
 use starknet_mempool_types::mempool_types::{
     AccountState, MempoolInput, MempoolResult, ThinTransaction,
 };
@@ -16,8 +16,10 @@ pub mod mempool_test;
 #[derive(Debug, Default)]
 pub struct Mempool {
     // TODO: add docstring explaining visibility and coupling of the fields.
-    txs_queue: TransactionQueue,
+    // All transactions currently held in the mempool.
     tx_pool: TransactionPool,
+    // Transactions eligible for sequencing.
+    tx_queue: TransactionQueue,
 }
 
 impl Mempool {
@@ -34,18 +36,20 @@ impl Mempool {
         Mempool::default()
     }
 
+    pub fn iter(&self) -> impl Iterator<Item = &TransactionReference> {
+        self.tx_queue.iter()
+    }
+
     /// Retrieves up to `n_txs` transactions with the highest priority from the mempool.
     /// Transactions are guaranteed to be unique across calls until `commit_block` is invoked.
     // TODO: the last part about commit_block is incorrect if we delete txs in get_txs and then push
     // back. TODO: Consider renaming to `pop_txs` to be more consistent with the standard
     // library.
     pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<ThinTransaction>> {
         let mut eligible_txs: Vec<ThinTransaction> = Vec::with_capacity(n_txs);
-
-        let txs = self.txs_queue.pop_last_chunk(n_txs);
-        for tx in txs {
-            self.tx_pool.remove(tx.tx_hash)?;
-            eligible_txs.push(tx.0);
+        for tx_hash in self.tx_queue.pop_last_chunk(n_txs) {
+            let tx = self.tx_pool.remove(tx_hash)?;
+            eligible_txs.push(tx);
         }
 
         Ok(eligible_txs)
@@ -75,7 +79,7 @@ impl Mempool {
         let tx = input.tx;
 
         self.tx_pool.insert(tx.clone())?;
-        self.txs_queue.insert(TransactionReference::new(tx));
+        self.tx_queue.insert(TransactionReference::new(&tx));
 
         Ok(())
     }
@@ -85,11 +89,21 @@ impl Mempool {
 /// execution fields).
 /// TODO(Mohammad): rename this struct to `ThinTransaction` once that name
 /// becomes available, to better reflect its purpose and usage.
-#[derive(Clone, Debug, Default, derive_more::Deref)]
-pub struct TransactionReference(pub ThinTransaction);
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
+pub struct TransactionReference {
+    pub sender_address: ContractAddress,
+    pub nonce: Nonce,
+    pub tx_hash: TransactionHash,
+    pub tip: Tip,
+}
 
 impl TransactionReference {
-    pub fn new(tx: ThinTransaction) -> Self {
-        TransactionReference(tx)
+    pub fn new(tx: &ThinTransaction) -> Self {
+        TransactionReference {
+            sender_address: tx.sender_address,
+            nonce: tx.nonce,
+            tx_hash: tx.tx_hash,
+            tip: tx.tip,
+        }
     }
 }

crates/mempool/src/mempool_test.rs

@@ -9,7 +9,7 @@ use starknet_api::{contract_address, patricia_key};
 use starknet_mempool_types::errors::MempoolError;
 use starknet_mempool_types::mempool_types::{Account, ThinTransaction};
 
-use crate::mempool::{Mempool, MempoolInput};
+use crate::mempool::{Mempool, MempoolInput, TransactionReference};
 
 /// Creates a valid input for mempool's `add_tx` with optional default value for
 /// `sender_address`.
@@ -48,12 +48,13 @@ fn mempool() -> Mempool {
 // transactions.
 #[track_caller]
 fn check_mempool_txs_eq(mempool: &Mempool, expected_txs: &[ThinTransaction]) {
-    let mempool_txs = mempool.txs_queue.iter();
+    let mempool_txs = mempool.tx_queue.iter();
+    let expected_txs = expected_txs.iter().map(TransactionReference::new);
 
     assert!(
         zip_eq(expected_txs, mempool_txs)
             // Deref the inner mempool tx type.
-            .all(|(expected_tx, mempool_tx)| *expected_tx == *mempool_tx)
+            .all(|(expected_tx, mempool_tx)| expected_tx == *mempool_tx)
     );
 }
 

crates/mempool/src/transaction_pool.rs

@@ -20,29 +20,29 @@ pub struct TransactionPool {
 }
 
 impl TransactionPool {
-    // TODO(Mohammad): Remove the cloning of tx once the `TransactionReference` is updated.
     pub fn insert(&mut self, tx: ThinTransaction) -> MempoolResult<()> {
         let tx_hash = tx.tx_hash;
 
-        // Insert transaction to pool, if it is new.
-        if let hash_map::Entry::Vacant(entry) = self.tx_pool.entry(tx_hash) {
-            entry.insert(tx.clone());
-        } else {
-            return Err(MempoolError::DuplicateTransaction { tx_hash });
-        }
-
+        // Insert transaction into the mapping by account.
         let txs_from_account_entry = self.txs_by_account.entry(tx.sender_address).or_default();
         match txs_from_account_entry.entry(tx.nonce) {
             btree_map::Entry::Vacant(txs_from_account) => {
-                txs_from_account.insert(TransactionReference::new(tx));
+                txs_from_account.insert(TransactionReference::new(&tx));
             }
             btree_map::Entry::Occupied(_) => {
-                panic!(
-                    "Transaction pool consistency error: transaction with hash {tx_hash} does not \
-                     appear in main mapping, but it appears in the account mapping"
-                );
+                return Err(MempoolError::DuplicateTransaction { tx_hash });
             }
         }
+
+        // Insert transaction to pool, if it is new.
+        if let hash_map::Entry::Vacant(entry) = self.tx_pool.entry(tx_hash) {
+            entry.insert(tx);
+        } else {
+            panic!(
+                "Transaction pool consistency error: transaction with hash {tx_hash} does not \
+                 appear in the account mapping, but it appears in the main mapping"
+            );
+        }
         Ok(())
     }
 

crates/mempool/src/transaction_queue.rs

@@ -2,6 +2,7 @@ use std::cmp::Ordering;
 use std::collections::{BTreeSet, HashMap, VecDeque};
 
 use starknet_api::core::{ContractAddress, Nonce};
+use starknet_api::transaction::TransactionHash;
 use starknet_mempool_types::mempool_types::ThinTransaction;
 
 use crate::mempool::TransactionReference;
@@ -29,38 +30,25 @@ impl TransactionQueue {
     }
 
     // TODO(gilad): remove collect
-    pub fn pop_last_chunk(&mut self, n_txs: usize) -> Vec<TransactionReference> {
+    pub fn pop_last_chunk(&mut self, n_txs: usize) -> Vec<TransactionHash> {
         let txs: Vec<TransactionReference> =
             (0..n_txs).filter_map(|_| self.queue.pop_last().map(|tx| tx.0)).collect();
-
         for tx in &txs {
             self.address_to_nonce.remove(&tx.sender_address);
         }
 
-        txs
+        txs.into_iter().map(|tx| tx.tx_hash).collect()
     }
 
-    // TODO(Mohammad): return reference once `TransactionReference` can impl Copy.
-    #[cfg(any(feature = "testing", test))]
-    pub fn iter(&self) -> impl Iterator<Item = TransactionReference> + '_ {
-        self.queue.iter().map(|tx| tx.0.clone())
+    pub fn iter(&self) -> impl Iterator<Item = &TransactionReference> {
+        self.queue.iter().map(|tx| &tx.0)
     }
 
     pub fn _get_nonce(&self, address: &ContractAddress) -> Option<&Nonce> {
         self.address_to_nonce.get(address)
     }
 }
 
-impl From<Vec<TransactionReference>> for TransactionQueue {
-    fn from(txs: Vec<TransactionReference>) -> Self {
-        let mut tx_queue = TransactionQueue::default();
-        for tx in txs {
-            tx_queue.insert(tx);
-        }
-        tx_queue
-    }
-}
-
 #[derive(Clone, Debug, derive_more::Deref, derive_more::From)]
 struct QueuedTransaction(pub TransactionReference);