bumped capability ID of the push_batch_votes rpc (#176)

We added an extra field to the Batch message, without considering the
fact that an extra field won't be recognized by old binaries and they
will fail to verify signatures on Batch messages. They will consider
sending such invalid signatures malicious and will disconnect from the
peer. To avoid this we create a new RPC instead (by just bumping the
capability id on push_batch_votes RPC) which will not be interpreted by
the old binaries at all.

node/actors/bft/src/testonly/twins/partition.rs

@@ -75,12 +75,12 @@ impl<'a, T> Partitioner<'a, T> {
     ///
     /// Take the table in [partitions] as an example with 4 items and 3 partitions.
     /// * `A` is the first item, so it can only go into `P1`; `P2` can't be used while `P1` is empty,
-    /// otherwise we'd be generating redundant combinations.
+    ///    otherwise we'd be generating redundant combinations.
     /// * `B` can go minimally into `P1` because there are 2 more items after it, which is enough
-    /// to fill all remaining partitions; or it can go into `P2`, because `P1` is filled.
+    ///    to fill all remaining partitions; or it can go into `P2`, because `P1` is filled.
     /// * `C` depends on what we did with `B`: if `B` is in `P1` then `C` has to minimally go into
-    /// `P2` to make sure no partition will be left empty at the end; if `B` is in `P2` then `C`
-    /// can go either in `P1`, `P2` or `P3`.
+    ///   `P2` to make sure no partition will be left empty at the end; if `B` is in `P2` then `C`
+    ///   can go either in `P1`, `P2` or `P3`.
     ///
     /// The algorithm has to traverse all possible placements with backtracking.
     fn go(&mut self, idx: usize, first_empty: usize) {

node/actors/executor/src/tests.rs

@@ -1,9 +1,8 @@
 //! High-level tests for `Executor`.
-use std::sync::{atomic::AtomicU64, Mutex};
-
 use super::*;
 use attestation::{AttestationStatusClient, AttestationStatusRunner};
 use rand::Rng as _;
+use std::sync::{atomic::AtomicU64, Mutex};
 use tracing::Instrument as _;
 use zksync_concurrency::{sync, testonly::abort_on_panic};
 use zksync_consensus_bft as bft;

node/actors/network/src/gossip/attestation_status.rs

@@ -1,10 +1,8 @@
+use crate::watch::Watch;
 use std::fmt;
-
 use zksync_concurrency::sync;
 use zksync_consensus_roles::attester;
 
-use crate::watch::Watch;
-
 /// Coordinate the attestation by showing the status as seen by the main node.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct AttestationStatus {

node/actors/network/src/gossip/mod.rs

@@ -9,14 +9,15 @@
 //! * static connections (explicitly declared in configs of both ends of the connection).
 //! * dynamic connections (additional randomized connections which are established to improve
 //!   the throughput of the network).
+//!
 //! Static connections constitute a rigid "backbone" of the gossip network, which is insensitive to
 //! eclipse attack. Dynamic connections are supposed to improve the properties of the gossip
 //! network graph (minimize its diameter, increase connectedness).
-pub use self::attestation_status::{
-    AttestationStatus, AttestationStatusReceiver, AttestationStatusWatch,
-};
-pub use self::batch_votes::BatchVotesPublisher;
 use self::batch_votes::BatchVotesWatch;
+pub use self::{
+    attestation_status::{AttestationStatus, AttestationStatusReceiver, AttestationStatusWatch},
+    batch_votes::BatchVotesPublisher,
+};
 use crate::{gossip::ValidatorAddrsWatch, io, pool::PoolWatch, Config, MeteredStreamStats};
 use fetch::RequestItem;
 use std::sync::{atomic::AtomicUsize, Arc};

node/actors/network/src/mux/mod.rs

@@ -40,10 +40,12 @@
 //! * capability 0 has 4 reusable streams
 //! * capability 1 has 3 reusable streams
 //! * capability 2 has 5 reusable streams
+//!
 //! then
 //! * streams 0,1,2,3 belong to capability 0.
 //! * streams 4,5,6 belong to capability 1.
 //! * streams 7,8,9,10,11 belong to capability 2.
+//!
 //! NOTE that the 2^13 limit applies to the TOTAL number of reusable streams across all
 //! capabilities.
 //!
@@ -59,7 +61,7 @@
 //! Multipexer protocol:
 //! 1. peer A and B exchange their multiplexer configs.
 //! 2. based on both configs, A and B determine how many stream IDs will be used and to which
-//!   capabilities they will belong.
+//!    capabilities they will belong.
 //! 3. For every stream IDs in use execute Reusable Stream protocol concurrently.
 //!
 //! Reusable stream protocol (for stream ID = x, with capability = c, where A is outbound peer and

node/actors/network/src/noise/stream.rs

@@ -48,7 +48,7 @@ const MAX_PAYLOAD_LEN: usize = MAX_TRANSPORT_MSG_LEN - AUTHDATA_LEN;
 /// <frame len:u16> ++ <frame data:[u8;len]>.
 ///
 /// Length of the frame len field.
-const LENGTH_FIELD_LEN: usize = std::mem::size_of::<u16>();
+const LENGTH_FIELD_LEN: usize = size_of::<u16>();
 
 /// Max size of the whole frame (length field + data).
 const MAX_FRAME_LEN: usize = MAX_TRANSPORT_MSG_LEN + LENGTH_FIELD_LEN;
@@ -236,6 +236,7 @@ where
     /// * The amount of data read can be determined by the increase
     ///   in the length of the slice returned by ReadBuf::filled.
     /// * If the difference is 0, EOF has been reached.
+    ///
     /// From std::io::Read:
     /// * If error was returned, no bytes were read.
     ///
@@ -309,6 +310,7 @@ where
 {
     /// from futures::io::AsyncWrite:
     /// * poll_write must try to make progress by flushing if needed to become writable
+    ///
     /// from std::io::Write:
     /// * call to write represents at most one attempt to write to any wrapped object.
     /// * 0 TYPICALLY means that that the underlying object is no longer able to accept bytes OR

node/actors/network/src/rpc/push_batch_votes.rs

@@ -8,8 +8,18 @@ use zksync_protobuf::ProtoFmt;
 /// PushBatchVotes RPC.
 pub(crate) struct Rpc;
 
+/// Deprecated, because adding `genesis_hash` to `validator::Batch`
+/// was not backward compatible - old binaries couldn't verify
+/// signatures on messages with `genesis_hash` and were treating it
+/// as malicious behavior.
+#[allow(dead_code)]
+pub(super) const V1: mux::CapabilityId = 5;
+
+/// Current version.
+pub(super) const V2: mux::CapabilityId = 8;
+
 impl super::Rpc for Rpc {
-    const CAPABILITY_ID: mux::CapabilityId = 5;
+    const CAPABILITY_ID: mux::CapabilityId = V2;
     const INFLIGHT: u32 = 1;
     const METHOD: &'static str = "push_batch_votes";
     type Req = Req;

node/actors/network/src/rpc/tests.rs

@@ -17,7 +17,8 @@ fn test_capability_rpc_correspondence() {
         push_block_store_state::Rpc::CAPABILITY_ID,
         get_block::Rpc::CAPABILITY_ID,
         ping::Rpc::CAPABILITY_ID,
-        push_batch_votes::Rpc::CAPABILITY_ID,
+        push_batch_votes::V1,
+        push_batch_votes::V2,
         push_batch_store_state::Rpc::CAPABILITY_ID,
         get_batch::Rpc::CAPABILITY_ID,
     ];

node/libs/concurrency/src/ctx/channel.rs

@@ -3,6 +3,7 @@
 //! Note that channel disconnection is not observable by default:
 //! * send() always succeeds, unless canceled
 //! * recv() always succeeds, unless canceled
+//!
 //! This
 //! * simplifies the channel interface
 //! * prevents users from relying on send/recv failure

node/libs/concurrency/src/sync/prunable_mpsc/mod.rs

@@ -32,7 +32,7 @@ pub enum SelectionFunctionResult {
 /// * [`T`]: The type of data that will be sent through the channel.
 /// * [`filter_predicate`]: A predicate that checks the newly sent value and avoids adding to the queue if it returns false
 /// * [`selection_function`]: A function that determines whether an unreceived, pending value in the buffer (represented by the first `T`) should be pruned
-/// based on a newly sent value (represented by the second `T`) or the new value should be filtered out, or both should be kept.
+///   based on a newly sent value (represented by the second `T`) or the new value should be filtered out, or both should be kept.
 pub fn channel<T>(
     filter_predicate: impl 'static + Sync + Send + Fn(&T) -> bool,
     selection_function: impl 'static + Sync + Send + Fn(&T, &T) -> SelectionFunctionResult,

node/libs/protobuf_build/src/lib.rs

@@ -14,6 +14,7 @@
 //!    cargo cache.
 //! 2. grep through all target/debug/build/*/output files to find all "PROTOBUF_DESCRIPTOR=..."
 //!    lines and merge the descriptor files by simply concatenating them.
+//!
 //! Note that you can run this procedure for 2 revisions of the repo and look for breaking
 //! changes by running "buf breaking <after.binpb> --against <before.binpb>" where before.binpb
 //! and after.binpb are the concatenated descriptors from those 2 revisions.