Merge rust-bitcoin/rust-bitcoin#1068: Implement human-readable serde for `Witness`

a1df62a3d9 Witness human-readable serde test (Dr Maxim Orlovsky)
68577dfb50 Witness human-readable serde (Dr Maxim Orlovsky)
93b66c55b3 Witness serde: test binary encoding to be backward-compatible (Dr Maxim Orlovsky)
b409ae78a4 witness: Refactor import statements (Tobin C. Harding)
e23d3a815c Remove unnecessary whitespace (Tobin C. Harding)
ac55b1017e Add whitespace between functions (Tobin C. Harding)

Pull request description:

  This is dr-orlovsky's [PR](https://github.com/rust-bitcoin/rust-bitcoin/pull/899) picked up at his permission in the discussion thread.

  I went through the review comments and implemented everything except the perf optimisations. Also includes a patch at the front of the PR that adds a unit test that can be run to see the "before and after", not sure if we want it in, perhaps it should be removed before merge.

  This PR implicitly fixes 942.

  To test this PR works as advertised run `cargo test display_transaction --features=serde -- --nocapture` after creating a unit test as follows:
  ```rust

      // Used to verify that parts of a transaction pretty print.
      // `cargo test display_transaction --features=serde -- --nocapture`
      #[cfg(feature = "serde")]
      #[test]
      fn serde_display_transaction() {
          let tx_bytes = Vec::from_hex(
              "02000000000101595895ea20179de87052b4046dfe6fd515860505d6511a9004cf12a1f93cac7c01000000\
              00ffffffff01deb807000000000017a9140f3444e271620c736808aa7b33e370bd87cb5a078702483045022\
              100fb60dad8df4af2841adc0346638c16d0b8035f5e3f3753b88db122e70c79f9370220756e6633b17fd271\
              0e626347d28d60b0a2d6cbb41de51740644b9fb3ba7751040121028fa937ca8cba2197a37c007176ed89410\
              55d3bcb8627d085e94553e62f057dcc00000000"
          ).unwrap();
          let tx: Transaction = deserialize(&tx_bytes).unwrap();
          let ser = serde_json::to_string_pretty(&tx).unwrap();
          println!("{}", ser);
      }
  ```

  Fixes: #942

ACKs for top commit:
  apoelstra:
    ACK a1df62a3d9
  Kixunil:
    ACK a1df62a3d9

Tree-SHA512: d0ef5b8cbf1cf8456eaaea490a793f1ac7dfb18067c4019a2c3a1bdd9627a231a4dd0a0151a4df9af2b32b909d4b384a5bec1dd3e38d44dc6a23f9c40aa4f1f9

src/blockdata/witness.rs

@@ -5,12 +5,13 @@
 //! This module contains the [`Witness`] struct and related methods to operate on it
 //!
 
+use secp256k1::ecdsa;
+
 use crate::blockdata::transaction::EcdsaSighashType;
 use crate::consensus::encode::{Error, MAX_VEC_SIZE};
 use crate::consensus::{Decodable, Encodable, WriteExt};
 use crate::io::{self, Read, Write};
 use crate::prelude::*;
-use secp256k1::ecdsa;
 use crate::VarInt;
 
 /// The Witness is the data used to unlock bitcoins since the [segwit upgrade](https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki)
@@ -124,7 +125,6 @@ impl Encodable for Witness {
 }
 
 impl Witness {
-
     /// Create a new empty [`Witness`]
     pub fn new() -> Self {
         Witness::default()
@@ -281,25 +281,77 @@ impl serde::Serialize for Witness {
     where
         S: serde::Serializer,
     {
-        use serde::ser::SerializeSeq; 
+        use hashes::hex::ToHex;
+        use serde::ser::SerializeSeq;
 
+        let human_readable = serializer.is_human_readable();
         let mut seq = serializer.serialize_seq(Some(self.witness_elements))?;
 
         for elem in self.iter() {
-            seq.serialize_element(&elem)?;
+            if human_readable {
+                seq.serialize_element(&elem.to_hex())?;
+            } else {
+                seq.serialize_element(&elem)?;
+            }
         }
-
         seq.end()
     }
 }
+
 #[cfg(feature = "serde")]
 impl<'de> serde::Deserialize<'de> for Witness {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: serde::Deserializer<'de>,
     {
-        let vec: Vec<Vec<u8>> = serde::Deserialize::deserialize(deserializer)?;
+        struct Visitor;         // Human-readable visitor.
-        Ok(Witness::from_vec(vec))
+        impl<'de> serde::de::Visitor<'de> for Visitor
+        {
+            type Value = Witness;
+
+            fn expecting(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+                write!(f, "a sequence of hex arrays")
+            }
+
+            fn visit_seq<A: serde::de::SeqAccess<'de>>(self, mut a: A) -> Result<Self::Value, A::Error>
+            {
+                use hashes::hex::FromHex;
+                use hashes::hex::Error::*;
+                use serde::de::{self, Unexpected};
+
+                let mut ret = match a.size_hint() {
+                    Some(len) => Vec::with_capacity(len),
+                    None => Vec::new(),
+                };
+
+                while let Some(elem) = a.next_element::<String>()? {
+                    let vec = Vec::<u8>::from_hex(&elem).map_err(|e| {
+                        match e {
+                            InvalidChar(b) => {
+                                match core::char::from_u32(b.into()) {
+                                    Some(c) => de::Error::invalid_value(Unexpected::Char(c), &"a valid hex character"),
+                                    None => de::Error::invalid_value(Unexpected::Unsigned(b.into()), &"a valid hex character")
+                                }
+                            }
+                            OddLengthString(len) => de::Error::invalid_length(len, &"an even length string"),
+                            InvalidLength(expected, got) => {
+                                let exp = format!("expected length: {}", expected);
+                                de::Error::invalid_length(got, &exp.as_str())
+                            }
+                        }
+                    })?;
+                    ret.push(vec);
+                }
+                Ok(Witness::from_vec(ret))
+            }
+        }
+
+        if deserializer.is_human_readable() {
+            deserializer.deserialize_seq(Visitor)
+        } else {
+            let vec: Vec<Vec<u8>> = serde::Deserialize::deserialize(deserializer)?;
+            Ok(Witness::from_vec(vec))
+        }
     }
 }
 
@@ -424,20 +476,35 @@ mod test {
 
     #[cfg(feature = "serde")]
     #[test]
-    fn test_serde() {
+    fn test_serde_bincode() {
-        use serde_json;
+        use bincode;
 
         let old_witness_format = vec![vec![0u8], vec![2]];
         let new_witness_format = Witness::from_vec(old_witness_format.clone());
 
-        let old = serde_json::to_string(&old_witness_format).unwrap();
+        let old = bincode::serialize(&old_witness_format).unwrap();
-        let new = serde_json::to_string(&new_witness_format).unwrap();
+        let new = bincode::serialize(&new_witness_format).unwrap();
 
         assert_eq!(old, new);
 
-        let back = serde_json::from_str(&new).unwrap();
+        let back: Witness = bincode::deserialize(&new).unwrap();
         assert_eq!(new_witness_format, back);
     }
+
+    #[cfg(feature = "serde")]
+    #[test]
+    fn test_serde_human() {
+        use serde_json;
+
+        let witness = Witness::from_vec(vec![vec![0u8, 123, 75], vec![2u8, 6, 3, 7, 8]]);
+
+        let json = serde_json::to_string(&witness).unwrap();
+
+        assert_eq!(json, r#"["007b4b","0206030708"]"#);
+
+        let back: Witness = serde_json::from_str(&json).unwrap();
+        assert_eq!(witness, back);
+    }
 }