Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(state-keeper): Disable some seal criteria for boojum #390

Merged
merged 3 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/lib/constants/src/ethereum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ pub static ETHEREUM_ADDRESS: Address = Address::zero();
/// to be published inside the body of transaction (i.e. excluding of factory deps).
pub const GUARANTEED_PUBDATA_PER_L1_BATCH: u64 = 4000;

/// The maximum number of pubdata per L1 batch.
/// The maximum number of pubdata per L1 batch. This limit is due to the fact that the Ethereum
/// nodes do not accept transactions that have more than 128kb of pubdata.
/// The 8kb margin is left in case of any inpreciseness of the pubdata calculation.
pub const MAX_PUBDATA_PER_L1_BATCH: u64 = 120000;

// TODO: import from zkevm_opcode_defs once VM1.3 is supported
Expand Down
9 changes: 5 additions & 4 deletions core/lib/types/src/storage/writes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ use crate::H256;
use serde::{Deserialize, Serialize};
use zksync_basic_types::{Address, U256};

use self::compression::{compress_with_best_strategy, COMPRESSION_VERSION_NUMBER};
pub(crate) use self::compression::{compress_with_best_strategy, COMPRESSION_VERSION_NUMBER};

mod compression;

const BYTES_PER_ENUMERATION_INDEX: u8 = 4;
// Total byte size of all fields in StateDiffRecord struct
// 20 + 32 + 32 +8 + 32 + 32
/// The number of bytes being used for state diff enumeration indices. Applicable to repeated writes.
pub const BYTES_PER_ENUMERATION_INDEX: u8 = 4;
/// Total byte size of all fields in StateDiffRecord struct
/// 20 + 32 + 32 + 8 + 32 + 32
const STATE_DIFF_RECORD_SIZE: usize = 156;

// 2 * 136 - the size that allows for two keccak rounds.
Expand Down
8 changes: 7 additions & 1 deletion core/lib/zksync_core/src/api_server/tx_sender/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,13 @@ impl<G: L1GasPriceProvider> TxSender<G> {
};

let seal_data = SealData::for_transaction(transaction, tx_metrics);
if let Some(reason) = ConditionalSealer::find_unexecutable_reason(sk_config, &seal_data) {
// Using `ProtocolVersionId::latest()` for a short period we might end up in a scenario where the StateKeeper is still pre-boojum
// but the API assumes we are post boojum. In this situation we will determine a tx as being executable but the StateKeeper will
// still reject them as it's not.
let protocol_version = ProtocolVersionId::latest();
perekopskiy marked this conversation as resolved.
Show resolved Hide resolved
if let Some(reason) =
ConditionalSealer::find_unexecutable_reason(sk_config, &seal_data, protocol_version)
{
let message = format!(
"Tx is Unexecutable because of {reason}; inputs for decision: {seal_data:?}"
);
Expand Down
11 changes: 10 additions & 1 deletion core/lib/zksync_core/src/gas_tracker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,16 @@ pub(crate) fn commit_gas_count_for_l1_batch(
let total_factory_deps_len: u32 = sorted_factory_deps
.map(|factory_dep| factory_dep.len() as u32)
.sum();
let additional_calldata_bytes = metadata.initial_writes_compressed.len() as u32

// Boojum upgrade changes how storage writes are communicated/compressed.
let state_diff_size = if header.protocol_version.unwrap().is_pre_boojum() {
metadata.initial_writes_compressed.len() as u32
+ metadata.repeated_writes_compressed.len() as u32
} else {
metadata.state_diffs_compressed.len() as u32
};

let additional_calldata_bytes = state_diff_size
+ metadata.repeated_writes_compressed.len() as u32
+ metadata.l2_l1_messages_compressed.len() as u32
+ total_messages_len
Expand Down
1 change: 1 addition & 0 deletions core/lib/zksync_core/src/state_keeper/keeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ impl ZkSyncStateKeeper {
updates_manager.pending_executed_transactions_len() + 1,
&block_data,
&tx_data,
updates_manager.protocol_version(),
)
} else {
SealResolution::NoSeal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! which unconditionally follows the instructions from the main node).

use zksync_config::configs::chain::StateKeeperConfig;
use zksync_types::ProtocolVersionId;

use super::{criteria, SealCriterion, SealData, SealResolution, AGGREGATION_METRICS};

Expand All @@ -22,12 +23,20 @@ impl ConditionalSealer {
pub(crate) fn find_unexecutable_reason(
config: &StateKeeperConfig,
data: &SealData,
protocol_version: ProtocolVersionId,
) -> Option<&'static str> {
for sealer in &Self::default_sealers() {
const MOCK_BLOCK_TIMESTAMP: u128 = 0;
const TX_COUNT: usize = 1;

let resolution = sealer.should_seal(config, MOCK_BLOCK_TIMESTAMP, TX_COUNT, data, data);
let resolution = sealer.should_seal(
config,
MOCK_BLOCK_TIMESTAMP,
TX_COUNT,
data,
data,
protocol_version,
);
if matches!(resolution, SealResolution::Unexecutable(_)) {
return Some(sealer.prom_criterion_name());
}
Expand Down Expand Up @@ -55,6 +64,7 @@ impl ConditionalSealer {
tx_count: usize,
block_data: &SealData,
tx_data: &SealData,
protocol_version: ProtocolVersionId,
) -> SealResolution {
tracing::trace!(
"Determining seal resolution for L1 batch #{l1_batch_number} with {tx_count} transactions \
Expand All @@ -70,6 +80,7 @@ impl ConditionalSealer {
tx_count,
block_data,
tx_data,
protocol_version,
);
match &seal_resolution {
SealResolution::IncludeAndSeal
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use zksync_types::ProtocolVersionId;

use crate::{
gas_tracker::new_block_gas_count,
state_keeper::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig},
Expand All @@ -20,6 +22,7 @@ impl SealCriterion for GasCriterion {
_tx_count: usize,
block_data: &SealData,
tx_data: &SealData,
_protocol_version_id: ProtocolVersionId,
) -> SealResolution {
let tx_bound =
(config.max_single_tx_gas as f64 * config.reject_tx_at_gas_percentage).round() as u32;
Expand Down Expand Up @@ -67,6 +70,7 @@ mod tests {
..SealData::default()
},
&SealData::default(),
ProtocolVersionId::latest(),
);
assert_eq!(empty_block_resolution, SealResolution::NoSeal);

Expand All @@ -88,6 +92,7 @@ mod tests {
gas_count: tx_gas,
..SealData::default()
},
ProtocolVersionId::latest(),
);
assert_eq!(
huge_transaction_resolution,
Expand All @@ -114,6 +119,7 @@ mod tests {
gas_count: tx_gas,
..SealData::default()
},
ProtocolVersionId::latest(),
);
assert_eq!(resolution_after_first_tx, SealResolution::NoSeal);

Expand All @@ -129,6 +135,7 @@ mod tests {
gas_count: tx_gas,
..SealData::default()
},
ProtocolVersionId::latest(),
);
assert_eq!(resolution_after_second_tx, SealResolution::ExcludeAndSeal);

Expand Down Expand Up @@ -157,6 +164,7 @@ mod tests {
gas_count: tx_gas,
..SealData::default()
},
ProtocolVersionId::latest(),
);
assert_eq!(resolution_after_first_tx, SealResolution::IncludeAndSeal);
}
Expand Down
Loading