diff --git a/crates/batcher/src/block_builder.rs b/crates/batcher/src/block_builder.rs index dc0590e21ef..40afe185834 100644 --- a/crates/batcher/src/block_builder.rs +++ b/crates/batcher/src/block_builder.rs @@ -132,7 +132,7 @@ impl BlockBuilderTrait for BlockBuilder { let mut executor_input_chunk = vec![]; for tx in &next_tx_chunk { // TODO(yair): Avoid this clone. - executor_input_chunk.push(BlockifierTransaction::try_from(tx.clone())?); + executor_input_chunk.push(BlockifierTransaction::from(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 14964e1a569..5dbba123a93 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::AccountTransaction 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 e1de6ee725f..7a0019dd3eb 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::AccountTransaction as Transaction; use starknet_api::transaction::fields::ValidResourceBounds; use starknet_api::transaction::TransactionVersion; @@ -8,7 +9,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, @@ -62,18 +62,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 ae3923cd0b2..3d7c9afaa5a 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::AccountTransaction as Transaction; use starknet_api::transaction::fields::{ 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, @@ -599,8 +599,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"), @@ -670,8 +670,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 c8d00ca06fe..7e68193a20c 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::AccountTransaction as Transaction; use starknet_api::execution_resources::GasVector; use starknet_api::transaction::fields::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 c4b37bb8426..6a9835bad1f 100644 --- a/crates/blockifier/src/test_utils/declare.rs +++ b/crates/blockifier/src/test_utils/declare.rs @@ -1,15 +1,17 @@ use starknet_api::contract_class::ClassInfo; +use starknet_api::executable_transaction::{AccountTransaction as Transaction, DeclareTransaction}; 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); // TODO(AvivG): use starknet_api::test_utils::declare::executable_declare_tx to // create executable_declare. - let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap(); - executable_declare.into() + let executable_tx = + Transaction::Declare(DeclareTransaction { tx: declare_tx, tx_hash, class_info }); + + executable_tx.into() } diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index e7f018930cf..c289cbc1884 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/crates/blockifier/src/test_utils/deploy_account.rs @@ -1,8 +1,8 @@ +use starknet_api::executable_transaction::AccountTransaction as 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, @@ -12,10 +12,7 @@ pub fn deploy_account_tx( deploy_tx_args, nonce_manager, ); + let executable_tx = Transaction::DeployAccount(deploy_account_tx); - // TODO(AvivG): use the "new" method. - let executable_deploy_account_tx = - DeployAccountTransaction { tx: deploy_account_tx, only_query: false }; - - executable_deploy_account_tx.into() + executable_tx.into() } diff --git a/crates/blockifier/src/test_utils/invoke.rs b/crates/blockifier/src/test_utils/invoke.rs index 2baf0877a2f..945f985adc0 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::{ + AccountTransaction as ExecutableTransaction, + InvokeTransaction as ExecutableInvokeTransaction, +}; 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, @@ -25,9 +28,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), - }; - invoke_tx.into() + let invoke_tx = + ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx: invoke_tx, tx_hash }); + + 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 f409bbad94c..18152788d2b 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -6,6 +6,12 @@ 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::{ + AccountTransaction as Transaction, + DeclareTransaction, + DeployAccountTransaction, + InvokeTransaction, +}; use starknet_api::transaction::fields::Resource::{L1DataGas, L1Gas, L2Gas}; use starknet_api::transaction::fields::{ AccountDeploymentData, @@ -50,16 +56,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, }; @@ -77,76 +81,55 @@ 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 { + // TODO(AvivG): Consider moving some of the logic to the Transaction enum. + 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::AccountTransaction> for AccountTransaction { - type Error = TransactionExecutionError; +impl From for AccountTransaction { + fn from(tx: Transaction) -> Self { + Self::new(tx) + } +} - fn try_from( - value: &starknet_api::executable_transaction::AccountTransaction, - ) -> Result { - match value { - starknet_api::executable_transaction::AccountTransaction::Declare(declare_tx) => { - Ok(Self::Declare(declare_tx.clone().try_into()?)) - } - starknet_api::executable_transaction::AccountTransaction::DeployAccount( - deploy_account_tx, - ) => Ok(Self::DeployAccount(DeployAccountTransaction { - tx: deploy_account_tx.clone(), - only_query: false, - })), - starknet_api::executable_transaction::AccountTransaction::Invoke(invoke_tx) => { - Ok(Self::Invoke(InvokeTransaction { tx: invoke_tx.clone(), only_query: false })) - } - } +impl From for AccountTransaction { + fn from(tx: DeclareTransaction) -> Self { + Transaction::Declare(tx).into() } } -impl TryFrom for AccountTransaction { - type Error = TransactionExecutionError; +impl From for AccountTransaction { + fn from(tx: DeployAccountTransaction) -> Self { + Transaction::DeployAccount(tx).into() + } +} - fn try_from( - executable_transaction: starknet_api::executable_transaction::AccountTransaction, - ) -> Result { - match executable_transaction { - starknet_api::executable_transaction::AccountTransaction::Declare(declare_tx) => { - Ok(Self::Declare(declare_tx.try_into()?)) - } - starknet_api::executable_transaction::AccountTransaction::DeployAccount( - deploy_account_tx, - ) => Ok(Self::DeployAccount(DeployAccountTransaction { - tx: deploy_account_tx, - only_query: false, - })), - starknet_api::executable_transaction::AccountTransaction::Invoke(invoke_tx) => { - Ok(Self::Invoke(InvokeTransaction { tx: invoke_tx, only_query: false })) - } - } +impl From for AccountTransaction { + fn from(tx: InvokeTransaction) -> Self { + Transaction::Invoke(tx).into() } } 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(), + // TODO(AvivG): Consider moving some of the logic to the Transaction enum. + match &self.tx { + Transaction::Declare(tx) => tx.tx.version(), + Transaction::DeployAccount(tx) => tx.tx.version(), + Transaction::Invoke(tx) => tx.tx.version(), } } @@ -166,44 +149,48 @@ impl AccountTransaction { (paymaster_data, PaymasterData) ); + pub fn new(tx: starknet_api::executable_transaction::AccountTransaction) -> Self { + AccountTransaction { tx, only_query: false } + } + + pub fn new_for_query(tx: starknet_api::executable_transaction::AccountTransaction) -> 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) } @@ -211,9 +198,9 @@ impl AccountTransaction { // Calldata for validation contains transaction fields that cannot be obtained by calling // `et_tx_info()`. 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(), @@ -222,36 +209,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 { @@ -259,9 +242,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, @@ -269,10 +252,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] } }; @@ -545,7 +528,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)); @@ -561,10 +544,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), } } @@ -579,7 +564,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. @@ -757,15 +742,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() @@ -855,10 +840,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 333d79c8e59..8548801fa8f 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::{ + AccountTransaction as ApiExecutableTransaction, + DeclareTransaction as ApiExecutableDeclareTransaction, +}; use starknet_api::execution_resources::GasAmount; use starknet_api::hash::StarkHash; use starknet_api::state::StorageKey; @@ -23,7 +27,12 @@ use starknet_api::transaction::fields::{ ResourceBounds, ValidResourceBounds, }; -use starknet_api::transaction::{DeclareTransactionV2, TransactionHash, TransactionVersion}; +use starknet_api::transaction::{ + DeclareTransaction, + DeclareTransactionV2, + TransactionHash, + TransactionVersion, +}; use starknet_api::{ calldata, class_hash, @@ -94,7 +103,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] @@ -716,8 +725,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); @@ -748,24 +757,20 @@ 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 = DeclareTransaction::new( - starknet_api::transaction::DeclareTransaction::V2(DeclareTransactionV2 { - nonce: next_nonce, - ..declare_tx - }), - TransactionHash::default(), + let declare_account_tx: AccountTransaction = ApiExecutableDeclareTransaction { + tx: DeclareTransaction::V2(DeclareTransactionV2 { nonce: next_nonce, ..declare_tx_v2 }), + tx_hash: TransactionHash::default(), class_info, - ) - .unwrap() + } .into(); // Fail execution, assert nonce and balance are unchanged. diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index 381d926b550..e077370b86c 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::{ + AccountTransaction as ApiExecutableTransaction, + DeclareTransaction, + DeployAccountTransaction, + InvokeTransaction, + L1HandlerTransaction, +}; use starknet_api::transaction::fields::Fee; use starknet_api::transaction::{Transaction as StarknetApiTransaction, TransactionHash}; @@ -22,14 +28,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)] @@ -38,17 +37,14 @@ pub enum Transaction { L1Handler(L1HandlerTransaction), } -impl TryFrom for Transaction { - type Error = super::errors::TransactionExecutionError; - fn try_from( - value: starknet_api::executable_transaction::Transaction, - ) -> Result { +impl From for Transaction { + fn from(value: starknet_api::executable_transaction::Transaction) -> Self { match value { starknet_api::executable_transaction::Transaction::Account(tx) => { - Ok(Transaction::Account(tx.try_into()?)) + Transaction::Account(tx.into()) } starknet_api::executable_transaction::Transaction::L1Handler(tx) => { - Ok(Transaction::L1Handler(tx)) + Transaction::L1Handler(tx) } } } @@ -84,25 +80,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 { @@ -114,27 +109,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()) } } @@ -237,21 +227,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/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index a5f216e2a54..3044e969ee1 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -11,6 +11,7 @@ use rstest::{fixture, rstest}; use starknet_api::block::GasPriceVector; use starknet_api::contract_class::EntryPointType; use starknet_api::core::{ChainId, ClassHash, ContractAddress, EthAddress, Nonce}; +use starknet_api::executable_transaction::AccountTransaction as ApiExecutableTransaction; use starknet_api::execution_resources::{GasAmount, GasVector}; use starknet_api::state::StorageKey; use starknet_api::test_utils::invoke::InvokeTxArgs; @@ -1728,8 +1729,8 @@ fn test_deploy_account_tx( // Build expected validate call info. // TODO(AvivG): When the AccountTransaction refactor is complete, simplify the creation of // `validate_calldata` by accessing the DeployAccountTransaction directly, without match. - let validate_calldata = match deploy_account { - AccountTransaction::DeployAccount(tx) => Calldata( + let validate_calldata = match &deploy_account.tx { + ApiExecutableTransaction::DeployAccount(tx) => Calldata( [ vec![tx.class_hash().0, tx.contract_address_salt().0], (*tx.constructor_calldata().0).clone(), @@ -1737,7 +1738,7 @@ fn test_deploy_account_tx( .concat() .into(), ), - AccountTransaction::Invoke(_) | AccountTransaction::Declare(_) => { + ApiExecutableTransaction::Invoke(_) | ApiExecutableTransaction::Declare(_) => { panic!("Expected DeployAccount transaction.") } }; diff --git a/crates/gateway/src/stateful_transaction_validator.rs b/crates/gateway/src/stateful_transaction_validator.rs index 6384ab039d1..2849606a8a7 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 6864d58fe1a..55cf53c4bef 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::fields::{ AccountDeploymentData, Fee, @@ -138,5 +138,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 805400a6cf5..9d4dbdb9301 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::fields::{ Calldata, ContractAddressSalt, @@ -102,5 +102,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 c1d572c22b3..bad90021caa 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::fields::{ AccountDeploymentData, Calldata, @@ -129,5 +129,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 3349bcfc5e5..bdf28426b08 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::AccountTransaction as ExecutableTransaction; use starknet_api::execution_resources::GasAmount; use starknet_api::transaction::fields::{ 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(), })