Staking: Introduce `MaxBackersPerWinner` #11935

ggwpez · 2022-07-29T12:26:42Z

Draft for @kianenigma
This is a preparation for bounding the Staking, eg #11585

Helps with paritytech/polkadot-sdk#473

Changes:

Add MaxBackersPerWinner and Bounder
Use BoundedSupports in ReadySolution to make it a bit more bounded. The BoundedSupports is still an unbound vector though and would otherwise require a second Get<u32> bound.
Add TruncateIntoBoundedSupports
Feasibility check errors when the input is too long.
elect_with_bounds returns bounded supports.
set_emergency_election_result errors when the input is too long.

Open:

Check if the off-chain miner needs to trim.
Fix score function to not consume self
Tests for trimming in unsigned.rs
From @kianenigma: "[…] once we mine a solution (this code is used by both offchain workers, or by staking-miner binary), we want to do a trim_assignment_backers as well."

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma · 2022-07-30T14:25:36Z

bin/node/runtime/src/lib.rs

@@ -658,6 +659,10 @@ impl onchain::Config for OnChainSeqPhragmen {
 	>;
 	type DataProvider = <Runtime as pallet_election_provider_multi_phase::Config>::DataProvider;
 	type WeightInfo = frame_election_provider_support::weights::SubstrateWeight<Runtime>;
+
+	// TODO no idea what to use here
+	type MaxBackersPerWinner = ConstU32<{ u32::MAX }>;


Should be 1024 or something a few orders larger than MaxNominatorRewardedPerValidator.

kianenigma · 2022-07-30T14:27:13Z

frame/election-provider-multi-phase/src/lib.rs

 	/// The final supports of the solution.
 	///
 	/// This is target-major vector, storing each winners, total backing, and each individual
 	/// backer.
-	pub supports: Supports<A>,
+	pub supports: BoundedSupports<AccountId, MaxBackersPerWinner>,


I generally move structs to be <T: Config> once they need more than one generic from T, just a suggestion.

kianenigma · 2022-07-30T14:34:11Z

frame/election-provider-multi-phase/src/lib.rs

+/// Try to convert the supports of a solution into a [`ReadySolution`].
+///
+/// Errors if any of the supports has more than [`MaxBackersPerWinner`] backers.
+impl<AccountId: PartialEq, MaxBackersPerWinner: Get<u32>>


Instead of this, I would implement just impl TryFrom<Supports<AccountId>> for BoundedSupports<..> in election-provider-support/src.

The other two elements of the tuple seems very out of context.

Also, IIRC a lot of such conversions need to re-allocate in safe rust but with unsafe we can circumvent it.

kianenigma · 2022-07-30T14:36:01Z

frame/election-provider-multi-phase/src/lib.rs

+			let solution: ReadySolution<_, _> =
+				(supports, Default::default(), ElectionCompute::Emergency)
+					.try_into()
+					.map_err(|_| Error::<T>::TooManyBackersPerWinner)?;


Suggested change

let solution: ReadySolution<_, _> =

(supports, Default::default(), ElectionCompute::Emergency)

.try_into()

.map_err(|_| Error::<T>::TooManyBackersPerWinner)?;

ReadySolution {

supports: supports.into_iter().map(|s| s.try_into()).collect::<Result<Vec<_>, _>>().map_err(|_| Error::<T>::TooManyBackersPerWinner)?,

score: Default::default(),

ElectionCompute::Emergency,

};

Supplement of https://github.com/paritytech/substrate/pull/11935/files#r933816968

kianenigma · 2022-07-30T14:38:30Z

frame/election-provider-support/src/lib.rs

+	pub voters: BoundedVec<(AccountId, ExtendedBalance), Bound>,
+}
+
+pub type BoundedSupportOf<E> = BoundedSupport<


Suggested change

pub type BoundedSupportOf<E> = BoundedSupport<

/// Same as [`BoundedSupport`], parameterized by something that implements [`ElectionProvider`]

pub type BoundedSupportOf<E> = BoundedSupport<

kianenigma · 2022-07-30T14:38:46Z

frame/election-provider-support/src/lib.rs

+	<E as ElectionProvider>::MaxBackersPerWinner,
+>;
+
+pub type BoundedSupports<AccountId, MaxBackersPerWinner> =


Suggested change

pub type BoundedSupports<AccountId, MaxBackersPerWinner> =

/// Bounded counter-part of [`sp_npos_elections::Supports`].

pub type BoundedSupports<AccountId, MaxBackersPerWinner> =

kianenigma · 2022-07-30T14:39:33Z

frame/election-provider-support/src/lib.rs

+pub type BoundedSupports<AccountId, MaxBackersPerWinner> =
+	Vec<(AccountId, BoundedSupport<AccountId, MaxBackersPerWinner>)>;
+
+/// Convenience to create a [`BoundedSupport`] from a [`ElectionProvider`].


Suggested change

/// Convenience to create a [`BoundedSupport`] from a [`ElectionProvider`].

/// Convenience to create a [`BoundedSupports`] from a [`ElectionProvider`].

I am very open to the idea of not relying on this damn 's' to distinguish Support and Supports. Suggestions?

kianenigma · 2022-07-30T16:27:32Z

From @kianenigma: "[…] once we mine a solution (this code is used by both offchain workers, or by staking-miner binary), we want to do a trim_assignment_backers as well."

This part of this work is quite nasty, deals with a lot of legacy code that I have admittedly not had the time to really re-think since years ago, so I decided to take a stab at it. Feel free to cherry pick this commit if it makes sense, else try your approach if you have one.

only checked things compile, without any test.

https://github.com/paritytech/substrate/compare/kiz-max-backers-per-winner...kiz-max-backers-per-winner-2?expand=1

PS. this is the same as what I meant by

Check if the off-chain miner needs to trim.

So these two TODOs are practically the same.

kianenigma · 2022-09-26T09:21:55Z

cc @Ank4n

kianenigma · 2022-12-05T14:05:47Z

Let's close this for now, and revisit later.

kianenigma and others added 7 commits July 24, 2022 15:00

draft for Oliver

de627ee

Tests compile

adce6cf

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Benchmarks compile

3a25c09

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cleanup

6f5fdb7

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

More cleanup

aa63ebc

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cleanup

9cbbe83

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Review fixes

b21f3ad

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez requested review from kianenigma and andresilva as code owners July 29, 2022 12:26

github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jul 29, 2022

ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Jul 29, 2022

kianenigma reviewed Jul 30, 2022

View reviewed changes

kianenigma mentioned this pull request Aug 23, 2022

Consolidate Nominator Support in Slashing and Rewards paritytech/polkadot-sdk#473

Closed

kianenigma mentioned this pull request Aug 24, 2023

Tracking issue for bounding pallet-staking storage items paritytech/polkadot-sdk#255

Open

kianenigma assigned Ank4n Sep 26, 2022

kianenigma mentioned this pull request Nov 3, 2022

Increase max rewardable nominators paritytech/polkadot#6230

Merged

kianenigma closed this Dec 5, 2022

louismerlin removed the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label May 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Staking: Introduce `MaxBackersPerWinner` #11935

Staking: Introduce `MaxBackersPerWinner` #11935

ggwpez commented Jul 29, 2022 •

edited by kianenigma

Loading

kianenigma Jul 30, 2022

kianenigma Jul 30, 2022

kianenigma Jul 30, 2022

kianenigma Jul 30, 2022

kianenigma Jul 30, 2022 •

edited

Loading

kianenigma Jul 30, 2022

kianenigma Jul 30, 2022

kianenigma Jul 30, 2022

kianenigma commented Jul 30, 2022 •

edited

Loading

kianenigma commented Sep 26, 2022

kianenigma commented Dec 5, 2022

	pub type BoundedSupportOf<E> = BoundedSupport<
	/// Same as [`BoundedSupport`], parameterized by something that implements [`ElectionProvider`]
	pub type BoundedSupportOf<E> = BoundedSupport<

	pub type BoundedSupports<AccountId, MaxBackersPerWinner> =
	/// Bounded counter-part of [`sp_npos_elections::Supports`].
	pub type BoundedSupports<AccountId, MaxBackersPerWinner> =

	/// Convenience to create a [`BoundedSupport`] from a [`ElectionProvider`].
	/// Convenience to create a [`BoundedSupports`] from a [`ElectionProvider`].

Staking: Introduce MaxBackersPerWinner #11935

Staking: Introduce MaxBackersPerWinner #11935

Conversation

ggwpez commented Jul 29, 2022 • edited by kianenigma Loading

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma Jul 30, 2022 • edited Loading

Choose a reason for hiding this comment

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma Jul 30, 2022

Choose a reason for hiding this comment

kianenigma commented Jul 30, 2022 • edited Loading

kianenigma commented Sep 26, 2022

kianenigma commented Dec 5, 2022

Staking: Introduce `MaxBackersPerWinner` #11935

Staking: Introduce `MaxBackersPerWinner` #11935

ggwpez commented Jul 29, 2022 •

edited by kianenigma

Loading

kianenigma Jul 30, 2022 •

edited

Loading

kianenigma commented Jul 30, 2022 •

edited

Loading