simplify checking attester cache for block and blobs. use ResourceUnavailable according to the spec

This commit is contained in:
realbigsean 2023-01-24 10:50:47 +01:00
parent e14550425d
commit b658cc7aaf
5 changed files with 66 additions and 82 deletions

View File

@ -957,22 +957,24 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
&self, &self,
block_root: &Hash256, block_root: &Hash256,
) -> Result< ) -> Result<
( Option<(
Option<Arc<SignedBeaconBlock<T::EthSpec>>>, Arc<SignedBeaconBlock<T::EthSpec>>,
Option<Arc<BlobsSidecar<T::EthSpec>>>, Arc<BlobsSidecar<T::EthSpec>>,
), )>,
Error, Error,
> { > {
if let (Some(block), Some(blobs)) = ( if let (Some(block), Some(blobs)) = (
self.early_attester_cache.get_block(*block_root), self.early_attester_cache.get_block(*block_root),
self.early_attester_cache.get_blobs(*block_root), self.early_attester_cache.get_blobs(*block_root),
) { ) {
return Ok((Some(block), Some(blobs))); return Ok(Some((block, blobs)));
}
if let Some(block) = self.get_block(block_root).await?.map(Arc::new) {
let blobs = self.get_blobs(block_root)?.map(Arc::new);
Ok(blobs.map(|blobs| (block, blobs)))
} else {
Ok(None)
} }
Ok((
self.get_block(block_root).await?.map(Arc::new),
self.get_blobs(block_root)?.map(Arc::new),
))
} }
/// Returns the block at the given root, if any. /// Returns the block at the given root, if any.
@ -1044,8 +1046,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// Returns the blobs at the given root, if any. /// Returns the blobs at the given root, if any.
/// ///
/// Returns `Ok(None)` if the blobs are not found. This could indicate the blob has been pruned /// Returns `Ok(None)` if the blobs and associated block are not found. The block referenced by
/// or that the block it is referenced by doesn't exist in our database. /// the blob doesn't exist in our database.
/// ///
/// If we can find the corresponding block in our database, we know whether we *should* have /// 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, /// blobs. If we should have blobs and no blobs are found, this will error. If we shouldn't,
@ -1055,6 +1057,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// - any database read errors /// - any database read errors
/// - block and blobs are inconsistent in the database /// - block and blobs are inconsistent in the database
/// - this method is called with a pre-eip4844 block root /// - this method is called with a pre-eip4844 block root
/// - this method is called for a blob that is beyond the prune depth
pub fn get_blobs( pub fn get_blobs(
&self, &self,
block_root: &Hash256, block_root: &Hash256,
@ -1073,10 +1076,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
Err(_) => return Err(Error::NoKzgCommitmentsFieldOnBlock), Err(_) => return Err(Error::NoKzgCommitmentsFieldOnBlock),
}; };
if expected_kzg_commitments.is_empty() { if expected_kzg_commitments.is_empty() {
Ok(Some(BlobsSidecar::empty_from_parts( Ok(BlobsSidecar::empty_from_parts(*block_root, block.slot()))
*block_root,
block.slot(),
)))
} else { } else {
if let Some(boundary) = self.data_availability_boundary() { if let Some(boundary) = self.data_availability_boundary() {
// We should have blobs for all blocks younger than the boundary. // We should have blobs for all blocks younger than the boundary.
@ -1084,11 +1084,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
return Err(Error::BlobsUnavailable); return Err(Error::BlobsUnavailable);
} }
} }
Err(Error::BlobsOlderThanDataAvailabilityBoundary) Err(Error::BlobsOlderThanDataAvailabilityBoundary(block.epoch()))
} }
}) })
.transpose() .transpose()
.map(Option::flatten)
} }
} }
} }

View File

@ -211,7 +211,7 @@ pub enum BeaconChainError {
ProposerHeadForkChoiceError(fork_choice::Error<proto_array::Error>), ProposerHeadForkChoiceError(fork_choice::Error<proto_array::Error>),
BlobsUnavailable, BlobsUnavailable,
NoKzgCommitmentsFieldOnBlock, NoKzgCommitmentsFieldOnBlock,
BlobsOlderThanDataAvailabilityBoundary, BlobsOlderThanDataAvailabilityBoundary(Epoch),
} }
easy_from_to!(SlotProcessingError, BeaconChainError); easy_from_to!(SlotProcessingError, BeaconChainError);

View File

@ -513,6 +513,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
Protocol::MetaData => PeerAction::LowToleranceError, Protocol::MetaData => PeerAction::LowToleranceError,
Protocol::Status => PeerAction::LowToleranceError, Protocol::Status => PeerAction::LowToleranceError,
}, },
RPCResponseErrorCode::BlobsNotFoundForBlock => PeerAction::LowToleranceError,
}, },
RPCError::SSZDecodeError(_) => PeerAction::Fatal, RPCError::SSZDecodeError(_) => PeerAction::Fatal,
RPCError::UnsupportedProtocol => { RPCError::UnsupportedProtocol => {

View File

@ -330,6 +330,7 @@ pub struct LightClientBootstrapRequest {
#[strum(serialize_all = "snake_case")] #[strum(serialize_all = "snake_case")]
pub enum RPCResponseErrorCode { pub enum RPCResponseErrorCode {
RateLimited, RateLimited,
BlobsNotFoundForBlock,
InvalidRequest, InvalidRequest,
ServerError, ServerError,
/// Error spec'd to indicate that a peer does not have blocks on a requested range. /// Error spec'd to indicate that a peer does not have blocks on a requested range.
@ -359,6 +360,7 @@ impl<T: EthSpec> RPCCodedResponse<T> {
2 => RPCResponseErrorCode::ServerError, 2 => RPCResponseErrorCode::ServerError,
3 => RPCResponseErrorCode::ResourceUnavailable, 3 => RPCResponseErrorCode::ResourceUnavailable,
139 => RPCResponseErrorCode::RateLimited, 139 => RPCResponseErrorCode::RateLimited,
142 => RPCResponseErrorCode::BlobsNotFoundForBlock,
_ => RPCResponseErrorCode::Unknown, _ => RPCResponseErrorCode::Unknown,
}; };
RPCCodedResponse::Error(code, err) RPCCodedResponse::Error(code, err)
@ -397,6 +399,7 @@ impl RPCResponseErrorCode {
RPCResponseErrorCode::ResourceUnavailable => 3, RPCResponseErrorCode::ResourceUnavailable => 3,
RPCResponseErrorCode::Unknown => 255, RPCResponseErrorCode::Unknown => 255,
RPCResponseErrorCode::RateLimited => 139, RPCResponseErrorCode::RateLimited => 139,
RPCResponseErrorCode::BlobsNotFoundForBlock => 140,
} }
} }
} }
@ -425,6 +428,7 @@ impl std::fmt::Display for RPCResponseErrorCode {
RPCResponseErrorCode::ServerError => "Server error occurred", RPCResponseErrorCode::ServerError => "Server error occurred",
RPCResponseErrorCode::Unknown => "Unknown error occurred", RPCResponseErrorCode::Unknown => "Unknown error occurred",
RPCResponseErrorCode::RateLimited => "Rate limited", RPCResponseErrorCode::RateLimited => "Rate limited",
RPCResponseErrorCode::BlobsNotFoundForBlock => "No blobs for the given root",
}; };
f.write_str(repr) f.write_str(repr)
} }

View File

@ -230,7 +230,7 @@ impl<T: BeaconChainTypes> Worker<T> {
.get_block_and_blobs_checking_early_attester_cache(root) .get_block_and_blobs_checking_early_attester_cache(root)
.await .await
{ {
Ok((Some(block), Some(blobs))) => { Ok(Some((block, blobs))) => {
self.send_response( self.send_response(
peer_id, peer_id,
Response::BlobsByRoot(Some(SignedBeaconBlockAndBlobsSidecar { Response::BlobsByRoot(Some(SignedBeaconBlockAndBlobsSidecar {
@ -241,7 +241,7 @@ impl<T: BeaconChainTypes> Worker<T> {
); );
send_block_count += 1; send_block_count += 1;
} }
Ok((None, None)) => { Ok(None) => {
debug!( debug!(
self.log, self.log,
"Peer requested unknown block and blobs"; "Peer requested unknown block and blobs";
@ -249,9 +249,41 @@ impl<T: BeaconChainTypes> Worker<T> {
"request_root" => ?root "request_root" => ?root
); );
} }
Ok((Some(block), None)) => { Err(BeaconChainError::BlobsUnavailable) => {
error!(
self.log,
"No blobs in the store for block root";
"request" => %request,
"peer" => %peer_id,
"block_root" => ?root
);
self.send_error_response(
peer_id,
RPCResponseErrorCode::BlobsNotFoundForBlock,
"Blobs not found for block root".into(),
request_id,
);
send_response = false;
break;
}
Err(BeaconChainError::NoKzgCommitmentsFieldOnBlock) => {
error!(
self.log,
"No kzg_commitments field in block";
"peer" => %peer_id,
"block_root" => ?root,
);
self.send_error_response(
peer_id,
RPCResponseErrorCode::ResourceUnavailable,
"Failed reading field kzg_commitments from block".into(),
request_id,
);
send_response = false;
break;
}
Err(BeaconChainError::BlobsOlderThanDataAvailabilityBoundary(block_epoch)) => {
let finalized_data_availability_boundary = self.chain.finalized_data_availability_boundary(); let finalized_data_availability_boundary = self.chain.finalized_data_availability_boundary();
let block_epoch = block.epoch();
match finalized_data_availability_boundary { match finalized_data_availability_boundary {
Some(boundary_epoch) => { Some(boundary_epoch) => {
@ -264,6 +296,14 @@ impl<T: BeaconChainTypes> Worker<T> {
"request_root" => ?root, "request_root" => ?root,
"finalized_data_availability_boundary" => %boundary_epoch, "finalized_data_availability_boundary" => %boundary_epoch,
); );
self.send_error_response(
peer_id,
RPCResponseErrorCode::ResourceUnavailable,
"Blobs older than data availability boundary".into(),
request_id,
);
send_response = false;
break;
} else { } else {
debug!( debug!(
self.log, self.log,
@ -287,32 +327,6 @@ impl<T: BeaconChainTypes> Worker<T> {
} }
} }
} }
Ok((None, Some(_))) => {
error!(
self.log,
"Peer requested block and blob, but no block found";
"request" => %request,
"peer" => %peer_id,
"request_root" => ?root
);
}
Err(BeaconChainError::BlobsUnavailable) => {
error!(
self.log,
"No blobs in the store for block root";
"request" => %request,
"peer" => %peer_id,
"block_root" => ?root
);
self.send_error_response(
peer_id,
RPCResponseErrorCode::ResourceUnavailable,
"Blobs unavailable".into(),
request_id,
);
send_response = false;
break;
}
Err(BeaconChainError::BlockHashMissingFromExecutionLayer(_)) => { Err(BeaconChainError::BlockHashMissingFromExecutionLayer(_)) => {
debug!( debug!(
self.log, self.log,
@ -807,40 +821,6 @@ impl<T: BeaconChainTypes> Worker<T> {
send_response = false; send_response = false;
break; break;
} }
Err(BeaconChainError::NoKzgCommitmentsFieldOnBlock) => {
error!(
self.log,
"No kzg_commitments field in block";
"request" => %req,
"peer" => %peer_id,
"block_root" => ?root,
);
self.send_error_response(
peer_id,
RPCResponseErrorCode::ResourceUnavailable,
"Failed reading field kzg_commitments from block".into(),
request_id,
);
send_response = false;
break;
}
Err(BeaconChainError::BlobsOlderThanDataAvailabilityBoundary) => {
error!(
self.log,
"Failed loading blobs older than data availability boundary";
"request" => %req,
"peer" => %peer_id,
"block_root" => ?root,
);
self.send_error_response(
peer_id,
RPCResponseErrorCode::ResourceUnavailable,
"Blobs older than data availability boundary".into(),
request_id,
);
send_response = false;
break;
}
Err(e) => { Err(e) => {
error!( error!(
self.log, self.log,