-
Notifications
You must be signed in to change notification settings - Fork 20
fix(cutover): harden migration + rollback from ENG-461 rehearsal findings (ENG-401/483/484) #433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fa95394
4513ff9
e8c8c18
70537ef
0926cc4
bc413f4
b0ac041
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import { Account } from "@services/mongoose/schema" | ||
|
|
||
| export type CashWalletCutoverAccountAuditIssue = { | ||
| accountId: string | ||
| role?: string | ||
| errors: string[] | ||
| } | ||
|
|
||
| export type CashWalletCutoverAccountAuditReport = { | ||
| scanned: number | ||
| invalid: number | ||
| issues: CashWalletCutoverAccountAuditIssue[] | ||
| } | ||
|
|
||
| type ValidatableAccountDocument = { | ||
| _id: { toString(): string } | ||
| role?: string | ||
| validateSync(): { errors: Record<string, { message: string }> } | undefined | ||
| } | ||
|
|
||
| /** | ||
| * Data-quality audit (ENG-484 / cutover runbook D1). The pointer-flip step | ||
| * saves the whole account document, so mongoose re-validates EVERY field — | ||
| * an account with pre-existing invalid data (empty username, null | ||
| * statusHistory entry, ...) fails mid-migration even though the cutover never | ||
| * touched those fields. Both were hit live in the ENG-461 rehearsal. | ||
| * | ||
| * Run before `prepare`: every issue reported here is an account that WILL | ||
| * fail at pointer flip unless fixed (or locked out of the run). | ||
| */ | ||
| export const auditCashWalletCutoverAccounts = async ({ | ||
| listAccountDocuments = () => | ||
| Account.find({}).cursor() as unknown as AsyncIterable<ValidatableAccountDocument>, | ||
| }: { | ||
| listAccountDocuments?: () => AsyncIterable<ValidatableAccountDocument> | ||
| } = {}): Promise<CashWalletCutoverAccountAuditReport> => { | ||
| const report: CashWalletCutoverAccountAuditReport = { | ||
| scanned: 0, | ||
| invalid: 0, | ||
| issues: [], | ||
| } | ||
|
|
||
| for await (const document of listAccountDocuments()) { | ||
| report.scanned += 1 | ||
|
|
||
| const validation = document.validateSync() | ||
| if (validation === undefined) continue | ||
|
|
||
| report.invalid += 1 | ||
| report.issues.push({ | ||
| accountId: document._id.toString(), | ||
| role: document.role, | ||
| errors: Object.values(validation.errors).map(({ message }) => message), | ||
| }) | ||
| } | ||
|
|
||
| return report | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| import { CashWalletCutoverRepository } from "@services/mongoose" | ||
|
|
||
| import { CashWalletMigrationFailedError } from "./errors" | ||
|
|
||
| type CashWalletRetryRepository = ReturnType<typeof CashWalletCutoverRepository> | ||
|
|
||
| export type CashWalletRetryFailedReport = { | ||
| dryRun: boolean | ||
| eligible: number | ||
| retried: number | ||
| /** Migrations skipped because money moved — those are operator-review work. */ | ||
| skippedMoneyMoved: number | ||
| /** Lost races: migrations concurrently moved out of `failed` mid-run. */ | ||
| conflicts: number | ||
| resumedAt: Partial<Record<CashWalletMigrationStatus, number>> | ||
| migrationIds: string[] | ||
| } | ||
|
|
||
| /** | ||
| * Re-drive `failed` migrations (ENG-484). Safe by construction: the runner | ||
| * routes failures in money-moving statuses to `requires_operator_review`, so | ||
| * anything sitting in `failed` has no ambiguous side effects. Resume point is | ||
| * field-driven, mirroring the forward pipeline's guards: | ||
| * | ||
| * - pointer flipped (`previousDefaultWalletId` set) → resume `pointer_flipped`, | ||
| * preserving the recorded pointer so a later rollback stays correct | ||
| * - otherwise → reset to `not_started`, clearing stale progress fields so the | ||
| * machine re-walks provisioning and balance read from scratch | ||
| * | ||
| * Migrations with a recorded payment transaction are never retried here — | ||
| * defense in depth should one ever land in `failed`. | ||
| */ | ||
| export const retryFailedCashWalletMigrations = async ({ | ||
| cutoverVersion, | ||
| runId, | ||
| accountId, | ||
| dryRun = false, | ||
| migrationsRepo = CashWalletCutoverRepository(), | ||
| }: { | ||
| cutoverVersion: number | ||
| runId: string | ||
| /** Single-account mode when set; all failed migrations when omitted. */ | ||
| accountId?: AccountId | ||
| dryRun?: boolean | ||
| migrationsRepo?: CashWalletRetryRepository | ||
| }): Promise<CashWalletRetryFailedReport | ApplicationError> => { | ||
| let candidates: CashWalletMigration[] | ||
|
|
||
| if (accountId !== undefined) { | ||
| const migration = await migrationsRepo.findMigrationByAccountId({ | ||
| accountId, | ||
| cutoverVersion, | ||
| runId, | ||
| }) | ||
| if (migration instanceof Error) return migration | ||
| if (migration === null) { | ||
| return new CashWalletMigrationFailedError( | ||
| `No migration found for accountId=${accountId} runId=${runId}`, | ||
| ) | ||
| } | ||
| if (migration.status !== "failed") { | ||
| return new CashWalletMigrationFailedError( | ||
| `Migration for accountId=${accountId} is ${migration.status}, not failed`, | ||
| ) | ||
| } | ||
| candidates = [migration] | ||
| } else { | ||
| const migrations = await migrationsRepo.listMigrationsByStatuses({ | ||
| cutoverVersion, | ||
| runId, | ||
| statuses: ["failed"], | ||
| }) | ||
| if (migrations instanceof Error) return migrations | ||
| candidates = migrations | ||
| } | ||
|
|
||
| const report: CashWalletRetryFailedReport = { | ||
| dryRun, | ||
| eligible: 0, | ||
| retried: 0, | ||
| skippedMoneyMoved: 0, | ||
| conflicts: 0, | ||
| resumedAt: {}, | ||
| migrationIds: [], | ||
| } | ||
|
|
||
| for (const migration of candidates) { | ||
| if ( | ||
| migration.balanceMovePaymentTransactionId !== undefined || | ||
| migration.feeReimbursementPaymentTransactionId !== undefined | ||
| ) { | ||
| report.skippedMoneyMoved += 1 | ||
| continue | ||
| } | ||
|
|
||
| const resumeStatus: CashWalletMigrationStatus = | ||
| migration.previousDefaultWalletId !== undefined ? "pointer_flipped" : "not_started" | ||
|
|
||
| report.eligible += 1 | ||
| report.migrationIds.push(migration.id) | ||
| report.resumedAt[resumeStatus] = (report.resumedAt[resumeStatus] ?? 0) + 1 | ||
| if (dryRun) continue | ||
|
|
||
| const transitioned = await migrationsRepo.transitionMigration({ | ||
| id: migration.id, | ||
| from: "failed", | ||
| to: resumeStatus, | ||
| cutoverVersion, | ||
| runId, | ||
| patch: { attempts: 0 }, | ||
| clear: | ||
| resumeStatus === "not_started" | ||
| ? [ | ||
| "lastError", | ||
| "sourceBalanceUsdCents", | ||
| "destinationAmountUsdtMicros", | ||
| "destinationStartingBalanceUsdtMicros", | ||
| "feeAmountUsdCents", | ||
| "feeAmountUsdtMicros", | ||
| "balanceMoveInvoicePaymentRequest", | ||
| "balanceMoveInvoicePaymentHash", | ||
| "feeReimbursementInvoicePaymentRequest", | ||
| "feeReimbursementInvoicePaymentHash", | ||
| ] | ||
| : ["lastError"], | ||
| }) | ||
| if (transitioned instanceof Error) { | ||
| // Lost a race (e.g. a concurrent rollback-request moved this migration | ||
| // out of `failed` — the guarded findOneAndUpdate matched nothing). | ||
| // Skip it and keep going: aborting would discard the report of | ||
| // migrations already reset by earlier iterations. | ||
| report.eligible -= 1 | ||
| report.migrationIds.pop() | ||
| report.resumedAt[resumeStatus] = (report.resumedAt[resumeStatus] ?? 1) - 1 | ||
| report.conflicts += 1 | ||
| continue | ||
| } | ||
| report.retried += 1 | ||
| } | ||
|
|
||
| return report | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { assertCanTransition, ROLLBACKABLE_STATUSES } from "./state-machine" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { legacyShortfallUsdtMicros } from "./amount-conversion" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { isInvoicePaymentRequestStale } from "./worker" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { CUTOVER_MIN_PAYABLE_USDT_MICROS, isInvoicePaymentRequestStale } from "./worker" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CashWalletMigrationFailedError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InvalidCashWalletMigrationTransitionError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -40,6 +40,9 @@ type CashWalletRollbackServices = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| readDestinationBalanceUsdtMicros( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| migration: CashWalletMigration, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<string | ApplicationError> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readDestinationSpendableUsdtMicros( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| migration: CashWalletMigration, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<string | ApplicationError> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| invoiceService: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| createNoAmountInvoice(args: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -194,17 +197,37 @@ export const executeCashWalletMigrationRollbackStep = async ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (current.destinationAmountUsdtMicros !== "0") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Fail closed: if the user transacted on the USDT wallet since the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // forward move, reversing the full amount is not possible without | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // operator judgment. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const usdtBalance = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await services.balanceReader.readDestinationBalanceUsdtMicros(current) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (usdtBalance instanceof Error) return usdtBalance | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (BigInt(usdtBalance) < BigInt(current.destinationAmountUsdtMicros)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Reverse the ACTUAL spendable balance (floored), capped at the forward | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // amount — never the raw target. The USDT wallet legitimately holds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // slightly less than target: the un-reimbursed forward routing fee plus | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // sub-micro rounding. Paying the target verbatim overspends → IBEX 400 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (ENG-401). Reversing the spendable balance makes the user whole minus | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // that sub-cent dust. Fail closed only when the wallet is short by MORE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // than tolerance — the signature of the user having spent USDT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // post-cutover, which needs operator judgment. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const target = BigInt(current.destinationAmountUsdtMicros) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const spendableStr = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await services.balanceReader.readDestinationSpendableUsdtMicros(current) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (spendableStr instanceof Error) return spendableStr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const spendable = BigInt(spendableStr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const reverseAmount = spendable < target ? spendable : target | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const shortfall = target - reverseAmount | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (shortfall > ROLLBACK_SHORTFALL_TOLERANCE_USDT_MICROS) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Boundary band where the treasury reimburses post-cutover user spending (low) Step 3's legacy top-up uses the same 10,000-micro strict-greater threshold on Exposure is bounded at ~1 cent + fee + dust per account, so it may be an acceptable tradeoff — but it deserves an explicit comment here, or subtract the step-2 shortfall (known spend) from the step-3 top-up. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new CashWalletMigrationFailedError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `USDT balance ${spendableStr} is short of reverse target ${target} by ${shortfall} micros (> tolerance) — likely spent post-cutover`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // IBEX rejects payments below ~1 sat (ENG-484): a drained or 1-cent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // wallet can pass the tolerance gate with a 0- or sub-minimum reverse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // amount — fail closed with a clear message instead of a doomed pay | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // attempt surfacing as a misleading IBEX 400. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (reverseAmount < CUTOVER_MIN_PAYABLE_USDT_MICROS) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new CashWalletMigrationFailedError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `USDT balance ${usdtBalance} no longer covers reverse amount ${current.destinationAmountUsdtMicros}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Reverse amount ${reverseAmount} micros is below IBEX's minimum payable — target ${target}, spendable ${spendableStr}; resolve manually`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const reverseAmountStr = reverseAmount.toString() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+214
to
+230
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rollback can attempt a 0-micro or sub-minimum-payable reverse payment (low) There's no lower bound on
Suggested change
(Define |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| current.rollbackInvoicePaymentRequest === undefined || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -240,7 +263,7 @@ export const executeCashWalletMigrationRollbackStep = async ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| const payment = await services.paymentService.payInvoice({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| senderWalletId: current.destinationUsdtWalletId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| paymentRequest: current.rollbackInvoicePaymentRequest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| senderAmountUsdtMicros: current.destinationAmountUsdtMicros, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| senderAmountUsdtMicros: reverseAmountStr, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (payment instanceof Error) return payment | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -273,6 +296,14 @@ export const executeCashWalletMigrationRollbackStep = async ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (shortfall instanceof Error) return shortfall | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Accepted boundary tradeoff: step 2 tolerates up to a 1-cent USDT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // shortfall (user spend + un-reimbursed fee), and this top-up covers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // whatever the legacy wallet is short vs the original balance — so up to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ~1 cent of a user's own post-cutover spending can be reimbursed by the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // treasury (spend ≤ tolerance at step 2, but spend + fee + receive dust | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // over it here). Exposure is bounded per account at roughly a cent plus | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // fee plus dust; the old strict fail-closed left every such account in | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // manual review instead. Documented in ENG-484. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (BigInt(shortfall) > ROLLBACK_SHORTFALL_TOLERANCE_USDT_MICROS) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (current.rollbackShortfallPaymentTransactionId !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Treasury already topped up once and the user is still short more | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absorbed fees will mass-flag false “shortfall” rows in the reconciliation audit (medium)
computeCutoverBalanceAudit(operator-dashboard.ts:504–552, untouched by this PR) compares the destination delta againstdestinationAmountUsdtMicroswith zero tolerance and flagsstatus: "shortfall"on any positive deficit. After the forward move the destination wallet holds exactly target − fee, and only the fee reimbursement (paid to the destination wallet) topped it back up.With fees now absorbed, every completed migration with an absorbed fee (88–9999 micros — widespread in the rehearsal) renders as a red shortfall row during the post-run reconciliation hold — drowning real shortfalls exactly when the runbook says to look.
Suggest giving the audit the same sub-cent tolerance the rollback got, or subtracting recorded-but-unpaid
feeAmountUsdtMicros(i.e.feeReimbursementPaymentTransactionIdunset) from the expected minimum.