Skip to content

fix: never time out an in-flight upload payment - #214

Merged
Nic-dorman merged 1 commit into
mainfrom
bug/wave-payment-timeout-race
Aug 12, 2026
Merged

fix: never time out an in-flight upload payment#214
Nic-dorman merged 1 commit into
mainfrom
bug/wave-payment-timeout-race

Conversation

@Nic-dorman

Copy link
Copy Markdown
Member

Problem

Both payment call sites in startRealUpload wrapped the payment helper in withTimeout(…, 300_000). payForQuotes / payForMerkleTree span wallet signing, tx broadcast and receipt polling, so the timer could expire after a tx had broadcast but before its receipt returned. The settled outer promise then discarded the tx hashes (or winner hash): the row reported a clean payment failure, confirm was never called, and a retry obtained a fresh quote and could pay again.

Reproduced deterministically by @dirvine during the #212 review (timeout fired with broadcast=true; the receipt arrived after the outer promise had settled). Inherited from before #212/#213. Tracked as V2-964.

Fix

  • Remove the outer timeout at both call sites — every classification now derives from the actual promise outcome. Wallet rejection remains the cancel path; a slow receipt is waited out, matching the existing no-frontend-timeout stance on chunk storage.
  • Delete the now-unused withTimeout helper.
  • Fake-timer regression test for the wave path: receipt at +400 s ⇒ row still paying at +300 s (not "Payment failed"), hashes preserved and passed to confirm_upload, payment helper called exactly once.

The multi-batch merkle loop got the identical treatment on the #213 branch (9a8429e) with its own regression test; this PR covers main's two call sites so the fix is in before 0.9.7-rc.1.

Testing

  • vitest: 73/73 (includes the new regression test)
  • vue-tsc --noEmit: clean

🤖 Generated with Claude Code

Both payment call sites raced the full broadcast+receipt span against a
300s timer: a slow receipt discarded tx hashes / the winner hash the
chain had already accepted, reported a clean payment failure, and let a
retry pay the same quotes again (reproduced deterministically in the
ant-ui#212 review). Drop the outer timeout at both sites — the wallet's
own reject is the cancel path — and delete the now-unused withTimeout
helper. Fake-timer regression test pins the wave path; the merkle
path's equivalent test lands with the multi-batch loop in #213.

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

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 19bd66be90c50676caa6b433177b80bf33c4c724.

No material blocker found. This removes the unsafe outer 300 s timeout from both payment paths, so a transaction broadcast while receipt polling is still in progress is no longer misclassified as an unpaid failure with its returned identifiers discarded. The existing wallet-rejection and payment-error paths remain intact.

The slow-receipt regression test is discriminating: it stays paying at the old 300 s boundary, completes at 400 s, calls payment once, and forwards the preserved transaction hash to confirm_upload. I also ran it as a negative control against the old production implementation, where it fails at the 300 s assertion as intended.

A reviewer raised viem’s default 180 s receipt timeout. That does not apply here: utils/payment.ts imports waitForTransactionReceipt from @wagmi/core; the locked Wagmi implementation defaults timeout to 0 and explicitly passes that to viem, disabling viem’s timer.

Local verification: 73/73 frontend tests, focused files-store tests 6/6, typecheck and diff checks passed. Current GitHub CI is green.

Non-blocking notes: a symmetric merkle slow-receipt test would strengthen coverage; and PR #213 will need the usual stack conflict resolution/rebase so it retains its multi-batch merkle flow alongside this timeout removal.

@Nic-dorman
Nic-dorman merged commit 3c9b8c9 into main Aug 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants