[Fix] Prevent settling the zero address from disrupting accounting (#86)

* add address zero checks * add test
equilibria-xyz · Aug 31, 2023 · 684b707 · 684b707
1 parent 4a182f4
commit 684b707
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 5 deletions.
diff --git a/packages/perennial-vault/contracts/Vault.sol b/packages/perennial-vault/contracts/Vault.sol
@@ -205,7 +205,7 @@ contract Vault is IVault, Instance {
         _settleUnderlying();
         Context memory context = _loadContext(account);
 
-        _settle(context);
+        _settle(context, account);
         _manage(context, UFixed6Lib.ZERO, false);
         _saveContext(context, account);
     }
@@ -224,7 +224,7 @@ contract Vault is IVault, Instance {
         _settleUnderlying();
         Context memory context = _loadContext(account);
 
-        _settle(context);
+        _settle(context, account);
         _checkpoint(context);
         _update(context, account, depositAssets, redeemShares, claimAssets);
         _saveContext(context, account);
@@ -326,7 +326,7 @@ contract Vault is IVault, Instance {
     /// @notice Handles settling the vault state
     /// @dev Run before every stateful operation to settle up the latest global state of the vault
     /// @param context The context to use
-    function _settle(Context memory context) private {
+    function _settle(Context memory context, address account) private {
         // settle global positions
         while (
             context.global.current > context.global.latest &&
@@ -346,6 +346,8 @@ contract Vault is IVault, Instance {
             _checkpoints[newLatestId].store(context.latestCheckpoint);
         }
 
+        if (account == address(0)) return;
+
         // settle local position
         if (
             context.local.current > context.local.latest &&
@@ -449,17 +451,17 @@ contract Vault is IVault, Instance {
             context.currentIds.update(marketId, local.currentId);
         }
 
+        if (account != address(0)) context.local = _accounts[account].read();
         context.global = _accounts[address(0)].read();
-        context.local = _accounts[account].read();
         context.latestCheckpoint = _checkpoints[context.global.latest].read();
     }
 
     /// @notice Saves the context into storage
     /// @param context Context to use
     /// @param account Account to save the context for
     function _saveContext(Context memory context, address account) private {
+        if (account != address(0)) _accounts[account].store(context.local);
         _accounts[address(0)].store(context.global);
-        _accounts[account].store(context.local);
         _checkpoints[context.currentId].store(context.currentCheckpoint);
     }
 

diff --git a/packages/perennial-vault/test/integration/vault/Vault.test.ts b/packages/perennial-vault/test/integration/vault/Vault.test.ts
@@ -1227,6 +1227,46 @@ describe('Vault', () => {
       expect(await vault.convertToShares(parse6decimal('1004').add(0))).to.equal(parse6decimal('1000'))
     })
 
+    it('zero address settle w/ settlement fee', async () => {
+      const makerFee = parse6decimal('0.001')
+      const riskParameters = { ...(await market.riskParameter()) }
+      riskParameters.makerFee = makerFee
+      await market.updateRiskParameter(riskParameters)
+      const btcRiskParameters = { ...(await btcMarket.riskParameter()) }
+      btcRiskParameters.makerFee = makerFee
+      await btcMarket.updateRiskParameter(btcRiskParameters)
+
+      const settlementFee = parse6decimal('1.00')
+      const marketParameter = { ...(await market.parameter()) }
+      marketParameter.settlementFee = settlementFee
+      await market.connect(owner).updateParameter(marketParameter)
+      const btcMarketParameter = { ...(await btcMarket.parameter()) }
+      btcMarketParameter.settlementFee = settlementFee
+      await btcMarket.connect(owner).updateParameter(btcMarketParameter)
+
+      expect(await vault.convertToAssets(parse6decimal('1'))).to.equal(parse6decimal('1'))
+      expect(await vault.convertToShares(parse6decimal('1'))).to.equal(parse6decimal('1'))
+
+      const smallDeposit = parse6decimal('1000')
+      const largeDeposit = parse6decimal('10000')
+      await vault.connect(user).update(user.address, smallDeposit, 0, 0)
+      await vault.connect(user2).update(user2.address, largeDeposit, 0, 0)
+      await updateOracle()
+      await vault.settle(constants.AddressZero)
+      await vault.settle(user.address)
+      await vault.settle(user2.address)
+
+      await vault.connect(user).update(user.address, 0, constants.MaxUint256, 0)
+      await vault.connect(user2).update(user2.address, 0, constants.MaxUint256, 0)
+      await updateOracle()
+      await vault.settle(constants.AddressZero)
+      await vault.settle(user.address)
+      await vault.settle(user2.address)
+
+      const totalAssets = BigNumber.from('10906553351')
+      expect((await vault.accounts(constants.AddressZero)).assets).to.equal(totalAssets)
+    })
+
     it('reverts when below settlement fee', async () => {
       const settlementFee = parse6decimal('1.00')
       const marketParameter = { ...(await market.parameter()) }