Add simple collator election mechanism #1340

georgepisaltu · 2023-08-31T16:03:10Z

Fixes #106

Port of cumulus PR paritytech/cumulus#2960

This PR adds the ability to bid for collator slots even after the max number of collators have already registered. This eliminates the first come, first served mechanism that was in place before.

Key changes:

added update_bond extrinsic to allow registered candidates to adjust their bonds in order to dynamically control their bids
added take_candidate_slot extrinsic to try to replace an already existing candidate by bidding more than them
candidates are now kept in a sorted list in the pallet storage, where the top DesiredCandidates out of MaxCandidates candidates in the list will be selected by the session pallet as collators
if the candidacy bond is increased through a set_candidacy_bond call, candidates which don't meet the new bond requirements are kicked

Checklist

My PR includes a detailed description as outlined in the "Description" section above
My PR follows the labeling requirements of this project (at minimum one label for T required)
I have made corresponding changes to the documentation (if applicable)
I have added tests that prove my fix is effective or that my feature works (if applicable)
If this PR alters any external APIs or interfaces used by Polkadot, the corresponding Polkadot PR is ready as well as the corresponding Cumulus PR (optional)

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

…ction

kianenigma · 2023-09-06T08:39:12Z

Can I get a brief summary of why bags-list was not considered for this line of work? It is not a lot of extra work to set up, and it would enable an infinitely list of sorted collators to be kept, showcasing the use of data-structures to solve scalability issues in blockchains.

kianenigma · 2023-09-06T08:40:50Z

cumulus/pallets/collator-selection/src/lib.rs

+		}
+
+		/// The caller `origin` replaces a candidate `target` in the collator
+		/// candidate list by reserving `deposit`. The amount `deposit`


line width is not according to our documentation guideline, please use exactly the correct line-width, not less.

paritytech/substrate#12404

georgepisaltu · 2023-09-06T08:57:33Z

Can I get a brief summary of why bags-list was not considered for this line of work? It is not a lot of extra work to set up, and it would enable an infinitely list of sorted collators to be kept, showcasing the use of data-structures to solve scalability issues in blockchains.

Long discussion here, but in short the benchmarks suggest bags-list starts to make sense when the element count is over 100, but the candidate list will not hold more than ~20 elements in the near future.

In the previous PR linked above I had POCs with bags-list and with the SortedListProvider abstraction satisfied by a simple sorted list. Out of all options, we chose to go simple at this time. It will also be fairly easy to slot in a SortedListProvider if needed and then users can configure it.

cumulus/pallets/collator-selection/src/benchmarking.rs

cumulus/pallets/collator-selection/src/lib.rs

joepetrowski · 2023-09-11T18:46:57Z

cumulus/pallets/collator-selection/src/lib.rs

+						idx += 1;
+						while idx < candidate_count && candidates[idx].deposit < new_deposit {
+							candidates.as_mut().swap(idx - 1, idx);
+							idx += 1;
+						}
+					} else {
+						while idx > 0 && candidates[idx].deposit >= new_deposit {
+							candidates.as_mut().swap(idx - 1, idx);
+							idx -= 1;


I think this is safe as long as candidates is less than u32::MAX, but not sure if it's still FRAME best practice to use something like saturating_accrue. @gpestana ?

The length of candidates can be at most u32::MAX, since the BoundedVec is capped by MaxCandidates, which is Get<u32>. However, idx must be less than u32::MAX. When creating idx through candidates.iter().position(), if a match is found, it will be at most u32::MAX - 1 even if it's the last element of a u32::MAX-sized list. As things stand, no operation above should overflow or underflow.

cumulus/pallets/collator-selection/src/lib.rs

joepetrowski · 2023-09-11T18:51:11Z

cumulus/pallets/collator-selection/src/lib.rs

+				idx += 1;
+				while idx < list.len() && list[idx].deposit < deposit {
+					list.as_mut().swap(idx - 1, idx);
+					idx += 1;


I think you should use checked_add here and return an Overflow error to prevent any state mutations.

The same assumptions mentioned previously about the list being capped by MaxCandidates: Get<u32> and the idx of elements still hold here. Would it be ok to leave as is if there were UTs targeting specifically these potential overflows?

I think so but prefer someone else on FRAME to comment.

cumulus/pallets/collator-selection/src/lib.rs

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

joepetrowski

Few comments but overall good. Still prefer safe math in the runtime even if it's paranoid.

cumulus/pallets/collator-selection/src/lib.rs

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

* ignore errors when dumping logs and container is missing * fixed typo * print correct payload length * increase conversion rate a bit when estimating fee (to avoid message rejects when rate update tx is active) * fmt

Fixes #4206 In #1340 one of the storage types was changed from `Candidates` to `CandidateList`. Since the actual key includes the hash of this value, all of the candidates stored here are (a) "missing" and (b) unable to unreserve their candidacy bond. This migration kills the storage values and refunds the deposit held for each candidate. --------- Signed-off-by: georgepisaltu <george.pisaltu@parity.io> Co-authored-by: georgepisaltu <52418509+georgepisaltu@users.noreply.github.com> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: georgepisaltu <george.pisaltu@parity.io>

Introduce simple election to collator-selection

e092644

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

georgepisaltu added A0-needs_burnin Pull request needs to be tested on a live validator node before merge. DevOps is notified via matrix I5-enhancement An additional feature request. T2-pallets This PR/Issue is related to a particular pallet. labels Aug 31, 2023

georgepisaltu requested review from gpestana, muharem and joepetrowski August 31, 2023 16:03

georgepisaltu self-assigned this Aug 31, 2023

georgepisaltu added 4 commits September 1, 2023 11:18

Add new weights fn to runtimes

00a3671

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

Merge remote-tracking branch 'origin' into george/collator-election

dde5418

Update collator-selection documentation

ea0b269

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

Merge remote-tracking branch 'origin/master' into george/collator-ele…

6f1869e

…ction

georgepisaltu requested review from ggwpez and liamaharon September 6, 2023 07:08

kianenigma reviewed Sep 6, 2023

View reviewed changes

joepetrowski reviewed Sep 11, 2023

View reviewed changes

georgepisaltu added 2 commits September 12, 2023 11:41

Minor code and docs cleanup

5526ea2

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

Fix doc line wrapping

06b3c79

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

georgepisaltu requested review from joepetrowski and a team and removed request for gpestana, muharem, ggwpez and liamaharon September 20, 2023 11:48

joepetrowski reviewed Sep 20, 2023

View reviewed changes

cumulus/pallets/collator-selection/src/lib.rs Outdated Show resolved Hide resolved

cumulus/pallets/collator-selection/src/lib.rs Show resolved Hide resolved

cumulus/pallets/collator-selection/src/lib.rs Outdated Show resolved Hide resolved

georgepisaltu added 3 commits September 21, 2023 15:45

Fix leftover documentation

c55c8e2

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

Update docs around unchecked arithmetic

24cae01

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

Add docs and test for unwraps

21988be

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

bkontur added a commit to bkontur/runtimes that referenced this pull request Jan 11, 2024

[pallet_collator_selection] adapt paritytech/polkadot-sdk#1340

5ee759a

bkontur added a commit to bkontur/runtimes that referenced this pull request Jan 12, 2024

[pallet_collator_selection] adapt paritytech/polkadot-sdk#1340

45b848e

bkontur added a commit to bkontur/runtimes that referenced this pull request Jan 12, 2024

[pallet_collator_selection] adapt paritytech/polkadot-sdk#1340

7fc2c2e

bkontur added a commit to bkontur/runtimes that referenced this pull request Jan 22, 2024

[pallet_collator_selection] adapt paritytech/polkadot-sdk#1340

c9083b1

bkontur added a commit to bkontur/runtimes that referenced this pull request Jan 23, 2024

[pallet_collator_selection] adapt paritytech/polkadot-sdk#1340

739170a

claravanstaden pushed a commit to Snowfork/runtimes that referenced this pull request Feb 5, 2024

[pallet_collator_selection] adapt paritytech/polkadot-sdk#1340

6ade0bc

github-actions bot mentioned this pull request Feb 19, 2024

Update substrate/polkadot/cumulus from v1.3.0 to v1.6.0 moondance-labs/tanssi#419

Closed

github-actions bot mentioned this pull request Mar 13, 2024

Update polkadot-sdk from v1.3.0 to v1.7.2 moonbeam-foundation/moonbeam#2703

Closed

joepetrowski mentioned this pull request Apr 21, 2024

Fix Stuck Collator Funds #4229

Merged

This was referenced Jun 5, 2024

Update polkadot-sdk from v1.7.0 to v1.11.0 moondance-labs/tanssi#573

Closed

Update polkadot-sdk from v1.10.0 to v1.11.0 moondance-labs/tanssi#577

Closed

wischli mentioned this pull request Jun 6, 2024

fix: collator selection v2 storage migration centrifuge/centrifuge-chain#1862

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple collator election mechanism #1340

Add simple collator election mechanism #1340

georgepisaltu commented Aug 31, 2023 •

edited

Loading

kianenigma commented Sep 6, 2023

kianenigma Sep 6, 2023

kianenigma Sep 6, 2023

georgepisaltu Sep 12, 2023

georgepisaltu commented Sep 6, 2023

joepetrowski Sep 11, 2023

georgepisaltu Sep 12, 2023

joepetrowski Sep 11, 2023

georgepisaltu Sep 12, 2023

joepetrowski Sep 12, 2023

joepetrowski left a comment

Add simple collator election mechanism #1340

Add simple collator election mechanism #1340

Conversation

georgepisaltu commented Aug 31, 2023 • edited Loading

Checklist

kianenigma commented Sep 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgepisaltu commented Sep 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joepetrowski left a comment

Choose a reason for hiding this comment

georgepisaltu commented Aug 31, 2023 •

edited

Loading