paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.79k stars 646 forks source link

Improve `ElectionFailed` error tracability #437

Open kianenigma opened 1 year ago

kianenigma commented 1 year ago
  1. The error events should be more detailed. ElectionFinalized and staking.ElectionFailed neither say much about why election failed.

  2. A lot of these is because we have less validator candidates than we wish to elect, the fix for which was discussed in the retreat and is returning ValidatorCount::get().min(Validators::count()) in fn desired_targets, combined with some warning log.

kianenigma commented 1 year ago
diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs
index c22a2bd2d1f..59d3253a37c 100644
--- a/frame/staking/src/pallet/impls.rs
+++ b/frame/staking/src/pallet/impls.rs
@@ -935,8 +935,20 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
        type MaxVotesPerVoter = T::MaxNominations;

        fn desired_targets() -> data_provider::Result<u32> {
-               Self::register_weight(T::DbWeight::get().reads(1));
-               Ok(Self::validator_count())
+               Self::register_weight(T::DbWeight::get().reads(2));
+               let desired_validators = Self::validator_count();
+               let available_validators = Validators::<T>::count();
+               if desired_validators > available_validators {
+                       log!(
+                               warn,
+                               "Staking has a total of {} validators, but requires {} to be elected, will \
+                               ask election provider to elect only {}",
+                               available_validators,
+                               desired_validators,
+                               available_validators
+                       )
+               }
+               Ok(desired_validators.min(available_validators))
        }

And some tests.

kianenigma commented 1 year ago

Slightly better, we also have MinimumValidators in staking.

If Validators::count() is less than Validators but more than MinimumValidators, take the minimum, otherwise return an error.

kianenigma commented 1 year ago
diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs
index c22a2bd2d1f..59d3253a37c 100644
--- a/frame/staking/src/pallet/impls.rs
+++ b/frame/staking/src/pallet/impls.rs
@@ -935,8 +935,20 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
        type MaxVotesPerVoter = T::MaxNominations;

        fn desired_targets() -> data_provider::Result<u32> {
-               Self::register_weight(T::DbWeight::get().reads(1));
-               Ok(Self::validator_count())
+               Self::register_weight(T::DbWeight::get().reads(2));
+               let desired_validators = Self::validator_count();
+               let available_validators = Validators::<T>::count();
+               if desired_validators > available_validators {
+                       log!(
+                               warn,
+                               "Staking has a total of {} validators, but requires {} to be elected, will \
+                               ask election provider to elect only {}",
+                               available_validators,
+                               desired_validators,
+                               available_validators
+                       )
+               }
+               Ok(desired_validators.min(available_validators))
        }

And some tests.

Is already done on the election provider side.