Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with PnL aggregation. #2446

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Fix bug with PnL aggregation. #2446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 50 additions & 27 deletions indexer/services/comlink/__tests__/lib/helpers.test.ts
View file
Open in desktop
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(),
}),
vincentwschau marked this conversation as resolved.
Show resolved Hide resolved
// 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
Open in desktop
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
Open in desktop
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)) {
vincentwschau marked this conversation as resolved.
Show resolved Hide resolved
ticksPerInterval.set(startOfIntervalStr, pnlTick);
}
});
Expand Down
39 changes: 22 additions & 17 deletions indexer/services/comlink/src/lib/helpers.ts
View file
Open in desktop
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
vincentwschau marked this conversation as resolved.
Show resolved Hide resolved
* @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(),
vincentwschau marked this conversation as resolved.
Show resolved Hide resolved
},
);
} else {
aggregatedPnlTicks.set(blockHeight, pnlTick);
hourlyPnlTicks.set(truncatedTime, pnlTick);
}
}
return aggregatedPnlTicks;
return Array.from(hourlyPnlTicks.values());
}
Loading