Merge rust-bitcoin/rust-bitcoin#903: Improve `SchnorrSigHashType`

35b682d495 Implement Display/FromStr for SchnorrSigHashType (Tobin Harding)
46c4164d67 Improve SigHashTypeParseError field (Tobin Harding)
c009210d4c Use full path for String in macro (Tobin Harding)

Pull request description:

  Implement Display/FromStr for SchnorrSigHashType

  We currently implement `Display` and `FromStr` on `EcdsaSigHashType` and use them in the `serde_string_impl` macro to implement ser/de.

  Mirror this logic in `SchnorrSigHashType`.

  Patch 1 and 2 are preparatory patches for patch 3.

  ## Notes to reviewers

  This PR has some conflicts with https://github.com/rust-bitcoin/rust-bitcoin/pull/898 but is pushing in the same direction, I'm happy to let 898 go in first and rebase on top.

ACKs for top commit:
  sanket1729:
    ACK 35b682d495. Thanks, much easier to review now that the diff is small
  dr-orlovsky:
    ACK 35b682d495

Tree-SHA512: 481f192a3064ff39acf8904737dfb25b54ef128a37e0ca765ebb39138edac772d4f01ed10aa98ff185a8ed5668d64fa5d5957206b920ffe87950cafcf5a3b516

src/blockdata/transaction.rs

@@ -758,7 +758,7 @@ impl str::FromStr for EcdsaSigHashType {
             "SIGHASH_ALL|SIGHASH_ANYONECANPAY" => Ok(EcdsaSigHashType::AllPlusAnyoneCanPay),
             "SIGHASH_NONE|SIGHASH_ANYONECANPAY" => Ok(EcdsaSigHashType::NonePlusAnyoneCanPay),
             "SIGHASH_SINGLE|SIGHASH_ANYONECANPAY" => Ok(EcdsaSigHashType::SinglePlusAnyoneCanPay),
-            _ => Err(SigHashTypeParseError { string: s.to_owned() }),
+            _ => Err(SigHashTypeParseError{ unrecognized: s.to_owned() }),
         }
     }
 }
@@ -840,12 +840,13 @@ impl From<EcdsaSigHashType> for u32 {
 /// This is currently returned for unrecognized sighash strings.
 #[derive(Debug, Clone)]
 pub struct SigHashTypeParseError {
-    string: String,
+    /// The unrecognized string we attempted to parse.
+    pub unrecognized: String,
 }
 
 impl fmt::Display for SigHashTypeParseError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "can't recognize SIGHASH string '{}'", self.string)
+        write!(f, "Unrecognized SIGHASH string '{}'", self.unrecognized)
     }
 }
 
@@ -1154,7 +1155,7 @@ mod tests {
             "SigHash_NONE",
         ];
         for s in sht_mistakes {
-            assert_eq!(EcdsaSigHashType::from_str(s).unwrap_err().to_string(), format!("can't recognize SIGHASH string '{}'", s));
+            assert_eq!(EcdsaSigHashType::from_str(s).unwrap_err().to_string(), format!("Unrecognized SIGHASH string '{}'", s));
         }
     }
 

src/internal_macros.rs

@@ -160,7 +160,7 @@ macro_rules! serde_string_impl {
                         self.visit_str(v)
                     }
 
-                    fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
+                    fn visit_string<E>(self, v: $crate::prelude::String) -> Result<Self::Value, E>
                     where
                         E: $crate::serde::de::Error,
                     {
@@ -222,7 +222,7 @@ macro_rules! serde_struct_human_string_impl {
                             self.visit_str(v)
                         }
 
-                        fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
+                        fn visit_string<E>(self, v: $crate::prelude::String) -> Result<Self::Value, E>
                         where
                             E: $crate::serde::de::Error,
                         {

src/util/sighash.rs

@@ -20,10 +20,12 @@
 //! and legacy (before Bip143).
 //!
 
-pub use blockdata::transaction::EcdsaSigHashType;
+use prelude::*;
+
+pub use blockdata::transaction::{EcdsaSigHashType, SigHashTypeParseError};
 use blockdata::witness::Witness;
 use consensus::{encode, Encodable};
-use core::fmt;
+use core::{str, fmt};
 use core::ops::{Deref, DerefMut};
 use core::borrow::Borrow;
 use hashes::{sha256, sha256d, Hash};
@@ -105,7 +107,6 @@ pub struct ScriptPath<'s> {
 /// Hashtype of an input's signature, encoded in the last byte of the signature
 /// Fixed values so they can be casted as integer types for encoding
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum SchnorrSigHashType {
     /// 0x0: Used when not explicitly specified, defaulting to [`SchnorrSigHashType::All`]
     Default = 0x00,
@@ -128,6 +129,41 @@ pub enum SchnorrSigHashType {
     /// Reserved for future use, `#[non_exhaustive]` is not available with current MSRV
     Reserved = 0xFF,
 }
+serde_string_impl!(SchnorrSigHashType, "a SchnorrSigHashType data");
+
+impl fmt::Display for SchnorrSigHashType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let s = match self {
+            SchnorrSigHashType::Default => "SIGHASH_DEFAULT",
+            SchnorrSigHashType::All => "SIGHASH_ALL",
+            SchnorrSigHashType::None => "SIGHASH_NONE",
+            SchnorrSigHashType::Single => "SIGHASH_SINGLE",
+            SchnorrSigHashType::AllPlusAnyoneCanPay => "SIGHASH_ALL|SIGHASH_ANYONECANPAY",
+            SchnorrSigHashType::NonePlusAnyoneCanPay => "SIGHASH_NONE|SIGHASH_ANYONECANPAY",
+            SchnorrSigHashType::SinglePlusAnyoneCanPay => "SIGHASH_SINGLE|SIGHASH_ANYONECANPAY",
+            SchnorrSigHashType::Reserved => "SIGHASH_RESERVED",
+        };
+        f.write_str(s)
+    }
+}
+
+impl str::FromStr for SchnorrSigHashType {
+    type Err = SigHashTypeParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "SIGHASH_DEFAULT" => Ok(SchnorrSigHashType::Default),
+            "SIGHASH_ALL" => Ok(SchnorrSigHashType::All),
+            "SIGHASH_NONE" => Ok(SchnorrSigHashType::None),
+            "SIGHASH_SINGLE" => Ok(SchnorrSigHashType::Single),
+            "SIGHASH_ALL|SIGHASH_ANYONECANPAY" => Ok(SchnorrSigHashType::AllPlusAnyoneCanPay),
+            "SIGHASH_NONE|SIGHASH_ANYONECANPAY" => Ok(SchnorrSigHashType::NonePlusAnyoneCanPay),
+            "SIGHASH_SINGLE|SIGHASH_ANYONECANPAY" => Ok(SchnorrSigHashType::SinglePlusAnyoneCanPay),
+            "SIGHASH_RESERVED" => Ok(SchnorrSigHashType::Reserved),
+            _ => Err(SigHashTypeParseError{ unrecognized: s.to_owned() }),
+        }
+    }
+}
 
 /// Possible errors in computing the signature message
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
@@ -1107,4 +1143,39 @@ mod tests {
         let json_str = include_str!("../../test_data/bip341_tests.json");
         serde_json::from_str(json_str).expect("JSON was not well-formatted")
     }
+
+    #[test]
+    fn sighashtype_fromstr_display() {
+        let sighashtypes = vec![
+            ("SIGHASH_DEFAULT", SchnorrSigHashType::Default),
+            ("SIGHASH_ALL", SchnorrSigHashType::All),
+            ("SIGHASH_NONE", SchnorrSigHashType::None),
+            ("SIGHASH_SINGLE", SchnorrSigHashType::Single),
+            ("SIGHASH_ALL|SIGHASH_ANYONECANPAY", SchnorrSigHashType::AllPlusAnyoneCanPay),
+            ("SIGHASH_NONE|SIGHASH_ANYONECANPAY", SchnorrSigHashType::NonePlusAnyoneCanPay),
+            ("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY", SchnorrSigHashType::SinglePlusAnyoneCanPay),
+            ("SIGHASH_RESERVED", SchnorrSigHashType::Reserved),
+        ];
+        for (s, sht) in sighashtypes {
+            assert_eq!(sht.to_string(), s);
+            assert_eq!(SchnorrSigHashType::from_str(s).unwrap(), sht);
+        }
+        let sht_mistakes = vec![
+            "SIGHASH_ALL | SIGHASH_ANYONECANPAY",
+            "SIGHASH_NONE |SIGHASH_ANYONECANPAY",
+            "SIGHASH_SINGLE| SIGHASH_ANYONECANPAY",
+            "SIGHASH_ALL SIGHASH_ANYONECANPAY",
+            "SIGHASH_NONE |",
+            "SIGHASH_SIGNLE",
+            "DEFAULT",
+            "ALL",
+            "sighash_none",
+            "Sighash_none",
+            "SigHash_None",
+            "SigHash_NONE",
+        ];
+        for s in sht_mistakes {
+            assert_eq!(SchnorrSigHashType::from_str(s).unwrap_err().to_string(), format!("Unrecognized SIGHASH string '{}'", s));
+        }
+    }
 }