feat: persist buyer payout payment hash and preimage on the order#870
feat: persist buyer payout payment hash and preimage on the order#870grunch wants to merge 1 commit into
Conversation
An order recorded the hold invoice hash/secret but nothing about the outgoing payment to the buyer. The payout's payment hash and preimage (proof of payment) were available on the payment result but discarded, leaving that proof only on the node. Add payout_hash and payout_preimage to the Order model and set them from payment.id / payment.secret in both buyer-payout success paths (payToBuyer and attemptPendingPayments). This makes the order a self-contained, verifiable record of the payment and lets reconciliation read the outbound hash directly instead of re-deriving it from buyer_invoice. Adds a regression spec for the payToBuyer success path. Closes #869. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5uNqd26cC88p5HVqErRDU
WalkthroughAdds ChangesPayout proof persistence
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
jobs/pending_payments.ts (1)
86-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest coverage gap: only
payToBuyerhas a regression test.The new
payout_hash/payout_preimageassignment here mirrorsln/pay_request.ts, buttests/ln/payout_proof.spec.tsonly exercisespayToBuyer. This confirmed-payment branch ofattemptPendingPaymentsis untested for the new fields.Want me to draft a proxyquire-based spec for
attemptPendingPaymentsmirroring the existingpayout_proof.spec.tsstructure?🤖 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 `@jobs/pending_payments.ts` around lines 86 - 93, The confirmed-payment branch in attemptPendingPayments is missing regression coverage for the new payout_hash and payout_preimage assignments. Add a spec alongside the existing payout_proof coverage that exercises attemptPendingPayments with a confirmed payment and asserts the order gets the hash/preimage copied over, using the same proxyquire-based setup as payout_proof.spec.ts and referencing attemptPendingPayments, payment.confirmed_at, and the order fields.models/order.ts (1)
15-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider a partial unique index for
payout_hash.
hashandsecretboth have unique indexes (withpartialFilterExpressionto allow nulls) to guard against duplicate outgoing/incoming payment records.payout_hash/payout_preimageget no such constraint, so a bug that pays two orders with the same payment (or double-writes) wouldn't be caught at the DB layer.♻️ Optional index suggestion
- payout_hash: { type: String }, // payment hash of the outgoing payment to the buyer + payout_hash: { + type: String, + index: { + unique: true, + partialFilterExpression: { payout_hash: { $type: 'string' } }, + }, + }, // payment hash of the outgoing payment to the buyer payout_preimage: { type: String }, // preimage (proof of payment) of the buyer payoutAlso applies to: 89-90
🤖 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 15 - 16, Add a partial unique index for payout_hash, similar to the existing hash/secret uniqueness guards, so duplicate payout records are caught at the DB layer while still allowing null values. Update the Order model/index definition where payout_hash and payout_preimage are declared, and make sure the new index uses a partialFilterExpression like the other unique payment indexes.
🤖 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.
Nitpick comments:
In `@jobs/pending_payments.ts`:
- Around line 86-93: The confirmed-payment branch in attemptPendingPayments is
missing regression coverage for the new payout_hash and payout_preimage
assignments. Add a spec alongside the existing payout_proof coverage that
exercises attemptPendingPayments with a confirmed payment and asserts the order
gets the hash/preimage copied over, using the same proxyquire-based setup as
payout_proof.spec.ts and referencing attemptPendingPayments,
payment.confirmed_at, and the order fields.
In `@models/order.ts`:
- Around line 15-16: Add a partial unique index for payout_hash, similar to the
existing hash/secret uniqueness guards, so duplicate payout records are caught
at the DB layer while still allowing null values. Update the Order model/index
definition where payout_hash and payout_preimage are declared, and make sure the
new index uses a partialFilterExpression like the other unique payment indexes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97b3f933-2fc9-49ae-8a30-b2b1cc713c75
📒 Files selected for processing (4)
jobs/pending_payments.tsln/pay_request.tsmodels/order.tstests/ln/payout_proof.spec.ts
|
@codex review |
Closes #869.
What
An order recorded the hold invoice hash/secret (
order.hash,order.secret) but nothing about the outgoing payment to the buyer. The payout's payment hash and its preimage (proof of payment) were available on the payment result (payment.id,payment.secret) but discarded — only the routing fee was kept.payout_hashandpayout_preimageto the Order model (models/order.ts).payment.id/payment.secretin both buyer-payout success paths:payToBuyer(ln/pay_request.ts) andattemptPendingPayments(jobs/pending_payments.ts).Why
buyer_invoice(which is fragile with retried invoices — see buyer_invoice is left stale after /setinvoice when the payout is retried with a different invoice #864).Scoped to buyer payouts (which have an order). Community earnings withdrawals have no order and already store their payout hash on the pending payment.
Scope note
Records the values going forward; orders completed before this change keep empty
payout_hash/payout_preimage, so consumers should keep deriving the hash frombuyer_invoicefor the historical backlog. The two are complementary.Tests
Adds
tests/ln/payout_proof.spec.tsasserting a successfulpayToBuyersetspayout_hash,payout_preimageandrouting_fee.npx tsccompiles cleanly; Prettier and ESLint report no issues.🤖 Generated with Claude Code
https://claude.ai/code/session_01X5uNqd26cC88p5HVqErRDU
Generated by Claude Code
Summary by CodeRabbit