fix tests

Author: kariy

Cargo.lock

@@ -4,19 +4,18 @@ use anyhow::{anyhow, Context, Result};
 use clap::Parser;
 use fixtures::{db, TempDb};
 use katana::cli::Cli;
-use katana_db::abstraction::Database;
 use katana_primitives::Felt;
 use katana_provider::api::block::BlockNumberProvider;
 use katana_provider::api::state::{StateFactoryProvider, StateRootProvider};
-use katana_provider::providers::db::DbProvider;
+use katana_provider::ProviderFactory;
 use rstest::*;
 
 mod fixtures;
 
 /// Verify that historical state roots can be retrieved for specific blocks
 /// Returns Ok(()) if the state root can be retrieved, Err if it cannot
-fn historical_roots<Db: Database>(
-    provider: &DbProvider<Db>,
+fn historical_roots(
+    provider: impl StateFactoryProvider,
     block_number: u64,
 ) -> Result<(Felt, Felt)> {
     let historical = provider
@@ -33,7 +32,7 @@ fn historical_roots<Db: Database>(
 }
 
 /// Get the current state roots (classes and contracts)
-fn latest_roots<Db: Database>(provider: &DbProvider<Db>) -> Result<(Felt, Felt)> {
+fn latest_roots(provider: impl StateFactoryProvider) -> Result<(Felt, Felt)> {
     let state_provider = provider.latest().context("failed to get latest state provider")?;
     let classes_root = state_provider.classes_root().context("failed to get classes root")?;
     let contracts_root = state_provider.contracts_root().context("failed to get contracts root")?;
@@ -42,7 +41,9 @@ fn latest_roots<Db: Database>(provider: &DbProvider<Db>) -> Result<(Felt, Felt)>
 
 #[rstest]
 fn prune_latest_removes_all_history(db: TempDb) {
-    let provider = db.provider_ro();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider();
+
     let latest_block = provider.latest_number().unwrap();
 
     for num in 1..=latest_block {
@@ -52,13 +53,16 @@ fn prune_latest_removes_all_history(db: TempDb) {
     }
 
     let (initial_classes_root, initial_contracts_root) = latest_roots(&provider).unwrap();
+
     drop(provider);
+    drop(provider_factory);
 
     // Will prune all historical tries (blocks < 15)
     let path = db.path_str();
     Cli::parse_from(["katana", "db", "prune", "--path", path, "--latest", "-y"]).run().unwrap();
 
-    let provider = db.provider_ro();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider();
 
     // Verify historical states (0 -> 14) are no longer accessible (ie zero)
     for num in 0..=14u64 {
@@ -85,7 +89,9 @@ fn prune_latest_removes_all_history(db: TempDb) {
 
 #[rstest]
 fn prune_keep_last_n_blocks(db: TempDb) {
-    let provider = db.provider_ro();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider();
+
     let latest_block = provider.latest_number().unwrap();
 
     // block -> (classes root, contracts root)
@@ -99,15 +105,19 @@ fn prune_keep_last_n_blocks(db: TempDb) {
     }
 
     let (initial_classes_root, initial_contracts_root) = latest_roots(&provider).unwrap();
+
     drop(provider);
+    drop(provider_factory);
 
     let keep_last_n = 3;
     let path = db.path_str();
     Cli::parse_from(["katana", "db", "prune", "--path", path, "--keep-last", "3", "-y"])
         .run()
         .unwrap();
 
-    let provider = db.provider_ro();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider();
+
     let (final_classes_root, final_contracts_root) = latest_roots(&provider).unwrap();
 
     // pruned blocks (ie blocks before the cuttoff point) should be zero
@@ -141,7 +151,9 @@ fn prune_keep_last_n_blocks(db: TempDb) {
 
 #[rstest]
 fn prune_keep_last_n_blocks_exceeds_available(db: TempDb) {
-    let provider = db.provider_ro();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider();
+
     let latest_block = provider.latest_number().unwrap();
 
     // block -> (classes root, contracts root)
@@ -156,7 +168,9 @@ fn prune_keep_last_n_blocks_exceeds_available(db: TempDb) {
     }
 
     let (initial_classes_root, initial_contracts_root) = latest_roots(&provider).unwrap();
+
     drop(provider);
+    drop(provider_factory);
 
     // Request to keep more blocks than are available
     let keep_last_n = latest_block + 10;
@@ -176,7 +190,9 @@ fn prune_keep_last_n_blocks_exceeds_available(db: TempDb) {
     .run()
     .unwrap();
 
-    let provider = db.provider_ro();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider();
+
     let (final_classes_root, final_contracts_root) = latest_roots(&provider).unwrap();
 
     // Verify that NO pruning occurred - all historical states should still be accessible

bin/katana/tests/db_prune.rs

@@ -6,7 +6,7 @@ use katana_primitives::contract::{ContractAddress, Nonce, StorageKey, StorageVal
 use katana_primitives::state::{StateUpdates, StateUpdatesWithClasses};
 use katana_provider::api::block::BlockWriter;
 use katana_provider::api::trie::TrieWriter;
-use katana_provider::providers::db::DbProvider;
+use katana_provider::{DbProviderFactory, MutableProvider, ProviderFactory};
 use katana_utils::arbitrary;
 use rstest::*;
 use tempfile::TempDir;
@@ -23,25 +23,17 @@ impl TempDb {
         Self { temp_dir }
     }
 
-    pub fn provider_ro(&self) -> DbProvider {
-        DbProvider::new(self.open_ro())
+    pub fn provider_factory(&self) -> DbProviderFactory {
+        DbProviderFactory::new(self.open_rw())
     }
 
-    pub fn provider_rw(&self) -> DbProvider {
-        DbProvider::new(self.open_rw())
-    }
-
-    fn open_ro(&self) -> katana_db::Db {
-        katana::cli::db::open_db_ro(self.path_str()).unwrap()
+    pub fn path_str(&self) -> &str {
+        self.temp_dir.path().to_str().unwrap()
     }
 
     fn open_rw(&self) -> katana_db::Db {
         katana::cli::db::open_db_rw(self.path_str()).unwrap()
     }
-
-    pub fn path_str(&self) -> &str {
-        self.temp_dir.path().to_str().unwrap()
-    }
 }
 
 impl Default for TempDb {
@@ -66,7 +58,8 @@ pub fn db() -> TempDb {
 
 /// Populate database with test data using the TrieWriter trait
 fn populate_db(db: &TempDb) {
-    let provider = db.provider_rw();
+    let provider_factory = db.provider_factory();
+    let provider = provider_factory.provider_mut();
 
     for num in 0..=15u64 {
         let mut classes = BTreeMap::new();
@@ -137,4 +130,6 @@ fn populate_db(db: &TempDb) {
             )
             .unwrap();
     }
+
+    provider.commit().expect("failed to commit");
 }

bin/katana/tests/fixtures.rs

@@ -8,14 +8,12 @@ version.workspace = true
 
 [dependencies]
 katana-chain-spec.workspace = true
-katana-db.workspace = true
 katana-executor.workspace = true
 katana-gas-price-oracle.workspace = true
 katana-metrics.workspace = true
 katana-pool.workspace = true
 katana-primitives = { workspace = true, features = [ "arbitrary" ] }
 katana-provider.workspace = true
-katana-rpc-client.workspace = true
 katana-rpc-types.workspace = true
 katana-tasks.workspace = true
 katana-trie.workspace = true

crates/core/Cargo.toml

@@ -10,6 +10,7 @@ use katana_primitives::state::StateUpdates;
 use katana_primitives::transaction::TxWithHash;
 use katana_primitives::{ContractAddress, Felt};
 use katana_provider::providers::db::DbProvider;
+use katana_provider::DbProviderFactory;
 use pprof::criterion::{Output, PProfProfiler};
 
 struct BlockConfig {
@@ -42,11 +43,11 @@ const BIG_BLOCK_CONFIG: BlockConfig = BlockConfig {
     nb_of_contracts: 100,
 };
 
-fn commit(block: UncommittedBlock<'_, DbProvider>) {
+fn commit(block: UncommittedBlock<'_, DbProviderFactory>) {
     let _ = block.commit();
 }
 
-fn commit_parallel(block: UncommittedBlock<'_, DbProvider>) {
+fn commit_parallel(block: UncommittedBlock<'_, DbProviderFactory>) {
     let _ = block.commit_parallel();
 }
 

crates/core/benches/commit.rs

@@ -1,7 +1,3 @@
-use std::sync::Arc;
-
-use anyhow::{bail, Context, Result};
-use katana_primitives::block::{BlockHashOrNumber, BlockIdOrTag, GasPrices};
 use katana_provider::api::block::{BlockIdReader, BlockProvider, BlockWriter};
 use katana_provider::api::contract::ContractClassWriter;
 use katana_provider::api::env::BlockEnvProvider;
@@ -13,136 +9,7 @@ use katana_provider::api::transaction::{
     TransactionsProviderExt,
 };
 use katana_provider::api::trie::TrieWriter;
-use katana_provider::{DbProviderFactory, ForkProviderFactory, MutableProvider, ProviderFactory};
-use katana_rpc_client::starknet::Client as StarknetClient;
-use katana_rpc_types::GetBlockWithTxHashesResponse;
-use num_traits::ToPrimitive;
-use starknet::core::utils::parse_cairo_short_string;
-use tracing::info;
-use url::Url;
-
-pub type GenericStorageProvider =
-    Arc<dyn ProviderFactory<Provider = Box<dyn DatabaseRO>, ProviderMut = Box<dyn DatabaseRW>>>;
-
-#[derive(Debug, Clone)]
-pub struct StorageProvider<P> {
-    provider_factory: P,
-}
-
-impl<P: ProviderFactory> StorageProvider<P> {
-    pub fn new(provider_factory: P) -> Self {
-        Self { provider_factory }
-    }
-}
-
-impl StorageProvider<DbProviderFactory<katana_db::Db>> {
-    pub fn new_with_db(db: katana_db::Db) -> Self {
-        Self::new(DbProviderFactory::new(db))
-    }
-
-    pub fn new_in_memory() -> Self {
-        Self::new(DbProviderFactory::new_in_memory())
-    }
-}
-
-impl StorageProvider<ForkProviderFactory> {
-    /// Builds a new blockchain with a forked block.
-    pub async fn new_forked(
-        db: katana_db::Db,
-        fork_url: Url,
-        fork_block: Option<BlockHashOrNumber>,
-        chain: &mut katana_chain_spec::dev::ChainSpec,
-    ) -> Result<Self> {
-        let client = StarknetClient::new(fork_url);
-        let chain_id = client.chain_id().await.context("failed to fetch forked network id")?;
-
-        // if the id is not in ASCII encoding, we display the chain id as is in hex.
-        let parsed_id = match parse_cairo_short_string(&chain_id) {
-            Ok(id) => id,
-            Err(_) => format!("{chain_id:#x}"),
-        };
-
-        // If the fork block number is not specified, we use the latest accepted block on the forked
-        // network.
-        let block_id = if let Some(id) = fork_block {
-            id
-        } else {
-            let res = client.block_number().await?;
-            BlockHashOrNumber::Num(res.block_number)
-        };
-
-        info!(chain = %parsed_id, block = %block_id, "Forking chain.");
-
-        let block = client
-            .get_block_with_tx_hashes(BlockIdOrTag::from(block_id))
-            .await
-            .context("failed to fetch forked block")?;
-
-        let GetBlockWithTxHashesResponse::Block(forked_block) = block else {
-            bail!("forking a pending block is not allowed")
-        };
-
-        let block_num = forked_block.block_number;
-        let genesis_block_num = block_num + 1;
-
-        chain.id = chain_id.into();
-
-        // adjust the genesis to match the forked block
-        chain.genesis.timestamp = forked_block.timestamp;
-        chain.genesis.number = genesis_block_num;
-        chain.genesis.state_root = Default::default();
-        chain.genesis.parent_hash = forked_block.parent_hash;
-        chain.genesis.sequencer_address = forked_block.sequencer_address;
-
-        // TODO: remove gas price from genesis
-        let eth_l1_gas_price =
-            forked_block.l1_gas_price.price_in_wei.to_u128().expect("should fit in u128");
-        let strk_l1_gas_price =
-            forked_block.l1_gas_price.price_in_fri.to_u128().expect("should fit in u128");
-        chain.genesis.gas_prices =
-            unsafe { GasPrices::new_unchecked(eth_l1_gas_price, strk_l1_gas_price) };
-
-        // TODO: convert this to block number instead of BlockHashOrNumber so that it is easier to
-        // check if the requested block is within the supported range or not.
-        let provider_factory = ForkProviderFactory::new(db, block_num, client.clone());
-
-        // update the genesis block with the forked block's data
-        // we dont update the `l1_gas_price` bcs its already done when we set the `gas_prices` in
-        // genesis. this flow is kinda flawed, we should probably refactor it out of the
-        // genesis.
-        let mut block = chain.block();
-
-        let eth_l1_data_gas_price =
-            forked_block.l1_data_gas_price.price_in_wei.to_u128().expect("should fit in u128");
-        let strk_l1_data_gas_price =
-            forked_block.l1_data_gas_price.price_in_fri.to_u128().expect("should fit in u128");
-
-        block.header.l1_data_gas_prices =
-            unsafe { GasPrices::new_unchecked(eth_l1_data_gas_price, strk_l1_data_gas_price) };
-
-        block.header.l1_da_mode = forked_block.l1_da_mode;
-
-        Ok(Self::new(provider_factory))
-    }
-}
-
-// impl<P> ProviderFactory for StorageProvider<P>
-// where
-//     P: ProviderFactory,
-//     <P as ProviderFactory>::Provider: DatabaseRO,
-//     <P as ProviderFactory>::ProviderMut: DatabaseRW,
-// {
-//     type Provider = Box<dyn DatabaseRO>;
-//     type ProviderMut = Box<dyn DatabaseRW>;
-
-//     fn provider(&self) -> Self::Provider {
-//         Box::new(self.provider_factory.provider_mut())
-//     }
-
-//     fn provider_mut(&self) -> Self::ProviderMut {
-//         Box::new(self.provider_factory.provider_mut())
-//     }
-// }
+use katana_provider::MutableProvider;
 
 pub trait DatabaseRO:
     BlockIdReader

crates/core/src/backend/storage.rs

@@ -9,12 +9,12 @@ use katana_provider::DbProviderFactory;
 
 use super::*;
 
-fn test_backend() -> Arc<Backend<NoopExecutorFactory>> {
+fn test_backend() -> Arc<Backend<NoopExecutorFactory, DbProviderFactory>> {
     let chain_spec = Arc::new(ChainSpec::dev());
     let executor_factory = NoopExecutorFactory::new();
-    let blockchain = Blockchain::new(DbProviderFactory::new_in_memory());
+    let storage = DbProviderFactory::new_in_memory();
     let gas_oracle = GasPriceOracle::create_for_testing();
-    let backend = Arc::new(Backend::new(chain_spec, blockchain, gas_oracle, executor_factory));
+    let backend = Arc::new(Backend::new(chain_spec, storage, gas_oracle, executor_factory));
     backend.init_genesis(false).expect("failed to initialize genesis");
     backend
 }