Do not create the wallet struct directly; instead, call new. (#707)

The bug comes with the SQLx-sqlite pool bug, where several connections are
created by default, but the `new` function takes care of that, fixing that bug
by making a single instance of the database.

If constructed directly, the pool would create several connections to the
database, which in most instances is fine, but with SQLite :memory: each
connection is entirely independent.

Also follow documentation to make sure that failed `acquire` will not end up
dropping connections by setting  test_before_acquire to false

     However, if your workload is sensitive to dropped connections such as using an in-memory
     SQLite database with a pool size of 1, you can pretty easily ensure that a cancelled
     `acquire()` call will never drop connections by tweaking your [`PoolOptions`]:

     * Set [`test_before_acquire(false)`][PoolOptions::test_before_acquire]
     * Never set [`before_acquire`][PoolOptions::before_acquire] or
       [`after_connect`][PoolOptions::after_connect].
This commit is contained in:
C
2025-04-06 02:13:14 -04:00
committed by GitHub
parent d224cc57b5
commit 43ab1fdde1
9 changed files with 43 additions and 36 deletions

View File

@@ -128,8 +128,6 @@ async fn main() -> Result<()> {
}
};
sql.migrate().await;
Arc::new(sql)
}
"redb" => {

View File

@@ -193,7 +193,6 @@ pub async fn create_and_start_test_mint() -> Result<Mint> {
let database = cdk_sqlite::MintSqliteDatabase::new(&path)
.await
.expect("Could not create sqlite db");
database.migrate().await;
Arc::new(database)
}
"redb" => {
@@ -206,7 +205,6 @@ pub async fn create_and_start_test_mint() -> Result<Mint> {
}
"memory" => {
let database = cdk_sqlite::mint::memory::empty().await?;
database.migrate().await;
Arc::new(database)
}
_ => {
@@ -289,7 +287,6 @@ pub async fn create_test_wallet_for_mint(mint: Mint) -> Result<Wallet> {
let database = cdk_sqlite::WalletSqliteDatabase::new(&path)
.await
.expect("Could not create sqlite db");
database.migrate().await;
Arc::new(database)
}
"redb" => {
@@ -302,7 +299,6 @@ pub async fn create_test_wallet_for_mint(mint: Mint) -> Result<Wallet> {
}
"memory" => {
let database = cdk_sqlite::wallet::memory::empty().await?;
database.migrate().await;
Arc::new(database)
}
_ => {

View File

@@ -199,7 +199,6 @@ async fn test_multimint_melt() -> Result<()> {
)?;
let db = Arc::new(memory::empty().await?);
db.migrate().await;
let wallet2 = Wallet::new(
&get_second_mint_url_from_env(),
CurrencyUnit::Sat,

View File

@@ -130,8 +130,6 @@ async fn main() -> anyhow::Result<()> {
#[cfg(feature = "sqlcipher")]
let sqlite_db = MintSqliteDatabase::new(&sql_db_path, args.password).await?;
sqlite_db.migrate().await;
Arc::new(sqlite_db)
}
#[cfg(feature = "redb")]

View File

@@ -23,13 +23,24 @@ pub async fn create_sqlite_pool(
#[cfg(feature = "sqlcipher")]
let db_options = db_options.pragma("key", password);
let pool = SqlitePoolOptions::new()
let is_memory = path.contains(":memory:");
let options = SqlitePoolOptions::new()
.min_connections(1)
.max_connections(1)
.idle_timeout(None)
.max_lifetime(None)
.connect_with(db_options)
.await?;
.max_connections(1);
let pool = if is_memory {
// Make sure that the connection is not closed after the first query, or any query, as long
// as the pool is not dropped
options
.idle_timeout(None)
.max_lifetime(None)
.test_before_acquire(false)
} else {
options
}
.connect_with(db_options)
.await?;
Ok(pool)
}

View File

@@ -18,7 +18,6 @@ pub async fn empty() -> Result<MintSqliteDatabase, database::Error> {
let db = MintSqliteDatabase::new(":memory:").await?;
#[cfg(feature = "sqlcipher")]
let db = MintSqliteDatabase::new(":memory:", "memory".to_string()).await?;
db.migrate().await;
Ok(db)
}

View File

@@ -81,29 +81,34 @@ impl MintSqliteDatabase {
/// Create new [`MintSqliteDatabase`]
#[cfg(not(feature = "sqlcipher"))]
pub async fn new<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
Ok(Self {
let db = Self {
pool: create_sqlite_pool(path.as_ref().to_str().ok_or(Error::InvalidDbPath)?).await?,
})
};
db.migrate().await?;
Ok(db)
}
/// Create new [`MintSqliteDatabase`]
#[cfg(feature = "sqlcipher")]
pub async fn new<P: AsRef<Path>>(path: P, password: String) -> Result<Self, Error> {
Ok(Self {
let db = Self {
pool: create_sqlite_pool(
path.as_ref().to_str().ok_or(Error::InvalidDbPath)?,
password,
)
.await?,
})
};
db.migrate().await?;
Ok(db)
}
/// Migrate [`MintSqliteDatabase`]
pub async fn migrate(&self) {
async fn migrate(&self) -> Result<(), Error> {
sqlx::migrate!("./src/mint/migrations")
.run(&self.pool)
.await
.expect("Could not run migrations");
.map_err(|_| Error::CouldNotInitialize)?;
Ok(())
}
}

View File

@@ -6,11 +6,9 @@ use super::WalletSqliteDatabase;
/// Creates a new in-memory [`WalletSqliteDatabase`] instance
pub async fn empty() -> Result<WalletSqliteDatabase, Error> {
let db = WalletSqliteDatabase {
pool: sqlx::sqlite::SqlitePool::connect(":memory:")
.await
.map_err(|e| Error::Database(Box::new(e)))?,
};
db.migrate().await;
#[cfg(not(feature = "sqlcipher"))]
let db = WalletSqliteDatabase::new(":memory:").await?;
#[cfg(feature = "sqlcipher")]
let db = WalletSqliteDatabase::new(":memory:", "memory".to_owned()).await?;
Ok(db)
}

View File

@@ -35,29 +35,34 @@ impl WalletSqliteDatabase {
/// Create new [`WalletSqliteDatabase`]
#[cfg(not(feature = "sqlcipher"))]
pub async fn new<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
Ok(Self {
let db = Self {
pool: create_sqlite_pool(path.as_ref().to_str().ok_or(Error::InvalidDbPath)?).await?,
})
};
db.migrate().await?;
Ok(db)
}
/// Create new [`WalletSqliteDatabase`]
#[cfg(feature = "sqlcipher")]
pub async fn new<P: AsRef<Path>>(path: P, password: String) -> Result<Self, Error> {
Ok(Self {
let db = Self {
pool: create_sqlite_pool(
path.as_ref().to_str().ok_or(Error::InvalidDbPath)?,
password,
)
.await?,
})
};
db.migrate().await?;
Ok(db)
}
/// Migrate [`WalletSqliteDatabase`]
pub async fn migrate(&self) {
async fn migrate(&self) -> Result<(), Error> {
sqlx::migrate!("./src/wallet/migrations")
.run(&self.pool)
.await
.expect("Could not run migrations");
.map_err(|_| Error::CouldNotInitialize)?;
Ok(())
}
}
@@ -1147,8 +1152,6 @@ mod tests {
#[cfg(not(feature = "sqlcipher"))]
let db = WalletSqliteDatabase::new(path).await.unwrap();
db.migrate().await;
// Create a proof with DLEQ
let keyset_id = Id::from_str("00deadbeef123456").unwrap();
let mint_url = MintUrl::from_str("https://example.com").unwrap();