From 05b6a2f9c25228fe0f3158aea4aaa88c85ee07de Mon Sep 17 00:00:00 2001 From: Dmitriy Shabanov Date: Fri, 29 Apr 2022 02:35:18 +0500 Subject: [PATCH 1/4] serde::{Serialize, Deserialize} for BoundedVec --- Cargo.lock | 1 + frame/support/Cargo.toml | 1 + frame/support/src/storage/bounded_vec.rs | 96 +++++++++++++++++++++++- 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 928be6b18fc15..2bd0cec3548c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2287,6 +2287,7 @@ dependencies = [ "pretty_assertions", "scale-info", "serde", + "serde_json", "smallvec", "sp-arithmetic", "sp-core", diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 4132b9f98ebca..741979ada63ab 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -38,6 +38,7 @@ sp-core-hashing-proc-macro = { version = "5.0.0", path = "../../primitives/core/ k256 = { version = "0.10.4", default-features = false, features = ["ecdsa"] } [dev-dependencies] +serde_json = "1.0.79" assert_matches = "1.3.0" pretty_assertions = "1.2.1" frame-system = { version = "4.0.0-dev", path = "../system" } diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 92ca167f98436..88ab744129a58 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -28,6 +28,11 @@ use core::{ ops::{Deref, Index, IndexMut, RangeBounds}, slice::SliceIndex, }; +#[cfg(feature = "std")] +use serde::{ + de::{Error, SeqAccess, Visitor}, + Deserialize, Deserializer, Serialize, +}; use sp_std::{marker::PhantomData, prelude::*}; /// A bounded vector. @@ -37,9 +42,71 @@ use sp_std::{marker::PhantomData, prelude::*}; /// /// As the name suggests, the length of the queue is always bounded. All internal operations ensure /// this bound is respected. +#[cfg_attr(feature = "std", derive(Serialize), serde(transparent))] #[derive(Encode, scale_info::TypeInfo)] #[scale_info(skip_type_params(S))] -pub struct BoundedVec(Vec, PhantomData); +pub struct BoundedVec( + Vec, + #[cfg_attr(feature = "std", serde(skip_serializing))] PhantomData, +); + +#[cfg(feature = "std")] +impl<'de, T, S: Get> Deserialize<'de> for BoundedVec +where + T: Deserialize<'de>, +{ + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct VecVisitor>(PhantomData, PhantomData); + + impl<'de, T, S: Get> Visitor<'de> for VecVisitor + where + T: Deserialize<'de>, + { + type Value = Vec; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a sequence") + } + + fn visit_seq(self, mut seq: A) -> Result + where + A: SeqAccess<'de>, + { + let size = seq.size_hint().unwrap_or(0); + let max = match usize::try_from(S::get()) { + Ok(n) => n, + Err(_) => return Err(A::Error::custom("can't convert to usize")), + }; + if size > max { + Err(A::Error::custom("out of bounds")) + } else { + let mut values = Vec::with_capacity(size); + + while let Some(value) = seq.next_element()? { + values.push(value); + if values.len() > max { + return Err(A::Error::custom("out of bounds")) + } + } + + Ok(values) + } + } + } + + let visitor: VecVisitor = VecVisitor(PhantomData, PhantomData); + match deserializer.deserialize_seq(visitor) { + Ok(v) => match BoundedVec::::try_from(v) { + Ok(r) => Ok(r), + Err(_) => Err(Error::custom("out of bounds")), + }, + Err(e) => Err(e), + } + } +} /// A bounded slice. /// @@ -908,4 +975,31 @@ pub mod test { assert!(b.try_extend(vec![4, 5, 6].into_iter()).is_err()); assert_eq!(*b, vec![1, 2, 3]); } + + #[test] + fn test_serializer() { + let c: BoundedVec> = bounded_vec![0, 1, 2]; + assert_eq!(serde_json::json!(&c).to_string(), r#"[0,1,2]"#); + } + + #[test] + fn test_deserializer() { + let c: BoundedVec> = serde_json::from_str(r#"[0,1,2]"#).unwrap(); + + assert_eq!(c.len(), 3); + assert_eq!(c[0], 0); + assert_eq!(c[1], 1); + assert_eq!(c[2], 2); + } + + #[test] + fn test_deserializer_failed() { + let c: Result>, serde_json::error::Error> = + serde_json::from_str(r#"[0,1,2,3,4,5]"#); + + match c { + Err(msg) => assert_eq!(msg.to_string(), "out of bounds at line 1 column 11"), + _ => unreachable!("deserializer must raise error"), + } + } } From 98c386aae08399d466c02a244f17b01329a0b088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 16 May 2022 11:15:04 +0200 Subject: [PATCH 2/4] Apply suggestions from code review --- frame/support/src/storage/bounded_vec.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 88ab744129a58..abab0494d4445 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -59,7 +59,7 @@ where where D: Deserializer<'de>, { - struct VecVisitor>(PhantomData, PhantomData); + struct VecVisitor>(PhantomData<(T, S)>); impl<'de, T, S: Get> Visitor<'de> for VecVisitor where @@ -97,14 +97,8 @@ where } } - let visitor: VecVisitor = VecVisitor(PhantomData, PhantomData); - match deserializer.deserialize_seq(visitor) { - Ok(v) => match BoundedVec::::try_from(v) { - Ok(r) => Ok(r), - Err(_) => Err(Error::custom("out of bounds")), - }, - Err(e) => Err(e), - } + let visitor: VecVisitor = VecVisitor(PhantomData); + deserializer.deserialize_seq(visitor).map(|v| BoundedVec::::try_from(v).map_err(|_| Err(Error::custom("out of bounds"))))? } } From cc2cd132980ec89de91ae70611f6e789ffe429f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 16 May 2022 11:32:42 +0200 Subject: [PATCH 3/4] FMT --- frame/support/src/storage/bounded_vec.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index abab0494d4445..9f0caf0aceecd 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -98,7 +98,9 @@ where } let visitor: VecVisitor = VecVisitor(PhantomData); - deserializer.deserialize_seq(visitor).map(|v| BoundedVec::::try_from(v).map_err(|_| Err(Error::custom("out of bounds"))))? + deserializer.deserialize_seq(visitor).map(|v| { + BoundedVec::::try_from(v).map_err(|_| Err(Error::custom("out of bounds"))) + })? } } From 39c2e7917aa8ada07c83c8a38c995850375080fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 16 May 2022 12:07:49 +0200 Subject: [PATCH 4/4] :facepalm: --- frame/support/src/storage/bounded_vec.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 9f0caf0aceecd..e99ec6850b9f4 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -98,9 +98,9 @@ where } let visitor: VecVisitor = VecVisitor(PhantomData); - deserializer.deserialize_seq(visitor).map(|v| { - BoundedVec::::try_from(v).map_err(|_| Err(Error::custom("out of bounds"))) - })? + deserializer + .deserialize_seq(visitor) + .map(|v| BoundedVec::::try_from(v).map_err(|_| Error::custom("out of bounds")))? } }