fix(vm): Fix VM divergences related to validation (#3567)

## What ❔

Fixes divergences in VM execution during account validation, or
reconsiders what is considered a divergence.

## Why ❔

Fixes some of divergences currently observed during oneshot VM execution
in the API server in CI (specifically, during the "custom account"
integration test).

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zkstack dev fmt` and `zkstack dev
lint`.

Author: slowli

core/lib/multivm/src/versions/shadow/mod.rs

@@ -18,14 +18,17 @@ use crate::{
     versions::testonly::{
         default_l1_batch, default_system_env, make_address_rich, ContractToDeploy,
     },
-    vm_fast, vm_latest,
+    vm_fast,
+    vm_fast::FastValidationTracer,
+    vm_latest,
     vm_latest::HistoryEnabled,
 };
 
 mod tests;
 
 type ReferenceVm<S = InMemoryStorage> = vm_latest::Vm<StorageView<S>, HistoryEnabled>;
-type ShadowedFastVm<S = InMemoryStorage, Tr = ()> = crate::vm_instance::ShadowedFastVm<S, Tr, ()>;
+type ShadowedFastVm<S = InMemoryStorage, Tr = ()> =
+    crate::vm_instance::ShadowedFastVm<S, Tr, FastValidationTracer>;
 
 fn hash_block(block_env: L2BlockEnv, tx_hashes: &[H256]) -> H256 {
     let mut hasher = L2BlockHasher::new(

core/lib/multivm/src/versions/testonly/account_validation_rules.rs

@@ -1,47 +1,104 @@
 use assert_matches::assert_matches;
-use zksync_test_contracts::TestContract;
-use zksync_types::{u256_to_h256, AccountTreeId, Address, StorageKey};
-use zksync_vm_interface::tracer::ViolatedValidationRule;
+use zksync_test_contracts::{Account, TestContract};
+use zksync_types::{address_to_h256, fee::Fee, AccountTreeId, Address, StorageKey, H256};
 
 use super::{
-    get_empty_storage, require_eip712::make_aa_transaction, tester::VmTesterBuilder,
-    ContractToDeploy, TestedVm, TestedVmForValidation,
+    default_system_env, get_empty_storage, require_eip712::make_aa_transaction,
+    tester::VmTesterBuilder, ContractToDeploy, TestedVm, TestedVmForValidation,
 };
-use crate::interface::TxExecutionMode;
+use crate::interface::{
+    tracer::ViolatedValidationRule, ExecutionResult, Halt, InspectExecutionMode, SystemEnv,
+    TxExecutionMode, VmExecutionResultAndLogs, VmInterfaceExt,
+};
+
+/// Corresponds to test cases in the `ValidationRuleBreaker` contract.
+#[derive(Debug, Clone, Copy)]
+#[repr(u32)]
+enum TestCase {
+    Baseline = 0,
+    ReadBootloaderBalance = 1,
+    CallEoa = 2,
+    ReadFromTrustedAddressSlot = 3,
+    RecursiveOutOfGas = 4,
+    PlainOutOfGas = 5,
+    PlainOutOfGasWithCatch = 6,
+}
 
 /// Checks that every limitation imposed on account validation results in an appropriate error.
 /// The actual misbehavior cases are found in "validation-rule-breaker.sol".
 pub(crate) fn test_account_validation_rules<VM: TestedVm + TestedVmForValidation>() {
-    assert_matches!(test_rule::<VM>(0), None);
+    let (result, violated_rule) = test_rule::<VM>(u32::MAX, TestCase::Baseline);
+    assert!(!result.result.is_failed(), "{result:#?}");
+    assert_matches!(violated_rule, None);
+
+    let (result, violated_rule) = test_rule::<VM>(u32::MAX, TestCase::ReadBootloaderBalance);
+    assert_matches!(
+        &result.result,
+        ExecutionResult::Halt {
+            reason: Halt::TracerCustom(_)
+        }
+    );
     assert_matches!(
-        test_rule::<VM>(1),
+        violated_rule,
         Some(ViolatedValidationRule::TouchedDisallowedStorageSlots(_, _))
     );
+
+    let (result, violated_rule) = test_rule::<VM>(u32::MAX, TestCase::CallEoa);
     assert_matches!(
-        test_rule::<VM>(2),
-        Some(ViolatedValidationRule::CalledContractWithNoCode(_))
+        &result.result,
+        ExecutionResult::Halt {
+            reason: Halt::TracerCustom(_)
+        }
     );
-    assert_matches!(test_rule::<VM>(3), None);
     assert_matches!(
-        test_rule::<VM>(4),
-        Some(ViolatedValidationRule::TookTooManyComputationalGas(_))
-    )
+        violated_rule,
+        Some(ViolatedValidationRule::CalledContractWithNoCode(_))
+    );
+
+    let (result, violated_rule) = test_rule::<VM>(u32::MAX, TestCase::ReadFromTrustedAddressSlot);
+    assert!(!result.result.is_failed(), "{result:#?}");
+    assert_matches!(violated_rule, None);
+
+    for test_case in [TestCase::RecursiveOutOfGas, TestCase::PlainOutOfGas] {
+        let (result, violated_rule) = test_rule::<VM>(u32::MAX, test_case);
+        assert_matches!(
+            &result.result,
+            ExecutionResult::Halt {
+                reason: Halt::TracerCustom(_)
+            }
+        );
+        assert_matches!(
+            violated_rule,
+            Some(ViolatedValidationRule::TookTooManyComputationalGas(_))
+        );
+    }
 }
 
-fn test_rule<VM: TestedVm + TestedVmForValidation>(rule: u32) -> Option<ViolatedValidationRule> {
+fn test_rule<VM: TestedVmForValidation>(
+    validation_gas_limit: u32,
+    test_case: TestCase,
+) -> (VmExecutionResultAndLogs, Option<ViolatedValidationRule>) {
     let aa_address = Address::repeat_byte(0x10);
     let beneficiary_address = Address::repeat_byte(0x20);
 
     // Set the type of misbehaviour of the AA contract
     let mut storage_with_rule_break_set = get_empty_storage();
     storage_with_rule_break_set.set_value(
-        StorageKey::new(AccountTreeId::new(aa_address), u256_to_h256(0.into())),
-        u256_to_h256(rule.into()),
+        StorageKey::new(AccountTreeId::new(aa_address), H256::zero()),
+        H256::from_low_u64_be(test_case as u64),
+    );
+    // Set the trusted address.
+    storage_with_rule_break_set.set_value(
+        StorageKey::new(AccountTreeId::new(aa_address), H256::from_low_u64_be(1)),
+        address_to_h256(&Address::from_low_u64_be(0x800a)),
     );
 
     let bytecode = TestContract::validation_test().bytecode.to_vec();
     let mut vm = VmTesterBuilder::new()
-        .with_empty_in_memory_storage()
+        .with_system_env(SystemEnv {
+            default_validation_computational_gas_limit: validation_gas_limit,
+            ..default_system_env()
+        })
         .with_custom_contracts(vec![
             ContractToDeploy::account(bytecode, aa_address).funded()
         ])
@@ -50,10 +107,102 @@ fn test_rule<VM: TestedVm + TestedVmForValidation>(rule: u32) -> Option<Violated
         .with_rich_accounts(1)
         .build::<VM>();
 
-    let private_account = vm.rich_accounts[0].clone();
+    let private_account = &mut vm.rich_accounts[0];
+    let tx = make_aa_transaction(aa_address, beneficiary_address, private_account, None);
+    vm.vm.run_validation(tx, 55)
+}
+
+const OUT_OF_GAS_CASES: [TestCase; 3] = [
+    TestCase::PlainOutOfGas,
+    TestCase::PlainOutOfGasWithCatch,
+    TestCase::RecursiveOutOfGas,
+];
+
+pub(crate) fn test_validation_out_of_gas_with_full_tracer<VM: TestedVmForValidation>() {
+    for test_case in OUT_OF_GAS_CASES {
+        println!("Testing case: {test_case:?}");
+        let (result, violated_rule) = test_rule::<VM>(300_000, test_case);
+        assert_matches!(
+            &result.result,
+            ExecutionResult::Halt {
+                reason: Halt::TracerCustom(_)
+            }
+        );
+        assert_matches!(
+            violated_rule,
+            Some(ViolatedValidationRule::TookTooManyComputationalGas(_))
+        );
+    }
+}
+
+pub(crate) fn test_validation_out_of_gas_with_fast_tracer<VM: TestedVm>() {
+    for test_case in OUT_OF_GAS_CASES {
+        println!("Testing case: {test_case:?}");
+
+        // Large tx gas limit should lead to a validation-specific halt reason.
+        let tx_gas_limits: &[_] = if matches!(test_case, TestCase::RecursiveOutOfGas) {
+            &[100_000_000, 200_000_000, 500_000_000, 1_000_000_000]
+        } else {
+            &[1_000_000, 10_000_000, 100_000_000, 1_000_000_000]
+        };
+
+        for &tx_gas_limit in tx_gas_limits {
+            println!("Testing tx with gas limit: {tx_gas_limit}");
+            let result = run_validation_with_gas_limit::<VM>(test_case, tx_gas_limit);
+            assert_matches!(
+                &result.result,
+                ExecutionResult::Halt {
+                    reason: Halt::ValidationOutOfGas,
+                }
+            );
+        }
+
+        // If the tx gas limit is lower than the validation gas limit, the bootloader should exit super-early.
+        println!("Testing tx with low gas limit");
+        let result = run_validation_with_gas_limit::<VM>(test_case, 250_000);
+        assert_matches!(
+            &result.result,
+            ExecutionResult::Halt {
+                reason: Halt::ValidationFailed(_)
+            }
+        );
+    }
+}
+
+fn run_validation_with_gas_limit<VM: TestedVm>(
+    test_case: TestCase,
+    tx_gas_limit: u32,
+) -> VmExecutionResultAndLogs {
+    let aa_address = Address::repeat_byte(0x10);
+    let beneficiary_address = Address::repeat_byte(0x20);
+    let bytecode = TestContract::validation_test().bytecode.to_vec();
+
+    // Configure the AA to run out of gas during validation.
+    let mut storage_with_rule_break_set = get_empty_storage();
+    storage_with_rule_break_set.set_value(
+        StorageKey::new(AccountTreeId::new(aa_address), H256::zero()),
+        H256::from_low_u64_be(test_case as u64),
+    );
+
+    let mut vm = VmTesterBuilder::new()
+        .with_system_env(SystemEnv {
+            default_validation_computational_gas_limit: 300_000,
+            ..default_system_env()
+        })
+        .with_custom_contracts(vec![
+            ContractToDeploy::account(bytecode, aa_address).funded()
+        ])
+        .with_storage(storage_with_rule_break_set)
+        .with_execution_mode(TxExecutionMode::VerifyExecute)
+        .with_rich_accounts(1)
+        .build::<VM>();
 
-    vm.vm.run_validation(
-        make_aa_transaction(aa_address, beneficiary_address, &private_account),
-        55,
-    )
+    let private_account = &mut vm.rich_accounts[0];
+    let fee = Fee {
+        gas_limit: tx_gas_limit.into(),
+        ..Account::default_fee()
+    };
+    let tx = make_aa_transaction(aa_address, beneficiary_address, private_account, Some(fee));
+    vm.vm.push_transaction(tx.into());
+    vm.vm.execute(InspectExecutionMode::OneTx)
 }

core/lib/multivm/src/versions/testonly/require_eip712.rs

@@ -3,7 +3,7 @@ use zksync_eth_signer::TransactionParameters;
 use zksync_test_contracts::{Account, TestContract};
 use zksync_types::{
     fee::Fee, l2::L2Tx, transaction_request::TransactionRequest, Address, Eip712Domain, Execute,
-    L2ChainId, Nonce, Transaction, U256,
+    L2ChainId, Transaction, U256,
 };
 
 use super::{tester::VmTesterBuilder, ContractToDeploy, TestedVm};
@@ -97,9 +97,8 @@ pub(crate) fn test_require_eip712<VM: TestedVm>() {
     );
 
     // Now send the 'classic' EIP712 transaction
-
     let transaction: Transaction =
-        make_aa_transaction(aa_address, beneficiary_address, &private_account).into();
+        make_aa_transaction(aa_address, beneficiary_address, &mut private_account, None).into();
     vm.vm.push_transaction(transaction);
     vm.vm.execute(InspectExecutionMode::OneTx);
 
@@ -116,20 +115,18 @@ pub(crate) fn test_require_eip712<VM: TestedVm>() {
 pub(crate) fn make_aa_transaction(
     aa_address: Address,
     beneficiary_address: Address,
-    private_account: &Account,
+    private_account: &mut Account,
+    fee: Option<Fee>,
 ) -> L2Tx {
     let chain_id: u32 = 270;
 
+    let nonce = private_account.nonce;
+    private_account.nonce += 1;
     let tx_712 = L2Tx::new(
         Some(beneficiary_address),
         vec![],
-        Nonce(1),
-        Fee {
-            gas_limit: U256::from(1000000000),
-            max_fee_per_gas: U256::from(1000000000),
-            max_priority_fee_per_gas: U256::from(1000000000),
-            gas_per_pubdata_limit: U256::from(1000000000),
-        },
+        nonce,
+        fee.unwrap_or_else(Account::default_fee),
         aa_address,
         U256::from(28374938),
         vec![],

core/lib/multivm/src/versions/testonly/tester/mod.rs

@@ -235,8 +235,12 @@ pub(crate) trait TestedVm:
     fn pubdata_input(&self) -> PubdataInput;
 }
 
-pub(crate) trait TestedVmForValidation {
-    fn run_validation(&mut self, tx: L2Tx, timestamp: u64) -> Option<ViolatedValidationRule>;
+pub(crate) trait TestedVmForValidation: TestedVm {
+    fn run_validation(
+        &mut self,
+        tx: L2Tx,
+        timestamp: u64,
+    ) -> (VmExecutionResultAndLogs, Option<ViolatedValidationRule>);
 }
 
 pub(crate) fn validation_params(tx: &L2Tx, system: &SystemEnv) -> ValidationParams {
@@ -248,7 +252,7 @@ pub(crate) fn validation_params(tx: &L2Tx, system: &SystemEnv) -> ValidationPara
         trusted_slots: Default::default(),
         trusted_addresses: Default::default(),
         // field `trustedAddress` of ValidationRuleBreaker
-        trusted_address_slots: [(Address::repeat_byte(0x10), 2.into())].into(),
+        trusted_address_slots: [(Address::repeat_byte(0x10), 1.into())].into(),
         computational_gas_limit: system.default_validation_computational_gas_limit,
         timestamp_asserter_params: None,
     }

core/lib/multivm/src/versions/vm_fast/mod.rs

@@ -2,7 +2,10 @@ pub use zksync_vm2::interface;
 
 pub(crate) use self::version::FastVmVersion;
 pub use self::{
-    tracers::{CallTracer, FullValidationTracer, StorageInvocationsTracer, ValidationTracer},
+    tracers::{
+        CallTracer, FastValidationTracer, FullValidationTracer, StorageInvocationsTracer,
+        ValidationTracer,
+    },
     vm::Vm,
 };
 

core/lib/multivm/src/versions/vm_fast/tests/account_validation_rules.rs

@@ -1,7 +1,23 @@
 use super::TestedFastVm;
-use crate::versions::testonly::account_validation_rules::test_account_validation_rules;
+use crate::{
+    versions::testonly::account_validation_rules::{
+        test_account_validation_rules, test_validation_out_of_gas_with_fast_tracer,
+        test_validation_out_of_gas_with_full_tracer,
+    },
+    vm_fast::FastValidationTracer,
+};
 
 #[test]
-fn test_account_validation_rules_fast() {
+fn account_validation_rules() {
     test_account_validation_rules::<TestedFastVm<(), _>>();
 }
+
+#[test]
+fn validation_out_of_gas_with_full_tracer() {
+    test_validation_out_of_gas_with_full_tracer::<TestedFastVm<(), _>>();
+}
+
+#[test]
+fn validation_out_of_gas_with_fast_tracer() {
+    test_validation_out_of_gas_with_fast_tracer::<TestedFastVm<(), FastValidationTracer>>();
+}