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.

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

Remove MinCollatorStk from parachain-staking pallet #2265

Merged
merged 1 commit into from May 3, 2023

Conversation

Agusrodri
Copy link
Contributor

What does it do?

This PR removes MinCollatorStk constant present in parachain-staking pallet.

The reason for removing it is that now both amounts MinCollatorStk and MinCandidateStk are the same, so it doesn't make any sense to have them duplicated. In this case, we keep using MinCandidateStk (minimum stake required for any account to be a collator candidate). Another reason is that it's even dangerous to have this constant (MinCollatorStk) as if the candidate gets under that amount, it won’t get included in the collators set, even if there are free slots. This could stall the chain.

@Agusrodri Agusrodri added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. D10-breaksdocs Changes to code that require changes in documentation labels Apr 25, 2023
@Agusrodri Agusrodri added the breaking Needs to be mentioned in breaking changes label Apr 25, 2023
@librelois
Copy link
Collaborator

I remember that the reason there were two different limits initially was to make sure that a collator is supported by enough delegators to be selected, with this change we assume that a collator can be selected even if it has no delegators, which is annoying since the delegators are supposed to play a security role: choosing the honest collators

@crystalin
Copy link
Collaborator

@librelois that is already the case. Self bonded collators can be selected at the moment. What is important is the minCandidateStk

@Agusrodri Agusrodri added D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels May 3, 2023
@Agusrodri Agusrodri merged commit 25d02f3 into master May 3, 2023
31 of 36 checks passed
@Agusrodri Agusrodri deleted the agustin-remove-MinCollatorStk branch May 3, 2023 16:42
@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 D10-breaksdocs Changes to code that require changes in documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants