From 1099fdf5b9752e0f9491880549a2e8697c04e0e0 Mon Sep 17 00:00:00 2001 From: Shrenuj Bansal Date: Thu, 16 Jan 2025 15:33:34 -0500 Subject: [PATCH 1/4] Add bech32 schema checks for addresses in url endpoints Signed-off-by: Shrenuj Bansal --- .../postgres/__tests__/helpers/constants.ts | 2 +- indexer/pnpm-lock.yaml | 2 ++ .../api/v4/addresses-controller.test.ts | 30 ++++++++--------- indexer/services/comlink/package.json | 1 + .../comlink/src/lib/validation/schemas.ts | 32 +++++++++++++++++++ 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/indexer/packages/postgres/__tests__/helpers/constants.ts b/indexer/packages/postgres/__tests__/helpers/constants.ts index aeaedeae3c9..2f1df758197 100644 --- a/indexer/packages/postgres/__tests__/helpers/constants.ts +++ b/indexer/packages/postgres/__tests__/helpers/constants.ts @@ -78,7 +78,7 @@ export const blockedAddress: string = 'dydx1f9k5qldwmqrnwy8hcgp4fw6heuvszt35egvt export const vaultAddress: string = 'dydx1c0m5x87llaunl5sgv3q5vd7j5uha26d2q2r2q0'; // ============== Subaccounts ============== -export const defaultWalletAddress: string = 'defaultWalletAddress'; +export const defaultWalletAddress: string = 'dydx199tqg4wdlnu4qjlxchpd7seg454937hjrknju4'; export const defaultSubaccount: SubaccountCreateObject = { address: defaultAddress, diff --git a/indexer/pnpm-lock.yaml b/indexer/pnpm-lock.yaml index dd450739665..5d1e3613d20 100644 --- a/indexer/pnpm-lock.yaml +++ b/indexer/pnpm-lock.yaml @@ -470,6 +470,7 @@ importers: '@types/response-time': ^2.3.5 '@types/supertest': ^2.0.12 '@types/swagger-ui-express': ^4.1.3 + bech32: 1.1.4 big.js: ^6.2.1 binary-searching: ^2.0.5 body-parser: ^1.20.0 @@ -510,6 +511,7 @@ importers: '@dydxprotocol-indexer/v4-protos': link:../../packages/v4-protos '@keplr-wallet/cosmos': 0.12.122 '@tsoa/runtime': 5.0.0 + bech32: 1.1.4 big.js: 6.2.1 binary-searching: 2.0.5 body-parser: 1.20.0 diff --git a/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts b/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts index aeed6ffb785..dc3bac60a91 100644 --- a/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts +++ b/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts @@ -22,6 +22,7 @@ import { Secp256k1 } from '@cosmjs/crypto'; import { toBech32 } from '@cosmjs/encoding'; import { DateTime } from 'luxon'; import { verifyADR36Amino } from '@keplr-wallet/cosmos'; +import { defaultAddress3 } from '@dydxprotocol-indexer/postgres/build/__tests__/helpers/constants'; jest.mock('@cosmjs/crypto', () => ({ ...jest.requireActual('@cosmjs/crypto'), @@ -211,7 +212,7 @@ describe('addresses-controller#V4', () => { it('Get / with non-existent address and subaccount number returns 404', async () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, - path: `/v4/addresses/${invalidAddress}/subaccountNumber/` + + path: `/v4/addresses/${defaultAddress3}/subaccountNumber/` + `${testConstants.defaultSubaccount.subaccountNumber}`, expectedStatus: 404, }); @@ -219,7 +220,7 @@ describe('addresses-controller#V4', () => { expect(response.body).toEqual({ errors: [ { - msg: `No subaccount found with address ${invalidAddress} and ` + + msg: `No subaccount found with address ${defaultAddress3} and ` + `subaccountNumber ${testConstants.defaultSubaccount.subaccountNumber}`, }, ], @@ -405,21 +406,19 @@ describe('addresses-controller#V4', () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, path: `/v4/addresses/${invalidAddress}`, - expectedStatus: 404, + expectedStatus: 400, }); expect(response.body).toEqual({ errors: [ { - msg: `No subaccounts found for address ${invalidAddress}`, + msg: 'address must be a valid dydx address', + location: 'params', + param: 'address', + value: 'invalidAddress', }, ], }); - expect(stats.increment).toHaveBeenCalledWith('comlink.addresses-controller.response_status_code.404', 1, - { - path: '/:address', - method: 'GET', - }); }); }); @@ -561,7 +560,7 @@ describe('addresses-controller#V4', () => { it('Get /:address/parentSubaccountNumber/ with non-existent address returns 404', async () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, - path: `/v4/addresses/${invalidAddress}/parentSubaccountNumber/` + + path: `/v4/addresses/${defaultAddress3}/parentSubaccountNumber/` + `${testConstants.defaultSubaccount.subaccountNumber}`, expectedStatus: 404, }); @@ -569,7 +568,7 @@ describe('addresses-controller#V4', () => { expect(response.body).toEqual({ errors: [ { - msg: `No subaccounts found for address ${invalidAddress} and ` + + msg: `No subaccounts found for address ${defaultAddress3} and ` + `parentSubaccountNumber ${testConstants.defaultSubaccount.subaccountNumber}`, }, ], @@ -737,14 +736,13 @@ describe('addresses-controller#V4', () => { expect(response.body).toEqual({ errors: [ { - msg: 'Address invalidAddress is not a valid dYdX V4 address', + msg: 'address must be a valid dydx address', + location: 'params', + param: 'address', + value: 'invalidAddress', }, ], }); - expect(statsSpy).toHaveBeenCalledWith('comlink.addresses-controller.response_status_code.400', 1, { - path: '/:address/registerToken', - method: 'POST', - }); }); it.each([ diff --git a/indexer/services/comlink/package.json b/indexer/services/comlink/package.json index 448ac57866c..fa92b9548b7 100644 --- a/indexer/services/comlink/package.json +++ b/indexer/services/comlink/package.json @@ -35,6 +35,7 @@ "@dydxprotocol-indexer/v4-protos": "workspace:^0.0.1", "@keplr-wallet/cosmos": "^0.12.122", "@tsoa/runtime": "^5.0.0", + "bech32": "1.1.4", "big.js": "^6.2.1", "binary-searching": "^2.0.5", "body-parser": "^1.20.0", diff --git a/indexer/services/comlink/src/lib/validation/schemas.ts b/indexer/services/comlink/src/lib/validation/schemas.ts index 98e1d2ed7e0..68df7664d10 100644 --- a/indexer/services/comlink/src/lib/validation/schemas.ts +++ b/indexer/services/comlink/src/lib/validation/schemas.ts @@ -1,9 +1,12 @@ +import * as console from 'node:console'; + import { isValidLanguageCode } from '@dydxprotocol-indexer/notifications'; import { perpetualMarketRefresher, MAX_PARENT_SUBACCOUNTS, CHILD_SUBACCOUNT_MULTIPLIER, } from '@dydxprotocol-indexer/postgres'; +import { decode } from 'bech32'; import { body, checkSchema, ParamSchema } from 'express-validator'; import config from '../../config'; @@ -12,6 +15,10 @@ export const CheckSubaccountSchema = checkSchema({ address: { in: ['params', 'query'], isString: true, + custom: { + options: isValidAddress, + }, + errorMessage: 'address must be a valid dydx address', }, subaccountNumber: { in: ['params', 'query'], @@ -26,6 +33,10 @@ export const CheckParentSubaccountSchema = checkSchema({ address: { in: ['params', 'query'], isString: true, + custom: { + options: isValidAddress, + }, + errorMessage: 'address must be a valid dydx address', }, parentSubaccountNumber: { in: ['params', 'query'], @@ -40,6 +51,10 @@ export const checkAddressSchemaRecord: Record = { address: { in: ['params'], isString: true, + custom: { + options: isValidAddress, + }, + errorMessage: 'address must be a valid dydx address', }, }; @@ -262,3 +277,20 @@ export const RegisterTokenValidationSchema = [ return true; }), ]; + +function verifyIsBech32(address: string): Error | undefined { + try { + decode(address); + } catch (error) { + return error; + } + + return undefined; +} + +export function isValidAddress(address: string): boolean { + // An address is valid if it starts with `dydx1` and is Bech32 format. + const ret: boolean = address.startsWith('dydx1') && (verifyIsBech32(address) === undefined); + console.log('is valid address: ', ret); + return ret; +} From d49bf25fc43b632562d7dab3843bc7a728c2ca2c Mon Sep 17 00:00:00 2001 From: Shrenuj Bansal Date: Thu, 16 Jan 2025 16:44:27 -0500 Subject: [PATCH 2/4] change schema check to only character limit and alphanumeric Signed-off-by: Shrenuj Bansal --- .../controllers/api/v4/addresses-controller.test.ts | 12 +++--------- .../src/controllers/api/v4/compliance-controller.ts | 1 + .../services/comlink/src/lib/validation/schemas.ts | 11 +++++++---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts b/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts index dc3bac60a91..11837e63a28 100644 --- a/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts +++ b/indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts @@ -406,16 +406,13 @@ describe('addresses-controller#V4', () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, path: `/v4/addresses/${invalidAddress}`, - expectedStatus: 400, + expectedStatus: 404, }); expect(response.body).toEqual({ errors: [ { - msg: 'address must be a valid dydx address', - location: 'params', - param: 'address', - value: 'invalidAddress', + msg: 'No subaccounts found for address invalidAddress', }, ], }); @@ -736,10 +733,7 @@ describe('addresses-controller#V4', () => { expect(response.body).toEqual({ errors: [ { - msg: 'address must be a valid dydx address', - location: 'params', - param: 'address', - value: 'invalidAddress', + msg: 'Address invalidAddress is not a valid dYdX V4 address', }, ], }); diff --git a/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts b/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts index f8f5bd23e19..a99d96a0e83 100644 --- a/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts @@ -23,6 +23,7 @@ import { complianceProvider } from '../../../helpers/compliance/compliance-clien import { create4xxResponse, handleControllerError } from '../../../lib/helpers'; import { rateLimiterMiddleware } from '../../../lib/rate-limit'; import { getIpAddr } from '../../../lib/utils'; +import { CheckAddressSchema } from '../../../lib/validation/schemas'; import { handleValidationErrors } from '../../../request-helpers/error-handler'; import ExportResponseCodeStats from '../../../request-helpers/export-response-code-stats'; import { ComplianceRequest, ComplianceResponse } from '../../../types'; diff --git a/indexer/services/comlink/src/lib/validation/schemas.ts b/indexer/services/comlink/src/lib/validation/schemas.ts index 68df7664d10..63ffcbec93a 100644 --- a/indexer/services/comlink/src/lib/validation/schemas.ts +++ b/indexer/services/comlink/src/lib/validation/schemas.ts @@ -288,9 +288,12 @@ function verifyIsBech32(address: string): Error | undefined { return undefined; } -export function isValidAddress(address: string): boolean { +export function isValidDydxAddress(address: string): boolean { // An address is valid if it starts with `dydx1` and is Bech32 format. - const ret: boolean = address.startsWith('dydx1') && (verifyIsBech32(address) === undefined); - console.log('is valid address: ', ret); - return ret; + return address.startsWith('dydx1') && (verifyIsBech32(address) === undefined); +} + +export function isValidAddress(address: string): boolean { + // Address is valid if its under 90 characters and alphanumeric + return address.length <= 90 && /^[a-zA-Z0-9]*$/.test(address); } From e92cd6e652833107b5f52dd185bfba26e0c046d0 Mon Sep 17 00:00:00 2001 From: Shrenuj Bansal Date: Fri, 17 Jan 2025 16:09:59 -0500 Subject: [PATCH 3/4] fix some tests Signed-off-by: Shrenuj Bansal --- .../controllers/api/v4/historical-pnl-controller.test.ts | 4 ++-- .../controllers/api/v4/transfers-controller.test.ts | 8 ++++---- .../src/controllers/api/v4/compliance-controller.ts | 1 - indexer/services/comlink/src/lib/validation/schemas.ts | 2 -- 4 files changed, 6 insertions(+), 9 deletions(-) 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 9ff5c2468c9..bca33ac4cd5 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 @@ -264,14 +264,14 @@ describe('pnlTicks-controller#V4', () => { it('Get /historical-pnl with non-existent address and subaccount number returns 404', async () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, - path: '/v4/historical-pnl?address=invalid_address&subaccountNumber=100', + path: '/v4/historical-pnl?address=invalidaddress&subaccountNumber=100', expectedStatus: 404, }); expect(response.body).toEqual({ errors: [ { - msg: 'No subaccount found with address invalid_address and subaccountNumber 100', + msg: 'No subaccount found with address invalidaddress and subaccountNumber 100', }, ], }); diff --git a/indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts b/indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts index 432e050cc91..79ddfd4d04c 100644 --- a/indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts +++ b/indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts @@ -433,14 +433,14 @@ describe('transfers-controller#V4', () => { it('Get /transfers with non-existent address and subaccount number returns 404', async () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, - path: '/v4/transfers?address=invalid_address&subaccountNumber=100', + path: '/v4/transfers?address=invalidaddress&subaccountNumber=100', expectedStatus: 404, }); expect(response.body).toEqual({ errors: [ { - msg: 'No subaccount found with address invalid_address and subaccountNumber 100', + msg: 'No subaccount found with address invalidaddress and subaccountNumber 100', }, ], }); @@ -981,14 +981,14 @@ describe('transfers-controller#V4', () => { it('Get /transfers/parentSubaccountNumber with non-existent address and subaccount number returns 404', async () => { const response: request.Response = await sendRequest({ type: RequestMethod.GET, - path: '/v4/transfers/parentSubaccountNumber?address=invalid_address&parentSubaccountNumber=100', + path: '/v4/transfers/parentSubaccountNumber?address=invalidaddress&parentSubaccountNumber=100', expectedStatus: 404, }); expect(response.body).toEqual({ errors: [ { - msg: 'No subaccount found with address invalid_address and parentSubaccountNumber 100', + msg: 'No subaccount found with address invalidaddress and parentSubaccountNumber 100', }, ], }); diff --git a/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts b/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts index a99d96a0e83..f8f5bd23e19 100644 --- a/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts @@ -23,7 +23,6 @@ import { complianceProvider } from '../../../helpers/compliance/compliance-clien import { create4xxResponse, handleControllerError } from '../../../lib/helpers'; import { rateLimiterMiddleware } from '../../../lib/rate-limit'; import { getIpAddr } from '../../../lib/utils'; -import { CheckAddressSchema } from '../../../lib/validation/schemas'; import { handleValidationErrors } from '../../../request-helpers/error-handler'; import ExportResponseCodeStats from '../../../request-helpers/export-response-code-stats'; import { ComplianceRequest, ComplianceResponse } from '../../../types'; diff --git a/indexer/services/comlink/src/lib/validation/schemas.ts b/indexer/services/comlink/src/lib/validation/schemas.ts index 63ffcbec93a..0d2ce534b70 100644 --- a/indexer/services/comlink/src/lib/validation/schemas.ts +++ b/indexer/services/comlink/src/lib/validation/schemas.ts @@ -1,5 +1,3 @@ -import * as console from 'node:console'; - import { isValidLanguageCode } from '@dydxprotocol-indexer/notifications'; import { perpetualMarketRefresher, From 0b5c03d820f361721f93bedbe8ebf415e801a369 Mon Sep 17 00:00:00 2001 From: Shrenuj Bansal Date: Fri, 17 Jan 2025 16:38:18 -0500 Subject: [PATCH 4/4] fix another test Signed-off-by: Shrenuj Bansal --- .../comlink/__tests__/lib/validation/schemas.test.ts | 7 ++++++- indexer/services/comlink/src/lib/validation/schemas.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/indexer/services/comlink/__tests__/lib/validation/schemas.test.ts b/indexer/services/comlink/__tests__/lib/validation/schemas.test.ts index 9c9ffb74fbc..399e7fe15c0 100644 --- a/indexer/services/comlink/__tests__/lib/validation/schemas.test.ts +++ b/indexer/services/comlink/__tests__/lib/validation/schemas.test.ts @@ -17,7 +17,12 @@ describe('schemas', () => { const defaultAddress: string = testConstants.defaultSubaccount.address; describe('CheckSubaccountSchema', () => { it.each([ - ['missing address', { subaccountNumber: defaultSubaccountNumber }, 'address', 'Invalid value'], + [ + 'missingaddress', + { subaccountNumber: defaultSubaccountNumber }, + 'address', + 'address must be a valid dydx address', + ], [ 'missing subaccountNumber', { address: defaultAddress }, diff --git a/indexer/services/comlink/src/lib/validation/schemas.ts b/indexer/services/comlink/src/lib/validation/schemas.ts index 0d2ce534b70..4156a2d1ea4 100644 --- a/indexer/services/comlink/src/lib/validation/schemas.ts +++ b/indexer/services/comlink/src/lib/validation/schemas.ts @@ -52,7 +52,7 @@ export const checkAddressSchemaRecord: Record = { custom: { options: isValidAddress, }, - errorMessage: 'address must be a valid dydx address', + errorMessage: 'address must be a valid address', }, };