Skip to content
Open
Show file tree
Hide file tree
Changes from 14 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
1 change: 1 addition & 0 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ jobs:
- apps/twap
- apps/notification-producer
- apps/telegram
- apps/token-list-updater
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
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,13 @@
import { execSync } from 'child_process';
import { join } from 'path';

describe('CLI tests', () => {
// TODO: implement properly this test
it.skip('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