From 7780963134e8be081906ef51104965051ad1045a Mon Sep 17 00:00:00 2001 From: Sachin Date: Fri, 15 Apr 2022 21:08:47 +0530 Subject: [PATCH] fix(PerpV2LeverageModuleV2): Round up during withdraw (#249) * Fix: round up during withdraw * Add unit test * Fix failing tests --- .../modules/v2/PerpV2LeverageModuleV2.sol | 7 +++- ...perpV2BasisTradingSlippageIssuance.spec.ts | 8 +---- .../perpV2LeverageV2SlippageIssuance.spec.ts | 8 +---- .../modules/v2/perpV2LeverageModuleV2.spec.ts | 33 ++++++++++++++++--- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/contracts/protocol/modules/v2/PerpV2LeverageModuleV2.sol b/contracts/protocol/modules/v2/PerpV2LeverageModuleV2.sol index 8cb900feb..96893be76 100644 --- a/contracts/protocol/modules/v2/PerpV2LeverageModuleV2.sol +++ b/contracts/protocol/modules/v2/PerpV2LeverageModuleV2.sol @@ -876,7 +876,12 @@ contract PerpV2LeverageModuleV2 is ModuleBaseV2, ReentrancyGuard, Ownable, SetTo returns (uint256) { uint256 initialCollateralPositionBalance = collateralToken.balanceOf(address(_setToken)); - uint256 collateralNotionalQuantity = _collateralQuantityUnits.preciseMul(_setToken.totalSupply()); + // Round up to calculate notional, so that we make atleast `_collateralQuantityUnits` position unit after withdraw. + // Example, let totalSupply = 1.005e18, _collateralQuantityUnits = 13159, then + // collateralNotionalQuantity = 13159 * 1.005e18 / 1e18 = 13225 (13224.795 rounded up) + // We withdraw 13225 from Perp and make a position unit from it. So newPositionUnit = (13225 / 1.005e18) * 1e18 + // = 13159 (13159.2039801 rounded down) + uint256 collateralNotionalQuantity = _collateralQuantityUnits.preciseMulCeil(_setToken.totalSupply()); _withdraw(_setToken, collateralNotionalQuantity); diff --git a/test/integration/perpV2BasisTradingSlippageIssuance.spec.ts b/test/integration/perpV2BasisTradingSlippageIssuance.spec.ts index a8576f252..a4d6a3c6a 100644 --- a/test/integration/perpV2BasisTradingSlippageIssuance.spec.ts +++ b/test/integration/perpV2BasisTradingSlippageIssuance.spec.ts @@ -778,7 +778,7 @@ describe("PerpV2BasisTradingSlippageIssuance", () => { expect(finalOwnerUSDCBalance).to.be.closeTo(expectedUSDCBalance, 1); }); - it("should remove the module when dust is in the account and be able to add module back", async () => { + it("should remove the module and be able to add module back", async () => { // Redeem to `1` await subject(); @@ -811,18 +811,12 @@ describe("PerpV2BasisTradingSlippageIssuance", () => { .connect(owner.wallet) .withdraw(subjectSetToken, freeCollateralPositionUnit); - const { - collateralBalance: finalCollateralBalance - } = await perpBasisTradingModule.getAccountInfo(subjectSetToken); - - /// Remove module await setToken.removeModule(perpBasisTradingModule.address); const finalModules = await setToken.getModules(); expect(finalModules.includes(perpBasisTradingModule.address)).eq(false); expect(positionInfo.length).eq(0); - expect(toUSDCDecimals(finalCollateralBalance)).eq(1); // <-- DUST // Restore module await setToken.connect(owner.wallet).addModule(perpBasisTradingModule.address); diff --git a/test/integration/perpV2LeverageV2SlippageIssuance.spec.ts b/test/integration/perpV2LeverageV2SlippageIssuance.spec.ts index 99fe1d9e3..893d1ef69 100644 --- a/test/integration/perpV2LeverageV2SlippageIssuance.spec.ts +++ b/test/integration/perpV2LeverageV2SlippageIssuance.spec.ts @@ -1129,7 +1129,7 @@ describe("PerpV2LeverageSlippageIssuance", () => { expect(finalOwnerUSDCBalance).to.be.closeTo(expectedUSDCBalance, 1); }); - it("should remove the module when dust is in the account and be able to add module back", async () => { + it("should remove the module and be able to add module back", async () => { // Redeem to `1` await subject(); @@ -1162,18 +1162,12 @@ describe("PerpV2LeverageSlippageIssuance", () => { .connect(owner.wallet) .withdraw(subjectSetToken, freeCollateralPositionUnit); - const { - collateralBalance: finalCollateralBalance - } = await perpLeverageModule.getAccountInfo(subjectSetToken); - - /// Remove module await setToken.removeModule(perpLeverageModule.address); const finalModules = await setToken.getModules(); expect(finalModules.includes(perpLeverageModule.address)).eq(false); expect(positionInfo.length).eq(0); - expect(toUSDCDecimals(finalCollateralBalance)).eq(1); // <-- DUST // Restore module await setToken.connect(owner.wallet).addModule(perpLeverageModule.address); diff --git a/test/protocol/modules/v2/perpV2LeverageModuleV2.spec.ts b/test/protocol/modules/v2/perpV2LeverageModuleV2.spec.ts index adf527dfc..1353f87a5 100644 --- a/test/protocol/modules/v2/perpV2LeverageModuleV2.spec.ts +++ b/test/protocol/modules/v2/perpV2LeverageModuleV2.spec.ts @@ -1505,13 +1505,16 @@ describe("PerpV2LeverageModuleV2", () => { }); describe("#withdraw", () => { - let depositQuantity: BigNumber; + const depositQuantity: BigNumber = usdcUnits(10); let subjectSetToken: SetToken; let subjectWithdrawQuantity: BigNumber; let subjectCaller: Account; let isInitialized: boolean; - const initializeContracts = async () => { + const initializeContracts = async ( + issueQuantity: BigNumber = ether(2), + depositQuantity: BigNumber = usdcUnits(10) + ) => { subjectSetToken = await setup.createSetToken( [usdc.address], [usdcUnits(100)], @@ -1524,13 +1527,11 @@ describe("PerpV2LeverageModuleV2", () => { if (isInitialized) { await perpLeverageModule.initialize(subjectSetToken.address); - const issueQuantity = ether(2); await usdc.approve(setup.issuanceModule.address, usdcUnits(1000)); await setup.issuanceModule.initialize(subjectSetToken.address, ADDRESS_ZERO); await setup.issuanceModule.issue(subjectSetToken.address, issueQuantity, owner.address); // Deposit 10 USDC - depositQuantity = usdcUnits(10); await perpLeverageModule .connect(owner.wallet) .deposit(subjectSetToken.address, depositQuantity); @@ -1713,6 +1714,30 @@ describe("PerpV2LeverageModuleV2", () => { await expect(subject()).to.be.revertedWith("Must be the SetToken manager"); }); }); + + describe("when rounding up notional amount is required", async () => { + beforeEach(async () => { + isInitialized = true; + const issueQuantity = ether(1.005); + const depositQuantity = usdcUnits(100); // deposit all + await initializeContracts(issueQuantity, depositQuantity); + + subjectWithdrawQuantity = BigNumber.from(13159); + }); + + it("should update USDC default position unit", async () => { + await subject(); + + const defaultPositionUnit = await subjectSetToken.getDefaultPositionRealUnit(usdc.address); + + // Round up to calculate notional, so that we make atleast `_collateralQuantityUnits` position unit after withdraw. + // Example, let totalSupply = 1.005e18, _collateralQuantityUnits = 13159, then + // collateralNotionalQuantity = 13159 * 1.005e18 / 1e18 = 13225 (13224.795 rounded up) + // We withdraw 13225 from Perp and make a position unit from it. So newPositionUnit = (13225 / 1.005e18) * 1e18 + // = 13159 (13159.2039801 rounded down) + expect(defaultPositionUnit).to.eq(subjectWithdrawQuantity); // 13159 + }); + }); }); describe("when module is not initialized", async () => {