diff --git a/crates/starknet_l1_provider/src/l1_provider.rs b/crates/starknet_l1_provider/src/l1_provider.rs index f895255129..6bb0ec60cf 100644 --- a/crates/starknet_l1_provider/src/l1_provider.rs +++ b/crates/starknet_l1_provider/src/l1_provider.rs @@ -49,7 +49,7 @@ impl L1Provider { ) -> L1ProviderResult { self.validate_height(height)?; match self.state { - ProviderState::Validate => Ok(self.tx_manager.tx_status(tx_hash)), + ProviderState::Validate => Ok(self.tx_manager.validate_tx(tx_hash)), ProviderState::Propose => Err(L1ProviderError::ValidateTransactionConsensusBug), ProviderState::Pending => Err(L1ProviderError::ValidateInPendingState), ProviderState::Uninitialized => panic!("Uninitialized L1 provider"), diff --git a/crates/starknet_l1_provider/src/l1_provider_tests.rs b/crates/starknet_l1_provider/src/l1_provider_tests.rs index 8ae1ce39d3..fdd5089b4c 100644 --- a/crates/starknet_l1_provider/src/l1_provider_tests.rs +++ b/crates/starknet_l1_provider/src/l1_provider_tests.rs @@ -62,7 +62,7 @@ fn validate_happy_flow() { // Transaction wasn't deleted after the validation. assert_eq!( l1_provider.validate(tx_hash!(1), BlockNumber(1)).unwrap(), - ValidationStatus::Validated + ValidationStatus::AlreadyIncludedInPropsedBlock ); } diff --git a/crates/starknet_l1_provider/src/lib.rs b/crates/starknet_l1_provider/src/lib.rs index 6c5fca52cd..8dbbb8c3ad 100644 --- a/crates/starknet_l1_provider/src/lib.rs +++ b/crates/starknet_l1_provider/src/lib.rs @@ -4,6 +4,8 @@ pub mod l1_provider; pub mod l1_scraper; pub(crate) mod transaction_manager; +mod soft_delete_index_map; + #[cfg(test)] pub mod test_utils; diff --git a/crates/starknet_l1_provider/src/soft_delete_index_map.rs b/crates/starknet_l1_provider/src/soft_delete_index_map.rs new file mode 100644 index 0000000000..ea3ac322b1 --- /dev/null +++ b/crates/starknet_l1_provider/src/soft_delete_index_map.rs @@ -0,0 +1,134 @@ +use std::collections::HashSet; + +use indexmap::map::Entry; +use indexmap::IndexMap; +use starknet_api::executable_transaction::L1HandlerTransaction; +use starknet_api::transaction::TransactionHash; + +/// An IndexMap that supports soft deletion of entries. +/// Entries marked as deleted remain hidden in the map, allowing for potential recovery, +/// selective permanent deletion, or rollback before being purged. +// TODO: replace with a fully generic struct if there's a need for it. +// TODO: replace with a BTreeIndexMap if commit performance becomes an issue, see note in commit. +#[derive(Clone, Debug, Default)] +pub struct SoftDeleteIndexMap { + txs: IndexMap, + staged_txs: HashSet, +} + +impl SoftDeleteIndexMap { + pub fn _new() -> Self { + Self::default() + } + + /// Inserts a transaction into the map, returning the previous transaction if it existed. + pub fn _insert(&mut self, tx: L1HandlerTransaction) -> Option { + match self.txs.entry(tx.tx_hash) { + Entry::Occupied(entry) => { + debug_assert_eq!(entry.get().transaction, tx); + Some(entry.get().transaction.clone()) + } + Entry::Vacant(entry) => { + entry.insert(TransactionEntry::new(tx)); + None + } + } + } + + /// Soft delete and return a reference to the first unstaged transaction, by insertion order. + pub fn soft_pop_front(&mut self) -> Option<&L1HandlerTransaction> { + let entry = self.txs.iter().find(|(_, tx)| tx.is_available()); + let (&tx_hash, _) = entry?; + self.soft_remove(tx_hash) + } + + /// Stages the given transaction with the given hash if it exists and is not already staged, and + /// returns a reference to it. + pub fn soft_remove(&mut self, tx_hash: TransactionHash) -> Option<&L1HandlerTransaction> { + let entry = self.txs.get_mut(&tx_hash)?; + + if !entry.is_available() { + return None; + } + + debug_assert_eq!(self.staged_txs.get(&tx_hash), None); + entry.set_state(TxState::Staged); + self.staged_txs.insert(tx_hash); + + Some(&entry.transaction) + } + + /// Commits given transactions by removing them entirely and returning the removed transactions. + /// Uncommitted staged transactions are rolled back to unstaged first. + // Performance note: This operation is linear time with both the number + // of known transactions and the number of committed transactions. This is assumed to be + // good enough while l1-handler numbers remain low, but if this changes and we need log(n) + // removals (amortized), replace indexmap with this (basically a BTreeIndexMap): + // BTreeMap, Hashmap and a counter: u32, such that + // every new tx is inserted to the map with key counter++ and the counter is not reduced + // when removing entries. Once the counter reaches u32::MAX/2 we recreate the DS in Theta(n). + pub fn _commit(&mut self, tx_hashes: &[TransactionHash]) -> Vec { + self._rollback_staging(); + let tx_hashes: HashSet<_> = tx_hashes.iter().copied().collect(); + if tx_hashes.is_empty() { + return Vec::new(); + } + + // NOTE: this takes Theta(|self.txs|), see docstring. + let (committed, not_committed): (Vec<_>, Vec<_>) = + self.txs.drain(..).partition(|(hash, _)| tx_hashes.contains(hash)); + self.txs.extend(not_committed); + + committed.into_iter().map(|(_, entry)| entry.transaction).collect() + } + + /// Rolls back all staged transactions, converting them to unstaged. + pub fn _rollback_staging(&mut self) { + for tx_hash in self.staged_txs.drain() { + self.txs.entry(tx_hash).and_modify(|entry| entry.set_state(TxState::Unstaged)); + } + } + + pub fn is_staged(&self, tx_hash: &TransactionHash) -> bool { + self.staged_txs.contains(tx_hash) + } +} + +impl From> for SoftDeleteIndexMap { + fn from(txs: Vec) -> Self { + let txs = txs.into_iter().map(|tx| (tx.tx_hash, TransactionEntry::new(tx))).collect(); + SoftDeleteIndexMap { txs, ..Default::default() } + } +} + +/// Indicates whether a transaction is unstaged or staged. +#[derive(Debug, Clone)] +enum TxState { + Unstaged, + Staged, +} + +/// Wraps an L1HandlerTransaction along with its current TxState, +/// and provides convenience methods for stage/unstage. +#[derive(Debug, Clone)] +struct TransactionEntry { + pub transaction: L1HandlerTransaction, + pub state: TxState, +} + +impl TransactionEntry { + pub fn new(transaction: L1HandlerTransaction) -> Self { + Self { transaction, state: TxState::Unstaged } + } + + pub fn set_state(&mut self, state: TxState) { + self.state = state + } + + pub fn is_available(&self) -> bool { + match self.state { + TxState::Unstaged => true, + TxState::Staged => false, + } + } +} diff --git a/crates/starknet_l1_provider/src/test_utils.rs b/crates/starknet_l1_provider/src/test_utils.rs index c2e36cf04a..0e8e4bc38c 100644 --- a/crates/starknet_l1_provider/src/test_utils.rs +++ b/crates/starknet_l1_provider/src/test_utils.rs @@ -1,11 +1,15 @@ -use indexmap::{IndexMap, IndexSet}; +use std::mem; + +use indexmap::IndexSet; use starknet_api::block::BlockNumber; use starknet_api::executable_transaction::L1HandlerTransaction; use starknet_api::transaction::TransactionHash; use crate::l1_provider::L1Provider; +use crate::soft_delete_index_map::SoftDeleteIndexMap; use crate::transaction_manager::TransactionManager; use crate::ProviderState; + // Represents the internal content of the L1 provider for testing. // Enables customized (and potentially inconsistent) creation for unit testing. #[derive(Debug, Default)] @@ -18,10 +22,7 @@ pub struct L1ProviderContent { impl From for L1Provider { fn from(content: L1ProviderContent) -> L1Provider { L1Provider { - tx_manager: content - .tx_manager_content - .map(|tm_content| tm_content.complete_to_tx_manager()) - .unwrap_or_default(), + tx_manager: content.tx_manager_content.map(Into::into).unwrap_or_default(), state: content.state.unwrap_or_default(), current_height: content.current_height, } @@ -53,8 +54,7 @@ impl L1ProviderContentBuilder { mut self, tx_hashes: impl IntoIterator, ) -> Self { - self.tx_manager_content_builder = - self.tx_manager_content_builder.with_on_l2_awaiting_l1_consumption(tx_hashes); + self.tx_manager_content_builder = self.tx_manager_content_builder.committed(tx_hashes); self } @@ -75,37 +75,39 @@ impl L1ProviderContentBuilder { // Enables customized (and potentially inconsistent) creation for unit testing. #[derive(Debug, Default)] struct TransactionManagerContent { - txs: Option>, - on_l2_awaiting_l1_consumption: Option>, + txs: Option>, + committed: Option>, } -impl TransactionManagerContent { - fn complete_to_tx_manager(self) -> TransactionManager { +impl From for TransactionManager { + fn from(mut content: TransactionManagerContent) -> TransactionManager { + let txs: Vec<_> = mem::take(&mut content.txs).unwrap(); TransactionManager { - txs: self.txs.unwrap_or_default(), - on_l2_awaiting_l1_consumption: self.on_l2_awaiting_l1_consumption.unwrap_or_default(), - ..Default::default() + txs: SoftDeleteIndexMap::from(txs), + committed: content + .committed + .unwrap_or_default() + .into_iter() + .map(|tx_hash| (tx_hash, None)) + .collect(), } } } #[derive(Debug, Default)] struct TransactionManagerContentBuilder { - txs: Option>, - on_l2_awaiting_l1_consumption: Option>, + txs: Option>, + committed: Option>, } impl TransactionManagerContentBuilder { fn with_txs(mut self, txs: impl IntoIterator) -> Self { - self.txs = Some(txs.into_iter().map(|tx| (tx.tx_hash, tx)).collect()); + self.txs = Some(txs.into_iter().collect()); self } - fn with_on_l2_awaiting_l1_consumption( - mut self, - tx_hashes: impl IntoIterator, - ) -> Self { - self.on_l2_awaiting_l1_consumption = Some(tx_hashes.into_iter().collect()); + fn committed(mut self, tx_hashes: impl IntoIterator) -> Self { + self.committed = Some(tx_hashes.into_iter().collect()); self } @@ -114,13 +116,10 @@ impl TransactionManagerContentBuilder { return None; } - Some(TransactionManagerContent { - txs: self.txs, - on_l2_awaiting_l1_consumption: self.on_l2_awaiting_l1_consumption, - }) + Some(TransactionManagerContent { txs: self.txs, committed: self.committed }) } fn is_default(&self) -> bool { - self.txs.is_none() && self.on_l2_awaiting_l1_consumption.is_none() + self.txs.is_none() && self.committed.is_none() } } diff --git a/crates/starknet_l1_provider/src/transaction_manager.rs b/crates/starknet_l1_provider/src/transaction_manager.rs index 3c00dcbd11..33c92635f1 100644 --- a/crates/starknet_l1_provider/src/transaction_manager.rs +++ b/crates/starknet_l1_provider/src/transaction_manager.rs @@ -1,34 +1,38 @@ -use indexmap::{IndexMap, IndexSet}; +use indexmap::IndexMap; use starknet_api::executable_transaction::L1HandlerTransaction; use starknet_api::transaction::TransactionHash; use starknet_l1_provider_types::ValidationStatus; +use crate::soft_delete_index_map::SoftDeleteIndexMap; + #[derive(Debug, Default)] pub struct TransactionManager { - pub txs: IndexMap, - pub proposed_txs: IndexSet, - pub on_l2_awaiting_l1_consumption: IndexSet, + pub txs: SoftDeleteIndexMap, + pub committed: IndexMap>, } impl TransactionManager { pub fn get_txs(&mut self, n_txs: usize) -> Vec { - let (tx_hashes, txs): (Vec<_>, Vec<_>) = self - .txs - .iter() - .skip(self.proposed_txs.len()) // Transactions are proposed FIFO. - .take(n_txs) - .map(|(&hash, tx)| (hash, tx.clone())) - .unzip(); - - self.proposed_txs.extend(tx_hashes); + let mut txs = Vec::with_capacity(n_txs); + + for _ in 0..n_txs { + match self.txs.soft_pop_front().cloned() { + Some(tx) => txs.push(tx), + None => break, + } + } txs } - pub fn tx_status(&self, tx_hash: TransactionHash) -> ValidationStatus { - if self.txs.contains_key(&tx_hash) { + pub fn validate_tx(&mut self, tx_hash: TransactionHash) -> ValidationStatus { + if self.committed.contains_key(&tx_hash) { + return ValidationStatus::AlreadyIncludedOnL2; + } + + if self.txs.soft_remove(tx_hash).is_some() { ValidationStatus::Validated - } else if self.on_l2_awaiting_l1_consumption.contains(&tx_hash) { - ValidationStatus::AlreadyIncludedOnL2 + } else if self.txs.is_staged(&tx_hash) { + ValidationStatus::AlreadyIncludedInPropsedBlock } else { ValidationStatus::ConsumedOnL1OrUnknown } diff --git a/crates/starknet_l1_provider_types/src/lib.rs b/crates/starknet_l1_provider_types/src/lib.rs index 84555f2c28..c6f8400d8d 100644 --- a/crates/starknet_l1_provider_types/src/lib.rs +++ b/crates/starknet_l1_provider_types/src/lib.rs @@ -23,6 +23,7 @@ pub type SharedL1ProviderClient = Arc; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ValidationStatus { + AlreadyIncludedInPropsedBlock, AlreadyIncludedOnL2, ConsumedOnL1OrUnknown, Validated,