Remove absolute position update

.github/workflows/CI.yml

@@ -7,6 +7,7 @@ on:
     branches:
       - main
       - v2.4
+      - prateek/pe-1629-remove-magic-values
   workflow_dispatch:
 
 env:

packages/core/contracts/Market.sol

@@ -181,15 +181,32 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         Fixed6 amount,
         Fixed6 collateral,
         address referrer
-    ) external nonReentrant whenNotPaused {
+    ) external {
+        update(account, amount, Fixed6Lib.ZERO, collateral, referrer);
+    }
+
+    /// @notice Updates the account's position and collateral
+    /// @param account The account to operate on
+    /// @param taker The taker delta amount of the order (positive for long, negative for short)
+    /// @param maker The maker delta amount of the order
+    /// @param collateral The collateral delta of the order (positive for deposit, negative for withdrawal)
+    /// @param referrer The referrer of the order
+    function update(
+        address account,
+        Fixed6 taker,
+        Fixed6 maker,
+        Fixed6 collateral,
+        address referrer
+    ) public nonReentrant whenNotPaused {
         (Context memory context, UpdateContext memory updateContext) =
             _loadForUpdate(account, address(0), referrer, address(0), UFixed6Lib.ZERO, UFixed6Lib.ZERO);
 
         // create new order & guarantee
         Order memory newOrder = OrderLib.from(
             context.currentTimestamp,
             updateContext.currentPositionLocal,
-            amount,
+            taker,
+            maker,
             collateral,
             updateContext.orderReferralFee
         );
@@ -199,68 +216,37 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         _updateAndStore(context, updateContext, newOrder, newGuarantee, referrer, address(0));
     }
 
-    /// @notice Updates the account's position and collateral
+    /// @notice Closes the account's position
     /// @param account The account to operate on
-    /// @param newMaker The new maker position for the account
-    /// @param newMaker The new long position for the account
-    /// @param newMaker The new short position for the account
-    /// @param collateral The collateral amount to add or remove from the account
     /// @param protect Whether to put the account into a protected status for liquidations
-    function update(
-        address account,
-        UFixed6 newMaker,
-        UFixed6 newLong,
-        UFixed6 newShort,
-        Fixed6 collateral,
-        bool protect
-    ) external {
-        update(account, newMaker, newLong, newShort, collateral, protect, address(0));
+    function close(address account, bool protect) external {
+        close(account, protect, address(0));
     }
 
-    /// @notice Updates the account's position and collateral
+    /// @notice Closes the account's position
     /// @param account The account to operate on
-    /// @param newMaker The new maker position for the account
-    /// @param newLong The new long position for the account
-    /// @param newShort The new short position for the account
-    /// @param collateral The collateral amount to add or remove from the account
     /// @param protect Whether to put the account into a protected status for liquidations
     /// @param referrer The referrer of the order
-    function update(
-        address account,
-        UFixed6 newMaker,
-        UFixed6 newLong,
-        UFixed6 newShort,
-        Fixed6 collateral,
-        bool protect,
-        address referrer
-    ) public nonReentrant whenNotPaused {
+    function close(address account, bool protect, address referrer) public nonReentrant whenNotPaused {
         (Context memory context, UpdateContext memory updateContext) =
             _loadForUpdate(account, address(0), referrer, address(0), UFixed6Lib.ZERO, UFixed6Lib.ZERO);
 
         // create new order & guarantee
         Order memory newOrder = OrderLib.from(
             context.currentTimestamp,
             updateContext.currentPositionLocal,
-            collateral,
-            newMaker,
-            newLong,
-            newShort,
+            Fixed6Lib.ZERO,
+            UFixed6Lib.ZERO,
+            UFixed6Lib.ZERO,
+            UFixed6Lib.ZERO,
             protect,
             updateContext.orderReferralFee
         );
         Guarantee memory newGuarantee; // no guarantee is created for a market order
-
         // process update
         _updateAndStore(context, updateContext, newOrder, newGuarantee, referrer, address(0));
     }
 
-    /// @notice Closes the account's position
-    /// @param account The account to operate on
-    /// @param protect Whether to put the account into a protected status for liquidations
-    function close(address account, bool protect) external {
-        update(account, UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO, Fixed6Lib.ZERO, protect, address(0));
-    }
-
     /// @notice Updates the beneficiary of the market
     /// @param newBeneficiary The new beneficiary address
     function updateBeneficiary(address newBeneficiary) external onlyOwner {
@@ -612,6 +598,7 @@ contract Market is IMarket, Instance, ReentrancyGuard {
             updateContext.currentPositionLocal,
             amount,
             Fixed6Lib.ZERO,
+            Fixed6Lib.ZERO,
             updateContext.orderReferralFee
         );
         Guarantee memory newGuarantee = GuaranteeLib.from(

packages/core/contracts/interfaces/IMarket.sol

@@ -161,9 +161,9 @@ interface IMarket is IInstance {
     function settle(address account) external;
     function update(address account, Intent calldata intent, bytes memory signature) external;
     function update(address account, Fixed6 amount, Fixed6 collateral, address referrer) external;
-    function update(address account, UFixed6 newMaker, UFixed6 newLong, UFixed6 newShort, Fixed6 collateral, bool protect) external;
-    function update(address account, UFixed6 newMaker, UFixed6 newLong, UFixed6 newShort, Fixed6 collateral, bool protect, address referrer) external;
+    function update(address account, Fixed6 taker, Fixed6 maker, Fixed6 collateral, address referrer) external;
     function close(address account, bool protect) external;
+    function close(address account, bool protect, address referrer) external;
     function parameter() external view returns (MarketParameter memory);
     function riskParameter() external view returns (RiskParameter memory);
     function updateBeneficiary(address newBeneficiary) external;

packages/core/contracts/test/MockToken.sol

@@ -9,23 +9,23 @@ import { IMarket } from "../interfaces/IMarket.sol";
 
 contract MockToken is ERC20 {
 
-    enum Function{ SETTLE, UPDATE, UPDATE_MAKER, UPDATE_INTENT }
+    enum Function{ SETTLE, UPDATE, UPDATE_INTENT, CLOSE }
 
     Function private functionToCall;
 
     constructor() ERC20("MockToken", "MTOKEN") {}
 
     function transferFrom(address, address, uint256) public override returns (bool) {
         // call method of market contract for reentrancy
-        if (functionToCall == Function.UPDATE_MAKER) {
-            IMarket(msg.sender).update(address(0), UFixed6Lib.from(0), UFixed6Lib.from(0), UFixed6Lib.from(0), Fixed6Lib.from(0), false);
-        } else if (functionToCall == Function.UPDATE) {
+        if (functionToCall == Function.UPDATE) {
             IMarket(msg.sender).update(address(0), Fixed6Lib.from(0), Fixed6Lib.from(0), address(0));
         } else if (functionToCall == Function.UPDATE_INTENT) {
             Intent memory intent;
             IMarket(msg.sender).update(address(0), intent, "");
         } else if (functionToCall == Function.SETTLE){
             IMarket(msg.sender).settle(address(0));
+        } else if (functionToCall == Function.CLOSE) {
+            IMarket(msg.sender).close(address(0), false);
         }
 
         return true;

packages/core/contracts/types/Order.sol

@@ -83,32 +83,38 @@ library OrderLib {
     /// @notice Creates a new order from the an intent order request
     /// @param timestamp The current timestamp
     /// @param position The current position
-    /// @param amount The magnitude and direction of the order
+    /// @param takerAmount The magnitude and direction of taker position
+    /// @param makerAmount The magnitude and direction of maker position
     /// @param collateral The change in the collateral
     /// @param referralFee The referral fee
     /// @return newOrder The resulting order
     function from(
         uint256 timestamp,
         Position memory position,
-        Fixed6 amount,
+        Fixed6 takerAmount,
+        Fixed6 makerAmount,
         Fixed6 collateral,
         UFixed6 referralFee
     ) internal pure returns (Order memory newOrder) {
         newOrder.timestamp = timestamp;
         newOrder.collateral = collateral;
-        newOrder.orders = amount.isZero() ? 0 : 1;
-        newOrder.takerReferral = amount.abs().mul(referralFee);
+        newOrder.orders = makerAmount.isZero() && takerAmount.isZero() ? 0 : 1;
+        newOrder.makerReferral = makerAmount.abs().mul(referralFee);
+        newOrder.takerReferral = takerAmount.abs().mul(referralFee);
 
         // If the order is not counter to the current position, it is opening
-        if (amount.sign() == 0 || position.skew().sign() == 0 || position.skew().sign() == amount.sign()) {
-            newOrder.longPos = amount.max(Fixed6Lib.ZERO).abs();
-            newOrder.shortPos = amount.min(Fixed6Lib.ZERO).abs();
+        if (takerAmount.sign() == 0 || position.skew().sign() == 0 || position.skew().sign() == takerAmount.sign()) {
+            newOrder.longPos = takerAmount.max(Fixed6Lib.ZERO).abs();
+            newOrder.shortPos = takerAmount.min(Fixed6Lib.ZERO).abs();
 
         // If the order is counter to the current position, it is closing
         } else {
-            newOrder.shortNeg = amount.max(Fixed6Lib.ZERO).abs();
-            newOrder.longNeg = amount.min(Fixed6Lib.ZERO).abs();
+            newOrder.shortNeg = takerAmount.max(Fixed6Lib.ZERO).abs();
+            newOrder.longNeg = takerAmount.min(Fixed6Lib.ZERO).abs();
         }
+
+        newOrder.makerPos = makerAmount.max(Fixed6Lib.ZERO).abs();
+        newOrder.makerNeg = makerAmount.min(Fixed6Lib.ZERO).abs();
     }
 
     /// @notice Creates a new order from the current position and an update request

packages/core/test/integration/core/closedProduct.test.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai'
 import 'hardhat'
 import { BigNumber, constants } from 'ethers'
 const { AddressZero } = constants
 
 import { InstanceVars, deployProtocol, createMarket, settle } from '../helpers/setupHelpers'
 import { Market } from '../../../types/generated'
@@ -21,13 +21,13 @@
   it('closes the market', async () => {
     const POSITION = parse6decimal('10')
     const COLLATERAL = parse6decimal('1000')
     const { owner, user, dsu, chainlink, beneficiaryB } = instanceVars
 
     const market = await createMarket(instanceVars)
     await dsu.connect(user).approve(market.address, COLLATERAL.mul(1e12))
     await market
       .connect(user)
-      ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, POSITION, 0, 0, COLLATERAL, false)
+      ['update(address,int256,int256,int256,address)'](user.address, 0, POSITION, COLLATERAL, constants.AddressZero)
 
     expect((await market.parameter()).closed).to.be.false
 
@@ -49,17 +49,17 @@
     const COLLATERAL = parse6decimal('1000')
 
     beforeEach(async () => {
       const { user, userB, dsu, beneficiaryB } = instanceVars
 
       market = await createMarket(instanceVars)
       await dsu.connect(user).approve(market.address, COLLATERAL.mul(1e12))
       await dsu.connect(userB).approve(market.address, COLLATERAL.mul(1e12))
       await market
         .connect(user)
-        ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, POSITION, 0, 0, COLLATERAL, false)
+        ['update(address,int256,int256,int256,address)'](user.address, 0, POSITION, COLLATERAL, constants.AddressZero)
       await market
         .connect(userB)
-        ['update(address,uint256,uint256,uint256,int256,bool)'](userB.address, 0, POSITION, 0, COLLATERAL, false)
+        ['update(address,int256,int256,int256,address)'](userB.address, POSITION, 0, COLLATERAL, constants.AddressZero)
       const parameters = { ...(await market.parameter()) }
       parameters.closed = true
       await market.updateParameter(parameters)
@@ -73,7 +73,7 @@
       await expect(
         market
           .connect(user)
-          ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, 0, POSITION, 0, 0, false),
+          ['update(address,int256,int256,int256,address)'](user.address, POSITION, 0, 0, constants.AddressZero),
       ).to.be.revertedWithCustomError(market, 'MarketClosedError')
     })
 
@@ -82,28 +82,24 @@
 
       await chainlink.next()
 
-      await expect(
-        await market
-          .connect(user)
-          ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, 0, 0, 0, 0, false),
-      ).to.not.be.reverted
+      await expect(await market.connect(user)['close(address,bool)'](user.address, false)).to.not.be.reverted
     })
   })
 
   it('zeroes PnL and fees', async () => {
     const POSITION = parse6decimal('10')
     const COLLATERAL = parse6decimal('1000')
     const { user, userB, chainlink, dsu, beneficiaryB } = instanceVars
 
     const market = await createMarket(instanceVars)
     await dsu.connect(user).approve(market.address, COLLATERAL.mul(1e12))
     await dsu.connect(userB).approve(market.address, COLLATERAL.mul(1e12))
     await market
       .connect(user)
-      ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, POSITION, 0, 0, COLLATERAL, false)
+      ['update(address,int256,int256,int256,address)'](user.address, 0, POSITION, COLLATERAL, constants.AddressZero)
     await market
       .connect(userB)
-      ['update(address,uint256,uint256,uint256,int256,bool)'](userB.address, 0, POSITION, 0, COLLATERAL, false)
+      ['update(address,int256,int256,int256,address)'](userB.address, POSITION, 0, COLLATERAL, constants.AddressZero)
 
     await chainlink.next()
     await chainlink.next()
@@ -122,9 +118,9 @@
     await chainlink.nextWithPriceModification(price => price.mul(4))
     await chainlink.nextWithPriceModification(price => price.mul(4))
 
     const LIQUIDATION_FEE = BigNumber.from('1000000000')
-    await market.connect(user)['update(address,uint256,uint256,uint256,int256,bool)'](user.address, 0, 0, 0, 0, true)
-    await market.connect(userB)['update(address,uint256,uint256,uint256,int256,bool)'](userB.address, 0, 0, 0, 0, true)
+    await market.connect(user)['close(address,bool)'](user.address, true)
+    await market.connect(userB)['close(address,bool)'](userB.address, true)
 
     expect((await market.locals(user.address)).collateral).to.equal(userCollateralBefore)
     expect((await market.locals(userB.address)).collateral).to.equal(userBCollateralBefore)
@@ -136,23 +132,21 @@
   it('handles closing during liquidations', async () => {
     const POSITION = parse6decimal('10')
     const COLLATERAL = parse6decimal('1000')
     const { user, userB, chainlink, dsu, beneficiaryB } = instanceVars
 
     const market = await createMarket(instanceVars)
     await dsu.connect(user).approve(market.address, COLLATERAL.mul(1e12))
     await dsu.connect(userB).approve(market.address, COLLATERAL.mul(1e12))
     await market
       .connect(user)
-      ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, POSITION, 0, 0, COLLATERAL, false)
+      ['update(address,int256,int256,int256,address)'](user.address, 0, POSITION, COLLATERAL, constants.AddressZero)
     await market
       .connect(userB)
-      ['update(address,uint256,uint256,uint256,int256,bool)'](userB.address, 0, POSITION, 0, COLLATERAL, false)
+      ['update(address,int256,int256,int256,address)'](userB.address, POSITION, 0, COLLATERAL, constants.AddressZero)
 
     await chainlink.next()
     await chainlink.nextWithPriceModification(price => price.mul(2))
-    await expect(
-      market.connect(userB)['update(address,uint256,uint256,uint256,int256,bool)'](user.address, 0, 0, 0, 0, true),
-    ).to.not.be.reverted
+    await expect(market.connect(userB)['close(address,bool)'](user.address, true)).to.not.be.reverted
     expect((await market.pendingOrders(user.address, 2)).protection).to.eq(1)
     const parameters = { ...(await market.parameter()) }
     parameters.closed = true
@@ -173,9 +167,9 @@
     await chainlink.nextWithPriceModification(price => price.mul(4))
     await chainlink.nextWithPriceModification(price => price.mul(4))
 
     const LIQUIDATION_FEE = BigNumber.from('1000000000')
     await settle(market, user)
-    await market.connect(userB)['update(address,uint256,uint256,uint256,int256,bool)'](userB.address, 0, 0, 0, 0, true)
+    await market.connect(userB)['close(address,bool)'](userB.address, true)
 
     expect((await market.locals(user.address)).collateral).to.equal(userCollateralBefore)
     expect((await market.locals(userB.address)).collateral).to.equal(userBCollateralBefore)