Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ pub async fn run_migrations(pool: &PgPool) -> Result<()> {
tracing::info!("Migrations complete");
Ok(())
}

/// True if `error` is a PostgreSQL unique/primary-key constraint violation.
/// Used across routes and services to gracefully handle insert races as `409 Conflict` (#55).
pub fn is_unique_violation(error: &sqlx::Error) -> bool {
matches!(error, sqlx::Error::Database(db_err) if db_err.is_unique_violation())
}

120 changes: 117 additions & 3 deletions src/routes/auth.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
db::is_unique_violation,
error::{AppError, AppResult},
middleware::auth::issue_jwt,
models::user::{AuthResponse, CreateUserRequest, LoginRequest, User, UserRow},
Expand Down Expand Up @@ -31,7 +32,7 @@ pub async fn register(
return Err(AppError::Validation("full_name is required".into()));
}

// Check uniqueness.
// Fast-path uniqueness check to avoid unnecessary bcrypt hashing cost.
let exists: bool = sqlx::query_scalar("SELECT EXISTS(SELECT 1 FROM users WHERE email = $1)")
.bind(&email)
.fetch_one(&state.pool)
Expand All @@ -47,7 +48,7 @@ pub async fn register(
AppError::Internal(anyhow::anyhow!("bcrypt error: {e}"))
})?;

// Insert.
// Insert with unique-violation protection against concurrent registration races (#55).
let id = Uuid::new_v4();
let row = sqlx::query_as::<_, UserRow>(
r#"
Expand All @@ -62,7 +63,14 @@ pub async fn register(
.bind(req.full_name.trim())
.bind(&req.stellar_address)
.fetch_one(&state.pool)
.await?;
.await
.map_err(|e| {
if is_unique_violation(&e) {
AppError::Conflict("Email".into())
} else {
e.into()
}
})?;

let user = User::from(row);

Expand Down Expand Up @@ -122,3 +130,109 @@ pub async fn login(
"data": AuthResponse { token, user }
})))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn is_unique_violation_helper_identifies_non_db_errors_as_false() {
let err = sqlx::Error::RowNotFound;
assert!(!is_unique_violation(&err));
}
}

/// End-to-end database tests for registration unique-constraint race condition (#55).
/// Run with `cargo test -- --ignored` against a real database instance.
#[cfg(test)]
mod db_tests {
use super::*;
use crate::config::Config;
use sqlx::postgres::PgPoolOptions;

async fn test_pool() -> PgPool {
let database_url = std::env::var("DATABASE_URL")
.expect("DATABASE_URL must be set to run this integration test");
let pool = PgPoolOptions::new()
.max_connections(5)
.connect(&database_url)
.await
.expect("failed to connect to DATABASE_URL");
sqlx::migrate!("./migrations")
.run(&pool)
.await
.expect("failed to run migrations");
pool
}

async fn cleanup_user_by_email(pool: &PgPool, email: &str) {
let _ = sqlx::query("DELETE FROM users WHERE email = $1")
.bind(email)
.execute(pool)
.await;
}

#[tokio::test]
#[ignore]
async fn concurrent_register_calls_for_same_email_returns_one_success_and_one_conflict() {
let pool = test_pool().await;
let email = format!("test-race-{}@example.test", Uuid::new_v4());

let config = Config::from_env().unwrap_or_else(|_| Config {
app_env: "test".into(),
host: "127.0.0.1".into(),
port: 8080,
database_url: std::env::var("DATABASE_URL").unwrap(),
database_max_connections: 5,
database_min_connections: 1,
database_connect_timeout_secs: 5,
jwt_secret: "test_secret_for_registration_tests_12345".into(),
jwt_expiry_hours: 24,
horizon_url: "https://horizon-testnet.stellar.org".into(),
stellar_network_passphrase: "Test SDF Network ; September 2015".into(),
soroban_rpc_url: "https://soroban-testnet.stellar.org".into(),
allowed_origins: vec!["*".into()],
rate_cache_ttl_secs: 30,
keeper_enabled: false,
keeper_poll_interval_secs: 60,
keeper_secret_key: None,
escrow_contract_id: None,
subscription_contract_id: None,
reconciliation_poll_interval_secs: 60,
reconciliation_stale_after_secs: 300,
});

let state = Arc::new(AppState {
pool: pool.clone(),
config,
loop_health: crate::BackgroundLoopHealth::default(),
});

let req1 = CreateUserRequest {
email: email.clone(),
password: "password123".into(),
full_name: "User One".into(),
stellar_address: None,
};
let req2 = CreateUserRequest {
email: email.clone(),
password: "password123".into(),
full_name: "User Two".into(),
stellar_address: None,
};

let (res1, res2) = tokio::join!(
register(State(state.clone()), Json(req1)),
register(State(state.clone()), Json(req2))
);

let one_ok = res1.is_ok() || res2.is_ok();
let one_conflict = matches!(&res1, Err(AppError::Conflict(_))) || matches!(&res2, Err(AppError::Conflict(_)));

assert!(one_ok, "Exactly one concurrent registration should succeed");
assert!(one_conflict, "The racing registration loser must receive Conflict (409), not a 500 Database error");

cleanup_user_by_email(&pool, &email).await;
}
}

6 changes: 1 addition & 5 deletions src/services/batch.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
db::is_unique_violation,
error::{AppError, AppResult},
models::{
batch_payment::{BatchPaymentResult, SendBatchPaymentRequest},
Expand All @@ -17,11 +18,6 @@ pub struct BatchPaymentService {
pool: PgPool,
}

/// True if `error` is a Postgres unique/primary-key violation — used to
/// detect losing the race to claim a `batch_submissions` row.
fn is_unique_violation(error: &sqlx::Error) -> bool {
matches!(error, sqlx::Error::Database(db_err) if db_err.is_unique_violation())
}

/// Decides what a failed submission attempt means for transaction status
/// (#30). The distinction is "did we get a definitive answer from Horizon
Expand Down
9 changes: 2 additions & 7 deletions src/services/escrow.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
config::Config,
db::is_unique_violation,
error::{AppError, AppResult},
models::{
escrow::{CreateEscrowRequest, Escrow, EscrowActionRequest, EscrowActor, EscrowRow, EscrowStatus},
Expand Down Expand Up @@ -32,17 +33,11 @@ impl EscrowAction {
}
}

/// True if `error` is a Postgres unique/primary-key violation — used to
/// detect a duplicate `onchain_escrow_id` (#56), mirroring the same pattern
/// in `services::batch::is_unique_violation`.
fn is_unique_violation(error: &sqlx::Error) -> bool {
matches!(error, sqlx::Error::Database(db_err) if db_err.is_unique_violation())
}

pub struct EscrowService {
pool: PgPool,
}


impl EscrowService {
pub fn new(pool: PgPool) -> Self {
Self { pool }
Expand Down
9 changes: 2 additions & 7 deletions src/services/subscription.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
config::Config,
db::is_unique_violation,
error::{AppError, AppResult},
models::{
subscription::{
Expand Down Expand Up @@ -34,17 +35,11 @@ pub struct KeeperRunSummary {
pub considered: usize,
}

/// True if `error` is a Postgres unique/primary-key violation — used to
/// detect a duplicate `onchain_subscription_id` (#56), mirroring the same
/// pattern in `services::batch::is_unique_violation`.
fn is_unique_violation(error: &sqlx::Error) -> bool {
matches!(error, sqlx::Error::Database(db_err) if db_err.is_unique_violation())
}

pub struct SubscriptionService {
pool: PgPool,
}


impl SubscriptionService {
pub fn new(pool: PgPool) -> Self {
Self { pool }
Expand Down