diff --git a/Cargo.lock b/Cargo.lock index 080bd31f..41c904d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2166,7 +2166,7 @@ dependencies = [ [[package]] name = "starknet_api" version = "0.12.0-dev.1" -source = "git+https://github.com/starkware-libs/starknet-api.git?rev=48d61bb#48d61bbc525bcfc96c88190e53142fb15dd41b6c" +source = "git+https://github.com/starkware-libs/starknet-api.git?rev=016ff6c#016ff6c26b75b2a2b8a46d6e7100776d5b65a59f" dependencies = [ "cairo-lang-starknet-classes", "derive_more", @@ -2208,6 +2208,7 @@ name = "starknet_mempool" version = "0.0.0" dependencies = [ "assert_matches", + "derive_more", "starknet_api", "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index 58fd0ace..ca3b63f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,15 +21,15 @@ as_conversions = "deny" [workspace.dependencies] assert_matches = "1.5.0" axum = "0.6.12" -clap = { version = "4.3.10" } +clap = "4.3.10" +derive_more = "0.99" hyper = "1.2.0" +papyrus_config = "0.3.0" pretty_assertions = "1.4.0" rstest = "0.17.0" serde = { version = "1.0.193", features = ["derive"] } serde_json = "1.0" -starknet_api = { git = "https://github.com/starkware-libs/starknet-api.git", rev = "48d61bb" } -papyrus_config = "0.3.0" +starknet_api = { git = "https://github.com/starkware-libs/starknet-api.git", rev = "016ff6c" } thiserror = "1.0" tokio = { version = "1", features = ["full"] } validator = "0.12" - diff --git a/crates/mempool/Cargo.toml b/crates/mempool/Cargo.toml index 10f9ffd0..4beff2d2 100644 --- a/crates/mempool/Cargo.toml +++ b/crates/mempool/Cargo.toml @@ -9,6 +9,7 @@ license.workspace = true workspace = true [dependencies] +derive_more.workspace = true starknet_api.workspace = true [dev-dependencies] diff --git a/crates/mempool/src/lib.rs b/crates/mempool/src/lib.rs index 15178a5d..b25f4f10 100644 --- a/crates/mempool/src/lib.rs +++ b/crates/mempool/src/lib.rs @@ -1 +1,2 @@ +// TODO: change to pub(crate) once this is used by the (not yet implemented) mempool struct. pub mod priority_queue; diff --git a/crates/mempool/src/priority_queue.rs b/crates/mempool/src/priority_queue.rs index df570b5e..02c17ac8 100644 --- a/crates/mempool/src/priority_queue.rs +++ b/crates/mempool/src/priority_queue.rs @@ -2,101 +2,74 @@ #[path = "priority_queue_test.rs"] pub mod priority_queue_test; -use std::{ - cmp::Ordering, - collections::{BinaryHeap, HashMap}, -}; +use std::{cmp::Ordering, collections::BTreeSet}; use starknet_api::{ internal_transaction::InternalTransaction, - transaction::{ - DeclareTransaction, DeployAccountTransaction, InvokeTransaction, Tip, TransactionHash, - }, + transaction::{DeclareTransaction, DeployAccountTransaction, InvokeTransaction, Tip}, }; -fn get_tip(tx: &InternalTransaction) -> Tip { - match tx { - InternalTransaction::Declare(declare_tx) => match &declare_tx.tx { - DeclareTransaction::V3(declare_tx_v3) => declare_tx_v3.tip, - _ => panic!("Unexpected transaction version."), - }, - InternalTransaction::DeployAccount(deploy_account_tx) => match &deploy_account_tx.tx { - DeployAccountTransaction::V3(tx_v3) => tx_v3.tip, - _ => panic!("Unexpected transaction version."), - }, - InternalTransaction::Invoke(invoke_tx) => match &invoke_tx.tx { - InvokeTransaction::V3(tx_v3) => tx_v3.tip, - _ => panic!("Unexpected transaction version."), - }, - } -} +// Assumption: for the MVP only one transaction from the same contract class can be in the mempool +// at a time. When this changes, saving the transactions themselves on the queu might no longer be +// appropriate, because we'll also need to stores transactions without indexing them. For example, +// transactions with future nonces will need to be stored, and potentially indexed on block commits. +#[derive(Clone, Debug, Default, derive_more::Deref, derive_more::DerefMut)] +pub struct PriorityQueue(BTreeSet); -fn get_tx_hash(tx: &InternalTransaction) -> TransactionHash { - match tx { - InternalTransaction::Declare(declare_tx) => declare_tx.tx_hash, - InternalTransaction::DeployAccount(deploy_account_tx) => deploy_account_tx.tx_hash, - InternalTransaction::Invoke(invoke_tx) => invoke_tx.tx_hash, +impl PriorityQueue { + pub fn push(&mut self, tx: InternalTransaction) { + let mempool_tx = Transaction(tx); + self.insert(mempool_tx); } -} -#[derive(Clone, Debug)] -pub struct MempoolTransaction { - pub tx: InternalTransaction, - pub tx_hash: TransactionHash, -} - -impl PartialEq for MempoolTransaction { - fn eq(&self, other: &MempoolTransaction) -> bool { - get_tip(&self.tx) == get_tip(&other.tx) + // Removes and returns the transaction with the highest tip. + pub fn pop(&mut self) -> Option { + self.pop_last().map(|tx| tx.0) } } -impl Eq for MempoolTransaction {} +#[derive(Clone, Debug, derive_more::Deref)] +pub struct Transaction(pub InternalTransaction); -impl PartialOrd for MempoolTransaction { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) +impl Transaction { + fn tip(&self) -> Tip { + match &self.0 { + InternalTransaction::Declare(declare_tx) => match &declare_tx.tx { + DeclareTransaction::V3(tx_v3) => tx_v3.tip, + _ => unimplemented!(), + }, + InternalTransaction::DeployAccount(deploy_account_tx) => match &deploy_account_tx.tx { + DeployAccountTransaction::V3(tx_v3) => tx_v3.tip, + _ => unimplemented!(), + }, + InternalTransaction::Invoke(invoke_tx) => match &invoke_tx.tx { + InvokeTransaction::V3(tx_v3) => tx_v3.tip, + _ => unimplemented!(), + }, + } } } -impl Ord for MempoolTransaction { - fn cmp(&self, other: &Self) -> Ordering { - get_tip(&self.tx).cmp(&get_tip(&other.tx)) +// Compare transactions based on their tip only, which implies `Eq`, because `tip` is uint. +impl PartialEq for Transaction { + fn eq(&self, other: &Transaction) -> bool { + self.tip() == other.tip() } } -#[derive(Clone, Default)] -pub struct PriorityQueue { - heap: BinaryHeap, - pub tx_hash_to_tx_map: HashMap, -} - -impl PriorityQueue { - pub fn new() -> Self { - PriorityQueue { - heap: BinaryHeap::new(), - tx_hash_to_tx_map: HashMap::new(), - } - } - - pub fn push(&mut self, tx: InternalTransaction) { - let tx_hash = get_tx_hash(&tx); - self.tx_hash_to_tx_map.insert(tx_hash, tx.clone()); - let mempool_tx = MempoolTransaction { tx, tx_hash }; +/// Marks Transaction as capable of strict equality comparisons, signaling to the compiler it +/// adheres to equality semantics. +// Note: this depends on the implementation of `PartialEq`, see its docstring. +impl Eq for Transaction {} - self.heap.push(mempool_tx); +impl Ord for Transaction { + fn cmp(&self, other: &Self) -> Ordering { + self.tip().cmp(&other.tip()) } +} - // Removes and returns the transaction with the highest tip. - pub fn pop(&mut self) -> Option { - let mempool_tx = self.heap.pop(); - match mempool_tx { - Some(mempool_tx) => { - let tx_hash = mempool_tx.tx_hash; - self.tx_hash_to_tx_map.remove(&tx_hash); - Some(tx_hash) - } - None => None, - } +impl PartialOrd for Transaction { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) } } diff --git a/crates/mempool/src/priority_queue_test.rs b/crates/mempool/src/priority_queue_test.rs index 4e8ef781..f9ceeca5 100644 --- a/crates/mempool/src/priority_queue_test.rs +++ b/crates/mempool/src/priority_queue_test.rs @@ -51,12 +51,12 @@ async fn test_priority_queue() { let tx1 = create_tx_for_testing(Tip(100), tx_hash_100); let tx3 = create_tx_for_testing(Tip(10), tx_hash_10); - let mut pq = PriorityQueue::new(); - pq.push(tx1); - pq.push(tx2); - pq.push(tx3); + let mut pq = PriorityQueue::default(); + pq.push(tx1.clone()); + pq.push(tx2.clone()); + pq.push(tx3.clone()); - assert_eq!(pq.pop().unwrap(), tx_hash_100); - assert_eq!(pq.pop().unwrap(), tx_hash_50); - assert_eq!(pq.pop().unwrap(), tx_hash_10); + assert_eq!(pq.pop().unwrap(), tx1); + assert_eq!(pq.pop().unwrap(), tx2); + assert_eq!(pq.pop().unwrap(), tx3); }