Skip to content

Commit

Permalink
fix(state-keeper): Add GasForBatchTip criterion (#1096)
Browse files Browse the repository at this point in the history
## What ❔

- add GasForBatchTip criterion instead of checking gas in batch executor
- removes the assumption that a single tx can't consume whole batch gas
(with or without batch tip)

## Why ❔

That check in batch executor
```
if !vm.has_enough_gas_for_batch_tip() {
    return TxExecutionResult::BootloaderOutOfGasForBlockTip;
}
```
conceptually was a criterion and should be extracted to a separate seal
criteria. This way EN doesn't perform this check which is a correct
behavior and code is cleaner.


## 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`.
- [x] Spellcheck has been run via `zk spellcheck`.
- [x] Linkcheck has been run via `zk linkcheck`.
  • Loading branch information
perekopskiy committed Feb 16, 2024
1 parent 6de8b84 commit de4d729
Show file tree
Hide file tree
Showing 39 changed files with 263 additions and 121 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 @@ -43,6 +43,7 @@ impl GlueFrom<crate::vm_m5::vm_instance::VmBlockResult> for crate::interface::Fi
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,
gas_remaining: value.full_result.gas_remaining,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down Expand Up @@ -104,6 +105,7 @@ impl GlueFrom<crate::vm_m6::vm_instance::VmBlockResult> for crate::interface::Fi
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,
gas_remaining: value.full_result.gas_remaining,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down Expand Up @@ -163,6 +165,7 @@ impl GlueFrom<crate::vm_1_3_2::vm_instance::VmBlockResult> for crate::interface:
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,
gas_remaining: value.full_result.gas_remaining,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down Expand Up @@ -236,6 +239,7 @@ impl GlueFrom<crate::vm_1_3_2::vm_instance::VmBlockResult>
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,
gas_remaining: value.full_result.gas_remaining,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down Expand Up @@ -267,6 +271,7 @@ impl GlueFrom<crate::vm_m5::vm_instance::VmBlockResult>
total_log_queries: value.full_result.total_log_queries,
computational_gas_used: 0,
gas_used: value.full_result.gas_used,
gas_remaining: value.full_result.gas_remaining,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down Expand Up @@ -314,6 +319,7 @@ impl GlueFrom<crate::vm_m6::vm_instance::VmBlockResult>
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,
gas_remaining: value.full_result.gas_remaining,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ impl GlueFrom<crate::vm_m5::vm_instance::VmPartialExecutionResult>
contracts_used: value.contracts_used,
cycles_used: value.cycles_used,
total_log_queries: value.logs.total_log_queries_count,
// There are no such fields in `m5`
// There are no such fields in `m5`.
gas_used: 0,
// There are no such fields in `m5`
gas_remaining: 0,
computational_gas_used: 0,
pubdata_published: 0,
circuit_statistic: Default::default(),
Expand All @@ -36,9 +36,11 @@ impl GlueFrom<crate::vm_m6::vm_instance::VmPartialExecutionResult>
statistics: crate::interface::VmExecutionStatistics {
contracts_used: value.contracts_used,
cycles_used: value.cycles_used,
gas_used: value.computational_gas_used,
computational_gas_used: value.computational_gas_used,
total_log_queries: value.logs.total_log_queries_count,
// There are no such fields in `m6`.
gas_used: 0,
gas_remaining: 0,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand All @@ -60,9 +62,11 @@ impl GlueFrom<crate::vm_1_3_2::vm_instance::VmPartialExecutionResult>
statistics: crate::interface::VmExecutionStatistics {
contracts_used: value.contracts_used,
cycles_used: value.cycles_used,
gas_used: value.computational_gas_used,
computational_gas_used: value.computational_gas_used,
total_log_queries: value.logs.total_log_queries_count,
// There are no such fields in `1_3_2`.
gas_used: 0,
gas_remaining: 0,
pubdata_published: 0,
circuit_statistic: Default::default(),
},
Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/interface/traits/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ pub trait VmInterface<S, H: HistoryMode> {
/// Record VM memory metrics.
fn record_vm_memory_metrics(&self) -> VmMemoryMetrics;

/// Whether the VM still has enough gas to execute the batch tip
fn has_enough_gas_for_batch_tip(&self) -> bool;
/// How much gas is left in the current stack frame.
fn gas_remaining(&self) -> u32;

/// Execute batch till the end and return the result, with final execution state
/// and bootloader memory.
Expand Down
2 changes: 2 additions & 0 deletions core/lib/multivm/src/interface/types/outputs/statistic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ pub struct VmExecutionStatistics {
pub cycles_used: u32,
/// Gas used by the VM during the tx execution.
pub gas_used: u32,
/// Gas remaining after the tx execution.
pub gas_remaining: u32,
/// Computational gas used by the VM during the tx execution.
pub computational_gas_used: u32,
/// Number of log queries produced by the VM during the tx execution.
Expand Down
19 changes: 19 additions & 0 deletions core/lib/multivm/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,25 @@ pub fn get_bootloader_max_txs_in_batch(version: VmVersion) -> usize {
}
}

pub fn gas_bootloader_batch_tip_overhead(version: VmVersion) -> u32 {
match version {
VmVersion::M5WithRefunds
| VmVersion::M5WithoutRefunds
| VmVersion::M6Initial
| VmVersion::M6BugWithCompressionFixed
| VmVersion::Vm1_3_2
| VmVersion::VmVirtualBlocks
| VmVersion::VmVirtualBlocksRefundsEnhancement => {
// For these versions the overhead has not been calculated and it has not been used with those versions.
0
}
VmVersion::VmBoojumIntegration => {
crate::vm_boojum_integration::constants::BOOTLOADER_BATCH_TIP_OVERHEAD
}
VmVersion::Vm1_4_1 => crate::vm_latest::constants::BOOTLOADER_BATCH_TIP_OVERHEAD,
}
}

pub fn get_max_gas_per_pubdata_byte(version: VmVersion) -> u64 {
match version {
VmVersion::M5WithRefunds | VmVersion::M5WithoutRefunds => {
Expand Down
6 changes: 2 additions & 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 @@ -284,10 +284,8 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface<S, H> for Vm<S, H> {
}
}

fn has_enough_gas_for_batch_tip(&self) -> bool {
// For this version this overhead has not been calculated and it has not been used with those versions.
// We return some value just in case for backwards compatibility
true
fn gas_remaining(&self) -> u32 {
self.vm.gas_remaining()
}

fn finish_batch(&mut self) -> FinishedL1Batch {
Expand Down
10 changes: 7 additions & 3 deletions core/lib/multivm/src/versions/vm_1_3_2/vm_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ pub struct VmExecutionResult {
/// is executed, but it's not enforced. So best we can do is to calculate the amount of gas before and
/// after the invocation, leaving the interpretation of this value to the user.
pub gas_used: u32,
pub gas_remaining: u32,
/// This value also depends on the context, the same as `gas_used`.
pub computational_gas_used: u32,
pub contracts_used: usize,
Expand Down Expand Up @@ -211,9 +212,8 @@ fn vm_may_have_ended<H: HistoryMode, S: WriteStorage>(
) -> Option<VmExecutionResult> {
let basic_execution_result = vm_may_have_ended_inner(&vm.state)?;

let gas_used = gas_before
.checked_sub(vm.gas_remaining())
.expect("underflow");
let gas_remaining = vm.gas_remaining();
let gas_used = gas_before.checked_sub(gas_remaining).expect("underflow");

match basic_execution_result {
NewVmExecutionResult::Ok(data) => {
Expand All @@ -226,6 +226,7 @@ fn vm_may_have_ended<H: HistoryMode, S: WriteStorage>(
l2_to_l1_logs: vec![],
return_data: data,
gas_used,
gas_remaining,
// The correct `computational_gas_used` value for this field should be set separately later.
computational_gas_used: 0,
contracts_used: vm
Expand Down Expand Up @@ -266,6 +267,7 @@ fn vm_may_have_ended<H: HistoryMode, S: WriteStorage>(
l2_to_l1_logs: vec![],
return_data: vec![],
gas_used,
gas_remaining,
// The correct `computational_gas_used` value for this field should be set separately later.
computational_gas_used: 0,
contracts_used: vm
Expand All @@ -289,6 +291,7 @@ fn vm_may_have_ended<H: HistoryMode, S: WriteStorage>(
l2_to_l1_logs: vec![],
return_data: vec![],
gas_used,
gas_remaining,
// The correct `computational_gas_used` value for this field should be set separately later.
computational_gas_used: 0,
contracts_used: vm
Expand Down Expand Up @@ -792,6 +795,7 @@ impl<H: HistoryMode, S: WriteStorage> VmInstance<S, H> {
l2_to_l1_logs: vec![],
return_data: vec![],
gas_used: 0,
gas_remaining: 0,
computational_gas_used: 0,
contracts_used: 0,
revert_reason: Some(VmRevertReasonParsingResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use zksync_state::WriteStorage;
use crate::{
interface::{
types::tracer::{TracerExecutionStatus, VmExecutionStopReason},
VmExecutionMode, VmExecutionResultAndLogs,
VmExecutionMode, VmExecutionResultAndLogs, VmInterface,
},
vm_boojum_integration::{
old_vm::utils::{vm_may_have_ended_inner, VmExecutionResult},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,12 @@
use zksync_state::WriteStorage;

use crate::{
interface::VmInterface,
vm_boojum_integration::{tracers::DefaultExecutionTracer, vm::Vm},
HistoryMode,
};

impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
/// Returns the amount of gas remaining to the VM.
/// Note that this *does not* correspond to the gas limit of a transaction.
/// To calculate the amount of gas spent by transaction, you should call this method before and after
/// the execution, and subtract these values.
///
/// Note: this method should only be called when either transaction is fully completed or VM completed
/// its execution. Remaining gas value is read from the current stack frame, so if you'll attempt to
/// read it during the transaction execution, you may receive invalid value.
pub(crate) fn gas_remaining(&self) -> u32 {
self.state.local_state.callstack.current.ergs_remaining
}

pub(crate) fn calculate_computational_gas_used(
&self,
tracer: &DefaultExecutionTracer<S, H::VmBoojumIntegration>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
.get_decommitted_bytecodes_after_timestamp(timestamp_initial),
cycles_used: self.state.local_state.monotonic_cycle_counter - cycles_initial,
gas_used: gas_remaining_before - gas_remaining_after,
gas_remaining: gas_remaining_after,
computational_gas_used,
total_log_queries: total_log_queries_count,
pubdata_published,
Expand Down
5 changes: 2 additions & 3 deletions core/lib/multivm/src/versions/vm_boojum_integration/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use zksync_types::{
};
use zksync_utils::bytecode::CompressedBytecodeInfo;

use super::constants::BOOTLOADER_BATCH_TIP_OVERHEAD;
use crate::{
glue::GlueInto,
interface::{
Expand Down Expand Up @@ -165,8 +164,8 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface<S, H> for Vm<S, H> {
self.record_vm_memory_metrics_inner()
}

fn has_enough_gas_for_batch_tip(&self) -> bool {
self.state.local_state.callstack.current.ergs_remaining >= BOOTLOADER_BATCH_TIP_OVERHEAD
fn gas_remaining(&self) -> u32 {
self.state.local_state.callstack.current.ergs_remaining
}

fn finish_batch(&mut self) -> FinishedL1Batch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use zksync_state::WriteStorage;
use crate::{
interface::{
types::tracer::{TracerExecutionStatus, VmExecutionStopReason},
VmExecutionMode, VmExecutionResultAndLogs,
VmExecutionMode, VmExecutionResultAndLogs, VmInterface,
},
vm_latest::{
old_vm::utils::{vm_may_have_ended_inner, VmExecutionResult},
Expand Down
13 changes: 1 addition & 12 deletions core/lib/multivm/src/versions/vm_latest/implementation/gas.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,12 @@
use zksync_state::WriteStorage;

use crate::{
interface::VmInterface,
vm_latest::{tracers::DefaultExecutionTracer, vm::Vm},
HistoryMode,
};

impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
/// Returns the amount of gas remaining to the VM.
/// Note that this *does not* correspond to the gas limit of a transaction.
/// To calculate the amount of gas spent by transaction, you should call this method before and after
/// the execution, and subtract these values.
///
/// Note: this method should only be called when either transaction is fully completed or VM completed
/// its execution. Remaining gas value is read from the current stack frame, so if you'll attempt to
/// read it during the transaction execution, you may receive invalid value.
pub(crate) fn gas_remaining(&self) -> u32 {
self.state.local_state.callstack.current.ergs_remaining
}

pub(crate) fn calculate_computational_gas_used(
&self,
tracer: &DefaultExecutionTracer<S, H::Vm1_4_1>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> {
.get_decommitted_bytecodes_after_timestamp(timestamp_initial),
cycles_used: self.state.local_state.monotonic_cycle_counter - cycles_initial,
gas_used: gas_remaining_before - gas_remaining_after,
gas_remaining: gas_remaining_after,
computational_gas_used,
total_log_queries: total_log_queries_count,
pubdata_published,
Expand Down
5 changes: 2 additions & 3 deletions core/lib/multivm/src/versions/vm_latest/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use zksync_types::{
};
use zksync_utils::bytecode::CompressedBytecodeInfo;

use super::constants::BOOTLOADER_BATCH_TIP_OVERHEAD;
use crate::{
glue::GlueInto,
interface::{
Expand Down Expand Up @@ -165,8 +164,8 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface<S, H> for Vm<S, H> {
self.record_vm_memory_metrics_inner()
}

fn has_enough_gas_for_batch_tip(&self) -> bool {
self.state.local_state.callstack.current.ergs_remaining >= BOOTLOADER_BATCH_TIP_OVERHEAD
fn gas_remaining(&self) -> u32 {
self.state.local_state.callstack.current.ergs_remaining
}

fn finish_batch(&mut self) -> FinishedL1Batch {
Expand Down
6 changes: 2 additions & 4 deletions core/lib/multivm/src/versions/vm_m5/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,8 @@ impl<S: Storage, H: HistoryMode> VmInterface<S, H> for Vm<S, H> {
}
}

fn has_enough_gas_for_batch_tip(&self) -> bool {
// For this version this overhead has not been calculated and it has not been used with those versions.
// We return some value just in case for backwards compatibility
true
fn gas_remaining(&self) -> u32 {
self.vm.gas_remaining()
}

fn finish_batch(&mut self) -> FinishedL1Batch {
Expand Down
7 changes: 6 additions & 1 deletion core/lib/multivm/src/versions/vm_m5/vm_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ pub struct VmExecutionResult {
/// is executed, but it's not enforced. So best we can do is to calculate the amount of gas before and
/// after the invocation, leaving the interpretation of this value to the user.
pub gas_used: u32,
pub gas_remaining: u32,
pub contracts_used: usize,
pub revert_reason: Option<VmRevertReasonParsingResult>,
pub trace: VmExecutionTrace,
Expand Down Expand Up @@ -228,7 +229,8 @@ fn vm_may_have_ended_inner<const B: bool, S: Storage>(
fn vm_may_have_ended<S: Storage>(vm: &VmInstance<S>, gas_before: u32) -> Option<VmExecutionResult> {
let basic_execution_result = vm_may_have_ended_inner(&vm.state)?;

let gas_used = gas_before - vm.gas_remaining();
let gas_remaining = vm.gas_remaining();
let gas_used = gas_before - gas_remaining;

match basic_execution_result {
NewVmExecutionResult::Ok(data) => {
Expand All @@ -241,6 +243,7 @@ fn vm_may_have_ended<S: Storage>(vm: &VmInstance<S>, gas_before: u32) -> Option<
l2_to_l1_logs: vec![],
return_data: data,
gas_used,
gas_remaining,
contracts_used: vm
.state
.decommittment_processor
Expand Down Expand Up @@ -279,6 +282,7 @@ fn vm_may_have_ended<S: Storage>(vm: &VmInstance<S>, gas_before: u32) -> Option<
l2_to_l1_logs: vec![],
return_data: vec![],
gas_used,
gas_remaining,
contracts_used: vm
.state
.decommittment_processor
Expand All @@ -300,6 +304,7 @@ fn vm_may_have_ended<S: Storage>(vm: &VmInstance<S>, gas_before: u32) -> Option<
l2_to_l1_logs: vec![],
return_data: vec![],
gas_used,
gas_remaining,
contracts_used: vm
.state
.decommittment_processor
Expand Down
6 changes: 2 additions & 4 deletions core/lib/multivm/src/versions/vm_m6/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,8 @@ impl<S: Storage, H: HistoryMode> VmInterface<S, H> for Vm<S, H> {
}
}

fn has_enough_gas_for_batch_tip(&self) -> bool {
// For this version this overhead has not been calculated and it has not been used with those versions.
// We return some value just in case for backwards compatibility
true
fn gas_remaining(&self) -> u32 {
self.vm.gas_remaining()
}

fn finish_batch(&mut self) -> FinishedL1Batch {
Expand Down

0 comments on commit de4d729

Please sign in to comment.