diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c5058d6..075761e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -3,7 +3,7 @@ name: CI on: push: branches: [ "master" ] - + pull_request: types: [ opened, synchronize, reopened ] branches: @@ -11,99 +11,11 @@ on: env: CARGO_TERM_COLOR: auto - PGPASSWORD: password - DATABASE_TEST_URL: postgres://postgres:password@localhost:5432/testdb + DATABASE_TEST_URL: postgres://postgres:password@localhost:5432/postgres jobs: - wallet-is-persisted: - name: Test wallet persistence - runs-on: ubuntu-latest - services: - postgres: - image: postgres:14 - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: password - POSTGRES_DB: postgres - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - ports: - - 5432:5432 - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - uses: Swatinem/rust-cache@v2 - - name: Create database - run: | - sudo apt-get install libpq-dev -y - psql -h localhost -p 5432 -U postgres -d postgres -c 'create user testuser' - psql -h localhost -p 5432 -U postgres -d postgres -c 'create database testdb with owner = testuser' - - name: Test wallet_is_persisted - run: cargo test wallet_is_persisted -- --show-output - - test-three-wallets: - name: Test three wallets list transactions - runs-on: ubuntu-latest - services: - postgres: - image: postgres:14 - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: password - POSTGRES_DB: postgres - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - ports: - - 5432:5432 - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - uses: Swatinem/rust-cache@v2 - - name: Create database - run: | - sudo apt-get install libpq-dev -y - psql -h localhost -p 5432 -U postgres -d postgres -c 'create user testuser' - psql -h localhost -p 5432 -U postgres -d postgres -c 'create database testdb with owner = testuser' - - name: Test test_three_wallets_list_transactions - run: cargo test test_three_wallets_list_transactions -- --show-output - - wallet-load-checks: - name: Test wallet load checks - runs-on: ubuntu-latest - services: - postgres: - image: postgres:14 - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: password - POSTGRES_DB: postgres - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - ports: - - 5432:5432 - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - uses: Swatinem/rust-cache@v2 - - name: Create database - run: | - sudo apt-get install libpq-dev -y - psql -h localhost -p 5432 -U postgres -d postgres -c 'create user testuser' - psql -h localhost -p 5432 -U postgres -d postgres -c 'create database testdb with owner = testuser' - - name: Test wallet_load_checks - run: cargo test wallet_load_checks -- --show-output - - single-descriptor-wallet: - name: Test single descriptor wallet + test: + name: Test suite runs-on: ubuntu-latest services: postgres: @@ -123,42 +35,8 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - - name: Create database - run: | - sudo apt-get install libpq-dev -y - psql -h localhost -p 5432 -U postgres -d postgres -c 'create user testuser' - psql -h localhost -p 5432 -U postgres -d postgres -c 'create database testdb with owner = testuser' - - name: Test single_descriptor_wallet_persist_and_recover - run: cargo test single_descriptor_wallet_persist_and_recover -- --show-output - - two-wallets-load: - name: Test two wallets load - runs-on: ubuntu-latest - services: - postgres: - image: postgres:14 - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: password - POSTGRES_DB: postgres - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - ports: - - 5432:5432 - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - uses: Swatinem/rust-cache@v2 - - name: Create database - run: | - sudo apt-get install libpq-dev -y - psql -h localhost -p 5432 -U postgres -d postgres -c 'create user testuser' - psql -h localhost -p 5432 -U postgres -d postgres -c 'create database testdb with owner = testuser' - - name: Test two_wallets_load - run: cargo test two_wallets_load -- --show-output + - name: Test + run: cargo test -- --show-output fmt-clippy: name: Check @@ -172,4 +50,13 @@ jobs: - name: Check fmt run: cargo fmt --all -- --check - name: Clippy - run: cargo clippy --all-targets -- -Dwarnings \ No newline at end of file + run: cargo clippy --all-targets -- -Dwarnings + + audit: + name: Audit dependencies + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: rustsec/audit-check@v2 + with: + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..6201729 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,102 @@ +# Changelog + +All notable defects found in review and fixed are listed here. Each fix is +guarded by an always-on regression test in the suite (`src/test.rs` and +`tests/builder_network.rs`). + +## Unreleased + +### Fixed + +- Two writers persisting different block hashes for the same previously + unoccupied height raced the block table's two unique indexes on postgres: + the loser's `DELETE` could not see the winner's uncommitted row, and its + `INSERT` then violated `idx_block_wallet_height` — an index the upsert's + `(wallet_name, hash)` conflict target does not cover — aborting the loser's + whole changeset with a raw `23505`. Block writes are now serialized per + wallet with a transaction-scoped advisory lock; the loser waits for the + winner to commit, then sees and replaces its row (last-writer-wins), exactly + as if the writes had been issued sequentially. sqlite needs no equivalent: + its single-writer lock already serializes the same interleaving. Reproduced + before the fix (`duplicate key value violates unique constraint + "idx_block_wallet_height"`); regression test + `concurrent_block_writes_at_same_height_both_land` covers both backends. +- The `tx.last_seen` upsert overwrote unconditionally, so a stale or replayed + changeset moved the timestamp backwards, contradicting bdk_chain's own + `Merge` (last_seen only ever increases). The conflict update now keeps the + maximum on both backends, matching the monotonic `last_revealed` update. + Regression test `last_seen_never_regresses` covers both backends. +- The sqlite backend propagated raw `BdkSqlxError::Sqlx` for statement + failures while postgres wrapped them in `BdkSqlxError::QueryError` with + table context, so callers could not match on one error kind for "the write + failed at the database". sqlite now wraps with the same table labels. + +### Added + +- `SqliteStoreBuilder`, mirroring `PgStoreBuilder` + (`new(wallet_name).network(..).migrate(..).pool(..).build()` / + `build_with_url(..)`; `build_with_url(None)` builds the single-connection + in-memory store). `Store::::new_with_url` now delegates to it. +- `Store::::migrate()`, mirroring `Store::::migrate`. + +### Changed + +- Tests no longer panic when `DATABASE_TEST_URL` is unset: postgres-backend + tests skip gracefully (with a one-time notice) and the sqlite backend still + runs. CI sets the variable, so full coverage always runs there. +- `#[tracing::instrument]` on persist-path helpers is uniformly `skip_all` on + both backends: one rule, no span records arguments. +- Module-internal free functions were `pub` in private modules (unreachable, + misleading); they are now `pub(crate)`. +- The README's "Resolved defects" section moved here. + +## Resolved defects (previous review round) + +- `tx.last_seen` for a tx not yet stored was silently dropped (the `UPDATE` + affected 0 rows); the write now upserts a stub row (`whole_tx` is nullable). +- Reads anchored on the `network` row, so rows persisted by a changeset that + carried no network were written but never read back; tx/block tables are now + read unconditionally. +- A changeset mapping the same block hash to several heights silently + collapsed to one block row, losing checkpoints; such changesets are now + rejected with `DuplicateBlockHash`. +- The postgres `write` path did not validate `changeset.network` against the + configured network, letting a foreign network overwrite the row and wedge + all subsequent reads; the write is now rejected with `InvalidNetwork`. +- `keychain.last_revealed INTEGER DEFAULT 0` made a wallet persisted before + its first address reveal reload with index 0 marked as used, skipping it + forever. New rows now store NULL explicitly and migration 04 drops the + default. Existing rows are deliberately untouched: a stored `0` is + ambiguous ("revealed index 0" vs "never revealed") and rewriting it could + cause address reuse. +- `update_last_revealed` was a plain `UPDATE`, letting a stale/replayed + changeset move the derivation index backwards and silently reuse addresses; + the update now never decreases the stored value. +- `Store::::read` ran at READ COMMITTED (per-statement snapshots), + so a concurrent writer could produce a mixed-generation changeset; the read + transaction now uses REPEATABLE READ. +- `initialize_network` had a check-then-set race that failed concurrent + same-network builds spuriously with `SetNetworkFailure`; a lost race now + re-validates instead. +- `Store` derived `Clone` with a `DB: Clone` bound that sqlx's `Postgres`/ + `Sqlite` marker types do not satisfy, making the impl unusable; a manual + bound-free impl is provided. +- Reads anchored keychain rows on the `network` row, so descriptors and + derivation state persisted by a changeset that carried no network were + written but never read back (the tx/block invisibility defect, one table + over); keychain rows are now read unconditionally. +- The sqlite backend had no network validation at all: its constructor took + no network and any stored or incoming network was accepted. It now takes + the network at construction (shared process-global with the postgres + backend, so one process can never mix networks) and applies the same + read/write guards. `Store::::new` and `new_with_url` therefore + take a `network` argument. +- `insert_descriptor`'s conflict update kept the stored `last_revealed` + unconditionally, so replacing a descriptor under the same + `(wallet_name, keychainkind)` made the new descriptor inherit the old + derivation index and silently skip those addresses on load. The keep is + now conditional on the descriptor being unchanged; a replaced descriptor + restarts derivation at NULL. +- A `keychainkind` value outside `'External'`/`'Internal'` was silently + ignored on load, dropping a keychain; corrupt rows now fail with + `InvalidKeychainKind`. diff --git a/Cargo.toml b/Cargo.toml index e181ee1..2243270 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,21 +4,21 @@ version = "0.0.1" edition = "2021" [dependencies] -bdk_wallet = { version = "1.2.0", features = ["test-utils"] } +bdk_wallet = { version = "1.2.0" } serde = { version = "1.0.208", features = ["derive"] } serde_json = "1.0.125" sqlx = { version = "0.8.1", default-features = false, features = ["runtime-tokio", "tls-rustls-ring","derive", "postgres", "sqlite", "json", "chrono", "uuid", "sqlx-macros", "migrate"] } thiserror = "1" -tokio = { version = "1.40.0", features = ["macros", "rt-multi-thread"] } tracing = "0.1.40" -tracing-subscriber = { version = "0.3.18", features = ["env-filter", "serde_json", "json"] } -sqlx-postgres-tester = "0.1.1" [dev-dependencies] assert_matches = "1.5.0" anyhow = "1.0.89" bdk_electrum = { version = "0.20.1"} +bdk_wallet = { version = "1.2.0", features = ["test-utils"] } rustls = "0.23.14" +tokio = { version = "1.40.0", features = ["macros", "rt-multi-thread", "sync"] } +tracing-subscriber = { version = "0.3.18", features = ["env-filter", "serde_json", "json"] } [[example]] name = "bdk_sqlx_postgres" diff --git a/README.md b/README.md index 6ce4bdb..5c35ff9 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,20 @@ This crate is still **EXPERIMENTAL** do not use with mainnet wallets. +Defects found in review and fixed (each guarded by an always-on regression +test) are listed in [CHANGELOG.md](CHANGELOG.md). + +## Security notes + +- Without an explicit `sslmode`, postgres connections default to `prefer`, which + silently falls back to plaintext if TLS negotiation fails. For any deployment where + the database is not on the same host, require TLS in the connection URL + (`?sslmode=require`, or `verify-full` to also authenticate the server). +- Connect with a least-privilege database role: the store only needs DML on the + `bdk_wallet` schema (plus DDL when running migrations). +- Stored descriptors are sensitive (xpubs reveal the entire wallet history and + structure); protect database backups and access accordingly. + ## Testing 1. Install postgresql with `psql` tool. For example (macos): @@ -11,18 +25,20 @@ This crate is still **EXPERIMENTAL** do not use with mainnet wallets. brew update brew install postgresql ``` -2. Create empty test database: +2. Set DATABASE_TEST_URL to a postgres server the tests may use: ``` - psql postgres - postgres=# create database test_bdk_wallet; + export DATABASE_TEST_URL=postgresql://localhost/postgres ``` -3. Set DATABASE_URL to test database: - ``` - export DATABASE_TEST_URL=postgresql://localhost/test_bdk_wallet - ``` -4. Run tests, must use a single test thread since we reuse the postgres db: + The connected role must be allowed to `CREATE DATABASE`: every test creates + (and later cleans up) its own uniquely named `bdk_sqlx_test_*` database, so + tests never touch existing data and are safe to run in parallel. Do not + point this at a production server. + + Without `DATABASE_TEST_URL` the postgres-backend tests skip gracefully and + only the sqlite backend runs; set it for full coverage (CI always does). +3. Run tests: ``` - cargo test -- --test-threads=1 + cargo test ``` ## Example diff --git a/migrations/postgres/01_bdk_wallet.sql b/migrations/postgres/01_bdk_wallet.sql index 91242c3..4c0dd2a 100644 --- a/migrations/postgres/01_bdk_wallet.sql +++ b/migrations/postgres/01_bdk_wallet.sql @@ -69,7 +69,7 @@ CREATE TABLE IF NOT EXISTS bdk_wallet.anchor_tx ( anchor JSONB NOT NULL, txid TEXT NOT NULL, PRIMARY KEY (wallet_name, block_hash, txid), - FOREIGN KEY (wallet_name, block_hash) REFERENCES bdk_wallet.block(wallet_name, hash), - FOREIGN KEY (wallet_name, txid) REFERENCES bdk_wallet.tx(wallet_name, txid) + FOREIGN KEY (wallet_name, block_hash) REFERENCES bdk_wallet.block(wallet_name, hash) ON DELETE CASCADE, + FOREIGN KEY (wallet_name, txid) REFERENCES bdk_wallet.tx(wallet_name, txid) ON DELETE CASCADE ); CREATE INDEX IF NOT EXISTS idx_anchor_tx_txid ON bdk_wallet.anchor_tx (txid); \ No newline at end of file diff --git a/migrations/postgres/02_anchor_tx_on_delete_cascade.sql b/migrations/postgres/02_anchor_tx_on_delete_cascade.sql new file mode 100644 index 0000000..f753f7f --- /dev/null +++ b/migrations/postgres/02_anchor_tx_on_delete_cascade.sql @@ -0,0 +1,25 @@ +-- Databases created before the anchor_tx foreign keys included ON DELETE CASCADE +-- reject reorg-driven block deletion while anchor_tx rows still reference the +-- block, wedging all further persistence. Recreate such constraints in place. +-- No-op for databases created from the current 01 migration. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE n.nspname = 'bdk_wallet' AND t.relname = 'anchor_tx' + AND c.contype = 'f' AND c.confdeltype <> 'c' + ) THEN + ALTER TABLE "bdk_wallet"."anchor_tx" + DROP CONSTRAINT IF EXISTS anchor_tx_wallet_name_block_hash_fkey, + DROP CONSTRAINT IF EXISTS anchor_tx_wallet_name_txid_fkey; + ALTER TABLE "bdk_wallet"."anchor_tx" + ADD CONSTRAINT anchor_tx_wallet_name_block_hash_fkey + FOREIGN KEY (wallet_name, block_hash) + REFERENCES "bdk_wallet"."block"(wallet_name, hash) ON DELETE CASCADE, + ADD CONSTRAINT anchor_tx_wallet_name_txid_fkey + FOREIGN KEY (wallet_name, txid) + REFERENCES "bdk_wallet"."tx"(wallet_name, txid) ON DELETE CASCADE; + END IF; +END $$; diff --git a/migrations/postgres/03_block_unique_height.sql b/migrations/postgres/03_block_unique_height.sql new file mode 100644 index 0000000..d8ddc5b --- /dev/null +++ b/migrations/postgres/03_block_unique_height.sql @@ -0,0 +1,11 @@ +-- A wallet's chain has exactly one block hash per height, but the block table only +-- enforced uniqueness on (wallet_name, hash): a reorg that replaced the block at a +-- height left both rows behind and made loads nondeterministic. Remove duplicates +-- (survivor arbitrary among the duplicates; their anchors cascade away) and enforce +-- one row per height from here on. +DELETE FROM "bdk_wallet"."block" b +WHERE EXISTS ( + SELECT 1 FROM "bdk_wallet"."block" b2 + WHERE b2.wallet_name = b.wallet_name AND b2.height = b.height AND b2.ctid > b.ctid +); +CREATE UNIQUE INDEX IF NOT EXISTS idx_block_wallet_height ON "bdk_wallet"."block" (wallet_name, height); diff --git a/migrations/postgres/04_keychain_last_revealed_drop_default.sql b/migrations/postgres/04_keychain_last_revealed_drop_default.sql new file mode 100644 index 0000000..e4d97fc --- /dev/null +++ b/migrations/postgres/04_keychain_last_revealed_drop_default.sql @@ -0,0 +1,7 @@ +-- The keychain table declared `last_revealed INTEGER DEFAULT 0`, which made a +-- wallet persisted before its first address reveal reload as if index 0 had +-- been revealed, skipping it forever. The store now inserts NULL explicitly, +-- so the default is removed. Existing values are left untouched on purpose: +-- a stored 0 is ambiguous ("revealed index 0" vs "never revealed") and cannot +-- be rewritten without risking address reuse. +ALTER TABLE "bdk_wallet"."keychain" ALTER COLUMN last_revealed DROP DEFAULT; diff --git a/migrations/postgres/05_drop_version_table_and_block_height_index.sql b/migrations/postgres/05_drop_version_table_and_block_height_index.sql new file mode 100644 index 0000000..35165ad --- /dev/null +++ b/migrations/postgres/05_drop_version_table_and_block_height_index.sql @@ -0,0 +1,7 @@ +-- Schema hygiene. The version table is dead schema from the hand-rolled +-- versioning scheme that sqlx's migration bookkeeping replaced; migration 01 +-- created it only so databases from before the migrator could be adopted +-- unchanged. idx_block_height(height) was made redundant by migration 03's +-- unique (wallet_name, height) index, which serves the same lookups. +DROP TABLE IF EXISTS "bdk_wallet"."version"; +DROP INDEX IF EXISTS "bdk_wallet"."idx_block_height"; diff --git a/migrations/sqlite/02_anchor_tx_on_delete_cascade.sql b/migrations/sqlite/02_anchor_tx_on_delete_cascade.sql new file mode 100644 index 0000000..cff7386 --- /dev/null +++ b/migrations/sqlite/02_anchor_tx_on_delete_cascade.sql @@ -0,0 +1,18 @@ +-- Rebuild anchor_tx so its foreign keys cascade on block/tx deletion. +-- Without this, a reorg (which deletes the disconnected block row) is rejected +-- with a FK violation while anchor_tx rows still reference the block, wedging +-- all further persistence. SQLite cannot alter FK clauses in place, so the +-- table is rebuilt. +CREATE TABLE anchor_tx_new ( + wallet_name TEXT NOT NULL, + block_hash TEXT NOT NULL, + anchor BLOB NOT NULL, + txid TEXT NOT NULL, + PRIMARY KEY (wallet_name, block_hash, txid), + FOREIGN KEY (wallet_name, block_hash) REFERENCES block(wallet_name, hash) ON DELETE CASCADE, + FOREIGN KEY (wallet_name, txid) REFERENCES tx(wallet_name, txid) ON DELETE CASCADE +); +INSERT INTO anchor_tx_new SELECT wallet_name, block_hash, anchor, txid FROM anchor_tx; +DROP TABLE anchor_tx; +ALTER TABLE anchor_tx_new RENAME TO anchor_tx; +CREATE INDEX idx_anchor_tx_txid ON anchor_tx (txid); diff --git a/migrations/sqlite/03_block_unique_height.sql b/migrations/sqlite/03_block_unique_height.sql new file mode 100644 index 0000000..c0fd703 --- /dev/null +++ b/migrations/sqlite/03_block_unique_height.sql @@ -0,0 +1,9 @@ +-- A wallet's chain has exactly one block hash per height, but the block table only +-- enforced uniqueness on (wallet_name, hash): a reorg that replaced the block at a +-- height left both rows behind and made loads nondeterministic. Remove duplicates, +-- keeping the most recently inserted row (their anchors cascade away), and enforce +-- one row per height from here on. +DELETE FROM block WHERE rowid NOT IN ( + SELECT MAX(rowid) FROM block GROUP BY wallet_name, height +); +CREATE UNIQUE INDEX idx_block_wallet_height ON block (wallet_name, height); diff --git a/migrations/sqlite/04_keychain_last_revealed_drop_default.sql b/migrations/sqlite/04_keychain_last_revealed_drop_default.sql new file mode 100644 index 0000000..aa8d23b --- /dev/null +++ b/migrations/sqlite/04_keychain_last_revealed_drop_default.sql @@ -0,0 +1,17 @@ +-- The keychain table declared `last_revealed INTEGER DEFAULT 0`, which made a +-- wallet persisted before its first address reveal reload as if index 0 had +-- been revealed, skipping it forever. The store now inserts NULL explicitly. +-- SQLite cannot alter a column default in place, so the table is rebuilt (data +-- preserved; a stored 0 is ambiguous and deliberately left as-is). +CREATE TABLE keychain_new ( + wallet_name TEXT NOT NULL, + keychainkind TEXT NOT NULL, + descriptor TEXT NOT NULL, + descriptor_id BLOB NOT NULL, + last_revealed INTEGER, + PRIMARY KEY (wallet_name, keychainkind) +); +INSERT INTO keychain_new + SELECT wallet_name, keychainkind, descriptor, descriptor_id, last_revealed FROM keychain; +DROP TABLE keychain; +ALTER TABLE keychain_new RENAME TO keychain; diff --git a/migrations/sqlite/05_drop_version_table_and_block_height_index.sql b/migrations/sqlite/05_drop_version_table_and_block_height_index.sql new file mode 100644 index 0000000..983c00c --- /dev/null +++ b/migrations/sqlite/05_drop_version_table_and_block_height_index.sql @@ -0,0 +1,6 @@ +-- Schema hygiene. The version table is dead schema from before sqlx's +-- migration bookkeeping, and idx_block_height(height) was made redundant by +-- migration 03's unique (wallet_name, height) index, which serves the same +-- lookups. +DROP TABLE IF EXISTS version; +DROP INDEX IF EXISTS idx_block_height; diff --git a/src/lib.rs b/src/lib.rs index ae613ac..0b04bda 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,7 @@ //! bdk-sqlx #![warn(missing_docs)] +#![forbid(unsafe_code)] mod postgres; mod sqlite; @@ -10,10 +11,21 @@ mod test; use std::future::Future; use std::pin::Pin; +use std::str::FromStr; +use std::sync::OnceLock; use bdk_wallet::bitcoin; -use bdk_wallet::bitcoin::Network; +use bdk_wallet::bitcoin::{BlockHash, Network, Txid}; use bdk_wallet::chain::miniscript; +use bdk_wallet::chain::DescriptorExt; +use bdk_wallet::descriptor::{Descriptor, DescriptorPublicKey}; +use bdk_wallet::ChangeSet; +/// Re-export of the [`sqlx`] crate this library is built on. +/// +/// Consumers that construct pools themselves should import sqlx types through this +/// re-export so their sqlx version always matches the one this crate links against. +/// Note this couples the crate's public API to sqlx's major version: a sqlx major +/// bump is a breaking change for this crate as well. pub use sqlx; use sqlx::Pool; use sqlx::{Database, PgPool}; @@ -22,8 +34,40 @@ use sqlx::{Database, PgPool}; #[derive(Debug, thiserror::Error)] pub enum BdkSqlxError { /// bitcoin parse hex error - #[error("bitoin parse hex error: {0}")] + #[error("bitcoin parse hex error: {0}")] HexToArray(#[from] bitcoin::hex::HexToArrayError), + /// bitcoin consensus decode error + #[error("bitcoin consensus decode error: {0}")] + Consensus(#[from] bitcoin::consensus::encode::Error), + /// stored transaction bytes decode to a different txid than the stored txid + #[error("decoded transaction txid {computed} does not match stored txid {stored}")] + TxidMismatch { + /// txid stored alongside the transaction bytes + stored: Txid, + /// txid computed from the decoded transaction + computed: Txid, + }, + /// stored anchor references a different block hash than the stored block_hash + #[error("anchor block hash {computed} does not match stored block_hash {stored}")] + AnchorBlockHashMismatch { + /// block hash stored in the block_hash column + stored: BlockHash, + /// block hash contained in the anchor payload + computed: BlockHash, + }, + /// a changeset maps the same block hash to more than one height, which the + /// block table cannot represent (its key is the hash itself) + #[error( + "changeset maps block hash {hash} to both height {first_height} and height {second_height}" + )] + DuplicateBlockHash { + /// offending block hash + hash: BlockHash, + /// first height the hash was mapped to + first_height: u32, + /// second, conflicting height + second_height: u32, + }, /// miniscript error #[error("miniscript error: {0}")] Miniscript(#[from] miniscript::Error), @@ -52,14 +96,19 @@ pub enum BdkSqlxError { /// New network network: Network, }, - /// Init failure - #[error("Cant initialize network correctly with: {0}")] - NetworkInitFailure(Network), + /// a stored keychainkind is neither of the two kinds this store writes + /// ('External'/'Internal'); the row is corrupt and must not be silently + /// skipped, or a keychain would vanish from the loaded wallet + #[error("stored keychain kind '{got}' is not 'External' or 'Internal'")] + InvalidKeychainKind { + /// offending keychainkind value + got: String, + }, /// Config error #[error("Network Missing")] MissingNetwork, /// Config error - #[error("Cant initialize Postgres connection")] + #[error("No database connection pool provided to the builder")] MissingPool, /// Config error #[error("Network Failed to set")] @@ -67,6 +116,14 @@ pub enum BdkSqlxError { /// Config error #[error("Cant get network because its not set")] GetNetworkFailure, + /// integer value outside the range representable for its destination + #[error("integer value out of range for {context}: {value}")] + IntOutOfRange { + /// column or field the value belongs to + context: &'static str, + /// offending value + value: i128, + }, /// Query execution error #[error("Failed to execute query on {table}: {source}")] QueryError { @@ -78,12 +135,24 @@ pub enum BdkSqlxError { } /// Manages a pool of database connections. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct Store { pub(crate) pool: Pool, wallet_name: String, } +// Manual impl: deriving Clone would bound `DB: Clone`, which sqlx's `Postgres` +// and `Sqlite` marker types do not satisfy, making the derived impl unusable +// for the actual backends. Cloning a store shares the connection pool. +impl Clone for Store { + fn clone(&self) -> Self { + Self { + pool: self.pool.clone(), + wallet_name: self.wallet_name.clone(), + } + } +} + /// Build a new instance of the PgStoreBuilder pub struct PgStoreBuilder { wallet_name: String, @@ -92,4 +161,121 @@ pub struct PgStoreBuilder { network: Option, } +/// Build a new instance of the SqliteStoreBuilder +pub struct SqliteStoreBuilder { + wallet_name: String, + pool: Option>, + migrate: bool, + network: Option, +} + type FutureResult<'a, T, E> = Pin> + Send + 'a>>; + +/// Converts an integer crossing the database boundary, erroring instead of wrapping +/// when the value does not fit the destination type (e.g. a negative amount or height). +pub(crate) fn checked_conv(value: T, context: &'static str) -> Result +where + T: Copy + Into, + U: TryFrom, +{ + U::try_from(value).map_err(|_| BdkSqlxError::IntOutOfRange { + context, + value: value.into(), + }) +} + +/// Process-global network configuration, shared by every store in the process +/// regardless of backend. Persisting data for the wrong network wedges a +/// wallet store, so the first store built fixes the network for the whole +/// process and all stores validate against it. +static NETWORK: OnceLock = OnceLock::new(); + +/// Returns the process-global network, or an error if no store has set it yet. +pub(crate) fn get_network() -> Result { + NETWORK + .get() + .copied() + .ok_or(BdkSqlxError::GetNetworkFailure) +} + +/// Fixes the process-global network on first call. Later calls with the same +/// network are no-ops; a different network is rejected. +pub(crate) fn initialize_network(network: Network) -> Result<(), BdkSqlxError> { + if NETWORK.get().is_none() { + // A racing `set` is fine: the winner's value is validated below, so + // concurrent same-network builds can no longer fail spuriously. + let _ = NETWORK.set(network); + } + match NETWORK.get() { + Some(current) if *current == network => Ok(()), + Some(current) => Err(BdkSqlxError::DuplicateInitNetwork { + current: *current, + network, + }), + // Unreachable in practice: either our `set` won or a racer's did. + None => Err(BdkSqlxError::SetNetworkFailure(network)), + } +} + +/// Rejects `network` when this process was configured for a different one. +/// +/// Applied on the write path (a changeset's network) and the read path (the +/// stored network) of every backend. When the process-global network is not +/// set there is nothing to validate against and the check passes; stores +/// built through the public constructors always set it. +pub(crate) fn validate_network_matches_configured(network: Network) -> Result<(), BdkSqlxError> { + if let Ok(configured) = get_network() { + if configured != network { + return Err(BdkSqlxError::InvalidNetwork { + expected: configured.to_string(), + got: network.to_string(), + }); + } + } + Ok(()) +} + +/// Parses a network name read back from the database, rejecting both +/// unparseable names and networks this process was not configured for. +pub(crate) fn parse_and_validate_network(stored: &str) -> Result { + let network = Network::from_str(stored).map_err(|_| BdkSqlxError::InvalidNetwork { + expected: get_network() + .map(|n| n.to_string()) + .unwrap_or_else(|_| "a known network".to_string()), + got: stored.to_string(), + })?; + validate_network_matches_configured(network)?; + Ok(network) +} + +/// Applies one stored keychain row to `changeset`. +/// +/// Shared by both backends so the parse and validation rules cannot drift +/// apart. `keychainkind` comes from a free-text column; anything other than +/// the two kinds this store writes is corrupt data and must fail the load +/// loudly rather than silently drop a keychain. +pub(crate) fn keychain_changeset_from_parts( + changeset: &mut ChangeSet, + keychainkind: &str, + descriptor_str: &str, + last_revealed: Option, +) -> Result<(), BdkSqlxError> { + let descriptor: Descriptor = descriptor_str.parse()?; + let did = descriptor.descriptor_id(); + match keychainkind { + "External" => changeset.descriptor = Some(descriptor), + "Internal" => changeset.change_descriptor = Some(descriptor), + other => { + return Err(BdkSqlxError::InvalidKeychainKind { + got: other.to_string(), + }) + } + } + if let Some(last_rev) = last_revealed { + changeset + .indexer + .last_revealed + .insert(did, checked_conv(last_rev, "keychain.last_revealed")?); + } + Ok(()) +} diff --git a/src/postgres.rs b/src/postgres.rs index d4acec8..2382a65 100644 --- a/src/postgres.rs +++ b/src/postgres.rs @@ -3,71 +3,32 @@ #![warn(missing_docs)] // Standard library imports -use std::{ - str::FromStr, - sync::{Arc, OnceLock}, -}; +use std::{str::FromStr, sync::Arc}; // Third party crates use bdk_chain::{ local_chain, tx_graph, Anchor, ConfirmationBlockTime, DescriptorExt, DescriptorId, Merge, }; use bdk_wallet::{ bitcoin::{ - self, - consensus::{self, Decodable}, - hashes::Hash, - Amount, BlockHash, Network, OutPoint, ScriptBuf, TxOut, Txid, + self, consensus, hashes::Hash, Amount, BlockHash, Network, OutPoint, ScriptBuf, TxOut, Txid, }, chain as bdk_chain, - descriptor::{Descriptor, DescriptorPublicKey, ExtendedDescriptor}, + descriptor::ExtendedDescriptor, AsyncWalletPersister, ChangeSet, KeychainKind, KeychainKind::{External, Internal}, }; -use serde_json::json; use sqlx::{ - postgres::{PgPool, PgRow, Postgres}, - FromRow, Pool, Row, Transaction, + postgres::{PgPool, Postgres}, + sqlx_macros::migrate, + Pool, Row, Transaction, }; -use tracing::{info, trace, warn}; +use tracing::trace; // First party imports use super::{BdkSqlxError, FutureResult, PgStoreBuilder, Store}; type Result = core::result::Result; -/// Thread-safe storage for the network configuration that's shared across all Store instances. -/// This ensures consistent network validation across multiple threads. -static NETWORK: OnceLock = OnceLock::new(); - -/// Retrieves the current global network configuration for validation operations. -/// -/// Returns the current network configuration or an error if not initialized. -fn get_network() -> Result { - NETWORK - .get() - .copied() - .ok_or_else(|| BdkSqlxError::GetNetworkFailure) -} - -/// Sets the global network configuration to ensure consistent validation across threads. -/// -/// Returns an error if the network is already initialized with a different network. -fn initialize_network(network: Network) -> Result<()> { - match NETWORK.get() { - Some(current) if *current == network => { - warn!("initialize_network called more than once"); - Ok(()) - } - Some(current) => Err(BdkSqlxError::DuplicateInitNetwork { - current: *current, - network, - }), - None => NETWORK - .set(network) - .map_err(BdkSqlxError::SetNetworkFailure), - } -} - impl AsyncWalletPersister for Store { type Error = BdkSqlxError; @@ -147,6 +108,12 @@ impl PgStoreBuilder { /// /// The network is required to build a valid [`Store`]. If not provided, /// the build operation will fail with a MissingNetwork error. + /// + /// The network is process-global and shared across backends: the first + /// store built (postgres or sqlite) fixes it for the whole process, and + /// later builds with a different network fail with + /// [`BdkSqlxError::DuplicateInitNetwork`]. Every store validates stored + /// and incoming data against it. pub fn network(mut self, network: Network) -> Self { self.network = Some(network); self @@ -178,7 +145,7 @@ impl PgStoreBuilder { store.migrate().await?; } - initialize_network(network)?; + crate::initialize_network(network)?; Ok(store) } @@ -191,6 +158,14 @@ impl PgStoreBuilder { /// This is a convenience method that creates a connection pool from the URL /// and then builds the [`Store`] using that pool. /// + /// # Security + /// + /// Without an explicit `sslmode`, postgres connections default to `prefer`, which + /// silently falls back to plaintext if TLS negotiation fails. For any deployment + /// where the database is not on the same host, require TLS in the URL (e.g. + /// `?sslmode=require`, or `verify-full` to also authenticate the server), and + /// connect with a least-privilege database role. + /// /// # Errors /// /// Returns an error if: @@ -204,102 +179,16 @@ impl PgStoreBuilder { } impl Store { - /// Runs Migrations for a [`Store`] without an existing pg connection. + /// Runs the versioned migrations in `migrations/postgres` for this [`Store`]. + /// + /// Databases created by earlier releases (which created the schema without + /// migration bookkeeping) are adopted transparently: migration 01 only uses + /// `CREATE ... IF NOT EXISTS`, and migration 02 upgrades pre-existing + /// `anchor_tx` constraints in place. #[tracing::instrument(skip_all)] pub async fn migrate(&self) -> Result<()> { trace!("migrating bdk sqlx"); - - let mut tx = self.pool.begin().await?; - - // Create the schema first - let create_schema_query = r#"CREATE SCHEMA IF NOT EXISTS "bdk_wallet""#; - sqlx::query(create_schema_query) - .execute(&mut *tx) - .await - .map_err(|e| BdkSqlxError::QueryError { - table: "create schema bdk_wallet".to_string(), - source: e, - })?; - - // Create the tables one by one - let queries = [ - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."version" ( - version INTEGER PRIMARY KEY - )"#, - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."network" ( - wallet_name TEXT PRIMARY KEY, - name TEXT NOT NULL - )"#, - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."keychain" ( - wallet_name TEXT NOT NULL, - keychainkind TEXT NOT NULL, - descriptor TEXT NOT NULL, - descriptor_id BYTEA NOT NULL, - last_revealed INTEGER DEFAULT 0, - PRIMARY KEY (wallet_name, keychainkind) - )"#, - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."block" ( - wallet_name TEXT NOT NULL, - hash TEXT NOT NULL, - height INTEGER NOT NULL, - PRIMARY KEY (wallet_name, hash) - )"#, - r#"CREATE INDEX IF NOT EXISTS idx_block_height ON "bdk_wallet"."block" (height)"#, - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."tx" ( - wallet_name TEXT NOT NULL, - txid TEXT NOT NULL, - whole_tx BYTEA, - last_seen BIGINT, - PRIMARY KEY (wallet_name, txid) - )"#, - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."txout" ( - wallet_name TEXT NOT NULL, - txid TEXT NOT NULL, - vout INTEGER NOT NULL, - value BIGINT NOT NULL, - script BYTEA NOT NULL, - PRIMARY KEY (wallet_name, txid, vout) - )"#, - r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."anchor_tx" ( - wallet_name TEXT NOT NULL, - block_hash TEXT NOT NULL, - anchor JSONB NOT NULL, - txid TEXT NOT NULL, - PRIMARY KEY (wallet_name, block_hash, txid), - FOREIGN KEY (wallet_name, block_hash) REFERENCES "bdk_wallet"."block"(wallet_name, hash), - FOREIGN KEY (wallet_name, txid) REFERENCES "bdk_wallet"."tx"(wallet_name, txid) - )"#, - r#"CREATE INDEX IF NOT EXISTS idx_anchor_tx_txid ON "bdk_wallet"."anchor_tx" (txid)"#, - ]; - - // Execute each query separately - for query in &queries { - sqlx::query(query) - .execute(&mut *tx) - .await - .map_err(|e| BdkSqlxError::QueryError { - table: query.to_string(), - source: e, - })?; - } - - // At the end of migration, insert the current version - // After all tables are created but before tx.commit() - sqlx::query( - r#"INSERT INTO "bdk_wallet"."version" (version) - VALUES ($1) - ON CONFLICT (version) DO NOTHING"#, - ) - .bind(1) // Current schema version - .execute(&mut *tx) - .await - .map_err(|e| BdkSqlxError::QueryError { - table: "insert version".to_string(), - source: e, - })?; - - tx.commit().await?; - + migrate!("./migrations/postgres").run(&self.pool).await?; Ok(()) } } @@ -309,76 +198,70 @@ impl Store { pub(crate) async fn read(&self) -> Result { trace!("reading"); let mut db_tx = self.pool.begin().await?; + // READ COMMITTED (the default) snapshots per statement, so a concurrent + // writer committing between the SELECTs below could produce a + // mixed-generation changeset. REPEATABLE READ gives one snapshot for + // the whole read. + sqlx::query("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ") + .execute(&mut *db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "read".to_string(), + source: e, + })?; let mut changeset = ChangeSet::default(); - let sql = r#"SELECT n.name as network, - k_int.descriptor as internal_descriptor, k_int.last_revealed as internal_last_revealed, - k_ext.descriptor as external_descriptor, k_ext.last_revealed as external_last_revealed - FROM "bdk_wallet"."network" n - LEFT JOIN "bdk_wallet"."keychain" k_int ON n.wallet_name = k_int.wallet_name AND k_int.keychainkind = 'Internal' - LEFT JOIN "bdk_wallet"."keychain" k_ext ON n.wallet_name = k_ext.wallet_name AND k_ext.keychainkind = 'External' - WHERE n.wallet_name = $1"#; - - // Fetch wallet data - let row = sqlx::query(sql) + + // Fetch the network row. It is optional: rows persisted by a changeset + // that carried no network must still be visible. + let row = sqlx::query(r#"SELECT name FROM "bdk_wallet"."network" WHERE wallet_name = $1"#) .bind(&self.wallet_name) .fetch_optional(&mut *db_tx) .await .map_err(|e| BdkSqlxError::QueryError { - table: "read".to_string(), + table: "read network".to_string(), source: e, })?; - if let Some(row) = row { - Self::changeset_from_row(&mut db_tx, &mut changeset, row, &self.wallet_name).await?; + let network: String = row.get("name"); + changeset.network = Some(crate::parse_and_validate_network(&network)?); } - Ok(changeset) - } - - #[tracing::instrument(skip(db_tx, changeset, row))] - pub(crate) async fn changeset_from_row( - db_tx: &mut Transaction<'_, Postgres>, - changeset: &mut ChangeSet, - row: PgRow, - wallet_name: &str, - ) -> Result<()> { - trace!("changeset from row"); - - let network: String = row.get("network"); - let internal_last_revealed: Option = row.get("internal_last_revealed"); - let external_last_revealed: Option = row.get("external_last_revealed"); - let internal_desc_str: Option = row.get("internal_descriptor"); - let external_desc_str: Option = row.get("external_descriptor"); - - changeset.network = - Some( - Network::from_str(&network).map_err(|got| BdkSqlxError::InvalidNetwork { - expected: get_network().unwrap().to_string(), - got: got.to_string(), - })?, - ); - - if let Some(desc_str) = external_desc_str { - let descriptor: Descriptor = desc_str.parse()?; - let did = descriptor.descriptor_id(); - changeset.descriptor = Some(descriptor); - if let Some(last_rev) = external_last_revealed { - changeset.indexer.last_revealed.insert(did, last_rev as u32); - } + // Fetch keychain rows unconditionally: anchoring them on the network + // row (as the old join did) made descriptors persisted without a + // network vanish from every subsequent read while sitting in the + // database. + let rows = sqlx::query( + r#"SELECT keychainkind, descriptor, last_revealed FROM "bdk_wallet"."keychain" WHERE wallet_name = $1"#, + ) + .bind(&self.wallet_name) + .fetch_all(&mut *db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "read keychain".to_string(), + source: e, + })?; + for row in rows { + let keychainkind: String = row.get("keychainkind"); + let descriptor: String = row.get("descriptor"); + let last_revealed: Option = row.get("last_revealed"); + crate::keychain_changeset_from_parts( + &mut changeset, + &keychainkind, + &descriptor, + last_revealed, + )?; } - if let Some(desc_str) = internal_desc_str { - let descriptor: Descriptor = desc_str.parse()?; - let did = descriptor.descriptor_id(); - changeset.change_descriptor = Some(descriptor); - if let Some(last_rev) = internal_last_revealed { - changeset.indexer.last_revealed.insert(did, last_rev as u32); - } - } + changeset.tx_graph = + tx_graph_changeset_from_postgres(&mut db_tx, &self.wallet_name).await?; + changeset.local_chain = + local_chain_changeset_from_postgres(&mut db_tx, &self.wallet_name).await?; - changeset.tx_graph = tx_graph_changeset_from_postgres(db_tx, wallet_name).await?; - changeset.local_chain = local_chain_changeset_from_postgres(db_tx, wallet_name).await?; - Ok(()) + // The reads happened inside one transaction for a consistent snapshot; + // close it out explicitly instead of relying on drop-rollback. + db_tx.commit().await?; + + Ok(changeset) } #[tracing::instrument(skip_all)] @@ -400,6 +283,10 @@ impl Store { } if let Some(network) = changeset.network { + // Refuse to persist data for a different network than this process + // was configured for; overwriting the network row would wedge all + // subsequent reads with InvalidNetwork. + crate::validate_network_matches_configured(network)?; insert_network(&mut tx, wallet_name, network).await?; } @@ -421,7 +308,7 @@ impl Store { } /// Insert keychain descriptors. -#[tracing::instrument(skip(db_tx, descriptor))] +#[tracing::instrument(skip_all)] async fn insert_descriptor( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, @@ -437,8 +324,21 @@ async fn insert_descriptor( Internal => "Internal", }; + // last_revealed is inserted explicitly as NULL: "no address revealed yet" + // must be distinguishable from "address index 0 was revealed". The + // historical DEFAULT 0 conflated the two and made never-revealed wallets + // skip index 0 on reload. The conflict update keeps the stored + // last_revealed only when the descriptor itself is unchanged; a different + // descriptor under the same (wallet_name, keychainkind) must NOT inherit + // the old derivation index, or the replacement wallet would silently skip + // those addresses on load. sqlx::query( - r#"INSERT INTO "bdk_wallet"."keychain" (wallet_name, keychainkind, descriptor, descriptor_id) VALUES ($1, $2, $3, $4)"#, + r#"INSERT INTO "bdk_wallet"."keychain" (wallet_name, keychainkind, descriptor, descriptor_id, last_revealed) VALUES ($1, $2, $3, $4, NULL) + ON CONFLICT (wallet_name, keychainkind) DO UPDATE SET + descriptor = excluded.descriptor, + descriptor_id = excluded.descriptor_id, + last_revealed = CASE WHEN keychain.descriptor_id = excluded.descriptor_id + THEN keychain.last_revealed ELSE NULL END"#, ) .bind(wallet_name) .bind(keychain) @@ -455,28 +355,31 @@ async fn insert_descriptor( } /// Insert network. -#[tracing::instrument(skip(db_tx, network))] +#[tracing::instrument(skip_all)] async fn insert_network( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, network: Network, ) -> Result<()> { trace!("insert network"); - sqlx::query(r#"INSERT INTO "bdk_wallet"."network" (wallet_name, name) VALUES ($1, $2)"#) - .bind(wallet_name) - .bind(network.to_string()) - .execute(&mut **db_tx) - .await - .map_err(|e| BdkSqlxError::QueryError { - table: "insert network".to_string(), - source: e, - })?; + sqlx::query( + r#"INSERT INTO "bdk_wallet"."network" (wallet_name, name) VALUES ($1, $2) + ON CONFLICT (wallet_name) DO UPDATE SET name = excluded.name"#, + ) + .bind(wallet_name) + .bind(network.to_string()) + .execute(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert network".to_string(), + source: e, + })?; Ok(()) } /// Update keychain last revealed -#[tracing::instrument(skip(db_tx, descriptor_id, last_revealed))] +#[tracing::instrument(skip_all)] async fn update_last_revealed( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, @@ -485,10 +388,14 @@ async fn update_last_revealed( ) -> Result<()> { trace!("update last revealed"); - sqlx::query( - r#"UPDATE "bdk_wallet"."keychain" SET last_revealed = $1 WHERE wallet_name = $2 AND descriptor_id = $3"#, + // Derivation state must never move backwards: a stale or replayed + // changeset carrying a smaller index would silently re-reveal already + // handed-out addresses on the next load. + let result = sqlx::query( + r#"UPDATE "bdk_wallet"."keychain" SET last_revealed = CASE WHEN last_revealed IS NULL OR $1 > last_revealed THEN $1 ELSE last_revealed END + WHERE wallet_name = $2 AND descriptor_id = $3"#, ) - .bind(last_revealed as i32) + .bind(crate::checked_conv::<_, i32>(last_revealed, "keychain.last_revealed")?) .bind(wallet_name) .bind(descriptor_id.to_byte_array()) .execute(&mut **db_tx) @@ -498,12 +405,20 @@ async fn update_last_revealed( source: e, })?; + // Silently updating 0 rows would lose derivation state and cause address reuse. + if result.rows_affected() == 0 { + return Err(BdkSqlxError::QueryError { + table: "keychain".to_string(), + source: sqlx::Error::RowNotFound, + }); + } + Ok(()) } /// Select transactions, txouts, and anchors. #[tracing::instrument(skip(db_tx))] -pub async fn tx_graph_changeset_from_postgres( +pub(crate) async fn tx_graph_changeset_from_postgres( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, ) -> Result> { @@ -529,12 +444,20 @@ pub async fn tx_graph_changeset_from_postgres( let last_seen: Option = row.get("last_seen"); if let Some(tx_bytes) = whole_tx { - if let Ok(tx) = bitcoin::Transaction::consensus_decode(&mut tx_bytes.as_slice()) { - changeset.txs.insert(Arc::new(tx)); + let tx: bitcoin::Transaction = consensus::deserialize(&tx_bytes)?; + let computed = tx.compute_txid(); + if computed != txid { + return Err(BdkSqlxError::TxidMismatch { + stored: txid, + computed, + }); } + changeset.txs.insert(Arc::new(tx)); } if let Some(last_seen) = last_seen { - changeset.last_seen.insert(txid, last_seen as u64); + changeset + .last_seen + .insert(txid, crate::checked_conv(last_seen, "tx.last_seen")?); } } @@ -560,34 +483,43 @@ pub async fn tx_graph_changeset_from_postgres( changeset.txouts.insert( OutPoint { txid, - vout: vout as u32, + vout: crate::checked_conv(vout, "txout.vout")?, }, TxOut { - value: Amount::from_sat(value as u64), + value: Amount::from_sat(crate::checked_conv(value, "txout.value")?), script_pubkey: ScriptBuf::from(script), }, ); } // Fetch anchors - let rows = - sqlx::query(r#"SELECT anchor, txid FROM "bdk_wallet"."anchor_tx" WHERE wallet_name = $1"#) - .bind(wallet_name) - .fetch_all(&mut **db_tx) - .await - .map_err(|e| BdkSqlxError::QueryError { - table: "select anchor tx".to_string(), - source: e, - })?; + let rows = sqlx::query( + r#"SELECT anchor, txid, block_hash FROM "bdk_wallet"."anchor_tx" WHERE wallet_name = $1"#, + ) + .bind(wallet_name) + .fetch_all(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "select anchor tx".to_string(), + source: e, + })?; for row in rows { let anchor: serde_json::Value = row.get("anchor"); let txid: String = row.get("txid"); let txid = Txid::from_str(&txid)?; - - if let Ok(anchor) = serde_json::from_value::(anchor) { - changeset.anchors.insert((anchor, txid)); + let block_hash: String = row.get("block_hash"); + let block_hash = BlockHash::from_str(&block_hash)?; + + let anchor: ConfirmationBlockTime = serde_json::from_value(anchor)?; + let computed = anchor.anchor_block().hash; + if computed != block_hash { + return Err(BdkSqlxError::AnchorBlockHashMismatch { + stored: block_hash, + computed, + }); } + changeset.anchors.insert((anchor, txid)); } Ok(changeset) @@ -595,7 +527,7 @@ pub async fn tx_graph_changeset_from_postgres( /// Insert transactions, txouts, and anchors. #[tracing::instrument(skip(db_tx, changeset))] -pub async fn tx_graph_changeset_persist_to_postgres( +pub(crate) async fn tx_graph_changeset_persist_to_postgres( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, changeset: &tx_graph::ChangeSet, @@ -618,12 +550,22 @@ pub async fn tx_graph_changeset_persist_to_postgres( } for (&txid, &last_seen) in &changeset.last_seen { + // Upsert a stub row when the full tx is not stored yet; a bare UPDATE + // would affect 0 rows and silently drop the timestamp. whole_tx stays + // NULL until a changeset carrying the full tx fills it in. The + // conflict update never moves the timestamp backwards: bdk_chain's own + // Merge only ever increases last_seen, and a stale or replayed + // changeset must not regress the stored value (the same guarantee + // update_last_revealed enforces for derivation state). sqlx::query( - r#"UPDATE "bdk_wallet"."tx" SET last_seen = $1 WHERE wallet_name = $2 AND txid = $3"#, + r#"INSERT INTO "bdk_wallet"."tx" (wallet_name, txid, last_seen) VALUES ($1, $2, $3) + ON CONFLICT (wallet_name, txid) DO UPDATE SET + last_seen = CASE WHEN tx.last_seen IS NULL OR $3 > tx.last_seen + THEN $3 ELSE tx.last_seen END"#, ) - .bind(last_seen as i64) .bind(wallet_name) .bind(txid.to_string()) + .bind(crate::checked_conv::<_, i64>(last_seen, "tx.last_seen")?) .execute(&mut **db_tx) .await .map_err(|e| BdkSqlxError::QueryError { @@ -639,8 +581,8 @@ pub async fn tx_graph_changeset_persist_to_postgres( ) .bind(wallet_name) .bind(op.txid.to_string()) - .bind(op.vout as i32) - .bind(txo.value.to_sat() as i64) + .bind(crate::checked_conv::<_, i32>(op.vout, "txout.vout")?) + .bind(crate::checked_conv::<_, i64>(txo.value.to_sat(), "txout.value")?) .bind(txo.script_pubkey.as_bytes()) .execute(&mut **db_tx) .await @@ -674,7 +616,7 @@ pub async fn tx_graph_changeset_persist_to_postgres( /// Select blocks. #[tracing::instrument(skip(db_tx))] -pub async fn local_chain_changeset_from_postgres( +pub(crate) async fn local_chain_changeset_from_postgres( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, ) -> Result { @@ -695,7 +637,10 @@ pub async fn local_chain_changeset_from_postgres( let hash: String = row.get("hash"); let height: i32 = row.get("height"); let block_hash = BlockHash::from_str(&hash)?; - changeset.blocks.insert(height as u32, Some(block_hash)); + changeset.blocks.insert( + crate::checked_conv(height, "block.height")?, + Some(block_hash), + ); } Ok(changeset) @@ -703,22 +648,77 @@ pub async fn local_chain_changeset_from_postgres( /// Insert blocks. #[tracing::instrument(skip(db_tx, changeset))] -pub async fn local_chain_changeset_persist_to_postgres( +pub(crate) async fn local_chain_changeset_persist_to_postgres( db_tx: &mut Transaction<'_, Postgres>, wallet_name: &str, changeset: &local_chain::ChangeSet, ) -> Result<()> { trace!("local chain changeset to postgres"); + if changeset.blocks.is_empty() { + return Ok(()); + } + // The delete+insert below keeps exactly one row per (wallet_name, height) + // and must observe every committed row at that height. A concurrent writer + // doing the same for a different hash at the same height is invisible to + // the DELETE until it commits, and its INSERT then loses to the + // idx_block_wallet_height unique index (which the upsert's + // (wallet_name, hash) conflict target does not cover), aborting its whole + // changeset with a raw 23505. Serialize the block-table section per wallet + // instead: the loser waits for the winner to commit, then sees and + // replaces its row (last-writer-wins), exactly as if the writes had been + // issued sequentially. The lock is transaction-scoped, so commit or + // rollback releases it. sqlite needs no equivalent: its single-writer + // lock already serializes the same interleaving. + sqlx::query("SELECT pg_advisory_xact_lock(hashtext($1))") + .bind(format!("bdk_sqlx_local_chain:{wallet_name}")) + .execute(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "lock local_chain".to_string(), + source: e, + })?; + // The block table keys rows by (wallet_name, hash), so a changeset mapping + // one hash to several heights cannot be represented: persisting it would + // silently collapse to a single row and lose checkpoints. Reject it loudly + // instead. Real chains never produce such changesets. + let mut seen = std::collections::HashMap::new(); + for (&height, &hash) in &changeset.blocks { + if let Some(hash) = hash { + if let Some(&first_height) = seen.get(&hash) { + return Err(BdkSqlxError::DuplicateBlockHash { + hash, + first_height, + second_height: height, + }); + } + seen.insert(hash, height); + } + } for (&height, &hash) in &changeset.blocks { match hash { Some(hash) => { + // A reorg can replace the block at this height with a different hash; + // remove the stale row first (its anchors cascade away) so exactly one + // row per (wallet_name, height) remains. + sqlx::query( + r#"DELETE FROM "bdk_wallet"."block" WHERE wallet_name = $1 AND height = $2 AND hash != $3"#, + ) + .bind(wallet_name) + .bind(crate::checked_conv::<_, i32>(height, "block.height")?) + .bind(hash.to_string()) + .execute(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "delete stale block".to_string(), + source: e, + })?; sqlx::query( r#"INSERT INTO "bdk_wallet"."block" (wallet_name, hash, height) VALUES ($1, $2, $3) ON CONFLICT (wallet_name, hash) DO UPDATE SET height = $3"#, ) .bind(wallet_name) .bind(hash.to_string()) - .bind(height as i32) + .bind(crate::checked_conv::<_, i32>(height, "block.height")?) .execute(&mut **db_tx) .await .map_err(|e| BdkSqlxError::QueryError { @@ -731,7 +731,7 @@ pub async fn local_chain_changeset_persist_to_postgres( r#"DELETE FROM "bdk_wallet"."block" WHERE wallet_name = $1 AND height = $2"#, ) .bind(wallet_name) - .bind(height as i32) + .bind(crate::checked_conv::<_, i32>(height, "block.height")?) .execute(&mut **db_tx) .await .map_err(|e| BdkSqlxError::QueryError { @@ -744,30 +744,3 @@ pub async fn local_chain_changeset_persist_to_postgres( Ok(()) } - -/// Collects information on all the wallets in the database and dumps it to stdout. -#[tracing::instrument] -pub async fn easy_backup(db: Pool) -> Result<()> { - trace!("Starting easy backup"); - - let statement = r#"SELECT * FROM "bdk_wallet"."keychain""#; - - #[derive(serde::Serialize, FromRow)] - struct KeychainEntry { - wallet_name: String, - keychainkind: String, - descriptor: String, - descriptor_id: Vec, - last_revealed: i32, - } - - let results = sqlx::query_as::<_, KeychainEntry>(statement) - .fetch_all(&db) - .await?; - - let json_array = json!(results); - println!("{}", serde_json::to_string_pretty(&json_array)?); - - info!("Easy backup completed successfully"); - Ok(()) -} diff --git a/src/sqlite.rs b/src/sqlite.rs index 590054b..376f3e8 100644 --- a/src/sqlite.rs +++ b/src/sqlite.rs @@ -10,35 +10,30 @@ use bdk_chain::{ local_chain, tx_graph, Anchor, ConfirmationBlockTime, DescriptorExt, DescriptorId, Merge, }; use bdk_wallet::bitcoin::{ - self, - consensus::{self, Decodable}, - hashes::Hash, - Amount, BlockHash, Network, OutPoint, ScriptBuf, TxOut, Txid, + self, consensus, hashes::Hash, Amount, BlockHash, Network, OutPoint, ScriptBuf, TxOut, Txid, }; use bdk_wallet::chain as bdk_chain; -use bdk_wallet::descriptor::{Descriptor, DescriptorPublicKey, ExtendedDescriptor}; +use bdk_wallet::descriptor::ExtendedDescriptor; use bdk_wallet::KeychainKind::{External, Internal}; use bdk_wallet::{AsyncWalletPersister, ChangeSet, KeychainKind}; -use serde_json::json; -use sqlx::sqlite::SqliteRow; use sqlx::sqlite::{SqlitePool, SqlitePoolOptions}; use sqlx::sqlx_macros::migrate; -use sqlx::{sqlite::Sqlite, FromRow, Pool, Row, Transaction}; -use tracing::info; +use sqlx::{sqlite::Sqlite, Pool, Row, Transaction}; +use tracing::trace; impl AsyncWalletPersister for Store { type Error = BdkSqlxError; - #[tracing::instrument] + #[tracing::instrument(skip_all)] fn initialize<'a>(store: &'a mut Self) -> FutureResult<'a, ChangeSet, Self::Error> where Self: 'a, { - info!("initialize store"); + trace!("initialize store"); Box::pin(store.read()) } - #[tracing::instrument] + #[tracing::instrument(skip_all)] fn persist<'a>( store: &'a mut Self, changeset: &'a ChangeSet, @@ -46,25 +41,58 @@ impl AsyncWalletPersister for Store { where Self: 'a, { - info!("persist store"); + trace!("persist store"); Box::pin(store.write(changeset)) } } impl Store { /// Construct a new [`Store`] with an existing sqlite connection pool. - #[tracing::instrument] + /// + /// `network` fixes the process-global network (shared with the postgres + /// backend): the first store built in the process sets it, later stores + /// must use the same one, and stored or incoming data for a different + /// network is rejected with [`BdkSqlxError::InvalidNetwork`]. + /// + /// # Warning + /// + /// Do not pass a pool connected to `:memory:` with more than one connection: + /// each sqlite connection gets its *own* private in-memory database, so a + /// multi-connection pool silently reads and writes different databases (and + /// per-connection `PRAGMA`s only apply to the connection that ran them). + /// Use [`Store::new_with_url`] with `None` instead, which configures a + /// single-connection pool correctly. + /// + /// The pool must not disable `PRAGMA foreign_keys` (sqlx enables it by + /// default): reorg handling relies on `ON DELETE CASCADE`, and with + /// foreign keys off a disconnected block's anchor rows are silently left + /// behind and reload forever. + #[tracing::instrument(skip_all)] pub async fn new( pool: Pool, wallet_name: String, + network: Network, migrate: bool, ) -> Result { - info!("new sqlite store"); + trace!("new sqlite store"); + let store = Self { pool, wallet_name }; if migrate { - info!("migrate"); - migrate!("./migrations/sqlite").run(&pool).await?; + trace!("migrate"); + store.migrate().await?; } - Ok(Self { pool, wallet_name }) + crate::initialize_network(network)?; + Ok(store) + } + + /// Runs the versioned migrations in `migrations/sqlite` for this [`Store`]. + /// + /// Mirrors [`Store::::migrate`]: migrations are recorded in + /// sqlx's bookkeeping table, so re-running them is a no-op. + #[tracing::instrument(skip_all)] + pub async fn migrate(&self) -> Result<(), BdkSqlxError> { + trace!("migrating bdk sqlx"); + migrate!("./migrations/sqlite").run(&self.pool).await?; + Ok(()) } /// Construct a new [`Store`] without an existing sqlite connection pool. @@ -73,15 +101,134 @@ impl Store { /// /// If no URL is given a memory DB (non-persisted) will be used. A memory DB /// is useful for testing. - #[tracing::instrument] + /// + /// `network` has the same process-global semantics as [`Store::new`]. + #[tracing::instrument(skip_all)] pub async fn new_with_url( url: Option, wallet_name: String, + network: Network, migrate: bool, ) -> Result, BdkSqlxError> { - info!("new store with url"); + trace!("new store with url"); + crate::SqliteStoreBuilder::new(wallet_name) + .network(network) + .migrate(migrate) + .build_with_url(url.as_deref()) + .await + } +} + +impl crate::SqliteStoreBuilder { + /// Creates a new builder for a [`Store`] with the given wallet name. + /// + /// # Required fields + /// Before building, you must set: + /// - `network` - The Bitcoin network to use + /// - Either provide a connection pool with `pool()` or a database URL with `build_with_url()` + /// + /// # Example + /// ``` + /// # async fn example() -> Result<(), bdk_sqlx::BdkSqlxError> { + /// use bdk_wallet::bitcoin::Network; + /// use bdk_sqlx::SqliteStoreBuilder; + /// + /// let store = SqliteStoreBuilder::new("bdk_wallet_name".to_string()) + /// .network(Network::Testnet) + /// .migrate(true) + /// .build_with_url(Some("sqlite://bdk_wallet.sqlite?mode=rwc")) + /// .await?; + /// # Ok(()) + /// # } + /// ``` + #[tracing::instrument] + pub fn new(wallet_name: String) -> Self { + Self { + wallet_name, + pool: None, + migrate: false, + network: None, + } + } + + /// Sets the database connection pool for the [`Store`]. + /// + /// The pool is required to build a valid [`Store`]. If not provided, + /// the build operation will fail with a MissingPool error. + /// + /// # Warning + /// + /// Do not pass a pool connected to `:memory:` with more than one connection: + /// each sqlite connection gets its *own* private in-memory database, so a + /// multi-connection pool silently reads and writes different databases (and + /// per-connection `PRAGMA`s only apply to the connection that ran them). + /// Use [`SqliteStoreBuilder::build_with_url`] with `None` instead, which + /// configures a single-connection pool correctly. + pub fn pool(mut self, pool: Pool) -> Self { + self.pool = Some(pool); + self + } + + /// Sets whether database migrations should be run during [`Store`] initialization. + /// + /// When set to true, the necessary database schema and tables will be created + /// if they don't already exist. + pub fn migrate(mut self, migrate: bool) -> Self { + self.migrate = migrate; + self + } + + /// Sets the Bitcoin network for the [`Store`]. + /// + /// The network is required to build a valid [`Store`]. If not provided, + /// the build operation will fail with a MissingNetwork error. + /// + /// The network is process-global and shared across backends: the first + /// store built (postgres or sqlite) fixes it for the whole process, and + /// later builds with a different network fail with + /// [`BdkSqlxError::DuplicateInitNetwork`]. Every store validates stored + /// and incoming data against it. + pub fn network(mut self, network: Network) -> Self { + self.network = Some(network); + self + } + + /// Builds the [`Store`] with the configured options. + /// + /// This method creates a new [`Store`] instance using the options that have been + /// set on this builder. It requires both a network and a pool to be specified + /// before building. + /// + /// # Errors + /// + /// Returns an error if: + /// - No network has been specified (MissingNetwork) + /// - No pool has been specified (MissingPool) + /// - Migration fails + /// - Network initialization fails + pub async fn build(self) -> Result, BdkSqlxError> { + let network = self.network.ok_or(BdkSqlxError::MissingNetwork)?; + match self.pool { + Some(pool) => Store::new(pool, self.wallet_name, network, self.migrate).await, + None => Err(BdkSqlxError::MissingPool), + } + } + + /// Builds the [`Store`] with a new connection pool created from the provided URL. + /// + /// This is a convenience method that creates a connection pool from the URL + /// and then builds the [`Store`] using that pool. The SQLite DB URL should + /// look like "sqlite://bdk_wallet.sqlite?mode=rwc". If no URL is given, a + /// single-connection in-memory database (useful for testing) is created. + /// + /// # Errors + /// + /// Returns an error if: + /// - Database connection fails + /// - Any error that could occur in the build() method + pub async fn build_with_url(self, url: Option<&str>) -> Result, BdkSqlxError> { let pool = if let Some(url) = url { - SqlitePool::connect(url.as_str()).await? + SqlitePool::connect(url).await? } else { // must limit to one connection and no timeout if using memory DB SqlitePoolOptions::new() @@ -92,83 +239,72 @@ impl Store { .connect(":memory:") .await? }; - Self::new(pool, wallet_name, migrate).await + self.pool(pool).build().await } } impl Store { - #[tracing::instrument] + #[tracing::instrument(skip_all)] pub(crate) async fn read(&self) -> Result { - info!("migrate and read"); + trace!("read"); let mut tx = self.pool.begin().await?; let mut changeset = ChangeSet::default(); - let sql = - "SELECT n.name as network, - k_int.descriptor as internal_descriptor, k_int.last_revealed as internal_last_revealed, - k_ext.descriptor as external_descriptor, k_ext.last_revealed as external_last_revealed - FROM network n - LEFT JOIN keychain k_int ON n.wallet_name = k_int.wallet_name AND k_int.keychainkind = 'Internal' - LEFT JOIN keychain k_ext ON n.wallet_name = k_ext.wallet_name AND k_ext.keychainkind = 'External' - WHERE n.wallet_name = $1"; - - // Fetch wallet data - let row = sqlx::query(sql) + + // Fetch the network row. It is optional: rows persisted by a changeset + // that carried no network must still be visible. + let row = sqlx::query("SELECT name FROM network WHERE wallet_name = $1") .bind(&self.wallet_name) .fetch_optional(&mut *tx) - .await?; - - //dbg!(&row); - + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "read network".to_string(), + source: e, + })?; if let Some(row) = row { - Self::changeset_from_row(&mut tx, &mut changeset, row, &self.wallet_name).await?; + let network: String = row.get("name"); + changeset.network = Some(crate::parse_and_validate_network(&network)?); } - Ok(changeset) - } - - //#[tracing::instrument] - pub(crate) async fn changeset_from_row( - tx: &mut Transaction<'_, Sqlite>, - changeset: &mut ChangeSet, - row: SqliteRow, - wallet_name: &str, - ) -> Result<(), BdkSqlxError> { - info!("changeset from row"); - - let network: String = row.get("network"); - let internal_last_revealed: Option = row.get("internal_last_revealed"); - let external_last_revealed: Option = row.get("external_last_revealed"); - let internal_desc_str: Option = row.get("internal_descriptor"); - let external_desc_str: Option = row.get("external_descriptor"); - - changeset.network = Some(Network::from_str(&network).expect("parse Network")); - - if let Some(desc_str) = external_desc_str { - let descriptor: Descriptor = desc_str.parse()?; - let did = descriptor.descriptor_id(); - changeset.descriptor = Some(descriptor); - if let Some(last_rev) = external_last_revealed { - changeset.indexer.last_revealed.insert(did, last_rev as u32); - } + // Fetch keychain rows unconditionally: anchoring them on the network + // row (as the old join did) made descriptors persisted without a + // network vanish from every subsequent read while sitting in the + // database. + let rows = sqlx::query( + "SELECT keychainkind, descriptor, last_revealed FROM keychain WHERE wallet_name = $1", + ) + .bind(&self.wallet_name) + .fetch_all(&mut *tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "read keychain".to_string(), + source: e, + })?; + for row in rows { + let keychainkind: String = row.get("keychainkind"); + let descriptor: String = row.get("descriptor"); + let last_revealed: Option = row.get("last_revealed"); + crate::keychain_changeset_from_parts( + &mut changeset, + &keychainkind, + &descriptor, + last_revealed, + )?; } - if let Some(desc_str) = internal_desc_str { - let descriptor: Descriptor = desc_str.parse()?; - let did = descriptor.descriptor_id(); - changeset.change_descriptor = Some(descriptor); - if let Some(last_rev) = internal_last_revealed { - changeset.indexer.last_revealed.insert(did, last_rev as u32); - } - } + changeset.tx_graph = tx_graph_changeset_from_sqlite(&mut tx, &self.wallet_name).await?; + changeset.local_chain = + local_chain_changeset_from_sqlite(&mut tx, &self.wallet_name).await?; - changeset.tx_graph = tx_graph_changeset_from_sqlite(tx, wallet_name).await?; - changeset.local_chain = local_chain_changeset_from_sqlite(tx, wallet_name).await?; - Ok(()) + // The reads happened inside one transaction for a consistent snapshot; + // close it out explicitly instead of relying on drop-rollback. + tx.commit().await?; + + Ok(changeset) } - #[tracing::instrument] + #[tracing::instrument(skip_all)] pub(crate) async fn write(&self, changeset: &ChangeSet) -> Result<(), BdkSqlxError> { - info!("changeset write"); + trace!("changeset write"); if changeset.is_empty() { return Ok(()); } @@ -185,6 +321,11 @@ impl Store { } if let Some(network) = changeset.network { + // Refuse to persist data for a different network than this process + // was configured for (the same guard the postgres backend + // applies); overwriting the network row would wedge all subsequent + // reads with InvalidNetwork. + crate::validate_network_matches_configured(network)?; insert_network(&mut tx, wallet_name, network).await?; } @@ -206,14 +347,14 @@ impl Store { } /// Insert keychain descriptors. -#[tracing::instrument] +#[tracing::instrument(skip_all)] async fn insert_descriptor( tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, descriptor: &ExtendedDescriptor, keychain: KeychainKind, ) -> Result<(), BdkSqlxError> { - info!("insert descriptor"); + trace!("insert descriptor"); let descriptor_str = descriptor.to_string(); let descriptor_id = descriptor.descriptor_id().to_byte_array(); @@ -222,72 +363,119 @@ async fn insert_descriptor( Internal => "Internal", }; + // last_revealed is inserted explicitly as NULL: "no address revealed yet" + // must be distinguishable from "address index 0 was revealed". The + // historical DEFAULT 0 conflated the two and made never-revealed wallets + // skip index 0 on reload. The conflict update keeps the stored + // last_revealed only when the descriptor itself is unchanged; a different + // descriptor under the same (wallet_name, keychainkind) must NOT inherit + // the old derivation index, or the replacement wallet would silently skip + // those addresses on load. sqlx::query( - "INSERT INTO keychain (wallet_name, keychainkind, descriptor, descriptor_id) VALUES ($1, $2, $3, $4)", + "INSERT INTO keychain (wallet_name, keychainkind, descriptor, descriptor_id, last_revealed) VALUES ($1, $2, $3, $4, NULL) + ON CONFLICT (wallet_name, keychainkind) DO UPDATE SET + descriptor = excluded.descriptor, + descriptor_id = excluded.descriptor_id, + last_revealed = CASE WHEN keychain.descriptor_id = excluded.descriptor_id + THEN keychain.last_revealed ELSE NULL END", ) .bind(wallet_name) .bind(keychain) .bind(descriptor_str) .bind(descriptor_id.as_slice()) .execute(&mut **tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert keychain".to_string(), + source: e, + })?; Ok(()) } /// Insert network. -#[tracing::instrument] +#[tracing::instrument(skip_all)] async fn insert_network( tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, network: Network, ) -> Result<(), BdkSqlxError> { - info!("insert network"); - sqlx::query("INSERT INTO network (wallet_name, name) VALUES ($1, $2)") - .bind(wallet_name) - .bind(network.to_string()) - .execute(&mut **tx) - .await?; + trace!("insert network"); + sqlx::query( + "INSERT INTO network (wallet_name, name) VALUES ($1, $2) + ON CONFLICT (wallet_name) DO UPDATE SET name = excluded.name", + ) + .bind(wallet_name) + .bind(network.to_string()) + .execute(&mut **tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert network".to_string(), + source: e, + })?; Ok(()) } /// Update keychain last revealed -#[tracing::instrument] +#[tracing::instrument(skip_all)] async fn update_last_revealed( tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, descriptor_id: DescriptorId, last_revealed: u32, ) -> Result<(), BdkSqlxError> { - info!("update last revealed"); - - sqlx::query::( - "UPDATE keychain SET last_revealed = $1 WHERE wallet_name = $2 AND descriptor_id = $3", + trace!("update last revealed"); + + // Derivation state must never move backwards: a stale or replayed + // changeset carrying a smaller index would silently re-reveal already + // handed-out addresses on the next load. + let result = sqlx::query::( + "UPDATE keychain SET last_revealed = CASE WHEN last_revealed IS NULL OR $1 > last_revealed THEN $1 ELSE last_revealed END + WHERE wallet_name = $2 AND descriptor_id = $3", ) - .bind(last_revealed as i32) + .bind(crate::checked_conv::<_, i32>( + last_revealed, + "keychain.last_revealed", + )?) .bind(wallet_name) .bind(descriptor_id.to_byte_array().as_slice()) .execute(&mut **tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "update keychain".to_string(), + source: e, + })?; + + // Silently updating 0 rows would lose derivation state and cause address reuse. + if result.rows_affected() == 0 { + return Err(BdkSqlxError::QueryError { + table: "keychain".to_string(), + source: sqlx::Error::RowNotFound, + }); + } Ok(()) } /// Select transactions, txouts, and anchors. -#[tracing::instrument] -pub async fn tx_graph_changeset_from_sqlite( +#[tracing::instrument(skip_all)] +pub(crate) async fn tx_graph_changeset_from_sqlite( db_tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, ) -> Result, BdkSqlxError> { - info!("tx graph changeset from sqlite"); + trace!("tx graph changeset from sqlite"); let mut changeset = tx_graph::ChangeSet::default(); // Fetch transactions let rows = sqlx::query("SELECT txid, whole_tx, last_seen FROM tx WHERE wallet_name = $1") .bind(wallet_name) .fetch_all(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "select tx".to_string(), + source: e, + })?; for row in rows { let txid: String = row.get("txid"); @@ -296,12 +484,20 @@ pub async fn tx_graph_changeset_from_sqlite( let last_seen: Option = row.get("last_seen"); if let Some(tx_bytes) = whole_tx { - if let Ok(tx) = bitcoin::Transaction::consensus_decode(&mut tx_bytes.as_slice()) { - changeset.txs.insert(Arc::new(tx)); + let tx: bitcoin::Transaction = consensus::deserialize(&tx_bytes)?; + let computed = tx.compute_txid(); + if computed != txid { + return Err(BdkSqlxError::TxidMismatch { + stored: txid, + computed, + }); } + changeset.txs.insert(Arc::new(tx)); } if let Some(last_seen) = last_seen { - changeset.last_seen.insert(txid, last_seen as u64); + changeset + .last_seen + .insert(txid, crate::checked_conv(last_seen, "tx.last_seen")?); } } @@ -309,7 +505,11 @@ pub async fn tx_graph_changeset_from_sqlite( let rows = sqlx::query("SELECT txid, vout, value, script FROM txout WHERE wallet_name = $1") .bind(wallet_name) .fetch_all(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "select txout".to_string(), + source: e, + })?; for row in rows { let txid: String = row.get("txid"); @@ -321,43 +521,56 @@ pub async fn tx_graph_changeset_from_sqlite( changeset.txouts.insert( OutPoint { txid, - vout: vout as u32, + vout: crate::checked_conv(vout, "txout.vout")?, }, TxOut { - value: Amount::from_sat(value as u64), + value: Amount::from_sat(crate::checked_conv(value, "txout.value")?), script_pubkey: ScriptBuf::from(script), }, ); } // Fetch anchors - let rows = - sqlx::query("SELECT json(anchor) as anchor, txid FROM anchor_tx WHERE wallet_name = $1") - .bind(wallet_name) - .fetch_all(&mut **db_tx) - .await?; + let rows = sqlx::query( + "SELECT json(anchor) as anchor, txid, block_hash FROM anchor_tx WHERE wallet_name = $1", + ) + .bind(wallet_name) + .fetch_all(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "select anchor tx".to_string(), + source: e, + })?; for row in rows { let anchor: serde_json::Value = row.get("anchor"); let txid: String = row.get("txid"); let txid = Txid::from_str(&txid)?; - - if let Ok(anchor) = serde_json::from_value::(anchor) { - changeset.anchors.insert((anchor, txid)); + let block_hash: String = row.get("block_hash"); + let block_hash = BlockHash::from_str(&block_hash)?; + + let anchor: ConfirmationBlockTime = serde_json::from_value(anchor)?; + let computed = anchor.anchor_block().hash; + if computed != block_hash { + return Err(BdkSqlxError::AnchorBlockHashMismatch { + stored: block_hash, + computed, + }); } + changeset.anchors.insert((anchor, txid)); } Ok(changeset) } /// Insert transactions, txouts, and anchors. -#[tracing::instrument] -pub async fn tx_graph_changeset_persist_to_sqlite( +#[tracing::instrument(skip_all)] +pub(crate) async fn tx_graph_changeset_persist_to_sqlite( db_tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, changeset: &tx_graph::ChangeSet, ) -> Result<(), BdkSqlxError> { - info!("tx graph changeset from sqlite"); + trace!("tx graph changeset from sqlite"); for tx in &changeset.txs { sqlx::query( "INSERT INTO tx (wallet_name, txid, whole_tx) VALUES ($1, $2, $3) @@ -367,16 +580,36 @@ pub async fn tx_graph_changeset_persist_to_sqlite( .bind(tx.compute_txid().to_string()) .bind(consensus::serialize(tx.as_ref())) .execute(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert tx".to_string(), + source: e, + })?; } for (&txid, &last_seen) in &changeset.last_seen { - sqlx::query("UPDATE tx SET last_seen = $1 WHERE wallet_name = $2 AND txid = $3") - .bind(last_seen as i64) - .bind(wallet_name) - .bind(txid.to_string()) - .execute(&mut **db_tx) - .await?; + // Upsert a stub row when the full tx is not stored yet; a bare UPDATE + // would affect 0 rows and silently drop the timestamp. whole_tx stays + // NULL until a changeset carrying the full tx fills it in. The + // conflict update never moves the timestamp backwards: bdk_chain's own + // Merge only ever increases last_seen, and a stale or replayed + // changeset must not regress the stored value (the same guarantee + // update_last_revealed enforces for derivation state). + sqlx::query( + "INSERT INTO tx (wallet_name, txid, last_seen) VALUES ($1, $2, $3) + ON CONFLICT (wallet_name, txid) DO UPDATE SET + last_seen = CASE WHEN tx.last_seen IS NULL OR $3 > tx.last_seen + THEN $3 ELSE tx.last_seen END", + ) + .bind(wallet_name) + .bind(txid.to_string()) + .bind(crate::checked_conv::<_, i64>(last_seen, "tx.last_seen")?) + .execute(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "update tx".to_string(), + source: e, + })?; } for (op, txo) in &changeset.txouts { @@ -386,11 +619,18 @@ pub async fn tx_graph_changeset_persist_to_sqlite( ) .bind(wallet_name) .bind(op.txid.to_string()) - .bind(op.vout as i32) - .bind(txo.value.to_sat() as i64) + .bind(crate::checked_conv::<_, i32>(op.vout, "txout.vout")?) + .bind(crate::checked_conv::<_, i64>( + txo.value.to_sat(), + "txout.value", + )?) .bind(txo.script_pubkey.as_bytes()) .execute(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert txout".to_string(), + source: e, + })?; } for (anchor, txid) in &changeset.anchors { @@ -405,94 +645,126 @@ pub async fn tx_graph_changeset_persist_to_sqlite( .bind(anchor) .bind(txid.to_string()) .execute(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert anchor tx".to_string(), + source: e, + })?; } Ok(()) } /// Select blocks. -#[tracing::instrument] -pub async fn local_chain_changeset_from_sqlite( +#[tracing::instrument(skip_all)] +pub(crate) async fn local_chain_changeset_from_sqlite( db_tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, ) -> Result { - info!("local chain changeset from sqlite"); + trace!("local chain changeset from sqlite"); let mut changeset = local_chain::ChangeSet::default(); let rows = sqlx::query("SELECT hash, height FROM block WHERE wallet_name = $1") .bind(wallet_name) .fetch_all(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "select block".to_string(), + source: e, + })?; for row in rows { let hash: String = row.get("hash"); let height: i32 = row.get("height"); let block_hash = BlockHash::from_str(&hash)?; - changeset.blocks.insert(height as u32, Some(block_hash)); + changeset.blocks.insert( + crate::checked_conv(height, "block.height")?, + Some(block_hash), + ); } Ok(changeset) } /// Insert blocks. -#[tracing::instrument] -pub async fn local_chain_changeset_persist_to_sqlite( +#[tracing::instrument(skip_all)] +pub(crate) async fn local_chain_changeset_persist_to_sqlite( db_tx: &mut Transaction<'_, Sqlite>, wallet_name: &str, changeset: &local_chain::ChangeSet, ) -> Result<(), BdkSqlxError> { - info!("local chain changeset to sqlite"); + trace!("local chain changeset to sqlite"); + if changeset.blocks.is_empty() { + return Ok(()); + } + // The block table keys rows by (wallet_name, hash), so a changeset mapping + // one hash to several heights cannot be represented: persisting it would + // silently collapse to a single row and lose checkpoints. Reject it loudly + // instead. Real chains never produce such changesets. + let mut seen = std::collections::HashMap::new(); + for (&height, &hash) in &changeset.blocks { + if let Some(hash) = hash { + if let Some(&first_height) = seen.get(&hash) { + return Err(BdkSqlxError::DuplicateBlockHash { + hash, + first_height, + second_height: height, + }); + } + seen.insert(hash, height); + } + } + // Concurrent writers persisting different hashes at the same height need + // no explicit serialization here (unlike postgres, which takes an advisory + // lock): sqlite admits only one writer at a time, so the second writer's + // DELETE blocks on the database write lock until the first commits, then + // sees and replaces its row -- last-writer-wins, as if the writes had been + // issued sequentially. for (&height, &hash) in &changeset.blocks { match hash { Some(hash) => { + // A reorg can replace the block at this height with a different hash; + // remove the stale row first (its anchors cascade away) so exactly one + // row per (wallet_name, height) remains. + sqlx::query( + "DELETE FROM block WHERE wallet_name = $1 AND height = $2 AND hash != $3", + ) + .bind(wallet_name) + .bind(crate::checked_conv::<_, i32>(height, "block.height")?) + .bind(hash.to_string()) + .execute(&mut **db_tx) + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "delete stale block".to_string(), + source: e, + })?; sqlx::query( "INSERT INTO block (wallet_name, hash, height) VALUES ($1, $2, $3) ON CONFLICT (wallet_name, hash) DO UPDATE SET height = $3", ) .bind(wallet_name) .bind(hash.to_string()) - .bind(height as i32) + .bind(crate::checked_conv::<_, i32>(height, "block.height")?) .execute(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "insert block".to_string(), + source: e, + })?; } None => { sqlx::query("DELETE FROM block WHERE wallet_name = $1 AND height = $2") .bind(wallet_name) - .bind(height as i32) + .bind(crate::checked_conv::<_, i32>(height, "block.height")?) .execute(&mut **db_tx) - .await?; + .await + .map_err(|e| BdkSqlxError::QueryError { + table: "delete block".to_string(), + source: e, + })?; } } } Ok(()) } - -/// Collects information on all the wallets in the database and dumps it to stdout. -#[tracing::instrument] -pub async fn easy_backup(db: Pool) -> Result<(), BdkSqlxError> { - info!("Starting easy backup"); - - let statement = "SELECT * FROM keychain"; - - let results = sqlx::query_as::<_, KeychainEntry>(statement) - .fetch_all(&db) - .await?; - - let json_array = json!(results); - println!("{}", serde_json::to_string_pretty(&json_array)?); - - info!("Easy backup completed successfully"); - Ok(()) -} - -/// Represents a row in the keychain table. -#[derive(serde::Serialize, FromRow)] -struct KeychainEntry { - wallet_name: String, - keychainkind: String, - descriptor: String, - descriptor_id: Vec, - last_revealed: i32, -} diff --git a/src/test.rs b/src/test.rs index 9a4b7fd..3c2bd88 100644 --- a/src/test.rs +++ b/src/test.rs @@ -1,10 +1,11 @@ use std::env; use std::ops::Add; use std::str::FromStr; -use std::sync::Once; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex, Once, OnceLock}; use assert_matches::assert_matches; -use bdk_chain::{BlockId, ConfirmationBlockTime}; +use bdk_chain::{BlockId, ConfirmationBlockTime, DescriptorExt, DescriptorId, Merge}; use bdk_wallet::{ bitcoin, chain as bdk_chain, descriptor::ExtendedDescriptor, @@ -19,19 +20,20 @@ use bitcoin::{ secp256k1::Secp256k1, Address, Amount, BlockHash, Network::{self, Regtest}, - OutPoint, Transaction, TxIn, TxOut, Txid, + OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid, }; -use sqlx::{Pool, Postgres, Sqlite, SqlitePool}; +use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; +use sqlx::{Pool, Postgres, Sqlite}; use test_utils::{ get_test_tr_single_sig_xprv_and_change_desc, get_test_wpkh, insert_anchor, insert_checkpoint, insert_tx, new_tx, }; use tracing::info; -use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter}; +use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, Layer}; -use crate::{BdkSqlxError, FutureResult, PgStoreBuilder, Store}; +use crate::{BdkSqlxError, FutureResult, PgStoreBuilder, SqliteStoreBuilder, Store}; -pub fn get_test_minisicript_with_change_desc() -> (&'static str, &'static str) { +fn get_test_miniscript_with_change_desc() -> (&'static str, &'static str) { ("wsh(andor(multi(2,[a0d3c79c/48'/1'/79'/2']tpubDEsGdqFaKUVnVNZZw8AixJ8C3yD8o6nN7hsdLfbtVRDTk3PNrQ2pcWNWNbxhdcNSgQP25pUpgRQ7qiVtN3YvSzACKizrvzSwH9SQ2Bjbbwt/0/*,[ea2484f9/48'/1'/79'/2']tpubDFjkswBXoRHKkvmHsxv4xdDqbjg1peX9zJytLeSLbXuwVgYhXgbABzC2r5MAWxqWoaUr7hWGW5TPjA9sNvxa3mX6DrNBdynDsEvwDoXGFpm/0/*,[93f245d7/48'/1'/79'/2']tpubDEVnR72gRgTsqaPFMacV6fCfaSEe56gcDomuGhk9MFeUdEi18riJCokgsZr2x1KKGRM59TJ4AQ6FuNun3khh95ceoH2ytN13nVD7yDLP5LJ/0/*),or_i(and_v(v:pkh([61cdf766/48'/1'/79'/2']tpubDEXETCw2WurhazfW5gW1z4njP6yLXDQmCGfjWGP5k3BuTQ5iZqovMr1zz1zWPhDMRn11hXGpZHodus1LysXnwREsD1ig96M24JhQCpPPpf6/0/*),after(1753228800)),thresh(2,pk([39bf48a9/48'/1'/0'/2']tpubDEr9rVFQbT1keErwxb6GuGy3RM6TEACSkFxBgziUvrDprYuM1Wm7wi6jb1gcaLrSgk6MSkGx84dS2kQQwJKxGRJ59rAvmuKTU7E3saHJLf5/0/*),s:pk([9467fdb3/48'/1'/0'/2']tpubDFEjX5BY88AbWpshPwGscwgKLtcCjeVodMbmhS6D6cbz1eGNUs3546ephbVmbHpxEhbCDrezGmFBArLxBKzPEfBcBdzQuncPm8ww2xa6UUQ/0/*),s:pk([01adf45e/48'/1'/0'/2']tpubDFPYZPeShApyWndvDUtpLSjDHGYK4tTT4BkMyTukGqbP9AXQeQhiWsbwEzyZhxgud9ZPew1FPsoLbWjfnE3veSXLeU4ViofrhVAHNXtjQWE/0/*),snl:after(1739836800))),and_v(v:thresh(2,pkh([39bf48a9/48'/1'/0'/2']tpubDEr9rVFQbT1keErwxb6GuGy3RM6TEACSkFxBgziUvrDprYuM1Wm7wi6jb1gcaLrSgk6MSkGx84dS2kQQwJKxGRJ59rAvmuKTU7E3saHJLf5/2/*),a:pkh([9467fdb3/48'/1'/0'/2']tpubDFEjX5BY88AbWpshPwGscwgKLtcCjeVodMbmhS6D6cbz1eGNUs3546ephbVmbHpxEhbCDrezGmFBArLxBKzPEfBcBdzQuncPm8ww2xa6UUQ/2/*),a:pkh([01adf45e/48'/1'/0'/2']tpubDFPYZPeShApyWndvDUtpLSjDHGYK4tTT4BkMyTukGqbP9AXQeQhiWsbwEzyZhxgud9ZPew1FPsoLbWjfnE3veSXLeU4ViofrhVAHNXtjQWE/2/*)),after(1757116800))))", "wsh(andor(multi(2,[a0d3c79c/48'/1'/79'/2']tpubDEsGdqFaKUVnVNZZw8AixJ8C3yD8o6nN7hsdLfbtVRDTk3PNrQ2pcWNWNbxhdcNSgQP25pUpgRQ7qiVtN3YvSzACKizrvzSwH9SQ2Bjbbwt/1/*,[ea2484f9/48'/1'/79'/2']tpubDFjkswBXoRHKkvmHsxv4xdDqbjg1peX9zJytLeSLbXuwVgYhXgbABzC2r5MAWxqWoaUr7hWGW5TPjA9sNvxa3mX6DrNBdynDsEvwDoXGFpm/1/*,[93f245d7/48'/1'/79'/2']tpubDEVnR72gRgTsqaPFMacV6fCfaSEe56gcDomuGhk9MFeUdEi18riJCokgsZr2x1KKGRM59TJ4AQ6FuNun3khh95ceoH2ytN13nVD7yDLP5LJ/1/*),or_i(and_v(v:pkh([61cdf766/48'/1'/79'/2']tpubDEXETCw2WurhazfW5gW1z4njP6yLXDQmCGfjWGP5k3BuTQ5iZqovMr1zz1zWPhDMRn11hXGpZHodus1LysXnwREsD1ig96M24JhQCpPPpf6/1/*),after(1753228800)),thresh(2,pk([39bf48a9/48'/1'/0'/2']tpubDEr9rVFQbT1keErwxb6GuGy3RM6TEACSkFxBgziUvrDprYuM1Wm7wi6jb1gcaLrSgk6MSkGx84dS2kQQwJKxGRJ59rAvmuKTU7E3saHJLf5/1/*),s:pk([9467fdb3/48'/1'/0'/2']tpubDFEjX5BY88AbWpshPwGscwgKLtcCjeVodMbmhS6D6cbz1eGNUs3546ephbVmbHpxEhbCDrezGmFBArLxBKzPEfBcBdzQuncPm8ww2xa6UUQ/1/*),s:pk([01adf45e/48'/1'/0'/2']tpubDFPYZPeShApyWndvDUtpLSjDHGYK4tTT4BkMyTukGqbP9AXQeQhiWsbwEzyZhxgud9ZPew1FPsoLbWjfnE3veSXLeU4ViofrhVAHNXtjQWE/1/*),snl:after(1739836800))),and_v(v:thresh(2,pkh([39bf48a9/48'/1'/0'/2']tpubDEr9rVFQbT1keErwxb6GuGy3RM6TEACSkFxBgziUvrDprYuM1Wm7wi6jb1gcaLrSgk6MSkGx84dS2kQQwJKxGRJ59rAvmuKTU7E3saHJLf5/3/*),a:pkh([9467fdb3/48'/1'/0'/2']tpubDFEjX5BY88AbWpshPwGscwgKLtcCjeVodMbmhS6D6cbz1eGNUs3546ephbVmbHpxEhbCDrezGmFBArLxBKzPEfBcBdzQuncPm8ww2xa6UUQ/3/*),a:pkh([01adf45e/48'/1'/0'/2']tpubDFPYZPeShApyWndvDUtpLSjDHGYK4tTT4BkMyTukGqbP9AXQeQhiWsbwEzyZhxgud9ZPew1FPsoLbWjfnE3veSXLeU4ViofrhVAHNXtjQWE/3/*)),after(1757116800))))") } @@ -46,58 +48,148 @@ fn parse_descriptor(s: &str) -> ExtendedDescriptor { static INIT: Once = Once::new(); +type SharedLogBuffer = Arc>>; +static LOG_BUFFER: OnceLock = OnceLock::new(); + +/// Buffer that captures every trace event emitted in this test process. +fn log_buffer() -> SharedLogBuffer { + LOG_BUFFER.get().expect("initialize() first").clone() +} + // This must only be called once. fn initialize() { INIT.call_once(|| { + let buf: SharedLogBuffer = Arc::new(Mutex::new(Vec::new())); + let writer_buf = buf.clone(); + LOG_BUFFER.set(buf).expect("log buffer set once"); tracing_subscriber::registry() - .with(EnvFilter::new( + .with(tracing_subscriber::fmt::layer().with_filter(EnvFilter::new( env::var("RUST_LOG").unwrap_or_else(|_| "sqlx=warn,bdk_sqlx=warn".into()), - )) - .with(tracing_subscriber::fmt::layer()) + ))) + // A permanent global capture layer for the descriptor-leak test. A + // scoped subscriber (`with_subscriber`) must NOT be used here: sqlx's + // sqlite worker threads hold span references, and tearing the scoped + // registry down while those threads are alive panics the worker and + // wedges the pool. Each layer carries its own filter: a bare EnvFilter + // layer would disable trace events globally for every layer. + .with( + tracing_subscriber::fmt::layer() + .with_writer(move || SharedWriter(writer_buf.clone())) + .with_filter(EnvFilter::new("trace")), + ) .try_init() .expect("setup tracing"); }); } -trait DropAll { - async fn drop_all(&self) -> anyhow::Result<()>; -} - -impl DropAll for Pool { - /// Drops all tables. - /// - /// Clean up (optional, depending on your test database strategy) - /// You might want to delete the test wallet from the database here. - #[tracing::instrument] - async fn drop_all(&self) -> anyhow::Result<()> { - let drop_statements = vec![ - "DROP TABLE IF EXISTS _sqlx_migrations", - "DROP TABLE IF EXISTS vault_addresses", - "DROP TABLE IF EXISTS used_anchorwatch_keys", - "DROP TABLE IF EXISTS anchorwatch_keys", - "DROP TABLE IF EXISTS psbts", - "DROP TABLE IF EXISTS whitelist_update", - "DROP TABLE IF EXISTS vault_parameters", - "DROP TABLE IF EXISTS users", - "DROP TABLE IF EXISTS version", - "DROP TABLE IF EXISTS anchor_tx", - "DROP TABLE IF EXISTS txout", - "DROP TABLE IF EXISTS tx", - "DROP TABLE IF EXISTS block", - "DROP TABLE IF EXISTS keychain", - "DROP TABLE IF EXISTS network", - ]; - - let mut tx = self.begin().await?; - - for statement in drop_statements { - sqlx::query(statement).execute(&mut *tx).await?; +static TEST_DB_COUNTER: AtomicU64 = AtomicU64::new(0); +static TEST_DB_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + +/// Name hashed into the postgres advisory lock that serializes test-database +/// management. `TEST_DB_LOCK` cannot exclude OTHER test processes sharing the +/// server, so creation/cleanup additionally runs under a session-scoped +/// advisory lock: only one process manages databases at a time, and a stale +/// database can never be dropped between its creation and first connection. +/// If a lock holder crashes, its session ends and postgres releases the lock. +const PG_MGMT_ADVISORY_NAME: &str = "bdk_sqlx_test_db_mgmt"; + +/// Takes the cross-process database-management lock on a dedicated session. +/// Must be paired with [`pg_mgmt_unlock`]; callers hold `TEST_DB_LOCK` first, +/// so contention can only come from other processes and no deadlock cycle +/// exists. +async fn pg_mgmt_lock( + admin_pool: &Pool, +) -> anyhow::Result> { + let mut conn = admin_pool.acquire().await?; + sqlx::query("SELECT pg_advisory_lock(hashtext($1))") + .bind(PG_MGMT_ADVISORY_NAME) + .execute(&mut *conn) + .await?; + Ok(conn) +} + +/// Releases the cross-process database-management lock. Best-effort: if the +/// session is already gone, postgres has released the lock anyway. +async fn pg_mgmt_unlock(mut conn: sqlx::pool::PoolConnection) { + let _ = sqlx::query("SELECT pg_advisory_unlock(hashtext($1))") + .bind(PG_MGMT_ADVISORY_NAME) + .execute(&mut *conn) + .await; +} + +/// Returns the `DATABASE_TEST_URL` postgres server the tests may use, or `None` +/// when it is not set. Postgres-backend tests skip gracefully in that case (a +/// contributor running bare `cargo test` without a server still exercises the +/// sqlite backend); CI always sets it, so full coverage runs there. +fn pg_test_url() -> Option { + static NOTICE: Once = Once::new(); + let url = env::var("DATABASE_TEST_URL").ok(); + if url.is_none() { + NOTICE.call_once(|| { + eprintln!( + "DATABASE_TEST_URL not set: skipping postgres-backend tests \ + (sqlite backend still runs)" + ); + }); + } + url +} + +/// Creates a uniquely named database on the postgres server at `DATABASE_TEST_URL` and +/// returns a pool connected to it, so every test gets an isolated database and no +/// pre-existing tables are ever dropped. Returns `None` when `DATABASE_TEST_URL` +/// is not set; callers skip the postgres half of their scenario in that case. +/// +/// Databases left behind by previous test runs are removed opportunistically; a database +/// is never dropped while any session is connected to it, and creation/cleanup are +/// serialized (in-process by `TEST_DB_LOCK`, cross-process by the advisory lock) so a +/// parallel test cannot drop a database between its creation and first connection. +async fn create_test_pg_pool() -> anyhow::Result>> { + let Some(admin_url) = pg_test_url() else { + return Ok(None); + }; + let admin_pool = Pool::::connect(&admin_url).await?; + + let db_name = format!( + "bdk_sqlx_test_{}_{}", + std::process::id(), + TEST_DB_COUNTER.fetch_add(1, Ordering::Relaxed) + ); + + let _guard = TEST_DB_LOCK.lock().await; + let mut mgmt = pg_mgmt_lock(&admin_pool).await?; + + let result = async { + let stale: Vec = sqlx::query_scalar( + "SELECT datname::text FROM pg_database d + WHERE datname LIKE 'bdk_sqlx_test_%' + AND NOT EXISTS (SELECT 1 FROM pg_stat_activity a WHERE a.datname = d.datname)", + ) + .fetch_all(&mut *mgmt) + .await?; + for stale_db in stale { + let _ = sqlx::query(&format!(r#"DROP DATABASE IF EXISTS "{stale_db}""#)) + .execute(&mut *mgmt) + .await; } - tx.commit().await?; + sqlx::query(&format!(r#"CREATE DATABASE "{db_name}""#)) + .execute(&mut *mgmt) + .await?; - Ok(()) + // min_connections(1) keeps a session open for the pool's lifetime, which protects + // this database from the stale-database cleanup of tests in other processes. + let opts = PgConnectOptions::from_str(&admin_url)?.database(&db_name); + let pool = PgPoolOptions::new() + .min_connections(1) + .connect_with(opts) + .await?; + anyhow::Ok(Some(pool)) } + .await; + + pg_mgmt_unlock(mgmt).await; + result } #[derive(Debug)] @@ -106,10 +198,20 @@ enum TestStore { Sqlite(Store), } +impl TestStore { + /// Read the full changeset, matching `Store::read` on either backend. + async fn read(&self) -> Result { + match self { + TestStore::Postgres(store) => store.read().await, + TestStore::Sqlite(store) => store.read().await, + } + } +} + impl AsyncWalletPersister for TestStore { type Error = BdkSqlxError; - #[tracing::instrument] + #[tracing::instrument(skip_all)] fn initialize<'a>(store: &'a mut Self) -> FutureResult<'a, ChangeSet, Self::Error> where Self: 'a, @@ -121,7 +223,7 @@ impl AsyncWalletPersister for TestStore { } } - #[tracing::instrument] + #[tracing::instrument(skip_all)] fn persist<'a>( store: &'a mut Self, changeset: &'a ChangeSet, @@ -137,54 +239,355 @@ impl AsyncWalletPersister for TestStore { } } -pub async fn _drop_tables() -> anyhow::Result<()> { - let url = env::var("DATABASE_TEST_URL").expect("DATABASE_TEST_URL must be set for tests"); - let pool = Pool::::connect(&url.clone()).await?; +/// A reorg that replaces the block at a height with a different hash must leave exactly +/// one row for that height (the old row's anchors cascade away), not accumulate +/// duplicate rows that make loads nondeterministic. +#[tokio::test] +async fn reorg_replaced_block_leaves_single_row() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + let stores = create_test_stores(wallet_name.clone()).await?; + for mut store in stores { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + let _txid = insert_fake_tx( + &mut wallet, + Amount::from_sat(20_000), + Amount::from_sat(10_000), + Amount::from_sat(1_000), + ); + assert!(wallet.persist_async(&mut store).await?); + + // Replace the block at height 2000 with a different hash, as a reorg does. + let new_hash = BlockHash::from_byte_array([77u8; 32]); + let mut reorg = ChangeSet::default(); + reorg.local_chain.blocks.insert(2_000, Some(new_hash)); + TestStore::persist(&mut store, &reorg).await?; + + let cs = TestStore::initialize(&mut store).await?; + assert_eq!(cs.local_chain.blocks.get(&2_000), Some(&Some(new_hash))); + // the anchor that referenced the replaced block is gone with it; the + // anchor on the untouched block at height 1000 survives + assert_eq!(cs.tx_graph.anchors.len(), 1); + + // exactly one row must remain at that height + let rows_at_height: i64 = match &store { + TestStore::Postgres(store) => sqlx::query_scalar( + r#"SELECT count(*) FROM "bdk_wallet"."block" WHERE wallet_name=$1 AND height=2000"#, + ) + .bind(&wallet_name) + .fetch_one(&store.pool) + .await?, + TestStore::Sqlite(store) => { + sqlx::query_scalar( + "SELECT count(*) FROM block WHERE wallet_name=$1 AND height=2000", + ) + .bind(&wallet_name) + .fetch_one(&store.pool) + .await? + } + }; + assert_eq!(rows_at_height, 1); + } + Ok(()) +} + +/// Re-persisting a merged/full changeset must be idempotent (upsert, not bare INSERT), +/// and updating last_revealed for a keychain that was never stored must error rather +/// than silently updating 0 rows and losing derivation state. +#[tokio::test] +async fn repersisting_full_changeset_is_idempotent() -> anyhow::Result<()> { + initialize(); - let mut tx = pool.begin().await?; + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; - // Drop tables in reverse order of creation to handle foreign key constraints - let queries = [ - r#"DROP INDEX IF EXISTS "bdk_wallet"."idx_anchor_tx_txid""#, - r#"DROP TABLE IF EXISTS "bdk_wallet"."anchor_tx""#, - r#"DROP TABLE IF EXISTS "bdk_wallet"."txout""#, - r#"DROP TABLE IF EXISTS "bdk_wallet"."tx""#, - r#"DROP INDEX IF EXISTS "bdk_wallet"."idx_block_height""#, - r#"DROP TABLE IF EXISTS "bdk_wallet"."block""#, - r#"DROP TABLE IF EXISTS "bdk_wallet"."keychain""#, - r#"DROP TABLE IF EXISTS "bdk_wallet"."network""#, - r#"DROP SCHEMA IF EXISTS "bdk_wallet" CASCADE"#, - ]; + let stores = create_test_stores(wallet_name).await?; + for mut store in stores { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + let _ = wallet.reveal_next_address(External); + assert!(wallet.persist_async(&mut store).await?); - // Execute each query separately - for query in &queries { - sqlx::query(query).execute(&mut *tx).await?; + // A merged changeset carries the descriptors and network again; persisting it + // previously failed with a unique-constraint violation on the bare INSERTs. + let full = TestStore::initialize(&mut store).await?; + assert!(full.descriptor.is_some() && full.network.is_some()); + TestStore::persist(&mut store, &full).await?; + + // last_revealed for a keychain that is not stored must error loudly + let mut cs = ChangeSet::default(); + cs.indexer.last_revealed.insert( + DescriptorId(bitcoin::hashes::sha256::Hash::hash(b"missing keychain")), + 5, + ); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::QueryError { .. }) + ); } + Ok(()) +} + +async fn corrupt_ranges_postgres(store: &Store, wallet_name: &str) -> anyhow::Result<()> { + let pool = store.pool.clone(); + + // a negative sat value must not wrap into astronomical amounts + sqlx::query(r#"UPDATE "bdk_wallet"."txout" SET value=-1 WHERE wallet_name=$1"#) + .bind(wallet_name) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::IntOutOfRange { .. })); + sqlx::query(r#"UPDATE "bdk_wallet"."txout" SET value=1 WHERE wallet_name=$1"#) + .bind(wallet_name) + .execute(&pool) + .await?; + + // a negative block height must not wrap into a huge height + sqlx::query( + r#"UPDATE "bdk_wallet"."block" SET height=-1 WHERE wallet_name=$1 AND height=2000"#, + ) + .bind(wallet_name) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::IntOutOfRange { .. })); + sqlx::query( + r#"UPDATE "bdk_wallet"."block" SET height=2000 WHERE wallet_name=$1 AND height=-1"#, + ) + .bind(wallet_name) + .execute(&pool) + .await?; + + store.read().await?; + Ok(()) +} + +async fn corrupt_ranges_sqlite(store: &Store, wallet_name: &str) -> anyhow::Result<()> { + let pool = store.pool.clone(); + + // a negative sat value must not wrap into astronomical amounts + sqlx::query("UPDATE txout SET value=-1 WHERE wallet_name=$1") + .bind(wallet_name) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::IntOutOfRange { .. })); + sqlx::query("UPDATE txout SET value=1 WHERE wallet_name=$1") + .bind(wallet_name) + .execute(&pool) + .await?; - tx.commit().await?; + // a negative block height must not wrap into a huge height + sqlx::query("UPDATE block SET height=-1 WHERE wallet_name=$1 AND height=2000") + .bind(wallet_name) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::IntOutOfRange { .. })); + sqlx::query("UPDATE block SET height=2000 WHERE wallet_name=$1 AND height=-1") + .bind(wallet_name) + .execute(&pool) + .await?; + store.read().await?; Ok(()) } -async fn create_test_stores(wallet_name: String) -> anyhow::Result> { - let mut stores: Vec = Vec::new(); +/// Out-of-range integers stored in the database (e.g. negative amounts or heights) +/// must error on load instead of wrapping around to huge unsigned values. +#[tokio::test] +async fn out_of_range_values_error_on_load() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + let stores = create_test_stores(wallet_name.clone()).await?; + for mut store in stores { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + let txid = insert_fake_tx( + &mut wallet, + Amount::from_sat(20_000), + Amount::from_sat(10_000), + Amount::from_sat(1_000), + ); + // add a floating txout row so the txout table is populated + let mut extra = ChangeSet::default(); + extra.tx_graph.txouts.insert( + OutPoint { txid, vout: 0 }, + TxOut { + value: Amount::from_sat(1), + script_pubkey: Default::default(), + }, + ); + assert!(wallet.persist_async(&mut store).await?); + TestStore::persist(&mut store, &extra).await?; + + match &store { + TestStore::Postgres(store) => corrupt_ranges_postgres(store, &wallet_name).await?, + TestStore::Sqlite(store) => corrupt_ranges_sqlite(store, &wallet_name).await?, + } + } + Ok(()) +} - // Set up postgres database URL (you might want to use a test-specific database) - let url = env::var("DATABASE_TEST_URL").expect("DATABASE_TEST_URL must be set for tests"); - let pool = Pool::::connect(&url.clone()).await?; +/// Data stored for a different network than the store was configured with (or an +/// unparseable network string) must fail the load instead of being silently accepted. +#[tokio::test] +async fn mismatched_network_errors_on_load() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; - // Drop all before creating new store for testing - pool.drop_all().await?; - let postgres_store = PgStoreBuilder::new(wallet_name.clone()) + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + let mut store = PgStoreBuilder::new(wallet_name.clone()) .network(NETWORK) .migrate(true) - .build_with_url(&url) + .pool(pool.clone()) + .build() + .await?; + Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + + let set_network = r#"UPDATE "bdk_wallet"."network" SET name=$2 WHERE wallet_name=$1"#; + + // a parseable but different network than the configured one + sqlx::query(set_network) + .bind(&wallet_name) + .bind("bitcoin") + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::InvalidNetwork { .. })); + + // an unparseable network string + sqlx::query(set_network) + .bind(&wallet_name) + .bind("junknet") + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::InvalidNetwork { .. })); + + sqlx::query(set_network) + .bind(&wallet_name) + .bind(NETWORK.to_string()) + .execute(&pool) + .await?; + store.read().await?; + Ok(()) +} + +#[derive(Clone)] +struct SharedWriter(std::sync::Arc>>); + +impl std::io::Write for SharedWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +/// Regression test for descriptor material leaking into tracing output: even at TRACE +/// verbosity, spans and events emitted while creating, persisting, and loading a wallet +/// must not record descriptors, public keys, or changesets. +/// +/// The events are captured through the permanent global layer installed by +/// `initialize()`. A scoped subscriber previously used here raced sqlx's sqlite +/// worker threads at registry teardown and made the whole suite flaky. +#[tokio::test] +async fn tracing_output_contains_no_descriptor_material() -> anyhow::Result<()> { + initialize(); + + let buf = log_buffer(); + buf.lock().unwrap().clear(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + let mut store = Store::::new_with_url(None, wallet_name.clone(), NETWORK, true).await?; + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) .await?; - stores.push(TestStore::Postgres(postgres_store)); + let _ = wallet.reveal_next_address(External); + wallet.persist_async(&mut store).await?; + Wallet::load().load_wallet_async(&mut store).await?; + + let logs = String::from_utf8_lossy(&buf.lock().unwrap()).into_owned(); + assert!(!logs.is_empty(), "expected tracing output to be captured"); + for needle in [ + "tprv", + "tpub", + "XPub", + "XPrv", + "DescriptorXKey", + "PublicKey(", + ] { + assert!( + !logs.contains(needle), + "tracing output leaked descriptor material ({needle}):\n{logs}" + ); + } + Ok(()) +} - // Setup sqlite in-memory database - let pool = SqlitePool::connect(":memory:").await?; - let sqlite_store = Store::::new(pool.clone(), wallet_name.clone(), true).await?; +async fn create_test_stores(wallet_name: String) -> anyhow::Result> { + let mut stores: Vec = Vec::new(); + + if let Some(pool) = create_test_pg_pool().await? { + let postgres_store = PgStoreBuilder::new(wallet_name.clone()) + .network(NETWORK) + .migrate(true) + .pool(pool) + .build() + .await?; + stores.push(TestStore::Postgres(postgres_store)); + } + + // Setup sqlite in-memory database. `new_with_url(None, ..)` configures the + // single-connection pool a shared in-memory database requires. + let sqlite_store = + Store::::new_with_url(None, wallet_name.clone(), NETWORK, true).await?; stores.push(TestStore::Sqlite(sqlite_store)); Ok(stores) @@ -234,11 +637,14 @@ pub fn insert_fake_tx(wallet: &mut Wallet, spent: Amount, change: Amount, fee: A ..new_tx(1) }; + // Checkpoints must use a distinct hash per height: the store rejects + // changesets that map one hash to several heights (it cannot represent + // them without silently losing checkpoints). insert_checkpoint( wallet, BlockId { height: 42, - hash: BlockHash::all_zeros(), + hash: block_hash(42), }, ); @@ -246,21 +652,21 @@ pub fn insert_fake_tx(wallet: &mut Wallet, spent: Amount, change: Amount, fee: A wallet, BlockId { height: 1_000, - hash: BlockHash::all_zeros(), + hash: block_hash(1), }, ); insert_checkpoint( wallet, BlockId { height: 2_000, - hash: BlockHash::all_zeros(), + hash: block_hash(2), }, ); let anchor = ConfirmationBlockTime { block_id: BlockId { height: 1_000, - hash: BlockHash::all_zeros(), + hash: block_hash(1), }, confirmation_time: 100, }; @@ -270,7 +676,7 @@ pub fn insert_fake_tx(wallet: &mut Wallet, spent: Amount, change: Amount, fee: A let anchor = ConfirmationBlockTime { block_id: BlockId { height: 2_000, - hash: BlockHash::all_zeros(), + hash: block_hash(2), }, confirmation_time: 200, }; @@ -387,7 +793,7 @@ async fn test_three_wallets_list_transactions() -> anyhow::Result<()> { TestCase::new(get_test_tr_single_sig_xprv_and_change_desc(), 20_000, 11_000, 2000).await, TestCase::new(("wpkh([bdb9a801/84'/1'/0']tpubDCopxf4CiXF9dicdGrXgZV9f8j3pYbWBVfF8WxjaFHtic4DZsgp1tQ58hZdsSu6M7FFzUyAh9rMn7RZASUkPgZCMdByYKXvVtigzGi8VJs6/0/*)#j8mkwdgr", "wpkh([bdb9a801/84'/1'/0']tpubDCopxf4CiXF9dicdGrXgZV9f8j3pYbWBVfF8WxjaFHtic4DZsgp1tQ58hZdsSu6M7FFzUyAh9rMn7RZASUkPgZCMdByYKXvVtigzGi8VJs6/1/*)#rn7hnccm"), 12_000, 30_000, 1500).await, - TestCase::new(get_test_minisicript_with_change_desc(), 44_444, 20_000, 5000).await + TestCase::new(get_test_miniscript_with_change_desc(), 44_444, 20_000, 5000).await ].into_iter().flatten().collect::>(); let mut saved_tx_ids = Vec::::new(); @@ -548,34 +954,332 @@ async fn single_descriptor_wallet_persist_and_recover() -> anyhow::Result<()> { Ok(()) } -#[tracing::instrument] +const BOGUS_TXID: &str = "1111111111111111111111111111111111111111111111111111111111111111"; + +async fn corrupt_and_check_postgres( + store: &Store, + wallet_name: &str, + txid: Txid, +) -> anyhow::Result<()> { + let pool = store.pool.clone(); + let txid = txid.to_string(); + + let valid_tx: Vec = sqlx::query_scalar( + r#"SELECT whole_tx FROM "bdk_wallet"."tx" WHERE wallet_name=$1 AND txid=$2"#, + ) + .bind(wallet_name) + .bind(&txid) + .fetch_one(&pool) + .await?; + let set_whole_tx = + r#"UPDATE "bdk_wallet"."tx" SET whole_tx=$3 WHERE wallet_name=$1 AND txid=$2"#; + + // undecodable tx bytes must fail the load, not silently drop the tx + sqlx::query(set_whole_tx) + .bind(wallet_name) + .bind(&txid) + .bind(vec![0xde_u8, 0xad, 0xbe, 0xef]) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::Consensus(_))); + + // trailing bytes after a valid tx must be rejected + let mut trailing = valid_tx.clone(); + trailing.push(0); + sqlx::query(set_whole_tx) + .bind(wallet_name) + .bind(&txid) + .bind(trailing) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::Consensus(_))); + + sqlx::query(set_whole_tx) + .bind(wallet_name) + .bind(&txid) + .bind(&valid_tx) + .execute(&pool) + .await?; + + // tx bytes that decode to a different txid than the stored txid must be rejected + sqlx::query(r#"INSERT INTO "bdk_wallet"."tx" (wallet_name, txid, whole_tx) VALUES ($1,$2,$3)"#) + .bind(wallet_name) + .bind(BOGUS_TXID) + .bind(&valid_tx) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::TxidMismatch { .. })); + sqlx::query(r#"DELETE FROM "bdk_wallet"."tx" WHERE wallet_name=$1 AND txid=$2"#) + .bind(wallet_name) + .bind(BOGUS_TXID) + .execute(&pool) + .await?; + + // an unparseable anchor must fail the load, not silently drop the anchor + let valid_anchor: serde_json::Value = sqlx::query_scalar( + r#"SELECT anchor FROM "bdk_wallet"."anchor_tx" WHERE wallet_name=$1 AND txid=$2"#, + ) + .bind(wallet_name) + .bind(&txid) + .fetch_one(&pool) + .await?; + let set_anchor = + r#"UPDATE "bdk_wallet"."anchor_tx" SET anchor=$3 WHERE wallet_name=$1 AND txid=$2"#; + sqlx::query(set_anchor) + .bind(wallet_name) + .bind(&txid) + .bind(serde_json::json!({"bogus": 1})) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::SerdeJson(_))); + + // an anchor whose payload points at a different block than the stored block_hash + let mut mutated = valid_anchor.clone(); + mutated["block_id"]["hash"] = serde_json::json!(BOGUS_TXID); + sqlx::query(set_anchor) + .bind(wallet_name) + .bind(&txid) + .bind(mutated) + .execute(&pool) + .await?; + assert_matches!( + store.read().await, + Err(BdkSqlxError::AnchorBlockHashMismatch { .. }) + ); + + sqlx::query(set_anchor) + .bind(wallet_name) + .bind(&txid) + .bind(valid_anchor) + .execute(&pool) + .await?; + store.read().await?; + Ok(()) +} + +async fn corrupt_and_check_sqlite( + store: &Store, + wallet_name: &str, + txid: Txid, +) -> anyhow::Result<()> { + let pool = store.pool.clone(); + let txid = txid.to_string(); + + let valid_tx: Vec = + sqlx::query_scalar("SELECT whole_tx FROM tx WHERE wallet_name=$1 AND txid=$2") + .bind(wallet_name) + .bind(&txid) + .fetch_one(&pool) + .await?; + let set_whole_tx = "UPDATE tx SET whole_tx=$3 WHERE wallet_name=$1 AND txid=$2"; + + // undecodable tx bytes must fail the load, not silently drop the tx + sqlx::query(set_whole_tx) + .bind(wallet_name) + .bind(&txid) + .bind(vec![0xde_u8, 0xad, 0xbe, 0xef]) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::Consensus(_))); + + // trailing bytes after a valid tx must be rejected + let mut trailing = valid_tx.clone(); + trailing.push(0); + sqlx::query(set_whole_tx) + .bind(wallet_name) + .bind(&txid) + .bind(trailing) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::Consensus(_))); + + sqlx::query(set_whole_tx) + .bind(wallet_name) + .bind(&txid) + .bind(&valid_tx) + .execute(&pool) + .await?; + + // tx bytes that decode to a different txid than the stored txid must be rejected + sqlx::query("INSERT INTO tx (wallet_name, txid, whole_tx) VALUES ($1,$2,$3)") + .bind(wallet_name) + .bind(BOGUS_TXID) + .bind(&valid_tx) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::TxidMismatch { .. })); + sqlx::query("DELETE FROM tx WHERE wallet_name=$1 AND txid=$2") + .bind(wallet_name) + .bind(BOGUS_TXID) + .execute(&pool) + .await?; + + // an unparseable anchor must fail the load, not silently drop the anchor + let valid_anchor: serde_json::Value = + sqlx::query_scalar("SELECT json(anchor) FROM anchor_tx WHERE wallet_name=$1 AND txid=$2") + .bind(wallet_name) + .bind(&txid) + .fetch_one(&pool) + .await?; + let set_anchor = "UPDATE anchor_tx SET anchor=jsonb($3) WHERE wallet_name=$1 AND txid=$2"; + sqlx::query(set_anchor) + .bind(wallet_name) + .bind(&txid) + .bind(serde_json::json!({"bogus": 1}).to_string()) + .execute(&pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::SerdeJson(_))); + + // an anchor whose payload points at a different block than the stored block_hash + let mut mutated = valid_anchor.clone(); + mutated["block_id"]["hash"] = serde_json::json!(BOGUS_TXID); + sqlx::query(set_anchor) + .bind(wallet_name) + .bind(&txid) + .bind(mutated.to_string()) + .execute(&pool) + .await?; + assert_matches!( + store.read().await, + Err(BdkSqlxError::AnchorBlockHashMismatch { .. }) + ); + + sqlx::query(set_anchor) + .bind(wallet_name) + .bind(&txid) + .bind(valid_anchor.to_string()) + .execute(&pool) + .await?; + store.read().await?; + Ok(()) +} + +/// Regression test for silent data loss: corrupted rows must produce an explicit error +/// on load instead of an `Ok` changeset that is quietly missing data. #[tokio::test] -async fn two_wallets_load() -> anyhow::Result<()> { +async fn corrupt_rows_error_on_load() -> anyhow::Result<()> { initialize(); - // Define descriptors - let (external_desc_wallet_1, internal_desc_wallet_1) = - get_test_tr_single_sig_xprv_and_change_desc(); - let (external_desc_wallet_2, internal_desc_wallet_2) = ("wpkh([bdb9a801/84'/1'/0']tpubDCopxf4CiXF9dicdGrXgZV9f8j3pYbWBVfF8WxjaFHtic4DZsgp1tQ58hZdsSu6M7FFzUyAh9rMn7RZASUkPgZCMdByYKXvVtigzGi8VJs6/0/*)#j8mkwdgr", "wpkh([bdb9a801/84'/1'/0']tpubDCopxf4CiXF9dicdGrXgZV9f8j3pYbWBVfF8WxjaFHtic4DZsgp1tQ58hZdsSu6M7FFzUyAh9rMn7RZASUkPgZCMdByYKXvVtigzGi8VJs6/1/*)#rn7hnccm"); - - // Generate a unique name for test wallets - let wallet_1_name = wallet_name_from_descriptor( - external_desc_wallet_1, - Some(internal_desc_wallet_1), + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), NETWORK, &Secp256k1::new(), )?; - let wallet_2_name = wallet_name_from_descriptor( - external_desc_wallet_2, - Some(internal_desc_wallet_2), + let stores = create_test_stores(wallet_name.clone()).await?; + for mut store in stores { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + let txid = insert_fake_tx( + &mut wallet, + Amount::from_sat(20_000), + Amount::from_sat(10_000), + Amount::from_sat(1_000), + ); + assert!(wallet.persist_async(&mut store).await?); + + match &store { + TestStore::Postgres(store) => { + corrupt_and_check_postgres(store, &wallet_name, txid).await? + } + TestStore::Sqlite(store) => corrupt_and_check_sqlite(store, &wallet_name, txid).await?, + } + } + Ok(()) +} + +/// Regression test for a reorg wedging persistence: deleting a block that still has +/// `anchor_tx` rows referencing it must succeed (the anchors are dropped with the block) +/// instead of aborting the whole persist transaction with a FK violation. +#[tokio::test] +async fn reorged_out_anchored_block_can_be_deleted() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), NETWORK, &Secp256k1::new(), )?; - let mut stores1 = create_test_stores(wallet_1_name).await?; - let mut stores2 = create_test_stores(wallet_2_name).await?; - + let stores = create_test_stores(wallet_name).await?; + for mut store in stores { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + + // Anchor transactions to blocks and persist, so anchor_tx rows reference block rows. + let _txid = insert_fake_tx( + &mut wallet, + Amount::from_sat(20_000), + Amount::from_sat(10_000), + Amount::from_sat(1_000), + ); + assert!(wallet.persist_async(&mut store).await?); + + // Simulate a reorg disconnecting the anchored block: BDK's local_chain + // changeset carries (height, None), which deletes the block row. + let mut reorg = ChangeSet::default(); + reorg.local_chain.blocks.insert(2_000, None); + TestStore::persist(&mut store, &reorg) + .await + .expect("persisting a reorg over an anchored block must not fail"); + + // The disconnected block and its anchor are gone; the rest survives. + let changeset = TestStore::initialize(&mut store).await?; + assert!(!changeset.local_chain.blocks.contains_key(&2_000)); + assert_eq!( + changeset.tx_graph.anchors.len(), + 1, + "only the anchor on the disconnected block is dropped" + ); + assert_eq!(changeset.tx_graph.txs.len(), 2); + + // Persistence still works afterwards. + let mut new_tip = ChangeSet::default(); + new_tip + .local_chain + .blocks + .insert(2_001, Some(BlockHash::from_byte_array([1u8; 32]))); + TestStore::persist(&mut store, &new_tip).await?; + } + Ok(()) +} + +#[tracing::instrument] +#[tokio::test] +async fn two_wallets_load() -> anyhow::Result<()> { + initialize(); + + // Define descriptors + let (external_desc_wallet_1, internal_desc_wallet_1) = + get_test_tr_single_sig_xprv_and_change_desc(); + let (external_desc_wallet_2, internal_desc_wallet_2) = ("wpkh([bdb9a801/84'/1'/0']tpubDCopxf4CiXF9dicdGrXgZV9f8j3pYbWBVfF8WxjaFHtic4DZsgp1tQ58hZdsSu6M7FFzUyAh9rMn7RZASUkPgZCMdByYKXvVtigzGi8VJs6/0/*)#j8mkwdgr", "wpkh([bdb9a801/84'/1'/0']tpubDCopxf4CiXF9dicdGrXgZV9f8j3pYbWBVfF8WxjaFHtic4DZsgp1tQ58hZdsSu6M7FFzUyAh9rMn7RZASUkPgZCMdByYKXvVtigzGi8VJs6/1/*)#rn7hnccm"); + + // Generate a unique name for test wallets + let wallet_1_name = wallet_name_from_descriptor( + external_desc_wallet_1, + Some(internal_desc_wallet_1), + NETWORK, + &Secp256k1::new(), + )?; + + let wallet_2_name = wallet_name_from_descriptor( + external_desc_wallet_2, + Some(internal_desc_wallet_2), + NETWORK, + &Secp256k1::new(), + )?; + + let mut stores1 = create_test_stores(wallet_1_name).await?; + let mut stores2 = create_test_stores(wallet_2_name).await?; + for _ in 0..stores1.len() { let mut store_1 = stores1.pop().unwrap(); let mut store_2 = stores2.pop().unwrap(); @@ -632,3 +1336,1988 @@ async fn two_wallets_load() -> anyhow::Result<()> { } Ok(()) } + +// --------------------------------------------------------------------------- +// Pure unit tests (no database) +// --------------------------------------------------------------------------- + +#[test] +fn checked_conv_accepts_in_range_values() { + assert_eq!(crate::checked_conv::(42, "t").unwrap(), 42u32); + assert_eq!( + crate::checked_conv::(i32::MAX, "t").unwrap(), + i32::MAX as u32 + ); + assert_eq!(crate::checked_conv::(0, "t").unwrap(), 0i32); + assert_eq!( + crate::checked_conv::(i32::MAX as u32, "t").unwrap(), + i32::MAX + ); + assert_eq!( + crate::checked_conv::(i64::MAX, "t").unwrap(), + i64::MAX as u64 + ); + assert_eq!( + crate::checked_conv::(i64::MAX as u64, "t").unwrap(), + i64::MAX + ); + assert_eq!(crate::checked_conv::(0, "t").unwrap(), 0u32); +} + +#[test] +fn checked_conv_rejects_out_of_range_values() { + // negative into unsigned + assert_matches!( + crate::checked_conv::(-1, "height"), + Err(BdkSqlxError::IntOutOfRange { + context: "height", + value: -1 + }) + ); + assert_matches!( + crate::checked_conv::(-5, "last_seen"), + Err(BdkSqlxError::IntOutOfRange { + context: "last_seen", + value: -5 + }) + ); + assert_matches!( + crate::checked_conv::(i32::MIN, "t"), + Err(BdkSqlxError::IntOutOfRange { .. }) + ); + // too large for destination + assert_matches!( + crate::checked_conv::(u32::MAX, "last_revealed"), + Err(BdkSqlxError::IntOutOfRange { + context: "last_revealed", + value + }) if value == u32::MAX as i128 + ); + assert_matches!( + crate::checked_conv::(u64::MAX, "value"), + Err(BdkSqlxError::IntOutOfRange { context: "value", value }) if value == u64::MAX as i128 + ); + assert_matches!( + crate::checked_conv::(i32::MAX as u32 + 1, "t"), + Err(BdkSqlxError::IntOutOfRange { .. }) + ); +} + +#[test] +fn error_display_messages_are_stable() { + assert_eq!(BdkSqlxError::MissingNetwork.to_string(), "Network Missing"); + assert_eq!( + BdkSqlxError::MissingPool.to_string(), + "No database connection pool provided to the builder" + ); + assert_eq!( + BdkSqlxError::IntOutOfRange { + context: "txout.value", + value: -1 + } + .to_string(), + "integer value out of range for txout.value: -1" + ); + assert_eq!( + BdkSqlxError::InvalidNetwork { + expected: "regtest".into(), + got: "bitcoin".into() + } + .to_string(), + "Invalid Network expected regtest, got bitcoin" + ); + assert!(BdkSqlxError::GetNetworkFailure + .to_string() + .contains("not set")); +} + +#[tokio::test] +async fn pg_builder_requires_network_and_pool() { + initialize(); + + // neither network nor pool set: network is validated first + assert_matches!( + PgStoreBuilder::new("w".into()).build().await, + Err(BdkSqlxError::MissingNetwork) + ); + + // network set but no pool. This must fail with MissingPool and must NOT + // touch the process-global network (build only initializes the global + // network once a pool exists). + assert_matches!( + PgStoreBuilder::new("w".into()) + .network(Regtest) + .build() + .await, + Err(BdkSqlxError::MissingPool) + ); +} + +// --------------------------------------------------------------------------- +// Shared helpers for the store-level tests +// --------------------------------------------------------------------------- + +/// A transaction with a single default input and a single output; `lock_time` and +/// `value` make each minted txid distinct. +fn sample_tx(lock_time: u32, value: u64) -> Transaction { + Transaction { + input: vec![TxIn::default()], + output: vec![TxOut { + value: Amount::from_sat(value), + script_pubkey: ScriptBuf::new(), + }], + ..new_tx(lock_time) + } +} + +fn block_hash(byte: u8) -> BlockHash { + BlockHash::from_byte_array([byte; 32]) +} + +fn anchor_at(height: u32, hash: BlockHash, confirmation_time: u64) -> ConfirmationBlockTime { + ConfirmationBlockTime { + block_id: BlockId { height, hash }, + confirmation_time, + } +} + +/// A changeset exercising every table: the network, a three-block chain with +/// distinct hashes, one confirmed tx (anchored to the block at height 10), one +/// unconfirmed tx (last_seen only), and txouts on both transactions. +fn populated_changeset() -> ChangeSet { + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + + cs.local_chain.blocks.insert(5, Some(block_hash(1))); + cs.local_chain.blocks.insert(10, Some(block_hash(2))); + cs.local_chain.blocks.insert(15, Some(block_hash(3))); + + let tx_a = sample_tx(0, 50_000); + let tx_b = sample_tx(1, 30_000); + let txid_a = tx_a.compute_txid(); + let txid_b = tx_b.compute_txid(); + cs.tx_graph.txs.insert(Arc::new(tx_a)); + cs.tx_graph.txs.insert(Arc::new(tx_b)); + cs.tx_graph.txouts.insert( + OutPoint { + txid: txid_a, + vout: 0, + }, + TxOut { + value: Amount::from_sat(50_000), + script_pubkey: ScriptBuf::from(vec![0x51]), + }, + ); + cs.tx_graph.txouts.insert( + OutPoint { + txid: txid_b, + vout: 1, + }, + TxOut { + value: Amount::from_sat(30_000), + script_pubkey: ScriptBuf::from(vec![0x52]), + }, + ); + cs.tx_graph.last_seen.insert(txid_b, 1_700_000_123); + cs.tx_graph + .anchors + .insert((anchor_at(10, block_hash(2), 12_345), txid_a)); + + cs +} + +fn assert_populated(loaded: &ChangeSet, expected: &ChangeSet) { + assert_eq!(loaded.network, expected.network); + assert_eq!(loaded.tx_graph, expected.tx_graph); + assert_eq!(loaded.local_chain, expected.local_chain); +} + +/// Row count for one of the wallet tables; table names are internal constants. +async fn table_count(store: &TestStore, table: &str, wallet_name: &str) -> anyhow::Result { + let count = match store { + TestStore::Postgres(store) => { + sqlx::query_scalar(&format!( + r#"SELECT count(*) FROM "bdk_wallet"."{table}" WHERE wallet_name=$1"# + )) + .bind(wallet_name) + .fetch_one(&store.pool) + .await? + } + TestStore::Sqlite(store) => { + sqlx::query_scalar(&format!( + "SELECT count(*) FROM {table} WHERE wallet_name=$1" + )) + .bind(wallet_name) + .fetch_one(&store.pool) + .await? + } + }; + Ok(count) +} + +const ALL_TABLES: [&str; 6] = ["network", "keychain", "block", "tx", "txout", "anchor_tx"]; + +// --------------------------------------------------------------------------- +// Store behaviour: empty stores and empty changesets +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn empty_store_reads_default_changeset() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "empty_store_reads_default_changeset".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let loaded = TestStore::initialize(&mut store).await?; + assert!(loaded.is_empty(), "fresh store must read back empty"); + } + Ok(()) +} + +#[tokio::test] +async fn persist_empty_changeset_writes_nothing() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "persist_empty_changeset_writes_nothing".to_string(); + for mut store in create_test_stores(wallet_name.clone()).await? { + TestStore::persist(&mut store, &ChangeSet::default()).await?; + for table in ALL_TABLES { + assert_eq!(table_count(&store, table, &wallet_name).await?, 0); + } + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Roundtrips +// --------------------------------------------------------------------------- + +/// Every table must survive a persist/load roundtrip unchanged, and reading +/// twice must be stable. +#[tokio::test] +async fn populated_changeset_roundtrip() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "populated_changeset_roundtrip".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let cs = populated_changeset(); + TestStore::persist(&mut store, &cs).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_populated(&loaded, &cs); + + let loaded_again = TestStore::initialize(&mut store).await?; + assert_eq!(loaded_again, loaded, "repeated reads must be identical"); + } + Ok(()) +} + +/// Changesets persisted in sequence must accumulate (merge), not replace. +#[tokio::test] +async fn changesets_merge_across_persists() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "changesets_merge_across_persists".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let tx_a = sample_tx(0, 50_000); + let tx_b = sample_tx(1, 30_000); + let txid_a = tx_a.compute_txid(); + let txid_b = tx_b.compute_txid(); + + let mut delta1 = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + delta1.tx_graph.txs.insert(Arc::new(tx_a)); + TestStore::persist(&mut store, &delta1).await?; + + let mut delta2 = ChangeSet::default(); + delta2.tx_graph.txs.insert(Arc::new(tx_b)); + delta2.tx_graph.last_seen.insert(txid_b, 42); + delta2.local_chain.blocks.insert(5, Some(block_hash(1))); + TestStore::persist(&mut store, &delta2).await?; + + let mut delta3 = ChangeSet::default(); + delta3.tx_graph.txouts.insert( + OutPoint { + txid: txid_a, + vout: 0, + }, + TxOut { + value: Amount::from_sat(50_000), + script_pubkey: ScriptBuf::new(), + }, + ); + TestStore::persist(&mut store, &delta3).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.network, Some(Regtest)); + assert_eq!(loaded.tx_graph.txs.len(), 2); + assert!(loaded + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_a)); + assert!(loaded + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_b)); + assert_eq!(loaded.tx_graph.last_seen.get(&txid_b), Some(&42)); + assert_eq!( + loaded.local_chain.blocks.get(&5), + Some(&Some(block_hash(1))) + ); + assert_eq!(loaded.tx_graph.txouts.len(), 1); + } + Ok(()) +} + +/// A reorg that evicts an anchored block drops the anchor with it; anchoring the +/// same tx to the replacement block must then roundtrip cleanly. +#[tokio::test] +async fn reanchor_after_reorg_roundtrip() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "reanchor_after_reorg_roundtrip".to_string(); + for mut store in create_test_stores(wallet_name).await? { + TestStore::persist(&mut store, &populated_changeset()).await?; + + // replace the anchored block at height 10 with a different hash + let mut reorg = ChangeSet::default(); + reorg.local_chain.blocks.insert(10, Some(block_hash(9))); + TestStore::persist(&mut store, &reorg).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert!( + loaded.tx_graph.anchors.is_empty(), + "anchors of the reorged-out block must be gone" + ); + + // re-anchor the same tx to the replacement block + let txid_a = sample_tx(0, 50_000).compute_txid(); + let mut reanchor = ChangeSet::default(); + reanchor + .tx_graph + .anchors + .insert((anchor_at(10, block_hash(9), 77_777), txid_a)); + TestStore::persist(&mut store, &reanchor).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.tx_graph.anchors.len(), 1); + assert!(loaded + .tx_graph + .anchors + .contains(&(anchor_at(10, block_hash(9), 77_777), txid_a))); + assert_eq!( + loaded.local_chain.blocks.get(&10), + Some(&Some(block_hash(9))) + ); + assert_eq!(loaded.tx_graph.txs.len(), 2, "txs must survive reorgs"); + } + Ok(()) +} + +/// Advancing the derivation index across multiple persist/load cycles. +#[tokio::test] +async fn derivation_index_advances_across_loads() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + for mut store in create_test_stores(wallet_name).await? { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + let _ = wallet.reveal_addresses_to(External, 3); + assert!(wallet.persist_async(&mut store).await?); + + let mut wallet = Wallet::load() + .load_wallet_async(&mut store) + .await? + .expect("wallet must exist"); + assert_eq!(wallet.derivation_index(External), Some(3)); + let addr = wallet.reveal_addresses_to(External, 7).last().unwrap(); + assert_eq!(addr.index, 7); + assert!(wallet.persist_async(&mut store).await?); + + let wallet = Wallet::load() + .load_wallet_async(&mut store) + .await? + .expect("wallet must exist"); + assert_eq!(wallet.derivation_index(External), Some(7)); + assert_eq!(wallet.peek_address(External, 7).address, addr.address); + } + Ok(()) +} + +/// Persisting a descriptor without any derivation state must load back with +/// `last_revealed` UNSET (NULL), not 0: a reloaded fresh wallet must behave +/// exactly like the never-persisted one (its next address is index 0). +#[tokio::test] +async fn descriptor_persist_leaves_last_revealed_unset() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let ext = parse_descriptor(external_desc); + let int = parse_descriptor(internal_desc); + + let wallet_name = "descriptor_persist_leaves_last_revealed_unset".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let cs = ChangeSet { + network: Some(Regtest), + descriptor: Some(ext.clone()), + change_descriptor: Some(int.clone()), + ..Default::default() + }; + TestStore::persist(&mut store, &cs).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.descriptor, Some(ext.clone())); + assert_eq!(loaded.change_descriptor, Some(int.clone())); + assert!( + loaded.indexer.last_revealed.is_empty(), + "no address was revealed, so no last_revealed entries must exist" + ); + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Transactionality +// --------------------------------------------------------------------------- + +/// A changeset that fails partway (here: an anchor referencing a block that was +/// never persisted, which violates the FK) must roll back everything it wrote. +#[tokio::test] +async fn failed_persist_rolls_back_everything() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "failed_persist_rolls_back_everything".to_string(); + for mut store in create_test_stores(wallet_name.clone()).await? { + let tx_a = sample_tx(0, 50_000); + let txid_a = tx_a.compute_txid(); + + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs.tx_graph.txs.insert(Arc::new(tx_a)); + // no block row for this anchor -> FK violation on anchor_tx + cs.tx_graph + .anchors + .insert((anchor_at(99, block_hash(9), 1), txid_a)); + + let result = TestStore::persist(&mut store, &cs).await; + assert_matches!( + result, + Err(BdkSqlxError::QueryError { .. }), + "both backends wrap statement failures in QueryError" + ); + + let loaded = TestStore::initialize(&mut store).await?; + assert!( + loaded.is_empty(), + "a failed persist must not leave partial data behind" + ); + for table in ALL_TABLES { + assert_eq!(table_count(&store, table, &wallet_name).await?, 0); + } + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Integer boundaries at the database boundary +// --------------------------------------------------------------------------- + +/// Values that do not fit the column types must error loudly on persist instead +/// of wrapping; maximum in-range values must roundtrip exactly. +#[tokio::test] +async fn integer_boundaries_checked_on_persist() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let ext = parse_descriptor(external_desc); + let ext_did = ext.descriptor_id(); + let txid = sample_tx(0, 1_000).compute_txid(); + + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + for mut store in create_test_stores(wallet_name).await? { + // arrange a network row and keychain row + let base = ChangeSet { + network: Some(Regtest), + descriptor: Some(ext.clone()), + ..Default::default() + }; + TestStore::persist(&mut store, &base).await?; + + // last_revealed: u32 that does not fit i32 must error + let mut cs = ChangeSet::default(); + cs.indexer.last_revealed.insert(ext_did, u32::MAX); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::IntOutOfRange { .. }), + "u32::MAX last_revealed must not wrap into i32" + ); + // the maximum representable value roundtrips + cs.indexer.last_revealed.insert(ext_did, i32::MAX as u32); + TestStore::persist(&mut store, &cs).await?; + + // txout value: u64 sats that do not fit BIGINT must error + let mut cs = ChangeSet::default(); + cs.tx_graph.txouts.insert( + OutPoint { txid, vout: 0 }, + TxOut { + value: Amount::from_sat(u64::MAX), + script_pubkey: ScriptBuf::new(), + }, + ); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::IntOutOfRange { .. }), + "u64::MAX sats must not wrap into i64" + ); + let mut cs = ChangeSet::default(); + cs.tx_graph.txouts.insert( + OutPoint { txid, vout: 0 }, + TxOut { + value: Amount::from_sat(i64::MAX as u64), + script_pubkey: ScriptBuf::new(), + }, + ); + TestStore::persist(&mut store, &cs).await?; + + // vout: u32 that does not fit INTEGER must error + let mut cs = ChangeSet::default(); + cs.tx_graph.txouts.insert( + OutPoint { + txid, + vout: u32::MAX, + }, + TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::new(), + }, + ); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::IntOutOfRange { .. }), + "u32::MAX vout must not wrap into i32" + ); + let mut cs = ChangeSet::default(); + cs.tx_graph.txouts.insert( + OutPoint { + txid, + vout: i32::MAX as u32, + }, + TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::new(), + }, + ); + TestStore::persist(&mut store, &cs).await?; + + // block height: u32 that does not fit INTEGER must error + let mut cs = ChangeSet::default(); + cs.local_chain.blocks.insert(u32::MAX, Some(block_hash(8))); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::IntOutOfRange { .. }), + "u32::MAX height must not wrap into i32" + ); + let mut cs = ChangeSet::default(); + cs.local_chain + .blocks + .insert(i32::MAX as u32, Some(block_hash(8))); + TestStore::persist(&mut store, &cs).await?; + + // last_seen: u64 epoch that does not fit BIGINT must error + let mut cs = ChangeSet::default(); + cs.tx_graph.txs.insert(Arc::new(sample_tx(0, 1_000))); + cs.tx_graph.last_seen.insert(txid, u64::MAX); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::IntOutOfRange { .. }), + "u64::MAX last_seen must not wrap into i64" + ); + let mut cs = ChangeSet::default(); + cs.tx_graph.txs.insert(Arc::new(sample_tx(0, 1_000))); + cs.tx_graph.last_seen.insert(txid, i64::MAX as u64); + TestStore::persist(&mut store, &cs).await?; + + // the failed persists rolled back; only the in-range values survive + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!( + loaded.indexer.last_revealed.get(&ext_did), + Some(&(i32::MAX as u32)) + ); + assert_eq!(loaded.tx_graph.txouts.len(), 2); + assert_eq!( + loaded + .tx_graph + .txouts + .get(&OutPoint { txid, vout: 0 }) + .map(|o| o.value), + Some(Amount::from_sat(i64::MAX as u64)) + ); + assert_eq!( + loaded.local_chain.blocks.get(&(i32::MAX as u32)), + Some(&Some(block_hash(8))) + ); + assert_eq!( + loaded.tx_graph.last_seen.get(&txid), + Some(&(i64::MAX as u64)) + ); + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Multi-tenancy and hostile input +// --------------------------------------------------------------------------- + +/// Wallet names are user-controlled input; they must be treated as data, never +/// as SQL. Names containing injection payloads and unicode must roundtrip and +/// leave the schema intact. +#[tokio::test] +async fn hostile_wallet_names_are_inert() -> anyhow::Result<()> { + initialize(); + + for wallet_name in [ + "'; DROP TABLE bdk_wallet.network; --".to_string(), + "钱包💰\"'\\;".to_string(), + ] { + for mut store in create_test_stores(wallet_name.clone()).await? { + let cs = populated_changeset(); + TestStore::persist(&mut store, &cs).await?; + let loaded = TestStore::initialize(&mut store).await?; + assert_populated(&loaded, &cs); + assert_eq!( + table_count(&store, "network", &wallet_name).await?, + 1, + "schema and rows must survive a hostile wallet name" + ); + } + } + Ok(()) +} + +/// Two wallets sharing one connection pool must only ever see their own rows. +#[tokio::test] +async fn wallets_sharing_a_pool_are_isolated() -> anyhow::Result<()> { + initialize(); + + let tx_a = sample_tx(0, 50_000); + let tx_b = sample_tx(1, 30_000); + let txid_a = tx_a.compute_txid(); + let txid_b = tx_b.compute_txid(); + + let mut cs_a = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs_a.tx_graph.txs.insert(Arc::new(tx_a)); + cs_a.local_chain.blocks.insert(5, Some(block_hash(1))); + + let mut cs_b = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs_b.tx_graph.txs.insert(Arc::new(tx_b)); + + // postgres: two builders over one pool + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + let pg_a = PgStoreBuilder::new("pg_wallet_a".into()) + .network(Regtest) + .migrate(true) + .pool(pool.clone()) + .build() + .await?; + let pg_b = PgStoreBuilder::new("pg_wallet_b".into()) + .network(Regtest) + .migrate(true) + .pool(pool) + .build() + .await?; + pg_a.write(&cs_a).await?; + pg_b.write(&cs_b).await?; + let loaded_a = pg_a.read().await?; + let loaded_b = pg_b.read().await?; + assert!(loaded_a + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_a)); + assert!(!loaded_a + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_b)); + assert!(loaded_b + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_b)); + assert!(!loaded_b + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_a)); + + // sqlite: two stores over one pool + let lite_a = Store::::new_with_url(None, "lite_wallet_a".into(), NETWORK, true).await?; + let lite_b = + Store::::new(lite_a.pool.clone(), "lite_wallet_b".into(), NETWORK, true).await?; + lite_a.write(&cs_a).await?; + lite_b.write(&cs_b).await?; + let loaded_a = lite_a.read().await?; + let loaded_b = lite_b.read().await?; + assert!(loaded_a + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_a)); + assert!(!loaded_a + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_b)); + assert!(loaded_b + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_b)); + assert!(!loaded_b + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_a)); + + Ok(()) +} + +/// Concurrent writers on separate connections of the same pool must both +/// succeed and both land. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn concurrent_persists_both_land() -> anyhow::Result<()> { + initialize(); + + let tx_a = sample_tx(0, 50_000); + let tx_b = sample_tx(1, 30_000); + let txid_a = tx_a.compute_txid(); + let txid_b = tx_b.compute_txid(); + + let mut cs_a = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs_a.tx_graph.txs.insert(Arc::new(tx_a)); + + let mut cs_b = ChangeSet::default(); + cs_b.tx_graph.txs.insert(Arc::new(tx_b)); + cs_b.tx_graph.last_seen.insert(txid_b, 123); + + let wallet_name = "concurrent_persists_both_land".to_string(); + for store in create_test_stores(wallet_name).await? { + // Cloning a store shares its connection pool; this also guards the + // manual `Clone` impl on `Store` (the derived impl was unusable because + // it bounded `DB: Clone`, which sqlx's marker types don't satisfy). + let (s1, s2) = match &store { + TestStore::Postgres(store) => ( + TestStore::Postgres(store.clone()), + TestStore::Postgres(store.clone()), + ), + TestStore::Sqlite(store) => ( + TestStore::Sqlite(store.clone()), + TestStore::Sqlite(store.clone()), + ), + }; + + let mut s1 = s1; + let mut s2 = s2; + let cs_a2 = cs_a.clone(); + let cs_b2 = cs_b.clone(); + let h1 = tokio::spawn(async move { TestStore::persist(&mut s1, &cs_a2).await }); + let h2 = tokio::spawn(async move { TestStore::persist(&mut s2, &cs_b2).await }); + h1.await??; + h2.await??; + + let mut store = store; + let loaded = TestStore::initialize(&mut store).await?; + assert!(loaded + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_a)); + assert!(loaded + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid_b)); + assert_eq!(loaded.tx_graph.last_seen.get(&txid_b), Some(&123)); + } + Ok(()) +} + +/// Regression test: two writers persisting different hashes for the same +/// (previously unoccupied) height raced the block table's two unique indexes. +/// The loser's DELETE could not see the winner's uncommitted row, and its +/// INSERT then violated idx_block_wallet_height -- an index the upsert's +/// (wallet_name, hash) conflict target does not cover -- aborting the loser's +/// whole changeset with a raw 23505. Block writes are now serialized per +/// wallet (a transaction-scoped advisory lock on postgres; sqlite's +/// single-writer lock), so both writes land and the loser cleanly replaces +/// the winner's row, exactly as if the writes had been issued sequentially. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn concurrent_block_writes_at_same_height_both_land() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "concurrent_block_writes_at_same_height_both_land".to_string(); + for store in create_test_stores(wallet_name.clone()).await? { + for round in 0..10u8 { + let height = 100 + u32::from(round); + // distinct hashes everywhere: reusing a hash at two heights would + // trip the DuplicateBlockHash guard instead of exercising the race + let hash_a = block_hash(round * 2 + 1); + let hash_b = block_hash(round * 2 + 2); + + let (s1, s2) = match &store { + TestStore::Postgres(store) => ( + TestStore::Postgres(store.clone()), + TestStore::Postgres(store.clone()), + ), + TestStore::Sqlite(store) => ( + TestStore::Sqlite(store.clone()), + TestStore::Sqlite(store.clone()), + ), + }; + let barrier = Arc::new(tokio::sync::Barrier::new(2)); + + let mut cs_a = ChangeSet::default(); + cs_a.local_chain.blocks.insert(height, Some(hash_a)); + let mut cs_b = ChangeSet::default(); + cs_b.local_chain.blocks.insert(height, Some(hash_b)); + + let (mut s1, mut s2) = (s1, s2); + let b1 = barrier.clone(); + let h1 = tokio::spawn(async move { + b1.wait().await; + TestStore::persist(&mut s1, &cs_a).await + }); + let h2 = tokio::spawn(async move { + barrier.wait().await; + TestStore::persist(&mut s2, &cs_b).await + }); + h1.await??; + h2.await??; + + // both writes landed; exactly one row per height survives + assert_eq!( + table_count(&store, "block", &wallet_name).await?, + i64::from(round) + 1, + "each raced height must hold exactly one block row" + ); + } + + let mut store = store; + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.local_chain.blocks.len(), 10); + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Corrupt stored data must fail the load loudly +// --------------------------------------------------------------------------- + +/// An unparseable descriptor string in the keychain table must error, not be +/// silently skipped. +#[tokio::test] +async fn corrupt_stored_descriptor_errors_on_load() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + for mut store in create_test_stores(wallet_name.clone()).await? { + let cs = ChangeSet { + network: Some(Regtest), + descriptor: Some(parse_descriptor(external_desc)), + ..Default::default() + }; + TestStore::persist(&mut store, &cs).await?; + + match &store { + TestStore::Postgres(store) => { + sqlx::query( + r#"UPDATE "bdk_wallet"."keychain" SET descriptor=$2 WHERE wallet_name=$1"#, + ) + .bind(&wallet_name) + .bind("wpkh(definitely-not-a-descriptor)") + .execute(&store.pool) + .await?; + } + TestStore::Sqlite(store) => { + sqlx::query("UPDATE keychain SET descriptor=$2 WHERE wallet_name=$1") + .bind(&wallet_name) + .bind("wpkh(definitely-not-a-descriptor)") + .execute(&store.pool) + .await?; + } + } + assert_matches!(store.read().await, Err(BdkSqlxError::Miniscript(_))); + } + Ok(()) +} + +/// A txid column that is not a valid txid must error on load. +#[tokio::test] +async fn corrupt_stored_txid_errors_on_load() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "corrupt_stored_txid_errors_on_load".to_string(); + for mut store in create_test_stores(wallet_name.clone()).await? { + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs.tx_graph.txs.insert(Arc::new(sample_tx(0, 1_000))); + TestStore::persist(&mut store, &cs).await?; + + match &store { + TestStore::Postgres(store) => { + sqlx::query(r#"UPDATE "bdk_wallet"."tx" SET txid=$2 WHERE wallet_name=$1"#) + .bind(&wallet_name) + .bind("not-a-txid") + .execute(&store.pool) + .await?; + } + TestStore::Sqlite(store) => { + sqlx::query("UPDATE tx SET txid=$2 WHERE wallet_name=$1") + .bind(&wallet_name) + .bind("not-a-txid") + .execute(&store.pool) + .await?; + } + } + assert_matches!(store.read().await, Err(BdkSqlxError::HexToArray(_))); + } + Ok(()) +} + +/// A block hash column that is not a valid hash must error on load. +#[tokio::test] +async fn corrupt_stored_block_hash_errors_on_load() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "corrupt_stored_block_hash_errors_on_load".to_string(); + for mut store in create_test_stores(wallet_name.clone()).await? { + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs.local_chain.blocks.insert(5, Some(block_hash(1))); + TestStore::persist(&mut store, &cs).await?; + + match &store { + TestStore::Postgres(store) => { + sqlx::query(r#"UPDATE "bdk_wallet"."block" SET hash=$2 WHERE wallet_name=$1"#) + .bind(&wallet_name) + .bind("not-a-block-hash") + .execute(&store.pool) + .await?; + } + TestStore::Sqlite(store) => { + sqlx::query("UPDATE block SET hash=$2 WHERE wallet_name=$1") + .bind(&wallet_name) + .bind("not-a-block-hash") + .execute(&store.pool) + .await?; + } + } + assert_matches!(store.read().await, Err(BdkSqlxError::HexToArray(_))); + } + Ok(()) +} + +/// Negative values in columns whose domain is unsigned must error on load: +/// last_revealed, last_seen, and vout. +#[tokio::test] +async fn negative_stored_values_error_on_load() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + let txid = sample_tx(0, 1_000).compute_txid(); + + for mut store in create_test_stores(wallet_name.clone()).await? { + let mut cs = ChangeSet { + network: Some(Regtest), + descriptor: Some(parse_descriptor(external_desc)), + ..Default::default() + }; + cs.tx_graph.txs.insert(Arc::new(sample_tx(0, 1_000))); + cs.tx_graph.last_seen.insert(txid, 100); + cs.tx_graph.txouts.insert( + OutPoint { txid, vout: 0 }, + TxOut { + value: Amount::from_sat(1_000), + script_pubkey: ScriptBuf::new(), + }, + ); + TestStore::persist(&mut store, &cs).await?; + + for (pg_sql, lite_sql) in [ + ( + r#"UPDATE "bdk_wallet"."keychain" SET last_revealed=-1 WHERE wallet_name=$1"#, + "UPDATE keychain SET last_revealed=-1 WHERE wallet_name=$1", + ), + ( + r#"UPDATE "bdk_wallet"."tx" SET last_seen=-1 WHERE wallet_name=$1"#, + "UPDATE tx SET last_seen=-1 WHERE wallet_name=$1", + ), + ( + r#"UPDATE "bdk_wallet"."txout" SET vout=-1 WHERE wallet_name=$1"#, + "UPDATE txout SET vout=-1 WHERE wallet_name=$1", + ), + ] { + match &store { + TestStore::Postgres(store) => { + sqlx::query(pg_sql) + .bind(&wallet_name) + .execute(&store.pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::IntOutOfRange { .. })); + sqlx::query(&pg_sql.replace("=-1", "=0")) + .bind(&wallet_name) + .execute(&store.pool) + .await?; + } + TestStore::Sqlite(store) => { + sqlx::query(lite_sql) + .bind(&wallet_name) + .execute(&store.pool) + .await?; + assert_matches!(store.read().await, Err(BdkSqlxError::IntOutOfRange { .. })); + sqlx::query(&lite_sql.replace("=-1", "=0")) + .bind(&wallet_name) + .execute(&store.pool) + .await?; + } + } + } + + store.read().await?; + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Backend construction +// --------------------------------------------------------------------------- + +/// Reading/writing a store whose migrations never ran must produce an error, +/// not a panic or silent success. +#[tokio::test] +async fn unmigrated_store_errors() -> anyhow::Result<()> { + initialize(); + + // postgres wraps statement failures in QueryError + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + let store = PgStoreBuilder::new("unmigrated".into()) + .network(Regtest) + .migrate(false) + .pool(pool) + .build() + .await?; + assert_matches!( + store.read().await, + Err(BdkSqlxError::QueryError { .. }), + "postgres read on unmigrated schema must error" + ); + let cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + assert_matches!( + store.write(&cs).await, + Err(BdkSqlxError::QueryError { .. }), + "postgres write on unmigrated schema must error" + ); + + // sqlite wraps statement failures in QueryError, same as postgres + let store = Store::::new_with_url(None, "unmigrated".into(), NETWORK, false).await?; + assert_matches!( + store.read().await, + Err(BdkSqlxError::QueryError { .. }), + "sqlite read on unmigrated schema must error" + ); + assert_matches!( + store.write(&cs).await, + Err(BdkSqlxError::QueryError { .. }), + "sqlite write on unmigrated schema must error" + ); + Ok(()) +} + +/// A file-backed sqlite store must keep data across connections. +#[tokio::test] +async fn sqlite_file_backed_store_persists_across_connections() -> anyhow::Result<()> { + initialize(); + + let path = std::env::temp_dir().join(format!( + "bdk_sqlx_test_{}_{}.sqlite3", + std::process::id(), + TEST_DB_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let url = format!("sqlite://{}?mode=rwc", path.display()); + let wallet_name = "sqlite_file_backed".to_string(); + + { + let store = + Store::::new_with_url(Some(url.clone()), wallet_name.clone(), NETWORK, true) + .await?; + store.write(&populated_changeset()).await?; + } + + { + // a fresh pool against the same file; migrate=false proves the schema + // really lives in the file + let store = + Store::::new_with_url(Some(url.clone()), wallet_name.clone(), NETWORK, false) + .await?; + let loaded = store.read().await?; + assert_populated(&loaded, &populated_changeset()); + } + + std::fs::remove_file(&path)?; + Ok(()) +} + +/// Running migrations twice must be a no-op the second time. +#[tokio::test] +async fn postgres_migrate_is_idempotent() -> anyhow::Result<()> { + initialize(); + + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + let store = PgStoreBuilder::new("migrate_twice".into()) + .network(Regtest) + .migrate(true) + .pool(pool) + .build() + .await?; + store.migrate().await?; + store.migrate().await?; + Ok(()) +} + +/// Running migrations twice must be a no-op the second time. +#[tokio::test] +async fn sqlite_migrate_is_idempotent() -> anyhow::Result<()> { + initialize(); + + let store = + Store::::new_with_url(None, "sqlite_migrate_twice".into(), NETWORK, true).await?; + store.migrate().await?; + store.migrate().await?; + Ok(()) +} + +#[tokio::test] +async fn sqlite_builder_requires_network_and_pool() { + initialize(); + + // neither network nor pool set: network is validated first + assert_matches!( + SqliteStoreBuilder::new("w".into()).build().await, + Err(BdkSqlxError::MissingNetwork) + ); + + // network set but no pool + assert_matches!( + SqliteStoreBuilder::new("w".into()) + .network(Regtest) + .build() + .await, + Err(BdkSqlxError::MissingPool) + ); +} + +/// The sqlite builder's URL path must produce a working store end to end; +/// `None` builds the single-connection in-memory store. +#[tokio::test] +async fn sqlite_build_with_url_creates_working_store() -> anyhow::Result<()> { + initialize(); + + let store = SqliteStoreBuilder::new("sqlite_with_url".into()) + .network(Regtest) + .migrate(true) + .build_with_url(None) + .await?; + assert!(store.read().await?.is_empty()); + store.write(&populated_changeset()).await?; + assert_populated(&store.read().await?, &populated_changeset()); + Ok(()) +} + +/// The builder's URL path must produce a working store end to end. +#[tokio::test] +async fn pg_build_with_url_creates_working_store() -> anyhow::Result<()> { + initialize(); + + let Some(admin_url) = pg_test_url() else { + return Ok(()); + }; + let admin_pool = Pool::::connect(&admin_url).await?; + let db_name = format!( + "bdk_sqlx_test_{}_{}", + std::process::id(), + TEST_DB_COUNTER.fetch_add(1, Ordering::Relaxed) + ); + + let base_url = admin_url + .rsplit_once('/') + .map(|(base, _)| base) + .expect("DATABASE_TEST_URL has a database path"); + let store_url = format!("{base_url}/{db_name}"); + + // Hold the database-management locks for the whole test: the scratch + // database's pool has no minimum connections, so without the advisory + // lock a concurrent cleanup could drop it between operations. + let _guard = TEST_DB_LOCK.lock().await; + let mut mgmt = pg_mgmt_lock(&admin_pool).await?; + + let result = async { + sqlx::query(&format!(r#"CREATE DATABASE "{db_name}""#)) + .execute(&mut *mgmt) + .await?; + + let result = async { + let store = PgStoreBuilder::new("with_url".into()) + .network(Regtest) + .migrate(true) + .build_with_url(&store_url) + .await?; + assert!(store.read().await?.is_empty()); + store.write(&populated_changeset()).await?; + assert_populated(&store.read().await?, &populated_changeset()); + anyhow::Ok(()) + } + .await; + + // best-effort cleanup of the scratch database + let _ = sqlx::query(&format!(r#"DROP DATABASE IF EXISTS "{db_name}""#)) + .execute(&mut *mgmt) + .await; + + result + } + .await; + + pg_mgmt_unlock(mgmt).await; + result +} + +// --------------------------------------------------------------------------- +// Regression tests for previously confirmed defects +// +// Each test below pinned down a bug that has since been fixed. They are kept +// as always-on regression tests guarding the fixed behaviour. +// --------------------------------------------------------------------------- + +/// Regression test: `tx.last_seen` was persisted with a bare `UPDATE tx ...` +/// that affected 0 rows when the tx row did not exist, silently dropping the +/// timestamp. The write now upserts a stub row (the schema's nullable +/// `whole_tx` column exists precisely for metadata-only rows). +#[tokio::test] +async fn bug_last_seen_without_tx_row_is_dropped() -> anyhow::Result<()> { + initialize(); + + let txid = sample_tx(0, 1_000).compute_txid(); + let wallet_name = "bug_last_seen_without_tx_row_is_dropped".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + // note: only last_seen, the full tx is not part of this changeset + cs.tx_graph.last_seen.insert(txid, 1_700_000_000); + TestStore::persist(&mut store, &cs).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!( + loaded.tx_graph.last_seen.get(&txid), + Some(&1_700_000_000), + "last_seen must survive even when the full tx was never persisted" + ); + } + Ok(()) +} + +/// Regression test: the `tx.last_seen` upsert overwrote unconditionally, so a +/// stale or replayed changeset moved the timestamp BACKWARDS, contradicting +/// bdk_chain's own `Merge` (last_seen only ever increases). The upsert now +/// keeps the maximum, matching the monotonic `last_revealed` update. +#[tokio::test] +async fn last_seen_never_regresses() -> anyhow::Result<()> { + initialize(); + + let txid = sample_tx(0, 1_000).compute_txid(); + let wallet_name = "last_seen_never_regresses".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let mut cs = ChangeSet::default(); + cs.tx_graph.txs.insert(Arc::new(sample_tx(0, 1_000))); + cs.tx_graph.last_seen.insert(txid, 200); + TestStore::persist(&mut store, &cs).await?; + + // a stale/replayed changeset must not move the stored value backwards + let mut stale = ChangeSet::default(); + stale.tx_graph.last_seen.insert(txid, 100); + TestStore::persist(&mut store, &stale).await?; + assert_eq!( + TestStore::initialize(&mut store) + .await? + .tx_graph + .last_seen + .get(&txid), + Some(&200), + "last_seen must never move backwards" + ); + + // a newer changeset still advances it + let mut fresh = ChangeSet::default(); + fresh.tx_graph.last_seen.insert(txid, 300); + TestStore::persist(&mut store, &fresh).await?; + assert_eq!( + TestStore::initialize(&mut store) + .await? + .tx_graph + .last_seen + .get(&txid), + Some(&300) + ); + } + Ok(()) +} + +/// Regression test: `read()` anchored its entire load on the `network` row, +/// so rows persisted by a changeset that carried no network landed in the +/// database but were invisible to every subsequent read. Reads now fetch the +/// tx/block tables unconditionally. +#[tokio::test] +async fn bug_rows_persisted_before_network_are_invisible() -> anyhow::Result<()> { + initialize(); + + let txid = sample_tx(0, 1_000).compute_txid(); + let wallet_name = "bug_rows_persisted_before_network_are_invisible".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let mut cs = ChangeSet::default(); + cs.tx_graph.txs.insert(Arc::new(sample_tx(0, 1_000))); + cs.local_chain.blocks.insert(5, Some(block_hash(1))); + // deliberately no network in this changeset + TestStore::persist(&mut store, &cs).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert!( + loaded + .tx_graph + .txs + .iter() + .any(|tx| tx.compute_txid() == txid), + "a persisted tx must be visible on load" + ); + assert_eq!( + loaded.local_chain.blocks.get(&5), + Some(&Some(block_hash(1))), + "a persisted block must be visible on load" + ); + } + Ok(()) +} + +/// Regression test: the block table keys rows by `(wallet_name, hash)` and +/// upserts on the hash, so a changeset mapping the same hash to two heights +/// silently moved the row and lost the other checkpoints. Such changesets are +/// now rejected with `DuplicateBlockHash` instead of being lossily persisted. +#[tokio::test] +async fn bug_same_hash_at_multiple_heights_collapses() -> anyhow::Result<()> { + initialize(); + + let hash = block_hash(7); + let wallet_name = "bug_same_hash_at_multiple_heights_collapses".to_string(); + for mut store in create_test_stores(wallet_name.clone()).await? { + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs.local_chain.blocks.insert(1, Some(hash)); + cs.local_chain.blocks.insert(2, Some(hash)); + cs.local_chain.blocks.insert(3, Some(hash)); + assert_matches!( + TestStore::persist(&mut store, &cs).await, + Err(BdkSqlxError::DuplicateBlockHash { + hash: h, + first_height: 1, + second_height: 2, + }) if h == hash, + "a non-injective block changeset must be rejected" + ); + assert_eq!( + table_count(&store, "block", &wallet_name).await?, + 0, + "the rejected changeset must not leave partial rows behind" + ); + + // the same heights with distinct hashes roundtrip fine + let mut cs = ChangeSet { + network: Some(Regtest), + ..Default::default() + }; + cs.local_chain.blocks.insert(1, Some(block_hash(1))); + cs.local_chain.blocks.insert(2, Some(block_hash(2))); + cs.local_chain.blocks.insert(3, Some(block_hash(3))); + TestStore::persist(&mut store, &cs).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.local_chain.blocks.len(), 3); + assert_eq!(table_count(&store, "block", &wallet_name).await?, 3); + } + Ok(()) +} + +/// Regression test: the postgres `write()` path never validated +/// `changeset.network` against the configured network, letting a foreign +/// network overwrite the network row and wedge all subsequent reads with +/// `InvalidNetwork`. The write is now rejected up front. +#[tokio::test] +async fn bug_postgres_write_accepts_foreign_network() -> anyhow::Result<()> { + initialize(); + + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + let store = PgStoreBuilder::new("bug_foreign_network".into()) + .network(Regtest) + .migrate(true) + .pool(pool) + .build() + .await?; + + let cs = ChangeSet { + network: Some(Network::Bitcoin), + ..Default::default() + }; + + // a store configured for regtest must refuse to persist a bitcoin row + assert_matches!( + store.write(&cs).await, + Err(_), + "write() must reject a changeset for a different network" + ); + // and the store must still be readable afterwards + assert!(store.read().await?.is_empty()); + Ok(()) +} + +/// Regression test: both schemas declared `keychain.last_revealed INTEGER +/// DEFAULT 0`, so a wallet persisted before ever revealing an address reloaded +/// with `last_revealed = Some(0)` instead of `None` and skipped index 0 +/// forever. New rows now store NULL explicitly (and migration 04 drops the +/// default), matching upstream bdk's semantics. +/// +/// Note: no data migration rewrites existing rows -- a stored `0` is ambiguous +/// ("revealed index 0" vs "never revealed") and guessing could cause address +/// reuse. Only new rows are protected. +#[tokio::test] +async fn bug_unrevealed_wallet_reloads_skipping_index_zero() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + for mut store in create_test_stores(wallet_name).await? { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + // never reveal anything; the creation changeset is already persisted + assert_eq!(wallet.derivation_index(External), None); + assert_eq!(wallet.reveal_next_address(External).index, 0); + + let mut loaded = Wallet::load() + .load_wallet_async(&mut store) + .await? + .expect("wallet must exist"); + assert_eq!( + loaded.derivation_index(External), + None, + "a never-revealed wallet must reload with no derivation index" + ); + assert_eq!( + loaded.reveal_next_address(External).index, + 0, + "a reloaded wallet must not skip address index 0" + ); + } + Ok(()) +} + +/// Regression test: `update_last_revealed` was a plain `UPDATE`, so a stale or +/// replayed changeset moved `last_revealed` BACKWARDS and the next load +/// silently re-revealed already handed-out addresses. The update now never +/// decreases the stored value. +#[tokio::test] +async fn bug_last_revealed_regresses_causing_address_reuse() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let wallet_name = wallet_name_from_descriptor( + external_desc, + Some(internal_desc), + NETWORK, + &Secp256k1::new(), + )?; + + for mut store in create_test_stores(wallet_name).await? { + let mut wallet = Wallet::create(external_desc, internal_desc) + .network(NETWORK) + .create_wallet_async(&mut store) + .await?; + let _ = wallet.reveal_addresses_to(External, 5); + assert!(wallet.persist_async(&mut store).await?); + + // a stale changeset (replayed backup, older app instance) regresses + // the derivation state to 2 + let mut stale = ChangeSet::default(); + stale + .indexer + .last_revealed + .insert(parse_descriptor(external_desc).descriptor_id(), 2); + TestStore::persist(&mut store, &stale).await?; + + let loaded = Wallet::load() + .load_wallet_async(&mut store) + .await? + .expect("wallet must exist"); + assert_eq!( + loaded.derivation_index(External), + Some(5), + "last_revealed must never move backwards" + ); + } + Ok(()) +} + +/// Regression test: `Store::::read` claimed "a consistent snapshot" +/// from running inside one transaction, but postgres ran it at the default +/// READ COMMITTED +/// isolation, which takes a NEW snapshot for every statement, so a writer +/// committing between the keychain SELECT and the tx/block SELECTs produced a +/// mixed-generation changeset. `Store::read` now opens its transaction with +/// REPEATABLE READ; this test pins the mechanism deterministically. +#[tokio::test] +async fn bug_postgres_read_tx_is_not_snapshot_consistent() -> anyhow::Result<()> { + initialize(); + + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + let _store = PgStoreBuilder::new("isolation_demo".into()) + .network(Regtest) + .migrate(true) + .pool(pool.clone()) + .build() + .await?; + + // one REPEATABLE READ transaction, two identical statements, a concurrent + // commit in between -- the second statement must see the same snapshot + let mut read_tx = pool.begin().await?; + sqlx::query("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ") + .execute(&mut *read_tx) + .await?; + let before: i64 = sqlx::query_scalar(r#"SELECT count(*) FROM "bdk_wallet"."block""#) + .fetch_one(&mut *read_tx) + .await?; + + sqlx::query( + r#"INSERT INTO "bdk_wallet"."block" (wallet_name, hash, height) VALUES ('isolation_demo', $1, 1)"#, + ) + .bind(block_hash(9).to_string()) + .execute(&pool) + .await?; + + let after: i64 = sqlx::query_scalar(r#"SELECT count(*) FROM "bdk_wallet"."block""#) + .fetch_one(&mut *read_tx) + .await?; + read_tx.rollback().await?; + + assert_eq!( + before, after, + "statements inside one read transaction must observe a single snapshot" + ); + Ok(()) +} +/// Migration 04 upgrade path: a database created with the old 01-03 schema +/// (with `last_revealed INTEGER DEFAULT 0`) must keep its data verbatim when 04 +/// is applied -- the ambiguous stored 0s are NOT rewritten -- and afterwards +/// new rows must default to NULL instead of 0. +#[tokio::test] +async fn migration_04_preserves_data_and_drops_default() -> anyhow::Result<()> { + initialize(); + + // sqlite: 01-03 by hand, old-schema rows, then 04 + let path = std::env::temp_dir().join(format!( + "bdk_sqlx_mig04_{}_{}.sqlite3", + std::process::id(), + TEST_DB_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let url = format!("sqlite://{}?mode=rwc", path.display()); + let pool = sqlx::SqlitePool::connect(&url).await?; + for file in [ + "01_bdk_wallet.sql", + "02_anchor_tx_on_delete_cascade.sql", + "03_block_unique_height.sql", + ] { + let sql = std::fs::read_to_string(format!("migrations/sqlite/{file}"))?; + sqlx::raw_sql(&sql).execute(&pool).await?; + } + // an old-schema row relying on DEFAULT 0 (never revealed) and one with a + // real revealed index + sqlx::query("INSERT INTO keychain (wallet_name, keychainkind, descriptor, descriptor_id) VALUES ('w','External','d1',x'01')").execute(&pool).await?; + sqlx::query("INSERT INTO keychain (wallet_name, keychainkind, descriptor, descriptor_id, last_revealed) VALUES ('w','Internal','d2',x'02',9)").execute(&pool).await?; + let default_row: Option = + sqlx::query_scalar("SELECT last_revealed FROM keychain WHERE keychainkind='External'") + .fetch_one(&pool) + .await?; + assert_eq!(default_row, Some(0), "old schema must have DEFAULT 0"); + + let sql04 = + std::fs::read_to_string("migrations/sqlite/04_keychain_last_revealed_drop_default.sql")?; + sqlx::raw_sql(&sql04).execute(&pool).await?; + + let ext: Option = + sqlx::query_scalar("SELECT last_revealed FROM keychain WHERE keychainkind='External'") + .fetch_one(&pool) + .await?; + let int: Option = + sqlx::query_scalar("SELECT last_revealed FROM keychain WHERE keychainkind='Internal'") + .fetch_one(&pool) + .await?; + assert_eq!(ext, Some(0), "existing 0 must NOT be rewritten (ambiguous)"); + assert_eq!(int, Some(9), "revealed index must survive the rebuild"); + let rows: i64 = sqlx::query_scalar("SELECT count(*) FROM keychain") + .fetch_one(&pool) + .await?; + assert_eq!(rows, 2); + + sqlx::query("INSERT INTO keychain (wallet_name, keychainkind, descriptor, descriptor_id) VALUES ('w2','External','d3',x'03')").execute(&pool).await?; + let new_row: Option = + sqlx::query_scalar("SELECT last_revealed FROM keychain WHERE wallet_name='w2'") + .fetch_one(&pool) + .await?; + assert_eq!(new_row, None, "new rows must default to NULL after 04"); + drop(pool); + std::fs::remove_file(&path)?; + + // postgres: same upgrade path + let Some(pool) = create_test_pg_pool().await? else { + return Ok(()); + }; + for file in [ + "01_bdk_wallet.sql", + "02_anchor_tx_on_delete_cascade.sql", + "03_block_unique_height.sql", + ] { + let sql = std::fs::read_to_string(format!("migrations/postgres/{file}"))?; + sqlx::raw_sql(&sql).execute(&pool).await?; + } + sqlx::query(r#"INSERT INTO "bdk_wallet"."keychain" (wallet_name, keychainkind, descriptor, descriptor_id) VALUES ('w','External','d1','\x01'::bytea)"#).execute(&pool).await?; + sqlx::query(r#"INSERT INTO "bdk_wallet"."keychain" (wallet_name, keychainkind, descriptor, descriptor_id, last_revealed) VALUES ('w','Internal','d2','\x02'::bytea,9)"#).execute(&pool).await?; + let default_row: Option = sqlx::query_scalar( + r#"SELECT last_revealed FROM "bdk_wallet"."keychain" WHERE keychainkind='External'"#, + ) + .fetch_one(&pool) + .await?; + assert_eq!(default_row, Some(0), "old schema must have DEFAULT 0"); + + let sql04 = + std::fs::read_to_string("migrations/postgres/04_keychain_last_revealed_drop_default.sql")?; + sqlx::raw_sql(&sql04).execute(&pool).await?; + + let ext: Option = sqlx::query_scalar( + r#"SELECT last_revealed FROM "bdk_wallet"."keychain" WHERE keychainkind='External'"#, + ) + .fetch_one(&pool) + .await?; + let int: Option = sqlx::query_scalar( + r#"SELECT last_revealed FROM "bdk_wallet"."keychain" WHERE keychainkind='Internal'"#, + ) + .fetch_one(&pool) + .await?; + assert_eq!(ext, Some(0), "existing 0 must NOT be rewritten (ambiguous)"); + assert_eq!(int, Some(9), "revealed index must survive"); + + sqlx::query(r#"INSERT INTO "bdk_wallet"."keychain" (wallet_name, keychainkind, descriptor, descriptor_id) VALUES ('w2','External','d3','\x03'::bytea)"#).execute(&pool).await?; + let new_row: Option = sqlx::query_scalar( + r#"SELECT last_revealed FROM "bdk_wallet"."keychain" WHERE wallet_name='w2'"#, + ) + .fetch_one(&pool) + .await?; + assert_eq!(new_row, None, "new rows must default to NULL after 04"); + Ok(()) +} + +/// Regression test: reads anchored keychain rows on the `network` row, so a +/// changeset carrying descriptors (and derivation state) but no network wrote +/// rows that every subsequent read silently skipped -- the same defect class +/// as the tx/block invisibility bug, one table over. Keychain rows are now +/// read unconditionally, like the tx/block tables. +#[tokio::test] +async fn keychain_persisted_without_network_roundtrips() -> anyhow::Result<()> { + initialize(); + + let (external_desc, internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let ext = parse_descriptor(external_desc); + let int = parse_descriptor(internal_desc); + let ext_did = ext.descriptor_id(); + + let wallet_name = "keychain_persisted_without_network_roundtrips".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let mut cs = ChangeSet { + descriptor: Some(ext.clone()), + change_descriptor: Some(int.clone()), + ..Default::default() + }; + cs.indexer.last_revealed.insert(ext_did, 3); + // deliberately no network in this changeset + TestStore::persist(&mut store, &cs).await?; + + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.network, None); + assert_eq!( + loaded.descriptor, + Some(ext.clone()), + "a descriptor persisted without a network must be visible on load" + ); + assert_eq!(loaded.change_descriptor, Some(int.clone())); + assert_eq!(loaded.indexer.last_revealed.get(&ext_did), Some(&3)); + } + Ok(()) +} + +/// Regression test: the sqlite backend had no network validation at all -- it +/// accepted and loaded data for any network, while postgres rejects foreign +/// networks on write and validates the stored network on read. The sqlite +/// store now takes the process-global network at construction and applies the +/// same guards. +#[tokio::test] +async fn sqlite_store_enforces_configured_network() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "sqlite_store_enforces_configured_network".to_string(); + let store = Store::::new_with_url(None, wallet_name.clone(), NETWORK, true).await?; + + // a write carrying a foreign network is rejected, and the store survives + let cs = ChangeSet { + network: Some(Network::Bitcoin), + ..Default::default() + }; + assert_matches!( + store.write(&cs).await, + Err(BdkSqlxError::InvalidNetwork { .. }), + "sqlite write must reject a changeset for a different network" + ); + assert!(store.read().await?.is_empty()); + + // data for the configured network roundtrips + store + .write(&ChangeSet { + network: Some(NETWORK), + ..Default::default() + }) + .await?; + assert_eq!(store.read().await?.network, Some(NETWORK)); + + // a foreign network written behind the store's back fails the load + sqlx::query("UPDATE network SET name=$2 WHERE wallet_name=$1") + .bind(&wallet_name) + .bind("bitcoin") + .execute(&store.pool) + .await?; + assert_matches!( + store.read().await, + Err(BdkSqlxError::InvalidNetwork { .. }), + "sqlite read must reject a stored foreign network" + ); + Ok(()) +} + +/// Regression test: `insert_descriptor`'s conflict update kept the stored +/// `last_revealed` unconditionally, so replacing a descriptor under the same +/// (wallet_name, keychainkind) made the NEW descriptor inherit the old +/// derivation index -- the wallet would silently skip those addresses on +/// load. The keep is now conditional on the descriptor being unchanged. +#[tokio::test] +async fn descriptor_rotation_resets_last_revealed() -> anyhow::Result<()> { + initialize(); + + let (external_desc, _) = get_test_tr_single_sig_xprv_and_change_desc(); + let other_desc = get_test_wpkh(); + let ext = parse_descriptor(external_desc); + let other = parse_descriptor(other_desc); + let ext_did = ext.descriptor_id(); + + let wallet_name = "descriptor_rotation_resets_last_revealed".to_string(); + for mut store in create_test_stores(wallet_name).await? { + let mut cs = ChangeSet { + network: Some(Regtest), + descriptor: Some(ext.clone()), + ..Default::default() + }; + cs.indexer.last_revealed.insert(ext_did, 5); + TestStore::persist(&mut store, &cs).await?; + + // re-persisting the SAME descriptor keeps the derivation state + TestStore::persist(&mut store, &cs).await?; + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.indexer.last_revealed.get(&ext_did), Some(&5)); + + // replacing the descriptor resets the derivation state + let rotated = ChangeSet { + descriptor: Some(other.clone()), + ..Default::default() + }; + TestStore::persist(&mut store, &rotated).await?; + let loaded = TestStore::initialize(&mut store).await?; + assert_eq!(loaded.descriptor, Some(other.clone())); + assert!( + loaded.indexer.last_revealed.is_empty(), + "a replaced descriptor must not inherit the old derivation index" + ); + } + Ok(()) +} + +/// A keychainkind value the store never writes is corrupt data and must fail +/// the load loudly rather than silently drop the keychain. +#[tokio::test] +async fn corrupt_keychainkind_errors_on_load() -> anyhow::Result<()> { + initialize(); + + let (external_desc, _) = get_test_tr_single_sig_xprv_and_change_desc(); + let ext = parse_descriptor(external_desc); + let wallet_name = "corrupt_keychainkind_errors_on_load".to_string(); + + for mut store in create_test_stores(wallet_name.clone()).await? { + let cs = ChangeSet { + network: Some(Regtest), + descriptor: Some(ext.clone()), + ..Default::default() + }; + TestStore::persist(&mut store, &cs).await?; + + match &store { + TestStore::Postgres(store) => { + sqlx::query( + r#"UPDATE "bdk_wallet"."keychain" SET keychainkind=$2 WHERE wallet_name=$1"#, + ) + .bind(&wallet_name) + .bind("Bogus") + .execute(&store.pool) + .await?; + } + TestStore::Sqlite(store) => { + sqlx::query("UPDATE keychain SET keychainkind=$2 WHERE wallet_name=$1") + .bind(&wallet_name) + .bind("Bogus") + .execute(&store.pool) + .await?; + } + } + assert_matches!( + store.read().await, + Err(BdkSqlxError::InvalidKeychainKind { .. }) + ); + } + Ok(()) +} + +/// Migration 05 drops the dead `version` table and the redundant +/// `idx_block_height` index; the store must keep working afterwards. +#[tokio::test] +async fn migration_05_drops_dead_schema() -> anyhow::Result<()> { + initialize(); + + let wallet_name = "migration_05_drops_dead_schema".to_string(); + for mut store in create_test_stores(wallet_name.clone()).await? { + match &store { + TestStore::Postgres(store) => { + let version_exists: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_schema='bdk_wallet' AND table_name='version')", + ) + .fetch_one(&store.pool) + .await?; + assert!(!version_exists, "version table must be dropped"); + let idx_exists: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_indexes WHERE schemaname='bdk_wallet' AND indexname='idx_block_height')", + ) + .fetch_one(&store.pool) + .await?; + assert!(!idx_exists, "idx_block_height must be dropped"); + } + TestStore::Sqlite(store) => { + let objects: Vec = sqlx::query_scalar( + "SELECT name FROM sqlite_master WHERE name IN ('version','idx_block_height')", + ) + .fetch_all(&store.pool) + .await?; + assert!( + objects.is_empty(), + "dead schema objects must be dropped: {objects:?}" + ); + } + } + + // the store still roundtrips every table + let cs = populated_changeset(); + TestStore::persist(&mut store, &cs).await?; + assert_populated(&TestStore::initialize(&mut store).await?, &cs); + } + Ok(()) +} diff --git a/tests/builder_network.rs b/tests/builder_network.rs new file mode 100644 index 0000000..a5b56e6 --- /dev/null +++ b/tests/builder_network.rs @@ -0,0 +1,81 @@ +//! Tests for the process-global network configuration of `PgStoreBuilder`. +//! +//! The configured network is held in a process-wide `OnceLock`, so these tests +//! live in their own integration-test binary: in-process unit tests share the +//! global with every other test and could not exercise it deterministically. +//! +//! The pools are lazy, so no database server is needed: `build()` with +//! `migrate(false)` never touches the pool. + +use bdk_sqlx::sqlx::postgres::PgPoolOptions; +use bdk_sqlx::sqlx::PgPool; +use bdk_sqlx::{BdkSqlxError, PgStoreBuilder}; +use bdk_wallet::bitcoin::Network; + +fn lazy_pool() -> PgPool { + PgPoolOptions::new() + .connect_lazy("postgres://127.0.0.1:1/bdk_sqlx_offline") + .expect("lazy pool creation does not connect") +} + +/// All scenarios in one test so ordering inside this process is deterministic: +/// the first build fixes the global network for the rest of the process. +#[tokio::test] +async fn network_config_is_process_global() { + // first build initializes the global network + PgStoreBuilder::new("wallet_a".into()) + .network(Network::Regtest) + .pool(lazy_pool()) + .build() + .await + .expect("first build must succeed"); + + // re-initializing with the same network is tolerated + PgStoreBuilder::new("wallet_b".into()) + .network(Network::Regtest) + .pool(lazy_pool()) + .build() + .await + .expect("re-init with the same network must succeed"); + + // a different network in the same process is rejected + let result = PgStoreBuilder::new("wallet_c".into()) + .network(Network::Bitcoin) + .pool(lazy_pool()) + .build() + .await; + assert!( + matches!( + result, + Err(BdkSqlxError::DuplicateInitNetwork { + current: Network::Regtest, + network: Network::Bitcoin, + }) + ), + "expected DuplicateInitNetwork, got {result:?}" + ); +} + +/// Regression test: `initialize_network` used to read the `OnceLock` and then +/// set it, so two threads racing the first initialization with the SAME +/// network could produce a spurious `SetNetworkFailure` for the loser. +/// Same-network initialization must be idempotent under concurrency. +#[tokio::test(flavor = "multi_thread", worker_threads = 8)] +async fn concurrent_same_network_init_never_fails() { + let mut handles = Vec::new(); + for i in 0..16 { + handles.push(tokio::spawn(async move { + PgStoreBuilder::new(format!("race_{i}")) + .network(Network::Regtest) + .pool(lazy_pool()) + .build() + .await + })); + } + for handle in handles { + handle + .await + .expect("task panicked") + .expect("same-network initialization must never fail"); + } +}