Skip to content

Commit 634b6dd

Browse files
authored
fix(EN): properly set chain_id for old batch transactions (#4488)
## What ❔ If received batch details contain None/Null chain_id (commit_chain_id, prove_chain_id, execute_chain_id) set it to config provided chain id for L1. This happens for old batches. ## Why ❔ If you try syning EN from scratch with a "old" chain you will encounter following error: ``` Error: One or more tasks failed: ["Task transaction_finality_updater failed: No batch transactions to process for chain id 1 while there are some for None chain_id.Error is thrown so node can restart and reload SL data. If node doesn't make any progress after restart, then it's bug, please contact developers."] ``` Logic for fetching batch transactions to process their finality status assumes chain_ids are set properly and no transaction should have chain_id = None. However this is the case for old chains. When fetching they return *_chain_id: null in zks_getL1BatchDetails for some old batches. ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`.
1 parent bb42dcf commit 634b6dd

File tree

4 files changed

+54
-18
lines changed

4 files changed

+54
-18
lines changed

core/bin/external_node/src/node_builder.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,9 @@ impl ExternalNodeBuilder {
281281
}
282282

283283
fn add_batch_transaction_fetcher_layer(mut self) -> anyhow::Result<Self> {
284-
self.node.add_layer(BatchStatusUpdaterLayer);
284+
self.node.add_layer(BatchStatusUpdaterLayer::new(
285+
self.config.local.networks.l1_chain_id,
286+
));
285287
Ok(self)
286288
}
287289

core/node/node_sync/src/batch_transaction_fetcher/mod.rs

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ use zksync_dal::{Connection, ConnectionPool, Core, CoreDal};
1313
use zksync_health_check::{Health, HealthStatus, HealthUpdater, ReactiveHealthCheck};
1414
use zksync_shared_metrics::EN_METRICS;
1515
use zksync_types::{
16-
aggregated_operations::L1BatchAggregatedActionType, api, L1BatchNumber, SLChainId, H256,
16+
aggregated_operations::L1BatchAggregatedActionType, api, L1BatchNumber, L1ChainId, SLChainId,
17+
H256,
1718
};
1819
use zksync_web3_decl::{
1920
client::{DynClient, L2},
@@ -41,7 +42,7 @@ struct BatchStatusChange {
4142
number: L1BatchNumber,
4243
l1_tx_hash: H256,
4344
happened_at: DateTime<Utc>,
44-
sl_chain_id: Option<SLChainId>,
45+
sl_chain_id: SLChainId,
4546
}
4647

4748
#[derive(Debug, Default)]
@@ -103,10 +104,14 @@ struct UpdaterCursor {
103104
last_executed_l1_batch: L1BatchNumber,
104105
last_proven_l1_batch: L1BatchNumber,
105106
last_committed_l1_batch: L1BatchNumber,
107+
default_sl_chain_id: SLChainId,
106108
}
107109

108110
impl UpdaterCursor {
109-
async fn new(storage: &mut Connection<'_, Core>) -> anyhow::Result<Self> {
111+
async fn new(
112+
storage: &mut Connection<'_, Core>,
113+
default_sl_chain_id: SLChainId,
114+
) -> anyhow::Result<Self> {
110115
let first_l1_batch_number = projected_first_l1_batch(storage).await?;
111116
// Use the snapshot L1 batch, or the genesis batch if we are not using a snapshot. Technically, the snapshot L1 batch
112117
// is not necessarily proven / executed yet, but since it and earlier batches are not stored, it serves
@@ -132,6 +137,7 @@ impl UpdaterCursor {
132137
last_executed_l1_batch,
133138
last_proven_l1_batch,
134139
last_committed_l1_batch,
140+
default_sl_chain_id,
135141
})
136142
}
137143

@@ -211,7 +217,7 @@ impl UpdaterCursor {
211217
number: batch_info.number,
212218
l1_tx_hash,
213219
happened_at,
214-
sl_chain_id,
220+
sl_chain_id: sl_chain_id.unwrap_or(self.default_sl_chain_id),
215221
});
216222
tracing::info!("Batch {}: {action_str}", batch_info.number);
217223
FETCHER_METRICS.l1_batch[&stage.into()].set(batch_info.number.0.into());
@@ -230,6 +236,7 @@ impl UpdaterCursor {
230236
/// L1 transaction information in `zks_getBlockDetails` and `zks_getL1BatchDetails` RPC methods.
231237
#[derive(Debug)]
232238
pub struct BatchStatusUpdater {
239+
l1_chain_id: SLChainId,
233240
client: Box<dyn MainNodeClient>,
234241
pool: ConnectionPool<Core>,
235242
health_updater: HealthUpdater,
@@ -242,20 +249,27 @@ pub struct BatchStatusUpdater {
242249
impl BatchStatusUpdater {
243250
const DEFAULT_SLEEP_INTERVAL: Duration = Duration::from_secs(5);
244251

245-
pub fn new(client: Box<DynClient<L2>>, pool: ConnectionPool<Core>) -> Self {
252+
pub fn new(
253+
l1_chain_id: L1ChainId,
254+
client: Box<DynClient<L2>>,
255+
pool: ConnectionPool<Core>,
256+
) -> Self {
246257
Self::from_parts(
258+
l1_chain_id.into(), // needed because this is used when l1 is sl
247259
Box::new(client.for_component("batch_transaction_fetcher")),
248260
pool,
249261
Self::DEFAULT_SLEEP_INTERVAL,
250262
)
251263
}
252264

253265
fn from_parts(
266+
l1_chain_id: SLChainId,
254267
client: Box<dyn MainNodeClient>,
255268
pool: ConnectionPool<Core>,
256269
sleep_interval: Duration,
257270
) -> Self {
258271
Self {
272+
l1_chain_id,
259273
client,
260274
pool,
261275
health_updater: ReactiveHealthCheck::new("batch_transaction_fetcher").1,
@@ -271,7 +285,7 @@ impl BatchStatusUpdater {
271285

272286
pub async fn run(self, mut stop_receiver: watch::Receiver<bool>) -> anyhow::Result<()> {
273287
let mut storage = self.pool.connection_tagged("sync_layer").await?;
274-
let mut cursor = UpdaterCursor::new(&mut storage).await?;
288+
let mut cursor = UpdaterCursor::new(&mut storage, self.l1_chain_id).await?;
275289
drop(storage);
276290
tracing::info!("Initialized batch status updater cursor: {cursor:?}");
277291
self.health_updater
@@ -400,7 +414,7 @@ impl BatchStatusUpdater {
400414
change.number,
401415
L1BatchAggregatedActionType::Commit,
402416
change.l1_tx_hash,
403-
change.sl_chain_id,
417+
Some(change.sl_chain_id),
404418
)
405419
.await?;
406420
cursor.last_committed_l1_batch = change.number;
@@ -424,7 +438,7 @@ impl BatchStatusUpdater {
424438
change.number,
425439
L1BatchAggregatedActionType::PublishProofOnchain,
426440
change.l1_tx_hash,
427-
change.sl_chain_id,
441+
Some(change.sl_chain_id),
428442
)
429443
.await?;
430444
cursor.last_proven_l1_batch = change.number;
@@ -448,7 +462,7 @@ impl BatchStatusUpdater {
448462
change.number,
449463
L1BatchAggregatedActionType::Execute,
450464
change.l1_tx_hash,
451-
change.sl_chain_id,
465+
Some(change.sl_chain_id),
452466
)
453467
.await?;
454468
cursor.last_executed_l1_batch = change.number;

core/node/node_sync/src/batch_transaction_fetcher/tests.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ fn mock_change(number: L1BatchNumber) -> BatchStatusChange {
237237
number,
238238
l1_tx_hash: H256::zero(),
239239
happened_at: DateTime::default(),
240-
sl_chain_id: Some(SLChainId(0)),
240+
sl_chain_id: SLChainId(0),
241241
}
242242
}
243243

@@ -246,8 +246,12 @@ fn mock_updater(
246246
pool: ConnectionPool<Core>,
247247
) -> (BatchStatusUpdater, mpsc::UnboundedReceiver<StatusChanges>) {
248248
let (changes_sender, changes_receiver) = mpsc::unbounded_channel();
249-
let mut updater =
250-
BatchStatusUpdater::from_parts(Box::new(client), pool, Duration::from_millis(10));
249+
let mut updater = BatchStatusUpdater::from_parts(
250+
SLChainId(1),
251+
Box::new(client),
252+
pool,
253+
Duration::from_millis(10),
254+
);
251255
updater.changes_sender = changes_sender;
252256
(updater, changes_receiver)
253257
}
@@ -263,7 +267,9 @@ async fn updater_cursor_for_storage_with_genesis_block() {
263267
seal_l1_batch(&mut storage, L1BatchNumber(number)).await;
264268
}
265269

266-
let mut cursor = UpdaterCursor::new(&mut storage).await.unwrap();
270+
let mut cursor = UpdaterCursor::new(&mut storage, SLChainId(1))
271+
.await
272+
.unwrap();
267273
assert_eq!(cursor.last_committed_l1_batch, L1BatchNumber(0));
268274
assert_eq!(cursor.last_proven_l1_batch, L1BatchNumber(0));
269275
assert_eq!(cursor.last_executed_l1_batch, L1BatchNumber(0));
@@ -283,7 +289,9 @@ async fn updater_cursor_for_storage_with_genesis_block() {
283289
assert_eq!(cursor.last_proven_l1_batch, L1BatchNumber(1));
284290
assert_eq!(cursor.last_executed_l1_batch, L1BatchNumber(0));
285291

286-
let restored_cursor = UpdaterCursor::new(&mut storage).await.unwrap();
292+
let restored_cursor = UpdaterCursor::new(&mut storage, SLChainId(1))
293+
.await
294+
.unwrap();
287295
assert_eq!(restored_cursor, cursor);
288296
}
289297

@@ -293,7 +301,9 @@ async fn updater_cursor_after_snapshot_recovery() {
293301
let mut storage = pool.connection().await.unwrap();
294302
prepare_recovery_snapshot(&mut storage, L1BatchNumber(23), L2BlockNumber(42), &[]).await;
295303

296-
let cursor = UpdaterCursor::new(&mut storage).await.unwrap();
304+
let cursor = UpdaterCursor::new(&mut storage, SLChainId(1))
305+
.await
306+
.unwrap();
297307
assert_eq!(cursor.last_committed_l1_batch, L1BatchNumber(23));
298308
assert_eq!(cursor.last_proven_l1_batch, L1BatchNumber(23));
299309
assert_eq!(cursor.last_executed_l1_batch, L1BatchNumber(23));

core/node/node_sync/src/node/batch_transaction_fetcher.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use zksync_node_framework::{
88
wiring_layer::{WiringError, WiringLayer},
99
FromContext, IntoContext,
1010
};
11+
use zksync_types::L1ChainId;
1112
use zksync_web3_decl::client::{DynClient, L2};
1213

1314
use crate::batch_transaction_fetcher::BatchStatusUpdater;
@@ -28,7 +29,15 @@ pub struct Output {
2829

2930
/// Wiring layer for `BatchStatusUpdater`, part of the external node.
3031
#[derive(Debug)]
31-
pub struct BatchStatusUpdaterLayer;
32+
pub struct BatchStatusUpdaterLayer {
33+
l1_chain_id: L1ChainId,
34+
}
35+
36+
impl BatchStatusUpdaterLayer {
37+
pub fn new(l1_chain_id: L1ChainId) -> Self {
38+
Self { l1_chain_id }
39+
}
40+
}
3241

3342
#[async_trait::async_trait]
3443
impl WiringLayer for BatchStatusUpdaterLayer {
@@ -46,7 +55,8 @@ impl WiringLayer for BatchStatusUpdaterLayer {
4655
app_health,
4756
} = input;
4857

49-
let updater = BatchStatusUpdater::new(main_node_client, pool.get().await?);
58+
let updater =
59+
BatchStatusUpdater::new(self.l1_chain_id, main_node_client, pool.get().await?);
5060

5161
// Insert healthcheck
5262
app_health

0 commit comments

Comments
 (0)