Seven defects in the crypto withdrawal path, found tracing a 400 whose body read
getaddrinfo EAI_AGAIN sandbox-api.fireblocks.io straight through to the user's screen,
plus the four faults that surfaced while testing the fixes end to end on Sepolia.
Money
-
Refund credited one platform fee too much. The refund took
amount + platformFeewhile the debit only ever tookamount, so every failed withdrawal handed the user a free fee. Refund and debit now derive from one rounded net amount. -
A second refund could be written for one withdrawal. Fireblocks redelivers out of order; a stale
SUBMITTEDlanding afterFAILEDwalked the row off its terminal state and re-armed theisFailed && wasNotFailedguard for the nextFAILED. Observed crediting a user twice on a local run. A terminal status can no longer be walked backwards, and the refund now asks the ledger whether it has already paid — the same idempotency keyhasDepositEntryuses, for the same reason: a status is not evidence that money moved. -
The provider call ran inside the DB transaction, ahead of every write. A commit that failed after Fireblocks accepted the transfer left funds gone with no row and no debit. Submission is now three phases: reserve, debit and commit; submit with no transaction open; record the outcome. A definite rejection refunds. An ambiguous failure does not — the transfer may have landed, and refunding would be a double-spend.
-
Nothing resolved those ambiguous rows, so there is now a reconciliation job. It settles a parked withdrawal by re-submitting under the original idempotency key, the only way to learn whether the first attempt landed. It refuses to guess: a row whose key cannot be rebuilt, or that has outlived the provider's dedupe window, or that has failed too many times, is escalated to a human and never auto-refunded.
-
The balance was read unlocked and outside the transaction, so two concurrent withdrawals both passed the same check and the second debit overwrote the first. The row is now locked
FOR UPDATEand validated inside the reservation. -
The idempotency key was seeded with
Date.now(), so it protected nothing on retry. An optional clientrequestIdnow drives a stable key, scoped per user along with the unique index and every lookup — unscoped, one account's id collides into another's withdrawal. A replay of a rejected or under-review reservation is refused rather than answered as success, and an id reused for a different payload is refused rather than silently substituted. -
amountwas validated as a non-empty string, soBigNumber('abc')produced NaN and every comparison against it was false: both the minimum and the balance guard were walked past. Validated now at the DTO and again in the service. -
Rounding could bill more than was asked.
toFixed(6)rounds half-up and the DTO admits eighteen decimal places, so a requested1.4999999against a0.5fee produced a1.000000transfer and a1.500000debit. Rounding down can only leave dust with the user.
What the user is told
-
Failures are classified by HTTP status, never by the sentence beside it. Infrastructure failures answer 503 with copy that does not claim the balance is untouched — it usually is not, because the debit happens before the transfer is attempted. Retry is offered only where pressing again can help, and carries the same
requestId, persisted so it survives the app dying mid-request. -
Only
COMPLETEDshows "Withdraw completed". A withdrawal answeredSUBMITTEDand showed a tick, then failed seconds later for want of gas in the sending vault. -
The result screen now finds out what actually happened. It polled nothing before: it rendered a snapshot taken when the HTTP call returned and never read again, so a withdrawal that failed four seconds later sat on "in progress" indefinitely and the user was told neither that it failed nor that their money was back. It now polls
GET /fireblocks/transactions/:idevery five seconds for three minutes, stopping the moment the status is terminal. It will not treat an unrecognised status as success, will not render failure because the poll itself errored, and will not promise a refund it cannot see in the ledger entries. -
That endpoint answered 500 to every call and had since it was written — one parameter compared against a uuid column and a varchar column, and Postgres types a parameter by its first use. Nothing had called it until now.
-
A background poll's failures no longer raise the global error modal. Requests can mark themselves
backgroundPoll; per-request, not the existing global flag, which a poll on its own timer would use to swallow other requests' errors too.
Verification
- Backend: 1381 tests pass,
yarn buildclean, eslint clean on every touched file. (yarn lintfails repo-wide ondeveloptoo — pre-existing debt in unrelated modules.) - Frontend: 53 suites / 390 tests pass,
pnpm type-checkandpnpm lintclean. - End to end on Sepolia: 3 withdrawals confirmed on chain (4.5 USDC net), 5 failed for want of gas and each refunded exactly once. Ledger balance reconciles to the penny against the destination wallet's on-chain balance.
- Codex review: six rounds, five P1 and one P2, all addressed.
Migrations
Two, both additive with real down methods: nullable request_id, submission_error,
submission_attempts, last_submission_attempt_at; fireblocks_transaction_id becomes
nullable; a partial unique index on (user_id, request_id) and a partial index for the
reconciliation query.
Not addressed
Same class of bug, different routes — flagged rather than fixed to keep this reviewable:
POST /fireblocks/internal-transfer has the same unlocked-balance race; deposits write the
balance cache without the lock; FileLogger kills the process when it cannot write its file;
no screen in the app shows a withdrawal's status once the result screen is closed.