Skip to content

Commit

Permalink
feat(vm): Make calculation for pubdata a bit more percise (#392)
Browse files Browse the repository at this point in the history
# 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

<!-- 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 <deniallugo@gmail.com>
  • Loading branch information
Deniallugo authored Nov 2, 2023
1 parent f9cc831 commit 6d0e61c
Show file tree
Hide file tree
Showing 14 changed files with 49 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 @@ -27,6 +27,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 @@ -61,6 +62,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 @@ -95,6 +97,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 @@ -138,6 +141,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 @@ -170,6 +174,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 @@ -202,6 +207,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 @@ -21,6 +21,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 Down Expand Up @@ -48,6 +49,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 Down Expand Up @@ -75,6 +77,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 @@ -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,
}
}
}
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 @@ -59,23 +59,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 @@ -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 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)]
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 @@ -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 {
Expand All @@ -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,
}
}

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

0 comments on commit 6d0e61c

Please sign in to comment.