Skip to content

Commit

Permalink
refactor(fee): transaction info creator may fail (#506)
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/506)
<!-- Reviewable:end -->
  • Loading branch information
@nimrod-starkware
nimrod-starkware committed Aug 28, 2024
1 parent 676c4a4 commit f7ff378
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 37 deletions.
8 changes: 7 additions & 1 deletion crates/blockifier/src/blockifier/stateful_validator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ 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, TransactionPreValidationError};
use crate::transaction::errors::{
TransactionExecutionError,
TransactionInfoCreationError,
TransactionPreValidationError,
};
use crate::transaction::transaction_execution::Transaction;
use crate::transaction::transactions::ValidatableTransaction;

Expand All @@ -36,6 +40,8 @@ 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
2 changes: 1 addition & 1 deletion crates/blockifier/src/concurrency/versioned_state_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn test_run_parallel_txs(max_resource_bounds: DeprecatedResourceBoundsMapping) {
&mut NonceManager::default(),
);
let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1);
let enforce_fee = account_tx_1.create_tx_info().enforce_fee();
let enforce_fee = account_tx_1.create_tx_info().unwrap().enforce_fee();

let class_hash = grindy_account.get_class_hash();
let ctor_storage_arg = felt!(1_u8);
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl BlockContext {
) -> TransactionContext {
TransactionContext {
block_context: self.clone(),
tx_info: tx_info_creator.create_tx_info(),
tx_info: tx_info_creator.create_tx_info().expect("todo"),
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion crates/blockifier/src/transaction/account_transaction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use crate::transaction::constants;
use crate::transaction::errors::{
TransactionExecutionError,
TransactionFeeError,
TransactionInfoCreationError,
TransactionPreValidationError,
};
use crate::transaction::objects::{
Expand Down Expand Up @@ -737,7 +738,8 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
}

impl TransactionInfoCreator for AccountTransaction {
fn create_tx_info(&self) -> TransactionInfo {
// TODO(Nimrod): This function should return `TransactionInfo` without a result.
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
match self {
Self::Declare(tx) => tx.create_tx_info(),
Self::DeployAccount(tx) => tx.create_tx_info(),
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/transaction/account_transactions_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn test_fee_enforcement(
);

let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
let enforce_fee = account_tx.create_tx_info().enforce_fee();
let enforce_fee = account_tx.create_tx_info().unwrap().enforce_fee();
let result = account_tx.execute(state, &block_context, true, true);
assert_eq!(result.is_err(), enforce_fee);
}
Expand Down Expand Up @@ -655,7 +655,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();
let tx_info = declare_account_tx.create_tx_info().unwrap();
let initial_balance = state
.get_fee_token_balance(account_address, chain_info.fee_token_address(&tx_info.fee_type()))
.unwrap();
Expand Down
9 changes: 9 additions & 0 deletions crates/blockifier/src/transaction/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,12 @@ 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: 2 additions & 1 deletion crates/blockifier/src/transaction/objects.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ 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 @@ -565,5 +566,5 @@ pub enum FeeType {
}

pub trait TransactionInfoCreator {
fn create_tx_info(&self) -> TransactionInfo;
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError>;
}
2 changes: 1 addition & 1 deletion crates/blockifier/src/transaction/post_execution_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,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();
let tx_info = approve_tx.create_tx_info().unwrap();
let approval_execution_info =
approve_tx.execute(&mut state, &block_context, true, true).unwrap();
assert!(!approval_execution_info.is_reverted());
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/transaction/transaction_execution.rs
View file
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;
use crate::transaction::errors::{TransactionFeeError, TransactionInfoCreationError};
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) -> TransactionInfo {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
match self {
Self::AccountTransaction(account_tx) => account_tx.create_tx_info(),
Self::L1HandlerTransaction(l1_handler_tx) => l1_handler_tx.create_tx_info(),
Expand Down
52 changes: 26 additions & 26 deletions crates/blockifier/src/transaction/transactions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::state::cached_state::TransactionalState;
use crate::state::errors::StateError;
use crate::state::state_api::{State, UpdatableState};
use crate::transaction::constants;
use crate::transaction::errors::TransactionExecutionError;
use crate::transaction::errors::{TransactionExecutionError, TransactionInfoCreationError};
use crate::transaction::objects::{
CommonAccountFields,
CurrentTransactionInfo,
Expand Down Expand Up @@ -268,7 +268,7 @@ impl<S: State> Executable<S> for DeclareTransaction {
}

impl TransactionInfoCreator for DeclareTransaction {
fn create_tx_info(&self) -> TransactionInfo {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
// TODO(Nir, 01/11/2023): Consider to move this (from all get_tx_info methods).
let common_fields = CommonAccountFields {
transaction_hash: self.tx_hash(),
Expand All @@ -282,27 +282,27 @@ impl TransactionInfoCreator for DeclareTransaction {
match &self.tx {
starknet_api::transaction::DeclareTransaction::V0(tx)
| starknet_api::transaction::DeclareTransaction::V1(tx) => {
TransactionInfo::Deprecated(DeprecatedTransactionInfo {
Ok(TransactionInfo::Deprecated(DeprecatedTransactionInfo {
common_fields,
max_fee: tx.max_fee,
})
}))
}
starknet_api::transaction::DeclareTransaction::V2(tx) => {
TransactionInfo::Deprecated(DeprecatedTransactionInfo {
Ok(TransactionInfo::Deprecated(DeprecatedTransactionInfo {
common_fields,
max_fee: tx.max_fee,
})
}))
}
starknet_api::transaction::DeclareTransaction::V3(tx) => {
TransactionInfo::Current(CurrentTransactionInfo {
Ok(TransactionInfo::Current(CurrentTransactionInfo {
common_fields,
resource_bounds: tx.resource_bounds.clone().try_into().expect("todo"),
resource_bounds: tx.resource_bounds.clone().try_into()?,
tip: tx.tip,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
paymaster_data: tx.paymaster_data.clone(),
account_deployment_data: tx.account_deployment_data.clone(),
})
}))
}
}
}
Expand Down Expand Up @@ -391,7 +391,7 @@ impl<S: State> Executable<S> for DeployAccountTransaction {
}

impl TransactionInfoCreator for DeployAccountTransaction {
fn create_tx_info(&self) -> TransactionInfo {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
let common_fields = CommonAccountFields {
transaction_hash: self.tx_hash(),
version: self.version(),
Expand All @@ -403,21 +403,21 @@ impl TransactionInfoCreator for DeployAccountTransaction {

match &self.tx() {
starknet_api::transaction::DeployAccountTransaction::V1(tx) => {
TransactionInfo::Deprecated(DeprecatedTransactionInfo {
Ok(TransactionInfo::Deprecated(DeprecatedTransactionInfo {
common_fields,
max_fee: tx.max_fee,
})
}))
}
starknet_api::transaction::DeployAccountTransaction::V3(tx) => {
TransactionInfo::Current(CurrentTransactionInfo {
Ok(TransactionInfo::Current(CurrentTransactionInfo {
common_fields,
resource_bounds: tx.resource_bounds.clone().try_into().expect("todo"),
resource_bounds: tx.resource_bounds.clone().try_into()?,
tip: tx.tip,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
paymaster_data: tx.paymaster_data.clone(),
account_deployment_data: AccountDeploymentData::default(),
})
}))
}
}
}
Expand Down Expand Up @@ -509,7 +509,7 @@ impl<S: State> Executable<S> for InvokeTransaction {
}

impl TransactionInfoCreator for InvokeTransaction {
fn create_tx_info(&self) -> TransactionInfo {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
let common_fields = CommonAccountFields {
transaction_hash: self.tx_hash(),
version: self.version(),
Expand All @@ -521,27 +521,27 @@ impl TransactionInfoCreator for InvokeTransaction {

match &self.tx() {
starknet_api::transaction::InvokeTransaction::V0(tx) => {
TransactionInfo::Deprecated(DeprecatedTransactionInfo {
Ok(TransactionInfo::Deprecated(DeprecatedTransactionInfo {
common_fields,
max_fee: tx.max_fee,
})
}))
}
starknet_api::transaction::InvokeTransaction::V1(tx) => {
TransactionInfo::Deprecated(DeprecatedTransactionInfo {
Ok(TransactionInfo::Deprecated(DeprecatedTransactionInfo {
common_fields,
max_fee: tx.max_fee,
})
}))
}
starknet_api::transaction::InvokeTransaction::V3(tx) => {
TransactionInfo::Current(CurrentTransactionInfo {
Ok(TransactionInfo::Current(CurrentTransactionInfo {
common_fields,
resource_bounds: tx.resource_bounds.clone().try_into().expect("todo"),
resource_bounds: tx.resource_bounds.clone().try_into()?,
tip: tx.tip,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
paymaster_data: tx.paymaster_data.clone(),
account_deployment_data: tx.account_deployment_data.clone(),
})
}))
}
}
}
Expand Down Expand Up @@ -607,8 +607,8 @@ impl<S: State> Executable<S> for L1HandlerTransaction {
}

impl TransactionInfoCreator for L1HandlerTransaction {
fn create_tx_info(&self) -> TransactionInfo {
TransactionInfo::Deprecated(DeprecatedTransactionInfo {
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError> {
Ok(TransactionInfo::Deprecated(DeprecatedTransactionInfo {
common_fields: CommonAccountFields {
transaction_hash: self.tx_hash,
version: self.tx.version,
Expand All @@ -618,6 +618,6 @@ impl TransactionInfoCreator for L1HandlerTransaction {
only_query: false,
},
max_fee: Fee::default(),
})
}))
}
}
2 changes: 1 addition & 1 deletion crates/native_blockifier/src/py_validator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl PyValidator {
if account_tx.tx_type() != TransactionType::InvokeFunction {
return Ok(false);
}
let tx_info = account_tx.create_tx_info();
let tx_info = account_tx.create_tx_info()?;
let nonce = self.stateful_validator.get_nonce(tx_info.sender_address())?;

let deploy_account_not_processed =
Expand Down

0 comments on commit f7ff378

Please sign in to comment.