From 8aba0bc2e3b5d09b3783b87eb0a29088b40796e4 Mon Sep 17 00:00:00 2001
From: Adam Soltys <adam@coinos.io>
Date: Mon, 25 May 2026 09:22:26 -0700
Subject: finalize: don't crash on non-integer fee delta (db.incrBy NaN)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Triggered by a recurring "finalize threw despite no failed_parts: ERR
value is not an integer or out of range" warning. Root cause: when xpay's
response shape is unexpected (or invoice is zero-amount and decoded
amount_msat is undefined), the computed p.fee becomes NaN. The
subsequent db.incrBy(balance:<uid>, maxfee - NaN) throws because Redis
can't increment by NaN.

Symptom: leak guard fires "ref set in db despite throw" — payment is
actually fine (p.ref persisted before the throw) but finalize never
completes the fee refund. User loses up to maxfee sats (was already
debited maxfee, gets nothing back).

Fix: validate the computed fee is a finite integer; if not, keep the
originally-reserved maxfee (zero refund, no balance corruption) and warn
loudly. Same guard on the refund delta before db.incrBy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 lib/payments.ts | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/lib/payments.ts b/lib/payments.ts
index e8fec540..ffd3ddc1 100644
--- a/lib/payments.ts
+++ b/lib/payments.ts
@@ -1055,15 +1055,35 @@ const finalize = async (r, p) => {
   nwcNotify(p);
 
   const maxfee = p.fee;
-  const { amount_msat } = await ln.decode(p.hash);
-  p.fee = Math.round((r.amount_sent_msat - amount_msat) / 1000);
+  // Compute the actual fee xpay paid (sent − invoice amount, both msat).
+  // Robust against zero-amount invoices (decoded amount_msat is undefined,
+  // fall back to r.amount_msat) and against CLN occasionally returning
+  // unexpected response shapes — if we can't compute a finite integer
+  // fee, keep p.fee at the originally-reserved maxfee so the subsequent
+  // refund delta is 0 instead of NaN (which would crash db.incrBy with
+  // "value is not an integer or out of range" and leave finalize in a
+  // partial state).
+  const decoded = await ln.decode(p.hash);
+  const invMsat = Number(decoded.amount_msat ?? r.amount_msat);
+  const sentMsat = Number(r.amount_sent_msat);
+  const computedFee = Math.round((sentMsat - invMsat) / 1000);
+  if (!Number.isFinite(computedFee)) {
+    warn("finalize: non-finite fee compute, keeping reserved maxfee", p.id,
+         "sent_msat=", r.amount_sent_msat, "inv_msat=", decoded.amount_msat ?? r.amount_msat);
+  }
+  p.fee = Number.isFinite(computedFee) ? computedFee : maxfee;
   p.ref = preimage;
 
   if (!(await g(`payment:${p.id}`)).ref) {
     await s(`payment:${p.id}`, p);
-
-    l("refunding fee", maxfee, p.fee, maxfee - p.fee, p.ref);
-    await db.incrBy(`balance:${p.uid}`, maxfee - p.fee);
+    const refund = maxfee - p.fee;
+    if (Number.isFinite(refund) && Number.isInteger(refund)) {
+      l("refunding fee", maxfee, p.fee, refund, p.ref);
+      await db.incrBy(`balance:${p.uid}`, refund);
+    } else {
+      warn("finalize: skipping fee refund (non-integer delta)", p.id,
+           "maxfee=", maxfee, "p.fee=", p.fee, "refund=", refund);
+    }
   }
 
   return p;
-- 
cgit v1.2.3