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

ETCM-719: Remove header work queue from persisted state and fix final header skeleton size #951

Merged
merged 6 commits into from
Mar 26, 2021

Conversation

1015bit
Copy link
Contributor

@1015bit 1015bit commented Mar 24, 2021

Description

  • a previous refactoring moved the block header work queue to sync state to keep it together with the work queues for block bodies and receipts - this had the unintended side-effect that it also got persisted (which we don't want to happen)
  • there was a bug that happened under certain conditions when downloading the final header skeleton
    • let's say we still need to download skeleton headers for the range 5100 to 5200 and our max batch size is 30 the request that would be made would have been to get 4 headers with a gap of 30 - so we're actually requesting up to 5220 which the peer might not have. In that case it would send us back only 3 headers which would fail our validation (because we expect 4).
    • improved the logging of outstanding requests: it would sometimes say waiting for [0] requests when it was actually waiting for the header skeleton
  • improves the check whether the peer that we're requesting the skeleton headers from has a high enough best block

Proposed Solution

  • moved the header work queue out of sync state again
  • changed the calculation of limit so it would only request 3 headers in the example above, so the header skeleton would contain headers 5100 to 5190. After that a new skeleton would be requested with one batch of size 10.

Testing

@1015bit 1015bit marked this pull request as ready for review March 26, 2021 13:53
Copy link
Contributor

@AnastasiiaL AnastasiiaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest I understand and it makes sense 👍

@1015bit 1015bit merged commit 9893aa0 into develop Mar 26, 2021
@1015bit 1015bit deleted the fix/etcm-719 branch March 26, 2021 16:50
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.

3 participants