-
Notifications
You must be signed in to change notification settings - Fork 30
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9e89dea
to
40c4849
Compare
de60b5c
to
e37b28a
Compare
40c4849
to
b79b11b
Compare
e37b28a
to
952a22d
Compare
b79b11b
to
b8f38ba
Compare
952a22d
to
6d9b085
Compare
b8f38ba
to
7effa61
Compare
1 task
21dafab
to
cf31227
Compare
msbrogli
commented
Nov 3, 2023
msbrogli
commented
Nov 3, 2023
msbrogli
commented
Nov 3, 2023
6dd0b59
to
eab8986
Compare
cf31227
to
8951ced
Compare
jansegre
reviewed
Nov 3, 2023
84e9d89
to
a3658a7
Compare
jansegre
previously approved these changes
Nov 6, 2023
msbrogli
commented
Nov 6, 2023
18ea220
to
b29481a
Compare
jansegre
previously approved these changes
Nov 6, 2023
b29481a
to
4fa759f
Compare
e08fa46
to
c6ac0f4
Compare
Codecov ReportAttention:
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
☔ View full report in Codecov by Sentry. |
glevco
reviewed
Nov 7, 2023
75ec9fc
to
2730df9
Compare
glevco
previously approved these changes
Nov 8, 2023
glevco
previously approved these changes
Nov 8, 2023
jansegre
previously approved these changes
Nov 8, 2023
jansegre
previously approved these changes
Nov 8, 2023
glevco
previously approved these changes
Nov 8, 2023
26ecf96
to
acb577d
Compare
acb577d
to
d0a1ff2
Compare
jansegre
approved these changes
Nov 8, 2023
glevco
approved these changes
Nov 8, 2023
1 task
2 tasks
Open
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
This PR addresses two significant issues in the current implementation:
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 arestart_from
,first_block_hash
, andend_block_hash
. The response is a sequence ofTRANSACTION
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, thepartial_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]
, wheretx_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 blocksb0, b1, ..., bn
respectively.Acceptance Criteria
DepsIndex
,MemoryDepsIndex
,RocksDBDepsIndex
because this index is not used anymore.HathorManager._sync_v2_resume_validations()
andHathorManager.sync_v2_step_validations()
because they are not used anymore.STOP_TRANSACTIONS_STREAMING
message to allow clients to request servers to stop streaming.sync_v2.NodeBlockSync.run_sync_transactions()
because it is not used anymore.TransactionsStreamingServer
to send transactions according to the new protocol.tests/consensus/test_soft_voided.py
because this test was failing.Checklist
master
, confirm this code is production-ready and can be included in future releases as soon as it gets merged