From d5f8f3892a14180f590cabab921d3a68dec903e3 Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Wed, 7 Aug 2024 11:54:18 +0200 Subject: [PATCH] fix: restrictive genesis parsing (#2605) The error about receiving an unsupported consensus genesis was unnecessarily delayed. I've disallowed unknown fields in genesis to fix that. --- core/lib/dal/src/consensus_dal.rs | 18 +++++++++++++++--- core/node/consensus/src/en.rs | 12 +++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/core/lib/dal/src/consensus_dal.rs b/core/lib/dal/src/consensus_dal.rs index 15c4c18b5d8..d8f28705421 100644 --- a/core/lib/dal/src/consensus_dal.rs +++ b/core/lib/dal/src/consensus_dal.rs @@ -7,6 +7,7 @@ use zksync_db_connection::{ error::{DalError, DalResult, SqlxContext}, instrument::{InstrumentExt, Instrumented}, }; +use zksync_protobuf::ProtoFmt as _; use zksync_types::L2BlockNumber; pub use crate::consensus::{AttestationStatus, Payload}; @@ -48,9 +49,20 @@ impl ConsensusDal<'_, '_> { let Some(genesis) = row.genesis else { return Ok(None); }; - let genesis: validator::GenesisRaw = - zksync_protobuf::serde::deserialize(genesis).decode_column("genesis")?; - Ok(Some(genesis.with_hash())) + // Deserialize the json, but don't allow for unknown fields. + // We might encounter an unknown fields here in case if support for the previous + // consensus protocol version is removed before the migration to a new version + // is performed. The node should NOT operate in such a state. + Ok(Some( + validator::GenesisRaw::read( + &zksync_protobuf::serde::deserialize_proto_with_options( + &genesis, /*deny_unknown_fields=*/ true, + ) + .decode_column("genesis")?, + ) + .decode_column("genesis")? + .with_hash(), + )) }) .instrument("genesis") .fetch_optional(self.storage) diff --git a/core/node/consensus/src/en.rs b/core/node/consensus/src/en.rs index 6fbc0049174..ce8a555e06d 100644 --- a/core/node/consensus/src/en.rs +++ b/core/node/consensus/src/en.rs @@ -11,6 +11,7 @@ use zksync_dal::consensus_dal; use zksync_node_sync::{ fetcher::FetchedBlock, sync_action::ActionQueueSender, MainNodeClient, SyncState, }; +use zksync_protobuf::ProtoFmt as _; use zksync_types::L2BlockNumber; use zksync_web3_decl::client::{DynClient, L2}; @@ -200,7 +201,16 @@ impl EN { .await? .context("fetch_consensus_genesis()")? .context("main node is not running consensus component")?; - Ok(zksync_protobuf::serde::deserialize(&genesis.0).context("deserialize(genesis)")?) + // Deserialize the json, but don't allow for unknown fields. + // We need to compute the hash of the Genesis, so simply ignoring the unknown fields won't + // do. + Ok(validator::GenesisRaw::read( + &zksync_protobuf::serde::deserialize_proto_with_options( + &genesis.0, /*deny_unknown_fields=*/ true, + ) + .context("deserialize")?, + )? + .with_hash()) } /// Fetches (with retries) the given block from the main node.