-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[NPoS] Implements dynamic number of nominators #12970
Changes from 6 commits
8cb3404
c9b8a17
fcc7657
3ae3e29
98f6e82
5434b47
0a7b714
1c347f9
9bf66c3
cdaeb7c
4e38839
11bd646
3fe1076
a1f77ad
905b8e0
775fe25
eb2456b
b0b21a4
82ac2db
8218e17
af0c52b
5b5231a
8e7ecbb
bfbb91b
2c44857
1bf29f7
125ff54
7d9fdfc
8fddf41
805e3f7
3317bd7
1c20979
8ba9e1e
761d7ae
3136966
8d78bee
397ce89
1349e6a
92b1231
42eeb97
5a29c9c
f51083b
d067a1c
422393c
fb418a9
b786d33
af3b8ba
7d8ffda
1246c73
325c91f
1f8f502
67c4734
70d8579
a68a37d
849a2e0
306668e
7ec6305
e8576b7
e12f287
c7acde4
61a8ca5
5c8a034
187ce07
9023997
ee5c863
c3b4375
9f0319d
bece9d5
8bd5ce9
7970ade
208ee20
6c5b3b1
6619b5d
02d8e78
4805516
d18b175
31ed378
4769857
a5cd416
742f07d
fe61184
87945f6
75be862
b3aab85
bc0e8bd
aab74d6
fe2dfe3
491ddfc
8bfd731
747aa52
0bda7a4
157e3a7
3c3fc5a
86ab41c
e4f7f9f
07c4a7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -231,8 +231,8 @@ | |
|
|
||
| use codec::{Decode, Encode}; | ||
| use frame_election_provider_support::{ | ||
| BoundedSupportsOf, ElectionDataProvider, ElectionProvider, ElectionProviderBase, | ||
| InstantElectionProvider, NposSolution, | ||
| BoundedSupportsOf, ElectionBounds, ElectionBoundsBuilder, ElectionDataProvider, | ||
| ElectionProvider, ElectionProviderBase, InstantElectionProvider, NposSolution, | ||
| }; | ||
| use frame_support::{ | ||
| dispatch::DispatchClass, | ||
|
|
@@ -671,6 +671,14 @@ pub mod pallet { | |
| #[pallet::constant] | ||
| type MaxWinners: Get<u32>; | ||
|
|
||
| // The limits of targets to include in the snapshot per block. | ||
| #[pallet::constant] | ||
| type TargetsBounds: Get<ElectionBounds>; | ||
|
|
||
| // The limits of voters to include in the snapshot per block. | ||
| #[pallet::constant] | ||
| type VotersBounds: Get<ElectionBounds>; | ||
|
|
||
| /// Handler for the slashed deposits. | ||
| type SlashHandler: OnUnbalanced<NegativeImbalanceOf<Self>>; | ||
|
|
||
|
|
@@ -1078,14 +1086,13 @@ pub mod pallet { | |
| ) -> DispatchResult { | ||
| T::ForceOrigin::ensure_origin(origin)?; | ||
| ensure!(Self::current_phase().is_emergency(), <Error<T>>::CallNotAllowed); | ||
|
|
||
| let supports = | ||
| T::GovernanceFallback::instant_elect(maybe_max_voters, maybe_max_targets).map_err( | ||
| |e| { | ||
| log!(error, "GovernanceFallback failed: {:?}", e); | ||
| Error::<T>::FallbackFailed | ||
| }, | ||
| )?; | ||
| let voters_bounds = ElectionBoundsBuilder::new().count(maybe_max_voters).build(); | ||
| let targets_bounds = ElectionBoundsBuilder::new().count(maybe_max_voters).build(); | ||
| let supports = T::GovernanceFallback::instant_elect(voters_bounds, targets_bounds) | ||
| .map_err(|e| { | ||
| log!(error, "GovernanceFallback failed: {:?}", e); | ||
| Error::<T>::FallbackFailed | ||
| })?; | ||
|
|
||
| // transform BoundedVec<_, T::GovernanceFallback::MaxWinners> into | ||
| // `BoundedVec<_, T::MaxWinners>` | ||
|
|
@@ -1401,15 +1408,19 @@ impl<T: Config> Pallet<T> { | |
| /// Extracted for easier weight calculation. | ||
| fn create_snapshot_external( | ||
| ) -> Result<(Vec<T::AccountId>, Vec<VoterOf<T>>, u32), ElectionError<T>> { | ||
| // TODO: do we need T::MaxElect* limits now or can we rely on T::*Bounds? | ||
| let target_limit = T::MaxElectableTargets::get().saturated_into::<usize>(); | ||
| let voter_limit = T::MaxElectingVoters::get().saturated_into::<usize>(); | ||
|
|
||
| let targets = T::DataProvider::electable_targets(Some(target_limit)) | ||
| .map_err(ElectionError::DataProvider)?; | ||
| let targets_bounds = T::TargetsBounds::get(); | ||
| let voters_bounds = T::VotersBounds::get(); | ||
|
|
||
| let voters = T::DataProvider::electing_voters(Some(voter_limit)) | ||
| let targets = T::DataProvider::electable_targets(targets_bounds) | ||
| .map_err(ElectionError::DataProvider)?; | ||
|
|
||
| let voters = | ||
| T::DataProvider::electing_voters(voters_bounds).map_err(ElectionError::DataProvider)?; | ||
|
|
||
| if targets.len() > target_limit || voters.len() > voter_limit { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can just get rid of this. It is double checking some bounds that are already "guaranteed" to be met. If we are to check, we should also check the byte size (if possible to do cheaply).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The goal here was to double check that the data coming from the election provider meet the expected bounds, since we have no guarantee that the data provider is properly implemented. fwiw, there are a couple of old test checking for this. Regarding checking the size in bytes of the voter list, this pallet doesn't have the concept of size in bytes of a voter list, since that is a concept only on the data provider side (the size tracker and bounds are implemented on the data provider side). Given the point made above that EPM should not blindly trust the data provider, perhaps we could also add the size in bytes of the voter list checks in EPM (needs a new bound config for this pallet), or we could just assume the data provider always behaves as expected and remove the checks altogether. wdyt?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added voter size checks here too.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your point about this pallet not dealing with byte size is correct, but in extension it would also not have to deal with In principle, this pallet only knows that there is an If you were to do this pedantically, you would make the fields of I would formulate this then as: and then inside
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a great input. I have refactored this code and it ended up much simpler, thanks! |
||
| return Err(ElectionError::DataProvider("Snapshot too big for submission.")) | ||
| } | ||
|
|
@@ -1589,15 +1600,18 @@ impl<T: Config> Pallet<T> { | |
| <QueuedSolution<T>>::take() | ||
| .ok_or(ElectionError::<T>::NothingQueued) | ||
| .or_else(|_| { | ||
| T::Fallback::instant_elect(None, None) | ||
| .map_err(|fe| ElectionError::Fallback(fe)) | ||
| .and_then(|supports| { | ||
| Ok(ReadySolution { | ||
| supports, | ||
| score: Default::default(), | ||
| compute: ElectionCompute::Fallback, | ||
| }) | ||
| T::Fallback::instant_elect( | ||
| ElectionBounds::new_unbounded(), | ||
|
gpestana marked this conversation as resolved.
Outdated
|
||
| ElectionBounds::new_unbounded(), | ||
| ) | ||
| .map_err(|fe| ElectionError::Fallback(fe)) | ||
| .and_then(|supports| { | ||
| Ok(ReadySolution { | ||
| supports, | ||
| score: Default::default(), | ||
| compute: ElectionCompute::Fallback, | ||
| }) | ||
| }) | ||
| }) | ||
| .map(|ReadySolution { compute, score, supports }| { | ||
| Self::deposit_event(Event::ElectionFinalized { compute, score }); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.