diff --git a/crates/executor/src/error_stack.rs b/crates/executor/src/error_stack.rs index 61087e1c11..50f42758f3 100644 --- a/crates/executor/src/error_stack.rs +++ b/crates/executor/src/error_stack.rs @@ -11,7 +11,7 @@ use pathfinder_common::{ClassHash, ContractAddress, EntryPoint}; use crate::IntoFelt; -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] pub struct ErrorStack(pub Vec); impl From for ErrorStack { @@ -53,7 +53,7 @@ impl From for ErrorStack { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum Frame { CallFrame(CallFrame), StringFrame(String), @@ -87,7 +87,7 @@ impl From for Frame { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct CallFrame { pub storage_address: ContractAddress, pub class_hash: ClassHash, diff --git a/crates/executor/src/estimate.rs b/crates/executor/src/estimate.rs index fd070da001..af0a7a2876 100644 --- a/crates/executor/src/estimate.rs +++ b/crates/executor/src/estimate.rs @@ -1,16 +1,19 @@ -use blockifier::transaction::objects::TransactionExecutionInfo; +use blockifier::transaction::objects::{HasRelatedFeeType, TransactionExecutionInfo}; use blockifier::transaction::transaction_execution::Transaction; +use pathfinder_common::TransactionHash; +use starknet_api::block::FeeType; use starknet_api::transaction::fields::GasVectorComputationMode; use super::error::TransactionExecutionError; use super::execution_state::ExecutionState; -use super::transaction::transaction_hash; use super::types::FeeEstimate; use crate::transaction::{ execute_transaction, find_l2_gas_limit_and_execute_transaction, l2_gas_accounting_enabled, + ExecutionBehaviorOnRevert, }; +use crate::IntoFelt; pub fn estimate( execution_state: ExecutionState<'_>, @@ -27,7 +30,7 @@ pub fn estimate( let _span = tracing::debug_span!( "estimate", block_number = %block_number, - transaction_hash = %transaction_hash(&tx), + transaction_hash = %TransactionHash(Transaction::tx_hash(&tx).0.into_felt()), transaction_index = %tx_index ) .entered(); @@ -44,9 +47,16 @@ pub fn estimate( tx_index, &mut state, &block_context, + ExecutionBehaviorOnRevert::Fail, )? } else { - execute_transaction(&tx, tx_index, &mut state, &block_context)? + execute_transaction( + &tx, + tx_index, + &mut state, + &block_context, + &ExecutionBehaviorOnRevert::Fail, + )? }; tracing::trace!( @@ -72,7 +82,7 @@ impl FeeEstimate { gas_vector_computation_mode: &GasVectorComputationMode, block_context: &blockifier::context::BlockContext, ) -> Self { - let fee_type = super::transaction::fee_type(transaction); + let fee_type = fee_type(transaction); let minimal_gas_vector = match transaction { Transaction::Account(account_transaction) => { Some(blockifier::fee::gas_usage::estimate_minimal_gas_vector( @@ -92,3 +102,10 @@ impl FeeEstimate { ) } } + +pub fn fee_type(transaction: &Transaction) -> FeeType { + match transaction { + Transaction::Account(tx) => tx.fee_type(), + Transaction::L1Handler(tx) => tx.fee_type(), + } +} diff --git a/crates/executor/src/lib.rs b/crates/executor/src/lib.rs index 5b8d1ac1b0..ca2c2105c0 100644 --- a/crates/executor/src/lib.rs +++ b/crates/executor/src/lib.rs @@ -28,4 +28,3 @@ pub use execution_state::{ExecutionState, L1BlobDataAvailability}; pub use felt::{IntoFelt, IntoStarkFelt}; pub use simulate::{simulate, trace, TraceCache}; pub use starknet_api::contract_class::ClassInfo; -pub use transaction::transaction_hash; diff --git a/crates/executor/src/simulate.rs b/crates/executor/src/simulate.rs index fed70a18ed..f368a44b70 100644 --- a/crates/executor/src/simulate.rs +++ b/crates/executor/src/simulate.rs @@ -29,7 +29,7 @@ use crate::transaction::{ execute_transaction, find_l2_gas_limit_and_execute_transaction, l2_gas_accounting_enabled, - transaction_hash, + ExecutionBehaviorOnRevert, }; use crate::types::{ DataAvailabilityResources, @@ -98,7 +98,7 @@ pub fn simulate( let _span = tracing::debug_span!( "simulate", block_number = %block_number, - transaction_hash = %transaction_hash(&tx), + transaction_hash = %TransactionHash(Transaction::tx_hash(&tx).0.into_felt()), transaction_index = %tx_index ) .entered(); @@ -116,9 +116,10 @@ pub fn simulate( tx_index, &mut tx_state, &block_context, + ExecutionBehaviorOnRevert::Continue, )? } else { - execute_transaction(&tx, tx_index, &mut tx_state, &block_context)? + execute_transaction(&tx, tx_index, &mut tx_state, &block_context, &ExecutionBehaviorOnRevert::Continue)? }; let state_diff = to_state_diff(&mut tx_state, transaction_declared_deprecated_class(&tx))?; tx_state.commit(); @@ -183,7 +184,7 @@ pub fn trace( let mut traces = Vec::with_capacity(transactions.len()); for (transaction_idx, tx) in transactions.into_iter().enumerate() { - let hash = transaction_hash(&tx); + let hash = TransactionHash(Transaction::tx_hash(&tx).0.into_felt()); let _span = tracing::debug_span!("trace", transaction_hash=%hash, %transaction_idx).entered(); diff --git a/crates/executor/src/transaction.rs b/crates/executor/src/transaction.rs index 178dece433..7daa1f9f4c 100644 --- a/crates/executor/src/transaction.rs +++ b/crates/executor/src/transaction.rs @@ -1,49 +1,27 @@ use blockifier::execution::contract_class::TrackedResource; use blockifier::state::cached_state::{CachedState, MutRefState}; use blockifier::state::state_api::UpdatableState; -use blockifier::transaction::objects::{HasRelatedFeeType, TransactionExecutionInfo}; +use blockifier::transaction::objects::TransactionExecutionInfo; use blockifier::transaction::transaction_execution::Transaction; use blockifier::transaction::transactions::ExecutableTransaction; -use pathfinder_common::TransactionHash; -use starknet_api::block::FeeType; use starknet_api::core::ClassHash; use starknet_api::execution_resources::GasAmount; use starknet_api::transaction::fields::GasVectorComputationMode; -use super::felt::IntoFelt; use crate::TransactionExecutionError; -// This workaround will not be necessary after the PR: -// https://github.com/starkware-libs/blockifier/pull/927 -pub fn transaction_hash(transaction: &Transaction) -> TransactionHash { - TransactionHash( - match transaction { - Transaction::Account(outer) => match &outer.tx { - starknet_api::executable_transaction::AccountTransaction::Declare(inner) => { - inner.tx_hash - } - starknet_api::executable_transaction::AccountTransaction::DeployAccount(inner) => { - inner.tx_hash() - } - starknet_api::executable_transaction::AccountTransaction::Invoke(inner) => { - inner.tx_hash() - } - }, - Transaction::L1Handler(outer) => outer.tx_hash, - } - .0 - .into_felt(), - ) +pub enum ExecutionBehaviorOnRevert { + Fail, + Continue, } -pub fn fee_type(transaction: &Transaction) -> FeeType { - match transaction { - Transaction::Account(tx) => tx.fee_type(), - Transaction::L1Handler(tx) => tx.fee_type(), +impl ExecutionBehaviorOnRevert { + pub fn should_fail_on_revert(&self) -> bool { + matches!(self, Self::Fail) } } -pub fn gas_vector_computation_mode(transaction: &Transaction) -> GasVectorComputationMode { +pub(crate) fn gas_vector_computation_mode(transaction: &Transaction) -> GasVectorComputationMode { match &transaction { Transaction::Account(account_transaction) => { use starknet_api::executable_transaction::AccountTransaction; @@ -140,6 +118,7 @@ pub(crate) fn find_l2_gas_limit_and_execute_transaction( tx_index: usize, state: &mut S, block_context: &blockifier::context::BlockContext, + revert_behavior: ExecutionBehaviorOnRevert, ) -> Result where S: UpdatableState, @@ -148,17 +127,18 @@ where // Start with MAX gas limit to get the consumed L2 gas. set_l2_gas_limit(tx, GasAmount::MAX); - let (tx_info, _) = match simulate_transaction(tx, tx_index, state, block_context) { - Ok(tx_info) => tx_info, - Err(TransactionSimulationError::ExecutionError(error)) => { - return Err(error); - } - Err(TransactionSimulationError::OutOfGas) => { - return Err( - anyhow::anyhow!("Fee estimation failed, maximum gas limit exceeded").into(), - ); - } - }; + let (tx_info, _) = + match simulate_transaction(tx, tx_index, state, block_context, &revert_behavior) { + Ok(tx_info) => tx_info, + Err(TransactionSimulationError::ExecutionError(error)) => { + return Err(error); + } + Err(TransactionSimulationError::OutOfGas) => { + return Err( + anyhow::anyhow!("Fee estimation failed, maximum gas limit exceeded").into(), + ); + } + }; let GasAmount(l2_gas_consumed) = tx_info.receipt.gas.l2_gas; @@ -167,7 +147,7 @@ where set_l2_gas_limit(tx, l2_gas_adjusted); let (l2_gas_limit, mut tx_info, tx_state) = - match simulate_transaction(tx, tx_index, state, block_context) { + match simulate_transaction(tx, tx_index, state, block_context, &revert_behavior) { Ok((tx_info, tx_state)) => { // If 110% of the actual transaction gas fee is enough, we use that // as the estimate and skip the binary search. @@ -200,7 +180,8 @@ where current_l2_gas_limit = upper_bound; } - match simulate_transaction(tx, tx_index, state, block_context) { + match simulate_transaction(tx, tx_index, state, block_context, &revert_behavior) + { Ok((tx_info, tx_state)) => { if search_done(lower_bound, upper_bound, L2_GAS_SEARCH_MARGIN) { break (tx_info, tx_state); @@ -236,7 +217,7 @@ where // Set the L2 gas limit to zero so that the transaction reverts with a detailed // `ExecutionError`. set_l2_gas_limit(tx, GasAmount::ZERO); - match execute_transaction(tx, tx_index, state, block_context) { + match execute_transaction(tx, tx_index, state, block_context, &revert_behavior) { Err(e @ TransactionExecutionError::ExecutionError { .. }) => { return Err(e); } @@ -257,21 +238,24 @@ pub(crate) fn execute_transaction( tx_index: usize, state: &mut S, block_context: &blockifier::context::BlockContext, + revert_behavior: &ExecutionBehaviorOnRevert, ) -> Result where S: UpdatableState, { match tx.execute(state, block_context) { Ok(tx_info) => { - if let Some(revert_error) = tx_info.revert_error { - let revert_string = revert_error.to_string(); - tracing::debug!(revert_error=%revert_string, "Transaction reverted"); - - return Err(TransactionExecutionError::ExecutionError { - transaction_index: tx_index, - error: revert_string, - error_stack: revert_error.into(), - }); + if revert_behavior.should_fail_on_revert() { + if let Some(revert_error) = tx_info.revert_error { + let revert_string = revert_error.to_string(); + tracing::debug!(revert_error=%revert_string, "Transaction reverted"); + + return Err(TransactionExecutionError::ExecutionError { + transaction_index: tx_index, + error: revert_string, + error_stack: revert_error.into(), + }); + } } Ok(tx_info) @@ -309,6 +293,7 @@ fn simulate_transaction<'state, S>( tx_index: usize, state: &'state mut S, block_context: &blockifier::context::BlockContext, + revert_behavior: &ExecutionBehaviorOnRevert, ) -> Result< ( TransactionExecutionInfo, @@ -324,7 +309,24 @@ where Ok(tx_info) if failed_with_insufficient_l2_gas(&tx_info) => { Err(TransactionSimulationError::OutOfGas) } - Ok(tx_info) => Ok((tx_info, tx_state)), + Ok(tx_info) => { + if revert_behavior.should_fail_on_revert() { + if let Some(revert_error) = tx_info.revert_error { + let revert_string = revert_error.to_string(); + tracing::debug!(revert_error=%revert_string, "Transaction reverted"); + + return Err(TransactionSimulationError::ExecutionError( + TransactionExecutionError::ExecutionError { + transaction_index: tx_index, + error: revert_string, + error_stack: revert_error.into(), + }, + )); + } + } + + Ok((tx_info, tx_state)) + } Err(error) => { tracing::debug!(%error, %tx_index, "Transaction simulation failed"); let error = TransactionExecutionError::new(tx_index, error); diff --git a/crates/rpc/src/method/estimate_fee.rs b/crates/rpc/src/method/estimate_fee.rs index 53c9ce2f1f..5af49ddc6b 100644 --- a/crates/rpc/src/method/estimate_fee.rs +++ b/crates/rpc/src/method/estimate_fee.rs @@ -939,6 +939,114 @@ mod tests { ); } + /// Invokes the test contract with an invalid entry point so that + /// the transaction is expected to be reverted. + fn invoke_v3_transaction_with_invalid_entry_point( + sender_address: ContractAddress, + nonce: TransactionNonce, + depth: CallParam, + ) -> BroadcastedTransaction { + BroadcastedTransaction::Invoke(BroadcastedInvokeTransaction::V3( + BroadcastedInvokeTransactionV3 { + version: TransactionVersion::THREE, + signature: vec![], + sender_address, + calldata: vec![ + // Number of calls + call_param!("0x1"), + // Address of the deployed test contract + CallParam(felt!( + "0x17c54b787c2eccfb057cf6aa2f941d612249549fff74140adc20bb949eab74b" + )), + // Entry point selector for the called contract, i.e. + CallParam(EntryPoint::hashed(b"bogus").0), + // Length of the call data for the called contract, i.e. + call_param!("1"), + depth, + ], + nonce, + resource_bounds: ResourceBounds { + l1_gas: ResourceBound { + max_amount: ResourceAmount(50), + max_price_per_unit: ResourcePricePerUnit(1000), + }, + l1_data_gas: Some(ResourceBound { + max_amount: ResourceAmount(100), + max_price_per_unit: ResourcePricePerUnit(1000), + }), + l2_gas: ResourceBound { + max_amount: ResourceAmount(800_000), + max_price_per_unit: ResourcePricePerUnit(1000), + }, + }, + tip: Tip(0), + paymaster_data: vec![], + account_deployment_data: vec![], + nonce_data_availability_mode: DataAvailabilityMode::L2, + fee_data_availability_mode: DataAvailabilityMode::L2, + }, + )) + } + + #[tokio::test] + async fn declare_deploy_and_invoke_sierra_class_reverts_on_starknet_0_13_4() { + let (context, last_block_header, account_contract_address, universal_deployer_address) = + crate::test_setup::test_context_with_starknet_version(StarknetVersion::new( + 0, 13, 4, 0, + )) + .await; + + // declare test class + let declare_transaction = declare_v3_transaction(account_contract_address); + // deploy with universal deployer contract + let deploy_transaction = + deploy_v3_transaction(account_contract_address, universal_deployer_address); + // invoke deployed contract + let invoke_transaction = invoke_v3_transaction_with_invalid_entry_point( + account_contract_address, + transaction_nonce!("0x2"), + call_param!("7"), + ); + + let input = Input { + request: vec![declare_transaction, deploy_transaction, invoke_transaction], + simulation_flags: vec![SimulationFlag::SkipValidate], + block_id: BlockId::Number(last_block_header.number), + }; + let error = super::estimate_fee(context, input).await.unwrap_err(); + + assert_matches::assert_matches!(error, EstimateFeeError::TransactionExecutionError { transaction_index, error, error_stack } => { + assert_eq!(transaction_index, 2); + assert_eq!(error, "Transaction execution has failed:\n\ + 0: Error in the called contract (contract address: 0x0000000000000000000000000000000000000000000000000000000000000c01, class hash: 0x019cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978, selector: 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):\n\ + Execution failed. Failure reason:\n\ + Error in contract (contract address: 0x0000000000000000000000000000000000000000000000000000000000000c01, class hash: 0x019cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978, selector: 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):\n\ + Error in contract (contract address: 0x017c54b787c2eccfb057cf6aa2f941d612249549fff74140adc20bb949eab74b, class hash: 0x01a48fd3f75d0a7c2288ac23fb6aba26cd375607ba63e4a3b3ed47fc8e99dc21, selector: 0x02a1f595e2db7bf53e1a4bc9834eef6b86d3cd66ec9c8b3588c09253d0affc51):\n\ + 0x454e545259504f494e545f4e4f545f464f554e44 ('ENTRYPOINT_NOT_FOUND').\n"); + assert_eq!(error_stack, pathfinder_executor::ErrorStack(vec![ + pathfinder_executor::Frame::CallFrame(pathfinder_executor::CallFrame { + storage_address: account_contract_address, + class_hash: crate::test_setup::OPENZEPPELIN_ACCOUNT_CLASS_HASH, + selector: Some(EntryPoint::hashed(b"__execute__")), + }), + pathfinder_executor::Frame::StringFrame( + "Cairo1RevertSummary { header: Execution, stack: \ + [Cairo1RevertFrame { \ + contract_address: ContractAddress(PatriciaKey(0xc01)), \ + class_hash: Some(ClassHash(0x19cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978)), \ + selector: EntryPointSelector(0x15d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad) \ + }, \ + Cairo1RevertFrame { \ + contract_address: ContractAddress(PatriciaKey(0x17c54b787c2eccfb057cf6aa2f941d612249549fff74140adc20bb949eab74b)), \ + class_hash: Some(ClassHash(0x1a48fd3f75d0a7c2288ac23fb6aba26cd375607ba63e4a3b3ed47fc8e99dc21)), \ + selector: EntryPointSelector(0x2a1f595e2db7bf53e1a4bc9834eef6b86d3cd66ec9c8b3588c09253d0affc51) \ + }], \ + last_retdata: Retdata([0x454e545259504f494e545f4e4f545f464f554e44]) }".to_owned() + ) + ])); + }); + } + #[tokio::test] async fn starknet_0_13_4_max_gas_exceeded() { let (context, last_block_header, account_contract_address, universal_deployer_address) =