Merge remote-tracking branch 'origin/main' into gprusak-payload-inter…

…face2

.github/workflows/rust.yml

@@ -34,10 +34,6 @@ jobs:
         uses: baptiste0928/cargo-install@v2
         with:
           crate: cargo-nextest
-      - name: install cranky
-        uses: baptiste0928/cargo-install@v2
-        with:
-          crate: cargo-cranky
       - name: install deny
         uses: baptiste0928/cargo-install@v2
         with:
@@ -46,10 +42,10 @@ jobs:
         run: cargo deny check
       - name: fmt
         run: cargo fmt --all --check
-      - name: cranky (all features)
-        run: cargo cranky --workspace --all-targets --all-features
-      - name: cranky (default features)
-        run: cargo cranky --workspace --exclude tools --all-targets
+      - name: clippy (all features)
+        run: cargo clippy --workspace --all-targets --all-features
+      - name: clippy (default features)
+        run: cargo clippy --workspace --exclude zksync_consensus_tools --all-targets
       - name: build
         run: cargo build --all-targets --locked
       - name: test

docs/style.md

@@ -22,24 +22,6 @@ And then run tests with:
 cargo nextest run
 ```
 
-### Linting
-
-We use a particular tool called `cargo cranky` in order to be able to apply a set of lints to an entire workspace. You
-just need to install it with:
-
-```
-cargo install cargo-cranky
-```
-
-and then you can run it just like you would run clippy:
-
-```
-cargo cranky --all-targets --all-features
-```
-
-Note: There's a [RFC](https://github.com/rust-lang/rfcs/pull/3389) underway to add lints to `Cargo.toml``. But until that
-happens, we are stuck with cranky.
-
 ## Codebase Layout
 
 We want the files to be relatively small in general (lets say, <500 LoC), so feel free to split the module into multiple files

node/Cargo.toml

@@ -1,17 +1,17 @@
 [workspace]
 members = [
-    "libs/concurrency",
-    "libs/protobuf_build",
-    "libs/protobuf",
     "actors/bft",
-    "libs/crypto",
     "actors/executor",
     "actors/network",
     "actors/sync_blocks",
+    "libs/concurrency",
+    "libs/crypto",
+    "libs/protobuf",
+    "libs/protobuf_build",
     "libs/roles",
     "libs/storage",
-    "tools",
     "libs/utils",
+    "tools",
 ]
 resolver = "2"
 
@@ -22,40 +22,49 @@ homepage = "https://matter-labs.io/"
 license = "MIT"
 
 [workspace.dependencies]
-zksync_protobuf_build = { path = "libs/protobuf_build" }
-zksync_protobuf = { path = "libs/protobuf" }
-zksync_concurrency = { path = "libs/concurrency" }
+# Crates from this repo.
 zksync_consensus_bft = { path = "actors/bft" }
 zksync_consensus_crypto = { path = "libs/crypto" }
 zksync_consensus_executor = { path = "actors/executor" }
 zksync_consensus_network = { path = "actors/network" }
-zksync_consensus_sync_blocks = { path = "actors/sync_blocks" }
 zksync_consensus_roles = { path = "libs/roles" }
 zksync_consensus_storage = { path = "libs/storage" }
+zksync_consensus_sync_blocks = { path = "actors/sync_blocks" }
 zksync_consensus_tools = { path = "tools" }
 zksync_consensus_utils = { path = "libs/utils" }
 
+# Crates from this repo that might become independent in the future.
+zksync_concurrency = { path = "libs/concurrency" }
+zksync_protobuf = { path = "libs/protobuf" }
+zksync_protobuf_build = { path = "libs/protobuf_build" }
+
+# Crates from Matter Labs.
+pairing = { package = "pairing_ce", git = "https://github.com/matter-labs/pairing.git", rev = "f55393f" }
+vise = { version = "0.1.0", git = "https://github.com/matter-labs/vise.git", rev = "dd0513" }
+vise-exporter = { version = "0.1.0", git = "https://github.com/matter-labs/vise.git", rev = "dd0513" }
+
+# Crates from third-parties.
 anyhow = "1"
 assert_matches = "1.5.0"
 async-trait = "0.1.71"
 bit-vec = "0.6"
 blst = "0.3.10"
-pairing = { package = "pairing_ce", git = "https://github.com/matter-labs/pairing.git", rev = "f55393f" }
-ff_ce = "0.14.3"
 clap = { version = "4.3.3", features = ["derive"] }
-heck = "0.4.1"
+criterion = "0.5.1"
 ed25519-dalek = { version = "2.0.0", features = ["rand_core"] }
+ff_ce = "0.14.3"
+heck = "0.4.1"
 hex = "0.4.3"
 im = "15.1.0"
 once_cell = "1.17.1"
 pin-project = "1.1.0"
+pretty_assertions = "1.4.0"
+prettyplease = "0.2.6"
 proc-macro2 = "1.0.66"
 prost = "0.12.0"
 prost-build = "0.12.0"
 prost-reflect = { version = "0.12.0", features = ["serde"] }
 protox = "0.5.0"
-prettyplease = "0.2.6"
-pretty_assertions = "1.4.0"
 quick-protobuf = "0.8.1"
 quote = "1.0.33"
 rand = "0.8.0"
@@ -70,13 +79,9 @@ tempfile = "3"
 test-casing = "0.1.0"
 thiserror = "1.0.40"
 time = "0.3.23"
-# TODO(gprusak): only concurrency crate should depend on tokio.
-# All the other crates should depend on concurrency.
 tokio = { version = "1.34.0", features = ["full"] }
 tracing = { version = "0.1.37", features = ["attributes"] }
 tracing-subscriber = { version = "0.3.16", features = ["env-filter", "fmt"] }
-vise = { version = "0.1.0", git = "https://github.com/matter-labs/vise.git", rev = "dd05139b76ab0843443ab3ff730174942c825dae" }
-vise-exporter = { version = "0.1.0", git = "https://github.com/matter-labs/vise.git", rev = "dd05139b76ab0843443ab3ff730174942c825dae" }
 
 # Note that "bench" profile inherits from "release" profile and
 # "test" profile inherits from "dev" profile.
@@ -104,3 +109,44 @@ opt-level = 3
 # is currently considered too slow for development.
 [profile.dev.package."*"]
 opt-level = 3
+
+[workspace.lints.rust]
+unsafe_code = "deny"
+noop_method_call = "warn"
+missing_docs = "warn"
+unreachable_pub = "warn"
+unused_qualifications = "warn"
+unused_tuple_struct_fields = "warn"
+
+[workspace.lints.clippy]
+# restriction group
+create_dir = "warn"
+empty_structs_with_brackets = "warn"
+float_arithmetic = "warn"
+missing_docs_in_private_items = "warn"
+non_ascii_literal = "warn"
+partial_pub_fields = "warn"
+print_stdout = "warn"
+string_to_string = "warn"
+suspicious_xor_used_as_pow = "warn"
+try_err = "warn"
+separated_literal_suffix = "warn"
+
+# pedantic group
+bool_to_int_with_if = "warn"
+default_trait_access = "warn"
+if_not_else = "warn"
+manual_assert = "warn"
+manual_instant_elapsed = "warn"
+manual_let_else = "warn"
+manual_ok_or = "warn"
+manual_string_new = "warn"
+match_bool = "warn"
+wildcard_imports = "warn"
+
+# cargo group
+wildcard_dependencies = "warn"
+
+# Produces too many false positives.
+redundant_locals = "allow"
+needless_pass_by_ref_mut = "allow"

node/actors/bft/Cargo.toml

@@ -27,3 +27,6 @@ vise.workspace = true
 tokio.workspace = true
 assert_matches.workspace = true
 pretty_assertions.workspace = true
+
+[lints]
+workspace = true

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

@@ -49,7 +49,7 @@ async fn replica_prepare_sanity_yield_leader_prepare_reproposal() {
     let ctx = &ctx::test_root(&ctx::RealClock);
     let mut util = UTHarness::new_many(ctx).await;
     util.new_replica_commit(ctx).await;
-    util.replica_timeout();
+    util.process_replica_timeout(ctx).await;
     let replica_prepare = util.new_replica_prepare(|_| {}).msg;
     let leader_prepare = util
         .process_replica_prepare_all(ctx, replica_prepare.clone())

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

@@ -26,7 +26,7 @@ async fn leader_prepare_reproposal_sanity() {
     let ctx = &ctx::test_root(&ctx::RealClock);
     let mut util = UTHarness::new_many(ctx).await;
     util.new_replica_commit(ctx).await;
-    util.replica_timeout();
+    util.process_replica_timeout(ctx).await;
     let leader_prepare = util.new_leader_prepare(ctx).await;
     assert!(leader_prepare.msg.proposal_payload.is_none());
     util.process_leader_prepare(ctx, leader_prepare)

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

@@ -57,6 +57,25 @@ impl UTHarness {
         UTHarness::new(ctx, num_validators).await
     }
 
+    /// Triggers replica timeout, validates the new ReplicaPrepare
+    /// then executes the whole new view to make sure that the consensus
+    /// 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,
+            view: self.consensus.replica.view.next(),
+            high_qc: self.consensus.replica.high_qc.clone(),
+            high_vote: self.consensus.replica.high_vote,
+        };
+        let replica_prepare = self.process_replica_timeout(ctx).await;
+        assert_eq!(want, replica_prepare.msg);
+
+        let leader_commit = self.new_leader_commit(ctx).await;
+        self.process_leader_commit(ctx, leader_commit)
+            .await
+            .unwrap();
+    }
+
     pub(crate) fn consensus_threshold(&self) -> usize {
         crate::misc::consensus_threshold(self.keys.len())
     }
@@ -94,11 +113,6 @@ impl UTHarness {
         self.consensus.replica.view = view
     }
 
-    pub(crate) fn replica_timeout(&mut self) {
-        self.consensus.replica.view = self.consensus.replica.view.next();
-        self.consensus.replica.phase = Phase::Prepare;
-    }
-
     pub(crate) fn new_replica_prepare(
         &mut self,
         mutate_fn: impl FnOnce(&mut ReplicaPrepare),
@@ -295,6 +309,22 @@ impl UTHarness {
         })
     }
 
+    pub(crate) async fn process_replica_timeout(
+        &mut self,
+        ctx: &ctx::Ctx,
+    ) -> Signed<ReplicaPrepare> {
+        self.consensus
+            .replica
+            .process_input(ctx, &self.consensus.inner, None)
+            .await
+            .unwrap();
+        self.try_recv().unwrap()
+    }
+
+    pub(crate) fn leader_phase(&self) -> Phase {
+        self.consensus.leader.phase
+    }
+
     pub(crate) fn view_leader(&self, view: ViewNumber) -> validator::PublicKey {
         self.consensus.inner.view_leader(view)
     }