latestId refactor (#42)

* refactor start

* market unit test update 1

* market errors gone

* almost working

* fix bug

packages/common/testutil/types.ts

@@ -6,7 +6,6 @@ export interface Accumulator {
 }
 
 export interface Position {
-  id: BigNumberish
   timestamp: BigNumberish
   maker: BigNumberish
   long: BigNumberish
@@ -18,6 +17,7 @@ export interface Position {
 
 export interface Global {
   currentId: BigNumberish
+  latestId: BigNumberish
   protocolFee: BigNumberish
   oracleFee: BigNumberish
   riskFee: BigNumberish
@@ -26,6 +26,7 @@ export interface Global {
 
 export interface Local {
   currentId: BigNumberish
+  latestId: BigNumberish
   collateral: BigNumberish
   reward: BigNumberish
   protection: BigNumberish
@@ -46,7 +47,6 @@ export interface Fee {
 }
 
 export function expectPositionEq(a: Position, b: Position): void {
-  expect(a.id).to.equal(b.id, 'Position:Id')
   expect(a.timestamp).to.equal(b.timestamp, 'Position:Timestamp')
   expect(a.maker).to.equal(b.maker, 'Position:Maker')
   expect(a.long).to.equal(b.long, 'Position:Long')
@@ -58,6 +58,7 @@ export function expectPositionEq(a: Position, b: Position): void {
 
 export function expectGlobalEq(a: Global, b: Global): void {
   expect(a.currentId).to.equal(b.currentId, 'Global:CurrentId')
+  expect(a.latestId).to.equal(b.latestId, 'Global:LatestId')
   expect(a.protocolFee).to.equal(b.protocolFee, 'Global:ProtocolFee')
   expect(a.oracleFee).to.equal(b.oracleFee, 'Global:OracleFee')
   expect(a.riskFee).to.equal(b.riskFee, 'Global:RiskFee')
@@ -66,6 +67,7 @@ export function expectGlobalEq(a: Global, b: Global): void {
 
 export function expectLocalEq(a: Local, b: Local): void {
   expect(a.currentId).to.equal(b.currentId, 'Local:Currentid')
+  expect(a.latestId).to.equal(b.latestId, 'Local:LatestId')
   expect(a.collateral).to.equal(b.collateral, 'Local:Collateral')
   expect(a.reward).to.equal(b.reward, 'Local:Reward')
   expect(a.protection).to.equal(b.protection, 'Local:Protection')
@@ -109,7 +111,6 @@ export class Big6Math {
 }
 
 export const DEFAULT_POSITION: Position = {
-  id: 0,
   timestamp: 0,
   long: 0,
   maker: 0,

packages/perennial-extensions/contracts/MultiInvoker.sol

@@ -294,7 +294,8 @@ contract MultiInvoker is IMultiInvoker, Kept {
         latestVersion = OracleVersion(latestPosition.timestamp, market.global().latestPrice, true);
 
         // scan pending position for any ready-to-be-settled positions
-        for (uint256 id = market.positions(account).id; id <= market.locals(account).currentId; id++) {
+        Local memory local = market.locals(account);
+        for (uint256 id = local.latestId; id <= local.currentId; id++) {
             Position memory pendingPosition = market.pendingPositions(account, id);
             OracleVersion memory oracleVersion = market.oracle().at(pendingPosition.timestamp);
 

packages/perennial-vault/contracts/Vault.sol

@@ -419,7 +419,6 @@ contract Vault is IVault, Instance {
             // global
             Global memory global = registration.market.global();
             Position memory currentPosition = registration.market.pendingPosition(global.currentId);
-            Position memory latestPosition = registration.market.position();
 
             context.markets[marketId].latestPrice = global.latestPrice.abs();
             context.markets[marketId].currentPosition = currentPosition.maker;
@@ -431,7 +430,7 @@ contract Vault is IVault, Instance {
             context.markets[marketId].collateral = local.collateral;
 
             // ids
-            context.latestIds.update(marketId, latestPosition.id);
+            context.latestIds.update(marketId, local.latestId);
             context.currentIds.update(marketId, local.currentId);
         }
 

packages/perennial-vault/contracts/lib/StrategyLib.sol

@@ -103,13 +103,12 @@ library StrategyLib {
         context.local = registration.market.locals(address(this));
         context.currentAccountPosition = registration.market.pendingPositions(address(this), context.local.currentId);
 
-        Position memory latestAccountPosition = registration.market.positions(address(this));
         Global memory global = registration.market.global();
 
         context.latestPrice = global.latestPrice.abs();
         context.currentPosition = registration.market.pendingPosition(global.currentId);
 
-        for (uint256 id = latestAccountPosition.id; id < context.local.currentId; id++)
+        for (uint256 id = context.local.latestId; id < context.local.currentId; id++)
             context.maintenance = registration.market.pendingPositions(address(this), id)
                 .maintenance(OracleVersion(0, global.latestPrice, true), context.riskParameter)
                 .max(context.maintenance);

packages/perennial/contracts/Market.sol

@@ -225,9 +225,9 @@ contract Market is IMarket, Instance, ReentrancyGuard {
     ) private view returns (PositionContext memory positionContext) {
         positionContext.global = _pendingPosition[context.global.currentId].read();
         positionContext.local = _pendingPositions[account][context.local.currentId].read();
-        if (context.global.currentId == context.latestPosition.global.id)
+        if (context.global.currentId == context.global.latestId)
             positionContext.global.invalidate(context.latestPosition.global);
-        if (context.local.currentId == context.latestPosition.local.id)
+        if (context.local.currentId == context.local.latestId)
             positionContext.local.invalidate(context.latestPosition.local);
     }
 
@@ -259,13 +259,20 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         if (newLong.eq(UFixed6Lib.MAX)) newLong = context.currentPosition.local.long;
         if (newShort.eq(UFixed6Lib.MAX)) newShort = context.currentPosition.local.short;
 
+        // advance to next id if applicable
+        if (context.currentTimestamp > context.currentPosition.local.timestamp) {
+            context.local.currentId++;
+            context.currentPosition.local.prepare();
+        }
+        if (context.currentTimestamp > context.currentPosition.global.timestamp) {
+            context.global.currentId++;
+            context.currentPosition.global.prepare();
+        }
+
         // update position
-        if (context.currentTimestamp > context.currentPosition.local.timestamp) context.local.currentId++;
-        Order memory newOrder = context.currentPosition.local
-            .update(context.local.currentId, context.currentTimestamp, newMaker, newLong, newShort);
-        if (context.currentTimestamp > context.currentPosition.global.timestamp) context.global.currentId++;
-        context.currentPosition.global
-            .update(context.global.currentId, context.currentTimestamp, newOrder, context.riskParameter);
+        Order memory newOrder =
+            context.currentPosition.local.update(context.currentTimestamp, newMaker, newLong, newShort);
+        context.currentPosition.global.update(context.currentTimestamp, newOrder, context.riskParameter);
 
         // update fee
         newOrder.registerFee(context.latestVersion, context.marketParameter, context.riskParameter);
@@ -348,17 +355,17 @@ contract Market is IMarket, Instance, ReentrancyGuard {
 
         // settle
         while (
-            context.global.currentId != context.latestPosition.global.id &&
-            (nextPosition = _pendingPosition[context.latestPosition.global.id + 1].read()).ready(context.latestVersion)
-        ) _processPositionGlobal(context, nextPosition);
+            context.global.currentId != context.global.latestId &&
+            (nextPosition = _pendingPosition[context.global.latestId + 1].read()).ready(context.latestVersion)
+        ) _processPositionGlobal(context, context.global.latestId + 1, nextPosition);
 
         while (
-            context.local.currentId != context.latestPosition.local.id &&
-            (nextPosition = _pendingPositions[account][context.latestPosition.local.id + 1].read())
+            context.local.currentId != context.local.latestId &&
+            (nextPosition = _pendingPositions[account][context.local.latestId + 1].read())
                 .ready(context.latestVersion)
         ) {
-            Fixed6 previousDelta = _pendingPositions[account][context.latestPosition.local.id].read().delta;
-            _processPositionLocal(context, account, nextPosition);
+            Fixed6 previousDelta = _pendingPositions[account][context.local.latestId].read().delta;
+            _processPositionLocal(context, account, context.local.latestId + 1, nextPosition);
             _checkpointCollateral(context, account, previousDelta, nextPosition);
         }
 
@@ -368,15 +375,15 @@ contract Market is IMarket, Instance, ReentrancyGuard {
 
         // sync
         if (context.latestVersion.timestamp > context.latestPosition.global.timestamp) {
-            nextPosition = _pendingPosition[context.latestPosition.global.id].read();
+            nextPosition = _pendingPosition[context.global.latestId].read();
             nextPosition.sync(context.latestVersion);
-            _processPositionGlobal(context, nextPosition);
+            _processPositionGlobal(context, context.global.latestId, nextPosition);
         }
 
         if (context.latestVersion.timestamp > context.latestPosition.local.timestamp) {
-            nextPosition = _pendingPositions[account][context.latestPosition.local.id].read();
+            nextPosition = _pendingPositions[account][context.local.latestId].read();
             nextPosition.sync(context.latestVersion);
-            _processPositionLocal(context, account, nextPosition);
+            _processPositionLocal(context, account, context.local.latestId, nextPosition);
         }
 
         _position.store(context.latestPosition.global);
@@ -396,24 +403,25 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         Fixed6 previousDelta,
         Position memory nextPosition
     ) private {
-        Position memory latestAccountPosition = _pendingPositions[account][context.latestPosition.local.id].read();
+        Position memory latestAccountPosition = _pendingPositions[account][context.local.latestId].read();
         Position memory currentAccountPosition = _pendingPositions[account][context.local.currentId].read();
         latestAccountPosition.collateral = context.local.collateral
             .sub(currentAccountPosition.delta.sub(previousDelta))         // deposits happen after snapshot point
             .add(Fixed6Lib.from(nextPosition.fee))                        // position fee happens after snapshot point
             .add(Fixed6Lib.from(nextPosition.keeper));                    // keeper fee happens after snapshot point
-        _pendingPositions[account][latestAccountPosition.id].store(latestAccountPosition);
+        _pendingPositions[account][context.local.latestId].store(latestAccountPosition);
     }
 
     /// @notice Processes the given global pending position into the latest position
     /// @param context The context to use
+    /// @param newPositionId The id of the pending position to process
     /// @param newPosition The pending position to process
-    function _processPositionGlobal(Context memory context, Position memory newPosition) private {
+    function _processPositionGlobal(Context memory context, uint256 newPositionId, Position memory newPosition) private {
         Version memory version = _versions[context.latestPosition.global.timestamp].read();
         OracleVersion memory oracleVersion = _oracleVersionAtPosition(context, newPosition);
         if (!oracleVersion.valid) newPosition.invalidate(context.latestPosition.global);
 
-        (uint256 fromTimestamp, uint256 fromId) = (context.latestPosition.global.timestamp, context.latestPosition.global.id);
+        (uint256 fromTimestamp, uint256 fromId) = (context.latestPosition.global.timestamp, context.global.latestId);
         (VersionAccumulationResult memory accumulationResult, UFixed6 accumulatedFee) = version.accumulate(
             context.global,
             context.latestPosition.global,
@@ -424,7 +432,7 @@ contract Market is IMarket, Instance, ReentrancyGuard {
             context.riskParameter
         );
         context.latestPosition.global.update(newPosition);
-        context.global.update(oracleVersion.price);
+        context.global.update(newPositionId, oracleVersion.price);
         context.global.incrementFees(
             accumulatedFee,
             newPosition.keeper,
@@ -438,20 +446,28 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         emit PositionProcessed(
             fromTimestamp,
             newPosition.timestamp,
-            fromId,
+            fromId, // TODO: arjun, should this be newPositionId?
             accumulationResult
         );
     }
 
     /// @notice Processes the given local pending position into the latest position
     /// @param context The context to use
+    /// @param account The account to process for
+    /// @param newPositionId The id of the pending position to process
     /// @param newPosition The pending position to process
-    function _processPositionLocal(Context memory context, address account, Position memory newPosition) private {
+    function _processPositionLocal(
+        Context memory context,
+        address account,
+        uint256 newPositionId,
+        Position memory newPosition
+    ) private {
         Version memory version = _versions[newPosition.timestamp].read();
         if (!version.valid) newPosition.invalidate(context.latestPosition.local);
 
-        (uint256 fromTimestamp, uint256 fromId) = (context.latestPosition.local.timestamp, context.latestPosition.local.id);
+        (uint256 fromTimestamp, uint256 fromId) = (context.latestPosition.local.timestamp, context.local.latestId);
         LocalAccumulationResult memory accumulationResult = context.local.accumulate(
+            newPositionId,
             context.latestPosition.local,
             newPosition,
             _versions[context.latestPosition.local.timestamp].read(),
@@ -464,7 +480,7 @@ contract Market is IMarket, Instance, ReentrancyGuard {
             account,
             fromTimestamp,
             newPosition.timestamp,
-            fromId,
+            fromId, // TODO: arjun, should this be newPositionId?
             accumulationResult
         );
     }
@@ -516,8 +532,8 @@ contract Market is IMarket, Instance, ReentrancyGuard {
         ) { if (LOG_REVERTS) console.log("MarketOperatorNotAllowedError"); revert MarketOperatorNotAllowedError(); }
 
         if (
-            context.global.currentId > context.latestPosition.global.id + context.marketParameter.maxPendingGlobal ||
-            context.local.currentId > context.latestPosition.local.id + context.marketParameter.maxPendingLocal
+            context.global.currentId > context.global.latestId + context.marketParameter.maxPendingGlobal ||
+            context.local.currentId > context.local.latestId + context.marketParameter.maxPendingLocal
         ) { if (LOG_REVERTS) console.log("MarketExceedsPendingIdLimitError"); revert MarketExceedsPendingIdLimitError(); }
 
         if (
@@ -561,10 +577,10 @@ contract Market is IMarket, Instance, ReentrancyGuard {
     ) private view returns (Position[] memory pendingLocalPositions, Fixed6 collateralAfterFees) {
         collateralAfterFees = context.local.collateral;
         pendingLocalPositions = new Position[](
-            context.local.currentId - Math.min(context.latestPosition.local.id, context.local.currentId)
+            context.local.currentId - Math.min(context.local.latestId, context.local.currentId)
         );
         for (uint256 i; i < pendingLocalPositions.length - 1; i++) {
-            pendingLocalPositions[i] = _pendingPositions[account][context.latestPosition.local.id + 1 + i].read();
+            pendingLocalPositions[i] = _pendingPositions[account][context.local.latestId + 1 + i].read();
             collateralAfterFees = collateralAfterFees
                 .sub(Fixed6Lib.from(pendingLocalPositions[i].fee))
                 .sub(Fixed6Lib.from(pendingLocalPositions[i].keeper));

packages/perennial/contracts/test/GlobalTester.sol

@@ -25,9 +25,9 @@ contract GlobalTester {
         global.store(newGlobal);
     }
 
-    function update(Fixed6 latestPrice) external {
+    function update(uint256 latestId, Fixed6 latestPrice) external {
         Global memory newGlobal = global.read();
-        newGlobal.update(latestPrice);
+        newGlobal.update(latestId, latestPrice);
         global.store(newGlobal);
     }
 }

packages/perennial/contracts/test/LocalTester.sol

@@ -21,13 +21,14 @@ contract LocalTester {
     }
 
     function accumulate(
+        uint256 latestId,
         Position memory fromPosition,
         Position memory toPosition,
         Version memory fromVersion,
         Version memory toVersion
     ) external returns (LocalAccumulationResult memory values) {
         Local memory newLocal = local.read();
-        values = newLocal.accumulate(fromPosition, toPosition, fromVersion, toVersion);
+        values = newLocal.accumulate(latestId, fromPosition, toPosition, fromVersion, toVersion);
         local.store(newLocal);
     }
 

packages/perennial/contracts/test/PositionTester.sol

@@ -19,25 +19,23 @@ abstract contract PositionTester {
     }
 
     function update(
-        uint256 currentId,
         uint256 currentTimestamp,
         UFixed6 newMaker,
         UFixed6 newLong,
         UFixed6 newShort
     ) external returns (Order memory newOrder) {
         Position memory _position = read();
-        newOrder = _position.update(currentId, currentTimestamp, newMaker, newLong, newShort);
+        newOrder = _position.update(currentTimestamp, newMaker, newLong, newShort);
         store(_position);
     }
 
     function update(
-        uint256 currentId,
         uint256 currentTimestamp,
         Order memory order,
         RiskParameter memory riskParameter
     ) external returns (Order memory updatedOrder) {
         Position memory _position = read();
-        _position.update(currentId, currentTimestamp, order, riskParameter);
+        _position.update(currentTimestamp, order, riskParameter);
         store(_position);
         return order;
     }
@@ -48,6 +46,12 @@ abstract contract PositionTester {
         store(_position);
     }
 
+    function prepare() external {
+        Position memory _position = read();
+        _position.prepare();
+        store(_position);
+    }
+
     function invalidate(Position memory latestPosition) external {
         Position memory _position = read();
         _position.invalidate(latestPosition);