Skip to content

Commit

Permalink
fix(en): Reduce amount of data in snapshot header (#1528)
Browse files Browse the repository at this point in the history
## What ❔

Reduces the amount of data in `SnapshotHeader` returned by the relevant
RPC method. This missing data is fetched from the main node.

## Why ❔

Right now, the snapshot header returned by JSON-RPC API includes the
entire L1BatchWithMetadata. Most of this information is not used, and
the data itself may be not present in DB! Indeed, new snapshots are
created for the penultimate L1 batch. It is guaranteed to have metadata
calculator run on it (i.e., have a state hash computed), but not the
commitment generator. The corresponding errors have been observed in CI
runs, and in local testing.

## 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`.
- [x] Spellcheck has been run via `zk spellcheck`.
- [x] Linkcheck has been run via `zk linkcheck`.
  • Loading branch information
slowli authored Apr 2, 2024
1 parent 3c28c25 commit afa1cf1
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 100 deletions.
14 changes: 13 additions & 1 deletion core/bin/snapshots_creator/src/creator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl SnapshotCreator {
latest_snapshot: Option<&SnapshotMetadata>,
conn: &mut Connection<'_, Core>,
) -> anyhow::Result<Option<SnapshotProgress>> {
// We subtract 1 so that after restore, EN node has at least one L1 batch to fetch
// We subtract 1 so that after restore, EN node has at least one L1 batch to fetch.
let sealed_l1_batch_number = conn.blocks_dal().get_sealed_l1_batch_number().await?;
let sealed_l1_batch_number = sealed_l1_batch_number.context("No L1 batches in Postgres")?;
anyhow::ensure!(
Expand All @@ -203,6 +203,18 @@ impl SnapshotCreator {
);
let l1_batch_number = sealed_l1_batch_number - 1;

// Sanity check: the selected L1 batch should have Merkle tree data; otherwise, it could be impossible
// to recover from the generated snapshot.
conn.blocks_dal()
.get_l1_batch_tree_data(l1_batch_number)
.await?
.with_context(|| {
format!(
"Snapshot L1 batch #{l1_batch_number} doesn't have tree data, meaning recovery from the snapshot \
could be impossible. This should never happen"
)
})?;

let latest_snapshot_l1_batch_number =
latest_snapshot.map(|snapshot| snapshot.l1_batch_number);
if latest_snapshot_l1_batch_number == Some(l1_batch_number) {
Expand Down
12 changes: 11 additions & 1 deletion core/bin/snapshots_creator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rand::{thread_rng, Rng};
use zksync_dal::{Connection, CoreDal};
use zksync_object_store::ObjectStore;
use zksync_types::{
block::{L1BatchHeader, MiniblockHeader},
block::{L1BatchHeader, L1BatchTreeData, MiniblockHeader},
snapshots::{
SnapshotFactoryDependencies, SnapshotFactoryDependency, SnapshotStorageLog,
SnapshotStorageLogsChunk, SnapshotStorageLogsStorageKey,
Expand Down Expand Up @@ -182,6 +182,16 @@ async fn create_l1_batch(
.insert_initial_writes(l1_batch_number, &written_keys)
.await
.unwrap();
conn.blocks_dal()
.save_l1_batch_tree_data(
l1_batch_number,
&L1BatchTreeData {
hash: H256::zero(),
rollup_last_leaf_index: 1,
},
)
.await
.unwrap();
}

async fn prepare_postgres(
Expand Down
65 changes: 47 additions & 18 deletions core/lib/snapshots_applier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use zksync_dal::{Connection, ConnectionPool, Core, CoreDal, SqlxError};
use zksync_health_check::{Health, HealthStatus, HealthUpdater, ReactiveHealthCheck};
use zksync_object_store::{ObjectStore, ObjectStoreError};
use zksync_types::{
api::en::SyncBlock,
api,
snapshots::{
SnapshotFactoryDependencies, SnapshotHeader, SnapshotRecoveryStatus, SnapshotStorageLog,
SnapshotStorageLogsChunk, SnapshotStorageLogsStorageKey, SnapshotVersion,
Expand All @@ -23,7 +23,7 @@ use zksync_utils::bytecode::hash_bytecode;
use zksync_web3_decl::{
error::{ClientRpcContext, EnrichedClientError, EnrichedClientResult},
jsonrpsee::{core::client, http_client::HttpClient},
namespaces::{EnNamespaceClient, SnapshotsNamespaceClient},
namespaces::{EnNamespaceClient, SnapshotsNamespaceClient, ZksNamespaceClient},
};

use self::metrics::{InitialStage, StorageLogsChunksStage, METRICS};
Expand Down Expand Up @@ -91,10 +91,15 @@ impl From<EnrichedClientError> for SnapshotsApplierError {
/// Main node API used by the [`SnapshotsApplier`].
#[async_trait]
pub trait SnapshotsApplierMainNodeClient: fmt::Debug + Send + Sync {
async fn fetch_l2_block(
async fn fetch_l1_batch_details(
&self,
number: L1BatchNumber,
) -> EnrichedClientResult<Option<api::L1BatchDetails>>;

async fn fetch_l2_block_details(
&self,
number: MiniblockNumber,
) -> EnrichedClientResult<Option<SyncBlock>>;
) -> EnrichedClientResult<Option<api::BlockDetails>>;

async fn fetch_newest_snapshot(&self) -> EnrichedClientResult<Option<SnapshotHeader>>;

Expand All @@ -106,12 +111,22 @@ pub trait SnapshotsApplierMainNodeClient: fmt::Debug + Send + Sync {

#[async_trait]
impl SnapshotsApplierMainNodeClient for HttpClient {
async fn fetch_l2_block(
async fn fetch_l1_batch_details(
&self,
number: L1BatchNumber,
) -> EnrichedClientResult<Option<api::L1BatchDetails>> {
self.get_l1_batch_details(number)
.rpc_context("get_l1_batch_details")
.with_arg("number", &number)
.await
}

async fn fetch_l2_block_details(
&self,
number: MiniblockNumber,
) -> EnrichedClientResult<Option<SyncBlock>> {
self.sync_l2_block(number, false)
.rpc_context("sync_l2_block")
) -> EnrichedClientResult<Option<api::BlockDetails>> {
self.get_block_details(number)
.rpc_context("get_block_details")
.with_arg("number", &number)
.await
}
Expand Down Expand Up @@ -387,26 +402,40 @@ impl<'a> SnapshotsApplier<'a> {
);
Self::check_snapshot_version(snapshot.version)?;

let l1_batch = main_node_client
.fetch_l1_batch_details(l1_batch_number)
.await?
.with_context(|| format!("L1 batch #{l1_batch_number} is missing on main node"))?;
let l1_batch_root_hash = l1_batch
.base
.root_hash
.context("snapshot L1 batch fetched from main node doesn't have root hash set")?;
let miniblock = main_node_client
.fetch_l2_block(miniblock_number)
.fetch_l2_block_details(miniblock_number)
.await?
.with_context(|| format!("miniblock #{miniblock_number} is missing on main node"))?;
let miniblock_hash = miniblock
.hash
.base
.root_hash
.context("snapshot miniblock fetched from main node doesn't have hash set")?;
let protocol_version = miniblock.protocol_version.context(
"snapshot miniblock fetched from main node doesn't have protocol version set",
)?;
if miniblock.l1_batch_number != l1_batch_number {
let err = anyhow::anyhow!(
"snapshot miniblock returned by main node doesn't belong to expected L1 batch #{l1_batch_number}: {miniblock:?}"
);
return Err(err.into());
}

Ok(SnapshotRecoveryStatus {
l1_batch_number,
l1_batch_timestamp: snapshot.last_l1_batch_with_metadata.header.timestamp,
l1_batch_root_hash: snapshot.last_l1_batch_with_metadata.metadata.root_hash,
l1_batch_timestamp: l1_batch.base.timestamp,
l1_batch_root_hash,
miniblock_number: snapshot.miniblock_number,
miniblock_timestamp: miniblock.timestamp,
miniblock_timestamp: miniblock.base.timestamp,
miniblock_hash,
protocol_version: snapshot
.last_l1_batch_with_metadata
.header
.protocol_version
.unwrap(),
protocol_version,
storage_logs_chunks_processed: vec![false; snapshot.storage_logs_chunks.len()],
})
}
Expand Down
98 changes: 45 additions & 53 deletions core/lib/snapshots_applier/src/tests/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use std::{collections::HashMap, fmt, sync::Arc};
use async_trait::async_trait;
use zksync_object_store::{Bucket, ObjectStore, ObjectStoreError, ObjectStoreFactory};
use zksync_types::{
api::en::SyncBlock,
block::L1BatchHeader,
commitment::{L1BatchMetaParameters, L1BatchMetadata, L1BatchWithMetadata},
api,
snapshots::{
SnapshotFactoryDependencies, SnapshotFactoryDependency, SnapshotHeader,
SnapshotRecoveryStatus, SnapshotStorageLog, SnapshotStorageLogsChunk,
Expand All @@ -23,17 +21,25 @@ use crate::SnapshotsApplierMainNodeClient;

#[derive(Debug, Default)]
pub(super) struct MockMainNodeClient {
pub fetch_l2_block_responses: HashMap<MiniblockNumber, SyncBlock>,
pub fetch_l1_batch_responses: HashMap<L1BatchNumber, api::L1BatchDetails>,
pub fetch_l2_block_responses: HashMap<MiniblockNumber, api::BlockDetails>,
pub fetch_newest_snapshot_response: Option<SnapshotHeader>,
pub tokens_response: Vec<TokenInfo>,
}

#[async_trait]
impl SnapshotsApplierMainNodeClient for MockMainNodeClient {
async fn fetch_l2_block(
async fn fetch_l1_batch_details(
&self,
number: L1BatchNumber,
) -> EnrichedClientResult<Option<api::L1BatchDetails>> {
Ok(self.fetch_l1_batch_responses.get(&number).cloned())
}

async fn fetch_l2_block_details(
&self,
number: MiniblockNumber,
) -> EnrichedClientResult<Option<SyncBlock>> {
) -> EnrichedClientResult<Option<api::BlockDetails>> {
Ok(self.fetch_l2_block_responses.get(&number).cloned())
}

Expand Down Expand Up @@ -99,57 +105,43 @@ impl ObjectStore for ObjectStoreWithErrors {
}
}

fn miniblock_metadata(
fn block_details_base(hash: H256) -> api::BlockDetailsBase {
api::BlockDetailsBase {
timestamp: 0,
l1_tx_count: 0,
l2_tx_count: 0,
root_hash: Some(hash),
status: api::BlockStatus::Sealed,
commit_tx_hash: None,
committed_at: None,
prove_tx_hash: None,
proven_at: None,
execute_tx_hash: None,
executed_at: None,
l1_gas_price: 0,
l2_fair_gas_price: 0,
base_system_contracts_hashes: Default::default(),
}
}

fn miniblock_details(
number: MiniblockNumber,
l1_batch_number: L1BatchNumber,
hash: H256,
) -> SyncBlock {
SyncBlock {
) -> api::BlockDetails {
api::BlockDetails {
number,
l1_batch_number,
last_in_batch: true,
timestamp: 0,
l1_gas_price: 0,
l2_fair_gas_price: 0,
fair_pubdata_price: None,
base_system_contracts_hashes: Default::default(),
base: block_details_base(hash),
operator_address: Default::default(),
transactions: None,
virtual_blocks: None,
hash: Some(hash),
protocol_version: Default::default(),
protocol_version: Some(ProtocolVersionId::latest()),
}
}

fn l1_block_metadata(l1_batch_number: L1BatchNumber, root_hash: H256) -> L1BatchWithMetadata {
L1BatchWithMetadata {
header: L1BatchHeader::new(
l1_batch_number,
0,
Default::default(),
ProtocolVersionId::default(),
),
metadata: L1BatchMetadata {
root_hash,
rollup_last_leaf_index: 0,
merkle_root_hash: H256::zero(),
initial_writes_compressed: Some(vec![]),
repeated_writes_compressed: Some(vec![]),
commitment: H256::zero(),
l2_l1_merkle_root: H256::zero(),
block_meta_params: L1BatchMetaParameters {
zkporter_is_available: false,
bootloader_code_hash: H256::zero(),
default_aa_code_hash: H256::zero(),
},
aux_data_hash: H256::zero(),
meta_parameters_hash: H256::zero(),
pass_through_data_hash: H256::zero(),
events_queue_commitment: None,
bootloader_initial_content_commitment: None,
state_diffs_compressed: vec![],
},
raw_published_factory_deps: vec![],
fn l1_batch_details(number: L1BatchNumber, root_hash: H256) -> api::L1BatchDetails {
api::L1BatchDetails {
number,
base: block_details_base(root_hash),
}
}

Expand Down Expand Up @@ -211,10 +203,6 @@ pub(super) fn mock_snapshot_header(status: &SnapshotRecoveryStatus) -> SnapshotH
version: SnapshotVersion::Version0.into(),
l1_batch_number: status.l1_batch_number,
miniblock_number: status.miniblock_number,
last_l1_batch_with_metadata: l1_block_metadata(
status.l1_batch_number,
status.l1_batch_root_hash,
),
storage_logs_chunks: vec![
SnapshotStorageLogsChunkMetadata {
chunk_id: 0,
Expand Down Expand Up @@ -267,9 +255,13 @@ pub(super) async fn prepare_clients(
}

client.fetch_newest_snapshot_response = Some(mock_snapshot_header(status));
client.fetch_l1_batch_responses.insert(
status.l1_batch_number,
l1_batch_details(status.l1_batch_number, status.l1_batch_root_hash),
);
client.fetch_l2_block_responses.insert(
status.miniblock_number,
miniblock_metadata(
miniblock_details(
status.miniblock_number,
status.l1_batch_number,
status.miniblock_hash,
Expand Down
1 change: 1 addition & 0 deletions core/lib/types/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ pub struct BlockDetailsBase {
pub timestamp: u64,
pub l1_tx_count: usize,
pub l2_tx_count: usize,
/// Hash for a miniblock, or the root hash (aka state hash) for an L1 batch.
pub root_hash: Option<H256>,
pub status: BlockStatus,
pub commit_tx_hash: Option<H256>,
Expand Down
5 changes: 1 addition & 4 deletions core/lib/types/src/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use zksync_basic_types::{AccountTreeId, L1BatchNumber, MiniblockNumber, H256};
use zksync_protobuf::{required, ProtoFmt};
use zksync_utils::u256_to_h256;

use crate::{
commitment::L1BatchWithMetadata, Bytes, ProtocolVersionId, StorageKey, StorageValue, U256,
};
use crate::{Bytes, ProtocolVersionId, StorageKey, StorageValue, U256};

/// Information about all snapshots persisted by the node.
#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -62,7 +60,6 @@ pub struct SnapshotHeader {
/// Ordered by chunk IDs.
pub storage_logs_chunks: Vec<SnapshotStorageLogsChunkMetadata>,
pub factory_deps_filepath: String,
pub last_l1_batch_with_metadata: L1BatchWithMetadata,
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,6 @@ impl SnapshotsNamespace {
})
})
.collect();
let l1_batch_with_metadata = storage_processor
.blocks_dal()
.get_l1_batch_metadata(l1_batch_number)
.await
.context("get_l1_batch_metadata")?
.with_context(|| format!("missing metadata for L1 batch #{l1_batch_number}"))?;
let (_, miniblock_number) = storage_processor
.blocks_dal()
.get_miniblock_range_of_l1_batch(l1_batch_number)
Expand All @@ -80,7 +74,6 @@ impl SnapshotsNamespace {
version: snapshot_metadata.version.into(),
l1_batch_number: snapshot_metadata.l1_batch_number,
miniblock_number,
last_l1_batch_with_metadata: l1_batch_with_metadata,
storage_logs_chunks: chunks,
factory_deps_filepath: snapshot_metadata.factory_deps_filepath,
}))
Expand Down
16 changes: 6 additions & 10 deletions core/tests/snapshot-recovery-test/tests/snapshot-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,14 @@ describe('snapshot recovery', () => {

const homeDir = process.env.ZKSYNC_HOME!!;

let zkscynEnv: string;
if (process.env.DEPLOYMENT_MODE == 'Validium') {
zkscynEnv = process.env.IN_DOCKER ? 'ext-node-validium-docker' : 'ext-node-validium';
} else if (process.env.DEPLOYMENT_MODE == 'Rollup') {
zkscynEnv = process.env.IN_DOCKER ? 'ext-node-docker' : 'ext-node';
} else {
throw new Error(`Unknown deployment mode: ${process.env.DEPLOYMENT_MODE}`);
}

const externalNodeEnvProfile =
'ext-node' +
(process.env.DEPLOYMENT_MODE === 'Validium' ? '-validium' : '') +
(process.env.IN_DOCKER ? '-docker' : '');
console.log('Using external node env profile', externalNodeEnvProfile);
const externalNodeEnv = {
...process.env,
ZKSYNC_ENV: zkscynEnv
ZKSYNC_ENV: externalNodeEnvProfile
};

let snapshotMetadata: GetSnapshotResponse;
Expand Down
Loading

0 comments on commit afa1cf1

Please sign in to comment.