-
Notifications
You must be signed in to change notification settings - Fork 38
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
build: override queue #180
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 think this implementation can lead to issues around unexpected outputs.
if override_queue
is used and a custom_queue different than default_queue is passed, it should revert. otherwise a user might expect the vault to withdraw from specific strategies and not get that (while that behaviour being important for some reason, e.g. gas usage).
maybe override_queue
should be renamed to allow_custom_queue
but I like the idea
I dont think we should revert. Seems the downside to always reverting is much bigger than servicing a withdraw with a different queue. I also dont want to iterate over an custom queue to check if it may be the same as the default. If someone is concerned enough to be passing a custom queue we should expect them to be able to check the public variable to know if the queue will be overriden. Question is what is the most obvious nameing/bool to know it will be used. I think "allow_custom_queue" could also be misleading since we would allow it to be sent, its just doesnt get used. |
I think the bool should be 'false' if custom queues are allowed to be used, since thats the default value and the switched needs to be flipped (turned True) to override the queue. perhaps 'override_custom_queue' would be more clear? or 'use_default_queue' |
changed the name to |
Description
Let governance override custom withdraw queues
Fixes # (issue)
Checklist