Merge branch 'main' into liveness_tests

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

@@ -3,11 +3,25 @@ use crate::{inner::ConsensusInner, metrics};
 use tracing::instrument;
 use zksync_concurrency::{ctx, metrics::LatencyHistogramExt as _};
 use zksync_consensus_network::io::{ConsensusInputMessage, Target};
-use zksync_consensus_roles::validator;
+use zksync_consensus_roles::validator::{self, ProtocolVersion};
 
 /// Errors that can occur when processing a "replica commit" message.
 #[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
+    /// Incompatible protocol version.
+    #[error("incompatible protocol version (message version: {message_version:?}, local version: {local_version:?}")]
+    IncompatibleProtocolVersion {
+        /// Message version.
+        message_version: ProtocolVersion,
+        /// Local version.
+        local_version: ProtocolVersion,
+    },
+    /// Message signer isn't part of the validator set.
+    #[error("Message signer isn't part of the validator set (signer: {signer:?})")]
+    NonValidatorSigner {
+        /// Signer of the message.
+        signer: validator::PublicKey,
+    },
     /// Unexpected proposal.
     #[error("unexpected proposal")]
     UnexpectedProposal,
@@ -58,6 +72,25 @@ impl StateMachine {
         let message = signed_message.msg;
         let author = &signed_message.key;
 
+        // Check protocol version compatibility.
+        if !consensus
+            .protocol_version
+            .compatible(&message.protocol_version)
+        {
+            return Err(Error::IncompatibleProtocolVersion {
+                message_version: message.protocol_version,
+                local_version: consensus.protocol_version,
+            });
+        }
+
+        // Check that the message signer is in the validator set.
+        consensus
+            .validator_set
+            .index(author)
+            .ok_or(Error::NonValidatorSigner {
+                signer: author.clone(),
+            })?;
+
         // If the message is from the "past", we discard it.
         if (message.view, validator::Phase::Commit) < (self.view, self.phase) {
             return Err(Error::Old {

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

@@ -5,11 +5,25 @@ use std::collections::HashMap;
 use tracing::instrument;
 use zksync_concurrency::ctx;
 use zksync_consensus_network::io::{ConsensusInputMessage, Target};
-use zksync_consensus_roles::validator;
+use zksync_consensus_roles::validator::{self, ProtocolVersion};
 
 /// Errors that can occur when processing a "replica prepare" message.
 #[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
+    /// Incompatible protocol version.
+    #[error("incompatible protocol version (message version: {message_version:?}, local version: {local_version:?}")]
+    IncompatibleProtocolVersion {
+        /// Message version.
+        message_version: ProtocolVersion,
+        /// Local version.
+        local_version: ProtocolVersion,
+    },
+    /// Message signer isn't part of the validator set.
+    #[error("Message signer isn't part of the validator set (signer: {signer:?})")]
+    NonValidatorSigner {
+        /// Signer of the message.
+        signer: validator::PublicKey,
+    },
     /// Past view or phase.
     #[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
     Old {
@@ -70,6 +84,25 @@ impl StateMachine {
         let message = signed_message.msg.clone();
         let author = &signed_message.key;
 
+        // Check protocol version compatibility.
+        if !consensus
+            .protocol_version
+            .compatible(&message.protocol_version)
+        {
+            return Err(Error::IncompatibleProtocolVersion {
+                message_version: message.protocol_version,
+                local_version: consensus.protocol_version,
+            });
+        }
+
+        // Check that the message signer is in the validator set.
+        consensus
+            .validator_set
+            .index(author)
+            .ok_or(Error::NonValidatorSigner {
+                signer: author.clone(),
+            })?;
+
         // If the message is from the "past", we discard it.
         if (message.view, validator::Phase::Prepare) < (self.view, self.phase) {
             return Err(Error::Old {

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

@@ -5,8 +5,8 @@ use crate::testonly::ut_harness::UTHarness;
 use assert_matches::assert_matches;
 use rand::Rng;
 use zksync_consensus_roles::validator::{
-    self, CommitQC, ConsensusMsg, LeaderCommit, LeaderPrepare, Phase, PrepareQC, ProtocolVersion,
-    ReplicaCommit, ReplicaPrepare, ViewNumber,
+    self, CommitQC, ConsensusMsg, LeaderCommit, LeaderPrepare, Phase, PrepareQC, ReplicaCommit,
+    ReplicaPrepare, ViewNumber,
 };
 
 #[tokio::test]
@@ -97,6 +97,41 @@ async fn replica_prepare_sanity_yield_leader_prepare_reproposal() {
     );
 }
 
+#[tokio::test]
+async fn replica_prepare_incompatible_protocol_version() {
+    let mut util = UTHarness::new_one().await;
+
+    let incompatible_protocol_version = util.incompatible_protocol_version();
+    let replica_prepare = util.new_current_replica_prepare(|msg| {
+        msg.protocol_version = incompatible_protocol_version;
+    });
+    let res = util.dispatch_replica_prepare_one(replica_prepare);
+    assert_matches!(
+        res,
+        Err(ReplicaPrepareError::IncompatibleProtocolVersion { message_version, local_version }) => {
+            assert_eq!(message_version, incompatible_protocol_version);
+            assert_eq!(local_version, util.protocol_version());
+        }
+    )
+}
+
+#[tokio::test]
+async fn replica_prepare_non_validator_signer() {
+    let mut util = UTHarness::new_one().await;
+
+    let replica_prepare = util.new_current_replica_prepare(|_| {}).cast().unwrap().msg;
+    let non_validator_key: validator::SecretKey = util.rng().gen();
+    let signed = non_validator_key.sign_msg(ConsensusMsg::ReplicaPrepare(replica_prepare));
+
+    let res = util.dispatch_replica_prepare_one(signed);
+    assert_matches!(
+        res,
+        Err(ReplicaPrepareError::NonValidatorSigner { signer }) => {
+            assert_eq!(signer, non_validator_key.public());
+        }
+    );
+}
+
 #[tokio::test]
 async fn replica_prepare_old_view() {
     let mut util = UTHarness::new_one().await;
@@ -250,25 +285,6 @@ async fn replica_prepare_high_qc_of_future_view() {
     );
 }
 
-#[ignore = "fails/unsupported"]
-#[tokio::test]
-async fn replica_prepare_non_validator_signer() {
-    let mut util = UTHarness::new_with(2).await;
-
-    let view = ViewNumber(2);
-    util.set_view(view);
-    assert_eq!(util.view_leader(view), util.key_at(0).public());
-
-    let replica_prepare = util.new_current_replica_prepare(|_| {});
-    let _ = util.dispatch_replica_prepare_one(replica_prepare.clone());
-
-    let non_validator: validator::SecretKey = util.rng().gen();
-    let replica_prepare = non_validator.sign_msg(replica_prepare.msg);
-    util.dispatch_replica_prepare_one(replica_prepare).unwrap();
-    // PANICS:
-    // "Couldn't create justification from valid replica messages!: Message signer isn't in the validator set"
-}
-
 #[tokio::test]
 async fn replica_commit_sanity() {
     let mut util = UTHarness::new_many().await;
@@ -314,6 +330,41 @@ async fn replica_commit_sanity_yield_leader_commit() {
     );
 }
 
+#[tokio::test]
+async fn replica_commit_incompatible_protocol_version() {
+    let mut util = UTHarness::new_one().await;
+
+    let incompatible_protocol_version = util.incompatible_protocol_version();
+    let replica_commit = util.new_current_replica_commit(|msg| {
+        msg.protocol_version = incompatible_protocol_version;
+    });
+    let res = util.dispatch_replica_commit_one(replica_commit);
+    assert_matches!(
+        res,
+        Err(ReplicaCommitError::IncompatibleProtocolVersion { message_version, local_version }) => {
+            assert_eq!(message_version, incompatible_protocol_version);
+            assert_eq!(local_version, util.protocol_version());
+        }
+    )
+}
+
+#[tokio::test]
+async fn replica_commit_non_validator_signer() {
+    let mut util = UTHarness::new_one().await;
+
+    let replica_commit = util.new_current_replica_commit(|_| {}).cast().unwrap().msg;
+    let non_validator_key: validator::SecretKey = util.rng().gen();
+    let signed = non_validator_key.sign_msg(ConsensusMsg::ReplicaCommit(replica_commit));
+
+    let res = util.dispatch_replica_commit_one(signed);
+    assert_matches!(
+        res,
+        Err(ReplicaCommitError::NonValidatorSigner { signer }) => {
+            assert_eq!(signer, non_validator_key.public());
+        }
+    );
+}
+
 #[tokio::test]
 async fn replica_commit_old() {
     let mut util = UTHarness::new_one().await;
@@ -427,38 +478,3 @@ async fn replica_commit_unexpected_proposal() {
     let res = util.dispatch_replica_commit_one(replica_commit);
     assert_matches!(res, Err(ReplicaCommitError::UnexpectedProposal));
 }
-
-#[ignore = "fails/unsupported"]
-#[tokio::test]
-async fn replica_commit_protocol_version_mismatch() {
-    let mut util = UTHarness::new_with(2).await;
-
-    let view = ViewNumber(2);
-    util.set_replica_view(view);
-    util.set_leader_view(view);
-    assert_eq!(util.view_leader(view), util.owner_key().public());
-
-    let replica_prepare_one = util.new_current_replica_prepare(|_| {});
-    let _ = util.dispatch_replica_prepare_one(replica_prepare_one.clone());
-    let replica_prepare_two = util.key_at(1).sign_msg(replica_prepare_one.msg);
-    util.dispatch_replica_prepare_one(replica_prepare_two)
-        .unwrap();
-
-    let leader_prepare = util.recv_signed().await.unwrap();
-    util.dispatch_leader_prepare(leader_prepare).await.unwrap();
-
-    let replica_commit = util.recv_signed().await.unwrap();
-    let _ = util.dispatch_replica_commit_one(replica_commit.clone());
-
-    let mut replica_commit_two = replica_commit.cast::<ReplicaCommit>().unwrap().msg;
-    replica_commit_two.protocol_version =
-        ProtocolVersion(replica_commit_two.protocol_version.0 + 1);
-
-    let replica_commit_two = util
-        .key_at(1)
-        .sign_msg(ConsensusMsg::ReplicaCommit(replica_commit_two));
-    util.dispatch_replica_commit_one(replica_commit_two)
-        .unwrap();
-    // PANICS:
-    // "Couldn't create justification from valid replica messages!: CommitQC can only be created from votes for the same message."
-}

node/actors/bft/src/lib.rs

@@ -101,14 +101,6 @@ impl Consensus {
 
             match input {
                 Some(InputMessage::Network(req)) => {
-                    if req.msg.msg.protocol_version() != self.inner.protocol_version {
-                        tracing::warn!(
-                            "bad protocol version (expected: {:?}, received: {:?})",
-                            self.inner.protocol_version,
-                            req.msg.msg.protocol_version()
-                        );
-                        continue;
-                    }
                     match &req.msg.msg {
                         validator::ConsensusMsg::ReplicaPrepare(_)
                         | validator::ConsensusMsg::ReplicaCommit(_) => {

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

@@ -2,11 +2,19 @@ use super::StateMachine;
 use crate::inner::ConsensusInner;
 use tracing::instrument;
 use zksync_concurrency::{ctx, error::Wrap};
-use zksync_consensus_roles::validator;
+use zksync_consensus_roles::validator::{self, ProtocolVersion};
 
 /// Errors that can occur when processing a "leader commit" message.
 #[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
+    /// Incompatible protocol version.
+    #[error("incompatible protocol version (message version: {message_version:?}, local version: {local_version:?}")]
+    IncompatibleProtocolVersion {
+        /// Message version.
+        message_version: ProtocolVersion,
+        /// Local version.
+        local_version: ProtocolVersion,
+    },
     /// Invalid leader.
     #[error(
         "invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})"
@@ -65,6 +73,17 @@ impl StateMachine {
         let author = &signed_message.key;
         let view = message.justification.message.view;
 
+        // Check protocol version compatibility.
+        if !consensus
+            .protocol_version
+            .compatible(&message.protocol_version)
+        {
+            return Err(Error::IncompatibleProtocolVersion {
+                message_version: message.protocol_version,
+                local_version: consensus.protocol_version,
+            });
+        }
+
         // Check that it comes from the correct leader.
         if author != &consensus.view_leader(view) {
             return Err(Error::InvalidLeader {

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

@@ -4,11 +4,19 @@ use std::collections::HashMap;
 use tracing::instrument;
 use zksync_concurrency::{ctx, error::Wrap};
 use zksync_consensus_network::io::{ConsensusInputMessage, Target};
-use zksync_consensus_roles::validator;
+use zksync_consensus_roles::validator::{self, ProtocolVersion};
 
 /// Errors that can occur when processing a "leader prepare" message.
 #[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
+    /// Incompatible protocol version.
+    #[error("incompatible protocol version (message version: {message_version:?}, local version: {local_version:?}")]
+    IncompatibleProtocolVersion {
+        /// Message version.
+        message_version: ProtocolVersion,
+        /// Local version.
+        local_version: ProtocolVersion,
+    },
     /// Invalid leader.
     #[error(
         "invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})"
@@ -132,6 +140,17 @@ impl StateMachine {
         let author = &signed_message.key;
         let view = message.view;
 
+        // Check protocol version compatibility.
+        if !consensus
+            .protocol_version
+            .compatible(&message.protocol_version)
+        {
+            return Err(Error::IncompatibleProtocolVersion {
+                message_version: message.protocol_version,
+                local_version: consensus.protocol_version,
+            });
+        }
+
         // Check that it comes from the correct leader.
         if author != &consensus.view_leader(view) {
             return Err(Error::InvalidLeader {