[Manual] Marking offline collators not producing blocks during X rounds

[Agusrodri]

What does it do?

This PR introduces the manual design (via extrinsics) of marking a collator offline if it doesn’t produce blocks within a specific number of rounds. This number is declared as a constant (MaxOfflineRounds) and it’s different depending on which runtime we are situated on. In Moonbeam this constant is equal only to 1 round while in Moonbase and Moonriver gets the value of 2 rounds. It is important to note that this constant (in the case of the manual design) must be less than or equal to the constant RewardPaymentDelay due to this design takes into account the rewards, and wouldn't work if we use them after they are reset.

Proposed solution

Create a new extrinsic called notify_inactive_collator that anyone can call to inform a collator is inactive (not producing blocks during X consecutive rounds).

This extrinsic performs a few checks. If we are in round 10 for instance, these checks will be applied to rounds 9 and 8 (in case MaxOfflineRounds is 2). For each round it checks:

• The collator has an AtStake<> storage key associated for the round. This means the candidate was selected as a collator in that round.
• The collator has zero points in its AwardedPts<> storage. This means the collator hasn't produced any block during that round.

Also, at the beginning of the extrinsic, there is an additional check to ensure the collators set length is not lower than 66% of the value stored in TotalSelected. This is done to prevent collators set going empty and make the chain stop producing blocks.

Once all these conditions are met, the extrinsic uses a custom handler called OnInactiveCollator to inform the runtime that a collator is inactive, and let the runtime decides what to do with it. The default behavior is to mark the collator as offline, but if we are dealing with moonbeam-orbiters for instance, we will probably use a different implementation.

⚠️ Breaking Changes ⚠️

• All changes are applied in pallet-parachain-staking.
• Added notify_inactive_collator extrinsic.
• Changed AtStake<> to be of type OptionQuery instead of previous ValueQuery.
• Added a new custom handler called OnInactiveCollator to notify the runtime when a collator is inactive.

[librelois]

Another good solution is to replace the extrinsic by a non mandatory inherent, It would still be more interesting than the automatic approach for 3 reasons:

1. Part of the computation still offchain (detection of which collators should be marked offline)
2. If the verification code panic, it will not stall the chain nor the collator because it's a non-mandatory inherent
3. Still zero cost for project that want to use the parachain staking pallet without this functionally

And obviously the non mandatory inherent approach has a big advantage over the extrinsic: collators do not need to run a 2nd specific program to submit the extrinsic, submission is natively managed by the inherent system

EDIT: we can also keep both the inherent ant the extrinsic

[Agusrodri]

Another good solution is to replace the extrinsic by a non mandatory inherent, It would still be more interesting than the automatic approach for 3 reasons:

1. Part of the computation still offchain (detection of which collators should be marked offline)
2. If the verification code panic, it will not stall the chain nor the collator because it's a non-mandatory inherent
3. Still zero cost for project that want to use the parachain staking pallet without this functionally

And obviously the non mandatory inherent approach has a big advantage over the extrinsic: collators do not need to run a 2nd specific program to submit the extrinsic, submission is natively managed by the inherent system

EDIT: we can also keep both the inherent ant the extrinsic

Sounds good. If you agree, we could open another PR for that case, and merge this one with the manual solution before diving into the inherents one. What do you think?

[librelois]

Sounds good. If you agree, we could open another PR for that case, and merge this one with the manual solution before diving into the inherents one. What do you think?

I discussed my inherent idea with @crystalin, and in fact the collators have incentive to not mark other collators offline, so the inherent would not work (it would work for a while, until someone maintains a different binary to optimize the interest of the collators, like the MeV on ethereum mainnet).

[github-actions]

Coverage generated "Fri Oct 6 16:08:31 UTC 2023":
https://d3ifz9vhxc2wtb.cloudfront.net/pulls/2259/html/index.html

Master coverage: 87.39%
Pull coverage:

[Agusrodri]

@notlesh do the weights of the notify_inactive_collator extrinsic look good for you? They are provisional until we run the benchmark and update the weights. Also, in my opinion the enable_marking_offline extrinsic doesn't need a benchmark, so let me know what do you think about its weights too :)

[notlesh]

@notlesh do the weights of the notify_inactive_collator extrinsic look good for you? They are provisional until we run the benchmark and update the weights. Also, in my opinion the enable_marking_offline extrinsic doesn't need a benchmark, so let me know what do you think about its weights too :)

I added some locally generated ones in 85301b1 and b5ea5cd. They should be regenerated on proper hardware.

[notlesh]

🎉