Skip to content

Commit

Permalink
cherry-pick: Make calculation for pubdata a bit more percise (#392)
Browse files Browse the repository at this point in the history
Calculate pubdata published only once for tx

For boojum pubdata calculation is getting more complicated in terms of
tx size and for keeping everything precise we can calculate pubdata only
once

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [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 <[email protected]>
  • Loading branch information
Deniallugo authored and ly0va committed Nov 3, 2023
1 parent 8603a1b commit 5001075
Show file tree
Hide file tree
Showing 15 changed files with 51 additions and 9 deletions.
6 changes: 6 additions & 0 deletions core/lib/multivm/src/glue/types/vm/vm_block_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl GlueFrom<crate::vm_m5::vm::VmBlockResult> 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(),
},
Expand Down Expand Up @@ -52,6 +53,7 @@ impl GlueFrom<crate::vm_m6::vm::VmBlockResult> 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(),
},
Expand Down Expand Up @@ -89,6 +91,7 @@ impl GlueFrom<crate::vm_1_3_2::vm::VmBlockResult> 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(),
},
Expand Down Expand Up @@ -135,6 +138,7 @@ impl GlueFrom<crate::vm_1_3_2::vm::VmBlockResult> 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(),
}
Expand Down Expand Up @@ -162,6 +166,7 @@ impl GlueFrom<crate::vm_m5::vm::VmBlockResult> 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(),
}
Expand Down Expand Up @@ -195,6 +200,7 @@ impl GlueFrom<crate::vm_m6::vm::VmBlockResult> 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(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ impl GlueFrom<crate::vm_m5::vm::VmPartialExecutionResult>
gas_used: 0,
// There are no such fields in m5
computational_gas_used: 0,
pubdata_published: 0,
},
refunds: crate::interface::Refunds {
gas_refunded: 0,
Expand All @@ -37,6 +38,7 @@ impl GlueFrom<crate::vm_m6::vm::VmPartialExecutionResult>
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,
Expand All @@ -59,6 +61,7 @@ impl GlueFrom<crate::vm_1_3_2::vm::VmPartialExecutionResult>
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,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,
}
}
}
1 change: 1 addition & 0 deletions core/lib/multivm/src/interface/types/outputs/statistic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,23 +62,24 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {

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,
&tx_tracer,
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
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(
Expand All @@ -38,6 +39,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
gas_used: gas_remaining_before - gas_remaining_after,
computational_gas_used,
total_log_queries: total_log_queries_count,
pubdata_published,
}
}

Expand Down
7 changes: 7 additions & 0 deletions core/lib/multivm/src/versions/vm_latest/tracers/refunds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -62,6 +63,7 @@ impl RefundsTracer {
spent_pubdata_counter_before: 0,
gas_spent_on_bytecodes_and_long_messages: 0,
l1_batch,
pubdata_published: 0,
}
}
}
Expand Down Expand Up @@ -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<S, H: HistoryMode> DynTracer<S, H> for RefundsTracer {
Expand Down Expand Up @@ -235,6 +241,7 @@ impl<S: WriteStorage, H: HistoryMode> VmTracer<S, H> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
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,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
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,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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,
}
}
Expand Down Expand Up @@ -225,6 +227,7 @@ impl<S: WriteStorage, H: HistoryMode> ExecutionProcessing<S, H> 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(
Expand Down Expand Up @@ -388,6 +391,7 @@ impl<S: WriteStorage, H: HistoryMode> VmTracer<S, H> 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;
}
}
1 change: 1 addition & 0 deletions core/lib/types/src/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub struct TransactionExecutionMetrics {
pub cycles_used: u32,
pub computational_gas_used: u32,
pub total_updated_values_size: usize,
pub pubdata_published: u32,
}

#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
Expand Down
3 changes: 3 additions & 0 deletions core/lib/types/src/tx/tx_execution_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub struct ExecutionMetrics {
pub total_log_queries: usize,
pub cycles_used: u32,
pub computational_gas_used: u32,
pub pubdata_published: u32,
}

impl ExecutionMetrics {
Expand All @@ -74,6 +75,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,
}
}

Expand Down Expand Up @@ -104,6 +106,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,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,5 +238,6 @@ pub(super) fn collect_tx_execution_metrics(
cycles_used: result.statistics.cycles_used,
computational_gas_used: result.statistics.computational_gas_used,
total_updated_values_size: writes_metrics.total_updated_values_size,
pubdata_published: result.statistics.pubdata_published,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
1 change: 1 addition & 0 deletions core/lib/zksync_core/src/state_keeper/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ pub(super) fn create_execution_result(
gas_used: 0,
computational_gas_used: 0,
total_log_queries,
pubdata_published: 0,
},
refunds: Refunds::default(),
}
Expand Down

0 comments on commit 5001075

Please sign in to comment.