Add a periodic chain re-broadcast mechanism #840
Conversation
Based on a configured rebroadcast interval periodically rebroadcast the longest chain currently being progressed by GPBFT. Extend the chain exchange broadcast message to include a timestamp rounded down to the rebroadcast interval as a way to work around gossipsub deduplication. This is to help gain better message propagation. The rebroadcast interval is reset as soon as the first message with a larger instance is observed, signaling the start of the next instance. This helps synchronize the chain rebroadcast intervals relative to the GPBFT instance start. Fixes #814
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #840 +/- ##
==========================================
- Coverage 67.39% 67.17% -0.23%
==========================================
Files 84 84
Lines 8915 8995 +80
==========================================
+ Hits 6008 6042 +34
- Misses 2380 2419 +39
- Partials 527 534 +7
|
| if err := pmm.chainex.Broadcast(ctx, *current); err != nil { | ||
| log.Errorw("Failed to immediately re-broadcast chain.", "instance", current.Instance, "chain", current.Chain, "error", err) | ||
| } | ||
| ticker.Reset(pmm.rebroadcastInterval) |
There was a problem hiding this comment.
Should we set the ticker to the rounded down time + interval instead, to line it up?
I don't think it matters; it just came up while I was reviewing.
There was a problem hiding this comment.
Yup that makes sense. Thanks Kuba
There was a problem hiding this comment.
On second thought I think this makes sense for the first firing of ticker but after that we want to use the interval. Right?
This makes the code complicated enough such that it puts me off from doing it. With the default interval of 2 seconds (and most likely sub 10s in any case?) I think it's not worth it.
Going to merge as is. Please let me know if you think otherwise and I will adjust in a follow up PR 🙏
Based on a configured rebroadcast interval periodically rebroadcast the longest chain currently being progressed by GPBFT.
Extend the chain exchange broadcast message to include a timestamp rounded down to the rebroadcast interval as a way to work around gossipsub deduplication. This is to help gain better message propagation.
The rebroadcast interval is reset as soon as the first message with a larger instance is observed, signaling the start of the next instance. This helps synchronize the chain rebroadcast intervals relative to the GPBFT instance start.
Fixes #814