Refactor error unpacking in test_validate_accounts_tx. (#1218)

* Refactor error unpacking in test_validate_accounts_tx. * Use macros to extract errors.
starkware-libs · Dec 6, 2023 · 590b620 · 590b620
1 parent a03627e
commit 590b620
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 60 deletions.
diff --git a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs
@@ -1,6 +1,10 @@
 use std::collections::{HashMap, HashSet};
 
 use cairo_felt::Felt252;
+use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
+use cairo_vm::vm::errors::hint_errors::HintError;
+use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
+use cairo_vm::vm::errors::vm_exception::VmException;
 use cairo_vm::vm::runners::builtin_runner::RANGE_CHECK_BUILTIN_NAME;
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources as VmExecutionResources;
 use num_traits::Pow;
@@ -21,21 +25,21 @@ use crate::abi::abi_utils::selector_from_name;
 use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
 use crate::execution::common_hints::ExecutionMode;
 use crate::execution::entry_point::{CallEntryPoint, CallType};
+use crate::execution::errors::{EntryPointExecutionError, VirtualMachineExecutionError};
 use crate::execution::execution_utils::felt_to_stark_felt;
-use crate::retdata;
 use crate::state::state_api::StateReader;
 use crate::test_utils::cached_state::{
     deprecated_create_deploy_test_state, deprecated_create_test_state,
 };
 use crate::test_utils::{
-    check_entry_point_execution_error_for_custom_hint, trivial_external_entry_point, CHAIN_ID_NAME,
-    CURRENT_BLOCK_NUMBER, CURRENT_BLOCK_TIMESTAMP, TEST_CLASS_HASH, TEST_CONTRACT_ADDRESS,
-    TEST_EMPTY_CONTRACT_CLASS_HASH, TEST_SEQUENCER_ADDRESS,
+    trivial_external_entry_point, CHAIN_ID_NAME, CURRENT_BLOCK_NUMBER, CURRENT_BLOCK_TIMESTAMP,
+    TEST_CLASS_HASH, TEST_CONTRACT_ADDRESS, TEST_EMPTY_CONTRACT_CLASS_HASH, TEST_SEQUENCER_ADDRESS,
 };
 use crate::transaction::constants::QUERY_VERSION_BASE_BIT;
 use crate::transaction::objects::{
     AccountTransactionContext, CommonAccountFields, DeprecatedAccountTransactionContext,
 };
+use crate::{check_entry_point_execution_error_for_custom_hint, retdata};
 
 #[test]
 fn test_storage_read_write() {
@@ -408,7 +412,7 @@ fn test_block_info_syscalls(
 
     if execution_mode == ExecutionMode::Validate && block_info_member_name == "sequencer_address" {
         let error = entry_point_call.execute_directly_in_validate_mode(&mut state).unwrap_err();
-        check_entry_point_execution_error_for_custom_hint(
+        check_entry_point_execution_error_for_custom_hint!(
             &error,
             &format!(
                 "Unauthorized syscall get_{} in execution mode Validate.",

diff --git a/crates/blockifier/src/execution/syscalls/syscalls_test.rs b/crates/blockifier/src/execution/syscalls/syscalls_test.rs
@@ -5,6 +5,10 @@ use std::io::Read;
 use assert_matches::assert_matches;
 use cairo_felt::Felt252;
 use cairo_lang_utils::byte_array::BYTE_ARRAY_MAGIC;
+use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
+use cairo_vm::vm::errors::hint_errors::HintError;
+use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
+use cairo_vm::vm::errors::vm_exception::VmException;
 use cairo_vm::vm::runners::builtin_runner::RANGE_CHECK_BUILTIN_NAME;
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources as VmExecutionResources;
 use itertools::concat;
@@ -32,26 +36,25 @@ use crate::execution::call_info::{
 use crate::execution::common_hints::ExecutionMode;
 use crate::execution::contract_class::ContractClassV0;
 use crate::execution::entry_point::{CallEntryPoint, CallType};
-use crate::execution::errors::EntryPointExecutionError;
+use crate::execution::errors::{EntryPointExecutionError, VirtualMachineExecutionError};
 use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt};
 use crate::execution::syscalls::hint_processor::{
     BLOCK_NUMBER_OUT_OF_RANGE_ERROR, L1_GAS, L2_GAS, OUT_OF_GAS_ERROR,
 };
-use crate::retdata;
 use crate::state::state_api::{State, StateReader};
 use crate::test_utils::cached_state::{create_deploy_test_state, create_test_state};
 use crate::test_utils::{
-    check_entry_point_execution_error_for_custom_hint, create_calldata,
-    trivial_external_entry_point, CHAIN_ID_NAME, CURRENT_BLOCK_NUMBER, CURRENT_BLOCK_TIMESTAMP,
-    LEGACY_TEST_CONTRACT_ADDRESS, LEGACY_TEST_CONTRACT_CAIRO1_PATH, TEST_CLASS_HASH,
-    TEST_CONTRACT_ADDRESS, TEST_EMPTY_CONTRACT_CAIRO0_PATH, TEST_EMPTY_CONTRACT_CLASS_HASH,
-    TEST_SEQUENCER_ADDRESS,
+    create_calldata, trivial_external_entry_point, CHAIN_ID_NAME, CURRENT_BLOCK_NUMBER,
+    CURRENT_BLOCK_TIMESTAMP, LEGACY_TEST_CONTRACT_ADDRESS, LEGACY_TEST_CONTRACT_CAIRO1_PATH,
+    TEST_CLASS_HASH, TEST_CONTRACT_ADDRESS, TEST_EMPTY_CONTRACT_CAIRO0_PATH,
+    TEST_EMPTY_CONTRACT_CLASS_HASH, TEST_SEQUENCER_ADDRESS,
 };
 use crate::transaction::constants::QUERY_VERSION_BASE_BIT;
 use crate::transaction::objects::{
     AccountTransactionContext, CommonAccountFields, CurrentAccountTransactionContext,
     DeprecatedAccountTransactionContext,
 };
+use crate::{check_entry_point_execution_error_for_custom_hint, retdata};
 
 pub const REQUIRED_GAS_STORAGE_READ_WRITE_TEST: u64 = 34650;
 pub const REQUIRED_GAS_CALL_CONTRACT_TEST: u64 = 128080;
@@ -173,7 +176,7 @@ fn test_get_block_hash() {
 
     // Negative flow. Execution mode is Validate.
     let error = entry_point_call.execute_directly_in_validate_mode(&mut state).unwrap_err();
-    check_entry_point_execution_error_for_custom_hint(
+    check_entry_point_execution_error_for_custom_hint!(
         &error,
         "Unauthorized syscall get_block_hash in execution mode Validate.",
     );

diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs
@@ -10,10 +10,6 @@ use std::fs;
 use std::path::PathBuf;
 
 use cairo_felt::Felt252;
-use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
-use cairo_vm::vm::errors::hint_errors::HintError;
-use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
-use cairo_vm::vm::errors::vm_exception::VmException;
 use num_traits::{One, Zero};
 use starknet_api::core::{ContractAddress, EntryPointSelector, Nonce, PatriciaKey};
 use starknet_api::deprecated_contract_class::{
@@ -28,10 +24,8 @@ use crate::abi::abi_utils::{get_fee_token_var_address, selector_from_name};
 use crate::abi::constants::{self};
 use crate::execution::contract_class::{ContractClass, ContractClassV0};
 use crate::execution::entry_point::{CallEntryPoint, CallType};
-use crate::execution::errors::{EntryPointExecutionError, VirtualMachineExecutionError};
 use crate::execution::execution_utils::felt_to_stark_felt;
 use crate::utils::const_max;
-
 // Addresses.
 pub const TEST_CONTRACT_ADDRESS: &str = "0x100";
 pub const TEST_CONTRACT_ADDRESS_2: &str = "0x200";
@@ -198,27 +192,58 @@ fn default_testing_resource_bounds() -> ResourceBoundsMapping {
 // Transactions.
 
 /// Checks that the given error is a `HintError::CustomHint` with the given hint.
-pub fn check_entry_point_execution_error_for_custom_hint(
-    error: &EntryPointExecutionError,
-    expected_hint: &str,
-) {
-    if let EntryPointExecutionError::VirtualMachineExecutionErrorWithTrace {
-        source:
-            VirtualMachineExecutionError::CairoRunError(CairoRunError::VmException(VmException {
-                inner_exc: VirtualMachineError::Hint(hint),
+#[macro_export]
+macro_rules! check_entry_point_execution_error_for_custom_hint {
+    ($error:expr, $expected_hint:expr $(,)?) => {
+        if let EntryPointExecutionError::VirtualMachineExecutionErrorWithTrace {
+            source:
+                VirtualMachineExecutionError::CairoRunError(CairoRunError::VmException(VmException {
+                    inner_exc: VirtualMachineError::Hint(hint),
+                    ..
+                })),
+            ..
+        } = $error
+        {
+            if let HintError::CustomHint(custom_hint) = &hint.1 {
+                assert_eq!(custom_hint.as_ref(), $expected_hint)
+            } else {
+                panic!("Unexpected hint: {:?}", hint);
+            }
+        } else {
+            panic!("Unexpected structure for error: {:?}", $error);
+        }
+    };
+}
+
+#[macro_export]
+macro_rules! check_transaction_execution_error_for_custom_hint {
+    ($error:expr, $expected_hint:expr, $variant:ident, $(,)?) => {
+        match $error {
+            TransactionExecutionError::$variant(error) => {
+                check_entry_point_execution_error_for_custom_hint!(error, $expected_hint)
+            }
+            _ => panic!("Unexpected structure for error: {:?}", $error),
+        }
+    };
+}
+
+#[macro_export]
+macro_rules! check_transaction_execution_error_for_diff_assert_values {
+    ($error:expr $(,)?) => {
+        if let TransactionExecutionError::ValidateTransactionError(
+            EntryPointExecutionError::VirtualMachineExecutionErrorWithTrace {
+                source:
+                    VirtualMachineExecutionError::CairoRunError(CairoRunError::VmException(
+                        VmException { inner_exc: VirtualMachineError::DiffAssertValues(_), .. },
+                    )),
                 ..
-            })),
-        ..
-    } = error
-    {
-        if let HintError::CustomHint(custom_hint) = &hint.1 {
-            assert_eq!(custom_hint.as_ref(), expected_hint)
+            },
+        ) = $error
+        {
         } else {
-            panic!("Unexpected hint: {:?}", hint);
+            panic!("Unexpected structure for error: {:?}", $error);
         }
-    } else {
-        panic!("Unexpected structure for error: {:?}", error);
-    }
+    };
 }
 
 pub fn create_calldata(

diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs
@@ -3,6 +3,10 @@ use std::sync::Arc;
 
 use assert_matches::assert_matches;
 use cairo_felt::Felt252;
+use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
+use cairo_vm::vm::errors::hint_errors::HintError;
+use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
+use cairo_vm::vm::errors::vm_exception::VmException;
 use cairo_vm::vm::runners::builtin_runner::{HASH_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME};
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources as VmExecutionResources;
 use itertools::concat;
@@ -28,7 +32,7 @@ use crate::block_context::BlockContext;
 use crate::execution::call_info::{CallExecution, CallInfo, OrderedEvent, Retdata};
 use crate::execution::contract_class::{ContractClass, ContractClassV0, ContractClassV1};
 use crate::execution::entry_point::{CallEntryPoint, CallType};
-use crate::execution::errors::EntryPointExecutionError;
+use crate::execution::errors::{EntryPointExecutionError, VirtualMachineExecutionError};
 use crate::execution::execution_utils::felt_to_stark_felt;
 use crate::fee::eth_gas_constants;
 use crate::fee::fee_utils::calculate_tx_fee;
@@ -41,9 +45,8 @@ use crate::test_utils::declare::declare_tx;
 use crate::test_utils::dict_state_reader::DictStateReader;
 use crate::test_utils::invoke::{invoke_tx, InvokeTxArgs};
 use crate::test_utils::{
-    check_entry_point_execution_error_for_custom_hint, create_calldata,
-    test_erc20_account_balance_key, test_erc20_sequencer_balance_key, NonceManager,
-    ACCOUNT_CONTRACT_CAIRO1_PATH, BALANCE, CHAIN_ID_NAME, CURRENT_BLOCK_NUMBER,
+    create_calldata, test_erc20_account_balance_key, test_erc20_sequencer_balance_key,
+    NonceManager, ACCOUNT_CONTRACT_CAIRO1_PATH, BALANCE, CHAIN_ID_NAME, CURRENT_BLOCK_NUMBER,
     CURRENT_BLOCK_TIMESTAMP, MAX_FEE, MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE,
     TEST_ACCOUNT_CONTRACT_ADDRESS, TEST_ACCOUNT_CONTRACT_CLASS_HASH, TEST_CLASS_HASH,
     TEST_CONTRACT_ADDRESS, TEST_CONTRACT_CAIRO1_PATH, TEST_EMPTY_CONTRACT_CAIRO0_PATH,
@@ -71,7 +74,12 @@ use crate::transaction::transaction_types::TransactionType;
 use crate::transaction::transactions::{
     DeployAccountTransaction, ExecutableTransaction, L1HandlerTransaction,
 };
-use crate::{declare_tx_args, deploy_account_tx_args, invoke_tx_args, retdata};
+use crate::{
+    check_entry_point_execution_error_for_custom_hint,
+    check_transaction_execution_error_for_custom_hint,
+    check_transaction_execution_error_for_diff_assert_values, declare_tx_args,
+    deploy_account_tx_args, invoke_tx_args, retdata,
+};
 
 #[derive(Clone, Copy)]
 enum CairoVersion {
@@ -1125,8 +1133,7 @@ fn test_validate_accounts_tx(#[case] tx_type: TransactionType) {
     let account_tx =
         create_account_tx_for_validate_test(tx_type, INVALID, None, &mut NonceManager::default());
     let error = account_tx.execute(state, block_context, true, true).unwrap_err();
-    // TODO(Noa,01/05/2023): Test the exact failure reason.
-    assert_matches!(error, TransactionExecutionError::ValidateTransactionError(_));
+    check_transaction_execution_error_for_diff_assert_values!(&error);
 
     // Trying to call another contract (forbidden).
     let account_tx = create_account_tx_for_validate_test(
@@ -1136,14 +1143,11 @@ fn test_validate_accounts_tx(#[case] tx_type: TransactionType) {
         &mut NonceManager::default(),
     );
     let error = account_tx.execute(state, block_context, true, true).unwrap_err();
-    if let TransactionExecutionError::ValidateTransactionError(error) = error {
-        check_entry_point_execution_error_for_custom_hint(
-            &error,
-            "Unauthorized syscall call_contract in execution mode Validate.",
-        );
-    } else {
-        panic!("Expected ValidateTransactionError.")
-    }
+    check_transaction_execution_error_for_custom_hint!(
+        &error,
+        "Unauthorized syscall call_contract in execution mode Validate.",
+        ValidateTransactionError,
+    );
 
     // Verify that the contract does not call another contract in the constructor of deploy account
     // as well.
@@ -1165,14 +1169,11 @@ fn test_validate_accounts_tx(#[case] tx_type: TransactionType) {
         );
         let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
         let error = account_tx.execute(state, block_context, true, true).unwrap_err();
-        if let TransactionExecutionError::ContractConstructorExecutionFailed(error) = error {
-            check_entry_point_execution_error_for_custom_hint(
-                &error,
-                "Unauthorized syscall call_contract in execution mode Validate.",
-            );
-        } else {
-            panic!("Expected ContractConstructorExecutionFailed.")
-        }
+        check_transaction_execution_error_for_custom_hint!(
+            &error,
+            "Unauthorized syscall call_contract in execution mode Validate.",
+            ContractConstructorExecutionFailed,
+        );
     }
 }