From 42d241d1ea263019bf184b1951dc51cae91d0159 Mon Sep 17 00:00:00 2001 From: Quang Le Date: Thu, 27 Aug 2026 20:56:15 +0700 Subject: [PATCH] test(cluster): tighten the test suite and fix crate docs --- crates/cluster/src/definition.rs | 539 +++++++++++++++++++--------- crates/cluster/src/distvalidator.rs | 11 +- crates/cluster/src/helpers.rs | 166 ++++++++- crates/cluster/src/lib.rs | 13 +- crates/cluster/src/load.rs | 37 +- crates/cluster/src/lock.rs | 235 ++++++++---- crates/cluster/src/operator.rs | 6 +- crates/cluster/src/ssz.rs | 120 +++++++ crates/cluster/src/version.rs | 2 +- 9 files changed, 844 insertions(+), 285 deletions(-) diff --git a/crates/cluster/src/definition.rs b/crates/cluster/src/definition.rs index fe26f65d..a6098bd7 100644 --- a/crates/cluster/src/definition.rs +++ b/crates/cluster/src/definition.rs @@ -43,7 +43,7 @@ pub struct NodeIdx { pub share_idx: u64, } -/// Definition defines an intended charon cluster configuration excluding +/// Definition defines an intended cluster configuration excluding /// validators. #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct Definition { @@ -68,7 +68,7 @@ pub struct Definition { /// Cluster's 4 byte beacon chain fork version /// (network/chain identifier). pub fork_version: Vec, - /// Charon nodes in the cluster and their operators. + /// Nodes in the cluster and their operators. /// Max 256 operators. pub operators: Vec, /// Creator identifies the creator of a cluster definition. They may also be @@ -859,7 +859,7 @@ pub struct DefinitionV1x0or1 { /// Human-readable cosmetic identifier. Max 256 chars. #[serde(default)] pub name: String, - /// Charon nodes in the cluster and their operators. + /// Nodes in the cluster and their operators. /// Max 256 operators. #[serde(default)] #[serde_as(as = "DefaultOnNull")] @@ -978,7 +978,7 @@ pub struct DefinitionV1x2or3 { /// Human-readable cosmetic identifier. Max 256 chars. #[serde(default)] pub name: String, - /// Charon nodes in the cluster and their operators. + /// Nodes in the cluster and their operators. /// Max 256 operators. #[serde(default)] #[serde_as(as = "DefaultOnNull")] @@ -1100,7 +1100,7 @@ pub struct DefinitionV1x4 { /// Creator identifies the creator of a cluster definition. They may also be /// an operator. pub creator: Creator, - /// Operators define the charon nodes in the cluster and their operators. + /// Operators define the nodes in the cluster and their operators. /// Max 256 operators. #[serde(default)] #[serde_as(as = "DefaultOnNull")] @@ -1223,7 +1223,7 @@ pub struct DefinitionV1x5to7 { /// Creator identifies the creator of a cluster definition. They may also be /// an operator. pub creator: Creator, - /// Charon nodes in the cluster and their operators. + /// Nodes in the cluster and their operators. /// Max 256 operators. #[serde(default)] #[serde_as(as = "DefaultOnNull")] @@ -1322,16 +1322,16 @@ impl From for Definition { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct DefinitionV1x8 { /// Name is a human-readable cosmetic identifier. Max 256 chars. - // charon marshals `name` with `omitempty`, so it is absent when empty. + // Omitted from JSON when empty. #[serde(default)] pub name: String, /// Creator identifies the creator of a cluster definition. They may also be /// an operator. pub creator: Creator, - /// Operators define the charon nodes in the cluster and their operators. + /// Operators define the nodes in the cluster and their operators. /// Max 256 operators. - // charon marshals a nil `operators` slice as JSON `null`, and older tools - // may omit the key entirely, so accept both. + // A nil slice is written as JSON `null`, and older tools may omit the + // key entirely, so accept both. #[serde(default)] #[serde_as(as = "DefaultOnNull")] pub operators: Vec, @@ -1342,7 +1342,7 @@ pub struct DefinitionV1x8 { /// Timestamp is the human-readable timestamp of this definition. Max 32 /// chars. Note that this was added in v1.1.0, so may be empty for older /// versions. - // charon marshals `timestamp` with `omitempty`, so it is absent when empty. + // Omitted from JSON when empty. #[serde(default)] pub timestamp: String, /// NumValidators is the number of DVs to be created in the cluster lock @@ -1352,8 +1352,8 @@ pub struct DefinitionV1x8 { /// for number of nodes/peers. pub threshold: u64, /// ValidatorAddresses define addresses of each validator. - // charon marshals a nil `validators` slice as JSON `null`, and older tools - // may omit the key entirely, so accept both. + // A nil slice is written as JSON `null`, and older tools may omit the + // key entirely, so accept both. #[serde(default, rename = "validators")] #[serde_as(as = "DefaultOnNull")] pub validator_addresses: Vec, @@ -1437,16 +1437,16 @@ impl From for Definition { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct DefinitionV1x9 { /// Name is a human-readable cosmetic identifier. Max 256 chars. - // charon marshals `name` with `omitempty`, so it is absent when empty. + // Omitted from JSON when empty. #[serde(default)] pub name: String, /// Creator identifies the creator of a cluster definition. They may also be /// an operator. pub creator: Creator, - /// Operators define the charon nodes in the cluster and their operators. + /// Operators define the nodes in the cluster and their operators. /// Max 256 operators. - // charon marshals a nil `operators` slice as JSON `null`, and older tools - // may omit the key entirely, so accept both. + // A nil slice is written as JSON `null`, and older tools may omit the + // key entirely, so accept both. #[serde(default)] #[serde_as(as = "DefaultOnNull")] pub operators: Vec, @@ -1457,7 +1457,7 @@ pub struct DefinitionV1x9 { /// Timestamp is the human-readable timestamp of this definition. Max 32 /// chars. Note that this was added in v1.1.0, so may be empty for older /// versions. - // charon marshals `timestamp` with `omitempty`, so it is absent when empty. + // Omitted from JSON when empty. #[serde(default)] pub timestamp: String, /// NumValidators is the number of DVs to be created in the cluster lock @@ -1467,8 +1467,8 @@ pub struct DefinitionV1x9 { /// for number of nodes/peers. pub threshold: u64, /// ValidatorAddresses define addresses of each validator. - // charon marshals a nil `validators` slice as JSON `null`, and older tools - // may omit the key entirely, so accept both. + // A nil slice is written as JSON `null`, and older tools may omit the + // key entirely, so accept both. #[serde(default, rename = "validators")] #[serde_as(as = "DefaultOnNull")] pub validator_addresses: Vec, @@ -1561,10 +1561,10 @@ pub struct DefinitionV1x10 { /// Creator identifies the creator of a cluster definition. They may also be /// an operator. pub creator: Creator, - /// Charon nodes in the cluster and their operators. + /// Nodes in the cluster and their operators. /// Max 256 operators. - // charon marshals a nil `operators` slice as JSON `null`, and older tools - // may omit the key entirely, so accept both. + // A nil slice is written as JSON `null`, and older tools may omit the + // key entirely, so accept both. #[serde(default)] #[serde_as(as = "DefaultOnNull")] pub operators: Vec, @@ -1575,7 +1575,7 @@ pub struct DefinitionV1x10 { /// Human-readable timestamp of this definition. Max 32 /// chars. Note that this was added in v1.1.0, so may be empty for older /// versions. - // charon marshals `timestamp` with `omitempty`, so it is absent when empty. + // Omitted from JSON when empty. #[serde(default)] pub timestamp: String, /// Number of DVs to be created in the cluster lock @@ -1585,8 +1585,8 @@ pub struct DefinitionV1x10 { /// for number of nodes/peers. pub threshold: u64, /// Addresses of each validator. - // charon marshals a nil `validators` slice as JSON `null`, and older tools - // may omit the key entirely, so accept both. + // A nil slice is written as JSON `null`, and older tools may omit the + // key entirely, so accept both. #[serde(default, rename = "validators")] #[serde_as(as = "DefaultOnNull")] pub validator_addresses: Vec, @@ -1685,7 +1685,7 @@ fn repeat_v_addresses( // SSZ_MAX_VALIDATORS (the CompositeList[65536] bound already enforced on the // validator-addresses field) *before* the clone loop, so a value up to // u64::MAX cannot drive an unbounded allocation / OOM. This is defensive - // hardening consistent with the SSZ bound Charon enforces; it cannot reject + // hardening consistent with the SSZ bound itself; it cannot reject // any definition that could round-trip through SSZ hashing. if num_validators > SSZ_MAX_VALIDATORS as u64 { return Err(DefinitionError::NumValidatorsTooLarge { @@ -1705,36 +1705,44 @@ fn repeat_v_addresses( mod tests { use super::*; - fn parse_example_definition(json: &str) -> Definition { - let mut value: serde_json::Value = serde_json::from_str(json).unwrap(); - let version = value - .get("version") - .and_then(serde_json::Value::as_str) - .unwrap_or_default() - .to_string(); - - if version == V1_4 { - if value.get("fee_recipient_address").is_none() { - value["fee_recipient_address"] = serde_json::Value::String( - "0x0000000000000000000000000000000000000000".to_string(), - ); - } - if value.get("withdrawal_address").is_none() { - value["withdrawal_address"] = serde_json::Value::String( - "0x0000000000000000000000000000000000000000".to_string(), - ); - } - } + /// The version-specific parser and the dispatching `Deserialize` must + /// agree, and the fixture must report the expected counts and verify. + fn assert_versioned_definition( + json: &str, + version: &str, + operators: usize, + validators: usize, + ) -> Definition + where + V: serde::de::DeserializeOwned, + Definition: TryFrom, + >::Error: std::fmt::Debug, + { + let versioned = serde_json::from_str::(json) + .unwrap_or_else(|err| panic!("{version} parser must accept the fixture: {err}")); + let definition = serde_json::from_str::(json) + .unwrap_or_else(|err| panic!("dispatching parser must accept {version}: {err}")); - if version == V1_10 && value.get("compounding").is_none() { - value["compounding"] = serde_json::Value::Bool(false); - } + assert_eq!( + Definition::try_from(versioned).expect("convert versioned definition"), + definition, + "dispatching parser disagrees with the {version} parser" + ); - serde_json::from_value(value).unwrap() - } + assert_eq!(definition.version, version); + assert_eq!(definition.operators.len(), operators, "operators"); + assert_eq!( + definition.validator_addresses.len(), + validators, + "validator addresses" + ); + assert_eq!( + definition.num_validators, + u64::try_from(validators).expect("validator count fits in u64") + ); + definition.verify_hashes().expect("hashes must verify"); - async fn test_eth1_client() -> EthClient { - EthClient::new("http://127.0.0.1:8545").await.unwrap() + definition } fn legacy_definition_json(num_validators: u64) -> String { @@ -1792,6 +1800,171 @@ mod tests { ); } + const ZERO_ADDRESS: &str = "0x0000000000000000000000000000000000000000"; + + /// `Definition::new` always stamps [`CURRENT_VERSION`]; `opts` is the only + /// way to reach the version-gated validations. + fn downgrade_to_v1_7(definition: &mut Definition) -> Definition { + definition.version = V1_7.to_owned(); + definition.clone() + } + + fn downgrade_to_v1_9(definition: &mut Definition) -> Definition { + definition.version = V1_9.to_owned(); + definition.clone() + } + + /// A valid v1.10 argument set; each test overrides exactly one field. + struct NewArgs { + num_validators: u64, + fee_recipient_addresses: Vec, + withdrawal_addresses: Vec, + deposit_amounts: Vec, + target_gas_limit: u64, + compounding: bool, + opts: Vec Definition>, + } + + impl Default for NewArgs { + fn default() -> Self { + Self { + num_validators: 2, + fee_recipient_addresses: vec![ZERO_ADDRESS.to_owned(); 2], + withdrawal_addresses: vec![ZERO_ADDRESS.to_owned(); 2], + deposit_amounts: Vec::new(), + target_gas_limit: 30_000_000, + compounding: false, + opts: Vec::new(), + } + } + } + + impl NewArgs { + fn build(self) -> Result { + Definition::new( + "test".to_owned(), + self.num_validators, + 2, + self.fee_recipient_addresses, + self.withdrawal_addresses, + "0x00000000".to_owned(), + Creator::default(), + Vec::new(), + self.deposit_amounts, + String::new(), + self.target_gas_limit, + self.compounding, + self.opts, + ) + } + } + + #[test] + fn definition_new_builds_a_current_version_definition() { + let definition = NewArgs::default().build().unwrap(); + + assert_eq!(definition.version, CURRENT_VERSION); + assert_eq!(definition.num_validators, 2); + assert_eq!(definition.validator_addresses.len(), 2); + assert_eq!(definition.dkg_algorithm, DKG_ALGO); + assert_eq!(definition.config_hash.len(), 32); + assert_eq!(definition.definition_hash.len(), 32); + definition.verify_hashes().expect("hashes must verify"); + } + + #[test] + fn definition_new_rejects_short_fee_recipient_list() { + let result = NewArgs { + fee_recipient_addresses: vec![ZERO_ADDRESS.to_owned()], + ..Default::default() + } + .build(); + + assert!(matches!( + result, + Err(DefinitionError::InsufficientFeeRecipientAddresses) + )); + } + + #[test] + fn definition_new_rejects_short_withdrawal_list() { + let result = NewArgs { + withdrawal_addresses: vec![ZERO_ADDRESS.to_owned()], + ..Default::default() + } + .build(); + + assert!(matches!( + result, + Err(DefinitionError::InsufficientWithdrawalAddresses) + )); + } + + /// Partial deposits arrived in v1.8. + #[test] + fn definition_new_rejects_partial_deposits_before_v1_8() { + let result = NewArgs { + deposit_amounts: vec![16_000_000_000, 16_000_000_000], + target_gas_limit: 0, + opts: vec![downgrade_to_v1_7], + ..Default::default() + } + .build(); + + assert!(matches!( + result, + Err(DefinitionError::InvalidDepositAmounts) + )); + } + + /// Compounding arrived in v1.10. + #[test] + fn definition_new_rejects_compounding_before_v1_10() { + let result = NewArgs { + compounding: true, + target_gas_limit: 0, + opts: vec![downgrade_to_v1_9], + ..Default::default() + } + .build(); + + assert!(matches!(result, Err(DefinitionError::InvalidCompounding))); + } + + /// A custom target gas limit arrived in v1.10. + #[test] + fn definition_new_rejects_target_gas_limit_before_v1_10() { + let result = NewArgs { + opts: vec![downgrade_to_v1_9], + ..Default::default() + } + .build(); + + assert!(matches!( + result, + Err(DefinitionError::InvalidTargetGasLimit( + InvalidGasLimitError::VersionDoesNotSupportCustomTargetGasLimit + )) + )); + } + + /// v1.10 has no default, so leaving it unset is rejected too. + #[test] + fn definition_new_requires_a_target_gas_limit() { + let result = NewArgs { + target_gas_limit: 0, + ..Default::default() + } + .build(); + + assert!(matches!( + result, + Err(DefinitionError::InvalidTargetGasLimit( + InvalidGasLimitError::GasLimitNotSet + )) + )); + } + #[test] fn cluster_definition_v1_10_0_fields() { let definition = serde_json::from_str::(include_str!( @@ -1901,24 +2074,22 @@ mod tests { #[test] fn cluster_definition_v1_0_0() { - let json_str = include_str!("testdata/cluster_definition_v1_0_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_0_0.json"), + V1_0, + 2, + 2, + ); } #[test] fn cluster_definition_v1_1_0() { - let json_str = include_str!("testdata/cluster_definition_v1_1_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_1_0.json"), + V1_1, + 2, + 2, + ); } #[test] @@ -1940,118 +2111,113 @@ mod tests { #[test] fn cluster_definition_v1_2_0() { - let json_str = include_str!("testdata/cluster_definition_v1_2_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_2_0.json"), + V1_2, + 2, + 2, + ); } #[test] fn cluster_definition_v1_3_0() { - let json_str = include_str!("testdata/cluster_definition_v1_3_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_3_0.json"), + V1_3, + 2, + 2, + ); } - #[test] - fn cluster_definition_v1_3_0_unsigned_operators() { - for fixture in [ - include_str!("testdata/cluster_definition_v1_3_0_unsigned.json"), - include_str!("testdata/cluster_definition_v1_3_0_partial_sigs.json"), - ] { - let definition = serde_json::from_str::(fixture).unwrap(); + #[test_case::test_case(include_str!("testdata/cluster_definition_v1_3_0_unsigned.json") ; "unsigned")] + #[test_case::test_case(include_str!("testdata/cluster_definition_v1_3_0_partial_sigs.json") ; "partial sigs")] + fn cluster_definition_v1_3_0_unsigned_operators(fixture: &str) { + let definition = assert_versioned_definition::(fixture, V1_3, 2, 2); - assert!(definition.verify_hashes().is_ok()); - } + // Operator signatures are absent or only partly filled in; the + // config/definition hashes must still verify. + assert!( + definition + .operators + .iter() + .any(|o| o.config_signature.is_empty() || o.enr_signature.is_empty()), + "fixture should carry at least one unsigned operator" + ); } #[test] fn cluster_definition_v1_4_0() { - let json_str = include_str!("testdata/cluster_definition_v1_4_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_4_0.json"), + V1_4, + 2, + 2, + ); } #[test] fn cluster_definition_v1_5_0() { - let json_str = include_str!("testdata/cluster_definition_v1_5_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_5_0.json"), + V1_5, + 2, + 2, + ); } #[test] fn cluster_definition_v1_6_0() { - let json_str = include_str!("testdata/cluster_definition_v1_6_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_6_0.json"), + V1_6, + 2, + 2, + ); } #[test] fn cluster_definition_v1_7_0() { - let json_str = include_str!("testdata/cluster_definition_v1_7_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_7_0.json"), + V1_7, + 2, + 2, + ); } #[test] fn cluster_definition_v1_8_0() { - let json_str = include_str!("testdata/cluster_definition_v1_8_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_8_0.json"), + V1_8, + 2, + 2, + ); } #[test] fn cluster_definition_v1_9_0() { - let json_str = include_str!("testdata/cluster_definition_v1_9_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_9_0.json"), + V1_9, + 2, + 2, + ); } #[test] fn cluster_definition_v1_10_0() { - let json_str = include_str!("testdata/cluster_definition_v1_10_0.json"); - - let _ = serde_json::from_str::(json_str).unwrap(); - - let definition = serde_json::from_str::(json_str).unwrap(); - - assert!(definition.verify_hashes().is_ok()); + assert_versioned_definition::( + include_str!("testdata/cluster_definition_v1_10_0.json"), + V1_10, + 2, + 2, + ); } - /// charon marshals nil `operators`/`validators` slices as JSON `null` and - /// omits `name`/`timestamp` (both `omitempty`). Every supported definition - /// version must accept those shapes rather than fail deserialization. + /// Nil `operators`/`validators` slices are written as JSON `null` and + /// `name`/`timestamp` are omitted when empty. Every supported version must + /// accept those shapes rather than fail deserialization. #[test] fn definition_accepts_null_slices_and_absent_omitempty_fields() { for fixture in [ @@ -2062,9 +2228,8 @@ mod tests { let mut value: serde_json::Value = serde_json::from_str(fixture).unwrap(); let obj = value.as_object_mut().unwrap(); obj.insert("operators".to_owned(), serde_json::Value::Null); - // charon only marshals `validators: null` when there are no - // validators, so keep the count consistent with the deserializer's - // num_validators/validators cross-check. + // `validators: null` only appears when there are no validators, so + // keep the count consistent with the num_validators cross-check. obj.insert("validators".to_owned(), serde_json::Value::Null); obj.insert("num_validators".to_owned(), serde_json::json!(0)); obj.remove("name"); @@ -2129,6 +2294,32 @@ mod tests { assert!(result.is_err()); } + #[test_case::test_case(include_str!("examples/cluster-definition-000.json"), V1_3, 4, 1 ; "v1.3")] + #[test_case::test_case(include_str!("examples/cluster-definition-001.json"), V1_4, 2, 1 ; "v1.4")] + #[test_case::test_case(include_str!("examples/cluster-definition-002.json"), V1_4, 4, 1 ; "v1.4-2")] + #[test_case::test_case(include_str!("examples/cluster-definition-003.json"), V1_5, 2, 2 ; "v1.5")] + #[test_case::test_case(include_str!("examples/cluster-definition-004.json"), V1_7, 4, 2 ; "v1.7")] + #[test_case::test_case(include_str!("examples/cluster-definition-005.json"), V1_8, 4, 2 ; "v1.8")] + #[test_case::test_case(include_str!("examples/cluster-definition-006.json"), V1_10, 4, 2 ; "v1.10")] + fn example_definition_parses_as_is( + definition_json: &str, + version: &str, + operators: usize, + validators: usize, + ) { + let definition = serde_json::from_str::(definition_json) + .unwrap_or_else(|err| panic!("pristine {version} example must parse: {err}")); + + assert_eq!(definition.version, version); + assert_eq!(definition.operators.len(), operators); + assert_eq!(definition.validator_addresses.len(), validators); + assert_eq!( + definition.num_validators, + u64::try_from(validators).expect("validator count fits in u64") + ); + definition.verify_hashes().expect("hashes must verify"); + } + #[test_case::test_case(include_str!("examples/cluster-definition-000.json") ; "v1.3")] #[test_case::test_case(include_str!("examples/cluster-definition-001.json") ; "v1.4")] #[test_case::test_case(include_str!("examples/cluster-definition-002.json") ; "v1.4-2")] @@ -2138,8 +2329,8 @@ mod tests { #[test_case::test_case(include_str!("examples/cluster-definition-006.json") ; "v1.10")] #[tokio::test] async fn verify_signatures_examples(definition_json: &str) { - let definition = parse_example_definition(definition_json); - let eth1 = test_eth1_client().await; + let definition = serde_json::from_str::(definition_json).unwrap(); + let eth1 = EthClient::Noop; assert!(definition.verify_signatures(ð1).await.is_ok()); } @@ -2150,7 +2341,7 @@ mod tests { "testdata/cluster_definition_v1_2_0.json" )) .unwrap(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; assert!(definition.verify_signatures(ð1).await.is_ok()); } @@ -2162,7 +2353,7 @@ mod tests { )) .unwrap(); definition.operators[0].config_signature = vec![1]; - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = definition.verify_signatures(ð1).await; assert!(matches!( @@ -2173,10 +2364,12 @@ mod tests { #[tokio::test] async fn verify_signatures_empty_operator_enr_signature() { - let mut definition = - parse_example_definition(include_str!("examples/cluster-definition-001.json")); + let mut definition = serde_json::from_str::(include_str!( + "examples/cluster-definition-001.json" + )) + .unwrap(); definition.operators[0].enr_signature = Vec::new(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = definition.verify_signatures(ð1).await; assert!(matches!( @@ -2187,10 +2380,12 @@ mod tests { #[tokio::test] async fn verify_signatures_empty_operator_config_signature() { - let mut definition = - parse_example_definition(include_str!("examples/cluster-definition-001.json")); + let mut definition = serde_json::from_str::(include_str!( + "examples/cluster-definition-001.json" + )) + .unwrap(); definition.operators[0].config_signature = Vec::new(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = definition.verify_signatures(ð1).await; assert!(matches!( @@ -2201,10 +2396,12 @@ mod tests { #[tokio::test] async fn verify_signatures_mixed_signed_and_unsigned_operators() { - let mut definition = - parse_example_definition(include_str!("examples/cluster-definition-001.json")); + let mut definition = serde_json::from_str::(include_str!( + "examples/cluster-definition-001.json" + )) + .unwrap(); definition.operators[0] = Operator::default(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = definition.verify_signatures(ð1).await; assert!(matches!( @@ -2215,10 +2412,12 @@ mod tests { #[tokio::test] async fn verify_signatures_creator_missing_signature_while_operators_signed() { - let mut definition = - parse_example_definition(include_str!("examples/cluster-definition-001.json")); + let mut definition = serde_json::from_str::(include_str!( + "examples/cluster-definition-001.json" + )) + .unwrap(); definition.creator.config_signature = Vec::new(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = definition.verify_signatures(ð1).await; assert!(matches!( @@ -2229,21 +2428,25 @@ mod tests { #[tokio::test] async fn verify_signatures_unsigned_creator_and_operators() { - let mut definition = - parse_example_definition(include_str!("examples/cluster-definition-001.json")); + let mut definition = serde_json::from_str::(include_str!( + "examples/cluster-definition-001.json" + )) + .unwrap(); definition.creator = Creator::default(); definition.operators = vec![Operator::default(), Operator::default()]; - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; assert!(definition.verify_signatures(ð1).await.is_ok()); } #[tokio::test] async fn verify_signatures_v1_3_rejects_creator_signature() { - let mut definition = - parse_example_definition(include_str!("examples/cluster-definition-000.json")); + let mut definition = serde_json::from_str::(include_str!( + "examples/cluster-definition-000.json" + )) + .unwrap(); definition.creator.config_signature = vec![1]; - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = definition.verify_signatures(ð1).await; assert!(matches!( diff --git a/crates/cluster/src/distvalidator.rs b/crates/cluster/src/distvalidator.rs index 00c8cf03..112a50f7 100644 --- a/crates/cluster/src/distvalidator.rs +++ b/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")] pub pub_shares: Vec>, /// 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, @@ -370,9 +368,8 @@ impl From 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 = diff --git a/crates/cluster/src/helpers.rs b/crates/cluster/src/helpers.rs index 8458b227..375c5838 100644 --- a/crates/cluster/src/helpers.rs +++ b/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: -/// /// 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> = (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::>(); + 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 + ); + } } diff --git a/crates/cluster/src/lib.rs b/crates/cluster/src/lib.rs index 47c40dd1..b93c3f38 100644 --- a/crates/cluster/src/lib.rs +++ b/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 diff --git a/crates/cluster/src/load.rs b/crates/cluster/src/load.rs index 4be0c1f5..c523fe17 100644 --- a/crates/cluster/src/load.rs +++ b/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, 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, ) -> Result { @@ -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, ð1) .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, ð1) .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, ð1) .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, ð1) .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, ð1) .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, ð1) .await diff --git a/crates/cluster/src/lock.rs b/crates/cluster/src/lock.rs index 4ae9d5cc..7617a980 100644 --- a/crates/cluster/src/lock.rs +++ b/crates/cluster/src/lock.rs @@ -159,7 +159,7 @@ pub struct Lock { /// BLS aggregate signature of the lock hash /// signed by all the private key shares of all the distributed /// validators. It acts as an attestation by all the distributed - /// validators of the charon cluster they are part of. + /// validators of the cluster they are part of. pub signature_aggregate: Vec, /// Signatures of the lock hash for each operator @@ -291,8 +291,7 @@ impl Lock { if self.signature_aggregate.is_empty() { if matches!(self.version.as_str(), V1_0 | V1_1) { - // Earlier versions of `charon create cluster` didn't populate - // SignatureAggregate. + // Earlier versions didn't populate SignatureAggregate. return Ok(()); } @@ -375,11 +374,9 @@ impl Lock { let fee_recipient_addresses = self.fee_recipient_addresses(); for (validator_idx, validator) in self.distributed_validators.iter().enumerate() { - // In Go, `noRegistration` checks `len == 0` (empty slice), which catches fields - // missing from JSON. The zero Ethereum address ([0;20]) is a valid - // fee_recipient (len=20 in Go, passes the check). Only BLS - // signature and pubkey can never be legitimately all-zero for a - // real registration. + // A missing registration shows up as zero-valued fields. The zero + // Ethereum address is a legitimate fee_recipient, so only the BLS + // signature and pubkey can be treated as never-legitimately-zero. let no_registration = validator.builder_registration.signature == EMPTY_SIGNATURE || validator.builder_registration.message.pub_key == EMPTY_VALIDATOR_PUBKEY; @@ -457,7 +454,7 @@ pub struct LockV1x0or1 { /// BLS aggregate signature of the lock hash /// signed by all the private key shares of all the distributed /// validators. It acts as an attestation by all the distributed - /// validators of the charon cluster they are part of. + /// validators of the cluster they are part of. #[serde_as(as = "Base64")] pub signature_aggregate: Vec, } @@ -513,7 +510,7 @@ pub struct LockV1x2to5 { /// BLS aggregate signature of the lock hash /// signed by all the private key shares of all the distributed /// validators. It acts as an attestation by all the distributed - /// validators of the charon cluster they are part of. + /// validators of the cluster they are part of. #[serde_as(as = "HexBytes")] pub signature_aggregate: Vec, } @@ -569,7 +566,7 @@ pub struct LockV1x6 { /// BLS aggregate signature of the lock hash /// signed by all the private key shares of all the distributed /// validators. It acts as an attestation by all the distributed - /// validators of the charon cluster they are part of. + /// validators of the cluster they are part of. #[serde_as(as = "HexBytes")] pub signature_aggregate: Vec, } @@ -625,7 +622,7 @@ pub struct LockV1x7 { /// BLS aggregate signature of the lock hash /// signed by all the private key shares of all the distributed /// validators. It acts as an attestation by all the distributed - /// validators of the charon cluster they are part of. + /// validators of the cluster they are part of. #[serde_as(as = "HexBytes")] pub signature_aggregate: Vec, @@ -687,7 +684,7 @@ pub struct LockV1x8orLater { /// BLS aggregate signature of the lock hash /// signed by all the private key shares of all the distributed /// validators. It acts as an attestation by all the distributed - /// validators of the charon cluster they are part of. + /// validators of the cluster they are part of. #[serde_as(as = "HexBytes")] pub signature_aggregate: Vec, @@ -733,21 +730,52 @@ impl From for Lock { mod tests { use super::*; - async fn test_eth1_client() -> EthClient { - EthClient::new("http://127.0.0.1:8545").await.unwrap() + /// The version-specific parser and the dispatching `Deserialize` must + /// agree, and the fixture must report the expected counts and verify. + fn assert_versioned_lock( + json: &str, + version: &str, + operators: usize, + validators: usize, + node_signatures: usize, + ) where + V: serde::de::DeserializeOwned, + Lock: From, + { + let versioned = serde_json::from_str::(json) + .unwrap_or_else(|err| panic!("{version} parser must accept the fixture: {err}")); + let lock = serde_json::from_str::(json) + .unwrap_or_else(|err| panic!("dispatching parser must accept {version}: {err}")); + + assert_eq!( + Lock::from(versioned), + lock, + "dispatching parser disagrees with the {version} parser" + ); + + assert_eq!(lock.version, version); + assert_eq!(lock.operators.len(), operators, "operators"); + assert_eq!( + lock.distributed_validators.len(), + validators, + "distributed validators" + ); + assert_eq!( + lock.node_signatures.len(), + node_signatures, + "node signatures" + ); + lock.verify_hashes().expect("hashes must verify"); } - /// Mirrors charon's `TestExamples`: every checked-in example cluster file — - /// definitions *and* locks — must deserialize with the versioned parsers - /// and then pass `verify_hashes`/`verify_signatures`. These fixtures - /// exercise charon's `null`/omitempty/empty-hex shapes across the full - /// supported version range (v1.0 through v1.10). + /// Every checked-in example cluster file — definitions *and* locks — must + /// deserialize with the versioned parsers and then pass + /// `verify_hashes`/`verify_signatures`. These fixtures exercise the + /// `null`/omitempty/empty-hex shapes across v1.0 through v1.10. #[tokio::test] async fn parses_every_example_file() { - // charon's `TestExamples` passes a nil eth1 client; the noop client - // returned for an empty address is the equivalent (none of the example - // fixtures carry ERC-1271 smart-contract signatures). - let eth1 = EthClient::new("").await.unwrap(); + // No example fixture carries an ERC-1271 smart-contract signature. + let eth1 = EthClient::Noop; let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/src/examples"); @@ -792,6 +820,34 @@ mod tests { assert!(locks > 0, "no example locks found"); } + /// Every example lock must parse from its *pristine* bytes and verify. + /// Named per file so a regression points at one version rather than at the + /// directory walk above. + #[test_case::test_case(include_str!("examples/cluster-lock-000.json"), V1_1, 4, 5, 0 ; "v1.1")] + #[test_case::test_case(include_str!("examples/cluster-lock-001.json"), V1_1, 6, 1, 0 ; "v1.1-2")] + #[test_case::test_case(include_str!("examples/cluster-lock-002.json"), V1_2, 4, 1, 0 ; "v1.2")] + #[test_case::test_case(include_str!("examples/cluster-lock-003.json"), V1_7, 4, 3, 4 ; "v1.7")] + #[tokio::test] + async fn example_lock_parses_as_is( + lock_json: &str, + version: &str, + operators: usize, + validators: usize, + node_signatures: usize, + ) { + let lock = serde_json::from_str::(lock_json) + .unwrap_or_else(|err| panic!("pristine {version} example must parse: {err}")); + + assert_eq!(lock.version, version); + assert_eq!(lock.operators.len(), operators); + assert_eq!(lock.distributed_validators.len(), validators); + assert_eq!(lock.node_signatures.len(), node_signatures); + lock.verify_hashes().expect("hashes must verify"); + lock.verify_signatures(&EthClient::Noop) + .await + .expect("signatures must verify"); + } + #[test] fn lock_v1_10_0() { let lock = serde_json::from_str::(include_str!("testdata/cluster_lock_v1_10_0.json")) @@ -985,92 +1041,123 @@ mod tests { #[test] fn cluster_lock_v1_10_0() { - let json_str = include_str!("testdata/cluster_lock_v1_10_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(include_str!("testdata/cluster_lock_v1_10_0.json")) - .unwrap(); - - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_10_0.json"), + V1_10, + 2, + 2, + 2, + ); } #[test] fn cluster_lock_v1_9_0() { - let json_str = include_str!("testdata/cluster_lock_v1_9_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_9_0.json"), + V1_9, + 2, + 2, + 2, + ); } #[test] fn cluster_lock_v1_8_0() { - let json_str = include_str!("testdata/cluster_lock_v1_8_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_8_0.json"), + V1_8, + 2, + 2, + 2, + ); } #[test] fn cluster_lock_v1_7_0() { - let json_str = include_str!("testdata/cluster_lock_v1_7_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_7_0.json"), + V1_7, + 2, + 2, + 2, + ); } #[test] fn cluster_lock_v1_6_0() { - let json_str = include_str!("testdata/cluster_lock_v1_6_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_6_0.json"), + V1_6, + 2, + 2, + 0, + ); } #[test] fn cluster_lock_v1_5_0() { - let json_str = include_str!("testdata/cluster_lock_v1_5_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_5_0.json"), + V1_5, + 2, + 2, + 0, + ); } #[test] fn cluster_lock_v1_4_0() { - let json_str = include_str!("testdata/cluster_lock_v1_4_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_4_0.json"), + V1_4, + 2, + 2, + 0, + ); } #[test] fn cluster_lock_v1_3_0() { - let json_str = include_str!("testdata/cluster_lock_v1_3_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_3_0.json"), + V1_3, + 2, + 2, + 0, + ); } #[test] fn cluster_lock_v1_2_0() { - let json_str = include_str!("testdata/cluster_lock_v1_2_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_2_0.json"), + V1_2, + 2, + 2, + 0, + ); } #[test] fn cluster_lock_v1_1_0() { - let json_str = include_str!("testdata/cluster_lock_v1_1_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_1_0.json"), + V1_1, + 2, + 2, + 0, + ); } #[test] fn cluster_lock_v1_0_0() { - let json_str = include_str!("testdata/cluster_lock_v1_0_0.json"); - let _ = serde_json::from_str::(json_str).unwrap(); - let lock = serde_json::from_str::(json_str).unwrap(); - assert!(lock.verify_hashes().is_ok()); + assert_versioned_lock::( + include_str!("testdata/cluster_lock_v1_0_0.json"), + V1_0, + 2, + 2, + 0, + ); } #[test] @@ -1123,7 +1210,7 @@ mod tests { serde_json::from_str::(include_str!("testdata/cluster_lock_v1_0_0.json")) .unwrap(); lock.signature_aggregate = Vec::new(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; assert!(lock.verify_signatures(ð1).await.is_ok()); } @@ -1134,7 +1221,7 @@ mod tests { serde_json::from_str::(include_str!("testdata/cluster_lock_v1_2_0.json")) .unwrap(); lock.signature_aggregate = Vec::new(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = lock.verify_signatures(ð1).await; assert!(matches!( @@ -1147,7 +1234,7 @@ mod tests { async fn verify_signatures_v1_7_happy_path() { let lock = serde_json::from_str::(include_str!("examples/cluster-lock-003.json")).unwrap(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; assert!(lock.verify_signatures(ð1).await.is_ok()); } @@ -1157,7 +1244,7 @@ mod tests { let mut lock = serde_json::from_str::(include_str!("examples/cluster-lock-003.json")).unwrap(); lock.node_signatures[0] = lock.node_signatures[1].clone(); - let eth1 = test_eth1_client().await; + let eth1 = EthClient::Noop; let result = lock.verify_signatures(ð1).await; assert!(matches!( diff --git a/crates/cluster/src/operator.rs b/crates/cluster/src/operator.rs index 59b63ce4..d47b89b6 100644 --- a/crates/cluster/src/operator.rs +++ b/crates/cluster/src/operator.rs @@ -3,7 +3,7 @@ use pluto_ssz::serde_utils::HexBytes; use serde::{Deserialize, Serialize}; use serde_with::{DefaultOnNull, serde_as}; -/// Operator represents a charon node operator. +/// Operator represents a cluster node operator. #[serde_as] #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default)] #[serde(rename_all = "snake_case")] @@ -57,8 +57,8 @@ pub struct OperatorV1X2OrLater { enr: String, /// The config signature of the operator. /// - /// Charon's `ethHex` marshals a nil slice as `""` and tolerates `null` on - /// read, so accept both as an empty signature. + /// A nil slice is written as `""` and `null` is tolerated on read, so + /// accept both as an empty signature. #[serde(default)] #[serde_as(as = "DefaultOnNull")] config_signature: Vec, diff --git a/crates/cluster/src/ssz.rs b/crates/cluster/src/ssz.rs index 44126780..eed2ec09 100644 --- a/crates/cluster/src/ssz.rs +++ b/crates/cluster/src/ssz.rs @@ -966,3 +966,123 @@ pub(crate) fn hash_registration( Ok(()) } + +#[cfg(test)] +mod tests { + use test_case::test_case; + + use super::*; + + /// A version string no dispatch table knows about. + const UNSUPPORTED: &str = "v0.0.1"; + + fn v1_10_definition() -> Definition { + serde_json::from_str(include_str!("testdata/cluster_definition_v1_10_0.json")).unwrap() + } + + fn v1_10_lock() -> Lock { + serde_json::from_str(include_str!("testdata/cluster_lock_v1_10_0.json")).unwrap() + } + + #[test] + fn definition_hash_func_rejects_unsupported_version() { + assert!(matches!( + get_definition_hash_func::(UNSUPPORTED), + Err(SSZError::UnsupportedVersion(v)) if v == UNSUPPORTED + )); + } + + /// v1.0-v1.2 hash through `hash_lock_legacy`, which never consults this + /// table, so those versions are deliberately absent rather than mapped. + #[test_case(V1_0 ; "v1.0")] + #[test_case(V1_1 ; "v1.1")] + #[test_case(V1_2 ; "v1.2")] + #[test_case(UNSUPPORTED ; "unknown")] + fn validator_hash_func_rejects_unsupported_version(version: &str) { + assert!(matches!( + get_validator_hash_func::(version), + Err(SSZError::UnsupportedVersion(v)) if v == version + )); + } + + #[test] + fn deposit_data_hash_func_rejects_unsupported_version() { + assert!(matches!( + get_deposit_data_hash_func::(UNSUPPORTED), + Err(SSZError::UnsupportedVersion(v)) if v == UNSUPPORTED + )); + } + + #[test] + fn registration_hash_func_rejects_unsupported_version() { + assert!(matches!( + get_registration_hash_func::(UNSUPPORTED), + Err(SSZError::UnsupportedVersion(v)) if v == UNSUPPORTED + )); + } + + #[test] + fn hash_definition_rejects_unsupported_version() { + let mut definition = v1_10_definition(); + definition.version = UNSUPPORTED.to_owned(); + + assert!(matches!( + hash_definition(&definition, false), + Err(SSZError::UnsupportedVersion(v)) if v == UNSUPPORTED + )); + } + + #[test] + fn hash_lock_rejects_unsupported_version() { + let mut lock = v1_10_lock(); + lock.definition.version = UNSUPPORTED.to_owned(); + + assert!(matches!( + hash_lock(&lock), + Err(SSZError::UnsupportedVersion(v)) if v == UNSUPPORTED + )); + } + + /// A byte *list* past its SSZ limit is rejected, not truncated. + #[test] + fn hash_definition_rejects_oversized_enr() { + let mut definition = v1_10_definition(); + definition.operators[0].enr = "e".repeat(SSZ_MAX_ENR + 1); + + match hash_definition(&definition, false) { + Err(SSZError::IncorrectListSize { + namespace, + field, + actual, + expected, + }) => { + assert_eq!(namespace, "put_byte_list"); + assert_eq!(field, "ENR"); + assert_eq!(actual, SSZ_MAX_ENR + 1); + assert_eq!(expected, SSZ_MAX_ENR); + } + other => panic!("expected IncorrectListSize, got {other:?}"), + } + } + + /// `put_bytes_n` left-pads short input but never accepts over-long input. + #[test] + fn hash_lock_rejects_oversized_pub_key() { + let mut lock = v1_10_lock(); + lock.distributed_validators[0].pub_key = vec![0u8; SSZ_LEN_PUB_KEY + 1]; + + match hash_lock(&lock) { + Err(SSZError::IncorrectListSize { + namespace, + actual, + expected, + .. + }) => { + assert_eq!(namespace, "put_bytes_n"); + assert_eq!(actual, SSZ_LEN_PUB_KEY + 1); + assert_eq!(expected, SSZ_LEN_PUB_KEY); + } + other => panic!("expected IncorrectListSize, got {other:?}"), + } + } +} diff --git a/crates/cluster/src/version.rs b/crates/cluster/src/version.rs index 6e16617a..2a19a376 100644 --- a/crates/cluster/src/version.rs +++ b/crates/cluster/src/version.rs @@ -26,7 +26,7 @@ pub mod versions { pub use versions::*; -/// The current version of the charon cluster definition format. +/// The cluster definition format version pluto writes for new definitions. pub const CURRENT_VERSION: &str = V1_10; /// Default DKG algorithm. pub const DKG_ALGO: &str = "default";