-
Notifications
You must be signed in to change notification settings - Fork 23
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
Genesis milestone 12 - Chain Sync Jumping #1033
Conversation
696280a
to
cae9d98
Compare
c0ec3b6
to
a2bfae0
Compare
664722d
to
8400afc
Compare
b34ada7
to
3164620
Compare
d57042c
to
4d8c607
Compare
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/Genesis/Tests/CSJ.hs
Show resolved
Hide resolved
3dc8fe2
to
ed5e5af
Compare
84fce19
to
5db13db
Compare
5db13db
to
f6764e7
Compare
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/Genesis/Setup/Classifiers.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/Genesis/Tests/CSJ.hs
Show resolved
Hide resolved
then "All headers may be downloaded twice" | ||
else "There exist headers that have to be downloaded exactly once" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the logic the other way round? i.e. either all headers have to be downloaded once or there exist some headers that could be downloaded twice?
Or maybe I'm misunderstanding this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meaning of headerDownloadEvents
may require a comment.
all headers have to be downloaded once
We always expect some headers to be downloaded more than once. The first time is when the first dynamo serves them. It reaches the end of the chain, and it is disengaged. The next time will be when the next dynamo serves the headers since its last jump as a jumper. It reaches the end of the chain, and it is disengaged. And so on ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to clarify the meaning of headerDownloadEvents
in 980c35a.
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PointSchedule.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PointSchedule/Shrinking.hs
Outdated
Show resolved
Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs
Outdated
Show resolved
Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs
Show resolved
Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs
Outdated
Show resolved
Hide resolved
...nsensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/Jumping.hs
Show resolved
Hide resolved
...consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/State.hs
Show resolved
Hide resolved
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/Genesis/Tests/CSJ.hs
Show resolved
Hide resolved
then "All headers may be downloaded twice" | ||
else "There exist headers that have to be downloaded exactly once" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meaning of headerDownloadEvents
may require a comment.
all headers have to be downloaded once
We always expect some headers to be downloaded more than once. The first time is when the first dynamo serves them. It reaches the end of the chain, and it is disengaged. The next time will be when the next dynamo serves the headers since its last jump as a jumper. It reaches the end of the chain, and it is disengaged. And so on ...
...nsensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/Jumping.hs
Show resolved
Hide resolved
...nsensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/Jumping.hs
Outdated
Show resolved
Hide resolved
...consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/State.hs
Show resolved
Hide resolved
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PointSchedule/Shrinking.hs
Outdated
Show resolved
Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no objections about merging this.
* Use real bucket parameters to compute time bounds * Add classifiers for empty/trivial adversarial schedules * Allow fetching of the same block from many peers * Time limited leashing attack: Extend the schedule and disable timeouts * Change BlockFetch initialization to use FetchModeDeadline Co-authored-by: Alexander Esgen <alexander.esgen@iohk.io> Co-authored-by: Nicolas BACQUEY <nicolas.bacquey@tweag.io>
* Disconnect peers with 0 density This affects ChainSync jumping, where genesis windows with no headers prevent jumps from happening. * Have GDD disconnect peers with disagreeing chains of equal density Before this change, the GDD governor would tolerate different chains having the same density. This is problematic to ChainSync jumping if the only active peers (dynamo and objector) disagree with chains of the same density, and GDD cannot disconnect any of them. * Don't mix idling in the computation of the upper bound * Simplify the condition to disconnect 0-density peers * Note the redundancy when checking for disagrement in chains * Peers with less than k+1 headers can disconnect peers which sent all headers in the genesis window * Rename fragment to clippedFragment in GDD
… explain the result of the test
The previous implementation would not pause the bucket while waiting for the time translation horizon to advance.
See the documentation in ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/Jumping.hs for an overview.
Co-authored-by: Nicolas “Niols” Jeannerod <nicolas.jeannerod@tweag.io>
Co-authored-by: Nicolas “Niols” Jeannerod <nicolas.jeannerod@tweag.io> Co-authored-by: Torsten Schmits <git@tryp.io>
The test logic failed to ever update the `killed` flag that indicated that a peer was selected for disconnection before, essentially making the test tautological. This revealed the need for another fix – the test must be terminated when the LOE fragment advances beyond the honest chain (because a branch forks off the last block).
Co-authored-by: Javier Sagredo <javier.sagredo@iohk.io>
980c35a
to
ce3b2c6
Compare
The main contribution of this PR is the implementation of ChainSync Jumping (0e265c4), a mechanism to avoid overloading the honest peers in the network when one or more peers connect to them for syncing. The details of ChainSync Jumping are discussed in the Jumping module.
A range of supporting commits are provided in addition.
More tests
GDD refinement
The ability of the GDD governor to disconnect nodes has increased in dcb20f5. With respect to #1015, the GDD governor can now disconnect peers in the following scenarios:
These changes help ChainSync Jumping make progress while downloading headers from only two disagreeing peers (even if both are adversarial).
Other changes