Skip to content

Commit

Permalink
switch to enum for block signing to allow differing types
Browse files Browse the repository at this point in the history
  • Loading branch information
@dknopik
dknopik committed Jan 9, 2025
1 parent 4510b73 commit b6a3f0e
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 132 deletions.
217 changes: 116 additions & 101 deletions validator_client/lighthouse_validator_store/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ use types::{
VoluntaryExit,
};
use validator_store::{
DoppelgangerStatus, Error as ValidatorStoreError, ProposalData, SignBlock, ValidatorStore,
DoppelgangerStatus, Error as ValidatorStoreError, ProposalData, SignedBlock, UnsignedBlock,
ValidatorStore,
};

pub type Error = ValidatorStoreError<SigningError>;
Expand Down Expand Up @@ -413,6 +414,102 @@ impl<T: SlotClock + 'static, E: EthSpec> LighthouseValidatorStore<T, E> {
)
})
}

async fn sign_abstract_block<Payload: AbstractExecPayload<E>>(
&self,
validator_pubkey: PublicKeyBytes,
block: BeaconBlock<E, Payload>,
current_slot: Slot,
) -> Result<SignedBeaconBlock<E, Payload>, Error> {
// Make sure the block slot is not higher than the current slot to avoid potential attacks.
if block.slot() > current_slot {
warn!(
self.log,
"Not signing block with slot greater than current slot";
"block_slot" => block.slot().as_u64(),
"current_slot" => current_slot.as_u64()
);
return Err(Error::GreaterThanCurrentSlot {
slot: block.slot(),
current_slot,
});
}

let signing_epoch = block.epoch();
let signing_context = self.signing_context(Domain::BeaconProposer, signing_epoch);
let domain_hash = signing_context.domain_hash(&self.spec);

let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?;

// Check for slashing conditions.
let slashing_status = if signing_method
.requires_local_slashing_protection(self.enable_web3signer_slashing_protection)
{
self.slashing_protection.check_and_insert_block_proposal(
&validator_pubkey,
&block.block_header(),
domain_hash,
)
} else {
Ok(Safe::Valid)
};

match slashing_status {
// We can safely sign this block without slashing.
Ok(Safe::Valid) => {
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::SUCCESS],
);

let signature = signing_method
.get_signature(
SignableMessage::BeaconBlock(&block),
signing_context,
&self.spec,
&self.task_executor,
)
.await?;
Ok(SignedBeaconBlock::from_block(block, signature))
}
Ok(Safe::SameData) => {
warn!(
self.log,
"Skipping signing of previously signed block";
);
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::SAME_DATA],
);
Err(Error::SameData)
}
Err(NotSafe::UnregisteredValidator(pk)) => {
warn!(
self.log,
"Not signing block for unregistered validator";
"msg" => "Carefully consider running with --init-slashing-protection (see --help)",
"public_key" => format!("{:?}", pk)
);
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::UNREGISTERED],
);
Err(Error::Slashable(NotSafe::UnregisteredValidator(pk)))
}
Err(e) => {
crit!(
self.log,
"Not signing slashable block";
"error" => format!("{:?}", e)
);
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::SLASHABLE],
);
Err(Error::Slashable(e))
}
}
}
}

impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore for LighthouseValidatorStore<T, E> {
Expand Down Expand Up @@ -571,6 +668,24 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore for LighthouseValidatorS
.set_index(validator_pubkey, index);
}

async fn sign_block(
&self,
validator_pubkey: PublicKeyBytes,
block: UnsignedBlock<E>,
current_slot: Slot,
) -> Result<SignedBlock<E>, Error> {
match block {
UnsignedBlock::Full(block) => self
.sign_abstract_block(validator_pubkey, block, current_slot)
.await
.map(SignedBlock::Full),
UnsignedBlock::Blinded(block) => self
.sign_abstract_block(validator_pubkey, block, current_slot)
.await
.map(SignedBlock::Blinded),
}
}

async fn sign_attestation(
&self,
validator_pubkey: PublicKeyBytes,
Expand Down Expand Up @@ -992,103 +1107,3 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore for LighthouseValidatorS
})
}
}

impl<T: SlotClock + 'static, E: EthSpec, P: AbstractExecPayload<E>>
SignBlock<E, P, signing_method::Error> for LighthouseValidatorStore<T, E>
{
async fn sign_block(
&self,
validator_pubkey: PublicKeyBytes,
block: BeaconBlock<E, P>,
current_slot: Slot,
) -> Result<SignedBeaconBlock<E, P>, Error> {
// Make sure the block slot is not higher than the current slot to avoid potential attacks.
if block.slot() > current_slot {
warn!(
self.log,
"Not signing block with slot greater than current slot";
"block_slot" => block.slot().as_u64(),
"current_slot" => current_slot.as_u64()
);
return Err(Error::GreaterThanCurrentSlot {
slot: block.slot(),
current_slot,
});
}

let signing_epoch = block.epoch();
let signing_context = self.signing_context(Domain::BeaconProposer, signing_epoch);
let domain_hash = signing_context.domain_hash(&self.spec);

let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?;

// Check for slashing conditions.
let slashing_status = if signing_method
.requires_local_slashing_protection(self.enable_web3signer_slashing_protection)
{
self.slashing_protection.check_and_insert_block_proposal(
&validator_pubkey,
&block.block_header(),
domain_hash,
)
} else {
Ok(Safe::Valid)
};

match slashing_status {
// We can safely sign this block without slashing.
Ok(Safe::Valid) => {
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::SUCCESS],
);

let signature = signing_method
.get_signature(
SignableMessage::BeaconBlock(&block),
signing_context,
&self.spec,
&self.task_executor,
)
.await?;
Ok(SignedBeaconBlock::from_block(block, signature))
}
Ok(Safe::SameData) => {
warn!(
self.log,
"Skipping signing of previously signed block";
);
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::SAME_DATA],
);
Err(Error::SameData)
}
Err(NotSafe::UnregisteredValidator(pk)) => {
warn!(
self.log,
"Not signing block for unregistered validator";
"msg" => "Carefully consider running with --init-slashing-protection (see --help)",
"public_key" => format!("{:?}", pk)
);
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::UNREGISTERED],
);
Err(Error::Slashable(NotSafe::UnregisteredValidator(pk)))
}
Err(e) => {
crit!(
self.log,
"Not signing slashable block";
"error" => format!("{:?}", e)
);
validator_metrics::inc_counter_vec(
&validator_metrics::SIGNED_BLOCKS_TOTAL,
&[validator_metrics::SLASHABLE],
);
Err(Error::Slashable(e))
}
}
}
}
27 changes: 16 additions & 11 deletions validator_client/validator_services/src/block_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,22 +333,27 @@ impl<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
) -> Result<(), BlockError> {
let signing_timer = validator_metrics::start_timer(&validator_metrics::BLOCK_SIGNING_TIMES);

let res = match unsigned_block {
let (block, maybe_blobs) = match unsigned_block {
UnsignedBlock::Full(block_contents) => {
let (block, maybe_blobs) = block_contents.deconstruct();
self.validator_store
.sign_block(*validator_pubkey, block, slot)
.await
.map(|b| SignedBlock::Full(PublishBlockRequest::new(Arc::new(b), maybe_blobs)))
(block.into(), maybe_blobs)
}
UnsignedBlock::Blinded(block) => self
.validator_store
.sign_block(*validator_pubkey, block, slot)
.await
.map(Arc::new)
.map(SignedBlock::Blinded),
UnsignedBlock::Blinded(block) => (block.into(), None),
};

let res = self
.validator_store
.sign_block(*validator_pubkey, block, slot)
.await
.map(|block| match block {
validator_store::SignedBlock::Full(block) => {
SignedBlock::Full(PublishBlockRequest::new(Arc::new(block), maybe_blobs))
}
validator_store::SignedBlock::Blinded(block) => {
SignedBlock::Blinded(Arc::new(block))
}
});

let signed_block = match res {
Ok(block) => block,
Err(ValidatorStoreError::UnknownPubkey(pubkey)) => {
Expand Down
63 changes: 43 additions & 20 deletions validator_client/validator_store/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use slashing_protection::NotSafe;
use std::fmt::Debug;
use std::future::Future;
use types::{
AbstractExecPayload, Address, Attestation, AttestationError, BeaconBlock, BlindedPayload,
Epoch, EthSpec, FullPayload, Graffiti, Hash256, PublicKeyBytes, SelectionProof, Signature,
SignedAggregateAndProof, SignedBeaconBlock, SignedContributionAndProof,
Address, Attestation, AttestationError, BeaconBlock, BlindedBeaconBlock, Epoch, EthSpec,
Graffiti, Hash256, PublicKeyBytes, SelectionProof, Signature, SignedAggregateAndProof,
SignedBeaconBlock, SignedBlindedBeaconBlock, SignedContributionAndProof,
SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, SyncCommitteeContribution,
SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, ValidatorRegistrationData,
VoluntaryExit,
Expand Down Expand Up @@ -37,12 +37,7 @@ pub struct ProposalData {
pub builder_proposals: bool,
}

pub trait ValidatorStore:
SignBlock<Self::E, FullPayload<Self::E>, Self::Error>
+ SignBlock<Self::E, BlindedPayload<Self::E>, Self::Error>
+ Send
+ Sync
{
pub trait ValidatorStore: Send + Sync {
type Error: Debug + Send + Sync;
type E: EthSpec;

Expand Down Expand Up @@ -99,6 +94,13 @@ pub trait ValidatorStore:

fn set_validator_index(&self, validator_pubkey: &PublicKeyBytes, index: u64);

fn sign_block(
&self,
validator_pubkey: PublicKeyBytes,
block: UnsignedBlock<Self::E>,
current_slot: Slot,
) -> impl Future<Output = Result<SignedBlock<Self::E>, Error<Self::Error>>> + Send;

fn sign_attestation(
&self,
validator_pubkey: PublicKeyBytes,
Expand Down Expand Up @@ -175,17 +177,38 @@ pub trait ValidatorStore:
fn proposal_data(&self, pubkey: &PublicKeyBytes) -> Option<ProposalData>;
}

// This is a workaround: directly adding this fn into `ValidatorStore`, abstract over P, causes
// weird compiler errors which I suspect are compiler bugs
// Another advantage of this separate trait is to allow implementors separate implementations for
// different payload
pub trait SignBlock<E: EthSpec, P: AbstractExecPayload<E>, Err> {
fn sign_block(
&self,
validator_pubkey: PublicKeyBytes,
block: BeaconBlock<E, P>,
current_slot: Slot,
) -> impl Future<Output = Result<SignedBeaconBlock<E, P>, Error<Err>>> + Send;
pub enum UnsignedBlock<E: EthSpec> {
Full(BeaconBlock<E>),
Blinded(BlindedBeaconBlock<E>),
}

impl<E: EthSpec> From<BeaconBlock<E>> for UnsignedBlock<E> {
fn from(block: BeaconBlock<E>) -> Self {
UnsignedBlock::Full(block)
}
}

impl<E: EthSpec> From<BlindedBeaconBlock<E>> for UnsignedBlock<E> {
fn from(block: BlindedBeaconBlock<E>) -> Self {
UnsignedBlock::Blinded(block)
}
}

pub enum SignedBlock<E: EthSpec> {
Full(SignedBeaconBlock<E>),
Blinded(SignedBlindedBeaconBlock<E>),
}

impl<E: EthSpec> From<SignedBeaconBlock<E>> for SignedBlock<E> {
fn from(block: SignedBeaconBlock<E>) -> Self {
SignedBlock::Full(block)
}
}

impl<E: EthSpec> From<SignedBlindedBeaconBlock<E>> for SignedBlock<E> {
fn from(block: SignedBlindedBeaconBlock<E>) -> Self {
SignedBlock::Blinded(block)
}
}

/// A wrapper around `PublicKeyBytes` which encodes information about the status of a validator
Expand Down

0 comments on commit b6a3f0e

Please sign in to comment.