Skip to content

refactor: removed dead code identified in issue #848

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
Arowolokehinde wants to merge 1 commit into
base: main
Choose a base branch
from
+56 −220
Open

refactor: removed dead code identified in issue #848

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
8 changes: 4 additions & 4 deletions src/app/admin_take_dispute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ pub async fn pubkey_event_can_solve(
}

// Sender must be a solver user
let Ok(solver) = find_solver_pubkey(pool, sender_pubkey.clone()).await else {
return false;
};
if solver.is_solver == 0_i64 {
if find_solver_pubkey(pool, sender_pubkey.clone())
.await
.is_err()
{
return false;
}

Expand Down
15 changes: 8 additions & 7 deletions src/app/dispute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,14 @@ pub async fn dispute_action(
let dispute = Dispute::new(order_id, order.status.clone());

// Setup dispute
if order.setup_dispute(is_buyer_dispute).is_ok() {
order
.clone()
.update(pool)
.await
.map_err(|cause| MostroInternalErr(ServiceError::DbAccessError(cause.to_string())))?;
}
order
.setup_dispute(is_buyer_dispute)
.map_err(MostroCantDo)?;
order
.clone()
.update(pool)
.await
.map_err(|cause| MostroInternalErr(ServiceError::DbAccessError(cause.to_string())))?;

// Save dispute to database
let dispute = dispute
Expand Down
10 changes: 0 additions & 10 deletions src/app/restore_session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,6 @@ pub async fn restore_session_action(
// Get trade key from the event rumor
let trade_key = event.sender.to_string();

// Validate the master key format
if !master_key.chars().all(|c| c.is_ascii_hexdigit()) || master_key.len() != 64 {
return Err(MostroCantDo(CantDoReason::InvalidPubkey));
}

// Validate the trade key format
if !trade_key.chars().all(|c| c.is_ascii_hexdigit()) || trade_key.len() != 64 {
return Err(MostroCantDo(CantDoReason::InvalidPubkey));
}

// No key in the log line — master_key is the user's persistent Nostr
// identity, not a per-trade ephemeral one (AGENTS.md: scrub Nostr keys
// from logs).
Expand Down
243 changes: 44 additions & 199 deletions src/db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,205 +79,63 @@ pub async fn find_active_trade_pubkeys(pool: &SqlitePool) -> Result<Vec<String>,
Ok(keys.into_iter().collect())
}

/// Helper function to rebuild disputes table without token columns when DROP COLUMN is unsupported.
async fn rebuild_disputes_table_without_tokens(pool: &SqlitePool) -> Result<(), MostroError> {
tracing::info!("Rebuilding disputes table without token columns (SQLite compatibility mode)");

// Create temporary table with new schema (without token columns)
sqlx::query(
r#"
CREATE TABLE IF NOT EXISTS disputes_temp (
id char(36) primary key not null,
order_id char(36) unique not null,
status varchar(10) not null,
order_previous_status varchar(10) not null,
solver_pubkey char(64),
created_at integer not null,
taken_at integer default 0
)
"#,
)
.execute(pool)
.await
.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to create temporary disputes table: {}",
e
)))
})?;

// Copy data from original table to temporary table (excluding token columns)
sqlx::query(
r#"
INSERT INTO disputes_temp (id, order_id, status, order_previous_status, solver_pubkey, created_at, taken_at)
SELECT id, order_id, status, order_previous_status, solver_pubkey, created_at, taken_at
FROM disputes
"#,
)
.execute(pool)
.await
.map_err(|e| MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to copy data to temporary table: {}", e
))))?;

// Drop original table
sqlx::query("DROP TABLE disputes")
.execute(pool)
.await
.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to drop original disputes table: {}",
e
)))
})?;

// Rename temporary table to disputes
sqlx::query("ALTER TABLE disputes_temp RENAME TO disputes")
.execute(pool)
.await
.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to rename temporary table: {}",
e
)))
})?;

tracing::info!("Successfully rebuilt disputes table without token columns");
Ok(())
}

/// Migrates legacy disputes table by removing deprecated buyer_token and seller_token columns if present.
/// Removes deprecated `buyer_token` and `seller_token` columns from the disputes table if present.
///
/// This function uses transactions for atomic operations and includes fallback logic for older SQLite versions
/// that don't support ALTER TABLE DROP COLUMN. The function handles both cases where columns exist (legacy databases)
/// and don't exist (newer databases).
/// Both drops run inside a single transaction so either both succeed or neither does.
/// This is a no-op when neither column exists (fresh installs and already-migrated databases).
async fn migrate_remove_token_columns(pool: &SqlitePool) -> Result<(), MostroError> {
// Check if token columns exist
let buyer_token_exists = sqlx::query_scalar::<_, i32>(
r#"
SELECT COUNT(*)
FROM pragma_table_info('disputes')
WHERE name = 'buyer_token'
"#,
)
.fetch_one(pool)
.await
.map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))?
> 0;

let seller_token_exists = sqlx::query_scalar::<_, i32>(
r#"
SELECT COUNT(*)
FROM pragma_table_info('disputes')
WHERE name = 'seller_token'
"#,
)
.fetch_one(pool)
.await
.map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))?
> 0;
let buyer_token_exists = table_column_exists(pool, "disputes", "buyer_token").await?;
let seller_token_exists = table_column_exists(pool, "disputes", "seller_token").await?;

// If no token columns exist, no migration needed
if !buyer_token_exists && !seller_token_exists {
tracing::debug!(
"No deprecated token columns found in disputes table - migration not needed"
);
return Ok(());
}

// Check SQLite version to determine if DROP COLUMN is supported
let sqlite_version = sqlx::query_scalar::<_, String>("SELECT sqlite_version()")
.fetch_one(pool)
.await
.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to get SQLite version: {}",
e
)))
})?;

tracing::info!("SQLite version: {}", sqlite_version);

// Parse version to check if DROP COLUMN is supported (requires 3.35.0+)
let supports_drop_column = sqlite_version
.split('.')
.take(3)
.map(|v| v.parse::<u32>().unwrap_or(0))
.collect::<Vec<_>>()
.get(..3)
.map(|parts| {
let major = parts[0];
let minor = parts.get(1).copied().unwrap_or(0);
major > 3 || (major == 3 && minor >= 35)
})
.unwrap_or(false);

if supports_drop_column {
// Try DROP COLUMN approach with transaction
tracing::info!(
"Attempting to remove token columns using DROP COLUMN (SQLite {})...",
sqlite_version
);

let mut transaction = pool.begin().await.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to begin transaction: {}",
e
)))
})?;

// Attempt to drop columns within transaction
let drop_result = async {
if buyer_token_exists {
sqlx::query("ALTER TABLE disputes DROP COLUMN buyer_token")
.execute(&mut *transaction)
.await?;
tracing::info!("Dropped buyer_token column");
}
let mut tx = pool.begin().await.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to begin transaction: {}",
e
)))
})?;

if seller_token_exists {
sqlx::query("ALTER TABLE disputes DROP COLUMN seller_token")
.execute(&mut *transaction)
.await?;
tracing::info!("Dropped seller_token column");
}
if buyer_token_exists {
sqlx::query("ALTER TABLE disputes DROP COLUMN buyer_token")
.execute(&mut *tx)
.await
.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to drop buyer_token column: {}",
e
)))
})?;
tracing::info!("Dropped buyer_token column from disputes table");
}

if seller_token_exists {
sqlx::query("ALTER TABLE disputes DROP COLUMN seller_token")
.execute(&mut *tx)
.await
.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to drop seller_token column: {}",
e
)))
})?;
tracing::info!("Dropped seller_token column from disputes table");
}

Ok::<(), sqlx::Error>(())
}
.await;
tx.commit().await.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to commit transaction: {}",
e
)))
})?;

match drop_result {
Ok(_) => {
transaction.commit().await.map_err(|e| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to commit transaction: {}",
e
)))
})?;
tracing::info!("Successfully removed token columns using DROP COLUMN");
Ok(())
}
Err(e) => {
tracing::warn!("DROP COLUMN failed ({}), falling back to table rebuild", e);
transaction.rollback().await.map_err(|rollback_err| {
MostroInternalErr(ServiceError::DbAccessError(format!(
"Failed to rollback transaction: {}",
rollback_err
)))
})?;

// Fall back to table rebuild
rebuild_disputes_table_without_tokens(pool).await
}
}
} else {
// SQLite version doesn't support DROP COLUMN, use table rebuild
tracing::info!(
"SQLite version {} doesn't support DROP COLUMN, using table rebuild method",
sqlite_version
);
rebuild_disputes_table_without_tokens(pool).await
}
tracing::info!("Successfully removed deprecated token columns from disputes table");
Ok(())
}

async fn table_column_exists(
Expand Down Expand Up @@ -3598,19 +3456,6 @@ mod migration_and_query_tests {
.unwrap());
}

#[tokio::test]
async fn rebuild_disputes_table_preserves_rows() {
let pool = migrated_pool().await;
let order_id = Uuid::new_v4();
insert_dispute(&pool, order_id, "in-progress", Some(HEX_KEY_B)).await;

rebuild_disputes_table_without_tokens(&pool).await.unwrap();

let dispute = find_dispute_by_order_id(&pool, order_id).await.unwrap();
assert_eq!(dispute.order_id, order_id);
assert_eq!(dispute.status, "in-progress");
}

// ── admin / permission queries ───────────────────────────────────────

#[tokio::test]
Expand Down