From 6fad20ef0ce9adf85969db5cc714d3721efa9932 Mon Sep 17 00:00:00 2001
From: Tobin Harding <me@tobin.cc>
Date: Wed, 19 Jan 2022 16:20:07 +1100
Subject: [PATCH] Fix the mess around Parity

Recently we made a wee mess with the `Parity` opaque type. Let's fix it
up by doing:

- Use an enum with variants `Even` and `Odd`.
- Add explicit conversion methods to/from u8 and i32
- Implement `BitXor`

Note: This patch is an API breaking change that does _not_ follow the
deprecation guidelines. Rust does not allow deprecating `From` impl
blocks AFAICT.
---
 src/key.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++++-------
 src/lib.rs |  3 +++
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/src/key.rs b/src/key.rs
index 1336bd7..5efb2ff 100644
--- a/src/key.rs
+++ b/src/key.rs
@@ -18,6 +18,7 @@
 #[cfg(any(test, feature = "rand"))] use rand::Rng;
 
 use core::{fmt, ptr, str};
+use core::ops::BitXor;
 
 use super::{from_hex, Secp256k1};
 use super::Error::{self, InvalidPublicKey, InvalidPublicKeySum, InvalidSecretKey};
@@ -890,7 +891,7 @@ impl XOnlyPublicKey {
                 return Err(Error::InvalidPublicKey);
             }
 
-            Ok(parity.into())
+            Parity::from_i32(parity)
         }
     }
 
@@ -918,7 +919,7 @@ impl XOnlyPublicKey {
             let err = ffi::secp256k1_xonly_pubkey_tweak_add_check(
                 secp.ctx,
                 tweaked_ser.as_c_ptr(),
-                tweaked_parity.into(),
+                tweaked_parity.to_i32(),
                 &self.0,
                 tweak.as_c_ptr(),
             );
@@ -928,19 +929,69 @@ impl XOnlyPublicKey {
     }
 }
 
-/// Opaque type used to hold the parity passed between FFI function calls.
+/// Represents the parity passed between FFI function calls.
 #[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, Hash)]
-pub struct Parity(i32);
+pub enum Parity {
+    /// Even parity.
+    Even = 0,
+    /// Odd parity.
+    Odd = 1,
+}
+
+impl Parity {
+    /// Converts parity into a integer (byte) value.
+    pub fn to_u8(self) -> u8 {
+        self as u8
+    }
+
+    /// Converts parity into a integer value.
+    pub fn to_i32(self) -> i32 {
+        self as i32
+    }
+
+    /// Constructs a [`Parity`] from a byte.
+    pub fn from_u8(parity: u8) -> Result<Parity, Error> {
+        Parity::from_i32(parity as i32)
+    }
+
+    /// Constructs a [`Parity`] from a signed integer.
+    pub fn from_i32(parity: i32) -> Result<Parity, Error> {
+        match parity {
+            0 => Ok(Parity::Even),
+            1 => Ok(Parity::Odd),
+            _ => Err(Error::InvalidParityValue),
+        }
+    }
+}
 
 impl From<i32> for Parity {
+    /// Please note, this method is deprecated and will be removed in an upcoming release, it
+    /// is not equivalent to `from_u32()`, it is better to use `Parity::from_u32`.
     fn from(parity: i32) -> Parity {
-        Parity(parity)
+        if parity % 2 == 0 {
+            Parity::Even
+        } else {
+            Parity::Odd
+        }
     }
 }
 
 impl From<Parity> for i32 {
     fn from(parity: Parity) -> i32 {
-        parity.0
+        parity.to_i32()
+    }
+}
+
+impl BitXor for Parity {
+    type Output = Parity;
+
+    fn bitxor(self, rhs: Parity) -> Self::Output {
+        // This works because Parity has only two values (i.e. only 1 bit of information).
+        if self == rhs {
+            Parity::Even        // 1^1==0 and 0^0==0
+        } else {
+            Parity::Odd         // 1^0==1 and 0^1==1
+        }
     }
 }
 
@@ -948,7 +999,7 @@ impl From<Parity> for i32 {
 #[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
 impl ::serde::Serialize for Parity {
     fn serialize<S: ::serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
-        s.serialize_i32(self.0)
+        s.serialize_i32(self.to_i32())
     }
 }
 
@@ -969,7 +1020,7 @@ impl<'de> ::serde::Deserialize<'de> for Parity {
             fn visit_i32<E>(self, v: i32) -> Result<Self::Value, E>
                 where E: ::serde::de::Error
             {
-                Ok(Parity::from(v))
+                Parity::from_i32(v).map_err(E::custom)
             }
         }
 
diff --git a/src/lib.rs b/src/lib.rs
index e552add..4220567 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -322,6 +322,8 @@ pub enum Error {
     NotEnoughMemory,
     /// Bad set of public keys
     InvalidPublicKeySum,
+    /// The only valid parity values are 0 or 1.
+    InvalidParityValue,
 }
 
 impl Error {
@@ -336,6 +338,7 @@ impl Error {
             Error::InvalidTweak => "secp: bad tweak",
             Error::NotEnoughMemory => "secp: not enough memory allocated",
             Error::InvalidPublicKeySum => "secp: the sum of public keys was invalid or the input vector lengths was less than 1",
+            Error::InvalidParityValue => "The only valid parity values are 0 or 1",
         }
     }
 }