Skip to content

feat(sync-v2): Implement new protocol for synchronizing transactions #850

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
Nov 8, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Nov 3, 2023

Motivation

This PR addresses two significant issues in the current implementation:

  1. In Sync-v2, a global dependencies index was utilized across all connections.
  2. Sync-v2 agents insisted on the fulfillment of all dependencies before progressing with the synchronization.

These combined challenges led to potential deadlocks. Specifically, if one peer introduced a dependency that was unavailable from others, it could cause all agents to endlessly attempt to satisfy this elusive dependency from their peers—a futile endeavor.

To counter this, we've established a new transaction synchronization protocol from blocks. With this enhanced approach, every agent manages its own unique dependency list, allowing for independent synchronization with each peer. Furthermore, it allows the synchronization loop to function even with outstanding dependencies.

This PR changes the behavior of the GET-TRANSACTIONS-BFS message. The new parameters are start_from, first_block_hash, and end_block_hash. The response is a sequence of TRANSACTION messages, each containing one transaction.

New protocol

The new protocol syncs blocks first and keeps track of partial_blocks which are the blocks that cannot be fully validated because of a missing dependency. Then, the partial_blocks list is passed to the transaction streamer. The streamer will then send all transactions confirmed by the first block followed by all transactions confirmed by the second block and so on. To determine the precise order of transactions within each block, the streamer implements a Breadth-First Search (BFS) algorithm, tracing back from the block in question.

To illustrate the ordering, consider partial_blocks enumerated as [b0, b1, b2, ..., bn], where tx_i_j represents the i-th transaction within the j-th block (b_j). The transaction streamer would then relay transactions in the ensuing sequence: [tx_0_0, tx_1_0, tx_2_0, ... tx_N0_0, tx_0_1, tx_1_1, tx_2_1, ..., tx_N1_1, ..., tx_0_n, tx_1_n, tx_2_n, ..., tx_Nn_n]. Here, N0, N1, ..., Nn correspond to the total number of transactions validated by blocks b0, b1, ..., bn respectively.

Acceptance Criteria

  1. Remove DepsIndex, MemoryDepsIndex, RocksDBDepsIndex because this index is not used anymore.
  2. Remove HathorManager._sync_v2_resume_validations() and HathorManager.sync_v2_step_validations() because they are not used anymore.
  3. Add STOP_TRANSACTIONS_STREAMING message to allow clients to request servers to stop streaming.
  4. Remove sync_v2.NodeBlockSync.run_sync_transactions() because it is not used anymore.
  5. Stop using partially validated vertex. All vertices added to the DAG are immediately fully validated.
  6. Modify TransactionsStreamingServer to send transactions according to the new protocol.
  7. Change fixed seed in tests/consensus/test_soft_voided.py because this test was failing.

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 Nov 3, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner November 3, 2023 15:27
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch 2 times, most recently from 9e89dea to 40c4849 Compare November 3, 2023 16:22
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from de60b5c to e37b28a Compare November 3, 2023 16:25
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from 40c4849 to b79b11b Compare November 3, 2023 16:26
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from e37b28a to 952a22d Compare November 3, 2023 17:52
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from b79b11b to b8f38ba Compare November 3, 2023 18:24
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from 952a22d to 6d9b085 Compare November 3, 2023 18:25
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from b8f38ba to 7effa61 Compare November 3, 2023 18:26
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch 3 times, most recently from 21dafab to cf31227 Compare November 3, 2023 20:15
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from 6dd0b59 to eab8986 Compare November 3, 2023 21:06
Base automatically changed from feat/sync-v2-improvements-3 to master November 3, 2023 21:06
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from cf31227 to 8951ced Compare November 3, 2023 21:15
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch 2 times, most recently from 84e9d89 to a3658a7 Compare November 6, 2023 19:53
jansegre
jansegre previously approved these changes Nov 6, 2023
jansegre
jansegre previously approved these changes Nov 6, 2023
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from e08fa46 to c6ac0f4 Compare November 7, 2023 03:05
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

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

Comparison is base (f528a06) 85.16% compared to head (acb577d) 85.45%.

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   85.16%   85.45%   +0.28%     
==========================================
  Files         285      282       -3     
  Lines       22652    22242     -410     
  Branches     3443     3361      -82     
==========================================
- Hits        19292    19007     -285     
+ Misses       2677     2570     -107     
+ Partials      683      665      -18     
Files Coverage Δ
hathor/indexes/manager.py 91.90% <ø> (+0.16%) ⬆️
hathor/manager.py 80.51% <100.00%> (-1.70%) ⬇️
hathor/p2p/manager.py 72.58% <ø> (-0.07%) ⬇️
hathor/p2p/messages.py 100.00% <100.00%> (ø)
hathor/p2p/sync_v2/exception.py 100.00% <100.00%> (ø)
hathor/p2p/sync_v2/mempool.py 93.42% <ø> (ø)
hathor/p2p/sync_v2/streamers.py 90.86% <96.87%> (+0.80%) ⬆️
hathor/p2p/sync_v2/blockchain_streaming_client.py 81.01% <68.75%> (+4.38%) ⬆️
hathor/p2p/sync_v2/transaction_streaming_client.py 90.09% <91.30%> (+14.68%) ⬆️
hathor/p2p/sync_v2/agent.py 79.96% <78.12%> (+2.33%) ⬆️

... and 9 files with indirect coverage changes

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

@msbrogli msbrogli requested a review from glevco November 7, 2023 18:39
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch 3 times, most recently from 75ec9fc to 2730df9 Compare November 8, 2023 18:16
glevco
glevco previously approved these changes Nov 8, 2023
glevco
glevco previously approved these changes Nov 8, 2023
jansegre
jansegre previously approved these changes Nov 8, 2023
@msbrogli msbrogli dismissed stale reviews from jansegre and glevco via 26ecf96 November 8, 2023 21:04
jansegre
jansegre previously approved these changes Nov 8, 2023
glevco
glevco previously approved these changes Nov 8, 2023
@msbrogli msbrogli dismissed stale reviews from glevco and jansegre via acb577d November 8, 2023 21:12
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from 26ecf96 to acb577d Compare November 8, 2023 21:12
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-4 branch from acb577d to d0a1ff2 Compare November 8, 2023 21:31
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