From 4fd1f29249de852c86a492c482632fa77381642f Mon Sep 17 00:00:00 2001 From: Aviv Greenburg Date: Thu, 28 Nov 2024 13:54:57 +0200 Subject: [PATCH] chore(blockifier): explicit creation of AccountTransaction --- crates/blockifier/src/test_utils/declare.rs | 3 +- .../src/test_utils/deploy_account.rs | 6 ++- crates/blockifier/src/test_utils/invoke.rs | 5 +-- .../src/transaction/account_transaction.rs | 41 +------------------ .../transaction/account_transactions_test.rs | 9 ++-- .../src/transaction/transaction_execution.rs | 8 +--- .../native_blockifier/src/py_transaction.rs | 21 ++++++---- crates/starknet_batcher/src/block_builder.rs | 19 ++++++++- .../src/stateful_transaction_validator.rs | 2 +- 9 files changed, 48 insertions(+), 66 deletions(-) diff --git a/crates/blockifier/src/test_utils/declare.rs b/crates/blockifier/src/test_utils/declare.rs index aaaf9b8e24..5ac24e5b49 100644 --- a/crates/blockifier/src/test_utils/declare.rs +++ b/crates/blockifier/src/test_utils/declare.rs @@ -1,4 +1,5 @@ use starknet_api::contract_class::ClassInfo; +use starknet_api::executable_transaction::AccountTransaction as ExecutableTransaction; use starknet_api::test_utils::declare::{executable_declare_tx, DeclareTxArgs}; use crate::transaction::account_transaction::AccountTransaction; @@ -6,5 +7,5 @@ use crate::transaction::account_transaction::AccountTransaction; pub fn declare_tx(declare_tx_args: DeclareTxArgs, class_info: ClassInfo) -> AccountTransaction { let declare_tx = executable_declare_tx(declare_tx_args, class_info); - declare_tx.into() + AccountTransaction { tx: ExecutableTransaction::Declare(declare_tx), only_query: false } } diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index e2d6ed36fe..1e40d4e188 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/crates/blockifier/src/test_utils/deploy_account.rs @@ -1,3 +1,4 @@ +use starknet_api::executable_transaction::AccountTransaction as ExecutableTransaction; use starknet_api::test_utils::deploy_account::{executable_deploy_account_tx, DeployAccountTxArgs}; use starknet_api::test_utils::NonceManager; @@ -9,5 +10,8 @@ pub fn deploy_account_tx( ) -> AccountTransaction { let deploy_account_tx = executable_deploy_account_tx(deploy_tx_args, nonce_manager); - deploy_account_tx.into() + AccountTransaction { + tx: ExecutableTransaction::DeployAccount(deploy_account_tx), + only_query: false, + } } diff --git a/crates/blockifier/src/test_utils/invoke.rs b/crates/blockifier/src/test_utils/invoke.rs index 569ca98861..b4e4b3a530 100644 --- a/crates/blockifier/src/test_utils/invoke.rs +++ b/crates/blockifier/src/test_utils/invoke.rs @@ -7,8 +7,5 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction { let only_query = invoke_args.only_query; let invoke_tx = ExecutableTransaction::Invoke(executable_invoke_tx(invoke_args)); - match only_query { - true => AccountTransaction::new_for_query(invoke_tx), - false => AccountTransaction::new(invoke_tx), - } + AccountTransaction { tx: invoke_tx, only_query } } diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index ca61f75576..2c06fda963 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -6,12 +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::{ - AccountTransaction as Transaction, - DeclareTransaction, - DeployAccountTransaction, - InvokeTransaction, -}; +use starknet_api::executable_transaction::AccountTransaction as Transaction; use starknet_api::transaction::fields::Resource::{L1DataGas, L1Gas, L2Gas}; use starknet_api::transaction::fields::{ AccountDeploymentData, @@ -86,7 +81,7 @@ mod post_execution_test; #[derive(Clone, Debug, derive_more::From)] pub struct AccountTransaction { pub tx: Transaction, - only_query: bool, + pub only_query: bool, } // TODO(AvivG): create additional macro that returns a reference. macro_rules! implement_tx_getter_calls { @@ -97,30 +92,6 @@ macro_rules! implement_tx_getter_calls { }; } -impl From for AccountTransaction { - fn from(tx: Transaction) -> Self { - Self::new(tx) - } -} - -impl From for AccountTransaction { - fn from(tx: DeclareTransaction) -> Self { - Transaction::Declare(tx).into() - } -} - -impl From for AccountTransaction { - fn from(tx: DeployAccountTransaction) -> Self { - Transaction::DeployAccount(tx).into() - } -} - -impl From for AccountTransaction { - fn from(tx: InvokeTransaction) -> Self { - Transaction::Invoke(tx).into() - } -} - impl HasRelatedFeeType for AccountTransaction { fn version(&self) -> TransactionVersion { self.tx.version() @@ -144,14 +115,6 @@ 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 class_hash(&self) -> Option { match &self.tx { Transaction::Declare(tx) => Some(tx.tx.class_hash()), diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index f677ada05f..b7eca58eaf 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -775,12 +775,15 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) { state.set_contract_class(class_hash, contract_class.clone().try_into().unwrap()).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 = ApiExecutableDeclareTransaction { + let executable_declare = ApiExecutableDeclareTransaction { tx: DeclareTransaction::V2(DeclareTransactionV2 { nonce: next_nonce, ..declare_tx_v2 }), tx_hash: TransactionHash::default(), class_info, - } - .into(); + }; + let declare_account_tx = AccountTransaction { + tx: ApiExecutableTransaction::Declare(executable_declare), + only_query: false, + }; // 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 864267d8a0..d305e0bdb8 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -40,7 +40,7 @@ impl From for Transaction { fn from(value: starknet_api::executable_transaction::Transaction) -> Self { match value { starknet_api::executable_transaction::Transaction::Account(tx) => { - Transaction::Account(tx.into()) + Transaction::Account(AccountTransaction { tx, only_query: false }) } starknet_api::executable_transaction::Transaction::L1Handler(tx) => { Transaction::L1Handler(tx) @@ -119,11 +119,7 @@ impl Transaction { } _ => unimplemented!(), }; - let account_tx = match only_query { - true => AccountTransaction::new_for_query(executable_tx), - false => AccountTransaction::new(executable_tx), - }; - Ok(account_tx.into()) + Ok(AccountTransaction { tx: executable_tx, only_query }.into()) } } diff --git a/crates/native_blockifier/src/py_transaction.rs b/crates/native_blockifier/src/py_transaction.rs index bdf28426b0..d012c0fa16 100644 --- a/crates/native_blockifier/src/py_transaction.rs +++ b/crates/native_blockifier/src/py_transaction.rs @@ -138,19 +138,22 @@ 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::new(ExecutableTransaction::Declare(py_declare( - tx, - non_optional_py_class_info, - )?)) + AccountTransaction { + tx: ExecutableTransaction::Declare(py_declare(tx, non_optional_py_class_info)?), + only_query: false, + } .into() } - TransactionType::DeployAccount => { - AccountTransaction::new(ExecutableTransaction::DeployAccount(py_deploy_account(tx)?)) - .into() + TransactionType::DeployAccount => AccountTransaction { + tx: ExecutableTransaction::DeployAccount(py_deploy_account(tx)?), + only_query: false, } - TransactionType::InvokeFunction => { - AccountTransaction::new(ExecutableTransaction::Invoke(py_invoke_function(tx)?)).into() + .into(), + TransactionType::InvokeFunction => AccountTransaction { + tx: ExecutableTransaction::Invoke(py_invoke_function(tx)?), + only_query: false, } + .into(), TransactionType::L1Handler => py_l1_handler(tx)?.into(), }) } diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index cbe767fd9a..0f1300ba2e 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -15,6 +15,7 @@ use blockifier::execution::contract_class::RunnableCompiledClass; use blockifier::state::cached_state::CommitmentStateDiff; use blockifier::state::errors::StateError; use blockifier::state::global_cache::GlobalContractCache; +use blockifier::transaction::account_transaction::AccountTransaction; use blockifier::transaction::objects::TransactionExecutionInfo; use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction; use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsOverrides}; @@ -159,8 +160,22 @@ 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::from(tx.clone())); + // TODO(AvivG): Create a 'from' tx to BlockifierTransaction to simplify & remove + // 'match'. + let executable_tx = match tx { + Transaction::Account(account_tx) => { + BlockifierTransaction::Account(AccountTransaction { + // TODO(yair): Avoid this clone. + tx: account_tx.clone(), + only_query: false, + }) + } + Transaction::L1Handler(l1_handler_tx) => { + // TODO(yair): Avoid this clone. + BlockifierTransaction::L1Handler(l1_handler_tx.clone()) + } + }; + executor_input_chunk.push(executable_tx); } let results = self.executor.add_txs_to_block(&executor_input_chunk); trace!("Transaction execution results: {:?}", results); diff --git a/crates/starknet_gateway/src/stateful_transaction_validator.rs b/crates/starknet_gateway/src/stateful_transaction_validator.rs index db04f52c79..0674b8cd4b 100644 --- a/crates/starknet_gateway/src/stateful_transaction_validator.rs +++ b/crates/starknet_gateway/src/stateful_transaction_validator.rs @@ -75,7 +75,7 @@ impl StatefulTransactionValidator { mut validator: V, ) -> StatefulTransactionValidatorResult<()> { let skip_validate = skip_stateful_validations(executable_tx, account_nonce); - let account_tx = AccountTransaction::new(executable_tx.clone()); + let account_tx = AccountTransaction { tx: executable_tx.clone(), only_query: false }; validator .validate(account_tx, skip_validate) .map_err(|err| GatewaySpecError::ValidationFailure { data: err.to_string() })?;