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

fix(sync-v2): adjust init assertion #875

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

jansegre
Copy link
Member

Motivation

During the QA of sync-v2 some errors were caught during a sync from scratch (see here). In particular this one:

2023-11-12 05:10:57 [warning  ] [hathor.p2p.protocol] send error                     msg=internal error peer_id=7181d23 remote=127.0.0.1:9001
2023-11-12 05:10:57 [error    ] [hathor.p2p.sync_v2.agent] unhandled exception            peer=7181d23
Traceback (most recent call last):
  File "/Users/jan/Projects/hathor-core/hathor/p2p/sync_v2/agent.py", line 296, in run_sync
    yield self._run_sync()
  File "/Users/jan/Library/Caches/pypoetry/virtualenvs/hathor-JIQwCn9w-py3.11/lib/python3.11/site-packages/twisted/internet/defer.py", line 1693, in _inlineCallbacks
    result = context.run(
             ^^^^^^^^^^^^
  File "/Users/jan/Library/Caches/pypoetry/virtualenvs/hathor-JIQwCn9w-py3.11/lib/python3.11/site-packages/twisted/python/failure.py", line 518, in throwExceptionIntoGenerator
    return g.throw(self.type, self.value, self.tb)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jan/Projects/hathor-core/hathor/p2p/sync_v2/agent.py", line 311, in _run_sync
    is_block_synced = yield self.run_sync_blocks()
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jan/Library/Caches/pypoetry/virtualenvs/hathor-JIQwCn9w-py3.11/lib/python3.11/site-packages/twisted/internet/defer.py", line 1697, in _inlineCallbacks
    result = context.run(gen.send, result)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jan/Projects/hathor-core/hathor/p2p/sync_v2/agent.py", line 397, in run_sync_blocks
    reason = yield self.start_transactions_streaming(partial_blocks)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jan/Projects/hathor-core/hathor/p2p/sync_v2/agent.py", line 834, in start_transactions_streaming
    self._tx_streaming_client = TransactionStreamingClient(self,
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jan/Projects/hathor-core/hathor/p2p/sync_v2/transaction_streaming_client.py", line 87, in __init__
    assert self._waiting_for
AssertionError
2023-11-12 05:10:57 [info     ] [hathor.p2p.protocol] disconnected                   peer_id=7181d23 reason=Connection was closed cleanly. remote=127.0.0.1:9001

This indicates that the assertion inside TransactionStreamingClient.__init__ is not correct.

Acceptance Criteria

  • Adjust TransactionStreamingClient.__init__ assertion to account for _existing_deps and not only _waiting_for.

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

@jansegre jansegre self-assigned this Nov 14, 2023
@jansegre jansegre requested a review from msbrogli as a code owner November 14, 2023 16:53
@jansegre jansegre requested a review from glevco November 14, 2023 16:54
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (29a622f) 85.29% compared to head (e09351c) 85.26%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #875      +/-   ##
==========================================
- Coverage   85.29%   85.26%   -0.03%     
==========================================
  Files         281      281              
  Lines       22364    22363       -1     
  Branches     3388     3388              
==========================================
- Hits        19075    19068       -7     
+ Misses       2610     2608       -2     
- Partials      679      687       +8     

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

@jansegre jansegre force-pushed the fix/sync-v2-tx-streaming-client-assert branch from 019fc6c to c4d350c Compare November 20, 2023 19:03
@jansegre jansegre mentioned this pull request Nov 20, 2023
2 tasks
@jansegre jansegre force-pushed the fix/sync-v2-tx-streaming-client-assert branch from c4d350c to 862aa73 Compare November 23, 2023 16:42
@jansegre jansegre force-pushed the fix/sync-v2-tx-streaming-client-assert branch from 862aa73 to e09351c Compare November 23, 2023 17:32
@jansegre jansegre force-pushed the fix/sync-v2-tx-streaming-client-assert branch from e09351c to 0643529 Compare November 24, 2023 14:57
@jansegre jansegre merged commit 2a86be0 into master Nov 24, 2023
8 checks passed
@jansegre jansegre deleted the fix/sync-v2-tx-streaming-client-assert branch November 24, 2023 14:58
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