From 06596a7ecfe1c3a2c186dc0cc95b72d5eb2f19ef Mon Sep 17 00:00:00 2001 From: zuphitf <51879558+zuphitf@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:41:50 +0200 Subject: [PATCH] feat: add the new stack trace to ValidateTransactionError (#1712) * feat: add the new stack trace to ValidateTransactionError * test: add test --- Cargo.lock | 9 +- Cargo.toml | 1 + crates/blockifier/Cargo.toml | 1 + .../src/execution/entry_point_test.rs | 91 ++++++++++++++++++- crates/blockifier/src/execution/errors.rs | 9 +- .../src/transaction/account_transaction.rs | 3 +- crates/blockifier/src/transaction/errors.rs | 3 +- 7 files changed, 106 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81a9aa0650..beefcac9db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -413,6 +413,7 @@ dependencies = [ "once_cell", "phf", "pretty_assertions", + "regex", "rstest", "serde", "serde_json", @@ -2802,9 +2803,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.10.2" +version = "1.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "380b951a9c5e80ddfd6136919eef32310721aa4aacd4889a8d39124b026ab343" +checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c" dependencies = [ "aho-corasick", "memchr", @@ -2814,9 +2815,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.3" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f804c7828047e88b2d32e2d7fe5a105da8ee3264f01902f796c8e067dc2483f" +checksum = "86b83b8b9847f9bf95ef68afb0b8e6cdb80f498442f5179a29fad448fcc1eaea" dependencies = [ "aho-corasick", "memchr", diff --git a/Cargo.toml b/Cargo.toml index 1dd65b9637..7408272037 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ phf = { version = "0.11", features = ["macros"] } pretty_assertions = "1.2.1" pyo3 = "0.19.1" pyo3-log = "0.8.1" +regex = "1.10.4" rstest = "0.17.0" serde = "1.0.184" serde_json = "1.0.81" diff --git a/crates/blockifier/Cargo.toml b/crates/blockifier/Cargo.toml index 17b7f5d3eb..b9ff0cd8e8 100644 --- a/crates/blockifier/Cargo.toml +++ b/crates/blockifier/Cargo.toml @@ -52,5 +52,6 @@ thiserror.workspace = true [dev-dependencies] assert_matches.workspace = true pretty_assertions.workspace = true +regex.workspace = true rstest.workspace = true test-case.workspace = true diff --git a/crates/blockifier/src/execution/entry_point_test.rs b/crates/blockifier/src/execution/entry_point_test.rs index 3da3ffc976..80be163b84 100644 --- a/crates/blockifier/src/execution/entry_point_test.rs +++ b/crates/blockifier/src/execution/entry_point_test.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use cairo_vm::serde::deserialize_program::BuiltinName; use num_bigint::BigInt; use pretty_assertions::assert_eq; +use regex::Regex; use rstest::rstest; use starknet_api::core::{EntryPointSelector, PatriciaKey}; use starknet_api::deprecated_contract_class::{EntryPointOffset, EntryPointType}; @@ -20,9 +21,20 @@ use crate::state::cached_state::CachedState; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; -use crate::test_utils::{create_calldata, trivial_external_entry_point_new, CairoVersion, BALANCE}; -use crate::transaction::constants::EXECUTE_ENTRY_POINT_NAME; -use crate::transaction::test_utils::{block_context, run_invoke_tx}; +use crate::test_utils::{ + create_calldata, trivial_external_entry_point_new, CairoVersion, NonceManager, BALANCE, +}; +use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::constants::{ + EXECUTE_ENTRY_POINT_NAME, VALIDATE_DECLARE_ENTRY_POINT_NAME, VALIDATE_DEPLOY_ENTRY_POINT_NAME, + VALIDATE_ENTRY_POINT_NAME, +}; +use crate::transaction::test_utils::{ + block_context, create_account_tx_for_validate_test, run_invoke_tx, FaultyAccountTxCreatorArgs, + INVALID, +}; +use crate::transaction::transaction_types::TransactionType; +use crate::transaction::transactions::ExecutableTransaction; use crate::versioned_constants::VersionedConstants; use crate::{invoke_tx_args, retdata}; @@ -995,3 +1007,76 @@ Execution failed. Failure reason: {expected_error}. assert_eq!(tx_execution_error.to_string(), expected_trace); } + +#[rstest] +#[case::validate(TransactionType::InvokeFunction, VALIDATE_ENTRY_POINT_NAME)] +#[case::validate_declare(TransactionType::Declare, VALIDATE_DECLARE_ENTRY_POINT_NAME)] +#[case::validate_deploy(TransactionType::DeployAccount, VALIDATE_DEPLOY_ENTRY_POINT_NAME)] +fn test_validate_trace( + #[case] tx_type: TransactionType, + #[case] entry_point_name: &str, + #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion, +) { + let create_for_account_testing = &BlockContext::create_for_account_testing(); + let block_context = create_for_account_testing; + let faulty_account = FeatureContract::FaultyAccount(cairo_version); + let mut sender_address = faulty_account.get_instance_address(0); + let class_hash = faulty_account.get_class_hash(); + let state = &mut test_state(&block_context.chain_info, 0, &[(faulty_account, 1)]); + let selector = selector_from_name(entry_point_name).0; + + // Logic failure. + let account_tx = create_account_tx_for_validate_test( + &mut NonceManager::default(), + FaultyAccountTxCreatorArgs { + scenario: INVALID, + tx_type, + sender_address, + class_hash, + ..Default::default() + }, + ); + + if let TransactionType::DeployAccount = tx_type { + // Deploy account uses the actual address as the sender address. + match &account_tx { + AccountTransaction::DeployAccount(tx) => { + sender_address = tx.contract_address; + } + _ => panic!("Expected DeployAccountTransaction type"), + } + } + + let contract_address = *sender_address.0.key(); + + let expected_error = match cairo_version { + CairoVersion::Cairo0 => format!( + "Transaction validation has failed: +0: Error in the called contract (contract address: {contract_address}, class hash: {class_hash}, \ + selector: {selector}): +Error at pc=0:0: +Cairo traceback (most recent call last): +Unknown location (pc=0:0) +Unknown location (pc=0:0) + +An ASSERT_EQ instruction failed: 1 != 0. +" + ), + CairoVersion::Cairo1 => format!( + "Transaction validation has failed: +0: Error in the called contract (contract address: {contract_address}, class hash: {class_hash}, \ + selector: {selector}): +Execution failed. Failure reason: 0x496e76616c6964207363656e6172696f ('Invalid scenario'). +" + ), + }; + + // Clean pc locations from the trace. + let re = Regex::new(r"pc=0:[0-9]+").unwrap(); + let cleaned_expected_error = &re.replace_all(&expected_error, "pc=0:*"); + let actual_error = account_tx.execute(state, block_context, true, true).unwrap_err(); + let actual_error_str = actual_error.to_string(); + let cleaned_actual_error = &re.replace_all(&actual_error_str, "pc=0:*"); + // Compare actual trace to the expected trace (sans pc locations). + assert_eq!(cleaned_actual_error.to_string(), cleaned_expected_error.to_string()); +} diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index 4afdc90da6..0b8400f5cf 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/crates/blockifier/src/execution/errors.rs @@ -125,9 +125,14 @@ pub fn gen_transaction_execution_error_trace(error: &TransactionExecutionError) class_hash, storage_address, selector, + } + | TransactionExecutionError::ValidateTransactionError { + error, + class_hash, + storage_address, + selector, } => { - // TODO(zuphit): activate the match on these types once all selectors are available. - // | TransactionExecutionError::ValidateTransactionError { error, storage_address, .. } + // TODO(zuphit): activate the match on this type once all selectors are available. // | TransactionExecutionError::ContractConstructorExecutionFailed { // error, // storage_address, diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 38a1b375a5..84ec0a334f 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -669,6 +669,7 @@ impl ValidatableTransaction for AccountTransaction { } let storage_address = tx_info.sender_address(); + let class_hash = state.get_class_hash_at(storage_address)?; let validate_selector = self.validate_entry_point_selector(); let validate_call = CallEntryPoint { entry_point_type: EntryPointType::External, @@ -686,13 +687,13 @@ impl ValidatableTransaction for AccountTransaction { validate_call.execute(state, resources, &mut context).map_err(|error| { TransactionExecutionError::ValidateTransactionError { error, + class_hash, storage_address, selector: validate_selector, } })?; // Validate return data. - let class_hash = state.get_class_hash_at(storage_address)?; let contract_class = state.get_compiled_contract_class(class_hash)?; if let ContractClass::V1(_) = contract_class { // The account contract class is a Cairo 1.0 contract; the `validate` entry point should diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index f7c1bb58df..f5a0949813 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -89,9 +89,10 @@ pub enum TransactionExecutionError { TransactionPreValidationError(#[from] TransactionPreValidationError), #[error(transparent)] TryFromIntError(#[from] std::num::TryFromIntError), - #[error("Transaction validation has failed: {error}")] + #[error("Transaction validation has failed:\n{}", gen_transaction_execution_error_trace(self))] ValidateTransactionError { error: EntryPointExecutionError, + class_hash: ClassHash, storage_address: ContractAddress, selector: EntryPointSelector, },