fix(rusqlite): fix get_pre_1_wallet_keychains migration helper

[notmandatory]

Description

While working on bitcoindevkit/book-of-bdk#141 I found some very rough edges with how I'd implemented the Pre1WalletKeychain struct. I've fixed these and added a new Pre1MigrationError type to cover new possible errors when parsing keychain name and checksum bytes.

Notes to the reviewers

• rename public function to get_pre_v1_wallet_keychains
• rename returned structure to PreV1WalletKeychain
• added PreV1MigrationError
• changed PreV1WalletKeychain::keychain to KeychainKind
• changed PreV1WalletKeychain::checksum to String

Changelog notice

Replace changelog notice from #364 to:

Add get_pre_v1_wallet_keychains to assist migration from pre-1.0 bdk wallets.

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran just p before pushing

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

[codecov]

Codecov Report

❌ Patch coverage is 89.53488% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.04%. Comparing base (fca6523) to head (c730973).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
src/wallet/migration.rs	89.53%	8 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
+ Coverage   79.77%   80.04%   +0.26%     
==========================================
  Files          24       24              
  Lines        5266     5336      +70     
  Branches      241      242       +1     
==========================================
+ Hits         4201     4271      +70     
+ Misses        988      987       -1     
- Partials       77       78       +1     

Flag	Coverage Δ
rust	80.04% <89.53%> (+0.26%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[notmandatory]

I'd like to get this into the 3.0 release and then I'll back port the patch to the release/2.x branch.

[notmandatory]

WIP update to example usage for book-of-bdk is here: https://github.com/notmandatory/book-of-bdk/blob/838a1b099fe378decb4909da356115db13435413/examples/rust/migrate-version/src/main.rs

See: bitcoindevkit/book-of-bdk#147

[notmandatory]

I re-pushed my commit with refactored and expanded test coverage, compliments of 🤖 LLM helper.

[oleonardolima]

utACK 849f88f

Overall looks good! I'll try to test it following bitcoindevkit/book-of-bdk#147

[ValuedMammal]

You're using a lot of cfg attributes for the same reason. Is it possible to refactor those to apply to the module instead?

[oleonardolima]

@notmandatory I updated it to apply the rusqlite feature configuration to the module, and also renamed it from Pre1... to PreV1... instead. If you'd like you can cherry-pick those from here: https://github.com/oleonardolima/bdk_wallet/commits/fix/sqlite_pre1_migration_helper/

[notmandatory]

Thanks @oleonardolima I cherry-picked your commits, these are good improvements addressing @ValuedMammal's comment.

[notmandatory]

I added #415 as a back port of this PR to the release/2.x branch.

[ValuedMammal]

In 849f88f: fix(rusqlite): fix get_pre_1_wallet_keychains migration helper

Can you include a newline before the body of the commit message?

[oleonardolima]

tACK a399408

I've tested it by using bdk-cli v0.27.1 importing and syncing the same testnet4 descriptors we have in examples.

I used the code from bitcoindevkit/book-of-bdk#147 and it created the new database as expected, and I did a sync with it's initial state.

[oleonardolima]

nit: it could probably be squashed into a single commit, though it's not a blocker.

[notmandatory]

I'll squash it.

[oleonardolima]

tACK c730973

I ran the test on the new commit.

[ValuedMammal]

ACK c730973