From 8a5052630f3528e35dd389a57c0e56880539a28b Mon Sep 17 00:00:00 2001 From: Arnon Hod Date: Tue, 3 Sep 2024 09:27:29 +0300 Subject: [PATCH] feat: use the command line compiler in gateway (#599) --- .../resources/versioned_constants.json | 10 ++++----- crates/blockifier/src/fee/actual_cost_test.rs | 5 ++++- crates/blockifier/src/fee/gas_usage_test.rs | 5 ++++- crates/blockifier/src/transaction/objects.rs | 15 +++++++------ .../src/transaction/transactions_test.rs | 3 ++- crates/blockifier/src/versioned_constants.rs | 21 +++++++++++++------ crates/gateway/build.rs | 3 +++ crates/gateway/src/compilation.rs | 6 ++++++ crates/gateway/src/compilation_test.rs | 11 +++++----- crates/gateway/src/gateway.rs | 2 +- crates/gateway/src/gateway_test.rs | 2 +- crates/tests-integration/build.rs | 3 +++ 12 files changed, 58 insertions(+), 28 deletions(-) create mode 100644 crates/gateway/build.rs create mode 100644 crates/tests-integration/build.rs diff --git a/crates/blockifier/resources/versioned_constants.json b/crates/blockifier/resources/versioned_constants.json index 7fc31ab16a7..c8fdef843a3 100644 --- a/crates/blockifier/resources/versioned_constants.json +++ b/crates/blockifier/resources/versioned_constants.json @@ -11,16 +11,16 @@ "invoke_tx_max_n_steps": 10000000, "l2_resource_gas_costs": { "gas_per_data_felt": [ - 128, - 1000 + 5120, + 1 ], "event_key_factor": [ 2, 1 ], "gas_per_code_byte": [ - 875, - 1000 + 35000, + 1 ] }, "disable_cairo0_redeclaration": true, @@ -32,7 +32,6 @@ "entry_point_gas_cost": 1, "step_gas_cost": 827, "range_check_gas_cost": 15 - }, "constructor_entry_point_selector": "0x28ffe4ff0f226a9107253e17a904099aa4f63a02a5621de0576e5aa71bc5194", "default_entry_point_selector": 0, @@ -68,7 +67,6 @@ "get_block_hash_gas_cost": { "step_gas_cost": 104, "range_check_gas_cost": 2 - }, "get_execution_info_gas_cost": { "step_gas_cost": 100, diff --git a/crates/blockifier/src/fee/actual_cost_test.rs b/crates/blockifier/src/fee/actual_cost_test.rs index 06cccec6a6e..f75dca94ffb 100644 --- a/crates/blockifier/src/fee/actual_cost_test.rs +++ b/crates/blockifier/src/fee/actual_cost_test.rs @@ -45,7 +45,7 @@ fn versioned_constants() -> &'static VersionedConstants { #[rstest] fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool) { // An empty transaction (a theoretical case for sanity check). - let versioned_constants = VersionedConstants::default(); + let versioned_constants = VersionedConstants::create_for_account_testing(); let empty_tx_starknet_resources = StarknetResources::default(); let empty_tx_gas_usage_vector = empty_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da); @@ -64,6 +64,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool std::iter::empty(), ); let code_gas_cost = versioned_constants.l2_resource_gas_costs.gas_per_code_byte + * versioned_constants.l1_to_l2_gas_price_ratio() * u128_from_usize( (class_info.bytecode_length() + class_info.sierra_program_length()) * eth_gas_constants::WORD_WIDTH @@ -98,6 +99,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool ); let calldata_and_signature_gas_cost = versioned_constants.l2_resource_gas_costs.gas_per_data_felt + * versioned_constants.l1_to_l2_gas_price_ratio() * u128_from_usize(calldata_length + signature_length); let manual_starknet_gas_usage = calldata_and_signature_gas_cost.to_integer(); let manual_gas_vector = GasVector { l1_gas: manual_starknet_gas_usage, ..Default::default() } @@ -125,6 +127,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let message_segment_length = get_message_segment_length(&[], Some(l1_handler_payload_size)); let calldata_and_signature_gas_cost = versioned_constants.l2_resource_gas_costs.gas_per_data_felt + * versioned_constants.l1_to_l2_gas_price_ratio() * u128_from_usize(l1_handler_payload_size + signature_length); let manual_starknet_gas_usage = message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD + eth_gas_constants::GAS_PER_COUNTER_DECREASE diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index b83517acd83..f818dfb4152 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -77,7 +77,10 @@ fn test_get_event_gas_cost( let call_infos_iter = call_infos.iter(); let expected = GasVector::from_l1_gas( // 8 keys and 11 data words overall. - (data_word_cost * (event_key_factor * 8_u128 + 11_u128)).to_integer(), + (data_word_cost + * versioned_constants.l1_to_l2_gas_price_ratio() + * (event_key_factor * 8_u128 + 11_u128)) + .to_integer(), ); let starknet_resources = StarknetResources::new(0, 0, 0, StateChangesCount::default(), None, call_infos_iter); diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index a542a358349..5c907e9f2ab 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -334,8 +334,9 @@ impl StarknetResources { // TODO(Avi, 20/2/2024): Calculate the number of bytes instead of the number of felts. let total_data_size = u128_from_usize(self.calldata_length + self.signature_length); let l1_gas = (versioned_constants.l2_resource_gas_costs.gas_per_data_felt - * total_data_size) - .to_integer(); + * total_data_size + * versioned_constants.l1_to_l2_gas_price_ratio()) + .to_integer(); GasVector::from_l1_gas(l1_gas) } @@ -384,27 +385,29 @@ impl StarknetResources { (message_segment_length, gas_weight) } - /// Returns the gas cost of declared class codes. + /// Returns the L1 gas cost of declared class codes. pub fn get_code_cost(&self, versioned_constants: &VersionedConstants) -> GasVector { GasVector::from_l1_gas( (versioned_constants.l2_resource_gas_costs.gas_per_code_byte - * u128_from_usize(self.code_size)) + * u128_from_usize(self.code_size) + * versioned_constants.l1_to_l2_gas_price_ratio()) .to_integer(), ) } - /// Returns the gas cost of the transaction's state changes. + /// Returns the L1 gas cost of the transaction's state changes. pub fn get_state_changes_cost(&self, use_kzg_da: bool) -> GasVector { // TODO(Nimrod, 29/3/2024): delete `get_da_gas_cost` and move it's logic here. get_da_gas_cost(&self.state_changes_for_fee, use_kzg_da) } - /// Returns the gas cost of the transaction's emmited events. + /// Returns the L1 gas cost of the transaction's emmited events. pub fn get_events_cost(&self, versioned_constants: &VersionedConstants) -> GasVector { let l2_resource_gas_costs = &versioned_constants.l2_resource_gas_costs; let (event_key_factor, data_word_cost) = (l2_resource_gas_costs.event_key_factor, l2_resource_gas_costs.gas_per_data_felt); let l1_gas: u128 = (data_word_cost + * versioned_constants.l1_to_l2_gas_price_ratio() * (event_key_factor * self.total_event_keys + self.total_event_data_size)) .to_integer(); diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index f52e7444386..ee941ab22a2 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1826,7 +1826,8 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { let test_contract = FeatureContract::TestContract(cairo_version); let chain_info = &ChainInfo::create_for_testing(); let state = &mut test_state(chain_info, BALANCE, &[(test_contract, 1)]); - let block_context = &BlockContext::create_for_account_testing_with_kzg(use_kzg_da); + let block_context = &mut BlockContext::create_for_account_testing_with_kzg(use_kzg_da); + block_context.versioned_constants = VersionedConstants::latest_constants().clone(); let contract_address = test_contract.get_instance_address(0); let versioned_constants = &block_context.versioned_constants; let tx = L1HandlerTransaction::create_for_testing(Fee(1), contract_address); diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index 9319949db95..5acc190355f 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -126,12 +126,16 @@ impl VersionedConstants { /// Converts from l1 gas cost to l2 gas cost with **upward rounding** pub fn l1_to_l2_gas_price_conversion(&self, l1_gas_price: u128) -> u128 { - let l1_to_l2_gas_price_ratio: Ratio = - Ratio::new(1, u128::from(self.os_constants.gas_costs.step_gas_cost)) - * self.vm_resource_fee_cost()["n_steps"]; + let l1_to_l2_gas_price_ratio = self.l1_to_l2_gas_price_ratio(); *(l1_to_l2_gas_price_ratio * l1_gas_price).ceil().numer() } + /// Returns the ratio between l1 gas price and l2 gas price. + pub fn l1_to_l2_gas_price_ratio(&self) -> Ratio { + Ratio::new(1, u128::from(self.os_constants.gas_costs.step_gas_cost)) + * self.vm_resource_fee_cost()["n_steps"] + } + /// Returns the initial gas of any transaction to run with. pub fn tx_initial_gas(&self) -> u64 { let os_consts = &self.os_constants; @@ -185,8 +189,9 @@ impl VersionedConstants { #[cfg(any(feature = "testing", test))] pub fn create_for_account_testing() -> Self { + let step_cost = ResourceCost::from_integer(1); let vm_resource_fee_cost = Arc::new(HashMap::from([ - (crate::abi::constants::N_STEPS_RESOURCE.to_string(), ResourceCost::from_integer(1)), + (crate::abi::constants::N_STEPS_RESOURCE.to_string(), step_cost), (BuiltinName::pedersen.to_str_with_suffix().to_string(), ResourceCost::from_integer(1)), ( BuiltinName::range_check.to_str_with_suffix().to_string(), @@ -204,8 +209,12 @@ impl VersionedConstants { (BuiltinName::add_mod.to_str_with_suffix().to_string(), ResourceCost::from_integer(1)), (BuiltinName::mul_mod.to_str_with_suffix().to_string(), ResourceCost::from_integer(1)), ])); - - Self { vm_resource_fee_cost, ..Self::create_for_testing() } + let latest = Self::create_float_for_testing(); + let latest_step_cost = latest.vm_resource_fee_cost["n_steps"]; + let mut l2_resource_gas_costs = latest.l2_resource_gas_costs; + l2_resource_gas_costs.gas_per_code_byte *= latest_step_cost / step_cost; + l2_resource_gas_costs.gas_per_data_felt *= latest_step_cost / step_cost; + Self { vm_resource_fee_cost, l2_resource_gas_costs, ..latest } } // A more complicated instance to increase test coverage. diff --git a/crates/gateway/build.rs b/crates/gateway/build.rs new file mode 100644 index 00000000000..2c3d009e87c --- /dev/null +++ b/crates/gateway/build.rs @@ -0,0 +1,3 @@ +// Sets up the environment variable OUT_DIR, which holds the cairo compiler binary. +// The binary is dowloaded to OUT_DIR by the starknet_sierra_compile crate. +fn main() {} diff --git a/crates/gateway/src/compilation.rs b/crates/gateway/src/compilation.rs index aab1affc40f..45330c01e7d 100644 --- a/crates/gateway/src/compilation.rs +++ b/crates/gateway/src/compilation.rs @@ -6,6 +6,7 @@ use starknet_api::contract_class::ClassInfo; use starknet_api::core::CompiledClassHash; use starknet_api::rpc_transaction::RpcDeclareTransaction; use starknet_sierra_compile::cairo_lang_compiler::CairoLangSierraToCasmCompiler; +use starknet_sierra_compile::command_line_compiler::CommandLineCompiler; use starknet_sierra_compile::config::SierraToCasmCompilationConfig; use starknet_sierra_compile::utils::into_contract_class_for_compilation; use starknet_sierra_compile::SierraToCasmCompiler; @@ -24,6 +25,11 @@ pub struct GatewayCompiler { } impl GatewayCompiler { + pub fn new_command_line_compiler(config: SierraToCasmCompilationConfig) -> Self { + Self { sierra_to_casm_compiler: Arc::new(CommandLineCompiler::new(config)) } + } + + // TODO(Arni): Cosider deleting `CairoLangSierraToCasmCompiler`. pub fn new_cairo_lang_compiler(config: SierraToCasmCompilationConfig) -> Self { Self { sierra_to_casm_compiler: Arc::new(CairoLangSierraToCasmCompiler { config }) } } diff --git a/crates/gateway/src/compilation_test.rs b/crates/gateway/src/compilation_test.rs index 8c41913c12f..fd1ac085007 100644 --- a/crates/gateway/src/compilation_test.rs +++ b/crates/gateway/src/compilation_test.rs @@ -19,7 +19,7 @@ use crate::errors::GatewaySpecError; #[fixture] fn gateway_compiler() -> GatewayCompiler { - GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default()) + GatewayCompiler::new_command_line_compiler(SierraToCasmCompilationConfig::default()) } #[fixture] @@ -58,14 +58,15 @@ fn test_compile_contract_class_compiled_class_hash_mismatch( #[rstest] fn test_compile_contract_class_bytecode_size_validation(declare_tx_v3: RpcDeclareTransactionV3) { let gateway_compiler = - GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig { + GatewayCompiler::new_command_line_compiler(SierraToCasmCompilationConfig { max_bytecode_size: 1, }); let result = gateway_compiler.process_declare_tx(&RpcDeclareTransaction::V3(declare_tx_v3)); assert_matches!(result.unwrap_err(), GatewaySpecError::CompilationFailed); - let expected_compilation_error = - CompilationUtilError::CompilationError("Code size limit exceeded.".to_owned()); + let expected_compilation_error = CompilationUtilError::CompilationError( + "Error: Compilation failed.\n\nCaused by:\n Code size limit exceeded.\n".to_owned(), + ); assert!(logs_contain(format!("Compilation failed: {:?}", expected_compilation_error).as_str())); } @@ -84,7 +85,7 @@ fn test_compile_contract_class_bad_sierra( assert_eq!(err, GatewaySpecError::CompilationFailed); let expected_compilation_error = - CompilationUtilError::CompilationError("Invalid Sierra program.".to_owned()); + CompilationUtilError::CompilationError("Error: Invalid Sierra program.\n".to_owned()); assert!(logs_contain(format!("Compilation failed: {:?}", expected_compilation_error).as_str())); } diff --git a/crates/gateway/src/gateway.rs b/crates/gateway/src/gateway.rs index 7a8ca450932..52a0bcd3ca4 100644 --- a/crates/gateway/src/gateway.rs +++ b/crates/gateway/src/gateway.rs @@ -163,7 +163,7 @@ pub fn create_gateway( mempool_client: SharedMempoolClient, ) -> Gateway { let state_reader_factory = Arc::new(RpcStateReaderFactory { config: rpc_state_reader_config }); - let gateway_compiler = GatewayCompiler::new_cairo_lang_compiler(compiler_config); + let gateway_compiler = GatewayCompiler::new_command_line_compiler(compiler_config); Gateway::new(config, state_reader_factory, gateway_compiler, mempool_client) } diff --git a/crates/gateway/src/gateway_test.rs b/crates/gateway/src/gateway_test.rs index 00158586095..46e09674173 100644 --- a/crates/gateway/src/gateway_test.rs +++ b/crates/gateway/src/gateway_test.rs @@ -34,7 +34,7 @@ pub fn app_state( stateful_tx_validator: Arc::new(StatefulTransactionValidator { config: StatefulTransactionValidatorConfig::create_for_testing(), }), - gateway_compiler: GatewayCompiler::new_cairo_lang_compiler( + gateway_compiler: GatewayCompiler::new_command_line_compiler( SierraToCasmCompilationConfig::default(), ), state_reader_factory: Arc::new(state_reader_factory), diff --git a/crates/tests-integration/build.rs b/crates/tests-integration/build.rs new file mode 100644 index 00000000000..2c3d009e87c --- /dev/null +++ b/crates/tests-integration/build.rs @@ -0,0 +1,3 @@ +// Sets up the environment variable OUT_DIR, which holds the cairo compiler binary. +// The binary is dowloaded to OUT_DIR by the starknet_sierra_compile crate. +fn main() {}