From 1ed52cbf48cae870a5b4cd0e3d9690bdd1785bf0 Mon Sep 17 00:00:00 2001 From: Savvanis Spyros Date: Wed, 29 Jul 2026 22:16:53 +0300 Subject: [PATCH] fix(money): every terminal state a money row reaches now has a way out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CF-08 — verify the manual-sweep amount against the transaction it names. An overstated figure was trusted and the excess credited to the user. CF-06 — abandon must prove a withdrawal can never apply, not that it has not yet. CF-07 — a stuck sweep froze its deposit address and the documented remedy was a silent no-op. CF-24 — a build-failing pending withdrawal is marked stuck instead of retrying for ever and blocking the chain. CF-45 — the XRP mark rebase no longer collides the synthetic deposit txid. CF-25 — stop minting deposit addresses on a network the deployment does not run. CF-22 — deposit addresses and their ids are reachable by an operator, so the remedy the watcher logs can actually be followed. CF-23 — the sweeper and withdrawal worker report to Beacon. CF-44 — MAP.md no longer says ETH cannot be sent. Co-Authored-By: Claude --- MAP.md | 200 +++++- README.md | 64 +- docs/money-review.md | 63 +- services/pay/src/chains.ts | 64 ++ services/pay/src/db/migrate.ts | 22 + services/pay/src/db/schema.ts | 16 + services/pay/src/index.ts | 9 + services/pay/src/network.test.ts | 117 ++++ services/pay/src/network.ts | 115 ++++ services/pay/src/opswatch.test.ts | 161 +++++ services/pay/src/opswatch.ts | 218 +++++++ services/pay/src/outbound.test.ts | 531 ++++++++++++++++ services/pay/src/outbound.ts | 819 ++++++++++++++++++++++++- services/pay/src/routes/admin.ts | 522 +++++++++++++++- services/pay/src/routes/deposits.ts | 13 +- services/pay/src/routes/internal.ts | 38 +- services/pay/src/routes/wallet.ts | 13 +- services/pay/src/routes/withdrawals.ts | 9 +- services/pay/src/store.ts | 599 +++++++++++++++++- services/pay/src/sweep.test.ts | 466 +++++++++++++- services/pay/src/sweeper.ts | 129 +++- services/pay/src/watcher.test.ts | 232 +++++++ services/pay/src/watcher.ts | 210 +++++-- services/pay/src/withdrawer.test.ts | 335 ++++++++++ services/pay/src/withdrawer.ts | 212 +++++-- services/pay/src/xrp.test.ts | 201 +++++- 26 files changed, 5105 insertions(+), 273 deletions(-) create mode 100644 services/pay/src/network.test.ts create mode 100644 services/pay/src/network.ts create mode 100644 services/pay/src/opswatch.test.ts create mode 100644 services/pay/src/opswatch.ts create mode 100644 services/pay/src/watcher.test.ts create mode 100644 services/pay/src/withdrawer.test.ts diff --git a/MAP.md b/MAP.md index c391548..30b886e 100644 --- a/MAP.md +++ b/MAP.md @@ -98,7 +98,7 @@ in the `x-request-id` response header (`src/obs.ts:187`, `src/index.ts:35`). | Method + path | Auth | What it does | Called by | |---|---|---|---| | `GET /deposit-coins` | **none** | Static catalog: the five `SUPPORTED_DEPOSIT_COINS` with family, decimals, confirmation depth, explorer prefixes (`:18`, shared-libs `packages/shared/src/deposits.ts:34`). | clients rendering the coin picker | -| `POST /deposits` | user | Find-or-create this user's address for `{coin, network}`; mints through keyvault with `purpose:'deposit'` and a deterministic `orderId` of `deposit:::` (`:21-55`, `src/keyvault.ts:58`). 201. Any keyvault failure is 502 `keyvault_error`; an EMBER row minted before Hearth's rebuild is 503 `address_retired` rather than a dead `ember1…` address. | game funding panel (`funding.tsx:94`) | +| `POST /deposits` | user | Find-or-create this user's address for `{coin}` **on `PAY_DEPOSIT_NETWORK`**; mints through keyvault with `purpose:'deposit'` and a deterministic `orderId` of `deposit:::` (`:21-55`, `src/keyvault.ts:58`). 201. A body naming the other network is 400 `network_mismatch` (`src/network.ts`); an omitted `network` means the server's, and the shared schema's `'testnet'` default is deliberately never read. Any keyvault failure is 502 `keyvault_error`; an EMBER row minted before Hearth's rebuild is 503 `address_retired` rather than a dead `ember1…` address. | game funding panel (`funding.tsx:94`) | | `GET /deposits` | user | `{ addresses, payments }` — my addresses (with the informational `pending` amount) and my last 50 detected payments (`:58`, `src/store.ts:712`, `:721`). | game Wallet and Store pages (`Wallet.tsx:33`, `Store.tsx:26`) | | `GET /deposits/:id` | user | One detected payment, owner-checked in the handler → 403 for someone else's (`:68-79`). | nothing in-estate today | @@ -143,10 +143,11 @@ Whole router behind `requireAdmin` (`:81`). Deliberately narrow: it does not con |---|---|---|---| | `GET /admin/prices` | admin | Every administered price with `usd` as an exact decimal string, `source:'administered'`, `setBy`, `setByHandle`, `updatedAt` (`:84`). Today that is EMBER alone. | admin console via the Nimbus proxy `GET /admin/pay/prices` (`repos/platform/services/nimbus/src/routes/pay.ts:109`) | | `PUT /admin/prices/:coin` | admin | Sets one. Body `{ usd: string }` — **a decimal string, never a JSON number** (`:39-47`). 404 `not_administered` for any coin with a market; 400 `validation` for a non-positive or non-numeric value. Writes the row, updates the read-through cache, and logs the previous value (`:110-129`). | admin console Prices panel via `PUT /admin/pay/prices/:coin` (`nimbus/src/routes/pay.ts:138`) | -| `GET /admin/treasury` | admin | Every treasury address and its **live on-chain** spendable balance, read per request rather than cached, because a stale copy of this number is wrong exactly when it matters (`:133-161`). `spendable: null` with a reason for the three chains that cannot be sent from. | admin console via `GET /admin/pay/treasury` (`nimbus/src/routes/pay.ts:148`) | -| `POST /admin/deposit-addresses/:id/sweeps` | admin | Records funds an operator moved **by hand**, so the deposit watcher's arithmetic closes. Body `{ amount, txid, destination? }` — `amount` is everything that left, fee included. **The `txid` is required and the route refuses until the movement is `chains.sweepIsMature` deep** (409 `sweep_not_matured`, with `confirmations` and `required`; also 409 `sweep_not_on_chain` / `sweep_rejected`, 503 `chain_unreachable`). Recording earlier is the same double credit the sweeper rewrite removed — see §10. Idempotent per `(address, txid)`, and it writes a `matured` sweep row rather than only bumping a running total. No-op on Bitcoin, whose probe is a cumulative counter. Logged at `warn` with the operator's id and handle. | nothing — **no UI exists**; curl only | +| `GET /admin/treasury` | admin | Every treasury address and its **live on-chain** spendable balance, read per request rather than cached, because a stale copy of this number is wrong exactly when it matters (`:133-161`). `spendable: null` with a reason for the two chains that cannot be sent from (BTC and SOL — see [§9](#9-what-this-does-not-do); ETH left that pair when keyvault split its EVM signing rule by purpose). | admin console via `GET /admin/pay/treasury` (`nimbus/src/routes/pay.ts:148`) | +| `GET /admin/deposit-addresses` | admin | Every customer deposit address **and its `id`** — the parameter of the route below, which until CF-22 existed only in Postgres for anyone but the address's own owner. Optional `?coin=`, `?address=`, `?userId=`, `?limit=` (default 50, max 200) and `?offset=` (the reply carries `offset`, `limit` and `nextOffset`, null on the last page — without it only the newest page was reachable at all). `?address=` matches a `0x…` address **in any case** — an explorer renders them lowercase and this table stores the checksummed form — and exactly for base58/bech32 coins, where case is significant. Returns `lastSeen`, `swept`, `pending` as decimal strings, `watched` (false for an address on the other network — CF-25), and every sweep of ours whose amount has never reached `swept`, **`stuck` ones included**, because that is usually why the address is frozen. `?scan=true` adds the one thing no table holds: whether the address is frozen **right now**, recomputed from a live confirmed-depth balance by `store.scanDepositAddress` — the watcher's own arithmetic with the writes taken out — as `ok` \| `uncredited` \| `settling` \| `regression` \| `rebasing`, or `unwatched` / `unreadable`. It costs one RPC per address, so it is capped at 25 per request (400 `scan_too_wide` if a wider `limit` is asked for) and it credits nothing. | nothing — **no UI exists**; curl only. Not relayed by Nimbus | +| `POST /admin/deposit-addresses/:id/sweeps` | admin | Records funds an operator moved **by hand**, so the deposit watcher's arithmetic closes. Body `{ amount, txid, destination? }` — `amount` is everything that left, fee included, and digits only. **The `txid` is required and the route refuses until the movement is `chains.sweepIsMature` deep** (409 `sweep_not_matured`, with `confirmations` and `required`; also 409 `sweep_not_on_chain` / `sweep_rejected`, 503 `chain_unreachable`). Recording earlier is the same double credit the sweeper rewrite removed — see §10. **`amount` is then checked against that transaction** (`outbound.checkSweepMovement`): it must equal `value + fee` exactly and the transaction must have been sent BY this deposit address, or 409 `sweep_amount_mismatch` (carrying `moved`, the exact figure to post) / `sweep_wrong_address` / `sweep_unverified`. **And WHERE it went is checked as well**: a transaction from the deposit address back to itself is 409 `sweep_wrong_destination` (nothing left custody, so recording it advances `swept` against funds the probe can still see, and the next tick credits them again), and one paid to *another customer's* deposit address is 409 `sweep_destination_custodial` (`store.custodialDepositAddress`) — that destination's own watcher credits the arrival as a fresh deposit, so one movement of coins would credit two people. A treasury destination is fine, and is the point. The `to_address` recorded on the sweep row is the chain's, not the operator's free-text note. An overstated amount used to go straight into `deposit_addresses.swept` and be credited to the depositor on the next watcher tick — CF-08. A coin whose transactions this service cannot look up (`outbound.canReadTransaction`) is refused outright with 409 `sweep_unverifiable_chain`; **BTC is the one exception**, because its probe is a cumulative counter and recording moves no total at all. Idempotent per `(address, txid)`, and it writes a `matured` sweep row rather than only bumping a running total. **A conflicting `(address, txid)` row that was never recorded — the `stuck` sweep this route is usually reached because of — is now matured in place and the reply says `reconciled: true`** (CF-07); it used to answer 200 `replayed: true` and change nothing, which made the documented remedy a no-op against the one state it was documented for. If that row states a different amount, nothing is recorded and both figures are named in a 409 `sweep_amount_disagrees`. Logged at `warn` with the operator's id and handle, refusals included. | nothing — **no UI exists**; curl only | | `GET /admin/withdrawals` | admin | Every user's withdrawals, newest first, max 200, optional `?status=` from the six valid states (`:215-221`). | admin console via `GET /admin/pay/withdrawals` (`nimbus/src/routes/pay.ts:151`) | -| `POST /admin/withdrawals/:id/abandon` | admin | Refunds a withdrawal — but asks the chain first, every time, and **refuses** with 409 `withdrawal_on_chain` if the network can still see the transaction (`:231-249`). Only from `pending`/`signed`/`broadcast`/`stuck`. | nothing — **no UI exists**; curl only | +| `POST /admin/withdrawals/:id/abandon` | admin | Refunds a withdrawal — but a row with `raw_tx` is refunded only against **positive proof that those exact bytes can never be applied**, never on the absence of a receipt (`outbound.judgeAbandon`, CF-06). The transaction id is **derived from the bytes** for both families, so a null `txid` no longer skips the chain — that branch was the XRP double-pay. EVM: no receipt for the derived hash **and** `eth_getTransactionCount(treasury,'latest')` past the nonce inside the bytes. XRP: not in the ledger, the treasury's `Sequence` has not reached the one signed (`withdrawals.signed_sequence`), and the ledger is past the blob's `LastLedgerSequence` (`withdrawals.signed_expiry`). Refuses 409 `withdrawal_on_chain` / `withdrawal_still_applicable` (which says how to retire the nonce, or which ledger to wait for) / `withdrawal_unprovable`, and 503 `chain_unreachable` rather than 500. A `pending` row has no signature and is refunded as before. Only from `pending`/`signed`/`broadcast`/`stuck`; the refund logs the `proof` that allowed it. | nothing — **no UI exists**; curl only | The Nimbus proxy exists because the write is a `PUT` and forwards the **caller's own** access token, never a service secret, so `set_by` records the real operator @@ -158,12 +159,14 @@ the supported path. Whole router behind one shared secret (`:85`). These routes take `userId` as a **parameter**, not from a token, so whoever reaches them can read, debit, credit and liquidate any user -(`:22-47`). Every route requires an idempotency key — the callers are settlement loops that -retry by design. +(`:22-47`). Every route that MOVES money requires an idempotency key — the callers are +settlement loops that retry by design. The two reads (`/internal/wallet/:userId` and +`/internal/money-loops`) take none, because a repeated read is a read. | Method + path | Auth | What it does | Called by | |---|---|---|---| | `GET /internal/wallet/:userId` | service | Any user's wallet snapshot (`:88`) | crucible, to mark a bot's equity (`repos/crucible/services/crucible/src/clients/pay.ts:114`) | +| `GET /internal/money-loops` | service | **Is anything this service does with real money waiting on a human?** `{ state: 'ok' \| 'degraded', reasons[], counts }` from `opswatch.judgeMoneyLoops` over three aggregate queries and **no chain traffic**, so it is safe to poll. `degraded` means a withdrawal is `stuck` (signed bytes exist; nothing retries or refunds them), a sweep is `stuck` (the deposit address it came out of has stopped crediting), or the withdrawal queue has aged past half its refund deadline (an unfunded treasury, while there is still time to act). Always 200 — a monitor cannot tell a non-2xx "I am reporting a problem" from "I am unreachable", and those page different people. Behind the service token rather than on `/health` because `/health` is public through the tunnel and the shape of the withdrawal queue is not a public reading. CF-23. | **nothing yet** — it exists for a Beacon probe that lives in the `stack` repo and is not written | | `POST /internal/charge` | service | Debit. Body `{ userId, amount:int≥1, reason, source, ref, idempotencyKey }`. `source`/`ref` are written to the ledger row for machine-readable reconciliation (`:93-126`). 409 `insufficient_balance`. | crucible performance/settlement fees, key `crucible:` (`crucible/.../clients/pay.ts:133`) | | `POST /internal/credit` | service | Credit — a refund or returned allocation. Same body (`:129-159`). 422 `shard_overflow`. | crucible | | `POST /internal/trade` | service | Moves value between Shards and a custodial coin balance in either direction at the current oracle rate. `side:'buy'` needs `shards`; `side:'sell'` needs `amount` (`:163-238`). 503 `rate_unavailable` — a stale price is not a discount. | crucible live bots, key `crucible:` (`crucible/.../clients/pay.ts:173`) | @@ -176,7 +179,7 @@ pre-claim a peer service's key from their own session (`src/store.ts:1407-1419`) ## 3. Background workers -Five timers, all `.unref()`-safe or plain `setInterval`, all guarded against overlapping +Six timers, all `.unref()`-safe or plain `setInterval`, all guarded against overlapping ticks with a local `inFlight` boolean. None of them is leader-elected: **running two replicas of this service doubles every worker.** The correctness guards are all in the database (row locks and partial unique indexes), so a second replica is safe but wasteful — except @@ -200,30 +203,47 @@ that the deposit watcher would double its RPC load. load the address list at all stops the whole tick (`:38-43`). A tick that overruns the interval is skipped with a warn (`:27-31`). -### 3.2 Withdrawal worker — `src/withdrawer.ts:216` +### 3.2 Withdrawal worker — `src/withdrawer.ts:335` - **Trigger:** 3s after boot, then every `PAY_WITHDRAWAL_POLL_SECONDS` (default 20, floored - at 5) (`:259-261`). + at 5) (`:387-389`). - **Gate:** none — **started regardless of `PAY_WITHDRAWALS_ENABLED`** (`src/index.ts:69-72`). That flag stops new requests being *accepted*; a payment already signed still has to finish. -- **Work:** loads every withdrawal in `pending`/`signed`/`broadcast` (`src/store.ts:1145`), +- **Work:** loads every withdrawal in `pending`/`signed`/`broadcast` (`src/store.ts:1630`), groups by `coin:network`, runs the groups in parallel and each group **strictly serially** - (`:232-243`). Within a group: advance every already-signed row, then start at most **one** - new `pending` row, and only if `hasUnsettledOutbound` is false (`src/store.ts:1162`). -- **State machine** (`src/store.ts:941-961`): - `pending` → build + sign (`src/outbound.ts:603`) → commit `raw_tx` conditionally on still - being `pending` (`src/store.ts:1194`) → `signed` → broadcast → `broadcast` → poll depth → + (`:351-376`). Within a group: advance every already-signed row, then walk the `pending` ones + oldest-first, and only if `hasUnsettledOutbound` is false (`src/store.ts:1647`). The walk + stops at the first row that **holds** the chain — so at most **one signature** per chain per + tick, but a row that was refunded without signing left the treasury exactly as it found it + and the row behind it may have its turn in the same tick (`:362-371`). Before CF-24 it was + literally one row per tick, so clearing a queue of stale rows took one poll each. +- **State machine** (`src/store.ts:1426-1452`): + `pending` → build + sign (`src/outbound.ts:980`) → commit `raw_tx` conditionally on still + being `pending` (`src/store.ts:1700`) → `signed` → broadcast → `broadcast` → poll depth → `confirmed`. Side exits: `stuck` after `PAY_WITHDRAWAL_STUCK_MINUTES` (never refunded), and `failed` (refunded). -- **Failure:** +- **Failure.** Every way a build can fail has an exit, and the only question is whether it is + immediate or bounded — `withdrawer.planBuildFailure` (`:126`) is that decision, pure and + tested per error shape. It is a **refund** and never `stuck` in all four cases, because + nothing on this path has been signed and `store.markWithdrawalStuck` is gated on + `signed`/`broadcast` — calling it for a `pending` row updates nothing. - `InsufficientTreasuryError` → stays `pending`, retried; **after the stuck deadline it is - refunded from `pending`** (`:85-101`) because nothing was signed. + refunded from `pending`** because nothing was signed. + - `UnsupportedDestinationError` → permanent, refunded immediately, and it is the one refusal + whose message the **user** is given, because a contract at the destination is theirs to fix. - `SignRefusedError` / `UnsupportedChainError` → permanent, refunded immediately from - `pending` (`:103-111`). - - Broadcast failure → left in `signed`; the next tick re-sends the identical bytes (`:148-152`). - - Chain reports `rejected` → refunded from `signed`/`broadcast` (`:184-193`). This is the + `pending`. + - **Anything else** — an XRP treasury the ledger has never heard of, a keyvault + `ensureTreasury` cannot reach, a node answering nothing — is assumed transient and + **bounded by the same deadline**: retried, then refunded from `pending` (CF-24). It used to + log `will retry` with no deadline on the retrying, so a withdrawal that could never be built + sat `pending` for as long as the process ran with the user's balance debited, and took + every other withdrawal on its chain down with it. + - Broadcast failure → left in `signed`; the next tick re-sends the identical bytes (`:266-270`). + - Chain reports `rejected` → refunded from `signed`/`broadcast` (`:302-311`). This is the **only** proof that makes refunding a signed withdrawal safe. - - Anything else unresolved past the deadline → `stuck`, which is a request for a human. + - Anything else unresolved past the deadline, once bytes exist → `stuck`, which is a request + for a human. ### 3.3 Treasury sweeper — `src/sweeper.ts:206` @@ -291,6 +311,23 @@ that the deposit watcher would double its RPC load. - **Failure:** a rejected sweep is marked `stuck`, **not** reversed — but under the maturity rule a rejection is by definition before anything was recorded, so there is nothing to reverse and the address's arithmetic still closes. +- **`stuck` is no longer terminal (CF-07).** A sweep that is mined but has not reached maturity + depth within `chains.sweepStuckMinutes` is marked `stuck`, which takes it out of *both* + `activeSweeps` and the set that explains an address's shortfall — so before CF-07 the money + had left, `swept` could never be advanced, and the address reported `regression` and credited + nobody, permanently. Two paths close it now: the sweeper re-checks every stuck sweep the + chain gave a `mined_height` for (`recoverableStuckSweeps` → `recoverStuckSweep`) and matures + it when the depth finally arrives, and `POST /admin/deposit-addresses/:id/sweeps` reconciles + the row rather than reporting a replay. A sweep the network *rejected*, or one no node ever + saw, is never re-polled — nothing left the address, so there is nothing owed to `swept`. +- **The stuck deadline is derived, not borrowed.** `chains.sweepStuckMinutes(coin)` is four + times what the coin's maturity depth takes at its chain's nominal block rate, floored at 15 + minutes — EMBER 61 min, ETH/SOL/XRP the floor, BTC 40 min. It used to be + `PAY_WITHDRAWAL_STUCK_MINUTES`, a flat hour about a different worker waiting for a different + thing; raising EMBER's deposit depth past ~59 blocks would have put nominal maturity beyond + that hour and marked every healthy sweep on the chain stuck. There is no environment + variable for it: the deadline is a property of the chain, and a slower `PAY_SWEEP_POLL_SECONDS` + already stretches it (it is only ever evaluated inside a tick). ### 3.4 Idempotency reaper — `src/store.ts:239` @@ -318,6 +355,28 @@ that the deposit watcher would double its RPC load. `reason`, rather than a silent gap (`:62`, `:446`). No quote, or a quote older than `PAY_ORACLE_MAX_AGE_SECONDS` (900), makes every conversion for that coin 503. +### 3.6 Money-loop watch — `src/opswatch.ts` + +- **Trigger:** 30s after boot, then every 5 minutes (`WATCH_INTERVAL_MS`). No env var and no + chain traffic: three aggregate queries over two indexed columns. +- **Work:** `store.moneyLoopCounts` → `judgeMoneyLoops` → a `log.error` naming every reason + while anything is unattended, and one `log.warn` when it clears. +- **Why it exists (CF-23):** the withdrawal worker and the sweeper both park rows for a human + — `markWithdrawalStuck` and `markSweepStuck` are, in their own comments, "a request for a + human" — and nothing carried the request to one. A stuck withdrawal has no automatic exit + (only `POST /admin/withdrawals/:id/abandon`) and a stuck sweep freezes the deposit address + it came out of, while every Beacon probe (`/health`, `/coins/rates`, `/deposit-coins`) and + every Beacon journey stays green. The operator surfaces that do exist are all **pull**. +- **Throttled, deliberately.** `planMoneyLoopLog` (pure, tested) says a CHANGED problem + immediately, an unchanged one once an hour (`RESTATE_MS`, the same hour the sweeper's + `BLOCK_LOG_MS` uses), and a recovery once. A single line at the moment a row got stuck is + what the workers already did, and it ages into the background of Lantern's issue list; a + line that keeps coming back is what somebody notices. +- **What is still missing, and it is not in this repo.** Nothing pushes this to a person yet: + Beacon has no target calling `GET /internal/money-loops`, Beacon's webhook fires on + incidents only, and Lantern has no notification path of any kind. All three live in the + `stack` repo (`infra/beacon`, `infra/lantern`). See §7. + --- ## 4. Data model @@ -334,8 +393,8 @@ is gone, not disabled. |---|---|---| | `ledger` | Append-only statement of Shard movement. `delta` is signed; `reason` is prose a user reads; `source`/`ref` are the machine-readable attribution added later and null on old rows. | `ledger_user_created_idx (user_id, created_at DESC, id DESC)` (`migrate.ts:26`) makes the per-transaction wallet snapshot a bounded index scan. `ledger_source_ref_idx` is partial on `source IS NOT NULL` (`:50`). | | `wallets` | Materialized Shard balance, one row per Nimbus subject. Kept in sync with `ledger` inside the same transaction. | PK `user_id`. Locked `FOR UPDATE` on every credit (`store.ts:443`, `:1482`). | -| `deposit_addresses` | One custodial address per (user, coin, network). `last_seen` is the confirmed high-water mark; `swept` is the cumulative amount moved out; `pending` is tip-minus-credited, display only. | **Unique `(user_id, coin, network)`** (`migrate.ts:69`) — the whole find-or-create. Locked `FOR UPDATE` in `recordDepositAndCredit` (`store.ts:801`) and `recordSweepIn` (`:920`). | -| `deposit_payments` | Each detected credit. `txid` is synthetic: `:
:`. | **Unique `(address, txid)`** (`migrate.ts:101`) is what makes the watcher idempotent — and because the txid derives from the *locked* total, two ticks cannot mint two ids for the same funds (`store.ts:846`). | +| `deposit_addresses` | One custodial address per (user, coin, network). `last_seen` is the confirmed high-water mark; `swept` is the cumulative amount moved out; `pending` is tip-minus-credited, display only. | **Unique `(user_id, coin, network)`** (`migrate.ts:69`) — the whole find-or-create. Locked `FOR UPDATE` in `recordDepositAndCredit` (`store.ts:801`) and `recordSweepIn` (`:920`). Plus `(address)` and `(lower(address))`, because three money decisions look a row up by its address and none of them had an index: `isPlatformAddress` (on every withdrawal request), `custodialDepositAddress` (CF-08) and `depositAddressOverview` (CF-22). Two, because EVM addresses are matched case-folded and base58/bech32 exactly. | +| `deposit_payments` | Each detected credit. `txid` is synthetic: `:
::`, where the basis is `v0` for every coin whose mark has never been restated (`store.depositPaymentTxid`). Rows written before CF-45 carry the old three-segment form. | **Unique `(address, txid)`** (`migrate.ts:101`) is what makes the watcher idempotent — and because the txid derives from the *locked* total, two ticks cannot mint two ids for the same funds. **The basis segment is CF-45**: the totals an address records are strictly increasing only while the mark is monotonic, and the one-time XRP restatement lowers it by a base reserve, after which the address climbs back onto a total it has already recorded and the insert conflicts (`store.ts`, `depositPaymentTxid`). | | `coin_balances` | Materialized per-coin custodial balance, as a decimal string. | **PK `(user_id, coin, network)`** (`migrate.ts:111`). Every path that reads-then-writes it goes through `store.moveCoinBalance`, which issues **three** statements in one order: `INSERT … ON CONFLICT DO NOTHING`, then `SELECT … FOR UPDATE`, then the write. The first is not redundant — `FOR UPDATE` locks nothing when the row does not exist, so before it, two concurrent **first** credits of a `(user, coin, network)` both read zero and the second's absolute overwrote the first's. This table's earlier entry claimed the `FOR UPDATE`s made it safe; that was true of the code and false of the effect. | | `entitlements` | Granted purchases. | **Partial unique `(user_id, sku) WHERE kind <> 'private_world'`** (`migrate.ts:244`) is the *only* double-buy guard for own-once SKUs; rentals are deliberately outside it. | | `idempotency_keys` | Retry guard. `key = ${userId}:${route}:${clientKey}`; `response` stays NULL until the original transaction commits. | **PK `key`** (`migrate.ts:121`). The user-id namespace stops one user squatting on another's; the route namespace stops a user squatting on a peer service's. | @@ -354,8 +413,13 @@ is gone, not disabled. it is the one place this service can credit the same coin twice. A shortfall no larger than the address's in-flight sweeps is `settling`; anything else is `regression`, and neither ever advances the high-water mark. -- **The txid is derived from the locked total**, so `duplicate` (the payment row exists but - the high-water mark is behind it) is surfaced rather than papered over (`:867`). +- **The txid is derived from the locked total and the mark basis**, so `duplicate` (the + payment row exists but the high-water mark is behind it) is surfaced rather than papered + over. Nothing is known to produce it any more: it was reachable exactly once per XRP + address, when the restatement dropped the mark by one reserve and a top-up of exactly that + reserve brought the total back onto an id already on file — the deposit was then not + credited and the mark did not advance, so every subsequent tick recomputed the same total + and refused again until some other movement pushed the address past it (CF-45). - **Only `probe.confirmed` is ever credited.** `probe.tip` is written to `pending` and is never spendable (`:829-830`). - **`confirmations` on the payment row records the depth actually verified**, not the @@ -611,6 +675,21 @@ realistic casualty; widening the gas limit is not the fix, a second shape would `PAY_DEPOSIT_NETWORK=mainnet` every EMBER probe throws `EMBER mainnet RPC endpoint not configured` (`src/chains.ts` `chainEndpoint`). +**`PAY_DEPOSIT_NETWORK` is the only opinion about which chain this deployment is on** +(`src/network.ts`). Every route that takes a `network` — `POST /deposits`, `/coins/convert`, +`/coins/convert-to-ember`, `/withdrawals`, `/internal/trade` — refuses a body that names the +other one with 400 `network_mismatch`, and uses the configured network when the field is +absent. It has to be read off the RAW body: `createDepositAddressSchema` defaults `network` to +the literal `'testnet'`, so a server that trusted the parsed value would mint every address on +testnet the moment an operator set `mainnet` — an address on a chain nothing here watches, +handed to the user as if it worked. Addresses left on the other network (minted before this +rule, or stranded by an operator flipping the variable) are **not watched and not swept**: the +deposit watcher names each one in a warning exactly once per process and skips it +(`src/watcher.ts`), because probing it would credit a `coin_balances` row `GET /wallet` does +not read, and sweeping it would move a customer's coins against a balance nobody was granted. +forge-keyvault still holds the key under `deposit:::`, so recovering such +an address is an operator task and not a lost cause. + **`HEARTH_*_RPC` now means port 8545, not 8645.** Hearth serves the Ethereum JSON-RPC on 8545 and keeps the REST API and SSE feed on 8645 for the explorer (`hearth/docs/evm-spec.md` §6); Forge Pay moved wholesale to the former and no longer calls a @@ -624,6 +703,37 @@ here abandons a withdrawal or records a sweep" (`platform/services/nimbus/src/ro — and the admin console only calls the price and read routes. Both are curl-only. The first is the *only* documented remedy for a frozen deposit address. +**Its `:id` is now obtainable without a database shell, which it was not (CF-22).** The +remedy above is keyed by `deposit_addresses.id`, a random UUID; `GET /deposits` returns it to +the address's *owner* and to nobody else, so the operator holding the alarm had one way to +learn the parameter of the route that unfreezes the address — `psql`. Two things changed, and +the cheap one matters most: every watcher line about an address now carries `addressId` +(`watcher.depositScanLine`, asserted for each outcome in `watcher.test.ts`), so the alarm that +names the remedy also names its argument; and `GET /admin/deposit-addresses` answers the other +direction, from an address or a coin, with `?scan=true` to say which of them are frozen. +**Neither is relayed by Nimbus and neither has a console screen** — that half of CF-22 lives +in `repos/platform` and is not done. Until it is, this is still curl against pay directly with +an operator's own admin token. The listing pages with `?offset=` (and returns `nextOffset`), +so a deployment with more addresses than one page holds can still be enumerated — without it +`?scan=true` could only ever look at the 25 most recently minted, which is not an answer to +"which addresses are frozen". + +**The sweeper's own alarms now name the address too.** `sweeper.sweepAlarmFields` puts +`addressId` alongside `sweepId` on every line the sweeper writes about a parked sweep, for the +reason the watcher's lines carry it: two of those lines end with "POST +/admin/deposit-addresses/:id/sweeps", and that `:id` is the *deposit address's* uuid, not the +sweep's. An alarm that printed only `sweepId` handed the reader one uuid and asked them for a +different one. The commonest way an address freezes is a sweep of ours going `stuck`, so this +is the same defect as CF-22 on the neighbouring path. + +**Nothing pushes an alert anywhere in this estate, and only half of that is fixable here +(CF-23).** `GET /internal/money-loops` and the money-loop watch (§3.6) are the pay half: a +machine-readable verdict for a probe, and a repeating log line for Lantern. The other half — +a Beacon target that calls the route, a Beacon journey that exercises a withdrawal, a webhook +that is actually configured, or any Lantern notification module at all — lives in the `stack` +repo under `infra/` and is **not done**. Until it is, a stuck withdrawal is still discovered +by somebody looking. + **`GET /deposits/:id` and `GET /withdrawals/:id` have no callers** in any of the nine repos. **`/internal` is closed when `PAY_SERVICE_TOKEN` is unset**, which is the correct default and @@ -661,9 +771,24 @@ An explicit list, because "money can enter and never leave" was true for months anyone writing it down, and that class of omission is what this section exists to prevent. - **It does not hold fiat, and has no payment provider.** See §8. -- **It cannot send BTC, ETH or SOL.** Custodied balances in those three coins are, today, - permanently inside the platform. They can be converted to Shards or to EMBER, and the EMBER - can be withdrawn — which is the only exit, and it goes through the administered price. +- **It cannot send BTC or SOL — and cannot sweep them either, so those two are stuck at both + ends.** `WITHDRAWABLE_COINS` is `['EMBER','ETH','XRP']` (`src/outbound.ts:62`) and all three + leave this platform on chain, so read as an outbound inventory this section covers **three** + treasuries, not one. (This bullet used to say ETH could not be sent and that EMBER was "the + only exit". Both were stale: ETH became sendable when keyvault split its EVM signing rule by + purpose — §6, §7 — and XRP has been sendable since the rewrite. A reader sizing the on-chain + blast radius off the old text sized it short by the ETH and XRP treasuries.) What is still + true is BTC and SOL: `withdrawalUnsupportedReason` refuses each with a sentence rather than + half-implementing it (`src/outbound.ts:48-60`) — BTC because a segwit signature commits to + each input's value, so keyvault's `signBitcoin` wants a PSBT this service has no library to + build; SOL because `signSolana` allows only SPL mint instructions and explicitly refuses the + SystemProgram transfer that moving SOL is. **Nor can the platform move them.** Keyvault signs + for a `deposit` address only in `SWEEPABLE_FAMILIES` = `{evm, ember, xrp}` + (`repos/forge-keyvault/services/forge-keyvault/src/routes/vault.ts:102`), so custodial BTC + and SOL can never be swept into a treasury either: the coin sits in the per-user deposit + address, reachable by neither the user nor an operator, and only the customer's *balance* is + spendable — by converting to Shards or to EMBER and withdrawing the EMBER, at the + administered price. That, for those two coins alone, is the only exit. - **It does not check that the treasury can back custodial balances.** `convertCoinToEmber` (`src/store.ts:1562`) and `buyCoinWithShards` (`src/store.ts:565`) credit a `coin_balances` row with **no on-chain movement at all** and no reserve check. A user can swap BTC into @@ -774,6 +899,18 @@ recover from, because a hand-moved amount has no `sweeps` row to explain the sho the confirmed view still holds it. Both callers now apply `chains.sweepIsMature`, which is why the route demands a `txid`. +**And the `txid` now has to account for the AMOUNT beside it, which is the second half of the +same hole (CF-08).** Depth was verified and size was taken on trust: `amount` went into +`deposit_addresses.swept` unread, so an operator who moved 100 EMBER and typed 1000 credited +the depositor 900 EMBER nobody deposited — spendable, convertible and withdrawable, and +reconciled by nothing. The chain is asked what left the address (`outbound.OutboundMovement`, +`value + fee`, plus the sender) and the posted amount must equal it exactly; the refusal +carries the exact figure so the retry is a paste. The gate on the whole block used to be +`canSend`, which is a fact about **signing** and not about **reading** — so SOL, which +`canSend` refuses, skipped every check while `recordSweepIn` still moved its `swept` total. +It asks `outbound.manualSweepGate` now: verify, or refuse, or (BTC only, whose probe is +cumulative and whose `swept` cannot move) record inertly. + **Calling it too EARLY is the other half of the same trap, and it is the one that was live.** Until this change the caller was `markSweepBroadcast`, and the argument recorded for that ordering was that recording early would overstate `swept` while recording late "merely @@ -851,7 +988,12 @@ address. minutes later, so a payment that underbids its own ledger sits unapplied until it expires (`src/outbound.ts:337-342`). `PAY_XRP_MAX_FEE_DROPS` (default 2000, real fees ~10) is the ceiling, and `PAY_XRP_LEDGER_WINDOW` (default 1000 ledgers ≈ 70 minutes) is what stops an -abandoned signature being replayable months later (`src/env.ts:100-106`). +abandoned signature being replayable months later (`src/env.ts:100-106`). **The window is +recorded, not only signed**: `buildAndSignXrp` writes the `Sequence` and `LastLedgerSequence` +it signed into `withdrawals.signed_sequence` / `signed_expiry` in the same statement as the +bytes, because this service has no XRPL deserializer and abandoning a withdrawal has to be +able to prove the blob is dead. A row without them cannot be abandoned at all — which is +every row signed before CF-06. **`src/obs.ts` is a vendored copy, not a dependency.** It is byte-identical across every service in the estate and is fixed *there* first, then re-copied (`src/obs.ts:1-12`). It is diff --git a/README.md b/README.md index f3eca8f..7e0adb7 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,11 @@ first probe rather than a week of unexplained 404s. locked total, so two watcher ticks cannot mint two credits for the same funds. The comparison is `balance + swept` against a high-water mark; an unexplained drop is recorded as a `regression` that pauses crediting for that address rather -than silently forgiving it. +than silently forgiving it. The txid also names the *basis* the total was +computed under, because a payment identity made of the total alone is unique only +while the mark rises — and the one-time XRP reserve restatement lowers it, after +which the address climbs back onto totals it has already recorded and the next +deposit is refused as a duplicate instead of being credited. **Conversion runs in both shapes.** `POST /coins/convert` turns a coin balance into Shards. `POST /coins/convert-to-ember` turns any of BTC/ETH/SOL/XRP into @@ -129,10 +133,20 @@ Sequence, and one of them could never land. **Money only goes back when the chain says it did not move.** A refund happens when nothing was signed at all, or when a node reports the payment as -applied-and-failed. Anything else that does not resolve becomes `stuck` and waits +applied-and-failed. "Nothing was signed at all" has a deadline on it: a build +that keeps failing for a reason this service cannot classify — a treasury +account that does not exist, a keyvault it cannot reach — is retried only until +`PAY_WITHDRAWAL_STUCK_MINUTES` and then refunded, because a withdrawal left +queued forever is both the user's money withheld and, payments being serial per +chain, every other withdrawal on that chain withheld behind it. Anything else +that does not resolve becomes `stuck` and waits for an operator, who can abandon it through `POST /admin/withdrawals/:id/abandon` — a route that asks the chain first and refuses to refund a payment the network -can still see. +can still see. It refuses more than that: once bytes have been signed, a refund +needs POSITIVE proof that those exact bytes can never be applied — a nonce taken +by another transaction on EVM, an unconsumed Sequence past its +`LastLedgerSequence` on XRP. A missing receipt is a fact about the node, not +about the signature, and refunding on one pays the user twice. ## The treasury, and the sweeper @@ -160,6 +174,27 @@ refuses with `no_treasury_pinned`, which sets that one chain aside and no others > crediting that address. That route takes the transaction id and **refuses until > the movement is as deep as the sweeper's own accounting requires**; recording it > at the moment of the POST is the same double credit described below. +> +> **Its `:id` is the deposit address row's uuid, and two things now hand it over**: +> the `regression` alarm itself carries `addressId`, so the line that names the +> remedy also names its argument, and `GET /admin/deposit-addresses` answers the +> other direction — from an address off an explorer, a coin, or a user — with +> `?scan=true` to say which addresses are frozen right now. Before that the only +> place the id existed for anyone but the address's own owner was a `psql` shell +> on the production database, which is not a remedy anybody reaches for at 3am. +> The sweeper's own alarms carry it too, which matters because the commonest way +> an address freezes is a sweep of *ours* going `stuck` rather than an operator +> moving anything. +> +> **The amount is checked against the transaction, and so is where it went.** The +> posted figure must equal `value + fee` exactly — the refusal states the number +> the chain gives, so the second attempt is a paste rather than arithmetic — and +> the transaction must have left this deposit address and arrived somewhere that +> is not this platform's custody. A transaction back to the same address moves +> nothing out of it; one paid to another customer's deposit address is credited +> to that customer by their own watcher. Either recorded as a sweep advances the +> swept total against coins that are still ours, and the watcher's next tick +> hands the difference to a depositor as a deposit nobody made. Three things about the sweeper are worth knowing before it is ever switched on. @@ -206,6 +241,29 @@ sign a transfer and a `treasury` cannot sign a creation. > deletes a treasury row on its own, because that row is the only record of where > the platform's money is. +## When money is waiting on a human + +Both loops that move funds on chain — the withdrawal worker and the sweeper — +have a state they cannot get themselves out of. A withdrawal whose signed bytes +the network will neither confirm nor reject becomes `stuck` and is *never* +auto-refunded, because refunding it while a valid signature exists is how a user +gets paid twice; a sweep becomes `stuck` and freezes the deposit address it came +out of. Both are, in the workers' own comments, a request for a human. + +Nothing carried that request to one. `GET /internal/money-loops` is the reading +that can: `{ state: 'ok' | 'degraded', reasons, counts }`, three aggregate +queries, no chain traffic, always 200 so a monitor can tell "reporting a +problem" from "unreachable". It is behind the service token — the shape of the +withdrawal queue is not a public reading. Alongside it, a five-minute watch +restates the same verdict into the log at error level for as long as it is true, +because a single line at the moment a row got stuck is exactly what already +existed and exactly what nobody sees. + +> **This is still only half an alert.** Nothing yet *calls* that route: the +> Beacon target, a journey that exercises a withdrawal, and any notification path +> at all live in the `stack` repo, and Lantern has no notify module. Until one of +> them lands, the log line and the route are for whoever is already looking. + ## Running it ```bash diff --git a/docs/money-review.md b/docs/money-review.md index 8e4b517..9085ba8 100644 --- a/docs/money-review.md +++ b/docs/money-review.md @@ -41,7 +41,37 @@ reports one or more `regression` ticks. That is unavoidable for an out-of-band m is no sweep row to explain it — and it self-heals on the POST. The gate permits recording from exactly the block the probe stops seeing the funds, so a prompt operator sees none of it. -Findings 3, 5, 6 and 8 are untouched and still live as written. +**Finding 1's last paragraph — "no comparison against the chain" — no longer holds either.** +The route now asks the node what the transaction actually moved (`outbound.OutboundMovement`) +and refuses unless three things are true: it was sent BY this deposit address, it moved +exactly the posted `value + fee`, and it went somewhere that is not this platform's own +custody. The third is not decoration: verifying only the sender and the size accepts a +transaction from the deposit address straight back to itself, which moves nothing out of +custody while advancing `swept` — the same mint as an overstated amount, reached by a pasted +destination (CF-08). A coin whose transactions this service cannot look up is refused outright +rather than recorded on trust, which closed the SOL hole the same finding did not see: that +path skipped the whole verification block and still added the operator's number to `swept`. + +Findings 3, 6 and 8 are untouched and still live as written. + +**Finding 5 is fixed, and this document was wrong about its scope.** `POST /admin/withdrawals/ +:id/abandon` now refuses to refund a row with `raw_tx` unless the chain proves those exact +bytes can never be applied: for EVM, no receipt for the hash derived from the bytes AND +`eth_getTransactionCount(treasury,'latest')` past the nonce read out of the RLP; for XRP, not +in the ledger, the treasury's `Sequence` still short of the one signed, and the ledger past the +blob's `LastLedgerSequence`. The decision is a pure function (`outbound.judgeAbandon`) and +every branch of it is a test. + +The correction is the sentence "XRP is protected by `LastLedgerSequence`". It is not. The +route asked the chain only `if (withdrawal.txid)`, and `buildAndSignXrp` returns `txid: null` — +so an XRP payment whose `submit` reached rippled and WAS APPLIED, but whose HTTP response was +lost, was refunded with no chain contact of any kind. `LastLedgerSequence` bounds when a blob +can still be applied; it says nothing about one that already was, and the double pay in that +case is permanent rather than bounded by ~70 minutes. The remedy needed two things this +service did not have: a transaction id derivable from an XRP blob (`outbound.xrpTxHash`, which +is rippled's own SHA-512Half over the `TXN\0` prefix) and the two numbers the blob commits to, +which are now written down at signing time (`withdrawals.signed_sequence` / `signed_expiry`) +because there is no XRPL deserializer here to read them back. See CF-06. --- @@ -276,9 +306,10 @@ The question posed was whether the new `settling` tolerance can absorb a genuine `observeDeposit`'s test is `inFlight >= shortfall` (`store.ts:847`) and `inFlight` counts sweeps in `signed | broadcast | confirmed` with `recorded_at IS NULL` (`store.ts:862-884`). In the normal case the tolerance is bounded — a sweep that never matures is marked `stuck` -after `PAY_WITHDRAWAL_STUCK_MINUTES` (`sweeper.ts:287-295`), `stuck` leaves the set, and the -address correctly flips to `regression`. So a theft alongside an in-flight sweep is hidden for -at most one hour, not permanently. That is sound. +after `chains.sweepStuckMinutes` (61 minutes for EMBER; it was `PAY_WITHDRAWAL_STUCK_MINUTES` +when this was written, see finding 8), `stuck` leaves the set, and the address correctly flips +to `regression`. So a theft alongside an in-flight sweep is hidden for about an hour, not +permanently. That is sound. **The unbounded case is when nothing advances the sweep at all.** Three ways to get there: @@ -306,16 +337,20 @@ settling any more. ### 8. Lower severity, recorded rather than argued -- **A sweep that goes `stuck` below maturity freezes its address.** `advanceSweep` - (`sweeper.ts:287-295`) marks a sweep `stuck` if maturity is not reached within - `PAY_WITHDRAWAL_STUCK_MINUTES` (60). `stuck` can never mature (`matureSweep` guards on - `broadcast|confirmed`, `store.ts:1560`) and stops explaining, so the funds have left, `swept` - was never recorded, and the address latches `regression`. The code names the trigger - ("reachable for EMBER on a stalled chain") but not the consequence. Recoverable via the admin - route — and by then the movement is >60 min deep, so finding 1 does not bite. EMBER maturity - is 61 blocks ≈ 15 min against a 60-minute deadline; the margin disappears if block time - triples, which `findings.md` P1-4 says is a live concern for Hearth. The sweep deadline should - be derived from the coin's maturity, not borrowed from the withdrawal one (`sweeper.ts:89`). +- **A sweep that goes `stuck` below maturity freezes its address.** FIXED, and this entry was + wrong about the one thing that mattered. `advanceSweep` marks a sweep `stuck` if maturity is + not reached inside the deadline. `stuck` could never mature (`matureSweep` guarded on + `broadcast|confirmed`) and stops explaining, so the funds have left, `swept` was never + recorded, and the address latches `regression`. "Recoverable via the admin route" was not + true: that route inserts a row with the same `(address_id, txid)` as the stuck sweep, hits + `sweeps_address_txid_idx`, records nothing, and answers 200 `replayed: true` — the documented + remedy was a silent no-op against exactly this state, and only a hand-written `UPDATE sweeps` + cleared it (CF-07). Now: `matureSweep` accepts `stuck`, the sweeper re-checks stuck sweeps the + chain gave a `mined_height` for and records them when the depth arrives, and the admin route + reconciles the conflicting row in place. The deadline is also derived from the coin's maturity + rather than borrowed from the withdrawal worker — `chains.sweepStuckMinutes` — which is what + this entry asked for and what stops the margin disappearing if Hearth's block time triples + (`findings.md` P1-4). - **The reorg controls that `@cloudsforge/shared` says live here do not exist here.** `shared/src/deposits.ts` (EMBER entry) states plainly that depth "is explicitly NOT the real control … the controls that matter are economic — per-user caps, a halt on any reorg past ~5 diff --git a/services/pay/src/chains.ts b/services/pay/src/chains.ts index 5f2f03a..f13edf4 100644 --- a/services/pay/src/chains.ts +++ b/services/pay/src/chains.ts @@ -125,6 +125,70 @@ export function sweepIsMature(coin: DepositCoin, confirmations: number): boolean return confirmations >= sweepMaturityConfirmations(coin) } +/** + * Nominal seconds per unit of the thing each family counts confirmations in — a block for + * the EVM chains and Bitcoin, a validated ledger for XRP, and for Solana the time its + * `finalized` commitment takes to catch up with a slot. + * + * Nothing here measures a chain and nothing depends on these being exact. They exist to + * answer one question — how long ought WAITING for a given depth to take? — and the answer + * is multiplied by a wide slack before anything is decided on it. + */ +const BLOCK_SECONDS: Record = { + // Hearth mines every 15 seconds (hearth/docs/evm-spec.md), which is the cadence EMBER's + // 60-block deposit depth was argued against in the first place. + hearth: 15, + // Ethereum's slot time. A missed slot is inside the slack below. + evm: 12, + bitcoin: 600, + // `finalized` is roughly 32 slots behind the tip at ~400 ms a slot. + solana: 13, + xrp: 4, +} + +/** + * How much slower than nominal a chain may run before a sweep that has not reached its depth + * is somebody's problem rather than the weather. + * + * Generous on purpose, and the asymmetry is the argument: declaring a sweep stuck too early + * costs a deposit address that stops crediting anybody (a `stuck` sweep leaves + * `store.UNMATURED_SWEEP_STATUSES` and stops explaining the address's shortfall), while + * declaring one too late costs nothing but a later alarm on a chain that has already stopped. + */ +export const SWEEP_STUCK_SLACK = 4 + +/** + * No chain's deadline is shorter than this, however fast its blocks are. + * + * XRP validates a ledger every four seconds and needs exactly one of them, so its derived + * deadline is sixteen seconds — which would mark a sweep stuck for one slow answer from + * rippled, or for one restart of this process. The floor is what keeps the deadline a + * statement about the chain rather than about a single RPC call. Note also that the deadline + * is only ever evaluated inside a sweeper tick, so a deployment that polls more slowly than + * the default five minutes stretches every deadline with it automatically and needs nothing + * configured here. + */ +export const SWEEP_STUCK_FLOOR_MINUTES = 15 + +/** + * How long a sweep may sit below the depth its accounting needs before it is `stuck`. + * + * DERIVED FROM THE DEPTH IT IS WAITING FOR. It used to be `PAY_WITHDRAWAL_STUCK_MINUTES`, a + * flat hour borrowed from a different worker waiting for a different thing. A withdrawal is + * late when a destination has not been paid, which is a question about one transaction being + * mined; a sweep is late when the HEAD has not moved `sweepMaturityConfirmations` past it, + * which is a question about the rate of the whole chain. Nothing ties the two together, and + * the borrowed number is wrong in both directions: EMBER's 61 blocks are fifteen minutes of a + * healthy chain, so an hour is four times the honest deadline; and a deposit depth this + * estate raises past ~59 blocks (hearth/docs/exchange-integration.md §4 already argues for + * depth) would put nominal maturity BEYOND the hour, marking every sweep on a perfectly + * healthy chain stuck — which freezes the deposit address of every depositor swept. + */ +export function sweepStuckMinutes(coin: DepositCoin): number { + const nominalSeconds = sweepMaturityConfirmations(coin) * BLOCK_SECONDS[depositChainInfo(coin).family] + return Math.max(SWEEP_STUCK_FLOOR_MINUTES, Math.ceil((nominalSeconds * SWEEP_STUCK_SLACK) / 60)) +} + /** A single address probe at both confirmation depths. */ export interface FundedProbe { /** Funded total at the required confirmation depth (smallest units). Creditable. */ diff --git a/services/pay/src/db/migrate.ts b/services/pay/src/db/migrate.ts index f81cf7b..fd78ee8 100644 --- a/services/pay/src/db/migrate.ts +++ b/services/pay/src/db/migrate.ts @@ -89,6 +89,20 @@ export async function migrate() { // every already-credited XRP address would report `regression` forever from the moment that // change shipped. See store.MARK_BASIS for the fence around the restatement. await client`ALTER TABLE deposit_addresses ADD COLUMN IF NOT EXISTS mark_basis TEXT` + // Look one up BY ADDRESS, which three money decisions do and none of them had an index for. + // `store.isPlatformAddress` refuses a withdrawal that would pay this platform's own custody + // and runs on every withdrawal request; `store.custodialDepositAddress` refuses a manual + // sweep into another customer's deposit address (CF-08); `store.depositAddressOverview` + // answers an operator holding an address off an explorer (CF-22). Two indexes because there + // are two comparisons and one cannot serve the other: EVM addresses are matched with the + // case folded (a node and an explorer both render them lowercase, this table stores the + // EIP-55 form), and base58/bech32 exactly, where folding would match an address nobody + // asked about. + await client`CREATE INDEX IF NOT EXISTS deposit_addresses_address_idx ON deposit_addresses (address)` + await client` + CREATE INDEX IF NOT EXISTS deposit_addresses_address_lower_idx + ON deposit_addresses (lower(address)) + ` await client` CREATE TABLE IF NOT EXISTS deposit_payments ( id TEXT PRIMARY KEY, @@ -185,6 +199,8 @@ export async function migrate() { status TEXT NOT NULL DEFAULT 'pending', txid TEXT, raw_tx TEXT, + signed_sequence TEXT, + signed_expiry TEXT, confirmations INTEGER NOT NULL DEFAULT 0, failure_reason TEXT, created_at TIMESTAMPTZ NOT NULL DEFAULT now(), @@ -193,6 +209,12 @@ export async function migrate() { confirmed_at TIMESTAMPTZ ) ` + // Added after the table: what an XRP blob committed to, which is the only evidence that can + // ever show an abandoned withdrawal's signature is dead (CF-06, schema.ts). Nullable and + // left null on every existing row on purpose — a row signed before this existed carries a + // blob nothing here can read, and it is refused rather than refunded on an assumption. + await client`ALTER TABLE withdrawals ADD COLUMN IF NOT EXISTS signed_sequence TEXT` + await client`ALTER TABLE withdrawals ADD COLUMN IF NOT EXISTS signed_expiry TEXT` await client`CREATE INDEX IF NOT EXISTS withdrawals_user_idx ON withdrawals (user_id, created_at DESC)` // The outbound worker's whole working set is "everything not finished yet", and on a table // that only grows that is a shrinking fraction of it. Partial, so the index stays small. diff --git a/services/pay/src/db/schema.ts b/services/pay/src/db/schema.ts index 2debfe4..49210b2 100644 --- a/services/pay/src/db/schema.ts +++ b/services/pay/src/db/schema.ts @@ -170,6 +170,22 @@ export const withdrawals = pgTable('withdrawals', { status: text('status').notNull().default('pending'), txid: text('txid'), rawTx: text('raw_tx'), + /** + * The account sequence `raw_tx` consumes, and the chain height past which it can never be + * applied — both as decimal strings, both written in the same statement as the bytes. + * + * XRP ONLY, and null everywhere else. They exist because abandoning a withdrawal refunds a + * user while a valid signature for the same money is still in `raw_tx`, so it may only + * happen against POSITIVE proof that those bytes can never apply (CF-06). On an EVM chain + * that proof is derivable from the bytes — `outbound.legacyNonce` reads the nonce back out + * of the RLP — but XRPL binary serialization is a format this service does not speak, so + * the two numbers `buildAndSignXrp` signs are written down here instead. A row that lacks + * them cannot be abandoned at all, which is the fail-closed answer and not an oversight: + * every withdrawal signed before this column existed is one whose blob nothing here can + * read. + */ + signedSequence: text('signed_sequence'), + signedExpiry: text('signed_expiry'), confirmations: integer('confirmations').notNull().default(0), /** Why it failed, or why it is stuck. Shown to the user, so it is written for one. */ failureReason: text('failure_reason'), diff --git a/services/pay/src/index.ts b/services/pay/src/index.ts index ff04ddd..2d9d224 100644 --- a/services/pay/src/index.ts +++ b/services/pay/src/index.ts @@ -9,6 +9,7 @@ import { internalRoutes } from './routes/internal.js' import { monetizationRoutes } from './routes/monetization.js' import { walletRoutes } from './routes/wallet.js' import { withdrawalRoutes } from './routes/withdrawals.js' +import { startMoneyLoopWatch } from './opswatch.js' import { startIdempotencyReaper } from './store.js' import { startPriceOracle } from './pricing.js' import { startDepositWatcher } from './watcher.js' @@ -93,6 +94,14 @@ if (!env.sweepEnabled) { // Expire spent idempotency keys, which otherwise accumulate forever. startIdempotencyReaper(app.log) +// Say — repeatedly, at error level, for as long as it is true — that one of the two loops +// above has parked money for a human. Both workers call marking a row `stuck` "a request for +// a human" and neither had any way to make the request: a stuck withdrawal has no automatic +// exit and a stuck sweep freezes a customer's deposit address, and every Beacon probe stayed +// green through both. The machine-readable half of the same reading is +// GET /internal/money-loops, which is what a probe should assert on. CF-23. +startMoneyLoopWatch(app.log) + const address = await step(app.log, 'listen', () => app.listen({ port: env.port, host: '0.0.0.0' }), ) diff --git a/services/pay/src/network.test.ts b/services/pay/src/network.test.ts new file mode 100644 index 0000000..5a4670d --- /dev/null +++ b/services/pay/src/network.test.ts @@ -0,0 +1,117 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { createDepositAddressSchema } from '@cloudsforge/shared' + +/** + * The one-deployment-one-network rule, tested against the configuration that breaks it. + * + * WHY THE ENV IS SET HERE AND THE IMPORT IS DYNAMIC. `env.ts` reads PAY_DEPOSIT_NETWORK once, + * at module load, and `network.ts` freezes it into DEPLOYMENT_NETWORK — which is the right + * shape for a process that settles on exactly one chain and the wrong shape for a static + * `import`. So the variable is set first and the module is pulled in afterwards. `node --test` + * runs every test file in its own child process, so this does not leak into the suites that + * assert against the default testnet configuration. + * + * WHY MAINNET. Because the bug is invisible on testnet: the shared schema defaults `network` + * to the LITERAL 'testnet', which happens to equal the default configuration, so every test + * written against a default deployment passes whether the server consults its own setting or + * the caller's. It is only when an operator sets PAY_DEPOSIT_NETWORK=mainnet — the one-line + * go-live change — that "the caller's network" and "this deployment's network" come apart, + * and every deposit address in the product is minted on a chain nothing watches. + */ +process.env.PAY_DEPOSIT_NETWORK = 'mainnet' +const { DEPLOYMENT_NETWORK, networkRefusal, partitionByDeploymentNetwork } = await import( + './network.js' +) + +test('the shared schema still defaults a missing network to testnet, whatever the server runs', () => { + // This is the defect in one assertion, and it is a property of shared-libs that this + // service cannot change: `createDepositAddressSchema` fills an absent `network` with a + // constant. Read straight out of `parsed.data` — which is what `POST /deposits` used to do + // — a mainnet deployment mints testnet addresses for every client in the estate, because + // the only client there is sends `{ coin }` and nothing else (funding.tsx). + const parsed = createDepositAddressSchema.safeParse({ coin: 'ETH' }) + assert.equal(parsed.success, true) + assert.equal(parsed.data!.network, 'testnet') + assert.equal(DEPLOYMENT_NETWORK, 'mainnet') + assert.notEqual(parsed.data!.network, DEPLOYMENT_NETWORK) +}) + +test('a body that omits network is served on the deployment network, not refused', () => { + // The half of the rule that keeps the product working. Every client posts `{ coin }`, so a + // check that refused an absent field would 400 the game's funding panel on the same deploy + // that fixed the minting. + assert.equal(networkRefusal({ coin: 'ETH' }), null) + assert.equal(networkRefusal({}), null) + assert.equal(networkRefusal(undefined), null) + assert.equal(networkRefusal(null), null) +}) + +test('a body that names the other network is refused', () => { + const refusal = networkRefusal({ coin: 'ETH', network: 'testnet' }) + assert.notEqual(refusal, null) + assert.equal(refusal!.code, 'network_mismatch') + // Both networks are named, because the caller's next question is which one they should + // have asked for and the answer is server configuration they cannot see. + assert.match(refusal!.error, /mainnet/) + assert.match(refusal!.error, /testnet/) +}) + +test('a body that restates the deployment network correctly is allowed through', () => { + assert.equal(networkRefusal({ coin: 'ETH', network: 'mainnet' }), null) +}) + +test('a network that is not a string is left to the schema, not given a second code', () => { + // `{ network: 42 }` is already a 400 `validation` from every route's safeParse. Answering + // it here as well would mean the same malformed body produced different codes depending on + // which route it hit. + assert.equal(networkRefusal({ coin: 'ETH', network: 42 }), null) + assert.equal(networkRefusal({ coin: 'ETH', network: ['mainnet'] }), null) +}) + +test('the watcher splits foreign rows out and reports each of them exactly once', () => { + const rows = [ + { id: 'a', network: 'mainnet' }, + { id: 'b', network: 'testnet' }, + { id: 'c', network: 'mainnet' }, + { id: 'd', network: 'testnet' }, + ] + const reported = new Set() + + const first = partitionByDeploymentNetwork(rows, reported) + assert.deepEqual( + first.watched.map((r) => r.id), + ['a', 'c'], + ) + assert.deepEqual( + first.unreported.map((r) => r.id), + ['b', 'd'], + ) + + // Second tick, same rows: the foreign ones are still skipped and still not probed, and the + // warning is not repeated. A watcher that logged them every PAY_DEPOSIT_POLL_SECONDS would + // write the same two lines 2,880 times a day, which is how a real warning gets filtered out. + const second = partitionByDeploymentNetwork(rows, reported) + assert.deepEqual( + second.watched.map((r) => r.id), + ['a', 'c'], + ) + assert.deepEqual(second.unreported, []) + + // A row that appears later is new information and is reported. + const third = partitionByDeploymentNetwork([...rows, { id: 'e', network: 'testnet' }], reported) + assert.deepEqual( + third.unreported.map((r) => r.id), + ['e'], + ) +}) + +test('every address is watched when nothing is on another network', () => { + const rows = [ + { id: 'a', network: 'mainnet' }, + { id: 'b', network: 'mainnet' }, + ] + const { watched, unreported } = partitionByDeploymentNetwork(rows, new Set()) + assert.equal(watched.length, 2) + assert.deepEqual(unreported, []) +}) diff --git a/services/pay/src/network.ts b/services/pay/src/network.ts new file mode 100644 index 0000000..6a4445e --- /dev/null +++ b/services/pay/src/network.ts @@ -0,0 +1,115 @@ +import type { ChainNetwork } from '@cloudsforge/shared' +import { env } from './env.js' + +/* ------------------------------------------------------------------ * + * One deployment, one network. + * + * WHY THIS IS NOT A CHOICE THE CALLER GETS TO MAKE. Every part of this service is wired to + * exactly one network at a time: `chains.chainEndpoint` resolves the RPC from + * `PAY_DEPOSIT_NETWORK`'s pair of URLs, the deposit watcher probes each address against the + * network stored on its row, and `store.walletSnapshot` reads `coin_balances` for + * `env.depositNetwork` and nothing else. A request that names the OTHER network is therefore + * not a request for a different product — it is a request to create state this deployment + * does not operate. On `POST /deposits` that state is an address: an EVM address is identical + * on both networks, so what comes back looks exactly like a working deposit address, and + * anything sent to it is watched by a node on the wrong chain, credited nowhere, and + * recoverable only by an operator re-deriving the key out of forge-keyvault. On the money + * routes it is a balance `GET /wallet` does not show, spendable only by whoever knew to ask + * for it by name. + * + * OMITTED IS NOT THE SAME AS WRONG, AND THAT DISTINCTION IS THE BUG. Every client in the + * estate posts `{ coin }` with no network at all; `createDepositAddressSchema` (shared-libs + * `packages/shared/src/deposits.ts`) then fills the hole with the LITERAL string 'testnet' + * rather than with whatever this server runs. So the moment an operator sets + * `PAY_DEPOSIT_NETWORK=mainnet`, honouring the parsed body mints every address on testnet and + * no deposit in the product can be credited again. Rejecting the parsed body would be just as + * wrong: it would 400 the game's funding panel for a field it never sent. + * + * So the rule is read off the RAW body, before any schema default has been applied: + * + * - `network` absent -> means "whatever this deployment settles on", always accepted. + * - `network` stated -> must equal `env.depositNetwork`, or the request is refused 400. + * + * and the value the handler then uses is `env.depositNetwork`, never the parsed one, because + * after the check they are either equal or the request is already over. + * ------------------------------------------------------------------ */ + +/** The single network this process mints on, watches and pays from. */ +export const DEPLOYMENT_NETWORK: ChainNetwork = env.depositNetwork + +/** The shape every route sends back for a body that names the other network. */ +export interface NetworkRefusal { + error: string + code: 'network_mismatch' +} + +/** + * The `network` the caller actually typed, as opposed to the one Zod supplied for them. + * + * Anything that is not a string is `undefined` here rather than a refusal: the schema parse + * that happens either side of this call already answers 400 `validation` for it, and two + * different codes for one malformed field would only make the failure harder to read. + */ +function statedNetwork(body: unknown): string | undefined { + if (typeof body !== 'object' || body === null) return undefined + const value = (body as { network?: unknown }).network + return typeof value === 'string' ? value : undefined +} + +/** + * Null when this body may be served on the deployment's network, or the 400 to send back. + * + * Takes the raw request body — NOT the parsed data — for the reason in the header comment: + * the deposit schema defaults the field, so by the time Zod is done "the caller said testnet" + * and "the caller said nothing" are the same value. + */ +export function networkRefusal(body: unknown): NetworkRefusal | null { + const stated = statedNetwork(body) + if (stated === undefined || stated === DEPLOYMENT_NETWORK) return null + return { + error: + `this deployment settles on ${DEPLOYMENT_NETWORK}, so it cannot mint an address, ` + + `credit a balance or make a payment on ${stated}. Omit "network" — it is taken from ` + + 'the server.', + code: 'network_mismatch', + } +} + +/** + * Split a tick's deposit-address rows into the ones this deployment watches and the ones it + * must report and leave alone, marking the reported ones so they are only said once. + * + * Rows on another network exist for two reasons and both need saying out loud exactly once. + * Either they were minted before this check existed, by a client that asked for the other + * network; or an operator has just flipped `PAY_DEPOSIT_NETWORK` at go-live and every address + * ever minted is suddenly foreign. The second is the one that matters, and it is why this is + * a warning per row and not a single counter: an operator needs the addresses in front of + * them, because each one is a key forge-keyvault still holds and a balance nothing here will + * ever move again. + * + * PROBING THEM ANYWAY WOULD BE WORSE. It would credit a `coin_balances` row `GET /wallet` + * does not read and no route will now spend, i.e. it would keep manufacturing the exact state + * an operator has to unpick by hand. Skipping leaves the coins where they are, on a chain, at + * an address whose key is still derivable — which is the recoverable end of the two. + * + * `reported` is passed in rather than held here so the caller owns its lifetime (one watcher, + * one process) and so this is testable without a clock or a logger. + */ +export function partitionByDeploymentNetwork( + rows: readonly T[], + reported: Set, + network: ChainNetwork = DEPLOYMENT_NETWORK, +): { watched: T[]; unreported: T[] } { + const watched: T[] = [] + const unreported: T[] = [] + for (const row of rows) { + if (row.network === network) { + watched.push(row) + continue + } + if (reported.has(row.id)) continue + reported.add(row.id) + unreported.push(row) + } + return { watched, unreported } +} diff --git a/services/pay/src/opswatch.test.ts b/services/pay/src/opswatch.test.ts new file mode 100644 index 0000000..5df475a --- /dev/null +++ b/services/pay/src/opswatch.test.ts @@ -0,0 +1,161 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { judgeMoneyLoops, planMoneyLoopLog, QUEUE_ALARM_FRACTION, RESTATE_MS } from './opswatch.js' +import type { MoneyLoopCounts } from './store.js' + +/** + * CF-23 — the verdict a monitor goes red on. + * + * The defect is not that this service hides the state of its money loops: `GET + * /admin/withdrawals?status=stuck` and `GET /admin/treasury` have always answered. It is that + * every one of those surfaces is a PULL, so somebody had to already suspect the problem, and + * the one thing that fires by itself — `log.error` from a worker as it parks a row — reaches + * Lantern's issue list and stops there. Beacon's pay probes cover `/health`, `/coins/rates` + * and `/deposit-coins`, and all three stay green through a treasury that cannot pay anybody. + * + * So the reading has to be one field a probe can assert on without knowing this schema, and + * that field has to be RIGHT — hence a pure function and this file. Everything it needs is a + * parameter: no database, no clock, no env. + */ + +const HEALTHY: MoneyLoopCounts = { + withdrawalsStuck: 0, + withdrawalsStuckOldestMinutes: null, + withdrawalsQueued: 0, + queuedOldestMinutes: null, + sweepsStuck: 0, + sweepsStuckOldestMinutes: null, + addressesFrozenBySweep: 0, +} + +const OPTS = { withdrawalStuckMinutes: 60 } + +test('a service with nothing waiting on a human reports ok', () => { + const verdict = judgeMoneyLoops(HEALTHY, OPTS) + assert.equal(verdict.state, 'ok') + assert.deepEqual(verdict.reasons, []) + + // A withdrawal that was accepted a moment ago is NOT an alarm. A monitor that goes amber + // every time somebody withdraws is a monitor that gets muted, and then the real one is + // muted too — every withdrawal spends a confirmation depth in the queue by design. + const busy = { ...HEALTHY, withdrawalsQueued: 3, queuedOldestMinutes: 2 } + assert.equal(judgeMoneyLoops(busy, OPTS).state, 'ok') +}) + +test('a stuck withdrawal is degraded, and the reason says what to do about it', () => { + // THE SCENARIO THE TICKET DESCRIBES, in the shape the reviewer note corrected it to: a + // signed and broadcast withdrawal with no automatic exit left. Nothing retries it, nothing + // refunds it, and before this it was invisible to every probe in the estate. + const verdict = judgeMoneyLoops( + { ...HEALTHY, withdrawalsStuck: 2, withdrawalsStuckOldestMinutes: 195 }, + OPTS, + ) + assert.equal(verdict.state, 'degraded') + assert.equal(verdict.reasons.length, 1) + assert.match(verdict.reasons[0]!, /2 withdrawal\(s\) stuck/) + assert.match(verdict.reasons[0]!, /195m/) + // The remedy, named in the alarm rather than in somebody's memory. + assert.match(verdict.reasons[0]!, /POST \/admin\/withdrawals\/:id\/abandon/) +}) + +test('a stuck sweep is reported by the customers it is costing, not by the sweep', () => { + // A stuck sweep does NOT lose anybody their deposit credit — the watcher credits + // independently of sweeps — but it does freeze the address it came out of: the balance has + // fallen below the high-water mark with nothing to explain it, so that customer stops being + // credited until an operator reconciles it. That is the number worth paging on. + const verdict = judgeMoneyLoops( + { ...HEALTHY, sweepsStuck: 3, sweepsStuckOldestMinutes: 400, addressesFrozenBySweep: 2 }, + OPTS, + ) + assert.equal(verdict.state, 'degraded') + assert.match(verdict.reasons[0]!, /3 sweep\(s\) stuck/) + assert.match(verdict.reasons[0]!, /2 deposit address\(es\)/) + assert.match(verdict.reasons[0]!, /POST \/admin\/deposit-addresses\/:id\/sweeps/) +}) + +test('the queue ages into an alarm BEFORE the deadline that refunds it', () => { + // Treasury starvation is the failure the sweeper being off by default actually produces, + // and it does not arrive as a stuck row: the withdrawals stay `pending`, are retried, and + // are refunded at `PAY_WITHDRAWAL_STUCK_MINUTES`. Alarming at the deadline would alarm + // after the money-moving decision had already been taken away from the operator. + const half = Math.floor(60 * QUEUE_ALARM_FRACTION) + const justBefore = judgeMoneyLoops( + { ...HEALTHY, withdrawalsQueued: 4, queuedOldestMinutes: half - 1 }, + OPTS, + ) + assert.equal(justBefore.state, 'ok', 'alarmed before there was anything to act on') + + const atThreshold = judgeMoneyLoops( + { ...HEALTHY, withdrawalsQueued: 4, queuedOldestMinutes: half }, + OPTS, + ) + assert.equal(atThreshold.state, 'degraded') + assert.match(atThreshold.reasons[0]!, /queued and unsigned/) + assert.match(atThreshold.reasons[0]!, /GET \/admin\/treasury/) + // And it leaves as much time again to act as it took to notice. + assert.ok(half < OPTS.withdrawalStuckMinutes, 'the warning must land before the refund') + + // The threshold follows the deployment's own deadline rather than a constant: an operator + // who sets a ten-minute deadline gets a five-minute warning, not a thirty-minute one that + // fires after every refund. + const short = judgeMoneyLoops( + { ...HEALTHY, withdrawalsQueued: 1, queuedOldestMinutes: 5 }, + { withdrawalStuckMinutes: 10 }, + ) + assert.equal(short.state, 'degraded') +}) + +test('every problem is reported, not just the first one', () => { + // The three arrive together more often than not — an unfunded treasury and a stuck sweeper + // are the same incident from two ends — and a monitor that names one of them sends somebody + // to fix half of it. + const verdict = judgeMoneyLoops( + { + withdrawalsStuck: 1, + withdrawalsStuckOldestMinutes: 90, + withdrawalsQueued: 5, + queuedOldestMinutes: 45, + sweepsStuck: 2, + sweepsStuckOldestMinutes: 300, + addressesFrozenBySweep: 2, + }, + OPTS, + ) + assert.equal(verdict.state, 'degraded') + assert.equal(verdict.reasons.length, 3) + // The counts travel with the verdict so a dashboard never has to parse the prose. + assert.equal(verdict.counts.addressesFrozenBySweep, 2) +}) + +test('the alarm repeats while it is true, without shouting', () => { + // A watchdog fails in one of two ways and both are decided here: it says nothing after the + // first line (Lantern holds one issue that ages into the background — which is what the + // workers' own single `log.error` already did, and is why CF-23 exists), or it says the + // same thing every tick until somebody mutes it. + const bad = judgeMoneyLoops({ ...HEALTHY, sweepsStuck: 1, sweepsStuckOldestMinutes: 90 }, OPTS) + const t0 = 1_000_000 + + const first = planMoneyLoopLog(bad, { signature: null, saidAt: 0 }, t0) + assert.equal(first.say, 'alarm', 'the first sight of a problem was not logged') + + // Five minutes later, unchanged: silent. + const quiet = planMoneyLoopLog(bad, first.memory, t0 + 5 * 60_000) + assert.equal(quiet.say, null) + + // An hour on, still unchanged: said again, so the signal does not die with the incident. + const restated = planMoneyLoopLog(bad, first.memory, t0 + RESTATE_MS) + assert.equal(restated.say, 'alarm') + + // Worse, inside the quiet hour: said immediately, because it is a different fact. + const worse = judgeMoneyLoops({ ...HEALTHY, sweepsStuck: 2, sweepsStuckOldestMinutes: 90 }, OPTS) + assert.equal(planMoneyLoopLog(worse, first.memory, t0 + 60_000).say, 'alarm') + + // And the end of the incident is written down once, then never again. + const clear = planMoneyLoopLog(judgeMoneyLoops(HEALTHY, OPTS), first.memory, t0 + 60_000) + assert.equal(clear.say, 'clear') + assert.equal(planMoneyLoopLog(judgeMoneyLoops(HEALTHY, OPTS), clear.memory, t0 + 120_000).say, null) + + // A process that boots healthy says nothing at all — there is no incident to close. + const boot = planMoneyLoopLog(judgeMoneyLoops(HEALTHY, OPTS), { signature: null, saidAt: 0 }, t0) + assert.equal(boot.say, null) +}) diff --git a/services/pay/src/opswatch.ts b/services/pay/src/opswatch.ts new file mode 100644 index 0000000..0ff4152 --- /dev/null +++ b/services/pay/src/opswatch.ts @@ -0,0 +1,218 @@ +import type { FastifyBaseLogger } from 'fastify' +import { env } from './env.js' +import { moneyLoopCounts, type MoneyLoopCounts } from './store.js' + +/* ------------------------------------------------------------------ * + * The two loops that move real money, watched — CF-23. + * + * WHAT WAS MISSING, PRECISELY. The withdrawal worker and the treasury sweeper are the only + * things in this estate that move funds on chain, and both of them park a row for a human + * when they run out of safe moves: `markWithdrawalStuck` and `markSweepStuck` are, in the + * workers' own words, "a request for a human". Nothing carried the request to one. Beacon's + * pay probes are `/health`, `/coins/rates` and `/deposit-coins`; its four pay journeys cover + * wallet reads, the Shard ledger, the convert round trip and the storefront. None of them + * touches a sweep, a treasury or a withdrawal, so every probe stays green while a customer's + * withdrawal sits signed-and-broadcast with no automatic exit and a stuck sweep freezes the + * deposit address it came out of. + * + * The operator is not blind — `GET /admin/withdrawals?status=stuck`, `GET /admin/treasury` + * and the user's own view of their row all exist, and they are deliberate. Every one of them + * is a PULL: somebody must already suspect the problem. What did not exist was anything that + * pushes. + * + * SO THIS FILE IS THE PUSH, AND IT IS IN TWO HALVES BECAUSE THE ESTATE IS. + * + * 1. `GET /internal/money-loops` (routes/internal.ts) answers `judgeMoneyLoops` over the + * wire, so a Beacon probe can go red on `state !== 'ok'` and name what is wrong without + * knowing anything about this schema. It is behind the service token and inside the + * compose network, exactly like the rest of `/internal`, because "how many customer + * withdrawals are stuck and how starved is the treasury" is not a public reading. + * 2. `startMoneyLoopWatch` re-states the same verdict into the log while anything is + * unattended. Lantern groups log lines into issues, so a single `log.error` at the + * moment a row got stuck becomes one issue that ages and goes quiet; a line that comes + * back every hour with a rising count is a thing somebody notices. It costs three + * aggregate queries per interval and touches no chain. + * + * WHAT IS STILL NOT DONE, AND IT IS THE HALF THAT REACHES A PERSON. The Beacon target that + * calls the route, and any notification path at all — Beacon's webhook fires on incidents + * only and Lantern has no notify module — live in the `stack` repository (`infra/beacon`, + * `infra/lantern`), not here. Until one of those lands, this is a signal with no siren on the + * end of it: better than nothing (the reading exists, is machine-readable, and repeats), and + * not yet an alert. See MAP.md §7. + * ------------------------------------------------------------------ */ + +export type MoneyLoopState = 'ok' | 'degraded' + +export interface MoneyLoopVerdict { + state: MoneyLoopState + /** One sentence per thing that is wrong, written for whoever gets paged. Empty when ok. */ + reasons: string[] + counts: MoneyLoopCounts +} + +/** + * How long a withdrawal may sit unsigned before the queue itself is the alarm. + * + * Half the stuck deadline, and the fraction is the point: at the FULL deadline the row is + * refunded and the moment to act on it has passed (`withdrawer.planBuildFailure`, the + * `treasury` classification). Warning at half of it leaves the same amount of time again to + * fund or sweep the treasury before a customer is told their withdrawal could not be paid. + */ +export const QUEUE_ALARM_FRACTION = 0.5 + +/** + * Is either money loop waiting on a human, and what would you tell them? + * + * Pure, so that the thing a monitor goes red on is testable without a database, a chain or a + * clock. Every threshold it applies is passed in rather than read from `env` for the same + * reason. + */ +export function judgeMoneyLoops( + counts: MoneyLoopCounts, + opts: { withdrawalStuckMinutes: number }, +): MoneyLoopVerdict { + const reasons: string[] = [] + if (counts.withdrawalsStuck > 0) { + reasons.push( + `${counts.withdrawalsStuck} withdrawal(s) stuck, oldest ${counts.withdrawalsStuckOldestMinutes ?? 0}m — ` + + 'signed bytes exist for these and nothing will retry or refund them on its own. ' + + 'GET /admin/withdrawals?status=stuck, then POST /admin/withdrawals/:id/abandon once ' + + 'the chain proves those bytes can never apply', + ) + } + if (counts.sweepsStuck > 0) { + reasons.push( + `${counts.sweepsStuck} sweep(s) stuck, oldest ${counts.sweepsStuckOldestMinutes ?? 0}m, freezing ` + + `${counts.addressesFrozenBySweep} deposit address(es) — those customers stop being credited ` + + 'until each is reconciled with POST /admin/deposit-addresses/:id/sweeps', + ) + } + // The queue ageing, which is what an unfunded treasury looks like BEFORE it becomes a + // refund. Deliberately not raised for a queue that is merely non-empty: a withdrawal takes + // a confirmation depth to settle at the best of times, and a monitor that goes amber every + // time somebody withdraws is a monitor that gets muted. + const queueAlarmMinutes = Math.max(1, Math.floor(opts.withdrawalStuckMinutes * QUEUE_ALARM_FRACTION)) + if (counts.queuedOldestMinutes !== null && counts.queuedOldestMinutes >= queueAlarmMinutes) { + reasons.push( + `${counts.withdrawalsQueued} withdrawal(s) queued and unsigned, oldest ${counts.queuedOldestMinutes}m ` + + `of the ${opts.withdrawalStuckMinutes}m after which they are refunded — usually a treasury ` + + 'that cannot cover them. GET /admin/treasury, and fund or sweep it before the deadline', + ) + } + return { state: reasons.length ? 'degraded' : 'ok', reasons, counts } +} + +/** Read the loops and judge them, with this deployment's own deadline. */ +export async function readMoneyLoops(now = new Date()): Promise { + return judgeMoneyLoops(await moneyLoopCounts(now), { + withdrawalStuckMinutes: env.withdrawalStuckMinutes, + }) +} + +/** How often the watch reads. Cheap enough to be frequent; not so frequent it is noise. */ +export const WATCH_INTERVAL_MS = 5 * 60_000 +/** + * How often an unchanged problem is said again. + * + * The same rule the sweeper applies to a chain that keeps refusing (`BLOCK_LOG_MS`): a + * persistent fault is said and not shouted. A CHANGED verdict is always logged immediately, + * because "one more withdrawal just got stuck" is news even inside the quiet hour. + */ +export const RESTATE_MS = 60 * 60_000 + +/** What the watch remembers between reads. Passed in and out so the decision stays pure. */ +export interface WatchMemory { + /** The SHAPE of the last verdict, or null before the first read. */ + signature: string | null + /** When the alarm was last written, in epoch ms. 0 if never. */ + saidAt: number +} + +/** + * The shape of a verdict, for deciding whether it is NEWS. + * + * Counts and state, never ages: a stuck withdrawal getting one minute older every minute is + * the same fact restated, and folding the age in here would defeat the throttle it exists to + * survive. + */ +function signatureOf(verdict: MoneyLoopVerdict): string { + const c = verdict.counts + return `${verdict.state}:${c.withdrawalsStuck}:${c.sweepsStuck}:${c.withdrawalsQueued}` +} + +/** + * Should this reading be written to the log, and as what? + * + * Pure, and separate from the loop, because the failure mode of a watchdog is that it is + * either silent or unreadable and both are decided here. Three rules: + * + * - A CHANGED problem is always said, immediately. "One more withdrawal just got stuck" is + * news even inside a quiet hour. + * - An UNCHANGED problem is said again every `RESTATE_MS`, so that Lantern holds a line + * that keeps coming back rather than one issue that ages into the background. The same + * rule, and the same hour, the sweeper applies to a chain that keeps refusing. + * - Recovery is said once and never repeated. A healthy service does not need a heartbeat + * here, but the end of an incident belongs in the log next to its start. + */ +export function planMoneyLoopLog( + verdict: MoneyLoopVerdict, + memory: WatchMemory, + now: number, +): { say: 'alarm' | 'clear' | null; memory: WatchMemory } { + const signature = signatureOf(verdict) + const changed = signature !== memory.signature + if (verdict.state === 'ok') { + const say = changed && memory.signature !== null ? ('clear' as const) : null + return { say, memory: { signature, saidAt: memory.saidAt } } + } + if (changed || now - memory.saidAt >= RESTATE_MS) { + return { say: 'alarm', memory: { signature, saidAt: now } } + } + return { say: null, memory: { signature, saidAt: memory.saidAt } } +} + +/** + * Say, repeatedly and at error level, that money is waiting on a human. + * + * Returns the timer so a caller can stop it; the process holds no other reference. Unref'd, + * because a monitor must never be the reason a process refuses to exit. + */ +export function startMoneyLoopWatch( + log: FastifyBaseLogger, + opts: { intervalMs?: number; firstDelayMs?: number } = {}, +): NodeJS.Timeout { + let memory: WatchMemory = { signature: null, saidAt: 0 } + + const tick = async () => { + let verdict: MoneyLoopVerdict + try { + verdict = await readMoneyLoops() + } catch (err) { + log.error({ err }, 'money-loop watch could not read the queues') + return + } + const plan = planMoneyLoopLog(verdict, memory, Date.now()) + memory = plan.memory + if (plan.say === 'alarm') { + log.error( + { counts: verdict.counts, reasons: verdict.reasons }, + `money is waiting on a human: ${verdict.reasons.join(' | ')}`, + ) + } + if (plan.say === 'clear') { + log.warn({ counts: verdict.counts }, 'money loops are clear again — nothing is waiting on a human') + } + } + + const timer = setInterval(() => { + void tick() + }, opts.intervalMs ?? WATCH_INTERVAL_MS) + timer.unref() + // Not immediately: a boot with a stuck row from yesterday should say so, but not before + // migrations have finished and not in the middle of the first tick of everything else. + const first = setTimeout(() => { + void tick() + }, opts.firstDelayMs ?? 30_000) + first.unref() + return timer +} diff --git a/services/pay/src/outbound.test.ts b/services/pay/src/outbound.test.ts index d5a9946..16a7113 100644 --- a/services/pay/src/outbound.test.ts +++ b/services/pay/src/outbound.test.ts @@ -1,13 +1,28 @@ import assert from 'node:assert/strict' +import { createHash } from 'node:crypto' import { test } from 'node:test' +import { depositChainInfo, type DepositCoin } from '@cloudsforge/shared' +import { PROBE_IS_CUMULATIVE } from './chains.js' import { env } from './env.js' import { canonicalDestination, + canReadTransaction, + canSend, + checkSweepMovement, gasPriceBid, + judgeAbandon, + legacyNonce, + manualSweepGate, isValidDestination, + readEvmMovement, + readXrpMovement, + signedTxId, TRANSFER_GAS, WITHDRAWABLE_COINS, withdrawalUnsupportedReason, + xrpTxHash, + type AbandonEvidence, + type OutboundMovement, } from './outbound.js' /** @@ -100,3 +115,519 @@ test('the withdrawable set matches the refusal reasons', () => { assert.match(withdrawalUnsupportedReason('BTC') ?? '', /PSBT/) assert.match(withdrawalUnsupportedReason('SOL') ?? '', /SPL/) }) + +/* ------------------------------------------------------------------ * + * CF-08 — the manual-sweep amount, checked against the transaction it names. + * ------------------------------------------------------------------ */ + +const ADDRESS = '0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed' +/** The treasury: where a hand sweep is supposed to end up. */ +const TREASURY = '0x8ba1f109551bD432803012645Ac136ddd64DBA72' +const WEI = 10n ** 18n + +/** 100 EMBER delivered plus 21,000 gas at 1 gwei — what a real hand sweep looks like. */ +const moved100: OutboundMovement = { + from: ADDRESS, + to: TREASURY, + value: 100n * WEI, + fee: TRANSFER_GAS * 1_000_000_000n, + total: 100n * WEI + TRANSFER_GAS * 1_000_000_000n, +} + +test('an overstated manual sweep amount is refused, and the refusal names the real one', () => { + // THE MINT. `amount` used to go straight into `deposit_addresses.swept`, and the watcher's + // next tick computes `observed = confirmed + swept`: an operator who moved 100 EMBER and + // typed 1000 credited the depositor 900 EMBER nobody deposited, spendable immediately. + const overstated = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: 1000n * WEI, + movement: moved100, + }) + assert.equal(overstated.ok, false) + assert.equal(overstated.ok === false && overstated.code, 'sweep_amount_mismatch') + // The exact figure comes back so the operator's second attempt is a paste, not a + // calculation — the check is exact, so a number they compute by hand will be wrong. + assert.equal(overstated.ok === false && overstated.moved, '100.000021') + assert.match(overstated.ok === false ? overstated.error : '', /100\.000021/) + + // A wei figure pasted into a field that expects whole coins: the same refusal, and this is + // the variant that would otherwise mint 1e18 times the deposit. + const pastedWei = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: 100n * WEI * WEI, + movement: moved100, + }) + assert.equal(pastedWei.ok === false && pastedWei.code, 'sweep_amount_mismatch') +}) + +test('the exact amount the transaction moved — value plus fee — is accepted', () => { + // The fee is inside the recorded amount because it left the address too: the deposit probe + // reads a balance, and the balance dropped by value + fee. Recording only the value leaves + // a permanent shortfall the size of the fee, which is a `regression` that never clears. + const exact = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: moved100.total, + movement: moved100, + }) + assert.equal(exact.ok, true) + + // One wei either side is refused. Understating is not "safe": it leaves the address behind + // a mark the chain can no longer reach, and the txid is the idempotency key, so there is no + // second POST that can top it up. + for (const off of [-1n, 1n]) { + const near = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: moved100.total + off, + movement: moved100, + }) + assert.equal(near.ok, false, `${off} wei away from the truth was accepted`) + } +}) + +test('a transaction that did not come out of this address is refused', () => { + // The route never checked that the transaction touched this deposit address at all, so any + // real, deep, successful transaction on the chain — somebody else's — was proof enough to + // record an arbitrary amount against. + const someoneElse = checkSweepMovement({ + coin: 'EMBER', + address: '0x8ba1f109551bD432803012645Ac136ddd64DBA72', + postedSmallest: moved100.total, + movement: moved100, + }) + assert.equal(someoneElse.ok === false && someoneElse.code, 'sweep_wrong_address') + + // And the comparison is the canonical one: the same account in a different spelling is the + // same account, or every EVM hand sweep would be refused for an address that matches. + const lowercased = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS.toLowerCase(), + postedSmallest: moved100.total, + movement: { ...moved100, from: ADDRESS.toUpperCase().replace('0X', '0x') }, + }) + assert.equal(lowercased.ok, true) +}) + +test('a movement the node would not describe is a refusal, never a zero', () => { + // `movement: null` is "this service does not know what moved". Fail closed: recording an + // unverified amount is exactly the defect, and there is no number here safe to assume. + const unknown = checkSweepMovement({ + coin: 'XRP', + address: 'rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn', + postedSmallest: 1n, + movement: null, + }) + assert.equal(unknown.ok === false && unknown.code, 'sweep_unverified') +}) + +test('a sweep that goes back to the address it came from is refused', () => { + // THE SECOND HALF OF CF-08, and the one the first pass missed. Every other check passes: + // the transaction is real, deep, sent BY this deposit address, and the operator posts the + // exact figure the chain names. Nothing left custody — the balance is still there for the + // probe to read — so recording it advances `swept` against funds that never moved, and the + // watcher's next tick computes `confirmed + swept` and credits the depositor the whole + // deposit a second time. Same mint, reached by a pasted destination instead of a typo. + const selfSend = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: moved100.total, + movement: { ...moved100, to: ADDRESS }, + }) + assert.equal(selfSend.ok === false && selfSend.code, 'sweep_wrong_destination') + + // And in any spelling, because an EVM address has three of them and a node's `to` field is + // the lowercase one. A comparison that missed this would refuse nothing in practice. + const lowercased = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: moved100.total, + movement: { ...moved100, to: ADDRESS.toLowerCase() }, + }) + assert.equal(lowercased.ok === false && lowercased.code, 'sweep_wrong_destination') + + // A contract creation credits no account at all. There is nothing it could be a sweep of. + const created = checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: moved100.total, + movement: { ...moved100, to: null }, + }) + assert.equal(created.ok === false && created.code, 'sweep_wrong_destination') + + // The honest sweep — same sender, same amount, a destination that is not this address — + // still passes, or the route would refuse the only movement it exists to record. + assert.equal( + checkSweepMovement({ + coin: 'EMBER', + address: ADDRESS, + postedSmallest: moved100.total, + movement: moved100, + }).ok, + true, + ) +}) + +test('XRP is compared in drops, and its address is not case-folded', () => { + const account = 'rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn' + const payment: OutboundMovement = { + from: account, + to: 'rPEPPER7kfTD9w2To4CQk6UCfuHM9c6GDY', + value: 5_000_000n, + fee: 24n, + total: 5_000_024n, + } + // The destination check applies to XRP as well, and compares base58 exactly. + const xrpSelfSend = checkSweepMovement({ + coin: 'XRP', + address: account, + postedSmallest: 5_000_024n, + movement: { ...payment, to: account }, + }) + assert.equal(xrpSelfSend.ok === false && xrpSelfSend.code, 'sweep_wrong_destination') + assert.equal( + checkSweepMovement({ coin: 'XRP', address: account, postedSmallest: 5_000_024n, movement: payment }).ok, + true, + ) + // Base58 is case-sensitive: a different case is a different account and must not match. + const folded = checkSweepMovement({ + coin: 'XRP', + address: account.toLowerCase(), + postedSmallest: 5_000_024n, + movement: payment, + }) + assert.equal(folded.ok === false && folded.code, 'sweep_wrong_address') +}) + +test('what an EVM node says a transaction moved, including where it went', () => { + const receipt = { + blockNumber: '0x384', + status: '0x1', + from: ADDRESS.toLowerCase(), + to: TREASURY.toLowerCase(), + gasUsed: '0x5208', + effectiveGasPrice: '0x3b9aca00', + } + const tx = { from: ADDRESS.toLowerCase(), to: TREASURY.toLowerCase(), value: '0x56bc75e2d63100000' } + const moved = readEvmMovement(receipt, tx) + // Both accounts come back in the canonical spelling, because the node reports them + // lowercase and every comparison downstream is against a checksummed row. + assert.equal(moved.from, ADDRESS) + assert.equal(moved.to, TREASURY) + assert.equal(moved.value, 100n * WEI) + assert.equal(moved.fee, 21_000n * 1_000_000_000n) + assert.equal(moved.total, moved.value + moved.fee) + + // A contract creation: `to` is null, and that is an answer rather than a failure. + assert.equal(readEvmMovement(receipt, { ...tx, to: null }).to, null) + + // A node that answers with something that is not an address at all is a failure, and must + // not collapse into "no destination" — that would read as a contract creation and be + // refused for the wrong reason. + assert.throws(() => readEvmMovement({ ...receipt, to: undefined }, { ...tx, to: 'not-an-address' })) + // Falls back to the receipt's `to` only when the body omits the key entirely. + assert.equal(readEvmMovement(receipt, { ...tx, to: undefined }).to, TREASURY) +}) + +test('an XRP payment is read from delivered_amount, and never from Amount', () => { + const account = 'rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn' + const destination = 'rPEPPER7kfTD9w2To4CQk6UCfuHM9c6GDY' + const base = { + TransactionType: 'Payment', + Account: account, + Destination: destination, + Amount: '5000000', + Fee: '24', + meta: { TransactionResult: 'tesSUCCESS', delivered_amount: '5000000' }, + } + const paid = readXrpMovement(base) + assert.deepEqual(paid, { from: account, to: destination, value: 5_000_000n, fee: 24n, total: 5_000_024n }) + + // A PARTIAL payment: `Amount` is the ceiling the sender authorised and `delivered_amount` + // is what the ledger actually moved. Recording `Amount` would overstate the debit by the + // shortfall, and the overstatement is credited to the depositor. + const partial = readXrpMovement({ ...base, meta: { delivered_amount: '1000000' } }) + assert.equal(partial?.value, 1_000_000n) + assert.equal(partial?.total, 1_000_024n) + + // And when rippled will not say — the literal string `unavailable`, which it returns for a + // partial payment in a pre-2014 ledger — the answer is null, NOT `Amount`. This function + // fell back to `Amount` in exactly that case, which is the one case where `Amount` is known + // to be wrong; the doc above it always claimed it did not. + assert.equal(readXrpMovement({ ...base, meta: { delivered_amount: 'unavailable' } }), null) + assert.equal(readXrpMovement({ ...base, meta: {} }), null) + // An issued currency is an object, not drops, and is not XRP at all. + assert.equal( + readXrpMovement({ ...base, meta: { delivered_amount: { currency: 'USD', value: '5' } } }), + null, + ) + // A destination we cannot read is a refusal too: the self-send check has nothing to compare. + assert.equal(readXrpMovement({ ...base, Destination: undefined }), null) + // AccountDelete moves an entire balance by rules this does not implement. + assert.equal(readXrpMovement({ ...base, TransactionType: 'AccountDelete' }), null) +}) + +test('no coin can record an unverified amount into a `swept` total that moves', () => { + // THE SOL HOLE, as an invariant rather than as a special case. The manual-sweep route gated + // its whole chain-verification block on `canSend`, and `canSend('SOL')` is false — so a SOL + // sweep skipped the txid lookup, the depth gate and the amount check, while + // `store.recordSweepIn` still added the operator's number to `swept`, because Solana's probe + // is a spot balance and not a cumulative counter. A fabricated txid with any amount at all + // was accepted with no chain contact whatsoever. + // + // Bitcoin is the only coin that may be unverifiable, and it is exempt on mechanism: its + // probe is a cumulative received counter, so `recordSweepIn` returns early and there is no + // total to overstate. + for (const coin of ['EMBER', 'ETH', 'XRP', 'BTC', 'SOL'] as DepositCoin[]) { + const gate = manualSweepGate(coin) + if (gate === 'verify') continue + // Anything not verified must be a coin where recording changes no total at all. + assert.equal( + PROBE_IS_CUMULATIVE[depositChainInfo(coin).family], + gate === 'inert', + `${coin} is gated '${gate}', which does not match whether recording can move its \`swept\` total`, + ) + } + assert.equal(manualSweepGate('SOL'), 'refuse', 'a SOL amount can still be recorded unverified') + assert.equal(manualSweepGate('BTC'), 'inert') + assert.equal(manualSweepGate('EMBER'), 'verify') + assert.equal(canReadTransaction('SOL'), false) + assert.equal(canReadTransaction('BTC'), false) + // Reading and sending are different capabilities and are asked about separately now. They + // agree today; the point is that nothing downstream may assume they always will. + for (const coin of ['EMBER', 'ETH', 'XRP'] as DepositCoin[]) { + assert.equal(canSend(coin), true) + assert.equal(canReadTransaction(coin), true) + } +}) + +/* ------------------------------------------------------------------ * + * CF-06 — abandoning a signed withdrawal. + * + * `POST /admin/withdrawals/:id/abandon` credits a user's balance back while a valid + * signature for the same money is still in `withdrawals.raw_tx`, so every one of these is a + * test about not paying somebody twice. The decision is a pure function on purpose + * (`judgeAbandon`), which is what makes all of it testable with no chain at all. + * ------------------------------------------------------------------ */ + +/** Minimal RLP, written independently of the decoder it is used to test. */ +function rlpBytes(value: bigint): Buffer { + if (value === 0n) return Buffer.alloc(0) + const hex = value.toString(16) + return Buffer.from(hex.length % 2 === 0 ? hex : `0${hex}`, 'hex') +} + +function rlpString(bytes: Buffer): Buffer { + if (bytes.length === 1 && bytes[0]! <= 0x7f) return bytes + if (bytes.length <= 55) return Buffer.concat([Buffer.from([0x80 + bytes.length]), bytes]) + const length = rlpBytes(BigInt(bytes.length)) + return Buffer.concat([Buffer.from([0xb7 + length.length]), length, bytes]) +} + +function rlpList(items: Buffer[]): Buffer { + const body = Buffer.concat(items) + if (body.length <= 55) return Buffer.concat([Buffer.from([0xc0 + body.length]), body]) + const length = rlpBytes(BigInt(body.length)) + return Buffer.concat([Buffer.from([0xf7 + length.length]), length, body]) +} + +/** A signed legacy (type 0) value transfer, exactly the shape this service asks keyvault for. */ +function legacyTx(nonce: bigint): string { + return `0x${rlpList([ + rlpString(rlpBytes(nonce)), + rlpString(rlpBytes(2_000_000_000n)), + rlpString(rlpBytes(TRANSFER_GAS)), + rlpString(Buffer.from('8ba1f109551bD432803012645Ac136ddd64DBA72', 'hex')), + rlpString(rlpBytes(10n ** 18n)), + rlpString(Buffer.alloc(0)), + rlpString(rlpBytes(0x1cn)), + rlpString(Buffer.alloc(32, 0x11)), + rlpString(Buffer.alloc(32, 0x22)), + ]).toString('hex')}` +} + +test('the nonce is read back out of the signed bytes, at every encoding RLP has for one', () => { + // The EVM half of the abandon proof is "the account has used more nonces than these bytes + // carry", so reading the wrong number is a refund of a payment that can still be mined. + // Every boundary of RLP's integer encoding is here: the empty string for zero, a bare byte + // below 0x80, the 0x81 form at 0x80 and above, and multi-byte quantities. + for (const nonce of [0n, 1n, 0x7fn, 0x80n, 0xffn, 0x100n, 65_535n, 4_294_967_295n]) { + assert.equal(legacyNonce(legacyTx(nonce)), nonce, `nonce ${nonce} did not survive a round trip`) + } + // The same bytes without the 0x prefix, because a hand-pasted raw transaction is either. + assert.equal(legacyNonce(legacyTx(7n).slice(2)), 7n) +}) + +test('anything that is not a legacy transaction reads as null, and null refuses the abandon', () => { + // Null is never a nonce of zero. A nonce of zero is a live signature at the very first slot + // of a fresh treasury, and reading an unparseable body as one would abandon it the moment + // the account had sent anything at all. + const cases: Record = { + 'an EIP-2718 typed envelope': `0x02${legacyTx(1n).slice(2)}`, + 'trailing bytes after the list': `${legacyTx(1n)}ff`, + 'a truncated body': legacyTx(1n).slice(0, 20), + 'not hex at all': '0xzzzz', + 'an odd number of hex digits': '0xc0f', + 'nothing': '0x', + 'a list whose first item is itself a list': `0x${rlpList([rlpList([]), rlpString(rlpBytes(1n))]).toString('hex')}`, + 'a non-canonical zero (0x00 rather than 0x80)': `0x${rlpList([ + Buffer.from([0x00]), + rlpString(rlpBytes(1n)), + ]).toString('hex')}`, + 'a nonce with a leading zero byte': `0x${rlpList([ + rlpString(Buffer.from([0x00, 0x01])), + rlpString(rlpBytes(1n)), + ]).toString('hex')}`, + } + for (const [what, rawTx] of Object.entries(cases)) { + assert.equal(legacyNonce(rawTx), null, `${what} should not have parsed as a nonce`) + } + + const unreadable = judgeAbandon('EMBER', { + statuses: [{ txid: '0xabc', status: { kind: 'unknown' } }], + evm: { nonce: null, accountUsed: 99n }, + }) + assert.equal(unreadable.ok, false) + assert.equal(unreadable.ok === false && unreadable.code, 'withdrawal_unprovable') +}) + +test('an XRP transaction id is derivable from the blob, so a null txid is not an excuse', () => { + // THE WHOLE OF THE XRP HALF OF CF-06. `buildAndSignXrp` returns `txid: null` because the id + // used to come back from `submit`, and the route read a null txid as "there is nothing to + // ask the chain about" — so a payment that WAS applied, and whose submit response was lost, + // was refunded with no evidence at all. rippled's own definition is + // SHA-512Half('TXN\0' ‖ blob) (HashPrefix::transactionID), which needs no deserializer. + const blob = '120000228000000024000000042E0000000061400000000098968068400000000000000A' + const expected = createHash('sha512') + .update(Buffer.concat([Buffer.from('54584E00', 'hex'), Buffer.from(blob, 'hex')])) + .digest('hex') + .slice(0, 64) + .toUpperCase() + assert.equal(xrpTxHash(blob), expected) + assert.equal(xrpTxHash(blob)!.length, 64) + // Uppercase, because that is how rippled spells ids and how `submit` hands them back — a + // derived id and a recorded one have to compare equal. + assert.equal(xrpTxHash(blob), xrpTxHash(blob)!.toUpperCase()) + assert.notEqual(xrpTxHash(blob), xrpTxHash(`${blob}00`)) + assert.equal(xrpTxHash('not hex'), null) + + assert.equal(signedTxId('XRP', blob), expected) + assert.equal(signedTxId('EMBER', legacyTx(1n)), signedTxId('ETH', legacyTx(1n))) + // No derivation, so no abandon: BTC and SOL cannot be withdrawn at all, and if that ever + // changes the id has to be derivable before a signature of theirs may be given up on. + assert.equal(signedTxId('BTC', '0xabcd'), null) + assert.equal(signedTxId('SOL', '0xabcd'), null) +}) + +test('a payment the chain can still see is never refunded, whichever id it was found under', () => { + const stillThere: AbandonEvidence = { + statuses: [ + { + txid: 'DERIVED', + status: { kind: 'confirmed', confirmations: 1, minedHeight: 900n, movement: null }, + }, + ], + // The XRP evidence says "abandon me": unconsumed sequence, expired blob. It must not be + // reached — a transaction in the ledger settles the question before any of it matters. + xrp: { sequence: 4n, lastLedger: 100n, accountSequence: 4n, currentLedger: 900n }, + } + const verdict = judgeAbandon('XRP', stillThere) + assert.equal(verdict.ok, false) + assert.equal(verdict.ok === false && verdict.code, 'withdrawal_on_chain') + + // The EVM shape of the same thing: mined but not yet at depth. + const pending = judgeAbandon('EMBER', { + statuses: [ + { txid: '0xdead', status: { kind: 'pending', confirmations: 3, minedHeight: 10n, movement: null } }, + ], + evm: { nonce: 4n, accountUsed: 99n }, + }) + assert.equal(pending.ok === false && pending.code, 'withdrawal_on_chain') + + // Mined and REVERTED is the one thing a chain can say that makes a refund safe by itself: + // the nonce is spent, the value did not move, and the bytes can never apply again. + const reverted = judgeAbandon('EMBER', { + statuses: [{ txid: '0xdead', status: { kind: 'rejected', reason: 'it reverted' } }], + evm: { nonce: 4n, accountUsed: 4n }, + }) + assert.equal(reverted.ok, true) +}) + +test('an EVM withdrawal is only abandonable once its nonce has been taken by something else', () => { + // `unknown` means "this node has no receipt", which is a fact about the NODE. The bytes are + // untouched by it: the nonce is unconsumed, a legacy transaction has no expiry, and any + // node still holding them can mine them after the refund. This is the hole CF-06 names. + const live = judgeAbandon('EMBER', { + statuses: [{ txid: '0xdead', status: { kind: 'unknown' } }], + evm: { nonce: 7n, accountUsed: 7n }, + }) + assert.equal(live.ok, false) + assert.equal(live.ok === false && live.code, 'withdrawal_still_applicable') + // And it says what to do about it, because "no" with no remedy is how an operator ends up + // editing the row by hand. + assert.match(live.ok === false ? live.error : '', /0-value transaction from the treasury to itself/) + + // `accountUsed` counts nonces USED, so 8 used means nonce 7 is spent — and no receipt for + // our own hash means it was spent by a different transaction. That is the proof. + const retired = judgeAbandon('EMBER', { + statuses: [{ txid: '0xdead', status: { kind: 'unknown' } }], + evm: { nonce: 7n, accountUsed: 8n }, + }) + assert.equal(retired.ok, true) + + // The boundary: an account that has used exactly `nonce` has NOT used this one yet. + for (const used of [0n, 6n, 7n]) { + assert.equal( + judgeAbandon('EMBER', { + statuses: [{ txid: '0xdead', status: { kind: 'unknown' } }], + evm: { nonce: 7n, accountUsed: used }, + }).ok, + false, + `nonce 7 read as retired at ${used} used`, + ) + } +}) + +test('an XRP withdrawal needs both halves: never applied, and never applicable', () => { + const base = { sequence: 12n, lastLedger: 1_000n, accountSequence: 12n, currentLedger: 1_001n } + const unknown = [{ txid: 'DERIVED', status: { kind: 'unknown' as const } }] + + // Both halves true: the ledger never saw it, the account never consumed the Sequence these + // bytes carry (so the payment cannot have been applied — XRPL applies a transaction only at + // its exact Sequence), and the ledger is past LastLedgerSequence (so it never will be). + assert.equal(judgeAbandon('XRP', { statuses: unknown, xrp: base }).ok, true) + + // Still inside the window: `broadcastXrp` re-submits these bytes on every worker tick, so + // refunding here is a refund with the payment still in flight. + const early = judgeAbandon('XRP', { statuses: unknown, xrp: { ...base, currentLedger: 1_000n } }) + assert.equal(early.ok === false && early.code, 'withdrawal_still_applicable') + + // The Sequence has moved past the one signed: either this payment applied or something else + // took the slot, and a node that has forgotten the transaction cannot tell them apart. That + // ambiguity is a refusal — it is precisely the "submit applied, response lost" case. + const consumed = judgeAbandon('XRP', { statuses: unknown, xrp: { ...base, accountSequence: 13n } }) + assert.equal(consumed.ok === false && consumed.code, 'withdrawal_unprovable') + + // A row signed before this service recorded what it signed. XRPL binary serialization is + // not a format this file speaks, so there is nothing to recover the numbers from and the + // answer is to refuse rather than to guess. + for (const missing of [{ sequence: null }, { lastLedger: null }, { sequence: null, lastLedger: null }]) { + const verdict = judgeAbandon('XRP', { statuses: unknown, xrp: { ...base, ...missing } }) + assert.equal(verdict.ok, false) + assert.equal(verdict.ok === false && verdict.code, 'withdrawal_unprovable') + } +}) + +test('with no evidence at all, nothing is refunded', () => { + // The fail-closed default, which is what the whole shape rests on: a coin this service + // cannot reason about does not fall through to "refund". BTC and SOL cannot be withdrawn + // today, so this is a guard against the day one of them can. + for (const coin of ['BTC', 'SOL'] as DepositCoin[]) { + const verdict = judgeAbandon(coin, { statuses: [{ txid: 'x', status: { kind: 'unknown' } }] }) + assert.equal(verdict.ok, false) + assert.equal(verdict.ok === false && verdict.code, 'withdrawal_unprovable') + } + assert.equal(judgeAbandon('EMBER', { statuses: [] }).ok, false) +}) diff --git a/services/pay/src/outbound.ts b/services/pay/src/outbound.ts index 31751b7..64c5787 100644 --- a/services/pay/src/outbound.ts +++ b/services/pay/src/outbound.ts @@ -1,7 +1,8 @@ import { createHash } from 'node:crypto' import type { FastifyBaseLogger } from 'fastify' import { depositChainInfo, type ChainNetwork, type DepositCoin } from '@cloudsforge/shared' -import { chainEndpoint, jsonRpc, xrpReserveDrops, type RpcCtx } from './chains.js' +import { formatUnits } from './amounts.js' +import { chainEndpoint, jsonRpc, PROBE_IS_CUMULATIVE, xrpReserveDrops, type RpcCtx } from './chains.js' import { env } from './env.js' import { isValidEvmAddress, keccak256Hex, toChecksumAddress } from './keccak.js' import { keyvaultSign, type SignerBinding } from './keyvault.js' @@ -68,6 +69,95 @@ export function canSend(coin: DepositCoin): boolean { return withdrawalUnsupportedReason(coin) === null } +/** + * Can this service ask a node about a transaction it did not build — how deep it is, and what + * it moved? `outboundStatus` answers for exactly these coins and throws for the rest. + * + * SEPARATE FROM `canSend`, AND THE COUPLING WAS A HOLE. The two sets are identical today and + * are not the same question: sending needs a signature from forge-keyvault, reading needs + * only an RPC endpoint. `POST /admin/deposit-addresses/:id/sweeps` gated its whole + * chain-verification block on `canSend`, so a SOL manual sweep skipped every check — no + * txid lookup, no depth gate, no amount comparison — while `recordSweepIn` still added the + * operator's number to `swept`, because Solana's probe is a spot balance and not a + * cumulative counter. An invented transaction id and an arbitrary amount were accepted with + * no chain contact at all, and the watcher credited the difference on its next tick. The + * route asks this now, and refuses what it cannot verify (CF-08). + */ +export function canReadTransaction(coin: DepositCoin): boolean { + switch (coin) { + case 'EMBER': + case 'ETH': + case 'XRP': + return true + case 'BTC': + case 'SOL': + // No transaction lookup is implemented for either. BTC's is an Esplora REST path this + // file has never spoken; SOL's is `getTransaction`, which this file has no reason to + // call for any other purpose. Both are refusals rather than half-implementations, for + // the same reason the send side refuses them. + return false + } +} + +/** + * What `POST /admin/deposit-addresses/:id/sweeps` may do with a hand-recorded amount on this + * coin. The route's whole decision, in one tested place rather than in a condition. + * + * verify the chain is asked what moved, and the posted amount must equal it. + * inert there is nothing to verify, because recording cannot move any total: the coin's + * probe is a cumulative received counter, so `store.recordSweepIn` returns early + * and `deposit_addresses.swept` does not change. BTC, and only BTC. The `sweeps` + * row is still written — the movement happened and should be reconstructible. + * refuse the amount would move `swept`, and this service cannot check it against + * anything. Fail closed; an unverified amount is credited to the depositor. + */ +export type ManualSweepGate = 'verify' | 'inert' | 'refuse' + +export function manualSweepGate(coin: DepositCoin): ManualSweepGate { + if (canReadTransaction(coin)) return 'verify' + return PROBE_IS_CUMULATIVE[depositChainInfo(coin).family] ? 'inert' : 'refuse' +} + +/** + * What one transaction actually took OUT of the account that sent it. + * + * The depth of a transaction and the SIZE of it are two different questions, and until now + * this file only answered the first. That was survivable for the two callers that poll their + * own withdrawals and sweeps — this service built those bytes, so it already knows what they + * move — and it was not survivable for the third: `POST /admin/deposit-addresses/:id/sweeps` + * takes the amount from an operator's keyboard and adds it to `deposit_addresses.swept`, and + * the watcher's next tick reads `confirmed + swept` against the high-water mark. An amount + * larger than the transaction really moved is therefore CREDITED to the depositor as a new + * deposit, convertible to Shards and withdrawable from the treasury, with nothing anywhere + * to reconcile it back. One mistyped zero mints money. + * + * So the chain is asked what moved, and the amount is checked against it — see + * `checkSweepMovement`, which is where the comparison lives, and CF-08 for the whole of it. + */ +export interface OutboundMovement { + /** The account that was DEBITED, in this service's canonical spelling for the coin. */ + from: string + /** + * The account that was CREDITED, in the same canonical spelling. Null when the transaction + * credited no account at all — an EVM contract creation, which has no `to`. + * + * HOW MUCH left is not the whole question, and reading only `from` and `total` left the + * defect half open: a transaction that goes from the deposit address straight BACK to it + * satisfies both, and the money never left custody. `swept` still advances, the probe still + * sees the balance, and the watcher's next tick reads `confirmed + swept` and credits the + * depositor the whole thing a second time. `checkSweepMovement` refuses that, and the route + * goes one further and asks the database whether the destination is some OTHER deposit + * address of ours, where the same coins would be credited twice to two different people. + */ + to: string | null + /** What the destination received, in smallest units. */ + value: bigint + /** What the network burned, in smallest units. Paid by `from`, on top of `value`. */ + fee: bigint + /** `value + fee` — everything that left `from`, which is what a sweep records. */ + total: bigint +} + /** What a node says about a transaction we broadcast. */ export type OutboundStatus = /** No record of it. In a mempool, or never accepted — indistinguishable from outside. */ @@ -76,9 +166,16 @@ export type OutboundStatus = * Seen on chain, not yet at the coin's confirmation depth. `minedHeight` is the block or * ledger index it landed in — recorded on a sweep, because a sweep's accounting is gated * on how far the head has moved past it (`chains.sweepMaturityConfirmations`). + * + * `movement` is null when the node answered about the transaction's DEPTH but not about + * its contents — a mined receipt whose transaction body the same node then failed to + * return, an XRP transaction the ledger has not validated yet, or any field that did not + * parse as the quantity it is supposed to be. Null is never "nothing moved": it is "this + * service does not know what moved", and every caller that spends money on the answer + * must treat it as a refusal rather than as a zero. */ - | { kind: 'pending'; confirmations: number; minedHeight: bigint } - | { kind: 'confirmed'; confirmations: number; minedHeight: bigint } + | { kind: 'pending'; confirmations: number; minedHeight: bigint; movement: OutboundMovement | null } + | { kind: 'confirmed'; confirmations: number; minedHeight: bigint; movement: OutboundMovement | null } /** * Applied and FAILED, or provably unable to ever apply. This is the only state in which a * signed withdrawal may be refunded automatically, because it is the only one in which the @@ -92,10 +189,33 @@ export interface SignedOutbound { rawTx: string /** * Null for chains whose transaction id only comes back from the node on submit (XRP). - * Safe, because an XRP payment carries a Sequence: re-submitting the same bytes can be - * applied at most once no matter how many times it is sent. + * Safe for the SENDING path, because an XRP payment carries a Sequence: re-submitting the + * same bytes can be applied at most once no matter how many times it is sent. It was NOT + * safe for the abandon path, which read a null id as "there is no transaction to ask + * about" — see `xrpTxHash`, which derives the id from the bytes so that no caller has to. */ txid: string | null + /** + * The account sequence these bytes consume, as a decimal string, when the chain will not + * give it back later. + * + * Null for EVM, and that is not an omission: a legacy transaction's nonce is the first item + * of its RLP body and `legacyNonce` reads it straight out of `rawTx`, which is better than + * remembering it — a derived value cannot drift from the bytes it describes. XRPL binary + * serialization is a format this service does not speak and will not learn for one caller, + * so an XRP blob's Sequence is written down here at signing time instead. Read by the + * abandon path, which cannot refund without it. See the section at the end of this file. + */ + sequence: string | null + /** + * The chain height past which these bytes can NEVER be applied, as a decimal string. + * + * XRP's `LastLedgerSequence`, and null for EVM — where it is null because there is no such + * height at all. A signed legacy transaction is valid forever and only a consumed nonce + * retires it, which is why the EVM half of the abandon proof is about the nonce and not + * about time. + */ + expiry: string | null } // --------------------------------------------------------------------------- @@ -285,7 +405,9 @@ async function buildAndSignEvm(coin: DepositCoin, input: BuildInput): Promise { const { url, ctx } = chainEndpoint(coin, network, log) - const receipt = (await jsonRpc(url, 'eth_getTransactionReceipt', [txid], ctx)) as { - blockNumber?: string - status?: string - } | null + const receipt = (await jsonRpc(url, 'eth_getTransactionReceipt', [txid], ctx)) as EvmReceipt | null if (!receipt || typeof receipt.blockNumber !== 'string') return { kind: 'unknown' } if (receipt.status === '0x0') { return { kind: 'rejected', reason: 'the transaction was mined but reverted, so nothing was transferred' } } - const head = quantity(await jsonRpc(url, 'eth_blockNumber', [], ctx), 'eth_blockNumber') + // The body is fetched alongside the head rather than after it, so reading what the + // transaction MOVED costs no round trip on top of reading how deep it is. The receipt + // carries `from`, `gasUsed` and `effectiveGasPrice`; only `value` needs the body. + // + // Its failure is caught rather than propagated, and that is the point of catching it here + // rather than around the pair: this call is new, and the two callers that were here first + // — settle a withdrawal, mature a sweep — must not start failing because a node cannot + // answer a question they never ask. It becomes a null `movement`, which only the caller + // that verifies an amount reads, and which that caller treats as a refusal. + const [headHex, tx] = await Promise.all([ + jsonRpc(url, 'eth_blockNumber', [], ctx), + (jsonRpc(url, 'eth_getTransactionByHash', [txid], ctx) as Promise).catch(() => null), + ]) + const head = quantity(headHex, 'eth_blockNumber') const mined = quantity(receipt.blockNumber, 'receipt.blockNumber') // The block containing it is its own first confirmation, the same convention the deposit // side counts by — and the depth is the same declared depth, so a withdrawal is only // final once it is as deep as an incoming deposit has to be to be credited. const confirmations = head >= mined ? Number(head - mined) + 1 : 0 const depth = depositChainInfo(coin).confirmations + const movement = evmMovement(receipt, tx, ctx) return confirmations >= depth - ? { kind: 'confirmed', confirmations, minedHeight: mined } - : { kind: 'pending', confirmations, minedHeight: mined } + ? { kind: 'confirmed', confirmations, minedHeight: mined, movement } + : { kind: 'pending', confirmations, minedHeight: mined, movement } +} + +export interface EvmReceipt { + blockNumber?: string + status?: string + from?: unknown + to?: unknown + gasUsed?: unknown + effectiveGasPrice?: unknown +} + +export interface EvmTx { + from?: unknown + to?: unknown + value?: unknown + gasPrice?: unknown +} + +/** + * What an EVM transaction took out of its sender: `value` plus the gas it actually burned. + * + * `effectiveGasPrice` from the receipt rather than `gasPrice` from the body, because those + * are the same number only for legacy transactions — a type-2 transaction pays the base fee + * plus its tip, which is what the receipt reports and what the account was actually debited. + * Ember signs legacy only (evm-spec.md §3) and this service builds legacy only, but an + * operator's own hand-built sweep is whatever their wallet sent, and a fee read from the + * wrong field is a `swept` total that does not match the balance the probe will read. + * + * Everything here is best effort and fails to NULL rather than throwing: the caller's other + * two uses — settle a withdrawal, mature a sweep — must not start failing because a node + * omitted a field, and the one use that spends money on it refuses on null by design. + * + * The reading is `readEvmMovement` below, which is pure and throws; this wrapper is the + * logging and the null. Split so the rules can be tested without a node or a logger. + */ +function evmMovement(receipt: EvmReceipt, tx: EvmTx | null, ctx: RpcCtx): OutboundMovement | null { + try { + return readEvmMovement(receipt, tx) + } catch (err) { + // Warned, not swallowed: a node that never answers this makes the manual-sweep route + // permanently unusable on that chain, and the operator's 409 cannot say why on its own. + ctx.log.warn( + { err, upstream: ctx.upstream }, + 'could not read what this transaction moved; anything that must verify an amount will refuse', + ) + return null + } +} + +/** The rules `evmMovement` applies. Throws the sentence its caller logs. Exported for tests. */ +export function readEvmMovement(receipt: EvmReceipt, tx: EvmTx | null): OutboundMovement { + if (!tx) throw new Error('the node returned a receipt but no transaction body') + const raw = typeof tx.from === 'string' ? tx.from : receipt.from + if (typeof raw !== 'string' || !isValidEvmAddress(raw)) { + throw new Error(`neither the transaction nor its receipt named a sender: ${JSON.stringify(raw)}`) + } + // `to` is JSON null — a present key with a null value, not a missing one — for exactly one + // kind of transaction: a contract creation. That is a real answer ("this credited no + // account") and is kept as null, which `checkSweepMovement` refuses. Anything else that is + // not an address is a node answering incompletely, and throws into a null MOVEMENT rather + // than quietly becoming "no destination", because the two refusals read differently to an + // operator and only one of them is the operator's mistake. + const rawTo = tx.to === undefined ? receipt.to : tx.to + if (rawTo !== null && (typeof rawTo !== 'string' || !isValidEvmAddress(rawTo))) { + throw new Error(`the transaction's destination is not an address: ${JSON.stringify(rawTo)}`) + } + const value = quantity(tx.value, 'tx.value') + const price = + receipt.effectiveGasPrice === undefined || receipt.effectiveGasPrice === null + ? quantity(tx.gasPrice, 'tx.gasPrice') + : quantity(receipt.effectiveGasPrice, 'receipt.effectiveGasPrice') + const fee = quantity(receipt.gasUsed, 'receipt.gasUsed') * price + return { + from: toChecksumAddress(raw), + to: rawTo === null ? null : toChecksumAddress(rawTo), + value, + fee, + total: value + fee, + } } // --------------------------------------------------------------------------- @@ -455,6 +667,9 @@ async function buildAndSignXrp(input: BuildInput): Promise { const needed = input.send + input.fee if (available < needed) throw new InsufficientTreasuryError('XRP', available, needed) + // Past this ledger the signed blob can never be applied. Without it an abandoned + // signature stays submittable forever — see PAY_XRP_LEDGER_WINDOW. + const lastLedgerSequence = ledger + env.xrpLedgerWindow const signed = await keyvaultSign( input.from, { @@ -464,13 +679,21 @@ async function buildAndSignXrp(input: BuildInput): Promise { Amount: input.send.toString(), Fee: input.fee.toString(), Sequence: sequence, - // Past this ledger the signed blob can never be applied. Without it an abandoned - // signature stays submittable forever — see PAY_XRP_LEDGER_WINDOW. - LastLedgerSequence: ledger + env.xrpLedgerWindow, + LastLedgerSequence: lastLedgerSequence, }, input.log, ) - return { rawTx: signed, txid: null } + // Both numbers are returned so that the caller can COMMIT them beside the bytes. They are + // the only two facts about an XRP blob that this service cannot get back out of it, and + // without them an abandon can prove neither that the payment never applied (Sequence) nor + // that it never will (LastLedgerSequence) — so a row that lacks them cannot be abandoned + // at all. They are exactly what was signed, because keyvault signs the payload it is given. + return { + rawTx: signed, + txid: null, + sequence: sequence.toString(), + expiry: lastLedgerSequence.toString(), + } } async function xrpCurrentLedger(url: string, ctx: RpcCtx): Promise { @@ -513,28 +736,92 @@ async function xrpStatus( log: FastifyBaseLogger, ): Promise { const { url, ctx } = chainEndpoint('XRP', network, log) - const result = await xrpCall<{ - validated?: boolean - ledger_index?: number - meta?: { TransactionResult?: string } - }>(url, 'tx', [{ transaction: txid, binary: false }], ctx) + const result = await xrpCall(url, 'tx', [{ transaction: txid, binary: false }], ctx) if (result.error) { if (result.error === 'txnNotFound') return { kind: 'unknown' } throw new Error(`xrp tx: ${result.error}`) } // Unvalidated: rippled may already name a ledger, but it is a proposal until consensus // says otherwise, and nothing downstream may treat it as a height. 0 reads as "not mined". - if (result.validated !== true) return { kind: 'pending', confirmations: 0, minedHeight: 0n } + // Nor as an amount: an unvalidated payment has not debited anybody yet. + if (result.validated !== true) { + return { kind: 'pending', confirmations: 0, minedHeight: 0n, movement: null } + } const outcome = result.meta?.TransactionResult ?? 'tesSUCCESS' // A validated ledger is final by consensus, so one confirmation is the whole depth XRP // has — which is exactly what the deposit side already credits at. It is also the whole // depth a sweep needs: the deposit probe reads the validated ledger itself rather than a // lagged view of it, so the money is gone from that view the instant this is true. const ledger = BigInt(result.ledger_index ?? 0) - if (outcome === 'tesSUCCESS') return { kind: 'confirmed', confirmations: 1, minedHeight: ledger } + if (outcome === 'tesSUCCESS') { + return { kind: 'confirmed', confirmations: 1, minedHeight: ledger, movement: xrpMovement(result, ctx) } + } return { kind: 'rejected', reason: `the ledger applied this payment as ${outcome}` } } +export interface XrpTx { + validated?: boolean + ledger_index?: number + TransactionType?: string + Account?: string + Destination?: unknown + Amount?: unknown + Fee?: unknown + meta?: { TransactionResult?: string; delivered_amount?: unknown } +} + +/** + * What an XRP payment took out of its sender: what was delivered, plus the fee. + * + * `meta.delivered_amount` and NEVER `Amount`, and that is not pedantry — under + * `tfPartialPayment` the two differ and only the metadata says what the ledger actually + * moved. `Amount` is the ceiling the sender authorised, so recording it would overstate the + * debit by exactly the shortfall. + * + * There is no fallback to `Amount`, and there was one here until a reviewer pointed out it + * was the very overstatement the paragraph above refuses. rippled itself already does the + * safe half of that fallback: for a non-partial payment it reports `delivered_amount` equal + * to `Amount`, and it reports the literal string `"unavailable"` only for a partial payment + * in a pre-2014 ledger, which is precisely the case where `Amount` is NOT what moved. So the + * fallback could only ever fire where it would be wrong. + * + * Anything that is not a plain drops string is refused into null: an issued-currency amount + * is an object rather than a string and is not XRP at all, and a TransactionType other than + * Payment (AccountDelete, above all, which moves an entire balance) debits the account by + * rules this function does not implement. Null makes the manual-sweep route refuse, which is + * the right answer for a movement this service cannot account for. + */ +function xrpMovement(result: XrpTx, ctx: RpcCtx): OutboundMovement | null { + const movement = readXrpMovement(result) + if (!movement) { + ctx.log.warn( + { upstream: ctx.upstream, transactionType: result.TransactionType }, + 'could not read what this xrp transaction moved; anything that must verify an amount will refuse', + ) + } + return movement +} + +/** The rules `xrpMovement` applies, without the logging. Exported for tests. */ +export function readXrpMovement(result: XrpTx): OutboundMovement | null { + const drops = (value: unknown): bigint | null => + typeof value === 'string' && /^\d+$/.test(value) ? BigInt(value) : null + const account = result.Account + const destination = result.Destination + const delivered = drops(result.meta?.delivered_amount) + const fee = drops(result.Fee) + if ( + result.TransactionType !== 'Payment' || + typeof account !== 'string' || + typeof destination !== 'string' || + delivered === null || + fee === null + ) { + return null + } + return { from: account, to: destination, value: delivered, fee, total: delivered + fee } +} + // --------------------------------------------------------------------------- // Destination validation // --------------------------------------------------------------------------- @@ -781,6 +1068,117 @@ export async function broadcast( } } +/** + * Does this transaction account for the amount an operator says left this deposit address? + * + * THE ONE PLACE A HAND-TYPED AMOUNT MEETS THE CHAIN. `POST /admin/deposit-addresses/:id/ + * sweeps` used to verify only that the txid existed, had not been rejected, and was deep + * enough; the amount beside it went straight into `deposit_addresses.swept` unread, and the + * watcher's next tick computes `observed = confirmed + swept` against the high-water mark. + * An operator who moved 100 EMBER and typed 1000 therefore credited the depositor 900 EMBER + * they never received — spendable, convertible and withdrawable, and reconciled by nothing. + * The route is the documented remedy for a frozen address; that is how it minted money. + * + * The comparison is EXACT and the refusal names the number. A tolerance was considered and + * rejected: the chain states the debit exactly (`value + fee`, both integers), so any band + * around it is a band inside which coin can still be created, and the size of the band would + * be an invented constant that nobody could defend. An operator posts, is told the exact + * figure the chain says, and posts that — two calls, no arithmetic done by a human. + * + * The cap the finding originally proposed — `last_seen - swept` — is deliberately NOT here. + * It refuses the case the route exists for: an address whose crediting is paused by + * `regression`, or one funded after the mark last advanced, holds MORE than that difference, + * and recording the full hand sweep of it is exactly what an operator must be able to do. + * + * THREE QUESTIONS, NOT TWO. Who sent it, WHERE IT WENT, and how much. The middle one was + * missing on the first pass and it left the whole defect reachable: a transaction from the + * deposit address straight back to itself passes "sent by this address" and "the amount is + * exactly what the chain says", and moves nothing out of custody, so `swept` advances against + * funds the probe can still see and the next tick credits them again. The self-send is refused + * here, where the address is in hand; the wider version of it — a sweep pasted to ANOTHER + * customer's deposit address, where the same coins are credited to two people — needs the + * database and is refused by the route (`store.custodialDepositAddress`). + */ +export type SweepMovementCheck = + | { ok: true; movement: OutboundMovement } + | { + ok: false + code: 'sweep_unverified' | 'sweep_wrong_address' | 'sweep_wrong_destination' | 'sweep_amount_mismatch' + error: string + moved?: string + } + +export function checkSweepMovement(input: { + coin: DepositCoin + /** The deposit address the funds are supposed to have left. */ + address: string + /** What the operator posted, in smallest units. */ + postedSmallest: bigint + /** What the chain said this transaction moved, or null if it would not say. */ + movement: OutboundMovement | null +}): SweepMovementCheck { + const { coin, movement } = input + const decimals = depositChainInfo(coin).decimals + if (!movement) { + return { + ok: false, + code: 'sweep_unverified', + error: + `the ${coin} node confirmed that transaction but would not say what it moved, so the amount ` + + 'cannot be checked against it. Nothing is recorded from an unverified amount — retry, and ' + + 'if it keeps failing the node is answering incompletely and an engineer has to look.', + } + } + // Case-insensitively for EVM, character for character everywhere else — the same rule + // `isPlatformAddress` and the withdrawal route compare addresses by, and for the same + // reason: an EVM address has three valid spellings and a plain comparison sees three + // different accounts. + if (canonicalDestination(coin, movement.from) !== canonicalDestination(coin, input.address)) { + return { + ok: false, + code: 'sweep_wrong_address', + error: + `that transaction was sent by ${movement.from}, not by this deposit address ` + + `(${input.address}). Nothing left this address in it, so there is nothing to record against it.`, + } + } + // WHERE IT WENT, not only how much left. A sweep is a movement OUT of custody-per-user and + // into somewhere this address's high-water mark stops applying; a transaction from the + // deposit address back to the deposit address moves nothing out of it, and recording one + // advances `swept` against funds that are still sitting there. The watcher's next tick then + // reads `observed = confirmed + swept` and credits the depositor the whole balance a second + // time — CF-08's exact mint, reached by a pasted wrong destination rather than a mistyped + // amount. The fee genuinely is burned, but the route has no way to record "the fee only" + // and no operator has a reason to want to. + if (movement.to === null || canonicalDestination(coin, movement.to) === canonicalDestination(coin, input.address)) { + return { + ok: false, + code: 'sweep_wrong_destination', + error: + movement.to === null + ? `that transaction credited no account at all (it has no destination), so nothing left ` + + `${input.address} to be recorded as swept.` + : `that transaction paid ${movement.to}, which IS this deposit address. Nothing left custody ` + + 'in it, and recording it as a sweep would advance the swept total against funds still ' + + 'sitting in the address — which the watcher credits to the depositor a second time.', + } + } + if (movement.total !== input.postedSmallest) { + const moved = formatUnits(movement.total, decimals) + return { + ok: false, + code: 'sweep_amount_mismatch', + moved, + error: + `that transaction moved ${moved} ${coin} out of this address ` + + `(${formatUnits(movement.value, decimals)} delivered plus ${formatUnits(movement.fee, decimals)} ` + + `in network fee), not ${formatUnits(input.postedSmallest, decimals)}. Record the amount the chain ` + + 'says: an overstated one is credited to the depositor as a deposit nobody made.', + } + } + return { ok: true, movement } +} + /** Ask the chain where one broadcast transaction has got to. */ export async function outboundStatus( coin: DepositCoin, @@ -796,3 +1194,376 @@ export async function outboundStatus( throw new UnsupportedChainError(coin) } } + +// --------------------------------------------------------------------------- +// Abandoning a signed withdrawal: proving the bytes can never be applied +// +// `POST /admin/withdrawals/:id/abandon` gives a user their money back while a valid +// signature for the same money is still sitting in `withdrawals.raw_tx`. It is the one +// operator action here that credits a balance, and the mistake it can make — refunding a +// payment that then lands — is the mistake this service cannot undo. Until CF-06 the whole +// safety of it was a single question, "does a node have a receipt for the txid on the row?", +// and that question has two holes. Both pay the user twice. +// +// NO RECEIPT IS NOT NO TRANSACTION. On an EVM chain `unknown` means exactly "this node has +// no receipt for that hash", and the signed bytes are untouched by it: the nonce is +// unconsumed, a legacy transaction has no expiry, and any node still holding them can mine +// them minutes or months after the refund. The only proof they can NEVER be mined is that +// the sending account's nonce has moved past the one inside them — `eth_getTransactionCount` +// at `latest`, which is a call this file never made. +// +// A NULL TXID WAS READ AS THE SAFE CASE AND IS THE DANGEROUS ONE. `buildAndSignXrp` returns +// `txid: null` because the id used to come back from the node on submit, so an XRP +// withdrawal whose submit REACHED rippled and was applied, but whose HTTP response was +// lost, carries a null txid and a live payment. The old route read that as "no transaction +// to ask about" and skipped the chain entirely — `if (withdrawal.txid)` — so the refund +// went out with no evidence of any kind. Bytes with no known id are bytes that nothing has +// ever settled, which is the opposite of safe. +// +// So an absence refunds nothing any more. Every abandon of a row that has `raw_tx` must +// produce a POSITIVE proof, and the proof is per family: +// +// EVM the id is DERIVED from the bytes (`evmTxHash`, so a null txid costs nothing here), +// no node has a receipt for it, and the sending account's nonce has moved past the +// nonce in the bytes — which together mean some other transaction took that slot and +// these bytes are permanently unmineable. +// XRP the id is derived the same way (`xrpTxHash`), the ledger does not have it, the +// sending account's Sequence has NOT yet reached the one in the blob — which is what +// proves the payment was never applied — and the current ledger is past the blob's +// LastLedgerSequence, which is what proves it never can be. +// +// The XRP pair is read from the ROW and not from the blob. XRPL binary serialization is a +// format this service does not speak, and it will not learn it for one caller on the path +// where being wrong pays a withdrawal twice; `buildAndSignXrp` writes down what it signed +// instead, and a row without those columns cannot be abandoned at all. That is the fail-closed +// half of the reviewer's two options, applied only where the evidence is genuinely missing. +// --------------------------------------------------------------------------- + +/** Raw bytes of a `0x`-prefixed or bare hex string, or null if it is not one. */ +function hexBytes(rawTx: string): Buffer | null { + const body = rawTx.startsWith('0x') || rawTx.startsWith('0X') ? rawTx.slice(2) : rawTx + if (body.length === 0 || body.length % 2 !== 0 || !/^[0-9a-fA-F]+$/.test(body)) return null + return Buffer.from(body, 'hex') +} + +/** + * The four bytes rippled prefixes a signed transaction with before hashing it: `TXN\0`, + * `HashPrefix::transactionID`. The id is the first half of the SHA-512 of prefix ‖ blob. + */ +const XRP_TXID_PREFIX = Buffer.from('54584E00', 'hex') + +/** + * The id a signed XRP payment will be known by, derived from exactly its bytes. + * + * The direct analogue of `evmTxHash`, and it exists for the same reason: a transaction id + * that is REMEMBERED is unavailable on precisely the paths where things went wrong. XRP's is + * worse than EVM's, because it was never remembered at all — `submit` returns it, so a submit + * whose response is lost leaves a payment that may well be in the ledger with no id on the row + * to poll for it. Deriving it turns the null-txid case from "the chain cannot be asked" into + * an ordinary lookup. + * + * Uppercase, because that is how rippled spells transaction ids everywhere, including in the + * `tx_json.hash` this service stores from `submit` — so a derived id and a recorded one + * compare equal. + */ +export function xrpTxHash(rawTx: string): string | null { + const bytes = hexBytes(rawTx) + if (!bytes) return null + return createHash('sha512') + .update(Buffer.concat([XRP_TXID_PREFIX, bytes])) + .digest('hex') + .slice(0, 64) + .toUpperCase() +} + +/** The id these signed bytes will be known by, for the coins that have a derivation. */ +export function signedTxId(coin: DepositCoin, rawTx: string): string | null { + if (!hexBytes(rawTx)) return null + if (isEvmCoin(coin)) return evmTxHash(rawTx) + return coin === 'XRP' ? xrpTxHash(rawTx) : null +} + +interface RlpHeader { + payloadAt: number + payloadLength: number + next: number + list: boolean + /** A byte in 0x00–0x7f, which encodes itself: the payload IS the tag. */ + single: boolean +} + +/** A big-endian RLP length prefix, refusing a non-canonical one rather than reading it. */ +function rlpLength(buf: Buffer, at: number, bytes: number): number | null { + if (bytes < 1 || bytes > 4 || at + bytes > buf.length || buf[at] === 0) return null + let value = 0 + for (let i = 0; i < bytes; i += 1) value = value * 256 + buf[at + i]! + return value +} + +function rlpHeader(buf: Buffer, at: number): RlpHeader | null { + if (at >= buf.length) return null + const tag = buf[at]! + const make = (payloadAt: number, payloadLength: number, list: boolean): RlpHeader | null => + payloadAt + payloadLength <= buf.length + ? { payloadAt, payloadLength, next: payloadAt + payloadLength, list, single: false } + : null + if (tag <= 0x7f) return { payloadAt: at, payloadLength: 1, next: at + 1, list: false, single: true } + if (tag <= 0xb7) return make(at + 1, tag - 0x80, false) + if (tag <= 0xbf) { + const size = tag - 0xb7 + const length = rlpLength(buf, at + 1, size) + return length === null ? null : make(at + 1 + size, length, false) + } + if (tag <= 0xf7) return make(at + 1, tag - 0xc0, true) + const size = tag - 0xf7 + const length = rlpLength(buf, at + 1, size) + return length === null ? null : make(at + 1 + size, length, true) +} + +/** + * The nonce inside a signed legacy EVM transaction, read out of the bytes themselves. + * + * DERIVED, NOT REMEMBERED, for the same reason `evmTxHash` is: the value has to describe the + * bytes that are actually in the row, and a column written beside them can drift from them in + * a way nothing would ever notice. It also means every withdrawal signed before this check + * existed can be reasoned about, which a new column could not do. + * + * A legacy transaction is `rlp([nonce, gasPrice, gasLimit, to, value, data, v, r, s])`, so + * this reads two RLP headers and stops. EVERYTHING ELSE IS NULL, and null is a refusal to + * abandon rather than a nonce of zero: an EIP-2718 typed envelope (whose first byte is the + * type and never a list tag), a body whose outer list does not span exactly these bytes, a + * nonce longer than eight bytes, or any non-canonical integer encoding. This service signs + * legacy only (evm-spec.md §3) and keyvault refuses to sign anything else for Ember, so the + * null branches are guards and not a fallback. + */ +export function legacyNonce(rawTx: string): bigint | null { + const bytes = hexBytes(rawTx) + if (!bytes) return null + const outer = rlpHeader(bytes, 0) + if (!outer || !outer.list || outer.next !== bytes.length) return null + const first = rlpHeader(bytes, outer.payloadAt) + if (!first || first.list || first.next > outer.next) return null + const payload = bytes.subarray(first.payloadAt, first.payloadAt + first.payloadLength) + // 0x00–0x7f encodes itself, except zero: canonical RLP writes zero as the empty string 0x80. + if (first.single) return payload[0]! === 0 ? null : BigInt(payload[0]!) + if (payload.length === 0) return 0n + if (payload.length > 8 || payload[0] === 0) return null + return BigInt(`0x${payload.toString('hex')}`) +} + +/** Why an abandon was refused. Each one is a code the admin route answers with. */ +export type AbandonRefusal = + /** A node can still see the payment. Refunding it would pay the user twice. */ + | 'withdrawal_on_chain' + /** The signature is still live: nothing has retired it and nothing has expired it. */ + | 'withdrawal_still_applicable' + /** The evidence needed to decide is not obtainable. Fails closed, on purpose. */ + | 'withdrawal_unprovable' + +export type AbandonCheck = + /** These exact bytes can never be applied, and `proof` says which fact settled it. */ + | { ok: true; proof: string } + | { ok: false; code: AbandonRefusal; error: string } + +/** Everything the chain was asked, gathered apart from the decision taken on it. */ +export interface AbandonEvidence { + /** What a node says about every id these bytes could be known by, derived one first. */ + statuses: { txid: string; status: OutboundStatus }[] + /** EVM: the nonce in the bytes, and how many the sending account has used at `latest`. */ + evm?: { nonce: bigint | null; accountUsed: bigint } + /** XRP: what was signed (from the row, per the section note) against where the ledger is. */ + xrp?: { + sequence: bigint | null + lastLedger: bigint | null + accountSequence: bigint + currentLedger: bigint + } +} + +/** + * Can these signed bytes still be applied? The whole decision, in one pure function. + * + * Separated from the RPC calls that feed it exactly as `checkSweepMovement` is, and for the + * same reason: this is the rule that decides whether a user is credited money that may also + * be leaving on chain, and a rule that can only be exercised against a live node is a rule + * nobody exercises. Every branch below is a test in outbound.test.ts. + */ +export function judgeAbandon(coin: DepositCoin, evidence: AbandonEvidence): AbandonCheck { + for (const { txid, status } of evidence.statuses) { + if (status.kind === 'pending' || status.kind === 'confirmed') { + return { + ok: false, + code: 'withdrawal_on_chain', + error: + `the network still has this payment — ${txid} is ${status.kind} at ${status.confirmations} ` + + 'confirmations. Refunding it now would credit the user money that has already left the ' + + 'treasury. Wait for it to confirm; it settles itself.', + } + } + if (status.kind === 'rejected') { + return { ok: true, proof: `the chain applied ${txid} and it did not deliver: ${status.reason}` } + } + } + + if (evidence.evm) { + const { nonce, accountUsed } = evidence.evm + if (nonce === null) { + return { + ok: false, + code: 'withdrawal_unprovable', + error: + 'the signed bytes on this withdrawal are not a legacy transaction this service can read a ' + + 'nonce out of, so there is no way to show they can never be mined. Nothing is refunded on ' + + 'an unread signature — an engineer has to look at the row.', + } + } + if (accountUsed > nonce) { + return { + ok: true, + proof: + `no node has a receipt for these bytes and the treasury has used ${accountUsed} nonces, past ` + + `the ${nonce} these bytes carry — the slot was taken by another transaction, so they can ` + + 'never be mined', + } + } + return { + ok: false, + code: 'withdrawal_still_applicable', + error: + `these signed bytes carry nonce ${nonce} and the treasury has used only ${accountUsed}, so any ` + + 'node still holding them can mine them at any time — a refund now can be followed by the ' + + 'payment landing. Retire the nonce first: send a 0-value transaction from the treasury to ' + + `itself at nonce ${nonce} with a higher gas price, and abandon this once it is mined.`, + } + } + + if (evidence.xrp) { + const { sequence, lastLedger, accountSequence, currentLedger } = evidence.xrp + if (sequence === null || lastLedger === null) { + return { + ok: false, + code: 'withdrawal_unprovable', + error: + 'this withdrawal was signed before this service recorded the Sequence and LastLedgerSequence ' + + 'it signed, and an XRP blob cannot be read back for them here. There is therefore no way to ' + + 'show the payment never applied, and it is not refunded. An engineer can settle it from the ' + + "treasury account's transaction history.", + } + } + if (accountSequence > sequence) { + return { + ok: false, + code: 'withdrawal_unprovable', + error: + `the treasury account is at Sequence ${accountSequence}, past the ${sequence} these bytes ` + + 'carry, so that sequence has been consumed — by this payment or by something else, and the ' + + 'ledger no longer has the transaction to tell the two apart. It is not refunded on that ' + + "ambiguity: check the treasury account's history for a payment to this destination.", + } + } + if (currentLedger <= lastLedger) { + return { + ok: false, + code: 'withdrawal_still_applicable', + error: + `these signed bytes stay applicable until ledger ${lastLedger} and the network is on ` + + `${currentLedger}, so re-submitting them — which the outbound worker does every tick — can ` + + `still deliver the payment. This can be abandoned after ledger ${lastLedger}, about ` + + `${Math.max(1, Number(lastLedger - currentLedger)) * 4} seconds away.`, + } + } + return { + ok: true, + proof: + `the ledger has no record of these bytes, the treasury account is still at Sequence ` + + `${accountSequence} and has never consumed the ${sequence} they carry, and ledger ` + + `${currentLedger} is past their LastLedgerSequence of ${lastLedger} — so they were never ` + + 'applied and can never be applied', + } + } + + return { + ok: false, + code: 'withdrawal_unprovable', + error: + `this service cannot prove that a signed ${coin} transaction has become unapplicable, so it will ` + + 'not refund one. An engineer has to settle this withdrawal by hand.', + } +} + +/** A decimal string from a database column, or null — never a partially-read number. */ +function decimalOrNull(value: string | null | undefined): bigint | null { + return typeof value === 'string' && /^\d+$/.test(value) ? BigInt(value) : null +} + +export interface AbandonInput { + coin: DepositCoin + network: ChainNetwork + /** `withdrawals.raw_tx` — the signed bytes the refund is being weighed against. */ + rawTx: string + /** `withdrawals.txid`, which is null far more often than the old route assumed. */ + txid: string | null + /** The account these bytes spend from: the treasury this coin is paid out of. */ + from: string + /** `withdrawals.signed_sequence` and `withdrawals.signed_expiry`. XRP only. */ + signedSequence: string | null + signedExpiry: string | null + log: FastifyBaseLogger +} + +/** Ask the chain everything `judgeAbandon` needs. Throws if it cannot be asked. */ +export async function abandonEvidence(input: AbandonInput): Promise { + const { coin, network, rawTx, log } = input + // The derived id first, because it is the one that exists when the row's does not. The + // row's is asked about too and only when it differs, so a recorded id this service could + // not have derived — a hand-edited row, or a chain that renames a transaction — is still + // checked rather than assumed identical. + const ids: string[] = [] + for (const id of [signedTxId(coin, rawTx), input.txid]) { + if (id && !ids.includes(id)) ids.push(id) + } + const statuses = await Promise.all( + ids.map(async (txid) => ({ txid, status: await outboundStatus(coin, network, txid, log) })), + ) + + if (isEvmCoin(coin)) { + const { url, ctx } = chainEndpoint(coin, network, log) + // 'latest' and never 'pending': a pending count includes transactions in this node's own + // mempool, and "a node is holding something at that nonce" is the opposite of proof that + // the slot has been taken permanently. + const used = quantity( + await jsonRpc(url, 'eth_getTransactionCount', [input.from, 'latest'], ctx), + 'eth_getTransactionCount', + ) + return { statuses, evm: { nonce: legacyNonce(rawTx), accountUsed: used } } + } + if (coin === 'XRP') { + const { url, ctx } = chainEndpoint('XRP', network, log) + const [account, currentLedger] = await Promise.all([ + // At `current`, which includes the open ledger — so a sequence consumed a moment ago + // reads as consumed and this refuses. Erring toward refusing is the whole point. + xrpAccount(url, input.from, ctx), + xrpCurrentLedger(url, ctx), + ]) + return { + statuses, + xrp: { + sequence: decimalOrNull(input.signedSequence), + lastLedger: decimalOrNull(input.signedExpiry), + accountSequence: BigInt(account.sequence), + currentLedger: BigInt(currentLedger), + }, + } + } + return { statuses } +} + +/** + * May this withdrawal's money go back to the user? Gather, then judge. + * + * Throws when the chain cannot be reached, and the caller must treat that as a refusal too: + * an unreachable node is the state in which the least is known about where the payment got to. + */ +export async function checkAbandonable(input: AbandonInput): Promise { + return judgeAbandon(input.coin, await abandonEvidence(input)) +} diff --git a/services/pay/src/routes/admin.ts b/services/pay/src/routes/admin.ts index 61900ad..85eca32 100644 --- a/services/pay/src/routes/admin.ts +++ b/services/pay/src/routes/admin.ts @@ -1,10 +1,24 @@ import type { FastifyInstance } from 'fastify' import { z } from 'zod' -import { depositChainInfo, type ChainNetwork, type DepositCoin } from '@cloudsforge/shared' +import { + depositChainInfo, + SUPPORTED_DEPOSIT_COINS, + type ChainNetwork, + type DepositCoin, +} from '@cloudsforge/shared' import { requireAdmin } from '../auth.js' import { formatUnits, parseUnits } from '../amounts.js' -import { sweepIsMature, sweepMaturityConfirmations } from '../chains.js' -import { canSend, outboundStatus, spendableBalance } from '../outbound.js' +import { fetchFundedTotals, sweepIsMature, sweepMaturityConfirmations } from '../chains.js' +import { DEPLOYMENT_NETWORK } from '../network.js' +import { + canSend, + checkAbandonable, + checkSweepMovement, + manualSweepGate, + outboundStatus, + spendableBalance, + type AbandonCheck, +} from '../outbound.js' import { ADMINISTERED_COINS, administeredPrice, @@ -16,14 +30,19 @@ import { type AdministeredPrice, } from '../pricing.js' import { + custodialDepositAddress, + depositAddressOverview, getDepositAddressById, - getWithdrawal, + getWithdrawalRow, listAllWithdrawals, recordManualSweep, refundWithdrawal, + scanDepositAddress, + toWithdrawal, + type DepositAddressState, type WithdrawalStatus, } from '../store.js' -import { listTreasuries, treasuryPin } from '../treasury.js' +import { findTreasury, listTreasuries, treasuryPin } from '../treasury.js' /* ------------------------------------------------------------------ * * /admin — the operator surface. This service had none at all until now. @@ -52,8 +71,12 @@ const recordSweepSchema = z.object({ * Everything that left the address, as a DECIMAL STRING and never a JSON number, for the * same reason the price above is one: this value is parsed straight into the integer the * watcher does its arithmetic in. + * + * Constrained to digits here rather than left to `parseUnits`, whose BigInt throws on + * anything else and would turn a typo into a 500 — the same rule `POST /withdrawals` + * applies to the one other hand-typed amount this service takes. */ - amount: z.string().min(1).max(64), + amount: z.string().min(1).max(64).regex(/^\d+(\.\d+)?$/), /** * The operator's own transaction. REQUIRED, and it is not bookkeeping: it is what this * route checks the movement's depth against before it will record anything, and it is the @@ -64,6 +87,39 @@ const recordSweepSchema = z.object({ destination: z.string().trim().min(1).max(200).optional(), }) +/** + * `GET /admin/deposit-addresses` — the query. Every field is optional and every one of them + * narrows: an operator arrives here holding either an address (off a block explorer, or off + * the watcher's own alarm) or a coin, and wants the row's `id`. + */ +const listDepositAddressesSchema = z.object({ + coin: z.string().trim().min(1).max(16).optional(), + address: z.string().trim().min(1).max(200).optional(), + userId: z.string().trim().min(1).max(200).optional(), + limit: z.coerce.number().int().min(1).max(200).optional(), + /** + * Rows to skip. Without it this route could only ever show the newest page, so on a + * deployment with more than `limit` addresses "list the frozen ones" was unanswerable — + * and `?scan=true`, capped at 25, could never look past the 25 most recently minted. + * There is no upper bound because there is no upper bound on how many addresses exist; + * the cost of a large one is a Postgres offset scan, not a chain probe. + */ + offset: z.coerce.number().int().min(0).optional(), + /** Query strings have no booleans. Explicit values only, so a typo is a 400 and not a scan. */ + scan: z.enum(['true', 'false']).optional(), +}) + +const DEFAULT_ADDRESS_PAGE = 50 +/** + * How many addresses one request may ask the chain about. + * + * `?scan=true` costs one RPC round trip per address, against a node this service also needs + * for crediting deposits and settling withdrawals. A listing that quietly turned into two + * hundred concurrent probes would make an operator's read the thing that stalls the watcher, + * so a wider page is refused rather than served slowly. + */ +const MAX_SCANNED_ADDRESSES = 25 + const WITHDRAWAL_STATUSES: WithdrawalStatus[] = [ 'pending', 'signed', @@ -190,6 +246,161 @@ export async function adminRoutes(app: FastifyInstance) { return { treasuries } }) + /** + * GET /admin/deposit-addresses — every customer deposit address, and its id. + * + * WHY THIS EXISTS AT ALL (CF-22). The deposit watcher freezes crediting for an address whose + * confirmed balance falls below its high-water mark with no sweep to account for it, and the + * alarm it writes names the remedy: `POST /admin/deposit-addresses/:id/sweeps`. That remedy + * is keyed by `deposit_addresses.id`, a random UUID. `GET /deposits` returns it — to the + * address's OWNER, and to nobody else — so the operator holding the alarm had exactly one + * way to obtain the parameter of the one route that unfreezes the address: open a SQL shell + * on the production database. An operator surface whose remedy needs a hand-written SELECT + * is a remedy that gets skipped in favour of "it will probably sort itself out", which for + * this state it will not. The id is now also in the log line itself (`watcher.depositScanLine`), + * which is the cheaper half of the same fix; this route is the half that answers the other + * direction — an address, or a coin, or a user, and no alarm in hand. + * + * WHAT IT COSTS TO ASK. Unqualified it is two indexed reads and no chain traffic: + * `last_seen`, `swept`, `pending`, and any sweep of ours that has never been added to + * `swept` (`stuck` ones included — that is usually the answer). `?scan=true` adds the one + * thing the database cannot answer: whether the address is FROZEN RIGHT NOW. That is a + * comparison against a live balance read at the coin's confirmation depth, so it costs an + * RPC per address and is capped at MAX_SCANNED_ADDRESSES; it is computed by + * `store.scanDepositAddress`, which is the watcher's own arithmetic with the writes taken + * out, and it credits nothing and changes nothing. + * + * Addresses on another network are listed and never probed — `watched: false` — for the + * reason `network.partitionByDeploymentNetwork` gives: this deployment has no node for them. + */ + app.get<{ Querystring: Record }>( + '/admin/deposit-addresses', + async (request, reply) => { + const parsed = listDepositAddressesSchema.safeParse(request.query) + if (!parsed.success) { + return reply.code(400).send({ error: 'invalid query', code: 'validation' }) + } + const coin = parsed.data.coin?.toUpperCase() + if (coin && !SUPPORTED_DEPOSIT_COINS.some((c) => c.coin === coin)) { + return reply.code(400).send({ error: `${coin} is not a deposit coin`, code: 'validation' }) + } + const scan = parsed.data.scan === 'true' + // A scan the caller did not size is sized for them. Refusing the DEFAULT page as too + // wide would mean `?scan=true` — the whole reason an operator comes here — never worked + // without a second parameter they have no reason to guess at. + const limit = parsed.data.limit ?? (scan ? MAX_SCANNED_ADDRESSES : DEFAULT_ADDRESS_PAGE) + if (scan && limit > MAX_SCANNED_ADDRESSES) { + return reply.code(400).send({ + error: + `a scan asks the chain about every address it returns, so it is limited to ` + + `${MAX_SCANNED_ADDRESSES} at a time. Narrow by coin, address or userId, or lower limit.`, + code: 'scan_too_wide', + }) + } + + const offset = parsed.data.offset ?? 0 + const overviews = await depositAddressOverview({ + coin, + address: parsed.data.address, + userId: parsed.data.userId, + limit, + offset, + }) + + const addresses = await Promise.all( + overviews.map(async ({ row, sweeps, unmatured }) => { + const info = depositChainInfo(row.coin as DepositCoin) + const watched = row.network === DEPLOYMENT_NETWORK + const base = { + /** The parameter of `POST /admin/deposit-addresses/:id/sweeps`. The point of this route. */ + id: row.id, + userId: row.userId, + coin: row.coin, + network: row.network, + address: row.address, + /** False for an address this deployment has no node for — see CF-25. */ + watched, + /** High-water mark of credited funds, as a decimal string like every other amount. */ + lastSeen: formatUnits(BigInt(row.lastSeen || '0'), info.decimals), + swept: formatUnits(BigInt(row.swept || '0'), info.decimals), + pending: formatUnits(BigInt(row.pending || '0'), info.decimals), + markBasis: row.markBasis, + createdAt: row.createdAt.toISOString(), + /** Sweeps of ours whose amount has never reached `swept`. Usually none. */ + sweeps: sweeps.map((sweep) => ({ + id: sweep.id, + status: sweep.status, + amount: formatUnits(BigInt(sweep.amount), info.decimals), + txid: sweep.txid, + destination: sweep.toAddress, + minedHeight: sweep.minedHeight, + failureReason: sweep.failureReason, + updatedAt: sweep.updatedAt.toISOString(), + })), + } + if (!scan) return { ...base, scan: null } + if (!watched) { + return { + ...base, + scan: { + state: 'unwatched' as const, + error: `this deployment settles on ${DEPLOYMENT_NETWORK} and has no ${row.network} node to ask`, + }, + } + } + try { + const probe = await fetchFundedTotals( + row.coin as DepositCoin, + row.network as ChainNetwork, + row.address, + request.log, + ) + const state: DepositAddressState = scanDepositAddress({ + coin: row.coin as DepositCoin, + markBasis: row.markBasis, + lastSeen: BigInt(row.lastSeen || '0'), + probe, + swept: unmatured, + }) + return { + ...base, + scan: { + ...state, + /** What the chain says, at the depth the watcher credits from. */ + confirmed: formatUnits(probe.confirmed, info.decimals), + tip: formatUnits(probe.tip, info.decimals), + }, + } + } catch (err) { + // One address failing to answer is not the others' problem, and it is not this + // read's problem either: the row is still listed, with its id, which is the thing + // the caller came for. + request.log.warn( + { err, addressId: row.id, coin: row.coin, address: row.address }, + 'admin listing: this deposit address could not be probed', + ) + return { + ...base, + scan: { state: 'unreadable' as const, error: `the ${row.coin} node could not be asked` }, + } + } + }), + ) + return { + addresses, + scanned: scan, + /** + * Where this page started and how wide it was, and the offset of the next one — null + * when this page was not full, which is the only cheap way to say "that is all of + * them" without a second COUNT over a table nobody is paging for fun. + */ + offset, + limit, + nextOffset: addresses.length === limit ? offset + limit : null, + } + }, + ) + /** * POST /admin/deposit-addresses/:id/sweeps — record funds an operator moved BY HAND. * @@ -214,9 +425,41 @@ export async function adminRoutes(app: FastifyInstance) { * minutes; the refusal says how deep it is and how deep it must be, so an operator can * come back rather than guess. * + * AND THE AMOUNT IS CHECKED AGAINST THAT TRANSACTION, which until CF-08 it was not. Depth + * was verified and size was taken on trust: `amount` went into `deposit_addresses.swept` + * unread, the watcher's next tick computed `observed = confirmed + swept`, and every unit + * of the excess was credited to the depositor as a deposit nobody made — immediately + * convertible to Shards and withdrawable from the treasury. One mistyped zero, or one wei + * figure pasted into a field that expects whole coins, mints money. So the chain is asked + * what left the address (`outbound.OutboundMovement`) and the posted amount must equal it + * exactly; the refusal states the figure the chain gives, so the second attempt is a + * paste rather than an arithmetic exercise. See `outbound.checkSweepMovement`. + * + * AND SO IS WHERE IT WENT, which the first pass at CF-08 left out and which left the whole + * defect reachable through a different key on the same keyboard. Verifying only the sender + * and the size accepts a transaction that goes from the deposit address straight back to + * it: nothing leaves custody, `swept` advances anyway, and the watcher's next tick reads + * `confirmed + swept` and credits the depositor their own balance a second time. Worse + * still is the near miss — a sweep pasted to ANOTHER customer's deposit address, where the + * source records a sweep and the destination's watcher credits a deposit, so one movement + * of coins credits two people. Both are refused: the self-send by `checkSweepMovement`, + * the second by `store.custodialDepositAddress` here. A treasury destination is fine, and + * is the point. + * + * AND IT NOW RECONCILES A SWEEP OF OUR OWN INSTEAD OF REPORTING A REPLAY (CF-07). The + * state this route is most often reached in is an address frozen by a `stuck` sweep — one + * this service signed and broadcast, that was mined and then took longer than its chain's + * deadline to reach maturity depth. That row holds the same transaction id the operator is + * about to post, so the POST used to collide with the `(address_id, txid)` unique index, + * insert nothing, and answer 200 `{ replayed: true }` with the swept total unchanged and + * the address exactly as frozen as before. The documented remedy was a silent no-op against + * the one state it was documented for. An unrecorded conflicting row is now matured in + * place — the same accounting `sweeper.matureSweep` does — and the reply says `reconciled`. + * * It is a no-op on Bitcoin, whose probe is a cumulative received counter that sweeping * does not move — the recording path knows, and says so by returning the total unchanged. * The row is still written, because the movement happened and should be reconstructible. + * Every OTHER coin this service cannot look a transaction up on is refused outright. */ app.post<{ Params: { id: string } }>('/admin/deposit-addresses/:id/sweeps', async (request, reply) => { const parsed = recordSweepSchema.safeParse(request.body) @@ -235,14 +478,38 @@ export async function adminRoutes(app: FastifyInstance) { return reply.code(400).send({ error: 'amount must be positive', code: 'validation' }) } - // What the chain says about the operator's transaction. BTC and SOL cannot be asked — - // this service has no way to look a transaction up on either (`outboundStatus` refuses - // them by name) — and neither needs to be: BTC's probe is a cumulative counter that a - // sweep does not move at all, and SOL's is read at the chain's own finalized view with - // no lag on top, so on both of them `sweepMaturityConfirmations` is the depth itself and - // there is no window in which recording early counts the same coins twice. + // What the chain says about the operator's transaction: that it exists, that it was not + // rejected, that it is deep enough — and, since CF-08, that it moved THIS amount out of + // THIS address. The last one is the difference between a route that reconciles an address + // and a route that mints coin from a mistyped number. + // + // A coin whose transactions this service cannot look up is refused rather than recorded + // on trust. That gate used to be `canSend`, which is a fact about signing and not about + // reading, and the two differ exactly where it hurts: `canSend('SOL')` is false, so a SOL + // manual sweep skipped this whole block — no txid check, no depth check, no amount check + // — while `recordSweepIn` still added the number to `swept`, because Solana's probe is a + // spot balance. An invented txid and any amount at all were accepted, and the watcher + // credited the difference on its next tick. + // + // BITCOIN IS THE ONE EXCEPTION AND IT IS EXEMPT ON MECHANISM, NOT ON TRUST: its probe is + // a cumulative received counter that a sweep does not move, so `recordSweepIn` returns + // early and `swept` cannot change. There is no number to overstate. The row is still + // written, because the movement happened and should be reconstructible. + const gate = manualSweepGate(coin) let minedHeight: bigint | null = null - if (canSend(coin)) { + /** Where the CHAIN says the funds went. Null on the `inert` path, which asks nothing. */ + let verifiedDestination: string | null = null + if (gate === 'refuse') { + return reply.code(409).send({ + error: + `this service cannot look a ${coin} transaction up, so it cannot verify that ${parsed.data.txid} ` + + `moved ${parsed.data.amount} ${coin} out of this address — and an amount recorded without that ` + + "check is credited to the depositor as a deposit nobody made. Reconcile this address's " + + 'accounting through an engineer rather than through this route.', + code: 'sweep_unverifiable_chain', + }) + } + if (gate === 'verify') { let status: Awaited> try { status = await outboundStatus(coin, network, parsed.data.txid, request.log) @@ -277,16 +544,107 @@ export async function adminRoutes(app: FastifyInstance) { required, }) } + // The amount, against the transaction. Last, because it is the only check whose + // refusal an operator answers by posting a different number rather than by waiting. + const check = checkSweepMovement({ + coin, + address: address.address, + postedSmallest: amountSmallest, + movement: status.movement, + }) + if (!check.ok) { + request.log.warn( + { + addressId: address.id, + coin, + address: address.address, + txid: parsed.data.txid, + posted: parsed.data.amount, + moved: check.moved ?? null, + code: check.code, + setBy: request.user!.sub, + }, + 'manual sweep refused: the transaction does not account for the amount posted', + ) + return reply.code(409).send({ error: check.error, code: check.code, moved: check.moved }) + } + // AND WHERE IT WENT, against the database rather than against the transaction. + // `checkSweepMovement` has already refused the destination it can see for itself — the + // deposit address sending to the deposit address — but the same paste one character out + // lands on SOMEBODY ELSE'S deposit address, and that one is worse: the source records a + // legitimate sweep while the destination's next watcher tick sees a confirmed balance + // appear and credits it to that second customer as a fresh deposit. One movement of + // coins, two credited users, and nothing anywhere reconciles it. A TREASURY destination + // is deliberately not refused — it is where a sweep is supposed to go. + const custodial = check.movement.to ? await custodialDepositAddress(check.movement.to) : null + if (custodial) { + request.log.error( + { + addressId: address.id, + coin, + address: address.address, + txid: parsed.data.txid, + destinationAddressId: custodial.id, + destinationUserId: custodial.userId, + setBy: request.user!.sub, + }, + 'manual sweep refused: the destination is another customer deposit address, which ' + + 'would credit those funds to a second user', + ) + return reply.code(409).send({ + error: + `that transaction paid ${check.movement.to}, which is another customer's deposit address ` + + 'on this platform. Its own watcher will credit those funds to that customer as a deposit, ' + + 'so recording a sweep out of this address as well would credit the same coins twice. ' + + 'Move the funds on to the treasury and record THAT transaction, and have an engineer ' + + 'reconcile the credit this one produces.', + code: 'sweep_destination_custodial', + }) + } minedHeight = status.minedHeight > 0n ? status.minedHeight : null + // The chain's word for where the funds went, in preference to the operator's. Everything + // above proves it; the body's `destination` is free text nothing checks. + verifiedDestination = check.movement.to } - const { swept, replayed } = await recordManualSweep({ + const outcome = await recordManualSweep({ address, txid: parsed.data.txid, amountSmallest, - destination: parsed.data.destination ?? `(not recorded, by ${request.user!.sub})`, + destination: verifiedDestination ?? parsed.data.destination ?? `(not recorded, by ${request.user!.sub})`, minedHeight, }) + + // A sweep of ours already claims this transaction and says a different amount left the + // address. Recording either number would put a figure into `swept` that no transaction + // accounts for, so neither is recorded and both are named — see `recordManualSweep`. + if (outcome.kind === 'disagrees') { + request.log.error( + { + addressId: address.id, + coin, + address: address.address, + txid: parsed.data.txid, + posted: parsed.data.amount, + sweepId: outcome.sweepId, + rowAmount: formatUnits(outcome.rowAmount, info.decimals), + setBy: request.user!.sub, + }, + 'manual sweep refused: this service holds its own sweep for that transaction and it ' + + 'states a different amount', + ) + return reply.code(409).send({ + error: + `this service already holds a sweep of its own for ${parsed.data.txid}, and that row says ` + + `${formatUnits(outcome.rowAmount, info.decimals)} ${coin} left this address, not ` + + `${parsed.data.amount}. Nothing is recorded: one of the two is wrong about what moved and ` + + 'no arithmetic here can say which. An engineer has to reconcile sweep ' + + `${outcome.sweepId} against the chain.`, + code: 'sweep_amount_disagrees', + recorded: formatUnits(outcome.rowAmount, info.decimals), + }) + } + // An operator moving custody funds by hand is the single least reversible thing anyone // does to this service. It gets a name and a number in the log, permanently. request.log.warn( @@ -296,21 +654,35 @@ export async function adminRoutes(app: FastifyInstance) { address: address.address, amount: parsed.data.amount, txid: parsed.data.txid, - swept, - replayed, + swept: outcome.swept, + outcome: outcome.kind, + sweepId: outcome.kind === 'reconciled' ? outcome.sweepId : undefined, + wasStatus: outcome.kind === 'reconciled' ? outcome.wasStatus : undefined, setBy: request.user!.sub, setByHandle: request.user!.handle, }, - replayed ? 'manual sweep already recorded — this POST changed nothing' : 'manual sweep recorded', + outcome.kind === 'replayed' + ? 'manual sweep already recorded — this POST changed nothing' + : outcome.kind === 'reconciled' + ? "manual sweep reconciled this service's own unrecorded sweep of that transaction — " + + 'its amount is now in the swept total and the address is crediting again' + : 'manual sweep recorded', ) return { addressId: address.id, coin: address.coin, address: address.address, - recorded: formatUnits(amountSmallest, info.decimals), - swept: formatUnits(BigInt(swept), info.decimals), - /** True when this exact (address, transaction) was already on record. */ - replayed, + /** What reached `swept`, which on the reconcile path is the existing row's amount. */ + recorded: formatUnits(outcome.recorded, info.decimals), + swept: formatUnits(BigInt(outcome.swept), info.decimals), + /** True when this exact (address, transaction) was already on record AND counted. */ + replayed: outcome.kind === 'replayed', + /** + * True when a sweep of this service's own for that transaction — stuck below its + * maturity depth, or still in flight — was matured in place rather than duplicated. + * This is the answer an operator un-freezing an address after a stuck sweep gets. + */ + reconciled: outcome.kind === 'reconciled', } }) @@ -331,26 +703,111 @@ export async function adminRoutes(app: FastifyInstance) { * refunded. That check is the whole point of the route: an operator's belief that a * payment did not land is not evidence, and a refund of a payment that did land is the one * mistake this service cannot undo. + * + * AND THE QUESTION IT ASKS CHANGED, because the old one had two holes it answered "refund" + * through (CF-06). It was `if (withdrawal.txid)` — ask the node for a receipt, and refund + * on anything that was not one. + * + * A MISSING RECEIPT IS NOT A DEAD SIGNATURE. `unknown` on an EVM chain means only that + * this node has no receipt for that hash. The bytes in `raw_tx` are untouched by it: the + * nonce is unconsumed, a legacy transaction never expires, and any node still holding + * them can mine them long after the refund has been credited. + * + * A NULL txid SKIPPED THE CHAIN ENTIRELY, and that is the branch that pays twice. XRP + * transaction ids came back from `submit`, so an XRP payment whose submit reached rippled + * and was APPLIED, but whose HTTP response was lost, is a row with a null txid and a live + * payment. The condition above read it as "there is nothing to ask about" and refunded on + * no evidence at all — the exact case the route exists to prevent. + * + * So the row's txid is no longer what decides anything. A row with `raw_tx` is refunded + * only against a positive proof that those exact bytes can never be applied — a consumed + * nonce for EVM, an unconsumed Sequence past its LastLedgerSequence for XRP — and the + * proof is written into the log line beside the refund. See `outbound.judgeAbandon`, which + * is where the rule lives, and the section above it for why the XRP half reads two columns + * rather than the blob. A row with no `raw_tx` has no signature to be afraid of and is + * refunded as it always was; that is the whole of the `pending` case. */ app.post<{ Params: { id: string } }>('/admin/withdrawals/:id/abandon', async (request, reply) => { - const withdrawal = await getWithdrawal(request.params.id) - if (!withdrawal) { + // The ROW, not the view: the view is what a user may see and `raw_tx` is not on it. + const row = await getWithdrawalRow(request.params.id) + if (!row) { return reply.code(404).send({ error: 'withdrawal not found', code: 'not_found' }) } - if (withdrawal.txid) { - const status = await outboundStatus( - withdrawal.coin, - withdrawal.network, - withdrawal.txid, - request.log, - ) + const withdrawal = toWithdrawal(row) + const coin = withdrawal.coin + const network = withdrawal.network + let proof = 'nothing was ever signed for this withdrawal, so there are no bytes that could apply' + + if (row.rawTx) { + // Which account those bytes spend from. Read-only — abandoning a withdrawal is not a + // reason to mint a treasury — and its absence is a refusal: without the address there + // is no nonce to check and no account to read a Sequence from. + const treasury = await findTreasury(coin, network) + if (!treasury) { + return reply.code(409).send({ + error: + `this deployment has no ${coin} ${network} treasury row, so the account these bytes spend ` + + 'from cannot be identified and there is no way to show they can never be applied. Nothing ' + + 'is refunded on that.', + code: 'withdrawal_unprovable', + }) + } + let verdict: AbandonCheck + try { + verdict = await checkAbandonable({ + coin, + network, + rawTx: row.rawTx, + txid: row.txid, + from: treasury.binding.address, + signedSequence: row.signedSequence, + signedExpiry: row.signedExpiry, + log: request.log, + }) + } catch (err) { + // An unreachable node is the state in which the LEAST is known about where the + // payment got to, so it is a refusal and not a 500 — and it is worth distinguishing, + // because the operator's answer is "try again", not "escalate". + request.log.error( + { err, coin, network, withdrawalId: row.id, txid: row.txid }, + 'abandon: the chain could not be asked whether these signed bytes can still apply', + ) + return reply.code(503).send({ + error: + `the ${coin} node could not be asked whether the signed bytes for this withdrawal can still ` + + 'be applied. Nothing is refunded until it answers — try again.', + code: 'chain_unreachable', + }) + } + if (!verdict.ok) { + request.log.warn( + { + withdrawalId: row.id, + coin, + network, + txid: row.txid, + code: verdict.code, + setBy: request.user!.sub, + }, + 'abandon refused: the signed bytes for this withdrawal can still be applied', + ) + return reply.code(409).send({ error: verdict.error, code: verdict.code }) + } + proof = verdict.proof + } else if (row.txid) { + // A txid with no bytes is a state the state machine cannot produce — `markWithdrawalSigned` + // writes both together — so this is here for a row somebody edited by hand. It gets the + // old check rather than a refusal, because the id alone is still worth asking about. + const status = await outboundStatus(coin, network, row.txid, request.log) if (status.kind === 'confirmed' || status.kind === 'pending') { return reply.code(409).send({ error: `the network still has this payment (${status.kind}) — it cannot be refunded`, code: 'withdrawal_on_chain', }) } + proof = `no bytes are recorded for this withdrawal and the chain reports ${row.txid} as ${status.kind}` } + const refunded = await refundWithdrawal( withdrawal.id, 'this withdrawal was cancelled and returned to your balance', @@ -369,6 +826,9 @@ export async function adminRoutes(app: FastifyInstance) { coin: refunded.coin, amount: refunded.amount, txid: refunded.txid, + // What the chain said that made the refund safe. The one line an audit of a + // double-paid withdrawal would go looking for. + proof, setBy: request.user!.sub, setByHandle: request.user!.handle, }, diff --git a/services/pay/src/routes/deposits.ts b/services/pay/src/routes/deposits.ts index 4159487..165ca10 100644 --- a/services/pay/src/routes/deposits.ts +++ b/services/pay/src/routes/deposits.ts @@ -8,6 +8,7 @@ import { import { requireAuth } from '../auth.js' import { isValidEvmAddress } from '../keccak.js' import { keyvaultCreateAddress } from '../keyvault.js' +import { DEPLOYMENT_NETWORK, networkRefusal } from '../network.js' import { findOrCreateDepositAddress, getDepositPayment, @@ -31,13 +32,21 @@ export async function depositRoutes(app: FastifyInstance) { // GET /deposit-coins — public catalog of watchable coins. app.get('/deposit-coins', async () => SUPPORTED_DEPOSIT_COINS) - // POST /deposits — find-or-create this user's deposit address for {coin, network}. + // POST /deposits — find-or-create this user's deposit address for {coin} on the network + // this deployment settles on. app.post('/deposits', { preHandler: requireAuth }, async (request, reply) => { const parsed = createDepositAddressSchema.safeParse(request.body) if (!parsed.success) { return reply.code(400).send({ error: 'invalid body', code: 'validation' }) } - const { coin, network } = parsed.data + // The raw body, and `parsed.data.network` is deliberately never read: the shared schema + // defaults it to the literal 'testnet', so trusting it mints every address on testnet the + // day an operator sets PAY_DEPOSIT_NETWORK=mainnet — an address on a chain nothing here + // watches, handed to a user as if it worked. See network.ts. + const refusal = networkRefusal(request.body) + if (refusal) return reply.code(400).send(refusal) + const { coin } = parsed.data + const network = DEPLOYMENT_NETWORK const info = depositChainInfo(coin) try { const addr = await findOrCreateDepositAddress( diff --git a/services/pay/src/routes/internal.ts b/services/pay/src/routes/internal.ts index f4a751e..7ed1a4a 100644 --- a/services/pay/src/routes/internal.ts +++ b/services/pay/src/routes/internal.ts @@ -1,7 +1,8 @@ import type { FastifyInstance, FastifyReply } from 'fastify' import { z } from 'zod' import { SUPPORTED_DEPOSIT_COINS, type DepositCoin } from '@cloudsforge/shared' -import { env } from '../env.js' +import { DEPLOYMENT_NETWORK, networkRefusal } from '../network.js' +import { readMoneyLoops } from '../opswatch.js' import { requireServiceToken } from '../serviceAuth.js' import { AmountTooSmallError, @@ -89,6 +90,33 @@ export async function internalRoutes(app: FastifyInstance) { getWallet(request.params.userId), ) + /** + * GET /internal/money-loops — is anything this service does with real money waiting on a + * human? CF-23. + * + * THE ONE THING A MONITOR HAS TO ASSERT ON IS `state`. `'degraded'` means at least one of: + * a withdrawal is `stuck` (signed bytes exist, nothing retries or refunds them on its own), + * a sweep is `stuck` (the deposit address it came out of has stopped crediting its + * customer), or the withdrawal queue has aged past half its refund deadline (usually a + * treasury nobody has funded). `reasons` is prose for whoever gets paged, and `counts` is + * for a dashboard. Nothing here reads a chain, so it is safe to poll. + * + * WHY IT IS BEHIND THE SERVICE TOKEN AND NOT ON `/health`. `/health` is public through the + * tunnel; how many customer withdrawals are stuck and how starved the treasury is are not + * public readings, and the shape of them is a useful thing for somebody timing an attempt + * at the withdrawal queue to know. `/internal` is bound to loopback and 404'd at the tunnel + * (see the header of this file), and Beacon runs inside the same compose network, so the + * probe reaches it the same way every other peer does. + * + * IT IS 200 EITHER WAY, deliberately. A monitor that reads a non-2xx cannot tell "this + * service is unreachable" from "this service is reporting a problem correctly", and those + * two page different people. + */ + app.get('/internal/money-loops', async () => { + const verdict = await readMoneyLoops() + return { ...verdict, service: 'pay', checkedAt: new Date().toISOString() } + }) + // POST /internal/charge — debit a user for work done on their behalf. app.post('/internal/charge', async (request, reply) => { const parsed = movementSchema.safeParse(request.body) @@ -171,7 +199,13 @@ export async function internalRoutes(app: FastifyInstance) { }) } const { userId, coin, side, shards, amount, idempotencyKey } = parsed.data - const network = parsed.data.network ?? env.depositNetwork + // A peer service is trusted with a user's money, not with which chain this deployment + // runs on. Buying a coin on the other network writes a `coin_balances` row `GET /wallet` + // does not read and no withdrawal can now spend, which is a user's Shards turned into + // something they cannot see. See network.ts. + const refusal = networkRefusal(request.body) + if (refusal) return reply.code(400).send({ ...refusal, requestId: request.id }) + const network = DEPLOYMENT_NETWORK if (side === 'buy' && shards === undefined) { return reply diff --git a/services/pay/src/routes/wallet.ts b/services/pay/src/routes/wallet.ts index 396ecd1..682c8bb 100644 --- a/services/pay/src/routes/wallet.ts +++ b/services/pay/src/routes/wallet.ts @@ -7,7 +7,7 @@ import { type DepositCoin, } from '@cloudsforge/shared' import { requireAuth } from '../auth.js' -import { env } from '../env.js' +import { DEPLOYMENT_NETWORK, networkRefusal } from '../network.js' import { AmountTooSmallError, convertCoinToEmber, @@ -119,7 +119,12 @@ export async function walletRoutes(app: FastifyInstance) { code: 'idempotency_key_required', }) } - const network = parsed.data.network ?? env.depositNetwork + // A stated network must be this deployment's; an omitted one means it. Never + // `parsed.data.network`: a balance on the other network is one `GET /wallet` cannot show, + // and letting a caller name it is what makes that balance reachable at all. See network.ts. + const refusal = networkRefusal(request.body) + if (refusal) return reply.code(400).send(refusal) + const network = DEPLOYMENT_NETWORK try { const conversion = await convertCoinToShards({ userId: request.user!.sub, @@ -177,7 +182,9 @@ export async function walletRoutes(app: FastifyInstance) { code: 'idempotency_key_required', }) } - const network = parsed.data.network ?? env.depositNetwork + const refusal = networkRefusal(request.body) + if (refusal) return reply.code(400).send(refusal) + const network = DEPLOYMENT_NETWORK try { const swap = await convertCoinToEmber({ userId: request.user!.sub, diff --git a/services/pay/src/routes/withdrawals.ts b/services/pay/src/routes/withdrawals.ts index 4b312c2..43fcd50 100644 --- a/services/pay/src/routes/withdrawals.ts +++ b/services/pay/src/routes/withdrawals.ts @@ -4,6 +4,7 @@ import { depositChainInfo, SUPPORTED_DEPOSIT_COINS, type DepositCoin } from '@cl import { requireAuth } from '../auth.js' import { formatUnits, parseUnits } from '../amounts.js' import { env } from '../env.js' +import { DEPLOYMENT_NETWORK, networkRefusal } from '../network.js' import { canonicalDestination, estimateFee, @@ -140,7 +141,13 @@ export async function withdrawalRoutes(app: FastifyInstance) { } const { coin } = parsed.data - const network = parsed.data.network ?? env.depositNetwork + // Checked before anything is quoted or debited: a payment on a network this deployment + // does not run would be built from a treasury nobody pinned, signed against the wrong + // chain id, and broadcast to a node that has never heard of the balance it spends. See + // network.ts. + const refusal = networkRefusal(request.body) + if (refusal) return reply.code(400).send(refusal) + const network = DEPLOYMENT_NETWORK const info = depositChainInfo(coin) if (!isValidDestination(coin, parsed.data.destination)) { diff --git a/services/pay/src/store.ts b/services/pay/src/store.ts index c993bb3..5dcb7ec 100644 --- a/services/pay/src/store.ts +++ b/services/pay/src/store.ts @@ -1,6 +1,6 @@ import { createHash, randomUUID } from 'node:crypto' import type { FastifyBaseLogger } from 'fastify' -import { and, desc, eq, gte, inArray, isNull, sql } from 'drizzle-orm' +import { and, desc, eq, gte, inArray, isNotNull, isNull, sql, type SQL } from 'drizzle-orm' import { depositChainInfo, shardsForCoinAmount, @@ -16,7 +16,7 @@ import { type Wallet, } from '@cloudsforge/shared' import { coinForShards, coinForUsd, formatUnits, parseUnits, usdForCoin } from './amounts.js' -import { PROBE_IS_CUMULATIVE } from './chains.js' +import { PROBE_IS_CUMULATIVE, sweepStuckMinutes } from './chains.js' import { client, db } from './db/client.js' import { coinBalances, @@ -856,7 +856,15 @@ export type DepositScan = | { kind: 'credited'; payment: DepositPayment } /** Nothing new at confirmation depth (funds may still be pending at the tip). */ | { kind: 'none' } - /** This exact total was already recorded — high-water mark left untouched. */ + /** + * This exact total, on this exact basis, was already recorded — high-water mark left + * untouched. Nothing in this service is known to produce it: the recorded totals for an + * address are strictly increasing within a basis, and the one thing that used to break that + * (the XRP restatement walking an address back onto an id it had already used) is now + * keyed apart. See `depositPaymentTxid`. It stays a refusal because if it ever does happen + * the payment row and the mark disagree about the same money, and crediting past that is a + * mint. + */ | { kind: 'duplicate' } /** * Confirmed funds went down and a sweep OF OURS accounts for it: the money has left the @@ -996,6 +1004,49 @@ export function currentMarkBasis(coin: DepositCoin): string | null { return MARK_BASIS[coin] ?? null } +/** + * The identity of one credit: `:
::`. + * + * There is no chain transaction id here to use. The probes this service runs read a BALANCE, + * not a transaction list, so a credit is identified by the total the balance reached — which + * works because a payment is only ever inserted when `observed > lastSeen` and `lastSeen` is + * then set to `observed`. Within one basis the recorded totals are therefore strictly + * increasing, so the id is unique, and because it is derived from the total read under the + * address lock, two overlapping ticks cannot mint two ids for the same funds. That is what + * makes the `(address, txid)` unique index the second belt under the high-water mark. + * + * THE BASIS SEGMENT IS LOAD-BEARING AND IS THE WHOLE OF CF-45. "Strictly increasing" holds + * only while the mark is monotonic, and there is exactly one mechanism in this service that + * lowers it: the one-time XRP restatement above, which drops the mark by one base reserve + * without crediting anything (see `MARK_BASIS`). After it fires, the address climbs back + * through totals it has already recorded, so a later `observed` can EQUAL an id already on + * file — the insert conflicts, `recordDepositAndCredit` returns `duplicate`, the deposit is + * not credited and the mark does not advance, and every tick afterwards recomputes the same + * total and refuses again until some other movement pushes the address past the collision. + * Naming the basis in the id makes the pre-rebase and post-rebase totals different things, + * which they are: the same number computed under two different definitions of "confirmed". + * + * Coins whose mark has never been restated are keyed `v0`, so the segment count does not + * depend on the coin — and so that a coin which acquires its FIRST basis later is in the same + * position XRP is in here rather than a new one. Rows written before this change carry the + * old three-segment id; nothing looks them up by shape, and the id of a credit is only ever + * compared for equality with another id built the same way. + * + * The one thing this does NOT do is credit on a `duplicate`. With the basis in the id, a + * conflict can no longer be produced by the restatement, so a conflict once again means only + * what its arm says it means: this exact total, on this exact basis, is already on file while + * the high-water mark sits behind it. That is an inconsistency no arithmetic here can resolve + * and crediting it would be a mint — see `DepositScan`'s `duplicate`. + */ +export function depositPaymentTxid( + coin: string, + address: string, + basis: string | null, + observed: bigint, +): string { + return `${coin}:${address}:${basis ?? 'v0'}:${observed.toString()}` +} + /** * May this address's mark be restated onto the current basis, rather than the drop being * reported as a regression? Every clause is load-bearing — see `MARK_BASIS`. @@ -1023,11 +1074,20 @@ export function markNeedsRebase( * reports `regression`. So a `settling` address still explained by a sweep this old is proof * that nothing is advancing sweeps at all, which is the state that otherwise reports at info * level forever while the address never credits another deposit. + * + * It takes the COIN now because that deadline does: `chains.sweepStuckMinutes` derives it + * from the depth the sweep is waiting for and the rate its chain produces blocks at, where it + * used to be one flat hour borrowed from the withdrawal worker. The argument above only holds + * while this is the same number the sweeper uses, so it must be read from the same place. */ -export function settlingStalledMinutes(oldestUpdatedAt: Date | null, now = Date.now()): number | null { +export function settlingStalledMinutes( + coin: DepositCoin, + oldestUpdatedAt: Date | null, + now = Date.now(), +): number | null { if (!oldestUpdatedAt) return null const minutes = Math.floor((now - oldestUpdatedAt.getTime()) / 60_000) - return minutes > Math.max(1, env.withdrawalStuckMinutes) ? minutes : null + return minutes > Math.max(1, sweepStuckMinutes(coin)) ? minutes : null } /** @@ -1038,8 +1098,19 @@ export function settlingStalledMinutes(oldestUpdatedAt: Date | null, now = Date. * are still sitting in the address. `stuck` is excluded deliberately and it is the harder * call: a stuck sweep may or may not have moved anything, and letting it go on explaining a * shortfall forever would turn "an operator must look at this" into permanent silence. So a - * stuck sweep stops explaining, the watcher reports `regression`, and the operator closes it - * with `POST /admin/deposit-addresses/:id/sweeps` — which is exactly what that route is for. + * stuck sweep stops explaining and the watcher reports `regression`. + * + * THAT IS ONLY DEFENSIBLE BECAUSE THERE IS NOW A WAY OUT OF `stuck`, and until CF-07 there + * was not. + * Leaving the explaining set is a decision to freeze the address until something records the + * movement, and the two things that could were both no-ops: `activeSweeps` skipped the row so + * nothing ever matured it, and `POST /admin/deposit-addresses/:id/sweeps` — the remedy the + * watcher's own log line names — inserted a row with the same `(address_id, txid)`, hit the + * unique index, and answered `replayed: true` having changed nothing. There was no route and + * no worker that could move a sweep out of `stuck`; only a hand-written UPDATE. Both are + * closed now: `sweeper.recoverStuckSweep` matures a stuck sweep whose transaction reaches its + * depth after the deadline, and `recordManualSweep` reconciles against the conflicting row + * instead of reporting a replay. */ const UNMATURED_SWEEP_STATUSES = ['signed', 'broadcast', 'confirmed'] @@ -1136,7 +1207,10 @@ export async function recordDepositAndCredit( ? { kind: 'settling', sweeping: missing, - stalledMinutes: settlingStalledMinutes(swept.oldestUpdatedAt ?? null), + stalledMinutes: settlingStalledMinutes( + row.coin as DepositCoin, + swept.oldestUpdatedAt ?? null, + ), } : { kind: 'regression', missing } } @@ -1157,9 +1231,11 @@ export async function recordDepositAndCredit( } const amount = formatUnits(delta, info.decimals) - // One synthetic txid per distinct confirmed total — derived from the LOCKED total, so - // two ticks can never mint two different txids for the same funds. - const txid = `${row.coin}:${row.address}:${observed.toString()}` + // One synthetic txid per distinct confirmed total ON THIS BASIS — derived from the LOCKED + // total, so two ticks can never mint two different txids for the same funds, and naming + // the basis so that the one mechanism that lowers the mark cannot walk the address back + // onto an id it has already used. See `depositPaymentTxid`. + const txid = depositPaymentTxid(row.coin, row.address, basis, observed) const inserted = await tx .insert(depositPayments) .values({ @@ -1201,10 +1277,34 @@ export async function recordDepositAndCredit( }) } +/** + * What recording an operator's hand movement did — see `recordManualSweep`. + * + * `recorded` is the amount that reached `deposit_addresses.swept`, and it is stated + * separately from the amount posted because the reconcile arm does not use the posted one: + * it records the amount on the row this service already holds for that transaction. + */ +export type ManualSweepOutcome = + /** A new row, its amount added to `swept`. The ordinary case: an operator moved funds. */ + | { kind: 'recorded'; swept: string; recorded: bigint } + /** This (address, txid) was on record AND already counted. Nothing changed. */ + | { kind: 'replayed'; swept: string; recorded: bigint } + /** + * A sweep of THIS service's for this transaction existed and had never been counted — + * `stuck` below its maturity depth, or in flight — so it was matured in place rather than + * duplicated. This is what un-freezes an address a stuck sweep stopped crediting. + */ + | { kind: 'reconciled'; swept: string; recorded: bigint; sweepId: string; wasStatus: string } + /** + * A row exists for this transaction and says a different amount left the address. Nothing + * is recorded: one of the two numbers is wrong and no arithmetic here can say which. + */ + | { kind: 'disagrees'; sweepId: string; rowAmount: bigint } + /** * Record a movement an OPERATOR made by hand, as a sweep row that is already matured. * - * Two things the bare `recordSweep` it replaces did not do. + * Three things the bare `recordSweep` it replaces did not do. * * It is idempotent. `recordSweepIn` is an unguarded addition, so a POST that timed out and * was retried added the amount twice — and an overstated `swept` credits the user the @@ -1216,6 +1316,27 @@ export async function recordDepositAndCredit( * much has left and never when, in which transaction, or on whose authority. An operator * moving custody funds by hand is the least reversible thing anyone does to this service and * it should be reconstructible from the database alone, not from a log line. + * + * AND IT RECONCILES AGAINST THE CONFLICTING ROW RATHER THAN REPORTING A REPLAY (CF-07). The + * index does not only catch a retry of this route: it also catches the case the route exists + * for. A sweep of this service's own that was marked `stuck` — mined, but below its maturity + * depth for longer than the chain can explain — holds the same `(address_id, txid)`, has + * never been added to `swept`, and has left the set of sweeps that explain the address's + * shortfall, so the watcher reports `regression` and credits that address nothing further. + * The operator follows the log line to this route, posts the real transaction and the real + * amount, and used to get 200 `{ replayed: true }` with `swept` unchanged and the address + * still frozen — the documented remedy was a silent no-op against the one state it was + * documented for. So an UNRECORDED conflicting row is matured here instead, in this + * transaction, exactly as `matureSweep` would have. + * + * It records the amount on THAT ROW and refuses if the posted amount differs. The row's + * amount is what this service actually signed and broadcast out of the address (`value + fee` + * for the transaction it built), so a disagreement means either the operator is describing a + * different movement under the same transaction id or the row is wrong about what it sent. + * Neither is reconcilable by arithmetic, and recording either number would put a figure into + * `swept` that no transaction accounts for — which is CF-08's mint, arrived at from the other + * side. The route's own chain check has already compared the posted amount against what the + * transaction moved, so in practice the two agree or the POST never reached here. */ export async function recordManualSweep(input: { address: DepositAddressRow @@ -1223,11 +1344,16 @@ export async function recordManualSweep(input: { txid: string /** Everything that left the address, fee included. */ amountSmallest: bigint - /** Where it went, if the operator said. Not verified — this service did not send it. */ + /** + * Where it went. The CHAIN's answer wherever the route could get one — it verifies the + * destination before it gets here (it must not be this address, and must not be another + * customer's) — falling back to the operator's free-text note on the one path that asks + * the chain nothing, which is Bitcoin, where recording moves no total at all. + */ destination: string /** The block or ledger index the movement was mined in, when the chain would say. */ minedHeight: bigint | null -}): Promise<{ swept: string; replayed: boolean }> { +}): Promise { if (input.amountSmallest <= 0n) throw new Error('sweep amount must be positive') return db.transaction(async (tx) => { const [row] = await tx @@ -1250,16 +1376,59 @@ export async function recordManualSweep(input: { }) .onConflictDoNothing() .returning() - if (!row) { - // This (address, txid) is already recorded. The conflicting insert blocked until the - // first one committed, so the total read here is the one it wrote. - const [existing] = await tx + if (row) { + return { + kind: 'recorded', + swept: await recordSweepIn(tx, input.address.id, input.amountSmallest), + recorded: input.amountSmallest, + } + } + + // Something already holds this (address, txid). Locked before it is read, and read in + // full: whether it was ever counted is the whole question, and a concurrent POST of the + // same transaction must not be able to answer it twice. The conflicting insert above + // already blocked until any competing transaction committed. + const [existing] = await tx + .select() + .from(sweeps) + .where(and(eq(sweeps.addressId, input.address.id), eq(sweeps.txid, input.txid))) + .for('update') + if (!existing) { + // The insert conflicted on an index that is not `sweeps_address_txid_idx`. Nothing else + // on this table can catch a `matured` row today — the only other unique index is the + // partial one over the unfinished statuses — so this is a schema change nobody taught + // this function about, and guessing at it would be guessing with somebody's deposit. + throw new Error(`sweep insert for ${input.txid} conflicted with a row that cannot be found`) + } + const rowAmount = BigInt(existing.amount) + if (existing.recordedAt) { + const [address] = await tx .select({ swept: depositAddresses.swept }) .from(depositAddresses) .where(eq(depositAddresses.id, input.address.id)) - return { swept: existing?.swept ?? '0', replayed: true } + return { kind: 'replayed', swept: address?.swept ?? '0', recorded: rowAmount } + } + if (rowAmount !== input.amountSmallest) { + return { kind: 'disagrees', sweepId: existing.id, rowAmount } + } + const swept = await recordSweepIn(tx, input.address.id, rowAmount) + await tx + .update(sweeps) + .set({ + status: 'matured', + recordedAt: new Date(), + updatedAt: new Date(), + // `failure_reason` is left exactly as it was. It is the only record of WHY this sweep + // needed an operator, and a reconciled sweep is still one that got stuck. + }) + .where(eq(sweeps.id, existing.id)) + return { + kind: 'reconciled', + swept, + recorded: rowAmount, + sweepId: existing.id, + wasStatus: existing.status, } - return { swept: await recordSweepIn(tx, input.address.id, input.amountSmallest), replayed: false } }) } @@ -1312,6 +1481,238 @@ export async function getDepositAddressById(id: string): Promise { + const [row] = await db + .select({ + id: depositAddresses.id, + userId: depositAddresses.userId, + coin: depositAddresses.coin, + address: depositAddresses.address, + }) + .from(depositAddresses) + .where(matchesDepositAddress(address)) + .limit(1) + return row ?? null +} + +/** One deposit address as an operator needs to see it: the row, and what is in flight on it. */ +export interface DepositAddressOverview { + row: DepositAddressRow + /** Sweeps of ours that have never been added to `swept`, oldest first. Usually empty. */ + sweeps: SweepRow[] + /** + * `swept` plus the subset of the above the arithmetic counts, in the shape `observeDeposit` + * reads. Built here, from the same statuses and the same `recorded_at IS NULL` rule + * `unmaturedSweptIn` applies under the address lock, so a read-only scan cannot disagree + * with the watcher about what a sweep explains. + */ + unmatured: SweptTotals +} + +/** + * Operator view: deposit addresses, newest first, with their unaccounted sweeps. + * + * TWO QUERIES FOR THE WHOLE PAGE, not one per address. The sweeps are fetched for the page's + * ids in a single `IN` — an address has at most one unfinished sweep, so the second result set + * is smaller than the first and usually empty. + * + * Nothing here probes a chain: this is the database's account of these addresses, which is the + * half that is free. Deciding whether an address is FROZEN needs a live balance, and that is + * the caller's choice to pay for — see `scanDepositAddress` and `GET /admin/deposit-addresses`. + * + * `address` matches by `matchesDepositAddress` — case-insensitively for `0x…`, exactly for + * base58 and bech32 — because an operator arriving here has usually pasted the address out of + * a block explorer, which renders EVM addresses lowercase, and an exact match would answer + * "no such address" for the one address the alarm is about. + * + * `offset` pages it. Without one, a deployment with more deposit addresses than `limit` + * (200 at most, and 25 when scanning) could not enumerate them at all — the newest page was + * the only page — so "list the frozen addresses" was unanswerable at any real size. The order + * is total (`created_at DESC, id DESC`), so the pages do not overlap or skip. + */ +export async function depositAddressOverview(filter: { + coin?: string + address?: string + userId?: string + limit?: number + /** How many rows to skip, for a deployment with more addresses than one page holds. */ + offset?: number +}): Promise { + const where = [ + ...(filter.coin ? [eq(depositAddresses.coin, filter.coin)] : []), + ...(filter.address ? [matchesDepositAddress(filter.address)] : []), + ...(filter.userId ? [eq(depositAddresses.userId, filter.userId)] : []), + ] + const query = db.select().from(depositAddresses).$dynamic() + const rows = await (where.length ? query.where(and(...where)) : query) + // `created_at` alone is not a total order — two addresses minted in the same millisecond + // would page unstably, silently skipping one row and repeating another — so the primary + // key breaks the tie. It only matters because this list is paged; it costs nothing. + .orderBy(desc(depositAddresses.createdAt), desc(depositAddresses.id)) + .limit(Math.max(1, filter.limit ?? 50)) + .offset(Math.max(0, filter.offset ?? 0)) + if (rows.length === 0) return [] + + const open = await db + .select() + .from(sweeps) + .where( + and( + inArray( + sweeps.addressId, + rows.map((r) => r.id), + ), + isNull(sweeps.recordedAt), + inArray(sweeps.status, UNACCOUNTED_SWEEP_STATUSES), + ), + ) + .orderBy(sweeps.createdAt) + + const byAddress = new Map() + for (const sweep of open) { + const list = byAddress.get(sweep.addressId) + if (list) list.push(sweep) + else byAddress.set(sweep.addressId, [sweep]) + } + return rows.map((row) => { + const mine = byAddress.get(row.id) ?? [] + let inFlight = 0n + let inFlightMined = 0n + let oldestUpdatedAt: Date | null = null + for (const sweep of mine) { + // The arithmetic's set, not the listing's: a `stuck` sweep is shown above and explains + // nothing here. See UNMATURED_SWEEP_STATUSES for why that is the harder half. + if (!UNMATURED_SWEEP_STATUSES.includes(sweep.status)) continue + const amount = BigInt(sweep.amount) + inFlight += amount + if (sweep.minedHeight !== null) inFlightMined += amount + if (!oldestUpdatedAt || sweep.updatedAt < oldestUpdatedAt) oldestUpdatedAt = sweep.updatedAt + } + return { + row, + sweeps: mine, + unmatured: { recorded: BigInt(row.swept || '0'), inFlight, inFlightMined, oldestUpdatedAt }, + } + }) +} + +/** + * What the watcher's next tick would make of this address — decided without writing anything. + * + * The read-only twin of `recordDepositAndCredit`, and it exists because "this address is + * frozen" is not a fact any table holds. `regression` is a comparison between a high-water + * mark and a balance read from a chain at confirmation depth, computed every tick and then + * thrown away; the only trace it leaves is a log line. So the operator surface that lists + * frozen addresses has to recompute it, and this is that computation. + * + * IT SHARES THE PARTS THAT DECIDE, not just the shape: `markNeedsRebase`, `observeDeposit` and + * `settlingStalledMinutes` are the same functions the credit path calls, in the same order, + * and the only thing restated here is which arm each of their answers lands in. What it cannot + * predict is `duplicate` — that arm is decided by an INSERT against `deposit_payments`, and + * asking a read to find out would mean writing. An address in that state reads as `uncredited` + * here, which is what it looks like from the outside: a delta the watcher has not credited. + */ +export type DepositAddressState = + /** Nothing owing: the mark, the chain and the sweeps all agree. */ + | { state: 'ok' } + /** New confirmed funds the next tick will credit. Not an alarm — a tick has not run yet. */ + | { state: 'uncredited'; amount: string } + /** A sweep of ours accounts for the shortfall; crediting resumes when it matures. */ + | { state: 'settling'; sweeping: string; stalledMinutes: number | null } + /** FROZEN. Funds left with nothing accounting for them — `POST .../sweeps` is the remedy. */ + | { state: 'regression'; missing: string } + /** The one-time restatement is owing; the next tick will do it and credit nothing. */ + | { state: 'rebasing'; from: string; to: string } + +export function scanDepositAddress(input: { + coin: DepositCoin + markBasis: string | null + lastSeen: bigint + probe: ProbeTotals + swept: SweptTotals + now?: number +}): DepositAddressState { + const info = depositChainInfo(input.coin) + const { observed, delta, shortfall, explained } = observeDeposit( + input.probe, + input.lastSeen, + input.swept, + ) + if (markNeedsRebase(input.coin, input.markBasis, input.lastSeen, observed)) { + return { + state: 'rebasing', + from: formatUnits(input.lastSeen, info.decimals), + to: formatUnits(observed, info.decimals), + } + } + if (shortfall > 0n) { + const missing = formatUnits(shortfall, info.decimals) + return explained + ? { + state: 'settling', + sweeping: missing, + stalledMinutes: settlingStalledMinutes( + input.coin, + input.swept.oldestUpdatedAt ?? null, + input.now, + ), + } + : { state: 'regression', missing } + } + if (delta > 0n) return { state: 'uncredited', amount: formatUnits(delta, info.decimals) } + return { state: 'ok' } +} + // --------------------------------------------------------------------------- // Withdrawals — the only path by which custodied coin leaves this platform // @@ -1319,7 +1720,12 @@ export async function getDepositAddressById(id: string): Promise return row ? toWithdrawal(row) : null } +/** + * The whole row, signed bytes included — deliberately separate from `getWithdrawal`. + * + * `WithdrawalView` is the shape a USER is allowed to see and `raw_tx` is not on it, so it is + * not enough for the one caller that has to reason about the signature itself: abandoning a + * withdrawal refunds a balance while those bytes are still valid, and the only safe version + * of that decision is made against the bytes (`POST /admin/withdrawals/:id/abandon`, CF-06). + * Keeping it a second function means the view cannot grow a `raw_tx` field by accident. + */ +export async function getWithdrawalRow(id: string): Promise { + const [row] = await db.select().from(withdrawals).where(eq(withdrawals.id, id)) + return row ?? null +} + /** Operator view: every user's withdrawals, optionally narrowed to one status. */ export async function listAllWithdrawals( status?: WithdrawalStatus, @@ -1569,12 +1989,22 @@ export async function pendingWithdrawalTotal(coin: string, network: string): Pro */ export async function markWithdrawalSigned( id: string, - rawTx: string, - txid: string | null, + signed: { rawTx: string; txid: string | null; sequence: string | null; expiry: string | null }, ): Promise { const rows = await db .update(withdrawals) - .set({ status: 'signed', rawTx, txid, updatedAt: new Date() }) + // `signed_sequence` and `signed_expiry` are set in the SAME statement as the bytes, which + // is what makes them usable as evidence: there is no window in which a row holds a + // signature whose Sequence and LastLedgerSequence this service has forgotten. See + // schema.ts and outbound.ts's abandon section for what they are for. + .set({ + status: 'signed', + rawTx: signed.rawTx, + txid: signed.txid, + signedSequence: signed.sequence, + signedExpiry: signed.expiry, + updatedAt: new Date(), + }) .where(and(eq(withdrawals.id, id), eq(withdrawals.status, 'pending'))) .returning({ id: withdrawals.id }) return rows.length > 0 @@ -1783,13 +2213,21 @@ export async function markSweepConfirmed(id: string): Promise { * checked `chains.sweepMaturityConfirmations` first. `recorded_at IS NULL` is what stops a * retry adding the amount twice; the status guard stops a sweep the operator has already * reconciled by hand being counted again on top. + * + * `stuck` IS INSIDE THE GUARD (CF-07) and it is the point of the guard changing. Stuck is not + * a statement that the money stayed put — it is a statement that the sweep had not reached + * its depth by the time the deadline said a healthy chain would have got it there. A chain + * that then catches up moves those funds out of the probe's view exactly as any other sweep + * does, and if this refused them the amount could never reach `swept`, so the address would + * report `regression` and credit nobody for as long as it existed. What the status guard is + * actually for is `matured` and `failed`, which are terminal, and it still excludes both. */ export async function matureSweep(id: string): Promise { return db.transaction(async (tx) => { const [row] = await tx .select() .from(sweeps) - .where(and(eq(sweeps.id, id), inArray(sweeps.status, ['broadcast', 'confirmed']))) + .where(and(eq(sweeps.id, id), inArray(sweeps.status, ['broadcast', 'confirmed', 'stuck']))) .for('update') if (!row || row.recordedAt) return false await recordSweepIn(tx, row.addressId, BigInt(row.amount)) @@ -1813,6 +2251,117 @@ export async function failSweep(id: string, reason: string): Promise { .where(and(eq(sweeps.id, id), eq(sweeps.status, 'pending'))) } +/** + * Stuck sweeps the chain has already told us where to find, newest work last. + * + * WHY THIS QUERY EXISTS AT ALL. `activeSweeps` deliberately does not return `stuck` rows — + * stuck means a human has been asked to look, and a worker that kept driving one would be + * asking and answering its own alarm. But the commonest way to become stuck is the least + * interesting one: a sweep that was mined on time and then waited out its deadline below the + * maturity depth because the chain slowed down. Nothing about that needs a human, and until + * CF-07 nothing else could ever record it, so a slow hour froze the depositor's address for + * good. + * + * WHY THE TWO NOT-NULLS ARE THE WHOLE FILTER, and they are chosen to be narrow rather than + * convenient. `mined_height` is set only from a node's own receipt (`markSweepMined`), so a + * row that has one is one the chain has SEEN — which excludes both of the stuck cases a + * re-check could not help: a sweep the network rejected (never mined, funds never left) and a + * sweep no node ever admitted to (nothing to look up). Neither is polled again by this, so a + * genuinely broken sweep costs no RPC per tick for the life of the deployment. Those are the + * operator's, through `POST /admin/deposit-addresses/:id/sweeps`, which reconciles them. + */ +export async function recoverableStuckSweeps(): Promise { + return db + .select() + .from(sweeps) + .where( + and( + eq(sweeps.status, 'stuck'), + isNull(sweeps.recordedAt), + isNotNull(sweeps.txid), + isNotNull(sweeps.minedHeight), + ), + ) + .orderBy(sweeps.createdAt) +} + +/** + * The state of the two loops that move real money, in numbers a monitor can assert on. + * + * CF-23. `markWithdrawalStuck` and `markSweepStuck` are, by the workers' own comments, "a + * request for a human" — and nothing in this estate carries that request TO one. Beacon's pay + * probes cover `/health`, `/coins/rates` and `/deposit-coins`; none of them touches a sweep, a + * treasury or a withdrawal, so a signed-and-broadcast withdrawal that has no automatic exit + * left can sit `stuck` indefinitely with every probe green. The operator surfaces that DO + * exist (`GET /admin/withdrawals?status=stuck`, `GET /admin/treasury`) are all pull: a human + * has to already suspect the problem before they help. + * + * This is the reading a push can be built on, and it is deliberately four cheap aggregates + * over two indexed columns rather than anything that touches a chain — it is polled, by + * definition, by something that must not become the load that breaks what it watches. + * + * `queuedOldestMinutes` is here alongside the stuck counts because it is the EARLY half of + * the same story: an unfunded treasury does not produce a stuck withdrawal, it produces + * `pending` rows that wait out `PAY_WITHDRAWAL_STUCK_MINUTES` and are then refunded. By the + * time those refunds show up the money never moved and the user has been told so; the queue + * ageing is the part that could still be acted on. + */ +export interface MoneyLoopCounts { + /** Signed or broadcast withdrawals parked for a human. Never auto-refunded. */ + withdrawalsStuck: number + /** Age of the oldest of them, in whole minutes. Null when there are none. */ + withdrawalsStuckOldestMinutes: number | null + /** Withdrawals accepted and not yet signed — the shape treasury starvation takes. */ + withdrawalsQueued: number + queuedOldestMinutes: number | null + /** Sweeps parked for a human. Each one freezes the deposit address it came out of. */ + sweepsStuck: number + sweepsStuckOldestMinutes: number | null + /** Deposit addresses frozen by one of those sweeps — customers not being credited. */ + addressesFrozenBySweep: number +} + +export async function moneyLoopCounts(now = new Date()): Promise { + // An aggregate comes back as whatever the driver makes of the expression's type, and for + // `min(timestamptz)` that is the ISO string rather than the Date a column select yields. + // Parsed here rather than trusted, because the difference is a 500 on the one route whose + // job is to be readable when everything else is broken. + const minutesSince = (at: unknown): number | null => { + const at2 = at instanceof Date ? at : typeof at === 'string' ? new Date(at) : null + if (!at2 || Number.isNaN(at2.getTime())) return null + return Math.max(0, Math.floor((now.getTime() - at2.getTime()) / 60_000)) + } + + const [stuckWithdrawals] = await db + .select({ n: sql`count(*)::int`, oldest: sql`min(${withdrawals.updatedAt})` }) + .from(withdrawals) + .where(eq(withdrawals.status, 'stuck')) + const [queued] = await db + .select({ n: sql`count(*)::int`, oldest: sql`min(${withdrawals.createdAt})` }) + .from(withdrawals) + .where(eq(withdrawals.status, 'pending')) + const [stuckSweeps] = await db + .select({ + n: sql`count(*)::int`, + oldest: sql`min(${sweeps.updatedAt})`, + // Distinct because one address can hold more than one stuck sweep, and the number that + // matters to a person is how many CUSTOMERS have stopped being credited. + addresses: sql`count(distinct ${sweeps.addressId})::int`, + }) + .from(sweeps) + .where(and(eq(sweeps.status, 'stuck'), isNull(sweeps.recordedAt))) + + return { + withdrawalsStuck: stuckWithdrawals?.n ?? 0, + withdrawalsStuckOldestMinutes: minutesSince(stuckWithdrawals?.oldest), + withdrawalsQueued: queued?.n ?? 0, + queuedOldestMinutes: minutesSince(queued?.oldest), + sweepsStuck: stuckSweeps?.n ?? 0, + sweepsStuckOldestMinutes: minutesSince(stuckSweeps?.oldest), + addressesFrozenBySweep: stuckSweeps?.addresses ?? 0, + } +} + /** Park a signed sweep for an operator, exactly as a stuck withdrawal is parked. */ export async function markSweepStuck(id: string, reason: string): Promise { await db diff --git a/services/pay/src/sweep.test.ts b/services/pay/src/sweep.test.ts index a8f28bc..64d1b9d 100644 --- a/services/pay/src/sweep.test.ts +++ b/services/pay/src/sweep.test.ts @@ -1,12 +1,40 @@ import assert from 'node:assert/strict' import { test } from 'node:test' -import { depositChainInfo } from '@cloudsforge/shared' -import { sweepIsMature, sweepMaturityConfirmations } from './chains.js' -import { env } from './env.js' -import { BLOCK_LOG_MS, BLOCK_RETRY_MS, ChainBlocks, sweepRefusalHandling } from './sweeper.js' -import { observeDeposit, settlingStalledMinutes, type SweptTotals } from './store.js' +import { depositChainInfo, SUPPORTED_DEPOSIT_COINS, type DepositCoin } from '@cloudsforge/shared' +import { + SWEEP_STUCK_FLOOR_MINUTES, + SWEEP_STUCK_SLACK, + sweepIsMature, + sweepMaturityConfirmations, + sweepStuckMinutes, +} from './chains.js' +import { BLOCK_LOG_MS, BLOCK_RETRY_MS, ChainBlocks, sweepAlarmFields, sweepRefusalHandling } from './sweeper.js' +import type { SweepRow } from './db/schema.js' +import { + currentMarkBasis, + depositPaymentTxid, + observeDeposit, + settlingStalledMinutes, + type SweptTotals, +} from './store.js' import { PIN_TTL_MS, PinCache, planTreasury, treasuryBinding } from './treasury.js' +/** + * How long each chain takes to carry a sweep to `sweepMaturityConfirmations` when it is + * running normally — the depth, times the cadence the chain publishes. + * + * Restated here rather than imported so that the deadline and the thing it is a deadline FOR + * are not derived from the same line of code: `chains.BLOCK_SECONDS` changing under a test + * that read it would move the assertion along with the answer and prove nothing. + */ +const NOMINAL_MATURITY_SECONDS: Record = { + EMBER: 61 * 15, + ETH: 13 * 12, + BTC: 1 * 600, + SOL: 1 * 13, + XRP: 1 * 4, +} + /** * The deposit/sweep accounting, simulated against a chain whose height actually moves. * @@ -36,6 +64,12 @@ interface Sweep { /** Null until the chain reports a receipt. */ minedHeight: number | null matured: boolean + /** + * True once the sweeper's deadline passed with the sweep still short of maturity depth — + * `sweeper.advanceSweep` calling `markSweepStuck`. A stuck sweep stops explaining the + * address's shortfall, which is what makes the watcher report `regression`. + */ + stuck: boolean } /** @@ -48,7 +82,10 @@ interface Sweep { * not cover this one. All the address has is the moment the operator's POST lands. */ interface HandSweep { - amount: bigint + /** What the transaction ACTUALLY took out of the address, as the chain reports it. */ + moved: bigint + /** What the operator typed into the route. Equal to `moved` unless they mistyped it. */ + posted: bigint /** The block the operator's own transaction was mined in. */ minedHeight: number recorded: boolean @@ -60,6 +97,17 @@ interface Outcome { regressions: number settlingTicks: number duplicates: number + /** Manual sweeps the route refused because the transaction did not account for them. */ + handRefusals: number + /** Sweeps the sweeper gave up on and asked a human to look at. */ + stuckSweeps: number + /** + * POSTs to `/admin/deposit-addresses/:id/sweeps` that answered 200 `{ replayed: true }` and + * changed nothing, because the txid the operator posted was already on a `sweeps` row. + */ + replayedNoOps: number + /** POSTs that matured this service's own unrecorded sweep of that transaction in place. */ + reconciles: number } class Sim { @@ -70,11 +118,27 @@ class Sim { private handSweeps: HandSweep[] = [] /** The gate the admin route applies before it will record a hand-moved amount. */ private handGate: (confirmations: number) => boolean = () => true + /** + * Whether the route checks the posted amount against the transaction, which is + * `outbound.checkSweepMovement`. False is the route as it shipped: the amount was taken on + * trust and only its DEPTH was ever verified. + */ + private verifyAmount = true /** deposit_addresses.last_seen */ lastSeen = 0n /** deposit_addresses.swept */ sweptRecorded = 0n - out: Outcome = { credited: 0n, credits: 0, regressions: 0, settlingTicks: 0, duplicates: 0 } + out: Outcome = { + credited: 0n, + credits: 0, + regressions: 0, + settlingTicks: 0, + duplicates: 0, + handRefusals: 0, + stuckSweeps: 0, + replayedNoOps: 0, + reconciles: 0, + } /** Synthetic txids already minted, which is the (address, txid) unique index. */ private txids = new Set() @@ -105,21 +169,107 @@ class Sim { /** A sweep of the whole balance: broadcast at `height`, mined in the next block. */ sweepAt(height: number, amount: bigint): this { - this.sweeps.push({ amount, broadcastAt: height, minedHeight: null, matured: false }) + this.sweeps.push({ amount, broadcastAt: height, minedHeight: null, matured: false, stuck: false }) return this.at(height + 1, -amount) } /** - * An operator moves the whole balance out by hand at `height`, and posts to + * The sweeper's stuck deadline, expressed in blocks because blocks are this file's clock. + * + * A wall-clock deadline and a depth are only comparable through the chain's RATE, and the + * whole of CF-07's scenario is a chain whose rate has collapsed: on a healthy chain 61 + * blocks take fifteen minutes and the deadline is 61 minutes, so it is never reached. Set + * this below the maturity depth to model the chain that has slowed down enough that it is. + */ + private stuckAfterBlocks: number | null = null + /** `sweeper.recoverStuckSweep` — false is the sweeper before CF-07, which had no such pass. */ + private recoversStuck = true + /** `store.recordManualSweep`'s reconcile arm — false is the route before CF-07. */ + private reconcilesManualSweep = true + + stuckAfter(blocks: number): this { + this.stuckAfterBlocks = blocks + return this + } + + withoutStuckRecovery(): this { + this.recoversStuck = false + return this + } + + withoutManualReconcile(): this { + this.reconcilesManualSweep = false + return this + } + + /** + * The operator does what the watcher's `regression` line tells them to: POST the stuck + * sweep's OWN transaction id and amount to `/admin/deposit-addresses/:id/sweeps`. + * + * That id is already on a `sweeps` row — this service broadcast it — so the insert hits the + * `(address_id, txid)` unique index and the route takes one of its two conflict paths. + */ + private manualPosts: { height: number; sweep: number; done: boolean }[] = [] + + recordStuckSweepAt(height: number, sweep = 0): this { + this.manualPosts.push({ height, sweep, done: false }) + return this + } + + /** POST /admin/deposit-addresses/:id/sweeps, against a txid this service already holds. */ + private manualPostTick(): void { + for (const post of this.manualPosts) { + if (post.done || this.head < post.height) continue + const sweep = this.sweeps[post.sweep]! + post.done = true + // The route's depth gate, which is the same one the sweeper matures on. + const confirmations = sweep.minedHeight === null ? 0 : this.head - sweep.minedHeight + 1 + if (!sweepIsMature(COIN, confirmations)) { + this.out.handRefusals += 1 + post.done = false + continue + } + if (sweep.matured) { + // Already counted. A genuine replay, and correctly a no-op. + this.out.replayedNoOps += 1 + continue + } + if (!this.reconcilesManualSweep) { + // The route as it shipped: `onConflictDoNothing` inserted nothing, `recordSweepIn` was + // never reached, and the reply was 200 `{ replayed: true }` with `swept` unchanged. + this.out.replayedNoOps += 1 + continue + } + sweep.matured = true + sweep.stuck = false + this.sweptRecorded += sweep.amount + this.out.reconciles += 1 + } + } + + /** + * An operator moves `moved` out by hand at `height`, and posts `posted` to * `POST /admin/deposit-addresses/:id/sweeps` at the first tick `gate` allows. * - * `gate` is the only thing under test: `() => true` is the route as it shipped, which - * records the instant the operator asks. + * `gate` is the depth rule: `() => true` is the route before the maturity fix, which + * records the instant the operator asks. `posted` defaults to what actually moved — pass + * a different number to model the operator typing one, which is CF-08. */ - handSweepAt(height: number, amount: bigint, gate: (confirmations: number) => boolean): this { + handSweepAt( + height: number, + moved: bigint, + gate: (confirmations: number) => boolean, + posted: bigint = moved, + ): this { this.handGate = gate - this.handSweeps.push({ amount, minedHeight: height, recorded: false }) - return this.at(height, -amount) + this.handSweeps.push({ moved, posted, minedHeight: height, recorded: false }) + return this.at(height, -moved) + } + + /** The route as it shipped: the amount beside the txid was never read off the chain. */ + skipAmountCheck(): this { + this.verifyAmount = false + return this } /** The admin route, retried every tick until its gate lets the record through. */ @@ -127,8 +277,14 @@ class Sim { for (const hand of this.handSweeps) { if (hand.recorded || this.head < hand.minedHeight) continue if (!this.handGate(this.head - hand.minedHeight + 1)) continue + // `outbound.checkSweepMovement` in one line: the chain says exactly what left the + // address, and an amount that is not that number is not recorded at all. + if (this.verifyAmount && hand.posted !== hand.moved) { + this.out.handRefusals += 1 + continue + } hand.recorded = true - this.sweptRecorded += hand.amount + this.sweptRecorded += hand.posted } } @@ -147,10 +303,21 @@ class Sim { if (this.head > sweep.broadcastAt) sweep.minedHeight = sweep.broadcastAt + 1 else continue } + // A stuck sweep is not in `activeSweeps`, so `advanceSweep` never sees it again. Only + // `recoverStuckSweep` can, and only because the chain told us where it was mined. + if (sweep.stuck && !this.recoversStuck) continue const confirmations = this.head - sweep.minedHeight + 1 if (confirmations >= this.maturityConfirmations) { sweep.matured = true + sweep.stuck = false this.sweptRecorded += sweep.amount + continue + } + // `advanceSweep` returns before this whenever the sweep matured, so a sweep can only be + // marked stuck while it is still short of the depth its accounting needs. + if (this.stuckAfterBlocks !== null && !sweep.stuck && this.head - sweep.broadcastAt > this.stuckAfterBlocks) { + sweep.stuck = true + this.out.stuckSweeps += 1 } } } @@ -160,7 +327,9 @@ class Sim { let inFlight = 0n let inFlightMined = 0n for (const sweep of this.sweeps) { - if (sweep.matured || this.head < sweep.broadcastAt) continue + // `stuck` is not in UNMATURED_SWEEP_STATUSES: a stuck sweep stops explaining the + // address's shortfall, on purpose, so that the watcher stops calling it routine. + if (sweep.matured || sweep.stuck || this.head < sweep.broadcastAt) continue inFlight += sweep.amount if (sweep.minedHeight !== null && sweep.minedHeight <= this.head) inFlightMined += sweep.amount } @@ -184,7 +353,7 @@ class Sim { return } if (seen.delta === 0n) return - const txid = `${COIN}:addr:${seen.observed}` + const txid = depositPaymentTxid(COIN, 'addr', currentMarkBasis(COIN), seen.observed) if (this.txids.has(txid)) { this.out.duplicates += 1 return @@ -202,6 +371,7 @@ class Sim { const sweeperRunning = this.sweeperStopsAt === null || this.head <= this.sweeperStopsAt if (sweeperRunning && this.head % this.sweeperPeriod === 0) this.sweeperTick() this.handSweepTick() + this.manualPostTick() this.watcherTick() } return this @@ -351,6 +521,62 @@ test('gated on the depth the sweeper uses, an operator sweep is credited exactly assert.equal(sim.out.credits, 2) }) +test('an overstated manual sweep mints the difference into the depositor’s balance', () => { + // CF-08, driven through the same simulation. The operator moves 100 EMBER by hand and + // types 1000 — a mistyped zero, or a wei figure pasted into a field that expects whole + // coins. Every check the route had passes: the txid is real, it was not rejected, and it + // is 61 confirmations deep. Only the SIZE of it was taken on trust. + // + // `swept` becomes 1000 while the address ever held 100, so the watcher's next tick + // computes `observed = confirmed + swept = 0 + 1000`, sees 900 of it as new, writes a + // deposit_payments row and credits 900 EMBER the depositor never received. It is + // convertible to Shards and withdrawable from the treasury, and nothing reconciles it back. + const sim = new Sim() + .skipAmountCheck() + .handSweepAt(100, 100n * ONE, (c) => sweepIsMature(COIN, c), 1000n * ONE) + sim.at(5, 100n * ONE).runTo(400) + + assert.equal(sim.out.credits, 2, 'expected the overstatement to be credited as a second deposit') + assert.equal(sim.out.credited, 1000n * ONE) + // 900 EMBER of coin balance that no deposit ever funded. This is the mint. + assert.equal(sim.out.credited - 100n * ONE, 900n * ONE) + assert.equal(sim.lastSeen, 1000n * ONE) +}) + +test('checked against the transaction, an overstated manual sweep records nothing', () => { + // The fix. `outbound.checkSweepMovement` compares the posted amount against what the chain + // says left the address — `value + fee`, exactly — and 1000 is not 100, so nothing is + // recorded and nothing is credited. The address stays where the operator found it: in + // `regression`, still refusing to credit, which is the honest state until a CORRECT amount + // is posted. Refusing is the whole point; recording a wrong number is what minted coin. + const sim = new Sim().handSweepAt(100, 100n * ONE, (c) => sweepIsMature(COIN, c), 1000n * ONE) + sim.at(5, 100n * ONE).runTo(400) + + assert.equal(sim.out.credits, 1, 'the overstated amount was credited as a second deposit') + assert.equal(sim.out.credited, 100n * ONE, 'coin balance was created out of a typo') + assert.equal(sim.sweptRecorded, 0n, 'an amount the transaction does not account for was recorded') + assert.ok(sim.out.handRefusals > 0, 'the route accepted an amount the chain contradicts') + // The mark is untouched, so posting the right number later still reconciles the address. + assert.equal(sim.lastSeen, 100n * ONE) + assert.ok(sim.out.regressions > 0, 'the address should still be reporting the unexplained drop') +}) + +test('the amount the chain confirms is recorded, and the address recovers', () => { + // The other half: the check must not stand in the way of the legitimate case. The operator + // posts what actually moved, it is recorded once, and the address credits its next deposit + // rather than staying frozen — which is the whole reason this route exists. + const sim = new Sim().handSweepAt(100, 100n * ONE, (c) => sweepIsMature(COIN, c)) + sim.at(5, 100n * ONE).runTo(400) + + assert.equal(sim.out.handRefusals, 0, 'a correct amount was refused') + assert.equal(sim.sweptRecorded, 100n * ONE) + assert.equal(sim.out.credits, 1) + assert.equal(sim.out.credited, 100n * ONE) + + sim.at(410, 4n * ONE).runTo(600) + assert.equal(sim.out.credited, 104n * ONE, 'the address stayed frozen after a hand sweep') +}) + test('the manual sweep gate is the same gate the sweeper matures on, at every depth', () => { // Driven across the whole window rather than at one height, because every off-by-one in // this file has been a one-block error: `>= depth` leaves exactly one block in which both @@ -391,23 +617,182 @@ test('a sweep that nothing advances stops the address crediting, for good, in si assert.equal(sim.out.credits, 1) }) +/** + * CF-07, in the three states the same slow chain can leave an address in. + * + * The scenario is the ticket's: an EMBER sweep is broadcast and mined, Hearth's block + * production slows, and the sweeper's deadline passes while the sweep is still short of the + * 61 confirmations its accounting needs. `markSweepStuck` runs. From that instant the sweep + * is in NEITHER set that matters — not `UNFINISHED_SWEEP_STATUSES`, so nothing advances it, + * and not `UNMATURED_SWEEP_STATUSES`, so it stops explaining the address's shortfall — and + * the money it moved can never reach `deposit_addresses.swept`. + * + * `stuckAfter(20)` is the slow chain: twenty blocks of it take longer than the deadline. + */ +const stuckSweepScenario = () => + new Sim().at(5, 10n * ONE).stuckAfter(20).sweepAt(100, 10n * ONE) + +test('a stuck sweep freezes its address and the documented remedy changes nothing', () => { + // The failure, exactly as reported. The deposit is credited, the sweep goes stuck, and once + // the confirmed view finally loses the swept funds the address reports `regression` on every + // tick with nothing left that could account for them. + const sim = stuckSweepScenario() + .withoutStuckRecovery() + .withoutManualReconcile() + .recordStuckSweepAt(300) + .runTo(600) + + assert.equal(sim.out.credits, 1) + assert.equal(sim.out.stuckSweeps, 1, 'the slow chain did not produce the stuck sweep') + assert.equal(sim.sweptRecorded, 0n, 'the swept funds were recorded, so this is not the bug') + assert.ok(sim.out.regressions > 100, 'the address should be reporting a regression every tick') + + // The remedy the watcher's own log line names: POST the transaction and the amount. It + // collides with the stuck sweep's own (address_id, txid) row, inserts nothing, and answers + // 200 with `swept` unchanged. The operator has done the documented thing and the address is + // exactly as frozen as it was. + assert.equal(sim.out.replayedNoOps, 1) + assert.equal(sim.out.reconciles, 0) + assert.equal(sim.sweptRecorded, 0n, 'the POST recorded something after all') + + // And the freeze is what it costs: the depositor's next deposit is never credited, because + // `observed` cannot climb back over a high-water mark that a missing 10 EMBER put out of + // reach. (A deposit LARGER than the shortfall would eventually credit the excess and + // silently swallow the difference — smaller ones, like this one, never credit at all.) + sim.at(700, 4n * ONE).runTo(900) + assert.equal(sim.out.credits, 1, 'a later deposit was credited, so the address is not frozen') + assert.equal(sim.out.credited, 10n * ONE) +}) + +test('a stuck sweep records itself once the chain catches up, and nothing stays frozen', () => { + // Half one of the fix: `sweeper.recoverStuckSweep`. The sweep is still stuck — a human was + // told about it and the row keeps saying why — but the transaction is on chain, the head + // has moved 61 blocks past it, and the funds it moved are gone from the view the watcher + // credits at. `matureSweep` now accepts a `stuck` row, so the amount reaches `swept` and the + // address is answered before anybody reads the alarm. + const sim = stuckSweepScenario().withoutManualReconcile().runTo(600) + + assert.equal(sim.out.stuckSweeps, 1, 'the sweep should still have been marked stuck') + assert.equal(sim.sweptRecorded, 10n * ONE, 'the stuck sweep never reached the swept total') + assert.equal(sim.out.credits, 1) + assert.equal(sim.lastSeen, 10n * ONE) + + // The regression is BOUNDED, and the bound is one sweeper poll. The confirmed view loses + // the swept funds at the same height the sweep becomes recoverable, and the watcher ticks + // every block while the sweeper ticks every twenty, so the address reports the drop until + // the next sweeper poll answers it. That window is real in production too — thirty seconds + // against five minutes — and it is the difference between an alarm somebody has to act on + // and one that clears itself. + assert.ok(sim.out.regressions > 0, 'expected the honest report of a drop nothing had explained') + assert.ok(sim.out.regressions <= 20, 'the regression outlived a single sweeper poll') + const bounded = sim.out.regressions + + // The whole point: the depositor's next deposit is credited, and nothing reports again. + sim.at(700, 4n * ONE).runTo(900) + assert.equal(sim.out.regressions, bounded, 'the address never came out of regression') + assert.equal(sim.out.credited, 14n * ONE) + assert.equal(sim.out.credits, 2) +}) + +test("the operator's POST reconciles the stuck sweep instead of reporting a replay", () => { + // Half two, with the sweeper's re-check switched off so the route is on its own — which in + // production is the stuck sweep `recoverableStuckSweeps` deliberately leaves alone, the one + // with no mined height because no node ever admitted to the transaction. `recordManualSweep` + // finds the conflicting row UNRECORDED and matures it in place, with that row's own amount, + // rather than answering `replayed: true` and changing nothing. + const sim = stuckSweepScenario().withoutStuckRecovery().recordStuckSweepAt(300).runTo(600) + + assert.equal(sim.out.reconciles, 1, 'the POST did not reconcile the stuck row') + assert.equal(sim.out.replayedNoOps, 0, 'the POST was reported as a replay') + assert.equal(sim.sweptRecorded, 10n * ONE) + // It is credited exactly once across the whole run — reconciling must not double-count the + // sweep, which is what recording the POSTED amount on top of the row's would have done. + assert.equal(sim.out.credits, 1) + assert.equal(sim.out.credited, 10n * ONE) + assert.equal(sim.lastSeen, 10n * ONE) + + // Frozen until the POST, crediting afterwards. Both halves matter: the regressions before + // it are the honest report of an unexplained drop, and their end is the remedy working. + assert.ok(sim.out.regressions > 0, 'the address should have reported the drop before the POST') + const before = sim.out.regressions + sim.at(700, 4n * ONE).runTo(900) + assert.equal(sim.out.regressions, before, 'the address is still frozen after the POST') + assert.equal(sim.out.credited, 14n * ONE) + assert.equal(sim.out.credits, 2) +}) + +test('a replay of an already-recorded manual sweep is still a no-op', () => { + // The idempotency the reconcile arm must not break. Once the row IS recorded — by the + // sweeper's re-check here — the same POST adds nothing: an unguarded second addition to + // `swept` is credited to the depositor as a deposit nobody made, which is the whole reason + // the (address_id, txid) index exists. + const sim = stuckSweepScenario().recordStuckSweepAt(300).runTo(600) + + assert.equal(sim.out.replayedNoOps, 1, 'a recorded sweep was not reported as a replay') + assert.equal(sim.out.reconciles, 0) + assert.equal(sim.sweptRecorded, 10n * ONE, 'the amount was counted twice') + assert.equal(sim.out.credits, 1) + assert.equal(sim.out.credited, 10n * ONE) +}) + test('a settling address that stops making progress is an alarm, not an info line', () => { // The bound. `settlingStalledMinutes` compares the explaining sweep's last status change // against the sweeper's OWN stuck deadline: past it, a running sweeper would already have // marked that sweep stuck, so a sweep older than that still explaining a shortfall is // proof that nothing is advancing sweeps. That is the state with no alarm attached, and // this is the alarm. - const bound = Math.max(1, env.withdrawalStuckMinutes) + const bound = Math.max(1, sweepStuckMinutes(COIN)) const now = Date.now() const minutesAgo = (n: number) => new Date(now - n * 60_000) - assert.equal(settlingStalledMinutes(minutesAgo(0), now), null, 'a fresh sweep is routine') - assert.equal(settlingStalledMinutes(minutesAgo(bound - 1), now), null) - assert.equal(settlingStalledMinutes(minutesAgo(bound), now), null, 'the deadline itself is not yet late') - assert.equal(settlingStalledMinutes(minutesAgo(bound + 1), now), bound + 1) - assert.equal(settlingStalledMinutes(minutesAgo(bound * 100), now), bound * 100) + assert.equal(settlingStalledMinutes(COIN, minutesAgo(0), now), null, 'a fresh sweep is routine') + assert.equal(settlingStalledMinutes(COIN, minutesAgo(bound - 1), now), null) + assert.equal(settlingStalledMinutes(COIN, minutesAgo(bound), now), null, 'the deadline itself is not yet late') + assert.equal(settlingStalledMinutes(COIN, minutesAgo(bound + 1), now), bound + 1) + assert.equal(settlingStalledMinutes(COIN, minutesAgo(bound * 100), now), bound * 100) // Nothing in flight at all is not a stall; it is a regression, and it is reported as one. - assert.equal(settlingStalledMinutes(null, now), null) + assert.equal(settlingStalledMinutes(COIN, null, now), null) + + // And it is the SWEEP deadline, per coin, not the withdrawal worker's flat hour. XRP needs + // one validated ledger and gets the floor; EMBER needs 61 blocks and gets four times what + // they take. An address is only ever compared against its own chain's number. + assert.equal(settlingStalledMinutes('XRP', minutesAgo(SWEEP_STUCK_FLOOR_MINUTES + 1), now), SWEEP_STUCK_FLOOR_MINUTES + 1) + assert.equal(settlingStalledMinutes(COIN, minutesAgo(SWEEP_STUCK_FLOOR_MINUTES + 1), now), null) +}) + +test('the stuck deadline is derived from the depth being waited for, not borrowed', () => { + // CF-07's second half. The sweeper marked a sweep stuck after PAY_WITHDRAWAL_STUCK_MINUTES, + // which is a number about a DIFFERENT worker waiting for a DIFFERENT thing: a withdrawal is + // late when one transaction has not been mined, a sweep is late when the whole chain has not + // moved 61 blocks. Nothing tied them together, and being stuck is not a small mistake — it + // is what stops a deposit address crediting anybody. + for (const info of SUPPORTED_DEPOSIT_COINS) { + const deadlineSeconds = sweepStuckMinutes(info.coin) * 60 + assert.ok( + deadlineSeconds >= SWEEP_STUCK_FLOOR_MINUTES * 60, + `${info.coin}'s deadline is under the floor, so one slow RPC could strand a sweep`, + ) + // The property that matters: a chain running at its nominal rate must reach maturity with + // room to spare. Anything less marks healthy sweeps stuck and freezes their addresses. + assert.ok( + deadlineSeconds >= SWEEP_STUCK_SLACK * NOMINAL_MATURITY_SECONDS[info.coin], + `${info.coin} can be marked stuck while its chain is running normally`, + ) + } + + // EMBER concretely: 61 blocks at 15 seconds is 15.25 minutes, so the deadline is 61 — + // near enough to the borrowed hour that the old number LOOKED right, which is why this + // went unnoticed. It stops looking right the moment the depth moves: raise EMBER's + // confirmations past 59 and nominal maturity crosses the hour, at which point the flat + // deadline marked every healthy sweep on the chain stuck and froze every swept address. + assert.equal(sweepStuckMinutes('EMBER'), 61) + assert.ok( + sweepStuckMinutes('EMBER') > 60, + 'the borrowed hour was ALREADY tighter than four times what EMBER maturity takes', + ) + // The fast chains are the floor, not four times sixteen seconds. + assert.equal(sweepStuckMinutes('XRP'), SWEEP_STUCK_FLOOR_MINUTES) + assert.equal(sweepStuckMinutes('SOL'), SWEEP_STUCK_FLOOR_MINUTES) }) test('one unconfigured chain does not stop the sweeper on every other chain', () => { @@ -631,3 +1016,36 @@ test('a tip reading that could not be taken is not a tip to add sweeps back to', assert.equal(standIn.shortfall, 0n) assert.equal(standIn.delta, 0n) }) + +test('every alarm the sweeper raises about a sweep names the deposit address it froze — CF-22', () => { + // THE DEFECT, ON THE NEIGHBOURING PATH TO THE WATCHER'S. Two of these alarms end with + // "POST /admin/deposit-addresses/:id/sweeps", and that `:id` is the DEPOSIT ADDRESS's uuid. + // They logged `sweepId` — the OTHER uuid in scope — so the operator the line just paged was + // handed the wrong one of the two and had to go and find the right one, which is exactly + // what CF-22 is about. `row.addressId` was in scope at every one of them. + const row: SweepRow = { + id: 'f7a0c0d4-1111-4b8e-9a4a-000000000001', + addressId: '0f2b4b1e-2c9a-4a5e-9a0f-6d21b4a2c111', + coin: 'EMBER', + network: 'testnet', + fromAddress: '0x00000000000000000000000000000000000000ab', + toAddress: '0x00000000000000000000000000000000000000cd', + amount: '10', + fee: '1', + status: 'stuck', + txid: '0xabc', + rawTx: null, + minedHeight: null, + failureReason: 'the sweep has not reached the depth its accounting needs', + recordedAt: null, + createdAt: new Date('2026-01-01T00:00:00.000Z'), + updatedAt: new Date('2026-01-01T00:00:00.000Z'), + } + const fields = sweepAlarmFields(row) + assert.equal(fields.addressId, row.addressId, 'the alarm does not name the address it froze') + // And it is NOT the sweep's own id, which is the substitution that made the line useless. + assert.notEqual(fields.addressId, fields.sweepId) + assert.equal(fields.sweepId, row.id) + assert.equal(fields.coin, 'EMBER') + assert.equal(fields.txid, '0xabc') +}) diff --git a/services/pay/src/sweeper.ts b/services/pay/src/sweeper.ts index c7d4475..eb23196 100644 --- a/services/pay/src/sweeper.ts +++ b/services/pay/src/sweeper.ts @@ -13,7 +13,7 @@ import { spendableBalance, UnsupportedChainError, } from './outbound.js' -import { sweepMaturityConfirmations } from './chains.js' +import { sweepMaturityConfirmations, sweepStuckMinutes } from './chains.js' import { activeSweeps, allDepositAddresses, @@ -27,6 +27,7 @@ import { markSweepStuck, matureSweep, pendingWithdrawalTotal, + recoverableStuckSweeps, unmaturedSweptByAddress, } from './store.js' import type { DepositAddressRow, SweepRow } from './db/schema.js' @@ -207,8 +208,85 @@ export class ChainBlocks { /** Not persisted: a restart clears every block, including the permanent ones. */ const chainBlocks = new ChainBlocks() +/** + * Has this sweep been short of the depth its accounting needs for longer than its chain can + * honestly explain? + * + * The deadline is `chains.sweepStuckMinutes` and it is DERIVED from the depth being waited + * for. It used to be `PAY_WITHDRAWAL_STUCK_MINUTES` — the withdrawal worker's flat hour, + * borrowed for a question it does not answer. See that function for why the two numbers are + * unrelated and why the borrowed one breaks the moment a deposit depth is raised. + * + * Measured from `updated_at`, which is why `advanceSweep` is careful to touch it only when + * something actually changes. + */ function stuckDeadlinePassed(row: SweepRow): boolean { - return Date.now() - row.updatedAt.getTime() > Math.max(1, env.withdrawalStuckMinutes) * 60_000 + const minutes = Math.max(1, sweepStuckMinutes(row.coin as DepositCoin)) + return Date.now() - row.updatedAt.getTime() > minutes * 60_000 +} + +/** + * The identifying fields every alarm about a sweep carries — CF-22. + * + * `addressId` is in here rather than repeated at each call site because it is the field that + * is easiest to leave out and most expensive to have left out. Two of these alarms end with + * "POST /admin/deposit-addresses/:id/sweeps", and that `:id` is the DEPOSIT ADDRESS's uuid, + * not the sweep's. An alarm that prints `sweepId` alone hands the reader one uuid and asks + * them for a different one, so the operator the line just paged has to go and find it — which + * is the whole defect CF-22 names, on the neighbouring path to the watcher's own log lines. + * + * Pure and exported so the invariant is a test rather than a habit. + */ +export function sweepAlarmFields(row: SweepRow): { + sweepId: string + addressId: string + coin: string + txid: string | null +} { + return { sweepId: row.id, addressId: row.addressId, coin: row.coin, txid: row.txid } +} + +/** + * A sweep that was marked `stuck` while it waited, whose transaction has since reached the + * depth its accounting needs. Record it and let the address credit again. + * + * THE STATE THIS EXISTS FOR IS THE ORDINARY ONE. `advanceSweep` marks a sweep stuck when the + * chain has not carried it to `sweepMaturityConfirmations` inside the deadline — on a slow + * EMBER that is a mined, perfectly healthy transaction that simply needed longer than an hour + * for sixty-one blocks. Before CF-07 that was terminal: `activeSweeps` stopped returning the + * row so nothing matured it, `UNMATURED_SWEEP_STATUSES` stopped counting it so it no longer + * explained the address's shortfall, and the watcher reported `regression` and credited that + * depositor nothing ever again. The money had left the address and no code path left could + * put its amount into `deposit_addresses.swept`. + * + * It is the same decision `advanceSweep` makes, deliberately: ask the chain, and record only + * at maturity. A transaction the node now calls `rejected`, or has lost sight of, is left + * stuck for the operator — the funds it would have moved never left, so nothing is owed to + * `swept` and there is nothing to un-freeze. + */ +async function recoverStuckSweep(row: SweepRow, log: FastifyBaseLogger): Promise { + if (!row.txid) return + const coin = row.coin as DepositCoin + const network = row.network as ChainNetwork + const status = await outboundStatus(coin, network, row.txid, log) + if (status.kind !== 'confirmed' && status.kind !== 'pending') return + if (status.confirmations < sweepMaturityConfirmations(coin)) return + if (!(await matureSweep(row.id))) return + // Warn rather than info: an operator was told this sweep needed them, and this is the line + // that says it no longer does. + log.warn( + { + // The same fields the alarm carried, so the line that clears it can be matched to the + // line that raised it — `addressId` above all, which is the address this says is no + // longer frozen. + ...sweepAlarmFields(row), + network, + confirmations: status.confirmations, + stuckReason: row.failureReason, + }, + 'a stuck sweep reached the depth its accounting needs and has been recorded — the deposit ' + + 'address it came out of is no longer frozen, and nothing needs doing by hand', + ) } /** @@ -451,12 +529,22 @@ async function advanceSweep(row: SweepRow, log: FastifyBaseLogger): Promise() for (const address of addresses) { if (!canSend(address.coin as DepositCoin)) continue + // The deposit watcher skips addresses on another network, so nothing credits them any + // more; sweeping one would move a customer's coins into the treasury against a + // balance that was never granted. PAY_TREASURY_TARGET_ is per COIN and not per + // network, so without this line a float target on a go-live deployment is enough to + // drain every address left over from testnet. The watcher does the reporting — see + // network.ts — so this is silent on purpose. + if (address.network !== env.depositNetwork) continue const key = `${address.coin}:${address.network}` const group = groups.get(key) if (group) group.push(address) diff --git a/services/pay/src/watcher.test.ts b/services/pay/src/watcher.test.ts new file mode 100644 index 0000000..c96207b --- /dev/null +++ b/services/pay/src/watcher.test.ts @@ -0,0 +1,232 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { depositScanLine, type DepositScanLine } from './watcher.js' +import { + currentMarkBasis, + scanDepositAddress, + type DepositScan, + type ProbeTotals, + type SweptTotals, +} from './store.js' +import type { DepositAddressRow } from './db/schema.js' + +/** + * The operator surface of a frozen deposit address — CF-22. + * + * The remedy for an address the watcher has stopped crediting is + * `POST /admin/deposit-addresses/:id/sweeps`, and its `:id` is a random UUID that lives in + * one place: the `deposit_addresses` row. `GET /deposits` hands it to the address's owner and + * to nobody else. So for the person actually holding the alarm there were exactly two ways to + * learn the parameter of the only route that unfreezes the address, and both of them were + * "open a SQL shell on production" — because the alarm itself, holding `row.id` in scope, + * printed the coin and the address and left the id out. + * + * Hence the two halves tested here. `depositScanLine` is the log line, pure so that what is IN + * it can be asserted; `scanDepositAddress` is the state that line reports, recomputed without + * writing, which is what lets `GET /admin/deposit-addresses?scan=true` list frozen addresses + * without a database shell. + */ + +const ROW: DepositAddressRow = { + id: '0f2b4b1e-2c9a-4a5e-9a0f-6d21b4a2c111', + userId: 'user-1', + coin: 'EMBER', + network: 'testnet', + address: '0x00000000000000000000000000000000000000ab', + lastSeen: '0', + swept: '0', + pending: '0', + markBasis: null, + createdAt: new Date('2026-01-01T00:00:00.000Z'), +} + +/** Every outcome the watcher can report, one of each. */ +const SCANS: DepositScan[] = [ + { + kind: 'credited', + payment: { + id: 'p1', + userId: ROW.userId, + coin: 'EMBER', + network: 'testnet', + address: ROW.address, + txid: 'EMBER:addr:v0:1', + amount: '1', + confirmations: 61, + status: 'credited', + seenAt: '2026-01-01T00:00:00.000Z', + }, + }, + { kind: 'regression', missing: '10' }, + { kind: 'settling', sweeping: '10', stalledMinutes: null }, + { kind: 'settling', sweeping: '10', stalledMinutes: 240 }, + { kind: 'rebased', from: '20', to: '10' }, + { kind: 'duplicate' }, +] + +function line(scan: DepositScan): DepositScanLine { + const out = depositScanLine(ROW, scan) + assert.ok(out, `expected a line for ${scan.kind}`) + return out +} + +test('every line the watcher writes about an address names that address\'s id', () => { + // THE DEFECT, IN ONE ASSERTION. Each of these lines is an operator's only view of the state + // it reports, and the value they need next — the row id — was in scope at every one of them + // and printed by none. + for (const scan of SCANS) { + const written = line(scan) + assert.equal(written.fields.addressId, ROW.id, `${scan.kind} line is missing addressId`) + assert.equal(written.fields.coin, ROW.coin) + assert.equal(written.fields.address, ROW.address) + } +}) + +test('the regression line states the remedy AND the parameter that remedy needs', () => { + const written = line({ kind: 'regression', missing: '10' }) + assert.equal(written.level, 'error') + assert.match(written.message, /POST \/admin\/deposit-addresses\/:id\/sweeps/) + // The id is a UUID and unguessable; naming the route without it is naming half a command. + assert.equal(written.fields.addressId, ROW.id) + assert.equal(written.fields.missing, '10') +}) + +test('nothing is said about an address with nothing to report', () => { + assert.equal(depositScanLine(ROW, { kind: 'none' }), null) +}) + +test('the levels are unchanged: a stalled sweep is an alarm, an in-flight one is not', () => { + assert.equal(line({ kind: 'settling', sweeping: '10', stalledMinutes: null }).level, 'info') + assert.equal(line({ kind: 'settling', sweeping: '10', stalledMinutes: 240 }).level, 'error') + assert.equal(line({ kind: 'regression', missing: '1' }).level, 'error') + assert.equal(line({ kind: 'rebased', from: '2', to: '1' }).level, 'warn') + assert.equal(line({ kind: 'duplicate' }).level, 'warn') + assert.equal(line(SCANS[0]!).level, 'info') +}) + +test('the stalled-sweep line carries the number of minutes it names in its message', () => { + const written = line({ kind: 'settling', sweeping: '10', stalledMinutes: 240 }) + assert.equal(written.fields.stalledMinutes, 240) + assert.match(written.message, /has not moved in 240 minutes/) +}) + +// --------------------------------------------------------------------------- +// The same states, recomputed read-only — what GET /admin/deposit-addresses?scan=true lists +// --------------------------------------------------------------------------- + +const WEI = 10n ** 18n + +function probe(confirmed: bigint, tip = confirmed): ProbeTotals { + return { confirmed, tip, cumulative: false, confirmedIsExact: true, tipIsExact: true } +} + +const NOTHING_SWEEPING: SweptTotals = { recorded: 0n, inFlight: 0n, inFlightMined: 0n } + +test('an address whose funds left with nothing accounting for it reads as frozen', () => { + // The literal shape of CF-22's How-it-fails: 10 EMBER credited, moved out by hand, no sweep + // row. This is the state the watcher logs `regression` for and refuses to credit past. + const state = scanDepositAddress({ + coin: 'EMBER', + markBasis: null, + lastSeen: 10n * WEI, + probe: probe(0n), + swept: NOTHING_SWEEPING, + }) + assert.deepEqual(state, { state: 'regression', missing: '10' }) +}) + +test('a sweep of ours explains the same shortfall, and says so without an alarm', () => { + const state = scanDepositAddress({ + coin: 'EMBER', + markBasis: null, + lastSeen: 10n * WEI, + probe: probe(0n), + swept: { + recorded: 0n, + inFlight: 10n * WEI, + inFlightMined: 0n, + oldestUpdatedAt: new Date(Date.now() - 60_000), + }, + }) + assert.deepEqual(state, { state: 'settling', sweeping: '10', stalledMinutes: null }) +}) + +test('a sweep that has not moved in longer than its chain\'s deadline is reported stalled', () => { + const state = scanDepositAddress({ + coin: 'EMBER', + markBasis: null, + lastSeen: 10n * WEI, + probe: probe(0n), + swept: { + recorded: 0n, + inFlight: 10n * WEI, + inFlightMined: 0n, + // EMBER's sweep deadline is 61 blocks x 15s x 4 = 61 minutes; four hours is past it. + oldestUpdatedAt: new Date(Date.now() - 240 * 60_000), + }, + }) + assert.equal(state.state, 'settling') + assert.equal(state.state === 'settling' && state.stalledMinutes, 240) +}) + +test('a recorded sweep leaves the address healthy, not frozen', () => { + const state = scanDepositAddress({ + coin: 'EMBER', + markBasis: null, + lastSeen: 10n * WEI, + probe: probe(0n), + swept: { recorded: 10n * WEI, inFlight: 0n, inFlightMined: 0n }, + }) + assert.deepEqual(state, { state: 'ok' }) +}) + +test('funds the next tick will credit are reported as uncredited, never as an alarm', () => { + const state = scanDepositAddress({ + coin: 'EMBER', + markBasis: null, + lastSeen: 10n * WEI, + probe: probe(12n * WEI), + swept: NOTHING_SWEEPING, + }) + assert.deepEqual(state, { state: 'uncredited', amount: '2' }) +}) + +test('a read-only scan NEVER reports the one-time XRP restatement as a regression', () => { + // The read has to make the same distinction the credit path does, or every XRP address that + // has not yet been restated reads as frozen and an operator records a sweep for coins that + // never moved. `markNeedsRebase` is the same function both sides call. + assert.equal(currentMarkBasis('XRP'), 'xrp-reserve-excluded') + const state = scanDepositAddress({ + coin: 'XRP', + markBasis: null, + lastSeen: 20_000_000n, + probe: probe(9_000_000n), + swept: NOTHING_SWEEPING, + }) + assert.deepEqual(state, { state: 'rebasing', from: '20', to: '9' }) + // Once the row carries the current basis the same drop IS a regression, as it should be. + assert.deepEqual( + scanDepositAddress({ + coin: 'XRP', + markBasis: 'xrp-reserve-excluded', + lastSeen: 20_000_000n, + probe: probe(9_000_000n), + swept: NOTHING_SWEEPING, + }), + { state: 'regression', missing: '11' }, + ) +}) + +test('bitcoin\'s cumulative probe is not read as a shortfall by the read-only scan either', () => { + // BTC's probe is a received counter that sweeping does not move, so adding `swept` back + // would count the same coins twice. `observeDeposit` owns that rule; this asserts the + // read-only path goes through it rather than around it. + const state = scanDepositAddress({ + coin: 'BTC', + markBasis: null, + lastSeen: 100_000n, + probe: { confirmed: 100_000n, tip: 100_000n, cumulative: true, confirmedIsExact: true, tipIsExact: true }, + swept: { recorded: 100_000n, inFlight: 0n, inFlightMined: 0n }, + }) + assert.deepEqual(state, { state: 'ok' }) +}) diff --git a/services/pay/src/watcher.ts b/services/pay/src/watcher.ts index 3b5fa39..c7fc644 100644 --- a/services/pay/src/watcher.ts +++ b/services/pay/src/watcher.ts @@ -2,9 +2,116 @@ import type { FastifyBaseLogger } from 'fastify' import type { DepositCoin } from '@cloudsforge/shared' import { fetchFundedTotals } from './chains.js' import { env } from './env.js' -import { allDepositAddresses, recordDepositAndCredit } from './store.js' +import { partitionByDeploymentNetwork } from './network.js' +import { allDepositAddresses, recordDepositAndCredit, type DepositScan } from './store.js' import type { DepositAddressRow } from './db/schema.js' +/** One scan, as the line an operator reads. Null when the outcome is worth nothing to say. */ +export interface DepositScanLine { + level: 'info' | 'warn' | 'error' + fields: Record + message: string +} + +/** + * What to write down about one probe of one address. + * + * Pure and exported so the CONTENTS of these lines can be tested, which matters more here + * than it looks: for three of these outcomes the log line IS the operator interface. There is + * no console screen for a frozen deposit address, and the remedy the `regression` line names + * — `POST /admin/deposit-addresses/:id/sweeps` — is keyed by a random UUID. + * + * WHICH IS WHY EVERY LINE CARRIES `addressId`, AND THAT IS THE WHOLE OF CF-22. These lines + * used to state the remedy and then omit its only parameter, while holding the value in scope + * one line above: an operator reading `regression` was told to call an endpoint whose `:id` + * they could only get by opening a SQL shell against `deposit_addresses`. `GET /deposits` + * returns the id, but only to the address's own owner, so it is no help to the person holding + * the alarm. The id is now in the line, in the same object as the coin and the address, and + * `watcher.test.ts` asserts it for every outcome so it cannot quietly fall out again. + */ +export function depositScanLine(row: DepositAddressRow, scan: DepositScan): DepositScanLine | null { + const at = { addressId: row.id, coin: row.coin, address: row.address } + switch (scan.kind) { + case 'credited': { + const p = scan.payment + return { + level: 'info', + fields: at, + message: `deposit: +${p.amount} ${p.coin} (${p.network}) -> user ${p.userId} @ ${p.address}`, + } + } + case 'regression': + return { + level: 'error', + fields: { ...at, missing: scan.missing }, + message: + 'deposit watcher: confirmed balance dropped below the high-water mark without a ' + + 'recorded sweep — crediting is paused for this address until the movement is ' + + 'accounted for with POST /admin/deposit-addresses/:id/sweeps', + } + case 'settling': + // NOT routine any more, once `stalledMinutes` is set. `settling` is bounded only while + // something is advancing sweeps: a sweep that never matures is marked `stuck`, leaves + // the explaining set, and the address correctly reports `regression`. If nothing is + // advancing sweeps at all — the worker disabled with one in flight, or a chain of + // theirs blocked — the sweep sits in `signed`/`broadcast` forever, this address never + // credits another deposit, and at info level nothing ever says so. The state is not the + // problem; the silence was. + // + // Below that bound it is deliberately info: a sweep of ours has left the address but is + // not deep enough for its amount to be added to `swept` yet, so for the length of one + // confirmation window neither side of the arithmetic is holding those coins. This used + // to be reported as a regression, which is how sweeping an address froze it: see + // chains.sweepMaturityConfirmations. + return scan.stalledMinutes !== null + ? { + level: 'error', + fields: { ...at, sweeping: scan.sweeping, stalledMinutes: scan.stalledMinutes }, + message: + 'deposit watcher: the sweep that explains this address has not moved in ' + + `${scan.stalledMinutes} minutes — crediting for it is stopped until something ` + + 'advances or fails that sweep', + } + : { + level: 'info', + fields: { ...at, sweeping: scan.sweeping }, + message: + 'deposit watcher: a sweep is in flight for this address — crediting resumes when it matures', + } + case 'rebased': + // Once per address, ever. Loud because it is the one thing in this service that moves a + // high-water mark DOWN, and both numbers are in the line so the change can be checked + // by hand afterwards. + return { + level: 'warn', + fields: { ...at, from: scan.from, to: scan.to }, + message: + "deposit watcher: restated this address's high-water mark onto the current " + + 'basis — nothing was credited, and it cannot happen again for this address', + } + case 'duplicate': + // Nothing is known to produce this any more. It used to be reachable, once per XRP + // address: the one-time restatement lowered the mark by a base reserve, the address + // climbed back onto a total it had already recorded, and the payment id — derived from + // the total alone — collided. The id now names the basis it was computed under + // (store.depositPaymentTxid, CF-45), so the pre- and post-restatement totals are + // different ids. What is left is a payment row and a high-water mark that disagree + // about the same money, which is not something this service can reconcile by + // arithmetic: crediting past it would be a mint, so this address credits nothing + // further until someone looks. + return { + level: 'warn', + fields: at, + message: + 'deposit watcher: this total was already recorded but the high-water mark is ' + + 'behind it — crediting is stalled for this address; compare its ' + + 'deposit_payments rows against deposit_addresses.last_seen', + } + case 'none': + return null + } +} + /** * Background deposit watcher. Every PAY_DEPOSIT_POLL_SECONDS it sweeps every deposit * address, asks the relevant chain what it has received AT CONFIRMATION DEPTH, and credits @@ -18,6 +125,11 @@ import type { DepositAddressRow } from './db/schema.js' */ export function startDepositWatcher(log: FastifyBaseLogger) { let inFlight = false + // Deposit addresses on a network this deployment does not run, already written down. Held + // for the life of the process rather than the life of a tick, because the alternative is + // the same warning about the same address every PAY_DEPOSIT_POLL_SECONDS forever — which + // is how an operator learns to stop reading the watcher's warnings. + const reportedForeign = new Set() const tick = async () => { if (inFlight) { @@ -42,9 +154,32 @@ export function startDepositWatcher(log: FastifyBaseLogger) { return } + // An address minted on the other network is watched by nothing here: its RPC pair, its + // treasury and the `coin_balances` row `GET /wallet` reads are all keyed on + // PAY_DEPOSIT_NETWORK. Probing it would keep crediting a balance the wallet does not + // show and no route will spend, so it is skipped — and said out loud once, because the + // coins sitting at it are real and only an operator can get them back. See network.ts. + const { watched, unreported } = partitionByDeploymentNetwork(rows, reportedForeign) + for (const row of unreported) { + log.warn( + { + addressId: row.id, + userId: row.userId, + coin: row.coin, + network: row.network, + address: row.address, + watching: env.depositNetwork, + }, + `deposit watcher: this address is on ${row.network} and this deployment settles on ` + + `${env.depositNetwork} — it is NOT being watched, and anything sent to it will ` + + 'never be credited. forge-keyvault still holds its key under ' + + `deposit:${row.userId}:${row.coin}:${row.network}`, + ) + } + // Probe every address independently so a slow/failing RPC can't stall the others. const results = await Promise.allSettled( - rows.map(async (row) => { + watched.map(async (row) => { try { const probe = await fetchFundedTotals( row.coin as DepositCoin, @@ -54,67 +189,24 @@ export function startDepositWatcher(log: FastifyBaseLogger) { ) // Pass the id, not the row: the credit decision re-reads the row under lock. const scan = await recordDepositAndCredit(row.id, probe) - if (scan.kind === 'credited') { - const p = scan.payment - log.info( - `deposit: +${p.amount} ${p.coin} (${p.network}) -> user ${p.userId} @ ${p.address}`, - ) - } else if (scan.kind === 'regression') { - log.error( - { coin: row.coin, address: row.address, missing: scan.missing }, - 'deposit watcher: confirmed balance dropped below the high-water mark without a ' + - 'recorded sweep — crediting is paused for this address until the movement is ' + - 'accounted for with POST /admin/deposit-addresses/:id/sweeps', - ) - } else if (scan.kind === 'settling' && scan.stalledMinutes !== null) { - // NOT routine any more. `settling` is bounded only while something is advancing - // sweeps: a sweep that never matures is marked `stuck`, leaves the explaining - // set, and the address correctly reports `regression`. If nothing is advancing - // sweeps at all — the worker disabled with one in flight, or a chain of theirs - // blocked — the sweep sits in `signed`/`broadcast` forever, this address never - // credits another deposit, and at info level nothing ever says so. The state is - // not the problem; the silence was. - log.error( - { - coin: row.coin, - address: row.address, - sweeping: scan.sweeping, - stalledMinutes: scan.stalledMinutes, - }, - 'deposit watcher: the sweep that explains this address has not moved in ' + - `${scan.stalledMinutes} minutes — crediting for it is stopped until something ` + - 'advances or fails that sweep', - ) - } else if (scan.kind === 'settling') { - // Routine, and deliberately not an error. A sweep of ours has left the address - // but is not deep enough for its amount to be added to `swept` yet, so for the - // length of one confirmation window neither side of the arithmetic is holding - // those coins. This used to be reported as a regression, which is how sweeping - // an address froze it: see chains.sweepMaturityConfirmations. - log.info( - { coin: row.coin, address: row.address, sweeping: scan.sweeping }, - 'deposit watcher: a sweep is in flight for this address — crediting resumes when it matures', - ) - } else if (scan.kind === 'rebased') { - // Once per address, ever. Loud because it is the one thing in this service that - // moves a high-water mark DOWN, and both numbers are in the line so the change - // can be checked by hand afterwards. - log.warn( - { coin: row.coin, address: row.address, from: scan.from, to: scan.to }, - 'deposit watcher: restated this address\'s high-water mark onto the current ' + - 'basis — nothing was credited, and it cannot happen again for this address', - ) - } else if (scan.kind === 'duplicate') { - log.warn( - { coin: row.coin, address: row.address }, - 'deposit watcher: this total was already recorded but the high-water mark is behind it', - ) - } + // What to say about it — including the id of the row it is about — is decided by + // `depositScanLine`, which is pure so that the contents of these lines are + // testable. For `regression`, `settling` and `duplicate` the line is the only + // interface an operator has to this state. See CF-22. + const line = depositScanLine(row, scan) + if (line) log[line.level](line.fields, line.message) } catch (err) { // Was `debug`, i.e. below the default level, i.e. never written down: a dead // RPC produced a week of total silence while nobody's deposits were credited. log.error( - { err, coin: row.coin, network: row.network, address: row.address, userId: row.userId }, + { + err, + addressId: row.id, + coin: row.coin, + network: row.network, + address: row.address, + userId: row.userId, + }, 'deposit watcher: probe failed — this address was not checked for deposits', ) } diff --git a/services/pay/src/withdrawer.test.ts b/services/pay/src/withdrawer.test.ts new file mode 100644 index 0000000..2c7803a --- /dev/null +++ b/services/pay/src/withdrawer.test.ts @@ -0,0 +1,335 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { SignRefusedError } from './keyvault.js' +import { + InsufficientTreasuryError, + UnsupportedChainError, + UnsupportedDestinationError, +} from './outbound.js' +import { env } from './env.js' +import { planBuildFailure, stuckDeadlinePassed } from './withdrawer.js' + +/** + * The outbound queue, driven by a clock — CF-24. + * + * WHY A SIMULATION AND NOT A UNIT TEST. The defect is not visible in any one call. Every + * individual step of `startWithdrawal` was right: a build that throws really might succeed + * next time, so logging it and returning is the correct thing to do on that tick. What was + * wrong is a property of the SEQUENCE — the unclassified arm had no deadline on "next time", + * and the worker starts only the oldest `pending` row of each chain per tick, so one + * withdrawal that can never be built holds its chain's entire queue for as long as the + * process runs. A test of one tick passes against the broken code and proves nothing, so + * this drives ticks. + * + * WHAT IS REAL AND WHAT IS MODELLED. The two decisions the defect lives in are the real + * production functions: `withdrawer.planBuildFailure` (what a build failure means) and + * `withdrawer.stuckDeadlinePassed` (when waiting has gone on too long). The queue, the + * chain, the clock and the store are modelled here, mirroring `startWithdrawalWorker`'s tick + * and the status gates in `store.markWithdrawalStuck` / `store.refundWithdrawal`. + */ + +const POLL_MS = Math.max(5, env.withdrawalPollSeconds) * 1000 +const STUCK_MS = Math.max(1, env.withdrawalStuckMinutes) * 60_000 +const ONE = 10n ** 18n + +/** Statuses `activeWithdrawals` returns — the worker's queue. */ +type Status = 'pending' | 'signed' | 'broadcast' | 'confirmed' | 'failed' | 'stuck' + +interface Row { + id: string + createdAt: number + broadcastAt: number | null + status: Status + amount: bigint + /** What `ensureTreasury` + `buildAndSign` do for this row, on every attempt. */ + build: () => void +} + +/** + * How `startWithdrawal` treats an unclassified build failure. + * + * `shipped` is the code CF-24 was raised against: log `will retry` and return, with no + * deadline and no way past the row. `stuck` is the remedy the ticket's title proposes, which + * the reviewer notes correct — `store.markWithdrawalStuck` is gated on `signed`/`broadcast`, + * so for a `pending` row it updates nothing. `refund` is what shipped instead. + */ +type Remedy = 'shipped' | 'stuck' | 'refund' + +class Sim { + now = 0 + ticks = 0 + private rows: Row[] = [] + /** The user's coin balance. Debited when the withdrawal was queued, credited by a refund. */ + balance = 0n + buildAttempts = 0 + signatures = 0 + refunds = 0 + stuckMarks = 0 + + constructor(private readonly remedy: Remedy = 'refund') {} + + /** Queue a withdrawal now, debiting the user exactly as `POST /withdrawals` does. */ + queue(id: string, build: () => void, amount = ONE): this { + this.rows.push({ id, createdAt: this.now, broadcastAt: null, status: 'pending', amount, build }) + return this + } + + row(id: string): Row { + const found = this.rows.find((r) => r.id === id) + assert.ok(found, `no row ${id}`) + return found + } + + /** store.markWithdrawalStuck — the status gate is the whole point of modelling it. */ + private markStuck(row: Row): void { + if (row.status !== 'signed' && row.status !== 'broadcast') return + row.status = 'stuck' + this.stuckMarks += 1 + } + + /** store.refundWithdrawal(id, reason, ['pending']) — gate, credit and close, together. */ + private refund(row: Row): boolean { + if (row.status !== 'pending') return false + row.status = 'failed' + this.balance += row.amount + this.refunds += 1 + return true + } + + /** store.hasUnsettledOutbound: one payment per chain in the air at a time. */ + private hasUnsettledOutbound(): boolean { + return this.rows.some((r) => r.status === 'signed' || r.status === 'broadcast') + } + + /** withdrawer.startWithdrawal, minus keyvault and the node. */ + private start(row: Row): 'held' | 'retired' { + this.buildAttempts += 1 + try { + row.build() + } catch (err) { + const plan = planBuildFailure(err) + if (this.remedy !== 'refund' && plan.classification === 'unclassified') { + // The shipped arm: a log line and nothing else. `stuck` differs only in calling a + // function that declines to act on a `pending` row. + if (this.remedy === 'stuck' && stuckDeadlinePassed(this.dates(row), this.now)) this.markStuck(row) + return 'held' + } + if (plan.refund === 'at-deadline' && !stuckDeadlinePassed(this.dates(row), this.now)) return 'held' + return this.refund(row) ? 'retired' : 'held' + } + row.status = 'signed' + this.signatures += 1 + return 'held' + } + + /** `stuckDeadlinePassed` reads Dates off the row; the sim keeps time in milliseconds. */ + private dates(row: Row): { broadcastAt: Date | null; createdAt: Date } { + return { + broadcastAt: row.broadcastAt === null ? null : new Date(row.broadcastAt), + createdAt: new Date(row.createdAt), + } + } + + /** + * A signed withdrawal settles on the next tick. + * + * `advanceWithdrawal`'s own paths are not what this file is about — all that matters to the + * queue is that a payment which does land eventually stops holding the chain. + */ + private advance(row: Row): void { + if (row.status === 'signed') { + row.status = 'broadcast' + row.broadcastAt = this.now + return + } + if (row.status === 'broadcast') row.status = 'confirmed' + } + + /** One pass of `startWithdrawalWorker`'s tick, for the single chain this sim models. */ + private tick(): void { + this.ticks += 1 + const active = this.rows.filter( + (r) => r.status === 'pending' || r.status === 'signed' || r.status === 'broadcast', + ) + for (const row of active) if (row.status !== 'pending') this.advance(row) + const queued = active.filter((r) => r.status === 'pending') + if (queued.length === 0) return + if (this.hasUnsettledOutbound()) return + for (const next of queued) { + if (this.start(next) !== 'retired') break + // `shipped` had no loop here at all: one attempt per chain per tick, full stop. Modelled + // by never returning 'retired' from the arm above, which is what the code did. + } + } + + /** Run the worker for this many milliseconds of wall clock. */ + run(ms: number): this { + const until = this.now + ms + while (this.now < until) { + this.now += POLL_MS + this.tick() + } + return this + } +} + +/** The realistic trigger: an XRP treasury address the ledger has never heard of. */ +function unfundedTreasury(): () => void { + return () => { + throw new Error('xrp account_info: actNotFound') + } +} + +function buildsFine(): () => void { + return () => {} +} + +test('CF-24: an unbuildable withdrawal used to hold its chain for as long as the process ran', () => { + // The code as it shipped. Three withdrawals; the oldest can never be built. + const sim = new Sim('shipped') + .queue('a', unfundedTreasury()) + .queue('b', buildsFine()) + .queue('c', buildsFine()) + .run(6 * 60 * 60_000) + + // Six hours — six times the stuck deadline — and nothing has moved. + assert.equal(sim.row('a').status, 'pending', 'the unbuildable row left `pending`') + assert.equal(sim.refunds, 0) + assert.equal(sim.balance, 0n, 'the user got their money back, which the shipped code never did') + assert.equal(sim.row('b').status, 'pending', 'the row behind it was started') + assert.equal(sim.row('c').status, 'pending') + assert.equal(sim.signatures, 0, 'no withdrawal on this chain was ever signed') + // ... and it cost an RPC round trip every 20 seconds to achieve none of it. + assert.equal(sim.buildAttempts, sim.ticks) +}) + +test('CF-24: marking it `stuck` would not have unblocked the chain', () => { + // The remedy in the ticket's title. `store.markWithdrawalStuck` only fires on + // `signed`/`broadcast` — because `stuck` means "a signed payment needs a human" — so for a + // `pending` row it is a silent no-op and the queue is exactly as blocked as before. + const sim = new Sim('stuck') + .queue('a', unfundedTreasury()) + .queue('b', buildsFine()) + .run(6 * 60 * 60_000) + + assert.equal(sim.stuckMarks, 0, 'a `pending` row cannot be marked stuck') + assert.equal(sim.row('a').status, 'pending') + assert.equal(sim.row('b').status, 'pending', 'the chain is still blocked') + assert.equal(sim.signatures, 0) +}) + +test('CF-24: the retry is bounded, and the queue behind it runs', () => { + const sim = new Sim() + .queue('a', unfundedTreasury()) + .queue('b', buildsFine()) + .queue('c', buildsFine()) + .run(6 * 60 * 60_000) + + // The head is refunded once the deadline passes: nothing was signed, so the money is the + // user's again and the row leaves the queue. + assert.equal(sim.row('a').status, 'failed') + assert.equal(sim.refunds, 1, 'refunded more than once — the `pending` gate did not hold') + assert.equal(sim.balance, ONE) + // Everything behind it then goes through. + assert.equal(sim.row('b').status, 'confirmed') + assert.equal(sim.row('c').status, 'confirmed') + assert.equal(sim.signatures, 2) + // And the doomed build stopped being attempted at the deadline rather than forever: one + // attempt per tick for an hour, and none of the five hours after it. + const attemptsWhileWaiting = Math.ceil(STUCK_MS / POLL_MS) + 1 + assert.ok( + sim.buildAttempts <= attemptsWhileWaiting + 2, + `kept rebuilding after the deadline: ${sim.buildAttempts} attempts`, + ) +}) + +test('CF-24: the refund happens at the deadline, not before it', () => { + // The bound has to be the stuck deadline and not "the first failure", or a node that is + // down for one poll would refund every withdrawal queued behind it. + const sim = new Sim().queue('a', unfundedTreasury()).run(STUCK_MS - POLL_MS) + assert.equal(sim.refunds, 0, 'gave up before the deadline') + assert.equal(sim.row('a').status, 'pending') + + sim.run(2 * POLL_MS) + assert.equal(sim.refunds, 1, 'never gave up after the deadline') + assert.equal(sim.row('a').status, 'failed') +}) + +test('CF-24: a queue of stale rows clears in one tick, not one row per poll', () => { + // Three unbuildable rows queued together. A refunded row signed nothing, so the treasury is + // untouched and the next row may have its turn immediately; without that, the last of them + // waits three polls and a hundred of them wait half an hour. + const sim = new Sim() + .queue('a', unfundedTreasury()) + .queue('b', unfundedTreasury()) + .queue('c', unfundedTreasury()) + .queue('d', buildsFine()) + .run(STUCK_MS + POLL_MS) + + const ticksAfterDeadline = sim.ticks - Math.ceil(STUCK_MS / POLL_MS) + assert.ok(ticksAfterDeadline <= 2, `the sim ran on past the deadline: ${ticksAfterDeadline} ticks`) + assert.equal(sim.refunds, 3) + assert.equal(sim.balance, 3n * ONE) + // One SIGNATURE per chain per tick is unchanged — the loop only continues past a row that + // was refunded, which is a row that left the treasury exactly as it found it. + assert.equal(sim.signatures, 1) + assert.equal(sim.row('d').status, 'signed') +}) + +test('CF-24: a permanent refusal still refunds immediately, and lets the next row start', () => { + const sim = new Sim() + .queue('a', () => { + throw new UnsupportedDestinationError('ETH', '0xcafe') + }) + .queue('b', buildsFine()) + .run(POLL_MS) + + assert.equal(sim.ticks, 1) + assert.equal(sim.row('a').status, 'failed') + assert.equal(sim.balance, ONE) + assert.equal(sim.row('b').status, 'signed', 'a permanent refusal cost the queue a whole poll') +}) + +test('every build failure has an exit, and only a plausibly-transient one waits for the deadline', () => { + const treasury = planBuildFailure(new InsufficientTreasuryError('ETH', 1n, 2n)) + assert.equal(treasury.classification, 'treasury') + assert.equal(treasury.refund, 'at-deadline') + assert.deepEqual(treasury.detail, { available: '1', needed: '2' }) + + const destination = new UnsupportedDestinationError('ETH', '0xcafe') + const refusedDestination = planBuildFailure(destination) + assert.equal(refusedDestination.classification, 'destination') + assert.equal(refusedDestination.refund, 'now') + // The one refusal a user can act on, so they are told what it actually was. + assert.equal(refusedDestination.reason, destination.message) + + for (const err of [new SignRefusedError('purpose_forbidden', 'no'), new UnsupportedChainError('BTC')]) { + const plan = planBuildFailure(err) + assert.equal(plan.classification, 'refused') + assert.equal(plan.refund, 'now') + } + + // Anything else at all. This is the arm that had no exit. + for (const err of [ + new Error('xrp account_info: actNotFound'), + new Error('fetch failed'), + 'a string nobody threw on purpose', + null, + ]) { + const plan = planBuildFailure(err) + assert.equal(plan.classification, 'unclassified') + assert.equal(plan.refund, 'at-deadline') + assert.ok(plan.reason.includes('returned to your balance')) + } +}) + +test('the stuck deadline is measured from the broadcast when there was one, and the queue otherwise', () => { + const now = 10 * STUCK_MS + const queued = { broadcastAt: null, createdAt: new Date(now - STUCK_MS - 1) } + assert.equal(stuckDeadlinePassed(queued, now), true) + assert.equal(stuckDeadlinePassed({ ...queued, createdAt: new Date(now - STUCK_MS) }, now), false) + + // A row that was broadcast an instant ago has not been waiting, however old it is. + const old = { broadcastAt: new Date(now - 1000), createdAt: new Date(now - 100 * STUCK_MS) } + assert.equal(stuckDeadlinePassed(old, now), false) +}) diff --git a/services/pay/src/withdrawer.ts b/services/pay/src/withdrawer.ts index 1d5d692..46f6bf3 100644 --- a/services/pay/src/withdrawer.ts +++ b/services/pay/src/withdrawer.ts @@ -10,6 +10,7 @@ import { outboundStatus, UnsupportedChainError, UnsupportedDestinationError, + type SignedOutbound, } from './outbound.js' import { activeWithdrawals, @@ -43,6 +44,12 @@ import { ensureTreasury } from './treasury.js' * nothing was signed at all, or a node reports the transaction as applied-and-failed. * Anything else that does not resolve becomes `stuck`, which is a request for a human, not * a refund. + * + * Those two rules split every giving-up decision by whether a signature exists. A `pending` + * withdrawal has signed nothing, so every way it can fail ends in a refund — immediately for + * a permanent refusal, and at the stuck deadline for one that might have cleared + * (`planBuildFailure`). A `signed` or `broadcast` one ends in `stuck` and waits for a human, + * because the bytes may still land. * ------------------------------------------------------------------ */ /** Groups the active set by chain, because outbound payments are serial PER chain. */ @@ -57,13 +64,128 @@ function byChain(rows: WithdrawalRow[]): Map { return groups } -function stuckDeadlinePassed(row: WithdrawalRow): boolean { +/** + * Has this withdrawal been unresolved for longer than an operator agreed to wait? + * + * Dated from the broadcast where there was one and from the queue otherwise, so the clock a + * `pending` row is measured against is the one the USER has been waiting on. Takes `now` + * because it is the deadline every giving-up decision in this file is made against and a + * test has to be able to stand on both sides of it. + */ +export function stuckDeadlinePassed( + row: Pick, + now = Date.now(), +): boolean { const since = (row.broadcastAt ?? row.createdAt).getTime() - return Date.now() - since > Math.max(1, env.withdrawalStuckMinutes) * 60_000 + return now - since > Math.max(1, env.withdrawalStuckMinutes) * 60_000 +} + +/** + * What to do with a `pending` withdrawal whose build threw, before anything was signed. + * + * EVERY BUILD FAILURE HAS AN EXIT, and the only question this answers is whether it is + * immediate or bounded. That is the whole of CF-24: the four classified refusals below each + * ended the withdrawal, and everything else — an XRP treasury the ledger has never heard of + * (`xrp account_info: actNotFound`), a keyvault `ensureTreasury` cannot reach, a node that + * answers nothing — fell through to a bare `will retry` with no deadline on the retrying. A + * withdrawal that can never be built then sat `pending` forever with the user's balance + * debited, and because a chain starts only its OLDEST `pending` row per tick (see the worker + * below, where outbound payments are serial per chain), it took every other withdrawal on + * that chain down with it. + * + * THE EXIT IS A REFUND AND NOT `stuck`. In this file `stuck` means "a signed payment needs a + * human", and `store.markWithdrawalStuck` is gated on `signed`/`broadcast` precisely so that + * meaning cannot drift — calling it for a `pending` row is a silent no-op and would leave the + * head of the queue exactly where it was. Nothing on this path has been signed (the signature + * is made and committed after this catch, and a signature made and not committed is + * discarded unbroadcast), so giving the money back is safe and it is what the three + * permanent arms already do. + * + * Pure, and separate from the effects, so the policy can be asserted per error shape without + * a database or a chain — `withdrawer.test.ts`. + */ +export interface BuildFailurePlan { + /** Which of the four refusal shapes this is. On the log line, and load-bearing in a test. */ + classification: 'treasury' | 'destination' | 'refused' | 'unclassified' + /** The log line. Kept distinct per shape so a log search can still tell them apart. */ + message: string + /** Log context only one of the shapes carries. */ + detail: Record + /** + * `now` for a refusal that cannot change (a destination with code at it, a signer that + * says no, a chain that cannot send); `at-deadline` for one that plausibly clears on its + * own, which is retried until `stuckDeadlinePassed` and then given up on. + */ + refund: 'now' | 'at-deadline' + /** What the user is told when the money goes back. */ + reason: string + /** The line logged once the refund has actually happened. */ + refunded: string +} + +export function planBuildFailure(err: unknown): BuildFailurePlan { + if (err instanceof InsufficientTreasuryError) { + // Not the user's fault and not a permanent refusal: the treasury needs funding (or + // sweeping). The withdrawal stays queued and is retried, and only gives up — with a + // refund, since nothing was signed — once the stuck deadline passes. + return { + classification: 'treasury', + message: 'withdrawal waiting: the treasury cannot cover it', + detail: { available: err.available.toString(), needed: err.needed.toString() }, + refund: 'at-deadline', + reason: 'this withdrawal could not be funded in time and has been returned to your balance', + refunded: 'withdrawal refunded: treasury never covered it', + } + } + if (err instanceof UnsupportedDestinationError) { + // Permanent, and the only refusal here whose cause is something the USER can act on — + // so the message is theirs rather than the generic one. Code at an address does not + // go away, and nothing has been signed, so the money goes straight back. + return { + classification: 'destination', + message: 'withdrawal destination refused', + detail: {}, + refund: 'now', + reason: err.message, + refunded: 'withdrawal refunded: the destination was refused', + } + } + if (err instanceof SignRefusedError || err instanceof UnsupportedChainError) { + // Permanent. Nothing has been signed, so the money goes straight back. + return { + classification: 'refused', + message: 'withdrawal refused before signing', + detail: {}, + refund: 'now', + reason: 'this withdrawal could not be signed and has been returned to your balance', + refunded: 'withdrawal refunded: it was refused before signing', + } + } + // Unclassified: assume transient, because most of them are, and bound the assumption. + return { + classification: 'unclassified', + message: 'withdrawal build failed — will retry until the stuck deadline', + detail: {}, + refund: 'at-deadline', + reason: 'this withdrawal could not be prepared in time and has been returned to your balance', + refunded: 'withdrawal refunded: it could never be built', + } } +/** + * What one attempt at the head of a chain's queue left behind. + * + * `retired` is the only one the worker may act on by moving to the next row in the same + * tick: it means the row was refunded, so it is out of the queue and — this is the part that + * makes it safe — nothing of it was ever signed, so the chain's treasury is still untouched + * and the serial rule below is not being broken. `held` covers both "bytes now exist for + * this row" and "it is still queued and keeps its place", and both mean the chain is done + * for this tick. + */ +type StartResult = 'held' | 'retired' + /** Build, sign, commit and broadcast one queued withdrawal. */ -async function startWithdrawal(row: WithdrawalRow, log: FastifyBaseLogger): Promise { +async function startWithdrawal(row: WithdrawalRow, log: FastifyBaseLogger): Promise { const coin = row.coin as DepositCoin const network = row.network as ChainNetwork const info = depositChainInfo(coin) @@ -71,7 +193,7 @@ async function startWithdrawal(row: WithdrawalRow, log: FastifyBaseLogger): Prom const fee = BigInt(row.fee) const send = amount - fee - let signed: { rawTx: string; txid: string | null } + let signed: SignedOutbound try { const treasury = await ensureTreasury(coin, network, log) signed = await buildAndSign(coin, { @@ -83,52 +205,38 @@ async function startWithdrawal(row: WithdrawalRow, log: FastifyBaseLogger): Prom log, }) } catch (err) { - if (err instanceof InsufficientTreasuryError) { - // Not the user's fault and not a permanent refusal: the treasury needs funding (or - // sweeping). The withdrawal stays queued and is retried, and only gives up — with a - // refund, since nothing was signed — once the stuck deadline passes. - log.error( - { coin, network, withdrawalId: row.id, available: err.available.toString(), needed: err.needed.toString() }, - 'withdrawal waiting: the treasury cannot cover it', - ) - if (stuckDeadlinePassed(row)) { - await refundWithdrawal( - row.id, - 'this withdrawal could not be funded in time and has been returned to your balance', - ['pending'], - ) - log.warn({ withdrawalId: row.id, coin }, 'withdrawal refunded: treasury never covered it') - } - return - } - if (err instanceof UnsupportedDestinationError) { - // Permanent, and the only refusal here whose cause is something the USER can act on — - // so the message is theirs rather than the generic one. Code at an address does not - // go away, and nothing has been signed, so the money goes straight back. - log.error({ err, coin, network, withdrawalId: row.id, destination: row.destination }, 'withdrawal destination refused') - await refundWithdrawal(row.id, err.message, ['pending']) - return - } - if (err instanceof SignRefusedError || err instanceof UnsupportedChainError) { - // Permanent. Nothing has been signed, so the money goes straight back. - log.error({ err, coin, network, withdrawalId: row.id }, 'withdrawal refused before signing') - await refundWithdrawal( - row.id, - 'this withdrawal could not be signed and has been returned to your balance', - ['pending'], - ) - return + const plan = planBuildFailure(err) + log.error( + { + err, + coin, + network, + withdrawalId: row.id, + destination: row.destination, + classification: plan.classification, + ...plan.detail, + }, + plan.message, + ) + if (plan.refund === 'at-deadline' && !stuckDeadlinePassed(row)) return 'held' + const refunded = await refundWithdrawal(row.id, plan.reason, ['pending']) + if (!refunded) { + // The row moved out of `pending` between the failed build and the refund, so someone + // else owns it now. Nothing was signed here; leave it alone and let the next tick read + // whatever it became. + log.warn({ withdrawalId: row.id, coin }, 'withdrawal was no longer pending when it was refunded') + return 'held' } - log.error({ err, coin, network, withdrawalId: row.id }, 'withdrawal build failed — will retry') - return + log.warn({ withdrawalId: row.id, coin }, plan.refunded) + return 'retired' } // The commit that makes everything after it recoverable. If another worker got here first // this returns false and the signature we just made is discarded UNBROADCAST, which is why // it is safe to have made it at all. - if (!(await markWithdrawalSigned(row.id, signed.rawTx, signed.txid))) { + if (!(await markWithdrawalSigned(row.id, signed))) { log.warn({ withdrawalId: row.id }, 'withdrawal was already signed elsewhere — discarding this signature') - return + return 'held' } log.info( { @@ -143,6 +251,7 @@ async function startWithdrawal(row: WithdrawalRow, log: FastifyBaseLogger): Prom 'withdrawal signed', ) await sendBytes({ ...row, rawTx: signed.rawTx, txid: signed.txid }, log) + return 'held' } /** Broadcast committed bytes and record the transaction id the network knows them by. */ @@ -247,10 +356,19 @@ export function startWithdrawalWorker(log: FastifyBaseLogger) { } // Oldest first, and only when nothing else from this chain's treasury is in the // air. Everything behind it waits its turn rather than racing it. - const next = group.find((r) => r.status === 'pending') - if (!next) return - if (await hasUnsettledOutbound(next.coin, next.network)) return - await startWithdrawal(next, log) + const queued = group.filter((r) => r.status === 'pending') + if (queued.length === 0) return + if (await hasUnsettledOutbound(queued[0].coin, queued[0].network)) return + for (const next of queued) { + // At most one SIGNATURE per chain per tick — the loop only continues past a + // row that was refunded, which signed nothing and left the treasury exactly as + // it found it. Without this, a chain whose head is a withdrawal that can never + // be built clears one row per poll: the refund frees the queue, but the row + // behind it waits another 20 seconds for its turn, and 50 such rows wait + // sixteen minutes. Anything else — signed, or still queued and retrying — is + // this chain's turn used up. + if ((await startWithdrawal(next, log)) !== 'retired') break + } } catch (err) { log.error({ err, chain }, 'withdrawal worker: chain failed this tick') } diff --git a/services/pay/src/xrp.test.ts b/services/pay/src/xrp.test.ts index 47dd525..28f62cf 100644 --- a/services/pay/src/xrp.test.ts +++ b/services/pay/src/xrp.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict' import { test } from 'node:test' import { xrpCreditableBalance } from './chains.js' -import { markNeedsRebase, observeDeposit } from './store.js' +import { currentMarkBasis, depositPaymentTxid, markNeedsRebase, observeDeposit } from './store.js' /** * XRP's account reserve, driven through a whole deposit lifecycle. @@ -141,3 +141,202 @@ test('the one-time restatement of an XRP mark cannot swallow a real regression', assert.equal(markNeedsRebase('ETH', null, 100_000_000n, 0n), false) assert.equal(markNeedsRebase('BTC', null, 100_000_000n, 0n), false) }) + +/* ------------------------------------------------------------------------------------------ + * CF-45: the restatement can walk an address back onto a payment id it has already used. + * + * The mark is a running maximum, so the totals an address records are strictly increasing and + * a payment id derived from the total alone is unique. The restatement above is the ONE thing + * that breaks that premise: it lowers the mark by a base reserve without crediting anything, + * and the address then climbs back through totals it has already recorded. + * ---------------------------------------------------------------------------------------- */ + +/** How a build derives the payment identity for a credit. */ +type TxidFn = (address: string, basis: string | null, observed: bigint) => string + +/** What shipped before this fix: the coin, the address and the confirmed total, nothing else. */ +const totalOnlyTxid: TxidFn = (address, _basis, observed) => `XRP:${address}:${observed}` + +/** What ships now. The basis is part of the identity. */ +const shippedTxid: TxidFn = (address, basis, observed) => + depositPaymentTxid('XRP', address, basis, observed) + +interface Outcome { + credited: bigint + /** Ticks that computed a positive delta and then refused it on the unique index. */ + duplicates: number + rebases: number +} + +/** + * `store.recordDepositAndCredit` for one XRP address, with the payment identity swappable. + * + * Every rule here is the production one — `xrpCreditableBalance` for the probe, + * `observeDeposit` for the arithmetic, `markNeedsRebase` for the fence, and the same order of + * operations (rebase before crediting, insert before advancing the mark). The only thing that + * varies between the two runs is how the inserted row is keyed, which is the whole defect. + */ +class Address { + private lastSeen = 0n + private markBasis: string | null = null + /** `deposit_payments` for this address, by `(address, txid)`. */ + private readonly txids = new Set() + readonly out: Outcome = { credited: 0n, duplicates: 0, rebases: 0 } + + constructor( + private readonly txid: TxidFn, + private readonly address = 'rAbc', + ) {} + + /** + * Seed the state a real address is in when this build first runs against it: credited on the + * OLD basis, and holding payment rows written by the OLD build — so they carry the old, + * total-only id whichever derivation this run uses. That is the deployment, not a fixture + * convenience: changing the derivation does not rewrite rows already in the table. + */ + creditedOnOldBasis(rawBalance: bigint): this { + this.lastSeen = rawBalance + this.txids.add(totalOnlyTxid(this.address, null, rawBalance)) + this.out.credited += rawBalance + return this + } + + /** One watcher tick against an account holding `rawBalance` drops. */ + tick(rawBalance: bigint): this { + const confirmed = xrpCreditableBalance(rawBalance, RESERVE) + const prev = this.lastSeen + const seen = observeDeposit( + { confirmed, tip: confirmed, cumulative: false, confirmedIsExact: true, tipIsExact: true }, + prev, + { recorded: 0n, inFlight: 0n, inFlightMined: 0n }, + ) + const basis = currentMarkBasis('XRP') + if (basis && this.markBasis !== basis) { + const restate = markNeedsRebase('XRP', this.markBasis, prev, seen.observed) + this.markBasis = basis + if (restate) { + this.lastSeen = seen.observed + this.out.rebases += 1 + return this + } + } + if (seen.shortfall > 0n || seen.delta === 0n) return this + const txid = this.txid(this.address, basis, seen.observed) + // `.onConflictDoNothing()` on the `(address, txid)` unique index: nothing is credited and + // the high-water mark is NOT advanced, so the next tick recomputes the identical total. + if (this.txids.has(txid)) { + this.out.duplicates += 1 + return this + } + this.txids.add(txid) + this.lastSeen = seen.observed + this.out.credited += seen.delta + return this + } +} + +/** 20 XRP in drops — the raw balance of an account credited before the reserve change. */ +const OLD_MARK = 20_000_000n + +test('the restated mark walks an XRP address back onto a payment id it already used', () => { + // The address was credited 20 XRP on the old basis (the mark WAS the raw balance, reserve + // included), so `deposit_payments` holds one row keyed on the total 20000000. + const before = new Address(totalOnlyTxid).creditedOnOldBasis(OLD_MARK) + // First probe on the new basis: creditable is 20 - 10 = 10 XRP, the mark restates down to + // 10000000, and nothing is credited. Exactly what MARK_BASIS is for. + before.tick(OLD_MARK) + assert.equal(before.out.rebases, 1) + + // The user now deposits exactly one base reserve: balance 30 XRP, creditable 20 XRP. The + // total is back on 20000000 — a value this address recorded under the OLD basis — so the + // id collides, the insert does nothing and the mark stays behind at 10000000. + before.tick(OLD_MARK + RESERVE) + assert.equal(before.out.duplicates, 1) + assert.equal(before.out.credited, OLD_MARK, 'the 10 XRP deposit was not credited') + + // And because the mark did not advance, the next tick recomputes the same total and refuses + // again. This is the latch: a warn line per tick and a depositor who is not paid. + before.tick(OLD_MARK + RESERVE).tick(OLD_MARK + RESERVE) + assert.equal(before.out.duplicates, 3) + assert.equal(before.out.credited, OLD_MARK) +}) + +test('naming the mark basis in the payment id credits the deposit that used to collide', () => { + const after = new Address(shippedTxid).creditedOnOldBasis(OLD_MARK) + after.tick(OLD_MARK) + assert.equal(after.out.rebases, 1, 'the restatement itself is unchanged — it still fires once') + + // The same deposit of exactly one reserve. `XRP:rAbc:xrp-reserve-excluded:20000000` is not + // the row on file (`XRP:rAbc:20000000`), because the two totals were computed under two + // different definitions of "confirmed" and are not the same reading. + after.tick(OLD_MARK + RESERVE) + assert.equal(after.out.duplicates, 0) + assert.equal(after.out.credited - OLD_MARK, RESERVE, 'the 10 XRP deposit was credited, once') + + // Still idempotent. Repeated ticks against an unchanged balance compute delta 0 and credit + // nothing — the mark advanced, which is what the collision prevented. + after.tick(OLD_MARK + RESERVE).tick(OLD_MARK + RESERVE) + assert.equal(after.out.credited - OLD_MARK, RESERVE) + assert.equal(after.out.duplicates, 0) +}) + +test('the latch is a stall and not a loss: the next deposit pays the stalled amount too', () => { + // The register says the address logs the warning "forever" and that no route clears it. Both + // reviewer notes correct that, and this is the correction: the latch persists only while + // `observed` stays pinned on the collided value. Any further movement — another deposit, or + // an operator recording a hand sweep, which raises `swept` and therefore `observed` — mints + // a fresh id and credits the whole delta from the restated mark, stalled amount included. + const before = new Address(totalOnlyTxid).creditedOnOldBasis(OLD_MARK) + before.tick(OLD_MARK) + before.tick(OLD_MARK + RESERVE).tick(OLD_MARK + RESERVE) + assert.equal(before.out.duplicates, 2) + + // 5 XRP more. Creditable 25 XRP, a total never recorded, so it credits 25 - 10 = 15 XRP: + // the 5 that just arrived plus the 10 that had been stalled. + before.tick(OLD_MARK + RESERVE + 5_000_000n) + assert.equal(before.out.credited - OLD_MARK, RESERVE + 5_000_000n) + assert.equal(before.out.duplicates, 2, 'and no further refusals') +}) + +test('only a top-up of exactly the reserve could collide, and it no longer does', () => { + // Reachability, per the reviewer notes: the restatement lowers the mark by exactly one + // reserve R, so the only recorded totals within reach are the ones inside that window. A + // top-up of anything other than R lands on a total this address never recorded and credits + // normally even on the old derivation. Both sizes are safe on the new one. + for (const [derivation, name] of [ + [totalOnlyTxid, 'total-only'], + [shippedTxid, 'basis-keyed'], + ] as const) { + for (const topUp of [RESERVE - 1n, RESERVE + 1n, RESERVE * 2n]) { + const addr = new Address(derivation).creditedOnOldBasis(OLD_MARK) + addr.tick(OLD_MARK).tick(OLD_MARK + topUp) + assert.equal(addr.out.duplicates, 0, `${name}: ${topUp} drops should not collide`) + assert.equal(addr.out.credited - OLD_MARK, topUp) + } + } +}) + +test('the payment id names the basis it was computed under, and only that changes', () => { + // A coin whose mark has never been restated is keyed `v0`, so the id has the same shape for + // every coin and a coin that acquires its first basis later is in XRP's position rather than + // a new one. + assert.equal(currentMarkBasis('EMBER'), null) + assert.equal(depositPaymentTxid('EMBER', '0xabc', null, 10n ** 19n), 'EMBER:0xabc:v0:10000000000000000000') + assert.equal( + depositPaymentTxid('XRP', 'rAbc', currentMarkBasis('XRP'), 20_000_000n), + 'XRP:rAbc:xrp-reserve-excluded:20000000', + ) + // Two readings of the same number under two definitions of "confirmed" are two ids... + assert.notEqual( + depositPaymentTxid('XRP', 'rAbc', null, 20_000_000n), + depositPaymentTxid('XRP', 'rAbc', 'xrp-reserve-excluded', 20_000_000n), + ) + // ...and neither of them is the id the old build wrote, which is what stops the collision. + assert.notEqual(depositPaymentTxid('XRP', 'rAbc', 'xrp-reserve-excluded', 20_000_000n), 'XRP:rAbc:20000000') + // The derivation is still a pure function of the locked total, so two overlapping ticks that + // read the same total mint the same id. That is the property the unique index rests on. + assert.equal( + depositPaymentTxid('XRP', 'rAbc', 'xrp-reserve-excluded', 20_000_000n), + depositPaymentTxid('XRP', 'rAbc', 'xrp-reserve-excluded', 20_000_000n), + ) +})