yearn / yearn-vaults-v3

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

build: limit modules #179

Closed Schlagonia closed 11 months ago

Schlagonia commented 1 year ago

Description

Fixes # (issue)

Checklist

Schlagonia commented 11 months ago
  1. I have a concern about a vault manager using both deposit_limit and deposit_limit_module and not have the expected output. (i.e. set deposit_limit to 0 while deposit_limit_module returns a different value can lead to the manager to think nobody can deposit, but that not being true). A potential solution could be to require deposit_limit to be uint256.max to set deposit_limit_module, and deposit_limit_module to be set to empty(address) if you want to set deposit_limit. Does that make sense?

This is a good point. Added asserts to both setters to make sure both arent used at once https://github.com/yearn/yearn-vaults-v3/pull/179/commits/5cc8b5f338238fbff562df56b5a0141a611e7aca

  1. The only think I don't like about deposit_limit_module and withdraw_limit_module is that it adds gas to the most basic functions (deposit and redeem). but I like freedom of set up

Yes, its not ideal. However the withdraw_limit_module could actually be cheaper due to the need to iterate over a queue in order to get a correct amount with the new version.