yearn / yearn-vaults-v3

GNU Affero General Public License v3.0
96 stars 35 forks source link

build: override queue #180

Closed Schlagonia closed 11 months ago

Schlagonia commented 1 year ago

Description

Let governance override custom withdraw queues

Fixes # (issue)

Checklist

Schlagonia commented 11 months ago

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.

Schlagonia commented 11 months ago

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 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'

Schlagonia commented 11 months ago

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

changed the name to use_default_queue https://github.com/yearn/yearn-vaults-v3/pull/180/commits/1332f5dbc1b6c7134812882f81a498cdc18e99ab