-
Notifications
You must be signed in to change notification settings - Fork 115
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
Remove extra db lookup #1645
Changes from all commits
6564dee
02a935f
5b1213b
a029cf7
97e0e98
ec5e702
8454979
a5a4bc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -692,7 +692,7 @@ export async function expectFillSubaccountKafkaMessageFromLiquidationEvent( | |||||
const positionUpdate = annotateWithPnl( | ||||||
convertPerpetualPosition(position!), | ||||||
perpetualMarketRefresher.getPerpetualMarketsMap(), | ||||||
marketIdToMarket, | ||||||
marketIdToMarket[parseInt(position!.perpetualId, 10)], | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor to use direct market access instead of - 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
|
||||||
); | ||||||
|
||||||
const contents: SubaccountMessageContents = { | ||||||
|
@@ -834,7 +834,7 @@ export async function expectOrderFillAndPositionSubaccountKafkaMessageFromIds( | |||||
const positionUpdate: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl( | ||||||
convertPerpetualPosition(position), | ||||||
perpetualMarketRefresher.getPerpetualMarketsMap(), | ||||||
marketIdToMarket, | ||||||
marketIdToMarket[parseInt(position.perpetualId, 10)], | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor to use direct market access instead of - 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 Committable suggestion
Suggested change
|
||||||
); | ||||||
contents.perpetualPositions = generatePerpetualPositionsContents( | ||||||
subaccountIdProto, | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -492,9 +491,6 @@ describe('kafka-helper', () => { | |
}); | ||
|
||
describe('pnl', () => { | ||
const defaultMarketMap: MarketsMap = { | ||
[testConstants.defaultMarket.id]: testConstants.defaultMarket, | ||
}; | ||
const updatedObject: UpdatedPerpetualPositionSubaccountKafkaObject = { | ||
perpetualId: '0', | ||
maxSize: '25', | ||
|
@@ -545,7 +541,7 @@ describe('kafka-helper', () => { | |
} = getPnl( | ||
updatedObject, | ||
perpetualMarketRefresher.getPerpetualMarketsMap()[updatedObject.perpetualId], | ||
defaultMarketMap, | ||
testConstants.defaultMarket, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification There are still multiple references to
Please update these references to ensure consistency with the new approach of using Analysis chainLGTM! Ensure all references to the old market map are updated. Also applies to: 565-565, 607-607 Scripts executedThe 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) | ||
|
@@ -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) | ||
|
@@ -608,7 +604,7 @@ describe('kafka-helper', () => { | |
const updatedObjectWithPnl: UpdatedPerpetualPositionSubaccountKafkaObject = annotateWithPnl( | ||
updatedObject2, | ||
perpetualMarketRefresher.getPerpetualMarketsMap(), | ||
defaultMarketMap, | ||
testConstants.defaultMarket, | ||
); | ||
expect( | ||
updatedObjectWithPnl, | ||
|
There was a problem hiding this comment.
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:
Length of output: 66
Script:
Length of output: 74
Script:
Length of output: 2576