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-808] Add pagination for pnl endpoint #1529

Merged
merged 2 commits into from
May 17, 2024
Merged

[CT-808] Add pagination for pnl endpoint #1529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82ab271
add pagination for pnl endpoint
dydxwill May 16, 2024
b3a6d5a
add unit tests
dydxwill May 16, 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
70 changes: 65 additions & 5 deletions indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import {
Ordering,
PnlTicksColumns,
PnlTicksCreateObject,
PnlTicksFromDatabase,
} from '../../src/types';
import * as PnlTicksTable from '../../src/stores/pnl-ticks-table';
import * as BlockTable from '../../src/stores/block-table';
import { clearData, migrate, teardown } from '../../src/helpers/db-helpers';
import { seedData } from '../helpers/mock-generators';
import {
defaultBlock, defaultBlock2,
defaultBlock,
defaultBlock2,
defaultPnlTick,
defaultSubaccountId,
defaultSubaccountId2,
Expand Down Expand Up @@ -58,7 +58,7 @@ describe('PnlTicks store', () => {
PnlTicksTable.create(pnlTick2),
]);

const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll({}, [], {
const { results: pnlTicks } = await PnlTicksTable.findAll({}, [], {
orderBy: [[PnlTicksColumns.blockHeight, Ordering.ASC]],
});

Expand All @@ -78,7 +78,7 @@ describe('PnlTicks store', () => {
blockTime: defaultBlock.time,
};
await PnlTicksTable.createMany([defaultPnlTick, pnlTick2]);
const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll({}, [], {
const { results: pnlTicks } = await PnlTicksTable.findAll({}, [], {
orderBy: [[PnlTicksColumns.blockHeight, Ordering.ASC]],
});

Expand All @@ -101,7 +101,7 @@ describe('PnlTicks store', () => {
}),
]);

const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll(
const { results: pnlTicks } = await PnlTicksTable.findAll(
{
subaccountId: [defaultSubaccountId],
},
Expand All @@ -112,6 +112,66 @@ describe('PnlTicks store', () => {
expect(pnlTicks.length).toEqual(2);
});

it('Successfully finds PnlTicks using pagination', async () => {
const blockTime: IsoString = '2023-01-01T00:00:00.000Z';
await Promise.all([
PnlTicksTable.create(defaultPnlTick),
PnlTicksTable.create({
...defaultPnlTick,
createdAt: '2020-01-01T00:00:00.000Z',
blockHeight: '1000',
blockTime,
}),
]);

const responsePageOne = await PnlTicksTable.findAll({
page: 1,
limit: 1,
}, [], {
orderBy: [[PnlTicksColumns.blockHeight, Ordering.DESC]],
});

expect(responsePageOne.results.length).toEqual(1);
expect(responsePageOne.results[0]).toEqual(expect.objectContaining({
...defaultPnlTick,
createdAt: '2020-01-01T00:00:00.000Z',
blockHeight: '1000',
blockTime,
}));
expect(responsePageOne.offset).toEqual(0);
expect(responsePageOne.total).toEqual(2);

const responsePageTwo = await PnlTicksTable.findAll({
page: 2,
limit: 1,
}, [], {
orderBy: [[PnlTicksColumns.blockHeight, Ordering.DESC]],
});

expect(responsePageTwo.results.length).toEqual(1);
expect(responsePageTwo.results[0]).toEqual(expect.objectContaining(defaultPnlTick));
expect(responsePageTwo.offset).toEqual(1);
expect(responsePageTwo.total).toEqual(2);

const responsePageAllPages = await PnlTicksTable.findAll({
page: 1,
limit: 2,
}, [], {
orderBy: [[PnlTicksColumns.blockHeight, Ordering.DESC]],
});

expect(responsePageAllPages.results.length).toEqual(2);
expect(responsePageAllPages.results[0]).toEqual(expect.objectContaining({
...defaultPnlTick,
createdAt: '2020-01-01T00:00:00.000Z',
blockHeight: '1000',
blockTime,
}));
expect(responsePageAllPages.results[1]).toEqual(expect.objectContaining(defaultPnlTick));
expect(responsePageAllPages.offset).toEqual(0);
expect(responsePageAllPages.total).toEqual(2);
});

it('Successfully finds latest block time', async () => {
const blockTime: IsoString = '2023-01-01T00:00:00.000Z';
await Promise.all([
Expand Down
37 changes: 34 additions & 3 deletions indexer/packages/postgres/src/stores/pnl-ticks-table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
PnlTicksQueryConfig,
QueryableField,
QueryConfig,
PaginationFromDatabase,
} from '../types';

export function uuid(
Expand All @@ -42,10 +43,11 @@ export async function findAll(
createdBeforeOrAtBlockHeight,
createdOnOrAfter,
createdOnOrAfterBlockHeight,
page,
}: PnlTicksQueryConfig,
requiredFields: QueryableField[],
options: Options = DEFAULT_POSTGRES_OPTIONS,
): Promise<PnlTicksFromDatabase[]> {
): Promise<PaginationFromDatabase<PnlTicksFromDatabase>> {
Comment on lines +46 to +50
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

Several files in the codebase still use the old function signature for PnlTicksTable.findAll.

  • Ensure that all instances of PnlTicksTable.findAll are updated to handle pagination correctly.

Locations to update:

  • validatePnlForSubaccount function
  • Test cases involving PnlTicksTable.findAll
  • Any other instances where PnlTicksTable.findAll is called without pagination parameters
Analysis chain

LGTM! Ensure pagination is handled correctly in the calling code.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify pagination handling in the calling code.

# Test: Search for the function usage. Expect: Proper handling of pagination.
rg --type typescript $'findAll'

Length of output: 65

Script:

#!/bin/bash
# Description: Verify pagination handling in the calling code.

# Find all TypeScript files and search for the `findAll` method usage within those files.
fd -e ts -e tsx -x rg 'findAll' {}

Length of output: 20249

Script:

#!/bin/bash
# Description: Extract context around the `PnlTicksTable.findAll` calls to verify pagination handling.

# Extract 5 lines before and after each `PnlTicksTable.findAll` call.
fd -e ts -e tsx -x rg -C 5 'PnlTicksTable.findAll' {}

Length of output: 3955

verifyAllRequiredFields(
{
limit,
Expand Down Expand Up @@ -128,11 +130,40 @@ export async function findAll(
);
}

if (limit !== undefined) {
if (limit !== undefined && page === undefined) {
baseQuery = baseQuery.limit(limit);
}

return baseQuery.returning('*');
/**
* If a query is made using a page number, then the limit property is used as 'page limit'
*/
if (page !== undefined && limit !== undefined) {
/**
* We make sure that the page number is always >= 1
*/
const currentPage: number = Math.max(1, page);
const offset: number = (currentPage - 1) * limit;

/**
* Ensure sorting is applied to maintain consistent pagination results.
* Also a casting of the ts type is required since the infer of the type
* obtained from the count is not performed.
*/
const count: { count?: string } = await baseQuery.clone().clearOrder().count({ count: '*' }).first() as unknown as { count?: string };

baseQuery = baseQuery.offset(offset).limit(limit);

return {
results: await baseQuery.returning('*'),
limit,
offset,
total: parseInt(count.count ?? '0', 10),
};
}

return {
results: await baseQuery.returning('*'),
};
}

export async function create(
Expand Down
2 changes: 1 addition & 1 deletion indexer/scripts/deploy-commit-to-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ case $env in
"dev4") account=525975847385;;
"dev5") account=917958511744;;
"staging") account=677285201534;;
"public-testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for cd commands.

- cd services/$service
+ cd services/$service || exit
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Double quote to prevent globbing and word splitting.

- cd services/$service
+ cd "services/$service"
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Use a subshell to avoid having to cd back.

- cd services/$service
- pnpm build
- cd -
+ (cd "services/$service" && pnpm build)
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Double quote to prevent globbing and word splitting.

- AWS_PROFILE=dydx-v4-$env aws ecr get-login-password --region ap-northeast-1 | docker login --username AWS --password-stdin $account.dkr.ecr.ap-northeast-1.amazonaws.com
+ AWS_PROFILE="dydx-v4-$env" aws ecr get-login-password --region ap-northeast-1 | docker login --username AWS --password-stdin "$account.dkr.ecr.ap-northeast-1.amazonaws.com"
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
AWS_PROFILE="dydx-v4-$env" aws ecr get-login-password --region ap-northeast-1 | docker login --username AWS --password-stdin "$account.dkr.ecr.ap-northeast-1.amazonaws.com"

Double quote to prevent globbing and word splitting.

- DOCKER_BUILDKIT=1 docker build \
-             --platform linux/amd64 \
-             -t $account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag \
-             -f $dockerfile \
-             --build-arg service=$service \
-             --build-arg NPM_TOKEN=$NPM_TOKEN .
+ DOCKER_BUILDKIT=1 docker build \
+             --platform linux/amd64 \
+             -t "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag" \
+             -f "$dockerfile" \
+             --build-arg service="$service" \
+             --build-arg NPM_TOKEN="$NPM_TOKEN" .
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
DOCKER_BUILDKIT=1 docker build \
--platform linux/amd64 \
-t "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag" \
-f "$dockerfile" \
--build-arg service="$service" \
--build-arg NPM_TOKEN="$NPM_TOKEN" .

Double quote to prevent globbing and word splitting.

- docker push $account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag
+ docker push "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag"
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
docker push "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag"

Double quote to prevent globbing and word splitting.

- AWS_PROFILE=dydx-v4-$env orb deploy $service $tag
+ AWS_PROFILE="dydx-v4-$env" orb deploy "$service" "$tag"
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
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

"mainnet") account=332066407361;; # mainnet
*) account=329916310755;;
esac
Expand Down
69 changes: 69 additions & 0 deletions indexer/services/comlink/__tests__/controllers/api/v4/historical-pnl-controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,75 @@ describe('pnlTicks-controller#V4', () => {
);
});

it('Get /historical-pnl respects pagination', async () => {
await testMocks.seedData();
const createdAt: string = '2000-05-25T00:00:00.000Z';
const blockHeight: string = '1';
const pnlTick2: PnlTicksCreateObject = {
...testConstants.defaultPnlTick,
createdAt,
blockHeight,
};
await Promise.all([
PnlTicksTable.create(testConstants.defaultPnlTick),
PnlTicksTable.create(pnlTick2),
]);

const responsePage1: request.Response = await sendRequest({
type: RequestMethod.GET,
path: `/v4/historical-pnl?address=${testConstants.defaultAddress}` +
`&subaccountNumber=${testConstants.defaultSubaccount.subaccountNumber}&page=1&limit=1`,
});

const responsePage2: request.Response = await sendRequest({
type: RequestMethod.GET,
path: `/v4/historical-pnl?address=${testConstants.defaultAddress}` +
`&subaccountNumber=${testConstants.defaultSubaccount.subaccountNumber}&page=2&limit=1`,
});

const expectedPnlTickResponse: PnlTicksResponseObject = {
...testConstants.defaultPnlTick,
id: PnlTicksTable.uuid(
testConstants.defaultPnlTick.subaccountId,
testConstants.defaultPnlTick.createdAt,
),
};

const expectedPnlTick2Response: PnlTicksResponseObject = {
...testConstants.defaultPnlTick,
createdAt,
blockHeight,
id: PnlTicksTable.uuid(
testConstants.defaultPnlTick.subaccountId,
createdAt,
),
};

expect(responsePage1.body.pageSize).toStrictEqual(1);
expect(responsePage1.body.offset).toStrictEqual(0);
expect(responsePage1.body.totalResults).toStrictEqual(2);
expect(responsePage1.body.historicalPnl).toHaveLength(1);
expect(responsePage1.body.historicalPnl).toEqual(
expect.arrayContaining([
expect.objectContaining({
...expectedPnlTickResponse,
}),
]),
);

expect(responsePage2.body.pageSize).toStrictEqual(1);
expect(responsePage2.body.offset).toStrictEqual(1);
expect(responsePage2.body.totalResults).toStrictEqual(2);
expect(responsePage2.body.historicalPnl).toHaveLength(1);
expect(responsePage2.body.historicalPnl).toEqual(
expect.arrayContaining([
expect.objectContaining({
...expectedPnlTick2Response,
}),
]),
);
});

it('Get /historical-pnl respects createdBeforeOrAt and createdBeforeOrAtHeight field', async () => {
await testMocks.seedData();
const createdAt: string = '2000-05-25T00:00:00.000Z';
Expand Down
14 changes: 14 additions & 0 deletions indexer/services/comlink/public/api-documentation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1192,13 +1192,17 @@ fetch('https://dydx-testnet.imperator.co/v4/historical-pnl?address=string&subacc
|createdBeforeOrAt|query|[IsoString](#schemaisostring)|false|none|
|createdOnOrAfterHeight|query|number(double)|false|none|
|createdOnOrAfter|query|[IsoString](#schemaisostring)|false|none|
|page|query|number(double)|false|none|

> Example responses

> 200 Response

```json
{
"pageSize": 0,
"totalResults": 0,
"offset": 0,
"historicalPnl": [
{
"id": "string",
Expand Down Expand Up @@ -1277,13 +1281,17 @@ fetch('https://dydx-testnet.imperator.co/v4/historical-pnl/parentSubaccount?addr
|createdBeforeOrAt|query|[IsoString](#schemaisostring)|false|none|
|createdOnOrAfterHeight|query|number(double)|false|none|
|createdOnOrAfter|query|[IsoString](#schemaisostring)|false|none|
|page|query|number(double)|false|none|

> Example responses

> 200 Response

```json
{
"pageSize": 0,
"totalResults": 0,
"offset": 0,
"historicalPnl": [
{
"id": "string",
Expand Down Expand Up @@ -3772,6 +3780,9 @@ This operation does not require authentication

```json
{
"pageSize": 0,
"totalResults": 0,
"offset": 0,
"historicalPnl": [
{
"id": "string",
Expand All @@ -3792,6 +3803,9 @@ This operation does not require authentication

|Name|Type|Required|Restrictions|Description|
|---|---|---|---|---|
|pageSize|number(double)|false|none|none|
|totalResults|number(double)|false|none|none|
|offset|number(double)|false|none|none|
|historicalPnl|[[PnlTicksResponseObject](#schemapnlticksresponseobject)]|true|none|none|

## TradingRewardAggregationPeriod
Expand Down
30 changes: 30 additions & 0 deletions indexer/services/comlink/public/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,18 @@
},
"HistoricalPnlResponse": {
"properties": {
"pageSize": {
"type": "number",
"format": "double"
},
"totalResults": {
"type": "number",
"format": "double"
},
"offset": {
"type": "number",
"format": "double"
},
"historicalPnl": {
"items": {
"$ref": "#/components/schemas/PnlTicksResponseObject"
Expand Down Expand Up @@ -1953,6 +1965,15 @@
"schema": {
"$ref": "#/components/schemas/IsoString"
}
},
{
"in": "query",
"name": "page",
"required": false,
"schema": {
"format": "double",
"type": "number"
}
}
]
}
Expand Down Expand Up @@ -2033,6 +2054,15 @@
"schema": {
"$ref": "#/components/schemas/IsoString"
}
},
{
"in": "query",
"name": "page",
"required": false,
"schema": {
"format": "double",
"type": "number"
}
}
]
}
Expand Down
Loading
Loading