bdk icon indicating copy to clipboard operation
bdk copied to clipboard

Published 20 hours ago • verified publisher icon bitcoindevkit

Add message signer & message signature verifier

Open q-src opened this issue 3 years ago • 4 comments

Description

Ths PR adds the possibility to

  • sign an arbitrary message
  • verify a given message signature

using ECDSA.

Checklists

All Submissions:

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

New Features:

  • [x] I've added tests for the new feature
  • [x] I've added docs for the new feature
  • [x] I've updated CHANGELOG.md

q-src avatar May 06 '22 09:05 q-src

Thanks for the review. The additions in this PR are intentionally disconnected from the rest of BDK for single responsibility reasons but also because I was not sure if / how you want it to be connected - especially regarding multi-sig wallets and other signing algorithms.

So I chose to add the traits MessageSigner and MessageSignatureVerifier. With this I wanted to prevent that the wallet struct has to be extended with another method for each signing scheme / algorithm which is added (e.g. sign_msg_ecdsa, sign_msg_schnorr,…). Maybe this is just me having too much of a non-rust way of thinking? Nevertheless, this is a design decision which I think should be made by the maintainers of BDK.

I intended to join your dev call yesterday to discuss this. However, I didn’t manage to attend due to a change in my schedule…

q-src avatar May 11 '22 16:05 q-src

Hey @q-src. I noticed that you didn't implement BIP-137 or BIP-322 are they not applicable to what you want to do?

It's not clear this functionality needs a trait around it in BDK. How did you imagine using these traits?

LLFourn avatar May 16 '22 06:05 LLFourn

@LLFourn I was not aware of BIP-322 and therefore did not implement it. However, it looks very much like what I want to achieve.

Apparently, BIP-322 is being added to rust-miniscript (https://github.com/rust-bitcoin/rust-miniscript/pull/444). Shall we wait until this is done and then use the miniscript implementation?

q-src avatar Aug 17 '22 14:08 q-src

@q-src TBH miniscript doesn't sound like the right place either. It might make sense to have a BIP322 implementation in bdk if it is supported somehow by the existing components of the library. I'm not sure if it is though.

LLFourn avatar Aug 17 '22 14:08 LLFourn