Kasumi3 continued work #175
Conversation
…ments - Calculate required balance as shortfall plus 10% buffer - Update error message to show detailed balance requirements - Prevent transactions when balance is insufficient for buffered stake
- Moved /status logic into handleStatus method - Updated /kasumi command to use handleStatus - Dynamic examples in help using predefined prompts - Removed duplicate /status command registration
- Integrate payment system with user wallet linking and AIUS token deposits - Implement dynamic gas estimation with configurable buffer percentages - Add user balance validation before task processing - Include random reward system for completed tasks - Display real-time progress updates during task processing - Add comprehensive test coverage for gas estimation logic - Handle task refunds and balance adjustments automatically
…dling - Added extensive test suites for BlockchainService, DepositMonitor, HealthCheckServer, and related components - Implemented edge case testing for transaction processing, deposit handling, and input validation - Added tests for IPFS integration including timeout and retry scenarios - Expanded coverage for task processing edge cases and payment finalization flows - Included unicode, special character, and boundary condition testing - Added concurrent access and error recovery test scenarios
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntegrates a payment system and Telegram-aware rewards/notifications into Kasumi3, centralizes constants (including gas buffer and rewards), implements buffered dynamic gas estimation with fallbacks, propagates telegramId and progress through TaskJob lifecycle, adds bot-backed deposit notifications, expands tests, and bumps several deps and CI actions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Telegram User
participant T as Telegraf Bot
participant K as Kasumi3Bot
participant P as TaskProcessor
participant B as BlockchainService
participant US as UserService
participant PS as PaymentSystem
U->>T: /task <prompt>
T->>K: update/command
K->>US: ensure wallet / validate
K->>P: submitAndQueueTask({prompt, telegramId})
P->>B: submitTask(...) (estimateGasWithBuffer)
B->>B: estimateGasWithBuffer → gasLimit (estimator or FALLBACK)
B-->>P: tx hash/receipt
P->>K: progress updates (caption edits)
P->>B: signalCommitment -> submitSolution (uses gasLimit)
B-->>P: receipt
alt success
P->>US: maybe adminCredit (REWARD_CHANCE)
P-->>K: completed {wonReward?}
K-->>U: result + optional WINNER_IMAGE_URL
else failure
P->>US: refundTask
P-->>K: failed
K-->>U: failure + refund notice
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
- Upgrade actions/checkout from v4 to v5 - Upgrade actions/setup-node from v4 to v5 This ensures compatibility with the latest features and security updates in GitHub Actions while maintaining existing workflow functionality.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
bots/kasumi-3/tests/services/InputValidation.test.ts (2)
472-482: Consider simplifying the address comparison.The assertion on line 481 uses
ethers.getAddresson both sides, which works but is verbose. Since the database stores lowercase addresses, a simpler comparison would be more direct.- expect(ethers.getAddress(result2.user?.wallet_address!)).toBe(ethers.getAddress(address2)); + expect(result2.user?.wallet_address).toBe(address2.toLowerCase());
691-733: Thorough length limit testing.The tests verify that prompts of various lengths (up to 100k characters) are accepted and preserved correctly. Note that no upper length limit is currently enforced by the validation logic - consider whether downstream systems (IPFS upload, model processing) might benefit from explicit length constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
bots/kasumi-3/.env.example(1 hunks)bots/kasumi-3/.gitignore(1 hunks)bots/kasumi-3/src/index.ts(11 hunks)bots/kasumi-3/src/services/BlockchainService.ts(7 hunks)bots/kasumi-3/src/services/TaskProcessor.ts(6 hunks)bots/kasumi-3/src/types/index.ts(1 hunks)bots/kasumi-3/tests/integration/errorRecovery.test.ts(1 hunks)bots/kasumi-3/tests/services/BlockchainService.comprehensive.test.ts(1 hunks)bots/kasumi-3/tests/services/BlockchainService.test.ts(1 hunks)bots/kasumi-3/tests/services/DepositMonitor.comprehensive.test.ts(1 hunks)bots/kasumi-3/tests/services/DynamicGasEstimation.README.md(1 hunks)bots/kasumi-3/tests/services/DynamicGasEstimation.test.ts(1 hunks)bots/kasumi-3/tests/services/HealthCheckServer.comprehensive.test.ts(1 hunks)bots/kasumi-3/tests/services/InputValidation.test.ts(1 hunks)bots/kasumi-3/tests/services/TaskProcessor.test.ts(4 hunks)bots/kasumi-3/tests/services/ipfs.comprehensive.test.ts(1 hunks)bots/kasumi-3/tests/services/ipfs.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
bots/kasumi-3/tests/services/BlockchainService.test.ts (1)
bots/kasumi-3/src/services/BlockchainService.ts (2)
BlockchainService(9-444)estimateGasWithBuffer(115-130)
bots/kasumi-3/tests/services/InputValidation.test.ts (2)
bots/kasumi-3/src/services/DatabaseService.ts (1)
DatabaseService(55-455)bots/kasumi-3/src/services/UserService.ts (1)
UserService(8-558)
bots/kasumi-3/tests/services/HealthCheckServer.comprehensive.test.ts (1)
bots/kasumi-3/src/services/HealthCheckServer.ts (1)
HealthCheckServer(20-205)
bots/kasumi-3/tests/services/BlockchainService.comprehensive.test.ts (1)
bots/kasumi-3/src/services/BlockchainService.ts (3)
BlockchainService(9-444)getNonceWithRetry(78-110)executeTransaction(135-176)
bots/kasumi-3/tests/services/DynamicGasEstimation.test.ts (1)
bots/kasumi-3/src/services/BlockchainService.ts (2)
BlockchainService(9-444)estimateGasWithBuffer(115-130)
bots/kasumi-3/tests/services/TaskProcessor.test.ts (1)
bots/kasumi-3/src/services/TaskProcessor.ts (1)
TaskProcessor(15-331)
bots/kasumi-3/tests/integration/errorRecovery.test.ts (4)
bots/kasumi-3/src/services/BlockchainService.ts (1)
BlockchainService(9-444)bots/kasumi-3/src/services/JobQueue.ts (1)
JobQueue(6-211)bots/kasumi-3/src/services/UserService.ts (1)
UserService(8-558)bots/kasumi-3/src/services/DatabaseService.ts (1)
DatabaseService(55-455)
bots/kasumi-3/tests/services/DepositMonitor.comprehensive.test.ts (3)
bots/kasumi-3/src/services/DepositMonitor.ts (1)
DepositMonitor(10-206)bots/kasumi-3/src/services/UserService.ts (1)
UserService(8-558)bots/kasumi-3/src/services/DatabaseService.ts (1)
DatabaseService(55-455)
bots/kasumi-3/src/index.ts (5)
bots/kasumi-3/src/services/BlockchainService.ts (1)
BlockchainService(9-444)bots/kasumi-3/src/services/ModelRegistry.ts (1)
ModelRegistry(6-126)bots/kasumi-3/src/services/JobQueue.ts (1)
JobQueue(6-211)bots/kasumi-3/src/services/TaskProcessor.ts (1)
TaskProcessor(15-331)bots/kasumi-3/src/initPaymentSystem.ts (1)
initializePaymentSystem(43-89)
bots/kasumi-3/tests/services/ipfs.comprehensive.test.ts (1)
bots/kasumi-3/src/ipfs.ts (1)
fetchFromIPFS(29-62)
🪛 dotenv-linter (3.3.0)
bots/kasumi-3/.env.example
[warning] 30-30: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 31-31: [UnorderedKey] The REWARD_AMOUNT key should go before the REWARD_CHANCE key
(UnorderedKey)
[warning] 31-31: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 32-32: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 35-35: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🪛 markdownlint-cli2 (0.18.1)
bots/kasumi-3/tests/services/DynamicGasEstimation.README.md
128-128: 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). (5)
- GitHub Check: lighthouse (website2, http://localhost:3000)
- GitHub Check: coverage (website2)
- GitHub Check: lighthouse (explorer, http://localhost:3001)
- GitHub Check: coverage (explorer)
- GitHub Check: coverage (contract)
🔇 Additional comments (4)
bots/kasumi-3/tests/services/InputValidation.test.ts (3)
7-386: Excellent comprehensive test coverage for input validation.The test suite thoroughly covers required fields, type validation, range checks, enum validation, defaults, edge cases, and special character handling. Well-organized with clear test descriptions.
486-643: Solid command parsing and sanitization tests.Comprehensive coverage of prompt extraction, model name normalization, ID validation, and amount parsing with appropriate edge cases. The tests correctly verify string manipulation, regex patterns, and ethers utility functions.
645-689: Good security-focused validation.The tests correctly verify that potentially malicious input (XSS, SQL injection, null bytes) is stored as-is without execution. The comments appropriately note that SQL injection prevention occurs at the database layer through parameterized queries, which is the correct approach.
bots/kasumi-3/tests/services/ipfs.comprehensive.test.ts (1)
21-167: Excellent expanded coverage on fetchFromIPFS edge cases.The parallel gateway, timeout, and large-payload scenarios significantly strengthen confidence around the new Promise.any implementation and error surfacing. Nicely done.
bots/kasumi-3/src/index.ts
Outdated
| ctx.reply( | ||
| `🔍 Kasumi-3 Status\n\n` + | ||
| `${healthStatus}\n\n` + | ||
| `**Wallet**\n` + | ||
| `Address: \`${address.slice(0, 10)}...${address.slice(-8)}\`\n` + | ||
| `AIUS: ${ethers.formatEther(arbiusBalance)} ${hasEnoughAius ? '✅' : '⚠️'}\n` + | ||
| `ETH: ${ethers.formatEther(ethBalance)} ${hasEnoughGas ? '✅' : '⚠️'}\n` + | ||
| `Staked: ${ethers.formatEther(validatorStaked)} / ${ethers.formatEther(validatorMinimum)} ${isStakedEnough ? '✅' : '⚠️'}\n\n` + | ||
| `**Job Queue**\n` + | ||
| `Total: ${queueStats.total}\n` + | ||
| `Pending: ${queueStats.pending}\n` + | ||
| `Processing: ${queueStats.processing} ${queueHealthy ? '✅' : '⚠️'}\n` + | ||
| `Completed: ${queueStats.completed}\n` + | ||
| `Failed: ${queueStats.failed}\n\n` + | ||
| `**System**\n` + | ||
| `Uptime: ${uptimeHours}h ${uptimeMinutes % 60}m\n` + | ||
| `Active Users: ${rateLimiterStats.activeUsers}\n` + | ||
| `Models: ${this.modelRegistry.getAllModels().length}\n` + | ||
| `Rate Limit: ${rateLimiterStats.config.maxRequests} req/${rateLimiterStats.config.windowMs / 1000}s` + | ||
| warningsText, | ||
| { parse_mode: 'Markdown' } |
There was a problem hiding this comment.
Fix Markdown formatting in /kasumi status reply
We send the status block with parse_mode: 'Markdown', but the text uses **bold** markers. Telegram’s legacy Markdown parser only understands single *bold*, so the user will literally see **Wallet**, **Job Queue**, etc. Please switch to single-asterisk formatting (or upgrade to MarkdownV2 + proper escaping) so the diagnostics render correctly for users.
🤖 Prompt for AI Agents
In bots/kasumi-3/src/index.ts around lines 292 to 312, the status reply uses
double-asterisk bold markers while sending parse_mode: 'Markdown' (Telegram
legacy), which causes the `**bold**` to render verbatim; change all
double-asterisk bold markers to single asterisks (e.g. `**Wallet**` ->
`*Wallet*`, `**Job Queue**` -> `*Job Queue*`, `**System**` -> `*System*`) so
headings render correctly, or alternatively set parse_mode to 'MarkdownV2' and
properly escape all special characters — pick one approach and apply
consistently across the template.
| // Random reward system: 1 in X chance to win reward | ||
| if (this.userService && job.chatId) { | ||
| const rewardChance = parseInt(process.env.REWARD_CHANCE || '20'); // Default 1 in 20 | ||
| const rewardAmount = ethers.parseEther(process.env.REWARD_AMOUNT || '1'); // Default 1 AIUS | ||
|
|
||
| const randomNum = Math.floor(Math.random() * rewardChance); | ||
| if (randomNum === 0) { | ||
| // Winner! | ||
| const telegramId = (job as any).telegramId; | ||
| if (telegramId) { | ||
| const success = this.userService.adminCredit( | ||
| telegramId, | ||
| rewardAmount, | ||
| `Lucky reward for task ${job.taskid}` | ||
| ); | ||
| if (success) { | ||
| log.info(`🎉 User ${telegramId} won ${ethers.formatEther(rewardAmount)} AIUS reward!`); | ||
| this.jobQueue.updateJobStatus(job.id, 'completed', { cid, wonReward: true }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard against zero/invalid reward chance before handing out credits
Math.floor(Math.random() * rewardChance) will always yield 0 whenever REWARD_CHANCE resolves to 0 (a very common value to disable rewards). That makes the if (randomNum === 0) branch fire on every successful task, silently crediting every user. Please treat non-positive/invalid configurations as “rewards disabled” instead of letting them fall through to the payout logic. While you are here you can rely on the newly typed job.telegramId rather than casting to any.
Apply something along these lines:
- const rewardChance = parseInt(process.env.REWARD_CHANCE || '20'); // Default 1 in 20
- const rewardAmount = ethers.parseEther(process.env.REWARD_AMOUNT || '1'); // Default 1 AIUS
-
- const randomNum = Math.floor(Math.random() * rewardChance);
- if (randomNum === 0) {
+ const rewardChanceRaw = parseInt(process.env.REWARD_CHANCE || '20', 10); // Default 1 in 20
+ if (!Number.isFinite(rewardChanceRaw) || rewardChanceRaw <= 0) {
+ log.warn(`Skipping reward payout: invalid REWARD_CHANCE="${process.env.REWARD_CHANCE}"`);
+ } else if (Math.floor(Math.random() * rewardChanceRaw) === 0) {
+ const rewardAmount = ethers.parseEther(process.env.REWARD_AMOUNT || '1'); // Default 1 AIUS
+ const telegramId = job.telegramId;
- const telegramId = (job as any).telegramId;
if (telegramId) {This keeps rewards opt-in and prevents accidental “reward flood” misconfiguration, while making the type usage consistent.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bots/kasumi-3/src/services/TaskProcessor.ts around lines 109 to 128, guard
against zero/invalid REWARD_CHANCE and use the typed job.telegramId: validate
rewardChance after parsing (treat NaN or <= 0 as rewards disabled and skip the
random/reward branch entirely), keep the default fallback but ensure parseInt
result is > 0 before calling Math.random(), and use job.telegramId (no any cast)
when calling userService.adminCredit; only call adminCredit and updateJobStatus
when rewardChance is valid and a winner is determined.
| jobQueue = new JobQueue(db); | ||
| }); |
There was a problem hiding this comment.
Fix JobQueue constructor usage
JobQueue expects a numeric maxConcurrent (and optional callbacks/timeouts). Passing the DatabaseService instance as the first argument fails TypeScript checks and, even if coerced, leaves maxConcurrent as [object Object], breaking queue throttling logic. Construct it with a number (or rely on the default) and keep the database separate.
- jobQueue = new JobQueue(db);
+ jobQueue = new JobQueue();📝 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.
| jobQueue = new JobQueue(db); | |
| }); | |
| jobQueue = new JobQueue(); | |
| }); |
🤖 Prompt for AI Agents
In bots/kasumi-3/tests/integration/errorRecovery.test.ts around lines 126-127,
the test calls new JobQueue(db) but JobQueue's first parameter is maxConcurrent
(a number), so passing the DatabaseService breaks TypeScript and queue
throttling; change the constructor call to provide a numeric maxConcurrent (or
omit it to use the default) and assign or inject the DatabaseService separately
(e.g., via a setter or by passing it in the proper options/parameters expected
by JobQueue) so the queue gets a valid number and the DB remains a separate
dependency.
| it('should cache nonce for 5 seconds', async () => { | ||
| const getNonceWithRetry = (blockchain as any).getNonceWithRetry.bind(blockchain); | ||
|
|
||
| // Mock provider getTransactionCount | ||
| const mockGetTxCount = vi.fn().mockResolvedValue(5); | ||
| vi.spyOn((blockchain as any).provider, 'getBlockNumber').mockResolvedValue(1000); | ||
|
|
||
| // First call should query RPC | ||
| const nonceCache = (blockchain as any).nonceCache; | ||
| expect(nonceCache).toBeNull(); | ||
|
|
||
| // Manually set up the mock for multiple RPC calls | ||
| const originalRpcUrls = (blockchain as any).rpcUrls; | ||
| expect(originalRpcUrls).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should increment cached nonce on subsequent calls', async () => { | ||
| // Set up nonce cache manually | ||
| (blockchain as any).nonceCache = { nonce: 10, timestamp: Date.now() }; | ||
|
|
||
| const getNonceWithRetry = (blockchain as any).getNonceWithRetry.bind(blockchain); | ||
|
|
||
| const nonce1 = await getNonceWithRetry(); | ||
| expect(nonce1).toBe(10); | ||
|
|
||
| const nonce2 = await getNonceWithRetry(); | ||
| expect(nonce2).toBe(11); | ||
|
|
||
| const nonce3 = await getNonceWithRetry(); | ||
| expect(nonce3).toBe(12); | ||
| }); | ||
|
|
||
| it('should clear cache after 5 seconds', async () => { | ||
| const fiveSecondsAgo = Date.now() - 6000; | ||
| (blockchain as any).nonceCache = { nonce: 5, timestamp: fiveSecondsAgo }; | ||
|
|
||
| const getNonceWithRetry = (blockchain as any).getNonceWithRetry.bind(blockchain); | ||
|
|
||
| // Should re-query because cache is stale | ||
| // This will fail in the test because we can't mock the RPC properly | ||
| // but demonstrates the cache expiry logic | ||
| const cache = (blockchain as any).nonceCache; | ||
| const isExpired = Date.now() - cache.timestamp >= 5000; | ||
| expect(isExpired).toBe(true); | ||
| }); | ||
|
|
||
| it('should use highest nonce from multiple RPCs', async () => { | ||
| const multiRpcBlockchain = new BlockchainService( | ||
| 'http://rpc1.com,http://rpc2.com,http://rpc3.com', | ||
| TEST_PRIVATE_KEY, | ||
| TEST_ARBIUS_ADDRESS, | ||
| TEST_ROUTER_ADDRESS, | ||
| TEST_TOKEN_ADDRESS | ||
| ); | ||
|
|
||
| const rpcUrls = (multiRpcBlockchain as any).rpcUrls; | ||
| expect(rpcUrls).toHaveLength(3); | ||
| expect(rpcUrls[0]).toBe('http://rpc1.com'); | ||
| expect(rpcUrls[1]).toBe('http://rpc2.com'); | ||
| expect(rpcUrls[2]).toBe('http://rpc3.com'); | ||
| }); | ||
|
|
||
| it('should handle RPC failure when getting nonce', async () => { | ||
| (blockchain as any).nonceCache = null; | ||
|
|
||
| const getNonceWithRetry = (blockchain as any).getNonceWithRetry.bind(blockchain); | ||
|
|
||
| // This tests that the error handling exists | ||
| // In a real scenario, at least one RPC should succeed | ||
| expect(getNonceWithRetry).toBeDefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Nonce cache tests never exercise the implementation
Both “should cache nonce for 5 seconds” and “should clear cache after 5 seconds” stop after grabbing getNonceWithRetry/nonceCache and inspecting fields; they never invoke the method or assert its behaviour. As written they will succeed even if getNonceWithRetry is completely broken, which gives us a false sense of coverage over the nonce cache/regression path. Please refactor these tests to actually drive getNonceWithRetry (e.g., by mocking ethers.JsonRpcProvider/getTransactionCount) or remove them until we can validate the behaviour for real.
| // DB stores checksummed address | ||
| expect(ethers.getAddress(result.user?.wallet_address!)).toBe(ethers.getAddress(lowercaseAddress)); | ||
| }); |
There was a problem hiding this comment.
Misleading comment about address storage format.
The comment states "DB stores checksummed address" but according to the DatabaseService implementation, addresses are stored in lowercase (via .toLowerCase() in upsertUser). The assertion works because ethers.getAddress normalizes both sides to checksummed format, but it doesn't verify the actual storage format in the database.
Consider updating the comment and assertion to be more precise:
- // DB stores checksummed address
- expect(ethers.getAddress(result.user?.wallet_address!)).toBe(ethers.getAddress(lowercaseAddress));
+ // DB stores lowercase address; verify it matches the input (case-insensitive)
+ expect(result.user?.wallet_address).toBe(lowercaseAddress.toLowerCase());📝 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.
| // DB stores checksummed address | |
| expect(ethers.getAddress(result.user?.wallet_address!)).toBe(ethers.getAddress(lowercaseAddress)); | |
| }); | |
| // DB stores lowercase address; verify it matches the input (case-insensitive) | |
| expect(result.user?.wallet_address).toBe(lowercaseAddress.toLowerCase()); | |
| }); |
🤖 Prompt for AI Agents
In bots/kasumi-3/tests/services/InputValidation.test.ts around lines 436 to 438,
the comment incorrectly says "DB stores checksummed address" but the
DatabaseService lowercases addresses; update the comment to "DB stores lowercase
address" and change the assertion to verify the stored DB value is lowercase
(e.g. compare result.user?.wallet_address to lowercaseAddress.toLowerCase())
instead of normalizing both sides with ethers.getAddress so the test asserts the
actual storage format.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/coverage.yml (1)
16-25: Same issue: v5 tags are unavailableThis workflow has the same problem:
actions/checkout@v5andactions/setup-node@v5are not published, so the run will error out before your jobs start. Revert to@v4(or pin commits) to keep CI green.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
bots/ipfs-oracle/package-lock.jsonis excluded by!**/package-lock.jsonbots/kasumi-3/package-lock.jsonis excluded by!**/package-lock.jsonexplorer/package-lock.jsonis excluded by!**/package-lock.jsonwebsite2/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/coverage.yml(1 hunks).github/workflows/lighthouse.yml(1 hunks)bots/ipfs-oracle/package.json(1 hunks)bots/ipfs-oracle/src/index.js(4 hunks)bots/ipfs-oracle/tests/main.test.js(1 hunks)bots/ipfs-oracle/tests/port-timeout.test.js(1 hunks)bots/ipfs-oracle/tests/startup.test.js(1 hunks)bots/kasumi-3/package.json(1 hunks)explorer/package.json(2 hunks)website2/package.json(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
bots/ipfs-oracle/tests/startup.test.js (2)
bots/ipfs-oracle/tests/main.test.js (1)
server(5-5)bots/ipfs-oracle/src/index.js (4)
app(52-52)app(52-52)autoStart(102-106)port(12-12)
bots/ipfs-oracle/tests/main.test.js (1)
bots/ipfs-oracle/src/index.js (4)
app(52-52)app(52-52)port(12-12)main(55-62)
⏰ 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). (3)
- GitHub Check: finish
- GitHub Check: lighthouse (website2, http://localhost:3000)
- GitHub Check: lighthouse (explorer, http://localhost:3001)
🔇 Additional comments (1)
website2/package.json (1)
29-31: Peer dependencies support React 19.2.0 Next 15.5.4 and next-themes both allow React ^19, so the bump is safe.
| uses: actions/checkout@v5 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| uses: actions/setup-node@v5 | ||
| with: |
There was a problem hiding this comment.
Broken by nonexistent v5 actions
actions/checkout@v5 and actions/setup-node@v5 do not exist (latest is v4), so the workflow will fail when GitHub tries to resolve these tags. Please stick with the published v4 release (or pin a specific commit) until v5 is available.
🤖 Prompt for AI Agents
.github/workflows/lighthouse.yml lines 22-26: the workflow references
nonexistent action versions actions/checkout@v5 and actions/setup-node@v5 which
will cause resolution failures; update those uses to the published v4 releases
(e.g., actions/checkout@v4 and actions/setup-node@v4) or pin to a specific
commit/tag, then test the workflow to ensure the actions resolve successfully.
| export function autoStart() { | ||
| if (process.env.NODE_ENV !== 'test') { | ||
| main(); | ||
| } |
There was a problem hiding this comment.
Preserve the main() promise in autoStart.
Right now we call main() but drop the promise, so any startup failure becomes an unhandled rejection and callers (including the new tests) cannot reliably await completion. Return the promise so errors surface and await autoStart() actually waits.
export function autoStart() {
if (process.env.NODE_ENV !== 'test') {
- main();
+ return main();
}
}📝 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 function autoStart() { | |
| if (process.env.NODE_ENV !== 'test') { | |
| main(); | |
| } | |
| export function autoStart() { | |
| if (process.env.NODE_ENV !== 'test') { | |
| return main(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In bots/ipfs-oracle/src/index.js around lines 102 to 105, autoStart currently
calls main() without returning its promise which causes unhandled rejections and
prevents callers from awaiting startup; change autoStart to return the promise
from main() when starting (and return a resolved promise when skipping start for
NODE_ENV === 'test') so callers can await autoStart() and receive any errors.
| }, | ||
| "devDependencies": { | ||
| "@types/uuid": "^11.0.0", | ||
| "@types/uuid": "^10.0.0", |
There was a problem hiding this comment.
Revert the @types/uuid downgrade
Runtime uuid remains at ^13.0.0, but @types/uuid was rolled back to ^10.0.0. That version predates the pure-ESM/inline typings shipped with uuid@13 and still declares the old CommonJS default export. Keeping it alongside uuid@13 resurrects conflicting ambient declarations and breaks TypeScript builds (TS2307/TS1259). Please either drop the external typings altogether (preferred—uuid already ships its own) or restore the prior @types/uuid version that matches the ESM signature.
🤖 Prompt for AI Agents
In bots/kasumi-3/package.json around line 37, the project lists "@types/uuid":
"^10.0.0" while runtime "uuid" is still ^13.0.0, which reintroduces conflicting
CommonJS ambient typings and breaks TypeScript; remove the external
"@types/uuid" entry (preferred) so the built-in ESM/inline typings from uuid@13
are used, or alternatively restore the previously working @types/uuid version
that explicitly matches uuid@13's ESM typings, then reinstall deps (npm/yarn)
and verify TypeScript builds cleanly.
Improve test reliability by properly managing the window.ethereum mock lifecycle. Only the ethereum property is deleted during cleanup instead of the entire window object, preventing potential interference with other tests that might rely on window properties.
…constants This commit introduces a centralized constants module to manage application-wide configuration values. Key changes include: - Moving hardcoded timeouts, rate limits, and other configuration values to src/constants.ts - Refactoring BlockchainService, DepositMonitor, TaskProcessor and other services to use constants - Updating IPFS gateway configuration and reward system parameters - Improving type safety with const assertions for constant objects The refactoring improves maintainability by centralizing configuration values and reducing hardcoded magic numbers throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bots/kasumi-3/src/index.ts (1)
87-103: Temporary null assignment creates unsafe stateLine 90 assigns
null as anytojobQueuein theTaskProcessorconstructor, then line 103 reassigns it via(taskProcessor as any).jobQueue = jobQueue. This creates a temporary state wheretaskProcessorhas an invalidjobQueuereference.This pattern is fragile and could cause issues if
TaskProcessormethods are called before line 103. Consider refactoring to avoid the circular dependency:Option 1: Use a setter method
// In TaskProcessor setJobQueue(jobQueue: JobQueue): void { this.jobQueue = jobQueue; }Option 2: Initialize TaskProcessor after JobQueue
If theTaskProcessordoesn't need to be passed toJobQueueconstructor, reorder the initialization:const jobQueue = new JobQueue(maxConcurrent, undefined, jobTimeoutMs); const taskProcessor = new TaskProcessor( blockchain, miningConfig, jobQueue, paymentSystem.userService, paymentSystem.gasAccounting ); // Set the processor callback after both are initialized jobQueue.setOnJobComplete(async (job: TaskJob) => { try { await taskProcessor.processTask(job); } catch (err: any) { log.error(`Failed to process job ${job.id}: ${err.message}`); } });This would require changes to
JobQueueto accept an optional callback and allow setting it after construction.
♻️ Duplicate comments (2)
bots/kasumi-3/src/services/TaskProcessor.ts (1)
111-129: Prevent invalid reward chance from auto-paying every taskA
REWARD_CHANCEof0(common to disable rewards) makesMath.floor(Math.random() * rewardChance)always0, so every successful task is paid out. This was already flagged earlier; please treat non‑positive/invalid configs as “rewards disabled” and only draw a random winner when the chance is valid. While adjusting, you can rely on the typedjob.telegramIdinstead of casting.- const rewardChance = parseInt(process.env.REWARD_CHANCE || String(REWARDS.CHANCE_DEFAULT)); - const rewardAmount = ethers.parseEther(process.env.REWARD_AMOUNT || REWARDS.AMOUNT_DEFAULT); - - const randomNum = Math.floor(Math.random() * rewardChance); - if (randomNum === 0) { - // Winner! - const telegramId = (job as any).telegramId; - if (telegramId) { - const success = this.userService.adminCredit( - telegramId, - rewardAmount, - `Lucky reward for task ${job.taskid}` - ); + const rewardChanceRaw = parseInt(process.env.REWARD_CHANCE || String(REWARDS.CHANCE_DEFAULT), 10); + if (!Number.isFinite(rewardChanceRaw) || rewardChanceRaw <= 0) { + log.warn(`Skipping reward payout: invalid REWARD_CHANCE="${process.env.REWARD_CHANCE}"`); + } else if (Math.floor(Math.random() * rewardChanceRaw) === 0) { + const rewardAmount = ethers.parseEther(process.env.REWARD_AMOUNT || REWARDS.AMOUNT_DEFAULT); + const telegramId = job.telegramId; + if (telegramId) { + const success = this.userService.adminCredit( + telegramId, + rewardAmount, + `Lucky reward for task ${job.taskid}` + );bots/kasumi-3/src/bot/Kasumi3Bot.ts (1)
289-310: Fix Markdown formatting in status replyThe status reply uses double-asterisk
**bold**markers withparse_mode: 'Markdown'. Telegram's legacy Markdown parser only recognizes single asterisks for bold (*text*), so the double asterisks will render literally as**Wallet**,**Job Queue**, etc.Apply this diff to fix the formatting:
ctx.reply( `🔍 Kasumi-3 Status\n\n` + `${healthStatus}\n\n` + - `**Wallet**\n` + + `*Wallet*\n` + `Address: \`${address.slice(0, 10)}...${address.slice(-8)}\`\n` + `AIUS: ${ethers.formatEther(arbiusBalance)} ${hasEnoughAius ? '✅' : '⚠️'}\n` + `ETH: ${ethers.formatEther(ethBalance)} ${hasEnoughGas ? '✅' : '⚠️'}\n` + `Staked: ${ethers.formatEther(validatorStaked)} / ${ethers.formatEther(validatorMinimum)} ${isStakedEnough ? '✅' : '⚠️'}\n\n` + - `**Job Queue**\n` + + `*Job Queue*\n` + `Total: ${queueStats.total}\n` + `Pending: ${queueStats.pending}\n` + `Processing: ${queueStats.processing} ${queueHealthy ? '✅' : '⚠️'}\n` + `Completed: ${queueStats.completed}\n` + `Failed: ${queueStats.failed}\n\n` + - `**System**\n` + + `*System*\n` + `Uptime: ${uptimeHours}h ${uptimeMinutes % 60}m\n` + `Active Users: ${rateLimiterStats.activeUsers}\n` + `Models: ${this.modelRegistry.getAllModels().length}\n` +
🧹 Nitpick comments (3)
bots/kasumi-3/src/bot/Kasumi3Bot.ts (2)
64-81: Consider extracting rate limit response messageThe rate limit error message is constructed inline. Consider extracting it to a constant or helper function for consistency with other messages and easier maintenance.
You could extract the message construction to a helper:
+ const formatRateLimitMessage = (resetTime: number, limit: string) => + `⏱️ Rate limit exceeded. Please wait ${resetTime} seconds before trying again.\n\n` + + `Limit: ${limit}`; + this.bot.use(async (ctx, next) => { const userId = ctx.from?.id; if (!userId) { return next(); } if (!this.rateLimiter.checkLimit(userId)) { const resetTime = this.rateLimiter.getResetTime(userId); log.debug(`User ${userId} rate limited, ${resetTime}s until reset`); - await ctx.reply( - `⏱️ Rate limit exceeded. Please wait ${resetTime} seconds before trying again.\n\n` + - `Limit: ${RATE_LIMITS.REQUESTS_PER_MINUTE_TEXT}` - ); + await ctx.reply(formatRateLimitMessage(resetTime, RATE_LIMITS.REQUESTS_PER_MINUTE_TEXT)); return; }
206-212: Fallback error handling could be more specificThe catch block at line 209 logs the error but uses a generic text fallback message. Consider providing more context about why the photo failed (e.g., network issue, invalid URL, Telegram API limits).
} catch (e) { - log.debug(`Failed to send initial photo, using text fallback: ${e}`); + log.debug(`Failed to send initial photo (${e instanceof Error ? e.message : String(e)}), using text fallback`); responseCtx = await ctx.reply(`🔄 Processing with ${modelConfig.template.meta.title}...`); }bots/kasumi-3/src/services/BlockchainService.ts (1)
226-240: Stake buffer calculation has potential precision lossLine 232-233 compute the
requiredBalancewith a buffer, but the arithmetic could lose precision due to the intermediate conversion toMath.floor()and BigInt operations:const bufferMultiplier = 1 + (BLOCKCHAIN.STAKE_BUFFER_PERCENT / 100); const requiredBalance = shortfall * BigInt(Math.floor(bufferMultiplier * 100)) / 100n;With
STAKE_BUFFER_PERCENT = 10, this becomes:
bufferMultiplier = 1.1Math.floor(1.1 * 100) = 110requiredBalance = shortfall * 110n / 100nThis is correct, but consider whether the precision is sufficient for all cases.
For better clarity and avoiding floating-point operations, consider:
- const bufferMultiplier = 1 + (BLOCKCHAIN.STAKE_BUFFER_PERCENT / 100); - const requiredBalance = shortfall * BigInt(Math.floor(bufferMultiplier * 100)) / 100n; + // Calculate required balance with buffer: shortfall * (100 + buffer%) / 100 + const requiredBalance = shortfall * BigInt(100 + BLOCKCHAIN.STAKE_BUFFER_PERCENT) / 100n;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
bots/kasumi-3/src/bot/Kasumi3Bot.ts(1 hunks)bots/kasumi-3/src/constants.ts(1 hunks)bots/kasumi-3/src/index.ts(3 hunks)bots/kasumi-3/src/initPaymentSystem.ts(2 hunks)bots/kasumi-3/src/services/BlockchainService.ts(13 hunks)bots/kasumi-3/src/services/DepositMonitor.ts(3 hunks)bots/kasumi-3/src/services/ModelHandler.ts(3 hunks)bots/kasumi-3/src/services/TaskProcessor.ts(11 hunks)bots/kasumi-3/tests/services/initPaymentSystem.test.ts(1 hunks)website2/src/lib/arbius-wallet/__tests__/components/AAWalletProvider.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
bots/kasumi-3/src/services/DepositMonitor.ts (2)
bots/kasumi-3/src/services/UserService.ts (1)
UserService(8-558)bots/kasumi-3/src/constants.ts (1)
DEPOSIT_MONITOR(54-57)
bots/kasumi-3/src/services/ModelHandler.ts (1)
bots/kasumi-3/src/constants.ts (1)
TIMEOUTS(6-13)
bots/kasumi-3/src/initPaymentSystem.ts (2)
bots/kasumi-3/src/services/DepositMonitor.ts (1)
DepositMonitor(12-227)bots/kasumi-3/src/constants.ts (1)
DEPOSIT_MONITOR(54-57)
bots/kasumi-3/src/services/BlockchainService.ts (1)
bots/kasumi-3/src/constants.ts (1)
BLOCKCHAIN(30-44)
bots/kasumi-3/src/bot/Kasumi3Bot.ts (9)
bots/kasumi-3/src/services/BlockchainService.ts (1)
BlockchainService(10-467)bots/kasumi-3/src/services/ModelRegistry.ts (1)
ModelRegistry(6-126)bots/kasumi-3/src/services/JobQueue.ts (1)
JobQueue(6-211)bots/kasumi-3/src/services/TaskProcessor.ts (1)
TaskProcessor(16-342)bots/kasumi-3/src/types/index.ts (3)
MiningConfig(4-16)ModelConfig(87-93)TaskJob(104-120)bots/kasumi-3/src/services/RateLimiter.ts (2)
RateLimiter(16-139)cleanup(100-114)bots/kasumi-3/src/services/HealthCheckServer.ts (1)
HealthCheckServer(20-205)bots/kasumi-3/src/services/UserService.ts (1)
UserService(8-558)bots/kasumi-3/src/constants.ts (7)
RATE_LIMITS(16-20)STARTUP(73-75)ARBIUS(83-85)HEALTH_CHECK(47-51)TIMEOUTS(6-13)REWARDS(60-64)JOB_QUEUE(23-27)
bots/kasumi-3/src/index.ts (5)
bots/kasumi-3/src/constants.ts (2)
JOB_QUEUE(23-27)TIMEOUTS(6-13)bots/kasumi-3/src/config.ts (1)
ConfigLoader(9-100)bots/kasumi-3/src/initPaymentSystem.ts (1)
initializePaymentSystem(44-92)bots/kasumi-3/src/services/TaskProcessor.ts (1)
TaskProcessor(16-342)bots/kasumi-3/src/bot/Kasumi3Bot.ts (1)
Kasumi3Bot(21-605)
bots/kasumi-3/src/services/TaskProcessor.ts (1)
bots/kasumi-3/src/constants.ts (2)
REWARDS(60-64)GAS_ESTIMATION(67-70)
⏰ 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). (3)
- GitHub Check: coverage (contract)
- GitHub Check: lighthouse (explorer, http://localhost:3001)
- GitHub Check: lighthouse (website2, http://localhost:3000)
🔇 Additional comments (19)
website2/src/lib/arbius-wallet/__tests__/components/AAWalletProvider.test.tsx (2)
22-27: LGTM! Improved test isolation.The change to preserve the existing
windowobject instead of replacing it entirely prevents potential side effects between tests and follows defensive programming best practices. The patternwindow = window || {}ensures safe initialization while preserving any existing properties.
30-35: LGTM! Precise teardown matches setup.The targeted cleanup of only
window.ethereumis consistent with the setup changes and prevents inadvertently removing other window properties that may be needed by subsequent tests. The safety check adds defensive robustness.bots/kasumi-3/src/bot/Kasumi3Bot.ts (6)
1-17: LGTM: Clean imports with proper organizationThe imports are well-organized with clear separation between external dependencies (telegraf, ethers, axios) and internal modules (services, utils, types, constants).
502-511: IPFS verification retry logic is solidGood use of
expretrywith 3 retries and exponential backoff to handle transient IPFS gateway issues. The timeout of 60 seconds per attempt is reasonable for large files.
549-580: Health check server initialization is well-guardedThe health check server is only started if
HEALTH_CHECK_PORTis configured and greater than 0. This is a good defensive pattern that avoids exposing the health endpoint unnecessarily.
567-574: Graceful shutdown handlers properly registeredThe shutdown logic correctly uses
process.once()to prevent duplicate handler execution and includes both SIGINT and SIGTERM for proper signal handling. The cleanup order (bot services → bot.stop) is correct.
34-60: Rate limit configuration precedence confirmedInstantiation in
index.tsomits the optionalrateLimitConfig, so the constructor correctly falls back to environment variables, then toRATE_LIMITSconstants. The intended precedence (parameter > env > constant) is implemented as designed.
139-161: Remove unnecessary conflict warning dynamic model commands run after explicit this.bot.command handlers, so/kasumi,/queue,/submit, and/processare handled first—and there are no built-in/helpor/startcommands.Likely an incorrect or invalid review comment.
bots/kasumi-3/src/constants.ts (1)
1-85: LGTM: Well-organized centralized constantsThe constants file provides excellent centralization of configuration values with:
- Clear logical grouping (TIMEOUTS, RATE_LIMITS, BLOCKCHAIN, etc.)
- Descriptive comments for time-based values
- Proper use of
as constfor type safety- Reasonable default values (15-minute timeouts, 20% gas buffer, 1-hour cleanup intervals)
This centralization will improve maintainability and make it easier to adjust configuration across the application.
Based on learnings about the Telegraf and ethers libraries, these constants align well with recommended practices for timeout values and buffer percentages.
bots/kasumi-3/src/services/BlockchainService.ts (7)
5-5: LGTM: Constants properly imported and appliedThe integration of
BLOCKCHAINconstants from the centralized constants module is clean. The readonly fields ensure immutability of critical configuration values.Also applies to: 18-22
77-109: Robust nonce management with multi-RPC fallbackThe
getNonceWithRetryimplementation is well-designed:
- Uses cached nonce for performance (5-second TTL)
- Queries all RPC endpoints and uses the highest nonce to avoid out-of-sync issues
- Falls back gracefully on individual RPC failures
- Clear error handling if all RPCs fail
114-129: Gas estimation with buffer and fallback is soundThe
estimateGasWithBufferhelper properly:
- Attempts dynamic gas estimation first
- Applies a configurable buffer percentage (default 20%)
- Falls back to predefined limits on estimation failure
- Logs both paths for debugging
This approach balances accuracy with reliability.
319-340: signalCommitment gas estimation follows the patternThe gas estimation for
signalCommitment(lines 322-326) correctly usesestimateGasWithBufferand passes the computedgasLimitto the transaction. The error handling preserves the original behavior of logging and re-throwing.
347-385: submitSolution gas estimation and commitment delay are correctLines 363-368 apply gas estimation to
submitSolution, and line 359 usesBLOCKCHAIN.COMMITMENT_DELAY_MSfor the sleep between commitment signaling and solution submission. This delay helps avoid nonce conflicts.
402-450: findTransactionByTaskId uses proper block lookbackLine 406 correctly uses
BLOCKCHAIN.BLOCK_LOOKBACKto constrain the search window forTaskSubmittedevents, preventing excessive blockchain queries.
257-290: No action needed—inline comment clarifies task vs transaction gas limits The inline comment at line 287 already notes that the 200 000 parameter is for task execution, separate from the transaction gasLimit.bots/kasumi-3/src/index.ts (3)
4-18: Clean import additions for new componentsThe new imports for
Telegraf,initializePaymentSystem,JOB_QUEUE,TIMEOUTS,Kasumi3Bot, andpathare properly organized and necessary for the refactored initialization flow.
106-114: Kasumi3Bot initialization is well-structuredThe
Kasumi3Botconstructor is now called with all necessary services properly initialized, including the pre-initializedbotinstance and payment system'suserService. This design allows for cleaner separation of concerns and easier testing.
68-82: DatabaseService handles directory creation The constructor checks and creates the parentdatadirectory withfs.mkdirSync(dir, { recursive: true })before opening the database.
| if (this.userService && ctx.from?.id) { | ||
| const user = this.userService.getUser(ctx.from.id); | ||
| if (!user) { | ||
| await ctx.reply( | ||
| '❌ Please link your wallet first using:\n/link <wallet_address>\n\n' + | ||
| 'Then deposit AIUS tokens with /deposit' | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const balance = this.userService.getBalance(ctx.from.id); | ||
| // Estimate cost: model fee + gas (~0.5 AIUS) | ||
| const estimatedCost = ethers.parseEther('0.5'); | ||
| if (balance < estimatedCost) { | ||
| await ctx.reply( | ||
| `❌ Insufficient balance\n\n` + | ||
| `Balance: ${ethers.formatEther(balance)} AIUS\n` + | ||
| `Estimated cost: ~${ethers.formatEther(estimatedCost)} AIUS\n\n` + | ||
| `Use /deposit to add funds` | ||
| ); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded gas estimation may be inaccurate
Line 190 uses a hardcoded gas estimate of 0.5 AIUS for user balance checks. This estimate may become inaccurate over time as gas prices fluctuate or model fees change. Consider using the actual gas estimation logic from GasAccountingService or making this configurable.
Consider using the gas estimation service:
- // Estimate cost: model fee + gas (~0.5 AIUS)
- const estimatedCost = ethers.parseEther('0.5');
+ // Get actual gas estimate from GasAccountingService
+ const modelFee = // get from modelConfig or blockchain
+ const gasEstimate = this.userService ? await this.gasAccounting?.estimateGasCost(...) : 0n;
+ const estimatedCost = modelFee + (gasEstimate || ethers.parseEther('0.5'));Note: This requires access to gasAccounting service, which would need to be passed to the constructor.
Committable suggestion skipped: line range outside the PR's diff.
| private async waitForJobCompletion(job: TaskJob, ctx: Context, responseCtx?: any): Promise<void> { | ||
| const maxWaitTime = parseInt(process.env.JOB_WAIT_TIMEOUT_MS || String(TIMEOUTS.JOB_WAIT_DEFAULT)); | ||
| let lastProgress = ''; | ||
|
|
||
| return new Promise((resolve) => { | ||
| const timeout = setTimeout(() => { | ||
| cleanup(); | ||
| ctx.reply(`⏰ Task is taking longer than expected. Check back later with /queue`); | ||
| resolve(); | ||
| }, maxWaitTime); | ||
|
|
||
| const onStatusChange = async (updatedJob: TaskJob) => { | ||
| if (updatedJob.id !== job.id) return; | ||
|
|
||
| // Update progress if changed | ||
| if (updatedJob.progress && updatedJob.progress !== lastProgress && responseCtx) { | ||
| lastProgress = updatedJob.progress; | ||
| try { | ||
| await this.bot.telegram.editMessageCaption( | ||
| responseCtx.chat.id, | ||
| responseCtx.message_id, | ||
| undefined, | ||
| `⏳ ${updatedJob.progress}` | ||
| ); | ||
| } catch (e) { | ||
| log.debug(`Failed to update progress: ${e}`); | ||
| } | ||
| } | ||
|
|
||
| if (updatedJob.status === 'completed' && updatedJob.cid) { | ||
| cleanup(); | ||
| await this.sendCompletedResult(ctx, responseCtx, updatedJob); | ||
| resolve(); | ||
| } else if (updatedJob.status === 'failed') { | ||
| cleanup(); | ||
| const errorMsg = updatedJob.error || 'Unknown error'; | ||
| ctx.reply(`❌ Task failed: ${errorMsg}\n\n💰 Your balance has been refunded`); | ||
| resolve(); | ||
| } | ||
| }; | ||
|
|
||
| const cleanup = () => { | ||
| clearTimeout(timeout); | ||
| this.jobQueue.off('jobStatusChange', onStatusChange); | ||
| }; | ||
|
|
||
| this.jobQueue.on('jobStatusChange', onStatusChange); | ||
|
|
||
| // Check if job is already completed (race condition) | ||
| const currentJob = this.jobQueue.getJob(job.id); | ||
| if (currentJob) { | ||
| if (currentJob.status === 'completed' && currentJob.cid) { | ||
| cleanup(); | ||
| this.sendCompletedResult(ctx, responseCtx, currentJob).then(resolve); | ||
| } else if (currentJob.status === 'failed') { | ||
| cleanup(); | ||
| const errorMsg = currentJob.error || 'Unknown error'; | ||
| ctx.reply(`❌ Task failed: ${errorMsg}\n\n💰 Your balance has been refunded`); | ||
| resolve(); | ||
| } | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Race condition in job completion check
Lines 477-489 attempt to handle a race condition by checking if the job is already completed after setting up the event listener. However, there's still a narrow window between lines 475 (setting up the listener) and 478 (checking current status) where a status change could be missed. The event could fire after line 475 but before the check at line 478, and then the check would see a non-terminal status, leading to waiting indefinitely (until timeout).
While this race is unlikely and protected by the timeout, consider restructuring to check status before attaching the listener:
+ // Check if job is already completed first (before attaching listener)
+ const currentJob = this.jobQueue.getJob(job.id);
+ if (currentJob) {
+ if (currentJob.status === 'completed' && currentJob.cid) {
+ this.sendCompletedResult(ctx, responseCtx, currentJob).then(resolve);
+ return;
+ } else if (currentJob.status === 'failed') {
+ const errorMsg = currentJob.error || 'Unknown error';
+ ctx.reply(`❌ Task failed: ${errorMsg}\n\n💰 Your balance has been refunded`);
+ resolve();
+ return;
+ }
+ }
+
const cleanup = () => {
clearTimeout(timeout);
this.jobQueue.off('jobStatusChange', onStatusChange);
};
this.jobQueue.on('jobStatusChange', onStatusChange);
-
- // Check if job is already completed (race condition)
- const currentJob = this.jobQueue.getJob(job.id);
- if (currentJob) {
- if (currentJob.status === 'completed' && currentJob.cid) {
- cleanup();
- this.sendCompletedResult(ctx, responseCtx, currentJob).then(resolve);
- } else if (currentJob.status === 'failed') {
- cleanup();
- const errorMsg = currentJob.error || 'Unknown error';
- ctx.reply(`❌ Task failed: ${errorMsg}\n\n💰 Your balance has been refunded`);
- resolve();
- }
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bots/kasumi-3/src/bot/Kasumi3Bot.ts around lines 429 to 491, the race exists
between attaching the jobStatusChange listener and the subsequent currentJob
status check; to fix it, first fetch currentJob = this.jobQueue.getJob(job.id)
and if it is terminal (completed with cid or failed) run the corresponding
cleanup/response and resolve immediately; only if currentJob is non-terminal
attach the listener, then re-read currentJob once more immediately after
registering the listener and if it became terminal handle cleanup/response and
resolve — this guarantees no updates are missed between the checks and listener
registration.
1 participant
Summary by CodeRabbit
New Features
Configuration
Improvements
Tests
Chores