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.

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

Remove extra db lookup #1645

Merged
merged 8 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions indexer/packages/postgres/__tests__/db/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ describe('helpers', () => {
);

const unrealizedPnl: string = getUnrealizedPnl(
perpetualPosition, defaultPerpetualMarket, marketIdToMarket,
perpetualPosition,
defaultPerpetualMarket,
marketIdToMarket[defaultPerpetualMarket.marketId],
);

expect(unrealizedPnl).toEqual(Big(-50000).toFixed());
Expand All @@ -125,7 +127,9 @@ describe('helpers', () => {
);

const unrealizedPnl: string = getUnrealizedPnl(
perpetualPosition, defaultPerpetualMarket, marketIdToMarket,
perpetualPosition,
defaultPerpetualMarket,
marketIdToMarket[defaultPerpetualMarket.marketId],
);

expect(unrealizedPnl).toEqual(Big(50000).toFixed());
Expand Down
18 changes: 5 additions & 13 deletions indexer/packages/postgres/src/db/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import { DateTime } from 'luxon';
import { NUM_SECONDS_IN_CANDLE_RESOLUTIONS, ONE_MILLION } from '../constants';
import {
CandleResolution,
FundingIndexMap,
MarketsMap,
FundingIndexMap, MarketFromDatabase,
PerpetualMarketFromDatabase,
PerpetualPositionFromDatabase,
TransferFromDatabase,
Expand Down Expand Up @@ -73,22 +72,15 @@ export function getUnsettledFunding(
* @param position Perpetual position object from the database, or the updated
* perpetual position subaccountKafkaObject.
* @param perpetualMarketsMap Map of perpetual ids to perpetual market objects from the database.
* @param market Market object from the database.
* @returns Unrealized pnl of the position.
*/
export function getUnrealizedPnl(
position: PerpetualPositionFromDatabase | UpdatedPerpetualPositionSubaccountKafkaObject,
perpetualMarket: PerpetualMarketFromDatabase,
marketsMap: MarketsMap,
market: MarketFromDatabase,
): string {
if (marketsMap[perpetualMarket.marketId] === undefined) {
logger.error({
at: 'getUnrealizedPnl',
message: 'Market is undefined',
marketId: perpetualMarket.marketId,
});
return Big(0).toFixed();
}
if (marketsMap[perpetualMarket.marketId].oraclePrice === undefined) {
if (market.oraclePrice === undefined) {
logger.error({
at: 'getUnrealizedPnl',
message: 'Oracle price is undefined for market',
Expand All @@ -98,7 +90,7 @@ export function getUnrealizedPnl(
}
return (
Big(position.size).times(
Big(marketsMap[perpetualMarket.marketId].oraclePrice!).minus(position.entryPrice),
Big(market.oraclePrice!).minus(position.entryPrice),
)
).toFixed();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ export function perpetualPositionToResponseObject(
exitPrice: position.exitPrice && Big(position.exitPrice).toFixed(),
realizedPnl,
unrealizedPnl: helpers.getUnrealizedPnl(
position, perpetualMarketsMap[position.perpetualId], marketsMap,
position, perpetualMarketsMap[position.perpetualId],
marketsMap[perpetualMarketsMap[position.perpetualId].marketId],
),
createdAt: position.createdAt,
createdAtHeight: position.createdAtHeight,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
assetRefresher,
AssetTable,
dbHelpers,
MarketsMap,
PerpetualMarketColumns,
PerpetualMarketFromDatabase,
perpetualMarketRefresher,
Expand Down Expand Up @@ -103,10 +102,6 @@ describe('subaccountUpdateHandler', () => {
isLong: true,
};

const defaultMarketMap: MarketsMap = {
[testConstants.defaultMarket.id]: testConstants.defaultMarket,
};

describe('getParallelizationIds', () => {
it('returns the correct parallelization ids', () => {
const transactionIndex: number = 0;
Expand Down Expand Up @@ -248,7 +243,7 @@ describe('subaccountUpdateHandler', () => {
UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(perpetualPosition!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

There are still multiple references to marketIdToMarket in the codebase that need to be updated.

  • indexer/packages/postgres/__tests__/db/helpers.test.ts
  • indexer/services/comlink/__tests__/lib/helpers.test.ts
  • indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts
  • indexer/services/ender/src/handlers/subaccount-update-handler.ts
  • indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts
  • indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts
  • indexer/services/ender/src/helpers/kafka-helper.ts

Please ensure all these references are updated to use the new market constant.

Analysis chain

LGTM! Ensure all references to the old market map are updated.

Also applies to: 309-309, 384-384, 390-390, 650-650

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify all references to the old market map have been updated.

# Test: Search for the old market map usage. Expect: No occurrences.
rg --type python $'marketIdToMarket'

Length of output: 66



Script:

#!/bin/bash
# Description: Verify all references to the old market map have been updated.

# Test: Search for the old market map usage. Expect: No occurrences.
rg --type typescript $'marketIdToMarket'

Length of output: 74



Script:

#!/bin/bash
# Description: Verify all references to the old market map have been updated.

# Test: Search for the old market map usage. Expect: No occurrences.
rg --type ts $'marketIdToMarket'

Length of output: 2576

);
await expectUpdatedPositionsSubaccountKafkaMessage(
producerSendMock,
Expand Down Expand Up @@ -311,7 +306,7 @@ describe('subaccountUpdateHandler', () => {
UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(perpetualPosition!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
);
await expectUpdatedPositionsSubaccountKafkaMessage(
producerSendMock,
Expand Down Expand Up @@ -386,13 +381,13 @@ describe('subaccountUpdateHandler', () => {
UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(closedPosition!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
);
const newPositionSubaccountKafkaObject:
UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(newPosition!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
);

await expectUpdatedPositionsSubaccountKafkaMessage(
Expand Down Expand Up @@ -567,7 +562,7 @@ describe('subaccountUpdateHandler', () => {
UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(closedPosition!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
);
await expectUpdatedPositionsSubaccountKafkaMessage(
producerSendMock,
Expand Down Expand Up @@ -652,7 +647,7 @@ describe('subaccountUpdateHandler', () => {
UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(perpetualPosition!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
);

const assetPosition: AssetPositionFromDatabase | undefined = await
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ export async function expectFillSubaccountKafkaMessageFromLiquidationEvent(
const positionUpdate = annotateWithPnl(
convertPerpetualPosition(position!),
perpetualMarketRefresher.getPerpetualMarketsMap(),
marketIdToMarket,
marketIdToMarket[parseInt(position!.perpetualId, 10)],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to use direct market access instead of marketIdToMarket map.

- marketIdToMarket[parseInt(position!.perpetualId, 10)],
+ marketIdToMarket[position!.perpetualId],

This change aligns with the PR's objective to streamline market data handling by removing unnecessary parsing and direct access using marketId.

Committable suggestion was skipped due to low confidence.

);

const contents: SubaccountMessageContents = {
Expand Down Expand Up @@ -834,7 +834,7 @@ export async function expectOrderFillAndPositionSubaccountKafkaMessageFromIds(
const positionUpdate: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(position),
perpetualMarketRefresher.getPerpetualMarketsMap(),
marketIdToMarket,
marketIdToMarket[parseInt(position.perpetualId, 10)],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to use direct market access instead of marketIdToMarket map.

- marketIdToMarket[parseInt(position.perpetualId, 10)],
+ marketIdToMarket[position.perpetualId],

This change aligns with the PR's objective to streamline market data handling by removing unnecessary parsing and direct access using marketId.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
marketIdToMarket[parseInt(position.perpetualId, 10)],
marketIdToMarket[position.perpetualId],

);
contents.perpetualPositions = generatePerpetualPositionsContents(
subaccountIdProto,
Expand Down
16 changes: 6 additions & 10 deletions indexer/services/ender/__tests__/helpers/kafka-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,21 @@ import {
dbHelpers,
MarketFromDatabase,
MarketMessageContents,
MarketsMap,
OraclePriceFromDatabase,
OraclePriceTable,
PerpetualMarketFromDatabase,
perpetualMarketRefresher,
PerpetualPositionFromDatabase,
PerpetualPositionStatus,
PerpetualPositionTable,
PositionSide,
SubaccountMessageContents,
SubaccountTable,
testConstants,
testMocks,
TransferFromDatabase,
PerpetualPositionTable,
UpdatedPerpetualPositionSubaccountKafkaObject,
TransferType,
SubaccountTable,
UpdatedPerpetualPositionSubaccountKafkaObject,
} from '@dydxprotocol-indexer/postgres';
import { IndexerSubaccountId } from '@dydxprotocol-indexer/v4-protos';
import { DateTime } from 'luxon';
Expand Down Expand Up @@ -492,9 +491,6 @@ describe('kafka-helper', () => {
});

describe('pnl', () => {
const defaultMarketMap: MarketsMap = {
[testConstants.defaultMarket.id]: testConstants.defaultMarket,
};
const updatedObject: UpdatedPerpetualPositionSubaccountKafkaObject = {
perpetualId: '0',
maxSize: '25',
Expand Down Expand Up @@ -545,7 +541,7 @@ describe('kafka-helper', () => {
} = getPnl(
updatedObject,
perpetualMarketRefresher.getPerpetualMarketsMap()[updatedObject.perpetualId],
defaultMarketMap,
testConstants.defaultMarket,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

There are still multiple references to marketIdToMarket across various files in the codebase. These references need to be updated to ensure consistency with the changes made in the kafka-helper.test.ts file.

  • Files with references to marketIdToMarket:
    • indexer/packages/postgres/__tests__/db/helpers.test.ts
    • indexer/services/ender/src/helpers/kafka-helper.ts
    • indexer/services/ender/src/handlers/subaccount-update-handler.ts
    • indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts
    • indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts
    • indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts
    • indexer/services/comlink/__tests__/lib/helpers.test.ts

Please update these references to ensure consistency with the new approach of using testConstants.defaultMarket.

Analysis chain

LGTM! Ensure all references to the old market map are updated.

Also applies to: 565-565, 607-607

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify all references to the old market map have been updated.

# Test: Search for the old market map usage. Expect: No occurrences.
rg --type python $'marketIdToMarket'

Length of output: 66



Script:

#!/bin/bash
# Description: Verify all references to the old market map have been updated.

# Test: Search for the old market map usage. Expect: No occurrences.
rg --type ts $'marketIdToMarket'

Length of output: 2576

);
expect(realizedPnl).toEqual('-199998'); // 0*0-199998
expect(unrealizedPnl).toEqual('1.5'); // 0.0001*(15000-0)
Expand All @@ -566,7 +562,7 @@ describe('kafka-helper', () => {
} = getPnl(
updatedObject2,
perpetualMarketRefresher.getPerpetualMarketsMap()[updatedObject2.perpetualId],
defaultMarketMap,
testConstants.defaultMarket,
);
expect(realizedPnl).toEqual('-199993'); // 1*5-199998
expect(unrealizedPnl).toEqual('1.5'); // 0.0001*(15000-0)
Expand Down Expand Up @@ -608,7 +604,7 @@ describe('kafka-helper', () => {
const updatedObjectWithPnl: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
updatedObject2,
perpetualMarketRefresher.getPerpetualMarketsMap(),
defaultMarketMap,
testConstants.defaultMarket,
);
expect(
updatedObjectWithPnl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import { logger } from '@dydxprotocol-indexer/base';
import {
FillFromDatabase,
FillModel,
MarketColumns,
MarketFromDatabase,
MarketsMap,
MarketTable,
MarketModel,
PerpetualMarketFromDatabase,
PerpetualMarketModel,
perpetualMarketRefresher,
Expand All @@ -15,7 +13,6 @@ import {
UpdatedPerpetualPositionSubaccountKafkaObject,
} from '@dydxprotocol-indexer/postgres';
import { DeleveragingEventV1 } from '@dydxprotocol-indexer/v4-protos';
import _ from 'lodash';
import * as pg from 'pg';

import { SUBACCOUNT_ORDER_FILL_EVENT_TYPE } from '../../constants';
Expand Down Expand Up @@ -60,31 +57,24 @@ export class DeleveragingHandler extends AbstractOrderFillHandler<DeleveragingEv
resultRow.offsetting_fill) as FillFromDatabase;
const perpetualMarket: PerpetualMarketFromDatabase = PerpetualMarketModel.fromJson(
resultRow.perpetual_market) as PerpetualMarketFromDatabase;
const market: MarketFromDatabase = MarketModel.fromJson(
resultRow.market) as MarketFromDatabase;
const liquidatedPerpetualPosition:
PerpetualPositionFromDatabase = PerpetualPositionModel.fromJson(
resultRow.liquidated_perpetual_position) as PerpetualPositionFromDatabase;
const offsettingPerpetualPosition:
PerpetualPositionFromDatabase = PerpetualPositionModel.fromJson(
resultRow.offsetting_perpetual_position) as PerpetualPositionFromDatabase;
const markets: MarketFromDatabase[] = await MarketTable.findAll(
{},
[],
{ txId: this.txId },
);
const marketIdToMarket: MarketsMap = _.keyBy(
markets,
MarketColumns.id,
);

const liquidatedPositionUpdate: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(liquidatedPerpetualPosition),
perpetualMarketRefresher.getPerpetualMarketsMap(),
marketIdToMarket,
market,
);
const offsettingPositionUpdate: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(offsettingPerpetualPosition),
perpetualMarketRefresher.getPerpetualMarketsMap(),
marketIdToMarket,
market,
);
const kafkaEvents: ConsolidatedKafkaEvent[] = [
this.generateConsolidatedKafkaEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,32 @@ import {
FillFromDatabase,
FillModel,
Liquidity,
MarketFromDatabase,
MarketModel,
OrderFromDatabase,
OrderModel,
OrderStatus,
OrderTable,
PerpetualMarketFromDatabase,
PerpetualMarketModel,
perpetualMarketRefresher,
PerpetualPositionFromDatabase,
PerpetualPositionModel,
SubaccountTable,
OrderStatus,
MarketFromDatabase,
MarketTable,
MarketsMap,
MarketColumns,
UpdatedPerpetualPositionSubaccountKafkaObject,
perpetualMarketRefresher,
} from '@dydxprotocol-indexer/postgres';
import { StateFilledQuantumsCache } from '@dydxprotocol-indexer/redis';
import { isStatefulOrder } from '@dydxprotocol-indexer/v4-proto-parser';
import {
LiquidationOrderV1, IndexerOrderId,
} from '@dydxprotocol-indexer/v4-protos';
import _ from 'lodash';
import { IndexerOrderId, LiquidationOrderV1 } from '@dydxprotocol-indexer/v4-protos';
import Long from 'long';
import * as pg from 'pg';

import { STATEFUL_ORDER_ORDER_FILL_EVENT_TYPE, SUBACCOUNT_ORDER_FILL_EVENT_TYPE } from '../../constants';
import { annotateWithPnl, convertPerpetualPosition } from '../../helpers/kafka-helper';
import { redisClient } from '../../helpers/redis/redis-controller';
import {
orderFillWithLiquidityToOrderFillEventWithLiquidation,
} from '../../helpers/translation-helper';
import { orderFillWithLiquidityToOrderFillEventWithLiquidation } from '../../helpers/translation-helper';
import { OrderFillWithLiquidity } from '../../lib/translated-types';
import {
ConsolidatedKafkaEvent,
OrderFillEventWithLiquidation,
} from '../../lib/types';
import { ConsolidatedKafkaEvent, OrderFillEventWithLiquidation } from '../../lib/types';
import { AbstractOrderFillHandler } from './abstract-order-fill-handler';

export class LiquidationHandler extends AbstractOrderFillHandler<OrderFillWithLiquidity> {
Expand Down Expand Up @@ -99,23 +89,15 @@ export class LiquidationHandler extends AbstractOrderFillHandler<OrderFillWithLi
resultRow[field].fill) as FillFromDatabase;
const perpetualMarket: PerpetualMarketFromDatabase = PerpetualMarketModel.fromJson(
resultRow[field].perpetual_market) as PerpetualMarketFromDatabase;
const market: MarketFromDatabase = MarketModel.fromJson(
resultRow[field].market) as MarketFromDatabase;
const position: PerpetualPositionFromDatabase = PerpetualPositionModel.fromJson(
resultRow[field].perpetual_position) as PerpetualPositionFromDatabase;

const markets: MarketFromDatabase[] = await MarketTable.findAll(
{},
[],
{ txId: this.txId },
);
const marketIdToMarket: MarketsMap = _.keyBy(
markets,
MarketColumns.id,
);

const positionUpdate: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl(
convertPerpetualPosition(position),
perpetualMarketRefresher.getPerpetualMarketsMap(),
marketIdToMarket,
market,
);

if (this.event.liquidity === Liquidity.MAKER) {
Expand Down
Loading
Loading