change(state): Stop re-downloading blocks that are in non-finalized side chains

[arya2]

Motivation

The sync task, inbound downloader, and the block verifier need to check if a block hash is present in the state to avoid redundant work and unnecessary errors. They currently call the state service with Depth requests, but this only checks if a block hash has been committed to the best chain, and returns None when the block is in a side chain or queued for validation.

This PR adds a Contains request to the state for checking if a block hash is present in any chain and uses it from sync, inbound and the block verifier instead of Depth.

Closes #862.

Solution

• Add a Contains request to the state service that responds with an Option<BlockLocation>
  • Checks if the block hash is in:
    • non-finalized best chain
    • non-finalized side chains
    • finalized best chain
• Call state with Contains request from sync, inbound, and block verifier to check if the block hash is in any chain

Review

Anyone can review.

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

[codecov]

Codecov Report

Merging #6335 (2c15632) into main (fc32e68) will increase coverage by 0.12%.
The diff coverage is 71.11%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6335      +/-   ##
==========================================
+ Coverage   77.71%   77.83%   +0.12%     
==========================================
  Files         304      304              
  Lines       39583    39614      +31     
==========================================
+ Hits        30761    30835      +74     
+ Misses       8822     8779      -43     

[upbqdn]

Some sections mentioned in #862 are not in this PR or are outdated. I think we should update #862 before we merge this PR.

[arya2]

did you want to catch up and talk it through?

Sure!

[mpguerra]

did you want to catch up and talk it through?

Sure!

Can we have an update here once you both have a catch up? :)

[arya2]

@teor2345 it only checks the chains now, I'll open a follow-up PR for checking the queues.

[teor2345]

Thanks, this looks like a worthwhile change, and it's low risk.

[arya2]

Can we have an update here once you both have a catch up? :)

We'll try merging this PR (which is low risk because it only checks for blocks that have been validated and committed), the follow-up PR that checks the queues is riskier and less important, so we'll try that too if it works well without any further changes but will close it otherwise.