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

Remove zebra_state::Request::FindBlockHeaders #1432

Closed
hdevalence opened this issue Dec 2, 2020 · 1 comment
Closed

Remove zebra_state::Request::FindBlockHeaders #1432

hdevalence opened this issue Dec 2, 2020 · 1 comment
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup

Comments

@hdevalence
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The zebra_state::Request::FindBlockHeaders request variant duplicates functionality of Request::FindBlockHashes, but adds more work, blocking the state queue from processing other requests. Once the list of block hashes has been atomically computed, there's no need to atomically fetch all of the blocks to obtain their headers, so there is no correctness problem with moving that phase of the work out of the state service. This simplifies the state implementation.

Describe the solution you'd like

  1. Change FindBlockHashes to include a max_len: usize, passed directly to self.find_chain_hashes;

  2. Replace the existing use of FindBlockHeaders in the Inbound service by a FindBlockHashes, followed by a sequence of Block requests;

  3. Remove FindBlockHeaders from the state API.

Describe alternatives you've considered

Leaving the code as-is. This prevents a clean separation of concerns and prevents us from composing middleware in front of the state service.

@hdevalence hdevalence added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Dec 2, 2020
@zfnd-bot zfnd-bot bot added this to To Do in 🦓 Dec 2, 2020
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Dec 3, 2020
@mpguerra mpguerra added this to No Estimate in Effort Affinity Grouping Dec 18, 2020
@dconnolly dconnolly moved this from No Estimate to S - 3 in Effort Affinity Grouping Feb 4, 2021
@dconnolly dconnolly added the A-rust Area: Updates to Rust code label Feb 4, 2021
@teor2345 teor2345 added C-cleanup Category: This is a cleanup P-Low and removed C-enhancement Category: This is an improvement labels Feb 4, 2021
@teor2345
Copy link
Collaborator

teor2345 commented Jun 2, 2022

This is obsolete - with the new database and state API, we should:

  • move this request to the ReadStateService
  • only query the finalized state block headers column family

@teor2345 teor2345 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup
Projects
No open projects
Development

No branches or pull requests

4 participants