Skip to content

fix: prevent concurrent bounty claim race condition with pessimistic write lock - #37

Open
windy202508 wants to merge 2 commits into
MergeFi:mainfrom
windy202508:fix/concurrent-claim-race-condition
Open

fix: prevent concurrent bounty claim race condition with pessimistic write lock#37
windy202508 wants to merge 2 commits into
MergeFi:mainfrom
windy202508:fix/concurrent-claim-race-condition

Conversation

@windy202508

Copy link
Copy Markdown

Problem

Issue #1 describes a race condition in BountiesService.claim(): two concurrent POST /api/bounties/:id/claim requests can both read the bounty in FUNDED status, both pass assertTransition, and both writes succeed — the second overwrites the first. This results in two contributors receiving 200 responses claiming they own the bounty, which can lead to disputes and potential double payouts.

The same race condition exists in all status-transition methods: fund(), markInReview(), markMergedAndRelease(), and refund().

Solution

Introduce a withLock() private method that wraps every status-transition operation in a database transaction with a pessimistic write lock (SELECT ... FOR UPDATE). This ensures that:

  1. When a contributor claims a bounty, the row is locked for the duration of the read-check-write sequence
  2. A second concurrent claim will block until the first transaction commits
  3. By the time the second claim reads the row, the status is already CLAIMED, so assertTransition correctly rejects it

Key changes

  • bounties.service.ts: Added DataSource injection and withLock() helper method that:

    • Opens a transaction via this.dataSource.transaction()
    • Acquires a pessimistic_write lock on the bounty row via createQueryBuilder().setLock("pessimistic_write")
    • Performs the status check and mutation inside the transaction
    • Refactored fund(), claim(), markInReview(), markMergedAndRelease(), and refund() to use withLock()
    • markMergedAndRelease() now uses the transaction runner for all DB operations (team/user lookups, saves)
  • bounties.service.spec.ts: Updated tests to mock DataSource.transaction() and the query builder chain, added a new test case for concurrent claim serialization

Why pessimistic locking over optimistic?

The issue description suggests either pessimistic write lock or optimistic @VersionColumn. I chose pessimistic locking because:

  • It provides stronger guarantees — the second request waits rather than failing with a conflict error that needs retry logic
  • It is simpler for the client — no need to implement retry-on-conflict
  • For a paid-bounty platform where real money is at stake, serializing access is safer than optimistic retries
  • TypeORM supports setLock("pessimistic_write") natively with PostgreSQL

Testing

  • All existing unit tests updated and passing
  • New test: concurrent claims for the same bounty are serialized by the pessimistic lock — verifies that a second claim after the first sees CLAIMED status and is rejected

Manual Testing

To verify the fix manually:

  1. Start the server with a PostgreSQL database
  2. Create and fund a bounty
  3. Run two concurrent curl requests: curl -X POST http://localhost:3000/api/bounties/<id>/claim -H "Authorization: Bearer <token1>" and the same with <token2> simultaneously
  4. Before fix: Both return 200, both contributors think they own the bounty
  5. After fix: First returns 200, second returns 409 (transition error) because the lock serializes access

Closes #1

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

@windy202508 is attempting to deploy a commit to the chonilius' projects Team on Vercel.

A member of the Team first needs to authorize it.

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.

Race condition: concurrent bounty claims can double-assign a single FUNDED bounty

1 participant