Use proper bounded vector type for nominations #10601

kianenigma · 2022-01-06T12:30:29Z

Small part of #9864, one little step at a time at making pallet-staking "bounded". Also needed and extracted out of my branch for paritytech/polkadot-sdk#461.

Please note carefully the fact that this might, in the VERY rare case that we reduce MaxNomination on a live network, lead to corrupt state being created. Nonetheless, this non-decodable state is pretty harmless, and can be cleaned via the already existing chill_other call.

All in all though, this PR has ZERO logical changes. It is all a matter of abstraction.

Also, note that the scenario that we reduce MAX_NOMINATIONS can also happen now, and we actually don't handle it. So if for any reason we don't merge this, we need to fix this case separately.

Polkadot companion: paritytech/polkadot#4709

kianenigma · 2022-01-06T12:31:09Z

frame/staking/src/mock.rs

@@ -533,7 +534,7 @@ fn post_conditions() {
 }

 fn check_count() {
-	let nominator_count = Nominators::<Test>::iter().count() as u32;
+	let nominator_count = Nominators::<Test>::iter_keys().count() as u32;


the reason for this is subtle and rather tricky. See if it is clear to you as a reviewer.

Because we can fail to decode?

yes we might fail to decode the values (in the rare case that we suddenly reduce MaxNominators to e.g. 4), so iter().count() would return only the number of decodable ones, while iter_keys().count() is all of the nominators.

gushc

frame/staking/src/pallet/mod.rs

frame/staking/src/pallet/impls.rs

frame/staking/src/pallet/mod.rs

frame/staking/src/tests.rs

emostov

Generally looks good but my main concern is the changes to chill_other

See

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

…strate into kiz-bound-nominator-votes

…nd-nominator-votes

kianenigma · 2022-01-24T16:31:58Z

bot merge

paritytech-processbot · 2022-01-24T16:32:11Z

Waiting for commit status.

paritytech-processbot · 2022-01-24T17:05:56Z

Merge cancelled due to error. Error: Checks failed for 7fe85d2

kianenigma · 2022-01-25T14:43:55Z

bot merge

kianenigma · 2022-01-25T14:44:06Z

bot merge

paritytech-processbot · 2022-01-25T14:44:34Z

Error: Github API says #10601 is not mergeable

* Use proper bounded vector type for nominations * add docs and tweak chill_other for cleanup purposes * Fix the build * remove TODO * add a bit more doc * even more docs gushc * Update frame/staking/src/pallet/mod.rs Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * Update frame/staking/src/pallet/mod.rs Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * Fix the nasty bug * also bound the Snapshot type * fix doc test * document bounded_vec * self-review * remove unused * Fix build * frame-support: repetition overload for bounded_vec Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * fix * remove the need to allocate into unbounded voters etc etc * Don't expect * unbreal the build again * handle macro a bit better Co-authored-by: Zeke Mostov <z.mostov@gmail.com> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma added 2 commits January 6, 2022 12:02

Use proper bounded vector type for nominations

5a990c4

add docs and tweak chill_other for cleanup purposes

fb60d30

kianenigma requested a review from andresilva as a code owner January 6, 2022 12:30

kianenigma commented Jan 6, 2022

View reviewed changes

kianenigma added 2 commits January 6, 2022 12:39

Fix the build

b96f3af

remove TODO

1b8376d

kianenigma requested review from emostov and gui1117 and removed request for andresilva January 6, 2022 12:45

kianenigma added 2 commits January 6, 2022 13:03

add a bit more doc

b93f109

even more docs

d3cb930

gushc

emostov reviewed Jan 6, 2022

View reviewed changes

frame/staking/src/pallet/mod.rs Show resolved Hide resolved

emostov reviewed Jan 6, 2022

View reviewed changes

frame/staking/src/pallet/mod.rs Outdated Show resolved Hide resolved

emostov reviewed Jan 6, 2022

View reviewed changes

frame/staking/src/pallet/mod.rs Outdated Show resolved Hide resolved

emostov reviewed Jan 6, 2022

View reviewed changes

frame/staking/src/pallet/impls.rs Show resolved Hide resolved

emostov reviewed Jan 7, 2022

View reviewed changes

frame/staking/src/pallet/mod.rs Outdated Show resolved Hide resolved

emostov reviewed Jan 7, 2022

View reviewed changes

frame/staking/src/tests.rs Show resolved Hide resolved

emostov reviewed Jan 7, 2022

View reviewed changes

kianenigma and others added 4 commits January 7, 2022 07:52

Update frame/staking/src/pallet/mod.rs

b55f9e2

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

Update frame/staking/src/pallet/mod.rs

85ff60f

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

Fix the nasty bug

ef51ec4

Merge branch 'kiz-bound-nominator-votes' of github.com:paritytech/sub…

6f5db81

…strate into kiz-bound-nominator-votes

kianenigma 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 Jan 7, 2022

Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…

5ec63fb

…nd-nominator-votes

kianenigma added 6 commits January 19, 2022 06:58

fix

eb8904e

remove the need to allocate into unbounded voters etc etc

587f715

Don't expect

88cb4a6

Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…

17bad6a

…nd-nominator-votes

Master.into()

437722a

unbreal the build again

a831b08

kianenigma mentioned this pull request Jan 19, 2022

Only maintain at most 1 UnlockChunk per era #10670

Merged

shawntabrizi approved these changes Jan 19, 2022

View reviewed changes

kianenigma added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Jan 20, 2022

kianenigma added 2 commits January 20, 2022 12:56

Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…

ac7fc12

…nd-nominator-votes

handle macro a bit better

5e1b8f0

jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jan 24, 2022

Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…

7fe85d2

…nd-nominator-votes

paritytech-processbot bot merged commit 9e9e4d3 into master Jan 25, 2022

paritytech-processbot bot deleted the kiz-bound-nominator-votes branch January 25, 2022 14:44

kianenigma mentioned this pull request Feb 28, 2022

Bring back MaxNominations as a metadata constant #10947

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use proper bounded vector type for nominations #10601

Use proper bounded vector type for nominations #10601

kianenigma commented Jan 6, 2022 •

edited

Loading

kianenigma Jan 6, 2022 •

edited

Loading

shawntabrizi Jan 19, 2022

kianenigma Jan 19, 2022 •

edited

Loading

emostov left a comment

kianenigma commented Jan 24, 2022

paritytech-processbot bot commented Jan 24, 2022

paritytech-processbot bot commented Jan 24, 2022

kianenigma commented Jan 25, 2022

kianenigma commented Jan 25, 2022

paritytech-processbot bot commented Jan 25, 2022

Use proper bounded vector type for nominations #10601

Use proper bounded vector type for nominations #10601

Conversation

kianenigma commented Jan 6, 2022 • edited Loading

kianenigma Jan 6, 2022 • edited Loading

Choose a reason for hiding this comment

shawntabrizi Jan 19, 2022

Choose a reason for hiding this comment

kianenigma Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

emostov left a comment

Choose a reason for hiding this comment

kianenigma commented Jan 24, 2022

paritytech-processbot bot commented Jan 24, 2022

paritytech-processbot bot commented Jan 24, 2022

kianenigma commented Jan 25, 2022

kianenigma commented Jan 25, 2022

paritytech-processbot bot commented Jan 25, 2022

kianenigma commented Jan 6, 2022 •

edited

Loading

kianenigma Jan 6, 2022 •

edited

Loading

kianenigma Jan 19, 2022 •

edited

Loading