Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
perekopskiy committed Oct 22, 2024
1 parent 0825647 commit b22e77c
Show file tree
Hide file tree
Showing 29 changed files with 78 additions and 96 deletions.
2 changes: 1 addition & 1 deletion core/lib/dal/src/models/storage_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl TryFrom<StorageSyncBlock> for SyncBlock {
protocol_version: parse_protocol_version(block.protocol_version)?,
pubdata_params: PubdataParams {
pubdata_type: L1BatchCommitmentMode::from_str(&block.pubdata_type)
.expect("Invalid pubdata type"),
.decode_column("Invalid pubdata type")?,
l2_da_validator_address: parse_h160(&block.l2_da_validator_address)
.decode_column("l2_da_validator_address")?,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub(crate) fn test_bytecode_publishing<VM: TestedVm>() {
let result = vm.vm.execute(InspectExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");

vm.vm.finish_batch(Some(default_pubdata_builder()));
vm.vm.finish_batch(default_pubdata_builder());

let state = vm.vm.get_current_execution_state();
let long_messages = VmEvent::extract_long_l2_to_l1_messages(&state.events);
Expand Down
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/testonly/default_aa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub(crate) fn test_default_aa_interaction<VM: TestedVm>() {
let result = vm.vm.execute(InspectExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");

vm.vm.finish_batch(Some(default_pubdata_builder()));
vm.vm.finish_batch(default_pubdata_builder());

vm.vm.get_current_execution_state();

Expand Down
6 changes: 3 additions & 3 deletions core/lib/multivm/src/versions/testonly/refunds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) fn test_predetermined_refunded_gas<VM: TestedVm>() {

let result_without_predefined_refunds = vm
.vm
.finish_batch(Some(default_pubdata_builder()))
.finish_batch(default_pubdata_builder())
.block_tip_execution_result;
let mut current_state_without_predefined_refunds = vm.vm.get_current_execution_state();
assert!(!result_without_predefined_refunds.result.is_failed(),);
Expand All @@ -61,7 +61,7 @@ pub(crate) fn test_predetermined_refunded_gas<VM: TestedVm>() {

let result_with_predefined_refunds = vm
.vm
.finish_batch(Some(default_pubdata_builder()))
.finish_batch(default_pubdata_builder())
.block_tip_execution_result;
let mut current_state_with_predefined_refunds = vm.vm.get_current_execution_state();

Expand Down Expand Up @@ -115,7 +115,7 @@ pub(crate) fn test_predetermined_refunded_gas<VM: TestedVm>() {
.push_transaction_with_refund(tx, changed_operator_suggested_refund);
let result = vm
.vm
.finish_batch(Some(default_pubdata_builder()))
.finish_batch(default_pubdata_builder())
.block_tip_execution_result;
let mut current_state_with_changed_predefined_refunds = vm.vm.get_current_execution_state();

Expand Down
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/testonly/simple_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub(crate) fn test_simple_execute<VM: TestedVm>() {
let tx = vm.execute(InspectExecutionMode::OneTx);
assert_matches!(tx.result, ExecutionResult::Success { .. });
let block_tip = vm
.finish_batch(Some(default_pubdata_builder()))
.finish_batch(default_pubdata_builder())
.block_tip_execution_result;
assert_matches!(block_tip.result, ExecutionResult::Success { .. });
}
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl<VM: TestedVm> VmTester<VM> {
for tx_test_info in txs {
self.execute_tx_and_verify(tx_test_info.clone());
}
self.vm.finish_batch(Some(default_pubdata_builder()));
self.vm.finish_batch(default_pubdata_builder());
let mut state = self.vm.get_current_execution_state();
state.used_contract_hashes.sort();
state
Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/versions/testonly/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn test_send_or_transfer<VM: TestedVm>(test_option: TestOptions) {

let batch_result = vm
.vm
.finish_batch(Some(default_pubdata_builder()))
.finish_batch(default_pubdata_builder())
.block_tip_execution_result;
assert!(!batch_result.result.is_failed(), "Batch wasn't successful");

Expand Down Expand Up @@ -191,7 +191,7 @@ fn test_reentrancy_protection_send_or_transfer<VM: TestedVm>(test_option: TestOp

let batch_result = vm
.vm
.finish_batch(Some(default_pubdata_builder()))
.finish_batch(default_pubdata_builder())
.block_tip_execution_result;
assert!(!batch_result.result.is_failed(), "Batch wasn't successful");
}
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_1_3_2/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
self.vm
.execute_till_block_end(
crate::vm_1_3_2::vm_with_bootloader::BootloaderJobType::BlockPostprocessing,
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_1_4_1/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let result = self.inspect_inner(
&mut TracerDispatcher::default(),
VmExecutionMode::Batch,
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_1_4_2/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let result = self.inspect_inner(TracerDispatcher::default(), VmExecutionMode::Batch, None);
let execution_state = self.get_current_execution_state();
let bootloader_memory = self.bootloader_state.bootloader_memory();
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_boojum_integration/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let result = self.inspect_inner(&mut TracerDispatcher::default(), VmExecutionMode::Batch);
let execution_state = self.get_current_execution_state();
let bootloader_memory = self.bootloader_state.bootloader_memory();
Expand Down
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/vm_fast/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl TestedVm for Vm<ImmutableStorageView<InMemoryStorage>> {
pubdata_builder: Rc<dyn PubdataBuilder>,
) -> VmExecutionResultAndLogs {
self.enforce_state_diffs(diffs);
self.finish_batch(Some(pubdata_builder))
self.finish_batch(pubdata_builder)
.block_tip_execution_result
}

Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_fast/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,10 +689,7 @@ impl<S: ReadStorage, Tr: Tracer + Default + 'static> VmInterface for Vm<S, Tr> {
self.bootloader_state.start_new_l2_block(l2_block_env)
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let result = self.inspect_inner(&mut Tr::default(), VmExecutionMode::Batch);
let execution_state = self.get_current_execution_state();
let bootloader_memory = self.bootloader_state.bootloader_memory();
Expand Down
3 changes: 1 addition & 2 deletions core/lib/multivm/src/versions/vm_latest/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch {
let pubdata_builder = pubdata_builder.expect("`pubdata_builder` is required");
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let pubdata_tracer = Some(PubdataTracer::new(
self.batch_env.clone(),
VmExecutionMode::Batch,
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_m5/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,7 @@ impl<S: Storage, H: HistoryMode> VmInterface for Vm<S, H> {
)
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
self.vm
.execute_till_block_end(
crate::vm_m5::vm_with_bootloader::BootloaderJobType::BlockPostprocessing,
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_m6/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ impl<S: Storage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
self.vm
.execute_till_block_end(
crate::vm_m6::vm_with_bootloader::BootloaderJobType::BlockPostprocessing,
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_refunds_enhancement/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let result = self.inspect_inner(&mut TracerDispatcher::default(), VmExecutionMode::Batch);
let execution_state = self.get_current_execution_state();
let bootloader_memory = self.bootloader_state.bootloader_memory();
Expand Down
5 changes: 1 addition & 4 deletions core/lib/multivm/src/versions/vm_virtual_blocks/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
}
}

fn finish_batch(
&mut self,
_pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
) -> FinishedL1Batch {
fn finish_batch(&mut self, _pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let result = self.inspect_inner(&mut TracerDispatcher::default(), VmExecutionMode::Batch);
let execution_state = self.get_current_execution_state();
let bootloader_memory = self.bootloader_state.bootloader_memory();
Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/vm_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<S: ReadStorage, H: HistoryMode> VmInterface for LegacyVmInstance<S, H> {
}

/// Return the results of execution of all batch
fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch {
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
dispatch_legacy_vm!(self.finish_batch(pubdata_builder))
}
}
Expand Down Expand Up @@ -293,7 +293,7 @@ impl<S: ReadStorage, Tr: Tracer + Default + 'static> VmInterface for FastVmInsta
}
}

fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch {
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
dispatch_fast_vm!(self.finish_batch(pubdata_builder))
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/tee_verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ fn execute_vm<S: ReadStorage>(

tracing::trace!("about to vm.finish_batch()");

Ok(vm.finish_batch(Some(pubdata_params_to_builder(pubdata_params))))
Ok(vm.finish_batch(pubdata_params_to_builder(pubdata_params)))
}

/// Map `LogQuery` and `TreeLogEntry` to a `TreeInstruction`
Expand Down
8 changes: 4 additions & 4 deletions core/lib/vm_executor/src/batch/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ impl<S: ReadStorage + Send + 'static, Tr: BatchTracer> BatchExecutorFactory<S>
storage,
l1_batch_params,
system_env,
Some(pubdata_params_to_builder(pubdata_params)),
pubdata_params_to_builder(pubdata_params),
)
});
Box::new(MainBatchExecutor::new(handle, commands_sender))
Expand Down Expand Up @@ -192,7 +192,7 @@ impl<S: ReadStorage, Tr: BatchTracer> BatchVm<S, Tr> {
dispatch_batch_vm!(self.start_new_l2_block(l2_block));
}

fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch {
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
dispatch_batch_vm!(self.finish_batch(pubdata_builder))
}

Expand Down Expand Up @@ -269,7 +269,7 @@ impl<S: ReadStorage + 'static, Tr: BatchTracer> CommandReceiver<S, Tr> {
storage: S,
l1_batch_params: L1BatchEnv,
system_env: SystemEnv,
pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
pubdata_builder: Rc<dyn PubdataBuilder>,
) -> anyhow::Result<StorageView<S>> {
tracing::info!("Starting executing L1 batch #{}", &l1_batch_params.number);

Expand Down Expand Up @@ -378,7 +378,7 @@ impl<S: ReadStorage + 'static, Tr: BatchTracer> CommandReceiver<S, Tr> {
fn finish_batch(
&self,
vm: &mut BatchVm<S, Tr>,
pubdata_builder: Option<Rc<dyn PubdataBuilder>>,
pubdata_builder: Rc<dyn PubdataBuilder>,
) -> anyhow::Result<FinishedL1Batch> {
// The vm execution was paused right after the last transaction was executed.
// There is some post-processing work that the VM needs to do before the block is fully processed.
Expand Down
2 changes: 1 addition & 1 deletion core/lib/vm_interface/src/utils/dump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<S: ReadStorage, Vm: VmTrackingContracts> VmInterface for DumpingVm<S, Vm> {
.inspect_transaction_with_bytecode_compression(tracer, tx, with_compression)
}

fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch {
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
self.inner.finish_batch(pubdata_builder)
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/vm_interface/src/utils/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ where
(main_bytecodes_result, main_tx_result)
}

fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch {
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch {
let main_batch = self.main.finish_batch(pubdata_builder.clone());
if let Some(shadow) = self.shadow.get_mut() {
let shadow_batch = shadow.vm.finish_batch(pubdata_builder.clone());
Expand Down
2 changes: 1 addition & 1 deletion core/lib/vm_interface/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub trait VmInterface {

/// Execute batch till the end and return the result, with final execution state
/// and bootloader memory.
fn finish_batch(&mut self, pubdata_builder: Option<Rc<dyn PubdataBuilder>>) -> FinishedL1Batch;
fn finish_batch(&mut self, pubdata_builder: Rc<dyn PubdataBuilder>) -> FinishedL1Batch;
}

/// Extension trait for [`VmInterface`] that provides some additional methods.
Expand Down
12 changes: 8 additions & 4 deletions core/node/commitment_generator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,11 @@ impl CommitmentGenerator {
.connection_pool
.connection_tagged("commitment_generator")
.await?;
let aggregation_root = read_aggregation_root(&mut connection, l1_batch_number).await?;
let aggregation_root = if protocol_version.is_pre_gateway() {
read_aggregation_root(&mut connection, l1_batch_number).await?
} else {
H256::zero()
};

CommitmentInput::PostBoojum {
common,
Expand Down Expand Up @@ -384,9 +388,9 @@ impl CommitmentGenerator {
// Do nothing
}
(L1BatchCommitmentMode::Validium, CommitmentInput::PostBoojum { blob_hashes, .. }) => {
blob_hashes
.iter_mut()
.for_each(|b| b.commitment = H256::zero());
for hashes in blob_hashes {
hashes.commitment = H256::zero();
}
}
(L1BatchCommitmentMode::Validium, _) => { /* Do nothing */ }
}
Expand Down
9 changes: 8 additions & 1 deletion core/node/consensus/src/storage/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ fn to_fetched_block(
.context("Integer overflow converting block number")?,
);
let payload = Payload::decode(payload).context("Payload::decode()")?;
let pubdata_params = if payload.protocol_version.is_pre_gateway() {
payload.pubdata_params.unwrap_or_default()
} else {
payload
.pubdata_params
.context("Missing `pubdata_params` for post-gateway payload")?
};
Ok(FetchedBlock {
number,
l1_batch_number: payload.l1_batch_number,
Expand All @@ -38,7 +45,7 @@ fn to_fetched_block(
l1_gas_price: payload.l1_gas_price,
l2_fair_gas_price: payload.l2_fair_gas_price,
fair_pubdata_price: payload.fair_pubdata_price,
pubdata_params: payload.pubdata_params.unwrap_or_default(),
pubdata_params,
virtual_blocks: payload.virtual_blocks,
operator_address: payload.operator_address,
transactions: payload
Expand Down
11 changes: 10 additions & 1 deletion core/node/node_sync/src/fetcher.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::Context;
use zksync_dal::{Connection, Core, CoreDal};
use zksync_shared_metrics::{TxStage, APP_METRICS};
use zksync_state_keeper::io::{common::IoCursor, L1BatchParams, L2BlockParams};
Expand Down Expand Up @@ -78,6 +79,14 @@ impl TryFrom<SyncBlock> for FetchedBlock {
));
}

let pubdata_params = if block.protocol_version.is_pre_gateway() {
block.pubdata_params.unwrap_or_default()
} else {
block
.pubdata_params
.context("Missing `pubdata_params` for post-gateway payload")?
};

Ok(Self {
number: block.number,
l1_batch_number: block.l1_batch_number,
Expand All @@ -94,7 +103,7 @@ impl TryFrom<SyncBlock> for FetchedBlock {
.into_iter()
.map(FetchedTransaction::new)
.collect(),
pubdata_params: block.pubdata_params.unwrap_or_default(),
pubdata_params,
})
}
}
Expand Down
Loading

0 comments on commit b22e77c

Please sign in to comment.