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

@@ -173,16 +173,16 @@ contract Market is IMarket, Instance, ReentrancyGuard {
 
     /// @notice Updates the account's position and collateral
     /// @param account The account to operate on
-    /// @param amount The position delta of the order (positive for long, negative for short)
+    /// @param takerAmount The position delta of the order (positive for long, negative for short)
     /// @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 amount,
+        Fixed6 takerAmount,
         Fixed6 collateral,
         address referrer
     ) external {
-        update(account, Fixed6Lib.ZERO, amount, collateral, referrer);
+        update(account, Fixed6Lib.ZERO, takerAmount, collateral, referrer);
     }
 
     /// @notice Updates the account's position and collateral
@@ -217,61 +217,6 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         _updateAndStore(context, updateContext, newOrder, newGuarantee, referrer, address(0));
     }
 
-    /// @notice Updates the account's position and collateral
-    /// @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));
-    }
-
-    /// @notice Updates the account's position and collateral
-    /// @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 {
-        (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,
-            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
@@ -284,7 +229,7 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         Fixed6 takerAmount;
 
         UFixed6 closable = context.latestPositionLocal.magnitude().sub(context.pendingLocal.neg());
-        
+
         if (updateContext.currentPositionLocal.maker.gt(UFixed6Lib.ZERO)) {
             makerAmount = Fixed6Lib.from(-1, closable);
         } else if (updateContext.currentPositionLocal.long.gt(UFixed6Lib.ZERO)) {

packages/core/contracts/interfaces/IMarket.sol

@@ -160,10 +160,8 @@ interface IMarket is IInstance {
     function guaranteeReferrers(address account, uint256 id) external view returns (address);
     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, Fixed6 takerAmount, Fixed6 collateral, address referrer) external;
     function update(address account, Fixed6 makerAmount, Fixed6 takerAmount, 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 close(address account, bool protect, address referrer) external;
     function parameter() external view returns (MarketParameter memory);
     function riskParameter() external view returns (RiskParameter memory);

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) {
-            IMarket(msg.sender).update(address(0), Fixed6Lib.from(0), Fixed6Lib.from(0), address(0));
+        if (functionToCall == Function.UPDATE) {
+            IMarket(msg.sender).update(address(0), Fixed6Lib.from(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, address(0));
         }
 
         return true;

packages/core/contracts/types/Order.sol

@@ -80,12 +80,13 @@ library OrderLib {
             (UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO);
     }
 
-    /// @notice Creates a new order from the an intent order request
+    /// @notice Creates a new order from an intent or delta order request
     /// @param timestamp The current timestamp
     /// @param position The current position
     /// @param makerAmount The magnitude and direction of maker position
     /// @param takerAmount The magnitude and direction of taker position
     /// @param collateral The change in the collateral
+    /// @param protect True when liquidating the position
     /// @param referralFee The referral fee
     /// @return newOrder The resulting order
     function from(
@@ -100,9 +101,9 @@ library OrderLib {
         newOrder.timestamp = timestamp;
         newOrder.collateral = collateral;
         newOrder.orders = makerAmount.isZero() && takerAmount.isZero() ? 0 : 1;
-        newOrder.protection = protect ? 1 : 0;
         newOrder.makerReferral = makerAmount.abs().mul(referralFee);
         newOrder.takerReferral = takerAmount.abs().mul(referralFee);
+        newOrder.protection = protect ? 1 : 0;
 
         // If the order is not counter to the current position, it is opening
         if (takerAmount.sign() == 0 || position.skew().sign() == 0 || position.skew().sign() == takerAmount.sign()) {
@@ -119,51 +120,6 @@ library OrderLib {
         newOrder.makerNeg = makerAmount.min(Fixed6Lib.ZERO).abs();
     }
 
-    /// @notice Creates a new order from the current position and an update request
-    /// @param timestamp The current timestamp
-    /// @param position The current position
-    /// @param collateral The change in the collateral
-    /// @param newMaker The new maker
-    /// @param newLong The new long
-    /// @param newShort The new short
-    /// @param protect Whether to protect the order
-    /// @param referralFee The referral fee
-    /// @return newOrder The resulting order
-    function from(
-        uint256 timestamp,
-        Position memory position,
-        Fixed6 collateral,
-        UFixed6 newMaker,
-        UFixed6 newLong,
-        UFixed6 newShort,
-        bool protect,
-        UFixed6 referralFee
-    ) internal pure returns (Order memory newOrder) {
-        (Fixed6 makerAmount, Fixed6 longAmount, Fixed6 shortAmount) = (
-            Fixed6Lib.from(newMaker).sub(Fixed6Lib.from(position.maker)),
-            Fixed6Lib.from(newLong).sub(Fixed6Lib.from(position.long)),
-            Fixed6Lib.from(newShort).sub(Fixed6Lib.from(position.short))
-        );
-
-        UFixed6 referral = makerAmount.abs().add(longAmount.abs()).add(shortAmount.abs()).mul(referralFee);
-
-        newOrder = Order(
-            timestamp,
-            0,
-            collateral,
-            makerAmount.max(Fixed6Lib.ZERO).abs(),
-            makerAmount.min(Fixed6Lib.ZERO).abs(),
-            longAmount.max(Fixed6Lib.ZERO).abs(),
-            longAmount.min(Fixed6Lib.ZERO).abs(),
-            shortAmount.max(Fixed6Lib.ZERO).abs(),
-            shortAmount.min(Fixed6Lib.ZERO).abs(),
-            protect ? 1 : 0,
-            makerAmount.isZero() ? UFixed6Lib.ZERO : referral,
-            makerAmount.isZero() ? referral : UFixed6Lib.ZERO
-        );
-        if (!isEmpty(newOrder)) newOrder.orders = 1;
-    }
-
     /// @notice Returns whether the order increases any of the account's positions
     /// @return Whether the order increases any of the account's positions
     function increasesPosition(Order memory self) internal pure returns (bool) {

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, POSITION, 0, 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, POSITION, 0, 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, 0, POSITION, 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, 0, POSITION, 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(user.address, false, constants.AddressZero)).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, POSITION, 0, 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, 0, POSITION, 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(user.address, true, constants.AddressZero)
+    await market.connect(userB).close(userB.address, true, constants.AddressZero)
 
     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, POSITION, 0, 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, 0, POSITION, 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(user.address, true, constants.AddressZero)).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(userB.address, true, constants.AddressZero)
 
     expect((await market.locals(user.address)).collateral).to.equal(userCollateralBefore)
     expect((await market.locals(userB.address)).collateral).to.equal(userBCollateralBefore)