Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enforce MaxCandidates limit for staking storage items #2378

Merged
merged 3 commits into from
Jul 10, 2023
Merged

Enforce MaxCandidates limit for staking storage items #2378

merged 3 commits into from
Jul 10, 2023

Conversation

nbaztec
Copy link
Contributor

@nbaztec nbaztec commented Jul 4, 2023 •
edited
Loading

What does it do?

enforces MaxCandidates limit for candidates.

⚠️ Breaking Changes ⚠️

Error

  • ❗ CandidateLimitReached is thrown if the number of candidates exceeds MaxCandidates
  • ❗ CannotSetAboveMaxCandidates is thrown if setting selected total candidates via set_total_selected extrinsic.

Extrinsics

  • ❗ join_candidates may throw CandidateLimitReached error

Storage

  • ❗ CandidatePool is now bound by MaxCandidates
  • ❗ SelectedCandidates is now bound by MaxCandidates

@nbaztec nbaztec added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited breaking Needs to be mentioned in breaking changes labels Jul 4, 2023
@nbaztec nbaztec changed the title [MOON-2433] enforce MaxCandidateCount limit for candidates [MOON-2433] enforce MaxCandidates limit for staking storage items Jul 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

Coverage generated "Tue Jul 4 19:58:00 UTC 2023":
https://s3.amazonaws.com/moonbeam-coverage/pulls/2378/html/index.html

Master coverage: 70.94%
Pull coverage: 70.94%

librelois
librelois approved these changes Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@librelois librelois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good job

notlesh
notlesh reviewed Jul 6, 2023
View reviewed changes
pallets/parachain-staking/src/set.rs
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
#[derive(RuntimeDebug, PartialEq, Eq, Encode, Decode, Clone, TypeInfo)]
#[scale_info(skip_type_params(S))]
pub struct BoundedOrderedSet<T, S: Get<u32>>(pub BoundedVec<T, S>);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this fully replace OrderedSet? ...in theory there should be nothing unbounded...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah in a follow up PR I'm cleaning up all the nested unbounded structures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://opslayer.atlassian.net/browse/MOON-2512

@nbaztec nbaztec merged commit 194bc7c into master Jul 10, 2023
27 checks passed
@nbaztec nbaztec deleted the nish-enforce-max-candidates branch July 10, 2023 11:00
@noandrea noandrea changed the title [MOON-2433] enforce MaxCandidates limit for staking storage items Enforce MaxCandidates limit for staking storage items Aug 30, 2023
@notlesh notlesh 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 Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notlesh notlesh notlesh approved these changes

@librelois librelois librelois approved these changes

Assignees
No one assigned
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbaztec @notlesh @librelois