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

feat(sync-v2): Stop running callLater when a block is voided in handle_get_peer_block_hashes() #873

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Nov 11, 2023

Motivation

The handle_get_peer_block_hashes() method used to schedule a later call if one of the requested blocks was voided. This behavior was implemented due to issues in a previous implementation of the n-ary search. The current n-ary search implementation properly handles when a reorg occur during the search.

Acceptance Criteria

  1. Stop scheduling a later call if a block is voided in handle_get_peer_block_hashes().
  2. If a voided block is found, skip it and all following blocks in the response of the handle_get_peer_block_hashes(). It's safe to do it because the n-ary search algorithm can handle empty responses.
  3. It's safe to skip the first voided block and all following blocks because this method assumes the list of heights will be increasingly sorted so all following blocks must be voided too.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@msbrogli msbrogli added the enhancement New feature or request label Nov 11, 2023
@msbrogli msbrogli requested a review from glevco November 11, 2023 06:36
@msbrogli msbrogli self-assigned this Nov 11, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner November 11, 2023 06:36
Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (090bd71) 85.42% compared to head (1a0f0b3) 85.36%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #873      +/-   ##
==========================================
- Coverage   85.42%   85.36%   -0.07%     
==========================================
  Files         281      281              
  Lines       22353    22354       +1     
  Branches     3388     3388              
==========================================
- Hits        19096    19082      -14     
- Misses       2582     2591       +9     
- Partials      675      681       +6     
Files Coverage Δ
hathor/p2p/sync_v2/agent.py 79.96% <80.00%> (-0.59%) ⬇️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
@msbrogli msbrogli force-pushed the feat/sync-v2-handle_get_peer_block_hashes branch from 1a0f0b3 to 0ce1dda Compare November 13, 2023 19:30
@msbrogli msbrogli merged commit 0ce1dda into master Nov 13, 2023
6 of 8 checks passed
@msbrogli msbrogli deleted the feat/sync-v2-handle_get_peer_block_hashes branch November 13, 2023 19:31
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants