Skip to content

IncrementAccum should not clip in batch #2809

@jaekwon

Description

@jaekwon

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions