From 43ce639d136a41289595749710212ef4f67f5f7f Mon Sep 17 00:00:00 2001
From: Jack O'Connor <oconnor663@gmail.com>
Date: Sun, 14 Jul 2024 19:27:37 -0700
Subject: [PATCH] Revert "serialize Hash with serde_bytes"

This mostly reverts commits 8416b1658c2690dc6351bdc7e0975b0d5f1a5282 and
dd0afd640ad97b5ebcf887107162009a23ffdca0.

Changing the serialization of Hash can only be backwards-compatible in
self-describing formats like CBOR. In non-self-describing formats like
bincode, the deserializer has to know in advance which serialization
format was used.

Fixes https://github.com/BLAKE3-team/BLAKE3/issues/414.

Reopens https://github.com/BLAKE3-team/BLAKE3/issues/412.
---
 Cargo.toml  |  3 ---
 src/lib.rs  |  7 +------
 src/test.rs | 29 ++++++++++++++++-------------
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 0d304a414..71ac87255 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -42,8 +42,6 @@ mmap = ["std", "dep:memmap2"]
 # Implement the zeroize::Zeroize trait for types in this crate.
 zeroize = ["dep:zeroize", "arrayvec/zeroize"]
 
-serde = ["dep:serde", "dep:serde_bytes"]
-
 # This crate implements traits from the RustCrypto project, exposed here as the
 # "traits-preview" feature. However, these traits aren't stable, and they're
 # expected to change in incompatible ways before they reach 1.0. For that
@@ -105,7 +103,6 @@ digest = { version = "0.10.1", features = [ "mac" ], optional = true }
 memmap2 = { version = "0.9", optional = true }
 rayon-core = { version = "1.12.1", optional = true }
 serde = { version = "1.0", default-features = false, features = ["derive"], optional = true }
-serde_bytes = { version = "0.11.15", optional = true }
 zeroize = { version = "1", default-features = false, features = ["zeroize_derive"], optional = true }
 
 [dev-dependencies]
diff --git a/src/lib.rs b/src/lib.rs
index e6a9d6771..d64e18fce 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -219,12 +219,7 @@ fn counter_high(counter: u64) -> u32 {
 #[cfg_attr(feature = "zeroize", derive(zeroize::Zeroize))]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 #[derive(Clone, Copy, Hash)]
-pub struct Hash(
-    #[rustfmt::skip]
-    // In formats like CBOR, bytestrings are more compact than lists of ints.
-    #[cfg_attr(feature = "serde", serde(with = "serde_bytes"))]
-    [u8; OUT_LEN],
-);
+pub struct Hash([u8; OUT_LEN]);
 
 impl Hash {
     /// The raw bytes of the `Hash`. Note that byte arrays don't provide
diff --git a/src/test.rs b/src/test.rs
index 9f73417ae..b716e1b2a 100644
--- a/src/test.rs
+++ b/src/test.rs
@@ -826,25 +826,28 @@ fn test_serde() {
     assert_eq!(
         cbor,
         [
-            0x58, 0x20, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe,
-            0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe,
-            0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe
+            0x98, 0x20, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
+            0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
+            0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
+            0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
+            0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
         ]
     );
     let hash_from_cbor: crate::Hash = ciborium::from_reader(&cbor[..]).unwrap();
     assert_eq!(hash_from_cbor, hash);
 
-    // Before we used serde_bytes, the hash [254; 32] would serialize as an array instead of a
-    // byte string, like this. Make sure we can still deserialize this representation.
-    let old_cbor: &[u8] = &[
-        0x98, 0x20, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18,
-        0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
-        0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18,
-        0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
-        0x18, 0xfe, 0x18, 0xfe, 0x18, 0xfe,
+    // Version 1.5.2 of this crate changed the default serialization format to a bytestring
+    // (instead of an array/list) to save bytes on the wire. That was a backwards compatibility
+    // mistake for non-self-describing formats, and it's been reverted. Since some small number of
+    // serialized bytestrings will probably exist forever in the wild, we shold test that we can
+    // still deserialize these from self-describing formats.
+    let bytestring_cbor: &[u8] = &[
+        0x58, 0x20, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe,
+        0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe,
+        0xfe, 0xfe, 0xfe, 0xfe,
     ];
-    let hash_from_old_cbor: crate::Hash = ciborium::from_reader(old_cbor).unwrap();
-    assert_eq!(hash_from_old_cbor, hash);
+    let hash_from_bytestring_cbor: crate::Hash = ciborium::from_reader(bytestring_cbor).unwrap();
+    assert_eq!(hash_from_bytestring_cbor, hash);
 }
 
 // `cargo +nightly miri test` currently works, but it takes forever, because some of our test