-
Notifications
You must be signed in to change notification settings - Fork 503
Cleanup migration for LastAuthoredBlock stale entries #1566
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
Conversation
Dinonard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can already integrate this into the runtimes.
| let mut write = 0u64; | ||
|
|
||
| // Use translate to selectively keep or drop keys | ||
| LastAuthoredBlock::<T>::translate::<BlockNumberFor<T>, _>(|account, old_value| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggestion I didn't submit - if you want to avoid translate, you can store keys in some vector, and then clean them up in another loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the migration and it works well with translate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't doubt it works, but it's purpose is to migrate from one type to another.
Dinonard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with the PR as it is right now, but I would suggest to add some 'sanity' limit to the translate iteration, if possible.
I don't think anyone can exploit this to brick the chain with a huge PoV, since we have permissioned collators now, but still it's a good practice.
|
@Dinonard I have added a sanity limit and removed the translate for a 2-step process: scan & remove.
I will do it later to avoid merging conflicts. The AH migration cleanup and the sudo pallet removal also have migrations. |
Minimum allowed line rate is |
ashutoshvarma
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Pull Request Summary
Closes #1549