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

refactor(sync-v2): Refactor sync_v2 agent to hold (block_height, block_id) information in an internal namedtuple #829

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Oct 26, 2023

Motivation

Sync-v2 agent code uses the pair (block_height, block_hash) multiple times. So this PR refactors it to use a namedtuple BlockInfo (instead of handling with both information separately).

Acceptance Criteria

  1. Add hathor.p2p.sync_v2.agent.BlockInfo.
  2. Add hathor.p2p.sync_v2.agent.NodeBlockSync.get_my_best_block() method.
  3. Refactor find_best_common_block() and run_sync_blocks() methds to use BlockInfo.
  4. Rename variables (synced, not_synced) to (lo, hi) in the n-ary search.
  5. There are two behavior changes in this PR: (i) change of keys in the get_status(), and (ii) using lo=0 instead of lo=self.synced_block.height.

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 self-assigned this Oct 26, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner October 26, 2023 23:39
@msbrogli msbrogli force-pushed the refactor/sync-v2-agent branch from 57bf117 to 78373d2 Compare October 27, 2023 03:48
@msbrogli msbrogli requested a review from glevco October 27, 2023 03:54
@msbrogli msbrogli force-pushed the refactor/sync-v2-agent branch 8 times, most recently from d178550 to c0cdfa3 Compare October 27, 2023 06:35
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
glevco
glevco previously approved these changes Oct 27, 2023
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
jansegre
jansegre previously approved these changes Oct 27, 2023
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
hathor/p2p/sync_v2/agent.py Show resolved Hide resolved
@msbrogli msbrogli force-pushed the refactor/sync-v2-agent branch from c0cdfa3 to b095c96 Compare October 27, 2023 16:09
@msbrogli msbrogli dismissed stale reviews from glevco and jansegre via cbba621 October 27, 2023 16:15
@msbrogli msbrogli force-pushed the refactor/sync-v2-agent branch from b095c96 to cbba621 Compare October 27, 2023 16:15
@msbrogli msbrogli changed the title refactor(sync-v2): Use BlockInfo in sync_v2 agent to hold (block_height, block_id) information refactor(sync-v2): Refactor sync_v2 agent to hold (block_height, block_id) information in an internal namedtuple Oct 27, 2023
@msbrogli msbrogli force-pushed the refactor/sync-v2-agent branch from 5a4c198 to f880a62 Compare October 27, 2023 16:25
glevco
glevco previously approved these changes Oct 27, 2023
jansegre
jansegre previously approved these changes Oct 27, 2023
@msbrogli msbrogli dismissed stale reviews from jansegre and glevco via 59d5104 October 27, 2023 18:09
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (20474fd) 84.56% compared to head (59d5104) 84.63%.

❗ Current head 59d5104 differs from pull request most recent head 89d347e. Consider uploading reports for the commit 89d347e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #829      +/-   ##
==========================================
+ Coverage   84.56%   84.63%   +0.07%     
==========================================
  Files         270      270              
  Lines       22291    22290       -1     
  Branches     3406     3404       -2     
==========================================
+ Hits        18850    18865      +15     
+ Misses       2769     2757      -12     
+ Partials      672      668       -4     
Files Coverage Δ
hathor/p2p/sync_v2/agent.py 75.00% <100.00%> (+0.83%) ⬆️

... and 5 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 refactor/sync-v2-agent branch from 59d5104 to 89d347e Compare October 27, 2023 18:46
@msbrogli msbrogli merged commit 89d347e into master Oct 27, 2023
@msbrogli msbrogli deleted the refactor/sync-v2-agent branch October 27, 2023 18:47
@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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants