Don't offset nonces yet negate to achieve an even Y coordinate

Replaces an iterative loop with an immediate result, if action is necessary.

Cargo.lock

@@ -1075,6 +1075,7 @@ dependencies = [
  "serde_json",
  "simple-request",
  "std-shims",
+ "subtle",
  "thiserror 1.0.64",
  "tokio",
  "zeroize",

crypto/frost/src/algorithm.rs

@@ -135,6 +135,8 @@ pub trait Hram<C: Curve>: Send + Sync + Clone {
 }
 
 /// Schnorr signature algorithm ((R, s) where s = r + cx).
+///
+/// `verify`, `verify_share` must be called after `sign_share` is called.
 #[derive(Clone)]
 pub struct Schnorr<C: Curve, T: Sync + Clone + Debug + Transcript, H: Hram<C>> {
   transcript: T,

networks/bitcoin/Cargo.toml

@@ -20,6 +20,7 @@ std-shims = { version = "0.1.1", path = "../../common/std-shims", default-featur
 
 thiserror = { version = "1", default-features = false, optional = true }
 
+subtle = { version = "2", default-features = false }
 zeroize = { version = "^1.5", default-features = false }
 rand_core = { version = "0.6", default-features = false }
 
@@ -46,6 +47,7 @@ std = [
 
   "thiserror",
 
+  "subtle/std",
   "zeroize/std",
   "rand_core/std",
 

networks/bitcoin/src/crypto.rs

@@ -1,3 +1,5 @@
+use subtle::{Choice, ConstantTimeEq, ConditionallySelectable};
+
 use k256::{
   elliptic_curve::sec1::{Tag, ToEncodedPoint},
   ProjectivePoint,
@@ -17,17 +19,9 @@ pub fn x_only(key: &ProjectivePoint) -> XOnlyPublicKey {
   XOnlyPublicKey::from_slice(&x(key)).expect("x_only was passed a point which was infinity or odd")
 }
 
-/// Make a point even by adding the generator until it is even.
+/// Return if a point must be negated to have an even Y coordinate and be eligible for use.
-///
+pub(crate) fn needs_negation(key: &ProjectivePoint) -> Choice {
-/// Returns the even point and the amount of additions required.
+  u8::from(key.to_encoded_point(true).tag()).ct_eq(&u8::from(Tag::CompressedOddY))
-#[cfg(any(feature = "std", feature = "hazmat"))]
-pub fn make_even(mut key: ProjectivePoint) -> (ProjectivePoint, u64) {
-  let mut c = 0;
-  while key.to_encoded_point(true).tag() == Tag::CompressedOddY {
-    key += ProjectivePoint::GENERATOR;
-    c += 1;
-  }
-  (key, c)
 }
 
 #[cfg(feature = "std")]
@@ -60,25 +54,27 @@ mod frost_crypto {
   #[allow(non_snake_case)]
   impl HramTrait<Secp256k1> for Hram {
     fn hram(R: &ProjectivePoint, A: &ProjectivePoint, m: &[u8]) -> Scalar {
-      // Convert the nonce to be even
-      let (R, _) = make_even(*R);
-
       const TAG_HASH: Sha256 = Sha256::const_hash(b"BIP0340/challenge");
 
       let mut data = Sha256::engine();
       data.input(TAG_HASH.as_ref());
       data.input(TAG_HASH.as_ref());
-      data.input(&x(&R));
+      data.input(&x(R));
       data.input(&x(A));
       data.input(m);
 
-      Scalar::reduce(U256::from_be_slice(Sha256::from_engine(data).as_ref()))
+      let c = Scalar::reduce(U256::from_be_slice(Sha256::from_engine(data).as_ref()));
+      // If the nonce was odd, sign `r - cx` instead of `r + cx`, allowing us to negate `s` at the
+      // end to sign as `-r + cx`
+      <_>::conditional_select(&c, &-c, needs_negation(R))
     }
   }
 
   /// BIP-340 Schnorr signature algorithm.
   ///
-  /// This must be used with a ThresholdKeys whose group key is even. If it is odd, this will panic.
+  /// This must be used with a ThresholdKeys whose group key is even. If it is odd, this may panic.
+  ///
+  /// `verify`, `verify_share` must be called after `sign_share` is called.
   #[derive(Clone)]
   pub struct Schnorr(FrostSchnorr<Secp256k1, Hram>);
   impl Schnorr {
@@ -141,11 +137,7 @@ mod frost_crypto {
       sum: Scalar,
     ) -> Option<Self::Signature> {
       self.0.verify(group_key, nonces, sum).map(|mut sig| {
-        // Make the R of the final signature even
+        sig.s = <_>::conditional_select(&sum, &-sum, needs_negation(&sig.R));
-        let offset;
-        (sig.R, offset) = make_even(sig.R);
-        // s = r + cx. Since we added to the r, add to s
-        sig.s += Scalar::from(offset);
         // Convert to a Bitcoin signature by dropping the byte for the point's sign bit
         sig.serialize()[1 ..].try_into().unwrap()
       })

networks/bitcoin/src/tests/crypto.rs

@@ -2,7 +2,6 @@ use rand_core::OsRng;
 
 use secp256k1::{Secp256k1 as BContext, Message, schnorr::Signature};
 
-use k256::Scalar;
 use frost::{
   curve::Secp256k1,
   Participant,
@@ -11,7 +10,8 @@ use frost::{
 
 use crate::{
   bitcoin::hashes::{Hash as HashTrait, sha256::Hash},
-  crypto::{x_only, make_even, Schnorr},
+  crypto::{x_only, Schnorr},
+  wallet::tweak_keys,
 };
 
 #[test]
@@ -20,8 +20,7 @@ fn test_algorithm() {
   const MESSAGE: &[u8] = b"Hello, World!";
 
   for keys in keys.values_mut() {
-    let (_, offset) = make_even(keys.group_key());
+    *keys = tweak_keys(keys.clone());
-    *keys = keys.offset(Scalar::from(offset));
   }
 
   let algo = Schnorr::new();

networks/bitcoin/src/wallet/mod.rs

@@ -26,7 +26,7 @@ use bitcoin::{hashes::Hash, consensus::encode::Decodable, TapTweakHash};
 
 use crate::crypto::x_only;
 #[cfg(feature = "std")]
-use crate::crypto::make_even;
+use crate::crypto::needs_negation;
 
 #[cfg(feature = "std")]
 mod send;
@@ -43,7 +43,7 @@ pub use send::*;
 /// existence of the unspendable script path may not provable, without an understanding of the
 /// algorithm used here.
 #[cfg(feature = "std")]
-pub fn tweak_keys(keys: &ThresholdKeys<Secp256k1>) -> ThresholdKeys<Secp256k1> {
+pub fn tweak_keys(keys: ThresholdKeys<Secp256k1>) -> ThresholdKeys<Secp256k1> {
   // Adds the unspendable script path per
   // https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#cite_note-23
   let keys = {
@@ -64,11 +64,14 @@ pub fn tweak_keys(keys: &ThresholdKeys<Secp256k1>) -> ThresholdKeys<Secp256k1> {
     )))
   };
 
-  // This doesn't risk re-introducing a script path as you'd have to find a preimage for the tweak
+  let needs_negation = needs_negation(&keys.group_key());
-  // hash with whatever increment, or manipulate the key so that the tweak hash and increment
+  keys
-  // equals the desired offset, yet manipulating the key would change the tweak hash
+    .scale(<_ as subtle::ConditionallySelectable>::conditional_select(
-  let (_, offset) = make_even(keys.group_key());
+      &Scalar::ONE,
-  keys.offset(Scalar::from(offset))
+      &-Scalar::ONE,
+      needs_negation,
+    ))
+    .expect("scaling keys by 1 or -1 yet interpreted as 0?")
 }
 
 /// Return the Taproot address payload for a public key.

networks/bitcoin/tests/wallet.rs

@@ -80,7 +80,7 @@ async fn send_and_get_output(rpc: &Rpc, scanner: &Scanner, key: ProjectivePoint)
 fn keys() -> (HashMap<Participant, ThresholdKeys<Secp256k1>>, ProjectivePoint) {
   let mut keys = key_gen(&mut OsRng);
   for keys in keys.values_mut() {
-    *keys = tweak_keys(keys);
+    *keys = tweak_keys(keys.clone());
   }
   let key = keys.values().next().unwrap().group_key();
   (keys, key)