Merge #1836: Fix SQLite panic when syncing many UTXOs by enabling WAL…

… journal mode 7c4850f release: bump version to 0.30.2 (Steve Myers) 1065511 fix(sqlite): set connection journal_mode to WAL and busy_timeout to 5000 ms (Steve Myers) 344fa3f test: repro bug with large num utxos and sqlite (Steve Myers) 957b219 deps: downgrade dev dep electrsd to 0.24.0 (Steve Myers) 3b38892 ci: fix pinned rustls and add ci/pin-msrv.sh (Steve Myers) Pull request description: ### Description fixes #1827 replaces #1828 I changed the `SqliteDatabase` struct to create new rusqlite `Connection`s with [WAL](https://www.sqlite.org/wal.html) journal mode enabled and 5000ms busy_timeout. This prevents a large sync from trying to start and commit a batch (db transaction) while the initial non-batch connection is still busy writing it's data. ### Notes to the reviewers I commented out the `electrum::test_electrum_large_num_utxos` test since it takes an hour to run. Before the fix in thie PR it would also fail with only 10 large TX. The dev dependency and pinning changes were required to run the new, and other tests. ### Changelog notice - Fix SQLite panic when syncing many UTXOs ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### Bugfixes: * [ ] This pull request breaks the existing API * [x] I've added tests to reproduce the issue which are now passing * [x] I'm linking the issue being fixed by this PR ACKs for top commit: ValuedMammal: utACK 7c4850f nymius: tACK 7c4850f evanlinjin: utACK 7c4850f Tree-SHA512: ad10b8b4354bdcbc5b9aeb1d06f68f30501dcf4ed687b387f514ff7de5e034b729dbdf25f59a8b269e562f0dab8992aa187bf187e3e0adf585838ae96afb40cc
bitcoindevkit · Feb 21, 2025 · f71bc34 · f71bc34
2 parents 128a6c8 + 7c4850f
commit f71bc34
Show file tree

Hide file tree

Showing 7 changed files with 221 additions and 44 deletions.
diff --git a/.github/workflows/cont_integration.yml b/.github/workflows/cont_integration.yml
@@ -57,16 +57,7 @@ jobs:
           components: clippy
       - name: Pin dependencies for MSRV
         if: matrix.rust.version == '1.63.0'
-        run: |
-          cargo update -p tokio --precise "1.38.1"
-          cargo update -p tokio-util --precise "0.7.11"
-          cargo update -p home --precise "0.5.5"
-          cargo update -p regex --precise "1.7.3"
-          cargo update -p security-framework-sys --precise "2.11.1"
-          cargo update -p url --precise "2.5.0"
-          cargo update -p [email protected] --precise "0.23.19"
-          cargo update -p [email protected] --precise "0.15.0"
-          cargo update -p ureq --precise "2.10.1"
+        run: ./ci/pin-msrv.sh
       - name: Build
         run: cargo build --features bitcoin/std,miniscript/std,${{ matrix.features }} --no-default-features
       - name: Clippy
@@ -204,11 +195,6 @@ jobs:
         toolchain: ${{ matrix.rust.version }}
     - name: Pin dependencies for MSRV
       if: matrix.rust.version == '1.63.0'
-      run: |
-        cargo update -p tokio --precise "1.38.1"
-        cargo update -p tokio-util --precise "0.7.11"
-        cargo update -p home --precise "0.5.5"
-        cargo update -p regex --precise "1.7.3"
-        cargo update -p security-framework-sys --precise "2.11.1"
+      run: ./ci/pin-msrv.sh
     - name: Test
       run: cargo test --features test-hardware-signer
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,9 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-## [v0.30.1]
+## [v0.30.2]
 
-### Fixed
+- Fix out of memory issue caused by batch fetching many large txs #1831
+- Fix SQLite panic when syncing many large txs #1836
+
+## [v0.30.1]
 
 - Fix electrum conftime_req filter for needs_block_height #1782
 
@@ -707,4 +710,5 @@ final transaction is created by calling `finish` on the builder.
 [v0.29.0]: https://github.com/bitcoindevkit/bdk/compare/v0.28.2...v0.29.0
 [v0.30.0]: https://github.com/bitcoindevkit/bdk/compare/v0.29.0...v0.30.0
 [v0.30.1]: https://github.com/bitcoindevkit/bdk/compare/v0.30.0...v0.30.1
-[Unreleased]: https://github.com/bitcoindevkit/bdk/compare/v0.30.1...release/0.29
+[v0.30.2]: https://github.com/bitcoindevkit/bdk/compare/v0.30.1...v0.30.2
+[Unreleased]: https://github.com/bitcoindevkit/bdk/compare/v0.30.2...release/0.30
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bdk"
-version = "0.30.1"
+version = "0.30.2"
 authors = ["Alekos Filini <[email protected]>", "Riccardo Casatta <[email protected]>"]
 homepage = "https://bitcoindevkit.org"
 repository = "https://github.com/bitcoindevkit/bdk"
@@ -94,10 +94,10 @@ reqwest-default-tls = ["esplora-client/async-https"]
 
 # Debug/Test features
 test-blockchains = ["bitcoincore-rpc", "electrum-client"]
-test-electrum = ["electrum", "electrsd/electrs_0_8_10", "electrsd/bitcoind_23_1", "test-blockchains"]
-test-rpc = ["rpc", "electrsd/electrs_0_8_10", "electrsd/bitcoind_23_1", "test-blockchains"]
-test-rpc-legacy = ["rpc", "electrsd/electrs_0_8_10", "electrsd/bitcoind_23_1", "test-blockchains"]
-test-esplora = ["electrsd/legacy", "electrsd/esplora_a33e97e1", "electrsd/bitcoind_23_1", "test-blockchains"]
+test-electrum = ["electrum", "electrsd/electrs_0_8_10", "electrsd/bitcoind_23_0", "test-blockchains"]
+test-rpc = ["rpc", "electrsd/electrs_0_8_10", "electrsd/bitcoind_23_0", "test-blockchains"]
+test-rpc-legacy = ["rpc", "electrsd/electrs_0_8_10", "electrsd/bitcoind_23_0", "test-blockchains"]
+test-esplora = ["electrsd/legacy", "electrsd/esplora_a33e97e1", "electrsd/bitcoind_23_0", "test-blockchains"]
 test-md-docs = ["electrum"]
 test-hardware-signer = ["hardware-signer"]
 
@@ -111,7 +111,7 @@ miniscript = { version = "10.0", features = ["std"] }
 bitcoin = { version = "0.30", features = ["std"] }
 lazy_static = "1.4"
 env_logger = { version = "0.7", default-features = false }
-electrsd = "0.29.0"
+electrsd = "0.24.0"
 assert_matches = "1.5.0"
 
 [[example]]
@@ -130,7 +130,7 @@ path = "examples/policy.rs"
 [[example]]
 name = "rpcwallet"
 path = "examples/rpcwallet.rs"
-required-features = ["keys-bip39", "key-value-db", "rpc", "electrsd/bitcoind_22_1"]
+required-features = ["keys-bip39", "key-value-db", "rpc", "electrsd/bitcoind_22_0"]
 
 [[example]]
 name = "psbt_signer"

diff --git a/README.md b/README.md
@@ -212,16 +212,4 @@ dual licensed as above, without any additional terms or conditions.
 
 This library should compile with any combination of features with Rust 1.63.0.
 
-To build with the MSRV you will need to pin dependencies as follows:
-
-```shell
-cargo update -p tokio --precise "1.38.1"
-cargo update -p tokio-util --precise "0.7.11"
-cargo update -p home --precise "0.5.5"
-cargo update -p regex --precise "1.7.3"
-cargo update -p security-framework-sys --precise "2.11.1"
-cargo update -p url --precise "2.5.0"
-cargo update -p [email protected] --precise "0.23.19"
-cargo update -p [email protected] --precise "0.15.0"
-cargo update -p ureq --precise "2.10.1"
-```
+To build with the MSRV of 1.63.0 you will need to pin dependencies by running the [`pin-msrv.sh`](./ci/pin-msrv.sh) script.
diff --git a/ci/pin-msrv.sh b/ci/pin-msrv.sh
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+set -x
+set -euo pipefail
+
+# Pin dependencies for MSRV
+
+# To pin deps, switch toolchain to MSRV and execute the below updates
+
+# cargo clean
+# rustup override set 1.63.0
+cargo update -p tokio --precise "1.38.1"
+cargo update -p tokio-util --precise "0.7.11"
+cargo update -p home --precise "0.5.5"
+cargo update -p regex --precise "1.7.3"
+cargo update -p security-framework-sys --precise "2.11.1"
+cargo update -p url --precise "2.5.0"
+cargo update -p [email protected] --precise "0.23.19"
+cargo update -p [email protected] --precise "0.15.0"
+cargo update -p ureq --precise "2.10.1"
diff --git a/src/blockchain/electrum.rs b/src/blockchain/electrum.rs
@@ -131,8 +131,8 @@ impl WalletSync for ElectrumBlockchain {
         let chunk_size = self.stop_gap + 1;
 
         // The electrum server has been inconsistent somehow in its responses during sync. For
-        // example, we do a batch request of transactions and the response contains less
-        // tranascations than in the request. This should never happen but we don't want to panic.
+        // example, we do a batch request of transactions and the response contains fewer
+        // transactions than in the request. This should never happen, but we don't want to panic.
         let electrum_goof = || Error::Generic("electrum server misbehaving".to_string());
 
         let batch_update = loop {
@@ -345,8 +345,6 @@ impl ConfigurableBlockchain for ElectrumBlockchain {
 #[cfg(test)]
 #[cfg(feature = "test-electrum")]
 mod test {
-    use std::sync::Arc;
-
     use super::*;
     use crate::database::MemoryDatabase;
     use crate::testutils::blockchain_tests::TestClient;
@@ -434,4 +432,179 @@ mod test {
 
         ElectrumTester.run();
     }
+
+    #[cfg(feature = "sqlite")]
+    #[test]
+    #[ignore] // takes ~1 hr to complete, here as reference for future testing
+    fn test_electrum_large_num_utxos() {
+        use crate::database::SqliteDatabase;
+        use crate::wallet::coin_selection::OldestFirstCoinSelection;
+        use crate::SignOptions;
+        use bitcoin::Amount;
+        use bitcoincore_rpc::RpcApi;
+        use std::time::{SystemTime, UNIX_EPOCH};
+
+        const NUM_TX: u32 = 50;
+        const NUM_UTXO: u32 = 700;
+
+        env_logger::init();
+        let mut test_client = TestClient::default();
+        let electrum_blockchain =
+            ElectrumBlockchain::from(Client::new(&test_client.electrsd.electrum_url).unwrap());
+
+        // fund bdk wallet 1 with regtest node coinbase txs
+        let mem_db = MemoryDatabase::new();
+        let wallet1_descriptor = "wpkh(tprv8i8F4EhYDMquzqiecEX8SKYMXqfmmb1Sm7deoA1Hokxzn281XgTkwsd6gL8aJevLE4aJugfVf9MKMvrcRvPawGMenqMBA3bRRfp4s1V7Eg3/0/*)";
+        let wallet1 =
+            Wallet::new(wallet1_descriptor, None, bitcoin::Network::Regtest, mem_db).unwrap();
+        let wallet1_address = wallet1.get_address(AddressIndex::New).unwrap().address;
+        test_client
+            .send_to_address(
+                &wallet1_address,
+                Amount::from_btc(5.0).unwrap(),
+                None,
+                None,
+                None,
+                None,
+                None,
+                None,
+            )
+            .unwrap();
+        test_client.generate(1, None);
+        wallet1
+            .sync(&electrum_blockchain, Default::default())
+            .unwrap();
+        assert_eq!(wallet1.get_balance().unwrap().confirmed, 5_0000_0000);
+        // bdk wallet 1 creates NUM_TX tx * NUM_UTXO utxos and sends them back to itself
+        for _ in 0..NUM_TX {
+            let amount = 2715;
+            let address_amounts = (0..NUM_UTXO)
+                .map(|_| {
+                    (
+                        wallet1
+                            .get_address(AddressIndex::New)
+                            .unwrap()
+                            .address
+                            .script_pubkey(),
+                        amount,
+                    )
+                })
+                .collect::<Vec<_>>();
+            let mut tx_builder = wallet1.build_tx().coin_selection(OldestFirstCoinSelection);
+            // only allow spending utxos greater than 2715 sats
+            let unspendable = wallet1
+                .list_unspent()
+                .unwrap()
+                .iter()
+                .filter(|utxo| utxo.txout.value <= amount)
+                .map(|utxo| utxo.outpoint)
+                .collect::<Vec<_>>();
+            tx_builder
+                .set_recipients(address_amounts)
+                .unspendable(unspendable);
+            let (mut psbt, _details) = tx_builder.finish().unwrap();
+            assert!(wallet1.sign(&mut psbt, SignOptions::default()).unwrap());
+            let tx = psbt.extract_tx();
+            electrum_blockchain.broadcast(&tx).unwrap();
+            // include test txs in a block
+            test_client.generate(1, None);
+            wallet1
+                .sync(&electrum_blockchain, Default::default())
+                .unwrap()
+        }
+        assert_eq!(
+            (NUM_TX * NUM_UTXO) as usize,
+            wallet1
+                .list_unspent()
+                .unwrap()
+                .iter()
+                .filter(|utxo| utxo.txout.value == 2715)
+                .count()
+        );
+
+        // bdk wallet 2 to receives NUM_TX tx with NUM_UTXO utxos from wallet 1
+        let time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
+        let mut dir = std::env::temp_dir();
+        dir.push(format!("bdk_{}", time.as_nanos()));
+        let sqlite_db = SqliteDatabase::new(String::from(dir.to_str().unwrap()));
+        let wallet2_descriptor = "wpkh(tprv8i8F4EhYDMquzqiecEX8SKYMXqfmmb1Sm7deoA1Hokxzn281XgTkwsd6gL8aJevLE4aJugfVf9MKMvrcRvPawGMenqMBA3bRRfp4s1V7Eg3/1/*)";
+        let wallet2 = Wallet::new(
+            wallet2_descriptor,
+            None,
+            bitcoin::Network::Regtest,
+            sqlite_db,
+        )
+        .unwrap();
+        wallet2
+            .sync(&electrum_blockchain, Default::default())
+            .unwrap();
+        assert_eq!(0, wallet2.get_balance().unwrap().confirmed);
+
+        // send NUM_TX tx with NUM_UTXO utxos each from wallet1 to wallet2
+        for _ in 0..NUM_TX {
+            let amount = 2715;
+            let address_amounts = (0..NUM_UTXO)
+                .map(|_| {
+                    (
+                        wallet2
+                            .get_address(AddressIndex::New)
+                            .unwrap()
+                            .address
+                            .script_pubkey(),
+                        amount,
+                    )
+                })
+                .collect::<Vec<_>>();
+            let fee_utxo = wallet1
+                .list_unspent()
+                .unwrap()
+                .iter()
+                .filter(|utxo| utxo.txout.value > amount)
+                .map(|utxo| utxo.outpoint)
+                .last()
+                .unwrap()
+                .clone();
+            let spend_utxos = wallet1
+                .list_unspent()
+                .unwrap()
+                .iter()
+                .filter(|utxo| utxo.txout.value == amount)
+                .map(|utxo| utxo.outpoint)
+                .take(NUM_UTXO as usize)
+                .collect::<Vec<_>>();
+            let mut tx_builder = wallet1.build_tx().coin_selection(OldestFirstCoinSelection);
+            tx_builder
+                .manually_selected_only()
+                .set_recipients(address_amounts)
+                .add_utxos(&spend_utxos)
+                .unwrap()
+                .add_utxo(fee_utxo)
+                .unwrap();
+            let (mut psbt, _details) = tx_builder.finish().unwrap();
+            assert!(wallet1.sign(&mut psbt, SignOptions::default()).unwrap());
+            let tx = psbt.extract_tx();
+            electrum_blockchain.broadcast(&tx).unwrap();
+            // include test txs in a block
+            test_client.generate(1, None);
+            wallet1
+                .sync(&electrum_blockchain, Default::default())
+                .unwrap()
+        }
+        wallet2
+            .sync(&electrum_blockchain, Default::default())
+            .unwrap();
+        assert_eq!(
+            (NUM_TX * NUM_UTXO) as usize,
+            wallet2
+                .list_unspent()
+                .unwrap()
+                .iter()
+                .filter(|utxo| utxo.txout.value == 2715)
+                .count()
+        );
+        assert_eq!(
+            wallet2.get_balance().unwrap().confirmed,
+            (2715 * NUM_UTXO * NUM_TX) as u64
+        );
+    }
 }
diff --git a/src/database/sqlite.rs b/src/database/sqlite.rs
@@ -86,7 +86,13 @@ impl SqliteDatabase {
     /// Instantiate a new SqliteDatabase instance by creating a connection
     /// to the database stored at path
     pub fn new<T: AsRef<Path>>(path: T) -> Self {
-        let connection = get_connection(&path).unwrap();
+        let connection = get_connection(&path).expect("Failed to open database");
+        connection
+            .execute_batch("PRAGMA journal_mode = WAL")
+            .expect("Failed to set WAL journal mode");
+        connection
+            .execute_batch("PRAGMA busy_timeout = 5000")
+            .expect("Failed to set busy_timeout");
         SqliteDatabase {
             path: PathBuf::from(path.as_ref()),
             connection,