fix(notifications): native currency token display #152
Conversation
WalkthroughAdds env template entries and tweaks, updates README with migration and notifications workflow, expands repositories public exports, introduces Erc20 native/fallback repositories with tests, and rewires services factory to use a fallback (Native → Viem) behind the cache; also adjusts Postgres pool initialization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Cache as Erc20RepositoryCache
participant Fallback as Erc20RepositoryFallback
participant Native as Erc20RepositoryNative
participant Viem as Erc20RepositoryViem
Client->>Cache: get(chainId, tokenAddress)
alt cache hit
Cache-->>Client: Erc20
else cache miss
Cache->>Fallback: get(chainId, tokenAddress)
Note over Fallback,Native: Ordered repositories: [Native, Viem]
Fallback->>Native: get(chainId, tokenAddress)
alt Native returns non-null
Native-->>Fallback: Erc20
Fallback-->>Cache: Erc20
Cache-->>Client: Erc20
else Native returns null
Native-->>Fallback: null
Fallback->>Viem: get(chainId, tokenAddress)
alt Viem returns non-null
Viem-->>Fallback: Erc20
Fallback-->>Cache: Erc20
Cache-->>Client: Erc20
else Viem returns null
Viem-->>Fallback: null
Fallback-->>Cache: null
Cache-->>Client: null
end
end
end
%% Styling notes
rect rgba(200,240,255,0.2)
Note over Fallback: New/changed control flow: fallback chain
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
New repository, to fetch native token details from SDK
There was a problem hiding this comment.
New fallback repository, to first check on native, then viem repos for token info
| const viem = new Erc20RepositoryViem(getViemClients()); | ||
| const native = new Erc20RepositoryNative(); | ||
|
|
||
| const fallback = new Erc20RepositoryFallback([native, viem]); |
There was a problem hiding this comment.
Connect native and viem fallback repos
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (15)
.env.example (2)
33-33: REDIS_HOST default changed to localhost — add a note for Docker users.With docker-compose, host is typically "redis". Consider adding a comment like “Set REDIS_HOST=redis when using docker-compose”.
-# REDIS_HOST=localhost +# REDIS_HOST=localhost +# If running via docker-compose, use: REDIS_HOST=redis
11-15: Confirm supported RPC URLs & clean up duplicates
- RPC_URL_56 maps to BNB Smart Chain (Chain ID 56) and RPC_URL_232 maps to Lens Chain (Chain ID 232); both SupportedChainId.BNB and SupportedChainId.LENS are present in NETWORKS, so getViemClients() will load those env vars (binance.com, lens.xyz)
- Remove the duplicate
RPC_URL_11155111entries in.env.exampleREADME.md (3)
79-83: Fix mislabeled Nx generator link.“setup-docker” is listed as the generator for Node libraries; it should be “library”.
-- [@nx/node:setup-docker](https://nx.dev/nx-api/node/generators/library): Generator for NodeJS Libraries +- [@nx/node:library](https://nx.dev/nx-api/node/generators/library): Generator for NodeJS Libraries
87-96: Add language to fenced code block (markdownlint MD040).-``` +```bash # Run all tests yarn test # Run test for repositories (in watch mode) nx run repositories:test --watch # Run test on specific file (in watch mode) nx run repositories:test --watch --testFile=libs/repositories/src/UsdRepository/UsdRepositoryRedis.spec.ts -``` +```
151-189: Migration docs look solid. Add a short note about DB envs.A one-liner reminding to set DATABASE_* envs can save setup friction.
## Run migrations @@ Once you have added the migration logic, you can run the migration with: @@ yarn migration:run+Note: Ensure DATABASE_* variables in your
.envare set (host, port, user, password, name).</blockquote></details> <details> <summary>libs/repositories/src/repos/Erc20Repository/Erc20RepositoryNative.spec.ts (2)</summary><blockquote> `11-19`: **Rename test description: it returns native (not wrapped) token info.** The behavior under test is native-currency resolution for the sentinel address. ```diff -it('returns wrapped native token info when given native sentinel address', async () => { +it('returns native token info when given native sentinel address', async () => {
21-26: Optional: add a case-insensitivity test for the sentinel address.This guards against regressions if address normalization changes.
it('accepts native sentinel address case-insensitively', async () => { const chainId = SupportedChainId.MAINNET; const result = await repo.get(chainId, NATIVE_CURRENCY_ADDRESS.toLowerCase()); expect(result?.address).toEqual(NATIVE_CURRENCY_ADDRESS); });libs/repositories/src/repos/Erc20Repository/Erc20RepositoryFallback.spec.ts (2)
5-10: Align mock signature with the interface for readability.TypeScript allows fewer params, but matching the interface avoids confusion.
class RepoReturns implements Erc20Repository { constructor(private value: Erc20 | null) {} - async get(): Promise<Erc20 | null> { + async get(_chainId: SupportedChainId, _tokenAddress: string): Promise<Erc20 | null> { return this.value; } }
34-41: Consider a test for error propagation/handling.If an inner repo throws, should the fallback continue to the next repo or surface the error? Add one test to lock this behavior.
it('continues to next repo if one throws', async () => { const repo1: Erc20Repository = { get: async () => { throw new Error('boom'); } }; const repo2 = new RepoReturns({ address: '0x2' }); const fallback = new Erc20RepositoryFallback([repo1, repo2]); const result = await fallback.get(chainId, token); expect(result?.address).toEqual('0x2'); });libs/services/src/factories.ts (1)
63-68: Negative caching behavior: InErc20RepositoryCache#get(libs/repositories/src/repos/Erc20Repository/Erc20RepositoryCache.ts), null responses are serialized as'null'and stored viacache.set(..., cacheTimeSeconds), so “unknown” tokens remain cached for the full TTL. If that’s undesirable, consider using a shorter TTL for null entries or skipping caching when the proxy returnsnull.libs/repositories/src/repos/Erc20Repository/Erc20RepositoryFallback.ts (2)
9-20: Make fallback resilient: catch per-repo errors and continue.Right now any thrown error short-circuits the whole fallback. Treat failures as misses so the next repo can try; also accept
undefinedas a miss.async get( chainId: SupportedChainId, tokenAddress: string ): Promise<Erc20 | null> { - for (const repository of this.repositories) { - const result = await repository.get(chainId, tokenAddress); - if (result !== null) { - return result; - } - } + for (const repository of this.repositories) { + try { + const result = await repository.get(chainId, tokenAddress); + if (result != null) { + return result; + } + } catch { + // treat as miss; try next repository + continue; + } + } return null; }
5-7: Confirm DI intent for this class.If this is always constructed manually,
@injectable()is unnecessary; if you plan to have the container build it, prefer explicit bindings (andmultiInjectif injecting an array). Please confirm usage to avoid DI confusion.libs/repositories/src/repos/Erc20Repository/Erc20RepositoryNative.ts (2)
19-25: Ensure returned object satisfies Erc20 (address included).Some chain metadata
nativeCurrencyshapes omitaddress. Construct the Erc20 explicitly to guarantee the sentinel address is present.- const chainInfo = ALL_SUPPORTED_CHAINS_MAP[chainId]; - if (!chainInfo?.nativeCurrency) { - return null; - } - - return chainInfo.nativeCurrency; + const chainInfo = ALL_SUPPORTED_CHAINS_MAP[chainId]; + const native = chainInfo?.nativeCurrency; + if (!native) { + return null; + } + return { + address: NATIVE_CURRENCY_ADDRESS, + name: native.name, + symbol: native.symbol, + decimals: native.decimals, + };If
nativeCurrencyalready includesaddress, this is a no-op improvement in clarity.
15-17: Micro: precompute lowercase sentinel to avoid repeated toLowerCase calls.Not critical, but you can cache the lowercased sentinel at module scope for tiny savings.
+const NATIVE_ADDR_LC = NATIVE_CURRENCY_ADDRESS.toLowerCase(); ... - if (tokenAddress.toLowerCase() !== NATIVE_CURRENCY_ADDRESS.toLowerCase()) { + if (tokenAddress.toLowerCase() !== NATIVE_ADDR_LC) { return null; }libs/repositories/src/index.ts (1)
6-14: Confirm intended public API surface expansion.Adding these data-source barrels broadens the package API. If this package is consumed externally, consider a minor version bump and ensure no internal-only modules are unintentionally exposed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.env.example(3 hunks)README.md(9 hunks)libs/repositories/src/index.ts(3 hunks)libs/repositories/src/repos/Erc20Repository/Erc20RepositoryFallback.spec.ts(1 hunks)libs/repositories/src/repos/Erc20Repository/Erc20RepositoryFallback.ts(1 hunks)libs/repositories/src/repos/Erc20Repository/Erc20RepositoryNative.spec.ts(1 hunks)libs/repositories/src/repos/Erc20Repository/Erc20RepositoryNative.ts(1 hunks)libs/services/src/factories.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
libs/repositories/src/repos/Erc20Repository/Erc20RepositoryFallback.spec.ts (1)
libs/repositories/src/repos/Erc20Repository/Erc20Repository.ts (2)
Erc20Repository(12-19)Erc20(5-10)
libs/repositories/src/repos/Erc20Repository/Erc20RepositoryFallback.ts (1)
libs/repositories/src/repos/Erc20Repository/Erc20Repository.ts (2)
Erc20Repository(12-19)Erc20(5-10)
libs/repositories/src/repos/Erc20Repository/Erc20RepositoryNative.ts (1)
libs/repositories/src/repos/Erc20Repository/Erc20Repository.ts (2)
Erc20Repository(12-19)Erc20(5-10)
libs/services/src/factories.ts (1)
libs/repositories/src/datasources/viem.ts (1)
getViemClients(36-69)
🪛 LanguageTool
README.md
[grammar] ~80-~80: There might be a mistake here.
Context: ...application): Generator for Applications - [@nx/node:application (Fastify)](https://...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...rator for API Applications using Fastify - [@nx/node:setup-docker](https://nx.dev/nx...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...library): Generator for NodeJS Libraries - [@nx/node:setup-docker](https://nx.dev/nx...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: docker (apps/twap)
- GitHub Check: docker (apps/api)
- GitHub Check: docker (apps/telegram)
- GitHub Check: docker (apps/notification-producer)
🔇 Additional comments (6)
.env.example (1)
107-107: Env var rename verified — no lingering old references. Only NOTIFICATIONS_PRODUCER_CHAINS is used (README.md, apps/notification-producer/src/main.ts).README.md (1)
41-46: Good call adding migrations to the Notification Producer workflow.This clarifies the local setup order. LGTM.
libs/services/src/factories.ts (2)
58-66: Correct fallback order (native → viem) and caching layer on top.This ensures native-currency lookups resolve instantly and fixes symbol display without extra RPC calls. LGTM.
155-161: Lazy-initialize Postgres pool via createNewPostgresPool.Good improvement; avoids premature connections and centralizes pool creation.
libs/repositories/src/index.ts (2)
26-28: Exports for Erc20 fallback/native look good.Publicly exposing these aligns with the new wiring and enables composition by consumers.
53-53: LGTM on Postgres indexer export.Matches the new implementation and keeps the barrel consistent.
Summary
Fix native currency symbol display
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Documentation