Merge #403: Parity conversion and error handling cleanup

5acf6d2 `Parity` conversion and error handling cleanup (Martin Habovstiak)

Pull request description:

  This removes the deprecated `From` conversion, replaces it with
  `TryFrom`, and adds more convenience conversions. A new error type is
  created for the invalid parity error with conversion to catch-all
  `Error`.

  This is intended for an API-breaking version.

ACKs for top commit:
  apoelstra:
    ACK 5acf6d2

Tree-SHA512: 49b73fc90455c172012b46f36eafa7d256b940f4b431b4eedb577ab07d9402eae40af931e00b3c409bbe502dbcac064a742e874a5e8bedd8d0cbe92a468ae4f6

Author: apoelstra

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,33 +1219,19 @@ 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())
     }
 
     /// Constructs a [`Parity`] from a signed integer.
     ///
     /// 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")))]

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.