Merge branch 'main-v0.13.2' into avi/replay_batcher/switch-versioned-…

…constants3

crates/blockifier/feature_contracts/cairo1/test_contract.cairo

@@ -553,4 +553,20 @@ mod TestContract {
 
         assert!(outputs.get_output(mul) == u384 { limb0: 6, limb1: 0, limb2: 0, limb3: 0 });
     }
+
+
+    // Add drop for AddInputResult as it only has PanicDestruct.
+    impl AddInputResultDrop<C> of Drop<core::circuit::AddInputResult<C>>;
+
+    #[external(v0)]
+    fn test_rc96_holes(ref self: ContractState) {
+        test_rc96_holes_helper();
+        test_rc96_holes_helper();
+    }
+
+    #[inline(never)]
+    fn test_rc96_holes_helper() {
+        let in1 = CircuitElement::<CircuitInput<0>> {};
+        (in1,).new_inputs().next([3, 0, 0, 0]);
+    }
 }

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -3,7 +3,9 @@ use std::sync::Mutex;
 
 use rstest::rstest;
 use starknet_api::core::{ContractAddress, Nonce, PatriciaKey};
-use starknet_api::transaction::{ContractAddressSalt, ResourceBoundsMapping};
+use starknet_api::transaction::{
+    ContractAddressSalt, Fee, ResourceBoundsMapping, TransactionVersion,
+};
 use starknet_api::{contract_address, felt, patricia_key};
 use starknet_types_core::felt::Felt;
 
@@ -31,7 +33,8 @@ use crate::transaction::account_transaction::AccountTransaction;
 use crate::transaction::constants::DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME;
 use crate::transaction::objects::HasRelatedFeeType;
 use crate::transaction::test_utils::{
-    account_invoke_tx, calculate_class_info_for_testing, emit_n_events_tx, max_resource_bounds,
+    account_invoke_tx, calculate_class_info_for_testing, emit_n_events_tx, max_fee,
+    max_resource_bounds,
 };
 use crate::transaction::transaction_execution::Transaction;
 use crate::{declare_tx_args, invoke_tx_args, nonce, storage_key};
@@ -525,7 +528,14 @@ fn test_worker_validate(max_resource_bounds: ResourceBoundsMapping) {
 }
 
 #[rstest]
-fn test_deploy_before_declare(max_resource_bounds: ResourceBoundsMapping) {
+#[case::declare_cairo0(CairoVersion::Cairo0, TransactionVersion::ONE)]
+#[case::declare_cairo1(CairoVersion::Cairo1, TransactionVersion::THREE)]
+fn test_deploy_before_declare(
+    max_fee: Fee,
+    max_resource_bounds: ResourceBoundsMapping,
+    #[case] cairo_version: CairoVersion,
+    #[case] version: TransactionVersion,
+) {
     // Create the state.
     let block_context = BlockContext::create_for_account_testing();
     let chain_info = &block_context.chain_info;
@@ -536,7 +546,7 @@ fn test_deploy_before_declare(max_resource_bounds: ResourceBoundsMapping) {
     // Create transactions.
     let account_address_0 = account_contract.get_instance_address(0);
     let account_address_1 = account_contract.get_instance_address(1);
-    let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
+    let test_contract = FeatureContract::TestContract(cairo_version);
     let test_class_hash = test_contract.get_class_hash();
     let test_class_info = calculate_class_info_for_testing(test_contract.get_class());
     let test_compiled_class_hash = test_contract.get_compiled_class_hash();
@@ -546,6 +556,8 @@ fn test_deploy_before_declare(max_resource_bounds: ResourceBoundsMapping) {
             resource_bounds: max_resource_bounds.clone(),
             class_hash: test_class_hash,
             compiled_class_hash: test_compiled_class_hash,
+            version,
+            max_fee,
             nonce: nonce!(0_u8),
         },
         test_class_info.clone(),
@@ -555,7 +567,7 @@ fn test_deploy_before_declare(max_resource_bounds: ResourceBoundsMapping) {
     let invoke_tx = account_invoke_tx(invoke_tx_args! {
         sender_address: account_address_1,
         calldata: create_calldata(
-            account_contract.get_instance_address(0),
+            account_address_0,
             DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME,
             &[
                 test_class_hash.0,                  // Class hash.

crates/blockifier/src/execution/entry_point_execution.rs

@@ -3,9 +3,13 @@ use std::collections::HashSet;
 use cairo_vm::types::builtin_name::BuiltinName;
 use cairo_vm::types::layout_name::LayoutName;
 use cairo_vm::types::relocatable::{MaybeRelocatable, Relocatable};
+use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
+use cairo_vm::vm::errors::memory_errors::MemoryError;
 use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
+use cairo_vm::vm::runners::builtin_runner::BuiltinRunner;
 use cairo_vm::vm::runners::cairo_runner::{CairoArg, CairoRunner, ExecutionResources};
-use num_traits::ToPrimitive;
+use cairo_vm::vm::security::verify_secure_runner;
+use num_traits::{ToPrimitive, Zero};
 use starknet_api::felt;
 use starknet_types_core::felt::Felt;
 
@@ -280,17 +284,86 @@ pub fn run_entry_point(
     args: Args,
     program_segment_size: usize,
 ) -> EntryPointExecutionResult<()> {
-    let verify_secure = true;
+    // Note that we run `verify_secure_runner` manually after filling the holes in the rc96 segment.
+    let verify_secure = false;
     let args: Vec<&CairoArg> = args.iter().collect();
-    let result = runner.run_from_entrypoint(
+    runner.run_from_entrypoint(
         entry_point.pc(),
         &args,
         verify_secure,
         Some(program_segment_size),
         hint_processor,
-    );
+    )?;
+
+    maybe_fill_holes(entry_point, runner)?;
+
+    verify_secure_runner(runner, false, Some(program_segment_size))
+        .map_err(CairoRunError::VirtualMachine)?;
+
+    Ok(())
+}
+
+/// Fills the holes after running the entry point.
+/// Currently only fills the holes in the rc96 segment.
+fn maybe_fill_holes(
+    entry_point: EntryPointV1,
+    runner: &mut CairoRunner,
+) -> Result<(), EntryPointExecutionError> {
+    let Some(rc96_offset) = entry_point
+        .builtins
+        .iter()
+        .rev()
+        .position(|name| name.as_str() == BuiltinName::range_check96.to_str_with_suffix())
+    else {
+        return Ok(());
+    };
+    let rc96_builtin_runner = runner
+        .vm
+        .get_builtin_runners()
+        .iter()
+        .find_map(|builtin| {
+            if let BuiltinRunner::RangeCheck96(rc96_builtin_runner) = builtin {
+                Some(rc96_builtin_runner)
+            } else {
+                None
+            }
+        })
+        .expect("RangeCheck96 builtin runner not found.");
+
+    // 'EntryPointReturnValues' is returned after the implicits and its size is 5,
+    // So the last implicit is at offset 5 + 1.
+    const IMPLICITS_OFFSET: usize = 6;
+    let rc_96_stop_ptr = (runner.vm.get_ap() - (IMPLICITS_OFFSET + rc96_offset))
+        .map_err(|err| CairoRunError::VirtualMachine(VirtualMachineError::Math(err)))?;
+
+    let rc96_base = rc96_builtin_runner.base();
+    let rc96_segment: isize =
+        rc96_base.try_into().expect("Builtin segment index must fit in isize.");
+
+    let Relocatable { segment_index: rc96_stop_segment, offset: stop_offset } =
+        runner.vm.get_relocatable(rc_96_stop_ptr).map_err(CairoRunError::MemoryError)?;
+    assert_eq!(rc96_stop_segment, rc96_segment);
+
+    // Update `segment_used_sizes` to include the holes.
+    runner
+        .vm
+        .segments
+        .segment_used_sizes
+        .as_mut()
+        .expect("Segments used sizes should be calculated at this point")[rc96_base] = stop_offset;
+
+    for offset in 0..stop_offset {
+        match runner
+            .vm
+            .insert_value(Relocatable { segment_index: rc96_segment, offset }, Felt::zero())
+        {
+            // If the value is already set, ignore the error.
+            Ok(()) | Err(MemoryError::InconsistentMemory(_)) => {}
+            Err(err) => panic!("Unexpected error when filling holes: {err}."),
+        }
+    }
 
-    Ok(result?)
+    Ok(())
 }
 
 pub fn finalize_execution(

crates/blockifier/src/execution/entry_point_test.rs

@@ -529,12 +529,11 @@ fn test_cairo1_entry_point_segment_arena() {
         ..trivial_external_entry_point_new(test_contract)
     };
 
-    assert!(
-        entry_point_call
-            .execute_directly(&mut state)
-            .unwrap()
-            .resources
-            .builtin_instance_counter
-            .contains_key(&BuiltinName::segment_arena)
+    assert_eq!(
+        entry_point_call.execute_directly(&mut state).unwrap().resources.builtin_instance_counter
+            [&BuiltinName::segment_arena],
+        // Note: the number of segment_arena instances should not depend on the compiler or VM
+        // version. Do not manually fix this then when upgrading them - it might be a bug.
+        2
     );
 }

crates/blockifier/src/execution/stack_trace_test.rs

@@ -239,7 +239,7 @@ Unknown location (pc=0:{expected_pc1})
 Error at pc=0:767:
 1: Error in the called contract (contract address: {contract_address_felt:#064x}, class hash: \
                  {test_contract_hash:#064x}, selector: {invoke_call_chain_selector_felt:#064x}):
-Error at pc=0:9508:
+Error at pc=0:9631:
 Cairo traceback (most recent call last):
 Unknown location (pc=0:{pc_location})
 
@@ -259,10 +259,10 @@ Execution failed. Failure reason: {expected_error}.
 #[case(CairoVersion::Cairo0, "invoke_call_chain", "Couldn't compute operand op0. Unknown value for memory cell 1:23", 1_u8, 1_u8, (49_u16, 1111_u16, 1081_u16, 1166_u16))]
 #[case(CairoVersion::Cairo0, "fail", "An ASSERT_EQ instruction failed: 1 != 0.", 0_u8, 0_u8, (37_u16, 1093_u16, 1184_u16, 1188_u16))]
 #[case(CairoVersion::Cairo0, "fail", "An ASSERT_EQ instruction failed: 1 != 0.", 0_u8, 1_u8, (49_u16, 1111_u16, 1184_u16, 1188_u16))]
-#[case(CairoVersion::Cairo1, "invoke_call_chain", "0x4469766973696f6e2062792030 ('Division by 0')", 1_u8, 0_u8, (9508_u16, 9508_u16, 0_u16, 0_u16))]
-#[case(CairoVersion::Cairo1, "invoke_call_chain", "0x4469766973696f6e2062792030 ('Division by 0')", 1_u8, 1_u8, (9508_u16, 9577_u16, 0_u16, 0_u16))]
-#[case(CairoVersion::Cairo1, "fail", "0x6661696c ('fail')", 0_u8, 0_u8, (9508_u16, 9508_u16, 0_u16, 0_u16))]
-#[case(CairoVersion::Cairo1, "fail", "0x6661696c ('fail')", 0_u8, 1_u8, (9508_u16, 9577_u16, 0_u16, 0_u16))]
+#[case(CairoVersion::Cairo1, "invoke_call_chain", "0x4469766973696f6e2062792030 ('Division by 0')", 1_u8, 0_u8, (9631_u16, 9631_u16, 0_u16, 0_u16))]
+#[case(CairoVersion::Cairo1, "invoke_call_chain", "0x4469766973696f6e2062792030 ('Division by 0')", 1_u8, 1_u8, (9631_u16, 9700_u16, 0_u16, 0_u16))]
+#[case(CairoVersion::Cairo1, "fail", "0x6661696c ('fail')", 0_u8, 0_u8, (9631_u16, 9631_u16, 0_u16, 0_u16))]
+#[case(CairoVersion::Cairo1, "fail", "0x6661696c ('fail')", 0_u8, 1_u8, (9631_u16, 9700_u16, 0_u16, 0_u16))]
 fn test_trace_call_chain_with_syscalls(
     block_context: BlockContext,
     #[case] cairo_version: CairoVersion,

crates/blockifier/src/test_utils.rs

@@ -47,6 +47,8 @@ pub const TEST_ERC20_CONTRACT_CLASS_HASH: &str = "0x1010";
 pub const ERC20_CONTRACT_PATH: &str = "./ERC20/ERC20_Cairo0/ERC20_without_some_syscalls/ERC20/\
                                        erc20_contract_without_some_syscalls_compiled.json";
 
+// TODO(Aviv, 14/7/2024): Move from test utils module, and use it in ContractClassVersionMismatch
+// error.
 #[derive(Clone, Copy, Debug)]
 pub enum CairoVersion {
     Cairo0,

crates/blockifier/src/transaction.rs

@@ -1,5 +1,7 @@
 pub mod account_transaction;
 pub mod constants;
+#[cfg(test)]
+pub mod error_format_test;
 pub mod errors;
 pub mod objects;
 #[cfg(any(feature = "testing", test))]

crates/blockifier/src/transaction/account_transactions_test.rs

@@ -1,6 +1,7 @@
 use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
 
+use cairo_vm::types::builtin_name::BuiltinName;
 use cairo_vm::vm::runners::cairo_runner::ResourceTracker;
 use pretty_assertions::assert_eq;
 use rstest::rstest;
@@ -84,6 +85,45 @@ fn test_circuit(block_context: BlockContext, max_resource_bounds: ResourceBounds
     assert_eq!(tx_execution_info.transaction_receipt.gas, GasVector::from_l1_gas(6682));
 }
 
+#[rstest]
+fn test_rc96_holes(block_context: BlockContext, max_resource_bounds: ResourceBoundsMapping) {
+    let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
+    let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
+    let chain_info = &block_context.chain_info;
+    let state = &mut test_state(chain_info, BALANCE, &[(test_contract, 1), (account, 1)]);
+    let test_contract_address = test_contract.get_instance_address(0);
+    let account_address = account.get_instance_address(0);
+    let mut nonce_manager = NonceManager::default();
+
+    // Invoke a function that changes the state and reverts.
+    let tx_args = invoke_tx_args! {
+        sender_address: account_address,
+        calldata: create_calldata(
+                test_contract_address,
+                "test_rc96_holes",
+                &[]
+            ),
+        nonce: nonce_manager.next(account_address)
+    };
+    let tx_execution_info = run_invoke_tx(
+        state,
+        &block_context,
+        invoke_tx_args! {
+            resource_bounds: max_resource_bounds,
+            ..tx_args
+        },
+    )
+    .unwrap();
+
+    assert!(!tx_execution_info.is_reverted());
+    assert_eq!(
+        tx_execution_info.transaction_receipt.resources.vm_resources.builtin_instance_counter
+            [&BuiltinName::range_check96],
+        24
+    );
+    assert_eq!(tx_execution_info.transaction_receipt.gas, GasVector::from_l1_gas(6598));
+}
+
 #[rstest]
 fn test_fee_enforcement(
     block_context: BlockContext,

crates/blockifier/src/transaction/error_format_test.rs

@@ -0,0 +1,57 @@
+use starknet_api::core::{ClassHash, ContractAddress, Nonce, PatriciaKey};
+use starknet_api::hash::StarkHash;
+use starknet_api::transaction::TransactionVersion;
+
+use crate::transaction::errors::{TransactionExecutionError, TransactionPreValidationError};
+
+#[test]
+fn test_contract_class_version_mismatch() {
+    let error = TransactionExecutionError::ContractClassVersionMismatch {
+        declare_version: TransactionVersion::ONE,
+        cairo_version: 2,
+    };
+    assert_eq!(
+        error.to_string(),
+        "Declare transaction version 1 must have a contract class of Cairo version 2."
+    );
+}
+
+#[test]
+fn test_declare_transaction_error_format() {
+    let error = TransactionExecutionError::DeclareTransactionError {
+        class_hash: ClassHash(StarkHash::THREE),
+    };
+    assert_eq!(
+        error.to_string(),
+        "Class with hash 0x0000000000000000000000000000000000000000000000000000000000000003 is \
+         already declared."
+    );
+}
+
+#[test]
+fn test_invalid_version_format() {
+    let error = TransactionExecutionError::InvalidVersion {
+        version: TransactionVersion::THREE,
+        allowed_versions: vec![TransactionVersion::ONE, TransactionVersion::TWO],
+    };
+    assert_eq!(
+        error.to_string(),
+        "Transaction version 0x3 is not supported. Supported versions: [0x1, 0x2]."
+    );
+}
+
+#[test]
+fn test_invalid_nonce_format() {
+    let error = TransactionPreValidationError::InvalidNonce {
+        address: ContractAddress(PatriciaKey::from(20_u8)),
+        account_nonce: Nonce(StarkHash::THREE),
+        incoming_tx_nonce: Nonce(StarkHash::TWO),
+    };
+    assert_eq!(
+        error.to_string(),
+        "Invalid transaction nonce of contract at address \
+         0x0000000000000000000000000000000000000000000000000000000000000014. Account nonce: \
+         0x0000000000000000000000000000000000000000000000000000000000000003; got: \
+         0x0000000000000000000000000000000000000000000000000000000000000002."
+    );
+}