[Fix][Sherlock M-7] Fix underflow when claiming after insolvency

equilibria-xyz · Jul 25, 2023 · 66670c1 · 66670c1
1 parent 9f153c9
commit 66670c1
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 5 deletions.
diff --git a/packages/perennial-vaults/contracts/balanced/BalancedVault.sol b/packages/perennial-vaults/contracts/balanced/BalancedVault.sol
@@ -477,10 +477,13 @@ contract BalancedVault is IBalancedVault, BalancedVaultDefinition, UInitializabl
      */
     function _rebalancePosition(EpochContext memory context, UFixed18 claimAmount) private {
         // Compute target collateral
-        UFixed18 targetCollateral = _totalAssetsAtEpoch(context).sub(claimAmount)
-            .mul(_totalSupplyAtEpoch(context).unsafeDiv(_totalSupplyAtEpoch(context).add(_redemption)))
-            .add(_deposit)
-            .div(TWO);
+        UFixed18 targetCollateral;
+        if (_totalAssetsAtEpoch(context).gte(claimAmount))
+            targetCollateral = _totalAssetsAtEpoch(context).sub(claimAmount)
+                .mul(_totalSupplyAtEpoch(context).unsafeDiv(_totalSupplyAtEpoch(context).add(_redemption)))
+                .add(_deposit)
+                .div(TWO);
+
         if (targetCollateral.muldiv(minWeight, totalWeight).lt(controller.minCollateral()))
             targetCollateral = UFixed18Lib.ZERO;
 

diff --git a/packages/perennial-vaults/contracts/single-balanced/SingleBalancedVault.sol b/packages/perennial-vaults/contracts/single-balanced/SingleBalancedVault.sol
@@ -398,7 +398,7 @@ contract SingleBalancedVault is ISingleBalancedVault, UInitializable {
      * @notice Rebalances the position of the vault
      */
     function _rebalancePosition(VersionContext memory context, UFixed18 claimAmount) private {
-        UFixed18 currentAssets = _totalAssetsAtVersion(context).sub(claimAmount);
+        UFixed18 currentAssets = _totalAssetsAtVersion(context).max(claimAmount).sub(claimAmount);
         UFixed18 currentUtilized = _totalSupply.add(_redemption).isZero() ?
             _deposit.add(currentAssets) :
             _deposit.add(currentAssets.muldiv(_totalSupply, _totalSupply.add(_redemption)));

diff --git a/packages/perennial-vaults/test/integration/BalancedVault/balancedVaultMulti.test.ts b/packages/perennial-vaults/test/integration/BalancedVault/balancedVaultMulti.test.ts
@@ -1696,6 +1696,40 @@ describe('BalancedVault (Multi-Payoff)', () => {
           'BalancedVaultDepositLimitExceeded',
         )
       })
+
+      it('handles pro-rata claims after insolvency', async () => {
+        // 1. Deposit initial amount into the vault
+        await vault.connect(user).deposit(utils.parseEther('50000'), user.address)
+        await vault.connect(user2).deposit(utils.parseEther('50000'), user2.address)
+        await updateOracle()
+        await vault.sync()
+
+        // 2. Redeem most of the amount, but leave it unclaimed
+        await vault.connect(user).redeem(utils.parseEther('40000'), user.address)
+        await vault.connect(user2).redeem(utils.parseEther('20000'), user2.address)
+        await updateOracle()
+        await vault.sync()
+
+        // 3. An oracle update makes the long position liquidatable, initiate take close
+        await updateOracle(utils.parseEther('20000'))
+        await long.connect(user).settleAccount(vault.address)
+        await short.connect(user).settleAccount(vault.address)
+        await long.connect(perennialUser).closeTake(utils.parseEther('700'))
+        await collateral.connect(liquidator).liquidate(vault.address, long.address)
+
+        // // 4. Settle the vault to recover and rebalance
+        await updateOracle() // let take settle at high price
+        await updateOracle(utils.parseEther('1500')) // return to normal price to let vault rebalance
+        await vault.sync()
+        await updateOracle()
+        await vault.sync()
+
+        // 6. Claim should be pro-rated
+        const initialBalanceOf = await asset.balanceOf(user2.address)
+        await vault.claim(user2.address)
+        expect(await vault.unclaimed(user2.address)).to.equal(0)
+        expect(await asset.balanceOf(user2.address)).to.equal(initialBalanceOf.add('19933982058344428779975'))
+      })
     })
   })
 })
diff --git a/packages/perennial-vaults/test/integration/SingleBalancedVault/singleBalancedVault.test.ts b/packages/perennial-vaults/test/integration/SingleBalancedVault/singleBalancedVault.test.ts
@@ -1121,6 +1121,40 @@ describe('SingleBalancedVault', () => {
           'BalancedVaultDepositLimitExceeded',
         )
       })
+
+      it('handles pro-rata claims after insolvency', async () => {
+        // 1. Deposit initial amount into the vault
+        await vault.connect(user).deposit(utils.parseEther('50000'), user.address)
+        await vault.connect(user2).deposit(utils.parseEther('50000'), user2.address)
+        await updateOracle()
+        await vault.sync()
+
+        // 2. Redeem most of the amount, but leave it unclaimed
+        await vault.connect(user).redeem(utils.parseEther('40000'), user.address)
+        await vault.connect(user2).redeem(utils.parseEther('20000'), user2.address)
+        await updateOracle()
+        await vault.sync()
+
+        // 3. An oracle update makes the long position liquidatable, initiate take close
+        await updateOracle(utils.parseEther('20000'))
+        await long.connect(user).settleAccount(vault.address)
+        await short.connect(user).settleAccount(vault.address)
+        await long.connect(perennialUser).closeTake(utils.parseEther('700'))
+        await collateral.connect(liquidator).liquidate(vault.address, long.address)
+
+        // // 4. Settle the vault to recover and rebalance
+        await updateOracle() // let take settle at high price
+        await updateOracle(utils.parseEther('1500')) // return to normal price to let vault rebalance
+        await vault.sync()
+        await updateOracle()
+        await vault.sync()
+
+        // 6. Claim should be pro-rated
+        const initialBalanceOf = await asset.balanceOf(user2.address)
+        await vault.claim(user2.address)
+        expect(await vault.unclaimed(user2.address)).to.equal(0)
+        expect(await asset.balanceOf(user2.address)).to.equal(initialBalanceOf.add('16584707579950957114103'))
+      })
     })
   })
 })