From d2ab9d8ed6fa2387d87560f30024ae347e0b64e6 Mon Sep 17 00:00:00 2001 From: Aviv Greenburg Date: Thu, 26 Sep 2024 10:58:17 +0300 Subject: [PATCH] refactor(blockifier): account_tx from enum to struct w api::executable_tx --- crates/batcher/src/block_builder.rs | 2 +- .../src/blockifier/stateful_validator.rs | 3 +- .../src/blockifier/stateful_validator_test.rs | 8 +- .../src/execution/stack_trace_test.rs | 10 +- crates/blockifier/src/fee/gas_usage.rs | 9 +- crates/blockifier/src/test_utils/declare.rs | 8 +- .../src/test_utils/deploy_account.rs | 11 +- crates/blockifier/src/test_utils/invoke.rs | 20 +- .../src/transaction/account_transaction.rs | 190 +++++++----------- .../transaction/account_transactions_test.rs | 34 ++-- .../src/transaction/transaction_execution.rs | 83 +++----- .../src/stateful_transaction_validator.rs | 5 +- crates/native_blockifier/src/py_declare.rs | 4 +- .../src/py_deploy_account.rs | 4 +- .../src/py_invoke_function.rs | 4 +- .../native_blockifier/src/py_transaction.rs | 12 +- 16 files changed, 176 insertions(+), 231 deletions(-) diff --git a/crates/batcher/src/block_builder.rs b/crates/batcher/src/block_builder.rs index a18a3f64dd..f247e36f09 100644 --- a/crates/batcher/src/block_builder.rs +++ b/crates/batcher/src/block_builder.rs @@ -164,7 +164,7 @@ impl BlockBuilderTrait for BlockBuilder { let mut executor_input_chunk = vec![]; for tx in &next_tx_chunk { executor_input_chunk - .push(BlockifierTransaction::Account(AccountTransaction::try_from(tx)?)); + .push(BlockifierTransaction::Account(AccountTransaction::new(tx.clone()))); } let results = self.executor.lock().await.add_txs_to_block(&executor_input_chunk); trace!("Transaction execution results: {:?}", results); diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 14964e1a56..a428ab3cb4 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use starknet_api::core::{ContractAddress, Nonce}; +use starknet_api::executable_transaction::Transaction as ApiTransaction; use thiserror::Error; use crate::blockifier::config::TransactionExecutorConfig; @@ -61,7 +62,7 @@ impl StatefulValidator { // Deploy account transactions should be fully executed, since the constructor must run // before `__validate_deploy__`. The execution already includes all necessary validations, // so they are skipped here. - if let AccountTransaction::DeployAccount(_) = tx { + if let ApiTransaction::DeployAccount(_) = tx.tx { self.execute(tx)?; return Ok(()); } diff --git a/crates/blockifier/src/blockifier/stateful_validator_test.rs b/crates/blockifier/src/blockifier/stateful_validator_test.rs index 42b2b57b04..69c5e56cbe 100644 --- a/crates/blockifier/src/blockifier/stateful_validator_test.rs +++ b/crates/blockifier/src/blockifier/stateful_validator_test.rs @@ -1,5 +1,6 @@ use assert_matches::assert_matches; use rstest::rstest; +use starknet_api::executable_transaction::Transaction; use starknet_api::transaction::{TransactionVersion, ValidResourceBounds}; use crate::blockifier::stateful_validator::StatefulValidator; @@ -7,7 +8,6 @@ use crate::context::BlockContext; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::initial_test_state::{fund_account, test_state}; use crate::test_utils::{CairoVersion, BALANCE}; -use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::test_utils::{ block_context, create_account_tx_for_validate_test_nonce_0, @@ -61,18 +61,18 @@ fn test_tx_validator( }; // Positive flow. - let tx = create_account_tx_for_validate_test_nonce_0(FaultyAccountTxCreatorArgs { + let account_tx = create_account_tx_for_validate_test_nonce_0(FaultyAccountTxCreatorArgs { scenario: VALID, ..tx_args }); - if let AccountTransaction::DeployAccount(deploy_tx) = &tx { + if let Transaction::DeployAccount(deploy_tx) = &account_tx.tx { fund_account(chain_info, deploy_tx.contract_address(), BALANCE, &mut state.state); } // Test the stateful validator. let mut stateful_validator = StatefulValidator::create(state, block_context); let skip_validate = false; - let result = stateful_validator.perform_validations(tx, skip_validate); + let result = stateful_validator.perform_validations(account_tx, skip_validate); assert!(result.is_ok(), "Validation failed: {:?}", result.unwrap_err()); } diff --git a/crates/blockifier/src/execution/stack_trace_test.rs b/crates/blockifier/src/execution/stack_trace_test.rs index f7e3ac97af..aed3f9a9ca 100644 --- a/crates/blockifier/src/execution/stack_trace_test.rs +++ b/crates/blockifier/src/execution/stack_trace_test.rs @@ -9,6 +9,7 @@ use starknet_api::core::{ EntryPointSelector, Nonce, }; +use starknet_api::executable_transaction::Transaction; use starknet_api::transaction::{ ContractAddressSalt, Fee, @@ -35,7 +36,6 @@ use crate::execution::syscalls::hint_processor::ENTRYPOINT_FAILED_ERROR; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::initial_test_state::{fund_account, test_state}; use crate::test_utils::{create_calldata, CairoVersion, BALANCE}; -use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::constants::{ DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME, EXECUTE_ENTRY_POINT_NAME, @@ -589,8 +589,8 @@ fn test_validate_trace( if let TransactionType::DeployAccount = tx_type { // Deploy account uses the actual address as the sender address. - match &account_tx { - AccountTransaction::DeployAccount(tx) => { + match &account_tx.tx { + Transaction::DeployAccount(tx) => { sender_address = tx.contract_address(); } _ => panic!("Expected DeployAccountTransaction type"), @@ -658,8 +658,8 @@ fn test_account_ctor_frame_stack_trace( }); // Fund the account so it can afford the deployment. - let deploy_address = match &deploy_account_tx { - AccountTransaction::DeployAccount(deploy_tx) => deploy_tx.contract_address(), + let deploy_address = match &deploy_account_tx.tx { + Transaction::DeployAccount(deploy_tx) => deploy_tx.contract_address(), _ => unreachable!("deploy_account_tx is a DeployAccount"), }; fund_account(chain_info, deploy_address, Fee(BALANCE.0 * 2), &mut state.state); diff --git a/crates/blockifier/src/fee/gas_usage.rs b/crates/blockifier/src/fee/gas_usage.rs index bf5cfa5e8a..c32d71c955 100644 --- a/crates/blockifier/src/fee/gas_usage.rs +++ b/crates/blockifier/src/fee/gas_usage.rs @@ -1,4 +1,5 @@ use cairo_vm::vm::runners::cairo_runner::ExecutionResources; +use starknet_api::executable_transaction::Transaction; use starknet_api::execution_resources::GasVector; use starknet_api::transaction::GasVectorComputationMode; @@ -160,24 +161,24 @@ pub fn estimate_minimal_gas_vector( ) -> GasVector { // TODO(Dori, 1/8/2023): Give names to the constant VM step estimates and regression-test them. let BlockContext { block_info, versioned_constants, .. } = block_context; - let state_changes_by_account_tx = match tx { + let state_changes_by_account_tx = match &tx.tx { // We consider the following state changes: sender balance update (storage update) + nonce // increment (contract modification) (we exclude the sequencer balance update and the ERC20 // contract modification since it occurs for every tx). - AccountTransaction::Declare(_) => StateChangesCount { + Transaction::Declare(_) => StateChangesCount { n_storage_updates: 1, n_class_hash_updates: 0, n_compiled_class_hash_updates: 0, n_modified_contracts: 1, }, - AccountTransaction::Invoke(_) => StateChangesCount { + Transaction::Invoke(_) => StateChangesCount { n_storage_updates: 1, n_class_hash_updates: 0, n_compiled_class_hash_updates: 0, n_modified_contracts: 1, }, // DeployAccount also updates the address -> class hash mapping. - AccountTransaction::DeployAccount(_) => StateChangesCount { + Transaction::DeployAccount(_) => StateChangesCount { n_storage_updates: 1, n_class_hash_updates: 1, n_compiled_class_hash_updates: 0, diff --git a/crates/blockifier/src/test_utils/declare.rs b/crates/blockifier/src/test_utils/declare.rs index 5a3ce61729..fbf0933178 100644 --- a/crates/blockifier/src/test_utils/declare.rs +++ b/crates/blockifier/src/test_utils/declare.rs @@ -1,13 +1,15 @@ use starknet_api::contract_class::ClassInfo; +use starknet_api::executable_transaction::{DeclareTransaction, Transaction}; use starknet_api::test_utils::declare::DeclareTxArgs; use crate::transaction::account_transaction::AccountTransaction; -use crate::transaction::transactions::DeclareTransaction; pub fn declare_tx(declare_tx_args: DeclareTxArgs, class_info: ClassInfo) -> AccountTransaction { let tx_hash = declare_tx_args.tx_hash; let declare_tx = starknet_api::test_utils::declare::declare_tx(declare_tx_args); - let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap(); - AccountTransaction::Declare(executable_declare) + let executable_tx = + Transaction::Declare(DeclareTransaction { tx: declare_tx, tx_hash, class_info }); + + AccountTransaction::new(executable_tx) } diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index 24397f650a..45c924c71b 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/crates/blockifier/src/test_utils/deploy_account.rs @@ -1,9 +1,9 @@ use starknet_api::core::calculate_contract_address; +use starknet_api::executable_transaction::{DeployAccountTransaction, Transaction}; use starknet_api::test_utils::deploy_account::DeployAccountTxArgs; use starknet_api::test_utils::NonceManager; use crate::transaction::account_transaction::AccountTransaction; -use crate::transaction::transactions::DeployAccountTransaction; pub fn deploy_account_tx( deploy_tx_args: DeployAccountTxArgs, @@ -22,8 +22,11 @@ pub fn deploy_account_tx( deploy_tx_args, nonce_manager.next(contract_address), ); - let executable_deploy_account_tx = - DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address); + let executable_tx = Transaction::DeployAccount(DeployAccountTransaction { + tx: deploy_account_tx, + tx_hash, + contract_address, + }); - AccountTransaction::DeployAccount(executable_deploy_account_tx) + AccountTransaction::new(executable_tx) } diff --git a/crates/blockifier/src/test_utils/invoke.rs b/crates/blockifier/src/test_utils/invoke.rs index b906283178..4570b16651 100644 --- a/crates/blockifier/src/test_utils/invoke.rs +++ b/crates/blockifier/src/test_utils/invoke.rs @@ -1,17 +1,20 @@ +use starknet_api::executable_transaction::{ + InvokeTransaction as ExecutableInvokeTransaction, + Transaction as ExecutableTransaction, +}; use starknet_api::test_utils::invoke::InvokeTxArgs; -use starknet_api::transaction::{InvokeTransactionV0, TransactionVersion}; +use starknet_api::transaction::{InvokeTransaction, InvokeTransactionV0, TransactionVersion}; use crate::abi::abi_utils::selector_from_name; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::constants::EXECUTE_ENTRY_POINT_NAME; -use crate::transaction::transactions::InvokeTransaction; pub fn invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction { let tx_hash = invoke_args.tx_hash; let only_query = invoke_args.only_query; // TODO: Make TransactionVersion an enum and use match here. let invoke_tx = if invoke_args.version == TransactionVersion::ZERO { - starknet_api::transaction::InvokeTransaction::V0(InvokeTransactionV0 { + InvokeTransaction::V0(InvokeTransactionV0 { max_fee: invoke_args.max_fee, calldata: invoke_args.calldata, contract_address: invoke_args.sender_address, @@ -23,10 +26,11 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction { starknet_api::test_utils::invoke::invoke_tx(invoke_args) }; - let invoke_tx = match only_query { - true => InvokeTransaction::new_for_query(invoke_tx, tx_hash), - false => InvokeTransaction::new(invoke_tx, tx_hash), - }; + let invoke_tx = + ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx: invoke_tx, tx_hash }); - AccountTransaction::Invoke(invoke_tx) + match only_query { + true => AccountTransaction::new_for_query(invoke_tx), + false => AccountTransaction::new(invoke_tx), + } } diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 271763cb74..d927f75271 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -6,6 +6,7 @@ use starknet_api::calldata; use starknet_api::contract_class::EntryPointType; use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; +use starknet_api::executable_transaction::Transaction; use starknet_api::transaction::Resource::{L1DataGas, L1Gas, L2Gas}; use starknet_api::transaction::{ AccountDeploymentData, @@ -51,16 +52,14 @@ use crate::transaction::objects::{ TransactionExecutionResult, TransactionInfo, TransactionInfoCreator, + TransactionInfoCreatorInner, TransactionPreValidationResult, }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::{ - DeclareTransaction, - DeployAccountTransaction, Executable, ExecutableTransaction, ExecutionFlags, - InvokeTransaction, ValidatableTransaction, }; @@ -78,76 +77,29 @@ mod post_execution_test; /// Represents a paid Starknet transaction. #[derive(Clone, Debug, derive_more::From)] -pub enum AccountTransaction { - Declare(DeclareTransaction), - DeployAccount(DeployAccountTransaction), - Invoke(InvokeTransaction), +pub struct AccountTransaction { + pub tx: Transaction, + only_query: bool, } macro_rules! implement_account_tx_inner_getters { ($(($field:ident, $field_type:ty)),*) => { $(pub fn $field(&self) -> $field_type { - match self { - Self::Declare(tx) => tx.tx.$field().clone(), - Self::DeployAccount(tx) => tx.tx.$field().clone(), - Self::Invoke(tx) => tx.tx.$field().clone(), + match &self.tx { + Transaction::Declare(tx) => tx.tx.$field().clone(), + Transaction::DeployAccount(tx) => tx.tx.$field().clone(), + Transaction::Invoke(tx) => tx.tx.$field().clone(), } })* }; } -impl TryFrom<&starknet_api::executable_transaction::Transaction> for AccountTransaction { - type Error = TransactionExecutionError; - - fn try_from( - value: &starknet_api::executable_transaction::Transaction, - ) -> Result { - match value { - starknet_api::executable_transaction::Transaction::Declare(declare_tx) => { - Ok(Self::Declare(declare_tx.clone().try_into()?)) - } - starknet_api::executable_transaction::Transaction::DeployAccount(deploy_account_tx) => { - Ok(Self::DeployAccount(DeployAccountTransaction { - tx: deploy_account_tx.clone(), - only_query: false, - })) - } - starknet_api::executable_transaction::Transaction::Invoke(invoke_tx) => { - Ok(Self::Invoke(InvokeTransaction { tx: invoke_tx.clone(), only_query: false })) - } - } - } -} - -impl TryFrom for AccountTransaction { - type Error = TransactionExecutionError; - - fn try_from( - executable_transaction: starknet_api::executable_transaction::Transaction, - ) -> Result { - match executable_transaction { - starknet_api::executable_transaction::Transaction::Declare(declare_tx) => { - Ok(Self::Declare(declare_tx.try_into()?)) - } - starknet_api::executable_transaction::Transaction::DeployAccount(deploy_account_tx) => { - Ok(Self::DeployAccount(DeployAccountTransaction { - tx: deploy_account_tx, - only_query: false, - })) - } - starknet_api::executable_transaction::Transaction::Invoke(invoke_tx) => { - Ok(Self::Invoke(InvokeTransaction { tx: invoke_tx, only_query: false })) - } - } - } -} - impl HasRelatedFeeType for AccountTransaction { fn version(&self) -> TransactionVersion { - match self { - Self::Declare(tx) => tx.tx.version(), - Self::DeployAccount(tx) => tx.tx.version(), - Self::Invoke(tx) => tx.tx.version(), + match &self.tx { + Transaction::Declare(tx) => tx.tx.version(), + Transaction::DeployAccount(tx) => tx.tx.version(), + Transaction::Invoke(tx) => tx.tx.version(), } } @@ -167,44 +119,48 @@ impl AccountTransaction { (paymaster_data, PaymasterData) ); + pub fn new(tx: starknet_api::executable_transaction::Transaction) -> Self { + AccountTransaction { tx, only_query: false } + } + + pub fn new_for_query(tx: starknet_api::executable_transaction::Transaction) -> Self { + AccountTransaction { tx, only_query: true } + } + pub fn sender_address(&self) -> ContractAddress { - match self { - Self::Declare(tx) => tx.tx.sender_address(), - Self::DeployAccount(tx) => tx.tx.contract_address(), - Self::Invoke(tx) => tx.tx.sender_address(), - } + self.tx.sender_address() } pub fn class_hash(&self) -> Option { - match self { - Self::Declare(tx) => Some(tx.tx.class_hash()), - Self::DeployAccount(tx) => Some(tx.tx.class_hash()), - Self::Invoke(_) => None, + match &self.tx { + Transaction::Declare(tx) => Some(tx.tx.class_hash()), + Transaction::DeployAccount(tx) => Some(tx.tx.class_hash()), + Transaction::Invoke(_) => None, } } pub fn account_deployment_data(&self) -> Option { - match self { - Self::Declare(tx) => Some(tx.tx.account_deployment_data().clone()), - Self::DeployAccount(_) => None, - Self::Invoke(tx) => Some(tx.tx.account_deployment_data().clone()), + match &self.tx { + Transaction::Declare(tx) => Some(tx.tx.account_deployment_data().clone()), + Transaction::DeployAccount(_) => None, + Transaction::Invoke(tx) => Some(tx.tx.account_deployment_data().clone()), } } // TODO(nir, 01/11/2023): Consider instantiating CommonAccountFields in AccountTransaction. pub fn tx_type(&self) -> TransactionType { - match self { - AccountTransaction::Declare(_) => TransactionType::Declare, - AccountTransaction::DeployAccount(_) => TransactionType::DeployAccount, - AccountTransaction::Invoke(_) => TransactionType::InvokeFunction, + match &self.tx { + Transaction::Declare(_) => TransactionType::Declare, + Transaction::DeployAccount(_) => TransactionType::DeployAccount, + Transaction::Invoke(_) => TransactionType::InvokeFunction, } } fn validate_entry_point_selector(&self) -> EntryPointSelector { - let validate_entry_point_name = match self { - Self::Declare(_) => constants::VALIDATE_DECLARE_ENTRY_POINT_NAME, - Self::DeployAccount(_) => constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME, - Self::Invoke(_) => constants::VALIDATE_ENTRY_POINT_NAME, + let validate_entry_point_name = match &self.tx { + Transaction::Declare(_) => constants::VALIDATE_DECLARE_ENTRY_POINT_NAME, + Transaction::DeployAccount(_) => constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME, + Transaction::Invoke(_) => constants::VALIDATE_ENTRY_POINT_NAME, }; selector_from_name(validate_entry_point_name) } @@ -212,9 +168,9 @@ impl AccountTransaction { // Calldata for validation contains transaction fields that cannot be obtained by calling // `et_tx_info()`. pub fn validate_entrypoint_calldata(&self) -> Calldata { - match self { - Self::Declare(tx) => calldata![tx.class_hash().0], - Self::DeployAccount(tx) => Calldata( + match &self.tx { + Transaction::Declare(tx) => calldata![tx.class_hash().0], + Transaction::DeployAccount(tx) => Calldata( [ vec![tx.class_hash().0, tx.contract_address_salt().0], (*tx.constructor_calldata().0).clone(), @@ -223,36 +179,32 @@ impl AccountTransaction { .into(), ), // Calldata for validation is the same calldata as for the execution itself. - Self::Invoke(tx) => tx.calldata(), + Transaction::Invoke(tx) => tx.calldata(), } } pub fn calldata_length(&self) -> usize { - let calldata = match self { - Self::Declare(_tx) => return 0, - Self::DeployAccount(tx) => tx.constructor_calldata(), - Self::Invoke(tx) => tx.calldata(), + let calldata = match &self.tx { + Transaction::Declare(_tx) => return 0, + Transaction::DeployAccount(tx) => tx.constructor_calldata(), + Transaction::Invoke(tx) => tx.calldata(), }; calldata.0.len() } pub fn signature_length(&self) -> usize { - let signature = match self { - Self::Declare(tx) => tx.signature(), - Self::DeployAccount(tx) => tx.signature(), - Self::Invoke(tx) => tx.signature(), + let signature = match &self.tx { + Transaction::Declare(tx) => tx.signature(), + Transaction::DeployAccount(tx) => tx.signature(), + Transaction::Invoke(tx) => tx.signature(), }; signature.0.len() } pub fn tx_hash(&self) -> TransactionHash { - match self { - Self::Declare(tx) => tx.tx_hash(), - Self::DeployAccount(tx) => tx.tx_hash(), - Self::Invoke(tx) => tx.tx_hash(), - } + self.tx.tx_hash() } pub fn enforce_fee(&self) -> bool { @@ -260,9 +212,9 @@ impl AccountTransaction { } fn verify_tx_version(&self, version: TransactionVersion) -> TransactionExecutionResult<()> { - let allowed_versions: Vec = match self { + let allowed_versions: Vec = match &self.tx { // Support `Declare` of version 0 in order to allow bootstrapping of a new system. - Self::Declare(_) => { + Transaction::Declare(_) => { vec![ TransactionVersion::ZERO, TransactionVersion::ONE, @@ -270,10 +222,10 @@ impl AccountTransaction { TransactionVersion::THREE, ] } - Self::DeployAccount(_) => { + Transaction::DeployAccount(_) => { vec![TransactionVersion::ONE, TransactionVersion::THREE] } - Self::Invoke(_) => { + Transaction::Invoke(_) => { vec![TransactionVersion::ZERO, TransactionVersion::ONE, TransactionVersion::THREE] } }; @@ -546,7 +498,7 @@ impl AccountTransaction { } let fee_transfer_call_info = - AccountTransaction::execute_fee_transfer(&mut transfer_state, tx_context, actual_fee); + Self::execute_fee_transfer(&mut transfer_state, tx_context, actual_fee); // Commit without updating the sequencer balance. let storage_writes = &mut transfer_state.cache.get_mut().writes.storage; storage_writes.remove(&(fee_address, sequencer_balance_key_low)); @@ -562,10 +514,12 @@ impl AccountTransaction { context: &mut EntryPointExecutionContext, remaining_gas: &mut u64, ) -> TransactionExecutionResult> { - match &self { - Self::Declare(tx) => tx.run_execute(state, resources, context, remaining_gas), - Self::DeployAccount(tx) => tx.run_execute(state, resources, context, remaining_gas), - Self::Invoke(tx) => tx.run_execute(state, resources, context, remaining_gas), + match &self.tx { + Transaction::Declare(tx) => tx.run_execute(state, resources, context, remaining_gas), + Transaction::DeployAccount(tx) => { + tx.run_execute(state, resources, context, remaining_gas) + } + Transaction::Invoke(tx) => tx.run_execute(state, resources, context, remaining_gas), } } @@ -580,7 +534,7 @@ impl AccountTransaction { let mut resources = ExecutionResources::default(); let validate_call_info: Option; let execute_call_info: Option; - if matches!(self, Self::DeployAccount(_)) { + if matches!(&self.tx, Transaction::DeployAccount(_)) { // Handle `DeployAccount` transactions separately, due to different order of things. // Also, the execution context required form the `DeployAccount` execute phase is // validation context. @@ -758,15 +712,15 @@ 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 Transaction::Declare(tx) = &self.tx { tx.class_info.code_size() } else { 0 } } fn is_non_revertible(&self, tx_info: &TransactionInfo) -> bool { // Reverting a Declare or Deploy transaction is not currently supported in the OS. - match self { - Self::Declare(_) => true, - Self::DeployAccount(_) => true, - Self::Invoke(_) => { + match &self.tx { + Transaction::Declare(_) => true, + Transaction::DeployAccount(_) => true, + Transaction::Invoke(_) => { // V0 transactions do not have validation; we cannot deduct fee for execution. Thus, // invoke transactions of are non-revertible iff they are of version 0. tx_info.is_v0() @@ -856,10 +810,10 @@ impl ExecutableTransaction for AccountTransaction { impl TransactionInfoCreator for AccountTransaction { fn create_tx_info(&self) -> TransactionInfo { - match self { - Self::Declare(tx) => tx.create_tx_info(), - Self::DeployAccount(tx) => tx.create_tx_info(), - Self::Invoke(tx) => tx.create_tx_info(), + match &self.tx { + Transaction::Declare(tx) => tx.create_tx_info(self.only_query), + Transaction::DeployAccount(tx) => tx.create_tx_info(self.only_query), + Transaction::Invoke(tx) => tx.create_tx_info(self.only_query), } } } diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index abe7d11f30..964b8ffe95 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -8,6 +8,10 @@ use pretty_assertions::{assert_eq, assert_ne}; use rstest::rstest; use starknet_api::block::GasPrice; use starknet_api::core::{calculate_contract_address, ClassHash, ContractAddress}; +use starknet_api::executable_transaction::{ + DeclareTransaction as ApiExecutableDeclareTransaction, + Transaction as ApiExecutableTransaction, +}; use starknet_api::execution_resources::GasAmount; use starknet_api::hash::StarkHash; use starknet_api::state::StorageKey; @@ -17,6 +21,7 @@ use starknet_api::transaction::{ AllResourceBounds, Calldata, ContractAddressSalt, + DeclareTransaction, DeclareTransactionV2, Fee, GasVectorComputationMode, @@ -96,7 +101,7 @@ use crate::transaction::test_utils::{ INVALID, }; use crate::transaction::transaction_types::TransactionType; -use crate::transaction::transactions::{DeclareTransaction, ExecutableTransaction, ExecutionFlags}; +use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags}; use crate::utils::u64_from_usize; #[rstest] @@ -718,8 +723,8 @@ fn test_fail_deploy_account( }); let fee_token_address = chain_info.fee_token_address(&deploy_account_tx.fee_type()); - let deploy_address = match &deploy_account_tx { - AccountTransaction::DeployAccount(deploy_tx) => deploy_tx.contract_address(), + let deploy_address = match &deploy_account_tx.tx { + ApiExecutableTransaction::DeployAccount(deploy_tx) => deploy_tx.contract_address(), _ => unreachable!("deploy_account_tx is a DeployAccount"), }; fund_account(chain_info, deploy_address, Fee(BALANCE.0 * 2), &mut state.state); @@ -750,26 +755,23 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) { let next_nonce = nonce_manager.next(account_address); // Cannot fail executing a declare tx unless it's V2 or above, and already declared. - let declare_tx = DeclareTransactionV2 { + let declare_tx_v2 = DeclareTransactionV2 { max_fee, class_hash, sender_address: account_address, ..Default::default() }; state.set_contract_class(class_hash, contract_class.clone().try_into().unwrap()).unwrap(); - state.set_compiled_class_hash(class_hash, declare_tx.compiled_class_hash).unwrap(); + state.set_compiled_class_hash(class_hash, declare_tx_v2.compiled_class_hash).unwrap(); let class_info = calculate_class_info_for_testing(contract_class); - let declare_account_tx = AccountTransaction::Declare( - DeclareTransaction::new( - starknet_api::transaction::DeclareTransaction::V2(DeclareTransactionV2 { - nonce: next_nonce, - ..declare_tx - }), - TransactionHash::default(), - class_info, - ) - .unwrap(), - ); + let declare_tx = + DeclareTransaction::V2(DeclareTransactionV2 { nonce: next_nonce, ..declare_tx_v2 }); + let executable_tx = ApiExecutableTransaction::Declare(ApiExecutableDeclareTransaction { + tx: declare_tx, + tx_hash: TransactionHash::default(), + class_info, + }); + let declare_account_tx = AccountTransaction::new(executable_tx); // Fail execution, assert nonce and balance are unchanged. let tx_info = declare_account_tx.create_tx_info(); diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index b5724df920..d619e9a287 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -3,7 +3,13 @@ use std::sync::Arc; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use starknet_api::contract_class::ClassInfo; use starknet_api::core::{calculate_contract_address, ContractAddress, Nonce}; -use starknet_api::executable_transaction::L1HandlerTransaction; +use starknet_api::executable_transaction::{ + DeclareTransaction, + DeployAccountTransaction, + InvokeTransaction, + L1HandlerTransaction, + Transaction as ApiExecutableTransaction, +}; use starknet_api::transaction::{Fee, Transaction as StarknetApiTransaction, TransactionHash}; use crate::bouncer::verify_tx_weights_within_max_capacity; @@ -21,14 +27,7 @@ use crate::transaction::objects::{ TransactionInfo, TransactionInfoCreator, }; -use crate::transaction::transactions::{ - DeclareTransaction, - DeployAccountTransaction, - Executable, - ExecutableTransaction, - ExecutionFlags, - InvokeTransaction, -}; +use crate::transaction::transactions::{Executable, ExecutableTransaction, ExecutionFlags}; // TODO: Move into transaction.rs, makes more sense to be defined there. #[derive(Clone, Debug, derive_more::From)] @@ -67,25 +66,24 @@ impl Transaction { deployed_contract_address: Option, only_query: bool, ) -> TransactionExecutionResult { - match tx { + let executable_tx = match tx { StarknetApiTransaction::L1Handler(l1_handler) => { - Ok(Self::L1Handler(L1HandlerTransaction { + return Ok(Self::L1Handler(L1HandlerTransaction { tx: l1_handler, tx_hash, paid_fee_on_l1: paid_fee_on_l1 .expect("L1Handler should be created with the fee paid on L1"), - })) + })); } StarknetApiTransaction::Declare(declare) => { let non_optional_class_info = class_info.expect("Declare should be created with a ClassInfo."); - let declare_tx = match only_query { - true => { - DeclareTransaction::new_for_query(declare, tx_hash, non_optional_class_info) - } - false => DeclareTransaction::new(declare, tx_hash, non_optional_class_info), - }; - Ok(declare_tx?.into()) + + ApiExecutableTransaction::Declare(DeclareTransaction { + tx: declare, + tx_hash, + class_info: non_optional_class_info, + }) } StarknetApiTransaction::DeployAccount(deploy_account) => { let contract_address = match deployed_contract_address { @@ -97,27 +95,22 @@ impl Transaction { ContractAddress::default(), )?, }; - let deploy_account_tx = match only_query { - true => DeployAccountTransaction::new_for_query( - deploy_account, - tx_hash, - contract_address, - ), - false => { - DeployAccountTransaction::new(deploy_account, tx_hash, contract_address) - } - }; - Ok(deploy_account_tx.into()) + ApiExecutableTransaction::DeployAccount(DeployAccountTransaction { + tx: deploy_account, + tx_hash, + contract_address, + }) } StarknetApiTransaction::Invoke(invoke) => { - let invoke_tx = match only_query { - true => InvokeTransaction::new_for_query(invoke, tx_hash), - false => InvokeTransaction::new(invoke, tx_hash), - }; - Ok(invoke_tx.into()) + ApiExecutableTransaction::Invoke(InvokeTransaction { tx: invoke, tx_hash }) } _ => unimplemented!(), - } + }; + let account_tx = match only_query { + true => AccountTransaction::new_for_query(executable_tx), + false => AccountTransaction::new(executable_tx), + }; + Ok(account_tx.into()) } } @@ -220,21 +213,3 @@ impl ExecutableTransaction for Transaction { Ok(tx_execution_info) } } - -impl From for Transaction { - fn from(value: DeclareTransaction) -> Self { - Self::Account(AccountTransaction::Declare(value)) - } -} - -impl From for Transaction { - fn from(value: DeployAccountTransaction) -> Self { - Self::Account(AccountTransaction::DeployAccount(value)) - } -} - -impl From for Transaction { - fn from(value: InvokeTransaction) -> Self { - Self::Account(AccountTransaction::Invoke(value)) - } -} diff --git a/crates/gateway/src/stateful_transaction_validator.rs b/crates/gateway/src/stateful_transaction_validator.rs index 51694eb868..16265c1b4b 100644 --- a/crates/gateway/src/stateful_transaction_validator.rs +++ b/crates/gateway/src/stateful_transaction_validator.rs @@ -75,10 +75,7 @@ impl StatefulTransactionValidator { mut validator: V, ) -> StatefulTransactionValidatorResult<()> { let skip_validate = skip_stateful_validations(executable_tx, account_nonce); - let account_tx = AccountTransaction::try_from(executable_tx).map_err(|error| { - error!("Failed to convert executable transaction into account transaction: {}", error); - GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } - })?; + let account_tx = AccountTransaction::new(executable_tx.clone()); validator .validate(account_tx, skip_validate) .map_err(|err| GatewaySpecError::ValidationFailure { data: err.to_string() })?; diff --git a/crates/native_blockifier/src/py_declare.rs b/crates/native_blockifier/src/py_declare.rs index b8d74535f0..75acb321ac 100644 --- a/crates/native_blockifier/src/py_declare.rs +++ b/crates/native_blockifier/src/py_declare.rs @@ -1,10 +1,10 @@ use std::convert::TryFrom; use blockifier::transaction::transaction_types::TransactionType; -use blockifier::transaction::transactions::DeclareTransaction; use pyo3::prelude::*; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; +use starknet_api::executable_transaction::DeclareTransaction; use starknet_api::transaction::{ AccountDeploymentData, DeclareTransactionV0V1, @@ -136,5 +136,5 @@ pub fn py_declare( }?; let tx_hash = TransactionHash(py_attr::(py_tx, "hash_value")?.0); let class_info = PyClassInfo::try_from(py_class_info, &tx)?; - Ok(DeclareTransaction::new(tx, tx_hash, class_info)?) + Ok(DeclareTransaction { tx, tx_hash, class_info }) } diff --git a/crates/native_blockifier/src/py_deploy_account.rs b/crates/native_blockifier/src/py_deploy_account.rs index b25b464103..53ef9442b2 100644 --- a/crates/native_blockifier/src/py_deploy_account.rs +++ b/crates/native_blockifier/src/py_deploy_account.rs @@ -1,10 +1,10 @@ use std::sync::Arc; use blockifier::transaction::transaction_types::TransactionType; -use blockifier::transaction::transactions::DeployAccountTransaction; use pyo3::prelude::*; use starknet_api::core::{ClassHash, ContractAddress, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; +use starknet_api::executable_transaction::DeployAccountTransaction; use starknet_api::transaction::{ Calldata, ContractAddressSalt, @@ -100,5 +100,5 @@ pub fn py_deploy_account(py_tx: &PyAny) -> NativeBlockifierResult(py_tx, "hash_value")?.0); let contract_address = ContractAddress::try_from(py_attr::(py_tx, "sender_address")?.0)?; - Ok(DeployAccountTransaction::new(tx, tx_hash, contract_address)) + Ok(DeployAccountTransaction { tx, tx_hash, contract_address }) } diff --git a/crates/native_blockifier/src/py_invoke_function.rs b/crates/native_blockifier/src/py_invoke_function.rs index 974d0b9b1d..dbc96f4c70 100644 --- a/crates/native_blockifier/src/py_invoke_function.rs +++ b/crates/native_blockifier/src/py_invoke_function.rs @@ -2,10 +2,10 @@ use std::convert::TryFrom; use std::sync::Arc; use blockifier::transaction::transaction_types::TransactionType; -use blockifier::transaction::transactions::InvokeTransaction; use pyo3::prelude::*; use starknet_api::core::{ContractAddress, EntryPointSelector, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; +use starknet_api::executable_transaction::InvokeTransaction; use starknet_api::transaction::{ AccountDeploymentData, Calldata, @@ -127,5 +127,5 @@ pub fn py_invoke_function(py_tx: &PyAny) -> NativeBlockifierResult(py_tx, "hash_value")?.0); - Ok(InvokeTransaction::new(tx, tx_hash)) + Ok(InvokeTransaction { tx, tx_hash }) } diff --git a/crates/native_blockifier/src/py_transaction.rs b/crates/native_blockifier/src/py_transaction.rs index ce73f14a4f..658cc61102 100644 --- a/crates/native_blockifier/src/py_transaction.rs +++ b/crates/native_blockifier/src/py_transaction.rs @@ -7,6 +7,7 @@ use pyo3::exceptions::PyValueError; use pyo3::prelude::*; use starknet_api::block::GasPrice; use starknet_api::contract_class::{ClassInfo, ContractClass}; +use starknet_api::executable_transaction::Transaction as ExecutableTransaction; use starknet_api::execution_resources::GasAmount; use starknet_api::transaction::{ DeprecatedResourceBoundsMapping, @@ -137,13 +138,18 @@ pub fn py_tx( TransactionType::Declare => { let non_optional_py_class_info: PyClassInfo = optional_py_class_info .expect("A class info must be passed in a Declare transaction."); - AccountTransaction::Declare(py_declare(tx, non_optional_py_class_info)?).into() + AccountTransaction::new(ExecutableTransaction::Declare(py_declare( + tx, + non_optional_py_class_info, + )?)) + .into() } TransactionType::DeployAccount => { - AccountTransaction::DeployAccount(py_deploy_account(tx)?).into() + AccountTransaction::new(ExecutableTransaction::DeployAccount(py_deploy_account(tx)?)) + .into() } TransactionType::InvokeFunction => { - AccountTransaction::Invoke(py_invoke_function(tx)?).into() + AccountTransaction::new(ExecutableTransaction::Invoke(py_invoke_function(tx)?)).into() } TransactionType::L1Handler => py_l1_handler(tx)?.into(), })