From a23a0c6ebe7474aa20ebfdc9aafcfb0023742e42 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Tue, 2 Apr 2024 00:49:26 +0800 Subject: [PATCH] fix(katana): block env wrongly updated with cli value (#1744) fix #1724 updated the test to make sure the gas prices doesn't get modified with the cli value in the [`update_block_env`](https://github.com/dojoengine/dojo/blob/ad58e43df7d162736fced6b071d59732183e77fb/crates/katana/core/src/backend/mod.rs#L182) method. now actual tx execution would result in a similar fee value that you get when doing fee estimation --- bin/katana/src/args.rs | 72 ++++++++++++++++-------- crates/katana/core/src/backend/config.rs | 13 +---- crates/katana/core/src/backend/mod.rs | 27 +++++++-- 3 files changed, 71 insertions(+), 41 deletions(-) diff --git a/bin/katana/src/args.rs b/bin/katana/src/args.rs index c583434afe..53c065a0a6 100644 --- a/bin/katana/src/args.rs +++ b/bin/katana/src/args.rs @@ -233,21 +233,29 @@ impl KatanaArgs { } pub fn starknet_config(&self) -> StarknetConfig { - let gas_price = GasPrices { - eth: self.starknet.environment.l1_eth_gas_price.unwrap_or(DEFAULT_ETH_L1_GAS_PRICE), - strk: self.starknet.environment.l1_strk_gas_price.unwrap_or(DEFAULT_STRK_L1_GAS_PRICE), - }; - let genesis = match self.starknet.genesis.clone() { Some(genesis) => genesis, None => { + let gas_prices = GasPrices { + eth: self + .starknet + .environment + .l1_eth_gas_price + .unwrap_or(DEFAULT_ETH_L1_GAS_PRICE), + strk: self + .starknet + .environment + .l1_strk_gas_price + .unwrap_or(DEFAULT_STRK_L1_GAS_PRICE), + }; + let accounts = DevAllocationsGenerator::new(self.starknet.total_accounts) .with_seed(parse_seed(&self.starknet.seed)) .with_balance(DEFAULT_PREFUNDED_ACCOUNT_BALANCE) .generate(); let mut genesis = Genesis { - gas_prices: gas_price.clone(), + gas_prices, sequencer_address: *DEFAULT_SEQUENCER_ADDRESS, ..Default::default() }; @@ -263,7 +271,6 @@ impl KatanaArgs { fork_rpc_url: self.rpc_url.clone(), fork_block_number: self.fork_block_number, env: Environment { - gas_price, chain_id: self.starknet.environment.chain_id, invoke_max_steps: self .starknet @@ -287,32 +294,51 @@ mod test { use super::*; #[test] - fn default_block_context_from_args() { + fn test_starknet_config_default() { let args = KatanaArgs::parse_from(["katana"]); - let block_context = args.starknet_config().block_env(); - assert_eq!(block_context.l1_gas_prices.eth, DEFAULT_ETH_L1_GAS_PRICE); - assert_eq!(block_context.l1_gas_prices.strk, DEFAULT_STRK_L1_GAS_PRICE); + let config = args.starknet_config(); + + assert!(!config.disable_fee); + assert!(!config.disable_validate); + assert_eq!(config.fork_rpc_url, None); + assert_eq!(config.fork_block_number, None); + assert_eq!(config.env.chain_id, ChainId::parse("KATANA").unwrap()); + assert_eq!(config.env.invoke_max_steps, DEFAULT_INVOKE_MAX_STEPS); + assert_eq!(config.env.validate_max_steps, DEFAULT_VALIDATE_MAX_STEPS); + assert_eq!(config.db_dir, None); + assert_eq!(config.genesis.gas_prices.eth, DEFAULT_ETH_L1_GAS_PRICE); + assert_eq!(config.genesis.gas_prices.strk, DEFAULT_STRK_L1_GAS_PRICE); + assert_eq!(config.genesis.sequencer_address, *DEFAULT_SEQUENCER_ADDRESS); } #[test] - fn custom_block_context_from_args() { + fn test_starknet_config_custom() { let args = KatanaArgs::parse_from([ "katana", - "--eth-gas-price", - "10", - "--strk-gas-price", - "20", + "--disable-fee", + "--disable-validate", "--chain-id", "SN_GOERLI", - "--validate-max-steps", - "100", "--invoke-max-steps", "200", + "--validate-max-steps", + "100", + "--db-dir", + "/path/to/db", + "--eth-gas-price", + "10", + "--strk-gas-price", + "20", ]); - - let block_context = args.starknet_config().block_env(); - - assert_eq!(block_context.l1_gas_prices.eth, 10); - assert_eq!(block_context.l1_gas_prices.strk, 20); + let config = args.starknet_config(); + + assert!(config.disable_fee); + assert!(config.disable_validate); + assert_eq!(config.env.chain_id, ChainId::GOERLI); + assert_eq!(config.env.invoke_max_steps, 200); + assert_eq!(config.env.validate_max_steps, 100); + assert_eq!(config.db_dir, Some(PathBuf::from("/path/to/db"))); + assert_eq!(config.genesis.gas_prices.eth, 10); + assert_eq!(config.genesis.gas_prices.strk, 20); } } diff --git a/crates/katana/core/src/backend/config.rs b/crates/katana/core/src/backend/config.rs index 622e550cf4..b7b25c3799 100644 --- a/crates/katana/core/src/backend/config.rs +++ b/crates/katana/core/src/backend/config.rs @@ -1,18 +1,13 @@ use std::path::PathBuf; use ::primitive_types::U256; -use katana_primitives::block::GasPrices; use katana_primitives::chain::ChainId; -use katana_primitives::env::BlockEnv; use katana_primitives::genesis::allocation::DevAllocationsGenerator; use katana_primitives::genesis::constant::DEFAULT_PREFUNDED_ACCOUNT_BALANCE; use katana_primitives::genesis::Genesis; use url::Url; -use crate::constants::{ - DEFAULT_ETH_L1_GAS_PRICE, DEFAULT_INVOKE_MAX_STEPS, DEFAULT_STRK_L1_GAS_PRICE, - DEFAULT_VALIDATE_MAX_STEPS, -}; +use crate::constants::{DEFAULT_INVOKE_MAX_STEPS, DEFAULT_VALIDATE_MAX_STEPS}; use crate::env::BlockContextGenerator; #[derive(Debug, Clone)] @@ -27,10 +22,6 @@ pub struct StarknetConfig { } impl StarknetConfig { - pub fn block_env(&self) -> BlockEnv { - BlockEnv { l1_gas_prices: self.env.gas_price.clone(), ..Default::default() } - } - pub fn block_context_generator(&self) -> BlockContextGenerator { BlockContextGenerator::default() } @@ -60,7 +51,6 @@ impl Default for StarknetConfig { #[derive(Debug, Clone)] pub struct Environment { pub chain_id: ChainId, - pub gas_price: GasPrices, pub invoke_max_steps: u32, pub validate_max_steps: u32, } @@ -71,7 +61,6 @@ impl Default for Environment { chain_id: ChainId::parse("KATANA").unwrap(), invoke_max_steps: DEFAULT_INVOKE_MAX_STEPS, validate_max_steps: DEFAULT_VALIDATE_MAX_STEPS, - gas_price: GasPrices { eth: DEFAULT_ETH_L1_GAS_PRICE, strk: DEFAULT_STRK_L1_GAS_PRICE }, } } } diff --git a/crates/katana/core/src/backend/mod.rs b/crates/katana/core/src/backend/mod.rs index 979c0903d2..5c2947bc62 100644 --- a/crates/katana/core/src/backend/mod.rs +++ b/crates/katana/core/src/backend/mod.rs @@ -179,7 +179,6 @@ impl Backend { block_env.number += 1; block_env.timestamp = timestamp; - block_env.l1_gas_prices = self.config.env.gas_price.clone(); } pub fn mine_empty_block( @@ -204,8 +203,12 @@ mod tests { use crate::backend::config::{Environment, StarknetConfig}; fn create_test_starknet_config() -> StarknetConfig { + let mut genesis = Genesis::default(); + genesis.gas_prices.eth = 2100; + genesis.gas_prices.strk = 3100; + StarknetConfig { - genesis: Genesis::default(), + genesis, disable_fee: true, env: Environment::default(), ..Default::default() @@ -219,17 +222,26 @@ mod tests { #[tokio::test] async fn test_creating_blocks() { let backend = create_test_backend().await; - let provider = backend.blockchain.provider(); - assert_eq!(BlockNumberProvider::latest_number(provider).unwrap(), 0); - let block_num = provider.latest_number().unwrap(); + let block_env = provider.block_env_at(block_num.into()).unwrap().unwrap(); + + assert_eq!(block_num, 0); + assert_eq!(block_env.number, 0); + assert_eq!(block_env.l1_gas_prices.eth, 2100); + assert_eq!(block_env.l1_gas_prices.strk, 3100); + let mut block_env = provider.block_env_at(block_num.into()).unwrap().unwrap(); backend.update_block_env(&mut block_env); backend.mine_empty_block(&block_env).unwrap(); let block_num = provider.latest_number().unwrap(); + assert_eq!(block_num, 1); + assert_eq!(block_env.number, 1); + assert_eq!(block_env.l1_gas_prices.eth, 2100); + assert_eq!(block_env.l1_gas_prices.strk, 3100); + let mut block_env = provider.block_env_at(block_num.into()).unwrap().unwrap(); backend.update_block_env(&mut block_env); backend.mine_empty_block(&block_env).unwrap(); @@ -237,8 +249,11 @@ mod tests { let block_num = provider.latest_number().unwrap(); let block_env = provider.block_env_at(block_num.into()).unwrap().unwrap(); - assert_eq!(BlockNumberProvider::latest_number(provider).unwrap(), 2); + let block_num = provider.latest_number().unwrap(); + assert_eq!(block_num, 2); assert_eq!(block_env.number, 2); + assert_eq!(block_env.l1_gas_prices.eth, 2100); + assert_eq!(block_env.l1_gas_prices.strk, 3100); let block0 = BlockProvider::block_by_number(provider, 0).unwrap().unwrap(); let block1 = BlockProvider::block_by_number(provider, 1).unwrap().unwrap();