From 05c2770f393c51cfbe99e09e52e258482488784d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 13:01:34 -0300 Subject: [PATCH 1/7] Add TODO on old test --- src/transaction/invoke_function.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/transaction/invoke_function.rs b/src/transaction/invoke_function.rs index bb87af56f..ba40ecbb1 100644 --- a/src/transaction/invoke_function.rs +++ b/src/transaction/invoke_function.rs @@ -706,6 +706,7 @@ mod tests { #[test] // Test fee calculation is done correctly but payment to sequencer fails due to been WIP. + // TODO: update this test fn test_execute_invoke_fee_payment_to_sequencer_should_fail() { let internal_invoke_function = InvokeFunction { contract_address: Address(0.into()), @@ -761,10 +762,10 @@ mod tests { (String::from("range_check_builtin"), 70.into()), ]); - let expected_error = internal_invoke_function.execute(&mut state, &block_context, 0); + let result = internal_invoke_function.execute(&mut state, &block_context, 0); let error_msg = "Fee transfer failure".to_string(); - assert!(expected_error.is_err()); - assert_matches!(expected_error.unwrap_err(), TransactionError::FeeError(msg) if msg == error_msg); + assert!(result.is_err()); + assert_matches!(result.unwrap_err(), TransactionError::FeeError(msg) if msg == error_msg); } #[test] From c211d702ca939196caf7f592def9032f328be35e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 15:17:30 -0300 Subject: [PATCH 2/7] Fix: send U256 in calldata in little-endian order --- src/transaction/fee.rs | 4 ++-- tests/internals.rs | 51 +++++++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/transaction/fee.rs b/src/transaction/fee.rs index f06222903..3608793be 100644 --- a/src/transaction/fee.rs +++ b/src/transaction/fee.rs @@ -36,8 +36,8 @@ pub(crate) fn execute_fee_transfer( let calldata = [ block_context.block_info.sequencer_address.0.clone(), - 0.into(), - Felt252::from(actual_fee), + Felt252::from(actual_fee), // U256.low + 0.into(), // U256.high ] .to_vec(); diff --git a/tests/internals.rs b/tests/internals.rs index fb2d2bc1c..c0d56ad7c 100644 --- a/tests/internals.rs +++ b/tests/internals.rs @@ -11,7 +11,7 @@ use cairo_vm::vm::{ }; use lazy_static::lazy_static; use num_bigint::BigUint; -use num_traits::{Num, One, ToPrimitive, Zero}; +use num_traits::{FromPrimitive, Num, One, ToPrimitive, Zero}; use starknet_in_rust::core::errors::state_errors::StateError; use starknet_in_rust::definitions::constants::{ DEFAULT_CAIRO_RESOURCE_FEE_WEIGHTS, VALIDATE_ENTRY_POINT_SELECTOR, @@ -20,6 +20,7 @@ use starknet_in_rust::execution::execution_entry_point::ExecutionEntryPoint; use starknet_in_rust::execution::TransactionExecutionContext; use starknet_in_rust::services::api::contract_classes::deprecated_contract_class::ContractClass; use starknet_in_rust::state::ExecutionResourcesManager; +use starknet_in_rust::transaction::fee::calculate_tx_fee; use starknet_in_rust::transaction::{DeclareV2, Deploy}; use starknet_in_rust::CasmContractClass; use starknet_in_rust::EntryPointType; @@ -78,6 +79,7 @@ lazy_static! { static ref TEST_FIB_COMPILED_CONTRACT_CLASS_HASH: Felt252 = felt_str!("27727"); // Storage keys. + // NOTE: this key corresponds to the lower 128 bits of an U256 static ref TEST_ERC20_ACCOUNT_BALANCE_KEY: Felt252 = felt_str!("1192211877881866289306604115402199097887041303917861778777990838480655617515"); static ref TEST_ERC20_SEQUENCER_BALANCE_KEY: Felt252 = @@ -91,8 +93,8 @@ lazy_static! { felt_str!("2542253978940891427830343982984992363331567580652119103860970381451088310289"); // Others. - // Blockifier had this value hardcoded to 2. - static ref ACTUAL_FEE: Felt252 = Felt252::zero(); + static ref INITIAL_BALANCE: Felt252 = Felt252::from_u64(100000).unwrap(); + static ref GAS_PRICE: u128 = 1; } fn get_contract_class

(path: P) -> Result> @@ -107,7 +109,7 @@ pub fn new_starknet_block_context_for_testing() -> BlockContext { StarknetOsConfig::new( StarknetChainId::TestNet, TEST_ERC20_CONTRACT_ADDRESS.clone(), - 0, + *GAS_PRICE, ), 0, 0, @@ -161,7 +163,7 @@ fn create_account_tx_test_state( let storage_view = HashMap::from([( (test_erc20_address, test_erc20_account_balance_key), - ACTUAL_FEE.clone(), + INITIAL_BALANCE.clone(), )]); let cached_state = CachedState::new( @@ -369,7 +371,7 @@ fn initial_in_memory_state_reader() -> InMemoryStateReader { TEST_ERC20_CONTRACT_ADDRESS.clone(), felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY.clone()), ), - Felt252::from(0), + INITIAL_BALANCE.clone(), )]), HashMap::from([ ( @@ -528,7 +530,7 @@ fn test_create_account_tx_test_state() { felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY), )) .unwrap(); - assert_eq!(value, *ACTUAL_FEE); + assert_eq!(value, *INITIAL_BALANCE); let class_hash = state.get_class_hash_at(&TEST_CONTRACT_ADDRESS).unwrap(); assert_eq!(class_hash, felt_to_hash(&TEST_CLASS_HASH)); @@ -687,7 +689,7 @@ fn declare_tx() -> Declare { tx_type: TransactionType::Declare, validate_entry_point_selector: VALIDATE_DECLARE_ENTRY_POINT_SELECTOR.clone(), version: 1.into(), - max_fee: 2, + max_fee: 100000, signature: vec![], nonce: 0.into(), hash_value: 0.into(), @@ -737,7 +739,7 @@ fn deploy_fib_syscall() -> Deploy { } } -fn expected_declare_fee_transfer_info() -> CallInfo { +fn expected_declare_fee_transfer_info(fee: u128) -> CallInfo { CallInfo { caller_address: TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), call_type: Some(CallType::Call), @@ -747,7 +749,7 @@ fn expected_declare_fee_transfer_info() -> CallInfo { entry_point_type: Some(EntryPointType::External), calldata: vec![ TEST_SEQUENCER_ADDRESS.0.clone(), - Felt252::zero(), + Felt252::from(fee), Felt252::zero(), ], retdata: vec![1.into()], @@ -759,12 +761,12 @@ fn expected_declare_fee_transfer_info() -> CallInfo { vec![ TEST_ACCOUNT_CONTRACT_ADDRESS.clone().0, TEST_SEQUENCER_ADDRESS.clone().0, - 0.into(), + Felt252::from(fee), 0.into(), ], )], storage_read_values: vec![ - Felt252::zero(), + INITIAL_BALANCE.clone(), Felt252::zero(), Felt252::zero(), Felt252::zero(), @@ -811,6 +813,13 @@ fn test_declare_tx() { // Check ContractClass is set after the declare_tx assert!(state.get_contract_class(&declare_tx.class_hash).is_ok()); + let resources = HashMap::from([ + ("range_check_builtin".to_string(), 57), + ("pedersen_builtin".to_string(), 15), + ("l1_gas_usage".to_string(), 0), + ]); + let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap(); + let expected_execution_info = TransactionExecutionInfo::new( Some(CallInfo { call_type: Some(CallType::Call), @@ -826,13 +835,9 @@ fn test_declare_tx() { ..Default::default() }), None, - Some(expected_declare_fee_transfer_info()), - 0, - HashMap::from([ - ("range_check_builtin".to_string(), 57), - ("pedersen_builtin".to_string(), 15), - ("l1_gas_usage".to_string(), 0), - ]), + Some(expected_declare_fee_transfer_info(fee)), + fee, + resources, Some(TransactionType::Declare), ); @@ -870,7 +875,7 @@ fn test_declarev2_tx() { ..Default::default() }), None, - Some(expected_declare_fee_transfer_info()), + Some(expected_declare_fee_transfer_info(0)), 0, HashMap::from([ ("range_check_builtin".to_string(), 57), @@ -1185,7 +1190,7 @@ fn test_deploy_account() { .clone(), TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY.to_be_bytes(), ), - ACTUAL_FEE.clone(), + INITIAL_BALANCE.clone(), ); let (state_before, state_after) = expected_deploy_account_states(); @@ -1231,14 +1236,14 @@ fn test_deploy_account() { let expected_fee_transfer_call_info = expected_fee_transfer_call_info( &block_context, deploy_account_tx.contract_address(), - ACTUAL_FEE.to_u64().unwrap(), + INITIAL_BALANCE.to_u64().unwrap(), ); let expected_execution_info = TransactionExecutionInfo::new( expected_validate_call_info.into(), expected_execute_call_info.into(), expected_fee_transfer_call_info.into(), - ACTUAL_FEE.to_u128().unwrap(), + INITIAL_BALANCE.to_u128().unwrap(), // Entry **not** in blockifier. // Default::default(), [ From 58994e678b129f538cd83bbede7bc437b1263063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 16:06:45 -0300 Subject: [PATCH 3/7] Return actual fee when exceeds max fee --- src/transaction/error.rs | 2 ++ src/transaction/fee.rs | 5 +++-- src/transaction/invoke_function.rs | 10 ++++++---- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/transaction/error.rs b/src/transaction/error.rs index c330e3878..4047b535b 100644 --- a/src/transaction/error.rs +++ b/src/transaction/error.rs @@ -33,6 +33,8 @@ pub enum TransactionError { InvokeFunctionZeroHasNonce, #[error("Invalid transaction nonce. Expected: {0} got {1}")] InvalidTransactionNonce(String, String), + #[error("Actual fee exceeds max fee. Actual: {0}, Max: {1}")] + ActualFeeExceedsMaxFee(u128, u128), #[error("{0}")] FeeError(String), #[error("Cairo resource names must be contained in fee weights dict")] diff --git a/src/transaction/fee.rs b/src/transaction/fee.rs index 3608793be..19aa52f3a 100644 --- a/src/transaction/fee.rs +++ b/src/transaction/fee.rs @@ -27,8 +27,9 @@ pub(crate) fn execute_fee_transfer( actual_fee: u128, ) -> Result { if actual_fee > tx_execution_context.max_fee { - return Err(TransactionError::FeeError( - "Actual fee exceeded max fee.".to_string(), + return Err(TransactionError::ActualFeeExceedsMaxFee( + actual_fee, + tx_execution_context.max_fee, )); } diff --git a/src/transaction/invoke_function.rs b/src/transaction/invoke_function.rs index ba40ecbb1..b0ea00ada 100644 --- a/src/transaction/invoke_function.rs +++ b/src/transaction/invoke_function.rs @@ -825,10 +825,12 @@ mod tests { ]); block_context.starknet_os_config.gas_price = 1; - let expected_error = internal_invoke_function.execute(&mut state, &block_context, 0); - let error_msg = "Actual fee exceeded max fee.".to_string(); - assert!(expected_error.is_err()); - assert_matches!(expected_error.unwrap_err(), TransactionError::FeeError(actual_error_msg) if actual_error_msg == error_msg); + let error = internal_invoke_function.execute(&mut state, &block_context, 0); + assert!(error.is_err()); + assert_matches!( + error.unwrap_err(), + TransactionError::ActualFeeExceedsMaxFee(_, _) + ); } #[test] From 2143db8b726f7f8350757d8d2e57cb4040c7212b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 16:39:32 -0300 Subject: [PATCH 4/7] Add `FeeTransferError` for fee transfer failures --- src/transaction/declare.rs | 2 +- src/transaction/error.rs | 2 ++ src/transaction/fee.rs | 2 +- src/transaction/invoke_function.rs | 3 +-- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/transaction/declare.rs b/src/transaction/declare.rs index 00bbf2a91..2f5899311 100644 --- a/src/transaction/declare.rs +++ b/src/transaction/declare.rs @@ -870,7 +870,7 @@ mod tests { // We expect a fee transfer failure because the fee token contract is not set up assert_matches!( internal_declare.execute(&mut state, &BlockContext::default()), - Err(TransactionError::FeeError(e)) if e == "Fee transfer failure" + Err(TransactionError::FeeTransferError(_)) ); } } diff --git a/src/transaction/error.rs b/src/transaction/error.rs index 4047b535b..1c32f90ee 100644 --- a/src/transaction/error.rs +++ b/src/transaction/error.rs @@ -35,6 +35,8 @@ pub enum TransactionError { InvalidTransactionNonce(String, String), #[error("Actual fee exceeds max fee. Actual: {0}, Max: {1}")] ActualFeeExceedsMaxFee(u128, u128), + #[error("Fee transfer failure: {0}")] + FeeTransferError(Box), #[error("{0}")] FeeError(String), #[error("Cairo resource names must be contained in fee weights dict")] diff --git a/src/transaction/fee.rs b/src/transaction/fee.rs index 19aa52f3a..780bcde08 100644 --- a/src/transaction/fee.rs +++ b/src/transaction/fee.rs @@ -62,7 +62,7 @@ pub(crate) fn execute_fee_transfer( false, ); // TODO: Avoid masking the error from the fee transfer. - fee_transfer_exec.map_err(|_| TransactionError::FeeError("Fee transfer failure".to_string())) + fee_transfer_exec.map_err(|e| TransactionError::FeeTransferError(Box::new(e))) } // ---------------------------------------------------------------------------------------- diff --git a/src/transaction/invoke_function.rs b/src/transaction/invoke_function.rs index b0ea00ada..c61c22d98 100644 --- a/src/transaction/invoke_function.rs +++ b/src/transaction/invoke_function.rs @@ -763,9 +763,8 @@ mod tests { ]); let result = internal_invoke_function.execute(&mut state, &block_context, 0); - let error_msg = "Fee transfer failure".to_string(); assert!(result.is_err()); - assert_matches!(result.unwrap_err(), TransactionError::FeeError(msg) if msg == error_msg); + assert_matches!(result.unwrap_err(), TransactionError::FeeTransferError(_)); } #[test] From d6ef3147e1482a7099795dfa87f27bf8ff86d14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 17:08:59 -0300 Subject: [PATCH 5/7] Fix: add gas usage to tests --- tests/internals.rs | 282 ++++++++++++++++++++++----------------------- 1 file changed, 139 insertions(+), 143 deletions(-) diff --git a/tests/internals.rs b/tests/internals.rs index c0d56ad7c..533dadac5 100644 --- a/tests/internals.rs +++ b/tests/internals.rs @@ -11,7 +11,7 @@ use cairo_vm::vm::{ }; use lazy_static::lazy_static; use num_bigint::BigUint; -use num_traits::{FromPrimitive, Num, One, ToPrimitive, Zero}; +use num_traits::{FromPrimitive, Num, One, Zero}; use starknet_in_rust::core::errors::state_errors::StateError; use starknet_in_rust::definitions::constants::{ DEFAULT_CAIRO_RESOURCE_FEE_WEIGHTS, VALIDATE_ENTRY_POINT_SELECTOR, @@ -93,7 +93,7 @@ lazy_static! { felt_str!("2542253978940891427830343982984992363331567580652119103860970381451088310289"); // Others. - static ref INITIAL_BALANCE: Felt252 = Felt252::from_u64(100000).unwrap(); + static ref INITIAL_BALANCE: Felt252 = Felt252::from_u128(100000).unwrap(); static ref GAS_PRICE: u128 = 1; } @@ -217,7 +217,7 @@ fn expected_state_before_tx() -> CachedState { ) } -fn expected_state_after_tx() -> CachedState { +fn expected_state_after_tx(fee: u128) -> CachedState { let in_memory_state_reader = initial_in_memory_state_reader(); let contract_classes_cache = ContractClassCache::from([ @@ -238,12 +238,12 @@ fn expected_state_after_tx() -> CachedState { CachedState::new_for_testing( in_memory_state_reader, Some(contract_classes_cache), - state_cache_after_invoke_tx(), + state_cache_after_invoke_tx(fee), Some(HashMap::new()), ) } -fn state_cache_after_invoke_tx() -> StateCache { +fn state_cache_after_invoke_tx(fee: u128) -> StateCache { let class_hash_initial_values = HashMap::from([ ( TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), @@ -275,7 +275,7 @@ fn state_cache_after_invoke_tx() -> StateCache { TEST_ERC20_CONTRACT_ADDRESS.clone(), felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY.clone()), ), - Felt252::zero(), + INITIAL_BALANCE.clone(), ), ( ( @@ -303,14 +303,14 @@ fn state_cache_after_invoke_tx() -> StateCache { TEST_ERC20_CONTRACT_ADDRESS.clone(), felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY.clone()), ), - Felt252::from(0), + Felt252::from(fee), ), ( ( TEST_ERC20_CONTRACT_ADDRESS.clone(), felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY.clone()), ), - Felt252::from(0), + INITIAL_BALANCE.clone() - Felt252::from(fee), ), ( ( @@ -467,7 +467,7 @@ fn expected_fee_transfer_call_info( ], ]), storage_read_values: vec![ - Felt252::zero(), + INITIAL_BALANCE.clone(), Felt252::zero(), Felt252::zero(), Felt252::zero(), @@ -487,8 +487,8 @@ fn expected_fee_transfer_call_info( fn validate_final_balances( state: &mut S, block_context: &BlockContext, - expected_sequencer_balance: Felt252, erc20_account_balance_storage_key: &ClassHash, + fee: u128, ) where S: State + StateReader, { @@ -501,7 +501,10 @@ fn validate_final_balances( *erc20_account_balance_storage_key, )) .unwrap(); - assert_eq!(account_balance, Felt252::zero()); + assert_eq!( + account_balance, + INITIAL_BALANCE.clone() - Felt252::from(fee) + ); let sequencer_balance = state .get_storage_at(&( @@ -512,7 +515,7 @@ fn validate_final_balances( felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY), )) .unwrap(); - assert_eq!(sequencer_balance, expected_sequencer_balance); + assert_eq!(sequencer_balance, fee.into()); } #[test] @@ -550,7 +553,7 @@ fn invoke_tx(calldata: Vec) -> InvokeFunction { InvokeFunction::new( TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), EXECUTE_ENTRY_POINT_SELECTOR.clone(), - 2, + 5000, TRANSACTION_VERSION.clone(), calldata, vec![], @@ -561,7 +564,7 @@ fn invoke_tx(calldata: Vec) -> InvokeFunction { .unwrap() } -fn expected_fee_transfer_info() -> CallInfo { +fn expected_fee_transfer_info(fee: u128) -> CallInfo { CallInfo { failure_flag: false, gas_consumed: 0, @@ -572,7 +575,7 @@ fn expected_fee_transfer_info() -> CallInfo { class_hash: Some(felt_to_hash(&TEST_ERC20_CONTRACT_CLASS_HASH)), entry_point_selector: Some(TRANSFER_ENTRY_POINT_SELECTOR.clone()), entry_point_type: Some(EntryPointType::External), - calldata: vec![Felt252::from(4096), Felt252::zero(), Felt252::zero()], + calldata: vec![Felt252::from(4096), Felt252::from(fee), Felt252::zero()], retdata: vec![Felt252::from(1)], execution_resources: ExecutionResources { n_steps: 525, @@ -590,12 +593,12 @@ fn expected_fee_transfer_info() -> CallInfo { data: vec![ Felt252::from(257), Felt252::from(4096), - Felt252::zero(), + Felt252::from(fee), Felt252::zero(), ], }], storage_read_values: vec![ - Felt252::zero(), + INITIAL_BALANCE.clone(), Felt252::zero(), Felt252::zero(), Felt252::zero(), @@ -621,7 +624,7 @@ fn expected_fee_transfer_info() -> CallInfo { } } -fn expected_fib_fee_transfer_info() -> CallInfo { +fn expected_fib_fee_transfer_info(fee: u128) -> CallInfo { CallInfo { failure_flag: false, gas_consumed: 0, @@ -632,7 +635,7 @@ fn expected_fib_fee_transfer_info() -> CallInfo { class_hash: Some(felt_to_hash(&TEST_ERC20_CONTRACT_CLASS_HASH)), entry_point_selector: Some(TRANSFER_ENTRY_POINT_SELECTOR.clone()), entry_point_type: Some(EntryPointType::External), - calldata: vec![Felt252::from(4096), Felt252::zero(), Felt252::zero()], + calldata: vec![Felt252::from(4096), Felt252::from(fee), Felt252::zero()], retdata: vec![Felt252::from(1)], execution_resources: ExecutionResources { n_steps: 525, @@ -650,14 +653,14 @@ fn expected_fib_fee_transfer_info() -> CallInfo { data: vec![ Felt252::from(257), Felt252::from(4096), - Felt252::zero(), + Felt252::from(fee), Felt252::zero(), ], }], storage_read_values: vec![ + INITIAL_BALANCE.clone() - Felt252::from(10), Felt252::zero(), - Felt252::zero(), - Felt252::zero(), + Felt252::from(10), Felt252::zero(), ], accessed_storage_keys: HashSet::from([ @@ -711,7 +714,7 @@ fn declarev2_tx() -> DeclareV2 { tx_type: TransactionType::Declare, validate_entry_point_selector: VALIDATE_DECLARE_ENTRY_POINT_SELECTOR.clone(), version: 1.into(), - max_fee: 2, + max_fee: 5000, signature: vec![], nonce: 0.into(), hash_value: 0.into(), @@ -860,6 +863,13 @@ fn test_declarev2_tx() { .get_contract_class(&declare_tx.compiled_class_hash.to_be_bytes()) .is_ok()); + let resources = HashMap::from([ + ("range_check_builtin".to_string(), 57), + ("pedersen_builtin".to_string(), 15), + ("l1_gas_usage".to_string(), 0), + ]); + let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap(); + let expected_execution_info = TransactionExecutionInfo::new( Some(CallInfo { call_type: Some(CallType::Call), @@ -875,13 +885,9 @@ fn test_declarev2_tx() { ..Default::default() }), None, - Some(expected_declare_fee_transfer_info(0)), - 0, - HashMap::from([ - ("range_check_builtin".to_string(), 57), - ("pedersen_builtin".to_string(), 15), - ("l1_gas_usage".to_string(), 0), - ]), + Some(expected_declare_fee_transfer_info(fee)), + fee, + resources, Some(TransactionType::Declare), ); @@ -1055,39 +1061,45 @@ fn expected_fib_validate_call_info_2() -> CallInfo { } } -fn expected_transaction_execution_info() -> TransactionExecutionInfo { +fn expected_transaction_execution_info(block_context: &BlockContext) -> TransactionExecutionInfo { + let resources = HashMap::from([ + ("pedersen_builtin".to_string(), 16), + ("l1_gas_usage".to_string(), 0), + ("range_check_builtin".to_string(), 72), + ]); + let fee = calculate_tx_fee(&resources, *GAS_PRICE, block_context).unwrap(); TransactionExecutionInfo::new( Some(expected_validate_call_info_2()), Some(expected_execute_call_info()), - Some(expected_fee_transfer_info()), - 0, - HashMap::from([ - ("pedersen_builtin".to_string(), 16), - ("l1_gas_usage".to_string(), 0), - ("range_check_builtin".to_string(), 72), - ]), + Some(expected_fee_transfer_info(fee)), + fee, + resources, Some(TransactionType::InvokeFunction), ) } -fn expected_fib_transaction_execution_info() -> TransactionExecutionInfo { +fn expected_fib_transaction_execution_info( + block_context: &BlockContext, +) -> TransactionExecutionInfo { + let resources = HashMap::from([ + ("l1_gas_usage".to_string(), 4896), + ("pedersen_builtin".to_string(), 16), + ("range_check_builtin".to_string(), 75), + ]); + let fee = calculate_tx_fee(&resources, *GAS_PRICE, block_context).unwrap(); TransactionExecutionInfo::new( Some(expected_fib_validate_call_info_2()), Some(expected_fib_execute_call_info()), - Some(expected_fib_fee_transfer_info()), - 0, - HashMap::from([ - ("pedersen_builtin".to_string(), 16), - ("l1_gas_usage".to_string(), 0), - ("range_check_builtin".to_string(), 75), - ]), + Some(expected_fib_fee_transfer_info(fee)), + fee, + resources, Some(TransactionType::InvokeFunction), ) } #[test] fn test_invoke_tx() { - let (starknet_general_context, state) = &mut create_account_tx_test_state().unwrap(); + let (block_context, state) = &mut create_account_tx_test_state().unwrap(); let Address(test_contract_address) = TEST_CONTRACT_ADDRESS.clone(); let calldata = vec![ test_contract_address, // CONTRACT_ADDRESS @@ -1099,10 +1111,8 @@ fn test_invoke_tx() { // Extract invoke transaction fields for testing, as it is consumed when creating an account // transaction. - let result = invoke_tx - .execute(state, starknet_general_context, 0) - .unwrap(); - let expected_execution_info = expected_transaction_execution_info(); + let result = invoke_tx.execute(state, block_context, 0).unwrap(); + let expected_execution_info = expected_transaction_execution_info(block_context); assert_eq!(result, expected_execution_info); } @@ -1122,11 +1132,11 @@ fn test_invoke_tx_state() { ]; let invoke_tx = invoke_tx(calldata); - invoke_tx + let result = invoke_tx .execute(state, starknet_general_context, 0) .unwrap(); - let expected_final_state = expected_state_after_tx(); + let expected_final_state = expected_state_after_tx(result.actual_fee); assert_eq!(*state, expected_final_state); } @@ -1156,12 +1166,12 @@ fn test_invoke_with_declarev2_tx() { ]; let invoke_tx = invoke_tx(calldata); - let expected_gas_consumed = 4380; + let expected_gas_consumed = 4908; let result = invoke_tx .execute(state, block_context, expected_gas_consumed) .unwrap(); - let expected_execution_info = expected_fib_transaction_execution_info(); + let expected_execution_info = expected_fib_transaction_execution_info(block_context); assert_eq!(result, expected_execution_info); } @@ -1169,9 +1179,11 @@ fn test_invoke_with_declarev2_tx() { fn test_deploy_account() { let (block_context, mut state) = create_account_tx_test_state().unwrap(); + let expected_fee = 3684; + let deploy_account_tx = DeployAccount::new( felt_to_hash(&TEST_ACCOUNT_CONTRACT_CLASS_HASH), - 2, + expected_fee, TRANSACTION_VERSION.clone(), Default::default(), Default::default(), @@ -1188,7 +1200,7 @@ fn test_deploy_account() { .starknet_os_config() .fee_token_address() .clone(), - TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY.to_be_bytes(), + felt_to_hash(&TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY), ), INITIAL_BALANCE.clone(), ); @@ -1236,24 +1248,27 @@ fn test_deploy_account() { let expected_fee_transfer_call_info = expected_fee_transfer_call_info( &block_context, deploy_account_tx.contract_address(), - INITIAL_BALANCE.to_u64().unwrap(), + expected_fee as u64, ); + let resources = HashMap::from([ + ("range_check_builtin".to_string(), 74), + ("pedersen_builtin".to_string(), 23), + ("l1_gas_usage".to_string(), 3672), + ]); + + let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap(); + + assert_eq!(fee, expected_fee); + let expected_execution_info = TransactionExecutionInfo::new( expected_validate_call_info.into(), expected_execute_call_info.into(), expected_fee_transfer_call_info.into(), - INITIAL_BALANCE.to_u128().unwrap(), + fee, // Entry **not** in blockifier. // Default::default(), - [ - ("l1_gas_usage", 3672), - ("range_check_builtin", 74), - ("pedersen_builtin", 23), - ] - .into_iter() - .map(|(k, v)| (k.to_string(), v)) - .collect(), + resources, TransactionType::DeployAccount.into(), ); assert_eq!(tx_info, expected_execution_info); @@ -1265,7 +1280,7 @@ fn test_deploy_account() { let hash = TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY.to_be_bytes(); - validate_final_balances(&mut state, &block_context, Felt252::zero(), &hash); + validate_final_balances(&mut state, &block_context, &hash, fee); let class_hash_from_state = state .get_class_hash_at(deploy_account_tx.contract_address()) @@ -1277,61 +1292,52 @@ fn expected_deploy_account_states() -> ( CachedState, CachedState, ) { + let fee = Felt252::from(3684); let mut state_before = CachedState::new( InMemoryStateReader::new( HashMap::from([ + (Address(0x101.into()), felt_to_hash(&0x111.into())), + (Address(0x100.into()), felt_to_hash(&0x110.into())), + (Address(0x1001.into()), felt_to_hash(&0x1010.into())), + ]), + HashMap::from([ + (Address(0x101.into()), Default::default()), + (Address(0x100.into()), Default::default()), + (Address(0x1001.into()), Default::default()), + ]), + HashMap::from([( ( - Address(0x101.into()), - felt_to_hash(&0x111.into()), + Address(0x1001.into()), + felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY), ), + INITIAL_BALANCE.clone(), + )]), + HashMap::from([ ( - Address(0x100.into()), felt_to_hash(&0x110.into()), + ContractClass::try_from(PathBuf::from(TEST_CONTRACT_PATH)).unwrap(), + ), + ( + felt_to_hash(&0x111.into()), + ContractClass::try_from(PathBuf::from(ACCOUNT_CONTRACT_PATH)).unwrap(), ), ( - Address(0x1001.into()), felt_to_hash(&0x1010.into()), - )]), - - HashMap::from([ - ( - Address(0x101.into()), - Default::default(), - ), - ( - Address(0x100.into()), - Default::default(), - ), - ( - Address(0x1001.into()), - Default::default(), - )]), - HashMap::from([ - ( - (Address(0x1001.into()), - felt_to_hash(&felt_str!("1192211877881866289306604115402199097887041303917861778777990838480655617515"))), - Felt252::zero(), - ), - ]), - HashMap::from([ - (felt_to_hash(&0x110.into()), ContractClass::try_from(PathBuf::from(TEST_CONTRACT_PATH)).unwrap()), - (felt_to_hash(&0x111.into()), ContractClass::try_from(PathBuf::from(ACCOUNT_CONTRACT_PATH)).unwrap()), - (felt_to_hash(&0x1010.into()), ContractClass::try_from(PathBuf::from(ERC20_CONTRACT_PATH)).unwrap()), + ContractClass::try_from(PathBuf::from(ERC20_CONTRACT_PATH)).unwrap(), + ), ]), HashMap::new(), - HashMap::new() + HashMap::new(), ), Some(ContractClassCache::new()), - Some(HashMap::new()) + Some(HashMap::new()), ); state_before.set_storage_at( &( Address(0x1001.into()), - felt_to_hash(&felt_str!( - "2542253978940891427830343982984992363331567580652119103860970381451088310289" - )), + felt_to_hash(&TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY), ), - 0.into(), + INITIAL_BALANCE.clone(), ); let mut state_after = state_before.clone(); @@ -1373,9 +1379,7 @@ fn expected_deploy_account_states() -> ( state_after.cache_mut().storage_initial_values_mut().insert( ( Address(0x1001.into()), - felt_to_hash(&felt_str!( - "3229073099929281304021185011369329892856197542079132996799046100564060768274" - )), + felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY), ), Felt252::zero(), ); @@ -1403,11 +1407,9 @@ fn expected_deploy_account_states() -> ( state_after.cache_mut().storage_writes_mut().insert( ( Address(0x1001.into()), - felt_to_hash(&felt_str!( - "2542253978940891427830343982984992363331567580652119103860970381451088310289" - )), + felt_to_hash(&TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY), ), - Felt252::zero(), + INITIAL_BALANCE.clone() - &fee, ); state_after.cache_mut().storage_writes_mut().insert( ( @@ -1419,11 +1421,9 @@ fn expected_deploy_account_states() -> ( state_after.cache_mut().storage_writes_mut().insert( ( Address(0x1001.into()), - felt_to_hash(&felt_str!( - "3229073099929281304021185011369329892856197542079132996799046100564060768274" - )), + felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY), ), - Felt252::zero(), + fee, ); state_after .set_contract_class( @@ -1496,7 +1496,7 @@ fn test_state_for_declare_tx() { TEST_ERC20_CONTRACT_ADDRESS.clone(), felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY) ), - Felt252::zero() + INITIAL_BALANCE.clone() ),]), ); @@ -1518,6 +1518,8 @@ fn test_state_for_declare_tx() { ]) ); + let fee = Felt252::from(10); + // Check state.cache assert_eq!( state.cache(), @@ -1533,74 +1535,68 @@ fn test_state_for_declare_tx() { ) ]), HashMap::new(), - HashMap::from([( - TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), - 0.into() - )]), + HashMap::from([(TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), 0.into())]), HashMap::from([ ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&felt_str!("3229073099929281304021185011369329892856197542079132996799046100564060768275")) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_BALANCE_KEY_2) ), 0.into() ), ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&felt_str!("1192211877881866289306604115402199097887041303917861778777990838480655617516")) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_BALANCE_KEY_1) ), 0.into() ), ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY) ), 0.into() ), ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY) ), - 0.into() + INITIAL_BALANCE.clone() ) ]), HashMap::new(), HashMap::new(), - HashMap::from([( - TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), - 1.into() - )]), + HashMap::from([(TEST_ACCOUNT_CONTRACT_ADDRESS.clone(), 1.into())]), HashMap::from([ ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&felt_str!("3229073099929281304021185011369329892856197542079132996799046100564060768275")) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_BALANCE_KEY_2) ), 0.into() ), ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&felt_str!("1192211877881866289306604115402199097887041303917861778777990838480655617516")) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_BALANCE_KEY_1) ), 0.into() ), ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_SEQUENCER_BALANCE_KEY) ), - 0.into() //Fee, 2 in blockifier + fee.clone(), ), ( ( - TEST_ERC20_CONTRACT_ADDRESS.clone(), - felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY) + TEST_ERC20_CONTRACT_ADDRESS.clone(), + felt_to_hash(&TEST_ERC20_ACCOUNT_BALANCE_KEY) ), - 0.into() + INITIAL_BALANCE.clone() - &fee, ), ]), HashMap::new() From 8f3daf9f856db30b6a2b2a8748c4a08531d37d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 17:31:34 -0300 Subject: [PATCH 6/7] Comment old test and fix error message --- src/transaction/invoke_function.rs | 43 +++++++++++++++--------------- src/utils.rs | 4 ++- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/transaction/invoke_function.rs b/src/transaction/invoke_function.rs index c61c22d98..36a96468b 100644 --- a/src/transaction/invoke_function.rs +++ b/src/transaction/invoke_function.rs @@ -705,11 +705,29 @@ mod tests { } #[test] - // Test fee calculation is done correctly but payment to sequencer fails due to been WIP. - // TODO: update this test - fn test_execute_invoke_fee_payment_to_sequencer_should_fail() { + // Test fee calculation is done correctly but payment to sequencer fails due + // to the token contract not being deployed + fn test_invoke_with_non_deployed_fee_token_should_fail() { + let contract_address = Address(0.into()); + + // Instantiate CachedState + let mut state_reader = InMemoryStateReader::default(); + // Set contract_class + let class_hash = [1; 32]; + let contract_class = + ContractClass::try_from(PathBuf::from("starknet_programs/fibonacci.json")).unwrap(); + // Set contact_state + let nonce = Felt252::zero(); + + state_reader + .address_to_class_hash_mut() + .insert(contract_address.clone(), class_hash); + state_reader + .address_to_nonce + .insert(contract_address.clone(), nonce); + let internal_invoke_function = InvokeFunction { - contract_address: Address(0.into()), + contract_address, entry_point_selector: Felt252::from_str_radix( "112e35f48499939272000bd72eb840e502ca4c3aefa8800992e8defb746e0c9", 16, @@ -729,23 +747,6 @@ mod tests { skip_fee_transfer: false, }; - // Instantiate CachedState - let mut state_reader = InMemoryStateReader::default(); - // Set contract_class - let class_hash = [1; 32]; - let contract_class = - ContractClass::try_from(PathBuf::from("starknet_programs/fibonacci.json")).unwrap(); - // Set contact_state - let contract_address = Address(0.into()); - let nonce = Felt252::zero(); - - state_reader - .address_to_class_hash_mut() - .insert(contract_address.clone(), class_hash); - state_reader - .address_to_nonce - .insert(contract_address, nonce); - let mut state = CachedState::new(state_reader.clone(), None, None); // Initialize state.contract_classes diff --git a/src/utils.rs b/src/utils.rs index e87c638fb..ba69dffeb 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -269,7 +269,9 @@ pub fn get_deployed_address_class_hash_at_address( .to_owned(); if class_hash == *UNINITIALIZED_CLASS_HASH { - return Err(TransactionError::NotDeployedContract(class_hash)); + return Err(TransactionError::NotDeployedContract(felt_to_hash( + &contract_address.0, + ))); } Ok(class_hash) } From 04985fa24c6a1947efefffe8d11c3facd91a3d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 6 Jul 2023 18:28:57 -0300 Subject: [PATCH 7/7] Fix test --- tests/syscalls_errors.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/syscalls_errors.rs b/tests/syscalls_errors.rs index cda562fb2..e708021d1 100644 --- a/tests/syscalls_errors.rs +++ b/tests/syscalls_errors.rs @@ -1,6 +1,7 @@ #![deny(warnings)] use cairo_vm::felt::Felt252; +use starknet_in_rust::utils::felt_to_hash; use starknet_in_rust::EntryPointType; use starknet_in_rust::{ core::errors::state_errors::StateError, @@ -151,6 +152,11 @@ fn call_contract_with_extra_arguments() { #[test] fn call_contract_not_deployed() { let contract_address = Address(2222.into()); + let wrong_address = contract_address.0.clone() - Felt252::new(2); // another address + let error_msg = format!( + "Contract address {:?} is not deployed", + felt_to_hash(&wrong_address) + ); test_contract( "starknet_programs/syscalls.json", "test_call_contract", @@ -162,11 +168,11 @@ fn call_contract_not_deployed() { [( [2u8; 32], Path::new("starknet_programs/syscalls-lib.json"), - Some((contract_address.clone(), vec![("lib_state", 10.into())])), + Some((contract_address, vec![("lib_state", 10.into())])), )] .into_iter(), - [contract_address.0 - Felt252::new(2)], // Wrong address - "Contract address [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] is not deployed", + [wrong_address], + &error_msg, ); }