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

[CT-970] Add debug logs to roundtable task, fix case where marketId!=perpetualId #1786

Merged
merged 15 commits into from
Jun 27, 2024
Merged

[CT-970] Add debug logs to roundtable task, fix case where marketId!=perpetualId #1786

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
da22485
add logs
dydxwill Jun 26, 2024
418b451
gh wf
dydxwill Jun 26, 2024
52e3a2e
add more logs
dydxwill Jun 26, 2024
ab15341
add more dbg logs
dydxwill Jun 26, 2024
87c1707
add script
dydxwill Jun 26, 2024
db7e0a7
found root cause
dydxwill Jun 26, 2024
18f06e1
update wf
dydxwill Jun 26, 2024
012bd52
rm unneeded files
dydxwill Jun 26, 2024
03f2f70
fix
dydxwill Jun 26, 2024
4e24619
update gh wf
dydxwill Jun 26, 2024
268c89f
fix
dydxwill Jun 26, 2024
8b51353
test
dydxwill Jun 26, 2024
ecfe97f
revert change
dydxwill Jun 26, 2024
02749a1
test
dydxwill Jun 27, 2024
3a81e2b
logs
dydxwill Jun 27, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/indexer-build-and-push-mainnet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Indexer Build & Push Images to AWS ECR for Mainnet
on: # yamllint disable-line rule:truthy
push:
branches:
- 'wl/pnldbg2'
- main
- 'release/indexer/v[0-9]+.[0-9]+.x' # e.g. release/indexer/v0.1.x
- 'release/indexer/v[0-9]+.x' # e.g. release/indexer/v1.x
Expand Down
68 changes: 68 additions & 0 deletions indexer/services/roundtable/__tests__/helpers/pnl-ticks-helper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ import {
Transaction,
TransferTable,
PositionSide,
PerpetualMarketStatus,
PerpetualMarketType,
MarketTable,
PerpetualMarketTable,
perpetualMarketRefresher,
} from '@dydxprotocol-indexer/postgres';
import {
calculateEquity,
Expand Down Expand Up @@ -52,10 +57,12 @@ describe('pnl-ticks-helper', () => {
const lastUpdatedFundingIndexMap: FundingIndexMap = {
[testConstants.defaultPerpetualMarket.id]: Big('10050'),
[testConstants.defaultPerpetualMarket2.id]: Big('5'),
5: Big('5'),
};
const currentFundingIndexMap: FundingIndexMap = {
[testConstants.defaultPerpetualMarket.id]: Big('11000'),
[testConstants.defaultPerpetualMarket2.id]: Big('8'),
5: Big('8'),
};
const marketPrices: PriceMap = {
[testConstants.defaultPerpetualMarket.id]: '20000',
Expand All @@ -73,6 +80,7 @@ describe('pnl-ticks-helper', () => {

beforeEach(async () => {
await testMocks.seedData();
await perpetualMarketRefresher.updatePerpetualMarkets();
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

Ensure isolation of market updates in test setup.

The updatePerpetualMarkets function is called in the test files but is not explicitly mocked. To prevent actual database or network interactions during tests, ensure that this function is properly mocked.

  • indexer/services/roundtable/__tests__/helpers/pnl-ticks-helper.test.ts
  • indexer/services/roundtable/__tests__/tasks/remove-expired-orders.test.ts
Analysis chain

Ensure isolation of market updates in test setup.

Calling updatePerpetualMarkets() in the beforeEach hook is crucial for setting a consistent state. However, ensure this is properly mocked to prevent actual database or network interactions during tests.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that `updatePerpetualMarkets` is mocked in tests to prevent real interactions.
rg --type py "updatePerpetualMarkets" "*/__tests__/*"

Length of output: 107

Script:

#!/bin/bash
# Verify that `updatePerpetualMarkets` is mocked in tests to prevent real interactions.
rg "updatePerpetualMarkets" "*/__tests__/*"

Length of output: 97

Script:

#!/bin/bash
# Find all TypeScript test files and search for `updatePerpetualMarkets` to verify if it is mocked.
fd --type f --extension ts --search-path "indexer/services/roundtable/__tests__" | xargs rg "updatePerpetualMarkets"

Length of output: 509

Script:

#!/bin/bash
# Verify if `updatePerpetualMarkets` is mocked in the identified test files.
rg "updatePerpetualMarkets" --context 10 "indexer/services/roundtable/__tests__/tasks/remove-expired-orders.test.ts" "indexer/services/roundtable/__tests__/helpers/pnl-ticks-helper.test.ts"

Length of output: 6610

});

afterAll(async () => {
Expand Down Expand Up @@ -348,6 +356,66 @@ describe('pnl-ticks-helper', () => {
expect(equity).toEqual(new Big('190530'));
});

it('calculateEquity with perpetualId/marketId mismatch', async () => {
await MarketTable.create({
id: 1000,
pair: 'TEST-USD',
exponent: -12,
minPriceChangePpm: 50,
oraclePrice: '1.00',
});
await PerpetualMarketTable.create({
id: '5',
clobPairId: '5',
ticker: 'TEST-USD',
marketId: 1000,
status: PerpetualMarketStatus.ACTIVE,
priceChange24H: '0.000000001',
volume24H: '10000000',
trades24H: 200,
nextFundingRate: '1.2',
openInterest: '40000',
quantumConversionExponent: -16,
atomicResolution: -2,
subticksPerTick: 10,
stepBaseQuantums: 1,
liquidityTierId: 0,
marketType: PerpetualMarketType.ISOLATED,
baseOpenInterest: '100000',
});
await perpetualMarketRefresher.updatePerpetualMarkets();
const positions2: PerpetualPositionFromDatabase[] = [
...positions,
{
...testConstants.defaultPerpetualPosition,
side: PositionSide.SHORT,
perpetualId: '5',
entryPrice: '20000',
sumOpen: '10',
size: '-10',
sumClose: '0',
openEventId: testConstants.defaultTendermintEventId2,
id: PerpetualPositionTable.uuid(
testConstants.defaultPerpetualPosition.subaccountId,
testConstants.defaultTendermintEventId2,
),
},
];
const marketPricesMissingPrice: PriceMap = {
[testConstants.defaultPerpetualMarket.id]: '20000',
1000: '1000',
};
const usdcPosition: Big = new Big('10000');
const equity: Big = calculateEquity(
usdcPosition,
positions2,
marketPricesMissingPrice,
lastUpdatedFundingIndexMap,
currentFundingIndexMap,
);
expect(equity).toEqual(new Big('190530'));
});

it('calculateTotalPnl', () => {
const equity: Big = new Big('200100');
const transfers: string = '-20.5';
Expand Down
66 changes: 61 additions & 5 deletions indexer/services/roundtable/src/helpers/pnl-ticks-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
helpers,
IsoString,
OraclePriceTable,
perpetualMarketRefresher,
PerpetualPositionFromDatabase,
PerpetualPositionTable,
PnlTicksCreateObject,
Expand Down Expand Up @@ -73,15 +74,32 @@ export async function getPnlTicksCreateObjects(
mostRecentPnlTicks,
(pnlTick: PnlTicksCreateObject) => pnlTick.blockTime,
);
logger.info({
at: 'pnl-ticks-helper#getPnlTicksCreateObjects',
message: 'Account to last updated block time',
accountToLastUpdatedBlockTime,
});
const subaccountIdsWithTranfers: string[] = _.map(subaccountsWithTransfers, 'id');
const newSubaccountIds: string[] = _.difference(
subaccountIdsWithTranfers, _.keys(accountToLastUpdatedBlockTime),
);
logger.info({
at: 'pnl-ticks-helper#getPnlTicksCreateObjects',
message: 'New subaccounts with transfers',
newSubaccountIds,
});
// get accounts to update based on last updated block height
const accountsToUpdate: string[] = [
...getAccountsToUpdate(accountToLastUpdatedBlockTime, blockTime),
...newSubaccountIds,
].slice(0, config.PNL_TICK_MAX_ACCOUNTS_PER_RUN);
logger.info({
at: 'pnl-ticks-helper#getPnlTicksCreateObjects',
message: 'Accounts to update',
accountsToUpdate,
blockHeight,
blockTime,
});
stats.gauge(
`${config.SERVICE_NAME}_get_ticks_accounts_to_update`,
accountsToUpdate.length,
Expand Down Expand Up @@ -176,13 +194,19 @@ export async function getPnlTicksCreateObjects(
logger.error({
at: 'pnl-ticks-helper#getPnlTicksCreateObjects',
message: 'Error when getting new pnl tick',
error,
account,
pnlTicksToBeCreatedAt,
blockHeight,
blockTime,
});
}
});
logger.info({
at: 'pnl-ticks-helper#getPnlTicksCreateObjects',
message: 'New ticks to create',
subaccountIds: _.map(newTicksToCreate, 'subaccountId'),
});
Comment on lines +192 to +204
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure error handling includes necessary details for debugging.

The error logging within getPnlTicksCreateObjects should include all relevant information to diagnose issues effectively. Consider adding more context if needed.

-        error,
-        account,
-        pnlTicksToBeCreatedAt,
-        blockHeight,
-        blockTime,
+        error: error.message,
+        account: account.id,  # Assuming `account` is an object
+        pnlTicksToBeCreatedAt: pnlTicksToBeCreatedAt.toISO(),
+        blockHeight: blockHeight.toString(),
+        blockTime: blockTime.toString(),

Committable suggestion was skipped due to low confidence.

stats.timing(
`${config.SERVICE_NAME}_get_ticks_compute_pnl`,
new Date().getTime() - computePnlStart,
Expand Down Expand Up @@ -290,24 +314,49 @@ export function getNewPnlTick(
lastUpdatedFundingIndexMap: FundingIndexMap,
currentFundingIndexMap: FundingIndexMap,
): PnlTicksCreateObject {
logger.info({
at: 'createPnlTicks#getNewPnlTick',
message: 'Creating new PNL tick',
subaccountId,
latestBlockHeight,
latestBlockTime,
// usdcPositionSize,
// openPerpetualPositionsForSubaccount: JSON.stringify(openPerpetualPositionsForSubaccount),
// marketPrices: JSON.stringify(marketPrices),
// lastUpdatedFundingIndexMap: JSON.stringify(lastUpdatedFundingIndexMap),
// currentFundingIndexMap: JSON.stringify(currentFundingIndexMap),
// subaccountTotalTransfersMap: JSON.stringify(subaccountTotalTransfersMap),
});
const currentEquity: Big = calculateEquity(
usdcPositionSize,
openPerpetualPositionsForSubaccount,
marketPrices,
lastUpdatedFundingIndexMap,
currentFundingIndexMap,
);
logger.info({
at: 'createPnlTicks#getNewPnlTick',
message: 'Calculated equity and total pnl',
subaccountId,
currentEquity: currentEquity.toFixed(),
});

const totalPnl: Big = calculateTotalPnl(
currentEquity,
subaccountTotalTransfersMap[subaccountId][USDC_ASSET_ID],
);
logger.info({
at: 'createPnlTicks#getNewPnlTick',
message: 'Calculated equity and total pnl',
subaccountId,
totalPnl: totalPnl.toFixed(),
});

const mostRecentPnlTick: PnlTicksCreateObject | undefined = mostRecentPnlTicks[subaccountId];

// if there has been a significant chagne in equity or totalPnl, log it for debugging purposes.
if (
mostRecentPnlTick &&
mostRecentPnlTick !== undefined &&
Big(mostRecentPnlTick.equity).gt(0) &&
currentEquity.div(mostRecentPnlTick.equity).gt(2) &&
totalPnl.gte(10000) &&
Expand All @@ -328,7 +377,7 @@ export function getNewPnlTick(
});
}

return {
const pnlTick: PnlTicksCreateObject = {
totalPnl: totalPnl.toFixed(6),
netTransfers: usdcNetTransfersSinceLastPnlTick.toFixed(6),
subaccountId,
Expand All @@ -337,6 +386,13 @@ export function getNewPnlTick(
blockHeight: latestBlockHeight,
blockTime: latestBlockTime,
};
logger.info({
at: 'createPnlTicks#getNewPnlTick',
message: 'New PNL tick',
subaccountId,
pnlTick,
});
return pnlTick;
}

/**
Expand Down Expand Up @@ -404,9 +460,9 @@ export function calculateEquity(

const signedPositionNotional: Big = positions.reduce(
(acc: Big, position: PerpetualPositionFromDatabase) => {
const positionNotional: Big = Big(position.size).times(
marketPrices[Number(position.perpetualId)],
);
const marketId:
number = perpetualMarketRefresher.getPerpetualMarketFromId(position.perpetualId)!.marketId;
const positionNotional: Big = Big(position.size).times(Big(marketPrices[marketId]));
// Add positionNotional to the accumulator
return acc.plus(positionNotional);
},
Expand Down
2 changes: 2 additions & 0 deletions indexer/services/roundtable/src/tasks/create-pnl-ticks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
PnlTicksCreateObject,
PnlTicksTable,
Transaction,
perpetualMarketRefresher,
} from '@dydxprotocol-indexer/postgres';
import { LatestAccountPnlTicksCache } from '@dydxprotocol-indexer/redis';
import _ from 'lodash';
Expand Down Expand Up @@ -53,6 +54,7 @@ export default async function runTask(): Promise<void> {
const txId: number = await Transaction.start();
let newTicksToCreate: PnlTicksCreateObject[] = [];
try {
await perpetualMarketRefresher.updatePerpetualMarkets();
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

Ensure proper error handling and transaction management when updating perpetual markets.

The function updatePerpetualMarkets does not perform any write operations that could affect the transaction's atomicity. However, consider wrapping the call to updatePerpetualMarkets in a try-catch block to handle potential exceptions explicitly and maintain clean transaction boundaries.

  • File: indexer/services/roundtable/src/tasks/create-pnl-ticks.ts
  • Line: 57
Analysis chain

Ensure proper error handling and transaction management when updating perpetual markets.

The addition of perpetualMarketRefresher.updatePerpetualMarkets() within a transaction is a critical update. Ensure that this method does not have side effects that could affect the transaction's atomicity.

Consider wrapping the call to updatePerpetualMarkets() in a try-catch block to handle potential exceptions explicitly and maintain clean transaction boundaries.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that `updatePerpetualMarkets` does not perform any write operations that could affect the transaction's integrity.
ast-grep --lang typescript --pattern $'async function updatePerpetualMarkets() {
  $$$
}'

Length of output: 90

Script:

#!/bin/bash
# Searching for the definition of `updatePerpetualMarkets` to inspect its implementation
rg 'updatePerpetualMarkets' --glob '*.{ts,js}'

Length of output: 9773

Script:

#!/bin/bash
# Extracting the implementation of `updatePerpetualMarkets` from the identified file
ast-grep --lang typescript --pattern $'export async function updatePerpetualMarkets($_): Promise<void> {
  $$$
}' indexer/packages/postgres/src/loops/perpetual-market-refresher.ts

Length of output: 2064

newTicksToCreate = await getPnlTicksCreateObjects(latestBlockHeight, latestBlockTime, txId);
} catch (error) {
logger.error({
Expand Down
Loading