Fixed race condition

Bug: https://github.com/cashubtc/cdk/actions/runs/15683152414/job/44190084378?pr=822#step:5:19212

Reason: a race condition between removing proofs while melting and the quote states being updated.

Solution:

1. Error on duplicate proofs
2. Read quote when updating to avoid race conditions and rollbacks

Real solution: A transaction trait in the storage layer. That is coming next
This commit is contained in:
Cesar Rodas
2025-06-16 13:59:56 -03:00
parent 86eb7b8676
commit 7146cb8934
8 changed files with 99 additions and 23 deletions

View File

@@ -4,7 +4,7 @@ use std::sync::{mpsc as std_mpsc, Arc, Mutex};
use std::thread::spawn;
use std::time::Instant;
use rusqlite::{Connection, TransactionBehavior};
use rusqlite::{ffi, Connection, ErrorCode, TransactionBehavior};
use tokio::sync::{mpsc, oneshot};
use crate::common::SqliteConnectionManager;
@@ -202,6 +202,26 @@ fn rusqlite_spawn_worker_threads(
Ok(ok) => reply_to.send(ok),
Err(err) => {
tracing::error!("Failed query with error {:?}", err);
let err = if let Error::Sqlite(rusqlite::Error::SqliteFailure(
ffi::Error {
code,
extended_code,
},
_,
)) = &err
{
if *code == ErrorCode::ConstraintViolation
&& (*extended_code == ffi::SQLITE_CONSTRAINT_PRIMARYKEY
|| *extended_code == ffi::SQLITE_CONSTRAINT_UNIQUE)
{
Error::Duplicate
} else {
err
}
} else {
err
};
reply_to.send(DbResponse::Error(err))
}
};
@@ -331,6 +351,27 @@ fn rusqlite_worker_manager(
tx_id,
err
);
let err = if let Error::Sqlite(
rusqlite::Error::SqliteFailure(
ffi::Error {
code,
extended_code,
},
_,
),
) = &err
{
if *code == ErrorCode::ConstraintViolation
&& (*extended_code == ffi::SQLITE_CONSTRAINT_PRIMARYKEY
|| *extended_code == ffi::SQLITE_CONSTRAINT_UNIQUE)
{
Error::Duplicate
} else {
err
}
} else {
err
};
reply_to.send(DbResponse::Error(err))
}
};

View File

@@ -9,6 +9,10 @@ pub enum Error {
#[error(transparent)]
Sqlite(#[from] rusqlite::Error),
/// Duplicate entry
#[error("Record already exists")]
Duplicate,
/// Pool error
#[error(transparent)]
Pool(#[from] crate::pool::Error<rusqlite::Error>),
@@ -98,6 +102,9 @@ pub enum Error {
impl From<Error> for cdk_common::database::Error {
fn from(e: Error) -> Self {
Self::Database(Box::new(e))
match e {
Error::Duplicate => Self::Duplicate,
e => Self::Database(Box::new(e)),
}
}
}

View File

@@ -674,10 +674,10 @@ ON CONFLICT(request_lookup_id) DO UPDATE SET
&self,
quote_id: &Uuid,
state: MeltQuoteState,
) -> Result<MeltQuoteState, Self::Err> {
) -> Result<(MeltQuoteState, mint::MeltQuote), Self::Err> {
let transaction = self.pool.begin().await?;
let quote = query(
let mut quote = query(
r#"
SELECT
id,
@@ -732,7 +732,10 @@ ON CONFLICT(request_lookup_id) DO UPDATE SET
}
};
Ok(quote.state)
let old_state = quote.state;
quote.state = state;
Ok((old_state, quote))
}
async fn remove_melt_quote(&self, quote_id: &Uuid) -> Result<(), Self::Err> {
@@ -816,7 +819,7 @@ impl MintProofsDatabase for MintSqliteDatabase {
for proof in proofs {
query(
r#"
INSERT OR IGNORE INTO proof
INSERT INTO proof
(y, amount, keyset_id, secret, c, witness, state, quote_id, created_time)
VALUES
(:y, :amount, :keyset_id, :secret, :c, :witness, :state, :quote_id, :created_time)
@@ -852,10 +855,11 @@ impl MintProofsDatabase for MintSqliteDatabase {
let total_deleted = query(
r#"
DELETE FROM proof WHERE y IN (:ys) AND state != 'SPENT'
DELETE FROM proof WHERE y IN (:ys) AND state NOT IN (:exclude_state)
"#,
)
.bind_vec(":ys", ys.iter().map(|y| y.to_bytes().to_vec()).collect())
.bind_vec(":exclude_state", vec![State::Spent.to_string()])
.execute(&transaction)
.await?;
@@ -974,7 +978,11 @@ impl MintProofsDatabase for MintSqliteDatabase {
if current_states.len() != ys.len() {
transaction.rollback().await?;
tracing::warn!("Attempted to update state of non-existent proof");
tracing::warn!(
"Attempted to update state of non-existent proof {} {}",
current_states.len(),
ys.len()
);
return Err(database::Error::ProofNotFound);
}