-
Notifications
You must be signed in to change notification settings - Fork 205
chore: integration example for tm websocket #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughA new integration module for CometBFT (Tendermint) event indexing was added under Changes
Sequence Diagram(s)sequenceDiagram
participant ExampleScript
participant CometBFTIndexer
participant WebSocketClient
participant CometBFTNode
participant BlockHandler
participant TxHandler
ExampleScript->>CometBFTIndexer: new CometBFTIndexer(endpoints)
ExampleScript->>CometBFTIndexer: subscribe()
CometBFTIndexer->>WebSocketClient: send subscription for NewBlock and Tx
WebSocketClient->>CometBFTNode: Connect and subscribe
CometBFTNode-->>WebSocketClient: Event message (NewBlock or Tx)
WebSocketClient->>CometBFTIndexer: onMessage(msg)
CometBFTIndexer->>BlockHandler: handleNewBlock(data) (if NewBlock)
CometBFTIndexer->>TxHandler: handleNewTx(data) (if Tx)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
integrations/tm/src/example.tsOops! Something went wrong! :( ESLint: 7.32.0 Error: Error while loading rule 'jest/unbound-method': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser. integrations/tm/jest.config.jsOops! Something went wrong! :( ESLint: 7.32.0 Error: Error while loading rule 'jest/unbound-method': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser. integrations/tm/src/logger.tsOops! Something went wrong! :( ESLint: 7.32.0 Error: Error while loading rule 'jest/unbound-method': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (21)
integrations/tm/LICENSE (1)
1-1: Update license year range to the current year.
The header states© 2021 - 2022, but the integration was added in 2025. Please update the year range to2021 - 2025(or just2025) to reflect the accurate timeline.- Copyright © 2021 - 2022 Injective Labs Inc. (https://injectivelabs.org/) + Copyright © 2021 - 2025 Injective Labs Inc. (https://injectivelabs.org/)tsconfig.build.json (1)
47-47: Scope exclusion more precisely.
Excluding"integrations"will skip the entire directory but might be ambiguous. Consider using a glob pattern to clearly exclude all subdirectories:- "integrations", + "integrations/**",integrations/tm/.npmignore (1)
1-4: Whitelist essential files explicitly.
Whilenpmalways includespackage.json,README.md, andLICENSEby default, it's safer to explicitly whitelist them to prevent future omissions:# .npmignore - * + * !dist/**/*.d.ts !dist/**/*.js + !README.md + !LICENSEintegrations/tm/tsconfig.json (1)
1-13: DRY: Extend root TypeScript configuration.
Instead of duplicating compiler options, consider extending the project's basetsconfig.jsonand overriding only what’s necessary:{ - "compilerOptions": { - "module": "ESNext", - "target": "ES2020", - "moduleResolution": "bundler", - "esModuleInterop": true, - "moduleDetection": "force", - "allowSyntheticDefaultImports": true, - "resolveJsonModule": true, - "isolatedModules": true, - "types": ["node", "jest"] - } + "extends": "../../tsconfig.json", + "compilerOptions": { + // override or add integration-specific options here + } }This promotes consistency and reduces maintenance overhead.
integrations/tm/src/logger.ts (2)
1-19: Logger implementation uses Winston effectively but has some duplicationThe logger configuration is well-structured but contains duplicate format settings in line 9-12 and line 16. This redundancy isn't necessary since you're only using one transport.
Consider simplifying the config by removing the duplicate format:
export const logger = winston.createLogger({ format: winston.format.combine( winston.format.colorize(), winston.format.simple(), ), transports: [ new winston.transports.Console({ level: LOG_LEVEL, - format: winston.format.simple(), }), ], })
6-6: Add validation for LOG_LEVEL environment variableThe LOG_LEVEL value is used directly without validation. Invalid log levels might cause unexpected behavior.
Consider validating the log level against Winston's valid levels:
-const LOG_LEVEL = process.env.LOG_LEVEL || 'info' +const validLogLevels = ['error', 'warn', 'info', 'http', 'verbose', 'debug', 'silly'] +const LOG_LEVEL = validLogLevels.includes(process.env.LOG_LEVEL?.toLowerCase() || '') + ? process.env.LOG_LEVEL + : 'info'integrations/tm/README.md (2)
25-25: Add alt text to image for accessibilityThe image lacks alt text, which is important for accessibility.
-<a href="https://iili.io/mNneZN.md.png"><img src="https://iili.io/mNneZN.md.png" style="width: 300px; max-width: 100%; height: auto" /> +<a href="https://iili.io/mNneZN.md.png"><img src="https://iili.io/mNneZN.md.png" alt="Injective Labs Logo" style="width: 300px; max-width: 100%; height: auto" />🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Images should have alternate text (alt text)
null(MD045, no-alt-text)
27-30: Fix license formatting and use Markdown syntaxThe license section has formatting issues including incorrect use of a colon and bare URLs.
-Originally released by Injective Labs Inc. under: <br /> -Apache License <br /> -Version 2.0, January 2004 <br /> -http://www.apache.org/licenses/ +Originally released by Injective Labs Inc. under <br /> +Apache License <br /> +Version 2.0, January 2004 <br /> +[http://www.apache.org/licenses/](http://www.apache.org/licenses/)🧰 Tools
🪛 LanguageTool
[typographical] ~27-~27: Do not use a colon (:) before a series that is introduced by a preposition (‘under’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...ginally released by Injective Labs Inc. under:
Apache License
Version 2....(RP_COLON)
🪛 markdownlint-cli2 (0.17.2)
30-30: Bare URL used
null(MD034, no-bare-urls)
integrations/tm/src/example.ts (1)
10-12: Improve empty interval implementation with commentsThe empty interval seems to be a placeholder to keep the process running, but lacks explanation.
Add a comment explaining the purpose of this interval:
setInterval(() => { - // + // Empty interval to keep the process running while waiting for WebSocket events }, 1000)integrations/tm/jest.config.js (2)
6-8: Update deprecated ts-jest configuration propertyThe property 'tsConfig' (camelCase) is deprecated in newer versions of ts-jest, which prefers 'tsconfig' (lowercase).
globals: { 'ts-jest': { - tsConfig: 'tsconfig.build.esm.json', + tsconfig: 'tsconfig.build.esm.json', }, },
1-10: Consider documenting test configuration purposeThe Jest configuration extends a base config but doesn't provide context on what tests are expected to run with this setup.
Add a comment explaining the purpose of this configuration:
import baseConfig from '../../jest.config.js' +/** + * Jest configuration for the TM integration tests + * Extends the base configuration and uses the ESM TypeScript config + */ export default { ...baseConfig, globals: { 'ts-jest': { tsConfig: 'tsconfig.build.esm.json', }, }, }integrations/tm/src/client/handlers/blocks.ts (1)
4-10: Function is marked async but doesn't await anythingThe function is declared as async but doesn't contain any awaited operations, which is unnecessary overhead until async operations are implemented.
This is fine if you plan to add async operations later as indicated by the comment, but for clarity you could add a TODO comment explaining the async intent:
export async function handleNewBlock(data: NewBlockEvent['value']) { const height = parseInt(data.block.header.height) logger.info(`🧱 Indexed block #${height}`) - // Add indexing logic... + // TODO: Add async indexing logic here }integrations/tm/src/client/handlers/txs.ts (1)
9-9: Consider extracting transaction details in the commentThe comment is a simple placeholder, but it would be more informative to mention what transaction details might be indexed.
- // Add indexing logic... + // TODO: Add indexing logic to process transaction details (hash, gas used, etc.)integrations/tm/package.json (1)
4-4: Add a meaningful description for the packageThe description field is empty, which misses an opportunity to explain the package's purpose.
- "description": "", + "description": "Integration module for CometBFT (Tendermint) WebSocket event subscription and indexing",integrations/tm/src/client/ws.ts (2)
54-66: Potential retry accumulation in send methodThe send method uses a setTimeout to retry sending if the connection isn't in OPEN state, but it doesn't limit retries which could lead to many queued calls.
Consider implementing a maximum retry count or an exponential backoff:
public send(data: object) { if (!this.ws) { logger.warn('⚠️ WebSocket not initialized. Cannot send.') return } + private sendRetryCount = 0 + private maxSendRetries = 10 if (this.ws.readyState === WebSocket.OPEN) { + this.sendRetryCount = 0 this.ws.send(JSON.stringify(data)) } else { + if (this.sendRetryCount >= this.maxSendRetries) { + logger.error('❌ Maximum send retries reached, dropping message') + this.sendRetryCount = 0 + return + } + this.sendRetryCount++ setTimeout(() => this.send(data), 100) } }
5-67: Add connection state tracking and eventsThe class doesn't expose its connection state or emit events about state changes, making it difficult for consumers to react to connection status.
Consider implementing a simple event system or state accessors:
+ import { EventEmitter } from 'events' import WebSocket from 'ws' import { logger } from '../logger.js' import { MessageHandler } from '../types.js' + export enum ConnectionState { + DISCONNECTED = 'disconnected', + CONNECTING = 'connecting', + CONNECTED = 'connected' + } - export class WebSocketClient { + export class WebSocketClient extends EventEmitter { private endpoints: string[] private currentIndex = 0 private ws: WebSocket | null = null private reconnectTimeout = 3000 private messageHandler: MessageHandler + private _state: ConnectionState = ConnectionState.DISCONNECTED constructor(endpoints: string[], messageHandler: MessageHandler) { + super() this.endpoints = endpoints this.messageHandler = messageHandler this.connect() } + get state(): ConnectionState { + return this._state + } + private setState(state: ConnectionState): void { + if (this._state !== state) { + this._state = state + this.emit('stateChange', state) + logger.debug(`WebSocket state changed to: ${state}`) + } + } private connect() { const endpoint = this.endpoints[this.currentIndex] + this.setState(ConnectionState.CONNECTING) logger.debug(`🔌 Connecting to ${endpoint}`) this.ws = new WebSocket(endpoint) this.ws.on('open', () => { + this.setState(ConnectionState.CONNECTED) logger.debug(`✅ Connected to ${endpoint}`) }) // ... rest of the code this.ws.on('close', () => { + this.setState(ConnectionState.DISCONNECTED) logger.warn(`⚠️ Disconnected from ${endpoint}`) // ... rest of the code }) } }integrations/tm/src/client/indexer.ts (3)
14-25: Consider more robust ID generation and subscription confirmation.The current implementation sends JSON-RPC subscription requests with randomly generated IDs, which works but could be improved.
Consider these improvements:
- Use a more deterministic ID generation method or UUID for better traceability
- Add confirmation handling for subscription requests
public subscribe() { const subs = [{ query: "tm.event='NewBlock'" }, { query: "tm.event='Tx'" }] for (const sub of subs) { this.ws.send({ jsonrpc: '2.0', method: 'subscribe', - id: Math.floor(Math.random() * 1_000_000), + id: `sub_${sub.query}_${Date.now()}`, // More traceable ID format params: sub, }) } + + // Consider adding logging for subscription attempts + logger.info(`🔔 Subscribed to ${subs.length} event types`) }
30-34: Fix typo in debug message.There's a minor grammatical error in the debug message.
if (!data) { - logger.debug('📦 Message data doesnt exist or is undefined:', msg) + logger.debug('📦 Message data doesn\'t exist or is undefined:', msg) return }
36-40: Fix typo in debug message.There's a minor grammatical error in the debug message.
if (!data.type) { - logger.debug('📦 Message type doesnt exist or is undefined:', msg) + logger.debug('📦 Message type doesn\'t exist or is undefined:', msg) return }integrations/tm/src/types.ts (2)
1-23: Consider adding documentation to the complex event structure.The
NewBlockEventinterface correctly models the CometBFT event structure but lacks documentation that would make it more maintainable.+ /** + * Represents a CometBFT NewBlock event structure. + * Contains information about newly added blocks in the blockchain. + */ export interface NewBlockEvent { type: 'tendermint/event/NewBlock' value: { block: { header: { + /** Block height as string (requires parsing to number) */ height: string + /** ISO timestamp of when the block was created */ time: string + /** Identifier for the blockchain */ chain_id: string } data: { txs: { tx: { body: { messages: { type: string }[] } } }[] } } } }
6-6: Consider parsing numeric values at the type level.Several numeric values are represented as strings in the event types, which will require parsing in the handler functions.
Consider defining helper types or utility functions to handle the conversion from string to number consistently:
// Helper type for string numbers export type StringNumber = string; // Utility function to safely parse string to number export const parseStringNumber = (value: StringNumber): number => { const parsed = parseInt(value, 10); return isNaN(parsed) ? 0 : parsed; };This would make handling these fields more consistent across the codebase.
Also applies to: 29-29, 40-41
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
integrations/tm/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
integrations/tm/.env.example(1 hunks)integrations/tm/.npmignore(1 hunks)integrations/tm/LICENSE(1 hunks)integrations/tm/README.md(1 hunks)integrations/tm/jest.config.js(1 hunks)integrations/tm/package.json(1 hunks)integrations/tm/src/client/handlers/blocks.ts(1 hunks)integrations/tm/src/client/handlers/txs.ts(1 hunks)integrations/tm/src/client/indexer.ts(1 hunks)integrations/tm/src/client/ws.ts(1 hunks)integrations/tm/src/example.ts(1 hunks)integrations/tm/src/logger.ts(1 hunks)integrations/tm/src/types.ts(1 hunks)integrations/tm/tsconfig.json(1 hunks)tsconfig.build.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
integrations/tm/src/client/handlers/txs.ts (2)
integrations/tm/src/types.ts (1)
NewTxEvent(25-47)integrations/tm/src/logger.ts (1)
logger(8-19)
integrations/tm/src/client/indexer.ts (5)
integrations/tm/src/client/ws.ts (1)
WebSocketClient(5-67)integrations/tm/src/types.ts (3)
MessageHandlerArgs(54-56)NewBlockEvent(1-23)NewTxEvent(25-47)integrations/tm/src/logger.ts (1)
logger(8-19)integrations/tm/src/client/handlers/blocks.ts (1)
handleNewBlock(4-10)integrations/tm/src/client/handlers/txs.ts (1)
handleNewTx(4-10)
integrations/tm/src/example.ts (2)
packages/networks/src/network.ts (1)
getNetworkEndpoints(65-66)integrations/tm/src/client/indexer.ts (1)
CometBFTIndexer(7-53)
integrations/tm/src/client/handlers/blocks.ts (2)
integrations/tm/src/types.ts (1)
NewBlockEvent(1-23)integrations/tm/src/logger.ts (1)
logger(8-19)
integrations/tm/src/client/ws.ts (2)
integrations/tm/src/types.ts (1)
MessageHandler(57-57)integrations/tm/src/logger.ts (1)
logger(8-19)
🪛 LanguageTool
integrations/tm/README.md
[typographical] ~23-~23: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...a> --- ## 🔓 License Copyright © 2021 - 2022 Injective Labs Inc. (https://inject...
(DASH_RULE)
[typographical] ~27-~27: Do not use a colon (:) before a series that is introduced by a preposition (‘under’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...ginally released by Injective Labs Inc. under:
Apache License
Version 2....
(RP_COLON)
🪛 markdownlint-cli2 (0.17.2)
integrations/tm/README.md
3-3: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
23-23: Bare URL used
null
(MD034, no-bare-urls)
25-25: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
30-30: Bare URL used
null
(MD034, no-bare-urls)
32-32: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
🔇 Additional comments (6)
integrations/tm/.env.example (1)
1-1:❌ Incorrect review comment
Include all required environment variables.
Currently.env.exampleonly definesLOG_LEVEL. To ensure users can configure the Tendermint integration, add placeholders for other environment variables such as the RPC endpoint, WebSocket endpoint, subscription topics, etc.Run this script to list all
process.envreferences and verify that.env.examplecovers them:
🏁 Script executed:
#!/bin/bash # List environment variables used in the integration source rg -o 'process\.env\.\w+' -r '$0' integrations/tm/src | sort | uniqLength of output: 121
Only LOG_LEVEL Is Referenced in the Tendermint Integration
The grep output shows the integration source only usesprocess.env.LOG_LEVEL, so no other environment-variable placeholders are needed inintegrations/tm/.env.example.Keep
.env.exampleas:LOG_LEVEL=infoLikely an incorrect or invalid review comment.
integrations/tm/src/example.ts (1)
2-2:✅ Verification successful
Verify the .js extension in import path
You're importing from a local file with the '.js' extension in a TypeScript file, which is unusual.
This might be intentional if you're using ES modules in your compiled output. If that's not the case, you should remove the extension:
-import { CometBFTIndexer } from './client/indexer.js' +import { CometBFTIndexer } from './client/indexer'
🏁 Script executed:
#!/bin/bash # Check if other TypeScript files in the project use .js extension in imports rg -t ts "import.*from.*\.js['\"]" --glob "!**/example.ts"Length of output: 67936
No change needed for .js extension in imports
Importing with the “.js” extension is deliberate and aligns with other TypeScript files targeting ES modules. We’ve verified numerous TS files (including
integrations/tm/src/client/ws.ts,packages/utils/src/time.ts, and many more) consistently use “.js” extensions, so no update is required.integrations/tm/src/client/indexer.ts (2)
1-5: Import structure looks good.The imports correctly bring in the necessary components from relative paths with explicit
.jsextensions, which is appropriate for ES modules. The code imports the WebSocketClient, event handlers, types, and logger from their respective locations.
7-12: Constructor design is appropriate.The class accepts an array of endpoint URLs to support failover scenarios and initializes the WebSocketClient with proper binding of the message handler. This approach provides good resilience if primary endpoints become unavailable.
integrations/tm/src/types.ts (2)
49-53: Union type provides good flexibility.The
CometBFTEventunion type provides a good balance between type safety for known events and flexibility for unknown event types.
54-57: Message handler types are well-defined.The types correctly model the structure of incoming message data with optional properties, which is appropriate for handling various message formats and potential absence of data.
| Copyright [2022] [InjectiveLabs] | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace placeholder boilerplate with actual project information.
The appendix still has placeholder fields: [2022] [InjectiveLabs]. Update these to concrete values, for example:
- Copyright [2022] [InjectiveLabs]
+ Copyright © 2025 Injective Labs Inc.📝 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.
| Copyright [2022] [InjectiveLabs] | |
| Licensed under the Apache License, Version 2.0 (the "License"); | |
| Copyright © 2025 Injective Labs Inc. | |
| Licensed under the Apache License, Version 2.0 (the "License"); |
| # 🌟 Injective Protocol - Integration TM | ||
|
|
||
|
|
||
| --- | ||
|
|
||
| ## 📚 Installation | ||
|
|
||
| --- | ||
|
|
||
| ## ⛑ Support | ||
|
|
||
| Reach out to us at one of the following places! | ||
|
|
||
| - Website at <a href="https://injective.com" target="_blank">`injective.com`</a> | ||
| - Twitter at <a href="https://twitter.com/Injective_" target="_blank">`@Injective`</a> | ||
| - Discord at <a href="https://discord.com/invite/NK4qdbv" target="_blank">`Discord`</a> | ||
| - Telegram at <a href="https://t.me/joininjective" target="_blank">`Telegram`</a> | ||
|
|
||
| --- | ||
|
|
||
| ## 🔓 License | ||
|
|
||
| Copyright © 2021 - 2022 Injective Labs Inc. (https://injectivelabs.org/) | ||
|
|
||
| <a href="https://iili.io/mNneZN.md.png"><img src="https://iili.io/mNneZN.md.png" style="width: 300px; max-width: 100%; height: auto" /> | ||
|
|
||
| Originally released by Injective Labs Inc. under: <br /> | ||
| Apache License <br /> | ||
| Version 2.0, January 2004 <br /> | ||
| http://www.apache.org/licenses/ | ||
|
|
||
|
|
||
| <p> </p> | ||
| <div align="center"> | ||
| <sub><em>Powering the future of decentralized finance.</em></sub> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance README with more comprehensive documentation
The README lacks essential information about what this integration does, how to use it, and installation instructions.
- Add a proper introduction explaining the purpose and functionality of the TM integration
- Fill in the installation section with actual instructions
- Consider adding a "Usage" section that references the example.ts file
- Fix the formatting issues identified by static analysis
# 🌟 Injective Protocol - Integration TM
+## Introduction
+This integration provides a client for CometBFT (formerly Tendermint) WebSocket connections to subscribe and process blockchain events such as new blocks and transactions.
---
## 📚 Installation
+```bash
+# Install dependencies
+npm install
+
+# Copy environment variables
+cp .env.example .env
+```
---
+## 🚀 Usage
+
+```typescript
+import { CometBFTIndexer } from '@injectivelabs/tm'
+
+// Initialize indexer with WebSocket endpoints
+const indexer = new CometBFTIndexer(['wss://tm.injective.network/websocket'])
+
+// Subscribe to events
+await indexer.subscribe()
+```
+
+See `src/example.ts` for a complete usage example.
+
+---
## ⛑ Support🧰 Tools
🪛 LanguageTool
[typographical] ~23-~23: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...a> --- ## 🔓 License Copyright © 2021 - 2022 Injective Labs Inc. (https://inject...
(DASH_RULE)
[typographical] ~27-~27: Do not use a colon (:) before a series that is introduced by a preposition (‘under’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...ginally released by Injective Labs Inc. under:
Apache License
Version 2....
(RP_COLON)
🪛 markdownlint-cli2 (0.17.2)
3-3: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
23-23: Bare URL used
null
(MD034, no-bare-urls)
25-25: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
30-30: Bare URL used
null
(MD034, no-bare-urls)
32-32: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
| import { getNetworkEndpoints, Network } from '@injectivelabs/networks' | ||
| import { CometBFTIndexer } from './client/indexer.js' | ||
|
|
||
| const endpoints = [`${getNetworkEndpoints(Network.Mainnet).rpc}/websocket`] | ||
|
|
||
| const indexer = new CometBFTIndexer(endpoints) | ||
|
|
||
| await indexer.subscribe() | ||
|
|
||
| setInterval(() => { | ||
| // | ||
| }, 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling and cleanup logic
The example lacks error handling for the subscription process and cleanup logic for proper termination.
Consider adding proper error handling and termination handling:
import { getNetworkEndpoints, Network } from '@injectivelabs/networks'
import { CometBFTIndexer } from './client/indexer.js'
const endpoints = [`${getNetworkEndpoints(Network.Mainnet).rpc}/websocket`]
const indexer = new CometBFTIndexer(endpoints)
-await indexer.subscribe()
+try {
+ await indexer.subscribe()
+ console.log('Successfully subscribed to events')
+} catch (error) {
+ console.error('Failed to subscribe to events:', error)
+ process.exit(1)
+}
+// Handle process termination
+process.on('SIGINT', () => {
+ console.log('Terminating indexer...')
+ // Add cleanup code here if needed
+ process.exit(0)
+})
setInterval(() => {
// Empty interval to keep the process running while waiting for WebSocket events
}, 1000)📝 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.
| import { getNetworkEndpoints, Network } from '@injectivelabs/networks' | |
| import { CometBFTIndexer } from './client/indexer.js' | |
| const endpoints = [`${getNetworkEndpoints(Network.Mainnet).rpc}/websocket`] | |
| const indexer = new CometBFTIndexer(endpoints) | |
| await indexer.subscribe() | |
| setInterval(() => { | |
| // | |
| }, 1000) | |
| import { getNetworkEndpoints, Network } from '@injectivelabs/networks' | |
| import { CometBFTIndexer } from './client/indexer.js' | |
| const endpoints = [`${getNetworkEndpoints(Network.Mainnet).rpc}/websocket`] | |
| const indexer = new CometBFTIndexer(endpoints) | |
| try { | |
| await indexer.subscribe() | |
| console.log('Successfully subscribed to events') | |
| } catch (error) { | |
| console.error('Failed to subscribe to events:', error) | |
| process.exit(1) | |
| } | |
| // Handle process termination | |
| process.on('SIGINT', () => { | |
| console.log('Terminating indexer...') | |
| // Add cleanup code here if needed | |
| process.exit(0) | |
| }) | |
| setInterval(() => { | |
| // Empty interval to keep the process running while waiting for WebSocket events | |
| }, 1000) |
| export async function handleNewBlock(data: NewBlockEvent['value']) { | ||
| const height = parseInt(data.block.header.height) | ||
|
|
||
| logger.info(`🧱 Indexed block #${height}`) | ||
|
|
||
| // Add indexing logic... | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
The handler should include error handling for malformed data
The function assumes data will always have the expected structure, which could lead to runtime errors if the block format changes or if malformed data is received.
Consider adding error handling:
export async function handleNewBlock(data: NewBlockEvent['value']) {
+ try {
const height = parseInt(data.block.header.height)
logger.info(`🧱 Indexed block #${height}`)
// Add indexing logic...
+ } catch (error) {
+ logger.error(`Failed to process block: ${error instanceof Error ? error.message : String(error)}`)
+ }
}📝 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.
| export async function handleNewBlock(data: NewBlockEvent['value']) { | |
| const height = parseInt(data.block.header.height) | |
| logger.info(`🧱 Indexed block #${height}`) | |
| // Add indexing logic... | |
| } | |
| export async function handleNewBlock(data: NewBlockEvent['value']) { | |
| try { | |
| const height = parseInt(data.block.header.height) | |
| logger.info(`🧱 Indexed block #${height}`) | |
| // Add indexing logic... | |
| } catch (error) { | |
| logger.error(`Failed to process block: ${error instanceof Error ? error.message : String(error)}`) | |
| } | |
| } |
| export async function handleNewTx(data: NewTxEvent['value']) { | ||
| const height = parseInt(data.TxResult.height) | ||
|
|
||
| logger.info(`📨 Indexed Tx at height ${height}`) | ||
|
|
||
| // Add indexing logic... | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for the transaction handler
Similar to the block handler, this function lacks error handling for malformed data, which could lead to unexpected crashes.
Implement try/catch to gracefully handle potential errors:
export async function handleNewTx(data: NewTxEvent['value']) {
+ try {
const height = parseInt(data.TxResult.height)
logger.info(`📨 Indexed Tx at height ${height}`)
// Add indexing logic...
+ } catch (error) {
+ logger.error(`Failed to process transaction: ${error instanceof Error ? error.message : String(error)}`)
+ }
}📝 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.
| export async function handleNewTx(data: NewTxEvent['value']) { | |
| const height = parseInt(data.TxResult.height) | |
| logger.info(`📨 Indexed Tx at height ${height}`) | |
| // Add indexing logic... | |
| } | |
| export async function handleNewTx(data: NewTxEvent['value']) { | |
| try { | |
| const height = parseInt(data.TxResult.height) | |
| logger.info(`📨 Indexed Tx at height ${height}`) | |
| // Add indexing logic... | |
| } catch (error) { | |
| logger.error(`Failed to process transaction: ${error instanceof Error ? error.message : String(error)}`) | |
| } | |
| } |
| "devDependencies": { | ||
| "@types/node-persist": "^3.1.8", | ||
| "shx": "^0.3.4" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing TypeScript and testing dependencies
The package has TypeScript source files but doesn't list TypeScript or any testing frameworks as dev dependencies.
Add the necessary development dependencies:
"devDependencies": {
+ "@types/jest": "^29.5.0",
+ "@types/node": "^20.8.0",
+ "@types/ws": "^8.5.10",
"@types/node-persist": "^3.1.8",
+ "jest": "^29.5.0",
+ "ts-jest": "^29.1.0",
+ "typescript": "^5.0.0",
"shx": "^0.3.4"
},📝 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.
| "devDependencies": { | |
| "@types/node-persist": "^3.1.8", | |
| "shx": "^0.3.4" | |
| }, | |
| "devDependencies": { | |
| "@types/jest": "^29.5.0", | |
| "@types/node": "^20.8.0", | |
| "@types/ws": "^8.5.10", | |
| "@types/node-persist": "^3.1.8", | |
| "jest": "^29.5.0", | |
| "ts-jest": "^29.1.0", | |
| "typescript": "^5.0.0", | |
| "shx": "^0.3.4" | |
| }, |
| this.ws.on('message', (data: WebSocket.Data) => { | ||
| try { | ||
| this.messageHandler(JSON.parse(data.toString())) | ||
| } catch (err) { | ||
| logger.error('❌ Error parsing message:', err) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling in message handler
The try/catch block only catches JSON parsing errors but doesn't handle errors that might occur within the message handler itself.
Expand error handling to catch all potential errors:
this.ws.on('message', (data: WebSocket.Data) => {
try {
- this.messageHandler(JSON.parse(data.toString()))
+ const parsedData = JSON.parse(data.toString())
+ this.messageHandler(parsedData)
} catch (err) {
- logger.error('❌ Error parsing message:', err)
+ if (err instanceof SyntaxError) {
+ logger.error('❌ Error parsing message:', err)
+ } else {
+ logger.error('❌ Error handling message:', err)
+ }
}
})📝 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.
| this.ws.on('message', (data: WebSocket.Data) => { | |
| try { | |
| this.messageHandler(JSON.parse(data.toString())) | |
| } catch (err) { | |
| logger.error('❌ Error parsing message:', err) | |
| } | |
| }) | |
| this.ws.on('message', (data: WebSocket.Data) => { | |
| try { | |
| const parsedData = JSON.parse(data.toString()) | |
| this.messageHandler(parsedData) | |
| } catch (err) { | |
| if (err instanceof SyntaxError) { | |
| logger.error('❌ Error parsing message:', err) | |
| } else { | |
| logger.error('❌ Error handling message:', err) | |
| } | |
| } | |
| }) |
| private connect() { | ||
| const endpoint = this.endpoints[this.currentIndex] | ||
|
|
||
| logger.debug(`🔌 Connecting to ${endpoint}`) | ||
|
|
||
| this.ws = new WebSocket(endpoint) | ||
|
|
||
| this.ws.on('open', () => { | ||
| logger.debug(`✅ Connected to ${endpoint}`) | ||
| }) | ||
|
|
||
| this.ws.on('message', (data: WebSocket.Data) => { | ||
| try { | ||
| this.messageHandler(JSON.parse(data.toString())) | ||
| } catch (err) { | ||
| logger.error('❌ Error parsing message:', err) | ||
| } | ||
| }) | ||
|
|
||
| this.ws.on('error', (err: Error) => { | ||
| logger.error('❌ WebSocket error:', err) | ||
| }) | ||
|
|
||
| this.ws.on('close', () => { | ||
| logger.warn(`⚠️ Disconnected from ${endpoint}`) | ||
|
|
||
| this.currentIndex = (this.currentIndex + 1) % this.endpoints.length | ||
|
|
||
| setTimeout(() => this.connect(), this.reconnectTimeout) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Check for empty endpoints array
The class doesn't validate if the endpoints array is empty, which could lead to undefined behavior when trying to connect.
Add validation in the constructor and connect method:
constructor(endpoints: string[], messageHandler: MessageHandler) {
+ if (!endpoints.length) {
+ throw new Error('At least one endpoint must be provided')
+ }
this.endpoints = endpoints
this.messageHandler = messageHandler
this.connect()
}
private connect() {
+ if (!this.endpoints.length) {
+ logger.error('❌ No endpoints available for connection')
+ return
+ }
const endpoint = this.endpoints[this.currentIndex]📝 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.
| private connect() { | |
| const endpoint = this.endpoints[this.currentIndex] | |
| logger.debug(`🔌 Connecting to ${endpoint}`) | |
| this.ws = new WebSocket(endpoint) | |
| this.ws.on('open', () => { | |
| logger.debug(`✅ Connected to ${endpoint}`) | |
| }) | |
| this.ws.on('message', (data: WebSocket.Data) => { | |
| try { | |
| this.messageHandler(JSON.parse(data.toString())) | |
| } catch (err) { | |
| logger.error('❌ Error parsing message:', err) | |
| } | |
| }) | |
| this.ws.on('error', (err: Error) => { | |
| logger.error('❌ WebSocket error:', err) | |
| }) | |
| this.ws.on('close', () => { | |
| logger.warn(`⚠️ Disconnected from ${endpoint}`) | |
| this.currentIndex = (this.currentIndex + 1) % this.endpoints.length | |
| setTimeout(() => this.connect(), this.reconnectTimeout) | |
| }) | |
| } | |
| constructor(endpoints: string[], messageHandler: MessageHandler) { | |
| if (!endpoints.length) { | |
| throw new Error('At least one endpoint must be provided') | |
| } | |
| this.endpoints = endpoints | |
| this.messageHandler = messageHandler | |
| this.connect() | |
| } | |
| private connect() { | |
| if (!this.endpoints.length) { | |
| logger.error('❌ No endpoints available for connection') | |
| return | |
| } | |
| const endpoint = this.endpoints[this.currentIndex] | |
| logger.debug(`🔌 Connecting to ${endpoint}`) | |
| this.ws = new WebSocket(endpoint) | |
| this.ws.on('open', () => { | |
| logger.debug(`✅ Connected to ${endpoint}`) | |
| }) | |
| this.ws.on('message', (data: WebSocket.Data) => { | |
| try { | |
| this.messageHandler(JSON.parse(data.toString())) | |
| } catch (err) { | |
| logger.error('❌ Error parsing message:', err) | |
| } | |
| }) | |
| this.ws.on('error', (err: Error) => { | |
| logger.error('❌ WebSocket error:', err) | |
| }) | |
| this.ws.on('close', () => { | |
| logger.warn(`⚠️ Disconnected from ${endpoint}`) | |
| this.currentIndex = (this.currentIndex + 1) % this.endpoints.length | |
| setTimeout(() => this.connect(), this.reconnectTimeout) | |
| }) | |
| } |
| export class WebSocketClient { | ||
| private endpoints: string[] | ||
|
|
||
| private currentIndex = 0 | ||
|
|
||
| private ws: WebSocket | null = null | ||
|
|
||
| private reconnectTimeout = 3000 | ||
|
|
||
| private messageHandler: MessageHandler | ||
|
|
||
| constructor(endpoints: string[], messageHandler: MessageHandler) { | ||
| this.endpoints = endpoints | ||
| this.messageHandler = messageHandler | ||
| this.connect() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add method to explicitly disconnect WebSocket
The WebSocketClient class doesn't provide a way to explicitly close the connection, which could lead to resources not being properly released when the client is no longer needed.
Add a disconnect method to properly clean up resources:
export class WebSocketClient {
private endpoints: string[]
private currentIndex = 0
private ws: WebSocket | null = null
private reconnectTimeout = 3000
+ private shouldReconnect = true
private messageHandler: MessageHandler
constructor(endpoints: string[], messageHandler: MessageHandler) {
this.endpoints = endpoints
this.messageHandler = messageHandler
this.connect()
}
+ public disconnect(): void {
+ this.shouldReconnect = false
+ if (this.ws) {
+ this.ws.close()
+ this.ws = null
+ }
+ }Then update the close handler to respect the reconnection flag:
this.ws.on('close', () => {
logger.warn(`⚠️ Disconnected from ${endpoint}`)
this.currentIndex = (this.currentIndex + 1) % this.endpoints.length
- setTimeout(() => this.connect(), this.reconnectTimeout)
+ if (this.shouldReconnect) {
+ setTimeout(() => this.connect(), this.reconnectTimeout)
+ }
})| switch (data.type) { | ||
| case 'tendermint/event/NewBlock': | ||
| await handleNewBlock(data.value as NewBlockEvent['value']) | ||
| break | ||
| case 'tendermint/event/Tx': | ||
| await handleNewTx(data.value as NewTxEvent['value']) | ||
| break | ||
| default: | ||
| logger.warn('📦 Unhandled message type:', data.type) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for async event handlers.
The function awaits event handlers but doesn't include try/catch blocks to handle potential errors.
switch (data.type) {
case 'tendermint/event/NewBlock':
+ try {
await handleNewBlock(data.value as NewBlockEvent['value'])
+ } catch (error) {
+ logger.error(`❌ Error handling NewBlock event: ${error.message}`, error)
+ }
break
case 'tendermint/event/Tx':
+ try {
await handleNewTx(data.value as NewTxEvent['value'])
+ } catch (error) {
+ logger.error(`❌ Error handling Tx event: ${error.message}`, error)
+ }
break
default:
logger.warn('📦 Unhandled message type:', data.type)
}📝 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.
| switch (data.type) { | |
| case 'tendermint/event/NewBlock': | |
| await handleNewBlock(data.value as NewBlockEvent['value']) | |
| break | |
| case 'tendermint/event/Tx': | |
| await handleNewTx(data.value as NewTxEvent['value']) | |
| break | |
| default: | |
| logger.warn('📦 Unhandled message type:', data.type) | |
| } | |
| } | |
| switch (data.type) { | |
| case 'tendermint/event/NewBlock': | |
| try { | |
| await handleNewBlock(data.value as NewBlockEvent['value']) | |
| } catch (error) { | |
| logger.error(`❌ Error handling NewBlock event: ${error.message}`, error) | |
| } | |
| break | |
| case 'tendermint/event/Tx': | |
| try { | |
| await handleNewTx(data.value as NewTxEvent['value']) | |
| } catch (error) { | |
| logger.error(`❌ Error handling Tx event: ${error.message}`, error) | |
| } | |
| break | |
| default: | |
| logger.warn('📦 Unhandled message type:', data.type) | |
| } | |
| } |
Summary by CodeRabbit
New Features
Chores
Refactor