yearn / yearn-protocol

Yearn smart contracts
https://yearn.finance
GNU Affero General Public License v3.0
441 stars 211 forks source link

Why can multisig change strategies instantly: Shouldn't only pausing strategies be instant? #98

Closed toonsevrin closed 3 years ago

toonsevrin commented 3 years ago

Hi, while auditing some of the risks of depositing to yearn vaults, I noticed that the 6/9 quorum multisig can update the controller (strategy) contract instantaneously.

I'm wondering what the security advantage of this compared to having it go with a week 'cooldown'/announcement period (eg. you can only actually activate the new strategy after 7 days).

The main objection about this is "but what if the current strategy turns out to have a bug?": But I feel like a panic() method that pauses the current strategy and moves all funds to the vault would be sufficient?

lehnberg commented 3 years ago

@toonsevrin thanks for the question, and your patience while we got back to you. We got a bit side-tracked dealing with last week's incident.

Yearn as a protocol hinges on the critical assumption that governance (right now set to mu-sig) is honest. A compromised governance can cause far more damage than setting a vault strategy to something malicious, so this particular concern is seen as a lesser security risk, if that makes sense.

Instead, priority is given on the ability to rapidly update and iterate on live strategies. Both so as not to advertise new investment strategies 7 days in advance, but also in order to rapidly improve our existing ones without interruption or seeing week-long downtimes whenever there's a bug or security vulnerability that needs to be fixed.

Does that answer your question?

toonsevrin commented 3 years ago

Thank you for the elaborate answer.

You talk about this being a minor risk compared to others. For my interests (depositing EUR in the EURcrv vault), what other damage can governance do then change the strategy? This felt like the only governance risk I am assuming.

I understand how advertising new strategies can be bad. This is a correct concern, a timelock of 12h (a la harvest.finance) should be sufficient so the second concern (week-long downtime) is less problematic.

So essentially the tradeoff is between: PRO: Advanced users (larger funds) can put event listeners and completely mitigate this black-swan event CON: Strategy is advertised 12 hours prior to deployment

Is it worth it? Alternative I'd like to propose another concept that changes the trade-off. Instead of using time, us advanced users can opt out of strategy upgrades (our funds will just sit idle in a vault as soon as the strategy is decommissioned).

There are many ways of implementing this: The costs are now shifted to development cost (because you need to rewrite the vault code) and maybe extra gas fees. I guess it really comes down to how much % of the assets would care about something like this.

I'd love to hear your thoughts on this.

lehnberg commented 3 years ago

what other damage can governance do

As I hinted in my previous post, if you don't trust Yearn's governance, you should not trust Yearn's vaults either. Our vaults are fully managed by Yearn's governance system, you can propose a modification to those processes in the forum.

saltyfacu commented 3 years ago

Closing due to inactivity