From e9d1d905f1ce86f9de2cf39d79be4b5aada4a81d Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Fri, 20 Sep 2024 03:28:24 +0200 Subject: [PATCH] feat: added seed_peers to consensus global config (#2920) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It will allow us to announce the recommended default list of peers to all ENs without manual intervention. Fixes BFT-509. --------- Co-authored-by: Bruno França --- core/lib/config/src/configs/consensus.rs | 2 + core/lib/config/src/testonly.rs | 8 +++- core/lib/dal/Cargo.toml | 1 + core/lib/dal/src/consensus/mod.rs | 34 +++++++++++++- core/lib/dal/src/consensus/proto/mod.proto | 7 +++ core/lib/dal/src/consensus_dal.rs | 4 ++ core/lib/protobuf_config/src/consensus.rs | 44 ++++++++++++++----- .../src/proto/core/consensus.proto | 5 ++- core/node/consensus/src/config.rs | 28 +++++++++++- core/node/consensus/src/en.rs | 3 +- core/node/consensus/src/mn.rs | 4 +- core/node/consensus/src/storage/connection.rs | 1 + core/node/consensus/src/testonly.rs | 18 +++++--- core/node/consensus/src/tests/attestation.rs | 1 + core/node/consensus/src/tests/mod.rs | 1 + prover/Cargo.lock | 1 + .../zk_inception/src/utils/consensus.rs | 1 + 17 files changed, 136 insertions(+), 27 deletions(-) diff --git a/core/lib/config/src/configs/consensus.rs b/core/lib/config/src/configs/consensus.rs index 759e1312833..2e277341b07 100644 --- a/core/lib/config/src/configs/consensus.rs +++ b/core/lib/config/src/configs/consensus.rs @@ -92,6 +92,8 @@ pub struct GenesisSpec { pub leader: ValidatorPublicKey, /// Address of the registry contract. pub registry_address: Option, + /// Recommended list of peers to connect to. + pub seed_peers: BTreeMap, } #[derive(Clone, Debug, PartialEq, Default)] diff --git a/core/lib/config/src/testonly.rs b/core/lib/config/src/testonly.rs index 21141ddefff..5a5a5430442 100644 --- a/core/lib/config/src/testonly.rs +++ b/core/lib/config/src/testonly.rs @@ -774,7 +774,9 @@ impl Distribution for EncodeDist { impl Distribution for EncodeDist { fn sample(&self, rng: &mut R) -> configs::consensus::GenesisSpec { - use configs::consensus::{GenesisSpec, ProtocolVersion, ValidatorPublicKey}; + use configs::consensus::{ + GenesisSpec, Host, NodePublicKey, ProtocolVersion, ValidatorPublicKey, + }; GenesisSpec { chain_id: L2ChainId::default(), protocol_version: ProtocolVersion(self.sample(rng)), @@ -782,6 +784,10 @@ impl Distribution for EncodeDist { attesters: self.sample_collect(rng), leader: ValidatorPublicKey(self.sample(rng)), registry_address: self.sample_opt(|| rng.gen()), + seed_peers: self + .sample_range(rng) + .map(|_| (NodePublicKey(self.sample(rng)), Host(self.sample(rng)))) + .collect(), } } } diff --git a/core/lib/dal/Cargo.toml b/core/lib/dal/Cargo.toml index 9c13eeb3014..ccca49525e4 100644 --- a/core/lib/dal/Cargo.toml +++ b/core/lib/dal/Cargo.toml @@ -19,6 +19,7 @@ zksync_utils.workspace = true zksync_system_constants.workspace = true zksync_contracts.workspace = true zksync_types.workspace = true +zksync_concurrency.workspace = true zksync_consensus_roles.workspace = true zksync_consensus_storage.workspace = true zksync_protobuf.workspace = true diff --git a/core/lib/dal/src/consensus/mod.rs b/core/lib/dal/src/consensus/mod.rs index f54938e8ec1..f01655d56a9 100644 --- a/core/lib/dal/src/consensus/mod.rs +++ b/core/lib/dal/src/consensus/mod.rs @@ -5,8 +5,11 @@ mod testonly; #[cfg(test)] mod tests; +use std::collections::BTreeMap; + use anyhow::{anyhow, Context as _}; -use zksync_consensus_roles::{attester, validator}; +use zksync_concurrency::net; +use zksync_consensus_roles::{attester, node, validator}; use zksync_protobuf::{read_required, required, ProtoFmt, ProtoRepr}; use zksync_types::{ abi, ethabi, @@ -27,6 +30,23 @@ use crate::models::{parse_h160, parse_h256}; pub struct GlobalConfig { pub genesis: validator::Genesis, pub registry_address: Option, + pub seed_peers: BTreeMap, +} + +impl ProtoRepr for proto::NodeAddr { + type Type = (node::PublicKey, net::Host); + fn read(&self) -> anyhow::Result { + Ok(( + read_required(&self.key).context("key")?, + net::Host(required(&self.addr).context("addr")?.clone()), + )) + } + fn build(this: &Self::Type) -> Self { + Self { + key: Some(this.0.build()), + addr: Some(this.1 .0.clone()), + } + } } impl ProtoFmt for GlobalConfig { @@ -41,6 +61,13 @@ impl ProtoFmt for GlobalConfig { .map(|a| parse_h160(a)) .transpose() .context("registry_address")?, + seed_peers: r + .seed_peers + .iter() + .enumerate() + .map(|(i, e)| e.read().context(i)) + .collect::>() + .context("seed_peers")?, }) } @@ -48,6 +75,11 @@ impl ProtoFmt for GlobalConfig { Self::Proto { genesis: Some(self.genesis.build()), registry_address: self.registry_address.map(|a| a.as_bytes().to_vec()), + seed_peers: self + .seed_peers + .iter() + .map(|(k, v)| ProtoRepr::build(&(k.clone(), v.clone()))) + .collect(), } } } diff --git a/core/lib/dal/src/consensus/proto/mod.proto b/core/lib/dal/src/consensus/proto/mod.proto index 3ea49e9c0cd..ab1245f3ef6 100644 --- a/core/lib/dal/src/consensus/proto/mod.proto +++ b/core/lib/dal/src/consensus/proto/mod.proto @@ -4,6 +4,7 @@ package zksync.dal; import "zksync/roles/validator.proto"; import "zksync/roles/attester.proto"; +import "zksync/roles/node.proto"; message Payload { // zksync-era ProtocolVersionId @@ -122,9 +123,15 @@ message AttesterCommittee { repeated roles.attester.WeightedAttester members = 1; // required } +message NodeAddr { + optional roles.node.PublicKey key = 1; // required + optional string addr = 2; // required; Host +} + message GlobalConfig { optional roles.validator.Genesis genesis = 1; // required optional bytes registry_address = 2; // optional; H160 + repeated NodeAddr seed_peers = 3; } message AttestationStatus { diff --git a/core/lib/dal/src/consensus_dal.rs b/core/lib/dal/src/consensus_dal.rs index 2dca58e2a6a..711ce3ddf39 100644 --- a/core/lib/dal/src/consensus_dal.rs +++ b/core/lib/dal/src/consensus_dal.rs @@ -66,6 +66,7 @@ impl ConsensusDal<'_, '_> { return Ok(Some(GlobalConfig { genesis, registry_address: None, + seed_peers: [].into(), })); } Ok(None) @@ -184,6 +185,7 @@ impl ConsensusDal<'_, '_> { } .with_hash(), registry_address: old.registry_address, + seed_peers: old.seed_peers, }; txn.consensus_dal().try_update_global_config(&new).await?; txn.commit().await?; @@ -681,6 +683,7 @@ mod tests { let cfg = GlobalConfig { genesis: genesis.with_hash(), registry_address: Some(rng.gen()), + seed_peers: [].into(), // TODO: rng.gen() for Host }; conn.consensus_dal() .try_update_global_config(&cfg) @@ -715,6 +718,7 @@ mod tests { let cfg = GlobalConfig { genesis: setup.genesis.clone(), registry_address: Some(rng.gen()), + seed_peers: [].into(), }; conn.consensus_dal() .try_update_global_config(&cfg) diff --git a/core/lib/protobuf_config/src/consensus.rs b/core/lib/protobuf_config/src/consensus.rs index f5eb5c5b2f1..a5b552dffc4 100644 --- a/core/lib/protobuf_config/src/consensus.rs +++ b/core/lib/protobuf_config/src/consensus.rs @@ -71,6 +71,13 @@ impl ProtoRepr for proto::GenesisSpec { .map(|x| parse_h160(x)) .transpose() .context("registry_address")?, + seed_peers: self + .seed_peers + .iter() + .enumerate() + .map(|(i, e)| e.read().context(i)) + .collect::>() + .context("seed_peers")?, }) } fn build(this: &Self::Type) -> Self { @@ -81,6 +88,11 @@ impl ProtoRepr for proto::GenesisSpec { attesters: this.attesters.iter().map(ProtoRepr::build).collect(), leader: Some(this.leader.0.clone()), registry_address: this.registry_address.map(|a| format!("{:?}", a)), + seed_peers: this + .seed_peers + .iter() + .map(|(k, v)| proto::NodeAddr::build(&(k.clone(), v.clone()))) + .collect(), } } } @@ -99,15 +111,25 @@ impl ProtoRepr for proto::RpcConfig { } } +impl ProtoRepr for proto::NodeAddr { + type Type = (NodePublicKey, Host); + fn read(&self) -> anyhow::Result { + Ok(( + NodePublicKey(required(&self.key).context("key")?.clone()), + Host(required(&self.addr).context("addr")?.clone()), + )) + } + fn build(this: &Self::Type) -> Self { + Self { + key: Some(this.0 .0.clone()), + addr: Some(this.1 .0.clone()), + } + } +} + impl ProtoRepr for proto::Config { type Type = ConsensusConfig; fn read(&self) -> anyhow::Result { - let read_addr = |e: &proto::NodeAddr| { - let key = NodePublicKey(required(&e.key).context("key")?.clone()); - let addr = Host(required(&e.addr).context("addr")?.clone()); - anyhow::Ok((key, addr)) - }; - let max_payload_size = required(&self.max_payload_size) .and_then(|x| Ok((*x).try_into()?)) .context("max_payload_size")?; @@ -144,8 +166,9 @@ impl ProtoRepr for proto::Config { .gossip_static_outbound .iter() .enumerate() - .map(|(i, e)| read_addr(e).context(i)) - .collect::>()?, + .map(|(i, e)| e.read().context(i)) + .collect::>() + .context("gossip_static_outbound")?, genesis_spec: read_optional_repr(&self.genesis_spec), rpc: read_optional_repr(&self.rpc_config), }) @@ -168,10 +191,7 @@ impl ProtoRepr for proto::Config { gossip_static_outbound: this .gossip_static_outbound .iter() - .map(|x| proto::NodeAddr { - key: Some(x.0 .0.clone()), - addr: Some(x.1 .0.clone()), - }) + .map(|(k, v)| proto::NodeAddr::build(&(k.clone(), v.clone()))) .collect(), genesis_spec: this.genesis_spec.as_ref().map(ProtoRepr::build), rpc_config: this.rpc.as_ref().map(ProtoRepr::build), diff --git a/core/lib/protobuf_config/src/proto/core/consensus.proto b/core/lib/protobuf_config/src/proto/core/consensus.proto index 835ead1ab65..6cabc45fc12 100644 --- a/core/lib/protobuf_config/src/proto/core/consensus.proto +++ b/core/lib/protobuf_config/src/proto/core/consensus.proto @@ -31,10 +31,10 @@ package zksync.core.consensus; import "zksync/std.proto"; -// (public key, ip address) of a gossip network node. +// (public key, host address) of a gossip network node. message NodeAddr { optional string key = 1; // required; NodePublicKey - optional string addr = 2; // required; IpAddr + optional string addr = 2; // required; Host } // Weighted member of a validator committee. @@ -58,6 +58,7 @@ message GenesisSpec { repeated WeightedAttester attesters = 5; // can be empty; attester committee. // Currently not in consensus genesis, but still a part of the global configuration. optional string registry_address = 6; // optional; H160 + repeated NodeAddr seed_peers = 7; } // Per peer connection RPC rate limits. diff --git a/core/node/consensus/src/config.rs b/core/node/consensus/src/config.rs index 22f8fc01192..cada58b0756 100644 --- a/core/node/consensus/src/config.rs +++ b/core/node/consensus/src/config.rs @@ -1,5 +1,5 @@ //! Configuration utilities for the consensus component. -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use anyhow::Context as _; use secrecy::{ExposeSecret as _, Secret}; @@ -44,6 +44,7 @@ pub(super) struct GenesisSpec { pub(super) attesters: Option, pub(super) leader_selection: validator::LeaderSelectionMode, pub(super) registry_address: Option, + pub(super) seed_peers: BTreeMap, } impl GenesisSpec { @@ -55,6 +56,7 @@ impl GenesisSpec { attesters: cfg.genesis.attesters.clone(), leader_selection: cfg.genesis.leader_selection.clone(), registry_address: cfg.registry_address, + seed_peers: cfg.seed_peers.clone(), } } @@ -98,6 +100,19 @@ impl GenesisSpec { Some(attester::Committee::new(attesters).context("attesters")?) }, registry_address: x.registry_address, + seed_peers: x + .seed_peers + .iter() + .map(|(key, addr)| { + anyhow::Ok(( + Text::new(&key.0) + .decode::() + .context("key")?, + net::Host(addr.0.clone()), + )) + }) + .collect::>() + .context("seed_peers")?, }) } } @@ -109,9 +124,18 @@ pub(super) fn node_key(secrets: &ConsensusSecrets) -> anyhow::Result, ) -> anyhow::Result { - let mut gossip_static_outbound = HashMap::new(); + // Always connect to seed peers. + // Once we implement dynamic peer discovery, + // we won't establish a persistent connection to seed peers + // but rather just ask them for more peers. + let mut gossip_static_outbound: HashMap<_, _> = global_config + .seed_peers + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); { let mut append = |key: &NodePublicKey, addr: &Host| { gossip_static_outbound.insert( diff --git a/core/node/consensus/src/en.rs b/core/node/consensus/src/en.rs index a52393c0f48..0b78662f8c2 100644 --- a/core/node/consensus/src/en.rs +++ b/core/node/consensus/src/en.rs @@ -125,7 +125,7 @@ impl EN { )); let executor = executor::Executor { - config: config::executor(&cfg, &secrets, build_version)?, + config: config::executor(&cfg, &secrets, &global_config, build_version)?, block_store, batch_store, validator: config::validator_key(&secrets) @@ -304,6 +304,7 @@ impl EN { Ok(consensus_dal::GlobalConfig { genesis: zksync_protobuf::serde::deserialize(&genesis.0).context("deserialize()")?, registry_address: None, + seed_peers: [].into(), }) } diff --git a/core/node/consensus/src/mn.rs b/core/node/consensus/src/mn.rs index 4d428346ebe..f80bfe58954 100644 --- a/core/node/consensus/src/mn.rs +++ b/core/node/consensus/src/mn.rs @@ -76,12 +76,12 @@ pub async fn run_main_node( s.spawn_bg(run_attestation_controller( ctx, &pool, - global_config, + global_config.clone(), attestation.clone(), )); let executor = executor::Executor { - config: config::executor(&cfg, &secrets, None)?, + config: config::executor(&cfg, &secrets, &global_config, None)?, block_store, batch_store, validator: Some(executor::Validator { diff --git a/core/node/consensus/src/storage/connection.rs b/core/node/consensus/src/storage/connection.rs index 512b37e81a1..2c297eed727 100644 --- a/core/node/consensus/src/storage/connection.rs +++ b/core/node/consensus/src/storage/connection.rs @@ -317,6 +317,7 @@ impl<'a> Connection<'a> { } .with_hash(), registry_address: spec.registry_address, + seed_peers: spec.seed_peers.clone(), }; txn.try_update_global_config(ctx, &new) diff --git a/core/node/consensus/src/testonly.rs b/core/node/consensus/src/testonly.rs index 241998f2692..bcd22186a4b 100644 --- a/core/node/consensus/src/testonly.rs +++ b/core/node/consensus/src/testonly.rs @@ -91,11 +91,8 @@ impl ConfigSet { } } -pub(super) fn new_configs( - rng: &mut impl Rng, - setup: &Setup, - gossip_peers: usize, -) -> Vec { +pub(super) fn new_configs(rng: &mut impl Rng, setup: &Setup, seed_peers: usize) -> Vec { + let net_cfgs = network::testonly::new_configs(rng, setup, 0); let genesis_spec = config::GenesisSpec { chain_id: setup.genesis.chain_id.0.try_into().unwrap(), protocol_version: config::ProtocolVersion(setup.genesis.protocol_version.0), @@ -117,8 +114,17 @@ pub(super) fn new_configs( .collect(), leader: config::ValidatorPublicKey(setup.validator_keys[0].public().encode()), registry_address: None, + seed_peers: net_cfgs[..seed_peers] + .iter() + .map(|c| { + ( + config::NodePublicKey(c.gossip.key.public().encode()), + config::Host(c.public_addr.0.clone()), + ) + }) + .collect(), }; - network::testonly::new_configs(rng, setup, gossip_peers) + net_cfgs .into_iter() .enumerate() .map(|(i, net)| ConfigSet { diff --git a/core/node/consensus/src/tests/attestation.rs b/core/node/consensus/src/tests/attestation.rs index abd35508c7f..e783dbecdc3 100644 --- a/core/node/consensus/src/tests/attestation.rs +++ b/core/node/consensus/src/tests/attestation.rs @@ -47,6 +47,7 @@ async fn test_attestation_status_api(version: ProtocolVersionId) { &consensus_dal::GlobalConfig { genesis: setup.genesis.clone(), registry_address: None, + seed_peers: [].into(), }, ) .await diff --git a/core/node/consensus/src/tests/mod.rs b/core/node/consensus/src/tests/mod.rs index 91f01f865a2..aabfff462a8 100644 --- a/core/node/consensus/src/tests/mod.rs +++ b/core/node/consensus/src/tests/mod.rs @@ -49,6 +49,7 @@ async fn test_validator_block_store(version: ProtocolVersionId) { &consensus_dal::GlobalConfig { genesis: setup.genesis.clone(), registry_address: None, + seed_peers: [].into(), }, ) .await diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 5624403d785..b943de65ce5 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -7551,6 +7551,7 @@ dependencies = [ "tokio", "tracing", "vise", + "zksync_concurrency", "zksync_consensus_roles", "zksync_consensus_storage", "zksync_contracts", diff --git a/zk_toolbox/crates/zk_inception/src/utils/consensus.rs b/zk_toolbox/crates/zk_inception/src/utils/consensus.rs index 06848334a6e..0b24bbe5cdc 100644 --- a/zk_toolbox/crates/zk_inception/src/utils/consensus.rs +++ b/zk_toolbox/crates/zk_inception/src/utils/consensus.rs @@ -95,6 +95,7 @@ pub fn get_genesis_specs( attesters: vec![attester], leader, registry_address: None, + seed_peers: [].into(), } }