Skip to content

Commit

Permalink
refactor(blockifier): account_tx from enum to struct w api::executabl…
Browse files Browse the repository at this point in the history
…e_tx (#1688)
  • Loading branch information
avivg-starkware authored Nov 12, 2024
1 parent d75daef commit 5911b9c
Show file tree
Hide file tree
Showing 17 changed files with 205 additions and 233 deletions.
2 changes: 1 addition & 1 deletion crates/batcher/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion crates/blockifier/src/blockifier/stateful_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -61,7 +62,7 @@ impl<S: StateReader> StatefulValidator<S> {
// 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(());
}
Expand Down
8 changes: 4 additions & 4 deletions crates/blockifier/src/blockifier/stateful_validator_test.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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());
}

Expand Down
10 changes: 5 additions & 5 deletions crates/blockifier/src/execution/stack_trace_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions crates/blockifier/src/fee/gas_usage.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions crates/blockifier/src/test_utils/declare.rs
Original file line number Diff line number Diff line change
@@ -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()
}
9 changes: 3 additions & 6 deletions crates/blockifier/src/test_utils/deploy_account.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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()
}
21 changes: 13 additions & 8 deletions crates/blockifier/src/test_utils/invoke.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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),
}
}
Loading

0 comments on commit 5911b9c

Please sign in to comment.