Merge pull request #4535 from qu0b/blob/empty

API - Handle blocks without blobs gracefully
This commit is contained in:
Jimmy Chen 2023-07-28 20:00:26 +10:00 committed by GitHub
commit 9c75d8088d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 40 additions and 69 deletions

View File

@ -1102,10 +1102,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
pub fn get_blobs_checking_early_attester_cache( pub fn get_blobs_checking_early_attester_cache(
&self, &self,
block_root: &Hash256, block_root: &Hash256,
) -> Result<Option<BlobSidecarList<T::EthSpec>>, Error> { ) -> Result<BlobSidecarList<T::EthSpec>, Error> {
self.early_attester_cache self.early_attester_cache
.get_blobs(*block_root) .get_blobs(*block_root)
.map_or_else(|| self.get_blobs(block_root), |blobs| Ok(Some(blobs))) .map_or_else(|| self.get_blobs(block_root), Ok)
} }
/// Returns the block at the given root, if any. /// Returns the block at the given root, if any.
@ -1185,11 +1185,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// - block and blobs are inconsistent in the database /// - block and blobs are inconsistent in the database
/// - this method is called with a pre-deneb block root /// - this method is called with a pre-deneb block root
/// - this method is called for a blob that is beyond the prune depth /// - this method is called for a blob that is beyond the prune depth
pub fn get_blobs( pub fn get_blobs(&self, block_root: &Hash256) -> Result<BlobSidecarList<T::EthSpec>, Error> {
&self, match self.store.get_blobs(block_root)? {
block_root: &Hash256, Some(blobs) => Ok(blobs),
) -> Result<Option<BlobSidecarList<T::EthSpec>>, Error> { None => Ok(BlobSidecarList::default()),
Ok(self.store.get_blobs(block_root)?) }
} }
pub fn get_blinded_block( pub fn get_blinded_block(

View File

@ -698,14 +698,14 @@ where
let block = self.chain.head_beacon_block(); let block = self.chain.head_beacon_block();
let block_root = block.canonical_root(); let block_root = block.canonical_root();
let blobs = self.chain.get_blobs(&block_root).unwrap(); let blobs = self.chain.get_blobs(&block_root).unwrap();
RpcBlock::new(block, blobs).unwrap() RpcBlock::new(block, Some(blobs)).unwrap()
} }
pub fn get_full_block(&self, block_root: &Hash256) -> RpcBlock<E> { pub fn get_full_block(&self, block_root: &Hash256) -> RpcBlock<E> {
let block = self.chain.get_blinded_block(block_root).unwrap().unwrap(); let block = self.chain.get_blinded_block(block_root).unwrap().unwrap();
let full_block = self.chain.store.make_full_block(block_root, block).unwrap(); let full_block = self.chain.store.make_full_block(block_root, block).unwrap();
let blobs = self.chain.get_blobs(block_root).unwrap(); let blobs = self.chain.get_blobs(block_root).unwrap();
RpcBlock::new(Arc::new(full_block), blobs).unwrap() RpcBlock::new(Arc::new(full_block), Some(blobs)).unwrap()
} }
pub fn get_all_validators(&self) -> Vec<usize> { pub fn get_all_validators(&self) -> Vec<usize> {

View File

@ -134,7 +134,8 @@ async fn produces_attestations() {
assert_eq!(data.target.root, target_root, "bad target root"); assert_eq!(data.target.root, target_root, "bad target root");
let rpc_block = let rpc_block =
RpcBlock::<MainnetEthSpec>::new(Arc::new(block.clone()), blobs.clone()).unwrap(); RpcBlock::<MainnetEthSpec>::new(Arc::new(block.clone()), Some(blobs.clone()))
.unwrap();
let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = chain let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = chain
.data_availability_checker .data_availability_checker
.check_rpc_block_availability(rpc_block) .check_rpc_block_availability(rpc_block)
@ -209,7 +210,8 @@ async fn early_attester_cache_old_request() {
.get_blobs(&head.beacon_block_root) .get_blobs(&head.beacon_block_root)
.expect("should get blobs"); .expect("should get blobs");
let rpc_block = RpcBlock::<MainnetEthSpec>::new(head.beacon_block.clone(), head_blobs).unwrap(); let rpc_block =
RpcBlock::<MainnetEthSpec>::new(head.beacon_block.clone(), Some(head_blobs)).unwrap();
let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = harness.chain let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = harness.chain
.data_availability_checker .data_availability_checker
.check_rpc_block_availability(rpc_block) .check_rpc_block_availability(rpc_block)

View File

@ -66,12 +66,12 @@ async fn get_chain_segment() -> (Vec<BeaconSnapshot<E>>, Vec<Option<BlobSidecarL
beacon_block: Arc::new(full_block), beacon_block: Arc::new(full_block),
beacon_state: snapshot.beacon_state, beacon_state: snapshot.beacon_state,
}); });
segment_blobs.push( segment_blobs.push(Some(
harness harness
.chain .chain
.get_blobs(&snapshot.beacon_block_root) .get_blobs(&snapshot.beacon_block_root)
.unwrap(), .unwrap(),
) ))
} }
(segment, segment_blobs) (segment, segment_blobs)
} }
@ -114,26 +114,22 @@ async fn get_chain_segment_with_signed_blobs() -> (
.chain .chain
.get_blobs(&snapshot.beacon_block_root) .get_blobs(&snapshot.beacon_block_root)
.unwrap() .unwrap()
.map(|blobs| { .into_iter()
let blobs = blobs .map(|blob| {
.into_iter() let block_root = blob.block_root;
.map(|blob| { let blob_index = blob.index;
let block_root = blob.block_root; SignedBlobSidecar {
let blob_index = blob.index; message: blob,
SignedBlobSidecar { signature: harness
message: blob, .blob_signature_cache
signature: harness .read()
.blob_signature_cache .get(&BlobSignatureKey::new(block_root, blob_index))
.read() .unwrap()
.get(&BlobSignatureKey::new(block_root, blob_index)) .clone(),
.unwrap() }
.clone(), })
} .collect::<Vec<_>>();
}) segment_blobs.push(Some(VariableList::from(signed_blobs)))
.collect::<Vec<_>>();
VariableList::from(blobs)
});
segment_blobs.push(signed_blobs)
} }
(segment, segment_blobs) (segment, segment_blobs)
} }

View File

@ -2179,7 +2179,7 @@ async fn weak_subjectivity_sync() {
beacon_chain beacon_chain
.process_block( .process_block(
full_block.canonical_root(), full_block.canonical_root(),
RpcBlock::new(Arc::new(full_block), blobs).unwrap(), RpcBlock::new(Arc::new(full_block), Some(blobs)).unwrap(),
NotifyExecutionLayer::Yes, NotifyExecutionLayer::Yes,
|| Ok(()), || Ok(()),
) )
@ -2239,7 +2239,7 @@ async fn weak_subjectivity_sync() {
if let MaybeAvailableBlock::Available(block) = harness if let MaybeAvailableBlock::Available(block) = harness
.chain .chain
.data_availability_checker .data_availability_checker
.check_rpc_block_availability(RpcBlock::new(Arc::new(full_block), blobs).unwrap()) .check_rpc_block_availability(RpcBlock::new(Arc::new(full_block), Some(blobs)).unwrap())
.expect("should check availability") .expect("should check availability")
{ {
available_blocks.push(block); available_blocks.push(block);

View File

@ -258,14 +258,9 @@ impl BlockId {
chain: &BeaconChain<T>, chain: &BeaconChain<T>,
) -> Result<BlobSidecarList<T::EthSpec>, warp::Rejection> { ) -> Result<BlobSidecarList<T::EthSpec>, warp::Rejection> {
let root = self.root(chain)?.0; let root = self.root(chain)?.0;
match chain.get_blobs(&root) { chain
Ok(Some(blob_sidecar_list)) => Ok(blob_sidecar_list), .get_blobs(&root)
Ok(None) => Err(warp_utils::reject::custom_not_found(format!( .map_err(warp_utils::reject::beacon_chain_error)
"No blobs with block root {} found in the store",
root
))),
Err(e) => Err(warp_utils::reject::beacon_chain_error(e)),
}
} }
pub async fn blob_sidecar_list_filtered<T: BeaconChainTypes>( pub async fn blob_sidecar_list_filtered<T: BeaconChainTypes>(

View File

@ -11,7 +11,7 @@ use lighthouse_network::rpc::methods::{
use lighthouse_network::rpc::StatusMessage; use lighthouse_network::rpc::StatusMessage;
use lighthouse_network::rpc::*; use lighthouse_network::rpc::*;
use lighthouse_network::{PeerId, PeerRequestId, ReportSource, Response, SyncInfo}; use lighthouse_network::{PeerId, PeerRequestId, ReportSource, Response, SyncInfo};
use slog::{debug, error, trace, warn}; use slog::{debug, error, warn};
use slot_clock::SlotClock; use slot_clock::SlotClock;
use std::collections::{hash_map::Entry, HashMap}; use std::collections::{hash_map::Entry, HashMap};
use std::sync::Arc; use std::sync::Arc;
@ -247,7 +247,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
}; };
match blob_list_result.as_ref() { match blob_list_result.as_ref() {
Ok(Some(blobs_sidecar_list)) => { Ok(blobs_sidecar_list) => {
'inner: for blob_sidecar in blobs_sidecar_list.iter() { 'inner: for blob_sidecar in blobs_sidecar_list.iter() {
if blob_sidecar.index == index { if blob_sidecar.index == index {
self.send_response( self.send_response(
@ -260,14 +260,6 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
} }
} }
} }
Ok(None) => {
debug!(
self.log,
"Peer requested unknown blobs";
"peer" => %peer_id,
"request_root" => ?root
);
}
Err(e) => { Err(e) => {
debug!( debug!(
self.log, self.log,
@ -767,7 +759,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
for root in block_roots { for root in block_roots {
match self.chain.get_blobs(&root) { match self.chain.get_blobs(&root) {
Ok(Some(blob_sidecar_list)) => { Ok(blob_sidecar_list) => {
for blob_sidecar in blob_sidecar_list.iter() { for blob_sidecar in blob_sidecar_list.iter() {
blobs_sent += 1; blobs_sent += 1;
self.send_network_message(NetworkMessage::SendResponse { self.send_network_message(NetworkMessage::SendResponse {
@ -777,15 +769,6 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
}); });
} }
} }
Ok(None) => {
trace!(
self.log,
"No blobs in the store for block root";
"request" => ?req,
"peer" => %peer_id,
"block_root" => ?root
);
}
Err(e) => { Err(e) => {
error!( error!(
self.log, self.log,

View File

@ -1087,12 +1087,7 @@ async fn test_blobs_by_range() {
.block_root_at_slot(Slot::new(slot), WhenSlotSkipped::None) .block_root_at_slot(Slot::new(slot), WhenSlotSkipped::None)
.unwrap(); .unwrap();
blob_count += root blob_count += root
.and_then(|root| { .map(|root| rig.chain.get_blobs(&root).unwrap_or_default().len())
rig.chain
.get_blobs(&root)
.unwrap_or_default()
.map(|blobs| blobs.len())
})
.unwrap_or(0); .unwrap_or(0);
} }
let mut actual_count = 0; let mut actual_count = 0;