-
Notifications
You must be signed in to change notification settings - Fork 11
Bdkwallet2 #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Bdkwallet2 #21
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Introduces schema versioning and SPK cache support for Postgres, renames backup helpers, and updates examples and tooling for regtest workflows.
- Adds migrations and runtime logic to handle new
last_evicted/first_seencolumns and akeychain_spktable in Postgres. - Renames
easy_backupto_easy_backupin SQLite and Postgres modules to suppress unused warnings. - Updates the Electrum example, Dockerfiles, and Justfile for streamlined setup and regtest execution.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/sqlite.rs | Changed public easy_backup to _easy_backup |
| src/postgres.rs | Implemented schema version checks, added migrations, and SPK load/persist logic |
| migrations/postgres/01_bdk_wallet.sql | Added UNIQUE constraints, new keychain_spk table, and last_* columns |
| examples/bdk_sqlx_postgres.rs | Simplified full_scan usage, added anyhow::Context, updated imports |
| electrs.toml, electrs.Dockerfile | Electrs configuration and Docker setup |
| bitcoin.Dockerfile | Bitcoin Core Dockerfile for regtest |
| Justfile | Tasks for network setup, containers, and example execution |
| Cargo.toml | Bumped bdk_wallet to 2.0.0, moved bdk_electrum to dev-dependencies |
Comments suppressed due to low confidence (6)
src/sqlite.rs:474
- Renaming the public function
easy_backupto_easy_backupbreaks the public API and can confuse users. Consider making it private (removepub) or use#[doc(hidden)]if you intend to hide it from documentation.
pub async fn _easy_backup(db: Pool<Sqlite>) -> Result<(), BdkSqlxError> {
src/postgres.rs:627
- Using
println!for diagnostics in library code will clutter stdout and bypass structured logging. Switch to atracing::debug!or similar logging macro for consistency.
println!("load_keychain_spks: Starting for wallet '{}', descriptor_id: {:?}", wallet_name, descriptor_id);
src/postgres.rs:337
- [nitpick] The
keychain_spkmigration is duplicated in both the v1→v2 and v2→v3 branches. Refactor into a single conditional or rely onIF NOT EXISTSin one place to reduce code duplication.
sqlx::query(r#"CREATE TABLE IF NOT EXISTS "bdk_wallet"."keychain_spk" ... )
examples/bdk_sqlx_postgres.rs:129
STOP_GAPandBATCH_SIZEare not imported or defined in this scope, resulting in a compile error. Import these constants or replace them (e.g., useDEFAULT_LOOKAHEAD).
let res = client.full_scan::<_>(request, STOP_GAP, BATCH_SIZE, true)
src/postgres.rs:518
- The new SPK cache persist/load logic is critical and currently untested. Add unit or integration tests covering
load_keychain_spksandpersist_keychain_spksto ensure correct behavior.
// Persist SPK cache
examples/bdk_sqlx_postgres.rs:7
- [nitpick]
DEFAULT_LOOKAHEADis imported but never used. Either remove the unused import or leverage it in place of the undefinedSTOP_GAPconstant.
use bdk_wallet::bitcoin::{constants, Network};
No description provided.