feat: end-to-end Idempotency-Key support for fund/claim/release/refund endpoints - #35
Merged
chonilius merged 8 commits intoJul 17, 2026
Conversation
One row per (Idempotency-Key header, endpoint scope, caller) tuple. The unique index on (key, scope, callerId) is the actual concurrency primitive: two simultaneous requests with the same key both attempt an INSERT, the DB allows exactly one to win, and the loser reads back the winner's row instead of racing into a duplicate execution — a "check then insert" at the JS level would just relocate the same TOCTOU race the issue's Difficulty Justification calls out, not close it. Status is PROCESSING while the underlying mutation is still running, then COMPLETED with the cached response once it finishes (see the interceptor, next commits, for exactly which outcomes get cached vs. released for retry).
Matches IdempotencyKey exactly: the unique (key, scope, callerId) index that makes concurrent-duplicate-request handling safe, and a secondary index on expiresAt for IdempotencyCleanupService's periodic sweep to use an index scan instead of a sequential one.
…or (MergeFi#16) Response-caching + concurrency-safe replay engine for the Idempotency-Key header: attempts an atomic insert into idempotency_keys and reacts to a unique-violation instead of check-then-insert, closing the same TOCTOU race the issue's difficulty justification warns about. Replays cached 2xx/4xx outcomes verbatim; releases the key on 5xx so a retry re-executes cleanly. Includes stale-PROCESSING-row reclaim (crash recovery) via an atomic compare-and-swap update.
…ergeFi#16) Global module so @idempotent resolves in every feature module without each one importing it individually. IdempotencyCleanupService sweeps expired rows hourly via @nestjs/schedule; retention itself is set per-row at claim time (IDEMPOTENCY_KEY_TTL_MS) and plays no part in correctness — a replay against an already-swept key just executes as new.
…points (MergeFi#16) Guards BountiesController.fund/claim/refund, EscrowController.fund/release/ splitRelease/refund, MilestonesController.fund/resolveIssue, and MaintenancePoolController.deposit/assignReward with the Idempotency-Key header contract. splitRelease is included alongside release since it moves funds through the same escrow in the same way.
…eFi#16) Replace the query-builder based compare-and-swap with a plain repo.update(criteria, partial) call — TypeORM ANDs multiple criteria fields together, so this keeps the same atomic (id, status, updatedAt) guard with far less code and no query-builder mocking needed in tests.
…ep (MergeFi#16) IdempotencyInterceptor: duplicate key replays the cached result without re-executing the handler; two genuinely concurrent same-key requests only execute the handler once (one gets 409, proven with a real interleaved race against an in-memory unique-index stand-in, not a mocked call count); different keys execute independently; 5xx outcomes are released, not cached, so a retry re-executes; 4xx outcomes are cached and replayed verbatim; a stale PROCESSING row is reclaimed instead of 409-ing forever; caller scoping keys by req.user.userId. IdempotencyCleanupService: sweeps expired rows via LessThan(now()). Also fixes escrow.controller.spec.ts, whose bare TestingModule now needs IdempotencyInterceptor's dependencies resolvable since fund/release/ splitRelease/refund carry @idempotent.
Covers which endpoints require it, the 2xx/4xx-cached vs 5xx-released caching policy, the 409 concurrent-duplicate behavior, and the 24h TTL / hourly cleanup sweep.
|
@abayomicornelius is attempting to deploy a commit to the chonilius' projects Team on Vercel. A member of the Team first needs to authorize it. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #16. Adds an
Idempotency-Keyheader contract to every fund/claim/release/refund mutation across Bounties, Escrow, Milestones, and MaintenancePool, so a client retry after a dropped connection can't double-execute a mutation.IdempotencyKeyentity + migration — newidempotency_keystable with a unique index on(key, scope, callerId). This unique index is the actual concurrency-safety primitive:IdempotencyInterceptoralways attempts a plaininsert()and reacts to a unique-violation rather than checking-then-inserting, since a JS-level "does a row exist?" check followed by a separate insert has the exact same TOCTOU race the issue's difficulty justification warns about — the DB constraint is what actually closes it.@Idempotent(scope)decorator — appliesIdempotencyInterceptorplus a Swagger@ApiHeaderdocumenting the contract. Applied toBountiesController.fund/claim/refund,EscrowController.fund/release/splitRelease/refund,MilestonesController.fund/resolveIssue,MaintenancePoolController.deposit/assignReward.HttpExceptionfailure) is not cached — the key is released instead — since forcing every future retry to replay a server error forever is worse than letting the retry run cleanly.PROCESSINGgets409 Conflict. APROCESSINGrow older than 30s is assumed to belong to a crashed request and is reclaimed via an atomic compare-and-swap (UPDATE ... WHERE id = :id AND status = 'processing' AND updatedAt = :observed), so two retries racing to reclaim the same stale row only let one through.IDEMPOTENCY_KEY_TTL_MS) and swept hourly byIdempotencyCleanupService(@nestjs/schedule). Documented in the README's new "Idempotency" section.JwtAuthGuard.resolveCallerIdusesreq.user.userIdwhen present and falls back to a shared'anonymous'bucket otherwise — documented as a known, accepted trade-off in the interceptor's doc comment rather than silently expanding scope by adding auth to these routes.Test plan
IdempotencyInterceptorunit tests (idempotency.interceptor.spec.ts): missing/invalid header → 400; first call executes the handler and caches the result; duplicate key replays the cached result without re-executing (spy call count); two genuinely concurrent same-key requests interleaved via real Promise/microtask racing against an in-memory unique-index stand-in — only one executes, the other gets 409; different keys execute independently; 5xx is not cached (retry re-executes); 4xx is cached and replayed as the same exception; stalePROCESSINGrow is reclaimed instead of 409-ing forever; caller scoping viareq.user.userId.IdempotencyCleanupServiceunit tests: sweeps rows pastexpiresAt.npx tsc --noEmit,npm run lint,npm run buildall pass.npx jest— all suites pass except the pre-existing*.integration.spec.tsfiles that require a live Postgres connection (not available in this environment; unrelated to this change).🤖 Generated with Claude Code