diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index fb2fc5822..df9cebe4b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -648,7 +648,7 @@ impl BeaconChain { Ok(()) } - pub fn persist_data_availabilty_checker(&self) -> Result<(), Error> { + pub fn persist_data_availability_checker(&self) -> Result<(), Error> { let _timer = metrics::start_timer(&metrics::PERSIST_DATA_AVAILABILITY_CHECKER); self.data_availability_checker.persist_all()?; @@ -6298,7 +6298,7 @@ impl Drop for BeaconChain { let drop = || -> Result<(), Error> { self.persist_head_and_fork_choice()?; self.persist_op_pool()?; - self.persist_data_availabilty_checker()?; + self.persist_data_availability_checker()?; self.persist_eth1_cache() }; diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index ed07e9176..c9388026b 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -1,12 +1,12 @@ use derivative::Derivative; use slot_clock::SlotClock; -use state_processing::state_advance::partial_state_advance; use std::sync::Arc; use crate::beacon_chain::{ BeaconChain, BeaconChainTypes, BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT, VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, }; +use crate::block_verification::cheap_state_advance_to_obtain_committees; use crate::data_availability_checker::AvailabilityCheckError; use crate::kzg_utils::{validate_blob, validate_blobs}; use crate::BeaconChainError; @@ -14,11 +14,10 @@ use kzg::Kzg; use slog::{debug, warn}; use ssz_derive::{Decode, Encode}; use ssz_types::VariableList; -use std::borrow::Cow; use types::blob_sidecar::BlobIdentifier; use types::{ - BeaconState, BeaconStateError, BlobSidecar, BlobSidecarList, ChainSpec, CloneConfig, EthSpec, - Hash256, RelativeEpoch, SignedBlobSidecar, Slot, + BeaconStateError, BlobSidecar, BlobSidecarList, CloneConfig, EthSpec, Hash256, + SignedBlobSidecar, Slot, }; /// An error occurred while validating a gossip blob. @@ -308,12 +307,13 @@ pub fn validate_blob_sidecar_for_gossip( "block_root" => %block_root, "index" => %blob_index, ); - let state = cheap_state_advance_to_obtain_committees( - &mut snapshot.beacon_state, - Some(snapshot.beacon_block_root), - blob_slot, - &chain.spec, - )?; + let state = + cheap_state_advance_to_obtain_committees::<_, GossipBlobError>( + &mut snapshot.beacon_state, + Some(snapshot.beacon_block_root), + blob_slot, + &chain.spec, + )?; ( state.get_beacon_proposer_index(blob_slot, &chain.spec)?, state.fork(), @@ -344,7 +344,7 @@ pub fn validate_blob_sidecar_for_gossip( parent_block.state_root() )) })?; - let state = cheap_state_advance_to_obtain_committees( + let state = cheap_state_advance_to_obtain_committees::<_, GossipBlobError>( &mut parent_state, Some(parent_block.state_root()), blob_slot, @@ -428,56 +428,6 @@ pub fn validate_blob_sidecar_for_gossip( }) } -/// Performs a cheap (time-efficient) state advancement so the committees and proposer shuffling for -/// `slot` can be obtained from `state`. -/// -/// The state advancement is "cheap" since it does not generate state roots. As a result, the -/// returned state might be holistically invalid but the committees/proposers will be correct (since -/// they do not rely upon state roots). -/// -/// If the given `state` can already serve the `slot`, the committees will be built on the `state` -/// and `Cow::Borrowed(state)` will be returned. Otherwise, the state will be cloned, cheaply -/// advanced and then returned as a `Cow::Owned`. The end result is that the given `state` is never -/// mutated to be invalid (in fact, it is never changed beyond a simple committee cache build). -/// -/// Note: This is a copy of the `block_verification::cheap_state_advance_to_obtain_committees` to return -/// a BlobError error type instead. -fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec>( - state: &'a mut BeaconState, - state_root_opt: Option, - blob_slot: Slot, - spec: &ChainSpec, -) -> Result>, GossipBlobError> { - let block_epoch = blob_slot.epoch(E::slots_per_epoch()); - - if state.current_epoch() == block_epoch { - // Build both the current and previous epoch caches, as the previous epoch caches are - // useful for verifying attestations in blocks from the current epoch. - state.build_committee_cache(RelativeEpoch::Previous, spec)?; - state.build_committee_cache(RelativeEpoch::Current, spec)?; - - Ok(Cow::Borrowed(state)) - } else if state.slot() > blob_slot { - Err(GossipBlobError::BlobIsNotLaterThanParent { - blob_slot, - parent_slot: state.slot(), - }) - } else { - let mut state = state.clone_with(CloneConfig::committee_caches_only()); - let target_slot = block_epoch.start_slot(E::slots_per_epoch()); - - // Advance the state into the same epoch as the block. Use the "partial" method since state - // roots are not important for proposer/attester shuffling. - partial_state_advance(&mut state, state_root_opt, target_slot, spec) - .map_err(|e| GossipBlobError::BeaconChainError(BeaconChainError::from(e)))?; - - state.build_committee_cache(RelativeEpoch::Previous, spec)?; - state.build_committee_cache(RelativeEpoch::Current, spec)?; - - Ok(Cow::Owned(state)) - } -} - /// Wrapper over a `BlobSidecar` for which we have completed kzg verification. /// i.e. `verify_blob_kzg_proof(blob, commitment, proof) == true`. #[derive(Debug, Derivative, Clone, Encode, Decode)] diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index cb7219ea4..6dae5deb4 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -90,6 +90,7 @@ use state_processing::{ StateProcessingStrategy, VerifyBlockRoot, }; use std::borrow::Cow; +use std::fmt::Debug; use std::fs; use std::io::Write; use std::sync::Arc; @@ -575,7 +576,7 @@ pub fn signature_verify_chain_segment( .map(|(_, block)| block.slot()) .unwrap_or_else(|| slot); - let state = cheap_state_advance_to_obtain_committees( + let state = cheap_state_advance_to_obtain_committees::<_, BlockError>( &mut parent.pre_state, parent.beacon_state_root, highest_slot, @@ -887,7 +888,7 @@ impl GossipVerifiedBlock { ); // The state produced is only valid for determining proposer/attester shuffling indices. - let state = cheap_state_advance_to_obtain_committees( + let state = cheap_state_advance_to_obtain_committees::<_, BlockError>( &mut parent.pre_state, parent.beacon_state_root, block.slot(), @@ -1017,7 +1018,7 @@ impl SignatureVerifiedBlock { let (mut parent, block) = load_parent(block_root, block, chain)?; - let state = cheap_state_advance_to_obtain_committees( + let state = cheap_state_advance_to_obtain_committees::<_, BlockError>( &mut parent.pre_state, parent.beacon_state_root, block.slot(), @@ -1067,7 +1068,7 @@ impl SignatureVerifiedBlock { load_parent(from.block_root, from.block, chain)? }; - let state = cheap_state_advance_to_obtain_committees( + let state = cheap_state_advance_to_obtain_committees::<_, BlockError>( &mut parent.pre_state, parent.beacon_state_root, block.slot(), @@ -1900,6 +1901,30 @@ fn load_parent>( result } +/// This trait is used to unify `BlockError` and `BlobError` so +/// `cheap_state_advance_to_obtain_committees` can be re-used in gossip blob validation. +pub trait CheapStateAdvanceError: From + From + Debug { + fn not_later_than_parent_error(block_slot: Slot, state_slot: Slot) -> Self; +} + +impl CheapStateAdvanceError for BlockError { + fn not_later_than_parent_error(block_slot: Slot, parent_slot: Slot) -> Self { + BlockError::BlockIsNotLaterThanParent { + block_slot, + parent_slot, + } + } +} + +impl CheapStateAdvanceError for GossipBlobError { + fn not_later_than_parent_error(blob_slot: Slot, parent_slot: Slot) -> Self { + GossipBlobError::BlobIsNotLaterThanParent { + blob_slot, + parent_slot, + } + } +} + /// Performs a cheap (time-efficient) state advancement so the committees and proposer shuffling for /// `slot` can be obtained from `state`. /// @@ -1911,12 +1936,12 @@ fn load_parent>( /// and `Cow::Borrowed(state)` will be returned. Otherwise, the state will be cloned, cheaply /// advanced and then returned as a `Cow::Owned`. The end result is that the given `state` is never /// mutated to be invalid (in fact, it is never changed beyond a simple committee cache build). -fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec>( +pub fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec, Err: CheapStateAdvanceError>( state: &'a mut BeaconState, state_root_opt: Option, block_slot: Slot, spec: &ChainSpec, -) -> Result>, BlockError> { +) -> Result>, Err> { let block_epoch = block_slot.epoch(E::slots_per_epoch()); if state.current_epoch() == block_epoch { @@ -1927,10 +1952,7 @@ fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec>( Ok(Cow::Borrowed(state)) } else if state.slot() > block_slot { - Err(BlockError::BlockIsNotLaterThanParent { - block_slot, - parent_slot: state.slot(), - }) + Err(Err::not_later_than_parent_error(block_slot, state.slot())) } else { let mut state = state.clone_with(CloneConfig::committee_caches_only()); let target_slot = block_epoch.start_slot(E::slots_per_epoch()); @@ -1938,7 +1960,7 @@ fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec>( // Advance the state into the same epoch as the block. Use the "partial" method since state // roots are not important for proposer/attester shuffling. partial_state_advance(&mut state, state_root_opt, target_slot, spec) - .map_err(|e| BlockError::BeaconChainError(BeaconChainError::from(e)))?; + .map_err(BeaconChainError::from)?; state.build_committee_cache(RelativeEpoch::Previous, spec)?; state.build_committee_cache(RelativeEpoch::Current, spec)?; diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index f7bbb861c..21fcdc9ef 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -691,9 +691,9 @@ impl OverflowLRUCache { // it is still LRU entry -> delete it from memory & record that it's on disk write_lock.in_memory.pop_entry(&lru_root); write_lock.store_keys.insert(lru_root); - stored = write_lock.in_memory.len(); } } + stored = write_lock.in_memory.len(); drop(write_lock); } diff --git a/beacon_node/beacon_processor/src/lib.rs b/beacon_node/beacon_processor/src/lib.rs index 00108f084..31d4e4aac 100644 --- a/beacon_node/beacon_processor/src/lib.rs +++ b/beacon_node/beacon_processor/src/lib.rs @@ -618,8 +618,8 @@ pub enum Work { Status(BlockingFn), BlocksByRangeRequest(BlockingFnWithManualSendOnIdle), BlocksByRootsRequest(BlockingFnWithManualSendOnIdle), - BlobsByRangeRequest(BlockingFnWithManualSendOnIdle), - BlobsByRootsRequest(BlockingFnWithManualSendOnIdle), + BlobsByRangeRequest(BlockingFn), + BlobsByRootsRequest(BlockingFn), GossipBlsToExecutionChange(BlockingFn), LightClientBootstrapRequest(BlockingFn), ApiRequestP0(BlockingOrAsync), @@ -1461,10 +1461,10 @@ impl BeaconProcessor { .spawn_async(async move { work.await; }), - Work::BlobsByRangeRequest(work) - | Work::BlobsByRootsRequest(work) - | Work::BlocksByRangeRequest(work) - | Work::BlocksByRootsRequest(work) => { + Work::BlobsByRangeRequest(process_fn) | Work::BlobsByRootsRequest(process_fn) => { + task_spawner.spawn_blocking(process_fn) + } + Work::BlocksByRangeRequest(work) | Work::BlocksByRootsRequest(work) => { task_spawner.spawn_blocking_with_manual_send_idle(work) } Work::ChainSegmentBackfill(process_fn) => task_spawner.spawn_async(process_fn), diff --git a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs index 322b1c0d6..2bcaec147 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs @@ -402,20 +402,15 @@ fn context_bytes( // NOTE: If you are adding another fork type here, be sure to modify the // `fork_context.to_context_bytes()` function to support it as well! SignedBeaconBlock::Deneb { .. } => { - // Deneb context being `None` implies that "merge never happened". fork_context.to_context_bytes(ForkName::Deneb) } SignedBeaconBlock::Capella { .. } => { - // Capella context being `None` implies that "merge never happened". fork_context.to_context_bytes(ForkName::Capella) } SignedBeaconBlock::Merge { .. } => { - // Merge context being `None` implies that "merge never happened". fork_context.to_context_bytes(ForkName::Merge) } SignedBeaconBlock::Altair { .. } => { - // Altair context being `None` implies that "altair never happened". - // This code should be unreachable if altair is disabled since only Version::V1 would be valid in that case. fork_context.to_context_bytes(ForkName::Altair) } SignedBeaconBlock::Base { .. } => Some(fork_context.genesis_context_bytes()), diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 2148ece56..438c2c754 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -297,6 +297,12 @@ pub struct BlobsByRangeRequest { pub count: u64, } +impl BlobsByRangeRequest { + pub fn max_blobs_requested(&self) -> u64 { + self.count.saturating_mul(E::max_blobs_per_block() as u64) + } +} + /// Request a number of beacon block roots from a peer. #[superstruct( variants(V1, V2), diff --git a/beacon_node/lighthouse_network/src/rpc/outbound.rs b/beacon_node/lighthouse_network/src/rpc/outbound.rs index 863fe501a..713e9e0ec 100644 --- a/beacon_node/lighthouse_network/src/rpc/outbound.rs +++ b/beacon_node/lighthouse_network/src/rpc/outbound.rs @@ -99,7 +99,7 @@ impl OutboundRequest { OutboundRequest::Goodbye(_) => 0, OutboundRequest::BlocksByRange(req) => *req.count(), OutboundRequest::BlocksByRoot(req) => req.block_roots().len() as u64, - OutboundRequest::BlobsByRange(req) => req.count * TSpec::max_blobs_per_block() as u64, + OutboundRequest::BlobsByRange(req) => req.max_blobs_requested::(), OutboundRequest::BlobsByRoot(req) => req.blob_ids.len() as u64, OutboundRequest::Ping(_) => 1, OutboundRequest::MetaData(_) => 1, diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index cdc7e4d74..95fdc2083 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -574,7 +574,7 @@ impl InboundRequest { InboundRequest::Goodbye(_) => 0, InboundRequest::BlocksByRange(req) => *req.count(), InboundRequest::BlocksByRoot(req) => req.block_roots().len() as u64, - InboundRequest::BlobsByRange(req) => req.count * TSpec::max_blobs_per_block() as u64, + InboundRequest::BlobsByRange(req) => req.max_blobs_requested::(), InboundRequest::BlobsByRoot(req) => req.blob_ids.len() as u64, InboundRequest::Ping(_) => 1, InboundRequest::MetaData(_) => 1, diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index ad885b5fe..322451dfe 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1186,7 +1186,7 @@ impl NetworkBeaconProcessor { | AvailabilityCheckError::MissingBlobs | AvailabilityCheckError::StoreError(_) | AvailabilityCheckError::DecodeError(_) => { - crit!( + warn!( self.log, "Internal availability check error"; "error" => ?err, diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index 88d119405..9655284aa 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -562,9 +562,8 @@ impl NetworkBeaconProcessor { request: BlobsByRangeRequest, ) -> Result<(), Error> { let processor = self.clone(); - let process_fn = move |send_idle_on_drop| { - processor.handle_blobs_by_range_request(send_idle_on_drop, peer_id, request_id, request) - }; + let process_fn = + move || processor.handle_blobs_by_range_request(peer_id, request_id, request); self.try_send(BeaconWorkEvent { drop_during_sync: false, @@ -580,9 +579,8 @@ impl NetworkBeaconProcessor { request: BlobsByRootRequest, ) -> Result<(), Error> { let processor = self.clone(); - let process_fn = move |send_idle_on_drop| { - processor.handle_blobs_by_root_request(send_idle_on_drop, peer_id, request_id, request) - }; + let process_fn = + move || processor.handle_blobs_by_root_request(peer_id, request_id, request); self.try_send(BeaconWorkEvent { drop_during_sync: false, diff --git a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs index 222b9c8fd..bc35c059c 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -220,7 +220,6 @@ impl NetworkBeaconProcessor { /// Handle a `BlobsByRoot` request from the peer. pub fn handle_blobs_by_root_request( self: Arc, - send_on_drop: SendOnDrop, peer_id: PeerId, request_id: PeerRequestId, request: BlobsByRootRequest, @@ -286,7 +285,6 @@ impl NetworkBeaconProcessor { if send_response { self.send_response(peer_id, Response::BlobsByRoot(None), request_id); } - drop(send_on_drop); } /// Handle a `BlocksByRoot` request from the peer. @@ -607,7 +605,6 @@ impl NetworkBeaconProcessor { /// Handle a `BlobsByRange` request from the peer. pub fn handle_blobs_by_range_request( self: Arc, - send_on_drop: SendOnDrop, peer_id: PeerId, request_id: PeerRequestId, req: BlobsByRangeRequest, @@ -619,7 +616,7 @@ impl NetworkBeaconProcessor { ); // Should not send more than max request blocks - if req.count * T::EthSpec::max_blobs_per_block() as u64 > MAX_REQUEST_BLOB_SIDECARS { + if req.max_blobs_requested::() > MAX_REQUEST_BLOB_SIDECARS { return self.send_error_response( peer_id, RPCResponseErrorCode::InvalidRequest, @@ -711,13 +708,16 @@ impl NetworkBeaconProcessor { } }; - // Pick out the required blocks, ignoring skip-slots. + // Use `WhenSlotSkipped::Prev` to get the most recent block root prior to + // `request_start_slot` in order to check whether the `request_start_slot` is a skip. let mut last_block_root = req.start_slot.checked_sub(1).and_then(|prev_slot| { self.chain .block_root_at_slot(Slot::new(prev_slot), WhenSlotSkipped::Prev) .ok() .flatten() }); + + // Pick out the required blocks, ignoring skip-slots. let maybe_block_roots = process_results(forwards_block_root_iter, |iter| { iter.take_while(|(_, slot)| slot.as_u64() < req.start_slot.saturating_add(req.count)) // map skip slots to None @@ -745,7 +745,7 @@ impl NetworkBeaconProcessor { }; // remove all skip slots - let block_roots = block_roots.into_iter().flatten().collect::>(); + let block_roots = block_roots.into_iter().flatten(); let mut blobs_sent = 0; let mut send_response = true; @@ -806,7 +806,5 @@ impl NetworkBeaconProcessor { id: request_id, }); } - - drop(send_on_drop); } } diff --git a/crypto/kzg/src/kzg_proof.rs b/crypto/kzg/src/kzg_proof.rs index 58ccf632a..06022ae47 100644 --- a/crypto/kzg/src/kzg_proof.rs +++ b/crypto/kzg/src/kzg_proof.rs @@ -24,9 +24,10 @@ impl From for c_kzg_min::Bytes48 { } impl KzgProof { + /// Creates a valid proof using `G1_POINT_AT_INFINITY`. pub fn empty() -> Self { let mut bytes = [0; BYTES_PER_PROOF]; - bytes[0] = 192; + bytes[0] = 0xc0; Self(bytes) } }