Skip to content
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.

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

Drop verify single RpcBlock fns #7008

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 2 additions & 43 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,15 +1254,8 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for Arc<SignedBeaconBlock
// Perform an early check to prevent wasting time on irrelevant blocks.
let block_root = check_block_relevancy(&self, block_root, chain)
.map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
let maybe_available = chain
.data_availability_checker
.verify_kzg_for_rpc_block(RpcBlock::new_without_blobs(Some(block_root), self.clone()))
.map_err(|e| {
BlockSlashInfo::SignatureNotChecked(
self.signed_block_header(),
BlockError::AvailabilityCheck(e),
)
})?;
let maybe_available =
MaybeAvailableBlock::from_block(self.clone(), block_root, chain.spec.clone());
SignatureVerifiedBlock::check_slashable(maybe_available, block_root, chain)?
.into_execution_pending_block_slashable(block_root, chain, notify_execution_layer)
}
Expand All @@ -1276,40 +1269,6 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for Arc<SignedBeaconBlock
}
}

impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for RpcBlock<T::EthSpec> {
/// Verifies the `SignedBeaconBlock` by first transforming it into a `SignatureVerifiedBlock`
/// and then using that implementation of `IntoExecutionPendingBlock` to complete verification.
fn into_execution_pending_block_slashable(
self,
block_root: Hash256,
chain: &Arc<BeaconChain<T>>,
notify_execution_layer: NotifyExecutionLayer,
) -> Result<ExecutionPendingBlock<T>, BlockSlashInfo<BlockError>> {
// Perform an early check to prevent wasting time on irrelevant blocks.
let block_root = check_block_relevancy(self.as_block(), block_root, chain)
.map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
let maybe_available = chain
.data_availability_checker
.verify_kzg_for_rpc_block(self.clone())
.map_err(|e| {
BlockSlashInfo::SignatureNotChecked(
self.signed_block_header(),
BlockError::AvailabilityCheck(e),
)
})?;
SignatureVerifiedBlock::check_slashable(maybe_available, block_root, chain)?
.into_execution_pending_block_slashable(block_root, chain, notify_execution_layer)
}

fn block(&self) -> &SignedBeaconBlock<T::EthSpec> {
self.as_block()
}

fn block_cloned(&self) -> Arc<SignedBeaconBlock<T::EthSpec>> {
self.block_cloned()
}
}

impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
/// Instantiates `Self`, a wrapper that indicates that the given `block` is fully valid. See
/// the struct-level documentation for more information.
Expand Down
82 changes: 19 additions & 63 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,69 +327,6 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
.remove_pending_components(block_root)
}

/// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may
/// include the fully available block.
///
/// WARNING: This function assumes all required blobs are already present, it does NOT
/// check if there are any missing blobs.
pub fn verify_kzg_for_rpc_block(
&self,
block: RpcBlock<T::EthSpec>,
) -> Result<MaybeAvailableBlock<T::EthSpec>, AvailabilityCheckError> {
let (block_root, block, blobs, data_columns) = block.deconstruct();
if self.blobs_required_for_block(&block) {
return if let Some(blob_list) = blobs.as_ref() {
verify_kzg_for_blob_list(blob_list.iter(), &self.kzg)
.map_err(AvailabilityCheckError::InvalidBlobs)?;
Ok(MaybeAvailableBlock::Available(AvailableBlock {
block_root,
block,
blobs,
blobs_available_timestamp: None,
data_columns: None,
spec: self.spec.clone(),
}))
} else {
Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block })
};
}
if self.data_columns_required_for_block(&block) {
return if let Some(data_column_list) = data_columns.as_ref() {
verify_kzg_for_data_column_list_with_scoring(
data_column_list
.iter()
.map(|custody_column| custody_column.as_data_column()),
&self.kzg,
)
.map_err(AvailabilityCheckError::InvalidColumn)?;
Ok(MaybeAvailableBlock::Available(AvailableBlock {
block_root,
block,
blobs: None,
blobs_available_timestamp: None,
data_columns: Some(
data_column_list
.into_iter()
.map(|d| d.clone_arc())
.collect(),
),
spec: self.spec.clone(),
}))
} else {
Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block })
};
}

Ok(MaybeAvailableBlock::Available(AvailableBlock {
block_root,
block,
blobs: None,
blobs_available_timestamp: None,
data_columns: None,
spec: self.spec.clone(),
}))
}

/// Checks if a vector of blocks are available. Returns a vector of `MaybeAvailableBlock`
/// This is more efficient than calling `verify_kzg_for_rpc_block` in a loop as it does
/// all kzg verification at once
Expand Down Expand Up @@ -802,4 +739,23 @@ impl<E: EthSpec> MaybeAvailableBlock<E> {
Self::AvailabilityPending { block, .. } => block.clone(),
}
}

pub fn from_block(
block: Arc<SignedBeaconBlock<E>>,
block_root: Hash256,
spec: Arc<ChainSpec>,
) -> Self {
if block.num_expected_blobs() == 0 {
MaybeAvailableBlock::Available(AvailableBlock {
block_root,
block,
blobs: None,
blobs_available_timestamp: None,
data_columns: None,
spec,
})
} else {
MaybeAvailableBlock::AvailabilityPending { block_root, block }
}
}
}
49 changes: 12 additions & 37 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2303,27 +2303,10 @@ where
pub async fn process_block(
&self,
slot: Slot,
block_root: Hash256,
block_contents: SignedBlockContentsTuple<E>,
) -> Result<SignedBeaconBlockHash, BlockError> {
self.set_current_slot(slot);
let (block, blob_items) = block_contents;

let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items)?;
let block_hash: SignedBeaconBlockHash = self
.chain
.process_block(
block_root,
rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::RangeSync,
|| Ok(()),
)
.await?
.try_into()
.expect("block blobs are available");
self.chain.recompute_head_at_current_slot().await;
Ok(block_hash)
self.process_block_result(block_contents).await
}

pub async fn process_block_result(
Expand All @@ -2334,20 +2317,18 @@ where

let block_root = block.canonical_root();
let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items)?;
let block_hash: SignedBeaconBlockHash = self
match self
.chain
.process_block(
block_root,
rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::RangeSync,
|| Ok(()),
)
.await?
.try_into()
.expect("block blobs are available");
.process_chain_segment(vec![rpc_block], NotifyExecutionLayer::Yes)
.await
{
crate::ChainSegmentResult::Successful { .. } => {}
crate::ChainSegmentResult::Failed { error, .. } => {
panic!("block import error {error:?}");
}
}
self.chain.recompute_head_at_current_slot().await;
Ok(block_hash)
Ok(block_root.into())
}

/// Builds an `Rpc` block from a `SignedBeaconBlock` and blobs or data columns retrieved from
Expand Down Expand Up @@ -2485,13 +2466,7 @@ where
self.set_current_slot(slot);
let (block_contents, new_state) = self.make_block(state, slot).await;

let block_hash = self
.process_block(
slot,
block_contents.0.canonical_root(),
block_contents.clone(),
)
.await?;
let block_hash = self.process_block_result(block_contents.clone()).await?;
Ok((block_hash, block_contents, new_state))
}

Expand Down
10 changes: 3 additions & 7 deletions beacon_node/beacon_chain/tests/rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,19 +310,15 @@ async fn test_rewards_base_multi_inclusion() {
// funky hack: on first try, the state root will mismatch due to our modification
// thankfully, the correct state root is reported back, so we just take that one :^)
// there probably is a better way...
let Err(BlockError::StateRootMismatch { local, .. }) = harness
.process_block(slot, block.0.canonical_root(), block.clone())
.await
let Err(BlockError::StateRootMismatch { local, .. }) =
harness.process_block(slot, block.clone()).await
else {
panic!("unexpected match of state root");
};
let mut new_block = block.0.message_base().unwrap().clone();
new_block.state_root = local;
block.0 = Arc::new(harness.sign_beacon_block(new_block.into(), &harness.get_current_state()));
harness
.process_block(slot, block.0.canonical_root(), block.clone())
.await
.unwrap();
harness.process_block(slot, block.clone()).await.unwrap();

harness
.extend_slots(E::slots_per_epoch() as usize * 2 - 4)
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/network_beacon_processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
pub fn send_rpc_beacon_block(
self: &Arc<Self>,
block_root: Hash256,
block: RpcBlock<T::EthSpec>,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
seen_timestamp: Duration,
process_type: BlockProcessType,
) -> Result<(), Error<T::EthSpec>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ use store::KzgCommitment;
use tokio::sync::mpsc;
use types::beacon_block_body::format_kzg_commitments;
use types::blob_sidecar::FixedBlobSidecarList;
use types::{BlockImportSource, DataColumnSidecar, DataColumnSidecarList, Epoch, Hash256};
use types::{
BlockImportSource, DataColumnSidecar, DataColumnSidecarList, Epoch, Hash256, SignedBeaconBlock,
};

/// Id associated to a batch processing request, either a sync batch or a parent lookup.
#[derive(Clone, Debug, PartialEq)]
Expand All @@ -52,7 +54,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
pub fn generate_rpc_beacon_block_process_fn(
self: Arc<Self>,
block_root: Hash256,
block: RpcBlock<T::EthSpec>,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
seen_timestamp: Duration,
process_type: BlockProcessType,
) -> AsyncFn {
Expand All @@ -76,7 +78,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
pub fn generate_rpc_beacon_block_fns(
self: Arc<Self>,
block_root: Hash256,
block: RpcBlock<T::EthSpec>,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
seen_timestamp: Duration,
process_type: BlockProcessType,
) -> (AsyncFn, BlockingFn) {
Expand All @@ -103,7 +105,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
pub async fn process_rpc_block(
self: Arc<NetworkBeaconProcessor<T>>,
block_root: Hash256,
block: RpcBlock<T::EthSpec>,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
seen_timestamp: Duration,
process_type: BlockProcessType,
reprocess_tx: mpsc::Sender<ReprocessQueueMessage>,
Expand Down
10 changes: 2 additions & 8 deletions beacon_node/network/src/sync/block_lookups/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::sync::block_lookups::{
};
use crate::sync::manager::BlockProcessType;
use crate::sync::network_context::{LookupRequestResult, SyncNetworkContext};
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::BeaconChainTypes;
use lighthouse_network::service::api_types::Id;
use parking_lot::RwLock;
Expand Down Expand Up @@ -97,13 +96,8 @@ impl<T: BeaconChainTypes> RequestState<T> for BlockRequestState<T::EthSpec> {
seen_timestamp,
..
} = download_result;
cx.send_block_for_processing(
id,
block_root,
RpcBlock::new_without_blobs(Some(block_root), value),
seen_timestamp,
)
.map_err(LookupRequestError::SendFailedProcessor)
cx.send_block_for_processing(id, block_root, value, seen_timestamp)
.map_err(LookupRequestError::SendFailedProcessor)
}

fn response_type() -> ResponseType {
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/sync/network_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
&self,
id: Id,
block_root: Hash256,
block: RpcBlock<T::EthSpec>,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
seen_timestamp: Duration,
) -> Result<(), SendErrorProcessor> {
let beacon_processor = self
Expand Down
Loading