From 2d275eb1ef6aa74d32f574991d60a0404f79bc62 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:40:45 -0400 Subject: [PATCH 01/12] docs: add wallet improvement plan and postgres storage audit --- docs/postgres-storage-audit.md | 163 ++++++++++++++++++++++++++++++++ docs/wallet-improvement-plan.md | 156 ++++++++++++++++++++++++++++++ 2 files changed, 319 insertions(+) create mode 100644 docs/postgres-storage-audit.md create mode 100644 docs/wallet-improvement-plan.md diff --git a/docs/postgres-storage-audit.md b/docs/postgres-storage-audit.md new file mode 100644 index 00000000..9056975a --- /dev/null +++ b/docs/postgres-storage-audit.md @@ -0,0 +1,163 @@ +# Postgres Storage Audit + +Audit of the Postgres wallet/contract storage (`ddk/src/storage/postgres/`) against +the code, plus 7 days of dlcd-rs logs from staging and production (2026-08). +The deployed storage code (ddk 1.1.2) is identical to local HEAD, so all +findings apply to both environments. + +**Headline findings:** + +1. The `block` table grows without bound and the full table is read on every + process start. Staging read **843,516 rows in up to 53.75 s** — the k8s + startupProbe kills the pod at 60 s, so staging trends toward a crash-loop. +2. `delete_contract` can never delete a row — it always errors and rolls back. +3. BDK changeset fields `first_seen`, `last_evicted`, and `spk_cache` are + silently dropped on persist. +4. Production logs show zero Postgres errors and zero slow statements in 7 + days; staging shows ~109 slow-statement warnings/week. All staging slowness + traces to the code patterns below. + +--- + +## Incorrect queries + +All in `ddk/src/storage/postgres/mod.rs` unless noted. Line numbers are at the +time of the audit. + +1. **`delete_contract` always fails** (lines 420-430). It runs + `query_as::<_, ContractMetadata>("DELETE FROM contract_metadata WHERE id = $1")` + with `fetch_one`. A `DELETE` without `RETURNING` yields zero rows, so + `fetch_one` returns `RowNotFound` and the transaction rolls back. Nothing is + ever deleted. dlcd-rs `repair_contract_state` (`server.rs:605`) depends on + this to evict phantom contracts and will always fail that step. + *Fix: use `.execute()` and check `rows_affected`.* + +2. **`block` table schema contradicts BDK's data model** (migration + `0002_bdk_wallet.up.sql`; write at lines 955-976, read at 933-945). BDK's + `local_chain::ChangeSet` is a map `height → Option`. The PK is + `(wallet_name, hash)`, so a reorg inserts a second row at the same height + and the stale row stays. The read builds a `BTreeMap` with no + `ORDER BY`, so the orphaned hash can win and the wallet resurrects a + reorged-out block. The reorg delete path + (`DELETE FROM block WHERE wallet_name = $1 AND height = $2`) can violate the + `anchor_tx → block` FK, which rolls back the whole persist and poisons every + retry. *Fix: PK `(wallet_name, height)` with upsert-on-conflict; decouple + `anchor_tx` from `block` (anchors carry their own block hash/height in BDK's + model — an anchor may reference a block no longer in the local chain).* + +3. **`update_last_revealed` is not monotonic** (lines 773-789). Plain + `UPDATE ... SET last_revealed = $1`. BDK's merge rule takes the maximum. A + stale write regresses the derivation index; after a restart the wallet + re-reveals used addresses (address reuse). + *Fix: `SET last_revealed = GREATEST(last_revealed, $1)`.* + +4. **Lossy ChangeSet serialization** (`write()` lines 233-292, `read()` lines + 156-189). `tx_graph::ChangeSet::first_seen` and `last_evicted`, and the + indexer `spk_cache`, are neither persisted nor read, and `persist_async` + clears the staged changeset on success — the data is permanently lost. + Losing `last_evicted` can resurrect an RBF-replaced or evicted funding + transaction as unconfirmed after a restart. + +5. **`last_seen` UPDATE with no upsert** (lines 884-891). If a changeset + carries `last_seen` for a txid whose row does not exist yet, the update + affects zero rows and the value is silently dropped. + +6. **`update_contract` non-atomic upsert with hardcoded values** (lines + 482-573). SELECT-then-INSERT/UPDATE under READ COMMITTED: two concurrent + updates for a new id both see "missing" and one dies with a unique + violation. The INSERT fallback hardcodes `is_offer_party = false` and + `fee_rate_per_vb = 1`, corrupting recreated metadata rows; + `get_contract_offers` filters on `is_offer_party = false`, so corrupted rows + leak into the offers list. *Fix: `INSERT ... ON CONFLICT (id) DO UPDATE`.* + +7. **`insert_descriptor` / `insert_network` are plain INSERTs** (lines 742-750, + 762-766). Re-staging either (wallet re-create path) hits a unique violation + and poisons the whole persist transaction. *Fix: idempotent upserts.* + +8. **`last_revealed INTEGER DEFAULT 0`** (migration 0002; read at lines + 211-224). A fresh wallet reads back `Some(0)`; BDK treats that as "index 0 + revealed" and skips the first address. *Fix: default NULL.* + +9. **Swallowed persist errors** (`ddk/src/wallet/mod.rs:296,307`). + `let _ = wallet.persist_async(...)` after revealing an address. A failed + persist is invisible; combined with finding 3 this enables silent address + reuse. + +Minor: `read()` runs five SELECTs in a READ COMMITTED transaction (each sees +its own snapshot — use REPEATABLE READ), and `changeset_from_row` panics on a +bad network string (`expect`, line 203). + +## Optimization opportunities + +1. **Unbounded `block` table + full read at startup** (lines 933-945). Staging: + 64 slow-statement warnings in 7 days on + `SELECT hash, height FROM block WHERE wallet_name = $1`, avg 8.8 s, max + 53.75 s at 843,516 rows (~2,880 new rows/day on mutinynet). The staging + startupProbe window is 60 s. Production grows ~144 rows/day (~15k total) — + same trajectory, slower. *Fix: BDK needs only sparse checkpoints plus anchor + blocks; prune non-anchor rows, or persist only the checkpoint set.* + +2. **Periodic check re-reads all contract blobs every 30 s.** + `get_signed_contracts` / `get_confirmed_contracts` / `get_preclosed_contracts` + (lines 580-666) each pull full serialized contracts by state. Production + runs exactly 2,880 cycles/day (30 s cadence; ZMQ not configured) — ~8,640 + blob scans/day. Staging: a `state = 4` scan took up to 5.0 s for 16 rows. + *Fix: filter maturity in SQL against `contract_metadata`, and/or configure + ZMQ to drop the cadence to 150 s.* + +3. **Contract blobs never compressed.** `is_compressed` is always bound `false` + (lines 397, 568). Single-row PK lookups on `contract_data` took 1.4-2.0 s in + staging (TOAST detoast of CET adaptor signature blobs). *Fix: zstd-compress, + or store adaptor signatures separately.* + +4. **Row-per-statement persist loops** (lines 872-921, 955-976). One INSERT per + tx/txout/anchor/block. Staging: slow `COMMIT` in the `write` span (5 hits, + max 3.5 s) and slow block INSERTs (4 hits). *Fix: batch with `UNNEST`.* + +5. **Duplicate indexes** (migration 0003). `idx_contract_metadata_id` and + `idx_contract_data_id` duplicate the PK indexes — drop both. `idx_block_height` + omits `wallet_name`, so the startup read has no ideal index anyway. + +6. **`get_contracts` unbounded** (lines 337-349; called from dlcd-rs gRPC + listing and the contract graph). Fetches and deserializes every contract + ever created, including closed ones. *Fix: state filters / pagination.* + +7. **Pool configuration.** dlcd-rs: `DATABASE_MAX_CONNECTIONS=25` (staging) / + `50` (production), 1 replica each, sqlx default 30 s acquire timeout. + Production's 50 is oversized but harmless. Staging logged 12 slow-acquire + warnings in 7 days — DB pressure, not pool exhaustion. + +## Log findings (7 days, source `dlcd-service`) + +| Metric | Staging | Production | +|---|---|---| +| Total log rows | ~817k (~116k/day, mostly DEBUG) | ~832k (~119k/day, mostly DEBUG) | +| sqlx slow statements (>1 s) | 109 | 0 | +| Slow pool acquires | 12 | 0 | +| Postgres errors | 0 | 0 | +| `Writing changeset` | ~1,390/day (per mutinynet block) | ~135/day (per mainnet block) | +| `Reading changeset` (process starts) | 3-24/day (pod churn) | ~0/day | +| ERROR lines | 314, all esplora/mutinynet HTTP | 1 (gRPC "Contract not found") | + +Slow-statement breakdown (staging): + +| Statement | Code location | Count | Avg / Max | Max rows | +|---|---|---|---|---| +| `SELECT hash, height FROM block WHERE wallet_name = $1` | mod.rs:933 | 64 | 8.8 s / 53.75 s | 843,516 | +| `SELECT * FROM contract_data WHERE id = $1` | mod.rs:323 | 13 | 1.64 s / 1.97 s | 1 | +| `SELECT * FROM contract_data WHERE state = 5` | mod.rs:651 | 13 | 1.6 s / 2.68 s | 2 | +| `SELECT * FROM contract_data WHERE state = 4` | mod.rs:629 | 8 | 2.05 s / 5.0 s | 16 | +| `COMMIT` (persist_bdk) | mod.rs:287 | 5 | 2.07 s / 3.51 s | — | +| `INSERT INTO block ...` | mod.rs:958 | 4 | 1.75 s / 1.96 s | — | + +The `Checking contract for oracle maturation` DEBUG line dominates production +volume (616,685 rows/7 days) — the visible half of optimization 2. + +## Not checked + +- Live database state (row counts inferred from sqlx `rows_returned` fields). +- `delete_contract` at runtime — no `RowNotFound` in the 7-day window; the + defect is confirmed by code inspection only. +- Root cause of staging pod churn (3-24 restarts/day) — k8s events not pulled. +- Query latency below 1 s (sqlx logs filter at `sqlx=info`, >1 s only). +- Testnet4 (out of scope). diff --git a/docs/wallet-improvement-plan.md b/docs/wallet-improvement-plan.md new file mode 100644 index 00000000..b16d3b30 --- /dev/null +++ b/docs/wallet-improvement-plan.md @@ -0,0 +1,156 @@ +# BDK Wallet Improvement Plan + +Status of the ecosystem (2026-08): we pin `bdk_wallet 3.0.0`, `bdk_chain 0.23.3`, `bdk_esplora 0.22.2`. +The chain crates are current. `bdk_wallet 3.1.0` is out (semver-compatible). +Multi-keychain wallets and the `bdk_tx` builder target Wallet 4.0 and are not released. +The two largest wins are already inside our pinned version and unused: +**persistent outpoint locking** and **wallet events**. + +--- + +## Phase 1 — Correctness fixes (~1 day) + +These are bugs in `ddk/src/wallet/command.rs` and `ddk/src/wallet/mod.rs`. + +1. **Scan both keychains on full scan.** The initial full scan only requests + External SPKs (`command.rs:39-47`). A wallet restored from seed does not find + its change outputs. Add `spks_for_keychain(KeychainKind::Internal, ...)`. + Also raise `stop_gap` (10 → 50) and `parallel_requests` (1 → 5). +2. **Remove the same-height short-circuit.** `command.rs:27-29` returns early + when the wallet tip equals the chain height. Unconfirmed transactions stay + invisible until the next block. Always run the sync. The 60 s timer in + `ddk.rs` already bounds the cost. +3. **Detect mempool eviction.** Seed the sync request with + `SyncRequestBuilder::expected_spk_txids` from our unconfirmed transactions. + BDK then stamps `evicted_at` and drops replaced/evicted transactions from the + canonical view. Without this, a stuck balance never recovers. +4. **Delete the fabricated `last_active_indices`.** The incremental branch + (`command.rs:68-77`) writes the current derivation indices into the update. + A sync (not full scan) must not set last-active indices. Build the `Update` + from the sync result alone. +5. **Bump `bdk_wallet` to 3.1.0.** No breaking changes. It fixes + `add_foreign_utxo` non-witness validation and a panic in `Utxo::txout` for + foreign UTXOs — both on our splice code paths. + +Also in this pass: `sign_psbt_input` clones and signs the full PSBT once per +input (`mod.rs:474-503`, called in a loop by `contract_updater.rs`). Sign the +PSBT once and copy out all requested inputs, or sign in place. + +## Phase 2 — UTXO reservation with BDK outpoint locking (~1 day) + +The manager calls `get_utxos_for_amount(..., lock_utxos: true)` and we ignore +the flag (`mod.rs:810-815`). `unreserve_utxos` is a no-op. Two concurrent +offers can select the same coins. BDK 3.0 ships the fix: + +1. In `get_utxos_for_amount`, filter `list_unspent()` through + `Wallet::is_outpoint_locked`, run coin selection, then call + `Wallet::lock_outpoint` on each selected outpoint when `lock_utxos` is true. +2. Implement `unreserve_utxos` with `Wallet::unlock_outpoint` (the manager + calls it when an offer fails or a contract is rejected). +3. Locks persist through the existing `ChangeSet` (`locked_outpoints` field) — + no storage-backend change needed; verify Postgres/Sled round-trip it. +4. Lock the funding inputs of a signed-but-unbroadcast funding transaction at + sign time, and unlock on confirmation. +5. Split the reported balance: `spendable` = unspent minus locked; + `reserved` = locked. Extend `crate::Balance` accordingly. + +## Phase 3 — Contract UTXO tracking, separate from the wallet balance (~2-3 days) + +The 2-of-2 funding outputs are invisible to BDK today. `ddk.rs::balance()` +computes the contract balance from collateral math, not from the chain. BDK +cannot show foreign outputs in `balance()`/`list_unspent()` even if inserted, +so we track them beside the wallet with the `bdk_chain` primitives we already +depend on: + +1. Add a `ContractUtxoTracker`: `SpkTxOutIndex` (or + `KeychainTxOutIndex` with a contract key type) over its own `TxGraph`, + sharing the wallet's `LocalChain` view. Index each contract's funding SPK at + offer/accept time (the script is derivable from the offer+accept messages — + same derivation `contract/splice.rs` already uses). +2. Extend `sync` to run a second, targeted `SyncRequest` carrying the funding + SPKs and outpoints. Esplora resolves outpoint spend status, so we learn both + confirmation of the funding tx and any spend of the funding output + (CET, refund, or counterparty close) in the same round trip. +3. Report `contract_confirmed` / `contract_pending` in `crate::Balance` from + `TxGraph::balance()` over the tracker's outpoints — chain truth instead of + collateral math. Keep PnL from contract state. +4. Persist the tracker with the serde `bdk_chain` changesets + (`tx_graph::ChangeSet`, indexer changeset) through the `Storage` trait, + next to the wallet ChangeSet. +5. Surface `contract_utxos()` on the wallet/DDK API for consumers (ddk-node, + FFI) so a UI can list locked collateral per contract. + +This also gives spend detection for the funding output — the input the manager +needs to notice a counterparty unilateral close without polling +`get_transaction_confirmations` per contract. + +## Phase 4 — Wallet events and real fee estimation (~1-2 days) + +1. Switch `apply_update` → `Wallet::apply_update_events` and forward + `WalletEvent`s (`TxConfirmed`, `TxReplaced { conflicts }`, `TxDropped`, + `TxUnconfirmed`) on a broadcast channel. Trigger the manager's + `PeriodicCheck` when a relevant event fires instead of only on a timer; + consumers get reorg-aware confirmation notifications for free. +2. Replace hardcoded fees. `fee_estimator()` (`mod.rs:869-897`) returns + constants, and `EsploraClient`'s `FeeEstimator` returns 1 sat/kw + (`esplora.rs:195-199`). Fetch `get_fee_estimates` from esplora on each sync, + cache into the `AtomicU32` map per `ConfirmationTarget`, and keep the + constants only as a floor/fallback. +3. Deduplicate `SendToAddress`/`SendAll` into one build-sign-broadcast helper. + +## Phase 5 — Labels and coin control (nice to have, ~2 days) + +1. BIP-329 labels via the `bip329` crate (the BDK-sanctioned approach — + bdk_wallet has no native support, issue #168). Key labels by + `Txid`/`OutPoint`/`Address`, store through the `Storage` trait. +2. Auto-label on contract events: funding tx and funding outpoint get the + contract id; CET/refund txs get the outcome. Export/import BIP-329 JSONL. +3. Coin-control send API: caller-selected UTXOs (`add_utxo` + + `manually_selected_only`), an `unspendable` exclusion list, and + `exclude_below_confirmations`. +4. Fee bumping via `Wallet::build_fee_bump` for stuck sends + (RBF is already on by default in BDK 3.x). +5. Make `MIN_CHANGE_SIZE` (25 000 sats) a builder option. + +--- + +## Splice signing: keep the PSBT boundary, do not chase descriptors + +Contract funding keys use a sha256-hardening step (`contract/keys.rs:192-208`). +They are **not** BIP32-derivable, so they can never live in a descriptor +keychain — "add contract keys to the keychain" is not expressible in BDK, and +that is fine: + +- Our signing design is already PSBT-first (`contract/psbt.rs`), which is the + exact direction BDK is moving (`Wallet::sign_psbt` in 3.2, `bdk_tx` + `Finalizer` in 4.0). The BDK `signer` module (including 3.1's + `sign_with_signers`) is deprecated for removal in 4.0 — do not build on it. +- For splices, keep `ContractKeyProvider` + `DlcInputSigningKey`. The Phase 3 + tracker makes the spliced funding UTXO visible and lockable, which is the + actual gap. +- If wallet UTXOs fund a splice alongside a DLC input, `add_foreign_utxo` (fixed + in 3.1.0) is the supported way to let the BDK wallet fund a transaction it + cannot fully sign. + +## Watch list (re-evaluate at Wallet 4.0) + +- **Multi-keychain `KeyRing`** — bdk_wallet PR #524, very active (updated + 2026-08-13). When released, the Phase 3 tracker can fold into the wallet as a + real contract keychain. +- **`bdk_tx` / `Wallet::create_psbt`** — unstable on master; a better fit for + multi-party funding transactions than `TxBuilder` once stable. +- **Balance trust classification** (bdk_wallet #431) — will fix + trusted/untrusted pending semantics that `ddk.rs::balance()` currently maps + to `change_unconfirmed`/`foreign_unconfirmed`. +- **bdk_kyoto 0.17** (compact block filters) — shipped in bdk-ffi mobile + bindings, but pre-1.0, and filters match scripts, not outpoints — weaker fit + for contract-outpoint watching than esplora. + +## Test additions (each phase lands with its tests) + +- Restore-from-seed with used change addresses (catches the Phase 1 scan bug). +- Receive-to-mempool visibility without a new block; eviction after RBF. +- Two concurrent offers must not select overlapping UTXOs; locks survive a + restart. +- Contract funding output: confirmation detection, spend detection (CET and + refund), and balance split (`spendable`/`reserved`/`contract_*`). From 9c66728f38b3fecd561e58920baa13586bf9ad36 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:41:48 -0400 Subject: [PATCH 02/12] storage: make delete_contract actually delete fetch_one on a DELETE with no RETURNING always returns RowNotFound, so every call errored and rolled back before deleting anything. Use execute instead, removing contract_data before contract_metadata so the foreign key never blocks the delete. --- ddk/src/storage/postgres/mod.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 1922e4bc..8ba74b50 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -417,15 +417,15 @@ impl ManagerStorage for PostgresStore { ) -> Result<(), ddk_manager::error::Error> { let mut tx = self.pool.begin().await.map_err(to_storage_error)?; let id = hex::encode(id); - sqlx::query_as::("DELETE FROM contract_metadata WHERE id = $1") + sqlx::query("DELETE FROM contract_data WHERE id = $1") .bind(id.clone()) - .fetch_one(&mut *tx) + .execute(&mut *tx) .await .map_err(to_storage_error)?; - sqlx::query_as::("DELETE FROM contract_data WHERE id = $1") + sqlx::query("DELETE FROM contract_metadata WHERE id = $1") .bind(id) - .fetch_one(&mut *tx) + .execute(&mut *tx) .await .map_err(to_storage_error)?; @@ -1087,4 +1087,19 @@ mod tests { let contracts = db.get_contracts().await.unwrap(); assert!(contracts.len() > 0); } + + #[tokio::test] + async fn delete_contract_removes_rows() { + let (_server, db) = seed_db().await; + + let contracts = db.get_contracts().await.unwrap(); + let id = contracts[0].get_id(); + + db.delete_contract(&id) + .await + .expect("delete_contract should succeed"); + + assert!(db.get_contract(&id).await.unwrap().is_none()); + assert!(db.get_contract_metadata(None).await.unwrap().is_empty()); + } } From 2fb335a0c8e84d8706475ec675128f0d5f783225 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:43:05 -0400 Subject: [PATCH 03/12] storage: key blocks by height and drop the anchor-to-block foreign key BDK's local chain maps height to hash, and a reorg replaces the hash at a height. Keying block rows by hash let both hashes accumulate at one height and the reader picked one nondeterministically, so the wallet could resurrect an orphaned block. The anchor_tx foreign key also made reorg deletes roll back the whole persist transaction; anchors carry their block in the JSONB payload and may reference blocks that left the sparse chain, so the constraint was wrong. The migration deduplicates existing rows, preferring the hash an anchor references. --- .../migrations/0004_block_by_height.down.sql | 7 +++ .../migrations/0004_block_by_height.up.sql | 52 +++++++++++++++++++ ddk/src/storage/postgres/mod.rs | 52 ++++++++++++++++++- 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 ddk/src/storage/postgres/migrations/0004_block_by_height.down.sql create mode 100644 ddk/src/storage/postgres/migrations/0004_block_by_height.up.sql diff --git a/ddk/src/storage/postgres/migrations/0004_block_by_height.down.sql b/ddk/src/storage/postgres/migrations/0004_block_by_height.down.sql new file mode 100644 index 00000000..61751b8c --- /dev/null +++ b/ddk/src/storage/postgres/migrations/0004_block_by_height.down.sql @@ -0,0 +1,7 @@ +ALTER TABLE block DROP CONSTRAINT block_pkey; +ALTER TABLE block ADD PRIMARY KEY (wallet_name, hash); +CREATE INDEX idx_block_height ON block (height); +-- NOT VALID: anchors may reference block rows that no longer exist. +ALTER TABLE anchor_tx + ADD FOREIGN KEY (wallet_name, block_hash) + REFERENCES block (wallet_name, hash) NOT VALID; diff --git a/ddk/src/storage/postgres/migrations/0004_block_by_height.up.sql b/ddk/src/storage/postgres/migrations/0004_block_by_height.up.sql new file mode 100644 index 00000000..18470230 --- /dev/null +++ b/ddk/src/storage/postgres/migrations/0004_block_by_height.up.sql @@ -0,0 +1,52 @@ +-- BDK's local chain is a map of height -> block hash: a reorg REPLACES the +-- hash at a height. Keying blocks by (wallet_name, hash) let the old and the +-- new hash accumulate at the same height, and the reader picked one of them +-- nondeterministically. Re-key the table by (wallet_name, height). + +-- Anchors carry their full anchor block inside the JSONB payload and may +-- legitimately reference blocks that are no longer part of the sparse local +-- chain after a reorg, so anchor_tx must not require a matching block row; +-- the constraint also made reorg deletes of block rows fail and roll back +-- the whole persist transaction. +DO $$ +DECLARE r RECORD; +BEGIN + FOR r IN + SELECT conname + FROM pg_constraint + WHERE conrelid = 'anchor_tx'::regclass + AND contype = 'f' + AND confrelid = 'block'::regclass + LOOP + EXECUTE format('ALTER TABLE anchor_tx DROP CONSTRAINT %I', r.conname); + END LOOP; +END $$; + +-- Deduplicate reorg leftovers. Prefer the hash an anchor still references; +-- tie-break on the greater hash. The next wallet sync repairs a wrong pick. +DELETE FROM block b +WHERE NOT EXISTS ( + SELECT 1 FROM anchor_tx a + WHERE a.wallet_name = b.wallet_name AND a.block_hash = b.hash) + AND EXISTS ( + SELECT 1 + FROM block b2 + JOIN anchor_tx a2 + ON a2.wallet_name = b2.wallet_name AND a2.block_hash = b2.hash + WHERE b2.wallet_name = b.wallet_name + AND b2.height = b.height + AND b2.hash <> b.hash); + +DELETE FROM block b +WHERE EXISTS ( + SELECT 1 FROM block b2 + WHERE b2.wallet_name = b.wallet_name + AND b2.height = b.height + AND b2.hash > b.hash); + +ALTER TABLE block DROP CONSTRAINT block_pkey; +ALTER TABLE block ADD PRIMARY KEY (wallet_name, height); + +-- Redundant now: the primary key covers (wallet_name, height) and every query +-- filters on wallet_name first. +DROP INDEX IF EXISTS idx_block_height; diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 8ba74b50..286555c0 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -957,7 +957,7 @@ async fn local_chain_changeset_persist_to_postgres( Some(hash) => { sqlx::query( "INSERT INTO block (wallet_name, hash, height) VALUES ($1, $2, $3) - ON CONFLICT (wallet_name, hash) DO UPDATE SET height = $3", + ON CONFLICT (wallet_name, height) DO UPDATE SET hash = EXCLUDED.hash", ) .bind(wallet_name) .bind(hash.to_string()) @@ -1088,6 +1088,56 @@ mod tests { assert!(contracts.len() > 0); } + #[tokio::test] + async fn block_reorg_replaces_hash_at_height() { + let (_server, db) = seed_db().await; + + let hash_a = BlockHash::from_byte_array([0xAA; 32]); + let hash_b = BlockHash::from_byte_array([0xBB; 32]); + + let mut changeset = ChangeSet::default(); + changeset.network = Some(Network::Regtest); + changeset.local_chain.blocks.insert(100, Some(hash_a)); + db.write(&changeset).await.unwrap(); + + // A reorg replaces the hash at the same height; the old row must go. + let mut reorg = ChangeSet::default(); + reorg.local_chain.blocks.insert(100, Some(hash_b)); + db.write(&reorg).await.unwrap(); + + let read = db.read().await.unwrap(); + assert_eq!(read.local_chain.blocks.get(&100), Some(&Some(hash_b))); + assert_eq!(read.local_chain.blocks.len(), 1); + + // An anchored tx must not block removing the block row. + let tx = bitcoin::Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![], + output: vec![], + }; + let mut anchor = ChangeSet::default(); + anchor.tx_graph.txs.insert(Arc::new(tx.clone())); + anchor.tx_graph.anchors.insert(( + ConfirmationBlockTime { + block_id: bdk_chain::BlockId { + height: 100, + hash: hash_b, + }, + confirmation_time: 1234, + }, + tx.compute_txid(), + )); + db.write(&anchor).await.unwrap(); + + let mut remove = ChangeSet::default(); + remove.local_chain.blocks.insert(100, None); + db.write(&remove).await.unwrap(); + + let read = db.read().await.unwrap(); + assert!(read.local_chain.blocks.get(&100).is_none()); + } + #[tokio::test] async fn delete_contract_removes_rows() { let (_server, db) = seed_db().await; From 7fbae9f0f06f760a71f76e7d260ed863f0129a4d Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:43:47 -0400 Subject: [PATCH 04/12] storage: never regress last_revealed BDK's merge rule for last_revealed keeps the greater index. A plain UPDATE let a stale changeset write a smaller index, and after a restart the wallet re-revealed already-used addresses. Clamp with GREATEST. --- ddk/src/storage/postgres/mod.rs | 35 ++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 286555c0..b636f496 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -776,8 +776,11 @@ async fn update_last_revealed( descriptor_id: DescriptorId, last_revealed: u32, ) -> Result<(), SqlxError> { + // BDK's merge rule for last_revealed keeps the greater index; a stale + // write must never regress it or the wallet re-reveals used addresses. sqlx::query( - "UPDATE keychain SET last_revealed = $1 WHERE wallet_name = $2 AND descriptor_id = $3", + "UPDATE keychain SET last_revealed = GREATEST(last_revealed, $1) + WHERE wallet_name = $2 AND descriptor_id = $3", ) .bind(last_revealed as i32) .bind(wallet_name) @@ -1088,6 +1091,36 @@ mod tests { assert!(contracts.len() > 0); } + #[tokio::test] + async fn last_revealed_never_regresses() { + let (_server, db) = seed_db().await; + + let descriptor: ExtendedDescriptor = "wpkh([73c5da0a/84'/1'/0']tpubDC8msFGeGuwnKG9Upg7DM2b4DaRqg3CUZa5g8v2SRQ6K4NSkxUgd7HsL2XVWbVm39yBA4LAxysQAm397zwQSQoQgewGiYZqrA9DsP4zbQ1M/0/*)" + .parse() + .unwrap(); + let did = descriptor.descriptor_id(); + + let mut changeset = ChangeSet::default(); + changeset.network = Some(Network::Regtest); + changeset.descriptor = Some(descriptor); + changeset.indexer.last_revealed.insert(did, 7); + db.write(&changeset).await.unwrap(); + + // A stale write with a smaller index must not regress the value. + let mut stale = ChangeSet::default(); + stale.indexer.last_revealed.insert(did, 3); + db.write(&stale).await.unwrap(); + let read = db.read().await.unwrap(); + assert_eq!(read.indexer.last_revealed.get(&did), Some(&7)); + + // A greater index still advances it. + let mut advance = ChangeSet::default(); + advance.indexer.last_revealed.insert(did, 9); + db.write(&advance).await.unwrap(); + let read = db.read().await.unwrap(); + assert_eq!(read.indexer.last_revealed.get(&did), Some(&9)); + } + #[tokio::test] async fn block_reorg_replaces_hash_at_height() { let (_server, db) = seed_db().await; From 2f742d38d3366f404d4d2e42bb032d92e9b0d44d Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:46:11 -0400 Subject: [PATCH 05/12] storage: persist first_seen, last_evicted, and the spk cache The tx_graph changeset also carries first_seen and last_evicted timestamps, and the keychain indexer carries a derived-spk cache. All three were dropped on persist while the staged changeset was cleared, so the data was permanently lost; losing last_evicted can resurrect an RBF-replaced or evicted transaction as unconfirmed after a restart. Upserts follow the BDK merge rules: first_seen only decreases, last_evicted only increases. --- .../0005_tx_timestamps_and_spk_cache.down.sql | 3 + .../0005_tx_timestamps_and_spk_cache.up.sql | 16 ++ ddk/src/storage/postgres/mod.rs | 174 ++++++++++++++++-- 3 files changed, 182 insertions(+), 11 deletions(-) create mode 100644 ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.down.sql create mode 100644 ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.up.sql diff --git a/ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.down.sql b/ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.down.sql new file mode 100644 index 00000000..5f195a02 --- /dev/null +++ b/ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.down.sql @@ -0,0 +1,3 @@ +DROP TABLE IF EXISTS spk_cache; +ALTER TABLE tx DROP COLUMN IF EXISTS last_evicted; +ALTER TABLE tx DROP COLUMN IF EXISTS first_seen; diff --git a/ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.up.sql b/ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.up.sql new file mode 100644 index 00000000..23e2ce2f --- /dev/null +++ b/ddk/src/storage/postgres/migrations/0005_tx_timestamps_and_spk_cache.up.sql @@ -0,0 +1,16 @@ +-- BDK's tx_graph changeset also carries first_seen (first time a tx was seen +-- in the mempool) and last_evicted (last time it was missing from the +-- mempool). Both were silently dropped on persist; losing last_evicted can +-- resurrect an RBF-replaced or evicted transaction as unconfirmed after a +-- restart. +ALTER TABLE tx ADD COLUMN first_seen BIGINT; +ALTER TABLE tx ADD COLUMN last_evicted BIGINT; + +-- The keychain indexer changeset carries a cache of derived script pubkeys. +CREATE TABLE spk_cache ( + wallet_name TEXT NOT NULL, + descriptor_id BYTEA NOT NULL, + spk_index INTEGER NOT NULL, + script BYTEA NOT NULL, + PRIMARY KEY (wallet_name, descriptor_id, spk_index) +); diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index b636f496..122633d5 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -13,7 +13,7 @@ use bdk_chain::{ use bdk_wallet::bitcoin::{ self, consensus::{self, Decodable}, - hashes::Hash, + hashes::{sha256, Hash}, Amount, BlockHash, Network, OutPoint, ScriptBuf, TxOut, Txid, }; use bdk_wallet::chain as bdk_chain; @@ -33,6 +33,7 @@ use serde_json::json; use sqlx::pool::PoolOptions; use sqlx::postgres::PgRow; use sqlx::{FromRow, Pool, Postgres, Row, Transaction}; +use std::collections::BTreeMap; use std::str::FromStr; use std::sync::Arc; @@ -226,6 +227,7 @@ impl PostgresStore { changeset.tx_graph = tx_graph_changeset_from_postgres(tx, wallet_name).await?; changeset.local_chain = local_chain_changeset_from_postgres(tx, wallet_name).await?; + changeset.indexer.spk_cache = spk_cache_from_postgres(tx, wallet_name).await?; Ok(()) } @@ -277,6 +279,10 @@ impl PostgresStore { } } + spk_cache_persist_to_postgres(&mut tx, wallet_name, &changeset.indexer.spk_cache) + .await + .map_err(StorageError::Sqlx)?; + local_chain_changeset_persist_to_postgres(&mut tx, wallet_name, &changeset.local_chain) .await .map_err(StorageError::Sqlx)?; @@ -800,16 +806,20 @@ async fn tx_graph_changeset_from_postgres( 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?; + let rows = sqlx::query( + "SELECT txid, whole_tx, last_seen, first_seen, last_evicted FROM tx WHERE wallet_name = $1", + ) + .bind(wallet_name) + .fetch_all(&mut **db_tx) + .await?; for row in rows { let txid: String = row.get("txid"); let txid = Txid::from_str(&txid)?; let whole_tx: Option> = row.get("whole_tx"); let last_seen: Option = row.get("last_seen"); + let first_seen: Option = row.get("first_seen"); + let last_evicted: Option = row.get("last_evicted"); if let Some(tx_bytes) = whole_tx { if let Ok(tx) = bitcoin::Transaction::consensus_decode(&mut tx_bytes.as_slice()) { @@ -819,6 +829,12 @@ async fn tx_graph_changeset_from_postgres( if let Some(last_seen) = last_seen { changeset.last_seen.insert(txid, last_seen as u64); } + if let Some(first_seen) = first_seen { + changeset.first_seen.insert(txid, first_seen as u64); + } + if let Some(last_evicted) = last_evicted { + changeset.last_evicted.insert(txid, last_evicted as u64); + } } // Fetch txouts @@ -893,6 +909,34 @@ async fn tx_graph_changeset_persist_to_postgres( .await?; } + // first_seen only ever decreases and last_evicted only ever increases, + // matching the tx_graph merge rules. LEAST/GREATEST ignore NULL. + for (&txid, &first_seen) in &changeset.first_seen { + sqlx::query( + "INSERT INTO tx (wallet_name, txid, first_seen) VALUES ($1, $2, $3) + ON CONFLICT (wallet_name, txid) + DO UPDATE SET first_seen = LEAST(tx.first_seen, EXCLUDED.first_seen)", + ) + .bind(wallet_name) + .bind(txid.to_string()) + .bind(first_seen as i64) + .execute(&mut **db_tx) + .await?; + } + + for (&txid, &last_evicted) in &changeset.last_evicted { + sqlx::query( + "INSERT INTO tx (wallet_name, txid, last_evicted) VALUES ($1, $2, $3) + ON CONFLICT (wallet_name, txid) + DO UPDATE SET last_evicted = GREATEST(tx.last_evicted, EXCLUDED.last_evicted)", + ) + .bind(wallet_name) + .bind(txid.to_string()) + .bind(last_evicted as i64) + .execute(&mut **db_tx) + .await?; + } + for (op, txo) in &changeset.txouts { sqlx::query( "INSERT INTO txout (wallet_name, txid, vout, value, script) VALUES ($1, $2, $3, $4, $5) @@ -925,6 +969,62 @@ async fn tx_graph_changeset_persist_to_postgres( Ok(()) } +/// Select the cached script pubkeys of the keychain indexer. +#[tracing::instrument(skip(db_tx))] +async fn spk_cache_from_postgres( + db_tx: &mut Transaction<'_, Postgres>, + wallet_name: &str, +) -> Result>, SqlxError> { + let mut cache: BTreeMap> = BTreeMap::new(); + + let rows = + sqlx::query("SELECT descriptor_id, spk_index, script FROM spk_cache WHERE wallet_name = $1") + .bind(wallet_name) + .fetch_all(&mut **db_tx) + .await?; + + for row in rows { + let descriptor_id: Vec = row.get("descriptor_id"); + let spk_index: i32 = row.get("spk_index"); + let script: Vec = row.get("script"); + let descriptor_id = <[u8; 32]>::try_from(descriptor_id.as_slice()) + .map_err(|_| SqlxError::Custom("descriptor_id is not 32 bytes".into()))?; + cache + .entry(DescriptorId(sha256::Hash::from_byte_array(descriptor_id))) + .or_default() + .insert(spk_index as u32, ScriptBuf::from(script)); + } + + Ok(cache) +} + +/// Insert cached script pubkeys of the keychain indexer. +#[tracing::instrument(skip_all)] +async fn spk_cache_persist_to_postgres( + db_tx: &mut Transaction<'_, Postgres>, + wallet_name: &str, + spk_cache: &BTreeMap>, +) -> Result<(), SqlxError> { + for (descriptor_id, spks) in spk_cache { + let descriptor_id = descriptor_id.to_byte_array(); + for (spk_index, script) in spks { + sqlx::query( + "INSERT INTO spk_cache (wallet_name, descriptor_id, spk_index, script) + VALUES ($1, $2, $3, $4) + ON CONFLICT (wallet_name, descriptor_id, spk_index) DO NOTHING", + ) + .bind(wallet_name) + .bind(descriptor_id.as_slice()) + .bind(*spk_index as i32) + .bind(script.as_bytes()) + .execute(&mut **db_tx) + .await?; + } + } + + Ok(()) +} + /// Select blocks. #[tracing::instrument(skip(db_tx))] async fn local_chain_changeset_from_postgres( @@ -1080,6 +1180,15 @@ mod tests { (server, store) } + fn dummy_tx() -> bitcoin::Transaction { + bitcoin::Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![], + output: vec![], + } + } + #[tokio::test] async fn postgres() { let (_server, db) = seed_db().await; @@ -1143,12 +1252,7 @@ mod tests { assert_eq!(read.local_chain.blocks.len(), 1); // An anchored tx must not block removing the block row. - let tx = bitcoin::Transaction { - version: bitcoin::transaction::Version::TWO, - lock_time: bitcoin::absolute::LockTime::ZERO, - input: vec![], - output: vec![], - }; + let tx = dummy_tx(); let mut anchor = ChangeSet::default(); anchor.tx_graph.txs.insert(Arc::new(tx.clone())); anchor.tx_graph.anchors.insert(( @@ -1171,6 +1275,54 @@ mod tests { assert!(read.local_chain.blocks.get(&100).is_none()); } + #[tokio::test] + async fn tx_timestamps_and_spk_cache_roundtrip() { + let (_server, db) = seed_db().await; + + let tx = dummy_tx(); + let txid = tx.compute_txid(); + let did = DescriptorId(sha256::Hash::from_byte_array([0x11; 32])); + let script = ScriptBuf::from(vec![0x00, 0x14]); + + let mut changeset = ChangeSet::default(); + changeset.network = Some(Network::Regtest); + changeset.tx_graph.txs.insert(Arc::new(tx)); + changeset.tx_graph.first_seen.insert(txid, 100); + changeset.tx_graph.last_evicted.insert(txid, 200); + changeset + .indexer + .spk_cache + .entry(did) + .or_default() + .insert(5, script.clone()); + db.write(&changeset).await.unwrap(); + + let read = db.read().await.unwrap(); + assert_eq!(read.tx_graph.first_seen.get(&txid), Some(&100)); + assert_eq!(read.tx_graph.last_evicted.get(&txid), Some(&200)); + assert_eq!( + read.indexer.spk_cache.get(&did).and_then(|m| m.get(&5)), + Some(&script) + ); + + // Merge rules: first_seen only decreases, last_evicted only increases. + let mut ignored = ChangeSet::default(); + ignored.tx_graph.first_seen.insert(txid, 150); + ignored.tx_graph.last_evicted.insert(txid, 150); + db.write(&ignored).await.unwrap(); + let read = db.read().await.unwrap(); + assert_eq!(read.tx_graph.first_seen.get(&txid), Some(&100)); + assert_eq!(read.tx_graph.last_evicted.get(&txid), Some(&200)); + + let mut taken = ChangeSet::default(); + taken.tx_graph.first_seen.insert(txid, 50); + taken.tx_graph.last_evicted.insert(txid, 250); + db.write(&taken).await.unwrap(); + let read = db.read().await.unwrap(); + assert_eq!(read.tx_graph.first_seen.get(&txid), Some(&50)); + assert_eq!(read.tx_graph.last_evicted.get(&txid), Some(&250)); + } + #[tokio::test] async fn delete_contract_removes_rows() { let (_server, db) = seed_db().await; From 8117f085a368041373ac52fe8a07e3a77fbee401 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:46:40 -0400 Subject: [PATCH 06/12] storage: upsert last_seen instead of updating a possibly-missing row A changeset can carry last_seen for a txid whose row does not exist yet; the plain UPDATE affected zero rows and silently dropped the value. Insert-or-update, clamped with GREATEST since last_seen only ever increases. --- ddk/src/storage/postgres/mod.rs | 41 ++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 122633d5..58d20797 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -900,13 +900,19 @@ async fn tx_graph_changeset_persist_to_postgres( .await?; } + // A last_seen entry can arrive before the row for its txid exists; a plain + // UPDATE silently dropped it. last_seen only ever increases. 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?; + sqlx::query( + "INSERT INTO tx (wallet_name, txid, last_seen) VALUES ($1, $2, $3) + ON CONFLICT (wallet_name, txid) + DO UPDATE SET last_seen = GREATEST(tx.last_seen, EXCLUDED.last_seen)", + ) + .bind(wallet_name) + .bind(txid.to_string()) + .bind(last_seen as i64) + .execute(&mut **db_tx) + .await?; } // first_seen only ever decreases and last_evicted only ever increases, @@ -1275,6 +1281,29 @@ mod tests { assert!(read.local_chain.blocks.get(&100).is_none()); } + #[tokio::test] + async fn last_seen_survives_missing_tx_row() { + let (_server, db) = seed_db().await; + + let txid = dummy_tx().compute_txid(); + + // No tx row exists yet for this txid; the value must not be dropped. + let mut changeset = ChangeSet::default(); + changeset.network = Some(Network::Regtest); + changeset.tx_graph.last_seen.insert(txid, 100); + db.write(&changeset).await.unwrap(); + + let read = db.read().await.unwrap(); + assert_eq!(read.tx_graph.last_seen.get(&txid), Some(&100)); + + // last_seen only ever increases. + let mut stale = ChangeSet::default(); + stale.tx_graph.last_seen.insert(txid, 50); + db.write(&stale).await.unwrap(); + let read = db.read().await.unwrap(); + assert_eq!(read.tx_graph.last_seen.get(&txid), Some(&100)); + } + #[tokio::test] async fn tx_timestamps_and_spk_cache_roundtrip() { let (_server, db) = seed_db().await; From 1d4695479791f77b93d592fb238b6078ee086c94 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:48:13 -0400 Subject: [PATCH 07/12] storage: atomic contract upsert with real metadata values update_contract did a SELECT-then-INSERT/UPDATE, which races under concurrent updates (both see missing, one dies on the unique violation), and its insert arm hardcoded is_offer_party = false and fee_rate_per_vb = 1 - corrupted rows then leaked into get_contract_offers, which filters on is_offer_party. Replace the branch with INSERT ... ON CONFLICT DO UPDATE and bind the contract's real values via a new Contract::get_fee_rate_per_vb accessor. --- ddk-manager/src/contract/mod.rs | 20 +++++ ddk/src/storage/postgres/mod.rs | 151 ++++++++++++++------------------ 2 files changed, 86 insertions(+), 85 deletions(-) diff --git a/ddk-manager/src/contract/mod.rs b/ddk-manager/src/contract/mod.rs index 20e41ae5..54b9eceb 100644 --- a/ddk-manager/src/contract/mod.rs +++ b/ddk-manager/src/contract/mod.rs @@ -256,6 +256,26 @@ impl Contract { } } + /// Get the fee rate per virtual byte for a contract. + pub fn get_fee_rate_per_vb(&self) -> u64 { + match self { + Contract::Offered(o) | Contract::Rejected(o) => o.fee_rate_per_vb, + Contract::Accepted(a) => a.offered_contract.fee_rate_per_vb, + Contract::Signed(s) | Contract::Confirmed(s) | Contract::Refunded(s) => { + s.accepted_contract.offered_contract.fee_rate_per_vb + } + Contract::PreClosed(p) => { + p.signed_contract + .accepted_contract + .offered_contract + .fee_rate_per_vb + } + Contract::FailedAccept(f) => f.offered_contract.fee_rate_per_vb, + Contract::FailedSign(f) => f.accepted_contract.offered_contract.fee_rate_per_vb, + Contract::Closed(_) => 0, + } + } + /// Get the profit and loss for a contract. pub fn get_pnl(&self) -> SignedAmount { match self { diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 58d20797..d75414b4 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -485,98 +485,60 @@ impl ManagerStorage for PostgresStore { .get_oracle_announcement() .map(|ann| ann.oracle_event.event_id.clone()); - let existing_metadata = sqlx::query_as::( - "SELECT * FROM contract_metadata WHERE id = $1", + // A single atomic upsert: the read-modify-write it replaces raced under + // concurrent updates, and its insert arm hardcoded is_offer_party and + // fee_rate_per_vb. The update arm deliberately leaves the columns set + // at insert time untouched and only advances the mutable ones. + sqlx::query( + r#" + INSERT INTO contract_metadata ( + id, state, is_offer_party, counter_party, + offer_collateral, accept_collateral, total_collateral, fee_rate_per_vb, + cet_locktime, refund_locktime, pnl, funding_txid, cet_txid, announcement_id, oracle_pubkey + ) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) + ON CONFLICT (id) DO UPDATE SET + state = EXCLUDED.state, + pnl = EXCLUDED.pnl, + funding_txid = COALESCE(EXCLUDED.funding_txid, contract_metadata.funding_txid), + cet_txid = COALESCE(EXCLUDED.cet_txid, contract_metadata.cet_txid) + "#, ) .bind(&contract_id) - .fetch_optional(&mut *tx) + .bind(prefix as i16) + .bind(contract.is_offer_party()) + .bind(hex::encode(contract.get_counter_party_id().serialize())) + .bind(offer_collateral.to_sat() as i64) + .bind(accept_collateral.to_sat() as i64) + .bind(total_collateral.to_sat() as i64) + .bind(contract.get_fee_rate_per_vb() as i64) + .bind(contract.get_cet_locktime() as i32) + .bind(contract.get_refund_locktime() as i32) + .bind(Some(contract.get_pnl().to_sat())) + .bind(&funding_txid) + .bind(&cet_txid) + .bind(announcement_id.unwrap_or_else(|| "legacy_data".to_string())) + .bind(oracle_pubkey.unwrap_or_else(|| "legacy_data".to_string())) + .execute(&mut *tx) .await .map_err(to_storage_error)?; - if existing_metadata.is_some() { - sqlx::query( - r#" - UPDATE contract_metadata SET - state = $2, - pnl = $3, - funding_txid = COALESCE($4, funding_txid), - cet_txid = COALESCE($5, cet_txid) - WHERE id = $1 - "#, - ) - .bind(&contract_id) - .bind(prefix as i16) - .bind(Some(contract.get_pnl().to_sat())) - .bind(&funding_txid) - .bind(&cet_txid) - .execute(&mut *tx) - .await - .map_err(to_storage_error)?; - } else { - sqlx::query( - r#" - INSERT INTO contract_metadata ( - id, state, is_offer_party, counter_party, - offer_collateral, accept_collateral, total_collateral, fee_rate_per_vb, - cet_locktime, refund_locktime, pnl, funding_txid, cet_txid, announcement_id, oracle_pubkey - ) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) - "#, - ) - .bind(&contract_id) - .bind(prefix as i16) - // need to track this - .bind(false) - .bind(hex::encode(contract.get_counter_party_id().serialize())) - .bind(offer_collateral.to_sat() as i64) - .bind(accept_collateral.to_sat() as i64) - .bind(total_collateral.to_sat() as i64) - // need to track this - .bind(1_i64) - .bind(contract.get_cet_locktime() as i32) - .bind(contract.get_refund_locktime() as i32) - .bind(Some(contract.get_pnl().to_sat())) - .bind(&funding_txid) - .bind(&cet_txid) - .bind(announcement_id) - .bind(&oracle_pubkey) - .execute(&mut *tx) - .await - .map_err(to_storage_error)?; - } - - let existing_data = - sqlx::query_as::("SELECT * FROM contract_data WHERE id = $1") - .bind(&contract_id) - .fetch_optional(&mut *tx) - .await - .map_err(to_storage_error)?; - - // Serialize the contract data let serialized_contract = serialize_contract(contract)?; - if existing_data.is_some() { - // Update existing contract data - sqlx::query("UPDATE contract_data SET contract_data = $2, state = $3 WHERE id = $1") - .bind(&contract_id) - .bind(&serialized_contract) - .bind(prefix as i16) - .execute(&mut *tx) - .await - .map_err(to_storage_error)?; - } else { - // Insert new contract data - sqlx::query( - "INSERT INTO contract_data (id, contract_data, is_compressed, state) VALUES ($1, $2, $3, $4)", - ) - .bind(&contract_id) - .bind(&serialized_contract) - .bind(false) // is_compressed - .bind(prefix as i16) - .execute(&mut *tx) - .await - .map_err(to_storage_error)?; - } + sqlx::query( + "INSERT INTO contract_data (id, state, contract_data, is_compressed) + VALUES ($1, $2, $3, $4) + ON CONFLICT (id) DO UPDATE SET + state = EXCLUDED.state, + contract_data = EXCLUDED.contract_data", + ) + .bind(&contract_id) + .bind(prefix as i16) + .bind(&serialized_contract) + .bind(false) + .execute(&mut *tx) + .await + .map_err(to_storage_error)?; tx.commit().await.map_err(to_storage_error)?; @@ -1281,6 +1243,25 @@ mod tests { assert!(read.local_chain.blocks.get(&100).is_none()); } + #[tokio::test] + async fn update_contract_inserts_real_metadata() { + let (_server, db) = seed_db().await; + + // The metadata row was recreated by update_contract after the temp-id + // delete (the Accepted transition); it must carry the contract's real + // values instead of hardcoded ones. + let accept = include_bytes!("../../../../testconfig/contract_binaries/Accepted"); + let accepted_contract = deserialize_contract(&accept.to_vec()).unwrap(); + + let metadata = db.get_contract_metadata(None).await.unwrap(); + assert_eq!(metadata.len(), 1); + assert_eq!(metadata[0].is_offer_party, accepted_contract.is_offer_party()); + assert_eq!( + metadata[0].fee_rate_per_vb as u64, + accepted_contract.get_fee_rate_per_vb() + ); + } + #[tokio::test] async fn last_seen_survives_missing_tx_row() { let (_server, db) = seed_db().await; From d050271756e4d3aa11ccccbec7e9ba2130459078 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:48:49 -0400 Subject: [PATCH 08/12] storage: make descriptor and network inserts idempotent Both were plain INSERTs, so re-staging either (the wallet re-create path) hit a unique violation and rolled back the whole persist transaction. Descriptors and network never change for a wallet, so DO NOTHING is the correct conflict action and preserves last_revealed. --- ddk/src/storage/postgres/mod.rs | 43 ++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index d75414b4..b49629b5 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -707,8 +707,11 @@ async fn insert_descriptor( Internal => "Internal", }; + // A wallet's descriptors never change once created; re-staging one must + // not poison the persist transaction with a unique violation. 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) VALUES ($1, $2, $3, $4) + ON CONFLICT (wallet_name, keychainkind) DO NOTHING", ) .bind(wallet_name) .bind(keychain) @@ -727,11 +730,14 @@ async fn insert_network( wallet_name: &str, network: Network, ) -> Result<(), SqlxError> { - sqlx::query("INSERT INTO network (wallet_name, name) VALUES ($1, $2)") - .bind(wallet_name) - .bind(network.to_string()) - .execute(&mut **tx) - .await?; + sqlx::query( + "INSERT INTO network (wallet_name, name) VALUES ($1, $2) + ON CONFLICT (wallet_name) DO NOTHING", + ) + .bind(wallet_name) + .bind(network.to_string()) + .execute(&mut **tx) + .await?; Ok(()) } @@ -1198,6 +1204,31 @@ mod tests { assert_eq!(read.indexer.last_revealed.get(&did), Some(&9)); } + #[tokio::test] + async fn descriptor_and_network_writes_are_idempotent() { + let (_server, db) = seed_db().await; + + let descriptor: ExtendedDescriptor = "wpkh([73c5da0a/84'/1'/0']tpubDC8msFGeGuwnKG9Upg7DM2b4DaRqg3CUZa5g8v2SRQ6K4NSkxUgd7HsL2XVWbVm39yBA4LAxysQAm397zwQSQoQgewGiYZqrA9DsP4zbQ1M/0/*)" + .parse() + .unwrap(); + let did = descriptor.descriptor_id(); + + let mut changeset = ChangeSet::default(); + changeset.network = Some(Network::Regtest); + changeset.descriptor = Some(descriptor.clone()); + changeset.indexer.last_revealed.insert(did, 4); + db.write(&changeset).await.unwrap(); + + // Re-staging the descriptor and network (wallet re-create path) must + // not violate unique constraints or reset last_revealed. + db.write(&changeset).await.unwrap(); + + let read = db.read().await.unwrap(); + assert_eq!(read.network, Some(Network::Regtest)); + assert_eq!(read.descriptor, Some(descriptor)); + assert_eq!(read.indexer.last_revealed.get(&did), Some(&4)); + } + #[tokio::test] async fn block_reorg_replaces_hash_at_height() { let (_server, db) = seed_db().await; From 0569ef499292bec8a22158912b18aebb420502bd Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:49:24 -0400 Subject: [PATCH 09/12] storage: drop the last_revealed default of 0 A fresh keychain row read back last_revealed = 0, which BDK interprets as index 0 already revealed, so the wallet skipped its first address. Existing rows at 0 are ambiguous and left untouched. --- .../0006_last_revealed_no_default.down.sql | 1 + .../0006_last_revealed_no_default.up.sql | 7 +++++++ ddk/src/storage/postgres/mod.rs | 15 ++++++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.down.sql create mode 100644 ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.up.sql diff --git a/ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.down.sql b/ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.down.sql new file mode 100644 index 00000000..abd89f2f --- /dev/null +++ b/ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.down.sql @@ -0,0 +1 @@ +ALTER TABLE keychain ALTER COLUMN last_revealed SET DEFAULT 0; diff --git a/ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.up.sql b/ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.up.sql new file mode 100644 index 00000000..6ca29ff5 --- /dev/null +++ b/ddk/src/storage/postgres/migrations/0006_last_revealed_no_default.up.sql @@ -0,0 +1,7 @@ +-- A fresh keychain row read back last_revealed = 0, which BDK interprets as +-- "derivation index 0 has been revealed", so the wallet skipped its first +-- address. NULL is the correct "nothing revealed yet" value. Existing rows at +-- 0 are ambiguous (genuinely revealed index 0, or never revealed) and are left +-- untouched; update_last_revealed only clamps upward, so the cost is at most +-- one skipped address on a wallet that never revealed anything. +ALTER TABLE keychain ALTER COLUMN last_revealed DROP DEFAULT; diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index b49629b5..2a39979c 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -1082,7 +1082,7 @@ struct KeychainEntry { keychainkind: String, descriptor: String, descriptor_id: Vec, - last_revealed: i32, + last_revealed: Option, } #[cfg(test)] @@ -1223,6 +1223,19 @@ mod tests { // not violate unique constraints or reset last_revealed. db.write(&changeset).await.unwrap(); + // A keychain row written without a revealed index must read back as + // "nothing revealed", not index 0. + let mut fresh = ChangeSet::default(); + fresh.change_descriptor = Some( + "wpkh([73c5da0a/84'/1'/0']tpubDC8msFGeGuwnKG9Upg7DM2b4DaRqg3CUZa5g8v2SRQ6K4NSkxUgd7HsL2XVWbVm39yBA4LAxysQAm397zwQSQoQgewGiYZqrA9DsP4zbQ1M/1/*)" + .parse() + .unwrap(), + ); + db.write(&fresh).await.unwrap(); + let read = db.read().await.unwrap(); + let fresh_did = fresh.change_descriptor.as_ref().unwrap().descriptor_id(); + assert!(read.indexer.last_revealed.get(&fresh_did).is_none()); + let read = db.read().await.unwrap(); assert_eq!(read.network, Some(Network::Regtest)); assert_eq!(read.descriptor, Some(descriptor)); From 52fb2ec271b1045c00c5aeeb67968cf74ed99297 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:50:14 -0400 Subject: [PATCH 10/12] wallet: surface persist errors when revealing addresses The address handlers dropped the persist result, so a failed persist lost the revealed derivation index invisibly and enabled address reuse after a restart. Return the error to the caller instead. --- ddk/src/wallet/mod.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/ddk/src/wallet/mod.rs b/ddk/src/wallet/mod.rs index ec335ee3..6c4cc83e 100644 --- a/ddk/src/wallet/mod.rs +++ b/ddk/src/wallet/mod.rs @@ -293,8 +293,14 @@ impl DlcDevKitWallet { } WalletCommand::NewExternalAddress(sender) => { let address = wallet.next_unused_address(KeychainKind::External); - let _ = wallet.persist_async(&mut storage).await; - let _ = sender.send(Ok(address)).map_err(|e| { + // A dropped persist error loses the revealed index and + // leads to address reuse after a restart. + let result = wallet + .persist_async(&mut storage) + .await + .map(|_| address) + .map_err(|e| WalletError::WalletPersistanceError(e.to_string())); + let _ = sender.send(result).map_err(|e| { log_error!( logger_clone, "Error sending new external address command. error={:?}", @@ -304,8 +310,12 @@ impl DlcDevKitWallet { } WalletCommand::NewChangeAddress(sender) => { let address = wallet.next_unused_address(KeychainKind::Internal); - let _ = wallet.persist_async(&mut storage).await; - let _ = sender.send(Ok(address)).map_err(|e| { + let result = wallet + .persist_async(&mut storage) + .await + .map(|_| address) + .map_err(|e| WalletError::WalletPersistanceError(e.to_string())); + let _ = sender.send(result).map_err(|e| { log_error!( logger_clone, "Error sending new change address command. error={:?}", From 7d7060687c0f57b084ee2f2596c08aa24f7d7501 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:51:06 -0400 Subject: [PATCH 11/12] storage: apply cargo fmt --- ddk/src/storage/postgres/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 2a39979c..1063ef27 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -951,11 +951,12 @@ async fn spk_cache_from_postgres( ) -> Result>, SqlxError> { let mut cache: BTreeMap> = BTreeMap::new(); - let rows = - sqlx::query("SELECT descriptor_id, spk_index, script FROM spk_cache WHERE wallet_name = $1") - .bind(wallet_name) - .fetch_all(&mut **db_tx) - .await?; + let rows = sqlx::query( + "SELECT descriptor_id, spk_index, script FROM spk_cache WHERE wallet_name = $1", + ) + .bind(wallet_name) + .fetch_all(&mut **db_tx) + .await?; for row in rows { let descriptor_id: Vec = row.get("descriptor_id"); @@ -1299,7 +1300,10 @@ mod tests { let metadata = db.get_contract_metadata(None).await.unwrap(); assert_eq!(metadata.len(), 1); - assert_eq!(metadata[0].is_offer_party, accepted_contract.is_offer_party()); + assert_eq!( + metadata[0].is_offer_party, + accepted_contract.is_offer_party() + ); assert_eq!( metadata[0].fee_rate_per_vb as u64, accepted_contract.get_fee_rate_per_vb() From e7c2723ad162338f4079265cc9e936c8f9dd78e6 Mon Sep 17 00:00:00 2001 From: bennyhodl Date: Thu, 13 Aug 2026 20:58:10 -0400 Subject: [PATCH 12/12] storage: export the embedded postgres migrator Consumers had to point sqlx-cli at the migration files deep inside the crate source to apply or revert the schema. The migrations are already compiled in via sqlx::migrate!, so expose the Migrator: MIGRATOR.run applies up and MIGRATOR.undo reverts, against any database, with no access to the source tree. --- ddk/src/storage/postgres/mod.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ddk/src/storage/postgres/mod.rs b/ddk/src/storage/postgres/mod.rs index 1063ef27..ab7326eb 100644 --- a/ddk/src/storage/postgres/mod.rs +++ b/ddk/src/storage/postgres/mod.rs @@ -56,6 +56,19 @@ fn max_connections_from_env() -> u32 { .unwrap_or(DEFAULT_MAX_CONNECTIONS) } +/// The embedded schema migrations for the Postgres storage backend. +/// +/// The migration files are compiled into the crate, so consumers can apply or +/// revert the schema against any database without access to the source tree: +/// +/// ```ignore +/// ddk::storage::postgres::MIGRATOR.run(&pool).await?; // apply up +/// ddk::storage::postgres::MIGRATOR.undo(&pool, version).await?; // revert down to `version` +/// ``` +/// +/// [`PostgresStore::new`] runs this same migrator when `migrations` is true. +pub static MIGRATOR: sqlx::migrate::Migrator = sqlx::migrate!("src/storage/postgres/migrations"); + /// Manages a pool of database connections. #[derive(Debug)] pub struct PostgresStore { @@ -82,10 +95,9 @@ impl PostgresStore { .connect(url) .await .map_err(|e| StorageError::Sqlx(e.into()))?; - // TODO: inline migrations if migrations { log_info!(logger, "Migrating postgres"); - sqlx::migrate!("src/storage/postgres/migrations") + MIGRATOR .run(&pool) .await .map_err(|e| StorageError::Sqlx(e.into()))?;