Merge pull request #113 from GenerationSoftware/gen-1776-134-deposit-…

…can-revert-in-some-cases-when-using-the

add extra precaution to maxDeposit when yield buffer is low

src/PrizeVault.sol

@@ -377,13 +377,16 @@ contract PrizeVault is TwabERC20, Claimable, IERC4626, ILiquidationSource, Ownab
     /// @dev Considers the TWAB mint limit
     /// @dev Returns zero if any deposit would result in a loss of assets
     /// @dev Returns zero if total assets cannot be determined
+    /// @dev Returns zero if the yield buffer is less than half full. This is a safety precaution to ensure
+    /// a deposit of the asset amount returned by this function cannot reasonably trigger a `LossyDeposit`
+    /// revert in the `deposit` or `mint` functions if the yield buffer has been configured properly.
     /// @dev Any latent balance of assets in the prize vault will be swept in with the deposit as a part of
     /// the "dust collection strategy". This means that the max deposit must account for the latent balance
     /// by subtracting it from the max deposit available otherwise.
     function maxDeposit(address /* receiver */) public view returns (uint256) {
         uint256 _totalDebt = totalDebt();
         (bool _success, uint256 _totalAssets) = _tryGetTotalPreciseAssets();
-        if (!_success || _totalAssets < _totalDebt) return 0;
+        if (!_success || _totalAssets < _totalDebt + yieldBuffer / 2) return 0;
 
         uint256 _latentBalance = _asset.balanceOf(address(this));
         uint256 _maxYieldVaultDeposit = yieldVault.maxDeposit(address(this));

test/invariant/PrizeVault/LossyPrizeVaultInvariant.t.sol

@@ -18,7 +18,8 @@ contract LossyPrizeVaultInvariant is Test {
         uint256 totalPreciseAssets = lossyVaultHarness.vault().totalPreciseAssets();
         uint256 totalDebt = lossyVaultHarness.vault().totalDebt();
         uint256 totalSupply = lossyVaultHarness.vault().totalSupply();
-        if (totalDebt > totalPreciseAssets || type(uint96).max - totalSupply == 0) {
+        uint256 yieldBuffer = lossyVaultHarness.vault().yieldBuffer();
+        if (totalDebt + yieldBuffer / 2 > totalPreciseAssets || type(uint96).max - totalSupply == 0) {
             assertEq(lossyVaultHarness.vault().maxDeposit(address(this)), 0);
         } else {
             assertGt(lossyVaultHarness.vault().maxDeposit(address(this)), 0);

test/unit/PrizeVault/DepositLossyProtection.t.sol

@@ -8,38 +8,48 @@ contract PrizeVaultDepositLossyProtection is UnitBaseSetup {
 
     /* ============ maxDeposit ============ */
 
-    function testMaxDeposit_zeroWhenLossy() external {
+    function testMaxDeposit_zeroWhenYieldBufferLessThanHalfFull() external {
         underlyingAsset.mint(alice, 1e18);
+
+        uint256 _yieldBuffer = vault.yieldBuffer();
+        assertGt(_yieldBuffer, 0);
+        underlyingAsset.mint(address(vault), _yieldBuffer / 2);
 
-        assertGe(vault.maxDeposit(alice), 1e18);
+        assertGe(vault.maxDeposit(alice), 1e18); // maxDeposit returns some normal value
 
         vm.startPrank(alice);
         underlyingAsset.approve(address(vault), 1e18);
         vault.deposit(1e18, alice);
         vm.stopPrank();
 
-        underlyingAsset.mint(alice, 1e18);
-        underlyingAsset.burn(address(yieldVault), 1); // lost 1 asset in yield vault, new deposits will be lossy
+        assertEq(vault.currentYieldBuffer(), _yieldBuffer / 2); // check that yield buffer is still the same
+        underlyingAsset.burn(address(yieldVault), 1); // lost 1 asset in yield vault
+        assertEq(vault.currentYieldBuffer(), _yieldBuffer / 2 - 1); // yield buffer no longer is now less than half
 
-        assertEq(vault.maxDeposit(alice), 0);
+        assertEq(vault.maxDeposit(alice), 0); // maxDeposit is now zero
     }
 
     /* ============ maxMint ============ */
-        
-    function testMaxMint_zeroWhenLossy() external {
+
+    function testMaxMint_zeroWhenYieldBufferLessThanHalfFull() external {
         underlyingAsset.mint(alice, 1e18);
+
+        uint256 _yieldBuffer = vault.yieldBuffer();
+        assertGt(_yieldBuffer, 0);
+        underlyingAsset.mint(address(vault), _yieldBuffer / 2);
 
-        assertGe(vault.maxMint(alice), 1e18);
+        assertGe(vault.maxMint(alice), 1e18); // maxMint returns some normal value
 
         vm.startPrank(alice);
         underlyingAsset.approve(address(vault), 1e18);
         vault.mint(1e18, alice);
         vm.stopPrank();
 
-        underlyingAsset.mint(alice, 1e18);
-        underlyingAsset.burn(address(yieldVault), 1); // lost 1 asset in yield vault, new deposits will be lossy
+        assertEq(vault.currentYieldBuffer(), _yieldBuffer / 2); // check that yield buffer is still the same
+        underlyingAsset.burn(address(yieldVault), 1); // lost 1 asset in yield vault
+        assertEq(vault.currentYieldBuffer(), _yieldBuffer / 2 - 1); // yield buffer no longer is now less than half
 
-        assertEq(vault.maxMint(alice), 0);
+        assertEq(vault.maxMint(alice), 0); // maxMint is now zero
     }
 
     /* ============ convertToShares ============ */

test/unit/PrizeVault/PrizeVault.t.sol

@@ -255,6 +255,18 @@ contract PrizeVaultTest is UnitBaseSetup {
     /* ============ maxDeposit / maxMint ============ */
 
     function testMaxDeposit_SubtractsLatentBalance() public {
+        // ensure yield buffer is full
+        if (vault.currentYieldBuffer() < vault.yieldBuffer()) {
+            underlyingAsset.mint(address(vault), vault.yieldBuffer() - vault.currentYieldBuffer());
+
+            // flush buffer to yield vault so it doesn't count as latent balance
+            underlyingAsset.mint(alice, 1e18);
+            vm.startPrank(alice);
+            underlyingAsset.approve(address(vault), 1e18);
+            vault.deposit(1e18, alice);
+            vm.stopPrank();
+        }
+
         uint256 yieldVaultMaxDeposit = 1e18;
 
         // no latent balance, so full amount available
@@ -273,6 +285,11 @@ contract PrizeVaultTest is UnitBaseSetup {
     }
 
     function testMaxDeposit_LimitedByTwabSupplyLimit() public {
+        // ensure yield buffer is full
+        if (vault.currentYieldBuffer() < vault.yieldBuffer()) {
+            underlyingAsset.mint(address(vault), vault.yieldBuffer() - vault.currentYieldBuffer());
+        }
+
         assertEq(vault.maxDeposit(address(this)), type(uint96).max);
 
         // deposit a bunch of tokens
@@ -286,6 +303,11 @@ contract PrizeVaultTest is UnitBaseSetup {
     }
 
     function testMaxDeposit_ReturnsZeroIfTotalPreciseAssetsFails() public {
+        // ensure yield buffer is full
+        if (vault.currentYieldBuffer() < vault.yieldBuffer()) {
+            underlyingAsset.mint(address(vault), vault.yieldBuffer() - vault.currentYieldBuffer());
+        }
+
         assertGt(vault.maxDeposit(address(this)), 0);
 
         vm.mockCallRevert(address(yieldVault), abi.encodeWithSelector(IERC4626.previewRedeem.selector, 0), "force previewRedeem fail");