Skip to content

Commit

Permalink
refactor(fee): transaction info creator may fail
Browse files Browse the repository at this point in the history
  • Loading branch information
nimrod-starkware committed Aug 19, 2024
1 parent 6c04dce commit d89e3fd
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 40 deletions.
8 changes: 7 additions & 1 deletion crates/blockifier/src/blockifier/stateful_validator.rs
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::{
TransactionInfoCreationError,
TransactionExecutionError,
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
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().unwrap();
let enforce_fee = account_tx_1.create_tx_info().unwrap().enforce_fee().unwrap();

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
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
3 changes: 2 additions & 1 deletion crates/blockifier/src/transaction/account_transaction.rs
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 @@ -718,7 +719,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
}

impl TransactionInfoCreator for AccountTransaction {
fn create_tx_info(&self) -> TransactionInfo {
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
11 changes: 7 additions & 4 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use starknet_api::transaction::{
Calldata,
ContractAddressSalt,
DeclareTransactionV2,
Fee,
DeprecatedResourceBoundsMapping,
Fee,
TransactionHash,
TransactionVersion,
};
Expand Down Expand Up @@ -114,7 +114,10 @@ fn test_circuit(block_context: BlockContext, max_resource_bounds: DeprecatedReso
}

#[rstest]
fn test_rc96_holes(block_context: BlockContext, max_resource_bounds: DeprecatedResourceBoundsMapping) {
fn test_rc96_holes(
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
) {
let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
let chain_info = &block_context.chain_info;
Expand Down Expand Up @@ -171,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().unwrap();
let enforce_fee = account_tx.create_tx_info().unwrap().enforce_fee().unwrap();
let result = account_tx.execute(state, &block_context, true, true);
assert_eq!(result.is_err(), enforce_fee);
}
Expand Down Expand Up @@ -652,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
8 changes: 8 additions & 0 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,11 @@ pub enum NumericConversionError {
#[error("Conversion of {0} to u128 unsuccessful.")]
U128ToUsizeError(u128),
}

#[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
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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 @@ -563,5 +564,5 @@ pub enum FeeType {
}

pub trait TransactionInfoCreator {
fn create_tx_info(&self) -> TransactionInfo;
fn create_tx_info(&self) -> Result<TransactionInfo, TransactionInfoCreationError>;
}
9 changes: 7 additions & 2 deletions crates/blockifier/src/transaction/post_execution_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ use assert_matches::assert_matches;
use rstest::rstest;
use starknet_api::core::{ContractAddress, PatriciaKey};
use starknet_api::state::StorageKey;
use starknet_api::transaction::{Calldata, Fee, DeprecatedResourceBoundsMapping, TransactionVersion};
use starknet_api::transaction::{
Calldata,
DeprecatedResourceBoundsMapping,
Fee,
TransactionVersion,
};
use starknet_api::{felt, patricia_key};
use starknet_types_core::felt::Felt;

Expand Down Expand Up @@ -112,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
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::{TransactionInfoCreationError, 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) -> 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
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::{TransactionInfoCreationError, TransactionExecutionError};
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.0.clone().try_into().expect("todo"),
resource_bounds: tx.resource_bounds.0.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 @@ -381,7 +381,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 @@ -393,21 +393,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.0.clone().try_into().expect("todo"),
resource_bounds: tx.resource_bounds.0.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 @@ -493,7 +493,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 @@ -505,27 +505,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.0.clone().try_into().expect("todo"),
resource_bounds: tx.resource_bounds.0.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 @@ -591,8 +591,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 @@ -602,6 +602,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
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 d89e3fd

Please sign in to comment.