Skip to content

Conversation

@notmandatory
Copy link
Member

Description

Add to rusqlite module the get_pre_1_wallet_keychains migration to help migrate users from a pre-1.0 bdk sqlite database. This new function returns the last revealed index and checksum value for each keychain it finds.

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK. I don't have an opinion as to where it should live per se, although this is mostly related to a pre-1.0 bdk Wallet database, so maybe in bdk_wallet would be closer to its target users? Works either way!

I tested with a local database and I get correct values. Only small comment would be that the string it pulls for the KeychainKind has escaped double quote characters and those are probably not really wanted (see example below).

Pre1WalletKeychain { keychain: "\"External\"", last_derivation_index: 3, checksum: [108, 55, 54, 100, 117, 121, 118, 55] }

@notmandatory
Copy link
Member Author

ah good catch, I didn't notice that with the double quotes, will fix.

@notmandatory notmandatory force-pushed the feat/sqlite_pre1_migration_helper branch from 9d0bd21 to fde4e87 Compare January 8, 2026 01:55
@oleonardolima oleonardolima self-requested a review January 8, 2026 22:37
Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK fde4e87

Are you looking forward to keep these changes here or move it to bdk_wallet ? I think it'd better live in bdk_wallet.

@notmandatory
Copy link
Member Author

Thanks for the reminder, I'm closing this one, replaced by bitcoindevkit/bdk_wallet#364 and bitcoindevkit/bdk_wallet#365.

@notmandatory notmandatory self-assigned this Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants