Skip to content

Commit 782c8b9

Browse files
authored
fix: only apply EN seal criteria checks on >=v29 protocol (#4491)
## What ❔ Only apply seal criteria checks through PanicSealer on >=v29 protocol ## Why ❔ Some older protocol versions had higher circuit (and maybe other) limits that are no longer avilable in code constants ## 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 e693fd0 commit 782c8b9

File tree

3 files changed

+8
-3
lines changed

3 files changed

+8
-3
lines changed

core/node/state_keeper/src/seal_criteria/conditional_sealer.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ impl ConditionalSealer for NoopSealer {
238238
}
239239

240240
/// Sealer for usage in EN. Panics if passed transaction should be excluded.
241+
/// Performs verification only from v29
241242
#[derive(Debug)]
242243
pub struct PanicSealer {
243244
sealers: Vec<Box<dyn SealCriterion>>,
@@ -293,6 +294,10 @@ impl ConditionalSealer for PanicSealer {
293294
tx_data: &SealData,
294295
protocol_version: ProtocolVersionId,
295296
) -> SealResolution {
297+
if protocol_version < ProtocolVersionId::Version29 {
298+
// we do not validate seal criteria before v29. Appropriate constants are not available.
299+
return SealResolution::NoSeal;
300+
}
296301
tracing::trace!(
297302
"Checking seal resolution for L1 batch #{l1_batch_number} with {tx_count} transactions \
298303
and metrics {:?}",
@@ -317,7 +322,7 @@ impl ConditionalSealer for PanicSealer {
317322
// no seal, don't need to do anything
318323
}
319324
SealResolution::ExcludeAndSeal => {
320-
panic!("Transaction should have been excluded, but was sequenced");
325+
panic!("Transaction from batch {}, tx_count {}, should have been excluded, but was sequenced due to sealer {}", l1_batch_number, tx_count, sealer.prom_criterion_name());
321326
}
322327
SealResolution::Unexecutable(reason) => {
323328
panic!("Unexecutable transaction was sequenced: {reason:?}");

core/node/state_keeper/src/testonly/test_batch_executor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ impl TestScenario {
288288
/// Provided `SealManager` is expected to be externally configured to adhere the written scenario logic.
289289
pub(crate) async fn run_panic(self, sealer: Arc<dyn ConditionalSealer>, message: &str) {
290290
let join_error = self.run(sealer).await.expect_err("Not paniced");
291-
let panic_message = join_error.into_panic().downcast::<&'static str>().unwrap();
291+
let panic_message = join_error.into_panic().downcast::<String>().unwrap();
292292
assert!(panic_message.contains(message), "Different panic message");
293293
}
294294
}

core/node/state_keeper/src/tests/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ async fn panic_sealer_panic_scenario() {
240240
.batch_sealed("Batch 1")
241241
.run_panic(
242242
Arc::new(sealer),
243-
"Transaction should have been excluded, but was sequenced",
243+
"should have been excluded, but was sequenced due to sealer test_slots",
244244
)
245245
.await;
246246
}

0 commit comments

Comments
 (0)