refactor: delegated fees

contracts/BaseStrategy.sol

@@ -210,6 +210,8 @@ abstract contract BaseStrategy {
      *  Strategy is somehow delegated inside another part of of Yearn's ecosystem e.g. another Vault.
      *  Note that this value must be strictly less than or equal to the amount provided by
      *  `estimatedTotalAssets()` below, as the TVL calc will be total assets minus delegated assets.
+     *  Also note that this value is used to determine the total assets under management by this
+     *  strategy, for the purposes of computing the management fee in `Vault`
      * @return
      *  The amount of assets this strategy manages that should not be included in Yearn's Total Value
      *  Locked (TVL) calculation across it's ecosystem.

contracts/Vault.vy

@@ -52,6 +52,7 @@ interface Strategy:
     def want() -> address: view
     def vault() -> address: view
     def isActive() -> bool: view
+    def delegatedAssets() -> uint256: view
     def estimatedTotalAssets() -> uint256: view
     def withdraw(_amount: uint256) -> uint256: nonpayable
     def migrate(_newStrategy: address): nonpayable
@@ -218,6 +219,9 @@ emergencyShutdown: public(bool)
 depositLimit: public(uint256)  # Limit for totalAssets the Vault can hold
 debtRatio: public(uint256)  # Debt ratio for the Vault across all strategies (in BPS, <= 10k)
 totalDebt: public(uint256)  # Amount of tokens that all strategies have borrowed
+delegatedAssets: public(uint256)  # Amount of tokens that all strategies delegate to other Vaults
+# NOTE: Cached value used solely for proper bookkeeping
+_strategy_delegatedAssets: HashMap[address, uint256]
 lastReport: public(uint256)  # block.timestamp of last report
 activation: public(uint256)  # block.timestamp of contract deployment
 lockedProfit: public(uint256) # how much profit is locked and cant be withdrawn
@@ -229,8 +233,8 @@ managementFee: public(uint256)
 # Governance Fee for performance of Vault (given to `rewards`)
 performanceFee: public(uint256)
 MAX_BPS: constant(uint256) = 10_000  # 100%, or 10k basis points
-# NOTE: A four-century period will be missing 3 of its 100 Julian leap years, leaving 97. 
-#       So the average year has 365 + 97/400 = 365.2425 days 
+# NOTE: A four-century period will be missing 3 of its 100 Julian leap years, leaving 97.
+#       So the average year has 365 + 97/400 = 365.2425 days
 #       ERROR(Julian): -0.0078
 #       ERROR(Gregorian): -0.0003
 SECS_PER_YEAR: constant(uint256) = 31_556_952  # 365.2425 days
@@ -440,7 +444,7 @@ def setRewards(rewards: address):
 def setLockedProfitDegration(degration: uint256):
     """
     @notice
-        Changes the locked profit degration. 
+        Changes the locked profit degration.
     @param degration The rate of degration in percent per second scaled to 1e18.
     """
     assert msg.sender == self.governance
@@ -883,7 +887,7 @@ def _shareValue(shares: uint256) -> uint256:
     # NOTE: using 1e3 for extra precision here, when decimals is low
     return ((10 ** 3 * (shares * freeFunds)) / self.totalSupply) / 10 ** 3
 
-    
+
 @view
 @internal
 def _sharesForAmount(amount: uint256) -> uint256:
@@ -1341,7 +1345,7 @@ def addStrategyToQueue(strategy: address):
         last_idx += 1
     # Check if queue is full
     assert last_idx < MAXIMUM_STRATEGIES
-        
+
     self.withdrawalQueue[MAXIMUM_STRATEGIES - 1] = strategy
     self._organizeWithdrawalQueue()
     log StrategyAddedToQueue(strategy)
@@ -1510,7 +1514,7 @@ def _reportLoss(strategy: address, loss: uint256):
     # Also, make sure we reduce our trust with the strategy by the same amount
     debtRatio: uint256 = self.strategies[strategy].debtRatio
     ratio_change: uint256 = min(loss * MAX_BPS / self._totalAssets(), debtRatio)
-    self.strategies[strategy].debtRatio -= ratio_change 
+    self.strategies[strategy].debtRatio -= ratio_change
     self.debtRatio -= ratio_change
 
 @internal
@@ -1519,7 +1523,11 @@ def _assessFees(strategy: address, gain: uint256):
     # NOTE: In effect, this reduces overall share price by the combined fee
     # NOTE: may throw if Vault.totalAssets() > 1e64, or not called for more than a year
     governance_fee: uint256 = (
-        (self.totalDebt * (block.timestamp - self.lastReport) * self.managementFee)
+        (
+            (self.totalDebt - self.delegatedAssets)
+            * (block.timestamp - self.lastReport)
+            * self.managementFee
+        )
         / MAX_BPS
         / SECS_PER_YEAR
     )
@@ -1637,6 +1645,17 @@ def report(gain: uint256, loss: uint256, _debtPayment: uint256) -> uint256:
         self.erc20_safe_transferFrom(self.token.address, msg.sender, self, totalAvail - credit)
     # else, don't do anything because it is balanced
 
+    # Update cached value of delegated assets
+    #   (used to properly account for mgmt fee in `_assessFees`)
+    self.delegatedAssets -= self._strategy_delegatedAssets[msg.sender]
+    # NOTE: Use `min(totalDebt, delegatedAssets)` as a guard against improper computation
+    delegatedAssets: uint256 = min(
+        self.strategies[msg.sender].totalDebt,
+        Strategy(msg.sender).delegatedAssets(),
+    )
+    self.delegatedAssets += delegatedAssets
+    self._strategy_delegatedAssets[msg.sender] = delegatedAssets
+
     # Update reporting time
     self.strategies[msg.sender].lastReport = block.timestamp
     self.lastReport = block.timestamp

contracts/test/TestStrategy.sol

@@ -12,6 +12,7 @@ import {BaseStrategyInitializable, StrategyParams, VaultAPI} from "../BaseStrate
 
 contract TestStrategy is BaseStrategyInitializable {
     bool public doReentrancy;
+    bool public delegateEverything;
 
     // Some token that needs to be protected for some reason
     // Initialize this to some fake address, because we're just using it
@@ -24,6 +25,19 @@ contract TestStrategy is BaseStrategyInitializable {
         return string(abi.encodePacked("TestStrategy ", apiVersion()));
     }
 
+    // NOTE: This is a test-only function to simulate delegation
+    function _toggleDelegation() public {
+        delegateEverything = !delegateEverything;
+    }
+
+    function delegatedAssets() external override view returns (uint256) {
+        if (delegateEverything) {
+            return vault.strategies(address(this)).totalDebt;
+        } else {
+            return 0;
+        }
+    }
+
     // NOTE: This is a test-only function to simulate losses
     function _takeFunds(uint256 amount) public {
         want.safeTransfer(msg.sender, amount);
@@ -33,7 +47,7 @@ contract TestStrategy is BaseStrategyInitializable {
     function _toggleReentrancyExploit() public {
         doReentrancy = !doReentrancy;
     }
-    
+
     // NOTE: This is a test-only function to simulate a wrong want token
     function _setWant(IERC20 _want) public {
         want = _want;

tests/functional/strategy/test_fees.py

@@ -65,3 +65,28 @@ def test_max_fees(gov, vault, token, TestStrategy, rewards, strategist):
         vault.updateStrategyPerformanceFee(
             strategy, FEE_MAX - vault_performance_fee + 1, {"from": gov}
         )
+
+
+def test_delegated_fees(chain, rewards, vault, strategy):
+    # Make sure funds are in the strategy
+    strategy.harvest()
+    assert strategy.estimatedTotalAssets() > 0
+
+    # Management fee is active...
+    bal_before = vault.balanceOf(rewards)
+    chain.mine(timedelta=60 * 60 * 24 * 365)  # Mine a year at 0% mgmt fee
+    strategy.harvest()
+    assert vault.balanceOf(rewards) > bal_before  # No increase in mgmt fees
+
+    # Check delegation math/logic
+    strategy._toggleDelegation()
+    assert strategy.delegatedAssets() == vault.strategies(strategy).dict()["totalDebt"]
+    assert vault.delegatedAssets() == 0  # NOTE: Cached 1 harvest period behind
+    strategy.harvest()
+    assert vault.delegatedAssets() == strategy.delegatedAssets()
+
+    # Delegated assets pay no fees (everything is delegated now)
+    bal_before = vault.balanceOf(rewards)
+    chain.mine(timedelta=60 * 60 * 24 * 365)  # Mine a year at 0% mgmt fee
+    strategy.harvest()
+    assert vault.balanceOf(rewards) == bal_before  # No increase in mgmt fees