De-duplicate and filter out invalid pnl ticks for megavault. (backport #2540)

indexer/services/comlink/__tests__/controllers/api/v4/vault-controller.test.ts

@@ -15,6 +15,7 @@ import {
   VaultTable,
   MEGAVAULT_MODULE_ADDRESS,
   MEGAVAULT_SUBACCOUNT_ID,
+  TransferTable,
 } from '@dydxprotocol-indexer/postgres';
 import { RequestMethod, VaultHistoricalPnl } from '../../../../src/types';
 import request from 'supertest';
@@ -181,11 +182,12 @@ describe('vault-controller#V4', () => {
       [
         'hourly resolution',
         '?resolution=hour',
-        [1, undefined, 2, 3, 4],
-        [undefined, 6, 7, 8, 9],
-        [11, undefined, 12, 13, 14],
+        [1, 2, 3, 4],
+        [undefined, 7, 8, 9],
+        [11, 12, 13, 14],
       ],
-    ])('Get /megavault/historicalPnl with 2 vault subaccounts and main subaccount (%s)', async (
+    ])('Get /megavault/historicalPnl with 2 vault subaccounts and main subaccount (%s), ' +
+       'excludes tick with missing vault ticks', async (
       _name: string,
       queryParam: string,
       expectedTicksIndex1: (number | undefined)[],
@@ -202,16 +204,24 @@ describe('vault-controller#V4', () => {
           ...testConstants.defaultVault,
           address: testConstants.defaultAddress,
           clobPairId: testConstants.defaultPerpetualMarket.clobPairId,
+          createdAt: twoDaysAgo.toISO(),
         }),
+        // Single tick for this vault will be excluded from result.
         VaultTable.create({
           ...testConstants.defaultVault,
           address: testConstants.vaultAddress,
           clobPairId: testConstants.defaultPerpetualMarket2.clobPairId,
+          createdAt: almostTwoDaysAgo.toISO(),
         }),
         AssetPositionTable.upsert({
           ...testConstants.defaultAssetPosition,
           subaccountId: MEGAVAULT_SUBACCOUNT_ID,
         }),
+        TransferTable.create({
+          ...testConstants.defaultTransfer,
+          recipientSubaccountId: MEGAVAULT_SUBACCOUNT_ID,
+          createdAt: twoDaysAgo.toISO(),
+        }),
       ]);
 
       const createdPnlTicks: PnlTicksFromDatabase[] = await createPnlTicks(
@@ -559,6 +569,7 @@ describe('vault-controller#V4', () => {
         ...testConstants.defaultPnlTick,
         subaccountId: testConstants.vaultSubaccountId,
       }),
+      // Invalid pnl tick to be excluded as only a single pnl tick but 2 pnl ticks should exist.
       PnlTicksTable.create({
         ...testConstants.defaultPnlTick,
         subaccountId: testConstants.vaultSubaccountId,

indexer/services/comlink/__tests__/lib/helpers.test.ts

@@ -58,7 +58,9 @@ import {
   defaultTendermintEventId2,
   defaultTendermintEventId3,
 } from '@dydxprotocol-indexer/postgres/build/__tests__/helpers/constants';
-import { AssetPositionsMap, PerpetualPositionWithFunding, SubaccountResponseObject } from '../../src/types';
+import {
+  AggregatedPnlTick, AssetPositionsMap, PerpetualPositionWithFunding, SubaccountResponseObject,
+} from '../../src/types';
 import { ZERO, ZERO_USDC_POSITION } from '../../src/lib/constants';
 import { DateTime } from 'luxon';
 
@@ -844,15 +846,20 @@ describe('helpers', () => {
         ),
       };
 
-      const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks([pnlTick]);
+      const aggregatedPnlTicks: AggregatedPnlTick[] = aggregateHourlyPnlTicks([pnlTick]);
       expect(
         aggregatedPnlTicks,
       ).toEqual(
-        [expect.objectContaining({ ...testConstants.defaultPnlTick })],
+        [expect.objectContaining(
+          {
+            pnlTick: expect.objectContaining(testConstants.defaultPnlTick),
+            numTicks: 1,
+          },
+        )],
       );
     });
 
-    it('aggregates multiple pnl ticks same height', () => {
+    it('aggregates multiple pnl ticks same height and de-dupes ticks', () => {
       const pnlTick: PnlTicksFromDatabase = {
         ...testConstants.defaultPnlTick,
         id: PnlTicksTable.uuid(
@@ -862,6 +869,7 @@ describe('helpers', () => {
       };
       const pnlTick2: PnlTicksFromDatabase = {
         ...testConstants.defaultPnlTick,
+        subaccountId: testConstants.defaultSubaccountId2,
         id: PnlTicksTable.uuid(
           testConstants.defaultSubaccountId2,
           testConstants.defaultPnlTick.createdAt,
@@ -883,8 +891,9 @@ describe('helpers', () => {
       const blockTime3: string = DateTime.fromISO(pnlTick.createdAt).plus({ minute: 61 }).toISO();
       const pnlTick4: PnlTicksFromDatabase = {
         ...testConstants.defaultPnlTick,
+        subaccountId: testConstants.defaultSubaccountId2,
         id: PnlTicksTable.uuid(
-          testConstants.defaultPnlTick.subaccountId,
+          testConstants.defaultSubaccountId2,
           blockTime3,
         ),
         equity: '1',
@@ -894,29 +903,52 @@ describe('helpers', () => {
         blockTime: blockTime3,
         createdAt: blockTime3,
       };
+      const blockHeight4: string = '82';
+      const blockTime4: string = DateTime.fromISO(pnlTick.createdAt).startOf('hour').plus({ minute: 63 }).toISO();
+      // should be de-duped
+      const pnlTick5: PnlTicksFromDatabase = {
+        ...testConstants.defaultPnlTick,
+        subaccountId: testConstants.defaultSubaccountId2,
+        id: PnlTicksTable.uuid(
+          testConstants.defaultSubaccountId2,
+          blockTime4,
+        ),
+        equity: '1',
+        totalPnl: '2',
+        netTransfers: '3',
+        blockHeight: blockHeight4,
+        blockTime: blockTime4,
+        createdAt: blockTime4,
+      };
 
-      const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks(
-        [pnlTick, pnlTick2, pnlTick3, pnlTick4],
+      const aggregatedPnlTicks: AggregatedPnlTick[] = aggregateHourlyPnlTicks(
+        [pnlTick, pnlTick2, pnlTick3, pnlTick4, pnlTick5],
       );
       expect(aggregatedPnlTicks).toEqual(
         expect.arrayContaining([
           // Combined pnl tick at initial hour
           expect.objectContaining({
-            equity: (parseFloat(testConstants.defaultPnlTick.equity) +
-            parseFloat(pnlTick2.equity)).toString(),
-            totalPnl: (parseFloat(testConstants.defaultPnlTick.totalPnl) +
-                parseFloat(pnlTick2.totalPnl)).toString(),
-            netTransfers: (parseFloat(testConstants.defaultPnlTick.netTransfers) +
-                parseFloat(pnlTick2.netTransfers)).toString(),
+            pnlTick: expect.objectContaining({
+              equity: (parseFloat(testConstants.defaultPnlTick.equity) +
+              parseFloat(pnlTick2.equity)).toString(),
+              totalPnl: (parseFloat(testConstants.defaultPnlTick.totalPnl) +
+                  parseFloat(pnlTick2.totalPnl)).toString(),
+              netTransfers: (parseFloat(testConstants.defaultPnlTick.netTransfers) +
+                  parseFloat(pnlTick2.netTransfers)).toString(),
+            }),
+            numTicks: 2,
           }),
           // Combined pnl tick at initial hour + 1 hour and initial hour + 1 hour, 1 minute
           expect.objectContaining({
-            equity: (parseFloat(pnlTick3.equity) +
-            parseFloat(pnlTick4.equity)).toString(),
-            totalPnl: (parseFloat(pnlTick3.totalPnl) +
-                parseFloat(pnlTick4.totalPnl)).toString(),
-            netTransfers: (parseFloat(pnlTick3.netTransfers) +
-                parseFloat(pnlTick4.netTransfers)).toString(),
+            pnlTick: expect.objectContaining({
+              equity: (parseFloat(pnlTick3.equity) +
+              parseFloat(pnlTick4.equity)).toString(),
+              totalPnl: (parseFloat(pnlTick3.totalPnl) +
+                  parseFloat(pnlTick4.totalPnl)).toString(),
+              netTransfers: (parseFloat(pnlTick3.netTransfers) +
+                  parseFloat(pnlTick4.netTransfers)).toString(),
+            }),
+            numTicks: 2,
           }),
         ]),
       );

indexer/services/comlink/package.json

@@ -28,14 +28,15 @@
     "@cosmjs/encoding": "^0.32.3",
     "@dydxprotocol-indexer/base": "workspace:^0.0.1",
     "@dydxprotocol-indexer/compliance": "workspace:^0.0.1",
+    "@dydxprotocol-indexer/notifications": "workspace:^0.0.1",
     "@dydxprotocol-indexer/postgres": "workspace:^0.0.1",
     "@dydxprotocol-indexer/redis": "workspace:^0.0.1",
     "@dydxprotocol-indexer/v4-proto-parser": "workspace:^0.0.1",
     "@dydxprotocol-indexer/v4-protos": "workspace:^0.0.1",
     "@keplr-wallet/cosmos": "^0.12.122",
-    "@dydxprotocol-indexer/notifications": "workspace:^0.0.1",
     "@tsoa/runtime": "^5.0.0",
     "big.js": "^6.2.1",
+    "binary-searching": "^2.0.5",
     "body-parser": "^1.20.0",
     "cors": "^2.8.5",
     "dd-trace": "^3.32.1",

indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts

@@ -11,6 +11,7 @@ import {
 } from '@dydxprotocol-indexer/postgres';
 import express from 'express';
 import { matchedData } from 'express-validator';
+import _ from 'lodash';
 import {
   Controller, Get, Query, Route,
 } from 'tsoa';
@@ -156,7 +157,10 @@ class HistoricalPnlController extends Controller {
     }
 
     // aggregate pnlTicks for all subaccounts grouped by blockHeight
-    const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks(pnlTicks);
+    const aggregatedPnlTicks: PnlTicksFromDatabase[] = _.map(
+      aggregateHourlyPnlTicks(pnlTicks),
+      'pnlTick',
+    );
 
     return {
       historicalPnl: aggregatedPnlTicks.map(