From 6d0e61cba86d61b68f3657852283dd99d2b6530f Mon Sep 17 00:00:00 2001 From: Danil Date: Thu, 2 Nov 2023 17:38:54 +0100 Subject: [PATCH] feat(vm): Make calculation for pubdata a bit more percise (#392) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What ❔ Calculate pubdata published only once for tx ## Why ❔ For boojum pubdata calculation is getting more complicated in terms of tx size and for keeping everything precise we can calculate pubdata only once ## 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 `zk fmt` and `zk lint`. --------- Signed-off-by: Danil --- .../multivm/src/glue/types/vm/vm_block_result.rs | 6 ++++++ .../glue/types/vm/vm_partial_execution_result.rs | 3 +++ .../src/interface/types/outputs/execution_result.rs | 1 + .../src/interface/types/outputs/statistic.rs | 1 + .../versions/vm_latest/implementation/execution.rs | 13 +++++++------ .../versions/vm_latest/implementation/statistics.rs | 2 ++ .../src/versions/vm_latest/tracers/refunds.rs | 7 +++++++ .../vm_virtual_blocks/implementation/statistics.rs | 2 ++ .../versions/vm_virtual_blocks/tracers/refunds.rs | 6 +++++- core/lib/types/src/fee.rs | 1 + core/lib/types/src/tx/tx_execution_info.rs | 3 +++ .../src/api_server/execution_sandbox/vm_metrics.rs | 1 + .../seal_criteria/criteria/pubdata_bytes.rs | 11 +++++++++-- core/lib/zksync_core/src/state_keeper/tests/mod.rs | 1 + 14 files changed, 49 insertions(+), 9 deletions(-) diff --git a/core/lib/multivm/src/glue/types/vm/vm_block_result.rs b/core/lib/multivm/src/glue/types/vm/vm_block_result.rs index e7d7939b669..00d5751f480 100644 --- a/core/lib/multivm/src/glue/types/vm/vm_block_result.rs +++ b/core/lib/multivm/src/glue/types/vm/vm_block_result.rs @@ -27,6 +27,7 @@ impl GlueFrom for crate::interface::FinishedL1B total_log_queries: value.block_tip_result.logs.total_log_queries_count, computational_gas_used: value.full_result.gas_used, gas_used: value.full_result.gas_used, + pubdata_published: 0, }, refunds: Refunds::default(), }, @@ -61,6 +62,7 @@ impl GlueFrom for crate::interface::FinishedL1B total_log_queries: value.block_tip_result.logs.total_log_queries_count, computational_gas_used: value.full_result.computational_gas_used, gas_used: value.full_result.gas_used, + pubdata_published: 0, }, refunds: Refunds::default(), }, @@ -95,6 +97,7 @@ impl GlueFrom for crate::interface::Finished total_log_queries: value.block_tip_result.logs.total_log_queries_count, computational_gas_used: value.full_result.computational_gas_used, gas_used: value.full_result.gas_used, + pubdata_published: 0, }, refunds: Refunds::default(), }, @@ -138,6 +141,7 @@ impl GlueFrom for crate::interface::VmExecut total_log_queries: value.full_result.total_log_queries, computational_gas_used: value.full_result.computational_gas_used, gas_used: value.full_result.gas_used, + pubdata_published: 0, }, refunds: Refunds::default(), } @@ -170,6 +174,7 @@ impl GlueFrom for crate::interface::VmExecution total_log_queries: value.full_result.total_log_queries, computational_gas_used: 0, gas_used: value.full_result.gas_used, + pubdata_published: 0, }, refunds: Refunds::default(), } @@ -202,6 +207,7 @@ impl GlueFrom for crate::interface::VmExecution total_log_queries: value.full_result.total_log_queries, computational_gas_used: value.full_result.computational_gas_used, gas_used: value.full_result.gas_used, + pubdata_published: 0, }, refunds: Refunds::default(), } diff --git a/core/lib/multivm/src/glue/types/vm/vm_partial_execution_result.rs b/core/lib/multivm/src/glue/types/vm/vm_partial_execution_result.rs index dc9bac295a3..096eaaa3b8d 100644 --- a/core/lib/multivm/src/glue/types/vm/vm_partial_execution_result.rs +++ b/core/lib/multivm/src/glue/types/vm/vm_partial_execution_result.rs @@ -21,6 +21,7 @@ impl GlueFrom gas_used: 0, // There are no such fields in m5 computational_gas_used: 0, + pubdata_published: 0, }, refunds: crate::interface::Refunds { gas_refunded: 0, @@ -48,6 +49,7 @@ impl GlueFrom gas_used: value.computational_gas_used, computational_gas_used: value.computational_gas_used, total_log_queries: value.logs.total_log_queries_count, + pubdata_published: 0, }, refunds: crate::interface::Refunds { gas_refunded: 0, @@ -75,6 +77,7 @@ impl GlueFrom gas_used: value.computational_gas_used, computational_gas_used: value.computational_gas_used, total_log_queries: value.logs.total_log_queries_count, + pubdata_published: 0, }, refunds: crate::interface::Refunds { gas_refunded: 0, diff --git a/core/lib/multivm/src/interface/types/outputs/execution_result.rs b/core/lib/multivm/src/interface/types/outputs/execution_result.rs index 5fed6a609b3..dfbcd23be0b 100644 --- a/core/lib/multivm/src/interface/types/outputs/execution_result.rs +++ b/core/lib/multivm/src/interface/types/outputs/execution_result.rs @@ -78,6 +78,7 @@ impl VmExecutionResultAndLogs { total_log_queries: self.statistics.total_log_queries, cycles_used: self.statistics.cycles_used, computational_gas_used: self.statistics.computational_gas_used, + pubdata_published: self.statistics.pubdata_published, } } } diff --git a/core/lib/multivm/src/interface/types/outputs/statistic.rs b/core/lib/multivm/src/interface/types/outputs/statistic.rs index da960cbbd0f..c1312fc95da 100644 --- a/core/lib/multivm/src/interface/types/outputs/statistic.rs +++ b/core/lib/multivm/src/interface/types/outputs/statistic.rs @@ -11,6 +11,7 @@ pub struct VmExecutionStatistics { pub computational_gas_used: u32, /// Number of log queries produced by the VM during the tx execution. pub total_log_queries: usize, + pub pubdata_published: u32, } /// Oracle metrics of the VM. diff --git a/core/lib/multivm/src/versions/vm_latest/implementation/execution.rs b/core/lib/multivm/src/versions/vm_latest/implementation/execution.rs index cd3254f109f..50ee4b53b73 100644 --- a/core/lib/multivm/src/versions/vm_latest/implementation/execution.rs +++ b/core/lib/multivm/src/versions/vm_latest/implementation/execution.rs @@ -59,6 +59,12 @@ impl Vm { let logs = self.collect_execution_logs_after_timestamp(timestamp_initial); + let (refunds, pubdata_published) = tx_tracer + .refund_tracer + .as_ref() + .map(|x| (x.get_refunds(), x.pubdata_published())) + .unwrap_or_default(); + let statistics = self.get_statistics( timestamp_initial, cycles_initial, @@ -66,16 +72,11 @@ impl Vm { gas_remaining_before, gas_remaining_after, spent_pubdata_counter_before, + pubdata_published, logs.total_log_queries_count, ); - let result = tx_tracer.result_tracer.into_result(); - let refunds = tx_tracer - .refund_tracer - .map(|x| x.get_refunds()) - .unwrap_or_default(); - let result = VmExecutionResultAndLogs { result, logs, diff --git a/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs index 530ad45a1af..3706e458257 100644 --- a/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs @@ -22,6 +22,7 @@ impl Vm { gas_remaining_before: u32, gas_remaining_after: u32, spent_pubdata_counter_before: u32, + pubdata_published: u32, total_log_queries_count: usize, ) -> VmExecutionStatistics { let computational_gas_used = self.calculate_computational_gas_used( @@ -38,6 +39,7 @@ impl Vm { gas_used: gas_remaining_before - gas_remaining_after, computational_gas_used, total_log_queries: total_log_queries_count, + pubdata_published, } } diff --git a/core/lib/multivm/src/versions/vm_latest/tracers/refunds.rs b/core/lib/multivm/src/versions/vm_latest/tracers/refunds.rs index 8b1a43d8da3..119551c86f1 100644 --- a/core/lib/multivm/src/versions/vm_latest/tracers/refunds.rs +++ b/core/lib/multivm/src/versions/vm_latest/tracers/refunds.rs @@ -48,6 +48,7 @@ pub(crate) struct RefundsTracer { spent_pubdata_counter_before: u32, gas_spent_on_bytecodes_and_long_messages: u32, l1_batch: L1BatchEnv, + pubdata_published: u32, } impl RefundsTracer { @@ -62,6 +63,7 @@ impl RefundsTracer { spent_pubdata_counter_before: 0, gas_spent_on_bytecodes_and_long_messages: 0, l1_batch, + pubdata_published: 0, } } } @@ -142,6 +144,10 @@ impl RefundsTracer { pub(crate) fn gas_spent_on_pubdata(&self, vm_local_state: &VmLocalState) -> u32 { self.gas_spent_on_bytecodes_and_long_messages + vm_local_state.spent_pubdata_counter } + + pub(crate) fn pubdata_published(&self) -> u32 { + self.pubdata_published + } } impl DynTracer for RefundsTracer { @@ -235,6 +241,7 @@ impl VmTracer for RefundsTracer { self.l1_batch.number, ); + self.pubdata_published = pubdata_published; let current_ergs_per_pubdata_byte = state.local_state.current_ergs_per_pubdata_byte; let tx_body_refund = self.tx_body_refund( bootloader_refund, diff --git a/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs index b51a3afc7ea..98bc5430bea 100644 --- a/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs @@ -38,6 +38,8 @@ impl Vm { gas_used: gas_remaining_before - gas_remaining_after, computational_gas_used, total_log_queries: total_log_queries_count, + // This field will be populated by the RefundTracer + pubdata_published: 0, } } diff --git a/core/lib/multivm/src/versions/vm_virtual_blocks/tracers/refunds.rs b/core/lib/multivm/src/versions/vm_virtual_blocks/tracers/refunds.rs index 2db5298c2d3..d8db7a2cbaf 100644 --- a/core/lib/multivm/src/versions/vm_virtual_blocks/tracers/refunds.rs +++ b/core/lib/multivm/src/versions/vm_virtual_blocks/tracers/refunds.rs @@ -48,6 +48,7 @@ pub(crate) struct RefundsTracer { gas_remaining_before: u32, spent_pubdata_counter_before: u32, gas_spent_on_bytecodes_and_long_messages: u32, + pubdata_published: u32, l1_batch: L1BatchEnv, } @@ -62,6 +63,7 @@ impl RefundsTracer { gas_remaining_before: 0, spent_pubdata_counter_before: 0, gas_spent_on_bytecodes_and_long_messages: 0, + pubdata_published: 0, l1_batch, } } @@ -225,6 +227,7 @@ impl ExecutionProcessing for RefundsTrace let pubdata_published = pubdata_published(state, self.timestamp_initial, self.l1_batch.number); + self.pubdata_published = pubdata_published; let current_ergs_per_pubdata_byte = state.local_state.current_ergs_per_pubdata_byte; let tx_body_refund = self.tx_body_refund( @@ -388,6 +391,7 @@ impl VmTracer for RefundsTracer { result.refunds = Refunds { gas_refunded: self.refund_gas, operator_suggested_refund: self.operator_refund.unwrap_or_default(), - } + }; + result.statistics.pubdata_published = self.pubdata_published; } } diff --git a/core/lib/types/src/fee.rs b/core/lib/types/src/fee.rs index 970b8d95664..4c614229b1d 100644 --- a/core/lib/types/src/fee.rs +++ b/core/lib/types/src/fee.rs @@ -22,6 +22,7 @@ pub struct TransactionExecutionMetrics { pub total_log_queries: usize, pub cycles_used: u32, pub computational_gas_used: u32, + pub pubdata_published: u32, } #[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] diff --git a/core/lib/types/src/tx/tx_execution_info.rs b/core/lib/types/src/tx/tx_execution_info.rs index 32f2f3c5ef1..c734c715f08 100644 --- a/core/lib/types/src/tx/tx_execution_info.rs +++ b/core/lib/types/src/tx/tx_execution_info.rs @@ -64,6 +64,7 @@ pub struct ExecutionMetrics { pub total_log_queries: usize, pub cycles_used: u32, pub computational_gas_used: u32, + pub pubdata_published: u32, } impl ExecutionMetrics { @@ -80,6 +81,7 @@ impl ExecutionMetrics { total_log_queries: tx_metrics.total_log_queries, cycles_used: tx_metrics.cycles_used, computational_gas_used: tx_metrics.computational_gas_used, + pubdata_published: tx_metrics.pubdata_published, } } @@ -107,6 +109,7 @@ impl Add for ExecutionMetrics { total_log_queries: self.total_log_queries + other.total_log_queries, cycles_used: self.cycles_used + other.cycles_used, computational_gas_used: self.computational_gas_used + other.computational_gas_used, + pubdata_published: self.pubdata_published + other.pubdata_published, } } } diff --git a/core/lib/zksync_core/src/api_server/execution_sandbox/vm_metrics.rs b/core/lib/zksync_core/src/api_server/execution_sandbox/vm_metrics.rs index 96982293a95..7bc1d39145e 100644 --- a/core/lib/zksync_core/src/api_server/execution_sandbox/vm_metrics.rs +++ b/core/lib/zksync_core/src/api_server/execution_sandbox/vm_metrics.rs @@ -237,5 +237,6 @@ pub(super) fn collect_tx_execution_metrics( total_log_queries: result.statistics.total_log_queries, cycles_used: result.statistics.cycles_used, computational_gas_used: result.statistics.computational_gas_used, + pubdata_published: result.statistics.pubdata_published, } } diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs index 70f35f4278e..6142f258d79 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs @@ -21,9 +21,16 @@ impl SealCriterion for PubDataBytesCriterion { (max_pubdata_per_l1_batch as f64 * config.reject_tx_at_eth_params_percentage).round(); let include_and_seal_bound = (max_pubdata_per_l1_batch as f64 * config.close_block_at_eth_params_percentage).round(); - let block_size = block_data.execution_metrics.size() + block_data.writes_metrics.size(); - let tx_size = tx_data.execution_metrics.size() + tx_data.writes_metrics.size(); + let block_size = block_data.execution_metrics.size() + block_data.writes_metrics.size(); + // For backward compatibility, we need to keep calculating the size of the pubdata based + // StorageDeduplication metrics. All vm versions + // after vm with virtual blocks will provide the size of the pubdata in the execution metrics. + let tx_size = if tx_data.execution_metrics.pubdata_published == 0 { + tx_data.execution_metrics.size() + tx_data.writes_metrics.size() + } else { + tx_data.execution_metrics.pubdata_published as usize + }; if tx_size > reject_bound as usize { let message = "Transaction cannot be sent to L1 due to pubdata limits"; SealResolution::Unexecutable(message.into()) diff --git a/core/lib/zksync_core/src/state_keeper/tests/mod.rs b/core/lib/zksync_core/src/state_keeper/tests/mod.rs index 7c61e7de80f..6d797e73c5f 100644 --- a/core/lib/zksync_core/src/state_keeper/tests/mod.rs +++ b/core/lib/zksync_core/src/state_keeper/tests/mod.rs @@ -191,6 +191,7 @@ pub(super) fn create_execution_result( gas_used: 0, computational_gas_used: 0, total_log_queries, + pubdata_published: 0, }, refunds: Refunds::default(), }