Test consensus-critical Amount deserialization (#2487)

* Add the constraint name to the Amount debug format * Test consensus-critical serialization for Amount Previously we were testing `serde` and `bincode` serialization, which uses a completely different code path. Co-authored-by: Alfredo Garcia <[email protected]>
ZcashFoundation · Jul 15, 2021 · cd78241 · cd78241
1 parent 81a13af
commit cd78241
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 23 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml
@@ -63,7 +63,6 @@ redjubjub = { git = "https://github.com/ZcashFoundation/redjubjub.git", rev = "f
 zebra-test = { path = "../zebra-test/", optional = true }
 
 [dev-dependencies]
-bincode = "1"
 color-eyre = "0.5.11"
 criterion = { version = "0.3", features = ["html_reports"] }
 itertools = "0.10.1"

diff --git a/zebra-chain/src/amount.rs b/zebra-chain/src/amount.rs
@@ -24,13 +24,11 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 #[serde(bound = "C: Constraint")]
 pub struct Amount<C = NegativeAllowed>(i64, PhantomData<C>);
 
-// in a world where specialization existed
-// https://github.com/rust-lang/rust/issues/31844
-// we could do much better here
-// for now, drop the constraint
 impl<C> std::fmt::Debug for Amount<C> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_tuple("Amount").field(&self.0).finish()
+        f.debug_tuple(&format!("Amount<{}>", std::any::type_name::<C>()))
+            .field(&self.0)
+            .finish()
     }
 }
 
@@ -425,6 +423,8 @@ where
 
 #[cfg(test)]
 mod test {
+    use crate::serialization::ZcashDeserializeInto;
+
     use super::*;
 
     use std::{collections::hash_map::RandomState, collections::HashSet, fmt::Debug};
@@ -593,20 +593,30 @@ mod test {
     fn deserialize_checks_bounds() -> Result<()> {
         zebra_test::init();
 
-        let big = MAX_MONEY * 2;
+        let big = (MAX_MONEY * 2)
+            .try_into()
+            .expect("unexpectedly large constant: multiplied constant should be within range");
         let neg = -10;
 
-        let big_bytes = bincode::serialize(&big)?;
-        let neg_bytes = bincode::serialize(&neg)?;
+        let mut big_bytes = Vec::new();
+        (&mut big_bytes)
+            .write_u64::<LittleEndian>(big)
+            .expect("unexpected serialization failure: vec should be infalliable");
+
+        let mut neg_bytes = Vec::new();
+        (&mut neg_bytes)
+            .write_i64::<LittleEndian>(neg)
+            .expect("unexpected serialization failure: vec should be infalliable");
 
-        bincode::deserialize::<Amount<NonNegative>>(&big_bytes)
+        Amount::<NonNegative>::zcash_deserialize(big_bytes.as_slice())
             .expect_err("deserialization should reject too large values");
-        bincode::deserialize::<Amount<NegativeAllowed>>(&big_bytes)
+        Amount::<NegativeAllowed>::zcash_deserialize(big_bytes.as_slice())
             .expect_err("deserialization should reject too large values");
 
-        bincode::deserialize::<Amount<NonNegative>>(&neg_bytes)
+        Amount::<NonNegative>::zcash_deserialize(neg_bytes.as_slice())
             .expect_err("NonNegative deserialization should reject negative values");
-        let amount = bincode::deserialize::<Amount<NegativeAllowed>>(&neg_bytes)
+        let amount: Amount<NegativeAllowed> = neg_bytes
+            .zcash_deserialize_into()
             .expect("NegativeAllowed deserialization should allow negative values");
 
         assert_eq!(amount.0, neg);