From 70fe33ac01f0cc2b869a03342b210fce196a5ba2 Mon Sep 17 00:00:00 2001 From: Meshi Peled <141231558+meship-starkware@users.noreply.github.com> Date: Wed, 10 Jul 2024 10:52:20 +0300 Subject: [PATCH 1/5] fix(concurrency): fix test_concurrent_fee_transfer_when_sender_is_sequencer to run concurrency mode (#2051) --- .../blockifier/src/transaction/account_transactions_test.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 427791943d..cf252e8b72 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -1332,10 +1332,10 @@ fn test_concurrent_fee_transfer_when_sender_is_sequencer( let fee_token_address = block_context.chain_info.fee_token_address(fee_type); let mut transactional_state = TransactionalState::create_transactional(state); - let charge_fee = true; - let validate = true; + let execution_flags = + ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true }; let result = - account_tx.execute(&mut transactional_state, &block_context, charge_fee, validate).unwrap(); + account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap(); assert!(!result.is_reverted()); // Check that the sequencer balance was updated (in this case, was not changed). for (seq_key, seq_value) in From f8dec817bd3b9818661720b6fa3cdc518e67aeb1 Mon Sep 17 00:00:00 2001 From: Meshi Peled <141231558+meship-starkware@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:33:32 +0300 Subject: [PATCH 2/5] fix(transaction): change deploy_account deafult version to be V3 (#2064) --- .../src/concurrency/versioned_state_test.rs | 21 +++++++++---------- .../src/test_utils/deploy_account.rs | 3 +-- .../transaction/account_transactions_test.rs | 9 ++++---- .../src/transaction/transactions_test.rs | 19 +++++++++++------ 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index d50fac2f48..ab99698b4e 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -7,7 +7,7 @@ use rstest::{fixture, rstest}; use starknet_api::core::{ calculate_contract_address, ClassHash, ContractAddress, Nonce, PatriciaKey, }; -use starknet_api::transaction::{Calldata, ContractAddressSalt, Fee, TransactionVersion}; +use starknet_api::transaction::{Calldata, ContractAddressSalt, ResourceBoundsMapping}; use starknet_api::{calldata, class_hash, contract_address, felt, patricia_key}; use crate::abi::abi_utils::{get_fee_token_var_address, get_storage_var_address}; @@ -28,10 +28,10 @@ use crate::test_utils::contracts::FeatureContract; use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; -use crate::test_utils::{CairoVersion, NonceManager, BALANCE, DEFAULT_STRK_L1_GAS_PRICE, MAX_FEE}; +use crate::test_utils::{CairoVersion, NonceManager, BALANCE, DEFAULT_STRK_L1_GAS_PRICE}; use crate::transaction::account_transaction::AccountTransaction; -use crate::transaction::objects::{FeeType, TransactionInfoCreator}; -use crate::transaction::test_utils::l1_resource_bounds; +use crate::transaction::objects::{HasRelatedFeeType, TransactionInfoCreator}; +use crate::transaction::test_utils::{l1_resource_bounds, max_resource_bounds}; use crate::transaction::transactions::ExecutableTransaction; use crate::{compiled_class_hash, deploy_account_tx_args, nonce, storage_key}; @@ -188,9 +188,9 @@ fn test_versioned_state_proxy() { ); } -#[test] +#[rstest] // Test parallel execution of two transactions that use the same versioned state. -fn test_run_parallel_txs() { +fn test_run_parallel_txs(max_resource_bounds: ResourceBoundsMapping) { let block_context = BlockContext::create_for_account_testing(); let chain_info = &block_context.chain_info; let zero_bounds = true; @@ -217,9 +217,7 @@ fn test_run_parallel_txs() { let deploy_account_tx_1 = deploy_account_tx( deploy_account_tx_args! { class_hash: account_without_validation.get_class_hash(), - max_fee: Fee(u128::from(!zero_bounds)), resource_bounds: l1_resource_bounds(u64::from(!zero_bounds), DEFAULT_STRK_L1_GAS_PRICE), - version: TransactionVersion::ONE, }, &mut NonceManager::default(), ); @@ -232,16 +230,18 @@ fn test_run_parallel_txs() { let constructor_calldata = calldata![ctor_grind_arg, ctor_storage_arg]; let deploy_tx_args = deploy_account_tx_args! { class_hash, - max_fee: Fee(MAX_FEE), + resource_bounds: max_resource_bounds, constructor_calldata: constructor_calldata.clone(), }; let nonce_manager = &mut NonceManager::default(); let deploy_account_tx_2 = deploy_account_tx(deploy_tx_args, nonce_manager); let account_address = deploy_account_tx_2.contract_address; let account_tx_2 = AccountTransaction::DeployAccount(deploy_account_tx_2); + let tx_context = block_context.to_tx_context(&account_tx_2); + let fee_type = tx_context.tx_info.fee_type(); let deployed_account_balance_key = get_fee_token_var_address(account_address); - let fee_token_address = chain_info.fee_token_address(&FeeType::Eth); + let fee_token_address = chain_info.fee_token_address(&fee_type); state_2 .set_storage_at(fee_token_address, deployed_account_balance_key, felt!(BALANCE)) .unwrap(); @@ -256,7 +256,6 @@ fn test_run_parallel_txs() { }); s.spawn(move || { account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap(); - // Check that the constructor wrote ctor_arg to the storage. let storage_key = get_storage_var_address("ctor_arg", &[]); let deployed_contract_address = calculate_contract_address( diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index d6ce95195d..9e13f00474 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/crates/blockifier/src/test_utils/deploy_account.rs @@ -32,8 +32,7 @@ impl Default for DeployAccountTxArgs { max_fee: Fee::default(), signature: TransactionSignature::default(), deployer_address: ContractAddress::default(), - // TODO(Meshi, 01/09/2024): Change default version to THREE. - version: TransactionVersion::ONE, + version: TransactionVersion::THREE, resource_bounds: default_testing_resource_bounds(), tip: Tip::default(), nonce_data_availability_mode: DataAvailabilityMode::L1, diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index cf252e8b72..3cfadcfa16 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -272,7 +272,6 @@ fn test_infinite_recursion( #[case(TransactionVersion::ONE)] #[case(TransactionVersion::THREE)] fn test_max_fee_limit_validate( - max_fee: Fee, block_context: BlockContext, #[case] version: TransactionVersion, max_resource_bounds: ResourceBoundsMapping, @@ -307,7 +306,7 @@ fn test_max_fee_limit_validate( chain_info, deploy_account_tx_args! { class_hash: grindy_class_hash, - max_fee, + resource_bounds: max_resource_bounds.clone(), constructor_calldata: calldata![ctor_grind_arg, ctor_storage_arg], }, ); @@ -323,7 +322,7 @@ fn test_max_fee_limit_validate( chain_info, deploy_account_tx_args! { class_hash: grindy_class_hash, - max_fee, + resource_bounds: max_resource_bounds.clone(), constructor_calldata: calldata![ctor_grind_arg, ctor_storage_arg], }, ); @@ -983,7 +982,7 @@ fn test_insufficient_max_fee_reverts( #[rstest] fn test_deploy_account_constructor_storage_write( - max_fee: Fee, + max_resource_bounds: ResourceBoundsMapping, block_context: BlockContext, #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion, ) { @@ -1001,7 +1000,7 @@ fn test_deploy_account_constructor_storage_write( chain_info, deploy_account_tx_args! { class_hash, - max_fee, + resource_bounds: max_resource_bounds, constructor_calldata: constructor_calldata.clone(), }, ); diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index c8d982a87d..c284db2cd0 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -770,6 +770,7 @@ fn assert_failure_if_resource_bounds_exceed_balance( #[rstest] fn test_max_fee_exceeds_balance( block_context: BlockContext, + max_resource_bounds: ResourceBoundsMapping, #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] account_cairo_version: CairoVersion, ) { let block_context = &block_context; @@ -811,7 +812,7 @@ fn test_max_fee_exceeds_balance( // Deploy. let invalid_tx = AccountTransaction::DeployAccount(deploy_account_tx( deploy_account_tx_args! { - max_fee: Fee(MAX_FEE), + resource_bounds: max_resource_bounds, class_hash: test_contract.get_class_hash() }, &mut NonceManager::default(), @@ -1220,6 +1221,7 @@ fn test_declare_tx( fn test_deploy_account_tx( #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion, #[values(false, true)] use_kzg_da: bool, + max_resource_bounds: ResourceBoundsMapping, ) { let block_context = &BlockContext::create_for_account_testing_with_kzg(use_kzg_da); let versioned_constants = &block_context.versioned_constants; @@ -1229,7 +1231,7 @@ fn test_deploy_account_tx( let account_class_hash = account.get_class_hash(); let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]); let deploy_account = deploy_account_tx( - deploy_account_tx_args! { max_fee: Fee(MAX_FEE), class_hash: account_class_hash }, + deploy_account_tx_args! { resource_bounds: max_resource_bounds.clone(), class_hash: account_class_hash }, &mut nonce_manager, ); @@ -1369,7 +1371,7 @@ fn test_deploy_account_tx( // Negative flow. // Deploy to an existing address. let deploy_account = deploy_account_tx( - deploy_account_tx_args! { max_fee: Fee(MAX_FEE), class_hash: account_class_hash }, + deploy_account_tx_args! { resource_bounds: max_resource_bounds, class_hash: account_class_hash }, &mut nonce_manager, ); let account_tx = AccountTransaction::DeployAccount(deploy_account); @@ -1388,21 +1390,26 @@ fn test_deploy_account_tx( } #[rstest] -fn test_fail_deploy_account_undeclared_class_hash(block_context: BlockContext) { +fn test_fail_deploy_account_undeclared_class_hash( + block_context: BlockContext, + max_resource_bounds: ResourceBoundsMapping, +) { let block_context = &block_context; let chain_info = &block_context.chain_info; let state = &mut test_state(chain_info, BALANCE, &[]); let mut nonce_manager = NonceManager::default(); let undeclared_hash = class_hash!("0xdeadbeef"); let deploy_account = deploy_account_tx( - deploy_account_tx_args! { max_fee: Fee(MAX_FEE), class_hash: undeclared_hash }, + deploy_account_tx_args! {resource_bounds: max_resource_bounds, class_hash: undeclared_hash }, &mut nonce_manager, ); + let tx_context = block_context.to_tx_context(&deploy_account); + let fee_type = tx_context.tx_info.fee_type(); // Fund account, so as not to fail pre-validation. state .set_storage_at( - chain_info.fee_token_address(&FeeType::Eth), + chain_info.fee_token_address(&fee_type), get_fee_token_var_address(deploy_account.contract_address), felt!(BALANCE), ) From b2e9f28c2201d2c4922c9c5952f1d66d94a2119d Mon Sep 17 00:00:00 2001 From: Meshi Peled <141231558+meship-starkware@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:34:04 +0300 Subject: [PATCH 3/5] fix(transaction): change declare default version to be V3 (#2065) --- crates/blockifier/src/concurrency/fee_utils_test.rs | 3 +-- crates/blockifier/src/concurrency/worker_logic_test.rs | 3 +-- crates/blockifier/src/test_utils/declare.rs | 2 +- .../src/transaction/account_transactions_test.rs | 7 +++---- crates/blockifier/src/transaction/transactions_test.rs | 7 +++---- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/blockifier/src/concurrency/fee_utils_test.rs b/crates/blockifier/src/concurrency/fee_utils_test.rs index 080dc792aa..8e7e15033e 100644 --- a/crates/blockifier/src/concurrency/fee_utils_test.rs +++ b/crates/blockifier/src/concurrency/fee_utils_test.rs @@ -1,7 +1,7 @@ use num_bigint::BigUint; use rstest::rstest; use starknet_api::felt; -use starknet_api::transaction::{Fee, ResourceBoundsMapping, TransactionVersion}; +use starknet_api::transaction::{Fee, ResourceBoundsMapping}; use starknet_types_core::felt::Felt; use crate::concurrency::fee_utils::{add_fee_to_sequencer_balance, fill_sequencer_balance_reads}; @@ -27,7 +27,6 @@ pub fn test_fill_sequencer_balance_reads( sender_address: account.get_instance_address(0), calldata: create_trivial_calldata(account.get_instance_address(0)), resource_bounds: max_resource_bounds, - version: TransactionVersion::THREE }); let chain_info = &block_context.chain_info; let state = &mut test_state_inner(chain_info, BALANCE, &[(account, 1)], erc20_version); diff --git a/crates/blockifier/src/concurrency/worker_logic_test.rs b/crates/blockifier/src/concurrency/worker_logic_test.rs index f698736bba..28b8cac3af 100644 --- a/crates/blockifier/src/concurrency/worker_logic_test.rs +++ b/crates/blockifier/src/concurrency/worker_logic_test.rs @@ -3,7 +3,7 @@ use std::sync::Mutex; use rstest::rstest; use starknet_api::core::{ContractAddress, Nonce, PatriciaKey}; -use starknet_api::transaction::{ContractAddressSalt, ResourceBoundsMapping, TransactionVersion}; +use starknet_api::transaction::{ContractAddressSalt, ResourceBoundsMapping}; use starknet_api::{contract_address, felt, patricia_key}; use starknet_types_core::felt::Felt; @@ -546,7 +546,6 @@ fn test_deploy_before_declare(max_resource_bounds: ResourceBoundsMapping) { resource_bounds: max_resource_bounds.clone(), class_hash: test_class_hash, compiled_class_hash: test_compiled_class_hash, - version: TransactionVersion::THREE, nonce: nonce!(0_u8), }, test_class_info.clone(), diff --git a/crates/blockifier/src/test_utils/declare.rs b/crates/blockifier/src/test_utils/declare.rs index 11e73cbc2b..d64f574d33 100644 --- a/crates/blockifier/src/test_utils/declare.rs +++ b/crates/blockifier/src/test_utils/declare.rs @@ -35,7 +35,7 @@ impl Default for DeclareTxArgs { max_fee: Fee::default(), signature: TransactionSignature::default(), sender_address: ContractAddress::default(), - version: TransactionVersion::ONE, + version: TransactionVersion::THREE, resource_bounds: default_testing_resource_bounds(), tip: Tip::default(), nonce_data_availability_mode: DataAvailabilityMode::L1, diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 3cfadcfa16..79c74ef721 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -68,7 +68,6 @@ fn test_circuit(block_context: BlockContext, max_resource_bounds: ResourceBounds "test_circuit", &[] ), - version: TransactionVersion::THREE, nonce: nonce_manager.next(account_address) }; let tx_execution_info = run_invoke_tx( @@ -278,8 +277,8 @@ fn test_max_fee_limit_validate( ) { let chain_info = &block_context.chain_info; let TestInitData { mut state, account_address, contract_address, mut nonce_manager } = - create_test_init_data(chain_info, CairoVersion::Cairo0); - let grindy_validate_account = FeatureContract::AccountWithLongValidate(CairoVersion::Cairo0); + create_test_init_data(chain_info, CairoVersion::Cairo1); + let grindy_validate_account = FeatureContract::AccountWithLongValidate(CairoVersion::Cairo1); let grindy_class_hash = grindy_validate_account.get_class_hash(); let block_info = &block_context.block_info; let class_info = calculate_class_info_for_testing(grindy_validate_account.get_class()); @@ -289,7 +288,7 @@ fn test_max_fee_limit_validate( declare_tx_args! { class_hash: grindy_class_hash, sender_address: account_address, - max_fee: Fee(MAX_FEE), + resource_bounds: max_resource_bounds.clone(), nonce: nonce_manager.next(account_address), }, class_info, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index c284db2cd0..f00601020a 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -803,8 +803,7 @@ fn test_max_fee_exceeds_balance( // V3 invoke. let invalid_tx = account_invoke_tx(invoke_tx_args! { - resource_bounds: invalid_resource_bounds, - version: TransactionVersion::THREE, + resource_bounds: invalid_resource_bounds.clone(), ..default_args }); assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); @@ -820,14 +819,14 @@ fn test_max_fee_exceeds_balance( assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); // Declare. - let contract_to_declare = FeatureContract::Empty(CairoVersion::Cairo0); + let contract_to_declare = FeatureContract::Empty(CairoVersion::Cairo1); let class_info = calculate_class_info_for_testing(contract_to_declare.get_class()); let invalid_tx = declare_tx( declare_tx_args! { class_hash: contract_to_declare.get_class_hash(), compiled_class_hash: contract_to_declare.get_compiled_class_hash(), sender_address: account_contract_address, - max_fee: invalid_max_fee, + resource_bounds: invalid_resource_bounds, }, class_info, ); From aa12f2213645e012597a4d9f1464ae0c9c7df450 Mon Sep 17 00:00:00 2001 From: AvivYossef-starkware <141143145+AvivYossef-starkware@users.noreply.github.com> Date: Thu, 11 Jul 2024 09:04:07 +0300 Subject: [PATCH 4/5] fix: state error undeclared class hash format (#2053) --- crates/blockifier/src/state.rs | 2 ++ crates/blockifier/src/state/error_format_test.rs | 14 ++++++++++++++ crates/blockifier/src/state/errors.rs | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 crates/blockifier/src/state/error_format_test.rs diff --git a/crates/blockifier/src/state.rs b/crates/blockifier/src/state.rs index 46e34c2bb3..e027d2b301 100644 --- a/crates/blockifier/src/state.rs +++ b/crates/blockifier/src/state.rs @@ -1,4 +1,6 @@ pub mod cached_state; +#[cfg(test)] +pub mod error_format_test; pub mod errors; pub mod global_cache; pub mod state_api; diff --git a/crates/blockifier/src/state/error_format_test.rs b/crates/blockifier/src/state/error_format_test.rs new file mode 100644 index 0000000000..fe7bf81fb0 --- /dev/null +++ b/crates/blockifier/src/state/error_format_test.rs @@ -0,0 +1,14 @@ +use starknet_api::core::ClassHash; +use starknet_types_core::felt::Felt; + +use crate::state::errors::StateError; + +#[test] +fn test_error_undeclared_class_hash_format() { + let error = StateError::UndeclaredClassHash(ClassHash(Felt::TWO)); + assert_eq!( + error.to_string(), + "Class with hash 0x0000000000000000000000000000000000000000000000000000000000000002 is \ + not declared." + ); +} diff --git a/crates/blockifier/src/state/errors.rs b/crates/blockifier/src/state/errors.rs index c098c4edc6..5347d182f7 100644 --- a/crates/blockifier/src/state/errors.rs +++ b/crates/blockifier/src/state/errors.rs @@ -21,7 +21,7 @@ pub enum StateError { ProgramError(#[from] ProgramError), #[error("Requested {0:?} is unavailable for deployment.")] UnavailableContractAddress(ContractAddress), - #[error("Class with hash {0} is not declared.")] + #[error("Class with hash {:#064x} is not declared.", **.0)] UndeclaredClassHash(ClassHash), #[error(transparent)] StarknetApiError(#[from] StarknetApiError), From 53636ad5280da38fd56baff55134ab8e3d722203 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Thu, 11 Jul 2024 09:27:23 +0300 Subject: [PATCH 5/5] No conflicts in main-v0.13.2 -> main merge, this commit is for any change needed to pass the CI.