-
Notifications
You must be signed in to change notification settings - Fork 273
Filter for incorrect sudo calls #2264
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
d6bc13d to
2833c55
Compare
167c319 to
76757b9
Compare
shamil-gadelshin
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.
Why do we add a sudo dependency to subtensor? Can we add the new tx extension to the runtime code instead?
Good idea. It will be cleaner in this case. |
…or/subtensor into feat/reject-incorrect-sudo-exts
runtime/src/sudo_wrapper.rs
Outdated
| const IDENTIFIER: &'static str = "SudoTransactionExtension"; | ||
|
|
||
| type Implicit = (); | ||
| type Val = Option<T::AccountId>; |
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.
| type Val = Option<T::AccountId>; | |
| type Val = (); |
I get the impression the Val is never used? So may be better to just use unit type.
It is only used if you have a prepare implementation because it is passed as a parameter.
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 also use it in return type for validate.
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 also use it in return type for validate.
Yes, but the validate return value is only used in the prepare function and because there is no implementation here, it is not used at all so you can just remove it from Val and from return type of validate
l0r1s
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.
small comments
…ncorrect-sudo-exts
…ncorrect-sudo-exts
Description
Incorrect Sudo transactions are filtered out inside TransactionExtension to prevent adding them into the transactions pool.
Related Issue(s)
Type of Change
Breaking Change
If this PR introduces a breaking change, please provide a detailed description of the impact and the migration path for existing applications.
Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctlyScreenshots (if applicable)
Please include any relevant screenshots or GIFs that demonstrate the changes made.
Additional Notes
Please provide any additional information or context that may be helpful for reviewers.