Fix notify inactive collators failures at the end of a round

[manuelmauro]

What does it do?

Addresses issue #3100 for which "notify_inactive_collator" only works at the beginning of a round.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

[github-actions]

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2284 KB (no changes) ✅

Moonbeam runtime: 2256 KB (no changes) ✅

Moonriver runtime: 2256 KB (no changes) ✅

Compared to latest release (runtime-3400)

Moonbase runtime: 2284 KB (+256 KB compared to latest release) ⚠️

Moonbeam runtime: 2256 KB (+244 KB compared to latest release) ⚠️

Moonriver runtime: 2256 KB (+244 KB compared to latest release) ⚠️

[crystalin]

For optimization purposes (knowing that Storage/PoV is usually the limiting factor), shouldn't we store those who didn't have stake. Instead of HadStake we could have WasInactive ? (What do you think @RomarQ ?)

[github-actions]

Coverage Report

@@                                             Coverage Diff                                             @@
##           master   manuel/notify-inactive-collators-works-only-at-the-beginning-of-a-round      +/-   ##
===========================================================================================================
+ Coverage   74.40%                                                                    74.44%   +0.04%     
  Files         376                                                                       376              
+ Lines       95432                                                                     95632     +200     
===========================================================================================================
+ Hits        70998                                                                     71193     +195     
+ Misses      24434                                                                     24439       +5     

Files Changed	Coverage
/pallets/parachain-staking/src/lib.rs	91.95% (+0.16%)	🔼
/pallets/parachain-staking/src/mock.rs	97.76% (+0.06%)	🔼
/pallets/parachain-staking/src/tests.rs	90.89% (+0.04%)	🔼
/pallets/parachain-staking/src/weights.rs	40.27% (+1.92%)	🔼
/runtime/moonbase/src/weights/pallet_parachain_staking.rs	21.95% (+2.50%)	🔼
/runtime/moonbeam/src/weights/pallet_parachain_staking.rs	21.85% (+2.49%)	🔼
/runtime/moonriver/src/weights/pallet_parachain_staking.rs	21.85% (+2.49%)	🔼

Coverage generated Tue Jan 14 11:26:38 UTC 2025

[RomarQ]

For optimization purposes (knowing that Storage/PoV is usually the limiting factor), shouldn't we store those who didn't have stake. Instead of HadStake we could have WasInactive ? (What do you think @RomarQ ?)

Yes, having a WasInactive storage map is preferable:

pub type WasInactive<T: Config> =
		StorageDoubleMap<_, Twox64Concat, RoundIndex, Twox64Concat, T::AccountId, (), OptionQuery>;

When starting a new round, we would check the inactivity like the example below, where stake.is_some() && pts.is_zero() would mean that the collator is inactive. We would check it against every candidate.

moonbeam/pallets/parachain-staking/src/lib.rs

Lines 1427 to 1430 in fef3821

	let stake = <AtStake<T>>::get(r, &collator);
	let pts = <AwardedPts<T>>::get(r, &collator);
	if stake.is_some() && pts.is_zero() {

We would still clear the storage after MaxOfflineRounds

[manuelmauro]

For optimization purposes (knowing that Storage/PoV is usually the limiting factor), shouldn't we store those who didn't have stake. Instead of HadStake we could have WasInactive ? (What do you think @RomarQ ?)

Addressed comment, as described by @RomarQ, in b4f4c00.
Now fixing weights.

[RomarQ]

After putting some thought into the mark_collators_as_inactive weights, the over-estimation is probably not necessary, instead we should add a num_of_collators argument to the mark_collators_as_inactive benchmark.

[manuelmauro]

After putting some thought into the mark_collators_as_inactive weights, the over-estimation is probably not necessary, instead we should add a num_of_collators argument to the mark_collators_as_inactive benchmark.

Nice! Addressed in b66ec1f