-
Notifications
You must be signed in to change notification settings - Fork 4
feat: separate process to do initial syncing #1065
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
Changes from all commits
4923173
8a4e34c
ee0b6ee
44a2970
dcb0f03
b50f0f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ case "$command" in | |
| eval "$base_command_db" mariadb-dump -h "$MAIN_DB_HOST" -u root -p"$MAIN_DB_ROOT_PASSWORD" "$@" | ||
| ;; | ||
| "databaseshell" | "dbs") | ||
| eval "$base_command_db" bash -l "$@" | ||
| eval "$base_command_db" bash -c bash -l "$@" | ||
| ;; | ||
|
Comment on lines
51
to
53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix shell invocation: 'bash -c bash -l' is not a login shell; also risks argument loss
Apply this minimal fix: - eval "$base_command_db" bash -c bash -l "$@"
+ eval "$base_command_db" bash -l "$@"- eval "$base_command_node" bash -c bash -l
+ eval "$base_command_node" bash -l "$@"- eval "$base_command_node_root" bash -c bash -l
+ eval "$base_command_node_root" bash -l "$@"Optional hardening (drop eval to preserve whitespace/args and avoid injection pitfalls): - eval "$base_command_db" bash -l "$@"
+ docker exec -it "$db_container_name" bash -l "$@"- eval "$base_command_node" bash -l "$@"
+ docker exec -it "$node_container_name" bash -l "$@"- eval "$base_command_node_root" bash -l "$@"
+ docker exec -it -u 0 "$node_container_name" bash -l "$@"Also applies to: 72-74, 75-77 🧰 Tools🪛 Shellcheck (0.11.0)[warning] 52-52: eval negates the benefit of arrays. Drop eval to preserve whitespace/symbols (or eval as string). (SC2294) 🤖 Prompt for AI Agents |
||
| "databasetest" | "dbt") | ||
| eval "$base_command_db" mariadb -h "$MAIN_DB_HOST" -u "$MAIN_DB_USER"-test -p"$MAIN_DB_PASSWORD" -D "$MAIN_DB_NAME"-test "$@" | ||
|
|
@@ -70,10 +70,10 @@ case "$command" in | |
| eval "$base_command_node" yarn test --coverage --verbose "$@" | ||
| ;; | ||
| "nodeshell" | "ns") | ||
| eval "$base_command_node" bash -l | ||
| eval "$base_command_node" bash -c bash -l | ||
| ;; | ||
| "rootnodeshell" | "rns") | ||
| eval "$base_command_node_root" bash -l | ||
| eval "$base_command_node_root" bash -c bash -l | ||
| ;; | ||
| "jobslogs" | "jl") | ||
| eval "$base_command_node" pm2 logs jobs | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,6 @@ import { | |||||||||||||||||||||||||||||||||||||
| upsertTransaction, | ||||||||||||||||||||||||||||||||||||||
| getSimplifiedTransactions, | ||||||||||||||||||||||||||||||||||||||
| getSimplifiedTrasaction, | ||||||||||||||||||||||||||||||||||||||
| connectAllTransactionsToPrices, | ||||||||||||||||||||||||||||||||||||||
| updateClientPaymentStatus, | ||||||||||||||||||||||||||||||||||||||
| getClientPayment | ||||||||||||||||||||||||||||||||||||||
| } from './transactionService' | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,7 +27,6 @@ import { OpReturnData, parseError, parseOpReturnData } from 'utils/validators' | |||||||||||||||||||||||||||||||||||||
| import { executeAddressTriggers, executeTriggersBatch } from './triggerService' | ||||||||||||||||||||||||||||||||||||||
| import { appendTxsToFile } from 'prisma-local/seeds/transactions' | ||||||||||||||||||||||||||||||||||||||
| import { PHASE_PRODUCTION_BUILD } from 'next/dist/shared/lib/constants' | ||||||||||||||||||||||||||||||||||||||
| import { syncPastDaysNewerPrices } from './priceService' | ||||||||||||||||||||||||||||||||||||||
| import { AddressType } from 'ecashaddrjs/dist/types' | ||||||||||||||||||||||||||||||||||||||
| import { DecimalJsLike } from '@prisma/client/runtime/library' | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
@@ -957,25 +955,14 @@ class MultiBlockchainClient { | |||||||||||||||||||||||||||||||||||||
| console.log('Initializing MultiBlockchainClient...') | ||||||||||||||||||||||||||||||||||||||
| this.initializing = true | ||||||||||||||||||||||||||||||||||||||
| void (async () => { | ||||||||||||||||||||||||||||||||||||||
| if (this.isRunningApp()) { | ||||||||||||||||||||||||||||||||||||||
| await syncPastDaysNewerPrices() | ||||||||||||||||||||||||||||||||||||||
| const asyncOperations: Array<Promise<void>> = [] | ||||||||||||||||||||||||||||||||||||||
| this.clients = { | ||||||||||||||||||||||||||||||||||||||
| ecash: this.instantiateChronikClient('ecash', asyncOperations), | ||||||||||||||||||||||||||||||||||||||
| bitcoincash: this.instantiateChronikClient('bitcoincash', asyncOperations) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| await Promise.all(asyncOperations) | ||||||||||||||||||||||||||||||||||||||
| this.setInitialized() | ||||||||||||||||||||||||||||||||||||||
| await connectAllTransactionsToPrices() | ||||||||||||||||||||||||||||||||||||||
| } else if (process.env.NODE_ENV === 'test') { | ||||||||||||||||||||||||||||||||||||||
| const asyncOperations: Array<Promise<void>> = [] | ||||||||||||||||||||||||||||||||||||||
| this.clients = { | ||||||||||||||||||||||||||||||||||||||
| ecash: this.instantiateChronikClient('ecash', asyncOperations), | ||||||||||||||||||||||||||||||||||||||
| bitcoincash: this.instantiateChronikClient('bitcoincash', asyncOperations) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| await Promise.all(asyncOperations) | ||||||||||||||||||||||||||||||||||||||
| this.setInitialized() | ||||||||||||||||||||||||||||||||||||||
| const asyncOperations: Array<Promise<void>> = [] | ||||||||||||||||||||||||||||||||||||||
| this.clients = { | ||||||||||||||||||||||||||||||||||||||
| ecash: this.instantiateChronikClient('ecash', asyncOperations), | ||||||||||||||||||||||||||||||||||||||
| bitcoincash: this.instantiateChronikClient('bitcoincash', asyncOperations) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| await Promise.all(asyncOperations) | ||||||||||||||||||||||||||||||||||||||
| this.setInitialized() | ||||||||||||||||||||||||||||||||||||||
| console.log('Finished initializing MultiBlockchainClient.') | ||||||||||||||||||||||||||||||||||||||
| })() | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+958
to
966
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race: Multi setInitialized before per-client Chronik is ready
Always gate Multi init on each client's void (async () => {
- const asyncOperations: Array<Promise<void>> = []
+ const asyncOperations: Array<Promise<void>> = []
this.clients = {
- ecash: this.instantiateChronikClient('ecash', asyncOperations),
- bitcoincash: this.instantiateChronikClient('bitcoincash', asyncOperations)
+ ecash: this.instantiateChronikClient('ecash', asyncOperations),
+ bitcoincash: this.instantiateChronikClient('bitcoincash', asyncOperations)
}
- await Promise.all(asyncOperations)
+ // Always wait for base client readiness (chronik selected).
+ asyncOperations.push(this.clients.ecash.waitForLatencyTest())
+ asyncOperations.push(this.clients.bitcoincash.waitForLatencyTest())
+ await Promise.all(asyncOperations)
this.setInitialized()
console.log('Finished initializing MultiBlockchainClient.')
})()Alternatively, push 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
@@ -1020,9 +1007,6 @@ class MultiBlockchainClient { | |||||||||||||||||||||||||||||||||||||
| await newClient.waitForLatencyTest() | ||||||||||||||||||||||||||||||||||||||
| console.log(`[CHRONIK — ${networkSlug}] Subscribing addresses in database...`) | ||||||||||||||||||||||||||||||||||||||
| await newClient.subscribeInitialAddresses() | ||||||||||||||||||||||||||||||||||||||
| console.log(`[CHRONIK — ${networkSlug}] Syncing missed transactions...`) | ||||||||||||||||||||||||||||||||||||||
| await newClient.syncMissedTransactions() | ||||||||||||||||||||||||||||||||||||||
| console.log(`[CHRONIK — ${networkSlug}] Finished instantiating client.`) | ||||||||||||||||||||||||||||||||||||||
| })() | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| } else if (process.env.NODE_ENV === 'test') { | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -1033,6 +1017,7 @@ class MultiBlockchainClient { | |||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| console.log(`Finished instantiating ${networkSlug} client.`) | ||||||||||||||||||||||||||||||||||||||
| return newClient | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
@@ -1090,10 +1075,31 @@ class MultiBlockchainClient { | |||||||||||||||||||||||||||||||||||||
| return await this.clients[networkSlug as MainNetworkSlugsType].getBalance(address) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public async syncMissedTransactions (): Promise<void> { | ||||||||||||||||||||||||||||||||||||||
| await this.waitForStart() | ||||||||||||||||||||||||||||||||||||||
| await Promise.all([ | ||||||||||||||||||||||||||||||||||||||
| this.clients.ecash.syncMissedTransactions(), | ||||||||||||||||||||||||||||||||||||||
| this.clients.bitcoincash.syncMissedTransactions() | ||||||||||||||||||||||||||||||||||||||
| ]) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1078
to
+1084
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard sync against not-yet-ready clients Even with Multi’s wait, explicitly ensure per-client readiness before syncing: public async syncMissedTransactions (): Promise<void> {
- await this.waitForStart()
- await Promise.all([
+ await this.waitForStart()
+ await Promise.all([
+ this.clients.ecash.waitForLatencyTest(),
+ this.clients.bitcoincash.waitForLatencyTest()
+ ])
+ await Promise.all([
this.clients.ecash.syncMissedTransactions(),
this.clients.bitcoincash.syncMissedTransactions()
])
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public async syncAndSubscribeAddresses (addresses: Address[]): Promise<SyncAndSubscriptionReturn> { | ||||||||||||||||||||||||||||||||||||||
| await this.subscribeAddresses(addresses) | ||||||||||||||||||||||||||||||||||||||
| return await this.syncAddresses(addresses) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public async destroy (): Promise<void> { | ||||||||||||||||||||||||||||||||||||||
| await Promise.all( | ||||||||||||||||||||||||||||||||||||||
| Object.values(this.clients).map(async (c) => { | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| c.chronikWSEndpoint.close() | ||||||||||||||||||||||||||||||||||||||
| c.wsEndpoint.close() | ||||||||||||||||||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||||||||||||||||||
| console.error(`Failed to close connections for client: ${err.message as string}`) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| export interface NodeJsGlobalMultiBlockchainClient extends NodeJS.Global { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
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.
Fire-and-forget async cleanup may not complete before process exit.
The
void (async () => {...})()pattern in thecompletedhandler creates an async operation that runs detached from the worker lifecycle. This means:destroy()call may not complete before the process exitsMove the cleanup to the job handler with proper awaiting:
const worker = new Worker( queueName, async (job) => { - console.log(`job ${job.id as string}: syncing missed transactions and connecting prices...`) - await multiBlockchainClient.syncMissedTransactions() - await connectAllTransactionsToPrices() + try { + console.log(`job ${job.id as string}: syncing missed transactions and connecting prices...`) + await multiBlockchainClient.syncMissedTransactions() + await connectAllTransactionsToPrices() + } finally { + console.log('Cleaning up MultiBlockchainClient global instance...') + await multiBlockchainClient.destroy() + console.log('Done.') + } }, { connection: redisBullMQ, lockDuration: DEFAULT_WORKER_LOCK_DURATION } ) worker.on('completed', (job) => { - // teardown - void (async () => { - console.log('Cleaning up MultiBlockchainClient global instance...') - await multiBlockchainClient.destroy() - console.log('Done.') - console.log(`job ${job.id as string}: blockchain + prices sync finished`) - })() + console.log(`job ${job.id as string}: blockchain + prices sync finished`) })📝 Committable suggestion