Skip to content

Commit

Permalink
refactor(fee): block context creator may fail
Browse files Browse the repository at this point in the history
  • Loading branch information
nimrod-starkware committed Aug 28, 2024
1 parent a658b50 commit a21e7a7
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 27 deletions.
4 changes: 2 additions & 2 deletions crates/blockifier/src/blockifier/stateful_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl<S: StateReader> StatefulValidator<S> {
return Ok(());
}

let tx_context = self.tx_executor.block_context.to_tx_context(&tx);
let tx_context = self.tx_executor.block_context.to_tx_context(&tx)?;
self.perform_pre_validation_stage(&tx, &tx_context)?;

if skip_validate {
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<S: StateReader> StatefulValidator<S> {
mut remaining_gas: u64,
) -> StatefulValidatorResult<(Option<CallInfo>, TransactionReceipt)> {
let mut execution_resources = ExecutionResources::default();
let tx_context = Arc::new(self.tx_executor.block_context.to_tx_context(tx));
let tx_context = Arc::new(self.tx_executor.block_context.to_tx_context(tx)?);

let limit_steps_by_resources = true;
let validate_call_info = tx.validate_tx(
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/concurrency/versioned_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ fn test_run_parallel_txs(max_resource_bounds: DeprecatedResourceBoundsMapping) {
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 tx_context = block_context.to_tx_context(&account_tx_2);
let tx_context = block_context.to_tx_context(&account_tx_2).unwrap();
let fee_type = tx_context.tx_info.fee_type();

let deployed_account_balance_key = get_fee_token_var_address(account_address);
Expand Down
5 changes: 4 additions & 1 deletion crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,10 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
&mut execution_output.as_mut().expect(EXECUTION_OUTPUTS_UNWRAP_ERROR).result;

if let Ok(tx_execution_info) = tx_result.as_mut() {
let tx_context = self.block_context.to_tx_context(&self.chunk[tx_index]);
let tx_context = self
.block_context
.to_tx_context(&self.chunk[tx_index])
.expect("Failed to create tx context.");
// Add the deleted sequencer balance key to the storage keys.
let concurrency_mode = true;
tx_state_changes_keys.update_sequencer_key_in_storage(
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/worker_logic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub fn test_commit_tx() {
assert_eq!(felt!(expected_sequencer_storage_read), actual_sequencer_storage_read,);
}
}
let tx_context = executor.block_context.to_tx_context(&txs[commit_idx]);
let tx_context = executor.block_context.to_tx_context(&txs[commit_idx]).unwrap();
expected_sequencer_balance_low += actual_fee;
// Check that the sequencer balance was updated correctly in the state.
verify_sequencer_balance_update(
Expand Down Expand Up @@ -228,7 +228,7 @@ fn test_commit_tx_when_sender_is_sequencer() {
let read_values_before_commit = fee_transfer_call_info.storage_read_values.clone();
drop(execution_task_outputs);

let tx_context = &executor.block_context.to_tx_context(&sequencer_tx[0]);
let tx_context = &executor.block_context.to_tx_context(&sequencer_tx[0]).unwrap();
let fee_token_address =
executor.block_context.chain_info.fee_token_address(&tx_context.tx_info.fee_type());
let sequencer_balance_high_before =
Expand Down
10 changes: 6 additions & 4 deletions crates/blockifier/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use starknet_api::core::{ChainId, ContractAddress};

use crate::blockifier::block::BlockInfo;
use crate::bouncer::BouncerConfig;
use crate::transaction::errors::TransactionInfoCreationError;
use crate::transaction::objects::{
FeeType,
HasRelatedFeeType,
Expand Down Expand Up @@ -62,14 +63,15 @@ impl BlockContext {
&self.versioned_constants
}

// TODO(Nimrod): Don't return `Result`.
pub fn to_tx_context(
&self,
tx_info_creator: &impl TransactionInfoCreator,
) -> TransactionContext {
TransactionContext {
) -> Result<TransactionContext, TransactionInfoCreationError> {
Ok(TransactionContext {
block_context: self.clone(),
tx_info: tx_info_creator.create_tx_info().expect("todo"),
}
tx_info: tx_info_creator.create_tx_info()?,
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/fee/fee_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ fn test_discounted_gas_overdraft(
let charge_fee = true;
let report = PostExecutionReport::new(
&mut state,
&block_context.to_tx_context(&tx),
&block_context.to_tx_context(&tx).unwrap(),
&tx_receipt,
charge_fee,
)
Expand Down
5 changes: 3 additions & 2 deletions crates/blockifier/src/fee/gas_usage_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,9 @@ fn test_get_message_segment_length(

#[rstest]
fn test_compute_discounted_gas_from_gas_vector() {
let tx_context =
BlockContext::create_for_testing().to_tx_context(&account_invoke_tx(invoke_tx_args! {}));
let tx_context = BlockContext::create_for_testing()
.to_tx_context(&account_invoke_tx(invoke_tx_args! {}))
.unwrap();
let gas_usage = GasVector { l1_gas: 100, l1_data_gas: 2, ..Default::default() };
let actual_result = compute_discounted_gas_from_gas_vector(&gas_usage, &tx_context);

Expand Down
6 changes: 5 additions & 1 deletion crates/blockifier/src/test_utils/prices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ fn fee_transfer_resources(
state,
&mut ExecutionResources::default(),
&mut EntryPointExecutionContext::new(
Arc::new(block_context.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))),
Arc::new(
block_context
.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))
.unwrap(),
),
ExecutionMode::Execute,
false,
),
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 @@ -681,7 +681,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
block_context: &BlockContext,
execution_flags: ExecutionFlags,
) -> TransactionExecutionResult<TransactionExecutionInfo> {
let tx_context = Arc::new(block_context.to_tx_context(self));
let tx_context = Arc::new(block_context.to_tx_context(self)?);
self.verify_tx_version(tx_context.tx_info.version())?;

// Nonce and fee check should be done before running user code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ fn test_max_fee_to_max_steps_conversion(
resource_bounds: l1_resource_bounds(actual_gas_used, actual_strk_gas_price.into()),
nonce: nonce_manager.next(account_address),
});
let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1));
let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1).unwrap());
let execution_context1 = EntryPointExecutionContext::new_invoke(tx_context1, true);
let max_steps_limit1 = execution_context1.vm_run_resources.get_n_steps();
let tx_execution_info1 = account_tx1.execute(&mut state, &block_context, true, true).unwrap();
Expand All @@ -952,7 +952,7 @@ fn test_max_fee_to_max_steps_conversion(
resource_bounds: l1_resource_bounds(2 * actual_gas_used, actual_strk_gas_price.into()),
nonce: nonce_manager.next(account_address),
});
let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2));
let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2).unwrap());
let execution_context2 = EntryPointExecutionContext::new_invoke(tx_context2, true);
let max_steps_limit2 = execution_context2.vm_run_resources.get_n_steps();
let tx_execution_info2 = account_tx2.execute(&mut state, &block_context, true, true).unwrap();
Expand Down
2 changes: 2 additions & 0 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ pub enum TransactionExecutionError {
InvalidSegmentStructure(usize, usize),
#[error(transparent)]
ProgramError(#[from] ProgramError),
#[error(transparent)]
TransactionInfoCreationError(#[from] TransactionInfoCreationError),
}

#[derive(Debug, Error)]
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/transaction/transaction_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for L1HandlerTransaction {
block_context: &BlockContext,
_execution_flags: ExecutionFlags,
) -> TransactionExecutionResult<TransactionExecutionInfo> {
let tx_context = Arc::new(block_context.to_tx_context(self));
let tx_context = Arc::new(block_context.to_tx_context(self)?);

let mut execution_resources = ExecutionResources::default();
let mut context = EntryPointExecutionContext::new_invoke(tx_context.clone(), true);
Expand Down Expand Up @@ -184,7 +184,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for Transaction {
let tx_execution_summary = tx_execution_info.summarize();
let mut tx_state_changes_keys = state.get_actual_state_changes()?.into_keys();
tx_state_changes_keys.update_sequencer_key_in_storage(
&block_context.to_tx_context(self),
&block_context.to_tx_context(self)?,
&tx_execution_info,
concurrency_mode,
);
Expand Down
16 changes: 8 additions & 8 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ 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(&account_tx).unwrap();

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

Expand Down Expand Up @@ -792,7 +792,7 @@ fn assert_failure_if_resource_bounds_exceed_balance(
block_context: &BlockContext,
invalid_tx: AccountTransaction,
) {
match block_context.to_tx_context(&invalid_tx).tx_info {
match block_context.to_tx_context(&invalid_tx).unwrap().tx_info {
TransactionInfo::Deprecated(context) => {
assert_matches!(
invalid_tx.execute(state, block_context, true, true).unwrap_err(),
Expand Down Expand Up @@ -1040,7 +1040,7 @@ fn test_invalid_nonce(
let invalid_nonce = nonce!(1_u8);
let invalid_tx =
account_invoke_tx(invoke_tx_args! { nonce: invalid_nonce, ..valid_invoke_tx_args.clone() });
let invalid_tx_context = block_context.to_tx_context(&invalid_tx);
let invalid_tx_context = block_context.to_tx_context(&invalid_tx).unwrap();
let pre_validation_err = invalid_tx
.perform_pre_validation_stage(&mut transactional_state, &invalid_tx_context, false, true)
.unwrap_err();
Expand All @@ -1060,7 +1060,7 @@ fn test_invalid_nonce(
let valid_tx =
account_invoke_tx(invoke_tx_args! { nonce: valid_nonce, ..valid_invoke_tx_args.clone() });

let valid_tx_context = block_context.to_tx_context(&valid_tx);
let valid_tx_context = block_context.to_tx_context(&valid_tx).unwrap();
valid_tx
.perform_pre_validation_stage(&mut transactional_state, &valid_tx_context, false, false)
.unwrap();
Expand All @@ -1069,7 +1069,7 @@ fn test_invalid_nonce(
let invalid_nonce = nonce!(0_u8);
let invalid_tx =
account_invoke_tx(invoke_tx_args! { nonce: invalid_nonce, ..valid_invoke_tx_args.clone() });
let invalid_tx_context = block_context.to_tx_context(&invalid_tx);
let invalid_tx_context = block_context.to_tx_context(&invalid_tx).unwrap();
let pre_validation_err = invalid_tx
.perform_pre_validation_stage(&mut transactional_state, &invalid_tx_context, false, false)
.unwrap_err();
Expand Down Expand Up @@ -1185,7 +1185,7 @@ fn test_declare_tx(
undeclared_class_hash == class_hash
);
let fee_type = &account_tx.fee_type();
let tx_context = &block_context.to_tx_context(&account_tx);
let tx_context = &block_context.to_tx_context(&account_tx).unwrap();
let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();

// Build expected validate call info.
Expand Down Expand Up @@ -1328,7 +1328,7 @@ fn test_deploy_account_tx(

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 tx_context = &block_context.to_tx_context(&account_tx).unwrap();
let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();

// Build expected validate call info.
Expand Down Expand Up @@ -1470,7 +1470,7 @@ fn test_fail_deploy_account_undeclared_class_hash(
deploy_account_tx_args! {resource_bounds: max_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(&deploy_account).unwrap();
let fee_type = tx_context.tx_info.fee_type();

// Fund account, so as not to fail pre-validation.
Expand Down

0 comments on commit a21e7a7

Please sign in to comment.