Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(fee): block context creator may fail #507

Merged
merged 1 commit into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading