mirror of
https://github.com/minibits-cash/minibits_wallet.git
synced 2026-10-05 11:18:24 +00:00
minibits-cashandClaude Opus 5
a5ac3d7487
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>
105 lines
4.2 KiB
TypeScript
105 lines
4.2 KiB
TypeScript
/**
|
|
* 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()
|
|
})
|
|
})
|