Fix serde allowing creation of illegal BoundedVec variants

.github/workflows/ci.yml

@@ -86,7 +86,7 @@ jobs:
         uses: actions/checkout@v2
       - name: Generate code coverage
         run: |
-          cargo tarpaulin --avoid-cfg-tarpaulin --timeout=360 --out lcov
+          cargo tarpaulin --all-features --avoid-cfg-tarpaulin --timeout=360 --out lcov
       - name: Push code coverage results to coveralls.io
         uses: coverallsapp/github-action@master
         with:

Cargo.toml

@@ -23,3 +23,4 @@ arbitrary = ["proptest"]
 
 [dev-dependencies]
 proptest = { version = "1.0.0" }
+serde_json = "1.0.140"

src/bounded_vec.rs

@@ -12,7 +12,7 @@
 #[derive(PartialEq, Eq, Debug, Clone, Hash, PartialOrd, Ord)]
 pub struct BoundedVec<T, const L: usize, const U: usize, W = witnesses::NonEmpty<L, U>> {
     inner: Vec<T>,
-    _marker: core::marker::PhantomData<W>,
+    witness: W,
 }
 
 /// BoundedVec errors
@@ -41,12 +41,12 @@
 
     // NOTE: we can have proves if needed for some cases like 8/16/32/64 upper bound, so can make memory and serde more compile safe and efficient
 
-    /// Compile-time proof of valid bounds. Must be consturcted with same bounds to instantiate `BoundedVec`.
-    #[derive(Clone, Copy, Debug, PartialEq, Eq)]
+    /// Compile-time proof of valid bounds. Must be constructed with same bounds to instantiate `BoundedVec`.
+    #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
     pub struct NonEmpty<const L: usize, const U: usize>(());
 
     /// Possibly empty vector with upper bound.
-    #[derive(Clone, Copy, Debug, PartialEq, Eq)]
+    #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
     pub struct Empty<const U: usize>(());
 
     /// Type a compile-time proof of valid bounds
@@ -88,7 +88,7 @@
     ///     BoundedVec::<_, 0, 8, witnesses::Empty<8>>::from_vec(vec![1u8, 2]).unwrap();
     /// ```
     pub fn from_vec(items: Vec<T>) -> Result<Self, BoundedVecOutOfBounds> {
-        let _witness = witnesses::empty::<U>();
+        let witness = witnesses::empty::<U>();
         let len = items.len();
         if len > U {
             Err(BoundedVecOutOfBounds::UpperBoundError {
@@ -98,7 +98,7 @@
         } else {
             Ok(BoundedVec {
                 inner: items,
-                _marker: core::marker::PhantomData,
+                witness,
             })
         }
     }
@@ -238,7 +238,7 @@
     ///     BoundedVec::<_, 2, 8, witnesses::NonEmpty<2, 8>>::from_vec(vec![1u8, 2]).unwrap();
     /// ```
     pub fn from_vec(items: Vec<T>) -> Result<Self, BoundedVecOutOfBounds> {
-        let _witness = witnesses::non_empty::<L, U>();
+        let witness = witnesses::non_empty::<L, U>();
         let len = items.len();
         if len < L {
             Err(BoundedVecOutOfBounds::LowerBoundError {
@@ -253,7 +253,7 @@
         } else {
             Ok(BoundedVec {
                 inner: items,
-                _marker: core::marker::PhantomData,
+                witness,
             })
         }
     }
@@ -321,7 +321,7 @@
     {
         BoundedVec {
             inner: self.inner.into_iter().map(map_fn).collect::<Vec<_>>(),
-            _marker: core::marker::PhantomData,
+            witness: self.witness,
         }
     }
 
@@ -344,7 +344,7 @@
     {
         BoundedVec {
             inner: self.inner.iter().map(map_fn).collect::<Vec<_>>(),
-            _marker: core::marker::PhantomData,
+            witness: self.witness,
         }
     }
 
@@ -500,7 +500,7 @@
     fn from(arr: [T; L]) -> Self {
         BoundedVec {
             inner: arr.into(),
-            _marker: core::marker::PhantomData,
+            witness: witnesses::non_empty(),
         }
     }
 }
@@ -564,9 +564,9 @@
     }
 }
 
 /// Option<BoundedVec<T, _, _>> to Vec<T>
 pub trait OptBoundedVecToVec<T> {
     /// Option<BoundedVec<T, _, _>> to Vec<T>
     fn to_vec(self) -> Vec<T>;
 }
 
@@ -610,7 +610,7 @@
     use serde::{Deserialize, Serialize};
 
     // direct impl to unify serde in one place instead of doing attribute on declaration and deserialize here
-    impl<T: Serialize, const L: usize, const U: usize> Serialize for BoundedVec<T, L, U> {
+    impl<T: Serialize, const L: usize, const U: usize, W> Serialize for BoundedVec<T, L, U, W> {
         fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
         where
             S: serde::Serializer,
@@ -627,23 +627,17 @@
             D: serde::Deserializer<'de>,
         {
             let inner = Vec::<T>::deserialize(deserializer)?;
-            if inner.len() < L {
-                return Err(serde::de::Error::custom(alloc::format!(
-                    "Lower bound violation: got {} (expected >= {})",
-                    inner.len(),
-                    L
-                )));
-            } else if inner.len() > U {
-                return Err(serde::de::Error::custom(alloc::format!(
-                    "Upper bound violation: got {} (expected <= {})",
-                    inner.len(),
-                    U
-                )));
-            };
-            Ok(BoundedVec {
-                inner,
-                _marker: core::marker::PhantomData,
-            })
+            BoundedVec::<T, L, U>::from_vec(inner).map_err(serde::de::Error::custom)
+        }
+    }
+
+    impl<'de, T: Deserialize<'de>, const U: usize> Deserialize<'de> for EmptyBoundedVec<T, U> {
+        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+        where
+            D: serde::Deserializer<'de>,
+        {
+            let inner = Vec::<T>::deserialize(deserializer)?;
+            EmptyBoundedVec::from_vec(inner).map_err(serde::de::Error::custom)
         }
     }
 
@@ -654,7 +648,7 @@
         use schemars::JsonSchema;
 
         // we cannot use attributes, because the do not work with `const`, only numeric literals supported
-        impl<T: JsonSchema, const L: usize, const U: usize> JsonSchema for BoundedVec<T, L, U> {
+        impl<T: JsonSchema, const L: usize, const U: usize, W> JsonSchema for BoundedVec<T, L, U, W> {
             fn schema_name() -> Cow<'static, str> {
                 alloc::format!("BoundedVec{}Min{}Max{}", T::schema_name(), L, U).into()
             }
@@ -802,6 +796,29 @@
     }
 }
 
+#[cfg(feature = "serde")]
+#[cfg(test)]
+#[allow(clippy::unwrap_used)]
+mod serde_tests {
+    use super::*;
+    use alloc::vec;
+    #[test]
+    fn deserialize_nonempty() {
+        assert_eq!(
+            serde_json::from_str::<BoundedVec::<u8, 2, 3>>("[1, 2]")
+                .unwrap()
+                .as_vec(),
+            &vec![1, 2]
+        );
+    }
+
+    #[test]
+    fn deserialize_empty() {
+        assert!(serde_json::from_str::<BoundedVec::<u8, 2, 3>>("[]").is_err());
+        assert!(serde_json::from_str::<EmptyBoundedVec::<u8, 3>>("[]").is_ok());
+    }
+}
+
 #[cfg(feature = "arbitrary")]
 #[cfg(test)]
 #[allow(clippy::len_zero)]