feat: use allocator #30
Conversation
|
Any reason the tests are failing? |
| event AddedNewVault( | ||
| address indexed vault, | ||
| address indexed debtAllocator, | ||
| uint256 rating |
There was a problem hiding this comment.
Is rating there for the future or is it used?
There was a problem hiding this comment.
It would be used once this is deployed, both for naming conventions as well as on chain tracking.
Also for future use by a strategy manager contract to potentially compare rating before adding certain strategies.
| /// @notice The minimum amount denominated in asset that will | ||
| // need to be moved to trigger a debt update. | ||
| uint256 public minimumChange; | ||
|
|
||
| /// @notice Time to wait between debt updates. | ||
| uint256 public minimumWait; |
There was a problem hiding this comment.
was this removed so that a timelock could be used instead?
There was a problem hiding this comment.
No it was just moved up above 'minimumChange` for formatting looks.
| @@ -82,13 +82,13 @@ contract GenericDebtAllocator is Governance { | |||
| // Can't be more than 10_000. | |||
| uint256 public debtRatio; | |||
|
|
|||
| /// @notice Time to wait between debt updates. | |||
| uint256 public minimumWait; | |||
|
|
|||
| /// @notice The minimum amount denominated in asset that will | |||
| // need to be moved to trigger a debt update. | |||
| uint256 public minimumChange; | |||
There was a problem hiding this comment.
Probably worth noting which units this uses (e.g. nominal, bps, percent)
| * @param _strategy The address of the strategy. | ||
| */ | ||
| function addNewStrategy(address _strategy) external virtual onlyGovernance { | ||
| address currentManager = IStrategy(_strategy).management(); |
There was a problem hiding this comment.
currentManager could be moved down in this function to save gas on reverts
|
|
||
| /// @notice Contract that holds the logic and oracles for each strategy. | ||
| AprOracle internal constant aprOracle = | ||
| AprOracle(0xF012fBb9283e03994A7829fCE994a105cC066c14); |
There was a problem hiding this comment.
Any reason this is a hardcoded address?
There was a problem hiding this comment.
Its one of the few contracts i am confident in that wont need to change.
| * and debt increases at the end. | ||
| * | ||
| * It is expected that the proposer does all needed checks for values such | ||
| * as max_debt, maxWithdraw, min total Idle etc. that are enforced on debt |
There was a problem hiding this comment.
"max_debt, maxWithdraw, min total Idle etc." this is like 3 different inconsistent formats
There was a problem hiding this comment.
updated it to make a little more clear fc3b236
Still snake case for the vault stuff though
| _accountedFor += _currentDebt; | ||
|
|
||
| // Get the current weighted APR the strategy is earning | ||
| uint256 _strategyApr = (aprOracle.getStrategyApr(_strategy, 0) * |
There was a problem hiding this comment.
not really apr, but i'll give you a pass
There was a problem hiding this comment.
question, what units is getStrategyApr in? I guess we don't care about the units here, but if we wanted it to be in token terms, we would need to divide by the apr units.
There was a problem hiding this comment.
its denominated in 1e18 for every vault
There was a problem hiding this comment.
Updated all the comments and variables to use "rate" instead of APR or yield fc3b236
| if (_currentDebt > _newDebt) { | ||
| // If we are pulling all debt from a strategy. | ||
| if (_newDebt == 0) { | ||
| // We need to report profits and have them immediately unlock to not lose out on locked profit. |
There was a problem hiding this comment.
Updated it be a bit more accurate for not immediately unlocking fc3b236
| _afterYield += (aprOracle.getStrategyApr( | ||
| _strategy, | ||
| int256(_newDebt) - int256(_currentDebt) | ||
| ) * _newDebt); |
| } | ||
|
|
||
| // Make sure the ending amounts are the same otherwise rates could be wrong. | ||
| require(_totalAssets == _accountedFor, "cheater"); |
There was a problem hiding this comment.
Might be worth adding some dev messages // dev: wtf
There was a problem hiding this comment.
i think "cheater" is unique enogh to find out where its reverting
|
also added max withdraw, max deposit and min idle checks into the allocations fc3b236 |
* build: use central factory * chore: comments * test: update for factory * fix: bump by 20%
No description provided.