Skip to content

Commit

Permalink
feat: vm2 account validation (#2863)
Browse files Browse the repository at this point in the history
Implements an account validation gas limit and the validation tracer for
vm2, along with better tests for validation.

Instead of a second gas limit like in vm_latest, the normal gas limit is
used. Unfortunately this means that the VM is not safe to use in the
sequencer until we forbid the use of gasleft. I didn't do it here
because it requires something like taint analysis and could break
existing contracts that didn't know that gasleft is forbidden.

---------

Co-authored-by: Alex Ostrovski <[email protected]>
Co-authored-by: Alex Ostrovski <[email protected]>
  • Loading branch information
3 people authored Dec 19, 2024
1 parent 18e4307 commit af149a0
Show file tree
Hide file tree
Showing 33 changed files with 1,220 additions and 383 deletions.
2 changes: 1 addition & 1 deletion core/lib/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub fn l1_messenger_contract() -> Contract {

/// Reads bytecode from the path RELATIVE to the Cargo workspace location.
pub fn read_bytecode(relative_path: impl AsRef<Path> + std::fmt::Debug) -> Vec<u8> {
read_bytecode_from_path(relative_path).expect("Exists")
read_bytecode_from_path(relative_path).expect("Failed to open file")
}

pub fn eth_contract() -> Contract {
Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/tracers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
pub use self::{
call_tracer::CallTracer, multivm_dispatcher::TracerDispatcher, prestate_tracer::PrestateTracer,
storage_invocation::StorageInvocations, validator::ValidationTracer,
call_tracer::CallTracer,
multivm_dispatcher::TracerDispatcher,
prestate_tracer::PrestateTracer,
storage_invocation::StorageInvocations,
validator::{ValidationTracer, TIMESTAMP_ASSERTER_FUNCTION_SELECTOR},
};

mod call_tracer;
Expand Down
12 changes: 5 additions & 7 deletions core/lib/multivm/src/tracers/validator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{
};

use once_cell::sync::OnceCell;
pub use vm_latest::TIMESTAMP_ASSERTER_FUNCTION_SELECTOR;
use zksync_system_constants::{
ACCOUNT_CODE_STORAGE_ADDRESS, BOOTLOADER_ADDRESS, CONTRACT_DEPLOYER_ADDRESS,
L2_BASE_TOKEN_ADDRESS, MSG_VALUE_SIMULATOR_ADDRESS, SYSTEM_CONTEXT_ADDRESS,
Expand All @@ -13,10 +14,7 @@ use zksync_types::{
address_to_u256, u256_to_h256, vm::VmVersion, web3::keccak256, AccountTreeId, Address,
StorageKey, H256, U256,
};
use zksync_vm_interface::{
tracer::{TimestampAsserterParams, ValidationTraces},
L1BatchEnv,
};
use zksync_vm_interface::tracer::{TimestampAsserterParams, ValidationTraces};

use self::types::{NewTrustedValidationItems, ValidationTracerMode};
use crate::{
Expand Down Expand Up @@ -54,7 +52,7 @@ pub struct ValidationTracer<H> {
computational_gas_limit: u32,
timestamp_asserter_params: Option<TimestampAsserterParams>,
vm_version: VmVersion,
l1_batch_env: L1BatchEnv,
l1_batch_timestamp: u64,
pub result: Arc<OnceCell<ViolatedValidationRule>>,
pub traces: Arc<Mutex<ValidationTraces>>,
_marker: PhantomData<fn(H) -> H>,
Expand All @@ -65,7 +63,7 @@ type ValidationRoundResult = Result<NewTrustedValidationItems, ViolatedValidatio
impl<H> ValidationTracer<H> {
const MAX_ALLOWED_SLOT_OFFSET: u32 = 127;

pub fn new(params: ValidationParams, vm_version: VmVersion, l1_batch_env: L1BatchEnv) -> Self {
pub fn new(params: ValidationParams, vm_version: VmVersion, l1_batch_timestamp: u64) -> Self {
Self {
validation_mode: ValidationTracerMode::NoValidation,
auxilary_allowed_slots: Default::default(),
Expand All @@ -83,7 +81,7 @@ impl<H> ValidationTracer<H> {
result: Arc::new(OnceCell::new()),
traces: Arc::new(Mutex::new(ValidationTraces::default())),
_marker: Default::default(),
l1_batch_env,
l1_batch_timestamp,
}
}

Expand Down
12 changes: 9 additions & 3 deletions core/lib/multivm/src/tracers/validator/vm_latest/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use zk_evm_1_5_0::{
tracing::{BeforeExecutionData, VmLocalStateData},
zkevm_opcode_defs::{ContextOpcode, FarCallABI, LogOpcode, Opcode},
zkevm_opcode_defs::{ContextOpcode, FarCallABI, LogOpcode, Opcode, RetOpcode},
};
use zksync_system_constants::KECCAK256_PRECOMPILE_ADDRESS;
use zksync_types::{
Expand Down Expand Up @@ -116,8 +116,7 @@ impl<H: HistoryMode> ValidationTracer<H> {
// using self.l1_batch_env.timestamp is ok here because the tracer is always
// used in a oneshot execution mode
if end
< self.l1_batch_env.timestamp
+ params.min_time_till_end.as_secs()
< self.l1_batch_timestamp + params.min_time_till_end.as_secs()
{
return Err(
ViolatedValidationRule::TimestampAssertionCloseToRangeEnd,
Expand Down Expand Up @@ -168,6 +167,13 @@ impl<H: HistoryMode> ValidationTracer<H> {
});
}
}

Opcode::Ret(RetOpcode::Panic)
if state.vm_local_state.callstack.current.ergs_remaining == 0 =>
{
// Actual gas limit was reached, not the validation gas limit.
return Err(ViolatedValidationRule::TookTooManyComputationalGas(0));
}
_ => {}
}

Expand Down
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/shadow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::{
mod tests;

type ReferenceVm<S = InMemoryStorage> = vm_latest::Vm<StorageView<S>, HistoryEnabled>;
type ShadowedFastVm<S = InMemoryStorage> = crate::vm_instance::ShadowedFastVm<S>;
type ShadowedFastVm<S = InMemoryStorage> = crate::vm_instance::ShadowedFastVm<S, (), ()>;

fn hash_block(block_env: L2BlockEnv, tx_hashes: &[H256]) -> H256 {
let mut hasher = L2BlockHasher::new(
Expand Down
59 changes: 59 additions & 0 deletions core/lib/multivm/src/versions/testonly/account_validation_rules.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
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 super::{
get_empty_storage, require_eip712::make_aa_transaction, tester::VmTesterBuilder,
ContractToDeploy, TestedVm, TestedVmForValidation,
};
use crate::interface::TxExecutionMode;

/// 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);
assert_matches!(
test_rule::<VM>(1),
Some(ViolatedValidationRule::TouchedDisallowedStorageSlots(_, _))
);
assert_matches!(
test_rule::<VM>(2),
Some(ViolatedValidationRule::CalledContractWithNoCode(_))
);
assert_matches!(test_rule::<VM>(3), None);
assert_matches!(
test_rule::<VM>(4),
Some(ViolatedValidationRule::TookTooManyComputationalGas(_))
)
}

fn test_rule<VM: TestedVm + TestedVmForValidation>(rule: u32) -> 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()),
);

let bytecode = TestContract::validation_test().bytecode.to_vec();
let mut vm = VmTesterBuilder::new()
.with_empty_in_memory_storage()
.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>();

let private_account = vm.rich_accounts[0].clone();

vm.vm.run_validation(
make_aa_transaction(aa_address, beneficiary_address, &private_account),
55,
)
}
4 changes: 1 addition & 3 deletions core/lib/multivm/src/versions/testonly/l1_messenger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ pub(crate) fn test_rollup_da_output_hash_match<VM: TestedVm>() {

// Firstly, deploy tx. It should publish the bytecode of the "test contract"
let counter_bytecode = TestContract::counter().bytecode;
let tx = account
.get_deploy_tx(&counter_bytecode, None, TxType::L2)
.tx;
let tx = account.get_deploy_tx(counter_bytecode, None, TxType::L2).tx;
// We do not use compression here, to have the bytecode published in full.
let (_, result) = vm
.vm
Expand Down
5 changes: 4 additions & 1 deletion core/lib/multivm/src/versions/testonly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ use zksync_vm_interface::{
pubdata::PubdataBuilder, L1BatchEnv, L2BlockEnv, SystemEnv, TxExecutionMode,
};

pub(super) use self::tester::{TestedVm, VmTester, VmTesterBuilder};
pub(super) use self::tester::{
validation_params, TestedVm, TestedVmForValidation, VmTester, VmTesterBuilder,
};
use crate::{
interface::storage::InMemoryStorage, pubdata_builders::RollupPubdataBuilder,
vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT,
};

pub(super) mod account_validation_rules;
pub(super) mod block_tip;
pub(super) mod bootloader;
pub(super) mod bytecode_publishing;
Expand Down
41 changes: 26 additions & 15 deletions core/lib/multivm/src/versions/testonly/require_eip712.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ethabi::Token;
use zksync_eth_signer::TransactionParameters;
use zksync_test_contracts::TestContract;
use zksync_test_contracts::{Account, TestContract};
use zksync_types::{
fee::Fee, l2::L2Tx, transaction_request::TransactionRequest, Address, Eip712Domain, Execute,
L2ChainId, Nonce, Transaction, U256,
Expand Down Expand Up @@ -30,7 +30,6 @@ pub(crate) fn test_require_eip712<VM: TestedVm>() {
.with_rich_accounts(1)
.build::<VM>();
assert_eq!(vm.get_eth_balance(beneficiary_address), U256::from(0));
let chain_id: u32 = 270;
let mut private_account = vm.rich_accounts[0].clone();

// First, let's set the owners of the AA account to the `private_address`.
Expand Down Expand Up @@ -97,7 +96,30 @@ pub(crate) fn test_require_eip712<VM: TestedVm>() {
vm.get_eth_balance(private_account.address)
);

// // Now send the 'classic' EIP712 transaction
// Now send the 'classic' EIP712 transaction

let transaction: Transaction =
make_aa_transaction(aa_address, beneficiary_address, &private_account).into();
vm.vm.push_transaction(transaction);
vm.vm.execute(InspectExecutionMode::OneTx);

assert_eq!(
vm.get_eth_balance(beneficiary_address),
U256::from(916375026)
);
assert_eq!(
private_account_balance,
vm.get_eth_balance(private_account.address)
);
}

pub(crate) fn make_aa_transaction(
aa_address: Address,
beneficiary_address: Address,
private_account: &Account,
) -> L2Tx {
let chain_id: u32 = 270;

let tx_712 = L2Tx::new(
Some(beneficiary_address),
vec![],
Expand Down Expand Up @@ -130,16 +152,5 @@ pub(crate) fn test_require_eip712<VM: TestedVm>() {
let mut l2_tx = L2Tx::from_request(aa_txn_request, 100000, false).unwrap();
l2_tx.set_input(encoded_tx, aa_hash);

let transaction: Transaction = l2_tx.into();
vm.vm.push_transaction(transaction);
vm.vm.execute(InspectExecutionMode::OneTx);

assert_eq!(
vm.get_eth_balance(beneficiary_address),
U256::from(916375026)
);
assert_eq!(
private_account_balance,
vm.get_eth_balance(private_account.address)
);
l2_tx
}
21 changes: 21 additions & 0 deletions core/lib/multivm/src/versions/testonly/tester/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{collections::HashSet, fmt, rc::Rc};
use zksync_contracts::BaseSystemContracts;
use zksync_test_contracts::{Account, TestContract, TxType};
use zksync_types::{
l2::L2Tx,
utils::{deployed_address_create, storage_key_for_eth_balance},
writes::StateDiffRecord,
Address, L1BatchNumber, StorageKey, Transaction, H256, U256,
Expand All @@ -14,6 +15,7 @@ use crate::{
interface::{
pubdata::{PubdataBuilder, PubdataInput},
storage::{InMemoryStorage, StoragePtr, StorageView},
tracer::{ValidationParams, ViolatedValidationRule},
CurrentExecutionState, InspectExecutionMode, L1BatchEnv, L2BlockEnv, SystemEnv,
TxExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterfaceExt,
VmInterfaceHistoryEnabled,
Expand Down Expand Up @@ -231,3 +233,22 @@ pub(crate) trait TestedVm:
/// Returns pubdata input.
fn pubdata_input(&self) -> PubdataInput;
}

pub(crate) trait TestedVmForValidation {
fn run_validation(&mut self, tx: L2Tx, timestamp: u64) -> Option<ViolatedValidationRule>;
}

pub(crate) fn validation_params(tx: &L2Tx, system: &SystemEnv) -> ValidationParams {
let user_address = tx.common_data.initiator_address;
let paymaster_address = tx.common_data.paymaster_params.paymaster;
ValidationParams {
user_address,
paymaster_address,
trusted_slots: Default::default(),
trusted_addresses: Default::default(),
// field `trustedAddress` of ValidationRuleBreaker
trusted_address_slots: [(Address::repeat_byte(0x10), 2.into())].into(),
computational_gas_limit: system.default_validation_computational_gas_limit,
timestamp_asserter_params: None,
}
}
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/vm_fast/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
utils::bytecode,
};

impl<S: ReadStorage, Tr> Vm<S, Tr> {
impl<S: ReadStorage, Tr, Val> Vm<S, Tr, Val> {
/// Checks the last transaction has successfully published compressed bytecodes and returns `true` if there is at least one is still unknown.
pub(crate) fn has_unpublished_bytecodes(&mut self) -> bool {
self.bootloader_state
Expand Down
6 changes: 3 additions & 3 deletions core/lib/multivm/src/versions/vm_fast/hook.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#[derive(Debug)]
#[derive(Debug, Copy, Clone)]
pub(crate) enum Hook {
AccountValidationEntered,
PaymasterValidationEntered,
AccountValidationExited,
ValidationExited,
ValidationStepEnded,
TxHasEnded,
DebugLog,
Expand All @@ -22,7 +22,7 @@ impl Hook {
match hook {
0 => Hook::AccountValidationEntered,
1 => Hook::PaymasterValidationEntered,
2 => Hook::AccountValidationExited,
2 => Hook::ValidationExited,
3 => Hook::ValidationStepEnded,
4 => Hook::TxHasEnded,
5 => Hook::DebugLog,
Expand Down
8 changes: 5 additions & 3 deletions core/lib/multivm/src/versions/vm_fast/mod.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
pub use zksync_vm2::interface;

pub(crate) use self::version::FastVmVersion;
pub use self::vm::Vm;
pub use self::{
tracers::{FullValidationTracer, ValidationTracer},
vm::Vm,
};

mod bootloader_state;
mod bytecode;
mod circuits_tracer;
mod events;
mod evm_deploy_tracer;
mod glue;
mod hook;
mod initial_bootloader_memory;
mod refund;
#[cfg(test)]
mod tests;
mod tracers;
mod transaction_data;
mod utils;
mod version;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
use super::TestedFastVm;
use crate::versions::testonly::account_validation_rules::test_account_validation_rules;

#[test]
fn test_account_validation_rules_fast() {
test_account_validation_rules::<TestedFastVm<(), _>>();
}
Loading

0 comments on commit af149a0

Please sign in to comment.