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 Oct 29, 2024
1 parent b35e972 commit 8e13fb4
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 54 deletions.
9 changes: 3 additions & 6 deletions crates/blockifier/src/concurrency/versioned_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,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 @@ -236,7 +235,7 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
let mut state_2 = TransactionalState::create_transactional(&mut versioned_state_proxy_2);

// Prepare transactions
let deploy_account_tx_1 = deploy_account_tx(
let account_tx_1 = deploy_account_tx(
deploy_account_tx_args! {
class_hash: account_without_validation.get_class_hash(),
resource_bounds: l1_resource_bounds(
Expand All @@ -246,7 +245,6 @@ 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 class_hash = grindy_account.get_class_hash();
Expand All @@ -259,9 +257,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 Down
9 changes: 7 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,9 @@ pub fn deploy_account_tx(
deploy_tx_args,
nonce_manager.next(contract_address),
);
DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address)
AccountTransaction::DeployAccount(DeployAccountTransaction::new(
deploy_account_tx,
tx_hash,
contract_address,
))
}
7 changes: 4 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 @@ -23,7 +24,7 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
};

match only_query {
true => InvokeTransaction::new_for_query(invoke_tx, tx_hash),
false => InvokeTransaction::new(invoke_tx, tx_hash),
true => AccountTransaction::Invoke(InvokeTransaction::new_for_query(invoke_tx, tx_hash)),
false => AccountTransaction::Invoke(InvokeTransaction::new(invoke_tx, tx_hash)),
}
}
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
12 changes: 7 additions & 5 deletions 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 All @@ -228,13 +228,15 @@ impl AccountTransaction {
}

pub fn calldata_length(&self) -> usize {
let calldata = match self {
self.calldata().0.len()
}

pub fn calldata(&self) -> Calldata {
match self {
Self::Declare(_tx) => calldata![],
Self::DeployAccount(tx) => tx.constructor_calldata(),
Self::Invoke(tx) => tx.calldata(),
};

calldata.0.len()
}
}

pub fn signature_length(&self) -> usize {
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 @@ -138,9 +138,8 @@ pub fn deploy_and_fund_account(
deploy_tx_args: DeployAccountTxArgs,
) -> (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_tx = deploy_account_tx(deploy_tx_args, nonce_manager);
let account_address = 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 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
43 changes: 18 additions & 25 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,17 +430,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 invoke_tx = invoke_tx(invoke_tx_args! {
let account_tx = invoke_tx(invoke_tx_args! {
sender_address: account_contract_address,
calldata: create_trivial_calldata(test_contract_address),
resource_bounds: default_l1_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 = Calldata(Arc::clone(&account_tx.calldata().0)); //Aviv how to access?
let calldata_length = account_tx.calldata_length();
let signature_length = account_tx.signature_length();
let state_changes_for_fee = StateChangesCount {
n_storage_updates: 1,
n_modified_contracts: 1,
Expand All @@ -454,9 +454,9 @@ fn test_invoke_tx(
None,
ExecutionSummary::default(),
);
let sender_address = invoke_tx.sender_address();
let sender_address = account_tx.sender_address();

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

let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
Expand Down Expand Up @@ -941,13 +941,13 @@ fn test_max_fee_exceeds_balance(
assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx);

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

// Declare.
Expand Down Expand Up @@ -1531,17 +1531,15 @@ fn test_deploy_account_tx(
let account = FeatureContract::AccountWithoutValidations(cairo_version);
let account_class_hash = account.get_class_hash();
let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]);
let deploy_account = deploy_account_tx(
let account_tx = deploy_account_tx(
deploy_account_tx_args! { resource_bounds: default_l1_resource_bounds, class_hash: account_class_hash },
&mut nonce_manager,
);

// 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 = account_tx.class_hash().unwrap();
let deployed_account_address = 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 @@ -1556,20 +1554,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();

// Build expected validate call info.
let validate_calldata =
[vec![class_hash.0, salt.0], (*constructor_calldata.0).clone()].concat();
let validate_calldata = account_tx.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 @@ -1674,11 +1670,10 @@ fn test_deploy_account_tx(

// Negative flow.
// Deploy to an existing address.
let deploy_account = deploy_account_tx(
let account_tx = deploy_account_tx(
deploy_account_tx_args! { resource_bounds: default_l1_resource_bounds, class_hash: account_class_hash },
&mut nonce_manager,
);
let account_tx = AccountTransaction::DeployAccount(deploy_account);
let error = account_tx.execute(state, block_context, true, true).unwrap_err();
assert_matches!(
error,
Expand All @@ -1703,23 +1698,22 @@ fn test_fail_deploy_account_undeclared_class_hash(
let state = &mut test_state(chain_info, BALANCE, &[]);
let mut nonce_manager = NonceManager::default();
let undeclared_hash = class_hash!("0xdeadbeef");
let deploy_account = deploy_account_tx(
let account_tx = deploy_account_tx(
deploy_account_tx_args! {resource_bounds: default_l1_resource_bounds, class_hash: undeclared_hash },
&mut nonce_manager,
);
let tx_context = block_context.to_tx_context(&deploy_account);
let tx_context = block_context.to_tx_context(&account_tx);
let fee_type = tx_context.tx_info.fee_type();

// Fund account, so as not to fail pre-validation.
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(account_tx.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();
assert_matches!(
error,
Expand Down Expand Up @@ -2044,13 +2038,12 @@ fn test_only_query_flag(
.concat()
.into(),
);
let invoke_tx = crate::test_utils::invoke::invoke_tx(invoke_tx_args! {
let account_tx = crate::test_utils::invoke::invoke_tx(invoke_tx_args! {
calldata: execute_calldata,
resource_bounds: default_l1_resource_bounds,
sender_address,
only_query,
});
let account_tx = AccountTransaction::Invoke(invoke_tx);

let tx_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
assert!(!tx_execution_info.is_reverted())
Expand Down

0 comments on commit 8e13fb4

Please sign in to comment.