-
Notifications
You must be signed in to change notification settings - Fork 532
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
Fusion mvp #665
base: main
Are you sure you want to change the base?
Fusion mvp #665
Conversation
let pool = pool_opt.as_mut().ok_or(Error::<T>::PoolNotFound)?; | ||
|
||
// If the caller is not root, ensure it's the nominator of the pool | ||
if let Some(caller) = who { |
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.
better naming is needed.
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.
What do you mean ?
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.
It's a bit unclear what's going on here.
let is_root = ensure_root(origin.clone()).is_ok();
let who = if is_root {
None
} else {
Some(ensure_signed(origin)?)
};
if let Some(caller) = who {
ensure!(Some(&caller) == pool.nominator.as_ref(), Error::<T>::NotAuthorized);
}
I will come up with a better option when I am done reviewing
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.
root can do whatever, if it's a regular address, only nominator can change nominations
targets: BoundedVec<T::AccountId, T::MaxTargets>, | ||
) -> DispatchResult { | ||
// Check if the origin is root, if not, check if it's a signed origin. | ||
let is_root = ensure_root(origin.clone()).is_ok(); |
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 would the root want to force a different target set for pools?
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.
Nominator for a pool is optional, for all regular pools, we use root, else the guy need to be nominator
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces: