add processing and processed caching to the DA checker (#4732)

* add processing and processed caching to the DA checker

* move processing cache out of critical cache

* get it compiling

* fix lints

* add docs to `AvailabilityView`

* some self review

* fix lints

* fix beacon chain tests

* cargo fmt

* make availability view easier to implement, start on testing

* move child component cache and finish test

* cargo fix

* cargo fix

* cargo fix

* fmt and lint

* make blob commitments not optional, rename some caches, add missing blobs struct

* Update beacon_node/beacon_chain/src/data_availability_checker/processing_cache.rs

Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com>

* marks review feedback and other general cleanup

* cargo fix

* improve availability view docs

* some renames

* some renames and docs

* fix should delay lookup logic

* get rid of some wrapper methods

* fix up single lookup changes

* add a couple docs

* add single blob merge method and improve process_... docs

* update some names

* lints

* fix merge

* remove blob indices from lookup creation log

* remove blob indices from lookup creation log

* delayed lookup logging improvement

* check fork choice before doing any blob processing

* remove unused dep

* Update beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

* Update beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

* Update beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

* Update beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

* Update beacon_node/network/src/sync/block_lookups/delayed_lookup.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

* remove duplicate deps

* use gen range in random blobs geneartor

* rename processing cache fields

* require block root in rpc block construction and check block root consistency

* send peers as vec in single message

* spawn delayed lookup service from network beacon processor

* fix tests

---------

Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com>
Co-authored-by: Michael Sproul <micsproul@gmail.com>
This commit is contained in:
realbigsean
2023-10-03 09:59:33 -04:00
committed by GitHub
parent 67aeb6bf6b
commit c7ddf1f0b1
38 changed files with 1894 additions and 1190 deletions

View File

@@ -8,8 +8,8 @@ use crate::sync::block_lookups::{
};
use crate::sync::manager::{BlockProcessType, Id, SingleLookupReqId};
use crate::sync::network_context::SyncNetworkContext;
use crate::sync::CachedChildComponents;
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::data_availability_checker::{AvailabilityView, ChildComponents};
use beacon_chain::{get_block_root, BeaconChainTypes};
use lighthouse_network::rpc::methods::BlobsByRootRequest;
use lighthouse_network::rpc::BlocksByRootRequest;
@@ -19,7 +19,7 @@ use ssz_types::VariableList;
use std::ops::IndexMut;
use std::sync::Arc;
use std::time::Duration;
use types::blob_sidecar::FixedBlobSidecarList;
use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList};
use types::{BlobSidecar, EthSpec, Hash256, SignedBeaconBlock};
#[derive(Debug, Copy, Clone)]
@@ -222,11 +222,12 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
/// triggered by `UnknownParent` errors.
fn add_to_child_components(
verified_response: Self::VerifiedResponseType,
components: &mut CachedChildComponents<T::EthSpec>,
components: &mut ChildComponents<T::EthSpec>,
);
/// Convert a verified response to the type we send to the beacon processor.
fn verified_to_reconstructed(
block_root: Hash256,
verified: Self::VerifiedResponseType,
) -> Self::ReconstructedResponseType;
@@ -326,15 +327,16 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlockRequestState<L>
fn add_to_child_components(
verified_response: Arc<SignedBeaconBlock<T::EthSpec>>,
components: &mut CachedChildComponents<T::EthSpec>,
components: &mut ChildComponents<T::EthSpec>,
) {
components.add_cached_child_block(verified_response);
components.merge_block(verified_response);
}
fn verified_to_reconstructed(
block_root: Hash256,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
) -> RpcBlock<T::EthSpec> {
RpcBlock::new_without_blobs(block)
RpcBlock::new_without_blobs(Some(block_root), block)
}
fn send_reconstructed_for_processing(
@@ -375,9 +377,9 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
type ReconstructedResponseType = FixedBlobSidecarList<T::EthSpec>;
fn new_request(&self) -> BlobsByRootRequest {
BlobsByRootRequest {
blob_ids: self.requested_ids.clone().into(),
}
let blob_id_vec: Vec<BlobIdentifier> = self.requested_ids.clone().into();
let blob_ids = VariableList::from(blob_id_vec);
BlobsByRootRequest { blob_ids }
}
fn make_request(
@@ -432,12 +434,13 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
fn add_to_child_components(
verified_response: FixedBlobSidecarList<T::EthSpec>,
components: &mut CachedChildComponents<T::EthSpec>,
components: &mut ChildComponents<T::EthSpec>,
) {
components.add_cached_child_blobs(verified_response);
components.merge_blobs(verified_response);
}
fn verified_to_reconstructed(
_block_root: Hash256,
blobs: FixedBlobSidecarList<T::EthSpec>,
) -> FixedBlobSidecarList<T::EthSpec> {
blobs

View File

@@ -1,81 +0,0 @@
use crate::network_beacon_processor::NetworkBeaconProcessor;
use beacon_chain::{BeaconChain, BeaconChainTypes};
use slog::crit;
use slot_clock::SlotClock;
use std::sync::Arc;
use tokio::sync::mpsc;
use tokio::time::interval_at;
use tokio::time::Instant;
use types::Hash256;
#[derive(Debug)]
pub enum DelayedLookupMessage {
/// A lookup for all components of a block or blob seen over gossip.
MissingComponents(Hash256),
}
/// This service is responsible for collecting lookup messages and sending them back to sync
/// for processing after a short delay.
///
/// We want to delay lookups triggered from gossip for the following reasons:
///
/// - We only want to make one request for components we are unlikely to see on gossip. This means
/// we don't have to repeatedly update our RPC request's state as we receive gossip components.
///
/// - We are likely to receive blocks/blobs over gossip more quickly than we could via an RPC request.
///
/// - Delaying a lookup means we are less likely to simultaneously download the same blocks/blobs
/// over gossip and RPC.
///
/// - We would prefer to request peers based on whether we've seen them attest, because this gives
/// us an idea about whether they *should* have the block/blobs we're missing. This is because a
/// node should not attest to a block unless it has all the blobs for that block. This gives us a
/// stronger basis for peer scoring.
pub fn spawn_delayed_lookup_service<T: BeaconChainTypes>(
executor: &task_executor::TaskExecutor,
beacon_chain: Arc<BeaconChain<T>>,
mut delayed_lookups_recv: mpsc::Receiver<DelayedLookupMessage>,
beacon_processor: Arc<NetworkBeaconProcessor<T>>,
log: slog::Logger,
) {
executor.spawn(
async move {
let slot_duration = beacon_chain.slot_clock.slot_duration();
let delay = beacon_chain.slot_clock.single_lookup_delay();
let interval_start = match (
beacon_chain.slot_clock.duration_to_next_slot(),
beacon_chain.slot_clock.seconds_from_current_slot_start(),
) {
(Some(duration_to_next_slot), Some(seconds_from_current_slot_start)) => {
let duration_until_start = if seconds_from_current_slot_start > delay {
duration_to_next_slot + delay
} else {
delay - seconds_from_current_slot_start
};
Instant::now() + duration_until_start
}
_ => {
crit!(log,
"Failed to read slot clock, delayed lookup service timing will be inaccurate.\
This may degrade performance"
);
Instant::now()
}
};
let mut interval = interval_at(interval_start, slot_duration);
loop {
interval.tick().await;
while let Ok(msg) = delayed_lookups_recv.try_recv() {
match msg {
DelayedLookupMessage::MissingComponents(block_root) => {
beacon_processor
.send_delayed_lookup(block_root)
}
}
}
}
},
"delayed_lookups",
);
}

View File

@@ -12,6 +12,7 @@ use crate::sync::block_lookups::single_block_lookup::{
};
use crate::sync::manager::{Id, SingleLookupReqId};
use beacon_chain::block_verification_types::{AsBlock, RpcBlock};
pub use beacon_chain::data_availability_checker::ChildComponents;
use beacon_chain::data_availability_checker::{AvailabilityCheckError, DataAvailabilityChecker};
use beacon_chain::validator_monitor::timestamp_now;
use beacon_chain::{AvailabilityProcessingStatus, BeaconChainTypes, BlockError};
@@ -23,7 +24,6 @@ use fnv::FnvHashMap;
use lighthouse_network::rpc::RPCError;
use lighthouse_network::{PeerAction, PeerId};
use lru_cache::LRUTimeCache;
pub use single_block_lookup::CachedChildComponents;
pub use single_block_lookup::{BlobRequestState, BlockRequestState};
use slog::{debug, error, trace, warn, Logger};
use smallvec::SmallVec;
@@ -37,7 +37,6 @@ use types::blob_sidecar::FixedBlobSidecarList;
use types::Slot;
pub mod common;
pub(crate) mod delayed_lookup;
mod parent_lookup;
mod single_block_lookup;
#[cfg(test)]
@@ -122,34 +121,10 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
pub fn search_block(
&mut self,
block_root: Hash256,
peer_source: PeerShouldHave,
peer_source: &[PeerShouldHave],
cx: &mut SyncNetworkContext<T>,
) {
let lookup = self.new_current_lookup(block_root, None, &[peer_source], cx);
if let Some(lookup) = lookup {
let msg = "Searching for block";
lookup_creation_logging(msg, &lookup, peer_source, &self.log);
self.trigger_single_lookup(lookup, cx);
}
}
/// Creates a lookup for the block with the given `block_root`.
///
/// The request is not immediately triggered, and should be triggered by a call to
/// `trigger_lookup_by_root`.
pub fn search_block_delayed(
&mut self,
block_root: Hash256,
peer_source: PeerShouldHave,
cx: &mut SyncNetworkContext<T>,
) {
let lookup = self.new_current_lookup(block_root, None, &[peer_source], cx);
if let Some(lookup) = lookup {
let msg = "Initialized delayed lookup for block";
lookup_creation_logging(msg, &lookup, peer_source, &self.log);
self.add_single_lookup(lookup)
}
self.new_current_lookup(block_root, None, peer_source, cx)
}
/// Creates a lookup for the block with the given `block_root`, while caching other block
@@ -161,44 +136,11 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
pub fn search_child_block(
&mut self,
block_root: Hash256,
child_components: CachedChildComponents<T::EthSpec>,
peer_source: PeerShouldHave,
child_components: ChildComponents<T::EthSpec>,
peer_source: &[PeerShouldHave],
cx: &mut SyncNetworkContext<T>,
) {
if child_components.is_missing_components() {
let lookup =
self.new_current_lookup(block_root, Some(child_components), &[peer_source], cx);
if let Some(lookup) = lookup {
let msg = "Searching for components of a block with unknown parent";
lookup_creation_logging(msg, &lookup, peer_source, &self.log);
self.trigger_single_lookup(lookup, cx);
}
}
}
/// Creates a lookup for the block with the given `block_root`, while caching other block
/// components we've already received. The block components are cached here because we haven't
/// imported it's parent and therefore can't fully validate it and store it in the data
/// availability cache.
///
/// The request is not immediately triggered, and should be triggered by a call to
/// `trigger_lookup_by_root`.
pub fn search_child_delayed(
&mut self,
block_root: Hash256,
child_components: CachedChildComponents<T::EthSpec>,
peer_source: PeerShouldHave,
cx: &mut SyncNetworkContext<T>,
) {
if child_components.is_missing_components() {
let lookup =
self.new_current_lookup(block_root, Some(child_components), &[peer_source], cx);
if let Some(lookup) = lookup {
let msg = "Initialized delayed lookup for block with unknown parent";
lookup_creation_logging(msg, &lookup, peer_source, &self.log);
self.add_single_lookup(lookup)
}
}
self.new_current_lookup(block_root, Some(child_components), peer_source, cx)
}
/// Attempts to trigger the request matching the given `block_root`.
@@ -230,47 +172,15 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
);
}
/// Trigger any lookups that are waiting for the given `block_root`.
pub fn trigger_lookup_by_root(&mut self, block_root: Hash256, cx: &SyncNetworkContext<T>) {
self.single_block_lookups.retain(|_id, lookup| {
if lookup.block_root() == block_root {
if lookup.da_checker.is_deneb() {
let blob_indices = lookup.blob_request_state.requested_ids.indices();
debug!(
self.log,
"Triggering delayed single lookup";
"block" => ?block_root,
"blob_indices" => ?blob_indices
);
} else {
debug!(
self.log,
"Triggering delayed single lookup";
"block" => ?block_root,
);
}
if let Err(e) = lookup.request_block_and_blobs(cx) {
debug!(self.log, "Delayed single block lookup failed";
"error" => ?e,
"block_root" => ?block_root,
);
return false;
}
}
true
});
}
/// Searches for a single block hash. If the blocks parent is unknown, a chain of blocks is
/// constructed.
pub fn new_current_lookup(
&mut self,
block_root: Hash256,
child_components: Option<CachedChildComponents<T::EthSpec>>,
child_components: Option<ChildComponents<T::EthSpec>>,
peers: &[PeerShouldHave],
cx: &mut SyncNetworkContext<T>,
) -> Option<SingleBlockLookup<Current, T>> {
) {
// Do not re-request a block that is already being requested
if let Some((_, lookup)) = self
.single_block_lookups
@@ -281,7 +191,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
if let Some(components) = child_components {
lookup.add_child_components(components);
}
return None;
return;
}
if let Some(parent_lookup) = self.parent_lookups.iter_mut().find(|parent_req| {
@@ -291,7 +201,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
// If the block was already downloaded, or is being downloaded in this moment, do not
// request it.
return None;
return;
}
if self
@@ -300,16 +210,30 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
.any(|(hashes, _last_parent_request)| hashes.contains(&block_root))
{
// we are already processing this block, ignore it.
return None;
return;
}
Some(SingleBlockLookup::new(
let msg = if child_components.is_some() {
"Searching for components of a block with unknown parent"
} else {
"Searching for block components"
};
let lookup = SingleBlockLookup::new(
block_root,
child_components,
peers,
self.da_checker.clone(),
cx.next_id(),
))
);
debug!(
self.log,
"{}", msg;
"peer_ids" => ?peers,
"block" => ?block_root,
);
self.trigger_single_lookup(lookup, cx);
}
/// If a block is attempted to be processed but we do not know its parent, this function is
@@ -337,7 +261,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
if let Some(parent_lookup) = self.parent_lookups.iter_mut().find(|parent_req| {
parent_req.contains_block(&block_root) || parent_req.is_for_block(block_root)
}) {
parent_lookup.add_peers(&[peer_source]);
parent_lookup.add_peer(peer_source);
// we are already searching for this block, ignore it
return;
}
@@ -540,7 +464,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
id,
self,
block_root,
R::verified_to_reconstructed(verified_response),
R::verified_to_reconstructed(block_root, verified_response),
seen_timestamp,
cx,
)?,
@@ -975,9 +899,9 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
AvailabilityCheckError::KzgNotInitialized
| AvailabilityCheckError::SszTypes(_)
| AvailabilityCheckError::MissingBlobs
| AvailabilityCheckError::UnorderedBlobs { .. }
| AvailabilityCheckError::StoreError(_)
| AvailabilityCheckError::DecodeError(_) => {
| AvailabilityCheckError::DecodeError(_)
| AvailabilityCheckError::Unexpected => {
warn!(self.log, "Internal availability check failure"; "root" => %root, "peer_id" => %peer_id, "error" => ?e);
lookup
.block_request_state
@@ -990,20 +914,12 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
lookup.request_block_and_blobs(cx)?
}
// Invalid block and blob comparison.
AvailabilityCheckError::NumBlobsMismatch { .. }
| AvailabilityCheckError::KzgCommitmentMismatch { .. }
| AvailabilityCheckError::BlockBlobRootMismatch { .. }
| AvailabilityCheckError::BlockBlobSlotMismatch { .. } => {
warn!(self.log, "Availability check failure in consistency"; "root" => %root, "peer_id" => %peer_id, "error" => ?e);
lookup.handle_consistency_failure(cx);
lookup.request_block_and_blobs(cx)?
}
// Malicious errors.
AvailabilityCheckError::Kzg(_)
| AvailabilityCheckError::BlobIndexInvalid(_)
| AvailabilityCheckError::KzgVerificationFailed => {
| AvailabilityCheckError::KzgCommitmentMismatch { .. }
| AvailabilityCheckError::KzgVerificationFailed
| AvailabilityCheckError::InconsistentBlobBlockRoots { .. } => {
warn!(self.log, "Availability check failure"; "root" => %root, "peer_id" => %peer_id, "error" => ?e);
lookup.handle_availability_check_failure(cx);
lookup.request_block_and_blobs(cx)?
@@ -1480,29 +1396,3 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
self.parent_lookups.drain(..).len()
}
}
fn lookup_creation_logging<L: Lookup, T: BeaconChainTypes>(
msg: &str,
lookup: &SingleBlockLookup<L, T>,
peer_source: PeerShouldHave,
log: &Logger,
) {
let block_root = lookup.block_root();
if lookup.da_checker.is_deneb() {
let blob_indices = lookup.blob_request_state.requested_ids.indices();
debug!(
log,
"{}", msg;
"peer_id" => ?peer_source,
"block" => ?block_root,
"blob_indices" => ?blob_indices
);
} else {
debug!(
log,
"{}", msg;
"peer_id" => ?peer_source,
"block" => ?block_root,
);
}
}

View File

@@ -5,7 +5,7 @@ use crate::sync::block_lookups::common::RequestState;
use crate::sync::{manager::SLOT_IMPORT_TOLERANCE, network_context::SyncNetworkContext};
use beacon_chain::block_verification_types::AsBlock;
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::data_availability_checker::DataAvailabilityChecker;
use beacon_chain::data_availability_checker::{ChildComponents, DataAvailabilityChecker};
use beacon_chain::BeaconChainTypes;
use itertools::Itertools;
use lighthouse_network::PeerId;
@@ -67,7 +67,7 @@ impl<T: BeaconChainTypes> ParentLookup<T> {
) -> Self {
let current_parent_request = SingleBlockLookup::new(
parent_root,
Some(<_>::default()),
Some(ChildComponents::empty(block_root)),
&[peer_id],
da_checker,
cx.next_id(),
@@ -179,7 +179,7 @@ impl<T: BeaconChainTypes> ParentLookup<T> {
.blob_request_state
.state
.register_failure_processing();
if let Some(components) = self.current_parent_request.cached_child_components.as_mut() {
if let Some(components) = self.current_parent_request.child_components.as_mut() {
components.downloaded_block = None;
components.downloaded_blobs = <_>::default();
}
@@ -211,8 +211,13 @@ impl<T: BeaconChainTypes> ParentLookup<T> {
Ok(root_and_verified)
}
pub fn add_peers(&mut self, peer_source: &[PeerShouldHave]) {
self.current_parent_request.add_peers(peer_source)
pub fn add_peer(&mut self, peer: PeerShouldHave) {
self.current_parent_request.add_peer(peer)
}
/// Adds a list of peers to the parent request.
pub fn add_peers(&mut self, peers: &[PeerShouldHave]) {
self.current_parent_request.add_peers(peers)
}
pub fn used_peers(&self) -> impl Iterator<Item = &PeerId> + '_ {

View File

@@ -3,20 +3,21 @@ use crate::sync::block_lookups::common::{Lookup, RequestState};
use crate::sync::block_lookups::Id;
use crate::sync::network_context::SyncNetworkContext;
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::data_availability_checker::{AvailabilityCheckError, DataAvailabilityChecker};
use beacon_chain::data_availability_checker::{
AvailabilityCheckError, DataAvailabilityChecker, MissingBlobs,
};
use beacon_chain::data_availability_checker::{AvailabilityView, ChildComponents};
use beacon_chain::BeaconChainTypes;
use lighthouse_network::rpc::methods::MaxRequestBlobSidecars;
use lighthouse_network::{PeerAction, PeerId};
use slog::{trace, Logger};
use ssz_types::VariableList;
use std::collections::HashSet;
use std::fmt::Debug;
use std::marker::PhantomData;
use std::sync::Arc;
use store::Hash256;
use strum::IntoStaticStr;
use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList};
use types::{EthSpec, SignedBeaconBlock};
use types::blob_sidecar::FixedBlobSidecarList;
use types::EthSpec;
#[derive(Debug, PartialEq, Eq)]
pub enum State {
@@ -58,23 +59,24 @@ pub struct SingleBlockLookup<L: Lookup, T: BeaconChainTypes> {
pub da_checker: Arc<DataAvailabilityChecker<T>>,
/// Only necessary for requests triggered by an `UnknownBlockParent` or `UnknownBlockParent`
/// because any blocks or blobs without parents won't hit the data availability cache.
pub cached_child_components: Option<CachedChildComponents<T::EthSpec>>,
pub child_components: Option<ChildComponents<T::EthSpec>>,
}
impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
pub fn new(
requested_block_root: Hash256,
unknown_parent_components: Option<CachedChildComponents<T::EthSpec>>,
child_components: Option<ChildComponents<T::EthSpec>>,
peers: &[PeerShouldHave],
da_checker: Arc<DataAvailabilityChecker<T>>,
id: Id,
) -> Self {
let is_deneb = da_checker.is_deneb();
Self {
id,
block_request_state: BlockRequestState::new(requested_block_root, peers),
blob_request_state: BlobRequestState::new(peers),
blob_request_state: BlobRequestState::new(requested_block_root, peers, is_deneb),
da_checker,
cached_child_components: unknown_parent_components,
child_components,
}
}
@@ -94,7 +96,7 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
self.block_request_state.requested_block_root = block_root;
self.block_request_state.state.state = State::AwaitingDownload;
self.blob_request_state.state.state = State::AwaitingDownload;
self.cached_child_components = Some(CachedChildComponents::default());
self.child_components = Some(ChildComponents::empty(block_root));
}
/// Get all unique peers across block and blob requests.
@@ -134,7 +136,7 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
/// 3. `Ok`: The child is required and we have downloaded it.
/// 4. `Err`: The child is required, but has failed consistency checks.
pub fn get_cached_child_block(&self) -> CachedChild<T::EthSpec> {
if let Some(components) = self.cached_child_components.as_ref() {
if let Some(components) = self.child_components.as_ref() {
let Some(block) = components.downloaded_block.as_ref() else {
return CachedChild::DownloadIncomplete;
};
@@ -143,7 +145,11 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
return CachedChild::DownloadIncomplete;
}
match RpcBlock::new_from_fixed(block.clone(), components.downloaded_blobs.clone()) {
match RpcBlock::new_from_fixed(
self.block_request_state.requested_block_root,
block.clone(),
components.downloaded_blobs.clone(),
) {
Ok(rpc_block) => CachedChild::Ok(rpc_block),
Err(e) => CachedChild::Err(e),
}
@@ -159,8 +165,8 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
&mut self,
verified_response: R::VerifiedResponseType,
) -> CachedChild<T::EthSpec> {
if let Some(cached_child_components) = self.cached_child_components.as_mut() {
R::add_to_child_components(verified_response, cached_child_components);
if let Some(child_components) = self.child_components.as_mut() {
R::add_to_child_components(verified_response, child_components);
self.get_cached_child_block()
} else {
CachedChild::NotRequired
@@ -168,34 +174,40 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
}
/// Add a child component to the lookup request. Merges with any existing child components.
pub fn add_child_components(&mut self, components: CachedChildComponents<T::EthSpec>) {
if let Some(ref mut existing_components) = self.cached_child_components {
let CachedChildComponents {
pub fn add_child_components(&mut self, components: ChildComponents<T::EthSpec>) {
if let Some(ref mut existing_components) = self.child_components {
let ChildComponents {
block_root: _,
downloaded_block,
downloaded_blobs,
} = components;
if let Some(block) = downloaded_block {
existing_components.add_cached_child_block(block);
existing_components.merge_block(block);
}
existing_components.add_cached_child_blobs(downloaded_blobs);
existing_components.merge_blobs(downloaded_blobs);
} else {
self.cached_child_components = Some(components);
self.child_components = Some(components);
}
}
/// Add all given peers to both block and blob request states.
pub fn add_peer(&mut self, peer: PeerShouldHave) {
match peer {
PeerShouldHave::BlockAndBlobs(peer_id) => {
self.block_request_state.state.add_peer(&peer_id);
self.blob_request_state.state.add_peer(&peer_id);
}
PeerShouldHave::Neither(peer_id) => {
self.block_request_state.state.add_potential_peer(&peer_id);
self.blob_request_state.state.add_potential_peer(&peer_id);
}
}
}
/// Add all given peers to both block and blob request states.
pub fn add_peers(&mut self, peers: &[PeerShouldHave]) {
for peer in peers {
match peer {
PeerShouldHave::BlockAndBlobs(peer_id) => {
self.block_request_state.state.add_peer(peer_id);
self.blob_request_state.state.add_peer(peer_id);
}
PeerShouldHave::Neither(peer_id) => {
self.block_request_state.state.add_potential_peer(peer_id);
self.blob_request_state.state.add_potential_peer(peer_id);
}
}
self.add_peer(*peer);
}
}
@@ -243,8 +255,8 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
/// Returns `true` if the block has already been downloaded.
pub(crate) fn block_already_downloaded(&self) -> bool {
if let Some(components) = self.cached_child_components.as_ref() {
components.downloaded_block.is_some()
if let Some(components) = self.child_components.as_ref() {
components.block_exists()
} else {
self.da_checker.has_block(&self.block_root())
}
@@ -260,26 +272,24 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
/// Updates this request with the most recent picture of which blobs still need to be requested.
pub fn update_blobs_request(&mut self) {
self.blob_request_state.requested_ids = self.missing_blob_ids().into()
self.blob_request_state.requested_ids = self.missing_blob_ids();
}
/// If `unknown_parent_components` is `Some`, we know block components won't hit the data
/// availability cache, so we don't check it. In either case we use the data availability
/// checker to get a picture of outstanding blob requirements for the block root.
pub(crate) fn missing_blob_ids(&self) -> Vec<BlobIdentifier> {
if let Some(components) = self.cached_child_components.as_ref() {
let blobs = components.downloaded_indices();
self.da_checker
.get_missing_blob_ids(
self.block_root(),
components.downloaded_block.as_ref(),
Some(blobs),
)
.unwrap_or_default()
/// If `child_components` is `Some`, we know block components won't hit the data
/// availability cache, so we don't check its processing cache unless `child_components`
/// is `None`.
pub(crate) fn missing_blob_ids(&self) -> MissingBlobs {
let block_root = self.block_root();
if let Some(components) = self.child_components.as_ref() {
self.da_checker.get_missing_blob_ids(block_root, components)
} else {
let Some(processing_availability_view) =
self.da_checker.get_processing_components(block_root)
else {
return MissingBlobs::new_without_block(block_root, self.da_checker.is_deneb());
};
self.da_checker
.get_missing_blob_ids_checking_cache(self.block_root())
.unwrap_or_default()
.get_missing_blob_ids(block_root, &processing_availability_view)
}
}
@@ -305,7 +315,7 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
/// necessary and clear the blob cache.
pub fn handle_consistency_failure(&mut self, cx: &SyncNetworkContext<T>) {
self.penalize_blob_peer(false, cx);
if let Some(cached_child) = self.cached_child_components.as_mut() {
if let Some(cached_child) = self.child_components.as_mut() {
cached_child.clear_blobs();
}
self.blob_request_state.state.register_failure_downloading()
@@ -315,49 +325,19 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
/// necessary and clear the blob cache.
pub fn handle_availability_check_failure(&mut self, cx: &SyncNetworkContext<T>) {
self.penalize_blob_peer(true, cx);
if let Some(cached_child) = self.cached_child_components.as_mut() {
if let Some(cached_child) = self.child_components.as_mut() {
cached_child.clear_blobs();
}
self.blob_request_state.state.register_failure_processing()
}
}
#[derive(Clone, Default)]
pub struct RequestedBlobIds(Vec<BlobIdentifier>);
impl From<Vec<BlobIdentifier>> for RequestedBlobIds {
fn from(value: Vec<BlobIdentifier>) -> Self {
Self(value)
}
}
impl Into<VariableList<BlobIdentifier, MaxRequestBlobSidecars>> for RequestedBlobIds {
fn into(self) -> VariableList<BlobIdentifier, MaxRequestBlobSidecars> {
VariableList::from(self.0)
}
}
impl RequestedBlobIds {
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}
pub fn contains(&self, blob_id: &BlobIdentifier) -> bool {
self.0.contains(blob_id)
}
pub fn remove(&mut self, blob_id: &BlobIdentifier) {
self.0.retain(|id| id != blob_id)
}
pub fn indices(&self) -> Vec<u64> {
self.0.iter().map(|id| id.index).collect()
}
}
/// The state of the blob request component of a `SingleBlockLookup`.
pub struct BlobRequestState<L: Lookup, T: EthSpec> {
/// The latest picture of which blobs still need to be requested. This includes information
/// from both block/blobs downloaded in the network layer and any blocks/blobs that exist in
/// the data availability checker.
pub requested_ids: RequestedBlobIds,
pub requested_ids: MissingBlobs,
/// Where we store blobs until we receive the stream terminator.
pub blob_download_queue: FixedBlobSidecarList<T>,
pub state: SingleLookupRequestState,
@@ -365,9 +345,10 @@ pub struct BlobRequestState<L: Lookup, T: EthSpec> {
}
impl<L: Lookup, E: EthSpec> BlobRequestState<L, E> {
pub fn new(peer_source: &[PeerShouldHave]) -> Self {
pub fn new(block_root: Hash256, peer_source: &[PeerShouldHave], is_deneb: bool) -> Self {
let default_ids = MissingBlobs::new_without_block(block_root, is_deneb);
Self {
requested_ids: <_>::default(),
requested_ids: default_ids,
blob_download_queue: <_>::default(),
state: SingleLookupRequestState::new(peer_source),
_phantom: PhantomData,
@@ -408,73 +389,6 @@ pub enum CachedChild<E: EthSpec> {
/// There was an error during consistency checks between block and blobs.
Err(AvailabilityCheckError),
}
/// For requests triggered by an `UnknownBlockParent` or `UnknownBlobParent`, this struct
/// is used to cache components as they are sent to the network service. We can't use the
/// data availability cache currently because any blocks or blobs without parents
/// won't pass validation and therefore won't make it into the cache.
#[derive(Default)]
pub struct CachedChildComponents<E: EthSpec> {
pub downloaded_block: Option<Arc<SignedBeaconBlock<E>>>,
pub downloaded_blobs: FixedBlobSidecarList<E>,
}
impl<E: EthSpec> From<RpcBlock<E>> for CachedChildComponents<E> {
fn from(value: RpcBlock<E>) -> Self {
let (block, blobs) = value.deconstruct();
let fixed_blobs = blobs.map(|blobs| {
FixedBlobSidecarList::from(blobs.into_iter().map(Some).collect::<Vec<_>>())
});
Self::new(Some(block), fixed_blobs)
}
}
impl<E: EthSpec> CachedChildComponents<E> {
pub fn new(
block: Option<Arc<SignedBeaconBlock<E>>>,
blobs: Option<FixedBlobSidecarList<E>>,
) -> Self {
Self {
downloaded_block: block,
downloaded_blobs: blobs.unwrap_or_default(),
}
}
pub fn clear_blobs(&mut self) {
self.downloaded_blobs = FixedBlobSidecarList::default();
}
pub fn add_cached_child_block(&mut self, block: Arc<SignedBeaconBlock<E>>) {
self.downloaded_block = Some(block);
}
pub fn add_cached_child_blobs(&mut self, blobs: FixedBlobSidecarList<E>) {
for (index, blob_opt) in self.downloaded_blobs.iter_mut().enumerate() {
if let Some(Some(downloaded_blob)) = blobs.get(index) {
*blob_opt = Some(downloaded_blob.clone());
}
}
}
pub fn downloaded_indices(&self) -> HashSet<usize> {
self.downloaded_blobs
.iter()
.enumerate()
.filter_map(|(i, blob_opt)| blob_opt.as_ref().map(|_| i))
.collect::<HashSet<_>>()
}
pub fn is_missing_components(&self) -> bool {
self.downloaded_block
.as_ref()
.map(|block| {
block.num_expected_blobs()
!= self.downloaded_blobs.iter().filter(|b| b.is_some()).count()
})
.unwrap_or(true)
}
}
/// Object representing the state of a single block or blob lookup request.
#[derive(PartialEq, Eq, Debug)]
pub struct SingleLookupRequestState {
@@ -516,6 +430,7 @@ impl SingleLookupRequestState {
}
}
}
Self {
state: State::AwaitingDownload,
available_peers,
@@ -703,10 +618,11 @@ mod tests {
Duration::from_secs(spec.seconds_per_slot),
);
let log = NullLoggerBuilder.build().expect("logger should build");
let store = HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal(), log)
.expect("store");
let store =
HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal(), log.clone())
.expect("store");
let da_checker = Arc::new(
DataAvailabilityChecker::new(slot_clock, None, store.into(), spec)
DataAvailabilityChecker::new(slot_clock, None, store.into(), &log, spec)
.expect("data availability checker"),
);
let mut sl = SingleBlockLookup::<TestLookup1, T>::new(
@@ -742,11 +658,12 @@ mod tests {
Duration::from_secs(spec.seconds_per_slot),
);
let log = NullLoggerBuilder.build().expect("logger should build");
let store = HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal(), log)
.expect("store");
let store =
HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal(), log.clone())
.expect("store");
let da_checker = Arc::new(
DataAvailabilityChecker::new(slot_clock, None, store.into(), spec)
DataAvailabilityChecker::new(slot_clock, None, store.into(), &log, spec)
.expect("data availability checker"),
);

View File

@@ -10,19 +10,18 @@ use super::*;
use crate::sync::block_lookups::common::ResponseType;
use beacon_chain::builder::Witness;
use beacon_chain::eth1_chain::CachingEth1Backend;
use beacon_chain::test_utils::{build_log, BeaconChainHarness, EphemeralHarnessType};
use beacon_chain::test_utils::{
build_log, generate_rand_block_and_blobs, BeaconChainHarness, EphemeralHarnessType, NumBlobs,
};
use beacon_processor::WorkEvent;
use lighthouse_network::rpc::RPCResponseErrorCode;
use lighthouse_network::{NetworkGlobals, Request};
use rand::Rng;
use slot_clock::{ManualSlotClock, SlotClock, TestingSlotClock};
use store::MemoryStore;
use tokio::sync::mpsc;
use types::{
map_fork_name, map_fork_name_with,
test_utils::{SeedableRng, TestRandom, XorShiftRng},
BeaconBlock, BlobSidecar, EthSpec, ForkName, FullPayloadDeneb, MinimalEthSpec as E,
SignedBeaconBlock,
test_utils::{SeedableRng, XorShiftRng},
BlobSidecar, EthSpec, ForkName, MinimalEthSpec as E, SignedBeaconBlock,
};
type T = Witness<ManualSlotClock, CachingEth1Backend<E>, E, MemoryStore<E>, MemoryStore<E>>;
@@ -36,11 +35,6 @@ struct TestRig {
const D: Duration = Duration::new(0, 0);
enum NumBlobs {
Random,
None,
}
impl TestRig {
fn test_setup(enable_log: bool) -> (BlockLookups<T>, SyncNetworkContext<T>, Self) {
let log = build_log(slog::Level::Debug, enable_log);
@@ -97,59 +91,10 @@ impl TestRig {
fork_name: ForkName,
num_blobs: NumBlobs,
) -> (SignedBeaconBlock<E>, Vec<BlobSidecar<E>>) {
let inner = map_fork_name!(fork_name, BeaconBlock, <_>::random_for_test(&mut self.rng));
let mut block =
SignedBeaconBlock::from_block(inner, types::Signature::random_for_test(&mut self.rng));
let mut blob_sidecars = vec![];
if let Ok(message) = block.message_deneb_mut() {
// get random number between 0 and Max Blobs
let payload: &mut FullPayloadDeneb<E> = &mut message.body.execution_payload;
let num_blobs = match num_blobs {
NumBlobs::Random => 1 + self.rng.gen::<usize>() % E::max_blobs_per_block(),
NumBlobs::None => 0,
};
let (bundle, transactions) =
execution_layer::test_utils::generate_random_blobs::<E, _>(
num_blobs,
self.harness.chain.kzg.as_ref().unwrap(),
&mut self.rng,
)
.unwrap();
let kzg = self.harness.chain.kzg.as_ref().unwrap();
let rng = &mut self.rng;
payload.execution_payload.transactions = <_>::default();
for tx in Vec::from(transactions) {
payload.execution_payload.transactions.push(tx).unwrap();
}
message.body.blob_kzg_commitments = bundle.commitments.clone();
let eth2::types::BlobsBundle {
commitments,
proofs,
blobs,
} = bundle;
let block_root = block.canonical_root();
for (index, ((blob, kzg_commitment), kzg_proof)) in blobs
.into_iter()
.zip(commitments.into_iter())
.zip(proofs.into_iter())
.enumerate()
{
blob_sidecars.push(BlobSidecar {
block_root,
index: index as u64,
slot: block.slot(),
block_parent_root: block.parent_root(),
proposer_index: block.message().proposer_index(),
blob: blob.clone(),
kzg_commitment,
kzg_proof,
});
}
}
(block, blob_sidecars)
generate_rand_block_and_blobs::<E>(fork_name, num_blobs, kzg.as_ref(), rng)
}
#[track_caller]
@@ -292,7 +237,11 @@ fn test_single_block_lookup_happy_path() {
let peer_id = PeerId::random();
let block_root = block.canonical_root();
// Trigger the request
bl.search_block(block_root, PeerShouldHave::BlockAndBlobs(peer_id), &mut cx);
bl.search_block(
block_root,
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let id = rig.expect_lookup_request(response_type);
// If we're in deneb, a blob request should have been triggered as well,
// we don't require a response because we're generateing 0-blob blocks in this test.
@@ -340,7 +289,11 @@ fn test_single_block_lookup_empty_response() {
let peer_id = PeerId::random();
// Trigger the request
bl.search_block(block_hash, PeerShouldHave::BlockAndBlobs(peer_id), &mut cx);
bl.search_block(
block_hash,
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let id = rig.expect_lookup_request(response_type);
// If we're in deneb, a blob request should have been triggered as well,
// we don't require a response because we're generateing 0-blob blocks in this test.
@@ -368,7 +321,11 @@ fn test_single_block_lookup_wrong_response() {
let peer_id = PeerId::random();
// Trigger the request
bl.search_block(block_hash, PeerShouldHave::BlockAndBlobs(peer_id), &mut cx);
bl.search_block(
block_hash,
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let id = rig.expect_lookup_request(response_type);
// If we're in deneb, a blob request should have been triggered as well,
// we don't require a response because we're generateing 0-blob blocks in this test.
@@ -406,7 +363,11 @@ fn test_single_block_lookup_failure() {
let peer_id = PeerId::random();
// Trigger the request
bl.search_block(block_hash, PeerShouldHave::BlockAndBlobs(peer_id), &mut cx);
bl.search_block(
block_hash,
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let id = rig.expect_lookup_request(response_type);
// If we're in deneb, a blob request should have been triggered as well,
// we don't require a response because we're generateing 0-blob blocks in this test.
@@ -440,7 +401,7 @@ fn test_single_block_lookup_becomes_parent_request() {
// Trigger the request
bl.search_block(
block.canonical_root(),
PeerShouldHave::BlockAndBlobs(peer_id),
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let id = rig.expect_lookup_request(response_type);
@@ -469,7 +430,7 @@ fn test_single_block_lookup_becomes_parent_request() {
// parent request after processing.
bl.single_block_component_processed::<BlockRequestState<Current>>(
id.id,
BlockError::ParentUnknown(block.into()).into(),
BlockError::ParentUnknown(RpcBlock::new_without_blobs(None, block)).into(),
&mut cx,
);
assert_eq!(bl.single_block_lookups.len(), 1);
@@ -978,7 +939,7 @@ fn test_parent_lookup_too_deep() {
// the processing result
bl.parent_block_processed(
chain_hash,
BlockError::ParentUnknown(block.into()).into(),
BlockError::ParentUnknown(RpcBlock::new_without_blobs(None, block)).into(),
&mut cx,
)
}
@@ -1026,7 +987,7 @@ fn test_single_block_lookup_ignored_response() {
// Trigger the request
bl.search_block(
block.canonical_root(),
PeerShouldHave::BlockAndBlobs(peer_id),
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let id = rig.expect_lookup_request(response_type);
@@ -1188,7 +1149,7 @@ fn test_same_chain_race_condition() {
} else {
bl.parent_block_processed(
chain_hash,
BlockError::ParentUnknown(block.into()).into(),
BlockError::ParentUnknown(RpcBlock::new_without_blobs(None, block)).into(),
&mut cx,
)
}
@@ -1223,6 +1184,7 @@ mod deneb_only {
use super::*;
use crate::sync::block_lookups::common::ResponseType;
use beacon_chain::data_availability_checker::AvailabilityCheckError;
use beacon_chain::test_utils::NumBlobs;
use std::ops::IndexMut;
use std::str::FromStr;
@@ -1278,7 +1240,7 @@ mod deneb_only {
RequestTrigger::AttestationUnknownBlock => {
bl.search_block(
block_root,
PeerShouldHave::BlockAndBlobs(peer_id),
&[PeerShouldHave::BlockAndBlobs(peer_id)],
&mut cx,
);
let block_req_id = rig.expect_lookup_request(ResponseType::Block);
@@ -1302,8 +1264,8 @@ mod deneb_only {
block_root = child_root;
bl.search_child_block(
child_root,
CachedChildComponents::new(Some(child_block), None),
PeerShouldHave::Neither(peer_id),
ChildComponents::new(child_root, Some(child_block), None),
&[PeerShouldHave::Neither(peer_id)],
&mut cx,
);
@@ -1340,8 +1302,8 @@ mod deneb_only {
*blobs.index_mut(0) = Some(child_blob);
bl.search_child_block(
child_root,
CachedChildComponents::new(None, Some(blobs)),
PeerShouldHave::Neither(peer_id),
ChildComponents::new(child_root, None, Some(blobs)),
&[PeerShouldHave::Neither(peer_id)],
&mut cx,
);
@@ -1359,7 +1321,7 @@ mod deneb_only {
)
}
RequestTrigger::GossipUnknownBlockOrBlob => {
bl.search_block(block_root, PeerShouldHave::Neither(peer_id), &mut cx);
bl.search_block(block_root, &[PeerShouldHave::Neither(peer_id)], &mut cx);
let block_req_id = rig.expect_lookup_request(ResponseType::Block);
let blob_req_id = rig.expect_lookup_request(ResponseType::Blob);
(Some(block_req_id), Some(blob_req_id), None, None)
@@ -1563,6 +1525,7 @@ mod deneb_only {
self.bl.parent_block_processed(
self.block_root,
BlockProcessingResult::Err(BlockError::ParentUnknown(RpcBlock::new_without_blobs(
Some(self.block_root),
self.parent_block.clone().expect("parent block"),
))),
&mut self.cx,