diff --git a/crates/mempool/src/mempool.rs b/crates/mempool/src/mempool.rs index 322ccf84..ff31fc65 100644 --- a/crates/mempool/src/mempool.rs +++ b/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, }; @@ -38,7 +38,7 @@ impl Mempool { /// Returns an iterator of the current eligible transactions for sequencing, ordered by their /// priority. - pub fn iter(&self) -> impl Iterator + '_ { + pub fn iter(&self) -> impl Iterator { self.tx_queue.iter() } @@ -49,11 +49,9 @@ impl Mempool { // library. pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult> { let mut eligible_txs: Vec = Vec::with_capacity(n_txs); - - let txs = self.tx_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) @@ -83,7 +81,7 @@ impl Mempool { let tx = input.tx; self.tx_pool.insert(tx.clone())?; - self.tx_queue.insert(TransactionReference::new(tx)); + self.tx_queue.insert(TransactionReference::new(&tx)); Ok(()) } @@ -93,11 +91,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, + } } } diff --git a/crates/mempool/src/mempool_test.rs b/crates/mempool/src/mempool_test.rs index 1c9eeb8f..027ae3cf 100644 --- a/crates/mempool/src/mempool_test.rs +++ b/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`. @@ -49,11 +49,12 @@ fn mempool() -> Mempool { #[track_caller] fn check_mempool_txs_eq(mempool: &Mempool, expected_txs: &[ThinTransaction]) { 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) ); } diff --git a/crates/mempool/src/transaction_pool.rs b/crates/mempool/src/transaction_pool.rs index a020b182..3c540e44 100644 --- a/crates/mempool/src/transaction_pool.rs +++ b/crates/mempool/src/transaction_pool.rs @@ -20,22 +20,24 @@ 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; + let tx_reference = TransactionReference::new(&tx); + let tx_hash = tx_reference.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()); + entry.insert(tx); } else { return Err(MempoolError::DuplicateTransaction { tx_hash }); } - let txs_from_account_entry = self.txs_by_account.entry(tx.sender_address).or_default(); - match txs_from_account_entry.entry(tx.nonce) { + let txs_from_account_entry = + self.txs_by_account.entry(tx_reference.sender_address).or_default(); + match txs_from_account_entry.entry(tx_reference.nonce) { btree_map::Entry::Vacant(txs_from_account) => { - txs_from_account.insert(TransactionReference::new(tx)); + txs_from_account.insert(tx_reference); } + // TODO: support fee escalation transactions. btree_map::Entry::Occupied(_) => { panic!( "Transaction pool consistency error: transaction with hash {tx_hash} does not \ diff --git a/crates/mempool/src/transaction_queue.rs b/crates/mempool/src/transaction_queue.rs index 36839d46..6edd3c60 100644 --- a/crates/mempool/src/transaction_queue.rs +++ b/crates/mempool/src/transaction_queue.rs @@ -2,6 +2,7 @@ use std::cmp::Ordering; use std::collections::{BTreeSet, HashMap}; use starknet_api::core::{ContractAddress, Nonce}; +use starknet_api::transaction::TransactionHash; use crate::mempool::TransactionReference; // Assumption: for the MVP only one transaction from the same contract class can be in the mempool @@ -28,19 +29,18 @@ impl TransactionQueue { } // TODO(gilad): remove collect - pub fn pop_last_chunk(&mut self, n_txs: usize) -> Vec { + pub fn pop_last_chunk(&mut self, n_txs: usize) -> Vec { let txs: Vec = (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() } - pub fn iter(&self) -> impl Iterator + '_ { - self.queue.iter().map(|tx| tx.0.clone()) + pub fn iter(&self) -> impl Iterator { + self.queue.iter().map(|tx| &tx.0) } pub fn _get_nonce(&self, address: &ContractAddress) -> Option<&Nonce> { @@ -48,16 +48,6 @@ impl TransactionQueue { } } -impl From> for TransactionQueue { - fn from(txs: Vec) -> 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);