De-dup attestation constructor logic

sigp · Jun 17, 2024 · eaed292 · eaed292
1 parent 3ac3ddb
commit eaed292
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 56 deletions.
diff --git a/consensus/types/src/aggregate_and_proof.rs b/consensus/types/src/aggregate_and_proof.rs
@@ -4,6 +4,7 @@ use super::{
     SignedRoot,
 };
 use crate::test_utils::TestRandom;
+use crate::Attestation;
 use serde::{Deserialize, Serialize};
 use ssz_derive::{Decode, Encode};
 use superstruct::superstruct;
@@ -88,28 +89,39 @@ impl<E: EthSpec> AggregateAndProof<E> {
         genesis_validators_root: Hash256,
         spec: &ChainSpec,
     ) -> Self {
-        let selection_proof = selection_proof
-            .unwrap_or_else(|| {
-                SelectionProof::new::<E>(
-                    aggregate.data().slot,
-                    secret_key,
-                    fork,
-                    genesis_validators_root,
-                    spec,
-                )
-            })
-            .into();
+        let selection_proof = selection_proof.unwrap_or_else(|| {
+            SelectionProof::new::<E>(
+                aggregate.data().slot,
+                secret_key,
+                fork,
+                genesis_validators_root,
+                spec,
+            )
+        });
 
+        Self::from_attestation(
+            aggregator_index,
+            aggregate.clone_as_attestation(),
+            selection_proof,
+        )
+    }
+
+    /// Produces a new `AggregateAndProof` given a `selection_proof`
+    pub fn from_attestation(
+        aggregator_index: u64,
+        aggregate: Attestation<E>,
+        selection_proof: SelectionProof,
+    ) -> Self {
         match aggregate {
-            AttestationRef::Base(attestation) => Self::Base(AggregateAndProofBase {
+            Attestation::Base(aggregate) => Self::Base(AggregateAndProofBase {
                 aggregator_index,
-                aggregate: attestation.clone(),
-                selection_proof,
+                aggregate,
+                selection_proof: selection_proof.into(),
             }),
-            AttestationRef::Electra(attestation) => Self::Electra(AggregateAndProofElectra {
+            Attestation::Electra(aggregate) => Self::Electra(AggregateAndProofElectra {
                 aggregator_index,
-                aggregate: attestation.clone(),
-                selection_proof,
+                aggregate,
+                selection_proof: selection_proof.into(),
             }),
         }
     }

diff --git a/consensus/types/src/signed_aggregate_and_proof.rs b/consensus/types/src/signed_aggregate_and_proof.rs
@@ -83,17 +83,19 @@ impl<E: EthSpec> SignedAggregateAndProof<E> {
         );
         let signing_message = message.signing_root(domain);
 
-        match message {
+        Self::from_aggregate_and_proof(message, secret_key.sign(signing_message))
+    }
+
+    /// Produces a new `SignedAggregateAndProof` given a `signature` of `aggregate`
+    pub fn from_aggregate_and_proof(aggregate: AggregateAndProof<E>, signature: Signature) -> Self {
+        match aggregate {
             AggregateAndProof::Base(message) => {
-                SignedAggregateAndProof::Base(SignedAggregateAndProofBase {
-                    message,
-                    signature: secret_key.sign(signing_message),
-                })
+                SignedAggregateAndProof::Base(SignedAggregateAndProofBase { message, signature })
             }
             AggregateAndProof::Electra(message) => {
                 SignedAggregateAndProof::Electra(SignedAggregateAndProofElectra {
                     message,
-                    signature: secret_key.sign(signing_message),
+                    signature,
                 })
             }
         }

diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs
@@ -18,16 +18,12 @@ use std::sync::Arc;
 use task_executor::TaskExecutor;
 use types::{
     attestation::Error as AttestationError, graffiti::GraffitiString, AbstractExecPayload, Address,
-    Attestation, BeaconBlock, BlindedPayload, ChainSpec, ContributionAndProof, Domain, Epoch,
-    EthSpec, Fork, ForkName, Graffiti, Hash256, PublicKeyBytes, SelectionProof, Signature,
-    SignedBeaconBlock, SignedContributionAndProof, SignedRoot, SignedValidatorRegistrationData,
-    SignedVoluntaryExit, Slot, SyncAggregatorSelectionData, SyncCommitteeContribution,
-    SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, ValidatorRegistrationData,
-    VoluntaryExit,
-};
-use types::{
-    AggregateAndProof, AggregateAndProofBase, AggregateAndProofElectra, SignedAggregateAndProof,
-    SignedAggregateAndProofBase, SignedAggregateAndProofElectra,
+    AggregateAndProof, Attestation, BeaconBlock, BlindedPayload, ChainSpec, ContributionAndProof,
+    Domain, Epoch, EthSpec, Fork, ForkName, Graffiti, Hash256, PublicKeyBytes, SelectionProof,
+    Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedContributionAndProof, SignedRoot,
+    SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, SyncAggregatorSelectionData,
+    SyncCommitteeContribution, SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId,
+    ValidatorRegistrationData, VoluntaryExit,
 };
 
 pub use crate::doppelganger_service::DoppelgangerStatus;
@@ -805,18 +801,8 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore<T, E> {
         let signing_epoch = aggregate.data().target.epoch;
         let signing_context = self.signing_context(Domain::AggregateAndProof, signing_epoch);
 
-        let message = match aggregate {
-            Attestation::Base(att) => AggregateAndProof::Base(AggregateAndProofBase {
-                aggregator_index,
-                aggregate: att,
-                selection_proof: selection_proof.into(),
-            }),
-            Attestation::Electra(att) => AggregateAndProof::Electra(AggregateAndProofElectra {
-                aggregator_index,
-                aggregate: att,
-                selection_proof: selection_proof.into(),
-            }),
-        };
+        let message =
+            AggregateAndProof::from_attestation(aggregator_index, aggregate, selection_proof);
 
         let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?;
         let signature = signing_method
@@ -830,17 +816,9 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore<T, E> {
 
         metrics::inc_counter_vec(&metrics::SIGNED_AGGREGATES_TOTAL, &[metrics::SUCCESS]);
 
-        match message {
-            AggregateAndProof::Base(message) => {
-                Ok(SignedAggregateAndProof::Base(SignedAggregateAndProofBase {
-                    message,
-                    signature,
-                }))
-            }
-            AggregateAndProof::Electra(message) => Ok(SignedAggregateAndProof::Electra(
-                SignedAggregateAndProofElectra { message, signature },
-            )),
-        }
+        Ok(SignedAggregateAndProof::from_aggregate_and_proof(
+            message, signature,
+        ))
     }
 
     /// Produces a `SelectionProof` for the `slot`, signed by with corresponding secret key to