[Fix][Sherlock M-3 + M-13] Handle position rebalance 0 value cases

packages/perennial-vaults/contracts/balanced/BalancedVault.sol

@@ -511,16 +511,22 @@ contract BalancedVault is IBalancedVault, BalancedVaultDefinition, UInitializabl
  UFixed18 makerAvailable = position.maker.gt(position.taker) ?
  position.maker.sub(position.taker) : UFixed18Lib.ZERO;
 
- product.closeMake(accountPosition.sub(targetPosition).min(makerAvailable));
- }
-
- if (targetPosition.gt(accountPosition)) {
+ // If there is no maker available (socialization), we still need a settlement but closing 0 value will revert,
+ // so instead open 0 value instead
+ if (makerAvailable.isZero()) product.openMake(UFixed18Lib.ZERO);
+ else product.closeMake(accountPosition.sub(targetPosition).min(makerAvailable));
+ } else if (targetPosition.gt(accountPosition)) {
  // compute headroom until hitting makerLimit
  UFixed18 currentMaker = product.positionAtVersion(product.latestVersion()).next(product.pre()).maker;
  UFixed18 makerLimit = product.makerLimit();
  UFixed18 makerAvailable = makerLimit.gt(currentMaker) ? makerLimit.sub(currentMaker) : UFixed18Lib.ZERO;
 
- product.openMake(targetPosition.sub(accountPosition).min(makerAvailable));
+ // If there is no maker available (maker limit), we still need a settlement but opening 0 value will revert,
+ // so instead close 0 value instead
+ if (makerAvailable.isZero()) product.closeMake(UFixed18Lib.ZERO);
+ else product.openMake(targetPosition.sub(accountPosition).min(makerAvailable));
+ } else {
+ product.closeMake(UFixed18Lib.ZERO);
  }
  }
 

packages/perennial-vaults/contracts/single-balanced/SingleBalancedVault.sol

@@ -433,14 +433,22 @@ contract SingleBalancedVault is ISingleBalancedVault, UInitializable {
  */
  function _updateMakerPosition(IProduct product, UFixed18 targetPosition) private {
  UFixed18 currentPosition = product.position(address(this)).next(product.pre(address(this))).maker;
- UFixed18 currentMaker = product.positionAtVersion(product.latestVersion()).next(product.pre()).maker;
- UFixed18 makerLimit = product.makerLimit();
- UFixed18 makerAvailable = makerLimit.gt(currentMaker) ? makerLimit.sub(currentMaker) : UFixed18Lib.ZERO;
 
  if (targetPosition.lt(currentPosition))
  product.closeMake(currentPosition.sub(targetPosition));
- if (targetPosition.gte(currentPosition))
- product.openMake(targetPosition.sub(currentPosition).min(makerAvailable));
+ else if (targetPosition.gt(currentPosition)) {
+ // compute headroom until hitting makerLimit
+ UFixed18 currentMaker = product.positionAtVersion(product.latestVersion()).next(product.pre()).maker;
+ UFixed18 makerLimit = product.makerLimit();
+ UFixed18 makerAvailable = makerLimit.gt(currentMaker) ? makerLimit.sub(currentMaker) : UFixed18Lib.ZERO;
+
+ // If there is no maker available (maker limit), we still need a settlement but opening 0 value will revert,
+ // so instead close 0 value instead
+ if (makerAvailable.isZero()) product.closeMake(UFixed18Lib.ZERO);
+ else product.openMake(targetPosition.sub(currentPosition).min(makerAvailable));
+ } else {
+ product.closeMake(UFixed18Lib.ZERO);
+ }
 
  emit PositionUpdated(product, targetPosition);
  }

packages/perennial-vaults/test/integration/BalancedVault/balancedVault.test.ts

@@ -1,6 +1,7 @@
 import HRE from 'hardhat'
 import { time, impersonate } from '../../../../common/testutil'
 import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
+import { anyUint } from '@nomicfoundation/hardhat-chai-matchers/withArgs'
 import { FakeContract, smock } from '@defi-wonderland/smock'
 import { expect, use } from 'chai'
 import {
@@ -18,6 +19,7 @@
  ICollateral__factory,
 } from '../../../types/generated'
 import { BigNumber, constants, utils } from 'ethers'
+import { impersonateWithBalance } from '../../../../common/testutil/impersonate'
 
 const { config, ethers } = HRE
 use(smock.matchers)
@@ -35,6 +37,7 @@
  let user2: SignerWithAddress
  let perennialUser: SignerWithAddress
  let liquidator: SignerWithAddress
+ let marketOwner: SignerWithAddress
  let long: IProduct
  let short: IProduct
  let leverage: BigNumber
@@ -120,7 +123,7 @@
  await setUpWalletWithDSU(user)
  await setUpWalletWithDSU(user2)
  await setUpWalletWithDSU(liquidator)
- await setUpWalletWithDSU(perennialUser, utils.parseEther('1000000'))
+ await setUpWalletWithDSU(perennialUser, utils.parseEther('10000000'))
 
  // Unfortunately, we can't make mocks of existing contracts.
  // So, we make a fake and initialize it with the values that the real contract had at this block.
@@ -134,6 +137,8 @@
  oracle.sync.returns(currentVersion)
  oracle.currentVersion.returns(currentVersion)
  oracle.atVersion.whenCalledWith(currentVersion[0]).returns(currentVersion)
+
+ marketOwner = await impersonateWithBalance(await controller['owner(address)'](long.address), utils.parseEther('10'))
  })
 
  describe('#constructor', () => {
@@ -576,7 +581,7 @@
  expect(await vault.maxRedeem(user.address)).to.equal(expectedShares)
 
  await vault.sync()
  const redeemAmount = (await vault.maxRedeem(user.address)).add(1)
 
  await expect(
  vault.connect(user).redeem((await vault.maxRedeem(user.address)).add(1), user.address),
@@ -758,6 +763,42 @@
  expect(await shortPosition()).to.equal(0)
  })
 
+ it('closes 0 value position if above maker limit', async () => {
+ // Get maker product very close to the makerLimit
+ await asset.connect(perennialUser).approve(collateral.address, constants.MaxUint256)
+ await collateral
+ .connect(perennialUser)
+ .depositTo(perennialUser.address, short.address, utils.parseEther('400000'))
+ const shortMakerAvailable = (await short.makerLimit()).sub(
+ (await short.positionAtVersion(await short['latestVersion()']())).maker,
+ )
+ await collateral.connect(perennialUser).depositTo(perennialUser.address, long.address, utils.parseEther('400000'))
+ const longMakerAvailable = (await long.makerLimit()).sub(
+ (await long.positionAtVersion(await long['latestVersion()']())).maker,
+ )
+
+ await short.connect(perennialUser).openMake(shortMakerAvailable)
+ await long.connect(perennialUser).openMake(longMakerAvailable)
+ await updateOracle()
+ await vault.sync()
+
+ await short.connect(marketOwner).updateMakerLimit(0)
+ await long.connect(marketOwner).updateMakerLimit(0)
+
+ // Deposit should create a greater position than what's available
+ const largeDeposit = utils.parseEther('10000')
+ await vault.connect(user).deposit(largeDeposit, user.address)
+ await updateOracle()
+ await expect(vault.sync())
+ .to.emit(short, 'MakeClosed')
+ .withArgs(vault.address, anyUint, 0)
+ .to.emit(long, 'MakeClosed')
+ .withArgs(vault.address, anyUint, 0)
+
+ expect(await longPosition()).to.equal(0)
+ expect(await shortPosition()).to.equal(0)
+ })
+
  it('close to taker', async () => {
  // Deposit should create a greater position than what's available
  const largeDeposit = utils.parseEther('10000')
@@ -786,26 +827,50 @@
  )
  })
 
+ it('opens 0 value position if in >=100% utilization', async () => {
+ // Deposit should create a greater position than what's available
+ const largeDeposit = utils.parseEther('10000')
+ await vault.connect(user).deposit(largeDeposit, user.address)
+ await updateOracle()
+ await vault.sync()
+
+ // Get taker product very close to the maker
+ await asset.connect(perennialUser).approve(collateral.address, constants.MaxUint256)
+ await collateral
+ .connect(perennialUser)
+ .depositTo(perennialUser.address, short.address, utils.parseEther('1000000'))
+
+ await short.settle()
+ const globalShort = await short.positionAtVersion(await short['latestVersion()']())
+ const shortAvailable = globalShort.maker.sub(globalShort.taker)
+ await short.connect(perennialUser).openTake(shortAvailable)
+ await updateOracle()
+ await short.settle()
+
+ // Increase the price to result in a profit for the vault on the short side, resulting in a close attempt on the next
+ // update
+ await updateOracle(originalOraclePrice.add(utils.parseEther('10')))
+ await vault.sync()
+ await vault.connect(user).redeem(await vault.maxRedeem(user.address), user.address)
+ await expect(vault.sync()).to.emit(short, 'MakeOpened').withArgs(vault.address, anyUint, 0)
+ })
+
  it('product closing closes all positions', async () => {
  const largeDeposit = utils.parseEther('10000')
  await vault.connect(user).deposit(largeDeposit, user.address)
  await updateOracleAndSync()
 
  expect(await longPosition()).to.be.greaterThan(0)
  expect(await shortPosition()).to.be.greaterThan(0)
- const productOwner = await impersonate.impersonateWithBalance(
- await controller['owner(address)'](long.address),
- utils.parseEther('10'),
- )
- await long.connect(productOwner).updateClosed(true)
+ await long.connect(marketOwner).updateClosed(true)
  await updateOracleAndSync()
  await updateOracleAndSync()
 
  // We should have closed all positions
  expect(await longPosition()).to.equal(0)
  expect(await shortPosition()).to.equal(0)
 
- await long.connect(productOwner).updateClosed(false)
+ await long.connect(marketOwner).updateClosed(false)
  await updateOracleAndSync()
  await updateOracleAndSync()
 
@@ -814,6 +879,18 @@
  expect(await shortPosition()).to.be.greaterThan(0)
  })
 
+ it('closes a 0 value position if account position is at target', async () => {
+ // Deposit should create a greater position than what's available
+ const largeDeposit = utils.parseEther('10000')
+ await vault.connect(user).deposit(largeDeposit, user.address)
+ await updateOracle()
+ await vault.sync()
+
+ await updateOracle()
+ await vault.connect(user).deposit(0, user.address)
+ await expect(vault.sync()).to.emit(short, 'MakeClosed').withArgs(vault.address, anyUint, 0)
+ })
+
  context('liquidation', () => {
  context('long', () => {
  it('recovers before being liquidated', async () => {

packages/perennial-vaults/test/integration/SingleBalancedVault/singleBalancedVault.test.ts

@@ -1,6 +1,7 @@
 import HRE from 'hardhat'
 import { time, impersonate } from '../../../../common/testutil'
 import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
+import { anyUint } from '@nomicfoundation/hardhat-chai-matchers/withArgs'
 import { FakeContract, smock } from '@defi-wonderland/smock'
 import { expect, use } from 'chai'
 import {
@@ -17,11 +18,12 @@ import {
  ICollateral__factory,
 } from '../../../types/generated'
 import { BigNumber, constants, utils } from 'ethers'
+import { impersonateWithBalance } from '../../../../common/testutil/impersonate'
 
 const { config, ethers } = HRE
 use(smock.matchers)
 
-const DSU_HOLDER = '0xaef566ca7e84d1e736f999765a804687f39d9094'
+const DSU_MINTER = '0xD05aCe63789cCb35B9cE71d01e4d632a0486Da4B'
 
 describe('SingleBalancedVault', () => {
  let vault: SingleBalancedVault
@@ -33,6 +35,7 @@ describe('SingleBalancedVault', () => {
  let user2: SignerWithAddress
  let perennialUser: SignerWithAddress
  let liquidator: SignerWithAddress
+ let marketOwner: SignerWithAddress
  let long: IProduct
  let short: IProduct
  let leverage: BigNumber
@@ -94,16 +97,21 @@ describe('SingleBalancedVault', () => {
  await vault.initialize('Perennial Vault Alpha', 'PVA')
  asset = IERC20Metadata__factory.connect(await vault.asset(), owner)
 
- const dsuHolder = await impersonate.impersonateWithBalance(DSU_HOLDER, utils.parseEther('10'))
- const setUpWalletWithDSU = async (wallet: SignerWithAddress) => {
- await dsu.connect(dsuHolder).transfer(wallet.address, utils.parseEther('200000'))
+ const dsuMinter = await impersonate.impersonateWithBalance(DSU_MINTER, utils.parseEther('10'))
+ const setUpWalletWithDSU = async (wallet: SignerWithAddress, amount?: BigNumber) => {
+ const dsuIface = new utils.Interface(['function mint(uint256)'])
+ await dsuMinter.sendTransaction({
+ to: dsu.address,
+ value: 0,
+ data: dsuIface.encodeFunctionData('mint', [amount ?? utils.parseEther('200000')]),
+ })
+ await dsu.connect(dsuMinter).transfer(wallet.address, amount ?? utils.parseEther('200000'))
  await dsu.connect(wallet).approve(vault.address, ethers.constants.MaxUint256)
  }
  await setUpWalletWithDSU(user)
  await setUpWalletWithDSU(user2)
  await setUpWalletWithDSU(liquidator)
- await setUpWalletWithDSU(perennialUser)
- await setUpWalletWithDSU(perennialUser)
+ await setUpWalletWithDSU(perennialUser, utils.parseEther('2000000'))
 
  // Unfortunately, we can't make mocks of existing contracts.
  // So, we make a fake and initialize it with the values that the real contract had at this block.
@@ -117,6 +125,8 @@ describe('SingleBalancedVault', () => {
  oracle.sync.returns(currentVersion)
  oracle.currentVersion.returns(currentVersion)
  oracle.atVersion.whenCalledWith(currentVersion[0]).returns(currentVersion)
+
+ marketOwner = await impersonateWithBalance(await controller['owner(address)'](long.address), utils.parseEther('10'))
  })
 
  describe('#initialize', () => {
@@ -837,6 +847,54 @@ describe('SingleBalancedVault', () => {
  expect(await shortPosition()).to.equal(0)
  })
 
+ it('closes 0 value position if above maker limit', async () => {
+ // Get maker product very close to the makerLimit
+ await asset.connect(perennialUser).approve(collateral.address, constants.MaxUint256)
+ await collateral
+ .connect(perennialUser)
+ .depositTo(perennialUser.address, short.address, utils.parseEther('400000'))
+ const shortMakerAvailable = (await short.makerLimit()).sub(
+ (await short.positionAtVersion(await short['latestVersion()']())).maker,
+ )
+ await collateral.connect(perennialUser).depositTo(perennialUser.address, long.address, utils.parseEther('400000'))
+ const longMakerAvailable = (await long.makerLimit()).sub(
+ (await long.positionAtVersion(await long['latestVersion()']())).maker,
+ )
+
+ await short.connect(perennialUser).openMake(shortMakerAvailable)
+ await long.connect(perennialUser).openMake(longMakerAvailable)
+ await updateOracle()
+ await vault.sync()
+
+ await short.connect(marketOwner).updateMakerLimit(0)
+ await long.connect(marketOwner).updateMakerLimit(0)
+
+ // Deposit should create a greater position than what's available
+ const largeDeposit = utils.parseEther('10000')
+ await vault.connect(user).deposit(largeDeposit, user.address)
+ await updateOracle()
+ await expect(vault.sync())
+ .to.emit(short, 'MakeClosed')
+ .withArgs(vault.address, anyUint, 0)
+ .to.emit(long, 'MakeClosed')
+ .withArgs(vault.address, anyUint, 0)
+
+ expect(await longPosition()).to.equal(0)
+ expect(await shortPosition()).to.equal(0)
+ })
+
+ it('closes a 0 value position if account position is at target', async () => {
+ // Deposit should create a greater position than what's available
+ const largeDeposit = utils.parseEther('10000')
+ await vault.connect(user).deposit(largeDeposit, user.address)
+ await updateOracle()
+ await vault.sync()
+
+ await updateOracle()
+ await vault.connect(user).deposit(0, user.address)
+ await expect(vault.sync()).to.emit(short, 'MakeClosed').withArgs(vault.address, anyUint, 0)
+ })
+
  context('liquidation', () => {
  context('long', () => {
  it('recovers before being liquidated', async () => {