Skip to content

Commit f331193

Browse files
JunirezzJunirezz
committed
Merge pull request #1070: idempotent retry-safe transfer orchestration
2 parents 14772c4 + ab59f47 commit f331193

8 files changed

Lines changed: 1424 additions & 72 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99
## [Unreleased]
1010

1111
### Features
12+
- Harden the transfer orchestration service to be idempotent and retry-safe: wallet-scoped idempotency keys, request validation and canonicalisation, submission-boundary failure classification that never blindly resubmits a transfer whose outcome is unknown, stored terminal rejections, circuit-breaker fail-fast, a submission timeout, and an operator reconciliation queue with metrics (#1043)
1213
- Harden the vault comparison screen for multi-strategy selection: numeric strategy catalog with locale-aware formatting, URL-synced shareable selection and column ordering, best-in-class marking with non-colour cues, and screen-reader announcements for selection and sort changes (#1036)
1314
- Add journalled partial-failure recovery for multi-step withdrawals: resume-forward retries, reverse-order compensation, an operator escalation queue with admin endpoints, and a background sweeper for crashed or backed-off sagas (#954)
1415
- Add deterministic admin proposal nonces with replay rejection for admin rotation (#736)

‎backend/docs/TRANSFER_ORCHESTRATION.md‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
# Idempotent Retry-Safe Transfer Orchestration (Issue #1043)
2+
3+
A vault transfer moves money. `submitVaultOperation` builds a **fresh**
4+
transaction — fresh sequence number, fresh signature — and pushes it to the
5+
network. So retrying that call is not a repeat of the first attempt; it is a
6+
second transaction.
7+
8+
That makes "just retry it" the wrong default. `src/transferOrchestrator.ts` is
9+
the service that decides, for every failure, whether a retry is safe.
10+
11+
## What the previous implementation covered, and what it didn't
12+
13+
The original service wrapped the RPC in `IdempotencyStore.execute`. That gives
14+
the happy path: same key twice → one transaction, second caller replays the
15+
stored response. Everything else was open:
16+
17+
| Gap | Consequence |
18+
| --- | --- |
19+
| Client key used verbatim as the store key | Wallet A's key `"1"` occupied wallet B's slot — B replayed A's transaction hash or got a spurious 409 |
20+
| No request validation | A negative or `NaN` amount reached the signing path |
21+
| Every failure retried blind | A failure *after* the envelope hit the network was retried, which can transfer twice |
22+
| Terminal rejections not stored | A guaranteed-to-fail 422 re-ran the full build/simulate path on every retry |
23+
| No fail-fast, no timeout | A dead RPC absorbed the whole retry budget per request; a hung call held the key's in-flight slot for the life of the process |
24+
| No metrics, no alert | None of the above was visible |
25+
26+
## The retry-safety contract
27+
28+
> For a given **(wallet, idempotency key)** pair, at most one transaction is ever
29+
> submitted. Every later call either replays the stored outcome or fails loudly.
30+
> It never silently submits a second transaction.
31+
32+
## Failure classification
33+
34+
Everything rests on one question: *did the signed envelope reach the network?*
35+
36+
| Class | Meaning | Stored under the key? | Caller may retry? |
37+
| --- | --- | --- | --- |
38+
| `retryable` | Proven to have failed **before** submission. Nothing moved. | No — so the retry re-executes | Yes, same key |
39+
| `terminal` | The request itself is invalid; retrying cannot help. | Yes, as a `rejected` record | Retrying replays the same rejection |
40+
| `indeterminate` | The envelope may or may not have landed. | Yes, as an `in_doubt` record | **No** — blocked until reconciled |
41+
42+
The mapping comes from reading `submitVaultOperation`'s own error codes:
43+
44+
| Soroban code | Class | Why |
45+
| --- | --- | --- |
46+
| `INVALID_ADDRESS`, `INVALID_AMOUNT` | `terminal` | Argument validation, raised before anything is built |
47+
| `SIMULATION_ERROR` | `retryable` | Raised on the simulate path; no envelope was sent |
48+
| `RESTORE_REQUIRED` | `retryable` | Simulation asked for a ledger restore; nothing was sent |
49+
| `RPC_ERROR` | `retryable` | RPC returned `status === 'ERROR'` — an explicit rejection, so no transaction exists |
50+
| `SOROBAN_CIRCUIT_OPEN` | `retryable` | Fail-fast; the RPC was never called |
51+
| `SUBMISSION_FAILED` | `indeterminate` | Submit returned an unexpected status — the envelope was already handed over |
52+
| `INTERNAL_ERROR` | `indeterminate` | Catch-all wrapper; may be a socket error mid-submit |
53+
| `TRANSFER_TIMEOUT` | `indeterminate` | The call never settled; silence is not proof |
54+
55+
**Unknown failures default to `indeterminate`.** This is deliberately the
56+
opposite of `classifyWithdrawalFailure`, which defaults to `retryable`. That
57+
coordinator retries *idempotent* steps; a retry here mints a new transaction.
58+
When we cannot prove nothing moved, we refuse to move again.
59+
60+
## Wallet-scoped keys
61+
62+
The client's key never reaches the store directly:
63+
64+
```
65+
transfer:<sha256(normalizedWallet)[0..16]>:<clientKey>
66+
```
67+
68+
Two wallets can therefore both use `"checkout-1"` without colliding. The wallet
69+
is hashed rather than embedded so the key stays bounded and no address leaks into
70+
a Redis `KEYS`/`SCAN` listing. Normalisation is case-insensitive, so
71+
`gabc…`/`GABC…` are one wallet.
72+
73+
Keys must be 8–255 characters (`TRANSFER_ORCHESTRATION_MAX_KEY_LENGTH`) from
74+
`[A-Za-z0-9._:~-]`. The charset rules out whitespace and newlines, which are
75+
unsafe in a Redis key segment; the minimum length rules out trivially guessable
76+
keys that invite cross-request collisions within one wallet.
77+
78+
## Request canonicalisation
79+
80+
Before fingerprinting, the request is canonicalised: wallet upper-cased, asset
81+
upper-cased, amount trimmed. Two spellings of the same transfer therefore share
82+
one fingerprint and replay correctly instead of raising a false conflict.
83+
84+
Amounts must match `^\d+(\.\d+)?$` and be `> 0`. Strings are used so JSON cannot
85+
round the value, and the pattern rejects `NaN`, `Infinity`, `1e3` and signed
86+
values — all of which `Number()` would otherwise coerce further down the stack.
87+
88+
## The in-doubt window
89+
90+
When a failure is `indeterminate`, the orchestrator:
91+
92+
1. Stores an `in_doubt` record under the key. **This is the mechanism that stops
93+
a retry** — the next call hits the stored record instead of the RPC.
94+
2. Registers the transfer in the in-doubt registry with everything an operator
95+
needs to reconcile it: wallet, operation, amount, asset, failing code, the
96+
original idempotency key, and the trace ID.
97+
3. Logs at `error` with `alert: "transfer-in-doubt"` and raises the in-doubt
98+
gauge.
99+
4. Throws `TransferInDoubtError` (409) — to the caller that opened the window and
100+
to every caller after it.
101+
102+
### Reconciling
103+
104+
An operator checks the chain, then calls:
105+
106+
```ts
107+
// The transfer did land — replays of the original key now return this hash.
108+
await resolveInDoubtTransfer(storeKey, { transactionHash: 'abc…' });
109+
110+
// Nothing landed — release the key so the client may retry from scratch.
111+
await resolveInDoubtTransfer(storeKey, { discard: true });
112+
```
113+
114+
`listInDoubtTransfers()` and `getInDoubtTransfer(storeKey)` expose the queue.
115+
Neither leaks the internal fingerprint.
116+
117+
## Errors
118+
119+
Every failure is a `TransferOrchestrationError` subclass carrying `code`,
120+
`statusCode` and `classification`, so an HTTP layer can map them uniformly.
121+
122+
| Error | Status | When |
123+
| --- | --- | --- |
124+
| `TransferValidationError` | 400 / 422 | Malformed key or request; never reaches the network |
125+
| `TransferConflictError` | 409 | Key reused with a different body |
126+
| `TransferInDoubtError` | 409 | Outcome unknown; needs reconciliation |
127+
| `TransferUnavailableError` | 503 | Dependency down, nothing submitted (`retryAfterMs` set when the circuit is open) |
128+
| `TransferOrchestrationError` | 422 (stored) | A replayed terminal rejection |
129+
130+
## Metrics
131+
132+
| Metric | Type | Labels |
133+
| --- | --- | --- |
134+
| `transfer_orchestration_total` | counter | `operation`, `outcome` |
135+
| `transfer_orchestration_replay_total` | counter | `operation`, `replay_of` |
136+
| `transfer_orchestration_failure_total` | counter | `operation`, `classification`, `code` |
137+
| `transfer_orchestration_duration_ms` | histogram | `operation`, `outcome` |
138+
| `transfer_orchestration_in_doubt` | gauge | — |
139+
140+
`transfer_orchestration_in_doubt > 0` is the page-worthy signal: money may have
141+
moved without a ledger record. Pair it with the `transfer-in-doubt` log alert.
142+
143+
## Configuration
144+
145+
| Variable | Default | Purpose |
146+
| --- | --- | --- |
147+
| `TRANSFER_ORCHESTRATION_TIMEOUT_MS` | `45000` | Caps a single submission so a hung RPC cannot pin the key's in-flight slot. A timeout is classified `indeterminate`. |
148+
| `TRANSFER_ORCHESTRATION_MAX_KEY_LENGTH` | `255` | Upper bound on a client-supplied key. |
149+
150+
Both are read per call, so an operator can change them without a restart-time
151+
re-import. The circuit breaker and idempotency TTL are configured by their own
152+
modules (`CIRCUIT_BREAKER_*`, `IDEMPOTENCY_KEY_TTL_MS`).
153+
154+
## Durability note
155+
156+
The in-doubt registry is in-process, the same trade-off the withdrawal saga
157+
journal and the dead-letter queue make. The `in_doubt` record that actually
158+
enforces retry-safety lives in the idempotency store, which **is** shared across
159+
replicas via Redis — so a pod recycle cannot turn a parked transfer back into a
160+
resubmittable one. What a recycle loses is the operator *queue* view, not the
161+
guarantee. Mirror the `transfer-in-doubt` alert to durable storage if you need
162+
the queue to survive a restart.
163+
164+
## Usage
165+
166+
```ts
167+
import { orchestrateTransfer } from './transferOrchestrator';
168+
169+
const { transactionHash, replayed } = await orchestrateTransfer(
170+
{ operationType: 'deposit', walletAddress, amount: '1000', asset: 'USDC' },
171+
request.header('Idempotency-Key')!,
172+
);
173+
```
174+
175+
Call sites adopt the service by calling `orchestrateTransfer` in place of
176+
`submitVaultOperation` and mapping the typed errors above onto responses.
177+
178+
## Tests
179+
180+
`src/__tests__/transferOrchestrator.test.ts` covers submission and replay,
181+
concurrent coalescing, canonicalisation, wallet-scoped key isolation, conflict
182+
detection, key/amount/wallet validation, each classification branch, fail-fast on
183+
an open circuit, stored terminal rejections, the timeout path, and both in-doubt
184+
resolutions — including the central assertion that a retry after an
185+
indeterminate failure does **not** call the RPC a second time.

0 commit comments

Comments
 (0)