Fix serde issue

key-share/src/lib.rs

@@ -21,6 +21,8 @@ use core::ops;
 use generic_ec::{serde::CurveName, Curve, NonZero, Point, Scalar, SecretScalar};
 use generic_ec_zkp::polynomial::lagrange_coefficient;
 
+#[cfg(feature = "serde")]
+mod serde_fix;
 #[cfg(feature = "spof")]
 pub mod trusted_dealer;
 mod utils;
@@ -107,19 +109,81 @@ use serde_with::As;
 /// If you need the smallest size of serialized key share, we advise implementing serialization manually (all fields of
 /// the key share are public!).
 #[derive(Clone)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-#[cfg_attr(feature = "serde", serde(bound = ""))]
 pub struct DirtyCoreKeyShare<E: Curve> {
     /// Index of local party in key generation protocol
     pub i: u16,
     /// Public key info
-    #[cfg_attr(feature = "serde", serde(flatten))]
     pub key_info: DirtyKeyInfo<E>,
     /// Secret share $x_i$
-    #[cfg_attr(feature = "serde", serde(with = "As::<generic_ec::serde::Compact>"))]
     pub x: NonZero<SecretScalar<E>>,
 }
 
+#[cfg(feature = "serde")]
+impl<E: Curve> serde::Serialize for DirtyCoreKeyShare<E> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        // See [`crate::serde_fix`] module docs
+        let Self {
+            i,
+            key_info:
+                DirtyKeyInfo {
+                    curve,
+                    shared_public_key,
+                    public_shares,
+                    vss_setup,
+                    #[cfg(feature = "hd-wallets")]
+                    chain_code,
+                },
+            x,
+        } = self.clone();
+        serde_fix::CoreKeyShare {
+            i,
+            curve,
+            shared_public_key,
+            public_shares,
+            vss_setup,
+            x,
+            #[cfg(feature = "hd-wallets")]
+            chain_code,
+        }
+        .serialize(serializer)
+    }
+}
+
+#[cfg(feature = "serde")]
+impl<'de, E: Curve> serde::Deserialize<'de> for DirtyCoreKeyShare<E> {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        // See [`crate::serde_fix`] module docs
+        let serde_fix::CoreKeyShare {
+            curve,
+            i,
+            shared_public_key,
+            public_shares,
+            vss_setup,
+            x,
+            #[cfg(feature = "hd-wallets")]
+            chain_code,
+        } = serde::Deserialize::deserialize(deserializer)?;
+        Ok(Self {
+            i,
+            key_info: DirtyKeyInfo {
+                curve,
+                shared_public_key,
+                public_shares,
+                vss_setup,
+                #[cfg(feature = "hd-wallets")]
+                chain_code,
+            },
+            x,
+        })
+    }
+}
+
 /// Public Key Info
 ///
 /// Contains public information about the TSS key, including shared public key, commitments to

key-share/src/serde_fix.rs

@@ -0,0 +1,42 @@
+//! Fixes serde serialization of the key share
+//!
+//! This module contains structs that help us implementing `serde::{Serialize, Deserialize}`
+//! for [`crate::DirtyCoreKeyShare`].
+//!
+//! Context: we used to have key share struct flatten, with all DirtyKeyInfo fields being
+//! right in the key share. However, at some point, we've decided to move all public common
+//! fields into a separate structure. In order to keep serialization format compatible,
+//! we used `#[serde(flatten)]` attribute, to make on-wire data appear like all the fields
+//! are still in the same struct. It turned out, that `#[serde(flatten)]` is buggy, and,
+//! specifically, it does not preserve `is_human_readable` flag, which broke (de)serialization
+//! code and compatibility with old key shares.
+//!
+//! See the issue for more details on the `serde` problem: <https://github.com/serde-rs/serde/issues/2704>
+//!
+//! Until the issue in `serde` crate is addressed, we have to use a workaround in this module.
+//! We basically reimplement `flatten` attribute manually at the cost of extra allocations.
+
+use generic_ec::{serde::CurveName, Curve, NonZero, Point, SecretScalar};
+use serde::{Deserialize, Serialize};
+use serde_with::As;
+
+#[derive(Serialize, Deserialize)]
+#[serde(bound = "")]
+pub struct CoreKeyShare<E: Curve> {
+    pub curve: CurveName<E>,
+    pub i: u16,
+    #[serde(with = "As::<generic_ec::serde::Compact>")]
+    pub shared_public_key: NonZero<Point<E>>,
+    #[serde(with = "As::<Vec<generic_ec::serde::Compact>>")]
+    pub public_shares: Vec<NonZero<Point<E>>>,
+    pub vss_setup: Option<crate::VssSetup<E>>,
+    #[cfg(feature = "hd-wallets")]
+    #[cfg_attr(
+        feature = "serde",
+        serde(default),
+        serde(with = "As::<Option<crate::utils::HexOrBin>>")
+    )]
+    pub chain_code: Option<slip_10::ChainCode>,
+    #[serde(with = "As::<generic_ec::serde::Compact>")]
+    pub x: NonZero<SecretScalar<E>>,
+}