Fishy issues

[flashfish0x]

1. Revoke doesn't change totalDebt. Any debt in revoked strategies remains on the balance sheet.

https://github.com/jmonteer/yearn-vaults-v3/blob/84c7bac21fe0b26542282019133d5fc69f3aed46/contracts/VaultV3.vy#L601

2. Migrate and tend break ERC4626 https://github.com/jmonteer/yearn-vaults-v3/blob/84c7bac21fe0b26542282019133d5fc69f3aed46/contracts/VaultV3.vy#L18 https://github.com/jmonteer/yearn-vaults-v3/blob/84c7bac21fe0b26542282019133d5fc69f3aed46/contracts/VaultV3.vy#L616

[jmonteer]

1. a revoked strategy needs to be with debt = 0 so there's no need to change total debt
2. migrate_strategy can skip migrate call in strategy by setting the second argument to False tend_strategy should not be called on ERC4626 protocols (tend is optional when operating a strategy). maybe it is good to add an extra check so we don't get reverts under any circumstance

[flashfish0x]

1. a revoked strategy needs to be with debt = 0 so there's no need to change total debt
2. migrate_strategy can skip migrate call in strategy by setting the second argument to False tend_strategy should not be called on ERC4626 protocols (tend is optional when operating a strategy). maybe it is good to add an extra check so we don't get reverts under any circumstance

how do you remove a strategy that is misbehaving?

[jmonteer]

you need to migrate

[flashfish0x]

you need to migrate

that's not good enough. Assume that a strategy is misbehaving and can no longer be trusted. there needs to be a way to remove it without asking for the strategies permission.

https://github.com/jmonteer/yearn-vaults-v3/blob/84c7bac21fe0b26542282019133d5fc69f3aed46/contracts/VaultV3.vy#L618

I know that you can set the flag to not call the old strategy. but without that you get a revert

[flashfish0x]

w/r to point two, why did you decide to include tend in the vault contract?

[jmonteer]

you need to migrate

that's not good enough. Assume that a strategy is misbehaving and can no longer be trusted. there needs to be a way to remove it without asking for the strategies permission.

https://github.com/jmonteer/yearn-vaults-v3/blob/84c7bac21fe0b26542282019133d5fc69f3aed46/contracts/VaultV3.vy#L618

I know that you can set the flag to not call the old strategy. but without that you get a revert

what does removing mean in your opinion? the way I would do it is by updating it's max_debt to 0

take into account that strategies cannot take funds on their own, it's the vault deciding when to do things so the main difference here is just about what to do with the remaining debt (keep it in the balance or declare loss)

is what you are proposing to declare everything as a loss? leaving the whole strategy behind forever. I am not against

[jmonteer]

w/r to point two, why did you decide to include tend in the vault contract?

to be able to "centralize" role keeping at a vault level so you can keep strategy setup at a minimum but I know it comes with certain compromises

[Schlagonia]

Fixed https://github.com/yearn/yearn-vaults-v3/commit/8f1abb2dfb5d3f71cdb561049d5cb68644eff1ba