refactor: explicit deny_unknown_fields for protobuf serde deserializa…

…tion (#200)

It turned out to be too easy to use it incorrectly with an implicit
default.

node/libs/protobuf/examples/conformance_test/main.rs

@@ -18,15 +18,20 @@ mod proto;
 /// Decodes a generated proto message from json for arbitrary ReflectMessage.
 fn decode_json_proto<T: ReflectMessage + Default>(json: &str) -> anyhow::Result<T> {
     let mut deserializer = serde_json::Deserializer::from_str(json);
-    let proto: T = zksync_protobuf::serde::deserialize_proto(&mut deserializer)?;
+    let proto: T = zksync_protobuf::serde::Deserialize {
+        deny_unknown_fields: true,
+    }
+    .proto(&mut deserializer)?;
     deserializer.end()?;
     Ok(proto)
 }
 
 /// Encodes a generated proto message to json for arbitrary ReflectMessage.
 fn encode_json_proto<T: ReflectMessage>(proto: &T) -> String {
     let mut serializer = serde_json::Serializer::pretty(vec![]);
-    zksync_protobuf::serde::serialize_proto(proto, &mut serializer).unwrap();
+    zksync_protobuf::serde::Serialize
+        .proto(proto, &mut serializer)
+        .unwrap();
     String::from_utf8(serializer.into_inner()).unwrap()
 }
 

node/libs/protobuf/src/serde.rs

@@ -3,64 +3,135 @@
 //! therefore it is suitable for version control.
 //! WARNING: Currently this serde implementation uses reflection,
 //! so it is not very efficient.
-use crate::ProtoFmt;
+use crate::{ProtoFmt, ProtoRepr};
 use prost::Message as _;
 use prost_reflect::ReflectMessage;
 
-/// ProtoFmt wrapper which implements serde Serialize/Deserialize.
-#[derive(Debug, Clone)]
-pub struct Serde<T>(pub T);
+/// Serialization options.
+pub struct Serialize;
 
-impl<T: ProtoFmt> serde::Serialize for Serde<T> {
-    fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
-        serialize(&self.0, s)
+impl Serialize {
+    /// Serializes ReflectMessage.
+    pub fn proto<T: ReflectMessage, S: serde::Serializer>(
+        &self,
+        x: &T,
+        s: S,
+    ) -> Result<S::Ok, S::Error> {
+        let opts = prost_reflect::SerializeOptions::new();
+        x.transcode_to_dynamic().serialize_with_options(s, &opts)
     }
-}
 
-impl<'de, T: ProtoFmt> serde::Deserialize<'de> for Serde<T> {
-    fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
-        Ok(Self(deserialize(d)?))
+    /// Serializes ProtoFmt.
+    pub fn proto_fmt<T: ProtoFmt, S: serde::Serializer>(
+        &self,
+        x: &T,
+        s: S,
+    ) -> Result<S::Ok, S::Error> {
+        self.proto(&x.build(), s)
     }
-}
 
-/// Implementation of serde::Serialize for arbitrary ReflectMessage.
-pub fn serialize_proto<T: ReflectMessage, S: serde::Serializer>(
-    x: &T,
-    s: S,
-) -> Result<S::Ok, S::Error> {
-    let opts = prost_reflect::SerializeOptions::new();
-    x.transcode_to_dynamic().serialize_with_options(s, &opts)
-}
+    /// Serializes ProtoFmt to json.
+    pub fn proto_fmt_to_json<T: ProtoFmt>(&self, x: &T) -> String {
+        let mut s = serde_json::Serializer::pretty(vec![]);
+        self.proto_fmt(x, &mut s).unwrap();
+        String::from_utf8(s.into_inner()).unwrap()
+    }
 
-/// Implementation of serde::Serialize for arbitrary ProtoFmt.
-pub fn serialize<T: ProtoFmt, S: serde::Serializer>(x: &T, s: S) -> Result<S::Ok, S::Error> {
-    serialize_proto(&x.build(), s)
-}
+    /// Serializes ProtoFmt to yaml
+    pub fn proto_fmt_to_yaml<T: ProtoFmt>(&self, x: &T) -> String {
+        let mut s = serde_yaml::Serializer::new(vec![]);
+        self.proto_fmt(x, &mut s).unwrap();
+        String::from_utf8(s.into_inner().unwrap()).unwrap()
+    }
 
-/// Implementation of serde::Deserialize for arbitrary ReflectMessage denying unknown fields
-pub fn deserialize_proto<'de, T: ReflectMessage + Default, D: serde::Deserializer<'de>>(
-    d: D,
-) -> Result<T, D::Error> {
-    deserialize_proto_with_options(d, true)
+    /// Serializes ProtoRepr.
+    pub fn proto_repr<T: ProtoRepr, S: serde::Serializer>(
+        &self,
+        x: &T::Type,
+        s: S,
+    ) -> Result<S::Ok, S::Error> {
+        self.proto(&T::build(x), s)
+    }
+
+    /// Serializes ProtoRepr to json.
+    pub fn proto_repr_to_json<T: ProtoRepr>(&self, x: &T::Type) -> String {
+        let mut s = serde_json::Serializer::pretty(vec![]);
+        self.proto_repr::<T, _>(x, &mut s).unwrap();
+        String::from_utf8(s.into_inner()).unwrap()
+    }
+
+    /// Serializes ProtoRepr to yaml
+    pub fn proto_repr_to_yaml<T: ProtoRepr>(&self, x: &T::Type) -> String {
+        let mut s = serde_yaml::Serializer::new(vec![]);
+        self.proto_repr::<T, _>(x, &mut s).unwrap();
+        String::from_utf8(s.into_inner().unwrap()).unwrap()
+    }
 }
 
-/// Implementation of serde::Deserialize for arbitrary ReflectMessage with deny_unknown_fields option
-pub fn deserialize_proto_with_options<
-    'de,
-    T: ReflectMessage + Default,
-    D: serde::Deserializer<'de>,
->(
-    d: D,
-    deny_unknown_fields: bool,
-) -> Result<T, D::Error> {
-    let mut p = T::default();
-    let options = prost_reflect::DeserializeOptions::new().deny_unknown_fields(deny_unknown_fields);
-    let msg = prost_reflect::DynamicMessage::deserialize_with_options(p.descriptor(), d, &options)?;
-    p.merge(msg.encode_to_vec().as_slice()).unwrap();
-    Ok(p)
+/// Deserialization options.
+#[derive(Default)]
+pub struct Deserialize {
+    /// true => returns an error when an unknown field is found.
+    /// false => silently ignores unknown fields.
+    pub deny_unknown_fields: bool,
 }
 
-/// Implementation of serde::Deserialize for arbitrary ProtoFmt.
-pub fn deserialize<'de, T: ProtoFmt, D: serde::Deserializer<'de>>(d: D) -> Result<T, D::Error> {
-    T::read(&deserialize_proto(d)?).map_err(serde::de::Error::custom)
+impl Deserialize {
+    /// Implementation of serde::Deserialize for arbitrary ReflectMessage with deny_unknown_fields option
+    pub fn proto<'de, T: ReflectMessage + Default, D: serde::Deserializer<'de>>(
+        &self,
+        d: D,
+    ) -> Result<T, D::Error> {
+        let mut p = T::default();
+        let options =
+            prost_reflect::DeserializeOptions::new().deny_unknown_fields(self.deny_unknown_fields);
+        let msg =
+            prost_reflect::DynamicMessage::deserialize_with_options(p.descriptor(), d, &options)?;
+        p.merge(msg.encode_to_vec().as_slice()).unwrap();
+        Ok(p)
+    }
+
+    /// Implementation of serde::Deserialize for arbitrary ProtoFmt.
+    pub fn proto_fmt<'de, T: ProtoFmt, D: serde::Deserializer<'de>>(
+        &self,
+        d: D,
+    ) -> Result<T, D::Error> {
+        T::read(&self.proto(d)?).map_err(serde::de::Error::custom)
+    }
+
+    /// Deserializes ProtoFmt from json.
+    pub fn proto_fmt_from_json<T: ProtoFmt>(&self, json: &str) -> anyhow::Result<T> {
+        let mut d = serde_json::Deserializer::from_str(json);
+        let p = self.proto_fmt(&mut d)?;
+        d.end()?;
+        Ok(p)
+    }
+
+    /// Deserializes ProtoFmt from yaml.
+    pub fn proto_fmt_from_yaml<T: ProtoFmt>(&self, yaml: &str) -> anyhow::Result<T> {
+        Ok(self.proto_fmt(serde_yaml::Deserializer::from_str(yaml))?)
+    }
+
+    /// Implementation of serde::Deserialize for arbitrary ProtoFmt.
+    pub fn proto_repr<'de, T: ProtoRepr, D: serde::Deserializer<'de>>(
+        &self,
+        d: D,
+    ) -> Result<T::Type, D::Error> {
+        self.proto::<T, D>(d)?
+            .read()
+            .map_err(serde::de::Error::custom)
+    }
+
+    /// Deserializes ProtoRepr from json.
+    pub fn proto_repr_from_json<T: ProtoRepr>(&self, json: &str) -> anyhow::Result<T::Type> {
+        let mut d = serde_json::Deserializer::from_str(json);
+        let p = self.proto_repr::<T, _>(&mut d)?;
+        d.end()?;
+        Ok(p)
+    }
+
+    /// Deserializes ProtoRepr from yaml.
+    pub fn proto_repr_from_yaml<T: ProtoRepr>(&self, yaml: &str) -> anyhow::Result<T::Type> {
+        Ok(self.proto_repr::<T, _>(serde_yaml::Deserializer::from_str(yaml))?)
+    }
 }

node/libs/protobuf/src/testonly/mod.rs

@@ -1,5 +1,9 @@
 //! Testonly utilities.
-use super::{canonical, canonical_raw, decode, encode, read_fields, ProtoFmt, ProtoRepr, Wire};
+use super::{
+    canonical, canonical_raw, decode, encode, read_fields,
+    serde::{Deserialize, Serialize},
+    ProtoFmt, ProtoRepr, Wire,
+};
 use prost::Message as _;
 use prost_reflect::ReflectMessage;
 use rand::{
@@ -114,24 +118,34 @@ fn decode_proto<X: ProtoConv>(bytes: &[u8]) -> anyhow::Result<X::Type> {
 
 fn encode_json<X: ProtoConv>(msg: &X::Type) -> String {
     let mut s = serde_json::Serializer::pretty(vec![]);
-    crate::serde::serialize_proto(&X::build(msg), &mut s).unwrap();
+    Serialize.proto(&X::build(msg), &mut s).unwrap();
     String::from_utf8(s.into_inner()).unwrap()
 }
 
 fn decode_json<X: ProtoConv>(json: &str) -> anyhow::Result<X::Type> {
     let mut d = serde_json::Deserializer::from_str(json);
-    X::read(&crate::serde::deserialize_proto(&mut d)?)
+    X::read(
+        &Deserialize {
+            deny_unknown_fields: true,
+        }
+        .proto(&mut d)?,
+    )
 }
 
 fn encode_yaml<X: ProtoConv>(msg: &X::Type) -> String {
     let mut s = serde_yaml::Serializer::new(vec![]);
-    crate::serde::serialize_proto(&X::build(msg), &mut s).unwrap();
+    Serialize.proto(&X::build(msg), &mut s).unwrap();
     String::from_utf8(s.into_inner().unwrap()).unwrap()
 }
 
 fn decode_yaml<X: ProtoConv>(yaml: &str) -> anyhow::Result<X::Type> {
     let d = serde_yaml::Deserializer::from_str(yaml);
-    X::read(&crate::serde::deserialize_proto(d)?)
+    X::read(
+        &Deserialize {
+            deny_unknown_fields: true,
+        }
+        .proto(d)?,
+    )
 }
 
 /// Wrapper for `ProtoRepr`, implementing ProtoConv;

node/tools/src/bin/localnet_config.rs

@@ -11,7 +11,7 @@ use std::{
 };
 use zksync_consensus_roles::{node, validator};
 use zksync_consensus_tools::config;
-use zksync_protobuf::serde::Serde;
+use zksync_protobuf::serde::Serialize;
 
 /// Command line arguments.
 #[derive(Debug, Parser)]
@@ -111,7 +111,7 @@ fn main() -> anyhow::Result<()> {
             .context("fs::set_permissions()")?;
 
         let config_path = root.join("config.json");
-        fs::write(&config_path, config::encode_json(&Serde(cfg))).context("fs::write()")?;
+        fs::write(&config_path, Serialize.proto_fmt_to_json(&cfg)).context("fs::write()")?;
         fs::set_permissions(&config_path, Permissions::from_mode(0o600))
             .context("fs::set_permissions()")?;
     }