feat(order): persist buyer payout hash and preimage#875
Merged
Conversation
The order records the hold invoice hash/secret but nothing about the outgoing payment to the buyer. The payout's payment hash (payment.id) and preimage (payment.secret) are available on every successful payment result but were discarded — only routing_fee was kept. Persist them as order.payout_hash and order.payout_preimage so an order is a self-contained, verifiable record of the trade: - Proof of payment: the preimage is cryptographic proof the buyer was paid; today it lives only on the node. - Robust reconciliation: consumers no longer need to re-derive the outbound hash by parsing buyer_invoice (which is also stale after a retried /setinvoice, see #864) — it becomes a direct field lookup. - Traceability: operators can see exactly which payment settled an order. Set at the single buyer-payout success choke point (completeOrderAsSuccess, covering the retry and heal paths) and in the first-attempt path in payToBuyer. Scoped to buyer payouts; community withdrawals already store their hash on the pending payment. Historical orders keep an empty payout_hash, so consumers should retain the buyer_invoice-derived fallback for the backlog; the two are complementary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E7M5Q5My6Sgvvo8qS6zZCE
Contributor
WalkthroughChangesThe order model now defines payout hash and preimage fields. Confirmed Lightning payment paths persist these values during direct payment handling and atomic order completion, with healing-flow tests covering the saved fields. Payout persistence
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
models/order.ts (1)
75-76: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider indexing
payout_hashfor reconciliation queries.The PR objectives mention reconciliation and auditing. If consumers will query orders by
payout_hash(e.g., to cross-reference a Lightning payment), a non-unique index would avoid collection scans. The existinghashfield uses a partial unique index for similar lookup purposes.💡 Optional index suggestion
payout_hash: { type: String, + index: true, }, // payment hash of the buyer payout (proof of payment / reconciliation)🤖 Prompt for 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. In `@models/order.ts` around lines 75 - 76, Add a non-unique index for the payout_hash field in the order schema, following the existing hash field indexing pattern, so reconciliation lookups avoid collection scans while allowing multiple orders to share a payout hash.
🤖 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 `@util/completeOrder.ts`:
- Around line 33-47: Update the pending-payments success fallback to persist
payout details consistently with completeOrderAsSuccess: in the relevant branch
of pending_payments.ts, route the order through completeOrderAsSuccess or set
payout_hash to payment.id and payout_preimage to payment.secret alongside status
SUCCESS.
---
Nitpick comments:
In `@models/order.ts`:
- Around line 75-76: Add a non-unique index for the payout_hash field in the
order schema, following the existing hash field indexing pattern, so
reconciliation lookups avoid collection scans while allowing multiple orders to
share a payout hash.
🪄 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: 6c8d4019-225b-4d3d-aff0-01efce11a13c
📒 Files selected for processing (4)
ln/pay_request.tsmodels/order.tstests/bot/pending-payments.spec.tsutil/completeOrder.ts
Comment on lines
+33
to
+47
| { | ||
| $set: { | ||
| status: 'SUCCESS', | ||
| routing_fee: payment.fee, | ||
| payout_hash: payment.id, | ||
| payout_preimage: payment.secret, | ||
| }, | ||
| }, | ||
| ); | ||
| if (won === null) return false; | ||
| // Keep the in-memory document consistent for any later save by the caller. | ||
| order.status = 'SUCCESS'; | ||
| order.routing_fee = payment.fee; | ||
| order.payout_hash = payment.id; | ||
| order.payout_preimage = payment.secret; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all code paths that set order status to SUCCESS
rg -n --type=ts "status.*=.*'SUCCESS'|status.*SUCCESS|'SUCCESS'" -g '!**/node_modules/**' -g '!**/tests/**' | grep -v "enum\|//\|status: {"Repository: lnp2pBot/bot
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## util/completeOrder.ts"
if [ -f util/completeOrder.ts ]; then
wc -l util/completeOrder.ts
sed -n '1,220p' util/completeOrder.ts | cat -n
else
echo "missing util/completeOrder.ts"
fi
echo
echo "## SUCCESS assignments and payout fields"
rg -n --type=ts --type=tsx --glob '!**/node_modules/**' --glob '!**/tests/**' \
"status\s*=\s*'SUCCESS'|status:\s*'SUCCESS'|payout_hash|payout_preimage|routing_fee" .Repository: lnp2pBot/bot
Length of output: 5502
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## all SUCCESS assignments in TypeScript files"
rg -n --type ts --glob '!**/node_modules/**' --glob '!**/tests/**' \
"order\.status\s*=\s*'SUCCESS'|status:\s*'SUCCESS'|status\s*:\s*'SUCCESS'|['\"]SUCCESS['\"]" .
echo
echo "## files mentioning completeOrderAsSuccess or healConfirmedOrder"
rg -n --type ts --glob '!**/node_modules/**' --glob '!**/tests/**' \
"completeOrderAsSuccess|healConfirmedOrder" .Repository: lnp2pBot/bot
Length of output: 2238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for file in jobs/pending_payments.ts util/index.ts bot/modules/community/actions.ts scripts/migrate_completed_by_admin_orders.ts ln/pay_request.ts; do
if [ -f "$file" ]; then
echo "## $file"
wc -l "$file"
sed -n '1,260p' "$file" | cat -n
echo
fi
doneRepository: lnp2pBot/bot
Length of output: 39363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## payout field usages"
rg -n --type ts --glob '!**/node_modules/**' --glob '!**/tests/**' \
"payout_hash|payout_preimage" .Repository: lnp2pBot/bot
Length of output: 920
Persist payout fields on the pending-payments success fallback
jobs/pending_payments.ts:113-114 still marks an order as SUCCESS without setting payout_hash or payout_preimage. That leaves some completed orders without payout records; route this branch through completeOrderAsSuccess or set the same fields here.
🤖 Prompt for 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.
In `@util/completeOrder.ts` around lines 33 - 47, Update the pending-payments
success fallback to persist payout details consistently with
completeOrderAsSuccess: in the relevant branch of pending_payments.ts, route the
order through completeOrderAsSuccess or set payout_hash to payment.id and
payout_preimage to payment.secret alongside status SUCCESS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #869.
Summary
An order records the hold invoice hash/secret (
order.hash,order.secret) but nothing about the outgoing payment to the buyer. The payout's payment hash (payment.id) and preimage (payment.secret) are available on every successful payment result, but they were discarded — onlyorder.routing_feewas kept.This persists them as two new optional fields on the order:
payout_hashandpayout_preimage.Why
order.buyer_invoice— which is also left stale after a retried/setinvoice(see buyer_invoice is left stale after /setinvoice when the payout is retried with a different invoice #864). Storingpayout_hashmakes that a direct field lookup and removes a class of edge cases. This matters in practice: it is exactly what makes it easy to tell a real payout anomaly apart from a reconciliation artifact when auditing against the node.Changes
models/order.ts— addpayout_hash?: string | nullandpayout_preimage?: string | null(plainString, no index).util/completeOrder.ts— set both incompleteOrderAsSuccess(the single buyer-payout success choke point, covering the pending-payment retry andhealConfirmedOrderpaths), inside the atomic$setand the in-memory copy.ln/pay_request.ts— set both in the first-attempt success branch ofpayToBuyer.tests/bot/pending-payments.spec.ts— assert both are persisted on the success routine.Scoped to buyer payouts (which have an order). Community earnings withdrawals have no order; their payout hash is already stored on the pending payment.
Note
This records the values going forward. Orders completed before this change will have empty
payout_hash/payout_preimage, so consumers should keep deriving the hash frombuyer_invoicefor the historical backlog. The two are complementary.Verification
tsc -p tsconfig.test.json— passes.eslint+prettier --checkon the changed files — clean.completeOrderAsSuccessconfirmspayout_hash/payout_preimage/routing_feeare set on success. (The full mocha suite could not run in this environment because the nativecanvasdependency fails to build without cairo/pango — unrelated to this change.)🤖 Generated with Claude Code
https://claude.ai/code/session_01E7M5Q5My6Sgvvo8qS6zZCE
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes