Skip to content

Commit fbd98b4

Browse files
committed
Merge #605: Fix sqlite database set_utxo to insert or update utxos
35feb10 [CI] Fix cont_integration test-blockchains to run all tests (Steve Myers) 2471908 Update CHANGELOG with warning about sqlite-db deleted wallet data (Steve Myers) 0b1a399 Update sqlite schema with unique index for utxos, change insert_utxo to upsert (Steve Myers) cea7987 Update database tests to verify set_utxo upserts (Steve Myers) Pull request description: ### Description This PR fixes #591 by: 1. Add sqlite `MIGRATIONS` statements to remove duplicate utxos and add unique utxos index on txid and vout. 2. Do an upsert (if insert fails update) instead of an insert in `set_utxo()`. 3. Update database::test::test_utxo to also verify `set_utxo()` doesn't insert duplicate utxos. ### Notes to the reviewers I verified the updated `test_utxo` fails as expected before my fix and passes after the fix. I tested the new migrations using the below `bdk-cli` command and a manually updated sqlite db with duplicate utxos. ```shell cargo run --no-default-features --features cli,sqlite-db,esplora-ureq -- wallet -w test1 --descriptor "wpkh(tpubEBr4i6yk5nf5DAaJpsi9N2pPYBeJ7fZ5Z9rmN4977iYLCGco1VyjB9tvvuvYtfZzjD5A8igzgw3HeWeeKFmanHYqksqZXYXGsw5zjnj7KM9/*)" sync ``` ### 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 #### New Features: * [ ] I've added tests for the new feature * [ ] I've added docs for the new feature * [ ] I've updated `CHANGELOG.md` #### 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: danielabrozzoni: utACK 35feb10 - Code looks good, but I didn't do any local test to see if the db gets wiped Tree-SHA512: 753c7a0cfd0e803b5e12f39181d9a718791c4ce229d5072e6498db75a7008e94d447b3d0b4b0c205e7a8f127f60102e12bac2d271b8bad3a3038856bfd54e99c
2 parents 87b0745 + 35feb10 commit fbd98b4

File tree

4 files changed

+20
-5
lines changed

4 files changed

+20
-5
lines changed

.github/workflows/cont_integration.yml

+5-1
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,16 @@ jobs:
9090
matrix:
9191
blockchain:
9292
- name: electrum
93+
testprefix: blockchain::electrum::test
9394
features: test-electrum,verify
9495
- name: rpc
96+
testprefix: blockchain::rpc::test
9597
features: test-rpc
9698
- name: esplora
99+
testprefix: esplora
97100
features: test-esplora,use-esplora-reqwest,verify
98101
- name: esplora
102+
testprefix: esplora
99103
features: test-esplora,use-esplora-ureq,verify
100104
steps:
101105
- name: Checkout
@@ -114,7 +118,7 @@ jobs:
114118
toolchain: stable
115119
override: true
116120
- name: Test
117-
run: cargo test --no-default-features --features ${{ matrix.blockchain.features }} ${{ matrix.blockchain.name }}::bdk_blockchain_tests
121+
run: cargo test --no-default-features --features ${{ matrix.blockchain.features }} ${{ matrix.blockchain.testprefix }}::bdk_blockchain_tests
118122

119123
check-wasm:
120124
name: Check WASM

CHANGELOG.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
- New MSRV set to `1.56`
1111
- Unpinned tokio to `1`
1212
- Add traits to reuse `Blockchain`s across multiple wallets (`BlockchainFactory` and `StatelessBlockchain`).
13-
- Upgrade to rust-bitcoin `0.28`
14-
13+
- Upgrade to rust-bitcoin `0.28`
14+
- If using the `sqlite-db` feature all cached wallet data is deleted due to a possible UTXO inconsistency, a wallet.sync will recreate it
1515

1616
## [v0.18.0] - [v0.17.0]
1717

src/database/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,8 @@ pub mod test {
323323
};
324324

325325
tree.set_utxo(&utxo).unwrap();
326-
326+
tree.set_utxo(&utxo).unwrap();
327+
assert_eq!(tree.iter_utxos().unwrap().len(), 1);
327328
assert_eq!(tree.get_utxo(&outpoint).unwrap(), Some(utxo));
328329
}
329330

src/database/sqlite.rs

+11-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ static MIGRATIONS: &[&str] = &[
4141
"INSERT INTO transaction_details SELECT txid, timestamp, received, sent, fee, height FROM transaction_details_old;",
4242
"DROP TABLE transaction_details_old;",
4343
"ALTER TABLE utxos ADD COLUMN is_spent;",
44+
// drop all data due to possible inconsistencies with duplicate utxos, re-sync required
45+
"DELETE FROM checksums;",
46+
"DELETE FROM last_derivation_indices;",
47+
"DELETE FROM script_pubkeys;",
48+
"DELETE FROM sync_time;",
49+
"DELETE FROM transaction_details;",
50+
"DELETE FROM transactions;",
51+
"DELETE FROM utxos;",
52+
"DROP INDEX idx_txid_vout;",
53+
"CREATE UNIQUE INDEX idx_utxos_txid_vout ON utxos(txid, vout);"
4454
];
4555

4656
/// Sqlite database stored on filesystem
@@ -86,7 +96,7 @@ impl SqliteDatabase {
8696
script: &[u8],
8797
is_spent: bool,
8898
) -> Result<i64, Error> {
89-
let mut statement = self.connection.prepare_cached("INSERT INTO utxos (value, keychain, vout, txid, script, is_spent) VALUES (:value, :keychain, :vout, :txid, :script, :is_spent)")?;
99+
let mut statement = self.connection.prepare_cached("INSERT INTO utxos (value, keychain, vout, txid, script, is_spent) VALUES (:value, :keychain, :vout, :txid, :script, :is_spent) ON CONFLICT(txid, vout) DO UPDATE SET value=:value, keychain=:keychain, script=:script, is_spent=:is_spent")?;
90100
statement.execute(named_params! {
91101
":value": value,
92102
":keychain": keychain,

0 commit comments

Comments
 (0)