core/qbft: send qcommit as decided message #508
Conversation
Codecov Report
@@ Coverage Diff @@
## main #508 +/- ##
==========================================
- Coverage 56.34% 56.28% -0.06%
==========================================
Files 87 87
Lines 8001 8029 +28
==========================================
+ Hits 4508 4519 +11
- Misses 2894 2908 +14
- Partials 599 602 +3
Continue to review full report at Codecov.
|
| for _, prepare := range prepares { | ||
| if !uniq(prepare) { |
There was a problem hiding this comment.
when counting/filtering messages, ensure that only one message per source/process is counted.
| @@ -695,7 +717,7 @@ func filterRoundChange[I any, V Value[V]](msgs []Msg[I, V], round int64) []Msg[I | |||
| func filterMsgs[I any, V Value[V]](msgs []Msg[I, V], typ MsgType, round int64, value *V, pr *int64, pv *V) []Msg[I, V] { | |||
| var ( | |||
| resp []Msg[I, V] | |||
| dups = make(map[dedupKey]bool) | |||
There was a problem hiding this comment.
we already did this in filterMsgs but since we do not always use this function, generalise the uniqSource[I, V]() helper function
| @@ -142,6 +142,20 @@ func TestQBFT(t *testing.T) { | |||
| RandomRound: true, | |||
| }) | |||
| }) | |||
|
|
|||
| t.Run("drop 30% const", func(t *testing.T) { | |||
There was a problem hiding this comment.
this issue was uncovered when running multiple 30% lossy tests
| cancel() | ||
| return | ||
| } | ||
| t.Logf("%s %v => %v@%d", clock.NowStr(), source, typ, round) |
There was a problem hiding this comment.
Add more test logs
corverroos
added
the
merge when ready
Broadcasting individual COMMIT messages after Decided can result in some being dropped. This can postpone finality in lossy networks.
Rather batch
Qcommitas justification of a new message typeMsgDecided. This ensures that if a valid justifiedMsgDecidedis received, that any process will decide. This also makes sending Qcommit explicit and aligns the API with that other messages, mitigating requirement of special case handling.Also ensure that when filtering/counting messages, that only unique sources/processes are included. Generalise this via
uniq := uniqSourcehelper function.category: refactor
ticket: #445