Skip to content

Commit e741cea

Browse files
michaelsproulmichaelsproul
committed
Move dangerous methods off trait
1 parent e03208c commit e741cea

File tree

2 files changed

+120
-133
lines changed
  • validator_client
    • lighthouse_validator_store/src
    • validator_store/src

2 files changed

+120
-133
lines changed

‎validator_client/lighthouse_validator_store/src/lib.rs‎

Lines changed: 120 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,126 @@ impl<T: SlotClock + 'static, E: EthSpec> LighthouseValidatorStore<T, E> {
557557
signature,
558558
})
559559
}
560+
561+
#[instrument(skip_all)]
562+
async fn sign_attestation_no_checks(
563+
&self,
564+
validator_pubkey: PublicKeyBytes,
565+
validator_committee_position: usize,
566+
attestation: &mut Attestation<E>,
567+
) -> Result<(), Error> {
568+
// Get the signing method and check doppelganger protection.
569+
let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?;
570+
571+
// Sign the attestation.
572+
let signing_epoch = attestation.data().target.epoch;
573+
let signing_context = self.signing_context(Domain::BeaconAttester, signing_epoch);
574+
575+
let signature = signing_method
576+
.get_signature::<E, BlindedPayload<E>>(
577+
SignableMessage::AttestationData(attestation.data()),
578+
signing_context,
579+
&self.spec,
580+
&self.task_executor,
581+
)
582+
.await?;
583+
attestation
584+
.add_signature(&signature, validator_committee_position)
585+
.map_err(Error::UnableToSignAttestation)?;
586+
587+
Ok(())
588+
}
589+
590+
#[instrument(
591+
name = "store_check_and_insert_attestations",
592+
level = "debug",
593+
skip_all
594+
)]
595+
fn check_and_insert_attestations(
596+
&self,
597+
attestations: Vec<(Attestation<E>, PublicKeyBytes)>,
598+
) -> Result<Vec<(Attestation<E>, PublicKeyBytes)>, Error> {
599+
let mut safe_attestations = vec![];
600+
let mut attestations_to_check = vec![];
601+
602+
// Split attestations into de-facto safe attestations (checked by web3signer's slashing
603+
// protection) and ones requiring checking against the slashing protection DB.
604+
for (attestation, validator_pubkey) in &attestations {
605+
let signing_method = self.doppelganger_checked_signing_method(*validator_pubkey)?;
606+
let signing_epoch = attestation.data().target.epoch;
607+
let signing_context = self.signing_context(Domain::BeaconAttester, signing_epoch);
608+
let domain_hash = signing_context.domain_hash(&self.spec);
609+
610+
let check_slashability = if signing_method
611+
.requires_local_slashing_protection(self.enable_web3signer_slashing_protection)
612+
{
613+
CheckSlashability::Yes
614+
} else {
615+
CheckSlashability::No
616+
};
617+
attestations_to_check.push((
618+
attestation.data(),
619+
validator_pubkey,
620+
domain_hash,
621+
check_slashability,
622+
));
623+
}
624+
625+
// Batch check the attestations against the slashing protection DB while preserving the
626+
// order so we can zip the results against the original vec.
627+
//
628+
// If the DB transaction fails then we consider the entire batch slashable and discard it.
629+
let results = self
630+
.slashing_protection
631+
.check_and_insert_attestations(&attestations_to_check)
632+
.map_err(Error::Slashable)?;
633+
634+
for ((attestation, validator_pubkey), slashing_status) in
635+
attestations.into_iter().zip(results.into_iter())
636+
{
637+
match slashing_status {
638+
Ok(Safe::Valid) => {
639+
safe_attestations.push((attestation, validator_pubkey));
640+
validator_metrics::inc_counter_vec(
641+
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
642+
&[validator_metrics::SUCCESS],
643+
);
644+
}
645+
Ok(Safe::SameData) => {
646+
warn!("Skipping previously signed attestation");
647+
validator_metrics::inc_counter_vec(
648+
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
649+
&[validator_metrics::SAME_DATA],
650+
);
651+
}
652+
Err(NotSafe::UnregisteredValidator(pk)) => {
653+
warn!(
654+
msg = "Carefully consider running with --init-slashing-protection (see --help)",
655+
public_key = ?pk,
656+
"Not signing attestation for unregistered validator"
657+
);
658+
validator_metrics::inc_counter_vec(
659+
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
660+
&[validator_metrics::UNREGISTERED],
661+
);
662+
}
663+
Err(e) => {
664+
// FIXME(sproul): remove attestation data + make this error less scary
665+
crit!(
666+
attestation = format!("{:?}", attestation.data()),
667+
error = format!("{:?}", e),
668+
"Not signing slashable attestation"
669+
);
670+
validator_metrics::inc_counter_vec(
671+
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
672+
&[validator_metrics::SLASHABLE],
673+
);
674+
}
675+
}
676+
}
677+
678+
Ok(safe_attestations)
679+
}
560680
}
561681

562682
impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore for LighthouseValidatorStore<T, E> {
@@ -806,126 +926,6 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore for LighthouseValidatorS
806926
Ok(safe_attestations.into_iter().map(|(a, _)| a).collect())
807927
}
808928

809-
#[instrument(skip_all)]
810-
async fn sign_attestation_no_checks(
811-
&self,
812-
validator_pubkey: PublicKeyBytes,
813-
validator_committee_position: usize,
814-
attestation: &mut Attestation<E>,
815-
) -> Result<(), Error> {
816-
// Get the signing method and check doppelganger protection.
817-
let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?;
818-
819-
// Sign the attestation.
820-
let signing_epoch = attestation.data().target.epoch;
821-
let signing_context = self.signing_context(Domain::BeaconAttester, signing_epoch);
822-
823-
let signature = signing_method
824-
.get_signature::<E, BlindedPayload<E>>(
825-
SignableMessage::AttestationData(attestation.data()),
826-
signing_context,
827-
&self.spec,
828-
&self.task_executor,
829-
)
830-
.await?;
831-
attestation
832-
.add_signature(&signature, validator_committee_position)
833-
.map_err(Error::UnableToSignAttestation)?;
834-
835-
Ok(())
836-
}
837-
838-
#[instrument(
839-
name = "store_check_and_insert_attestations",
840-
level = "debug",
841-
skip_all
842-
)]
843-
fn check_and_insert_attestations(
844-
&self,
845-
attestations: Vec<(Attestation<E>, PublicKeyBytes)>,
846-
) -> Result<Vec<(Attestation<E>, PublicKeyBytes)>, Error> {
847-
let mut safe_attestations = vec![];
848-
let mut attestations_to_check = vec![];
849-
850-
// Split attestations into de-facto safe attestations (checked by web3signer's slashing
851-
// protection) and ones requiring checking against the slashing protection DB.
852-
for (attestation, validator_pubkey) in &attestations {
853-
let signing_method = self.doppelganger_checked_signing_method(*validator_pubkey)?;
854-
let signing_epoch = attestation.data().target.epoch;
855-
let signing_context = self.signing_context(Domain::BeaconAttester, signing_epoch);
856-
let domain_hash = signing_context.domain_hash(&self.spec);
857-
858-
let check_slashability = if signing_method
859-
.requires_local_slashing_protection(self.enable_web3signer_slashing_protection)
860-
{
861-
CheckSlashability::Yes
862-
} else {
863-
CheckSlashability::No
864-
};
865-
attestations_to_check.push((
866-
attestation.data(),
867-
validator_pubkey,
868-
domain_hash,
869-
check_slashability,
870-
));
871-
}
872-
873-
// Batch check the attestations against the slashing protection DB while preserving the
874-
// order so we can zip the results against the original vec.
875-
//
876-
// If the DB transaction fails then we consider the entire batch slashable and discard it.
877-
let results = self
878-
.slashing_protection
879-
.check_and_insert_attestations(&attestations_to_check)
880-
.map_err(Error::Slashable)?;
881-
882-
for ((attestation, validator_pubkey), slashing_status) in
883-
attestations.into_iter().zip(results.into_iter())
884-
{
885-
match slashing_status {
886-
Ok(Safe::Valid) => {
887-
safe_attestations.push((attestation, validator_pubkey));
888-
validator_metrics::inc_counter_vec(
889-
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
890-
&[validator_metrics::SUCCESS],
891-
);
892-
}
893-
Ok(Safe::SameData) => {
894-
warn!("Skipping previously signed attestation");
895-
validator_metrics::inc_counter_vec(
896-
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
897-
&[validator_metrics::SAME_DATA],
898-
);
899-
}
900-
Err(NotSafe::UnregisteredValidator(pk)) => {
901-
warn!(
902-
msg = "Carefully consider running with --init-slashing-protection (see --help)",
903-
public_key = ?pk,
904-
"Not signing attestation for unregistered validator"
905-
);
906-
validator_metrics::inc_counter_vec(
907-
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
908-
&[validator_metrics::UNREGISTERED],
909-
);
910-
}
911-
Err(e) => {
912-
// FIXME(sproul): remove attestation data + make this error less scary
913-
crit!(
914-
attestation = format!("{:?}", attestation.data()),
915-
error = format!("{:?}", e),
916-
"Not signing slashable attestation"
917-
);
918-
validator_metrics::inc_counter_vec(
919-
&validator_metrics::SIGNED_ATTESTATIONS_TOTAL,
920-
&[validator_metrics::SLASHABLE],
921-
);
922-
}
923-
}
924-
}
925-
926-
Ok(safe_attestations)
927-
}
928-
929929
async fn sign_validator_registration_data(
930930
&self,
931931
validator_registration_data: ValidatorRegistrationData,

‎validator_client/validator_store/src/lib.rs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -109,19 +109,6 @@ pub trait ValidatorStore: Send + Sync {
109109
attestations: Vec<(PublicKeyBytes, usize, Attestation<Self::E>)>,
110110
) -> impl Future<Output = Result<Vec<Attestation<Self::E>>, Error<Self::Error>>> + Send;
111111

112-
fn sign_attestation_no_checks(
113-
&self,
114-
validator_pubkey: PublicKeyBytes,
115-
validator_committee_position: usize,
116-
attestation: &mut Attestation<Self::E>,
117-
) -> impl Future<Output = Result<(), Error<Self::Error>>> + Send;
118-
119-
#[allow(clippy::type_complexity)]
120-
fn check_and_insert_attestations(
121-
&self,
122-
attestations: Vec<(Attestation<Self::E>, PublicKeyBytes)>,
123-
) -> Result<Vec<(Attestation<Self::E>, PublicKeyBytes)>, Error<Self::Error>>;
124-
125112
fn sign_validator_registration_data(
126113
&self,
127114
validator_registration_data: ValidatorRegistrationData,

0 commit comments

Comments
 (0)