-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Description
Related to #2718.
IncrementAccum() was written to efficiently calculate the accum as if IncrementAccum(1) was called times times, but clipping as implemented breaks this invariant. This mechanism is being used now to skip rounds in consensus.
If we're going to use clipping, (which may be OK in terms of satisfying the need for a roughly fair distribution, even given the negative-bias'ing solution in 2718 which would eventually cause the clip to be hit), then we need to stop doing the lazy calculation where we add power*times once for each valudator. Instead, we need to have a for-loop that calls IncrementAccum(1) times times.
This should be OK because rounds shouldn't go up to very large numbers (e.g. > 10K) easily due to the timeout deltas.
An alternative solution is to just use modular arithmetic, but that goes against what #2718 is proposing to do.
As is, this bug can cause consensus failure.