-
Notifications
You must be signed in to change notification settings - Fork 30
feat(code): Implement Solution 2 for the RoundSync #1032
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## anca/gossip #1032 +/- ##
===================================
===================================
|
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.
Looks good, I did the review of this together with #997 because my understanding is that it complements it and that we want to merge it there. I left some comments but two main things are:
- do we cancel the timeout when receiving the round cert?
- do we recalculate timeoutRebroadcast to account for changes/increasing in the values of other timeouts as the round increase?
) * Add gossip messages and handling * Add the new gossip and msg handling files * Add serialization test for round certificate * Send gossip messages over /gossip channel * Reuse exsiting encode and decode vote type functions * Update version and breaking changes * Store the round certificate when new round condition occurs * On hidden lock suspicion (round > x), restream also the proposal * Add vote by vote verification for round certificate * Fix standalone build, less cloning * Less cloning * Rebroadcast the round certificate * Fix clippy * Add `RebroadcastRoundCertificate` event * Move middlewares in their own module * Add round and vote type to `expect_vote_rebroadcast` * Show log when `on_event` step fails * Add basic test for vote and round certificate rebroadcast * Update comments in test * Rename gossip to liveness * Rename gossip to liveness * For hidden lock rebroadcast the proposal message also. * Apply suggestions from code review Co-authored-by: nenadmilosevic95 <50905385+nenadmilosevic95@users.noreply.github.com> Signed-off-by: Anca Zamfir <ancazamfir@users.noreply.github.com> * Correct comments * Rebroadcast proposal and polka for HIDDEN_LOCK_ROUND also * Apply suggestions from code review Co-authored-by: nenadmilosevic95 <50905385+nenadmilosevic95@users.noreply.github.com> Signed-off-by: Anca Zamfir <ancazamfir@users.noreply.github.com> * Fix formatting * Add comments to liveness message processing * Fix spelling * Panic when certificates are not found suggesting internal errors * Add test for hidden lock * Fix reuse of RestreamProposal with nil valid round for hidden lock * chore(code): Send rebroadcast votes to the liveness topic (#1012) * Send rebroadcasted votes over the gossip topic * Send rebroadcast vote event * Rename `GossipMessage` proto to `LivenessMessage` --------- Co-authored-by: Romain Ruetschi <romain@informal.systems> * Re-use `HIDDEN_LOCK_ROUND` constant from liveness test * Update BREAKING_CHANGES.md * Cleanup debugs and consensus traces * Enable the rebroadcast wal tests * Disable again the rebroadcast mode tests * Use a single rebroadcast timer * Try fix for round_certificate_rebroadcast test * Store the enter_round with the certificate, check it matches current round on (re)broadcast * Check in test broadcast of round certificate only, no rebroadcast * feat(code): Implement Solution 2 for the RoundSync (#1032) * Implement solution 2 wrt rebroadcast timer * Change names according to spec * Cancel rebroadcast timer when round cert is received * Increase the rebroadcast timeout each round * Update the config.toml files used by tests * Apply suggestions from code review Co-authored-by: nenadmilosevic95 <50905385+nenadmilosevic95@users.noreply.github.com> Signed-off-by: Anca Zamfir <ancazamfir@users.noreply.github.com> * Apply suggestions from code review * Apply suggestions from code review * Update comments Co-authored-by: nenadmilosevic95 <50905385+nenadmilosevic95@users.noreply.github.com> --------- Signed-off-by: Anca Zamfir <ancazamfir@users.noreply.github.com> Co-authored-by: Romain Ruetschi <romain@informal.systems> Co-authored-by: nenadmilosevic95 <50905385+nenadmilosevic95@users.noreply.github.com>
Closes: #XXX
From the Round sync spec:
r
processes broadcast the certificate EnterRoundCertificate, reset lastPrevote and lastPrecommit, and start rebroadcastTimeoutREBROADCAST()
and starts therebroadcastTimeout
againtimeoutRebroadcast
is approximation on how long the whole round should take so it can be something liketimeoutPropose(r)+timeoutPrevote(r)+timeoutPrecommit(r)
REBROADCAST() - broadcasts the current non-nil values of EnterRoundCertificate, lastPrevote and lastPrecommit
Summary of changes as compared to parent branch
anca/gossip
:timeout_propose + timeout_prevote + timeout_precommit
The rest of the requirements above are already in the parent branch
PR author checklist
For all contributors
RELEASE_NOTES.md
if the change warrants itBREAKING_CHANGES.md
if the change warrants itFor external contributors