[Fixes] Multiple

.github/workflows/rust.yml

@@ -3,107 +3,19 @@ name: CI
 on:
   push:
     branches: [ "master" ]
-  
+
   pull_request:
     types: [ opened, synchronize, reopened ]
     branches:
       - master
 
 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
+        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 }}

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::<Sqlite>::new_with_url` now delegates to it.
+- `Store::<Sqlite>::migrate()`, mirroring `Store::<Postgres>::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::<Postgres>::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::<Sqlite>::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`.

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"

README.md

@@ -4,25 +4,41 @@
 
 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):
    ```
    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

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

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 $$;