From e133e7473866f88ce17a90f5ff919bfc3b842c16 Mon Sep 17 00:00:00 2001 From: giladchase Date: Mon, 18 Nov 2024 10:00:56 +0200 Subject: [PATCH] feat(l1_provider): implement get_txs (#2069) Co-Authored-By: Gilad Chase --- Cargo.lock | 2 + crates/starknet_l1_provider/Cargo.toml | 3 + .../src/l1_provider_tests.rs | 64 ++++++++++++++++++- crates/starknet_l1_provider/src/lib.rs | 42 ++++++++++-- 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7e9e0338b1..a554df546e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10329,7 +10329,9 @@ name = "starknet_l1_provider" version = "0.0.0" dependencies = [ "assert_matches", + "indexmap 2.6.0", "papyrus_base_layer", + "pretty_assertions", "starknet_api", "thiserror", ] diff --git a/crates/starknet_l1_provider/Cargo.toml b/crates/starknet_l1_provider/Cargo.toml index 2340234fcb..be12f4533d 100644 --- a/crates/starknet_l1_provider/Cargo.toml +++ b/crates/starknet_l1_provider/Cargo.toml @@ -6,12 +6,15 @@ repository.workspace = true license.workspace = true [dependencies] +indexmap.workspace = true papyrus_base_layer.workspace = true starknet_api.workspace = true thiserror.workspace = true [dev-dependencies] assert_matches.workspace = true +pretty_assertions.workspace = true +starknet_api = { workspace = true, features = ["testing"] } [lints] workspace = true diff --git a/crates/starknet_l1_provider/src/l1_provider_tests.rs b/crates/starknet_l1_provider/src/l1_provider_tests.rs index be8dcbc608..aa5db6cecc 100644 --- a/crates/starknet_l1_provider/src/l1_provider_tests.rs +++ b/crates/starknet_l1_provider/src/l1_provider_tests.rs @@ -1,7 +1,69 @@ use assert_matches::assert_matches; +use indexmap::IndexMap; +use pretty_assertions::assert_eq; +use starknet_api::executable_transaction::L1HandlerTransaction; +use starknet_api::hash::StarkHash; +use starknet_api::l1_handler_tx_args; +use starknet_api::test_utils::l1_handler::executable_l1_handler_tx; +use starknet_api::transaction::TransactionHash; +use crate::errors::L1ProviderError; use crate::errors::L1ProviderError::UnexpectedProviderStateTransition; -use crate::{L1Provider, ProviderState}; +use crate::{L1Provider, ProviderState, TransactionManager}; + +#[macro_export] +macro_rules! tx { + (tx_hash: $tx_hash:expr) => {{ + executable_l1_handler_tx( + l1_handler_tx_args!( + tx_hash: TransactionHash(StarkHash::from($tx_hash)) , ..Default::default() + ) + ) + }}; +} + +// TODO: change to something more robust once we have more tests. +fn l1_provider(txs: Vec) -> L1Provider { + let n_txs = txs.len(); + let awaiting_l2_inclusion: IndexMap<_, _> = + txs.into_iter().map(|tx| (tx.tx_hash, tx)).collect(); + assert_eq!( + awaiting_l2_inclusion.len(), + n_txs, + "Transactions given to this constructor should have unique hashes." + ); + + L1Provider { + tx_manager: TransactionManager { txs: awaiting_l2_inclusion, ..Default::default() }, + ..Default::default() + } +} + +#[test] +fn get_txs_happy_flow() { + // Setup. + let txs = vec![tx!(tx_hash: 0), tx!(tx_hash: 1), tx!(tx_hash: 2)]; + let mut l1_provider = l1_provider(txs.clone()); + l1_provider.proposal_start().unwrap(); + + // Test. + assert_eq!(l1_provider.get_txs(0).unwrap(), vec![]); + assert_eq!(l1_provider.get_txs(1).unwrap(), vec![txs[0].clone()]); + assert_eq!(l1_provider.get_txs(3).unwrap(), txs[1..=2].to_vec()); + assert_eq!(l1_provider.get_txs(1).unwrap(), vec![]); +} + +#[test] +fn pending_state_errors() { + // Setup. + let mut l1_provider = l1_provider(vec![tx!(tx_hash: 0)]); + + // Test. + assert_matches!( + l1_provider.get_txs(1).unwrap_err(), + L1ProviderError::GetTransactionsInPendingState + ); +} #[test] fn proposal_start_errors() { diff --git a/crates/starknet_l1_provider/src/lib.rs b/crates/starknet_l1_provider/src/lib.rs index 0fbc9d0488..fa47ac4415 100644 --- a/crates/starknet_l1_provider/src/lib.rs +++ b/crates/starknet_l1_provider/src/lib.rs @@ -1,5 +1,6 @@ pub mod errors; +use indexmap::{IndexMap, IndexSet}; use starknet_api::executable_transaction::L1HandlerTransaction; use starknet_api::transaction::TransactionHash; @@ -15,7 +16,9 @@ pub mod l1_provider_tests; // is compatible with it. #[derive(Debug, Default)] pub struct L1Provider { - unconsumed_l1_not_in_l2_block_txs: PendingMessagesFromL1, + tx_manager: TransactionManager, + // TODO(Gilad): consider transitioning to a generic phantom state once the infra is stabilized + // and we see how well it handles consuming the L1Provider when moving between states. state: ProviderState, } @@ -27,9 +30,10 @@ impl L1Provider { ); } - pub fn get_txs(&mut self, n_txs: usize) -> L1ProviderResult<&[L1HandlerTransaction]> { + /// Retrieves up to `n_txs` transactions that have yet to be proposed or accepted on L2. + pub fn get_txs(&mut self, n_txs: usize) -> L1ProviderResult> { match self.state { - ProviderState::Propose => Ok(self.unconsumed_l1_not_in_l2_block_txs.get(n_txs)), + ProviderState::Propose => Ok(self.tx_manager.get_txs(n_txs)), ProviderState::Pending => Err(L1ProviderError::GetTransactionsInPendingState), ProviderState::Validate => Err(L1ProviderError::GetTransactionConsensusBug), } @@ -88,11 +92,35 @@ impl L1Provider { } #[derive(Debug, Default)] -struct PendingMessagesFromL1; +struct TransactionManager { + txs: IndexMap, + proposed_txs: IndexSet, + _on_l2_awaiting_l1_consumption: IndexSet, +} + +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); + txs + } + + pub fn _add_unconsumed_l1_not_in_l2_block_tx(&mut self, _tx: L1HandlerTransaction) { + todo!( + "Check if tx is in L2, if it isn't on L2 add it to the txs buffer, otherwise print + debug and do nothing." + ) + } -impl PendingMessagesFromL1 { - fn get(&self, n_txs: usize) -> &[L1HandlerTransaction] { - todo!("stage and return {n_txs} txs") + pub fn _mark_tx_included_on_l2(&mut self, _tx_hash: &TransactionHash) { + todo!("Adds the tx hash to l2 buffer; remove tx from the txs storage if it's there.") } }