Safe desired targets call

[Ank4n]

ElectionProvider expects ElectionDataProvider to return a desired_targets value that is not larger than the MaxWinners supported by the ElectionProvider. This is a little opaque rule for the ElectionDataProvider to be aware of. This PR tries to better communicate this rule by exposing a desired_targets_checked method to the trait ElectionProvider which verifies the above condition.

Fixes #12756

[Ank4n]

I am leaning towards throwing an error if desired_targets < MaxWinners. Alternatively, we can cap the desired_targets to MaxWinners silently.

[gpestana]

My feeling is that error-ing the snapshot creation because desired_targets < MaxWinners is probably too harsh. I think that capping the desired_targets to MaxWinners would be a good option, but I'd do it when in fn snapshot_creation_external() when handling the error from this call. And add a warning too. Something along the lines of:

fn create_snapshot_external(
  // ...
  let mut desired_targets = <Pallet<T> as ElectionProviderBase>::desired_targets_checked().or_else(|e| 
     match e {
        Error::DesiredTargetsGreaterThanMaxWinners => {
             // warning: capping the desired targets
             Ok(<Pallet<T> as ElectionProviderBase>::MaxWinners::get())
          }
          _ => Err(ElectionError::DataProvider(e)),
  })?;

  // ...

[Ank4n]

I have been split about this. We are already capping desired_targets to electable_targets so in that way it makes sense to cap desired_targets to MaxWinners as well.

But to present another perspective why this is a different case, electable_targets is a dynamic value that depends on total number of validator intentions while MaxWinners and desired_targets are static value configured in the runtime. If we consider this as a configuration issue, we should ideally fail till this configuration is fixed. We also have try_state checks to make sure we are not making this configuration error before it becomes part of the runtime.

[kianenigma]

MaxWinners is the cap that should NEVER be reached, I think returning an error is better.

[gpestana]

My feeling is that error-ing the snapshot creation because desired_targets < MaxWinners is probably too harsh. I think that capping the desired_targets to MaxWinners would be a good option, but I'd do it when in fn snapshot_creation_external() when handling the error from this call. And add a warning too. Something along the lines of:

fn create_snapshot_external(
  // ...
  let mut desired_targets = <Pallet<T> as ElectionProviderBase>::desired_targets_checked().or_else(|e| 
     match e {
        Error::DesiredTargetsGreaterThanMaxWinners => {
             // warning: capping the desired targets
             Ok(<Pallet<T> as ElectionProviderBase>::MaxWinners::get())
          }
          _ => Err(ElectionError::DataProvider(e)),
  })?;

  // ...

[kianenigma]

Labels not to be forgotten ;)