From 399696efbcc06a82c659ed3bba502121ab9ce6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Delabrouille?= Date: Tue, 20 Aug 2024 02:14:43 +0200 Subject: [PATCH] better names --- .../blockifier/src/execution/entry_point.rs | 8 +-- .../blockifier/src/test_utils/struct_impls.rs | 38 ++++------ .../src/transaction/account_transaction.rs | 41 +++++------ .../transaction/account_transactions_test.rs | 71 ++++++------------- .../src/transaction/transaction_execution.rs | 16 ++--- crates/papyrus_execution/src/lib.rs | 28 ++------ 6 files changed, 63 insertions(+), 139 deletions(-) diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index 34443dd8518..79b74309c8a 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -16,9 +16,7 @@ use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::CallInfo; use crate::execution::common_hints::ExecutionMode; use crate::execution::errors::{ - ConstructorEntryPointExecutionError, - EntryPointExecutionError, - PreExecutionError, + ConstructorEntryPointExecutionError, EntryPointExecutionError, PreExecutionError, }; use crate::execution::execution_utils::execute_entry_point_call; use crate::state::state_api::State; @@ -147,14 +145,14 @@ impl EntryPointExecutionContext { }) } - pub fn new_validate( + pub fn new_validation( tx_context: Arc, limit_steps_by_resources: bool, ) -> TransactionExecutionResult { Self::new(tx_context, ExecutionMode::Validate, limit_steps_by_resources) } - pub fn new_execute( + pub fn new_execution( tx_context: Arc, limit_steps_by_resources: bool, ) -> TransactionExecutionResult { diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs index 55615c4b74b..f681e35702e 100644 --- a/crates/blockifier/src/test_utils/struct_impls.rs +++ b/crates/blockifier/src/test_utils/struct_impls.rs @@ -16,38 +16,22 @@ use crate::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionCont use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::contract_class::{ContractClassV0, ContractClassV1}; use crate::execution::entry_point::{ - CallEntryPoint, - EntryPointExecutionContext, - EntryPointExecutionResult, + CallEntryPoint, EntryPointExecutionContext, EntryPointExecutionResult, }; use crate::fee::fee_utils::get_fee_by_gas_vector; use crate::state::state_api::State; use crate::test_utils::{ - get_raw_contract_class, - CHAIN_ID_NAME, - CURRENT_BLOCK_NUMBER, - CURRENT_BLOCK_TIMESTAMP, - DEFAULT_ETH_L1_DATA_GAS_PRICE, - DEFAULT_ETH_L1_GAS_PRICE, - DEFAULT_STRK_L1_DATA_GAS_PRICE, - DEFAULT_STRK_L1_GAS_PRICE, - TEST_ERC20_CONTRACT_ADDRESS, - TEST_ERC20_CONTRACT_ADDRESS2, + get_raw_contract_class, CHAIN_ID_NAME, CURRENT_BLOCK_NUMBER, CURRENT_BLOCK_TIMESTAMP, + DEFAULT_ETH_L1_DATA_GAS_PRICE, DEFAULT_ETH_L1_GAS_PRICE, DEFAULT_STRK_L1_DATA_GAS_PRICE, + DEFAULT_STRK_L1_GAS_PRICE, TEST_ERC20_CONTRACT_ADDRESS, TEST_ERC20_CONTRACT_ADDRESS2, TEST_SEQUENCER_ADDRESS, }; use crate::transaction::objects::{ - DeprecatedTransactionInfo, - FeeType, - TransactionFeeResult, - TransactionInfo, - TransactionResources, + DeprecatedTransactionInfo, FeeType, TransactionFeeResult, TransactionInfo, TransactionResources, }; use crate::transaction::transactions::L1HandlerTransaction; use crate::versioned_constants::{ - GasCosts, - OsConstants, - VersionedConstants, - VERSIONED_CONSTANTS_LATEST_JSON, + GasCosts, OsConstants, VersionedConstants, VERSIONED_CONSTANTS_LATEST_JSON, }; impl CallEntryPoint { @@ -69,9 +53,11 @@ impl CallEntryPoint { ) -> EntryPointExecutionResult { let tx_context = TransactionContext { block_context: BlockContext::create_for_testing(), tx_info }; - let mut context = - EntryPointExecutionContext::new_execute(Arc::new(tx_context), limit_steps_by_resources) - .unwrap(); + let mut context = EntryPointExecutionContext::new_execution( + Arc::new(tx_context), + limit_steps_by_resources, + ) + .unwrap(); self.execute(state, &mut ExecutionResources::default(), &mut context) } @@ -96,7 +82,7 @@ impl CallEntryPoint { ) -> EntryPointExecutionResult { let tx_context = TransactionContext { block_context: BlockContext::create_for_testing(), tx_info }; - let mut context = EntryPointExecutionContext::new_validate( + let mut context = EntryPointExecutionContext::new_validation( Arc::new(tx_context), limit_steps_by_resources, ) diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 6abba7976eb..86362934417 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -15,9 +15,7 @@ use crate::execution::entry_point::{CallEntryPoint, CallType, EntryPointExecutio use crate::fee::actual_cost::TransactionReceipt; use crate::fee::fee_checks::{FeeCheckReportFields, PostExecutionReport}; use crate::fee::fee_utils::{ - get_fee_by_gas_vector, - get_sequencer_balance_keys, - verify_can_pay_committed_bounds, + get_fee_by_gas_vector, get_sequencer_balance_keys, verify_can_pay_committed_bounds, }; use crate::fee::gas_usage::{compute_discounted_gas_from_gas_vector, estimate_minimal_gas_vector}; use crate::retdata; @@ -25,29 +23,18 @@ use crate::state::cached_state::{StateChanges, TransactionalState}; use crate::state::state_api::{State, StateReader, UpdatableState}; use crate::transaction::constants; use crate::transaction::errors::{ - TransactionExecutionError, - TransactionFeeError, - TransactionPreValidationError, + TransactionExecutionError, TransactionFeeError, TransactionPreValidationError, }; use crate::transaction::objects::{ - DeprecatedTransactionInfo, - HasRelatedFeeType, - TransactionExecutionInfo, - TransactionExecutionResult, - TransactionInfo, - TransactionInfoCreator, + DeprecatedTransactionInfo, HasRelatedFeeType, TransactionExecutionInfo, + TransactionExecutionResult, TransactionInfo, TransactionInfoCreator, TransactionPreValidationResult, }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transaction_utils::update_remaining_gas; use crate::transaction::transactions::{ - DeclareTransaction, - DeployAccountTransaction, - Executable, - ExecutableTransaction, - ExecutionFlags, - InvokeTransaction, - ValidatableTransaction, + DeclareTransaction, DeployAccountTransaction, Executable, ExecutableTransaction, + ExecutionFlags, InvokeTransaction, ValidatableTransaction, }; #[cfg(test)] @@ -370,7 +357,7 @@ impl AccountTransaction { initial_gas: block_context.versioned_constants.os_constants.gas_costs.initial_gas_cost, }; - let mut context = EntryPointExecutionContext::new_execute(tx_context, true)?; + let mut context = EntryPointExecutionContext::new_execution(tx_context, true)?; Ok(fee_transfer_call .execute(state, &mut ExecutionResources::default(), &mut context) @@ -439,7 +426,7 @@ impl AccountTransaction { // Also, the execution context required form the `DeployAccount` execute phase is // validation context. let mut execution_context = - EntryPointExecutionContext::new_validate(tx_context.clone(), charge_fee)?; + EntryPointExecutionContext::new_validation(tx_context.clone(), charge_fee)?; execute_call_info = self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?; validate_call_info = self.handle_validate_tx( @@ -452,7 +439,7 @@ impl AccountTransaction { )?; } else { let mut execution_context = - EntryPointExecutionContext::new_execute(tx_context.clone(), charge_fee)?; + EntryPointExecutionContext::new_execution(tx_context.clone(), charge_fee)?; validate_call_info = self.handle_validate_tx( state, &mut resources, @@ -496,7 +483,7 @@ impl AccountTransaction { ) -> TransactionExecutionResult { let mut resources = ExecutionResources::default(); let mut execution_context = - EntryPointExecutionContext::new_execute(tx_context.clone(), charge_fee)?; + EntryPointExecutionContext::new_execution(tx_context.clone(), charge_fee)?; // Run the validation, and if execution later fails, only keep the validation diff. let validate_call_info = self.handle_validate_tx( state, @@ -610,7 +597,11 @@ impl AccountTransaction { /// Returns 0 on non-declare transactions; for declare transactions, returns the class code /// size. pub(crate) fn declare_code_size(&self) -> usize { - if let Self::Declare(tx) = self { tx.class_info.code_size() } else { 0 } + if let Self::Declare(tx) = self { + tx.class_info.code_size() + } else { + 0 + } } fn is_non_revertible(&self, tx_info: &TransactionInfo) -> bool { @@ -757,7 +748,7 @@ impl ValidatableTransaction for AccountTransaction { limit_steps_by_resources: bool, ) -> TransactionExecutionResult> { let mut context = - EntryPointExecutionContext::new_validate(tx_context, limit_steps_by_resources)?; + EntryPointExecutionContext::new_validation(tx_context, limit_steps_by_resources)?; let tx_info = &context.tx_context.tx_info; if tx_info.is_v0() { return Ok(None); diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 2194bbfa9d9..03a6e3460f3 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -9,21 +9,14 @@ use starknet_api::core::{calculate_contract_address, ClassHash, ContractAddress, use starknet_api::hash::StarkHash; use starknet_api::state::StorageKey; use starknet_api::transaction::{ - Calldata, - ContractAddressSalt, - DeclareTransactionV2, - Fee, - ResourceBoundsMapping, - TransactionHash, - TransactionVersion, + Calldata, ContractAddressSalt, DeclareTransactionV2, Fee, ResourceBoundsMapping, + TransactionHash, TransactionVersion, }; use starknet_api::{calldata, class_hash, contract_address, felt, patricia_key}; use starknet_types_core::felt::Felt; use crate::abi::abi_utils::{ - get_fee_token_var_address, - get_storage_var_address, - selector_from_name, + get_fee_token_var_address, get_storage_var_address, selector_from_name, }; use crate::context::BlockContext; use crate::execution::contract_class::{ContractClass, ContractClassV1}; @@ -39,44 +32,23 @@ use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::initial_test_state::{fund_account, test_state}; use crate::test_utils::invoke::InvokeTxArgs; use crate::test_utils::{ - create_calldata, - create_trivial_calldata, - get_syscall_resources, - get_tx_resources, - u64_from_usize, - CairoVersion, - NonceManager, - BALANCE, - DEFAULT_STRK_L1_GAS_PRICE, - MAX_FEE, + create_calldata, create_trivial_calldata, get_syscall_resources, get_tx_resources, + u64_from_usize, CairoVersion, NonceManager, BALANCE, DEFAULT_STRK_L1_GAS_PRICE, MAX_FEE, }; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::constants::TRANSFER_ENTRY_POINT_NAME; use crate::transaction::objects::{FeeType, GasVector, HasRelatedFeeType, TransactionInfoCreator}; use crate::transaction::test_utils::{ - account_invoke_tx, - block_context, - calculate_class_info_for_testing, - create_account_tx_for_validate_test_nonce_0, - create_test_init_data, - deploy_and_fund_account, - l1_resource_bounds, - max_fee, - max_resource_bounds, - run_invoke_tx, - FaultyAccountTxCreatorArgs, - TestInitData, - INVALID, + account_invoke_tx, block_context, calculate_class_info_for_testing, + create_account_tx_for_validate_test_nonce_0, create_test_init_data, deploy_and_fund_account, + l1_resource_bounds, max_fee, max_resource_bounds, run_invoke_tx, FaultyAccountTxCreatorArgs, + TestInitData, INVALID, }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::{DeclareTransaction, ExecutableTransaction, ExecutionFlags}; use crate::{ - check_transaction_execution_error_for_invalid_scenario, - declare_tx_args, - deploy_account_tx_args, - invoke_tx_args, - nonce, - storage_key, + check_transaction_execution_error_for_invalid_scenario, declare_tx_args, + deploy_account_tx_args, invoke_tx_args, nonce, storage_key, }; #[rstest] @@ -324,12 +296,10 @@ fn test_infinite_recursion( if success { assert!(tx_execution_info.revert_error.is_none()); } else { - assert!( - tx_execution_info - .revert_error - .unwrap() - .contains("RunResources has no remaining steps.") - ); + assert!(tx_execution_info + .revert_error + .unwrap() + .contains("RunResources has no remaining steps.")); } } @@ -930,7 +900,7 @@ fn test_max_fee_to_max_steps_conversion( nonce: nonce_manager.next(account_address), }); let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1)); - let execution_context1 = EntryPointExecutionContext::new_execute(tx_context1, true).unwrap(); + let execution_context1 = EntryPointExecutionContext::new_execution(tx_context1, true).unwrap(); let max_steps_limit1 = execution_context1.vm_run_resources.get_n_steps(); let tx_execution_info1 = account_tx1.execute(&mut state, &block_context, true, true).unwrap(); let n_steps1 = tx_execution_info1.transaction_receipt.resources.vm_resources.n_steps; @@ -950,7 +920,7 @@ fn test_max_fee_to_max_steps_conversion( nonce: nonce_manager.next(account_address), }); let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2)); - let execution_context2 = EntryPointExecutionContext::new_execute(tx_context2, true).unwrap(); + let execution_context2 = EntryPointExecutionContext::new_execution(tx_context2, true).unwrap(); let max_steps_limit2 = execution_context2.vm_run_resources.get_n_steps(); let tx_execution_info2 = account_tx2.execute(&mut state, &block_context, true, true).unwrap(); let n_steps2 = tx_execution_info2.transaction_receipt.resources.vm_resources.n_steps; @@ -1042,9 +1012,10 @@ fn test_insufficient_max_fee_reverts( .unwrap(); assert!(tx_execution_info3.is_reverted()); assert!(tx_execution_info3.transaction_receipt.fee == actual_fee_depth1); - assert!( - tx_execution_info3.revert_error.unwrap().contains("RunResources has no remaining steps.") - ); + assert!(tx_execution_info3 + .revert_error + .unwrap() + .contains("RunResources has no remaining steps.")); } #[rstest] diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index d66d4c56e1c..479c5b39ef6 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -14,19 +14,11 @@ use crate::state::state_api::UpdatableState; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::TransactionFeeError; use crate::transaction::objects::{ - TransactionExecutionInfo, - TransactionExecutionResult, - TransactionInfo, - TransactionInfoCreator, + TransactionExecutionInfo, TransactionExecutionResult, TransactionInfo, TransactionInfoCreator, }; use crate::transaction::transactions::{ - DeclareTransaction, - DeployAccountTransaction, - Executable, - ExecutableTransaction, - ExecutionFlags, - InvokeTransaction, - L1HandlerTransaction, + DeclareTransaction, DeployAccountTransaction, Executable, ExecutableTransaction, + ExecutionFlags, InvokeTransaction, L1HandlerTransaction, }; // TODO: Move into transaction.rs, makes more sense to be defined there. @@ -118,7 +110,7 @@ impl ExecutableTransaction for L1HandlerTransaction { let tx_context = Arc::new(block_context.to_tx_context(self)); let mut execution_resources = ExecutionResources::default(); - let mut context = EntryPointExecutionContext::new_execute(tx_context.clone(), true)?; + let mut context = EntryPointExecutionContext::new_execution(tx_context.clone(), true)?; let mut remaining_gas = block_context.versioned_constants.tx_initial_gas(); let execute_call_info = self.run_execute(state, &mut execution_resources, &mut context, &mut remaining_gas)?; diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs index 9d76ff1acfb..d89dbf1d947 100644 --- a/crates/papyrus_execution/src/lib.rs +++ b/crates/papyrus_execution/src/lib.rs @@ -31,16 +31,12 @@ use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses, Transactio use blockifier::execution::call_info::CallExecution; use blockifier::execution::contract_class::{ClassInfo, ContractClass as BlockifierContractClass}; use blockifier::execution::entry_point::{ - CallEntryPoint, - CallType as BlockifierCallType, - EntryPointExecutionContext, + CallEntryPoint, CallType as BlockifierCallType, EntryPointExecutionContext, }; use blockifier::state::cached_state::CachedState; use blockifier::transaction::errors::TransactionExecutionError as BlockifierTransactionExecutionError; use blockifier::transaction::objects::{ - DeprecatedTransactionInfo, - TransactionExecutionInfo, - TransactionInfo, + DeprecatedTransactionInfo, TransactionExecutionInfo, TransactionInfo, }; use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction; use blockifier::transaction::transactions::ExecutableTransaction; @@ -63,23 +59,13 @@ use starknet_api::core::{ChainId, ClassHash, ContractAddress, EntryPointSelector use starknet_api::data_availability::L1DataAvailabilityMode; // TODO: merge multiple EntryPointType structs in SN_API into one. use starknet_api::deprecated_contract_class::{ - ContractClass as DeprecatedContractClass, - EntryPointType, + ContractClass as DeprecatedContractClass, EntryPointType, }; use starknet_api::state::{StateNumber, ThinStateDiff}; use starknet_api::transaction::{ - Calldata, - DeclareTransaction, - DeclareTransactionV0V1, - DeclareTransactionV2, - DeclareTransactionV3, - DeployAccountTransaction, - Fee, - InvokeTransaction, - L1HandlerTransaction, - Transaction, - TransactionHash, - TransactionVersion, + Calldata, DeclareTransaction, DeclareTransactionV0V1, DeclareTransactionV2, + DeclareTransactionV3, DeployAccountTransaction, Fee, InvokeTransaction, L1HandlerTransaction, + Transaction, TransactionHash, TransactionVersion, }; use starknet_api::{contract_address, felt, patricia_key, StarknetApiError}; use state_reader::ExecutionStateReader; @@ -255,7 +241,7 @@ pub fn execute_call( override_kzg_da_to_false, )?; - let mut context = EntryPointExecutionContext::new_execute( + let mut context = EntryPointExecutionContext::new_execution( // TODO(yair): fix when supporting v3 transactions Arc::new(TransactionContext { block_context,