feat(crypto): CRP-2591 Add BIP-341 sig support to secp256k1 utility crate #2756
Conversation
There was a problem hiding this comment.
Thanks for implementing BIP341 @randombit , nice to see it working!
I have a few minor, mostly cosmetic comments.
| @@ -330,6 +340,31 @@ fn pem_encode(raw: &[u8], label: &'static str) -> String { | |||
| }) | |||
| } | |||
|
|
|||
| fn bip341_hash(pk_x: &[u8], ttr: &[u8]) -> Result<Scalar, InvalidTaprootHash> { | |||
There was a problem hiding this comment.
It could be useful to add descriptions for the added functions, e.g., what ttr is or that a BIP341 hash is actually the tweak. Maybe we should actually call it bip341_tweak. We could also add the assumptions that pk_x and ttr have a valid length or add debug_asserts.
|
|
||
| let t = k256::ProjectivePoint::mul_by_generator(&bip341_hash(&pk[1..], ttr)?); | ||
| let pk_y_is_even = pk[0] == 0x02; | ||
| println!("y_is_even {}", pk_y_is_even); |
| assert!(pk.verify_bip340_signature(&msg, &sig)); | ||
|
|
||
| for ttr_len in [0, 32] { |
There was a problem hiding this comment.
Hmm, shouldn't this be another test? Something like should_accept_bip341_signatures_that_we_generate?
|
|
||
| let pk = XOnlyPublicKey::from_slice(&sk.public_key().serialize_sec1(true)[1..]).unwrap(); | ||
|
|
||
| let tnh = TapBranchHash::from_hex(&hex::encode(ttr)).unwrap(); |
|
|
||
| let tnh = TapBranchHash::from_hex(&hex::encode(ttr)).unwrap(); | ||
|
|
||
| let dk = pk.tap_tweak(&secp256k1, Some(tnh)).0.to_inner(); |
There was a problem hiding this comment.
Does dk mean derived key? I think the BIPs always speak about this as tweaking. Maybe we could use that instead of "derive" in general.
| @@ -837,6 +907,44 @@ impl PublicKey { | |||
| } | |||
| } | |||
|
|
|||
| /// BIP341 derivation | |||
| fn derive_bip341(&self, ttr: &[u8]) -> Result<Self, InvalidTaprootHash> { | |||
There was a problem hiding this comment.
We could also check in unit tests that functions produce correct results against third-party libraries. But not necessarily in this PR.
There was a problem hiding this comment.
Differential fuzzers might also make sense
No description provided.