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): Fix issue when a reorg occurs during a streaming of transactions #855

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Nov 9, 2023

Motivation

An unhandled exception is raised by current_block.get_next_block_best_chain() when a reorg occurs during a streaming of transactions. This unhandled exception causes the send_next() to stop being called because it is not scheduled to run anymore, so the other peer stays hanging waiting for more transactions.

Acceptance Criteria

  1. Check if block is still in the best blockchain before calling get_next_block_best_chain(). If not, stop the streaming.
  2. Add a safe_send_next() that will either schedule the next call or stop the streaming.
  3. Add StreamEnd.INTERNAL_ERROR.

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 requested a review from glevco November 9, 2023 15:44
@msbrogli msbrogli self-assigned this Nov 9, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner November 9, 2023 15:44
@msbrogli msbrogli force-pushed the feat/sync-v2-streamers-reorg-issue branch 2 times, most recently from 8d5505f to df21071 Compare November 9, 2023 15:54
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

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

Comparison is base (db38db4) 85.33% compared to head (df21071) 85.43%.

❗ Current head df21071 differs from pull request most recent head 9fff604. Consider uploading reports for the commit 9fff604 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #855      +/-   ##
==========================================
+ Coverage   85.33%   85.43%   +0.09%     
==========================================
  Files         282      282              
  Lines       22247    22253       +6     
  Branches     3363     3363              
==========================================
+ Hits        18984    19011      +27     
+ Misses       2595     2576      -19     
+ Partials      668      666       -2     
Files Coverage Δ
hathor/p2p/sync_v2/streamers.py 87.30% <37.50%> (-3.56%) ⬇️

... and 7 files with indirect coverage changes

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

@msbrogli msbrogli force-pushed the feat/sync-v2-streamers-reorg-issue branch from df21071 to 9fff604 Compare November 9, 2023 17:05
@msbrogli msbrogli merged commit 9fff604 into master Nov 9, 2023
1 check passed
@msbrogli msbrogli deleted the feat/sync-v2-streamers-reorg-issue branch November 9, 2023 17:05
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
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
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants