diff --git a/node/Cargo.lock b/node/Cargo.lock index dcdeafb9..eae1555f 100644 --- a/node/Cargo.lock +++ b/node/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -3499,7 +3499,7 @@ dependencies = [ [[package]] name = "tester" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "clap", @@ -4300,7 +4300,7 @@ dependencies = [ [[package]] name = "zksync_concurrency" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "assert_matches", @@ -4318,7 +4318,7 @@ dependencies = [ [[package]] name = "zksync_consensus_bft" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "assert_matches", @@ -4342,7 +4342,7 @@ dependencies = [ [[package]] name = "zksync_consensus_crypto" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "blst", @@ -4362,7 +4362,7 @@ dependencies = [ [[package]] name = "zksync_consensus_executor" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "async-trait", @@ -4384,7 +4384,7 @@ dependencies = [ [[package]] name = "zksync_consensus_network" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "assert_matches", @@ -4422,7 +4422,7 @@ dependencies = [ [[package]] name = "zksync_consensus_roles" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "assert_matches", @@ -4443,7 +4443,7 @@ dependencies = [ [[package]] name = "zksync_consensus_storage" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "assert_matches", @@ -4465,7 +4465,7 @@ dependencies = [ [[package]] name = "zksync_consensus_tools" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "async-trait", @@ -4500,7 +4500,7 @@ dependencies = [ [[package]] name = "zksync_consensus_utils" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "rand", @@ -4510,7 +4510,7 @@ dependencies = [ [[package]] name = "zksync_protobuf" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "bit-vec", @@ -4532,7 +4532,7 @@ dependencies = [ [[package]] name = "zksync_protobuf_build" -version = "0.6.0" +version = "0.7.0" dependencies = [ "anyhow", "heck", diff --git a/node/libs/crypto/src/bls12_381/mod.rs b/node/libs/crypto/src/bls12_381/mod.rs index a84dacfd..22c8a781 100644 --- a/node/libs/crypto/src/bls12_381/mod.rs +++ b/node/libs/crypto/src/bls12_381/mod.rs @@ -174,6 +174,7 @@ impl std::hash::Hash for Signature { } /// Type safety wrapper around a `blst` aggregate signature +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Debug)] pub struct AggregateSignature(bls::AggregateSignature); diff --git a/node/libs/crypto/src/keccak256/mod.rs b/node/libs/crypto/src/keccak256/mod.rs index ce7bd753..be860b29 100644 --- a/node/libs/crypto/src/keccak256/mod.rs +++ b/node/libs/crypto/src/keccak256/mod.rs @@ -7,6 +7,7 @@ mod test; pub mod testonly; /// Keccak256 hash. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Keccak256(pub(crate) [u8; 32]); diff --git a/node/libs/roles/src/validator/keys/aggregate_signature.rs b/node/libs/roles/src/validator/keys/aggregate_signature.rs index c7c2e0f1..622b89a5 100644 --- a/node/libs/roles/src/validator/keys/aggregate_signature.rs +++ b/node/libs/roles/src/validator/keys/aggregate_signature.rs @@ -5,6 +5,7 @@ use zksync_consensus_crypto::{bls12_381, ByteFmt, Text, TextFmt}; use zksync_consensus_utils::enum_util::Variant; /// An aggregate signature from a validator. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Default)] pub struct AggregateSignature(pub(crate) bls12_381::AggregateSignature); diff --git a/node/libs/roles/src/validator/messages/block.rs b/node/libs/roles/src/validator/messages/block.rs index 2c0f74af..1f5ebf8b 100644 --- a/node/libs/roles/src/validator/messages/block.rs +++ b/node/libs/roles/src/validator/messages/block.rs @@ -39,6 +39,7 @@ impl Payload { } /// Hash of the Payload. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct PayloadHash(pub(crate) Keccak256); @@ -62,6 +63,7 @@ impl fmt::Debug for PayloadHash { } /// Sequential number of the block. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct BlockNumber(pub u64); @@ -91,6 +93,7 @@ impl fmt::Display for BlockNumber { } /// A block header. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct BlockHeader { /// Number of the block. diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index 1ff67b4d..0d9e476a 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -92,6 +92,7 @@ impl Variant for ReplicaTimeout { } /// View specification. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct View { /// Genesis of the chain this view belongs to. @@ -125,6 +126,7 @@ impl View { } /// A struct that represents a view number. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct ViewNumber(pub u64); @@ -148,6 +150,7 @@ impl fmt::Display for ViewNumber { /// Struct that represents a bit map of validators. We use it to compactly store /// which validators signed a given message. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Signers(pub BitVec); diff --git a/node/libs/roles/src/validator/messages/genesis.rs b/node/libs/roles/src/validator/messages/genesis.rs index 19d6daeb..ad6626ad 100644 --- a/node/libs/roles/src/validator/messages/genesis.rs +++ b/node/libs/roles/src/validator/messages/genesis.rs @@ -33,6 +33,7 @@ impl GenesisRaw { } /// Hash of the genesis specification. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct GenesisHash(pub(crate) Keccak256); diff --git a/node/libs/roles/src/validator/messages/replica_commit.rs b/node/libs/roles/src/validator/messages/replica_commit.rs index 13563a84..6ea9fc40 100644 --- a/node/libs/roles/src/validator/messages/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/replica_commit.rs @@ -2,6 +2,7 @@ use super::{BlockHeader, Genesis, Signed, Signers, View}; use crate::validator; /// A commit message from a replica. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct ReplicaCommit { /// View of this message. @@ -31,6 +32,7 @@ pub enum ReplicaCommitVerifyError { /// A Commit Quorum Certificate. It is an aggregate of signed ReplicaCommit messages. /// The Commit Quorum Certificate is over identical messages, so we only need one message. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct CommitQC { /// The ReplicaCommit message that the QC is for. diff --git a/node/libs/roles/src/validator/messages/replica_timeout.rs b/node/libs/roles/src/validator/messages/replica_timeout.rs index 0a9edc3f..87956948 100644 --- a/node/libs/roles/src/validator/messages/replica_timeout.rs +++ b/node/libs/roles/src/validator/messages/replica_timeout.rs @@ -6,6 +6,7 @@ use crate::validator; use std::collections::{BTreeMap, HashMap}; /// A timeout message from a replica. +/// WARNING: any change to this struct may invalidate preexisting signatures. See `TimeoutQC` docs. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct ReplicaTimeout { /// View of this message. @@ -53,7 +54,20 @@ pub enum ReplicaTimeoutVerifyError { /// A quorum certificate of ReplicaTimeout messages. Since not all ReplicaTimeout messages are /// identical (they have different high blocks and high QCs), we need to keep the ReplicaTimeout -/// messages in a map. We can still aggregate the signatures though. +/// messages in a map. We can still aggregate the signatures though. +/// +/// WARNING: `TimeoutQC` message contains a map indexed by `ReplicaTimeout` messages. +/// Therefore, `Ord` implementation of `ReplicaTimeout` affects the unique encoding of the `TimeoutQC` message. +/// This `Ord` implementation is the derived lexicographic ordering of the fields of +/// `ReplicaTimeout` (and transitively on types of the fields AS WELL). +/// As a result ANY change to type of ANY transitive field of ReplicaTimeout struct may invalidate +/// preexisting signatures of `TimeoutQC` messages (or messages containing `TimeoutQC`). +/// +/// The proper fix would be to add support for unordered collections on the protobuf level +/// (for example, by adding a custom option "unordered" for repeated fields, which would make the encoder +/// sort the encoded elements before encoding the whole message). However the current protobuf +/// encoding of TimeoutQC keeps the keys and values in separate repeated fields. Until this is +/// fixed, ANY change to the above mentioned types may be backward incompatible. #[derive(Clone, Debug, PartialEq, Eq)] pub struct TimeoutQC { /// View of this QC.