bug: two portfolio-monitoring systems run simultaneously — race conditions and potential double-rebalancing

[Uchechukwu-Ekezie]

Description

The backend starts two independent systems that both scan portfolios and trigger rebalancing on overlapping schedules. There is no coordination, locking, or deduplication between them, creating race conditions where the same portfolio can be rebalanced twice simultaneously.

The Two Systems

System 1: RebalancingService (legacy cron — monitoring/rebalancer.ts)

// monitoring/rebalancer.ts, line 20
start() {
    cron.schedule('*/2 * * * *', async () => {  // Every 2 minutes
        await this.checkAllPortfolios()
    })
}

System 2: AutoRebalancerService (queue-backed — services/autoRebalancer.ts)

// autoRebalancer.ts, line 17
private readonly CHECK_INTERVAL = 30 * 60 * 1000  // Every 30 minutes via BullMQ

Both Started in index.ts

// index.ts, lines 224-231
// Note: comment says "now queue-backed, no cron" but cron still runs
const rebalancingService = new RebalancingService(wss)
rebalancingService.start()   // ← Cron fires every 2 minutes

// ... later ...
await autoRebalancer.start() // ← BullMQ queue fires every 30 minutes

The comment on line 224 reads // now queue-backed, no cron — suggesting RebalancingService was intended to be removed when AutoRebalancerService was introduced, but it was never actually removed.

Failure Scenarios

Scenario A — Double rebalance:
At T=0, cron fires and finds Portfolio X needs rebalancing. It enqueues a rebalance job. At T=30min, BullMQ also fires and finds Portfolio X still needs rebalancing (because the first job is still processing). A second rebalance job is enqueued. Both execute, resulting in over-trading and incorrect final allocation.

Scenario B — Duplicate database writes:
Both systems independently call recordRebalanceEvent(), inserting two records for the same rebalance into rebalance_events. The history endpoint returns duplicate entries.

Scenario C — Conflicting circuit breaker states:
Cron-triggered rebalance opens a circuit breaker. Queue-triggered rebalance sees the circuit as open and skips. Neither system knows the other's state.

Steps to Identify

grep -n "rebalancingService\|autoRebalancer" backend/src/index.ts
# Lines 224-231: both are started sequentially with no exclusion

grep -rn "checkAllPortfolios\|portfolioCheckWorker" backend/src/
# Both scan the same portfolios table

Proposed Fix

Option A (Recommended): Remove RebalancingService entirely since AutoRebalancerService is the intended replacement. Delete monitoring/rebalancer.ts and remove its startup from index.ts.

Option B: If RebalancingService has functionality not present in AutoRebalancerService (e.g., WebSocket broadcast on drift detection), extract that behavior into the queue-backed system and then remove the cron.

Option C (Minimum viable): Add a distributed lock (Redis SET NX EX) around portfolio checks so only one system can run at a time. This prevents double-execution without requiring a refactor.

// In both systems' check loops:
const lock = await redis.set('portfolio-check-lock', '1', 'NX', 'EX', 120)
if (!lock) return // Another check is in progress
try {
    await checkAllPortfolios()
} finally {
    await redis.del('portfolio-check-lock')
}

Files Affected

• backend/src/index.ts — lines 224–231
• backend/src/monitoring/rebalancer.ts — remove or repurpose
• backend/src/services/autoRebalancer.ts — add deduplication if needed

[Menjay7]

Hi! I came across this issue and would love to work on it. I have experience with similar tasks and I'm confident I can investigate, implement a clean solution, and thoroughly test it before submitting a PR. If the issue is still open, I would appreciate it if you could assign it to me. Thank you!

[Menjay7]

Hi! I came across this issue and would love to work on it. I have experience with similar tasks and I'm confident I can investigate, implement a clean solution, and thoroughly test it before submitting a PR. If the issue is still open, I would appreciate it if you could assign it to me. Thank you!

[ayomideadeniran]

I'm a full-stack developer and I'd like to take this issue.
I understand the requirements and will follow the existing project structure, keep the implementation clean, and include proper tests where applicable.

ETA: 1–3 days (depending on scope)
Portfolio: https://knightsdev-portfolio.vercel.app/

[Marvy247]

I've reviewed the issue description, and I'm confident I can implement all the requested details smoothly.​Please assign this issue to me! I can have a PR ready for review in a few hours. Thank You.

[Jaydbrown]

Application — Dual Rebalancing System Race Condition Fix

Hi, I'd like to take this issue.

I've read through the issue, monitoring/rebalancer.ts, services/autoRebalancer.ts, and the relevant section of index.ts. Here's exactly what I found and what I'll do.

---

What the code actually looks like today

Reading monitoring/rebalancer.ts, the cron has already been partially cleaned up — RebalancingService.start() no longer schedules */2 * * * *. However the class is still being instantiated and started in index.ts:

// index.ts, line 229-235
// Start existing rebalancing service (now queue-backed, no cron)
try {
    const rebalancingService = new RebalancingService(wss)
    rebalancingService.start()   // ← no-op today, but the class still exists
    ...
}

And separately:

// index.ts, line 277-293
if (shouldStartAutoRebalancer) {
    await autoRebalancer.start()  // ← BullMQ queue, every 30 min
}

The race condition risk remains because:

1. RebalancingService still has forceCheckPortfolio() which independently enqueues jobs to the same portfolioCheckQueue that AutoRebalancerService uses — with no deduplication
2. The dead instantiation and start() call adds noise and confusion for anyone reading the startup sequence
3. If RebalancingService.start() is ever accidentally restored with the cron (e.g. during a merge conflict), the double-firing bug comes back immediately

---

Fix I'll implement — Option A (remove the legacy service)

Step 1 — Remove RebalancingService from index.ts

-import { RebalancingService } from './monitoring/rebalancer.js'
 import { AutoRebalancerService } from './services/autoRebalancer.js'

-// Start existing rebalancing service (now queue-backed, no cron)
-try {
-    const rebalancingService = new RebalancingService(wss)
-    rebalancingService.start()
-    console.log('[REBALANCING-SERVICE] Monitoring service started (queue-backed)')
-} catch (error) {
-    console.error('Failed to start rebalancing service:', error)
-}

Step 2 — Migrate WebSocket broadcasting into AutoRebalancerService

RebalancingService holds the only copy of notifyClients() and broadcastToAllClients() (which push events to connected WebSocket clients). Before deleting the file I'll move these two private methods — and the wss: WebSocketServer dependency — into AutoRebalancerService, so the queue-backed worker can still broadcast drift/rebalance events to the frontend in real time.

// services/autoRebalancer.ts — additions
constructor(wss: WebSocketServer) {
    this.stellarService = new StellarService()
    this.reflectorService = new ReflectorService()
    this.wss = wss
}

private notifyClients(portfolioId: string, event: string, data: any = {}) {
    const message = JSON.stringify({
        type: 'portfolio_update',
        portfolioId,
        event,
        data,
        timestamp: new Date().toISOString(),
    })
    this.wss.clients.forEach(client => {
        if (client.readyState === 1) client.send(message)
    })
}

Step 3 — Delete monitoring/rebalancer.ts

Once the WebSocket methods are migrated, the file has no remaining purpose and can be removed.

Step 4 — Update index.ts construction call

-const autoRebalancer = new AutoRebalancerService()
+const autoRebalancer = new AutoRebalancerService(wss)

---

Why not Option C (Redis lock)?

Option C keeps both systems alive and adds a distributed lock around them. That prevents double-execution but doesn't remove the dead code or the structural confusion. The comment on line 229 already says "now queue-backed, no cron" — the intent was always to fully remove RebalancingService. Option A completes that intent cleanly.

---

Failure scenarios resolved

Scenario from issue	After fix
Double rebalance (cron + queue both fire)	Impossible — only one scheduler exists
Duplicate recordRebalanceEvent() writes	Impossible — only one code path calls it
Conflicting circuit breaker states	Impossible — single service owns circuit breaker checks
Accidental cron restoration in future	Impossible — class is deleted

---

Files changed

File	Change
backend/src/index.ts	Remove RebalancingService import + startup block; pass wss to AutoRebalancerService
backend/src/monitoring/rebalancer.ts	Delete
backend/src/services/autoRebalancer.ts	Accept wss in constructor; add notifyClients + broadcastToAllClients

No new dependencies. No changes to the queue, workers, or scheduler.

I can have a PR ready within a few hours of being assigned.

[grantfox-oss]

🦊 GrantFox — @Jaydbrown has been assigned to this issue as part of the Official Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #5)
2. Your PR will be reviewed by the grantFoxin maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Official Campaign campaign!

@Jaydbrown's PR #23 was approved and merged by @Uchechukwu-Ekezie.

🏆 @Jaydbrown: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (261 total points). Track your full progress on GrantFox.

👏 Great work, @Jaydbrown! Keep contributing to grantFoxin.