groLabs / GSquared

GNU General Public License v3.0
1 stars 0 forks source link

Timing mismatch between time-gated actions #8

Open kitty-the-kat opened 1 year ago

kitty-the-kat commented 1 year ago

_calculateLockedProfit() uses a 24 hour interval releaseTime to release profits to the vault. In contrast, the lockedProfit amount is only updated when report() is called by the strategy which has a MIN_REPORT_DELAY value of 48 hours.

Technical Details

_calculateLockedProfit() is designed to slowly release profits to the GVault depositors. If the releaseTime is surpassed, which is 24 hours, it returns zero, which means no profit is locked. The lockedProfit value is updated when report() in GVault is called by a strategy's runHarvest(). ConvexStrategy's canHarvest(), which indicates when a keeper can call runHarvest(), has a constant MIN_REPORT_DELAY value of 48 hours. This means that if MIN_REPORT_DELAY is the determining factor for when a harvest happens, _calculateLockedProfit() will return zero for roughly half of that minimum time period. During the time that _calculateLockedProfit() returns zero, the profit calculation in PnLFixedRate of totalValue - lastTotal will return zero. This will causes the Senior Tranche to take value from the Junior Tranche to pay the fixed yield of the Senior Tranche.

An example of how this could be leveraged to cause a loss for Junior Tranche holders:

  1. Depositor deposits 3CRV into the GVault
  2. The GVault does not move 3CRV to the strategy until report() is called by a strategy, so the assets may not maximize their yield for some time
  3. Depositor deposits their GVault ERC4626 tokens into the Senior Tranche and soon withdraws. The deposit and withdrawal may both be done in a short period (say, 24 hours) while _calculateLockedProfit() is returning zero. This means before report() will not have been called to deposit the loose 3CRV in the GVault to maximize yield, yet the depositor claims their fixed yield from the Senior Tranche at the cost of Junior Tranche depositors, who don't reap the full benefits of the 3CRV deposit into the GVault.

If the above actions are performed frequently with large amounts of capital, say twice a week, the Junior Tranche may be less appealing for depositors. This is because the Junior Tranche is willing to pay the Senior Tranche the borrowing cost for leverage, but this cost makes more sense if the Junior Tranche is able to use the 3CRV in the strategy and not when it is sitting idle in the GVault. This scenario would get worse if the tranche had a large utilization ratio, say 10x, because the fee paid to the Senior Tranche would be larger. The flip side of this is that Junior Tranche depositors would be incentivized to withdraw their deposits during the time when _calculateLockedProfit() returns zero for the same reason.

Impact

Medium. Many withdrawals from the Senior Tranche may extract substantial value from the Junior Tranche when _calculateLockedProfit() returns zero.

Recommendation

Consider requiring strategies to use the same MIN_REPORT_DELAY value as the GVault's releaseTime. Other redesigns should be considered as well to minimize the time when _calculateLockedProfit() returns zero.

kitty-the-kat commented 1 year ago

challenged - informational or low acknowledged - no fix needed, if running multiple strategies (which is the recommended approach) the harvest can be staggered in a way so as to minimize the 0 downtime Reasoning - MIN_REPORT_DELAY does not guarantee profit for the strategy. We can imagine a situation where the strategy harvest is in synch with the release factor, but the locked profit doesn't change or is less than the 2% that the Senior tranche is taking from the junior tranche - the only things that would guarantee that lockedProfit wouldn't return 0 is to have profitable strategies :D

engn33r commented 1 year ago

I think the recommendation section could have been improved. But since you might have better ideas in that department, I'll elaborate on the issue I see and you can tell me whether this is a problem or not. If there is a withdrawal fee in the process to disincentivize short term deposits then this may be a non-issue:

  1. User deposits $10 mil into senior tranche at a strategic time, right after a harvest. This $10 mil of value is not earning yield until deployed by a strategy
  2. User withdraws $10 mil in under 24 hours, before a strategy can use the value to generate yield
  3. The result of this process is the senior vault paid out yield on value that sat dormant and did not earn any yield, so the yield paid to the user came from the junior vault
  4. User deposits $10 mil into senior tranche at a strategic time, right after a harvest. The cycle continues and the junior vault loses some value each time.