From c182cd94e4577551c79a5bf95b0f79be7fbecbba Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 3 Oct 2024 12:06:31 -0400 Subject: [PATCH] Fix bug with PnL aggregation. (backport #2446) (#2451) Co-authored-by: vincentwschau <99756290+vincentwschau@users.noreply.github.com> --- .../comlink/__tests__/lib/helpers.test.ts | 77 ++++++++++++------- .../api/v4/historical-pnl-controller.ts | 6 +- .../controllers/api/v4/vault-controller.ts | 42 ++++------ indexer/services/comlink/src/lib/helpers.ts | 39 ++++++---- 4 files changed, 90 insertions(+), 74 deletions(-) diff --git a/indexer/services/comlink/__tests__/lib/helpers.test.ts b/indexer/services/comlink/__tests__/lib/helpers.test.ts index 8c9e27bebf..d814827235 100644 --- a/indexer/services/comlink/__tests__/lib/helpers.test.ts +++ b/indexer/services/comlink/__tests__/lib/helpers.test.ts @@ -44,7 +44,7 @@ import { getPerpetualPositionsWithUpdatedFunding, initializePerpetualPositionsWithFunding, getChildSubaccountNums, - aggregatePnlTicks, + aggregateHourlyPnlTicks, getSubaccountResponse, } from '../../src/lib/helpers'; import _ from 'lodash'; @@ -60,6 +60,7 @@ import { } from '@dydxprotocol-indexer/postgres/build/__tests__/helpers/constants'; import { AssetPositionsMap, PerpetualPositionWithFunding, SubaccountResponseObject } from '../../src/types'; import { ZERO, ZERO_USDC_POSITION } from '../../src/lib/constants'; +import { DateTime } from 'luxon'; describe('helpers', () => { afterEach(async () => { @@ -833,7 +834,7 @@ describe('helpers', () => { }); }); - describe('aggregatePnlTicks', () => { + describe('aggregateHourlyPnlTicks', () => { it('aggregates single pnl tick', () => { const pnlTick: PnlTicksFromDatabase = { ...testConstants.defaultPnlTick, @@ -843,10 +844,12 @@ describe('helpers', () => { ), }; - const aggregatedPnlTicks: Map = aggregatePnlTicks([pnlTick]); + const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks([pnlTick]); expect( - aggregatedPnlTicks.get(parseInt(pnlTick.blockHeight, 10)), - ).toEqual(expect.objectContaining({ ...testConstants.defaultPnlTick })); + aggregatedPnlTicks, + ).toEqual( + [expect.objectContaining({ ...testConstants.defaultPnlTick })], + ); }); it('aggregates multiple pnl ticks same height', () => { @@ -865,38 +868,58 @@ describe('helpers', () => { ), }; const blockHeight2: string = '80'; + const blockTime2: string = DateTime.fromISO(pnlTick.createdAt).plus({ hour: 1 }).toISO(); const pnlTick3: PnlTicksFromDatabase = { ...testConstants.defaultPnlTick, id: PnlTicksTable.uuid( testConstants.defaultPnlTick.subaccountId, - testConstants.defaultPnlTick.createdAt, + blockTime2, ), blockHeight: blockHeight2, + blockTime: blockTime2, + createdAt: blockTime2, + }; + const blockHeight3: string = '81'; + const blockTime3: string = DateTime.fromISO(pnlTick.createdAt).plus({ minute: 61 }).toISO(); + const pnlTick4: PnlTicksFromDatabase = { + ...testConstants.defaultPnlTick, + id: PnlTicksTable.uuid( + testConstants.defaultPnlTick.subaccountId, + blockTime3, + ), + equity: '1', + totalPnl: '2', + netTransfers: '3', + blockHeight: blockHeight3, + blockTime: blockTime3, + createdAt: blockTime3, }; - const aggregatedPnlTicks: Map = aggregatePnlTicks( - [pnlTick, pnlTick2, pnlTick3], + const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks( + [pnlTick, pnlTick2, pnlTick3, pnlTick4], ); - // Combined pnl tick at initial block height. - expect( - aggregatedPnlTicks.get(parseInt(pnlTick.blockHeight, 10)), - ).toEqual(expect.objectContaining({ - equity: (parseFloat(testConstants.defaultPnlTick.equity) + + 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(), - createdAt: testConstants.defaultPnlTick.createdAt, - blockHeight: testConstants.defaultPnlTick.blockHeight, - blockTime: testConstants.defaultPnlTick.blockTime, - })); - // Single pnl tick at second block height. - expect( - aggregatedPnlTicks.get(parseInt(blockHeight2, 10)), - ).toEqual(expect.objectContaining({ - ...pnlTick3, - })); + totalPnl: (parseFloat(testConstants.defaultPnlTick.totalPnl) + + parseFloat(pnlTick2.totalPnl)).toString(), + netTransfers: (parseFloat(testConstants.defaultPnlTick.netTransfers) + + parseFloat(pnlTick2.netTransfers)).toString(), + }), + // 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(), + }), + ]), + ); }); }); }); diff --git a/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts b/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts index 6ea7a5ce58..96a3131ea9 100644 --- a/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts @@ -19,7 +19,7 @@ import { getReqRateLimiter } from '../../../caches/rate-limiters'; import config from '../../../config'; import { complianceAndGeoCheck } from '../../../lib/compliance-and-geo-check'; import { NotFoundError } from '../../../lib/errors'; -import { aggregatePnlTicks, getChildSubaccountIds, handleControllerError } from '../../../lib/helpers'; +import { aggregateHourlyPnlTicks, getChildSubaccountIds, handleControllerError } from '../../../lib/helpers'; import { rateLimiterMiddleware } from '../../../lib/rate-limit'; import { CheckLimitAndCreatedBeforeOrAtAndOnOrAfterSchema, @@ -156,10 +156,10 @@ class HistoricalPnlController extends Controller { } // aggregate pnlTicks for all subaccounts grouped by blockHeight - const aggregatedPnlTicks: Map = aggregatePnlTicks(pnlTicks); + const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks(pnlTicks); return { - historicalPnl: Array.from(aggregatedPnlTicks.values()).map( + historicalPnl: aggregatedPnlTicks.map( (pnlTick: PnlTicksFromDatabase) => { return pnlTicksToResponseObject(pnlTick); }), diff --git a/indexer/services/comlink/src/controllers/api/v4/vault-controller.ts b/indexer/services/comlink/src/controllers/api/v4/vault-controller.ts index ee4a31fe64..81f710b315 100644 --- a/indexer/services/comlink/src/controllers/api/v4/vault-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/vault-controller.ts @@ -38,7 +38,7 @@ import { import { getReqRateLimiter } from '../../../caches/rate-limiters'; import config from '../../../config'; import { - aggregatePnlTicks, + aggregateHourlyPnlTicks, getSubaccountResponse, handleControllerError, } from '../../../lib/helpers'; @@ -93,7 +93,7 @@ class VaultController extends Controller { ]); // aggregate pnlTicks for all vault subaccounts grouped by blockHeight - const aggregatedPnlTicks: Map = aggregatePnlTicks(vaultPnlTicks); + const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks(vaultPnlTicks); const currentEquity: string = Array.from(vaultPositions.values()) .map((position: VaultPosition): string => { @@ -473,46 +473,34 @@ function getPnlTicksWithCurrentTick( } /** - * Takes in a map of block heights to PnlTicks and filters out the closest pnl tick per interval. - * @param pnlTicksByBlock Map of block number to pnl tick. + * Takes in an array of PnlTicks and filters out the closest pnl tick per interval. + * @param pnlTicks Array of pnl ticks. * @param resolution Resolution of interval. * @returns Array of PnlTicksFromDatabase, one per interval. */ function filterOutIntervalTicks( - pnlTicksByBlock: Map, + pnlTicks: PnlTicksFromDatabase[], resolution: PnlTickInterval, ): PnlTicksFromDatabase[] { - // Track block to block time. - const blockToBlockTime: Map = new Map(); - // Track start of days to closest block by block time. - const blocksPerInterval: Map = new Map(); - // Track start of days to closest Pnl tick. + // Track start of intervals to closest Pnl tick. const ticksPerInterval: Map = new Map(); - pnlTicksByBlock.forEach((pnlTick: PnlTicksFromDatabase, block: number): void => { + pnlTicks.forEach((pnlTick: PnlTicksFromDatabase): void => { const blockTime: DateTime = DateTime.fromISO(pnlTick.blockTime).toUTC(); - blockToBlockTime.set(block, blockTime); const startOfInterval: DateTime = blockTime.toUTC().startOf(resolution); const startOfIntervalStr: string = startOfInterval.toISO(); - const startOfIntervalBlock: number | undefined = blocksPerInterval.get(startOfIntervalStr); - // No block for the start of interval, set this block as the block for the interval. - if (startOfIntervalBlock === undefined) { - blocksPerInterval.set(startOfIntervalStr, block); - ticksPerInterval.set(startOfIntervalStr, pnlTick); - return; - } - - const startOfDayBlockTime: DateTime | undefined = blockToBlockTime.get(startOfIntervalBlock); - // Invalid block set as start of day block, set this block as the block for the day. - if (startOfDayBlockTime === undefined) { - blocksPerInterval.set(startOfIntervalStr, block); + const tickForInterval: PnlTicksFromDatabase | undefined = ticksPerInterval.get( + startOfIntervalStr, + ); + // No tick for the start of interval, set this tick as the block for the interval. + if (tickForInterval === undefined) { ticksPerInterval.set(startOfIntervalStr, pnlTick); return; } + const tickPerIntervalBlockTime: DateTime = DateTime.fromISO(tickForInterval.blockTime); - // This block is closer to the start of the day, set it as the block for the day. - if (blockTime.diff(startOfInterval) < startOfDayBlockTime.diff(startOfInterval)) { - blocksPerInterval.set(startOfIntervalStr, block); + // This tick is closer to the start of the interval, set it as the tick for the interval. + if (blockTime.diff(startOfInterval) < tickPerIntervalBlockTime.diff(startOfInterval)) { ticksPerInterval.set(startOfIntervalStr, pnlTick); } }); diff --git a/indexer/services/comlink/src/lib/helpers.ts b/indexer/services/comlink/src/lib/helpers.ts index 6da4106907..b3f5d3bf19 100644 --- a/indexer/services/comlink/src/lib/helpers.ts +++ b/indexer/services/comlink/src/lib/helpers.ts @@ -28,6 +28,7 @@ import { import Big from 'big.js'; import express from 'express'; import _ from 'lodash'; +import { DateTime } from 'luxon'; import config from '../config'; import { @@ -672,32 +673,36 @@ export function getSubaccountResponse( /* ------- PNL HELPERS ------- */ /** - * Aggregates a list of PnL ticks, combining any PnL ticks for the same blockheight by summing + * Aggregates a list of PnL ticks, combining any PnL ticks for the same hour by summing * the equity, totalPnl, and net transfers. * Returns a map of block height to the resulting PnL tick. * @param pnlTicks * @returns */ -export function aggregatePnlTicks( +export function aggregateHourlyPnlTicks( pnlTicks: PnlTicksFromDatabase[], -): Map { - const aggregatedPnlTicks: Map = new Map(); +): PnlTicksFromDatabase[] { + const hourlyPnlTicks: Map = new Map(); for (const pnlTick of pnlTicks) { - const blockHeight: number = parseInt(pnlTick.blockHeight, 10); - if (aggregatedPnlTicks.has(blockHeight)) { - const currentPnlTick: PnlTicksFromDatabase = aggregatedPnlTicks.get( - blockHeight, + const truncatedTime: string = DateTime.fromISO(pnlTick.createdAt).startOf('hour').toISO(); + if (hourlyPnlTicks.has(truncatedTime)) { + const aggregatedTick: PnlTicksFromDatabase = hourlyPnlTicks.get( + truncatedTime, ) as PnlTicksFromDatabase; - aggregatedPnlTicks.set(blockHeight, { - ...currentPnlTick, - equity: (parseFloat(currentPnlTick.equity) + parseFloat(pnlTick.equity)).toString(), - totalPnl: (parseFloat(currentPnlTick.totalPnl) + parseFloat(pnlTick.totalPnl)).toString(), - netTransfers: (parseFloat(currentPnlTick.netTransfers) + - parseFloat(pnlTick.netTransfers)).toString(), - }); + hourlyPnlTicks.set( + truncatedTime, + { + ...aggregatedTick, + equity: (parseFloat(aggregatedTick.equity) + parseFloat(pnlTick.equity)).toString(), + totalPnl: (parseFloat(aggregatedTick.totalPnl) + parseFloat(pnlTick.totalPnl)).toString(), + netTransfers: ( + parseFloat(aggregatedTick.netTransfers) + parseFloat(pnlTick.netTransfers) + ).toString(), + }, + ); } else { - aggregatedPnlTicks.set(blockHeight, pnlTick); + hourlyPnlTicks.set(truncatedTime, pnlTick); } } - return aggregatedPnlTicks; + return Array.from(hourlyPnlTicks.values()); }