Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(consensus): preventing config update reverts #3148

Merged
merged 1 commit into from
Oct 23, 2024
Merged

fix(consensus): preventing config update reverts #3148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 40 additions & 24 deletions core/lib/dal/src/consensus_dal/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion core/node/consensus/src/en.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}"
)
Expand Down
Loading