Skip to content

fix(sync-v2): Fix dependencies update for the first block #897

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

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Dec 20, 2023

Motivation

During QA, a full node syncing from scratch got stuck with the following logs:

2023-12-20 22:26:29 [info     ] [hathor.p2p.sync_v2.agent] requesting blocks streaming    end_block=[4094036, "b'\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x01[\\xf20\\x82a\\x08\\x14\\xb2\\x83\\xe8\\x82\\xfc\\x8d*\\xe0f Ri6\\xb0@OS'"] peer=5a3040d quantity=424075 start_block=[3669962, "b'\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x11\\x00\\xd7\\x93o\\xd9Y\\x02\\x0e\\x85\\xec\\xa6y*\\x0b7:LD;\\xa2\\xc2V\\x04'"]
2023-12-20 22:26:29 [info     ] [hathor.p2p.sync_v2.agent] requesting transactions streaming first_block=0000000000000062a0a3c362758d468f37692a14f1cf5cf2df2e08d5c92eab5a last_block=00000000000000005ea2ade4fdc528cd7ce5bb64bf942350e28c46aa8f79cf47 peer=5a3040d start_from=[]
2023-12-20 22:26:29 [info     ] [hathor.p2p.sync_v2.transaction_streaming_client] unexpected vertex received     peer=5a3040d tx_id=00000742224ab9e7084908b61a43ccbd9f0125b6e075e04d7f2355e8a67b1a06
2023-12-20 22:26:29 [info     ] [hathor.p2p.sync_v2.agent] tx streaming failed            peer=5a3040d reason=UnexpectedVertex('00000742224ab9e7084908b61a43ccbd9f0125b6e075e04d7f2355e8a67b1a06')

It got stuck because it was not adding dependencies it already had to the _existing_deps, so it was closing the streaming after receiving an unexpected vertex. In this case, the block has a direct dependency of the transaction 00000742224ab9e7084908b61a43ccbd9f0125b6e075e04d7f2355e8a67b1a06.

Acceptance Criteria

  1. Call TransactionStreamingClient._update_dependencies() instead of running an old-and-buggy duplicated code.
  2. Rename TransactionStreamingClient.update_dependencies() to _update_dependencies().

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 bug Something isn't working label Dec 20, 2023
@msbrogli msbrogli self-assigned this Dec 20, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner December 20, 2023 23:16
Copy link

codecov bot commented Dec 20, 2023

Codecov Report

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

Comparison is base (27381c1) 85.35% compared to head (8261720) 85.39%.
Report is 3 commits behind head on master.

Files Patch % Lines
hathor/reactor/reactor.py 63.33% 8 Missing and 3 partials ⚠️
hathor/debug_resources.py 70.00% 3 Missing ⚠️
hathor/stratum/stratum.py 57.14% 2 Missing and 1 partial ⚠️
hathor/p2p/sync_v2/transaction_streaming_client.py 60.00% 2 Missing ⚠️
hathor/websocket/factory.py 71.42% 2 Missing ⚠️
hathor/builder/cli_builder.py 66.66% 1 Missing ⚠️
hathor/wallet/resources/thin_wallet/send_tokens.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #897      +/-   ##
==========================================
+ Coverage   85.35%   85.39%   +0.03%     
==========================================
  Files         282      283       +1     
  Lines       22280    22305      +25     
  Branches     3366     3363       -3     
==========================================
+ Hits        19018    19048      +30     
+ Misses       2592     2588       -4     
+ Partials      670      669       -1     

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

@msbrogli msbrogli merged commit 8261720 into master Dec 20, 2023
@msbrogli msbrogli deleted the fix/sync-v2-tx-streaming-deps branch December 20, 2023 23:42
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
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants