Allow pallet-election-provider to accept smaller solutions as well

[crystalin]

This is taken from a relay network based on westend-local.
After some modification we finished an era with 7 targets, and a staking desired target of 10.

When trying to submit a solution for the election the feasibility_check fails because of this check:

ensure!(winners.len() as u32 == desired_targets, FeasibilityError::WrongWinnerCount);

https://github.com/paritytech/substrate/blob/master/frame/election-provider-multi-phase/src/lib.rs#L1367

I'm not sure if it is a valid check here or if this should check if the winners is not over the desired_targets only.
(cc @JoshOrndorff and @kianenigma )

[kianenigma]

This is intentional. Most chain specs are configured to have 1 initial validator, but require 2 (ValidatorCount == 2). All in all, to be safe, we wrote this pallet for now in a way that only full solutions are acceptable.

We can allow the pallet-election-provider to accept solutions that have smaller count of winners as well. This might security implications and not all chains want to do it, therefore it needs to be configurable. Some chains want to opt to the default, which is to only accept full solutions.

[kianenigma]

A rahter dumb way to do it is to just create a type WinnerCountCriteria = ... type in the trait that tells us how to handle this.

Slightly more complicated would be to incorporate this in the score. We must think about this scenario:

Solution A has score[0] = 100 and presents 9 winners
Solution B has score[0] = 90 and presents 10 winners

Which one do we chose?

We could actually drop this check and assume: a solution that has a higher score will inevitably also have more winners. All score components are based on stake, so to have higher score, you need to have a solution with more winners.

Much better would be to incorporate the winner_count in the score and let it be compared as usual. We know DesiredTargets. We put WinnerCount - DesiredTargets into the score, and the less this value is, the better. Then the question will be, on which index of the score should this live. Will this have a higher priority than score[0] or not?

[crystalin]

I see that there is no real best solution for that, probably keeping it as it is is enough. Maybe making sure the documentation reflects your thoughts would be good too

[kianenigma]

My new opinion on how to do this would be:

1. Start by changing fn desired_targets() -> u32 to fn desired_targets -> (u32, u32). This should be interpreted as (min_acceptable, max_acceptable).
2. From staking, these values should be read from ValidatorCount and MinimumValidatorCount respectively.
3. Incorporate the number of winners a solution is providing into its score as the MOST SIGNIFICANT component, and it should be maximized (related: refactor election score #10834).

• This implies that a solution that has 10 winners, and the minimum staker in it has 100 backing is always better than a solution that has 8 winners, with minimum backing of 120 etc..

4. on the election-provider-multi-phase side, we check that the number of winners are within this range only. The rest is incorporated in the part where we compare election scores.

[kianenigma]

After consulting the researchers a bit, the above approach is reasonable but still slightly controversial and needs further thinking. In fact, we should re-think other emergency scenarios as well, like what should the value of MinimumValidatorCount be?

For now, this is the super duper simple way to solve this, and make life easier for testnets:

• The core scenario that we want to prevent is when ElectionDataProvider::targets() returns x, and ElectionDataProvider::desired_targets() returns y, but x < y. If this ever happens, the call-site of ElectionDataProvider must emit a warning..
• ..and instead of writing y as the desired targets, it should write min{x, y} to storage.
• This whole scenario should be concisely documented in ElectionDataProvider trait.
• Note: in election-provider-multi-phase, we still check that the solution has at least MinimumUntrustedScore.
• Note: In the pallet-staking side, we still check that the solution that comes back has at least MinimumValidatorCount.

[emostov]

Just to get some more context, what are common scenarios where a test net might have a data provider that returns targets.len() < desired_targets?