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

build(fee): tx_context and create_tx_info cannot_fail #538

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
12 changes: 3 additions & 9 deletions crates/blockifier/src/blockifier/stateful_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ use crate::state::cached_state::CachedState;
use crate::state::errors::StateError;
use crate::state::state_api::StateReader;
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::errors::{
TransactionExecutionError,
TransactionInfoCreationError,
TransactionPreValidationError,
};
use crate::transaction::errors::{TransactionExecutionError, TransactionPreValidationError};
use crate::transaction::transaction_execution::Transaction;
use crate::transaction::transactions::ValidatableTransaction;

Expand All @@ -40,8 +36,6 @@ pub enum StatefulValidatorError {
TransactionExecutorError(#[from] TransactionExecutorError),
#[error(transparent)]
TransactionPreValidationError(#[from] TransactionPreValidationError),
#[error(transparent)]
TransactionCreationError(#[from] TransactionInfoCreationError),
}

pub type StatefulValidatorResult<T> = Result<T, StatefulValidatorError>;
Expand Down Expand Up @@ -71,7 +65,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 +112,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
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/versioned_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn test_run_parallel_txs(max_resource_bounds: ValidResourceBounds) {
&mut NonceManager::default(),
);
let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1);
let enforce_fee = account_tx_1.create_tx_info().unwrap().enforce_fee();
let enforce_fee = account_tx_1.create_tx_info().enforce_fee();

let class_hash = grindy_account.get_class_hash();
let ctor_storage_arg = felt!(1_u8);
Expand All @@ -248,7 +248,7 @@ fn test_run_parallel_txs(max_resource_bounds: ValidResourceBounds) {
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).unwrap();
let tx_context = block_context.to_tx_context(&account_tx_2);
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: 1 addition & 4 deletions crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,7 @@ 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])
.expect("Failed to create tx context.");
let tx_context = self.block_context.to_tx_context(&self.chunk[tx_index]);
// 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]).unwrap();
let tx_context = executor.block_context.to_tx_context(&txs[commit_idx]);
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]).unwrap();
let tx_context = &executor.block_context.to_tx_context(&sequencer_tx[0]);
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: 4 additions & 6 deletions crates/blockifier/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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 @@ -63,15 +62,14 @@ impl BlockContext {
&self.versioned_constants
}

// TODO(Nimrod): Don't return `Result`.
pub fn to_tx_context(
&self,
tx_info_creator: &impl TransactionInfoCreator,
) -> Result<TransactionContext, TransactionInfoCreationError> {
Ok(TransactionContext {
) -> TransactionContext {
TransactionContext {
block_context: self.clone(),
tx_info: tx_info_creator.create_tx_info()?,
})
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).unwrap(),
&block_context.to_tx_context(&tx),
&tx_receipt,
charge_fee,
)
Expand Down
5 changes: 2 additions & 3 deletions crates/blockifier/src/fee/gas_usage_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,8 @@ 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! {}))
.unwrap();
let tx_context =
BlockContext::create_for_testing().to_tx_context(&account_invoke_tx(invoke_tx_args! {}));
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: 1 addition & 5 deletions crates/blockifier/src/test_utils/prices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,7 @@ fn fee_transfer_resources(
state,
&mut ExecutionResources::default(),
&mut EntryPointExecutionContext::new(
Arc::new(
block_context
.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))
.unwrap(),
),
Arc::new(block_context.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))),
ExecutionMode::Execute,
false,
),
Expand Down
6 changes: 2 additions & 4 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use crate::transaction::constants;
use crate::transaction::errors::{
TransactionExecutionError,
TransactionFeeError,
TransactionInfoCreationError,
TransactionPreValidationError,
};
use crate::transaction::objects::{
Expand Down Expand Up @@ -681,7 +680,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 Expand Up @@ -738,8 +737,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
}

impl TransactionInfoCreator for AccountTransaction {
// TODO(Nimrod): This function should return `TransactionInfo` without a result.
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
fn create_tx_info(&self) -> TransactionInfo {
match self {
Self::Declare(tx) => tx.create_tx_info(),
Self::DeployAccount(tx) => tx.create_tx_info(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ fn test_fee_enforcement(
);

let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
let enforce_fee = account_tx.create_tx_info().unwrap().enforce_fee();
let enforce_fee = account_tx.create_tx_info().enforce_fee();
let result = account_tx.execute(state, &block_context, true, true);
assert_eq!(result.is_err(), enforce_fee);
}
Expand Down Expand Up @@ -652,7 +652,7 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) {
);

// Fail execution, assert nonce and balance are unchanged.
let tx_info = declare_account_tx.create_tx_info().unwrap();
let tx_info = declare_account_tx.create_tx_info();
let initial_balance = state
.get_fee_token_balance(account_address, chain_info.fee_token_address(&tx_info.fee_type()))
.unwrap();
Expand Down Expand Up @@ -931,7 +931,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).unwrap());
let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1));
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 @@ -951,7 +951,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).unwrap());
let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2));
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
11 changes: 0 additions & 11 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ pub enum TransactionExecutionError {
InvalidSegmentStructure(usize, usize),
#[error(transparent)]
ProgramError(#[from] ProgramError),
#[error(transparent)]
TransactionInfoCreationError(#[from] TransactionInfoCreationError),
}

#[derive(Debug, Error)]
Expand All @@ -140,12 +138,3 @@ pub enum NumericConversionError {
#[error("Conversion of {0} to u128 unsuccessful.")]
U128ToUsizeError(u128),
}

// TODO(Nimrod): Delete this error once `create_tx_info` stops returning a `Result`.
#[derive(Debug, Error)]
pub enum TransactionInfoCreationError {
#[error("Invalid ResourceMapping combination was given: {0}")]
InvalidResourceMapping(String),
#[error(transparent)]
StarknetAPIError(#[from] StarknetApiError),
}
3 changes: 1 addition & 2 deletions crates/blockifier/src/transaction/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use crate::transaction::constants;
use crate::transaction::errors::{
TransactionExecutionError,
TransactionFeeError,
TransactionInfoCreationError,
TransactionPreValidationError,
};
use crate::utils::{u128_from_usize, usize_from_u128};
Expand Down Expand Up @@ -570,5 +569,5 @@ pub enum FeeType {
}

pub trait TransactionInfoCreator {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError>;
fn create_tx_info(&self) -> TransactionInfo;
}
2 changes: 1 addition & 1 deletion crates/blockifier/src/transaction/post_execution_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fn test_revert_on_overdraft(
resource_bounds: max_resource_bounds.clone(),
nonce: nonce_manager.next(account_address),
});
let tx_info = approve_tx.create_tx_info().unwrap();
let tx_info = approve_tx.create_tx_info();
let approval_execution_info =
approve_tx.execute(&mut state, &block_context, true, true).unwrap();
assert!(!approval_execution_info.is_reverted());
Expand Down
8 changes: 4 additions & 4 deletions crates/blockifier/src/transaction/transaction_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::fee::actual_cost::TransactionReceipt;
use crate::state::cached_state::TransactionalState;
use crate::state::state_api::UpdatableState;
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::errors::{TransactionFeeError, TransactionInfoCreationError};
use crate::transaction::errors::TransactionFeeError;
use crate::transaction::objects::{
TransactionExecutionInfo,
TransactionExecutionResult,
Expand Down Expand Up @@ -100,7 +100,7 @@ impl Transaction {
}

impl TransactionInfoCreator for Transaction {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
fn create_tx_info(&self) -> TransactionInfo {
match self {
Self::AccountTransaction(account_tx) => account_tx.create_tx_info(),
Self::L1HandlerTransaction(l1_handler_tx) => l1_handler_tx.create_tx_info(),
Expand All @@ -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
Loading
Loading