fix(PerpV2LeverageModuleV2): Round up during withdraw (#249)

* Fix: round up during withdraw * Add unit test * Fix failing tests
SetProtocol · Apr 15, 2022 · 7780963 · 7780963
1 parent 64822a8
commit 7780963
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 () => {