Fix SQLite panic when syncing many UTXOs by enabling WAL journal mode #1836
+221
−44
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
notmandatory
force-pushed
the
fix/sqlite_wal_mode
branch
from
b41987b to
406ccff
Compare
|
I verified this builds with |
6 tasks
8 tasks
|
I used I also build And then also locally imported those |
|
For those who are trying to reproduce the diff --git a/Cargo.toml b/Cargo.toml
index 1e322643..525e586f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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]]
diff --git a/src/blockchain/electrum.rs b/src/blockchain/electrum.rs
index 5bfd51ef..12e85a67 100644
--- a/src/blockchain/electrum.rs
+++ b/src/blockchain/electrum.rs
@@ -429,4 +429,178 @@ mod test {
ElectrumTester.run();
}
+
+ #[cfg(feature = "sqlite")]
+ #[test]
+ 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
+ );
+ }
}and run the test with the following command: cargo test --features test-electrum,sqlite --no-default-features -- test_electrum_large_num_utxos |
…000 ms
This prevents: Error { code: DatabaseBusy, extended_code: 5 }, Some("database is locked")
which occured when syncing very large number of utxos.
See: electrum::test::test_electrum_large_num_utxos
notmandatory
force-pushed
the
fix/sqlite_wal_mode
branch
from
406ccff to
7c4850f
Compare
|
I've rebased this PR and it's ready to review/merge. |
nymius
approved these changes
Feb 19, 2025
There was a problem hiding this comment.
tACK 7c4850f
I executed the test with the failing tip and took me ~2hs. Did the same on 7c4850f and got the following output:
test blockchain::electrum::test::test_electrum_large_num_utxos has been running for over 60 seconds
test blockchain::electrum::test::test_electrum_large_num_utxos ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 327 filtered out; finished in 8520.98s
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
fixes #1827
replaces #1828
I changed the
SqliteDatabasestruct to create new rusqliteConnections with WAL 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_utxostest 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
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingBugfixes: