Skip to content

Fix the refund retry cap and a reverse swap skipped on a lagging read - #1410

Merged
ebma merged 4 commits into
stagingfrom
fix/monerium-refund-retry-cap
Oct 8, 2026
Merged

ebma merged 4 commits into
stagingfrom
fix/monerium-refund-retry-cap

Conversation

@ebma

@ebma ebma commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

What this fixes

Two bugs in the refund path (recovery.ts), both found in a local end-to-end rehearsal on Sepolia.

1. A refused refund step retried forever

A refund step that keeps failing (for example a SEPA redeem order Monerium refuses) was retried about every 20 seconds forever. The deposit never reached recovery_failed, REFUND FAILED was never logged, and the refund queue stayed blocked.

Root cause, in recovery.ts:

  • Below the five-attempt cap, retryOrFail wrote the failure reason into monerium_recoveries.error.
  • runRecoveryOrchestrator reads an error on a deposit that is still recovering as the operator's retry (the operator moved a parked refund back to recovering) and resets attempts to 0.
  • So every keeper cycle started over at attempt 1, and the cap was never reached.

Seen live: a refund payout refused by Monerium (HTTP 400) was logged as "attempt 1" on every cycle. With the fix applied, the next refused payout parked as recovery_failed after exactly 5 attempts.

2. A lagging balance read skipped the reverse swap

The refund's first step reads the refund wallet's USDC right after the recover receipt and took a zero balance as "a swap already landed" (the lost-hash case). A node behind the recover's block answers zero for both balances, so the swap was skipped, the float topped up the whole gap, and the recovered USDC stayed on the refund wallet with no step that ever moves it.

Seen live: a half-converted EUR 60 refund (35 EURe + 27.9 USDC recovered) skipped the swap and the float paid 25 EURe instead of a few cents; the payer's refund amount stays correct, but the float overpays and the USDC is stranded.

Fix: zero USDC counts as a landed swap only when the swap's EURe output shows (EURe above the recovered amount); otherwise the step reads again on the next cycle.

Changes

Retry cap:

  • retryOrFail below the cap now writes only attempts and phase; the warn log still carries each attempt's reason.
  • Only fail() sets error, so the orchestrator's reset fires only after a terminal failure that the operator moved back to recovering, which then gets five fresh attempts as before.
  • Docs: the architecture doc and the security spec described a backoff that does not exist and a per-step count; both now match the code (attempts count per refund, one per keeper cycle).

Reverse swap:

  • Moved: with USDC recovered but neither the USDC nor a swap output visible, return without changing the phase; the next cycle reads again.

Behaviour change: a refund step that fails five cycles in a row now parks as recovery_failed (about 100 s at the 20 s cycle), the documented outcome, instead of retrying indefinitely. No partner-visible field or webhook payload changes: error and attempts are not part of any snapshot.

Tests

  • New: recovery.test.ts feeds a lagging read (both balances zero) right after recover: the step stays in Moved with no calls, and once the read catches up it sends the reverse swap of the full USDC. It fails on the old code.
  • New: recovery.test.ts drives the real runRecoveryOrchestrator through five refused redeem orders. It expects attempts 1 to 4 with error empty, then recovery_failed with "after 5 attempts", no further Monerium call while parked, and five fresh attempts after an operator retry. It fails on the old code at the first cycle.
  • recovery.test.ts and monitoring.test.ts: 47 pass. tsc and Biome clean.
  • Two independent adversarial reviews could not refute the fix. Their notes, left out of scope: there is no backoff, so a Monerium outage of about five cycles parks a healthy refund for the operator; errors thrown outside the counted steps (listing or reading an order) are still retried without a cap and are surfaced by the refund monitor's age rule.

Cleanup pass

  • Changed: the orchestrator keeps its original one-line operator-retry comment; the invariant (only fail() writes error) is stated once, in retryOrFail (5cd656a, comment only).
  • Kept: the stale-read guard is one line in the existing Moved branch with no new state; nothing else in the diff is dead, duplicated or orphaned; no dependency or wire-contract change.
  • Skipped: folding the three copies of the confirmed recover execution fixture in recovery.test.ts into a helper. It would save about 15 lines but rewrite two tests this PR does not otherwise touch.
  • Gates: B2B and worker tests 217 pass on an isolated test database; tsc, bun run verify and wire-contract:check clean.

ebma added 2 commits October 8, 2026 10:45
Below the attempt cap, retryOrFail stored the failure in the recovery's
error, and the orchestrator reads an error on a recovering deposit as the
operator's retry and resets the attempts. So every keeper cycle started
over at attempt 1: a redeem Monerium keeps refusing was retried about
every 20 s indefinitely, REFUND FAILED was never logged and the deposit
never reached recovery_failed. Only the terminal failure now sets error;
the warn log still carries each attempt's reason.
Refund steps retry on later keeper cycles, without backoff, and the recovery row keeps only the error that parked it.
@netlify

netlify Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for vrtx-dashboard canceled.

Name Link
🔨 Latest commit 5cd656a
🔍 Latest deploy log https://app.netlify.com/projects/vrtx-dashboard/deploys/6ac78a40ae12800008f2618c

@netlify

netlify Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for vortex-sandbox ready!

Name Link
🔨 Latest commit 5cd656a
🔍 Latest deploy log https://app.netlify.com/projects/vortex-sandbox/deploys/6ac78a40a887400008f4437d
😎 Deploy Preview https://deploy-preview-1410--vortex-sandbox.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for vortexfi canceled.

Name Link
🔨 Latest commit 5cd656a
🔍 Latest deploy log https://app.netlify.com/projects/vortexfi/deploys/6ac78a40f5da740008bac7cb

The refund's first step read the refund wallet's USDC balance right after
the recover receipt and took zero as "a swap already landed". A node
behind the recover's block answers zero for both balances, so the swap
was skipped, the float topped up the whole gap, and the recovered USDC
stayed on the refund wallet with no step that moves it. Zero USDC now
counts as a landed swap only when the swap's EURe output shows; otherwise
the step reads again next cycle.

Found live in the sandbox rehearsal: a half-converted EUR 60 refund
skipped the swap of 27.9 USDC and the float paid 25 EURe instead.
@ebma ebma changed the title Stop a refused refund step from retrying forever Fix the refund retry cap and a reverse swap skipped on a lagging read Oct 8, 2026
retryOrFail already records why only fail() writes the error; the orchestrator keeps its original one-line comment.
@ebma
ebma merged commit 1a001e0 into staging Oct 8, 2026
6 checks passed
@ebma
ebma deleted the fix/monerium-refund-retry-cap branch October 8, 2026 15:24
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.

1 participant