浏览代码

Do not create the wallet struct directly; instead, call new.

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].
Cesar Rodas 3 周之前
父节点
当前提交
d8864dbcb6

+ 0 - 2
crates/cdk-cli/src/main.rs

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

+ 0 - 4
crates/cdk-integration-tests/src/init_pure_tests.rs

@@ -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)
             }
             _ => {

+ 0 - 1
crates/cdk-integration-tests/tests/regtest.rs

@@ -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,

+ 0 - 2
crates/cdk-mintd/src/main.rs

@@ -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")]

+ 17 - 6
crates/cdk-sqlite/src/common.rs

@@ -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)
 }

+ 0 - 1
crates/cdk-sqlite/src/mint/memory.rs

@@ -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)
 }
 

+ 11 - 6
crates/cdk-sqlite/src/mint/mod.rs

@@ -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(())
     }
 }
 

+ 4 - 6
crates/cdk-sqlite/src/wallet/memory.rs

@@ -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)
 }

+ 11 - 8
crates/cdk-sqlite/src/wallet/mod.rs

@@ -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();