Stop deleting the melt recovery record when a melt may have succeeded

WalletStore.payLightningMelt deleted the melt recovery record for any error
whose message did not mention a timeout or a network failure. That heuristic
is wrong for the case cashu-ts 4.10 now names explicitly.

MeltChangeError is raised by completeMelt only AFTER the mint executed the
payment: the inputs are spent, the payment stands, and solely the NUT-08
change could not be reconstructed. Its message mentions neither timeout nor
network, so the old test fell through to the delete branch — one step before
TransferOperationApi._handleExecuteError re-checks the quote, finds it PAID,
and calls recoverMeltQuoteChange, which reads exactly that record. Recovery
then failed with "MeltPreview not found", was swallowed as "Change recovery
failed", and the transaction was marked RECOVERED with zero change. The
payment went through and the user silently forfeited the change.

This predates 4.10 — on 4.7.1 the same path was reachable whenever
createMeltChangeProofs threw, which was MORE likely then, since 4.10 loads
the change keyset's keys before building. 4.10 only made it a named type.

The fix is not to special-case one error but to give the record a coherent
owner. WalletStore cannot know whether the mint acted on a request that
threw, so it no longer guesses: both melt catches keep the record. Removal
moves to the code that learns the quote's terminal state:

  PAID     recoverMeltQuoteChange / _unblindMeltChange remove it (already did)
  PENDING  kept — the async monitor still needs it
  UNPAID   removed, in _handleExecuteError and the async revert path

The UNPAID half also closes a pre-existing leak: an async melt resolving
UNPAID left an orphaned row forever, because the only two cleanups sat on the
PAID path. Reproduced on device — two failed melts left two orphans while a
successful one cleaned up after itself. It is included here rather than split
out because fixing only the catch would have made that leak worse, and
meltRecovery.test.ts already documents the intended contract as "removed on
terminal success/failure".

Adds meltChangeError.test.ts pinning the cashu-ts contract this now depends
on: the type is exported, distinguishable by instanceof, and carries
outputData and quote. It also pins the bug itself — asserting that the old
message heuristic would NOT have kept the record — so the reasoning cannot
quietly rot.

Verified: tsc --noEmit unchanged against baseline (89 pre-existing, none new),
47 suites / 637 tests pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

__tests__/meltChangeError.test.ts

@@ -0,0 +1,104 @@
+/**
+ * The cashu-ts contract that melt-change recovery depends on (`MeltChangeError`).
+ *
+ * cashu-ts 4.10 raises `MeltChangeError` from `completeMelt` in one specific
+ * situation: the melt request SUCCEEDED — the mint executed the payment and the
+ * inputs are spent — but the NUT-08 change could not be reconstructed from the
+ * blank outputs. Its own docs are explicit: "The inputs are spent and the payment
+ * stands." It carries `outputData` and the merged `quote` precisely so the change
+ * can be rebuilt later.
+ *
+ * Why this file exists: `WalletStore.payLightningMelt` used to delete the melt
+ * recovery record for any error whose message did not mention a timeout or a
+ * network failure. `MeltChangeError`'s message mentions neither, so that heuristic
+ * deleted the record — one step before
+ * `TransferOperationApi._handleExecuteError` re-checks the quote, finds it PAID,
+ * and calls `recoverMeltQuoteChange`, which reads exactly that record. The user
+ * silently forfeited the change on a payment that had actually gone through.
+ *
+ * The wallet no longer message-sniffs, so what it now relies on is this error
+ * TYPE existing and being distinguishable. These tests pin that dependency: if
+ * cashu-ts v5 renames the class, drops the payload, or changes the message such
+ * that the old heuristic would have "worked", this fails and says why.
+ *
+ * @jest-environment node
+ */
+import {CTSError, MeltChangeError} from '@cashu/cashu-ts'
+import type {OutputDataLike} from '@cashu/cashu-ts'
+
+/** Stand-ins with the right shape; nothing here needs real crypto. */
+const outputData = [
+  {blindedMessage: {amount: '2', id: '00aa', B_: '02ff'}},
+] as unknown as OutputDataLike[]
+
+const quote = {
+  quote: 'quote-id-1',
+  amount: '21',
+  unit: 'sat',
+  state: 'PAID',
+} as never
+
+describe('MeltChangeError is a distinguishable type', () => {
+  test('cashu-ts still exports it', () => {
+    expect(typeof MeltChangeError).toBe('function')
+  })
+
+  test('an instance is recognisable by instanceof — no message matching needed', () => {
+    const error = new MeltChangeError(outputData, quote)
+
+    expect(error).toBeInstanceOf(MeltChangeError)
+    expect(error).toBeInstanceOf(Error)
+  })
+
+  test('it is a CTSError, so it travels the same path as other library errors', () => {
+    expect(new MeltChangeError(outputData, quote)).toBeInstanceOf(CTSError)
+  })
+})
+
+describe('it carries what change recovery needs', () => {
+  test('outputData — the blank outputs the change proofs are rebuilt from', () => {
+    const error = new MeltChangeError(outputData, quote)
+
+    expect(Array.isArray(error.outputData)).toBe(true)
+    expect(error.outputData).toHaveLength(1)
+  })
+
+  test('quote — merged from the preview and the mint response', () => {
+    const error = new MeltChangeError(outputData, quote)
+
+    expect(error.quote).toBeDefined()
+    expect(error.quote.quote).toBe('quote-id-1')
+  })
+
+  test('an underlying cause is preserved for diagnosis', () => {
+    const cause = new Error('undefined key for amount 2')
+    const error = new MeltChangeError(outputData, quote, {cause})
+
+    expect((error as unknown as {cause?: Error}).cause).toBe(cause)
+  })
+})
+
+describe('why the old message heuristic was wrong', () => {
+  // The exact predicate WalletStore.payLightningMelt used to decide whether the
+  // melt might still have gone through, and therefore whether to KEEP the record.
+  const oldHeuristicWouldKeepRecord = (e: Error) =>
+    e.message.toLowerCase().includes('timeout') ||
+    e.message.toLowerCase().includes('network request failed')
+
+  test('a MeltChangeError does not look like a timeout or a network failure', () => {
+    const error = new MeltChangeError(outputData, quote)
+
+    // So the old code fell through to the delete branch — for an error that means
+    // the payment SUCCEEDED. This assertion is the bug, pinned.
+    expect(oldHeuristicWouldKeepRecord(error)).toBe(false)
+  })
+
+  test('yet it is exactly the case where the record must survive', () => {
+    const error = new MeltChangeError(outputData, quote)
+
+    // The payload is only useful to a reader that still has the record; the two
+    // are the same recovery. Keeping one and dropping the other is incoherent.
+    expect(error.outputData.length).toBeGreaterThan(0)
+    expect(error.quote).toBeDefined()
+  })
+})

src/models/WalletStore.ts

@@ -1336,12 +1336,25 @@ export const WalletStoreModel = types
                 return meltResponse
 
             } catch (e: any) {
-                if(!e.message.toLowerCase().includes('timeout') &&
-                   !e.message.toLowerCase().includes('network request failed')) {
-                  // remove only if it was not a timeout or network error
-                  Database.removeMeltRecovery(transactionId)
-                }   
-
+                // The melt recovery record is deliberately NOT removed here.
+                //
+                // By the time completeMelt throws, the melt request may already have
+                // been executed by the mint — cashu-ts 4.10 makes that explicit with
+                // MeltChangeError, which is raised only AFTER the payment went through
+                // and means solely that the NUT-08 change could not be reconstructed.
+                // The inputs are spent and the payment stands.
+                //
+                // Callers handle exactly that: TransferOperationApi._handleExecuteError
+                // re-checks the quote and, when it comes back PAID, calls
+                // recoverMeltQuoteChange to rebuild the change from this record. Deleting
+                // it here — which the old `unless the message says timeout/network` test
+                // did for every other error, MeltChangeError included — destroyed the one
+                // input that recovery needs, one step before it was read, and the user
+                // silently forfeited the change.
+                //
+                // Removal belongs with whoever learns the quote's terminal state:
+                // recoverMeltQuoteChange on PAID/UNPAID, and the UNPAID paths in
+                // TransferOperationApi. A PENDING melt must keep it either way.
                 let message = 'Lightning payment failed.'
                 if (isOnionMint(mintUrl)) message += TorVPNSetupInstructions;
                 throw new AppError(
@@ -1533,11 +1546,11 @@ export const WalletStoreModel = types
                 return meltResponse
 
             } catch (e: any) {
-                if(!e.message.toLowerCase().includes('timeout') &&
-                   !e.message.toLowerCase().includes('network request failed')) {
-                    Database.removeMeltRecovery(transactionId)
-                }
-
+                // Kept for the same reason as payLightningMelt: the mint may already
+                // have executed the melt, and the record is what change recovery reads.
+                // NUT-30 makes this sharper — an onchain melt is asynchronous by
+                // mandate, so "the request threw" says even less about whether the mint
+                // acted on it.
                 let message = 'Onchain payment failed.'
                 if (isOnionMint(mintUrl)) message += TorVPNSetupInstructions;
                 throw new AppError(

src/services/wallet/operations/transferOperationApi.ts

@@ -923,6 +923,13 @@ async function refresh(transactionId: number): Promise<Transaction> {
         })
         tx.update({status: TransactionStatus.REVERTED, data: JSON.stringify(txData)})
 
+        // Terminal failure: no change will ever come back for this quote, so the
+        // melt recovery record written before submission is now dead weight. Without
+        // this, every async melt that resolves UNPAID left an orphaned row behind —
+        // the row is only ever cleaned on the PAID path (_unblindMeltChange) and by
+        // recoverMeltQuoteChange, neither of which this branch reaches.
+        Database.removeMeltRecovery(transactionId)
+
         log.debug('[TransferOperationApi.refresh] Transaction reverted (UNPAID)', {transactionId})
 
         EventEmitter.emit('ev_asyncMeltResult', {
@@ -1080,6 +1087,13 @@ async function _handleExecuteError(
     }
 
     // ── UNPAID by mint ──────────────────────────────────────────────────
+    // The mint did not pay, so this quote will never return change to unblind and
+    // the melt recovery record is now dead weight. Dropped here rather than in
+    // WalletStore's catch, which cannot know the quote's terminal state and used to
+    // delete the record even when the melt had in fact succeeded. Covers all three
+    // exits below, since every one of them is reached only with state UNPAID.
+    Database.removeMeltRecovery(tx.id)
+
     if (WalletUtils.isTokenAlreadySpentError(e)) {
         // Mint says one of our inputs is already spent. Sync will reconcile;
         // drop the reservation without restoring (proofs likely SPENT at mint).