refactor(bdk): stabilize sqlx wallet db cache semantics#56
Conversation
Restructure SqlxWalletDb internals to isolate context, batch state, and shared caches while keeping BDK trait behavior compatible. This avoids eager cache hydration side effects, makes cache errors recoverable, and commits DB writes before cache mutation for safer sync behavior at large wallet scale.
Move keychain filtering into SQL and add a lightweight iter_txs(false) path to reduce wallet sync query cost. Replace panic-prone unimplemented/unwrap/expect paths in pg wallet database adapters with recoverable errors to avoid process crashes under inconsistent data.
Simplify control flow and batching code in the Postgres BDK adapters without changing behavior. This keeps recent sync performance optimizations while reducing duplication and making cache/DB paths easier to reason about.
Execute script pubkeys, utxos, and transactions persistence in a single SQL transaction during commit_batch to avoid partial DB state on failures. Also unify tx lookup behavior for raw vs summary reads, remove unused script_pubkeys load-all path, and document retained non-transactional persist helpers.
Avoid repeated full load_all deserialization in iter_txs(true) by memoizing the raw transaction cache after first successful load. Preserve cache correctness for staged updates and add focused unit coverage for the raw-cache-loaded flag.
There was a problem hiding this comment.
Pull request overview
Refactors the Postgres-backed SqlxWalletDb (BDK Database/BatchDatabase) implementation to make batch commits atomic, improve cache semantics, and harden DB adapters by removing panic-prone paths while also optimizing some read paths.
Changes:
- Make
commit_batchatomic by persisting script pubkeys, UTXOs, and transactions within a single SQL transaction and only updating in-memory caches after commit. - Add transactional persistence helpers (
persist_all_in_tx) and replaceunwrap/expectdeserialization with recoverable errors. - Optimize read paths by pushing script-pubkey filtering into SQL and adding a summary-only transaction load path (
load_all_summaries) plus memoization for raw tx loads.
Reviewed changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/bdk/pg/mod.rs |
Splits runtime context vs batch staging vs shared cache; makes commit_batch transactional and updates cache post-commit; adds summary/raw tx iteration behavior and cache tests. |
src/bdk/pg/script_pubkeys.rs |
Adds transactional persist API and improves query efficiency by using fetch_optional and SQL-level keychain filtering; removes load_all. |
src/bdk/pg/transactions.rs |
Adds transactional persist API, replaces panics with errors, introduces load_all_summaries to avoid loading raw txs when not needed. |
src/bdk/pg/utxos.rs |
Adds transactional persist API and replaces panics with errors during JSON (de)serialization and row parsing. |
.sqlx/query-dfa815aeb090f27d4fd45326c706b23f3f84c9e843addc1b05160fd0e28fdf2c.json |
Adds SQLx offline metadata for the new load_all_summaries query. |
.sqlx/query-90eecb0b6744da1d056aae9b32706840136afd1c44dbe358dd1d414f32d01cb4.json |
Updates SQLx offline metadata for the keychain-filtered script pubkey listing query. |
.sqlx/query-6c30dc4e7c409d797b957058f3f358ceeb651c8f971f92951b5e2d1e113b1286.json |
Removes SQLx offline metadata for the deleted load_all script-pubkeys query. |
.sqlx/query-561f73d1c3ae19876eef43a68dad50bedd904b0e8934a065bad8ef339b3d41eb.json |
Adds SQLx offline metadata for listing all script pubkeys without keychain filtering. |
.gitignore |
Ignores .bats-e2e artifacts produced by E2E Bats tests. |
.cargo/audit.toml |
Adds additional RustSec ignores with TODO notes for dependency remediation. |
Files not reviewed (3)
- .sqlx/query-561f73d1c3ae19876eef43a68dad50bedd904b0e8934a065bad8ef339b3d41eb.json: Language not supported
- .sqlx/query-6c30dc4e7c409d797b957058f3f358ceeb651c8f971f92951b5e2d1e113b1286.json: Language not supported
- .sqlx/query-dfa815aeb090f27d4fd45326c706b23f3f84c9e843addc1b05160fd0e28fdf2c.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add focused unit coverage for transaction cache behavior used by iter_txs summary/raw paths, and harden summary loading with negative-height validation. Also strengthen raw-cache-loaded atomic ordering with acquire/release semantics.
There was a problem hiding this comment.
Pull request overview
Refactors the BDK Postgres-backed SqlxWalletDb to make batch commits atomic and to stabilize/clarify cache semantics while improving read-path performance and removing panic-prone production code paths.
Changes:
- Split
SqlxWalletDbinto runtime context, per-batch staged state, and shared cache; makecommit_batchpersist SPKs/UTXOs/txs in a single SQL transaction and update caches after commit. - Optimize reads by pushing keychain filtering into SQL for script pubkey listing and adding a “summaries” transaction load path (
load_all_summaries). - Replace
unwrap/expect/unimplemented!in PG adapter paths with recoverablebdk::Error/BdkErrorreturns; update SQLx offline metadata accordingly.
Reviewed changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/bdk/pg/mod.rs |
Introduces WalletDbContext/WalletBatchState/WalletCache, atomic commit_batch, and new tx lookup / caching behavior. |
src/bdk/pg/script_pubkeys.rs |
Adds transactional persist helper and pushes keychain filtering into SQL for list_scripts; removes bulk load usage. |
src/bdk/pg/transactions.rs |
Adds transactional persist helper, safer JSON parsing, and new load_all_summaries query/path. |
src/bdk/pg/utxos.rs |
Adds transactional persist helper and replaces panic-based JSON handling with recoverable errors. |
.sqlx/query-*.json |
Updates/introduces SQLx offline query metadata for new/changed SQL queries. |
.gitignore |
Adds .bats-e2e to ignored paths. |
.cargo/audit.toml |
Expands ignored advisories list with TODO notes. |
Files not reviewed (3)
- .sqlx/query-561f73d1c3ae19876eef43a68dad50bedd904b0e8934a065bad8ef339b3d41eb.json: Language not supported
- .sqlx/query-6c30dc4e7c409d797b957058f3f358ceeb651c8f971f92951b5e2d1e113b1286.json: Language not supported
- .sqlx/query-dfa815aeb090f27d4fd45326c706b23f3f84c9e843addc1b05160fd0e28fdf2c.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reduce duplication and clarify control flow in the BDK Postgres adapters while preserving the recent atomic commit and cache semantics. This keeps behavior unchanged but makes error handling and query paths easier to reason about.
There was a problem hiding this comment.
Pull request overview
Refactors the BDK Postgres (sqlx) wallet DB adapter to make batch commits atomic and to stabilize/clarify in-memory cache behavior while improving sync-time read performance.
Changes:
- Split
SqlxWalletDbinto runtime context, per-batch staged state, and shared caches; makecommit_batchpersist within a single SQL transaction and update caches only after commit. - Optimize read paths by filtering script pubkeys in SQL and adding a “transaction summaries” load path for
iter_txs(false). - Replace panic-prone JSON (de)serialization / parsing paths with recoverable
bdk::Errors; update SQLx offline metadata accordingly.
Reviewed changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/bdk/pg/mod.rs | Refactors wallet DB internals (context/batch/cache), makes commit_batch atomic, adds cache-focused unit tests. |
| src/bdk/pg/script_pubkeys.rs | Pushes optional keychain filtering into SQL; adds transactional persistence helper. |
| src/bdk/pg/transactions.rs | Adds summary-loading path and transactional persistence; removes panics in deserialization/parsing. |
| src/bdk/pg/utxos.rs | Adds transactional persistence and replaces unwrap/expect with recoverable errors. |
| .sqlx/query-*.json | Updates/extends SQLx offline query metadata for the new/changed queries. |
| .gitignore | Adds .bats-e2e (but currently includes an unintended - entry). |
| .cargo/audit.toml | Expands advisory ignore list with TODO context for transitive deps. |
Files not reviewed (3)
- .sqlx/query-561f73d1c3ae19876eef43a68dad50bedd904b0e8934a065bad8ef339b3d41eb.json: Language not supported
- .sqlx/query-6c30dc4e7c409d797b957058f3f358ceeb651c8f971f92951b5e2d1e113b1286.json: Language not supported
- .sqlx/query-dfa815aeb090f27d4fd45326c706b23f3f84c9e843addc1b05160fd0e28fdf2c.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Respect the Database::get_tx include_raw flag by stripping raw transaction data when requested. Also document the process-local raw tx cache hint and clarify commit_batch atomic scope boundaries.
Summary
SqlxWalletDbinternals to separate runtime context, staged batch state, and shared cache state while keeping BDK trait behavior compatiblecommit_batchatomic by persisting script pubkeys, utxos, and transactions inside a single SQL transaction, then updating caches only after commit succeedsbdk.script_pubkeys.list_scripts, adding a lightweightiter_txs(false)summaries path, and memoizingiter_txs(true)raw loads after the first successful fetchunimplemented!/unwrap/expectproduction paths with recoverable errorsmod.rs,script_pubkeys.rs,transactions.rs,utxos.rs), remove unused script-pubkeyload_all, and clarify lookup/persist helper intent with inline comments