Skip to content

Commit

Permalink
build(fee): tx_context and create_tx_info cannot_fail (#538)
Browse files Browse the repository at this point in the history
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/starkware-libs/sequencer/538)
<!-- Reviewable:end -->
  • Loading branch information
nimrod-starkware authored Sep 4, 2024
1 parent 2dcefb9 commit 9745223
Show file tree
Hide file tree
Showing 17 changed files with 60 additions and 90 deletions.
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

0 comments on commit 9745223

Please sign in to comment.