From 22e5d4de9a97ccc9bf202ddb7e0f87bdc3637a93 Mon Sep 17 00:00:00 2001
From: Grzegorz Prusak <pompon.pompon@gmail.com>
Date: Tue, 22 Oct 2024 11:12:32 +0200
Subject: [PATCH] preventing config update reverts

---
 core/lib/dal/src/consensus_dal/mod.rs | 64 +++++++++++++++++----------
 core/node/consensus/src/en.rs         |  7 ++-
 2 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/core/lib/dal/src/consensus_dal/mod.rs b/core/lib/dal/src/consensus_dal/mod.rs
index 9515e93f2b3c..4516434868c4 100644
--- a/core/lib/dal/src/consensus_dal/mod.rs
+++ b/core/lib/dal/src/consensus_dal/mod.rs
@@ -16,10 +16,48 @@ use crate::{Core, CoreDal};
 #[cfg(test)]
 mod tests;
 
+/// Hash of the batch.
 pub fn batch_hash(info: &StoredBatchInfo) -> attester::BatchHash {
     attester::BatchHash(Keccak256::from_bytes(info.hash().0))
 }
 
+/// Verifies that the transition from `old` to `new` is admissible.
+pub fn verify_config_transition(old: &GlobalConfig, new: &GlobalConfig) -> anyhow::Result<()> {
+    anyhow::ensure!(
+        old.genesis.chain_id == new.genesis.chain_id,
+        "changing chain_id is not allowed: old = {:?}, new = {:?}",
+        old.genesis.chain_id,
+        new.genesis.chain_id,
+    );
+    // Note that it may happen that the fork number didn't change,
+    // in case the binary was updated to support more fields in genesis struct.
+    // In such a case, the old binary was not able to connect to the consensus network,
+    // because of the genesis hash mismatch.
+    // TODO: Perhaps it would be better to deny unknown fields in the genesis instead.
+    // It would require embedding the genesis either as a json string or protobuf bytes within
+    // the global config, so that the global config can be parsed with
+    // `deny_unknown_fields:false` while genesis would be parsed with
+    // `deny_unknown_fields:true`.
+    anyhow::ensure!(
+        old.genesis.fork_number <= new.genesis.fork_number,
+        "transition to a past fork is not allowed: old = {:?}, new = {:?}",
+        old.genesis.fork_number,
+        new.genesis.fork_number,
+    );
+    new.genesis.verify().context("genesis.verify()")?;
+    // This is a temporary hack until the `consensus_genesis()` RPC is disabled.
+    if new
+        == (&GlobalConfig {
+            genesis: old.genesis.clone(),
+            registry_address: None,
+            seed_peers: [].into(),
+        })
+    {
+        anyhow::bail!("new config is equal to truncated old config, which means that it was sourced from the wrong endpoint");
+    }
+    Ok(())
+}
+
 /// Storage access methods for `zksync_core::consensus` module.
 #[derive(Debug)]
 pub struct ConsensusDal<'a, 'c> {
@@ -94,6 +132,8 @@ impl ConsensusDal<'_, '_> {
             if got == want {
                 return Ok(());
             }
+            verify_config_transition(got, want)?;
+
             // If genesis didn't change, just update the config.
             if got.genesis == want.genesis {
                 let s = zksync_protobuf::serde::Serialize;
@@ -112,30 +152,6 @@ impl ConsensusDal<'_, '_> {
                 txn.commit().await?;
                 return Ok(());
             }
-
-            // Verify the genesis change.
-            anyhow::ensure!(
-                got.genesis.chain_id == want.genesis.chain_id,
-                "changing chain_id is not allowed: old = {:?}, new = {:?}",
-                got.genesis.chain_id,
-                want.genesis.chain_id,
-            );
-            // Note that it may happen that the fork number didn't change,
-            // in case the binary was updated to support more fields in genesis struct.
-            // In such a case, the old binary was not able to connect to the consensus network,
-            // because of the genesis hash mismatch.
-            // TODO: Perhaps it would be better to deny unknown fields in the genesis instead.
-            // It would require embedding the genesis either as a json string or protobuf bytes within
-            // the global config, so that the global config can be parsed with
-            // `deny_unknown_fields:false` while genesis would be parsed with
-            // `deny_unknown_fields:true`.
-            anyhow::ensure!(
-                got.genesis.fork_number <= want.genesis.fork_number,
-                "transition to a past fork is not allowed: old = {:?}, new = {:?}",
-                got.genesis.fork_number,
-                want.genesis.fork_number,
-            );
-            want.genesis.verify().context("genesis.verify()")?;
         }
 
         // Reset the consensus state.
diff --git a/core/node/consensus/src/en.rs b/core/node/consensus/src/en.rs
index 518a7ebb29aa..8158cc5aeb26 100644
--- a/core/node/consensus/src/en.rs
+++ b/core/node/consensus/src/en.rs
@@ -100,7 +100,12 @@ impl EN {
                     let old = old;
                     loop {
                         if let Ok(new) = self.fetch_global_config(ctx).await {
-                            if new != old {
+                            // We verify the transition here to work around the situation
+                            // where `consenus_global_config()` RPC fails randomly and fallback
+                            // to `consensus_genesis()` RPC activates.
+                            if new != old
+                                && consensus_dal::verify_config_transition(&old, &new).is_ok()
+                            {
                                 return Err(anyhow::format_err!(
                                     "global config changed: old {old:?}, new {new:?}"
                                 )