From 5acf6d23d3636d88d628cb53306b37a8af81b133 Mon Sep 17 00:00:00 2001
From: Martin Habovstiak <martin.habovstiak@gmail.com>
Date: Thu, 10 Feb 2022 23:50:02 +0100
Subject: [PATCH] `Parity` conversion and error handling cleanup

This removes the deprecated `From` conversion and adds a new error
type for the invalid parity error with a conversion to the catch-all
`Error`.
---
 src/key.rs | 53 +++++++++++++++++++++++++++++++++++------------------
 src/lib.rs | 22 +++++++++++++++++++---
 2 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/src/key.rs b/src/key.rs
index 1d66ab4cc..ad552dafc 100644
--- a/src/key.rs
+++ b/src/key.rs
@@ -1134,7 +1134,7 @@ impl XOnlyPublicKey {
                 return Err(Error::InvalidPublicKey);
             }
 
-            Parity::from_i32(parity)
+            Parity::from_i32(parity).map_err(Into::into)
         }
     }
 
@@ -1219,7 +1219,7 @@ impl Parity {
     ///
     /// The only allowed values are `0` meaning even parity and `1` meaning odd.
     /// Other values result in error being returned.
-    pub fn from_u8(parity: u8) -> Result<Parity, Error> {
+    pub fn from_u8(parity: u8) -> Result<Parity, InvalidParityValue> {
         Parity::from_i32(parity.into())
     }
 
@@ -1227,25 +1227,11 @@ impl Parity {
     ///
     /// The only allowed values are `0` meaning even parity and `1` meaning odd.
     /// Other values result in error being returned.
-    pub fn from_i32(parity: i32) -> Result<Parity, Error> {
+    pub fn from_i32(parity: i32) -> Result<Parity, InvalidParityValue> {
         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`.
-    ///
-    /// This method returns same parity as the parity of input integer.
-    fn from(parity: i32) -> Parity {
-        if parity % 2 == 0 {
-            Parity::Even
-        } else {
-            Parity::Odd
+            _ => Err(InvalidParityValue(parity)),
         }
     }
 }
@@ -1257,6 +1243,13 @@ impl From<Parity> for i32 {
     }
 }
 
+/// The conversion returns `0` for even parity and `1` for odd.
+impl From<Parity> for u8 {
+    fn from(parity: Parity) -> u8 {
+        parity.to_u8()
+    }
+}
+
 /// Returns even parity if the operands are equal, odd otherwise.
 impl BitXor for Parity {
     type Output = Parity;
@@ -1271,6 +1264,30 @@ impl BitXor for Parity {
     }
 }
 
+/// Error returned when conversion from an integer to `Parity` fails.
+//
+// Note that we don't allow inspecting the value because we may change the type.
+// Yes, this comment is intentionally NOT doc comment.
+// Too many derives for compatibility with current Error type.
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
+pub struct InvalidParityValue(i32);
+
+impl fmt::Display for InvalidParityValue {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "invalid value {} for Parity - must be 0 or 1", self.0)
+    }
+}
+
+#[cfg(feature = "std")]
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl ::std::error::Error for InvalidParityValue {}
+
+impl From<InvalidParityValue> for Error {
+    fn from(error: InvalidParityValue) -> Self {
+        Error::InvalidParityValue(error)
+    }
+}
+
 /// The parity is serialized as `u8` - `0` for even, `1` for odd.
 #[cfg(feature = "serde")]
 #[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
diff --git a/src/lib.rs b/src/lib.rs
index 93e1c1ab4..3dc1d7f93 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -361,7 +361,7 @@ pub enum Error {
     /// Bad set of public keys.
     InvalidPublicKeySum,
     /// The only valid parity values are 0 or 1.
-    InvalidParityValue,
+    InvalidParityValue(key::InvalidParityValue),
 }
 
 impl Error {
@@ -376,7 +376,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",
+            Error::InvalidParityValue(_) => "couldn't create parity",
         }
     }
 }
@@ -390,7 +390,23 @@ impl fmt::Display for Error {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl std::error::Error for Error {}
+impl std::error::Error for Error {
+    #[allow(deprecated)]
+    fn cause(&self) -> Option<&dyn std::error::Error> {
+        match self {
+            Error::IncorrectSignature => None,
+            Error::InvalidMessage => None,
+            Error::InvalidPublicKey => None,
+            Error::InvalidSignature => None,
+            Error::InvalidSecretKey => None,
+            Error::InvalidRecoveryId => None,
+            Error::InvalidTweak => None,
+            Error::NotEnoughMemory => None,
+            Error::InvalidPublicKeySum => None,
+            Error::InvalidParityValue(error) => Some(error),
+        }
+    }
+}
 
 
 /// The secp256k1 engine, used to execute all signature operations.