Fix ticket handling, switch to exponential ticket election #578
Conversation
Kubuxu
force-pushed
the
test/deny-quality2
branch
from
7837fba to
9ff5048
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #578 +/- ##
==========================================
- Coverage 79.64% 79.39% -0.26%
==========================================
Files 50 51 +1
Lines 4530 4557 +27
==========================================
+ Hits 3608 3618 +10
- Misses 565 574 +9
- Partials 357 365 +8
|
Kubuxu
force-pushed
the
test/deny-quality2
branch
2 times, most recently
from
0d7fd74 to
112be82
Compare
Kubuxu
changed the title
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
| c.tickets[key] = append(c.tickets[key], ConvergeTicket{Sender: sender, Ticket: ticket}) | ||
| senderPower, _ := table.Get(sender) | ||
| // Keep only the first justification and best ticket | ||
| if v, found := c.values[key]; !found { |
There was a problem hiding this comment.
Do we even need to keep multiple values? Instead, can we just keep the best ticket value (or, possibly, the best ticket for the best candidate)?
There was a problem hiding this comment.
We could, but that will change if we do the audit recommendation.
There was a problem hiding this comment.
Wasn't the audit recommendation to skip to the second-best ticket if the best ticket doesn't have a valid candidate? In that case, we can filter candidates up-front. I.e.:
- Ignore all CONVERGE messages with values that aren't in our candidate set.
- Take the value with the best ticket from the remaining values.
There was a problem hiding this comment.
yeah, I guess that could work
There was a problem hiding this comment.
Quick sanity check: Has the audit recommendation been implemented in this PR?
| maxTicket = weightedTicket | ||
| maxValue = value | ||
| } | ||
| var bestValue ConvergeValue |
There was a problem hiding this comment.
We can simplify this logic by initializing this value to our own value. Then we can skip the c.HasSelfValue part.
There was a problem hiding this comment.
I removed the self-value and did some cleanup; please review the last commit.
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Kubuxu
force-pushed
the
test/deny-quality2
branch
from
06a26eb to
7297962
Compare
|
(happy to iterate in a new PR) |
No description provided.