Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Gossip verification compiles
Browse files Browse the repository at this point in the history
pawanjay176 committed Nov 2, 2023

Unverified

This user has not yet uploaded their public signing key.
1 parent 412517c commit 8c6be8a
Showing 14 changed files with 96 additions and 169 deletions.
107 changes: 39 additions & 68 deletions beacon_node/beacon_chain/src/blob_verification.rs
Original file line number Diff line number Diff line change
@@ -16,10 +16,7 @@ use ssz_derive::{Decode, Encode};
use ssz_types::VariableList;
use tree_hash::TreeHash;
use types::blob_sidecar::BlobIdentifier;
use types::{
BeaconStateError, BlobSidecar, BlobSidecarList, CloneConfig, EthSpec, Hash256,
SignedBlobSidecar, Slot,
};
use types::{BeaconStateError, BlobSidecar, BlobSidecarList, CloneConfig, EthSpec, Hash256, Slot};

/// An error occurred while validating a gossip blob.
#[derive(Debug)]
@@ -118,7 +115,7 @@ impl<T: EthSpec> std::fmt::Display for GossipBlobError<T> {
write!(
f,
"BlobParentUnknown(parent_root:{})",
blob_sidecar.block_parent_root
blob_sidecar.block_parent_root()
)
}
other => write!(f, "{:?}", other),
@@ -147,62 +144,56 @@ pub type GossipVerifiedBlobList<T> = VariableList<
/// the p2p network.
#[derive(Debug)]
pub struct GossipVerifiedBlob<T: BeaconChainTypes> {
blob: SignedBlobSidecar<T::EthSpec>,
blob: Arc<BlobSidecar<T::EthSpec>>,
}

impl<T: BeaconChainTypes> GossipVerifiedBlob<T> {
pub fn new(
blob: SignedBlobSidecar<T::EthSpec>,
blob: Arc<BlobSidecar<T::EthSpec>>,
chain: &BeaconChain<T>,
) -> Result<Self, GossipBlobError<T::EthSpec>> {
let blob_index = blob.message.index;
let blob_index = blob.index;
validate_blob_sidecar_for_gossip(blob, blob_index, chain)
}
/// Construct a `GossipVerifiedBlob` that is assumed to be valid.
///
/// This should ONLY be used for testing.
pub fn __assumed_valid(blob: SignedBlobSidecar<T::EthSpec>) -> Self {
pub fn __assumed_valid(blob: Arc<BlobSidecar<T::EthSpec>>) -> Self {
Self { blob }
}
pub fn id(&self) -> BlobIdentifier {
self.blob.message.id()
self.blob.id()
}
pub fn block_root(&self) -> Hash256 {
self.blob.message.block_root
}
pub fn to_blob(self) -> Arc<BlobSidecar<T::EthSpec>> {
self.blob.message
}
pub fn as_blob(&self) -> &BlobSidecar<T::EthSpec> {
&self.blob.message
}
pub fn signed_blob(&self) -> SignedBlobSidecar<T::EthSpec> {
self.blob.clone()
self.blob.block_root()
}
pub fn slot(&self) -> Slot {
self.blob.message.slot
self.blob.slot()
}
pub fn index(&self) -> u64 {
self.blob.message.index
self.blob.index
}
pub fn kzg_commitment(&self) -> KzgCommitment {
self.blob.message.kzg_commitment
self.blob.kzg_commitment
}
pub fn cloned(&self) -> Arc<BlobSidecar<T::EthSpec>> {
self.blob.clone()
}
pub fn proposer_index(&self) -> u64 {
self.blob.message.proposer_index
pub fn into_inner(self) -> Arc<BlobSidecar<T::EthSpec>> {
self.blob
}
}

pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
signed_blob_sidecar: SignedBlobSidecar<T::EthSpec>,
blob_sidecar: Arc<BlobSidecar<T::EthSpec>>,
subnet: u64,
chain: &BeaconChain<T>,
) -> Result<GossipVerifiedBlob<T>, GossipBlobError<T::EthSpec>> {
let blob_slot = signed_blob_sidecar.message.slot;
let blob_index = signed_blob_sidecar.message.index;
let block_parent_root = signed_blob_sidecar.message.block_parent_root;
let blob_proposer_index = signed_blob_sidecar.message.proposer_index;
let block_root = signed_blob_sidecar.message.block_root;
let blob_slot = blob_sidecar.slot();
let blob_index = blob_sidecar.index;
let block_parent_root = blob_sidecar.block_parent_root();
let blob_proposer_index = blob_sidecar.block_proposer_index();
let block_root = blob_sidecar.block_root();
let blob_epoch = blob_slot.epoch(T::EthSpec::slots_per_epoch());

// Verify that the blob_sidecar was received on the correct subnet.
@@ -213,7 +204,16 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
});
}

let blob_root = get_blob_root(&signed_blob_sidecar);
// This condition is not possible if we have received the blob from the network
// since we only subscribe to `MaxBlobsPerBlock` subnets over gossip network.
// We include this check only for completeness.
// Getting this error would imply something very wrong with our networking decoding logic.
if blob_index >= T::EthSpec::max_blobs_per_block() as u64 {
return Err(GossipBlobError::InvalidSubnet {
expected: subnet,
received: blob_index,
});
}

// Verify that the sidecar is not from a future slot.
let latest_permissible_slot = chain
@@ -244,7 +244,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
if chain
.observed_blob_sidecars
.read()
.is_known(&signed_blob_sidecar.message)
.is_known(&blob_sidecar)
.map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
{
return Err(GossipBlobError::RepeatBlob {
@@ -261,9 +261,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
.fork_choice_read_lock()
.get_block(&block_parent_root)
else {
return Err(GossipBlobError::BlobParentUnknown(
signed_blob_sidecar.message,
));
return Err(GossipBlobError::BlobParentUnknown(blob_sidecar));
};

if parent_block.slot >= blob_slot {
@@ -392,31 +390,6 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
});
}

// Signature verification
let signature_is_valid = {
let pubkey_cache = chain
.validator_pubkey_cache
.try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT)
.ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout)
.map_err(GossipBlobError::BeaconChainError)?;

let pubkey = pubkey_cache
.get(proposer_index)
.ok_or_else(|| GossipBlobError::UnknownValidator(proposer_index as u64))?;

signed_blob_sidecar.verify_signature(
Some(blob_root),
pubkey,
&fork,
chain.genesis_validators_root,
&chain.spec,
)
};

if !signature_is_valid {
return Err(GossipBlobError::ProposerSignatureInvalid);
}

// Now the signature is valid, store the proposal so we don't accept another blob sidecar
// with the same `BlobIdentifier`.
// It's important to double-check that the proposer still hasn't been observed so we don't
@@ -431,7 +404,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
if chain
.observed_blob_sidecars
.write()
.observe_sidecar(&signed_blob_sidecar.message)
.observe_sidecar(&blob_sidecar)
.map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
{
return Err(GossipBlobError::RepeatBlob {
@@ -441,9 +414,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
});
}

Ok(GossipVerifiedBlob {
blob: signed_blob_sidecar,
})
Ok(GossipVerifiedBlob { blob: blob_sidecar })
}

/// Wrapper over a `BlobSidecar` for which we have completed kzg verification.
@@ -478,7 +449,7 @@ impl<T: EthSpec> KzgVerifiedBlob<T> {
self.blob.clone()
}
pub fn block_root(&self) -> Hash256 {
self.blob.block_root
self.blob.block_root()
}
pub fn blob_index(&self) -> u64 {
self.blob.index
@@ -537,10 +508,10 @@ pub fn verify_kzg_for_blob_list<T: EthSpec>(
/// Returns the canonical root of the given `blob`.
///
/// Use this function to ensure that we report the blob hashing time Prometheus metric.
pub fn get_blob_root<E: EthSpec>(blob: &SignedBlobSidecar<E>) -> Hash256 {
pub fn get_blob_root<E: EthSpec>(blob: &BlobSidecar<E>) -> Hash256 {
let blob_root_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_BLOB_ROOT);

let blob_root = blob.message.tree_hash_root();
let blob_root = blob.tree_hash_root();

metrics::stop_timer(blob_root_timer);

4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
@@ -682,12 +682,12 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlockContents<T> for GossipVerifiedB
fn inner_block(&self) -> &SignedBeaconBlock<T::EthSpec> {
self.0.block.as_block()
}
fn inner_blobs(&self) -> Option<SignedBlobSidecarList<T::EthSpec>> {
fn inner_blobs(&self) -> Option<BlobSidecarList<T::EthSpec>> {
self.1.as_ref().map(|blobs| {
VariableList::from(
blobs
.into_iter()
.map(GossipVerifiedBlob::signed_blob)
.map(GossipVerifiedBlob::cloned)
.collect::<Vec<_>>(),
)
})
7 changes: 0 additions & 7 deletions beacon_node/beacon_chain/src/block_verification_types.rs
Original file line number Diff line number Diff line change
@@ -91,13 +91,6 @@ impl<E: EthSpec> RpcBlock<E> {
return Err(AvailabilityCheckError::MissingBlobs);
}
for (blob, &block_commitment) in blobs.iter().zip(block_commitments.iter()) {
let blob_block_root = blob.block_root;
if blob_block_root != block_root {
return Err(AvailabilityCheckError::InconsistentBlobBlockRoots {
block_root,
blob_block_root,
});
}
let blob_commitment = blob.kzg_commitment;
if blob_commitment != block_commitment {
return Err(AvailabilityCheckError::KzgCommitmentMismatch {
6 changes: 3 additions & 3 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
@@ -221,7 +221,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
) -> Result<Availability<T::EthSpec>, AvailabilityCheckError> {
// Verify the KZG commitments.
let kzg_verified_blob = if let Some(kzg) = self.kzg.as_ref() {
verify_kzg_for_blob(gossip_blob.to_blob(), kzg)?
verify_kzg_for_blob(gossip_blob.into_inner(), kzg)?
} else {
return Err(AvailabilityCheckError::KzgNotInitialized);
};
@@ -310,8 +310,8 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
block_root: Hash256,
blob: &GossipVerifiedBlob<T>,
) {
let index = blob.as_blob().index;
let commitment = blob.as_blob().kzg_commitment;
let index = blob.index();
let commitment = blob.kzg_commitment();
self.processing_cache
.write()
.entry(block_root)
Original file line number Diff line number Diff line change
@@ -16,10 +16,6 @@ pub enum Error {
BlobIndexInvalid(u64),
StoreError(store::Error),
DecodeError(ssz::DecodeError),
InconsistentBlobBlockRoots {
block_root: Hash256,
blob_block_root: Hash256,
},
ParentStateMissing(Hash256),
BlockReplayError(state_processing::BlockReplayError),
RebuildingStateCaches(BeaconStateError),
@@ -47,8 +43,7 @@ impl Error {
Error::Kzg(_)
| Error::BlobIndexInvalid(_)
| Error::KzgCommitmentMismatch { .. }
| Error::KzgVerificationFailed
| Error::InconsistentBlobBlockRoots { .. } => ErrorCategory::Malicious,
| Error::KzgVerificationFailed => ErrorCategory::Malicious,
}
}
}
Original file line number Diff line number Diff line change
@@ -125,7 +125,10 @@ impl<T: EthSpec> PendingComponents<T> {
for maybe_blob in self.verified_blobs.iter() {
if maybe_blob.is_some() {
return maybe_blob.as_ref().map(|kzg_verified_blob| {
kzg_verified_blob.as_blob().slot.epoch(T::slots_per_epoch())
kzg_verified_blob
.as_blob()
.slot()
.epoch(T::slots_per_epoch())
});
}
}
@@ -421,12 +424,6 @@ impl<T: BeaconChainTypes> OverflowLRUCache<T> {
// Initial check to ensure all provided blobs have a consistent block root.
for blob in kzg_verified_blobs {
let blob_block_root = blob.block_root();
if blob_block_root != block_root {
return Err(AvailabilityCheckError::InconsistentBlobBlockRoots {
block_root,
blob_block_root,
});
}
if let Some(blob_opt) = fixed_blobs.get_mut(blob.blob_index() as usize) {
*blob_opt = Some(blob);
}
@@ -651,7 +648,7 @@ impl<T: BeaconChainTypes> OverflowLRUCache<T> {
OverflowKey::Blob(_, _) => {
KzgVerifiedBlob::<T::EthSpec>::from_ssz_bytes(value_bytes.as_slice())?
.as_blob()
.slot
.slot()
.epoch(T::EthSpec::slots_per_epoch())
}
};
19 changes: 10 additions & 9 deletions beacon_node/beacon_chain/src/observed_blob_sidecars.rs
Original file line number Diff line number Diff line change
@@ -50,36 +50,36 @@ impl<T: EthSpec> ObservedBlobSidecars<T> {
/// This will update `self` so future calls to it indicate that this `blob_sidecar` is known.
///
/// The supplied `blob_sidecar` **MUST** have completed proposer signature verification.
pub fn observe_sidecar(&mut self, blob_sidecar: &Arc<BlobSidecar<T>>) -> Result<bool, Error> {
pub fn observe_sidecar(&mut self, blob_sidecar: &BlobSidecar<T>) -> Result<bool, Error> {
self.sanitize_blob_sidecar(blob_sidecar)?;

let did_not_exist = self
.items
.entry((blob_sidecar.block_root, blob_sidecar.slot))
.entry((blob_sidecar.block_root(), blob_sidecar.slot()))
.or_insert_with(|| HashSet::with_capacity(T::max_blobs_per_block()))
.insert(blob_sidecar.index);

Ok(!did_not_exist)
}

/// Returns `true` if the `blob_sidecar` has already been observed in the cache within the prune window.
pub fn is_known(&self, blob_sidecar: &Arc<BlobSidecar<T>>) -> Result<bool, Error> {
pub fn is_known(&self, blob_sidecar: &BlobSidecar<T>) -> Result<bool, Error> {
self.sanitize_blob_sidecar(blob_sidecar)?;
let is_known = self
.items
.get(&(blob_sidecar.block_root, blob_sidecar.slot))
.get(&(blob_sidecar.block_root(), blob_sidecar.slot()))
.map_or(false, |set| set.contains(&blob_sidecar.index));
Ok(is_known)
}

fn sanitize_blob_sidecar(&self, blob_sidecar: &Arc<BlobSidecar<T>>) -> Result<(), Error> {
fn sanitize_blob_sidecar(&self, blob_sidecar: &BlobSidecar<T>) -> Result<(), Error> {
if blob_sidecar.index >= T::max_blobs_per_block() as u64 {
return Err(Error::InvalidBlobIndex(blob_sidecar.index));
}
let finalized_slot = self.finalized_slot;
if finalized_slot > 0 && blob_sidecar.slot <= finalized_slot {
if finalized_slot > 0 && blob_sidecar.slot() <= finalized_slot {
return Err(Error::FinalizedBlob {
slot: blob_sidecar.slot,
slot: blob_sidecar.slot(),
finalized_slot,
});
}
@@ -107,8 +107,9 @@ mod tests {

fn get_blob_sidecar(slot: u64, block_root: Hash256, index: u64) -> Arc<BlobSidecar<E>> {
let mut blob_sidecar = BlobSidecar::empty();
blob_sidecar.block_root = block_root;
blob_sidecar.slot = slot.into();
// TODO(pawan): have a block root setter for tests
// blob_sidecar.block_root = block_root;
blob_sidecar.signed_block_header.message.slot = slot.into();
blob_sidecar.index = index;
Arc::new(blob_sidecar)
}
Loading

0 comments on commit 8c6be8a

Please sign in to comment.