Skip to content

Commit

Permalink
implement bikeshedding-type review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
joonazan committed Nov 14, 2024
1 parent 9df63c2 commit a33ec8e
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 43 deletions.
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/testonly/require_eip712.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ use zksync_vm_interface::tracer::ValidationParams;
use super::{
circuits_tracer::CircuitsTracer,
evm_deploy_tracer::{DynamicBytecodes, EvmDeployTracer},
validation_tracer::{ValidationGasLimitOnly, ValidationMode, ValidationTracer},
validation_tracer::{FullValidationTracer, ValidationGasLimitOnly, ValidationTracer},
};

#[derive(Debug, Default)]
pub struct WithBuiltinTracers<External, Validation>(
(External, (Validation, (CircuitsTracer, EvmDeployTracer))),
);

pub type DefaultTracers = WithBuiltinTracersForSequencer<()>;
pub type DefaultTracers = SequencerTracers<()>;

pub type WithBuiltinTracersForValidation<Tr> = WithBuiltinTracers<Tr, ValidationTracer>;
pub type ValidationTracers<Tr> = WithBuiltinTracers<Tr, FullValidationTracer>;

impl<External> WithBuiltinTracersForValidation<External> {
impl<External> ValidationTracers<External> {
pub fn for_validation(
external: External,
validation_params: ValidationParams,
Expand All @@ -25,24 +25,24 @@ impl<External> WithBuiltinTracersForValidation<External> {
Self((
external,
(
ValidationTracer::new(validation_params, timestamp),
FullValidationTracer::new(validation_params, timestamp),
Default::default(),
),
))
}
}

pub type WithBuiltinTracersForApi<Tr> = WithBuiltinTracers<Tr, ValidationGasLimitOnly>;
pub type ApiTracers<Tr> = WithBuiltinTracers<Tr, ValidationGasLimitOnly>;

impl<External> WithBuiltinTracersForApi<External> {
impl<External> ApiTracers<External> {
pub fn for_api(external: External) -> Self {
Self((external, Default::default()))
}
}

pub type WithBuiltinTracersForSequencer<Tr> = WithBuiltinTracers<Tr, ValidationGasLimitOnly>;
pub type SequencerTracers<Tr> = WithBuiltinTracers<Tr, ValidationGasLimitOnly>;

impl<External> WithBuiltinTracersForSequencer<External> {
impl<External> SequencerTracers<External> {
pub fn for_sequencer(external: External) -> Self {
Self((external, Default::default()))
}
Expand Down Expand Up @@ -70,7 +70,7 @@ impl<External, Validation> WithBuiltinTracers<External, Validation> {
}
}

impl<External: Tracer, Validation: ValidationMode> Tracer
impl<External: Tracer, Validation: ValidationTracer> Tracer
for WithBuiltinTracers<External, Validation>
{
fn before_instruction<
Expand Down
9 changes: 4 additions & 5 deletions core/lib/multivm/src/versions/vm_fast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ pub use zksync_vm2::interface;

pub(crate) use self::version::FastVmVersion;
pub use self::{
vm::Vm,
with_builtin_tracers::{
DefaultTracers, WithBuiltinTracers, WithBuiltinTracersForApi,
WithBuiltinTracersForSequencer, WithBuiltinTracersForValidation,
builtin_tracers::{
ApiTracers, DefaultTracers, SequencerTracers, ValidationTracers, WithBuiltinTracers,
},
vm::Vm,
};

mod bootloader_state;
mod builtin_tracers;
mod bytecode;
mod circuits_tracer;
mod events;
Expand All @@ -25,4 +25,3 @@ mod utils;
mod validation_tracer;
mod version;
mod vm;
mod with_builtin_tracers;
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use super::TestedFastVm;
use crate::{
versions::testonly::account_validation_rules::test_account_validation_rules,
vm_fast::{Vm, WithBuiltinTracersForValidation},
vm_fast::ValidationTracers,
};

#[test]
fn test_account_validation_rules_fast() {
test_account_validation_rules::<Vm<_, WithBuiltinTracersForValidation<()>>>();
test_account_validation_rules::<TestedFastVm<ValidationTracers<()>>>();
}
13 changes: 5 additions & 8 deletions core/lib/multivm/src/versions/vm_fast/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ use zksync_vm_interface::{
VmExecutionResultAndLogs, VmInterface,
};

use super::{
validation_tracer::ValidationMode, Vm, WithBuiltinTracers, WithBuiltinTracersForValidation,
};
use super::{validation_tracer::ValidationTracer, ValidationTracers, Vm, WithBuiltinTracers};
use crate::{
interface::storage::{ImmutableStorageView, InMemoryStorage},
versions::testonly::{validation_params, TestedVm, TestedVmForValidation},
Expand Down Expand Up @@ -81,8 +79,9 @@ impl PartialEq for VmStateDump {
}
}

impl<E: Tracer + Default, V: ValidationMode> TestedVm
for Vm<ImmutableStorageView<InMemoryStorage>, WithBuiltinTracers<E, V>>
pub(crate) type TestedFastVm<Tr> = Vm<ImmutableStorageView<InMemoryStorage>, Tr>;

impl<E: Tracer + Default, V: ValidationTracer> TestedVm for TestedFastVm<WithBuiltinTracers<E, V>>
where
WithBuiltinTracers<E, V>: std::fmt::Debug + 'static,
{
Expand Down Expand Up @@ -172,9 +171,7 @@ where
}
}

impl TestedVmForValidation
for Vm<ImmutableStorageView<InMemoryStorage>, WithBuiltinTracersForValidation<()>>
{
impl TestedVmForValidation for Vm<ImmutableStorageView<InMemoryStorage>, ValidationTracers<()>> {
fn run_validation(&mut self, tx: L2Tx, timestamp: u64) -> Option<ViolatedValidationRule> {
let validation_params = validation_params(&tx, &self.system_env);
self.push_transaction(tx.into());
Expand Down
12 changes: 6 additions & 6 deletions core/lib/multivm/src/versions/vm_fast/validation_tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use zksync_vm_interface::tracer::{
use super::utils::read_fat_pointer;
use crate::tracers::TIMESTAMP_ASSERTER_FUNCTION_SELECTOR;

pub trait ValidationMode: Tracer + Default {
pub trait ValidationTracer: Tracer + Default {
const STOP_AFTER_VALIDATION: bool;
fn account_validation_entered(&mut self);
fn validation_exited(&mut self);
Expand All @@ -26,7 +26,7 @@ pub trait ValidationMode: Tracer + Default {
#[derive(Debug, Default)]
pub struct ValidationGasLimitOnly;
impl Tracer for ValidationGasLimitOnly {}
impl ValidationMode for ValidationGasLimitOnly {
impl ValidationTracer for ValidationGasLimitOnly {
const STOP_AFTER_VALIDATION: bool = false;
fn account_validation_entered(&mut self) {}
fn validation_exited(&mut self) {}
Expand Down Expand Up @@ -57,7 +57,7 @@ impl ValidationMode for ValidationGasLimitOnly {
/// that a contract adheres to the rules ahead of time would be challenging or even impossible,
/// considering that contracts that the code depends on may get upgraded.
#[derive(Debug, Default)]
pub struct ValidationTracer {
pub struct FullValidationTracer {
in_validation: bool,
add_return_value_to_allowed_slots: bool,

Expand All @@ -75,7 +75,7 @@ pub struct ValidationTracer {
traces: ValidationTraces,
}

impl ValidationMode for ValidationTracer {
impl ValidationTracer for FullValidationTracer {
const STOP_AFTER_VALIDATION: bool = true;

fn account_validation_entered(&mut self) {
Expand All @@ -87,7 +87,7 @@ impl ValidationMode for ValidationTracer {
}
}

impl Tracer for ValidationTracer {
impl Tracer for FullValidationTracer {
fn before_instruction<OP: OpcodeType, S: GlobalStateInterface>(&mut self, state: &mut S) {
if !self.in_validation {
return;
Expand Down Expand Up @@ -223,7 +223,7 @@ impl Tracer for ValidationTracer {
}
}

impl ValidationTracer {
impl FullValidationTracer {
pub fn new(params: ValidationParams, l1_batch_timestamp: u64) -> Self {
let ValidationParams {
user_address,
Expand Down
15 changes: 8 additions & 7 deletions core/lib/multivm/src/versions/vm_fast/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use super::{
hook::Hook,
initial_bootloader_memory::bootloader_initial_memory,
transaction_data::TransactionData,
validation_tracer::ValidationMode,
validation_tracer::ValidationTracer,
DefaultTracers, WithBuiltinTracers,
};
use crate::{
Expand Down Expand Up @@ -366,7 +366,12 @@ impl<S: ReadStorage, Tr: Tracer> Vm<S, Tr> {
}
}

impl<S: ReadStorage, E, V: ValidationMode> Vm<S, WithBuiltinTracers<E, V>>
struct AccountValidationGasSplit {
gas_given: u32,
gas_hidden: u32,
}

impl<S: ReadStorage, E, V: ValidationTracer> Vm<S, WithBuiltinTracers<E, V>>
where
WithBuiltinTracers<E, V>: Tracer,
{
Expand All @@ -377,10 +382,6 @@ where
track_refunds: bool,
pubdata_builder: Option<&dyn PubdataBuilder>,
) -> VmRunResult {
struct AccountValidationGasSplit {
gas_given: u32,
gas_hidden: u32,
}
let mut gas_left_for_account_validation =
self.system_env.default_validation_computational_gas_limit;
let mut account_validation_gas_split = None;
Expand Down Expand Up @@ -744,7 +745,7 @@ where
}
}

impl<S: ReadStorage, E: Tracer + Default, V: ValidationMode> VmInterface
impl<S: ReadStorage, E: Tracer + Default, V: ValidationTracer> VmInterface
for Vm<S, WithBuiltinTracers<E, V>>
{
type TracerDispatcher = WithBuiltinTracers<E, V>;
Expand Down
4 changes: 2 additions & 2 deletions core/lib/vm_executor/src/batch/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use zksync_multivm::{
is_supported_by_fast_vm,
pubdata_builders::pubdata_params_to_builder,
tracers::CallTracer,
vm_fast::{self, WithBuiltinTracersForSequencer},
vm_fast::{self, SequencerTracers},
vm_latest::HistoryEnabled,
FastVmInstance, LegacyVmInstance, MultiVmTracer,
};
Expand Down Expand Up @@ -150,7 +150,7 @@ type BytecodeResult = Result<Vec<CompressedBytecodeInfo>, BytecodeCompressionErr
#[derive(Debug)]
enum BatchVm<S: ReadStorage, Tr: BatchTracer> {
Legacy(LegacyVmInstance<S, HistoryEnabled>),
Fast(FastVmInstance<S, WithBuiltinTracersForSequencer<Tr::Fast>>),
Fast(FastVmInstance<S, SequencerTracers<Tr::Fast>>),
}

macro_rules! dispatch_batch_vm {
Expand Down
4 changes: 2 additions & 2 deletions core/lib/vm_executor/src/oneshot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use zksync_multivm::{
is_supported_by_fast_vm,
tracers::{CallTracer, StorageInvocations, TracerDispatcher, ValidationTracer},
utils::adjust_pubdata_price_for_tx,
vm_fast::{self, WithBuiltinTracers, WithBuiltinTracersForApi},
vm_fast::{self, ApiTracers, WithBuiltinTracers},
vm_latest::{HistoryDisabled, HistoryEnabled},
zk_evm_latest::ethereum_types::U256,
FastVmInstance, HistoryMode, LegacyVmInstance, MultiVmTracer,
Expand Down Expand Up @@ -317,7 +317,7 @@ enum Vm<S: ReadStorage, Tr = vm_fast::DefaultTracers> {
Fast(FastVmInstance<S, Tr>),
}

impl<S: ReadStorage> Vm<S, WithBuiltinTracersForApi<()>> {
impl<S: ReadStorage> Vm<S, ApiTracers<()>> {
fn inspect_transaction_with_bytecode_compression(
&mut self,
missed_storage_invocation_limit: usize,
Expand Down

0 comments on commit a33ec8e

Please sign in to comment.