Skip to content

Commit

Permalink
refactor(blockifier): change return type of invoke_tx deploy_account_…
Browse files Browse the repository at this point in the history
…tx to account_transaction
  • Loading branch information
avivg-starkware committed Nov 5, 2024
1 parent f3a7c10 commit 2cd23a7
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 55 deletions.
12 changes: 5 additions & 7 deletions crates/blockifier/src/concurrency/versioned_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ use crate::test_utils::deploy_account::deploy_account_tx;
use crate::test_utils::dict_state_reader::DictStateReader;
use crate::test_utils::initial_test_state::test_state;
use crate::test_utils::{CairoVersion, BALANCE, DEFAULT_STRK_L1_GAS_PRICE};
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::objects::HasRelatedFeeType;
use crate::transaction::test_utils::{default_all_resource_bounds, l1_resource_bounds};
use crate::transaction::transactions::ExecutableTransaction;
Expand Down Expand Up @@ -239,8 +238,7 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
},
&mut NonceManager::default(),
);
let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1);
let enforce_fee = account_tx_1.enforce_fee();
let enforce_fee = deploy_account_tx_1.enforce_fee();

let class_hash = grindy_account.get_class_hash();
let ctor_storage_arg = felt!(1_u8);
Expand All @@ -252,9 +250,8 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
constructor_calldata: constructor_calldata.clone(),
};
let nonce_manager = &mut NonceManager::default();
let deploy_account_tx_2 = deploy_account_tx(deploy_tx_args, nonce_manager);
let account_address = deploy_account_tx_2.contract_address();
let account_tx_2 = AccountTransaction::DeployAccount(deploy_account_tx_2);
let account_tx_2 = deploy_account_tx(deploy_tx_args, nonce_manager);
let account_address = account_tx_2.sender_address();
let tx_context = block_context.to_tx_context(&account_tx_2);
let fee_type = tx_context.tx_info.fee_type();

Expand All @@ -270,7 +267,8 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
// Execute transactions
thread::scope(|s| {
s.spawn(move || {
let result = account_tx_1.execute(&mut state_1, &block_context_1, enforce_fee, true);
let result =
deploy_account_tx_1.execute(&mut state_1, &block_context_1, enforce_fee, true);
assert_eq!(result.is_err(), enforce_fee); // Transaction fails iff we enforced the fee charge (as the acount is not funded).
});
s.spawn(move || {
Expand Down
7 changes: 5 additions & 2 deletions crates/blockifier/src/test_utils/deploy_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ use starknet_api::core::calculate_contract_address;
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,
nonce_manager: &mut NonceManager,
) -> DeployAccountTransaction {
) -> AccountTransaction {
let tx_hash = deploy_tx_args.tx_hash;
let contract_address = calculate_contract_address(
deploy_tx_args.contract_address_salt,
Expand All @@ -21,5 +22,7 @@ pub fn deploy_account_tx(
deploy_tx_args,
nonce_manager.next(contract_address),
);
DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address)
let executable_deploy_account_tx =
DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address);
AccountTransaction::DeployAccount(executable_deploy_account_tx)
}
8 changes: 5 additions & 3 deletions crates/blockifier/src/test_utils/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ use starknet_api::test_utils::invoke::InvokeTxArgs;
use starknet_api::transaction::{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) -> 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.
Expand All @@ -22,8 +23,9 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
starknet_api::test_utils::invoke::invoke_tx(invoke_args)
};

match only_query {
let invoke_tx = match only_query {
true => InvokeTransaction::new_for_query(invoke_tx, tx_hash),
false => InvokeTransaction::new(invoke_tx, tx_hash),
}
};
AccountTransaction::Invoke(invoke_tx)
}
5 changes: 2 additions & 3 deletions crates/blockifier/src/test_utils/transfers_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,13 @@ impl TransfersGenerator {
felt!(0_u8) // Calldata: msb amount.
];

let tx = invoke_tx(invoke_tx_args! {
invoke_tx(invoke_tx_args! {
max_fee: self.config.max_fee,
sender_address,
calldata: execute_calldata,
version: self.config.tx_version,
nonce,
});
AccountTransaction::Invoke(tx)
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl AccountTransaction {

// Calldata for validation contains transaction fields that cannot be obtained by calling
// `et_tx_info()`.
fn validate_entrypoint_calldata(&self) -> Calldata {
pub fn validate_entrypoint_calldata(&self) -> Calldata {
match self {
Self::Declare(tx) => calldata![tx.class_hash().0],
Self::DeployAccount(tx) => Calldata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ fn test_fee_enforcement(
) {
let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo0);
let state = &mut test_state(&block_context.chain_info, BALANCE, &[(account, 1)]);
let deploy_account_tx = deploy_account_tx(
let account_tx = deploy_account_tx(
deploy_account_tx_args! {
class_hash: account.get_class_hash(),
max_fee: Fee(if zero_bounds { 0 } else { MAX_FEE.0 }),
Expand All @@ -205,7 +205,6 @@ fn test_fee_enforcement(
&mut NonceManager::default(),
);

let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
let enforce_fee = account_tx.create_tx_info().enforce_fee();
assert_ne!(zero_bounds, enforce_fee);
let result = account_tx.execute(state, &block_context, enforce_fee, true);
Expand Down
17 changes: 7 additions & 10 deletions crates/blockifier/src/transaction/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ pub fn deploy_and_fund_account(
) -> (AccountTransaction, ContractAddress) {
// Deploy an account contract.
let deploy_account_tx = deploy_account_tx(deploy_tx_args, nonce_manager);
let account_address = deploy_account_tx.contract_address();
let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
let account_address = deploy_account_tx.sender_address();

// Update the balance of the about-to-be deployed account contract in the erc20 contract, so it
// can pay for the transaction execution.
Expand All @@ -153,7 +152,7 @@ pub fn deploy_and_fund_account(
.unwrap();
}

(account_tx, account_address)
(deploy_account_tx, account_address)
}

/// Initializes a state and returns a `TestInitData` instance.
Expand Down Expand Up @@ -282,7 +281,7 @@ pub fn create_account_tx_for_validate_test(
true => constants::FELT_TRUE,
false => constants::FELT_FALSE,
})];
let deploy_account_tx = deploy_account_tx(
deploy_account_tx(
deploy_account_tx_args! {
max_fee,
resource_bounds,
Expand All @@ -293,28 +292,26 @@ pub fn create_account_tx_for_validate_test(
constructor_calldata,
},
nonce_manager,
);
AccountTransaction::DeployAccount(deploy_account_tx)
)
}
TransactionType::InvokeFunction => {
let execute_calldata = create_calldata(sender_address, "foo", &[]);
let invoke_tx = invoke_tx(invoke_tx_args! {
invoke_tx(invoke_tx_args! {
max_fee,
resource_bounds,
signature,
sender_address,
calldata: execute_calldata,
version: tx_version,
nonce: nonce_manager.next(sender_address),
});
AccountTransaction::Invoke(invoke_tx)
})
}
_ => panic!("{tx_type:?} is not an account transaction."),
}
}

pub fn account_invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction {
AccountTransaction::Invoke(invoke_tx(invoke_args))
invoke_tx(invoke_args)
}

pub fn run_invoke_tx(
Expand Down
46 changes: 19 additions & 27 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,17 +438,17 @@ fn test_invoke_tx(
let state = &mut test_state(chain_info, BALANCE, &[(account_contract, 1), (test_contract, 1)]);
let test_contract_address = test_contract.get_instance_address(0);
let account_contract_address = account_contract.get_instance_address(0);
let calldata = create_trivial_calldata(test_contract_address);
let invoke_tx = invoke_tx(invoke_tx_args! {
sender_address: account_contract_address,
calldata: create_trivial_calldata(test_contract_address),
calldata: Calldata(Arc::clone(&calldata.0)),
resource_bounds,
});

// Extract invoke transaction fields for testing, as it is consumed when creating an account
// transaction.
let calldata = Calldata(Arc::clone(&invoke_tx.calldata().0));
let calldata_length = invoke_tx.calldata().0.len();
let signature_length = invoke_tx.signature().0.len();
let calldata_length = invoke_tx.calldata_length();
let signature_length = invoke_tx.signature_length();
let state_changes_for_fee = StateChangesCount {
n_storage_updates: 1,
n_modified_contracts: 1,
Expand All @@ -464,10 +464,9 @@ fn test_invoke_tx(
);
let sender_address = invoke_tx.sender_address();

let account_tx = AccountTransaction::Invoke(invoke_tx);
let tx_context = block_context.to_tx_context(&account_tx);
let tx_context = block_context.to_tx_context(&invoke_tx);

let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
let actual_execution_info = invoke_tx.execute(state, block_context, true, true).unwrap();

let tracked_resource = account_contract
.get_runnable_class()
Expand Down Expand Up @@ -948,13 +947,13 @@ fn test_max_fee_exceeds_balance(
)};

// Deploy.
let invalid_tx = AccountTransaction::DeployAccount(deploy_account_tx(
let invalid_tx = deploy_account_tx(
deploy_account_tx_args! {
resource_bounds,
class_hash: test_contract.get_class_hash()
},
&mut NonceManager::default(),
));
);
assert_resource_bounds_exceed_balance_failure(state, block_context, invalid_tx);

// V1 Invoke.
Expand Down Expand Up @@ -1683,10 +1682,8 @@ fn test_deploy_account_tx(

// Extract deploy account transaction fields for testing, as it is consumed when creating an
// account transaction.
let class_hash = deploy_account.class_hash();
let deployed_account_address = deploy_account.contract_address();
let constructor_calldata = deploy_account.constructor_calldata();
let salt = deploy_account.contract_address_salt();
let class_hash = deploy_account.class_hash().unwrap();
let deployed_account_address = deploy_account.sender_address();
let user_initial_gas = user_initial_gas_from_bounds(default_all_resource_bounds);

// Update the balance of the about to be deployed account contract in the erc20 contract, so it
Expand All @@ -1702,20 +1699,18 @@ fn test_deploy_account_tx(
.unwrap();
}

let account_tx = AccountTransaction::DeployAccount(deploy_account);
let fee_type = &account_tx.fee_type();
let tx_context = &block_context.to_tx_context(&account_tx);
let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
let fee_type = &deploy_account.fee_type();
let tx_context = &block_context.to_tx_context(&deploy_account);
let actual_execution_info = deploy_account.execute(state, block_context, true, true).unwrap();

// Build expected validate call info.
let validate_calldata =
[vec![class_hash.0, salt.0], (*constructor_calldata.0).clone()].concat();
let validate_calldata = deploy_account.validate_entrypoint_calldata();
let expected_gas_consumed = 0;
let expected_validate_call_info = expected_validate_call_info(
account_class_hash,
constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME,
expected_gas_consumed,
Calldata(validate_calldata.into()),
validate_calldata,
deployed_account_address,
cairo_version,
account
Expand Down Expand Up @@ -1827,8 +1822,7 @@ fn test_deploy_account_tx(
},
&mut nonce_manager,
);
let account_tx = AccountTransaction::DeployAccount(deploy_account);
let error = account_tx.execute(state, block_context, true, true).unwrap_err();
let error = deploy_account.execute(state, block_context, true, true).unwrap_err();
assert_matches!(
error,
TransactionExecutionError::ContractConstructorExecutionFailed(
Expand Down Expand Up @@ -1865,13 +1859,12 @@ fn test_fail_deploy_account_undeclared_class_hash(
state
.set_storage_at(
chain_info.fee_token_address(&fee_type),
get_fee_token_var_address(deploy_account.contract_address()),
get_fee_token_var_address(deploy_account.sender_address()),
felt!(BALANCE.0),
)
.unwrap();

let account_tx = AccountTransaction::DeployAccount(deploy_account);
let error = account_tx.execute(state, block_context, true, true).unwrap_err();
let error = deploy_account.execute(state, block_context, true, true).unwrap_err();
assert_matches!(
error,
TransactionExecutionError::ContractConstructorExecutionFailed(
Expand Down Expand Up @@ -2204,9 +2197,8 @@ fn test_only_query_flag(
sender_address,
only_query,
});
let account_tx = AccountTransaction::Invoke(invoke_tx);

let tx_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
let tx_execution_info = invoke_tx.execute(state, block_context, true, true).unwrap();
assert_eq!(tx_execution_info.revert_error, None);
}

Expand Down

0 comments on commit 2cd23a7

Please sign in to comment.