This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
make pool roles optional #11411
Merged
make pool roles optional #11411
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kianenigma
added
A0-please_review
Comment on lines
+52
to
+54
| log!( | ||
| info, | ||
| "Running migration with current storage version {:?} / onchain {:?}", |
There was a problem hiding this comment.
is migration needed? are any pools already there?
There was a problem hiding this comment.
There are on westend and there will be on Kusama by the time this PR makes it into a release.
Comment on lines
+1024
to
+1025
| if let Some(newest_era_to_remove) = | ||
| current_era.checked_sub(T::PostUnbondingPoolsWindow::get()) |
There was a problem hiding this comment.
This seems like a subtle change. I assume the value is the same, but just one less storage read?
There was a problem hiding this comment.
made a small change to make maybe_merge_pools a bit more comprehensible. This function is only used in a few instances and should be trivial to check.
Should be easy to verify, and it was intentional.
shawntabrizi
approved these changes
May 13, 2022
ggwpez
approved these changes
May 13, 2022
kianenigma
added
B7-runtimenoteworthy
and removed
B0-silent
|
bot merge |
|
Waiting for commit status. |
AKsphere
reviewed
Jun 15, 2022
godcodehunter
pushed a commit
to sensoriumxr/substrate
that referenced
this pull request
Jun 22, 2022
* make pool roles optional * undo lock file changes? * add migration * Fix * fix review comments
DaviRain-Su
pushed a commit
to octopus-network/substrate
that referenced
this pull request
Aug 23, 2022
* make pool roles optional * undo lock file changes? * add migration * Fix * fix review comments
ark0f
pushed a commit
to gear-tech/substrate
that referenced
this pull request
Feb 27, 2023
* make pool roles optional * undo lock file changes? * add migration * Fix * fix review comments
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Main feature that's being added is:
None.rootto none, I will never change any of the roles again.nominatorto none, I will never change my nominations.state_togglerto none, I will never be able to destroy this pool (unless if it gets slashed up to 90%, which should really rarely happen).Any pool needs to set nominations at least once. To prevent mistakes about this, the
createcall MUST specify all roles. Then, a call toupdate_rolescan remove any of the roles that should no longer exist. Similar to a chain, a pool can operate for a while withrootand then destroy it.Other than this
I added a test to demonstrate a confusion that you cannot bail out of slashing, if you unbond in the same era
made a small change to make
maybe_merge_poolsa bit more comprehensible. This function is only used in a few instances and should be trivial to check.(unfortunately) Needs a migration that moves the current pool roles in westend and kusama to the new format. This should be fine. This is exactly for now we proposed deploying pools on kusama with a bound on the maximum number of pools (128), so we can do such surgeries easier.
To avoid needing to migrate Polkadot pools as well, we should merge this before 0.23 and deploying anything to Polkadot.
polkadot compnaion: paritytech/polkadot#5524