diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md index 8a0ebee3..5b3cc47e 100644 --- a/PR_DESCRIPTION.md +++ b/PR_DESCRIPTION.md @@ -1,23 +1,28 @@ -# PR Summary +## What does this PR do? -Adds focused tests for three behaviors: +This PR makes background startup and watcher shutdown recoverable, and invokes the declared deposit and SEP-10 challenge plugin hooks. -- sqlite-backed `getInteractiveTransactionById()` returns `null` for unknown transaction IDs -- `TransactionWatcher.stop()` before `start()` is a safe no-op -- webhook route accepts an empty request body and returns a generated `event_id` +- Stops the queue and earlier watchers after a later startup failure while preserving the original error. +- Coordinates watcher start/stop races and tolerates in-flight tick failures during shutdown. +- Runs deposit and challenge hooks in registration order with typed context; hook failures prevent persistence. +- Adds focused lifecycle and HTTP integration tests. -## How to test +## How to test? -- Run `bun test tests/runtime/sql-adapter-interactive-tx.test.ts tests/runtime/transaction-watcher.unit.test.ts tests/mvp-express.integration.test.ts` -- Confirm all tests pass +- Run `bun test` (434 tests pass across 45 files). +- For focused coverage, run `bun test tests/runtime/background-lifecycle.test.ts tests/runtime/transaction-watcher.unit.test.ts tests/mvp-express.integration.test.ts`. ## Checklist -- [x] My code follows the code style of this project. +- [ ] My code follows the code style of this project. - [x] I have added tests for my changes. -- [ ] I have updated the documentation accordingly. -- [x] I have run `bun test` locally. +- [x] I have updated the hook contract documentation. +- [x] I have run `bun run test` locally. +- [ ] I have run `bun run lint` locally. ## Issue Reference -Closes #243, #240, and #245 +Closes #584 +Closes #585 +Closes #589 +Closes #590 diff --git a/src/core/factory.ts b/src/core/factory.ts index e8d83298..a97414de 100644 --- a/src/core/factory.ts +++ b/src/core/factory.ts @@ -17,7 +17,7 @@ import { InMemoryQueueAdapter } from '@/runtime/queue/in-memory-queue.ts'; import { TransactionWatcher } from '@/runtime/watchers/transaction-watcher.ts'; import { DefaultWebhookProcessor } from '@/runtime/webhooks/default-webhook-processor.ts'; import { AnchorKitConfig } from '@/types/config.ts'; -import { AnchorPlugin } from '@/types/plugin.ts'; +import type { AnchorPlugin, AnchorPluginHooks } from '@/types/plugin.ts'; /** * AnchorInstance @@ -106,10 +106,23 @@ export class AnchorInstance { ]; } + const depositRequestHooks: NonNullable[] = []; + const sep10ChallengeHooks: NonNullable[] = []; + for (const plugin of this.plugins.values()) { + if (plugin.hooks?.onDepositRequest) { + depositRequestHooks.push(plugin.hooks.onDepositRequest); + } + if (plugin.hooks?.onSep10Challenge) { + sep10ChallengeHooks.push(plugin.hooks.onSep10Challenge); + } + } + this.expressRouter = new AnchorExpressRouter({ config: this.config, database: this.database, webhookProcessor: this.webhookProcessor, + depositRequestHooks, + sep10ChallengeHooks, pluginRoutes, }).getMiddleware(); diff --git a/src/runtime/http/express-router-impl.ts b/src/runtime/http/express-router-impl.ts index 09f3553d..c4231b78 100644 --- a/src/runtime/http/express-router-impl.ts +++ b/src/runtime/http/express-router-impl.ts @@ -3,6 +3,7 @@ import { ConfigError, PayloadTooLargeError, ValidationError } from '@/core/error import { InMemoryRateLimiter, type RateLimitRule } from '@/runtime/http/rate-limiter.ts'; import type { DatabaseAdapter, WebhookProcessor } from '@/runtime/interfaces.ts'; import type { PluginRouteContext, RouteDefinition } from '@/types/foundation.ts'; +import type { AnchorPluginHooks, DepositRequestBody } from '@/types/plugin.ts'; import { IdempotencyUtils } from '@/utils/idempotency.ts'; import { Account, @@ -45,6 +46,8 @@ export interface ExpressRouterContext { requestTimeout: number; rateLimiter: InMemoryRateLimiter; rateRules: Record<'auth_challenge' | 'auth_token' | 'webhook' | 'deposit', RateLimitRule>; + depositRequestHooks?: NonNullable[]; + sep10ChallengeHooks?: NonNullable[]; pluginRoutes?: Array<{ pluginId: string; route: RouteDefinition }>; } @@ -782,8 +785,29 @@ async function handleAuthChallenge( const challengeTx = challengeBuilder.build(); - challengeTx.sign(context.sep10ServerKeypair); - const challengeXdr = challengeTx.toXDR(); + let hookedChallengeTx = challengeTx; + try { + for (const hook of context.sep10ChallengeHooks ?? []) { + hookedChallengeTx = await hook(hookedChallengeTx); + } + } catch { + sendJson(res, 500, { + error: 'challenge_hook_failed', + message: 'SEP-10 challenge hook failed', + }); + return; + } + + if (extractNonceFromChallenge(hookedChallengeTx) !== nonce) { + sendJson(res, 500, { + error: 'challenge_hook_failed', + message: 'SEP-10 challenge hook returned an invalid transaction', + }); + return; + } + + hookedChallengeTx.sign(context.sep10ServerKeypair); + const challengeXdr = hookedChallengeTx.toXDR(); const expiresAt = new Date(expiresAtUnix * 1000).toISOString(); await context.database.insertAuthChallenge({ @@ -1029,6 +1053,28 @@ async function handleDepositInteractive( return; } + const depositHookContext = { + config: context.config.getConfig(), + db: context.database, + params: {}, + query: {}, + body: parsedBody.body as DepositRequestBody, + account: auth.account, + asset: selectedAsset, + }; + + try { + for (const hook of context.depositRequestHooks ?? []) { + await hook(depositHookContext); + } + } catch { + sendJson(res, 400, { + error: 'deposit_rejected', + message: 'Deposit request was rejected by a plugin', + }); + return; + } + const idempotencyKey = IdempotencyUtils.extractIdempotencyHeader(req.headers, 'idempotency-key'); const IDEMPOTENCY_KEY_MAX_BYTES = 255; diff --git a/src/runtime/http/express-router.ts b/src/runtime/http/express-router.ts index 79285111..d78eb5e8 100644 --- a/src/runtime/http/express-router.ts +++ b/src/runtime/http/express-router.ts @@ -3,6 +3,7 @@ import { errorHandler } from '@/utils/error-handler.ts'; import { InMemoryRateLimiter, type RateLimitRule } from '@/runtime/http/rate-limiter.ts'; import type { DatabaseAdapter, WebhookProcessor } from '@/runtime/interfaces.ts'; import type { RouteDefinition } from '@/types/foundation.ts'; +import type { AnchorPluginHooks } from '@/types/plugin.ts'; import { Keypair } from '@stellar/stellar-sdk'; import type { IncomingMessage, ServerResponse } from 'node:http'; import { @@ -21,6 +22,8 @@ interface RouterDependencies { config: AnchorConfig; database: DatabaseAdapter; webhookProcessor: WebhookProcessor; + depositRequestHooks?: NonNullable[]; + sep10ChallengeHooks?: NonNullable[]; pluginRoutes?: Array<{ pluginId: string; route: RouteDefinition }>; } @@ -52,6 +55,8 @@ export class AnchorExpressRouter { config, database: dependencies.database, webhookProcessor: dependencies.webhookProcessor, + depositRequestHooks: dependencies.depositRequestHooks ?? [], + sep10ChallengeHooks: dependencies.sep10ChallengeHooks ?? [], sep10ServerKeypair, networkPassphrase, maxBodyBytes, diff --git a/src/runtime/watchers/transaction-watcher.ts b/src/runtime/watchers/transaction-watcher.ts index a6e73543..f0412519 100644 --- a/src/runtime/watchers/transaction-watcher.ts +++ b/src/runtime/watchers/transaction-watcher.ts @@ -19,6 +19,7 @@ export class TransactionWatcher implements Watcher { private timer: ReturnType | null = null; private tickPromise: Promise | null = null; private startPromise: Promise | null = null; + private stopPromise: Promise | null = null; constructor(database: DatabaseAdapter, queue: QueueAdapter, options: TransactionWatcherOptions) { this.database = database; @@ -29,6 +30,11 @@ export class TransactionWatcher implements Watcher { } public async start(): Promise { + if (this.stopPromise) { + await this.stopPromise; + return; + } + // If already started, return immediately if (this.timer) return; @@ -58,22 +64,53 @@ export class TransactionWatcher implements Watcher { } public async stop(): Promise { - if (this.timer) { - clearInterval(this.timer); - this.timer = null; - } + if (this.stopPromise) return this.stopPromise; - const activeTick = this.tickPromise; - if (activeTick) { - try { - await activeTick; - } catch { - // Ignore transient tick failures during shutdown so timers and lifecycle state can settle. + this.stopPromise = (async () => { + if (this.timer) { + clearInterval(this.timer); + this.timer = null; } - } - this.isTickInProgress = false; - this.tickPromise = null; + const activeStart = this.startPromise; + if (activeStart) { + try { + await activeStart; + } catch { + // A failed initial tick should not prevent shutdown from settling. + } + } + + if (this.timer) { + clearInterval(this.timer); + this.timer = null; + } + + const activeTick = this.tickPromise; + if (activeTick) { + try { + await activeTick; + } catch { + // Ignore transient tick failures during shutdown so timers and lifecycle state can settle. + } + } + + // An initial tick can finish after the first timer check and create the + // interval while shutdown is waiting for that tick. + if (this.timer) { + clearInterval(this.timer); + this.timer = null; + } + + this.isTickInProgress = false; + this.tickPromise = null; + })(); + + try { + await this.stopPromise; + } finally { + this.stopPromise = null; + } } private async tick(): Promise { diff --git a/src/types/index.ts b/src/types/index.ts index 4cd26262..10cef901 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -38,6 +38,8 @@ export type { AnchorPlugin, AnchorPluginContext, AnchorPluginHooks, + DepositRequestBody, + DepositRequestHookContext, Context as PluginContext, } from './plugin'; export type { RouteDefinition, SchemaDefinition, SepErrorCode } from './foundation'; diff --git a/src/types/plugin.ts b/src/types/plugin.ts index a1033245..7fbaefd4 100644 --- a/src/types/plugin.ts +++ b/src/types/plugin.ts @@ -1,5 +1,8 @@ import { RouteDefinition, SchemaDefinition } from './foundation'; +import type { AnchorKitConfigSnapshot, Asset } from '@/types/config.ts'; import type { TransactionStatus } from './transaction-status.ts'; +import type { DatabaseAdapter } from '@/runtime/interfaces.ts'; +import type { Transaction } from '@stellar/stellar-sdk'; export interface AnchorPluginContext< TConfig = unknown, @@ -17,10 +20,27 @@ export interface AnchorPluginContext< export type Context = AnchorPluginContext; +export type DepositRequestBody = Record & { + asset_code: string; + amount: string | number; +}; + +export interface DepositRequestHookContext extends AnchorPluginContext< + AnchorKitConfigSnapshot, + DatabaseAdapter, + DepositRequestBody, + Record, + Record +> { + account: string; + asset: Asset; +} + export interface AnchorPluginHooks { - onDepositRequest?: (ctx: Context) => Promise; + /** Throw to reject with HTTP 400 `deposit_rejected` before persistence. */ + onDepositRequest?: (ctx: DepositRequestHookContext) => Promise; onWithdrawalRequest?: (ctx: Context) => Promise; - onSep10Challenge?: (tx: unknown) => Promise; + onSep10Challenge?: (tx: Transaction) => Promise; onTransactionStatusChange?: ( tx: unknown, oldStatus: TransactionStatus, diff --git a/tests/mvp-express.integration.test.ts b/tests/mvp-express.integration.test.ts index 8c301548..4fcebd16 100644 --- a/tests/mvp-express.integration.test.ts +++ b/tests/mvp-express.integration.test.ts @@ -1,6 +1,7 @@ import { makeSqliteDbUrlForTests } from '@/core/factory.ts'; import { createAnchor, type AnchorInstance } from '@/index.ts'; import type { DatabaseAdapter } from '@/runtime/interfaces.ts'; +import type { AnchorPlugin } from '@/types/plugin.ts'; import { ACCESS_TOKEN_AUDIENCE, ACCESS_TOKEN_ISSUER } from '@/runtime/http/express-router-impl.ts'; import { Account, Keypair, Operation, Transaction, TransactionBuilder } from '@stellar/stellar-sdk'; import { createHash, createHmac } from 'node:crypto'; @@ -130,6 +131,51 @@ describe('MVP Express-mounted integration', () => { let transactionId = ''; let depositInteractiveUrl = ''; + async function createPluginTestAnchor(plugins: AnchorPlugin[]) { + const pluginDatabaseUrl = makeSqliteDbUrlForTests(); + const config = anchor.config.getConfig(); + const pluginAnchor = createAnchor({ + ...config, + server: { + ...config.server, + corsOrigins: config.server.corsOrigins ? [...config.server.corsOrigins] : undefined, + }, + assets: { + ...config.assets, + assets: config.assets.assets.map((asset) => ({ ...asset })), + }, + kycRequired: config.kycRequired + ? Object.fromEntries( + Object.entries(config.kycRequired).map(([key, requirements]) => [ + key, + [...requirements], + ]), + ) + : undefined, + framework: { + ...config.framework, + plugins: config.framework.plugins?.map((plugin) => ({ ...plugin })), + database: { ...config.framework.database, url: pluginDatabaseUrl }, + }, + }); + + for (const plugin of plugins) pluginAnchor.use(plugin); + await pluginAnchor.init(); + + return { + invoke: createMountedInvoker(pluginAnchor), + database: (pluginAnchor as unknown as { database: DatabaseAdapter }).database, + async cleanup() { + await pluginAnchor.shutdown(); + try { + unlinkSync(pluginDatabaseUrl.slice('file:'.length)); + } catch { + // The adapter may already have removed the temporary database. + } + }, + }; + } + beforeAll(async () => { anchor = createAnchor({ network: { network: 'testnet' }, @@ -4071,4 +4117,100 @@ describe('MVP Express-mounted integration', () => { expect(response.body.error).toBe('invalid_amount'); expect(response.body.message).toContain('maximum allowed'); }); + + it('runs deposit hooks in registration order and stops before persistence on rejection', async () => { + const order: string[] = []; + const harness = await createPluginTestAnchor([ + { + id: 'deposit-hook-first', + hooks: { onDepositRequest: async () => void order.push('first') }, + }, + { + id: 'deposit-hook-second', + hooks: { onDepositRequest: async () => void order.push('second') }, + }, + ]); + + const response = await harness.invoke({ + method: 'POST', + path: '/transactions/deposit/interactive', + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${accessToken}`, + }, + body: { asset_code: 'USDC', amount: '25.5' }, + }); + + expect(response.status).toBe(201); + expect(order).toEqual(['first', 'second']); + await harness.cleanup(); + + let rejectedAccount: string | undefined; + let rejectedAssetCode: string | undefined; + const rejected = await createPluginTestAnchor([ + { + id: 'deposit-hook-reject', + hooks: { + onDepositRequest: async (context) => { + rejectedAccount = context.account; + rejectedAssetCode = context.asset.code; + throw new Error('reject'); + }, + }, + }, + ]); + const insertSpy = vi.spyOn(rejected.database, 'insertInteractiveTransaction'); + const rejectedResponse = await rejected.invoke({ + method: 'POST', + path: '/transactions/deposit/interactive', + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${accessToken}`, + }, + body: { asset_code: 'USDC', amount: '25.5' }, + }); + + expect(rejectedResponse.status).toBe(400); + expect(rejectedResponse.body.error).toBe('deposit_rejected'); + expect(rejectedAccount).toBe(clientKeypair.publicKey()); + expect(rejectedAssetCode).toBe('USDC'); + expect(insertSpy).not.toHaveBeenCalled(); + await rejected.cleanup(); + }); + + it('runs SEP-10 challenge hooks and does not persist after a hook failure', async () => { + let hookCalls = 0; + const harness = await createPluginTestAnchor([ + { + id: 'challenge-hook', + hooks: { + onSep10Challenge: async (transaction) => { + hookCalls += 1; + return transaction; + }, + }, + }, + ]); + const response = await harness.invoke({ + path: `/auth/challenge?account=${clientKeypair.publicKey()}`, + }); + expect(response.status).toBe(200); + expect(hookCalls).toBe(1); + await harness.cleanup(); + + const failing = await createPluginTestAnchor([ + { + id: 'challenge-hook-failure', + hooks: { onSep10Challenge: async () => Promise.reject(new Error('reject')) }, + }, + ]); + const insertSpy = vi.spyOn(failing.database, 'insertAuthChallenge'); + const failedResponse = await failing.invoke({ + path: `/auth/challenge?account=${clientKeypair.publicKey()}`, + }); + expect(failedResponse.status).toBe(500); + expect(failedResponse.body.error).toBe('challenge_hook_failed'); + expect(insertSpy).not.toHaveBeenCalled(); + await failing.cleanup(); + }); }); diff --git a/tests/runtime/transaction-watcher.unit.test.ts b/tests/runtime/transaction-watcher.unit.test.ts index 9b71e8bf..16d68117 100644 --- a/tests/runtime/transaction-watcher.unit.test.ts +++ b/tests/runtime/transaction-watcher.unit.test.ts @@ -72,6 +72,101 @@ describe('TransactionWatcher Unit Tests', () => { expect(mockQueue.stop).not.toHaveBeenCalled(); }); + it('waits for an in-progress initial tick before stopping and allows a clean restart', async () => { + vi.useFakeTimers(); + + try { + let resolveFirstTick!: (transactions: unknown[]) => void; + mockDatabase.listPendingTransactionsBefore = vi + .fn() + .mockImplementationOnce( + () => new Promise((resolve) => (resolveFirstTick = resolve)), + ) + .mockResolvedValue([]); + + const firstStart = transactionWatcher.start(); + const concurrentStart = transactionWatcher.start(); + const stopping = transactionWatcher.stop(); + resolveFirstTick([]); + + await expect(Promise.all([firstStart, concurrentStart, stopping])).resolves.toBeDefined(); + expect(mockDatabase.listPendingTransactionsBefore).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(1000); + expect(mockDatabase.listPendingTransactionsBefore).toHaveBeenCalledTimes(1); + + await transactionWatcher.start(); + expect(mockDatabase.listPendingTransactionsBefore).toHaveBeenCalledTimes(2); + await transactionWatcher.stop(); + } finally { + vi.useRealTimers(); + await transactionWatcher.stop(); + } + }); + + it('does not restart polling when start races with stop during a scheduled tick', async () => { + vi.useFakeTimers(); + + try { + await transactionWatcher.start(); + + let resolveScheduledTick!: (transactions: InteractiveTransactionRecord[]) => void; + const pendingScheduledTick = vi.fn( + () => + new Promise( + (resolve) => (resolveScheduledTick = resolve), + ), + ); + mockDatabase.listPendingTransactionsBefore = pendingScheduledTick; + vi.advanceTimersByTime(1000); + + const stopping = transactionWatcher.stop(); + const startingDuringStop = transactionWatcher.start(); + resolveScheduledTick([]); + await Promise.all([stopping, startingDuringStop]); + + vi.advanceTimersByTime(1000); + expect(pendingScheduledTick).toHaveBeenCalledTimes(1); + + mockDatabase.listPendingTransactionsBefore = vi.fn().mockResolvedValue([]); + await transactionWatcher.start(); + expect(mockDatabase.listPendingTransactionsBefore).toHaveBeenCalledTimes(1); + await transactionWatcher.stop(); + } finally { + vi.useRealTimers(); + await transactionWatcher.stop(); + } + }); + + it('keeps shutdown safe when an in-flight scheduled tick fails', async () => { + vi.useFakeTimers(); + + try { + await transactionWatcher.start(); + + let rejectScheduledTick!: (reason: Error) => void; + mockDatabase.listPendingTransactionsBefore = vi.fn( + () => + new Promise((_, reject) => { + rejectScheduledTick = reject; + }), + ); + vi.advanceTimersByTime(1000); + + const stopping = transactionWatcher.stop(); + rejectScheduledTick(new Error('transient tick failure')); + await expect(stopping).resolves.toBeUndefined(); + + mockDatabase.listPendingTransactionsBefore = vi.fn().mockResolvedValue([]); + await expect(transactionWatcher.start()).resolves.toBeUndefined(); + expect(mockDatabase.listPendingTransactionsBefore).toHaveBeenCalledTimes(1); + await transactionWatcher.stop(); + } finally { + vi.useRealTimers(); + await transactionWatcher.stop(); + } + }); + it('enqueues expiration jobs for stale pending deposits', async () => { const staleTransaction = { id: 'stale-tx-1',