Skip to content

fix: prevent double-credit race condition with atomic conditional UPDATE #43

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

Open
memosr wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: prevent double-credit race condition with atomic conditional UPDATE #43

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
57 changes: 41 additions & 16 deletions app/api/circle/webhook/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,33 @@ async function updateTransactionStatus(notification: CircleNotification) {
const isSuccessfulUpdate = (mappedStatus === 'confirmed' || mappedStatus === 'complete');
const wasAlreadyProcessed = (currentStatus === 'confirmed' || currentStatus === 'complete');

// Only increment credits if the transaction is moving to a success state
// for the first time. This prevents double-crediting.
if (isSuccessfulUpdate && !wasAlreadyProcessed) {
// Atomic update: the WHERE status='pending' condition ensures only one concurrent
// caller can transition this transaction. If the PATCH handler processed it first,
// maybeSingle() returns null and we skip crediting.
const { data: atomicResult, error: atomicError } = await supabaseAdminClient
.from("transactions")
.update({
status: mappedStatus,
updated_at: new Date().toISOString(),
})
.eq("id", transaction.id)
.eq("status", "pending")
.select("id")
.maybeSingle();

if (atomicError) {
console.error(`Failed updating transaction status for ${transaction.id}:`, atomicError);
continue;
}

if (!atomicResult) {
// Another path (e.g. PATCH handler) already transitioned this transaction
console.log(`Transaction ${transaction.id} already processed by another path, skipping credit`);
continue;
}

// This path won the atomic race — safe to increment credits
console.log(`Transaction ${transaction.id} confirmed. Crediting user ${transaction.user_id} with ${transaction.credit_amount} credits.`);

const { error: creditsError } = await supabaseAdminClient.rpc("increment_credits", {
Expand All @@ -158,26 +182,27 @@ async function updateTransactionStatus(notification: CircleNotification) {
});

if (creditsError) {
// Log the error but continue, so we at least update the transaction status.
console.error(`CRITICAL: Failed to increment credits for user ${transaction.user_id} on transaction ${transaction.id}. Error:`, creditsError);
} else {
console.log(`Successfully credited user ${transaction.user_id}.`);
}
}

// Update the transaction status regardless of the credit operation.
const { error: updateError } = await supabaseAdminClient
.from("transactions")
.update({
status: mappedStatus,
updated_at: new Date().toISOString()
})
.eq("id", transaction.id);

if (updateError) {
console.error(`Failed updating transaction status for ${transaction.id}:`, updateError);
} else {
console.log(`Updated transaction ${transaction.id} status from '${currentStatus}' to '${mappedStatus}'`);
} else {
// Non-credit status update (e.g. confirmed→complete, any→failed): no race risk
const { error: updateError } = await supabaseAdminClient
.from("transactions")
.update({
status: mappedStatus,
updated_at: new Date().toISOString(),
})
.eq("id", transaction.id);

if (updateError) {
console.error(`Failed updating transaction status for ${transaction.id}:`, updateError);
} else {
console.log(`Updated transaction ${transaction.id} status from '${currentStatus}' to '${mappedStatus}'`);
}
}
}
}
Expand Down
59 changes: 38 additions & 21 deletions app/api/transactions/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ export async function PATCH(
);
}

// Only update if currently in 'pending' status
// Don't override Circle's authoritative updates
// Fast path: if already past pending, no work needed (not a race guard — see atomic update below)
if (transaction.status !== "pending") {
return NextResponse.json(
{
Expand All @@ -231,24 +230,9 @@ export async function PATCH(
},
};

// Increment user credits if this is a credit transaction
if (transaction.direction === "credit" && transaction.credit_amount && transaction.user_id) {
console.log(`Transaction ${transaction.id} completed. Crediting user ${transaction.user_id} with ${transaction.credit_amount} credits.`);

const { error: creditsError } = await supabaseAdminClient.rpc("increment_credits", {
user_id_to_update: transaction.user_id,
amount_to_add: transaction.credit_amount,
});

if (creditsError) {
console.error(`CRITICAL: Failed to increment credits for user ${transaction.user_id} on transaction ${transaction.id}. Error:`, creditsError);
// Continue with status update even if credits fail - we can fix this manually
} else {
console.log(`Successfully credited user ${transaction.user_id}.`);
}
}

// Update transaction to 'complete' status
// Atomic update: the WHERE status='pending' condition ensures only one concurrent
// caller can transition this transaction. If the Circle webhook processed it first,
// maybeSingle() returns null and we skip crediting.
const { data: updatedTransaction, error: updateError } =
await supabaseAdminClient
.from("transactions")
Expand All @@ -258,8 +242,9 @@ export async function PATCH(
updated_at: new Date().toISOString(),
})
.eq("id", id)
.eq("status", "pending")
.select()
.single();
.maybeSingle();

if (updateError) {
console.error("[transactions/PATCH] Update error:", updateError);
Expand All @@ -269,6 +254,38 @@ export async function PATCH(
);
}

if (!updatedTransaction) {
// Another path (e.g. Circle webhook) already transitioned this transaction
return NextResponse.json(
{
ok: true,
message: "Transaction already processed by another path, no update needed",
transaction: {
id: transaction.id,
status: transaction.status,
updatedAt: transaction.updated_at,
},
},
{ status: 200 }
);
}

// This path won the atomic race — safe to increment credits
if (transaction.direction === "credit" && transaction.credit_amount && transaction.user_id) {
console.log(`Transaction ${transaction.id} completed. Crediting user ${transaction.user_id} with ${transaction.credit_amount} credits.`);

const { error: creditsError } = await supabaseAdminClient.rpc("increment_credits", {
user_id_to_update: transaction.user_id,
amount_to_add: transaction.credit_amount,
});

if (creditsError) {
console.error(`CRITICAL: Failed to increment credits for user ${transaction.user_id} on transaction ${transaction.id}. Error:`, creditsError);
} else {
console.log(`Successfully credited user ${transaction.user_id}.`);
}
}

return NextResponse.json(
{
ok: true,
Expand Down