Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: vm2 account validation #2863

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
30a12bc
integrate stage1 (tracers don't handle hooks yet)
joonazan Oct 28, 2024
1983bd1
version of vm2 that inlines ExecutionStatus::merge
joonazan Oct 29, 2024
488c93a
improve naming: paymaster/account validation use the same end hook!
joonazan Sep 12, 2024
3837492
implement account validation gas limit
joonazan Sep 12, 2024
fcb559d
pass the validation gas limit test
joonazan Oct 8, 2024
603937b
clean up validation gas limit
joonazan Oct 10, 2024
1689cf2
add comment explaining why revert is ok
joonazan Oct 10, 2024
b4608ff
more general passing of hooks to tracers
joonazan Oct 10, 2024
dbc1742
stop validation run after validation
joonazan Oct 11, 2024
07a202e
make it clear that revert is not supposed to happen
joonazan Oct 11, 2024
31e05ae
improve error message
joonazan Oct 22, 2024
3c96ad3
first working test for account validation
joonazan Oct 22, 2024
79b3302
framework for testing every validation rule violation is done
joonazan Oct 23, 2024
0878b3b
long comment explaining validation rules' purpose
joonazan Oct 23, 2024
25a3757
prohibit gasleft
joonazan Oct 23, 2024
7b837aa
pass tests
joonazan Oct 25, 2024
fcd9185
properly use all ValidationParams
joonazan Oct 25, 2024
97911d5
pass validation params to tracer in oneshot executor
joonazan Oct 25, 2024
84f85ac
reminder
joonazan Oct 25, 2024
920b928
unused import
joonazan Oct 25, 2024
4430472
correctly set fast vm mode
joonazan Oct 28, 2024
2644a7f
special case the builtin tracers to eliminate TracerExt
joonazan Oct 29, 2024
85196b4
stop validation on error
joonazan Oct 29, 2024
50e2fad
hide some of the type complexity
joonazan Oct 30, 2024
96f1886
VmFactory no longer requires VmInterface
joonazan Oct 30, 2024
bf1f2e6
use DefaultTracers type alias
joonazan Oct 30, 2024
95f1bbf
do not expose ValidationTracer, CircuitsTracer
joonazan Oct 30, 2024
9ec83f2
try to make CI green
joonazan Oct 30, 2024
7525074
fix validation test
joonazan Oct 30, 2024
660cd9f
get up to date with latest main
joonazan Nov 1, 2024
eef1347
grant access to timestamp to timestampasserter
joonazan Nov 1, 2024
0f47e5d
properly compare shadow validation results
joonazan Nov 1, 2024
b9fcc3e
fix bug where proxies were not allowed
joonazan Nov 13, 2024
938f37a
remove gasleft test
joonazan Nov 13, 2024
2582006
Merge branch 'main' into jms-pla-908-account-validation-properly-hand…
joonazan Nov 13, 2024
f973d1c
Merge branch 'main' into jms-pla-908-account-validation-properly-hand…
joonazan Nov 13, 2024
f9c9f77
fix compilation
joonazan Nov 13, 2024
3136879
test upgradeable proxy
joonazan Nov 13, 2024
408d9b0
test catching out of gas
joonazan Nov 13, 2024
29cfabe
fix test solidity
joonazan Nov 13, 2024
85f33c3
make vm_latest validation fail on out of gas, even if caught
joonazan Nov 13, 2024
40484a7
Merge branch 'main' into jms-pla-908-account-validation-properly-hand…
joonazan Nov 14, 2024
9df63c2
reduce repetition
joonazan Nov 14, 2024
a33ec8e
implement bikeshedding-type review comments
joonazan Nov 14, 2024
4798cb5
crazy solution to .0.1.1 chains
joonazan Nov 14, 2024
0345875
proper hlist library to shut up clippy
joonazan Nov 15, 2024
727e2fe
Update from upstream
slowli Dec 3, 2024
1343f90
Simplify tracers in fast VM
slowli Dec 3, 2024
d293f8c
Structure fast VM tracers
slowli Dec 3, 2024
9f18a9c
Revert `VmInterface` as supertrait for `VmFactory`
slowli Dec 4, 2024
64c4b2b
Refactor validation in oneshot executor
slowli Dec 4, 2024
04a7a7f
Merge branch 'main' into jms-pla-908-account-validation-properly-hand…
slowli Dec 10, 2024
f9d334f
Merge branch 'main' into jms-pla-908-account-validation-properly-hand…
slowli Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading