Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
8 changes: 8 additions & 0 deletions apps/api/src/app/inversify.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getPushNotificationsRepository,
getPushSubscriptionsRepository,
getSimulationRepository,
getTokenCacheRepository,
getTokenHolderRepository,
getUsdRepository,
} from '@cowprotocol/services';
Expand All @@ -14,13 +15,15 @@ import {
PushNotificationsRepository,
PushSubscriptionsRepository,
SimulationRepository,
TokenCacheRepository,
TokenHolderRepository,
UsdRepository,
cacheRepositorySymbol,
erc20RepositorySymbol,
pushNotificationsRepositorySymbol,
pushSubscriptionsRepositorySymbol,
tenderlyRepositorySymbol,
tokenCacheRepositorySymbol,
tokenHolderRepositorySymbol,
usdRepositorySymbol,
} from '@cowprotocol/repositories';
Expand Down Expand Up @@ -52,6 +55,7 @@ function getApiContainer(): Container {
const cacheRepository = getCacheRepository();
const erc20Repository = getErc20Repository(cacheRepository);
const simulationRepository = getSimulationRepository();
const tokenCacheRepository = getTokenCacheRepository();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard token-cache DI binding; don’t crash when Redis is absent

getTokenCacheRepository() throws without Redis. Wrap creation, bind only if available, and keep startup consistent with main.ts’ graceful path.

-  const tokenCacheRepository = getTokenCacheRepository();
+  let tokenCacheRepository: TokenCacheRepository | undefined;
+  try {
+    tokenCacheRepository = getTokenCacheRepository();
+  } catch (err) {
+    logger.warn('Token cache repository not initialized; proceeding without Redis', err);
+  }
...
-  apiContainer
-    .bind<TokenCacheRepository>(tokenCacheRepositorySymbol)
-    .toConstantValue(tokenCacheRepository);
+  if (tokenCacheRepository) {
+    apiContainer
+      .bind<TokenCacheRepository>(tokenCacheRepositorySymbol)
+      .toConstantValue(tokenCacheRepository);
+  }

Longer-term, consider sourcing the single instance from DI and calling setTokenCacheRepository() with that instance to avoid duplication and ensure consistent lifecycle.

Also applies to: 92-95

🤖 Prompt for AI Agents
In apps/api/src/app/inversify.config.ts around line 58 (and similarly lines
92-95), getTokenCacheRepository() can throw when Redis is unavailable; wrap the
repository creation in a try/catch and only perform the DI bind when creation
succeeds, logging or no-oping on failure to preserve the existing graceful
startup path used in main.ts; as a follow-up consider obtaining a single
token-cache instance via DI and calling setTokenCacheRepository(instance) so the
instance lifecycle is consistent and not duplicated.

const tokenHolderRepository = getTokenHolderRepository(cacheRepository);
const usdRepository = getUsdRepository(cacheRepository, erc20Repository);
const pushNotificationsRepository = getPushNotificationsRepository();
Expand Down Expand Up @@ -85,6 +89,10 @@ function getApiContainer(): Container {
.bind<TokenHolderRepository>(tokenHolderRepositorySymbol)
.toConstantValue(tokenHolderRepository);

apiContainer
.bind<TokenCacheRepository>(tokenCacheRepositorySymbol)
.toConstantValue(tokenCacheRepository);

// Services
apiContainer
.bind<SlippageService>(slippageServiceSymbol)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { getTokenListBySearchParam } from '@cowprotocol/repositories';
import { FastifyPluginAsync } from 'fastify';
import {
errorSchema,
ErrorSchema,
paramsSchema,
RouteSchema,
successSchema,
SuccessSchema,
} from './schemas';

const root: FastifyPluginAsync = async (fastify): Promise<void> => {
// example: http://localhost:3010/1/tokens/search/USDC
fastify.get<{
Params: RouteSchema;
Reply: SuccessSchema | ErrorSchema;
}>(
'/',
{
schema: {
params: paramsSchema,
response: {
'2XX': successSchema,
'500': errorSchema,
},
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
async function (request, reply) {
const { chainId, searchParam } = request.params;

try {
const tokens = await getTokenListBySearchParam(chainId, searchParam);

fastify.log.info(
`Token search for "${searchParam}" on chain ${chainId}: ${tokens.length} tokens found`
);

reply.send(tokens);
} catch (error) {
fastify.log.error('Error searching tokens:', error);
reply.code(500).send({
message: 'Internal server error while searching tokens',
});
}
}
);
};

export default root;
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { FromSchema, JSONSchema } from 'json-schema-to-ts';
import { SupportedChainIdSchema } from '../../../../../schemas';
import { AllChainIds } from '@cowprotocol/shared';

export const paramsSchema = {
type: 'object',
required: ['chainId', 'searchParam'],
additionalProperties: false,
properties: {
chainId: SupportedChainIdSchema,
searchParam: {
title: 'Search Parameter',
description: 'Token search parameter (name, symbol, or address)',
type: 'string',
minLength: 3,
maxLength: 100,
},
},
} as const satisfies JSONSchema;

export const successSchema = {
type: 'array',
items: {
type: 'object',
required: ['chainId', 'address', 'name', 'symbol', 'decimals', 'logoURI'],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd say the logoURI is optional

Suggested change
required: ['chainId', 'address', 'name', 'symbol', 'decimals', 'logoURI'],
required: ['chainId', 'address', 'name', 'symbol', 'decimals'],

additionalProperties: false,
properties: {
chainId: {
title: 'Chain ID',
description: 'Blockchain network identifier.',
type: 'integer',
enum: AllChainIds,
},
address: {
title: 'Token Address',
description: 'Contract address of the token.',
type: 'string',
pattern: '^0x[a-fA-F0-9]{40}$',
},
name: {
title: 'Name',
description: 'Full name of the token.',
type: 'string',
},
symbol: {
title: 'Symbol',
description: 'Token symbol/ticker.',
type: 'string',
},
decimals: {
title: 'Decimals',
description: 'Number of decimal places for the token.',
type: 'integer',
minimum: 0,
maximum: 18,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The standard is 18, but it's actually not the max AFAICT.

https://ethereum.stackexchange.com/questions/118896/can-an-erc-20-have-more-than-18-decimals.

I'd remove it:

Suggested change
maximum: 18,

},
logoURI: {
title: 'Logo URI',
description: 'URI to the token logo.',
type: 'string',
},
},
},
} as const satisfies JSONSchema;

export const errorSchema = {
type: 'object',
required: ['message'],
additionalProperties: false,
properties: {
message: {
title: 'Message',
description: 'Message describing the error.',
type: 'string',
},
},
} as const satisfies JSONSchema;

export type RouteSchema = FromSchema<typeof paramsSchema>;
export type SuccessSchema = FromSchema<typeof successSchema>;
export type ErrorSchema = FromSchema<typeof errorSchema>;
5 changes: 5 additions & 0 deletions apps/api/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Fastify from 'fastify';
import { app } from './app/app';
import { logger } from '@cowprotocol/shared';
import { getTokenCacheRepository } from '@cowprotocol/services';
import { setTokenCacheRepository } from '@cowprotocol/repositories';

const host = process.env.HOST ?? 'localhost';
const port = process.env.PORT ? Number(process.env.PORT) : 3001;
Expand All @@ -10,6 +12,9 @@ export const server = Fastify({
logger,
});

const tokenCacheRepository = getTokenCacheRepository();
setTokenCacheRepository(tokenCacheRepository);

Comment on lines +15 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Unconditional Redis dependency here can crash API startup; remove wiring from main or guard it.
If REDIS_* isn’t set, getTokenCacheRepository() throws and the process exits before Fastify starts. Prefer sourcing the instance via DI only (recommended), or guard creation.

Recommended (remove from main; let inversify.config.ts own lifecycle):

-import { getTokenCacheRepository } from '@cowprotocol/services';
-import { setTokenCacheRepository } from '@cowprotocol/repositories';
...
-const tokenCacheRepository = getTokenCacheRepository();
-setTokenCacheRepository(tokenCacheRepository);

If you must keep it in main, at least guard:

+import { getTokenCacheRepository } from '@cowprotocol/services';
+import { setTokenCacheRepository } from '@cowprotocol/repositories';
...
-const tokenCacheRepository = getTokenCacheRepository();
-setTokenCacheRepository(tokenCacheRepository);
+try {
+  const tokenCacheRepository = getTokenCacheRepository();
+  setTokenCacheRepository(tokenCacheRepository);
+  server.log.info('Token cache repository initialized');
+} catch (err) {
+  server.log.warn({ err }, 'Token cache repository not initialized; proceeding without Redis');
+}

Also applies to: 4-5

🤖 Prompt for AI Agents
In apps/api/src/main.ts around lines 15 to 17, the call to
getTokenCacheRepository() unconditionally requires Redis and can throw if
REDIS_* env vars are missing; remove this wiring from main (let
inversify.config.ts/inversion of control own the lifecycle and inject the
repository where needed) or, if you must keep it here, guard creation by
checking the required REDIS_* env vars and only call
setTokenCacheRepository(...) when they are present (or wrap
getTokenCacheRepository() in a try/catch and skip wiring on failure), ensuring
Fastify startup is not blocked by a missing Redis configuration.

// Register your application as a normal plugin.
server.register(app);

Expand Down
3 changes: 3 additions & 0 deletions apps/notification-producer/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ export default {
transform: {
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }],
},
transformIgnorePatterns: [
'node_modules/(?!(node-fetch|data-uri-to-buffer|fetch-blob|formdata-polyfill|@cowprotocol|@uniswap)/)',
],
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/apps/notification-producer',
setupFilesAfterEnv: ['../../jest.setup.ts'],
Expand Down
10 changes: 10 additions & 0 deletions apps/token-list-updater-e2e/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"extends": ["../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
}
]
}
17 changes: 17 additions & 0 deletions apps/token-list-updater-e2e/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* eslint-disable */
export default {
displayName: 'token-list-updater-e2e',
preset: '../..//jest.preset.js',
setupFiles: ['<rootDir>/src/test-setup.ts'],
testEnvironment: 'node',
transform: {
'^.+\\.[tj]s$': [
'ts-jest',
{
tsconfig: '<rootDir>/tsconfig.spec.json',
},
],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../..//coverage/token-list-updater-e2e',
};
22 changes: 22 additions & 0 deletions apps/token-list-updater-e2e/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "token-list-updater-e2e",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"implicitDependencies": ["token-list-updater"],
"targets": {
"e2e": {
"executor": "@nx/jest:jest",
"outputs": ["{workspaceRoot}/coverage/{e2eProjectRoot}"],
"options": {
"jestConfig": "apps/token-list-updater-e2e/jest.config.ts",
"passWithNoTests": true
}
},
"lint": {
"executor": "@nx/linter:eslint",
"outputs": ["{options.outputFile}"],
"options": {
"lintFilePatterns": ["apps/token-list-updater-e2e/**/*.{js,ts}"]
}
}
}
}
1 change: 1 addition & 0 deletions apps/token-list-updater-e2e/src/test-setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
//
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { execSync } from 'child_process';
import { join } from 'path';

describe('CLI tests', () => {
it('should print a message', () => {
const cliPath = join(process.cwd(), 'dist/apps/token-list-updater');

const output = execSync(`node ${cliPath}`).toString();

expect(output).toMatch(/Hello World/);
});
Comment on lines +1 to +12
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Point to the built entry file and use execFileSync with a timeout

The current test runs a directory path via a shell. Use the compiled main file and avoid shell interpolation; add a timeout to prevent hangs.

-import { execSync } from 'child_process';
+import { execFileSync } from 'child_process';
 import { join } from 'path';

 describe('CLI tests', () => {
   it('should print a message', () => {
-    const cliPath = join(process.cwd(), 'dist/apps/token-list-updater');
-
-    const output = execSync(`node ${cliPath}`).toString();
+    const cliPath = join(process.cwd(), 'dist/apps/token-list-updater/main.js');
+    const output = execFileSync('node', [cliPath], {
+      encoding: 'utf8',
+      timeout: 15_000,
+    });
 
     expect(output).toMatch(/Hello World/);
   });
 });
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { execSync } from 'child_process';
import { join } from 'path';
describe('CLI tests', () => {
it('should print a message', () => {
const cliPath = join(process.cwd(), 'dist/apps/token-list-updater');
const output = execSync(`node ${cliPath}`).toString();
expect(output).toMatch(/Hello World/);
});
import { execFileSync } from 'child_process';
import { join } from 'path';
describe('CLI tests', () => {
it('should print a message', () => {
const cliPath = join(process.cwd(), 'dist/apps/token-list-updater/main.js');
const output = execFileSync('node', [cliPath], {
encoding: 'utf8',
timeout: 15_000,
});
expect(output).toMatch(/Hello World/);
});
});
🤖 Prompt for AI Agents
In apps/token-list-updater-e2e/src/token-list-updater/token-list-updater.spec.ts
around lines 1–11, the test invokes a directory via execSync with shell
interpolation and no timeout; change it to point to the compiled entry file
(e.g. join(process.cwd(), 'dist/apps/token-list-updater', 'main.js')) and use
execFileSync to call the Node binary with the entry file as an argument (no
shell interpolation), passing a timeout option (e.g. 5000 ms) to avoid hangs.

});
13 changes: 13 additions & 0 deletions apps/token-list-updater-e2e/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "../../tsconfig.base.json",
"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.spec.json"
}
],
"compilerOptions": {
"esModuleInterop": true
}
}
9 changes: 9 additions & 0 deletions apps/token-list-updater-e2e/tsconfig.spec.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../..//dist/out-tsc",
"module": "commonjs",
"types": ["jest", "node"]
},
"include": ["jest.config.ts", "src/**/*.ts"]
}
18 changes: 18 additions & 0 deletions apps/token-list-updater/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": ["../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
24 changes: 24 additions & 0 deletions apps/token-list-updater/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# This file is generated by Nx.
#
# Build the docker image with `npx nx docker-build token-list-updater`.
# Tip: Modify "docker-build" options in project.json to change docker build args.
#
# Run the container with `docker run -p 3000:3000 -t token-list-updater`.
FROM docker.io/node:lts-alpine

ENV HOST=0.0.0.0
ENV PORT=3000

WORKDIR /app

RUN addgroup --system token-list-updater && \
adduser --system -G token-list-updater token-list-updater

COPY dist/apps/token-list-updater token-list-updater
RUN chown -R token-list-updater:token-list-updater .

Comment on lines +14 to +19
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Run as non-root for container hardening.

You create the user and chown but never switch to it.

Apply:

 RUN addgroup --system token-list-updater && \
           adduser --system -G token-list-updater token-list-updater
@@
 RUN chown -R token-list-updater:token-list-updater .
+
+USER token-list-updater:token-list-updater
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN addgroup --system token-list-updater && \
adduser --system -G token-list-updater token-list-updater
COPY dist/apps/token-list-updater token-list-updater
RUN chown -R token-list-updater:token-list-updater .
RUN addgroup --system token-list-updater && \
adduser --system -G token-list-updater token-list-updater
COPY dist/apps/token-list-updater token-list-updater
RUN chown -R token-list-updater:token-list-updater .
USER token-list-updater:token-list-updater
🤖 Prompt for AI Agents
In apps/token-list-updater/Dockerfile around lines 14 to 19, you create a system
user/group and chown the app files but never switch the container to run as that
non-root user; update the Dockerfile to set the working directory to the copied
app (ensure it's owned by token-list-updater) and add a USER token-list-updater
instruction after the chown so the container runs as the non-root
token-list-updater user.

# You can remove this install step if you build with `--bundle` option.
# The bundled output will include external dependencies.
RUN npm --prefix token-list-updater --omit=dev -f install

CMD [ "node", "token-list-updater/main.js" ]
11 changes: 11 additions & 0 deletions apps/token-list-updater/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* eslint-disable */
export default {
displayName: 'token-list-updater',
preset: '../../jest.preset.js',
testEnvironment: 'node',
transform: {
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/apps/token-list-updater',
};
Loading