chore: use BlockWithParent for StageError (#13198)

This commit is contained in:
Dan Cline
2024-12-09 19:21:46 -05:00
committed by GitHub
parent 980e62a5b8
commit c9bd64018a
15 changed files with 117 additions and 75 deletions

View File

@ -1,12 +1,11 @@
use alloy_consensus::Header;
use alloy_eips::eip1898::BlockWithParent;
use alloy_primitives::B256;
use reth_metrics::{
metrics::{Counter, Gauge},
Metrics,
};
use reth_primitives::SealedHeader;
use schnellru::{ByLength, LruMap};
use std::{fmt::Debug, sync::Arc};
use std::fmt::Debug;
use tracing::warn;
/// The max hit counter for invalid headers in the cache before it is forcefully evicted.
@ -17,20 +16,20 @@ const INVALID_HEADER_HIT_EVICTION_THRESHOLD: u8 = 128;
/// Keeps track of invalid headers.
#[derive(Debug)]
pub struct InvalidHeaderCache<H = Header> {
pub struct InvalidHeaderCache {
/// This maps a header hash to a reference to its invalid ancestor.
headers: LruMap<B256, HeaderEntry<H>>,
headers: LruMap<B256, HeaderEntry>,
/// Metrics for the cache.
metrics: InvalidHeaderCacheMetrics,
}
impl<H: Debug> InvalidHeaderCache<H> {
impl InvalidHeaderCache {
/// Invalid header cache constructor.
pub fn new(max_length: u32) -> Self {
Self { headers: LruMap::new(ByLength::new(max_length)), metrics: Default::default() }
}
fn insert_entry(&mut self, hash: B256, header: Arc<H>) {
fn insert_entry(&mut self, hash: B256, header: BlockWithParent) {
self.headers.insert(hash, HeaderEntry { header, hit_count: 0 });
}
@ -38,7 +37,7 @@ impl<H: Debug> InvalidHeaderCache<H> {
///
/// If this is called, the hit count for the entry is incremented.
/// If the hit count exceeds the threshold, the entry is evicted and `None` is returned.
pub fn get(&mut self, hash: &B256) -> Option<Arc<H>> {
pub fn get(&mut self, hash: &B256) -> Option<BlockWithParent> {
{
let entry = self.headers.get(hash)?;
entry.hit_count += 1;
@ -53,7 +52,11 @@ impl<H: Debug> InvalidHeaderCache<H> {
}
/// Inserts an invalid block into the cache, with a given invalid ancestor.
pub fn insert_with_invalid_ancestor(&mut self, header_hash: B256, invalid_ancestor: Arc<H>) {
pub fn insert_with_invalid_ancestor(
&mut self,
header_hash: B256,
invalid_ancestor: BlockWithParent,
) {
if self.get(&header_hash).is_none() {
warn!(target: "consensus::engine", hash=?header_hash, ?invalid_ancestor, "Bad block with existing invalid ancestor");
self.insert_entry(header_hash, invalid_ancestor);
@ -65,12 +68,10 @@ impl<H: Debug> InvalidHeaderCache<H> {
}
/// Inserts an invalid ancestor into the map.
pub fn insert(&mut self, invalid_ancestor: SealedHeader<H>) {
if self.get(&invalid_ancestor.hash()).is_none() {
let hash = invalid_ancestor.hash();
let header = invalid_ancestor.unseal();
warn!(target: "consensus::engine", ?hash, ?header, "Bad block with hash");
self.insert_entry(hash, Arc::new(header));
pub fn insert(&mut self, invalid_ancestor: BlockWithParent) {
if self.get(&invalid_ancestor.block.hash).is_none() {
warn!(target: "consensus::engine", ?invalid_ancestor, "Bad block with hash");
self.insert_entry(invalid_ancestor.block.hash, invalid_ancestor);
// update metrics
self.metrics.unique_inserts.increment(1);
@ -79,11 +80,11 @@ impl<H: Debug> InvalidHeaderCache<H> {
}
}
struct HeaderEntry<H> {
struct HeaderEntry {
/// Keeps track how many times this header has been hit.
hit_count: u8,
/// The actually header entry
header: Arc<H>,
/// The actual header entry
header: BlockWithParent,
}
/// Metrics for the invalid headers cache.
@ -103,13 +104,15 @@ struct InvalidHeaderCacheMetrics {
#[cfg(test)]
mod tests {
use super::*;
use alloy_consensus::Header;
use reth_primitives::SealedHeader;
#[test]
fn test_hit_eviction() {
let mut cache = InvalidHeaderCache::new(10);
let header = Header::default();
let header = SealedHeader::seal(header);
cache.insert(header.clone());
cache.insert(header.block_with_parent());
assert_eq!(cache.headers.get(&header.hash()).unwrap().hit_count, 0);
for hit in 1..INVALID_HEADER_HIT_EVICTION_THRESHOLD {

View File

@ -760,14 +760,14 @@ where
// iterate over ancestors in the invalid cache
// until we encounter the first valid ancestor
let mut current_hash = parent_hash;
let mut current_header = self.invalid_headers.get(&current_hash);
while let Some(header) = current_header {
current_hash = header.parent_hash;
current_header = self.invalid_headers.get(&current_hash);
let mut current_block = self.invalid_headers.get(&current_hash);
while let Some(block) = current_block {
current_hash = block.parent;
current_block = self.invalid_headers.get(&current_hash);
// If current_header is None, then the current_hash does not have an invalid
// ancestor in the cache, check its presence in blockchain tree
if current_header.is_none() &&
if current_block.is_none() &&
self.blockchain.find_block_by_hash(current_hash, BlockSource::Any)?.is_some()
{
return Ok(Some(current_hash))
@ -806,13 +806,13 @@ where
head: B256,
) -> ProviderResult<Option<PayloadStatus>> {
// check if the check hash was previously marked as invalid
let Some(header) = self.invalid_headers.get(&check) else { return Ok(None) };
let Some(block) = self.invalid_headers.get(&check) else { return Ok(None) };
// populate the latest valid hash field
let status = self.prepare_invalid_response(header.parent_hash)?;
let status = self.prepare_invalid_response(block.parent)?;
// insert the head block into the invalid header cache
self.invalid_headers.insert_with_invalid_ancestor(head, header);
self.invalid_headers.insert_with_invalid_ancestor(head, block);
Ok(Some(status))
}
@ -821,10 +821,10 @@ where
/// to a forkchoice update.
fn check_invalid_ancestor(&mut self, head: B256) -> ProviderResult<Option<PayloadStatus>> {
// check if the head was previously marked as invalid
let Some(header) = self.invalid_headers.get(&head) else { return Ok(None) };
let Some(block) = self.invalid_headers.get(&head) else { return Ok(None) };
// populate the latest valid hash field
Ok(Some(self.prepare_invalid_response(header.parent_hash)?))
Ok(Some(self.prepare_invalid_response(block.parent)?))
}
/// Record latency metrics for one call to make a block canonical
@ -1454,7 +1454,7 @@ where
fn on_pipeline_outcome(&mut self, ctrl: ControlFlow) -> RethResult<()> {
// Pipeline unwound, memorize the invalid block and wait for CL for next sync target.
if let ControlFlow::Unwind { bad_block, .. } = ctrl {
warn!(target: "consensus::engine", invalid_hash=?bad_block.hash(), invalid_number=?bad_block.number, "Bad block detected in unwind");
warn!(target: "consensus::engine", invalid_num_hash=?bad_block.block, "Bad block detected in unwind");
// update the `invalid_headers` cache with the new invalid header
self.invalid_headers.insert(*bad_block);
return Ok(())
@ -1673,7 +1673,7 @@ where
self.latest_valid_hash_for_invalid_payload(block.parent_hash)?
};
// keep track of the invalid header
self.invalid_headers.insert(block.header);
self.invalid_headers.insert(block.header.block_with_parent());
PayloadStatus::new(
PayloadStatusEnum::Invalid { validation_error: error.to_string() },
latest_valid_hash,
@ -1782,7 +1782,7 @@ where
let (block, err) = err.split();
warn!(target: "consensus::engine", invalid_number=?block.number, invalid_hash=?block.hash(), %err, "Marking block as invalid");
self.invalid_headers.insert(block.header);
self.invalid_headers.insert(block.header.block_with_parent());
}
}
}
@ -2035,7 +2035,7 @@ mod tests {
.await;
assert_matches!(
res.await,
Ok(Err(BeaconConsensusEngineError::Pipeline(n))) if matches!(*n.as_ref(),PipelineError::Stage(StageError::ChannelClosed))
Ok(Err(BeaconConsensusEngineError::Pipeline(n))) if matches!(*n.as_ref(), PipelineError::Stage(StageError::ChannelClosed))
);
}
@ -2141,7 +2141,7 @@ mod tests {
assert_matches!(
rx.await,
Ok(Err(BeaconConsensusEngineError::Pipeline(n))) if matches!(*n.as_ref(),PipelineError::Stage(StageError::ChannelClosed))
Ok(Err(BeaconConsensusEngineError::Pipeline(n))) if matches!(*n.as_ref(), PipelineError::Stage(StageError::ChannelClosed))
);
}