Merge pull request #3833 from realbigsean/empty-blobs

Empty blobs validation
This commit is contained in:
realbigsean 2022-12-28 10:32:06 -05:00 committed by GitHub
commit 019467840f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 143 additions and 97 deletions

View File

@ -42,6 +42,7 @@ pub enum Error {
// Boxed to avoid an infinite-size recursion issue.
BeaconChain(Box<BeaconChainError>),
MissingBeaconState(Hash256),
MissingBlobs,
FailedToTransitionState(StateAdvanceError),
CannotAttestToFutureState {
state_slot: Slot,

View File

@ -2935,7 +2935,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// If the write fails, revert fork choice to the version from disk, else we can
// end up with blocks in fork choice that are missing from disk.
// See https://github.com/sigp/lighthouse/issues/2028
let (signed_block, blobs) = signed_block.deconstruct();
let (signed_block, blobs) = signed_block.deconstruct(Some(block_root));
let block = signed_block.message();
let mut ops: Vec<_> = confirmed_state_roots
.into_iter()
@ -2944,7 +2944,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
ops.push(StoreOp::PutBlock(block_root, signed_block.clone()));
ops.push(StoreOp::PutState(block.state_root(), &state));
if let Some(blobs) = blobs {
if let Some(blobs) = blobs? {
//FIXME(sean) using this for debugging for now
info!(self.log, "Writing blobs to store"; "block_root" => ?block_root);
ops.push(StoreOp::PutBlobs(block_root, blobs));

View File

@ -3,6 +3,7 @@ use slot_clock::SlotClock;
use crate::beacon_chain::{BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY};
use crate::{kzg_utils, BeaconChainError};
use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions;
use types::signed_beacon_block::BlobReconstructionError;
use types::{BeaconStateError, BlobsSidecar, Hash256, KzgCommitment, Slot, Transactions};
#[derive(Debug)]
@ -89,6 +90,12 @@ pub enum BlobError {
MissingBlobs,
}
impl From<BlobReconstructionError> for BlobError {
fn from(_: BlobReconstructionError) -> Self {
BlobError::MissingBlobs
}
}
impl From<BeaconChainError> for BlobError {
fn from(e: BeaconChainError) -> Self {
BlobError::BeaconChainError(e)

View File

@ -87,6 +87,7 @@ use std::time::Duration;
use store::{Error as DBError, HotStateSummary, KeyValueStore, StoreOp};
use task_executor::JoinHandle;
use tree_hash::TreeHash;
use types::signed_beacon_block::BlobReconstructionError;
use types::signed_block_and_blobs::BlockWrapper;
use types::ExecPayload;
use types::{
@ -479,6 +480,12 @@ impl<T: EthSpec> From<ArithError> for BlockError<T> {
}
}
impl<T: EthSpec> From<BlobReconstructionError> for BlockError<T> {
fn from(e: BlobReconstructionError) -> Self {
BlockError::BlobValidation(BlobError::from(e))
}
}
/// Stores information about verifying a payload against an execution engine.
pub struct PayloadVerificationOutcome {
pub payload_verification_status: PayloadVerificationStatus,
@ -905,7 +912,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
// Validate the block's execution_payload (if any).
validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?;
if let Some(blobs_sidecar) = block.blobs() {
if let Some(blobs_sidecar) = block.blobs(Some(block_root))? {
let kzg_commitments = block
.message()
.body()
@ -919,7 +926,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
.map_err(|_| BlockError::BlobValidation(BlobError::TransactionsMissing))?
.ok_or(BlockError::BlobValidation(BlobError::TransactionsMissing))?;
validate_blob_for_gossip(
blobs_sidecar,
&blobs_sidecar,
kzg_commitments,
transactions,
block.slot(),
@ -1134,12 +1141,8 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for Arc<SignedBeaconBlock
let block_root = check_block_relevancy(&self, block_root, chain)
.map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
SignatureVerifiedBlock::check_slashable(
BlockWrapper::Block { block: self },
block_root,
chain,
)?
.into_execution_pending_block_slashable(block_root, chain, notify_execution_layer)
SignatureVerifiedBlock::check_slashable(BlockWrapper::Block(self), block_root, chain)?
.into_execution_pending_block_slashable(block_root, chain, notify_execution_layer)
}
fn block(&self) -> &SignedBeaconBlock<T::EthSpec> {
@ -1563,7 +1566,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
if let Some(data_availability_boundary) = chain.data_availability_boundary() {
if block_slot.epoch(T::EthSpec::slots_per_epoch()) >= data_availability_boundary {
let sidecar = block
.blobs()
.blobs(Some(block_root))?
.ok_or(BlockError::BlobValidation(BlobError::MissingBlobs))?;
let kzg = chain.kzg.as_ref().ok_or(BlockError::BlobValidation(
BlobError::TrustedSetupNotInitialized,
@ -1586,7 +1589,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
block.slot(),
block_root,
kzg_commitments,
sidecar,
&sidecar,
)
.map_err(|e| BlockError::BlobValidation(BlobError::KzgError(e)))?
{

View File

@ -69,7 +69,7 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
},
};
let (block, blobs) = block.deconstruct();
let (block, blobs) = block.deconstruct(Some(beacon_block_root));
let item = CacheItem {
epoch,
committee_lengths,
@ -77,7 +77,7 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
source,
target,
block,
blobs,
blobs: blobs.map_err(|_| Error::MissingBlobs)?,
proto_block,
};

View File

@ -42,9 +42,7 @@ pub async fn publish_block<T: BeaconChainTypes>(
network_tx,
PubsubMessage::BeaconBlockAndBlobsSidecars(block_and_blobs.clone()),
)?;
BlockWrapper::BlockAndBlob {
block_sidecar_pair: block_and_blobs,
}
BlockWrapper::BlockAndBlob(block_and_blobs)
} else {
//FIXME(sean): This should probably return a specific no-blob-cached error code, beacon API coordination required
return Err(warp_utils::reject::broadcast_without_import(format!(
@ -53,7 +51,7 @@ pub async fn publish_block<T: BeaconChainTypes>(
}
} else {
crate::publish_pubsub_message(network_tx, PubsubMessage::BeaconBlock(block.clone()))?;
BlockWrapper::Block { block }
BlockWrapper::Block(block)
};
// Determine the delay after the start of the slot, register it with metrics.

View File

@ -1699,7 +1699,7 @@ impl<T: BeaconChainTypes> BeaconProcessor<T> {
message_id,
peer_id,
peer_client,
BlockWrapper::Block { block },
BlockWrapper::Block(block),
work_reprocessing_tx,
duplicate_cache,
seen_timestamp,
@ -1721,7 +1721,7 @@ impl<T: BeaconChainTypes> BeaconProcessor<T> {
message_id,
peer_id,
peer_client,
BlockWrapper::BlockAndBlob { block_sidecar_pair },
BlockWrapper::BlockAndBlob(block_sidecar_pair),
work_reprocessing_tx,
duplicate_cache,
seen_timestamp,

View File

@ -191,11 +191,9 @@ impl<T: BeaconChainTypes> Worker<T> {
let unwrapped = downloaded_blocks
.into_iter()
.map(|block| match block {
BlockWrapper::Block { block } => block,
BlockWrapper::Block(block) => block,
//FIXME(sean) handle blobs in backfill
BlockWrapper::BlockAndBlob {
block_sidecar_pair: _,
} => todo!(),
BlockWrapper::BlockAndBlob(_) => todo!(),
})
.collect();

View File

@ -51,16 +51,14 @@ impl<T: EthSpec> BlockBlobRequestInfo<T> {
{
let blobs_sidecar =
accumulated_sidecars.pop_front().ok_or("missing sidecar")?;
Ok(BlockWrapper::BlockAndBlob {
block_sidecar_pair: SignedBeaconBlockAndBlobsSidecar {
Ok(BlockWrapper::BlockAndBlob(
SignedBeaconBlockAndBlobsSidecar {
beacon_block,
blobs_sidecar,
},
})
))
} else {
Ok(BlockWrapper::Block {
block: beacon_block,
})
Ok(BlockWrapper::Block(beacon_block))
}
})
.collect::<Result<Vec<_>, _>>();

View File

@ -734,14 +734,14 @@ impl<T: BeaconChainTypes> SyncManager<T> {
RequestId::SingleBlock { id } => self.block_lookups.single_block_lookup_response(
id,
peer_id,
beacon_block.map(|block| BlockWrapper::Block { block }),
beacon_block.map(|block| BlockWrapper::Block(block)),
seen_timestamp,
&mut self.network,
),
RequestId::ParentLookup { id } => self.block_lookups.parent_lookup_response(
id,
peer_id,
beacon_block.map(|block| BlockWrapper::Block { block }),
beacon_block.map(|block| BlockWrapper::Block(block)),
seen_timestamp,
&mut self.network,
),
@ -756,7 +756,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
batch_id,
&peer_id,
id,
beacon_block.map(|block| BlockWrapper::Block { block }),
beacon_block.map(|block| BlockWrapper::Block(block)),
) {
Ok(ProcessResult::SyncCompleted) => self.update_sync_state(),
Ok(ProcessResult::Successful) => {}
@ -780,7 +780,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
chain_id,
batch_id,
id,
beacon_block.map(|block| BlockWrapper::Block { block }),
beacon_block.map(|block| BlockWrapper::Block(block)),
);
self.update_sync_state();
}
@ -930,9 +930,11 @@ impl<T: BeaconChainTypes> SyncManager<T> {
RequestId::SingleBlock { id } => self.block_lookups.single_block_lookup_response(
id,
peer_id,
block_sidecar_pair.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob {
// TODO: why is this in an arc
block_sidecar_pair: (*block_sidecar_pair).clone(),
block_sidecar_pair.map(|block_sidecar_pair| {
BlockWrapper::BlockAndBlob(
// TODO: why is this in an arc
(*block_sidecar_pair).clone(),
)
}),
seen_timestamp,
&mut self.network,
@ -940,9 +942,11 @@ impl<T: BeaconChainTypes> SyncManager<T> {
RequestId::ParentLookup { id } => self.block_lookups.parent_lookup_response(
id,
peer_id,
block_sidecar_pair.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob {
// TODO: why is this in an arc
block_sidecar_pair: (*block_sidecar_pair).clone(),
block_sidecar_pair.map(|block_sidecar_pair| {
BlockWrapper::BlockAndBlob(
// TODO: why is this in an arc
(*block_sidecar_pair).clone(),
)
}),
seen_timestamp,
&mut self.network,

View File

@ -26,11 +26,11 @@ impl<T: EthSpec> BatchTy<T> {
match self {
BatchTy::Blocks(blocks) => blocks
.into_iter()
.map(|block| BlockWrapper::Block { block })
.map(|block| BlockWrapper::Block(block))
.collect(),
BatchTy::BlocksAndBlobs(block_sidecar_pair) => block_sidecar_pair
.into_iter()
.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob { block_sidecar_pair })
.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob(block_sidecar_pair))
.collect(),
}
}
@ -415,7 +415,7 @@ impl<T: EthSpec, B: BatchConfig> BatchInfo<T, B> {
match self.batch_type {
ExpectedBatchTy::OnlyBlockBlobs => {
let blocks = blocks.into_iter().map(|block| {
let BlockWrapper::BlockAndBlob { block_sidecar_pair: block_and_blob } = block else {
let BlockWrapper::BlockAndBlob(block_and_blob) = block else {
panic!("Batches should never have a mixed type. This is a bug. Contact D")
};
block_and_blob
@ -424,7 +424,7 @@ impl<T: EthSpec, B: BatchConfig> BatchInfo<T, B> {
}
ExpectedBatchTy::OnlyBlock => {
let blocks = blocks.into_iter().map(|block| {
let BlockWrapper::Block { block } = block else {
let BlockWrapper::Block(block) = block else {
panic!("Batches should never have a mixed type. This is a bug. Contact D")
};
block
@ -559,7 +559,7 @@ impl<T: EthSpec, B: BatchConfig> slog::KV for BatchInfo<T, B> {
serializer.emit_usize("processed", self.failed_processing_attempts.len())?;
serializer.emit_u8("processed_no_penalty", self.non_faulty_processing_attempts)?;
serializer.emit_arguments("state", &format_args!("{:?}", self.state))?;
serializer.emit_arguments("batch_ty", &format_args!("{}", self.batch_type));
serializer.emit_arguments("batch_ty", &format_args!("{}", self.batch_type))?;
slog::Result::Ok(())
}
}

View File

@ -1,5 +1,6 @@
use crate::test_utils::TestRandom;
use crate::{Blob, EthSpec, Hash256, SignedRoot, Slot};
use derivative::Derivative;
use kzg::KzgProof;
use serde_derive::{Deserialize, Serialize};
use ssz::Encode;
@ -10,9 +11,10 @@ use tree_hash_derive::TreeHash;
#[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
#[derive(
Debug, Clone, Serialize, Deserialize, Encode, Decode, TreeHash, PartialEq, Default, TestRandom,
Debug, Clone, Serialize, Deserialize, Encode, Decode, TreeHash, Default, TestRandom, Derivative,
)]
#[serde(bound = "T: EthSpec")]
#[derivative(PartialEq, Hash(bound = "T: EthSpec"))]
pub struct BlobsSidecar<T: EthSpec> {
pub beacon_block_root: Hash256,
pub beacon_block_slot: Slot,
@ -28,6 +30,15 @@ impl<T: EthSpec> BlobsSidecar<T> {
Self::default()
}
pub fn empty_from_parts(beacon_block_root: Hash256, beacon_block_slot: Slot) -> Self {
Self {
beacon_block_root,
beacon_block_slot,
blobs: VariableList::empty(),
kzg_aggregated_proof: KzgProof::empty(),
}
}
#[allow(clippy::integer_arithmetic)]
pub fn max_size() -> usize {
// Fixed part

View File

@ -36,6 +36,10 @@ impl From<SignedBeaconBlockHash> for Hash256 {
}
}
pub enum BlobReconstructionError {
BlobsMissing,
}
/// A `BeaconBlock` and a signature from its proposer.
#[superstruct(
variants(Base, Altair, Merge, Capella, Eip4844),
@ -235,6 +239,29 @@ impl<E: EthSpec, Payload: AbstractExecPayload<E>> SignedBeaconBlock<E, Payload>
pub fn canonical_root(&self) -> Hash256 {
self.message().tree_hash_root()
}
/// Reconstructs an empty `BlobsSidecar`, using the given block root if provided, else calculates it.
/// If this block has kzg commitments, an error will be returned. If this block is from prior to the
/// Eip4844 fork, `None` will be returned.
pub fn reconstruct_empty_blobs(
&self,
block_root_opt: Option<Hash256>,
) -> Result<Option<BlobsSidecar<E>>, BlobReconstructionError> {
self.message()
.body()
.blob_kzg_commitments()
.map(|kzg_commitments| {
if kzg_commitments.len() > 0 {
Err(BlobReconstructionError::BlobsMissing)
} else {
Ok(Some(BlobsSidecar::empty_from_parts(
block_root_opt.unwrap_or(self.canonical_root()),
self.slot(),
)))
}
})
.unwrap_or(Ok(None))
}
}
// We can convert pre-Bellatrix blocks without payloads into blocks with payloads.

View File

@ -1,4 +1,6 @@
use crate::signed_beacon_block::BlobReconstructionError;
use crate::{BlobsSidecar, EthSpec, Hash256, SignedBeaconBlock, SignedBeaconBlockEip4844, Slot};
use derivative::Derivative;
use serde_derive::{Deserialize, Serialize};
use ssz::{Decode, DecodeError};
use ssz_derive::{Decode, Encode};
@ -12,8 +14,8 @@ pub struct SignedBeaconBlockAndBlobsSidecarDecode<T: EthSpec> {
pub blobs_sidecar: BlobsSidecar<T>,
}
#[derive(Debug, Clone, Serialize, Deserialize, Encode, TreeHash, PartialEq)]
#[serde(bound = "T: EthSpec")]
#[derive(Debug, Clone, Serialize, Deserialize, Encode, TreeHash, Derivative)]
#[derivative(PartialEq, Hash(bound = "T: EthSpec"))]
pub struct SignedBeaconBlockAndBlobsSidecar<T: EthSpec> {
pub beacon_block: Arc<SignedBeaconBlock<T>>,
pub blobs_sidecar: Arc<BlobsSidecar<T>>,
@ -33,62 +35,55 @@ impl<T: EthSpec> SignedBeaconBlockAndBlobsSidecar<T> {
}
/// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`].
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Derivative)]
#[derivative(PartialEq, Hash(bound = "T: EthSpec"))]
pub enum BlockWrapper<T: EthSpec> {
Block {
block: Arc<SignedBeaconBlock<T>>,
},
BlockAndBlob {
block_sidecar_pair: SignedBeaconBlockAndBlobsSidecar<T>,
},
Block(Arc<SignedBeaconBlock<T>>),
BlockAndBlob(SignedBeaconBlockAndBlobsSidecar<T>),
}
impl<T: EthSpec> BlockWrapper<T> {
pub fn slot(&self) -> Slot {
match self {
BlockWrapper::Block { block } => block.slot(),
BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
BlockWrapper::Block(block) => block.slot(),
BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
block_sidecar_pair.beacon_block.slot()
}
}
}
pub fn block(&self) -> &SignedBeaconBlock<T> {
match self {
BlockWrapper::Block { block } => &block,
BlockWrapper::BlockAndBlob { block_sidecar_pair } => &block_sidecar_pair.beacon_block,
BlockWrapper::Block(block) => &block,
BlockWrapper::BlockAndBlob(block_sidecar_pair) => &block_sidecar_pair.beacon_block,
}
}
pub fn block_cloned(&self) -> Arc<SignedBeaconBlock<T>> {
match self {
BlockWrapper::Block { block } => block.clone(),
BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
BlockWrapper::Block(block) => block.clone(),
BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
block_sidecar_pair.beacon_block.clone()
}
}
}
pub fn blobs(&self) -> Option<&BlobsSidecar<T>> {
pub fn blobs(
&self,
block_root: Option<Hash256>,
) -> Result<Option<Arc<BlobsSidecar<T>>>, BlobReconstructionError> {
match self {
BlockWrapper::Block { .. } => None,
BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
Some(&block_sidecar_pair.blobs_sidecar)
}
}
}
pub fn blobs_cloned(&self) -> Option<Arc<BlobsSidecar<T>>> {
match self {
BlockWrapper::Block { block: _ } => None,
BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
Some(block_sidecar_pair.blobs_sidecar.clone())
BlockWrapper::Block(block) => block
.reconstruct_empty_blobs(block_root)
.map(|blob_opt| blob_opt.map(Arc::new)),
BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
Ok(Some(block_sidecar_pair.blobs_sidecar.clone()))
}
}
}
pub fn message(&self) -> crate::BeaconBlockRef<T> {
match self {
BlockWrapper::Block { block } => block.message(),
BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
BlockWrapper::Block(block) => block.message(),
BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
block_sidecar_pair.beacon_block.message()
}
}
@ -98,43 +93,39 @@ impl<T: EthSpec> BlockWrapper<T> {
self.block().parent_root()
}
pub fn deconstruct(self) -> (Arc<SignedBeaconBlock<T>>, Option<Arc<BlobsSidecar<T>>>) {
pub fn deconstruct(
self,
block_root: Option<Hash256>,
) -> (
Arc<SignedBeaconBlock<T>>,
Result<Option<Arc<BlobsSidecar<T>>>, BlobReconstructionError>,
) {
match self {
BlockWrapper::Block { block } => (block, None),
BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
BlockWrapper::Block(block) => {
let blobs = block
.reconstruct_empty_blobs(block_root)
.map(|blob_opt| blob_opt.map(Arc::new));
(block, blobs)
}
BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
let SignedBeaconBlockAndBlobsSidecar {
beacon_block,
blobs_sidecar,
} = block_sidecar_pair;
(beacon_block, Some(blobs_sidecar))
(beacon_block, Ok(Some(blobs_sidecar)))
}
}
}
}
// TODO: probably needes to be changed. This is needed because SignedBeaconBlockAndBlobsSidecar
// does not implement Hash
impl<T: EthSpec> std::hash::Hash for BlockWrapper<T> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
match self {
BlockWrapper::Block { block } => block.hash(state),
BlockWrapper::BlockAndBlob {
block_sidecar_pair: block_and_blob,
} => block_and_blob.beacon_block.hash(state),
}
}
}
impl<T: EthSpec> From<SignedBeaconBlock<T>> for BlockWrapper<T> {
fn from(block: SignedBeaconBlock<T>) -> Self {
BlockWrapper::Block {
block: Arc::new(block),
}
BlockWrapper::Block(Arc::new(block))
}
}
impl<T: EthSpec> From<Arc<SignedBeaconBlock<T>>> for BlockWrapper<T> {
fn from(block: Arc<SignedBeaconBlock<T>>) -> Self {
BlockWrapper::Block { block }
BlockWrapper::Block(block)
}
}

View File

@ -12,6 +12,14 @@ const KZG_PROOF_BYTES_LEN: usize = 48;
#[ssz(struct_behaviour = "transparent")]
pub struct KzgProof(pub [u8; KZG_PROOF_BYTES_LEN]);
impl KzgProof {
pub fn empty() -> Self {
let mut bytes = [0; KZG_PROOF_BYTES_LEN];
bytes[0] = 192;
Self(bytes)
}
}
impl fmt::Display for KzgProof {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", eth2_serde_utils::hex::encode(self.0))