Removed protocol_version from config (#49)

## What ❔

Removed protocol_version from config.
Instead, I've added a constant in bft crate indicating the version of
the protocol implementation.
The payload format should be versioned separately. Payload format
version should be included in the payload itself (set via
PayloadSource::propose, verified via WriteBlockStore::verify_payload).

## Why ❔

What is being versioned is actually the implementation of the consensus,
so it doesn't make sense to configure it via config. If it was
configurable the implementation wouldn't be able to reason about the
protocol version and compatibility of the peers.

node/actors/bft/src/inner.rs

@@ -18,8 +18,6 @@ pub(crate) struct ConsensusInner {
     pub(crate) secret_key: validator::SecretKey,
     /// A vector of public keys for all the validators in the network.
     pub(crate) validator_set: validator::ValidatorSet,
-    /// Current protocol version for the consensus messages.
-    pub(crate) protocol_version: validator::ProtocolVersion,
 }
 
 impl ConsensusInner {

node/actors/bft/src/leader/replica_commit.rs

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::{inner::ConsensusInner, metrics};
+use crate::{inner::ConsensusInner, metrics, Consensus};
 use tracing::instrument;
 use zksync_concurrency::{ctx, metrics::LatencyHistogramExt as _};
 use zksync_consensus_network::io::{ConsensusInputMessage, Target};
@@ -73,13 +73,10 @@ impl StateMachine {
         let author = &signed_message.key;
 
         // Check protocol version compatibility.
-        if !consensus
-            .protocol_version
-            .compatible(&message.protocol_version)
-        {
+        if !Consensus::PROTOCOL_VERSION.compatible(&message.protocol_version) {
             return Err(Error::IncompatibleProtocolVersion {
                 message_version: message.protocol_version,
-                local_version: consensus.protocol_version,
+                local_version: Consensus::PROTOCOL_VERSION,
             });
         }
 
@@ -181,7 +178,7 @@ impl StateMachine {
                 .secret_key
                 .sign_msg(validator::ConsensusMsg::LeaderCommit(
                     validator::LeaderCommit {
-                        protocol_version: consensus.protocol_version,
+                        protocol_version: Consensus::PROTOCOL_VERSION,
                         justification,
                     },
                 )),

node/actors/bft/src/leader/replica_prepare.rs

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::{inner::ConsensusInner, metrics};
+use crate::{inner::ConsensusInner, metrics, Consensus};
 use std::collections::HashMap;
 use tracing::instrument;
 use zksync_concurrency::{ctx, error::Wrap};
@@ -99,13 +99,10 @@ impl StateMachine {
         let author = &signed_message.key;
 
         // Check protocol version compatibility.
-        if !consensus
-            .protocol_version
-            .compatible(&message.protocol_version)
-        {
+        if !Consensus::PROTOCOL_VERSION.compatible(&message.protocol_version) {
             return Err(Error::IncompatibleProtocolVersion {
                 message_version: message.protocol_version,
-                local_version: consensus.protocol_version,
+                local_version: Consensus::PROTOCOL_VERSION,
             });
         }
 
@@ -260,7 +257,7 @@ impl StateMachine {
                 .secret_key
                 .sign_msg(validator::ConsensusMsg::LeaderPrepare(
                     validator::LeaderPrepare {
-                        protocol_version: consensus.protocol_version,
+                        protocol_version: Consensus::PROTOCOL_VERSION,
                         view: self.view,
                         proposal,
                         proposal_payload: payload,

node/actors/bft/src/lib.rs

@@ -56,12 +56,14 @@ pub struct Consensus {
 }
 
 impl Consensus {
+    /// Protocol version of this BFT implementation.
+    pub const PROTOCOL_VERSION: validator::ProtocolVersion = validator::ProtocolVersion::EARLIEST;
+
     /// Creates a new Consensus struct.
     #[instrument(level = "trace", skip(payload_source))]
     pub async fn new(
         ctx: &ctx::Ctx,
         pipe: ActorPipe<InputMessage, OutputMessage>,
-        protocol_version: validator::ProtocolVersion,
         secret_key: validator::SecretKey,
         validator_set: validator::ValidatorSet,
         storage: ReplicaStore,
@@ -72,7 +74,6 @@ impl Consensus {
                 pipe,
                 secret_key,
                 validator_set,
-                protocol_version,
             },
             replica: replica::StateMachine::new(ctx, storage).await?,
             leader: leader::StateMachine::new(ctx, payload_source),

node/actors/bft/src/replica/leader_commit.rs

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::inner::ConsensusInner;
+use crate::{inner::ConsensusInner, Consensus};
 use tracing::instrument;
 use zksync_concurrency::{ctx, error::Wrap};
 use zksync_consensus_roles::validator::{self, ProtocolVersion};
@@ -74,13 +74,10 @@ impl StateMachine {
         let view = message.justification.message.view;
 
         // Check protocol version compatibility.
-        if !consensus
-            .protocol_version
-            .compatible(&message.protocol_version)
-        {
+        if !Consensus::PROTOCOL_VERSION.compatible(&message.protocol_version) {
             return Err(Error::IncompatibleProtocolVersion {
                 message_version: message.protocol_version,
-                local_version: consensus.protocol_version,
+                local_version: Consensus::PROTOCOL_VERSION,
             });
         }
 

node/actors/bft/src/replica/leader_prepare.rs

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::inner::ConsensusInner;
+use crate::{inner::ConsensusInner, Consensus};
 use std::collections::HashMap;
 use tracing::instrument;
 use zksync_concurrency::{ctx, error::Wrap};
@@ -144,13 +144,10 @@ impl StateMachine {
         let view = message.view;
 
         // Check protocol version compatibility.
-        if !consensus
-            .protocol_version
-            .compatible(&message.protocol_version)
-        {
+        if !Consensus::PROTOCOL_VERSION.compatible(&message.protocol_version) {
             return Err(Error::IncompatibleProtocolVersion {
                 message_version: message.protocol_version,
-                local_version: consensus.protocol_version,
+                local_version: Consensus::PROTOCOL_VERSION,
             });
         }
 
@@ -300,7 +297,7 @@ impl StateMachine {
 
         // Create our commit vote.
         let commit_vote = validator::ReplicaCommit {
-            protocol_version: consensus.protocol_version,
+            protocol_version: Consensus::PROTOCOL_VERSION,
             view,
             proposal: message.proposal,
         };

node/actors/bft/src/replica/new_view.rs

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::ConsensusInner;
+use crate::{Consensus, ConsensusInner};
 use tracing::instrument;
 use zksync_concurrency::{ctx, error::Wrap as _};
 use zksync_consensus_network::io::{ConsensusInputMessage, Target};
@@ -34,7 +34,7 @@ impl StateMachine {
                 .secret_key
                 .sign_msg(validator::ConsensusMsg::ReplicaPrepare(
                     validator::ReplicaPrepare {
-                        protocol_version: consensus.protocol_version,
+                        protocol_version: Consensus::PROTOCOL_VERSION,
                         view: next_view,
                         high_vote: self.high_vote,
                         high_qc: self.high_qc.clone(),

node/actors/bft/src/testonly/make.rs

@@ -41,7 +41,6 @@ pub async fn make_consensus(
     let consensus = Consensus::new(
         ctx,
         consensus_pipe,
-        genesis_block.justification.message.protocol_version,
         key.clone(),
         validator_set.clone(),
         ReplicaStore::from_store(Arc::new(storage)),
@@ -57,7 +56,6 @@ pub async fn make_consensus(
 /// and a validator set for the chain.
 pub fn make_genesis(
     keys: &[validator::SecretKey],
-    protocol_version: validator::ProtocolVersion,
     payload: validator::Payload,
     block_number: validator::BlockNumber,
 ) -> (validator::FinalBlock, validator::ValidatorSet) {
@@ -67,7 +65,7 @@ pub fn make_genesis(
         .iter()
         .map(|sk| {
             sk.sign_msg(validator::ReplicaCommit {
-                protocol_version,
+                protocol_version: validator::ProtocolVersion::EARLIEST,
                 view: validator::ViewNumber(0),
                 proposal: header,
             })

node/actors/bft/src/testonly/run.rs

@@ -32,12 +32,8 @@ impl Test {
             .iter()
             .map(|node| node.consensus_config().key.clone())
             .collect();
-        let (genesis_block, _) = testonly::make_genesis(
-            &keys,
-            validator::ProtocolVersion::EARLIEST,
-            validator::Payload(vec![]),
-            validator::BlockNumber(0),
-        );
+        let (genesis_block, _) =
+            testonly::make_genesis(&keys, validator::Payload(vec![]), validator::BlockNumber(0));
         let nodes: Vec<_> = nodes
             .into_iter()
             .enumerate()
@@ -107,7 +103,6 @@ async fn run_nodes(ctx: &ctx::Ctx, network: Network, nodes: &[Node]) -> anyhow::
                     let consensus = Consensus::new(
                         ctx,
                         consensus_actor_pipe,
-                        validator::ProtocolVersion::EARLIEST,
                         node.net.consensus_config().key.clone(),
                         validator_set,
                         storage,

node/actors/bft/src/testonly/ut_harness.rs

@@ -32,12 +32,8 @@ impl UTHarness {
     pub(crate) async fn new(ctx: &ctx::Ctx, num_validators: usize) -> UTHarness {
         let mut rng = ctx.rng();
         let keys: Vec<_> = (0..num_validators).map(|_| rng.gen()).collect();
-        let (genesis, val_set) = crate::testonly::make_genesis(
-            &keys,
-            validator::ProtocolVersion::EARLIEST,
-            Payload(vec![]),
-            validator::BlockNumber(0),
-        );
+        let (genesis, val_set) =
+            crate::testonly::make_genesis(&keys, Payload(vec![]), validator::BlockNumber(0));
         let (mut consensus, pipe) =
             crate::testonly::make_consensus(ctx, &keys[0], &val_set, &genesis).await;
 
@@ -62,7 +58,7 @@ impl UTHarness {
     /// recovers after a timeout.
     pub(crate) async fn produce_block_after_timeout(&mut self, ctx: &ctx::Ctx) {
         let want = ReplicaPrepare {
-            protocol_version: self.consensus.inner.protocol_version,
+            protocol_version: self.protocol_version(),
             view: self.consensus.replica.view.next(),
             high_qc: self.consensus.replica.high_qc.clone(),
             high_vote: self.consensus.replica.high_vote,
@@ -81,7 +77,7 @@ impl UTHarness {
     }
 
     pub(crate) fn protocol_version(&self) -> validator::ProtocolVersion {
-        self.consensus.inner.protocol_version
+        Consensus::PROTOCOL_VERSION
     }
 
     pub(crate) fn incompatible_protocol_version(&self) -> validator::ProtocolVersion {

node/actors/executor/src/config/mod.rs

@@ -23,8 +23,6 @@ pub struct ConsensusConfig {
     /// Public TCP address that other validators are expected to connect to.
     /// It is announced over gossip network.
     pub public_addr: net::SocketAddr,
-    /// Currently used protocol version for consensus messages.
-    pub protocol_version: validator::ProtocolVersion,
 }
 
 impl ProtoFmt for ConsensusConfig {
@@ -34,18 +32,13 @@ impl ProtoFmt for ConsensusConfig {
         Ok(Self {
             key: read_required_text(&proto.key).context("key")?,
             public_addr: read_required_text(&proto.public_addr).context("public_addr")?,
-            protocol_version: required(&proto.protocol_version)
-                .copied()
-                .and_then(validator::ProtocolVersion::try_from)
-                .context("protocol_version")?,
         })
     }
 
     fn build(&self) -> Self::Proto {
         Self::Proto {
             key: Some(self.key.encode()),
             public_addr: Some(self.public_addr.encode()),
-            protocol_version: Some(self.protocol_version.as_u32()),
         }
     }
 }

node/actors/executor/src/config/proto/mod.proto

@@ -48,8 +48,6 @@ message NodeAddr {
 message ConsensusConfig {
   optional string key = 1; // [required] ValidatorPublicKey
   optional string public_addr = 2; // [required] IpAddr
-  // Currently used protocol version for consensus messages.
-  optional uint32 protocol_version = 3; // [required]
 }
 
 // Config of the gossip network.

node/actors/executor/src/config/tests.rs

@@ -16,7 +16,6 @@ impl Distribution<ConsensusConfig> for Standard {
         ConsensusConfig {
             key: rng.gen::<validator::SecretKey>().public(),
             public_addr: make_addr(rng),
-            protocol_version: validator::ProtocolVersion::EARLIEST,
         }
     }
 }

node/actors/executor/src/lib.rs

@@ -181,7 +181,6 @@ impl<S: WriteBlockStore + 'static> Executor<S> {
             let consensus = Consensus::new(
                 ctx,
                 consensus_actor_pipe,
-                validator.config.protocol_version,
                 validator.key.clone(),
                 validator_set.clone(),
                 consensus_storage,

node/actors/executor/src/testonly.rs

@@ -8,14 +8,10 @@ use zksync_consensus_network::{consensus, testonly::Instance};
 use zksync_consensus_roles::{node, validator};
 
 impl ConsensusConfig {
-    fn from_network_config(
-        src: consensus::Config,
-        protocol_version: validator::ProtocolVersion,
-    ) -> Self {
+    fn from_network_config(src: consensus::Config) -> Self {
         Self {
             key: src.key.public(),
             public_addr: src.public_addr,
-            protocol_version,
         }
     }
 }
@@ -36,11 +32,8 @@ pub struct FullValidatorConfig {
 
 impl FullValidatorConfig {
     /// Generates a validator config for a network with a single validator.
-    ///
-    /// `protocol_version` is used both for the genesis block and as the current protocol version.
     pub fn for_single_validator(
         rng: &mut impl Rng,
-        protocol_version: validator::ProtocolVersion,
         genesis_block_payload: validator::Payload,
         genesis_block_number: validator::BlockNumber,
     ) -> Self {
@@ -49,12 +42,10 @@ impl FullValidatorConfig {
         let net_config = net_configs.pop().unwrap();
         let consensus_config = net_config.consensus.unwrap();
         let validator_key = consensus_config.key.clone();
-        let consensus_config =
-            ConsensusConfig::from_network_config(consensus_config, protocol_version);
+        let consensus_config = ConsensusConfig::from_network_config(consensus_config);
 
         let (genesis_block, validators) = make_genesis(
             &[validator_key.clone()],
-            protocol_version,
             genesis_block_payload,
             genesis_block_number,
         );