From 82ab2710dcd4e9f1f0179f9a01689b5f1e1b0fbf Mon Sep 17 00:00:00 2001 From: Will Liu Date: Thu, 16 May 2024 10:42:54 -0400 Subject: [PATCH 1/2] add pagination for pnl endpoint --- .../__tests__/stores/pnl-ticks-table.test.ts | 10 ++--- .../postgres/src/stores/pnl-ticks-table.ts | 37 +++++++++++++++-- indexer/scripts/deploy-commit-to-env.sh | 2 +- .../comlink/public/api-documentation.md | 14 +++++++ indexer/services/comlink/public/swagger.json | 30 ++++++++++++++ .../api/v4/historical-pnl-controller.ts | 40 ++++++++++++++++--- indexer/services/comlink/src/types.ts | 2 +- .../__tests__/tasks/create-pnl-ticks.test.ts | 11 +++-- .../src/helpers/pnl-validation-helpers.ts | 2 +- 9 files changed, 125 insertions(+), 23 deletions(-) diff --git a/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts b/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts index cf2fd12ef3..8a6fc98db8 100644 --- a/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts +++ b/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts @@ -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, @@ -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]], }); @@ -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]], }); @@ -101,7 +101,7 @@ describe('PnlTicks store', () => { }), ]); - const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll( + const { results: pnlTicks } = await PnlTicksTable.findAll( { subaccountId: [defaultSubaccountId], }, diff --git a/indexer/packages/postgres/src/stores/pnl-ticks-table.ts b/indexer/packages/postgres/src/stores/pnl-ticks-table.ts index 9c78d44eaf..2a2278f3c4 100644 --- a/indexer/packages/postgres/src/stores/pnl-ticks-table.ts +++ b/indexer/packages/postgres/src/stores/pnl-ticks-table.ts @@ -16,6 +16,7 @@ import { PnlTicksQueryConfig, QueryableField, QueryConfig, + PaginationFromDatabase, } from '../types'; export function uuid( @@ -42,10 +43,11 @@ export async function findAll( createdBeforeOrAtBlockHeight, createdOnOrAfter, createdOnOrAfterBlockHeight, + page, }: PnlTicksQueryConfig, requiredFields: QueryableField[], options: Options = DEFAULT_POSTGRES_OPTIONS, -): Promise { +): Promise> { verifyAllRequiredFields( { limit, @@ -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( diff --git a/indexer/scripts/deploy-commit-to-env.sh b/indexer/scripts/deploy-commit-to-env.sh index 0339576879..ea4a7b1d8a 100755 --- a/indexer/scripts/deploy-commit-to-env.sh +++ b/indexer/scripts/deploy-commit-to-env.sh @@ -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 "mainnet") account=332066407361;; # mainnet *) account=329916310755;; esac diff --git a/indexer/services/comlink/public/api-documentation.md b/indexer/services/comlink/public/api-documentation.md index c3bf137dd1..33fc60e7e3 100644 --- a/indexer/services/comlink/public/api-documentation.md +++ b/indexer/services/comlink/public/api-documentation.md @@ -1192,6 +1192,7 @@ 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 @@ -1199,6 +1200,9 @@ fetch('https://dydx-testnet.imperator.co/v4/historical-pnl?address=string&subacc ```json { + "pageSize": 0, + "totalResults": 0, + "offset": 0, "historicalPnl": [ { "id": "string", @@ -1277,6 +1281,7 @@ 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 @@ -1284,6 +1289,9 @@ fetch('https://dydx-testnet.imperator.co/v4/historical-pnl/parentSubaccount?addr ```json { + "pageSize": 0, + "totalResults": 0, + "offset": 0, "historicalPnl": [ { "id": "string", @@ -3772,6 +3780,9 @@ This operation does not require authentication ```json { + "pageSize": 0, + "totalResults": 0, + "offset": 0, "historicalPnl": [ { "id": "string", @@ -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 diff --git a/indexer/services/comlink/public/swagger.json b/indexer/services/comlink/public/swagger.json index 1301f9534f..34e035efa8 100644 --- a/indexer/services/comlink/public/swagger.json +++ b/indexer/services/comlink/public/swagger.json @@ -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" @@ -1953,6 +1965,15 @@ "schema": { "$ref": "#/components/schemas/IsoString" } + }, + { + "in": "query", + "name": "page", + "required": false, + "schema": { + "format": "double", + "type": "number" + } } ] } @@ -2033,6 +2054,15 @@ "schema": { "$ref": "#/components/schemas/IsoString" } + }, + { + "in": "query", + "name": "page", + "required": false, + "schema": { + "format": "double", + "type": "number" + } } ] } diff --git a/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts b/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts index 6f70c54ca8..8b9cc052dd 100644 --- a/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts @@ -2,7 +2,7 @@ import { stats } from '@dydxprotocol-indexer/base'; import { DEFAULT_POSTGRES_OPTIONS, IsoString, - Ordering, + Ordering, PaginationFromDatabase, PnlTicksFromDatabase, PnlTicksTable, QueryableField, @@ -22,7 +22,9 @@ import { NotFoundError } from '../../../lib/errors'; import { getChildSubaccountIds, handleControllerError } from '../../../lib/helpers'; import { rateLimiterMiddleware } from '../../../lib/rate-limit'; import { - CheckLimitAndCreatedBeforeOrAtAndOnOrAfterSchema, CheckParentSubaccountSchema, + CheckLimitAndCreatedBeforeOrAtAndOnOrAfterSchema, + CheckPaginationSchema, + CheckParentSubaccountSchema, CheckSubaccountSchema, } from '../../../lib/validation/schemas'; import { handleValidationErrors } from '../../../request-helpers/error-handler'; @@ -44,12 +46,20 @@ class HistoricalPnlController extends Controller { @Query() createdBeforeOrAt?: IsoString, @Query() createdOnOrAfterHeight?: number, @Query() createdOnOrAfter?: IsoString, + @Query() page?: number, ): Promise { const subaccountId: string = SubaccountTable.uuid(address, subaccountNumber); - const [subaccount, pnlTicks]: [ + const [subaccount, + { + results: pnlTicks, + limit: pageSize, + offset, + total, + }, + ]: [ SubaccountFromDatabase | undefined, - PnlTicksFromDatabase[], + PaginationFromDatabase, ] = await Promise.all([ SubaccountTable.findById( subaccountId, @@ -66,6 +76,7 @@ class HistoricalPnlController extends Controller { ? createdOnOrAfterHeight.toString() : undefined, createdOnOrAfter, + page, }, [QueryableField.LIMIT], { @@ -84,6 +95,9 @@ class HistoricalPnlController extends Controller { historicalPnl: pnlTicks.map((pnlTick: PnlTicksFromDatabase) => { return pnlTicksToResponseObject(pnlTick); }), + pageSize, + totalResults: total, + offset, }; } @@ -96,13 +110,21 @@ class HistoricalPnlController extends Controller { @Query() createdBeforeOrAt?: IsoString, @Query() createdOnOrAfterHeight?: number, @Query() createdOnOrAfter?: IsoString, + @Query() page?: number, ): Promise { const childSubaccountIds: string[] = getChildSubaccountIds(address, parentSubaccountNumber); - const [subaccounts, pnlTicks]: [ + const [subaccounts, + { + results: pnlTicks, + limit: pageSize, + offset, + total, + }, + ]: [ SubaccountFromDatabase[], - PnlTicksFromDatabase[], + PaginationFromDatabase, ] = await Promise.all([ SubaccountTable.findAll( { @@ -122,6 +144,7 @@ class HistoricalPnlController extends Controller { ? createdOnOrAfterHeight.toString() : undefined, createdOnOrAfter, + page, }, [QueryableField.LIMIT], { @@ -162,6 +185,9 @@ class HistoricalPnlController extends Controller { (pnlTick: PnlTicksFromDatabase) => { return pnlTicksToResponseObject(pnlTick); }), + pageSize, + totalResults: total, + offset, }; } } @@ -171,6 +197,7 @@ router.get( rateLimiterMiddleware(getReqRateLimiter), ...CheckSubaccountSchema, ...CheckLimitAndCreatedBeforeOrAtAndOnOrAfterSchema, + ...CheckPaginationSchema, handleValidationErrors, complianceAndGeoCheck, ExportResponseCodeStats({ controllerName }), @@ -221,6 +248,7 @@ router.get( rateLimiterMiddleware(getReqRateLimiter), ...CheckParentSubaccountSchema, ...CheckLimitAndCreatedBeforeOrAtAndOnOrAfterSchema, + ...CheckPaginationSchema, handleValidationErrors, complianceAndGeoCheck, ExportResponseCodeStats({ controllerName }), diff --git a/indexer/services/comlink/src/types.ts b/indexer/services/comlink/src/types.ts index c39fd57ae2..b99414f8bb 100644 --- a/indexer/services/comlink/src/types.ts +++ b/indexer/services/comlink/src/types.ts @@ -199,7 +199,7 @@ export interface ParentSubaccountTransferResponseObject { /* ------- PNL TICKS TYPES ------- */ -export interface HistoricalPnlResponse { +export interface HistoricalPnlResponse extends PaginationResponse { historicalPnl: PnlTicksResponseObject[], } diff --git a/indexer/services/roundtable/__tests__/tasks/create-pnl-ticks.test.ts b/indexer/services/roundtable/__tests__/tasks/create-pnl-ticks.test.ts index ad1caff4ee..08a77b425f 100644 --- a/indexer/services/roundtable/__tests__/tasks/create-pnl-ticks.test.ts +++ b/indexer/services/roundtable/__tests__/tasks/create-pnl-ticks.test.ts @@ -3,7 +3,6 @@ import { dbHelpers, OraclePriceTable, PerpetualPositionTable, - PnlTicksFromDatabase, PnlTicksTable, testConstants, testMocks, @@ -99,7 +98,7 @@ describe('create-pnl-ticks', () => { jest.spyOn(Date, 'now').mockImplementation(() => date); jest.spyOn(DateTime, 'utc').mockImplementation(() => dateTime); await createPnlTicksTask(); - const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll( + const { results: pnlTicks } = await PnlTicksTable.findAll( {}, [], {}, @@ -144,7 +143,7 @@ describe('create-pnl-ticks', () => { }), ]); await createPnlTicksTask(); - const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll( + const { results: pnlTicks } = await PnlTicksTable.findAll( {}, [], {}, @@ -196,7 +195,7 @@ describe('create-pnl-ticks', () => { }), ]); await createPnlTicksTask(); - const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll( + const { results: pnlTicks } = await PnlTicksTable.findAll( {}, [], {}, @@ -248,7 +247,7 @@ describe('create-pnl-ticks', () => { }), ]); await createPnlTicksTask(); - const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll( + const { results: pnlTicks } = await PnlTicksTable.findAll( {}, [], {}, @@ -297,7 +296,7 @@ describe('create-pnl-ticks', () => { }), ]); await createPnlTicksTask(); - const pnlTicks: PnlTicksFromDatabase[] = await PnlTicksTable.findAll( + const { results: pnlTicks } = await PnlTicksTable.findAll( {}, [], {}, diff --git a/indexer/services/scripts/src/helpers/pnl-validation-helpers.ts b/indexer/services/scripts/src/helpers/pnl-validation-helpers.ts index 82ff65131d..fdc1470bbc 100644 --- a/indexer/services/scripts/src/helpers/pnl-validation-helpers.ts +++ b/indexer/services/scripts/src/helpers/pnl-validation-helpers.ts @@ -175,7 +175,7 @@ export async function validatePnl( export async function validatePnlForSubaccount( subaccountId: string, ): Promise { - const pnlTicks: PnlTicksFromDatabase[] = await + const { results: pnlTicks } = await PnlTicksTable.findAll( { subaccountId: [subaccountId] }, [], From b3a6d5a4ea77a124c17b742bb6951e6fc8f9a637 Mon Sep 17 00:00:00 2001 From: Will Liu Date: Thu, 16 May 2024 11:12:07 -0400 Subject: [PATCH 2/2] add unit tests --- .../__tests__/stores/pnl-ticks-table.test.ts | 60 ++++++++++++++++ .../api/v4/historical-pnl-controller.test.ts | 69 +++++++++++++++++++ .../api/v4/historical-pnl-controller.ts | 2 + indexer/services/comlink/src/types.ts | 3 +- 4 files changed, 133 insertions(+), 1 deletion(-) diff --git a/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts b/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts index 8a6fc98db8..76776ee4b2 100644 --- a/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts +++ b/indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts @@ -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([ diff --git a/indexer/services/comlink/__tests__/controllers/api/v4/historical-pnl-controller.test.ts b/indexer/services/comlink/__tests__/controllers/api/v4/historical-pnl-controller.test.ts index 6436dae861..9ff5c2468c 100644 --- a/indexer/services/comlink/__tests__/controllers/api/v4/historical-pnl-controller.test.ts +++ b/indexer/services/comlink/__tests__/controllers/api/v4/historical-pnl-controller.test.ts @@ -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'; diff --git a/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts b/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts index 8b9cc052dd..a6559b05a7 100644 --- a/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts @@ -211,6 +211,7 @@ router.get( createdBeforeOrAt, createdOnOrAfterHeight, createdOnOrAfter, + page, }: PnlTicksRequest = matchedData(req) as PnlTicksRequest; try { @@ -223,6 +224,7 @@ router.get( createdBeforeOrAt, createdOnOrAfterHeight, createdOnOrAfter, + page, ); return res.send(response); diff --git a/indexer/services/comlink/src/types.ts b/indexer/services/comlink/src/types.ts index b99414f8bb..5a3a5b505a 100644 --- a/indexer/services/comlink/src/types.ts +++ b/indexer/services/comlink/src/types.ts @@ -425,7 +425,8 @@ export interface TradeRequest extends LimitAndCreatedBeforeRequest, PaginationRe export interface PerpetualMarketRequest extends LimitRequest, TickerRequest {} -export interface PnlTicksRequest extends SubaccountRequest, LimitAndCreatedBeforeAndAfterRequest {} +export interface PnlTicksRequest + extends SubaccountRequest, LimitAndCreatedBeforeAndAfterRequest, PaginationRequest {} export interface ParentSubaccountPnlTicksRequest extends ParentSubaccountRequest, LimitAndCreatedBeforeAndAfterRequest {