Skip to content

Commit

Permalink
Add schema checks for addresses in url endpoints (#2692)
Browse files Browse the repository at this point in the history
Signed-off-by: Shrenuj Bansal <[email protected]>
  • Loading branch information
shrenujb authored Jan 17, 2025
1 parent 56cc48e commit 247dff9
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 22 deletions.
2 changes: 1 addition & 1 deletion indexer/packages/postgres/__tests__/helpers/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions indexer/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -211,15 +212,15 @@ 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,
});

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}`,
},
],
Expand Down Expand Up @@ -411,15 +412,10 @@ describe('addresses-controller#V4', () => {
expect(response.body).toEqual({
errors: [
{
msg: `No subaccounts found for address ${invalidAddress}`,
msg: 'No subaccounts found for address invalidAddress',
},
],
});
expect(stats.increment).toHaveBeenCalledWith('comlink.addresses-controller.response_status_code.404', 1,
{
path: '/:address',
method: 'GET',
});
});
});

Expand Down Expand Up @@ -561,15 +557,15 @@ 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,
});

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}`,
},
],
Expand Down Expand Up @@ -741,10 +737,6 @@ describe('addresses-controller#V4', () => {
},
],
});
expect(statsSpy).toHaveBeenCalledWith('comlink.addresses-controller.response_status_code.400', 1, {
path: '/:address/registerToken',
method: 'POST',
});
});

it.each([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
});
Expand Down Expand Up @@ -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',
},
],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
1 change: 1 addition & 0 deletions indexer/services/comlink/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 33 additions & 0 deletions indexer/services/comlink/src/lib/validation/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
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';
Expand All @@ -12,6 +13,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'],
Expand All @@ -26,6 +31,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'],
Expand All @@ -40,6 +49,10 @@ export const checkAddressSchemaRecord: Record<string, ParamSchema> = {
address: {
in: ['params'],
isString: true,
custom: {
options: isValidAddress,
},
errorMessage: 'address must be a valid address',
},
};

Expand Down Expand Up @@ -262,3 +275,23 @@ export const RegisterTokenValidationSchema = [
return true;
}),
];

function verifyIsBech32(address: string): Error | undefined {
try {
decode(address);
} catch (error) {
return error;
}

return undefined;
}

export function isValidDydxAddress(address: string): boolean {
// An address is valid if it starts with `dydx1` and is Bech32 format.
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);
}

0 comments on commit 247dff9

Please sign in to comment.