add missing data in position modified events (#1580)

Synthetixio · Nov 2, 2021 · 6a1f7ee · 6a1f7ee
1 parent 095d193
commit 6a1f7ee
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 31 deletions.
diff --git a/contracts/FuturesMarket.sol b/contracts/FuturesMarket.sol
@@ -987,10 +987,10 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures
         }
 
         Position storage position = positions[sender];
-        Position memory _position = position;
+        Position memory oldPosition = position;
 
         // Determine new margin, ensuring that the result is positive.
-        (uint margin, Status status) = _realisedMargin(_position, fundingIndex, price, marginDelta);
+        (uint margin, Status status) = _realisedMargin(oldPosition, fundingIndex, price, marginDelta);
         _revertIfError(status);
 
         // Update the debt correction.
@@ -1024,16 +1024,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures
         if (marginDelta != 0) {
             emitMarginTransferred(sender, marginDelta);
         }
-        emitPositionModified(
-            position.id,
-            sender,
-            margin,
-            positionSize,
-            0,
-            positionSize != 0 ? price : position.lastPrice,
-            positionSize != 0 ? fundingIndex : position.fundingIndex,
-            0
-        );
+        emitPositionModified(position.id, sender, margin, positionSize, 0, price, fundingIndex, 0);
     }
 
     /*
@@ -1088,30 +1079,26 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures
         _applyDebtCorrection(newPosition, oldPosition);
 
         // Record the trade
+        uint id = oldPosition.id;
         if (newPosition.size == 0) {
             // If the position is being closed, we no longer need to track these details.
             delete position.id;
             delete position.size;
             delete position.lastPrice;
             delete position.fundingIndex;
-            // Note we still emit the old position id in the event to indicate that it's closing.
-            emitPositionModified(oldPosition.id, sender, newPosition.margin, 0, sizeDelta, 0, 0, fee);
         } else {
-            uint id;
             if (oldPosition.size == 0) {
                 // New positions get new ids.
                 id = _nextPositionId;
                 _nextPositionId += 1;
-                position.id = id;
-            } else {
-                // If an existing position is just being modified, reuse the existing id.
-                id = newPosition.id;
             }
+            position.id = id;
             position.size = newPosition.size;
             position.lastPrice = price;
             position.fundingIndex = fundingIndex;
-            emitPositionModified(id, sender, newPosition.margin, newPosition.size, sizeDelta, price, fundingIndex, fee);
         }
+        // emit the modification event
+        emitPositionModified(id, sender, position.margin, position.size, sizeDelta, price, fundingIndex, fee);
     }
 
     /*
@@ -1200,7 +1187,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures
         // Issue the reward to the liquidator.
         _manager().issueSUSD(liquidator, liquidationFee);
 
-        emitPositionModified(positionId, account, 0, 0, 0, 0, 0, 0);
+        emitPositionModified(positionId, account, 0, 0, 0, price, fundingIndex, 0);
         emitPositionLiquidated(positionId, account, liquidator, positionSize, lPrice, liquidationFee);
     }
 

diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js
@@ -935,8 +935,8 @@ contract('FuturesMarket', accounts => {
 						toUnit('1000'),
 						toBN('0'),
 						toBN('0'),
-						toBN('0'),
-						toBN('0'),
+						(await futuresMarket.assetPrice()).price,
+						await futuresMarket.fundingSequenceLength(),
 						toBN('0'),
 					],
 					log: decodedLogs[3],
@@ -981,8 +981,8 @@ contract('FuturesMarket', accounts => {
 						toUnit('0'),
 						toBN('0'),
 						toBN('0'),
-						toBN('0'),
-						toBN('0'),
+						(await futuresMarket.assetPrice()).price,
+						await futuresMarket.fundingSequenceLength(),
 						toBN('0'),
 					],
 					log: decodedLogs[3],
@@ -1638,8 +1638,8 @@ contract('FuturesMarket', accounts => {
 						toUnit('2000'),
 						toBN('0'),
 						toUnit('-10'),
-						toBN('0'),
-						toBN('0'),
+						(await futuresMarket.assetPrice()).price,
+						await futuresMarket.fundingSequenceLength(),
 						toBN('0'),
 					],
 					log: decodedLogs[1],
@@ -1718,7 +1718,7 @@ contract('FuturesMarket', accounts => {
 				assert.bnEqual(positionId, toBN('1'));
 			});
 
-			it('closing a position deletes the id but emits in in the event', async () => {
+			it('closing a position deletes the id but emits it in the event', async () => {
 				await setPrice(baseAsset, toUnit('100'));
 				await futuresMarket.transferMargin(toUnit('1000'), { from: trader });
 				await futuresMarket.transferMargin(toUnit('1000'), { from: trader2 });
@@ -3432,7 +3432,16 @@ contract('FuturesMarket', accounts => {
 				decodedEventEqual({
 					event: 'PositionModified',
 					emittedFrom: proxyFuturesMarket.address,
-					args: [positionId, trader, toBN('0'), toBN('0'), toBN('0'), toBN('0'), toBN('0')],
+					args: [
+						positionId,
+						trader,
+						toBN('0'),
+						toBN('0'),
+						toBN('0'),
+						(await futuresMarket.assetPrice()).price,
+						await futuresMarket.fundingSequenceLength(),
+						toBN('0'),
+					],
 					log: decodedLogs[2],
 				});
 				decodedEventEqual({
@@ -3474,7 +3483,16 @@ contract('FuturesMarket', accounts => {
 				decodedEventEqual({
 					event: 'PositionModified',
 					emittedFrom: proxyFuturesMarket.address,
-					args: [positionId, trader3, toBN('0'), toBN('0'), toBN('0'), toBN('0'), toBN('0')],
+					args: [
+						positionId,
+						trader3,
+						toBN('0'),
+						toBN('0'),
+						toBN('0'),
+						(await futuresMarket.assetPrice()).price,
+						await futuresMarket.fundingSequenceLength(),
+						toBN('0'),
+					],
 					log: decodedLogs[2],
 				});
 				decodedEventEqual({
@@ -3511,7 +3529,16 @@ contract('FuturesMarket', accounts => {
 				decodedEventEqual({
 					event: 'PositionModified',
 					emittedFrom: proxyFuturesMarket.address,
-					args: [positionId, trader, toBN('0'), toBN('0'), toBN('0'), toBN('0'), toBN('0')],
+					args: [
+						positionId,
+						trader,
+						toBN('0'),
+						toBN('0'),
+						toBN('0'),
+						(await futuresMarket.assetPrice()).price,
+						await futuresMarket.fundingSequenceLength(),
+						toBN('0'),
+					],
 					log: decodedLogs[2],
 				});
 				decodedEventEqual({