applied comments

node/actors/consensus/src/leader/state_machine.rs

@@ -61,24 +61,19 @@ impl StateMachine {
         let now = ctx.now();
         let label = match &input.msg {
             validator::ConsensusMsg::ReplicaPrepare(_) => {
-                let res = match self.process_replica_prepare(ctx, consensus, input.cast().unwrap())
-                {
-                    Err(err) => {
-                        tracing::warn!("{err:#}");
-                        Err(())
-                    }
-                    Ok(()) => Ok(()),
-                };
+                let res = self
+                    .process_replica_prepare(ctx, consensus, input.cast().unwrap())
+                    .map_err(|err| {
+                        tracing::warn!("process_replica_prepare: {err:#}");
+                    });
                 metrics::ConsensusMsgLabel::ReplicaPrepare.with_result(&res)
             }
             validator::ConsensusMsg::ReplicaCommit(_) => {
-                let res = match self.process_replica_commit(ctx, consensus, input.cast().unwrap()) {
-                    Err(err) => {
-                        tracing::warn!("{err:#}");
-                        Err(())
-                    }
-                    Ok(()) => Ok(()),
-                };
+                let res = self
+                    .process_replica_commit(ctx, consensus, input.cast().unwrap())
+                    .map_err(|err| {
+                        tracing::warn!("process_replica_commit: {err:#}");
+                    });
                 metrics::ConsensusMsgLabel::ReplicaCommit.with_result(&res)
             }
             _ => unreachable!(),

node/actors/consensus/src/replica/state_machine.rs

@@ -115,7 +115,7 @@ impl StateMachine {
                             return Err(err).context("process_leader_prepare()")
                         }
                         Err(err) => {
-                            tracing::warn!("{err:#}");
+                            tracing::warn!("process_leader_prepare(): {err:#}");
                             Err(())
                         }
                         Ok(()) => Ok(()),
@@ -129,7 +129,7 @@ impl StateMachine {
                             return Err(err).context("process_leader_commit()")
                         }
                         Err(err) => {
-                            tracing::warn!("{err:#}");
+                            tracing::warn!("process_leader_commit(): {err:#}");
                             Err(())
                         }
                         Ok(()) => Ok(()),

node/libs/roles/src/validator/messages/block.rs

@@ -102,7 +102,7 @@ impl fmt::Debug for BlockHeaderHash {
 /// A block header.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct BlockHeader {
-    ///
+    /// Protocol version according to which this block should be interpreted.
     pub protocol_version: ProtocolVersion,
     /// Hash of the parent block.
     pub parent: BlockHeaderHash,

node/libs/roles/src/validator/messages/consensus.rs

@@ -343,17 +343,6 @@ impl Signers {
     }
 }
 
-/*
-impl ByteFmt for Signers {
-    fn decode(bytes: &[u8]) -> anyhow::Result<Self> {
-        Ok(Signers(BitVec::from_bytes(bytes)))
-    }
-
-    fn encode(&self) -> Vec<u8> {
-        self.0.to_bytes()
-    }
-}*/
-
 /// A struct that represents a set of validators. It is used to store the current validator set.
 /// We represent each validator by its validator public key.
 #[derive(Clone, Debug, PartialEq, Eq)]

node/libs/roles/src/validator/tests.rs

@@ -181,7 +181,7 @@ fn test_commit_qc() {
     .unwrap();
     let validator_set2 =
         ValidatorSet::new(vec![rng.gen(), rng.gen(), rng.gen(), rng.gen(), rng.gen()]).unwrap();
-    //let validator_set3 = ValidatorSet::new(vec![sk1.public(), sk2.public(), sk3.public()]).unwrap();
+    let validator_set3 = ValidatorSet::new(vec![sk1.public(), sk2.public(), sk3.public()]).unwrap();
 
     let qc = CommitQC::from(
         &[sk1.sign_msg(msg), sk2.sign_msg(msg), sk3.sign_msg(msg)],
@@ -199,7 +199,7 @@ fn test_commit_qc() {
 
     // Mismatching validator sets.
     assert!(qc.verify(&validator_set2, 3).is_err());
-    //assert!(qc.verify(&validator_set3, 3).is_err());
+    assert!(qc.verify(&validator_set3, 3).is_err());
 }
 
 #[test]

node/libs/schema/proto/std.proto

@@ -38,13 +38,13 @@ message SocketAddr {
 // Note: this is not a highly optimized data structure:
 // - since it is a proto message it requires its length to be stored.
 // - it has 2 fields, which require storing their own tag
-// - the `vector` field requires storing its length, which theoretically
+// - the `bytes_` field requires storing its length, which theoretically
 //   could be derived from `size` field value.
-// If we eventually start caring about the size, we could encode BitVector as
-// a field of type `bytes` with delimiter of the form "01...1".
+// If we eventually start caring about the size, we could encode BitVector
+// directly as a field of type `bytes` with delimiter of the form "01...1".
 message BitVector {
   // Number of bits in the vector.
   optional uint64 size = 1;
   // Vector of bits encoded as bytes in big endian order.
-  optional bytes vector = 2;
+  optional bytes bytes_ = 2;
 }

node/libs/schema/src/std_conv.rs

@@ -86,7 +86,7 @@ impl ProtoFmt for bit_vec::BitVec {
 
     fn read(r: &Self::Proto) -> anyhow::Result<Self> {
         let size = *required(&r.size).context("size")? as usize;
-        let mut this = Self::from_bytes(required(&r.vector).context("vector")?);
+        let mut this = Self::from_bytes(required(&r.bytes).context("bytes_")?);
         if this.len() < size {
             anyhow::bail!("'vector' has less than 'size' bits");
         }
@@ -97,7 +97,7 @@ impl ProtoFmt for bit_vec::BitVec {
     fn build(&self) -> Self::Proto {
         Self::Proto {
             size: Some(self.len() as u64),
-            vector: Some(self.to_bytes()),
+            bytes: Some(self.to_bytes()),
         }
     }
 }