From 9bd7556e22f4ed50e16011a7241e8a3a134e5932 Mon Sep 17 00:00:00 2001 From: Daniel Knopik Date: Mon, 11 Aug 2025 12:08:45 +0200 Subject: [PATCH 1/2] Revert "refactor(signature_collector): Remove `base_hash` (#437)" This reverts commit 22df8e1bd1c0957a1ea26f2acc1902a3f1308a99. --- anchor/signature_collector/src/lib.rs | 33 +++++++++++++++------------ anchor/validator_store/src/lib.rs | 24 +++++++++++++++---- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/anchor/signature_collector/src/lib.rs b/anchor/signature_collector/src/lib.rs index 24de51699..9cd2aa715 100644 --- a/anchor/signature_collector/src/lib.rs +++ b/anchor/signature_collector/src/lib.rs @@ -103,7 +103,7 @@ impl SignatureCollectorManager { self: &Arc, metadata: SignatureMetadata, requester: SignatureRequester, - signing_data: SigningData, + validator_signing_data: ValidatorSigningData, ) -> Result, CollectionError> { let Some(signer) = self.operator_id.get() else { return Err(CollectionError::OwnOperatorIdUnknown); @@ -114,8 +114,8 @@ impl SignatureCollectorManager { debug!( ?metadata, ?requester, - root=?signing_data.root, - index=?signing_data.index, + root=?validator_signing_data.root, + index=?validator_signing_data.index, "sign_and_collect called", ); @@ -125,8 +125,8 @@ impl SignatureCollectorManager { self.processor.permitless.send_immediate( move |drop_on_finish| { let sender = manager.get_or_spawn( - signing_data.root, - signing_data.index, + validator_signing_data.root, + validator_signing_data.index, cloned_metadata.slot, ); let _ = sender.send(CollectorMessage { @@ -144,21 +144,21 @@ impl SignatureCollectorManager { let manager = self.clone(); self.processor.urgent_consensus.send_blocking( move || { - trace!(root = ?signing_data.root, "Signing..."); + trace!(root = ?validator_signing_data.root, "Signing..."); // If we have no share, we can not actually sign the message, because we are running // in impostor mode. - let partial_signature = if let Some(share) = &signing_data.share { - share.sign(signing_data.root) + let partial_signature = if let Some(share) = &validator_signing_data.share { + share.sign(validator_signing_data.root) } else { Signature::empty() }; - trace!(root = ?signing_data.root, "Signed"); + trace!(root = ?validator_signing_data.root, "Signed"); let message = PartialSignatureMessage { partial_signature, - signing_root: signing_data.root, + signing_root: validator_signing_data.root, signer, - validator_index: signing_data.index, + validator_index: validator_signing_data.index, }; match requester { SignatureRequester::SingleValidator { pubkey } => { @@ -178,12 +178,13 @@ impl SignatureCollectorManager { } SignatureRequester::Committee { num_signatures_to_collect, + base_hash, } => { // We have to collect all signatures from the given validators. // To check this create or get an entry from the `committee_signatures` map. let mut entry = match manager .committee_signatures - .entry((signing_data.root, metadata.committee_id)) + .entry((base_hash, metadata.committee_id)) { Entry::Occupied(occupied) => occupied, Entry::Vacant(vacant) => vacant.insert_entry(CommitteeSignatures { @@ -224,7 +225,7 @@ impl SignatureCollectorManager { // Finally, make the local instance aware of the partial signature, if it is a real // signature. - if signing_data.share.is_some() { + if validator_signing_data.share.is_some() { let _ = manager.receive_partial_signature(message, metadata.slot); } }, @@ -391,11 +392,15 @@ pub enum SignatureRequester { Committee { /// The number of signatures we have to wait for. num_signatures_to_collect: usize, + /// A hash that identifies what we are signing. Note that the actual signing root might be + /// different - for example, because we are in different beacon chain attestation + /// committees, and the attestation data differs therefore. + base_hash: Hash256, }, } #[derive(Clone)] -pub struct SigningData { +pub struct ValidatorSigningData { pub root: Hash256, pub index: ValidatorIndex, pub share: Option, diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index fbd683811..06044dc95 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -24,7 +24,8 @@ use qbft_manager::{ }; use safe_arith::{ArithError, SafeArith}; use signature_collector::{ - CollectionError, SignatureCollectorManager, SignatureMetadata, SignatureRequester, SigningData, + CollectionError, SignatureCollectorManager, SignatureMetadata, SignatureRequester, + ValidatorSigningData, }; use slashing_protection::{NotSafe, Safe, SlashingDatabase}; use slot_clock::SlotClock; @@ -32,8 +33,8 @@ use ssv_types::{ Cluster, CommitteeId, ValidatorIndex, ValidatorMetadata, consensus::{ BEACON_ROLE_AGGREGATOR, BEACON_ROLE_PROPOSER, BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION, - BeaconVote, Contribution, ContributionWrapper, Contributions, ValidatorConsensusData, - ValidatorDuty, + BeaconVote, Contribution, ContributionWrapper, Contributions, QbftData, + ValidatorConsensusData, ValidatorDuty, }, msgid::Role, partial_sig::PartialSignatureKind, @@ -342,6 +343,7 @@ impl AnchorValidatorStore { &self, signature_kind: PartialSignatureKind, role: Role, + base_hash: Option, validator: InitializedValidator, signing_root: Hash256, slot: Slot, @@ -360,7 +362,7 @@ impl AnchorValidatorStore { committee_id, }; - let requester = if role == Role::Committee { + let requester = if let Some(base_hash) = base_hash { let metadata = self.get_slot_metadata(slot).await?; SignatureRequester::Committee { num_signatures_to_collect: self @@ -382,6 +384,7 @@ impl AnchorValidatorStore { .sum() }) .unwrap_or_default(), + base_hash, } } else { SignatureRequester::SingleValidator { @@ -389,7 +392,7 @@ impl AnchorValidatorStore { } }; - let signing_data = SigningData { + let signing_data = ValidatorSigningData { root: signing_root, index: validator .metadata @@ -521,6 +524,7 @@ impl AnchorValidatorStore { .collect_signature( PartialSignatureKind::PostConsensus, Role::Proposer, + None, self.validator(validator_pubkey)?, signing_root, header.slot, @@ -623,6 +627,7 @@ impl AnchorValidatorStore { .collect_signature( PartialSignatureKind::VoluntaryExit, Role::VoluntaryExit, + None, self.validator(validator_pubkey)?, signing_root, slot, @@ -842,6 +847,7 @@ impl ValidatorStore for AnchorValidatorStore { self.collect_signature( PartialSignatureKind::RandaoPartialSig, Role::Proposer, + None, self.validator(validator_pubkey)?, signing_root, self.slot_clock.now().ok_or(SpecificError::SlotClock)?, @@ -979,6 +985,7 @@ impl ValidatorStore for AnchorValidatorStore { Completed::TimedOut => return Err(Error::SpecificError(SpecificError::Timeout)), Completed::Success(data) => data, }; + let data_hash = data.hash(); attestation.data_mut().beacon_block_root = data.block_root; attestation.data_mut().source = data.source; attestation.data_mut().target = data.target; @@ -999,6 +1006,7 @@ impl ValidatorStore for AnchorValidatorStore { .collect_signature( PartialSignatureKind::PostConsensus, Role::Committee, + Some(data_hash), validator, signing_root, attestation.data().slot, @@ -1043,6 +1051,7 @@ impl ValidatorStore for AnchorValidatorStore { .collect_signature( PartialSignatureKind::ValidatorRegistration, Role::ValidatorRegistration, + None, self.validator(validator_registration_data.pubkey)?, signing_root, validity_slot, @@ -1156,6 +1165,7 @@ impl ValidatorStore for AnchorValidatorStore { .collect_signature( PartialSignatureKind::PostConsensus, Role::Aggregator, + None, validator, signing_root, message.aggregate().get_slot(), @@ -1197,6 +1207,7 @@ impl ValidatorStore for AnchorValidatorStore { self.collect_signature( PartialSignatureKind::SelectionProofPartialSig, Role::Aggregator, + None, self.validator(validator_pubkey)?, signing_root, slot, @@ -1241,6 +1252,7 @@ impl ValidatorStore for AnchorValidatorStore { self.collect_signature( PartialSignatureKind::ContributionProofs, Role::SyncCommittee, + None, self.validator(*validator_pubkey)?, signing_root, slot, @@ -1301,6 +1313,7 @@ impl ValidatorStore for AnchorValidatorStore { .collect_signature( PartialSignatureKind::PostConsensus, Role::Committee, + Some(data.hash()), validator, signing_root, slot, @@ -1448,6 +1461,7 @@ impl ValidatorStore for AnchorValidatorStore { self.collect_signature( PartialSignatureKind::PostConsensus, Role::SyncCommittee, + None, validator, signing_root, slot, From 5489b0bd4c4e6f7cfc33033f95cf13cd58c00645 Mon Sep 17 00:00:00 2001 From: Daniel Knopik Date: Mon, 11 Aug 2025 12:28:15 +0200 Subject: [PATCH 2/2] Change misleading comment --- anchor/signature_collector/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/anchor/signature_collector/src/lib.rs b/anchor/signature_collector/src/lib.rs index 9cd2aa715..830982924 100644 --- a/anchor/signature_collector/src/lib.rs +++ b/anchor/signature_collector/src/lib.rs @@ -392,9 +392,10 @@ pub enum SignatureRequester { Committee { /// The number of signatures we have to wait for. num_signatures_to_collect: usize, - /// A hash that identifies what we are signing. Note that the actual signing root might be - /// different - for example, because we are in different beacon chain attestation - /// committees, and the attestation data differs therefore. + /// A hash that identifies what we are signing. We wait with sending the message until we + /// have created enough signatures with this `base_hash`. We need this to differentiate + /// "groups" of signatures. We cannot use the signing root, as we need to group signatures + /// with differing signing roots. base_hash: Hash256, }, }