Skip to content

fix(backend): make transfer lifecycle transitions concurrency-safe - #145

Open
woahwhattheheck wants to merge 1 commit into
RemitFlow:mainfrom
woahwhattheheck:latch/remitflow-133-concurrency-lifecycle
Open

woahwhattheheck wants to merge 1 commit into
RemitFlow:mainfrom
woahwhattheheck:latch/remitflow-133-concurrency-lifecycle

Conversation

@woahwhattheheck

Copy link
Copy Markdown

Summary

Closes #133.

Makes terminal transfer mutations (claim / cancel) concurrency-safe end-to-end: allowed transitions, optimistic version checks, one terminal outcome, idempotent provider settlement, and safe conflict responses. Strictly stronger than a version-only check by adding a per-transfer lifecycle lease so two different operation keys cannot both prepare provider work for the same transfer.

What changed

  • Transfers carry a monotonically increasing version (starts at 1); create / get / archive / unarchive / claim / cancel responses expose it as a strong ETag.
  • Claim and cancel require If-Match (428 if missing) plus Idempotency-Key; stale versions and illegal transitions return 409 with expected/actual version and current status.
  • Single compare-and-set commit path (compareAndSetTransition) for terminal status changes.
  • Per-transfer lifecycle lease blocks concurrent prepare windows (closes the double-settlement race when two claim keys race).
  • Actor-scoped lifecycle idempotency ledger replays the first terminal result on retry.
  • New settlementWorker settles claims under a stable operation id; receipts live in the shared store so a worker-module reload still returns the first artifact.
  • Mock Stellar adapters accept stable operation keys and reuse provider artifacts on retry.
  • Provider failure occurs before local terminal commit; lease + reservation are released for a safe retry.
  • Archive / unarchive advance the same resource version so a stale terminal mutation cannot silently commit afterward.

Design tradeoffs

  • Lease + version, not version alone: a version check after provider prepare still allows two keys to both call the provider. The lease ensures only one lifecycle mutation prepares provider work per transfer.
  • Prepare-before-CAS: provider work runs before the local commit so a failed settlement never leaves a claimed transfer. With leases + idempotent receipts, a retry reuses the first artifact instead of settling twice.
  • Compatibility: service callers without a lifecycle context still work (internal/tests synthesize actor/key/version). HTTP claim/cancel now require If-Match + Idempotency-Key — intentional optimistic-concurrency contract.
  • Storage boundary: demo store is process-local; docs note that a durable store must CAS on (id, version) and share leases/receipts across workers.

Acceptance criteria mapping

Criterion Evidence
Only valid transitions commit State-machine tests + compareAndSetTransition
One terminal outcome wins Claim vs cancel race + reentrant cancel-during-claim
Provider retries cannot settle twice Duplicate callback replay + dual-key lease test (settleCalls === 1)
Workers idempotent / restart-safe Settlement worker reload keeps shared receipts
Safe conflict outcomes HTTP 428 / 409 tests with conflict details
Rollback on provider failure Failure leaves pending/v1; retry succeeds
Regression for original failure mode Reentrant dual-key claim cannot double-settle

Test evidence

npm test
# 274 pass / 0 fail

Focused suites: test/transferLifecycleConcurrency.test.js, test/transferLifecycleHttp.test.js. Existing suite updated only where claim/cancel HTTP calls needed the new headers (test/requireScope.test.js); no unrelated tests skipped or weakened.

Out of scope

  • No broad rewrite of unrelated services or user flows
  • No durable database CAS yet (contract documented for when the store moves off process memory)

Add optimistic versioning, per-transfer lifecycle leases, and an
idempotent settlement worker so concurrent claim/cancel cannot produce
impossible states or double settlement. Terminal mutations require
If-Match plus Idempotency-Key and return explicit 409/428 conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(backend): make transfer lifecycle transitions concurrency-safe

1 participant