KickThreshold is too short so new candidates got kicked before they are able to author blocks

[xlc]

statemint/runtime/statemine/src/lib.rs

Lines 610 to 611 in 9664809

	// should be a multiple of session or things will get inconsistent
	type KickThreshold = Period;

New candidates can only be author blocks after 2 sessions, but threshold is just 1 period.

And why KickThreshold needs to be a multiple of session?

Should really only track last authored blocks & kick inactive one only for current collator.

[joepetrowski]

The threshold is 1 era (6 sessions):

statemint/runtime/statemine/src/lib.rs

Line 566 in 9664809

pub const Period: u32 = 6 * HOURS;

[JesseAbram]

As you register as a candidate you get that block written in so you can't be kicked out on that session

And why KickThreshold needs to be a multiple of session?

This is because the new round of collators are chosen on every session so you get selected or kicked when that happens, so if it isn't this will lead to unexpected results

New candidates can only be author blocks after 2 sessions, but threshold is just 1 period.

On every session we reset the authorities

[xlc]

@joepetrowski a session is 6 hours?

statemint/runtime/statemine/src/lib.rs

Line 592 in 9664809

pub const SessionLength: BlockNumber = 6 * HOURS;

[xlc]

@JesseAbram

say session length & kick threshold are 100 blocks

Alice join at block 100, session 1. She will be collator at session 3, block 300. But at block 300, the last authored block for Alice is 100 and 100 + 100 is less than 300?

[JesseAbram]

I am not sure where you are getting the session 1 session 3. There is no logic for that in collator selection and none I could see in session and aura, although @kianenigma is the expert there if I am missing something.

Alice joins at session one, she is a collator in session 2 and as long as she produces a block in that session she will not be kicked.

[xlc]

Currently there is hardcoded 1 session delay. i.e. the change won't be applied on next session, but the one after next.

paritytech/polkadot-sdk#463

[JesseAbram]

ahh I see now, ok the tests were always making 4 author which is why they missed it, will create a PR that adds either the (session or the kickThreshold, still thinking about what is best) to the init block. Good catch

[xlc]

FYI we are going to use a different way to handle this AcalaNetwork/Acala#1049

[kianenigma]

Solutions:

1. when onboarding a collator, if possible, we should determine when they will become 'active' and set that as their last authored block. Although this might be tricky.

2. kicking is a simple replacement for the im-online pallet. Ideally, it should be a new pallet iself that directly connects to the session pallet. Being a collator in collator-selection does not mean that you are actually actively authoring blocks. Therefore, the collator-selection listening for authorship events and making decisions based on it is inherently flawed. To the contrary, being a validator in pallet-session does mean that you are authoring blocks. So ideally there should be a new pallet to handle this, which directly connects to pallet-session, instead of living inside collator-selection.

For now, I recommend

1. disable this feature, or set its threshold to something high (+ 2 sessions)
2. implement one of the proposals above.

[JesseAbram]

closed with paritytech/substrate#88 will open another issue to talk about a better solution based on the comment from Kian