Feature/bip322 integration

[aagbotemi]

Description

This PR added BIP322 feature into BDK CLI.
It also has a usage file for testing purposes

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

New Features:

• I've added docs for the new feature
• I've updated CHANGELOG.md

[notmandatory]

I'd rather not have a separate key file for this feature and try to protect it with a password and all that. This bdk-cli tool is primarily meant as a manual testing tool and example of how to use the bdk_wallet APIs so providing the public or private key descriptor via command line or environment var should also work for this signing feature.

More importantly the use of bip322 as I understand it is to be able to sign a message using what ever bitcoin descriptor script your bitcoin wallet has. If you have a private key descriptor you should be able to sign the invalid input message transaction, and if not you should still be able to create a PSBT that your hardware signers can sign. Does that make sense? Also have you seen the bdk-reserves crate? it does something similar.

[aagbotemi]

I'd rather not have a separate key file for this feature and try to protect it with a password and all that.

Still actively working on this. Will push updates once I'm done (likely within this week).

[tvpeter]

Hi @aagbotemi, can you rebase and fix the CI failures?

[tvpeter]

This PR is similar to the reserves feature that was taken out in the v1.0 update.
@aagbotemi, please consider changing bip322 feature flag to reserves.

Also, after rebasing, check whether there is need to update the workflows. I would prefer the workflows to go in a separate PR if necessary.

@notmandatory I don't know whether it is ideal that the crate he is referencing should be published for security reasons.

[aagbotemi]

@aagbotemi, please consider changing bip322 feature flag to reserves.

Alright @tvpeter, will do that.

@notmandatory I don't know whether it is ideal that the crate he is referencing should be published for security reasons.

Yes, I'm revamping the crate to use descriptor instead of private key, but I will continue the PR with the reserves crate.

[aagbotemi]

Thanks for the review and suggestion to rename the bip322 feature to reserves @tvpeter. I’m blocked on integrating bdk-reserves into this PR due to a version mismatch: bdk-cli uses bdk_wallet 2.0.0, but bdk-reserves uses another bdk_wallet version as the ProofOfReserves trait in bdk-reserves is implemented for it.

A PR (bitcoindevkit/bdk-reserves#39) updates bdk-reserves to support bdk_wallet 1.2.0. Once merged, we can bump bdk-reserves to use bdk_wallet 2.0.0.

[aagbotemi]

The update is a follow-up to the BIP322 refactor that migrated signing from raw keys to descriptor-based

[coveralls]

Pull Request Test Coverage Report for Build 21173945658

Details

• 0 of 40 (0.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.2%) to 10.549%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/error.rs	0	3	0.0%
src/utils.rs	0	11	0.0%
src/handlers.rs	0	26	0.0%

Totals
Change from base Build 21153868360:	-0.2%
Covered Lines:	269
Relevant Lines:	2550

---

💛 - Coveralls

[tvpeter]

Thank you for working on this @aagbotemi

I have left some comments.

[aagbotemi]

Thank you for working on this @aagbotemi
I have left some comments.

Thank you for the review. I'll attend to the comments shortly.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 10.69%. Comparing base (a675935) to head (5184d45).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/handlers.rs	0.00%	29 Missing ⚠️
src/utils.rs	0.00%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
- Coverage   10.86%   10.69%   -0.18%     
==========================================
  Files           8        8              
  Lines        2466     2506      +40     
==========================================
  Hits          268      268              
- Misses       2198     2238      +40     

Flag	Coverage Δ
rust	10.69% <0.00%> (-0.18%)	⬇️

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.

[tvpeter]

Hi @aagbotemi ,

So I was testing the feature, and some tests failed. In the attached screenshot, I generated an address from a wallet, signed a message, and verified the message. I changed the message, and it still came back true. I thought the expected output would have been an error indicating that verification failed

[notmandatory]

Some good hardware signing new on this topic. The latest ColdCard firmware supports BIP-322, see: https://nitter.net/COLDCARDwallet/status/2029684130938531965

[aagbotemi]

I changed the message, and it still came back true. I thought the expected output would have been an error indicating that verification failed

Thanks for pointing this out. I’ve fixed the verification logic so that verification now correctly fails when the message is tampered with.

Also, I noticed the feature was referenced as reserve during testing, while the current feature name is bip322. Would you prefer that we rename it to reserve?

[aagbotemi]

Some good hardware signing new on this topic. The latest ColdCard firmware supports BIP-322, see: https://nitter.net/COLDCARDwallet/status/2029684130938531965

Thanks for the heads-up! Just checked the ColdCard firmware announcement, it's awesome to see the BIP-322 support.

[tvpeter]

Also, I noticed the feature was referenced as reserve during testing, while the current feature name is bip322. Would you prefer that we rename it to reserve?

No, it is the wallet I used during testing that I named reserve.

[tvpeter]

tACK 5184d45

Weldone @aagbotemi

This is a great feature, and thanks for working on it.

[aagbotemi]

UPDATE: Removed signature_type from verify_message() to make the verification API more ergonomic. The signature format is now auto-detected from the signature bytes. Callers no longer need to know which format produced a signature to verify it, and a verifier should only need the signature, message, and address. SignatureFormat is still used for signing.