Missed rounds

[drsk0]

Purpose

Compute the missed rounds for validators and update them in the scheduler.

Changes

See above

Checklist

• My code follows the style of this project.
• The code compiles without warnings.
• I have performed a self-review of the changes.
• I have documented my code, in particular the intent of the
  hard-to-understand areas.
• (If necessary) I have updated the CHANGELOG.

CLA acceptance

_Remove if not applicable.

By submitting the contribution I accept the terms and conditions of the
Contributor License Agreement v1.0

• link: https://developers.concordium.com/CLAs/Contributor-License-Agreement-v1.0.pdf

• I accept the above linked CLA.

[td202]

Use Word64 so we don't even have to consider overflow being a problem for this.

[td202]

This field also has to be conditional, because it won't be present before P8.

[drsk0]

I changed it to Word64. I think even Word16 is unlikely to overflow though. General question: Do we optimize on the size of serialized types or we don't care?

[drsk0]

I also made the field optional. Another general question: What are the benefits of this current approach of checking for backwards compatibility in the type system, over just supporting one protocol version at HEAD and have well tested migration paths from the previous protocol version? I feel the effort is not proportional to the benefits at the moment, but maybe I'm missing something.

[td202]

Word16 shouldn't overflow with the current chain parameters, but we shouldn't rely on that.

I think it is generally good to optimize for space somewhat, but balance the amount of space saved vs complexity. In this case, I think it could be worth doing some space saving. For instance, finalizationAwake takes a whole byte of serialization, but only the low-order bit is used - you could use another bit to record if missedRounds is non-zero. Usually it will be zero, and as there's going to be at least one of these written per block, the savings could be worth while.

On the backwards compatibility question, I'm not entirely sure what the alternative you are suggesting is. In practise, the node would need to support at least 2 protocol versions at once in order to migrate. As it stands, the node can catch up from the original genesis to the latest block. If we were implementing a new node fresh today, maybe we wouldn't bother with the older protocol versions at all, and we might do a bunch of things differently from the outset. However, the node as it exists is a result of a series of incremental steps, and the approach to protocol versions that we have allows us to make them reasonably easily, in my opinion.

[td202]

This is not really needed, because the current round will be tcRound tc + 1 if the timeout certificate is present.

[drsk0]

Can't the tcRound point to some earlier round?

[td202]

No. In the timeout certificate checking code, we have:

                Present tc
                    -- Check that the TC round is correct.
                    | tcRound tc /= blockRound pendingBlock - 1 -> do
                        logEvent Konsensus LLTrace $ "Block " <> show pbHash <> " timeout certificate is not for the correct round."
                        flag $ BlockTCRoundInconsistent sBlock
                        rejectBlock

[td202]

Looks good. I think the main thing is to add/update testing for rotating the capital distribution.

[td202]

We should have some tests for this. SchedulerTests.KonsensusV1.EpochTransition already has tests for rotating capital distribution, so I'd suggest adding some there. In particular, we want to test cases where there are bakers with missed rounds and some bakers are added and removed to ensure the result is correct.

[drsk0]

These tests are currently pinned to P7. I will move those to P8 so we can test something meaningful.

[td202]

No. In the timeout certificate checking code, we have:

                Present tc
                    -- Check that the TC round is correct.
                    | tcRound tc /= blockRound pendingBlock - 1 -> do
                        logEvent Konsensus LLTrace $ "Block " <> show pbHash <> " timeout certificate is not for the correct round."
                        flag $ BlockTCRoundInconsistent sBlock
                        rejectBlock