Skip to content

fix(campaign): return 409 on double claim instead of 200 #667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
scarface-dev1 wants to merge 1 commit into from
Closed

fix(campaign): return 409 on double claim instead of 200 #667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions backend/src/api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,29 @@ describe('Campaign Lifecycle API', () => {
expect(claimRes.status).toBe(200);
expect(claimRes.data.data.progress.status).toBe('claimed');

// Duplicate Claim is idempotent (returns 200 with the same status)
// Verify no duplicate claim event in history
const historyRes = await get(`/api/campaigns/${campaignId}/history`);
const claimEvents = historyRes.data.events?.filter(
(e: { eventType: string }) => e.eventType === 'claimed',
);
expect(claimEvents?.length).toBe(1);

// Duplicate Claim returns 409 Conflict
const duplicateClaimRes = await post(`/api/campaigns/${campaignId}/claim`, {
creator: CREATOR,
transactionHash: 'a'.repeat(64),
transactionHash: 'b'.repeat(64),
confirmedAt: Math.floor(Date.now() / 1000),
});
expect(duplicateClaimRes.status).toBe(200);
expect(duplicateClaimRes.status).toBe(409);
expect(duplicateClaimRes.data.error.code).toBe('CAMPAIGN_ALREADY_CLAIMED');
expect(duplicateClaimRes.data.error.message).toContain('already claimed');

// Verify claim event still not duplicated
const historyRes2 = await get(`/api/campaigns/${campaignId}/history`);
const claimEvents2 = historyRes2.data.events?.filter(
(e: { eventType: string }) => e.eventType === 'claimed',
);
expect(claimEvents2?.length).toBe(1);
});

it('covers create, pledge, failed, refund end-to-end', async () => {
Expand Down
7 changes: 5 additions & 2 deletions backend/src/services/__tests__/mutation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,14 @@ describe('claimCampaign – guards', () => {
expect(claimEvent!.blockchainMetadata?.txHash).toBe(TX_HASH);
});

it('second claim is idempotent (returns same claimedAt)', () => {
it('second claim throws 409 CAMPAIGN_ALREADY_CLAIMED', () => {
const c = fundedExpiredCampaign();
claimCampaign(c.id, { creator: CREATOR, transactionHash: TX_HASH });
const first = getCampaign(c.id)!.claimedAt;
claimCampaign(c.id, { creator: CREATOR, transactionHash: TX_HASH2 });
expect(() =>
claimCampaign(c.id, { creator: CREATOR, transactionHash: TX_HASH2 }),
).toThrow('Campaign already claimed');
// claimedAt must remain unchanged after failed double-claim
expect(getCampaign(c.id)!.claimedAt).toBe(first);
});
});
Expand Down
2 changes: 1 addition & 1 deletion backend/src/services/campaignStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ function reconcileOnChainClaim(campaignId: string, input: ReconciledClaimInput):
}

if (campaign.claimedAt) {
return campaign;
throw toServiceError('Campaign already claimed.', 409, 'CAMPAIGN_ALREADY_CLAIMED');
}
Comment on lines 1002 to 1004

@coderabbitai coderabbitai Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files matching campaignStore:\n'
fd -a 'campaignStore\.ts$' . || true

file="$(fd 'campaignStore\.ts$' . | head -n1 || true)"
if [ -n "$file" ]; then
  printf '\nLine count:\n'
  wc -l "$file"
  printf '\nRelevant section around claim event:\n'
  sed -n '960,1045p' "$file" | nl -ba -v960
  printf '\nSchema/migrations mentioning claimed_at/campaigns:\n'
  rg -n --glob '*.sql' --glob '*.ts' --glob '*.js' 'claimed_at|CREATE TABLE .*campaign|campaigns' .
fi

Repository: ritik4ever/stellar-goal-vault

Length of output: 391

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="backend/src/services/campaignStore.ts"

printf 'Relevant section around claim event:\n'
sed -n '960,1045p' "$file"

printf '\nSchema/migrations mentioning claimed_at/campaigns:\n'
rg -n --glob '*.sql' --glob '*.ts' --glob '*.js' 'claimed_at|CREATE TABLE .*campaign|campaigns' .

printf '\nCheck for claimed_at uniqueness/index definitions:\n'
rg -n --glob '*.sql' --glob '*.ts' --glob '*.js' 'UNIQUE|INDEX| claimed_at|campaigns' .

Repository: ritik4ever/stellar-goal-vault

Length of output: 50385

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Campaign schema and migrations:\n'
sed -n '78,112p' backend/src/services/db.ts
sed -n '158,260p' backend/src/services/db.ts

printf '\nrecordEvent implementation/usages:\n'
rg -n "function recordEvent|const recordEvent|recordEvent\\(" backend/src/services/campaignStore.ts backend/src/services/db.ts

printf '\nConcurrent duplicate claim behavior probe (SQLite, no repository code executed):\n'
if command -v sqlite3 >/dev/null 2>&1; then
  tmp="$(mktemp)"
  cat > "$tmp" <<'SQL'
PRAGMA foreign_keys=off;
BEGIN;
CREATE TABLE campaign_events (
 campaign_id INTEGER PRIMARY KEY,
 event_type TEXT,
 event_data JSON NOT NULL
);
CREATE TABLE campaigns (id TEXT PRIMARY KEY, claimed_at INTEGER);
INSERT INTO campaigns(id, claimed_at) VALUES ('c1', NULL);
COMMIT;

PRAGMA foreign_keys=off;
BEGIN IMMEDIATE;
INSERT INTO campaign_events(campaign_id, event_type, event_data) VALUES ('c1', 'claimed', '{}');
UPDATE campaigns SET claimed_at = 2 WHERE id = 'c1' AND claimed_at IS NULL;
COMMIT;

SELECT sqlite3_changes();
SELECT event_data, campaign_id FROM campaign_events ORDER BY rowid;
SELECT claimed_at FROM campaigns WHERE id='c1';
SQL
 sqlite3 ":memory:" < "$tmp"
 rm -f "$tmp"
else
  echo "sqlite3 not available"
fi

Repository: ritik4ever/stellar-goal-vault

Length of output: 5228

Make claim detection atomic with the claim write.

campaign.claimedAt is read before db.transaction, so concurrent claimers can both observe an unclaimed campaign and append duplicate claimed events. Keep the current transaction shape but make the update conditional and fail before recordEvent:

UPDATE campaigns SET claimed_at = ? WHERE id = ? AND claimed_at IS NULL

Then throw CAMPAIGN_ALREADY_CLAIMED when the affected row count is less than 1.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/campaignStore.ts` around lines 1002 - 1004, Move claim
detection into the existing db.transaction by conditionally updating campaigns
through the claim write using campaign id and claimed_at IS NULL. Check the
update’s affected-row count before recordEvent and throw toServiceError with
CAMPAIGN_ALREADY_CLAIMED when fewer than one row was updated; remove reliance on
the pre-transaction campaign.claimedAt check.


const progress = calculateProgress(campaign);
Expand Down