Improve parent lookup logging (#5451)
* upgrade parent lookup result processing logs to debug, use display instead of debug for BlockError in case a blob parent unknown error is hit, add block root to BlockIsAlreadyKnown * fix compile * fix compile * fix compile
This commit is contained in:
parent
5ce16192c7
commit
21cdc64bfe
@ -2651,7 +2651,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
|
||||
// If the block is relevant, add it to the filtered chain segment.
|
||||
Ok(_) => filtered_chain_segment.push((block_root, block)),
|
||||
// If the block is already known, simply ignore this block.
|
||||
Err(BlockError::BlockIsAlreadyKnown) => continue,
|
||||
Err(BlockError::BlockIsAlreadyKnown(_)) => continue,
|
||||
// If the block is the genesis block, simply ignore this block.
|
||||
Err(BlockError::GenesisBlock) => continue,
|
||||
// If the block is is for a finalized slot, simply ignore this block.
|
||||
@ -2879,7 +2879,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
|
||||
.fork_choice_read_lock()
|
||||
.contains_block(&block_root)
|
||||
{
|
||||
return Err(BlockError::BlockIsAlreadyKnown);
|
||||
return Err(BlockError::BlockIsAlreadyKnown(blob.block_root()));
|
||||
}
|
||||
|
||||
if let Some(event_handler) = self.event_handler.as_ref() {
|
||||
@ -2911,7 +2911,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
|
||||
.fork_choice_read_lock()
|
||||
.contains_block(&block_root)
|
||||
{
|
||||
return Err(BlockError::BlockIsAlreadyKnown);
|
||||
return Err(BlockError::BlockIsAlreadyKnown(block_root));
|
||||
}
|
||||
|
||||
if let Some(event_handler) = self.event_handler.as_ref() {
|
||||
|
@ -190,7 +190,7 @@ pub enum BlockError<T: EthSpec> {
|
||||
/// ## Peer scoring
|
||||
///
|
||||
/// The block is valid and we have already imported a block with this hash.
|
||||
BlockIsAlreadyKnown,
|
||||
BlockIsAlreadyKnown(Hash256),
|
||||
/// The block slot exceeds the MAXIMUM_BLOCK_SLOT_NUMBER.
|
||||
///
|
||||
/// ## Peer scoring
|
||||
@ -832,7 +832,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
|
||||
// already know this block.
|
||||
let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock();
|
||||
if fork_choice_read_lock.contains_block(&block_root) {
|
||||
return Err(BlockError::BlockIsAlreadyKnown);
|
||||
return Err(BlockError::BlockIsAlreadyKnown(block_root));
|
||||
}
|
||||
|
||||
// Do not process a block that doesn't descend from the finalized root.
|
||||
@ -966,7 +966,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
|
||||
SeenBlock::Slashable => {
|
||||
return Err(BlockError::Slashable);
|
||||
}
|
||||
SeenBlock::Duplicate => return Err(BlockError::BlockIsAlreadyKnown),
|
||||
SeenBlock::Duplicate => return Err(BlockError::BlockIsAlreadyKnown(block_root)),
|
||||
SeenBlock::UniqueNonSlashable => {}
|
||||
};
|
||||
|
||||
@ -1784,7 +1784,7 @@ pub fn check_block_relevancy<T: BeaconChainTypes>(
|
||||
.fork_choice_read_lock()
|
||||
.contains_block(&block_root)
|
||||
{
|
||||
return Err(BlockError::BlockIsAlreadyKnown);
|
||||
return Err(BlockError::BlockIsAlreadyKnown(block_root));
|
||||
}
|
||||
|
||||
Ok(block_root)
|
||||
|
@ -1087,7 +1087,7 @@ async fn block_gossip_verification() {
|
||||
assert!(
|
||||
matches!(
|
||||
unwrap_err(harness.chain.verify_block_for_gossip(Arc::new(block.clone())).await),
|
||||
BlockError::BlockIsAlreadyKnown,
|
||||
BlockError::BlockIsAlreadyKnown(_),
|
||||
),
|
||||
"should register any valid signature against the proposer, even if the block failed later verification"
|
||||
);
|
||||
@ -1115,7 +1115,7 @@ async fn block_gossip_verification() {
|
||||
.verify_block_for_gossip(block.clone())
|
||||
.await
|
||||
.expect_err("should error when processing known block"),
|
||||
BlockError::BlockIsAlreadyKnown
|
||||
BlockError::BlockIsAlreadyKnown(_)
|
||||
),
|
||||
"the second proposal by this validator should be rejected"
|
||||
);
|
||||
|
@ -113,7 +113,7 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlockConten
|
||||
let (gossip_verified_block, gossip_verified_blobs) =
|
||||
match block_contents.into_gossip_verified_block(&chain) {
|
||||
Ok(b) => b,
|
||||
Err(BlockContentsError::BlockError(BlockError::BlockIsAlreadyKnown))
|
||||
Err(BlockContentsError::BlockError(BlockError::BlockIsAlreadyKnown(_)))
|
||||
| Err(BlockContentsError::BlobError(
|
||||
beacon_chain::blob_verification::GossipBlobError::RepeatBlob { .. },
|
||||
)) => {
|
||||
@ -133,7 +133,7 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlockConten
|
||||
log,
|
||||
"Not publishing block - not gossip verified";
|
||||
"slot" => slot,
|
||||
"error" => ?e
|
||||
"error" => %e
|
||||
);
|
||||
return Err(warp_utils::reject::custom_bad_request(e.to_string()));
|
||||
}
|
||||
|
@ -963,7 +963,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
|
||||
self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
|
||||
return None;
|
||||
}
|
||||
Err(BlockError::BlockIsAlreadyKnown) => {
|
||||
Err(BlockError::BlockIsAlreadyKnown(_)) => {
|
||||
debug!(
|
||||
self.log,
|
||||
"Gossip block is already known";
|
||||
|
@ -276,7 +276,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
|
||||
"slot" => %slot,
|
||||
);
|
||||
}
|
||||
Err(BlockError::BlockIsAlreadyKnown) => {
|
||||
Err(BlockError::BlockIsAlreadyKnown(_)) => {
|
||||
debug!(
|
||||
self.log,
|
||||
"Blobs have already been imported";
|
||||
@ -639,7 +639,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
|
||||
peer_action: Some(PeerAction::LowToleranceError),
|
||||
})
|
||||
}
|
||||
BlockError::BlockIsAlreadyKnown => {
|
||||
BlockError::BlockIsAlreadyKnown(_) => {
|
||||
// This can happen for many reasons. Head sync's can download multiples and parent
|
||||
// lookups can download blocks before range sync
|
||||
Ok(())
|
||||
|
@ -811,7 +811,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
|
||||
let root = lookup.block_root();
|
||||
trace!(self.log, "Single block processing failed"; "block" => %root, "error" => %e);
|
||||
match e {
|
||||
BlockError::BlockIsAlreadyKnown => {
|
||||
BlockError::BlockIsAlreadyKnown(_) => {
|
||||
// No error here
|
||||
return Ok(None);
|
||||
}
|
||||
@ -898,17 +898,17 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
|
||||
match &result {
|
||||
BlockProcessingResult::Ok(status) => match status {
|
||||
AvailabilityProcessingStatus::Imported(block_root) => {
|
||||
trace!(self.log, "Parent block processing succeeded"; &parent_lookup, "block_root" => ?block_root)
|
||||
debug!(self.log, "Parent block processing succeeded"; &parent_lookup, "block_root" => ?block_root)
|
||||
}
|
||||
AvailabilityProcessingStatus::MissingComponents(_, block_root) => {
|
||||
trace!(self.log, "Parent missing parts, triggering single block lookup "; &parent_lookup,"block_root" => ?block_root)
|
||||
debug!(self.log, "Parent missing parts, triggering single block lookup "; &parent_lookup,"block_root" => ?block_root)
|
||||
}
|
||||
},
|
||||
BlockProcessingResult::Err(e) => {
|
||||
trace!(self.log, "Parent block processing failed"; &parent_lookup, "error" => %e)
|
||||
debug!(self.log, "Parent block processing failed"; &parent_lookup, "error" => %e)
|
||||
}
|
||||
BlockProcessingResult::Ignored => {
|
||||
trace!(
|
||||
debug!(
|
||||
self.log,
|
||||
"Parent block processing job was ignored";
|
||||
"action" => "re-requesting block",
|
||||
@ -954,7 +954,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
|
||||
self.request_parent(parent_lookup, cx);
|
||||
}
|
||||
BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(_))
|
||||
| BlockProcessingResult::Err(BlockError::BlockIsAlreadyKnown { .. }) => {
|
||||
| BlockProcessingResult::Err(BlockError::BlockIsAlreadyKnown(_)) => {
|
||||
// Check if the beacon processor is available
|
||||
let Some(beacon_processor) = cx.beacon_processor_if_enabled() else {
|
||||
return trace!(
|
||||
|
@ -458,7 +458,11 @@ fn test_parent_lookup_happy_path() {
|
||||
rig.expect_empty_network();
|
||||
|
||||
// Processing succeeds, now the rest of the chain should be sent for processing.
|
||||
bl.parent_block_processed(chain_hash, BlockError::BlockIsAlreadyKnown.into(), &mut cx);
|
||||
bl.parent_block_processed(
|
||||
chain_hash,
|
||||
BlockError::BlockIsAlreadyKnown(block_root).into(),
|
||||
&mut cx,
|
||||
);
|
||||
rig.expect_parent_chain_process();
|
||||
let process_result = BatchProcessResult::Success {
|
||||
was_non_empty: true,
|
||||
@ -1117,7 +1121,11 @@ fn test_same_chain_race_condition() {
|
||||
// the processing result
|
||||
if i + 2 == depth {
|
||||
// one block was removed
|
||||
bl.parent_block_processed(chain_hash, BlockError::BlockIsAlreadyKnown.into(), &mut cx)
|
||||
bl.parent_block_processed(
|
||||
chain_hash,
|
||||
BlockError::BlockIsAlreadyKnown(block.canonical_root()).into(),
|
||||
&mut cx,
|
||||
)
|
||||
} else {
|
||||
bl.parent_block_processed(
|
||||
chain_hash,
|
||||
|
Loading…
Reference in New Issue
Block a user