diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index b1f9c8d319..dac3dee8d9 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -971,7 +971,7 @@ impl BeaconChain { } Ok(( self.get_block(block_root).await?.map(Arc::new), - self.get_blobs(block_root).ok().flatten().map(Arc::new), + self.get_blobs(block_root)?.map(Arc::new), )) } @@ -1044,9 +1044,17 @@ impl BeaconChain { /// Returns the blobs at the given root, if any. /// - /// ## Errors + /// Returns `Ok(None)` if the blobs are not found. This could indicate the blob has been pruned + /// or that the block it is referenced by doesn't exist in our database. /// - /// May return a database error. + /// If we can find the corresponding block in our database, we know whether we *should* have + /// blobs. If we should have blobs and no blobs are found, this will error. If we shouldn't, + /// this will reconstruct an empty `BlobsSidecar`. + /// + /// ## Errors + /// - any database read errors + /// - block and blobs are inconsistent in the database + /// - this method is called with a pre-eip4844 block root pub fn get_blobs( &self, block_root: &Hash256, @@ -1054,23 +1062,33 @@ impl BeaconChain { match self.store.get_blobs(block_root)? { Some(blobs) => Ok(Some(blobs)), None => { - if let Ok(Some(block)) = self.get_blinded_block(block_root) { - let expected_kzg_commitments = block.message().body().blob_kzg_commitments()?; - - if expected_kzg_commitments.len() > 0 { - Err(Error::DBInconsistent(format!( - "Expected kzg commitments but no blobs stored for block root {}", - block_root - ))) - } else { - Ok(Some(BlobsSidecar::empty_from_parts( - *block_root, - block.slot(), - ))) - } - } else { - Ok(None) - } + // Check for the corresponding block to understand whether we *should* have blobs. + self + .get_blinded_block(block_root)? + .map(|block| { + // If there are no KZG commitments in the block, we know the sidecar should + // be empty. + let expected_kzg_commitments = block.message().body().blob_kzg_commitments()?; + if expected_kzg_commitments.is_empty() { + Ok(Some(BlobsSidecar::empty_from_parts( + *block_root, + block.slot(), + ))) + } else { + if let Some(boundary) = self.data_availability_boundary() { + // We should have blobs for all blocks after the boundary. + if boundary <= block.epoch() { + return Err(Error::DBInconsistent(format!( + "Expected kzg commitments but no blobs stored for block root {}", + block_root + ))) + } + } + Ok(None) + } + }) + .transpose() + .map(Option::flatten) } } } diff --git a/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs b/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs index b697c0e7bf..6e48389b55 100644 --- a/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs @@ -247,35 +247,38 @@ impl Worker { ); } Ok((Some(block), None)) => { - let data_availability_boundary_by_root = self.chain.finalized_data_availability_boundary(); + let finalized_data_availability_boundary = self.chain.finalized_data_availability_boundary(); let block_epoch = block.epoch(); - if Some(block_epoch) >= data_availability_boundary_by_root { + if Some(block_epoch) >= finalized_data_availability_boundary { debug!( self.log, "Peer requested block and blob that should be available, but no blob found"; "peer" => %peer_id, "request_root" => ?root, - "data_availability_boundary_by_root" => data_availability_boundary_by_root, + "finalized_data_availability_boundary" => finalized_data_availability_boundary, ); self.send_error_response( peer_id, RPCResponseErrorCode::ResourceUnavailable, - "No blob for requested block.".into(), + "Blobs unavailable".into(), request_id, ); + send_response = false; + break; } else { debug!( self.log, - "Peer requested block and blob older than the data availability boundary for ByRoot request, no blob found"; + "Peer requested block and blob older than the data availability \ + boundary for ByRoot request, no blob found"; "peer" => %peer_id, "request_root" => ?root, - "data_availability_boundary_by_root" => data_availability_boundary_by_root, + "finalized_data_availability_boundary" => finalized_data_availability_boundary, ); self.send_error_response( peer_id, RPCResponseErrorCode::ResourceUnavailable, - format!("No blob for requested block. Requested blob is older than the data availability boundary for a ByRoot request, currently at epoch {:?}", data_availability_boundary_by_root), + "Blobs unavailable".into(), request_id, ); } @@ -717,7 +720,7 @@ impl Worker { let block_roots = block_roots.into_iter().flatten().collect::>(); let mut blobs_sent = 0; - let send_response = true; + let mut send_response = true; for root in block_roots { match self.chain.get_blobs(&root) { @@ -735,6 +738,13 @@ impl Worker { "No blobs or block in the store for block root"; "block_root" => ?root ); + self.send_error_response( + peer_id, + RPCResponseErrorCode::ResourceUnavailable, + "Blobs unavailable".into(), + request_id, + ); + send_response = false; break; } Err(e) => { @@ -744,6 +754,13 @@ impl Worker { "block_root" => ?root, "error" => ?e ); + self.send_error_response( + peer_id, + RPCResponseErrorCode::ServerError, + "Failed fetching blobs".into(), + request_id, + ); + send_response = false; break; } }