register has a check-then-insert email race whose loser gets a 500 instead of 409, unlike batch.rs's unique-violation handling

[abayomicornelius]

Overview

POST /api/auth/register (src/routes/auth.rs:16-81) checks email uniqueness with a SELECT EXISTS query, then performs a separate INSERT — a textbook check-then-act race:

// Check uniqueness.
let exists: bool = sqlx::query_scalar("SELECT EXISTS(SELECT 1 FROM users WHERE email = $1)")
    .bind(&email).fetch_one(&state.pool).await?;

if exists {
    return Err(AppError::Conflict("Email".into()));
}

// ... hash password ...

// Insert.
let row = sqlx::query_as::<_, UserRow>(
    "INSERT INTO users (id, email, password_hash, full_name, stellar_address) VALUES ($1, $2, $3, $4, $5) RETURNING *"
).bind(id).bind(&email)...fetch_one(&state.pool).await?;

Two concurrent registration requests for the same email can both execute the SELECT EXISTS before either commits an INSERT — both see exists == false, both proceed to hash a password (bcrypt at DEFAULT_COST, ~100-300ms, widening the race window considerably compared to a cheap check), and both attempt the INSERT. The users.email column is protected by a real UNIQUE constraint at the database level (migrations/001_initial.sql:19: email TEXT NOT NULL UNIQUE), so this cannot result in two rows with the same email — the database itself prevents the data-corruption outcome. But the error handling for the loser of that race is wrong:

let row = sqlx::query_as::<_, UserRow>(...)
    .fetch_one(&state.pool)
    .await?;    // <-- `?` on a sqlx::Error converts via AppError::Database(#[from] sqlx::Error)

Unlike BatchPaymentService, which built a dedicated is_unique_violation helper (src/services/batch.rs:20-24) specifically to catch a sqlx::Error::Database unique-constraint violation and map it to a proper 409 Conflict, register's INSERT uses a bare ?, which falls through to AppError::Database(#[from] sqlx::Error) — mapped by error.rs's status_and_code (error.rs:112) to a generic 500 INTERNAL_SERVER_ERROR / "A database error occurred". The loser of the race gets told the server is broken, when the actual, entirely-expected situation is "someone (possibly you, in another tab, or a double-submitted form) already registered this email a moment ago" — exactly the same semantic outcome the earlier exists check was already designed to communicate as 409 Conflict, just reached via a different code path that wasn't given the same care.

Requirements

• Wrap the INSERT in register with the same unique-violation detection pattern batch.rs already established (is_unique_violation, or a shared helper promoted to error.rs/a common module so both call sites — and any future one — use the identical check), mapping a unique-constraint violation on users.email to AppError::Conflict("Email".into()), matching the existing pre-check's error exactly.
• Consider whether the pre-check (SELECT EXISTS) is even worth keeping once the INSERT's own violation handling is correct — it currently exists purely as an optimization to avoid paying the bcrypt cost for an obviously-doomed request, which is a reasonable thing to keep, but its result should no longer be treated as the sole source of truth for uniqueness now that both paths would independently return the correct error.

Acceptance Criteria

• A unique-constraint violation on INSERT INTO users is mapped to 409 Conflict, not 500.
• A concurrency test issuing two simultaneous register calls with the same email asserts exactly one succeeds (201/200 with a token) and the other receives 409 Conflict — not 500.
• The shared unique-violation-detection helper (promoted from batch.rs or newly written) has its own unit test covering both a real unique-violation sqlx::Error and a non-violation database error, confirming only the former maps to Conflict.

Additional Notes

Edge cases

• If is_unique_violation is promoted to a shared location, double check batch.rs's existing usage continues to compile and behave identically — this should be a pure refactor for that call site, not a behavior change.
• The stellar_address column has no uniqueness constraint at all (unlike email) — a quick look while touching this code is worth it, though whether duplicate stellar_address values across users should even be disallowed is a separate product question, not assumed here.

Testing strategy

• A tokio::join! of two concurrent register calls against a real (or #[ignore]d integration-test) Postgres instance, following the concurrency-test patterns already established for the reconciliation/batch crash-consistency work.

Cross-references

• Directly parallels the unique-violation handling batch.rs already implements correctly for batch_submissions (is_unique_violation, batch.rs:20-24) — this issue brings register's handling of the exact same class of database-level race up to the same standard, in an entirely different part of the codebase (auth, not payments) that evidently didn't inherit that pattern.