kick patch

[JesseAbram]

related to #87

[kianenigma]

Some nits, but looks overall good to me.

[kianenigma]

As I read in the comment, this will not fix #87, right? what's the plan around that?

[JesseAbram]

As I read in the comment, this will not fix #87, right? what's the plan around that?

I think it does, here it adds https://github.com/paritytech/statemint/pull/88/files#diff-a0359a0add814d66aac74493f7e17aefdcee21d3de1ae592e97059082b6feedfR314 kick threshold to the initial block. So if let's say a session is 100 blocks alice enters at 100 then here start block would be 200 and she would be able to produce a block in session 200-300 and not be kicked, I see it as a temp solution and we should eventually build out the Im online type suggestion you had before.

[kianenigma]

Sorry, I thought a bit more, and want to discuss: Logic-wise, the change of this PR is that it is giving every onboarded collator an initial slack of T::KickThreshold. Once set to Sessionlength, this will have the desired effect, yet it is somewhat convoluted and the detail is hidden in the implementation, rather than being in the interface.

To achieve the same thing, I recommend just adding a comment that type KickThreshold should take into account session delay, whatever that may be. In our case, we want Type KickThreshold = 2 * SessionLength.

I am still okay with the other changes in the PR, such as making a new map, but that is not relevant as far as I can see.

[kianenigma]

Sorry, I thought a bit more, and want to discuss: Logic-wise, the change of this PR is that it is giving every onboarded collator an initial slack of T::KickThreshold. Once set to Sessionlength, this will have the desired effect, yet it is somewhat convoluted and the detail is hidden in the implementation, rather than being in the interface.

To achieve the same thing, I recommend just adding a comment that type KickThreshold should take into account session delay, whatever that may be. In our case, we want Type KickThreshold = 2 * SessionLength.

I am still okay with the other changes in the PR, such as making a new map, but that is not relevant as far as I can see.

This is wrong and was consolidated offline.