From 1963e53072c771ad14e687498760083bf3155542 Mon Sep 17 00:00:00 2001 From: ety001 Date: Mon, 27 Jul 2026 08:25:58 +0800 Subject: [PATCH] fix(recovery): roll back confirm record to 'confirmed' on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CAS in recovery/confirm sets status to 'processing' but only the success path transitions to 'closed'. Every failure after the claim — owner-key mismatch, conveyor config missing (503), requestAccountRecovery RPC error — left the record permanently stuck in 'processing', so the user could never retry and the subsequent recover-account broadcast (which requires status='closed') was wedged. A single transient RPC error would brick a recovery request. Add a rollbackToConfirmed() helper that resets 'processing' → 'confirmed' (CAS-guarded on status='processing' to avoid clobbering concurrent success) and call it on every failure path: owner-key mismatch, conveyor preflight, and the catch block. The catch-block rollback is best-effort. Tests: add rollback assertions for the RPC-error and conveyor-missing paths; extend setupUpdateMocks to chain a third update (rollback). --- src/app/api/recovery/confirm/route.ts | 36 ++++++++++++++- tests/unit/recovery-confirm-route.test.ts | 56 +++++++++++++++++++---- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/app/api/recovery/confirm/route.ts b/src/app/api/recovery/confirm/route.ts index 958e935f..bb13b022 100644 --- a/src/app/api/recovery/confirm/route.ts +++ b/src/app/api/recovery/confirm/route.ts @@ -64,6 +64,9 @@ export async function POST(request: NextRequest) { ); } + // Track whether the CAS claim succeeded so we can roll it back on failure. + let claimed = false; + try { // Step 1: Atomically claim the record by setting status to 'processing'. // This prevents TOCTOU races — only one request will win the CAS. @@ -86,6 +89,7 @@ export async function POST(request: NextRequest) { { status: 400 } ); } + claimed = true; // Step 1b: Cross-validate old_owner_key against the DB record. // This ensures the client-submitted key matches the original request. @@ -94,6 +98,7 @@ export async function POST(request: NextRequest) { columns: { id: true, ownerKey: true }, }); if (!record || (record.ownerKey && record.ownerKey !== body.old_owner_key)) { + await rollbackToConfirmed(db, body.code, body.account_name); return NextResponse.json( { status: 'error', error: 'Owner key mismatch' }, { status: 400 } @@ -101,7 +106,6 @@ export async function POST(request: NextRequest) { } // Step 2: Call kingdom.recovery_account (broadcasts request_account_recovery on-chain). - // If this fails, the record stays in 'processing' and won't be re-processed. const { SteemService } = await import('@/lib/steem/server'); // Preflight: the recovery-signing key (CONVEYOR_POSTING_WIF) is a @@ -110,6 +114,7 @@ export async function POST(request: NextRequest) { const conveyorError = SteemService.validateConveyorConfig(); if (conveyorError) { console.error('Recovery confirm blocked:', conveyorError); + await rollbackToConfirmed(db, body.code, body.account_name); return NextResponse.json( { status: 'error', error: 'Recovery service unavailable' }, { status: 503 } @@ -140,9 +145,38 @@ export async function POST(request: NextRequest) { return NextResponse.json({ status: 'ok' }); } catch (err) { console.error('Recovery confirm failed:', err); + // Roll back the CAS claim so the user can retry. Without this the record + // would be stuck in 'processing' forever (no other path resets it), and a + // single transient RPC error would permanently brick the recovery. + if (claimed) { + await rollbackToConfirmed(db, body.code, body.account_name).catch(() => {}); + } return NextResponse.json( { status: 'error', error: 'Internal server error' }, { status: 500 } ); } } + +/** + * Revert a recovery record from 'processing' back to 'confirmed' so the user + * can retry. Best-effort: swallows errors (the response error is already + * decided by the caller). Only resets records still in 'processing' to avoid + * clobbering a concurrent success. + */ +async function rollbackToConfirmed( + db: NonNullable>, + code: string, + accountName: string +): Promise { + await db + .update(arecs) + .set({ status: 'confirmed' }) + .where( + and( + eq(arecs.validationCode, code), + eq(arecs.accountName, accountName), + eq(arecs.status, 'processing') + ) + ); +} diff --git a/tests/unit/recovery-confirm-route.test.ts b/tests/unit/recovery-confirm-route.test.ts index 1cd77e2e..fd2e91a2 100644 --- a/tests/unit/recovery-confirm-route.test.ts +++ b/tests/unit/recovery-confirm-route.test.ts @@ -50,18 +50,22 @@ function makeRequest(body: Record): NextRequest { }); } -function setupUpdateMocks(firstResult: unknown, secondResult?: unknown) { - const chain1: Record> = {}; - chain1.where = vi.fn().mockResolvedValue(firstResult); - chain1.set = vi.fn().mockReturnValue({ where: chain1.where }); +function setupUpdateMocks(firstResult: unknown, secondResult?: unknown, thirdResult?: unknown) { + const makeChain = (result: unknown) => { + const chain: Record> = {}; + chain.where = vi.fn().mockResolvedValue(result ?? undefined); + chain.set = vi.fn().mockReturnValue({ where: chain.where }); + return chain; + }; - const chain2: Record> = {}; - chain2.where = vi.fn().mockResolvedValue(secondResult ?? undefined); - chain2.set = vi.fn().mockReturnValue({ where: chain2.where }); + const chain1 = makeChain(firstResult); + const chain2 = makeChain(secondResult); + const chain3 = makeChain(thirdResult); mockUpdateFn = vi.fn() .mockReturnValueOnce({ set: chain1.set }) - .mockReturnValueOnce({ set: chain2.set }); + .mockReturnValueOnce({ set: chain2.set }) + .mockReturnValueOnce({ set: chain3.set }); } describe('POST /api/recovery/confirm', () => { @@ -176,6 +180,9 @@ describe('POST /api/recovery/confirm', () => { expect(res.status).toBe(400); const data = await res.json(); expect(data.error).toBe('Owner key mismatch'); + // Rollback: the record is reverted from 'processing' to 'confirmed' so the + // user can retry. 3 update calls: CAS claim → findFirst → rollback. + expect(mockUpdateFn).toHaveBeenCalledTimes(2); }); it('allows confirm when DB ownerKey is null (legacy records)', async () => { @@ -215,6 +222,39 @@ describe('POST /api/recovery/confirm', () => { expect(data.status).toBe('error'); }); + it('rolls back to confirmed when requestAccountRecovery throws (retryable)', async () => { + // Regression test: without rollback, a transient RPC error leaves the + // record stuck in 'processing' forever — the user can never retry. + setupUpdateMocks({ affectedRows: 1 }); + + const { SteemService } = await import('@/lib/steem/server'); + vi.mocked(SteemService.requestAccountRecovery).mockRejectedValueOnce( + new Error('Kingdom unreachable') + ); + + const req = makeRequest(validPayload); + const res = await POST(req); + expect(res.status).toBe(500); + + // CAS claim (1st) → rollback in catch (2nd). Success close never runs. + expect(mockUpdateFn).toHaveBeenCalledTimes(2); + }); + + it('rolls back to confirmed when conveyor config is missing (503 retryable)', async () => { + setupUpdateMocks({ affectedRows: 1 }); + + const { SteemService } = await import('@/lib/steem/server'); + vi.mocked(SteemService.validateConveyorConfig).mockReturnValueOnce( + 'CONVEYOR_POSTING_WIF missing' + ); + + const req = makeRequest(validPayload); + const res = await POST(req); + expect(res.status).toBe(503); + // CAS claim (1st) → rollback (2nd). User can retry. + expect(mockUpdateFn).toHaveBeenCalledTimes(2); + }); + it('returns 500 when database update throws', async () => { mockUpdateFn = vi.fn().mockImplementation(() => { throw new Error('Connection lost');