Handle concurrent order taking: User PerOrderIdMutex in takeBuy and takeSell [2/2]

[Luquitasjeffrey]

Summary by CodeRabbit

• Bug Fixes

  • Order take operations now execute serially per order ID to ensure consistency
  • Added 10-second delay in sell order status updates

• Refactor

  • Consolidated internal utility imports for improved code organization

[coderabbitai]

Warning

Review limit reached

@Luquitasjeffrey, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 22 minutes. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b8494397-24be-4125-ada0-20d2be9a06cd

📥 Commits

Reviewing files that changed from the base of the PR and between a905c2b and e4dee55.

📒 Files selected for processing (1)

• bot/modules/orders/takeOrder.ts

Walkthrough

This PR centralizes per-order concurrency control by introducing PerOrderIdMutex in util/index.ts with reference-counted lifecycle, integrating it into takebuy and takesell to serialize critical sections per order ID, and migrating the utility from ln/subscribe_invoice.ts while updating dependent module imports.

Changes

Per-order concurrency control with PerOrderIdMutex

Layer / File(s)	Summary
PerOrderIdMutex utility creation util/index.ts	PerOrderIdMutex is introduced as a reference-counted, per-order locking utility built on async-mutex's Mutex. It maintains a Map of order ID → {lockCount, mutex} entries, with automatic cleanup when lock count reaches zero. The utility is exported from util/index.ts.
Serialize takeOrder operations with mutex bot/modules/orders/takeOrder.ts	takebuy and takesell wrap their full critical sections—order validation, DB lookups, status updates, persistence, and message sending—inside PerOrderIdMutex.instance.runExclusive(orderId, ...). takesell additionally includes a console log and 10-second delay after order.save() before setting status to in-progress.
Migrate PerOrderIdMutex to util and update imports ln/subscribe_invoice.ts, jobs/cancel_orders.ts, jobs/check_hold_invoice_expired.ts	PerOrderIdMutex is removed from ln/subscribe_invoice.ts exports and its local implementation is removed. Import statements across ln/subscribe_invoice.ts, jobs/cancel_orders.ts, and jobs/check_hold_invoice_expired.ts are updated to source PerOrderIdMutex from ../util instead of ../ln/subscribe_invoice.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• lnp2pBot/bot#746: Introduces checkHoldInvoiceExpired job that depends on per-order locking via PerOrderIdMutex from util/index.ts.
• lnp2pBot/bot#665: Modifies takesell in bot/modules/orders/takeOrder.ts with seller null-check and typing improvements, overlapping the same order-taking flow being mutex-protected here.

Suggested reviewers

• mostronatorcoder
• grunch

Poem

🐰 A mutex guards each order's dance,
Per ID, no chaotic prance,
From subscribe to util it flows,
Ten seconds pause, the takesell knows,
Concurrency tamed, the locks grow wise! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: implementing PerOrderIdMutex in takeBuy and takeSell order handling to manage concurrent order taking. It is specific, clear, and reflects the primary objective of the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch handle-concurrent-order-taking

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bot/modules/orders/takeOrder.ts`:
- Around line 144-145: Remove the leftover debug and artificial delay that block
the per-order mutex: delete the console.log('takeSell: Ups...') and the await
new Promise(setTimeout 10000) inside takeSell/runExclusive so the per-order
mutex (orderId) isn't held for 10s; if you need non-blocking diagnostics, log
via the project logger (e.g., logger.debug) outside the critical section or move
any intentional delay outside runExclusive so cancelOrders,
checkHoldInvoiceExpired and invoice subscription aren't stalled.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e7ab9c7c-c9c3-40c3-a805-c6ed34aae816

📥 Commits

Reviewing files that changed from the base of the PR and between e838c79 and a905c2b.

📒 Files selected for processing (5)

• bot/modules/orders/takeOrder.ts
• jobs/cancel_orders.ts
• jobs/check_hold_invoice_expired.ts
• ln/subscribe_invoice.ts
• util/index.ts

[ermeme]

Reviewed only the changes in scope of this PR, including the conversation history. I did not find blocking issues.

[mostronatorcoder]

Reviewed on current head e4dee55. The per-order mutex is now applied consistently in the high-contention paths this PR targets: takebuy, takesell, hold-invoice expiration handling, and hold-invoice settlement/subscription flow. The previously flagged debug delay / console artifact is gone from the current head. I do not see a remaining blocker in the changes that are actually in scope for this PR.