test(cluster): tighten the test suite and fix crate docs

crates/cluster/src/distvalidator.rs

@@ -327,15 +327,13 @@ pub struct DistValidatorV1x8orLater {
     /// Public shares are the public keys corresponding to each node's secret
     /// key share. It can be used to verify a partial signature created by
     /// any node in the cluster.
-    // charon marshals `public_shares` with `omitempty`, so it is absent when
-    // empty.
+    // Omitted from JSON when empty.
     #[serde(default, rename = "public_shares")]
     #[serde_as(as = "Vec<HexBytes>")]
     pub pub_shares: Vec<Vec<u8>>,
 
     /// Deposit data defines the deposit data to activate a validator.
-    // charon marshals `partial_deposit_data` with `omitempty`, so it is absent
-    // when empty.
+    // Omitted from JSON when empty.
     #[serde(default)]
     pub partial_deposit_data: Vec<DepositData>,
 
@@ -370,9 +368,8 @@ impl From<DistValidatorV1x8orLater> for DistValidator {
 mod tests {
     use super::*;
 
-    /// charon marshals `public_shares` and `partial_deposit_data` with
-    /// `omitempty`, so both keys are absent when empty. Deserialization must
-    /// tolerate that rather than require the keys.
+    /// `public_shares` and `partial_deposit_data` are `omitempty`: both keys
+    /// are absent when empty, and deserialization must tolerate that.
     #[test]
     fn dist_validator_v1x8_accepts_absent_omitempty_fields() {
         let lock: serde_json::Value =

crates/cluster/src/helpers.rs

@@ -197,8 +197,6 @@ pub fn sign_operator(
 }
 
 /// Returns minimum threshold required for a cluster with given nodes.
-/// This formula has been taken from: <https://github.com/ObolNetwork/charon/blob/a8fc3185bdda154412fe034dcd07c95baf5c1aaf/core/qbft/qbft.go#L63>
-///
 /// Computes ceil(2*nodes / 3) using integer arithmetic to avoid floating point
 /// conversions.
 pub fn threshold(nodes: u64) -> u64 {
@@ -229,9 +227,13 @@ pub fn agg_sign(
 #[cfg(test)]
 mod tests {
     use crate::test_cluster;
+    use pluto_crypto::tbls;
+    use pluto_eth2util::helpers::public_key_to_address;
     use pluto_ssz::serde_utils::HexBytes;
+    use rand::SeedableRng;
     use serde::{Deserialize, Serialize};
     use serde_with::serde_as;
+    use test_case::test_case;
 
     #[serde_as]
     #[derive(Serialize, Deserialize, Debug, PartialEq)]
@@ -357,4 +359,164 @@ mod tests {
             .unwrap_err();
         assert!(matches!(err, e if e.kind() == std::io::ErrorKind::NotFound));
     }
+
+    /// Pinned oracle table for `ceil(2n/3)`: the expected values are written
+    /// out rather than recomputed from the implementation under test.
+    #[test_case(1, 1)]
+    #[test_case(2, 2)]
+    #[test_case(3, 2)]
+    #[test_case(4, 3)]
+    #[test_case(5, 4)]
+    #[test_case(6, 4)]
+    #[test_case(7, 5)]
+    #[test_case(8, 6)]
+    #[test_case(9, 6)]
+    #[test_case(10, 7)]
+    #[test_case(11, 8)]
+    #[test_case(12, 8)]
+    #[test_case(13, 9)]
+    #[test_case(14, 10)]
+    #[test_case(15, 10)]
+    #[test_case(16, 11)]
+    #[test_case(17, 12)]
+    #[test_case(18, 12)]
+    #[test_case(19, 13)]
+    #[test_case(20, 14)]
+    #[test_case(21, 14)]
+    #[test_case(22, 15)]
+    fn threshold_matches_oracle_table(nodes: u64, expected: u64) {
+        assert_eq!(super::threshold(nodes), expected, "nodes = {nodes}");
+    }
+
+    /// `ceil(0/3)` is 0, not 1.
+    #[test]
+    fn threshold_zero_nodes() {
+        assert_eq!(super::threshold(0), 0);
+    }
+
+    /// Exact `ceil(2n/3)` well past the table: the smallest `t` with `3t >=
+    /// 2n`.
+    #[test]
+    fn threshold_is_exact_ceil_of_two_thirds() {
+        for nodes in 0..=4096u64 {
+            let t = super::threshold(nodes);
+            assert!(3 * t >= 2 * nodes, "threshold({nodes}) = {t} is below 2n/3");
+            assert!(
+                3 * t < 2 * nodes + 3,
+                "threshold({nodes}) = {t} overshoots ceil(2n/3)"
+            );
+        }
+    }
+
+    /// A fixed scalar, so the recovered address is stable across runs.
+    fn k1_secret(byte: u8) -> k256::SecretKey {
+        k256::SecretKey::from_slice(&[byte; 32]).expect("valid secp256k1 scalar")
+    }
+
+    #[test]
+    fn verify_sig_accepts_the_signing_address() {
+        let secret = k1_secret(1);
+        let digest = [7u8; 32];
+        let sig = pluto_k1util::sign(&secret, &digest).unwrap();
+        let addr = public_key_to_address(&secret.public_key());
+
+        assert!(super::verify_sig(&addr, &digest, &sig).unwrap());
+    }
+
+    /// The wrong signer is `Ok(false)`, not an error: recovery still succeeds.
+    #[test]
+    fn verify_sig_rejects_another_signers_address() {
+        let signer = k1_secret(1);
+        let other = k1_secret(2);
+        let digest = [7u8; 32];
+        let sig = pluto_k1util::sign(&signer, &digest).unwrap();
+        let other_addr = public_key_to_address(&other.public_key());
+
+        assert!(!super::verify_sig(&other_addr, &digest, &sig).unwrap());
+    }
+
+    /// A different digest recovers some other public key.
+    #[test]
+    fn verify_sig_rejects_a_different_digest() {
+        let secret = k1_secret(1);
+        let sig = pluto_k1util::sign(&secret, &[7u8; 32]).unwrap();
+        let addr = public_key_to_address(&secret.public_key());
+
+        assert!(!super::verify_sig(&addr, &[8u8; 32], &sig).unwrap());
+    }
+
+    #[test]
+    fn verify_sig_rejects_a_malformed_expected_address() {
+        let secret = k1_secret(1);
+        let digest = [7u8; 32];
+        let sig = pluto_k1util::sign(&secret, &digest).unwrap();
+
+        assert!(matches!(
+            super::verify_sig("not-an-address", &digest, &sig),
+            Err(super::VerifySigError::InvalidExpectedAddress(_))
+        ));
+    }
+
+    #[test]
+    fn verify_sig_surfaces_recovery_failure() {
+        let addr = public_key_to_address(&k1_secret(1).public_key());
+
+        // Too short to be a 65-byte recoverable signature.
+        assert!(matches!(
+            super::verify_sig(&addr, &[7u8; 32], &[0u8; 10]),
+            Err(super::VerifySigError::FailedToRecoverPubKey(_))
+        ));
+
+        // Right length, but an all-zero signature recovers no public key.
+        assert!(matches!(
+            super::verify_sig(&addr, &[7u8; 32], &[0u8; 65]),
+            Err(super::VerifySigError::FailedToRecoverPubKey(_))
+        ));
+    }
+
+    /// One signature per share, so the aggregate must verify against the
+    /// flattened list of share public keys.
+    #[test]
+    fn agg_sign_round_trips() {
+        let mut rng = rand::rngs::StdRng::seed_from_u64(603);
+        let secrets: Vec<Vec<pluto_crypto::types::PrivateKey>> = (0..3)
+            .map(|_| {
+                (0..2)
+                    .map(|_| tbls::generate_insecure_secret(&mut rng).unwrap())
+                    .collect()
+            })
+            .collect();
+        let public_keys = secrets
+            .iter()
+            .flatten()
+            .map(|s| tbls::secret_to_public_key(s).unwrap())
+            .collect::<Vec<_>>();
+        let message = b"cluster lock hash";
+
+        let aggregate = super::agg_sign(&secrets, message).unwrap();
+
+        tbls::verify_aggregate(&public_keys, aggregate, message)
+            .expect("aggregate must verify against every signing share");
+
+        assert!(tbls::verify_aggregate(&public_keys, aggregate, b"other message").is_err());
+
+        assert!(tbls::verify_aggregate(&public_keys[1..], aggregate, message).is_err());
+    }
+
+    /// Not an error: aggregating nothing yields the G2 compressed point at
+    /// infinity.
+    #[test]
+    fn agg_sign_of_no_shares_is_the_identity_signature() {
+        let mut identity = [0u8; 96];
+        identity[0] = 0xc0;
+
+        assert_eq!(
+            super::agg_sign(&[], b"cluster lock hash").unwrap(),
+            identity
+        );
+        assert_eq!(
+            super::agg_sign(&[vec![]], b"cluster lock hash").unwrap(),
+            identity
+        );
+    }
 }

crates/cluster/src/lib.rs

@@ -1,8 +1,11 @@
-//! # Charon Cluster
+//! # Pluto Cluster
 //!
-//! Cluster management and coordination for Charon distributed validator nodes.
-//! This crate handles the formation, management, and coordination of validator
-//! clusters in the Charon network.
+//! The [`Definition`](definition::Definition) a cluster's operators agree on,
+//! the [`Lock`](lock::Lock) that finalises it after distributed key generation,
+//! and the hashing, signing and verification tying the two together.
+//!
+//! Ported from charon's `cluster` package and wire-compatible with it across
+//! definition versions v1.0.0 to v1.10.0.
 
 /// `Definition` type representing the intended cluster configuration
 /// (operators, validators, fork version) with EIP-712 hashing and verification.
@@ -22,7 +25,7 @@ pub mod load;
 /// `Lock` type representing the finalized cluster configuration, including
 /// distributed validators and node signatures.
 pub mod lock;
-/// `Operator` type representing a charon node operator with Ethereum address,
+/// `Operator` type representing a cluster node operator with Ethereum address,
 /// ENR, and config/ENR signatures.
 pub mod operator;
 /// `BuilderRegistration` and `Registration` types for pre-generated signed

crates/cluster/src/load.rs

@@ -1,6 +1,4 @@
 //! Loading and verification of a cluster `Lock` from disk.
-//!
-//! Mirrors Charon's `cluster.LoadClusterLock` (`cluster/load.go`).
 
 use std::path::Path;
 
@@ -49,14 +47,12 @@ pub enum LoadError {
 /// [`Lock`], and verifies its hashes and signatures.
 ///
 /// When `no_verify` is set, verification failures are logged as warnings
-/// instead of being returned as errors (mirrors Charon's `--no-verify`): both
+/// instead of being returned as errors (the `--no-verify` flag): both
 /// [`Lock::verify_hashes`] and [`Lock::verify_signatures`] still run.
 ///
-/// `eth1` backs EIP-1271 smart-contract operator-signature verification. Pass a
-/// no-op client (from `EthClient::new("")`) to skip only the contract-based
-/// checks; BLS-aggregate and node signatures are still verified.
-///
-/// Mirrors Charon's `cluster.LoadClusterLock`.
+/// `eth1` backs EIP-1271 smart-contract operator-signature verification. Pass
+/// [`EthClient::Noop`] to skip only the contract-based checks; BLS-aggregate
+/// and node signatures are still verified.
 pub async fn load_cluster_lock(
     lock_file_path: impl AsRef<Path>,
     no_verify: bool,
@@ -99,8 +95,6 @@ pub async fn load_cluster_lock(
 /// execution-layer endpoint to inject. EIP-1271 smart-contract operator
 /// signatures are skipped; BLS-aggregate and node signatures are still
 /// verified.
-///
-/// Mirrors Charon's `cluster.LoadClusterLockAndVerify`.
 pub async fn load_cluster_lock_and_verify(
     lock_file_path: impl AsRef<Path>,
 ) -> Result<Lock, LoadError> {
@@ -119,13 +113,6 @@ mod tests {
 
     const LOCK_V1_10_0: &str = include_str!("testdata/cluster_lock_v1_10_0.json");
 
-    /// A no-op execution-layer client: BLS-aggregate and node signatures are
-    /// still verified, only EIP-1271 contract-based operator signatures are
-    /// skipped.
-    async fn noop_eth1() -> EthClient {
-        EthClient::new("").await.expect("noop eth1 client")
-    }
-
     /// Writes `contents` to a temporary file that `load_cluster_lock` can read
     /// by path.
     fn write_lock(contents: &str) -> NamedTempFile {
@@ -136,12 +123,12 @@ mod tests {
         file
     }
 
-    /// Ports Charon's `TestLoadClusterLock`: the lock is read and parsed and
-    /// its fields are populated (verification skipped via `no_verify`).
+    /// The lock is read and parsed and its fields are populated (verification
+    /// skipped via `no_verify`).
     #[tokio::test]
     async fn load_cluster_lock_reads_and_parses() {
         let file = write_lock(LOCK_V1_10_0);
-        let eth1 = noop_eth1().await;
+        let eth1 = EthClient::Noop;
 
         let lock = load_cluster_lock(file.path(), true, &eth1)
             .await
@@ -180,7 +167,7 @@ mod tests {
         let mut lock: Lock = serde_json::from_str(LOCK_V1_10_0).unwrap();
         lock.lock_hash[0] ^= 0xff;
         let file = write_lock(&serde_json::to_string(&lock).unwrap());
-        let eth1 = noop_eth1().await;
+        let eth1 = EthClient::Noop;
 
         let err = load_cluster_lock(file.path(), false, &eth1)
             .await
@@ -196,7 +183,7 @@ mod tests {
         let mut lock: Lock = serde_json::from_str(LOCK_V1_10_0).unwrap();
         lock.lock_hash[0] ^= 0xff;
         let file = write_lock(&serde_json::to_string(&lock).unwrap());
-        let eth1 = noop_eth1().await;
+        let eth1 = EthClient::Noop;
 
         let loaded = load_cluster_lock(file.path(), true, &eth1)
             .await
@@ -208,7 +195,7 @@ mod tests {
     /// A missing file surfaces a read error rather than a parse/verify error.
     #[tokio::test]
     async fn load_cluster_lock_missing_file() {
-        let eth1 = noop_eth1().await;
+        let eth1 = EthClient::Noop;
 
         let err = load_cluster_lock("/nonexistent/cluster-lock.json", false, &eth1)
             .await
@@ -221,7 +208,7 @@ mod tests {
     #[tokio::test]
     async fn load_cluster_lock_malformed_json() {
         let file = write_lock("{ not valid json");
-        let eth1 = noop_eth1().await;
+        let eth1 = EthClient::Noop;
 
         let err = load_cluster_lock(file.path(), false, &eth1)
             .await
@@ -236,7 +223,7 @@ mod tests {
     async fn load_cluster_lock_verifies_generated_lock() {
         let (lock, ..) = crate::test_cluster::new_for_test(1, 2, 3, 1);
         let file = write_lock(&serde_json::to_string(&lock).expect("serialize generated lock"));
-        let eth1 = noop_eth1().await;
+        let eth1 = EthClient::Noop;
 
         let loaded = load_cluster_lock(file.path(), false, &eth1)
             .await