refactor(signature_collector): Remove base_hash (#437)

Author: dknopik

anchor/signature_collector/src/lib.rs

@@ -103,7 +103,7 @@ impl SignatureCollectorManager {
         self: &Arc<Self>,
         metadata: SignatureMetadata,
         requester: SignatureRequester,
-        validator_signing_data: ValidatorSigningData,
+        signing_data: SigningData,
     ) -> Result<Arc<Signature>, CollectionError> {
         let Some(signer) = self.operator_id.get() else {
             return Err(CollectionError::OwnOperatorIdUnknown);
@@ -114,8 +114,8 @@ impl SignatureCollectorManager {
         debug!(
             ?metadata,
             ?requester,
-            root=?validator_signing_data.root,
-            index=?validator_signing_data.index,
+            root=?signing_data.root,
+            index=?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(
-                    validator_signing_data.root,
-                    validator_signing_data.index,
+                    signing_data.root,
+                    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 = ?validator_signing_data.root, "Signing...");
+                trace!(root = ?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) = &validator_signing_data.share {
-                    share.sign(validator_signing_data.root)
+                let partial_signature = if let Some(share) = &signing_data.share {
+                    share.sign(signing_data.root)
                 } else {
                     Signature::empty()
                 };
-                trace!(root = ?validator_signing_data.root, "Signed");
+                trace!(root = ?signing_data.root, "Signed");
 
                 let message = PartialSignatureMessage {
                     partial_signature,
-                    signing_root: validator_signing_data.root,
+                    signing_root: signing_data.root,
                     signer,
-                    validator_index: validator_signing_data.index,
+                    validator_index: signing_data.index,
                 };
                 match requester {
                     SignatureRequester::SingleValidator { pubkey } => {
@@ -178,13 +178,12 @@ 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((base_hash, metadata.committee_id))
+                            .entry((signing_data.root, metadata.committee_id))
                         {
                             Entry::Occupied(occupied) => occupied,
                             Entry::Vacant(vacant) => vacant.insert_entry(CommitteeSignatures {
@@ -225,7 +224,7 @@ impl SignatureCollectorManager {
 
                 // Finally, make the local instance aware of the partial signature, if it is a real
                 // signature.
-                if validator_signing_data.share.is_some() {
+                if signing_data.share.is_some() {
                     let _ = manager.receive_partial_signature(message, metadata.slot);
                 }
             },
@@ -392,15 +391,11 @@ 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 ValidatorSigningData {
+pub struct SigningData {
     pub root: Hash256,
     pub index: ValidatorIndex,
     pub share: Option<SecretKey>,

anchor/validator_store/src/lib.rs

@@ -24,17 +24,16 @@ use qbft_manager::{
 };
 use safe_arith::{ArithError, SafeArith};
 use signature_collector::{
-    CollectionError, SignatureCollectorManager, SignatureMetadata, SignatureRequester,
-    ValidatorSigningData,
+    CollectionError, SignatureCollectorManager, SignatureMetadata, SignatureRequester, SigningData,
 };
 use slashing_protection::{NotSafe, Safe, SlashingDatabase};
 use slot_clock::SlotClock;
 use ssv_types::{
     Cluster, CommitteeId, ValidatorIndex, ValidatorMetadata,
     consensus::{
         BEACON_ROLE_AGGREGATOR, BEACON_ROLE_PROPOSER, BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION,
-        BeaconVote, Contribution, ContributionWrapper, Contributions, QbftData,
-        ValidatorConsensusData, ValidatorDuty,
+        BeaconVote, Contribution, ContributionWrapper, Contributions, ValidatorConsensusData,
+        ValidatorDuty,
     },
     msgid::Role,
     partial_sig::PartialSignatureKind,
@@ -336,7 +335,6 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
         &self,
         signature_kind: PartialSignatureKind,
         role: Role,
-        base_hash: Option<Hash256>,
         validator: InitializedValidator,
         signing_root: Hash256,
         slot: Slot,
@@ -355,7 +353,7 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
             committee_id,
         };
 
-        let requester = if let Some(base_hash) = base_hash {
+        let requester = if role == Role::Committee {
             let metadata = self.get_slot_metadata(slot).await?;
             SignatureRequester::Committee {
                 num_signatures_to_collect: self
@@ -377,15 +375,14 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
                             .sum()
                     })
                     .unwrap_or_default(),
-                base_hash,
             }
         } else {
             SignatureRequester::SingleValidator {
                 pubkey: validator.metadata.public_key,
             }
         };
 
-        let signing_data = ValidatorSigningData {
+        let signing_data = SigningData {
             root: signing_root,
             index: validator
                 .metadata
@@ -517,7 +514,6 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
             .collect_signature(
                 PartialSignatureKind::PostConsensus,
                 Role::Proposer,
-                None,
                 self.validator(validator_pubkey)?,
                 signing_root,
                 header.slot,
@@ -620,7 +616,6 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
             .collect_signature(
                 PartialSignatureKind::VoluntaryExit,
                 Role::VoluntaryExit,
-                None,
                 self.validator(validator_pubkey)?,
                 signing_root,
                 slot,
@@ -840,7 +835,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
             self.collect_signature(
                 PartialSignatureKind::RandaoPartialSig,
                 Role::Proposer,
-                None,
                 self.validator(validator_pubkey)?,
                 signing_root,
                 self.slot_clock.now().ok_or(SpecificError::SlotClock)?,
@@ -978,7 +972,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                 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,7 +992,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                 .collect_signature(
                     PartialSignatureKind::PostConsensus,
                     Role::Committee,
-                    Some(data_hash),
                     validator,
                     signing_root,
                     attestation.data().slot,
@@ -1044,7 +1036,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                 .collect_signature(
                     PartialSignatureKind::ValidatorRegistration,
                     Role::ValidatorRegistration,
-                    None,
                     self.validator(validator_registration_data.pubkey)?,
                     signing_root,
                     validity_slot,
@@ -1158,7 +1149,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                 .collect_signature(
                     PartialSignatureKind::PostConsensus,
                     Role::Aggregator,
-                    None,
                     validator,
                     signing_root,
                     message.aggregate().get_slot(),
@@ -1200,7 +1190,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                     self.collect_signature(
                         PartialSignatureKind::SelectionProofPartialSig,
                         Role::Aggregator,
-                        None,
                         self.validator(validator_pubkey)?,
                         signing_root,
                         slot,
@@ -1245,7 +1234,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                     self.collect_signature(
                         PartialSignatureKind::ContributionProofs,
                         Role::SyncCommittee,
-                        None,
                         self.validator(*validator_pubkey)?,
                         signing_root,
                         slot,
@@ -1306,7 +1294,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                 .collect_signature(
                     PartialSignatureKind::PostConsensus,
                     Role::Committee,
-                    Some(data.hash()),
                     validator,
                     signing_root,
                     slot,
@@ -1454,7 +1441,6 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
             self.collect_signature(
                 PartialSignatureKind::PostConsensus,
                 Role::SyncCommittee,
-                None,
                 validator,
                 signing_root,
                 slot,