Skip to content

Commit

Permalink
Fix bug with PnL aggregation. (backport #2446) (#2451)
Browse files Browse the repository at this point in the history 
Co-authored-by: vincentwschau <[email protected]>
  • Loading branch information
@mergify @vincentwschau
mergify[bot] and vincentwschau authored Oct 3, 2024
1 parent 0b9e8a2 commit c182cd9
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 74 deletions.
77 changes: 50 additions & 27 deletions indexer/services/comlink/__tests__/lib/helpers.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
getPerpetualPositionsWithUpdatedFunding,
initializePerpetualPositionsWithFunding,
getChildSubaccountNums,
aggregatePnlTicks,
aggregateHourlyPnlTicks,
getSubaccountResponse,
} from '../../src/lib/helpers';
import _ from 'lodash';
Expand All @@ -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 () => {
Expand Down Expand Up @@ -833,7 +834,7 @@ describe('helpers', () => {
});
});

describe('aggregatePnlTicks', () => {
describe('aggregateHourlyPnlTicks', () => {
it('aggregates single pnl tick', () => {
const pnlTick: PnlTicksFromDatabase = {
...testConstants.defaultPnlTick,
Expand All @@ -843,10 +844,12 @@ describe('helpers', () => {
),
};

const aggregatedPnlTicks: Map<number, PnlTicksFromDatabase> = 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', () => {
Expand All @@ -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<number, PnlTicksFromDatabase> = 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(),
}),
]),
);
});
});
});
6 changes: 3 additions & 3 deletions indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -156,10 +156,10 @@ class HistoricalPnlController extends Controller {
}

// aggregate pnlTicks for all subaccounts grouped by blockHeight
const aggregatedPnlTicks: Map<number, PnlTicksFromDatabase> = aggregatePnlTicks(pnlTicks);
const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks(pnlTicks);

return {
historicalPnl: Array.from(aggregatedPnlTicks.values()).map(
historicalPnl: aggregatedPnlTicks.map(
(pnlTick: PnlTicksFromDatabase) => {
return pnlTicksToResponseObject(pnlTick);
}),
Expand Down
42 changes: 15 additions & 27 deletions indexer/services/comlink/src/controllers/api/v4/vault-controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
import { getReqRateLimiter } from '../../../caches/rate-limiters';
import config from '../../../config';
import {
aggregatePnlTicks,
aggregateHourlyPnlTicks,
getSubaccountResponse,
handleControllerError,
} from '../../../lib/helpers';
Expand Down Expand Up @@ -93,7 +93,7 @@ class VaultController extends Controller {
]);

// aggregate pnlTicks for all vault subaccounts grouped by blockHeight
const aggregatedPnlTicks: Map<number, PnlTicksFromDatabase> = aggregatePnlTicks(vaultPnlTicks);
const aggregatedPnlTicks: PnlTicksFromDatabase[] = aggregateHourlyPnlTicks(vaultPnlTicks);

const currentEquity: string = Array.from(vaultPositions.values())
.map((position: VaultPosition): string => {
Expand Down Expand Up @@ -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<number, PnlTicksFromDatabase>,
pnlTicks: PnlTicksFromDatabase[],
resolution: PnlTickInterval,
): PnlTicksFromDatabase[] {
// Track block to block time.
const blockToBlockTime: Map<number, DateTime> = new Map();
// Track start of days to closest block by block time.
const blocksPerInterval: Map<string, number> = new Map();
// Track start of days to closest Pnl tick.
// Track start of intervals to closest Pnl tick.
const ticksPerInterval: Map<string, PnlTicksFromDatabase> = 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);
}
});
Expand Down
39 changes: 22 additions & 17 deletions indexer/services/comlink/src/lib/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<number, PnlTicksFromDatabase> {
const aggregatedPnlTicks: Map<number, PnlTicksFromDatabase> = new Map();
): PnlTicksFromDatabase[] {
const hourlyPnlTicks: Map<string, PnlTicksFromDatabase> = 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());
}

0 comments on commit c182cd9

Please sign in to comment.