feat: added tx orphaning #1101
feat: added tx orphaning #1101
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant ChronikService
participant TransactionService
participant Database
ChronikService->>ChronikService: Receive TX_CONFIRMED event
ChronikService->>Database: Attempt transaction confirmation
Database-->>ChronikService: 404 Not Found Error
ChronikService->>TransactionService: markTransactionsOrphaned(txid)
TransactionService->>Database: UPDATE Transaction SET orphaned=true WHERE hash=txid
Database-->>TransactionService: Confirmation
TransactionService-->>ChronikService: Return
ChronikService->>ChronikService: Log orphaned transaction
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
services/transactionService.ts (1)
632-641: Consider returning the update count for observability.The implementation is correct, but returning the count of affected transactions would aid debugging and logging. Additionally, consider adding a log statement to track orphaning events.
🔎 Proposed enhancement
-export async function markTransactionsOrphaned (hash: string): Promise<void> { - await prisma.transaction.updateMany({ +export async function markTransactionsOrphaned (hash: string): Promise<number> { + const result = await prisma.transaction.updateMany({ where: { hash }, data: { orphaned: true } }) + if (result.count > 0) { + console.log(`[TX] Marked ${result.count} transaction(s) as orphaned for hash ${hash}`) + } + return result.count }services/chronikService.ts (2)
636-644: Consider wrapping the orphan-marking call in a try-catch.If
markTransactionsOrphanedfails (e.g., database connectivity issue), the error will propagate silently since the outer catch block has already handled the 404. This could leave transactions in an inconsistent state without proper logging.🔎 Proposed fix
} catch (e: any) { const msg404 = String(e?.message ?? e) const is404 = /not found in the index|404/.test(msg404) if (is404) { console.log(`${this.CHRONIK_MSG_PREFIX}: [${msg.msgType}] tx ${msg.txid} not found in chronik, marking as orphaned`) - await markTransactionsOrphaned(msg.txid) + try { + await markTransactionsOrphaned(msg.txid) + } catch (orphanErr: any) { + console.error(`${this.CHRONIK_MSG_PREFIX}: failed to mark tx ${msg.txid} as orphaned`, orphanErr) + } } else { console.error(`${this.CHRONIK_MSG_PREFIX}: confirmed tx handler failed for ${msg.txid}`, e) } }
637-638: Consider extracting the 404 detection pattern.The regex
/not found in the index|404/is duplicated (also at line 601). Consider extracting it as a constant or helper function. Also, the variablemsg404is misleading since it holds the full error message, not specifically a 404 message.🔎 Proposed refactor
+const NOT_FOUND_PATTERN = /not found in the index|404/ + +const isNotFoundError = (err: any): boolean => { + const message = String(err?.message ?? err) + return NOT_FOUND_PATTERN.test(message) +} // Then in fetchTxWithRetry (line 601): - const is404 = /not found in the index|404/.test(msg) + const is404 = isNotFoundError(e) // And in TX_CONFIRMED handler: } catch (e: any) { - const msg404 = String(e?.message ?? e) - const is404 = /not found in the index|404/.test(msg404) + const is404 = isNotFoundError(e) if (is404) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
prisma-local/migrations/20260104000000_transaction_orphaned_column/migration.sqlprisma-local/schema.prismaservices/chronikService.tsservices/transactionService.ts
🧰 Additional context used
🧬 Code graph analysis (1)
services/chronikService.ts (1)
services/transactionService.ts (1)
markTransactionsOrphaned(632-641)
⏰ 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). (1)
- GitHub Check: Run Tests
🔇 Additional comments (3)
prisma-local/schema.prisma (1)
74-74: Schema change looks good.The new
orphanedfield with@default(false)is correctly defined. If future queries frequently filter byorphanedstatus, consider adding an index.prisma-local/migrations/20260104000000_transaction_orphaned_column/migration.sql (1)
1-2: Migration is correct and safe.The migration correctly adds the
orphanedcolumn withNOT NULL DEFAULT false, ensuring existing transactions remain unaffected.services/chronikService.ts (1)
11-11: Import added correctly.
Description
To be able to properly save orphaned txs, we need to have an orphaned column, which this adds.
Test plan
For now, it's sufficient to make sure things are just running as expected after adding the db column. We'll be utilizing this in a follow-up branch.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.