Skip to content

refactor(signature_collector): Remove base_hash #437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from
Open

refactor(signature_collector): Remove base_hash #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions anchor/signature_collector/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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",
);

Expand All @@ -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 {
Expand All @@ -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 } => {
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
},
Expand Down Expand Up @@ -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>,
Expand Down
24 changes: 5 additions & 19 deletions anchor/validator_store/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain this?

let metadata = self.get_slot_metadata(slot).await?;
SignatureRequester::Committee {
num_signatures_to_collect: self
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)?,
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down