-
Notifications
You must be signed in to change notification settings - Fork 741
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
Validate code when scheduling upgrades from registrar #3232
Conversation
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 would move the check, otherwise it looks good.
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
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.
Looks good! Left a nit suggestion, not a blocker.
// Validation code sanity checks: | ||
ensure!(new_code.check_sanity(), Error::<T>::InvalidCode); | ||
ensure!(new_code.0.len() <= config.max_code_size as usize, Error::<T>::InvalidCode); |
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 still think that this should be moved to schedule_code_upgrade
.
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.
Ty! Still some benchmarks are failing.
🙈Should be good now. |
7e7c488
Currently, anyone can registrar a code that exceeds the code size limit when performing the upgrade from the registrar. This PR fixes that and adds a new test to cover this.
cc @bkchr @eskimor