Skip to content

Commit

Permalink
chore(blockifier): explicit creation of AccountTransaction (#2331)
Browse files Browse the repository at this point in the history
  • Loading branch information
avivg-starkware authored Dec 3, 2024
1 parent ea7fdad commit c34c59b
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 66 deletions.
3 changes: 2 additions & 1 deletion crates/blockifier/src/test_utils/declare.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
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;

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 }
}
6 changes: 5 additions & 1 deletion crates/blockifier/src/test_utils/deploy_account.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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,
}
}
5 changes: 1 addition & 4 deletions crates/blockifier/src/test_utils/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
41 changes: 2 additions & 39 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -97,30 +92,6 @@ macro_rules! implement_tx_getter_calls {
};
}

impl From<Transaction> for AccountTransaction {
fn from(tx: Transaction) -> Self {
Self::new(tx)
}
}

impl From<DeclareTransaction> for AccountTransaction {
fn from(tx: DeclareTransaction) -> Self {
Transaction::Declare(tx).into()
}
}

impl From<DeployAccountTransaction> for AccountTransaction {
fn from(tx: DeployAccountTransaction) -> Self {
Transaction::DeployAccount(tx).into()
}
}

impl From<InvokeTransaction> for AccountTransaction {
fn from(tx: InvokeTransaction) -> Self {
Transaction::Invoke(tx).into()
}
}

impl HasRelatedFeeType for AccountTransaction {
fn version(&self) -> TransactionVersion {
self.tx.version()
Expand All @@ -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<ClassHash> {
match &self.tx {
Transaction::Declare(tx) => Some(tx.tx.class_hash()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 2 additions & 6 deletions crates/blockifier/src/transaction/transaction_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl From<starknet_api::executable_transaction::Transaction> 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)
Expand Down Expand Up @@ -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())
}
}

Expand Down
21 changes: 12 additions & 9 deletions crates/native_blockifier/src/py_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
}
Expand Down
19 changes: 17 additions & 2 deletions crates/starknet_batcher/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() })?;
Expand Down

0 comments on commit c34c59b

Please sign in to comment.