Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore unknown block root events for processing blocks #5682

Closed
wants to merge 1 commit into from

Conversation

dapplion
Copy link
Collaborator

@dapplion dapplion commented May 1, 2024

Issue Addressed

From testing on v5.2.0 of Sepolia and Gnosis there is a very large volume of UnknownBlockHashFromAttestation and one single block lookup created every slot.

UnknownBlockHashFromAttestation errors are triggered if the block is not on fork-choice, but we may have the block stuck somewhere in the processing pipeline.

Proposed Changes

This PR attempts to prevent UnknownBlockHashFromAttestation events for blocks that are not yet on fork-choice but we are processing.

Caveats

If a single peer imports the block before we receive it and sends us an attestation we will still have the problem of constant lookups for blocks that arrive anyway. Not sure if this is an issue at all, but worth to think about it.

We have an ample collection of places a block may be stuck into:

  • database
  • fork-choice
  • early attester cache
  • da_checker
  • pre_import_cache (former processing_cache)
  • duplicate_cache

ReqResp lookups only check the da_checker (from #5681) while other codepaths check more places. Might be worth it to consolidate their usage a bit to have more consistency.

@dapplion
Copy link
Collaborator Author

dapplion commented Sep 4, 2024

Does not apply anymore, fixed by the debounce of similar unknown attestation events and

match self.chain.get_block_process_status(&block_root) {
// Unknown block, continue request to download
BlockProcessStatus::Unknown => {}
// Block is known are currently processing, expect a future event with the result of
// processing.
BlockProcessStatus::NotValidated { .. } => {
// Lookup sync event safety: If the block is currently in the processing cache, we
// are guaranteed to receive a `SyncMessage::GossipBlockProcessResult` that will
// make progress on this lookup
return Ok(LookupRequestResult::Pending("block in processing cache"));
}
// Block is fully validated. If it's not yet imported it's waiting for missing block
// components. Consider this request completed and do nothing.
BlockProcessStatus::ExecutionValidated { .. } => {
return Ok(LookupRequestResult::NoRequestNeeded)
}
}

@dapplion dapplion closed this Sep 4, 2024
@dapplion dapplion deleted the skip-unknown-root-events branch September 4, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant