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/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/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/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 1922e4bc..ab7326eb 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; @@ -55,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 { @@ -81,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()))?; @@ -226,6 +239,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 +291,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)?; @@ -417,15 +435,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)?; @@ -479,98 +497,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)?; @@ -739,8 +719,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) @@ -759,11 +742,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(()) } @@ -776,8 +762,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) @@ -797,16 +786,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()) { @@ -816,6 +809,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 @@ -881,13 +880,47 @@ 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, + // 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 { @@ -922,6 +955,63 @@ 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( @@ -957,7 +1047,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()) @@ -1005,7 +1095,7 @@ struct KeychainEntry { keychainkind: String, descriptor: String, descriptor_id: Vec, - last_revealed: i32, + last_revealed: Option, } #[cfg(test)] @@ -1077,6 +1167,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; @@ -1087,4 +1186,225 @@ mod tests { let contracts = db.get_contracts().await.unwrap(); 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 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(); + + // 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)); + 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; + + 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 = dummy_tx(); + 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 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; + + 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; + + 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; + + 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()); + } } 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={:?}", 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_*`).