Refactored readable serde and fix SuiAddress serialization (MystenLab…

…s#1487)

* * Refactored readable serde to use serde_as instead, greatly simplified the code and removed a lot of duplicated logics
* Changed SuiAddress serde to tuple array instead of Bytes
* Refactored ObjectID serde to reuse readable_serde, without changing the output
* Fixed/Improved serde for MoveObject and MovePackage

* add docs

* update format

* remove code block quote from Readable docs

* add serialization test for SuiAddress and ObjectID with hardcoded values

sui_core/src/authority/authority_store.rs

@@ -1057,7 +1057,6 @@ impl<const A: bool, S: Eq + Serialize + for<'de> Deserialize<'de>> ModuleResolve
                     .serialized_module_map()
                     .get(module_id.name().as_str())
                     .cloned()
-                    .map(|m| m.into_vec())
             }))
     }
 }

sui_core/src/authority/temporary_store.rs

@@ -358,8 +358,7 @@ impl<S: BackingPackageStore> ModuleResolver for AuthorityTemporaryStore<S> {
             Data::Package(c) => Ok(c
                 .serialized_module_map()
                 .get(module_id.name().as_str())
-                .cloned()
-                .map(|m| m.into_vec())),
+                .cloned()),
             _ => Err(SuiError::BadObjectType {
                 error: "Expected module object".to_string(),
             }),

sui_core/tests/staged/sui.yaml

@@ -208,13 +208,12 @@ MoveFieldLayout:
 MoveModulePublish:
   STRUCT:
     - modules:
-        SEQ:
-          SEQ: U8
+        SEQ: BYTES
 MoveObject:
   STRUCT:
     - type_:
         TYPENAME: StructTag
-    - contents: STR
+    - contents: BYTES
 MovePackage:
   STRUCT:
     - id:
@@ -435,7 +434,10 @@ StructTag:
         SEQ:
           TYPENAME: TypeTag
 SuiAddress:
-  NEWTYPESTRUCT: BYTES
+  NEWTYPESTRUCT:
+    TUPLEARRAY:
+      CONTENT: U8
+      SIZE: 20
 SuiError:
   ENUM:
     0:

sui_programmability/adapter/src/unit_tests/adapter_tests.rs

@@ -160,11 +160,9 @@ impl ModuleResolver for InMemoryStorage {
         Ok(self
             .read_object(&ObjectID::from(*module_id.address()))
             .and_then(|o| match &o.data {
-                Data::Package(m) => Some(
-                    m.serialized_module_map()[module_id.name().as_str()]
-                        .clone()
-                        .into_vec(),
-                ),
+                Data::Package(m) => {
+                    Some(m.serialized_module_map()[module_id.name().as_str()].clone())
+                }
                 Data::Move(_) => None,
             }))
     }

sui_types/src/base_types.rs

@@ -1,27 +1,27 @@
 // Copyright (c) 2021, Facebook, Inc. and its affiliates
 // Copyright (c) 2022, Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
-use base64ct::{Base64, Encoding};
+use base64ct::Encoding;
 use std::collections::{HashMap, HashSet};
 use std::convert::{TryFrom, TryInto};
 use std::fmt;
 
-use crate::readable_serde::BytesOrBase64;
-use crate::readable_serde::BytesOrHex;
+use crate::crypto::PublicKeyBytes;
+use crate::error::SuiError;
+use crate::readable_serde::encoding::Base64;
+use crate::readable_serde::encoding::Hex;
+use crate::readable_serde::Readable;
 use ed25519_dalek::Digest;
 use hex::FromHex;
 use move_core_types::account_address::AccountAddress;
 use move_core_types::ident_str;
 use move_core_types::identifier::IdentStr;
 use opentelemetry::{global, Context};
 use rand::Rng;
-use serde::de::Error;
 use serde::{Deserialize, Serialize};
+use serde_with::serde_as;
+use serde_with::Bytes;
 use sha3::Sha3_256;
-
-use crate::crypto::PublicKeyBytes;
-use crate::error::SuiError;
-
 #[cfg(test)]
 #[path = "unit_tests/base_types_tests.rs"]
 mod base_types_tests;
@@ -38,14 +38,16 @@ pub struct UserData(pub Option<[u8; 32]>);
 
 pub type AuthorityName = PublicKeyBytes;
 
-#[derive(Eq, PartialEq, Clone, Copy, PartialOrd, Ord, Hash)]
-pub struct ObjectID(AccountAddress);
+#[serde_as]
+#[derive(Eq, PartialEq, Clone, Copy, PartialOrd, Ord, Hash, Serialize, Deserialize)]
+pub struct ObjectID(#[serde_as(as = "Readable<Hex, _>")] AccountAddress);
 
 pub type ObjectRef = (ObjectID, SequenceNumber, ObjectDigest);
 
 pub const SUI_ADDRESS_LENGTH: usize = ObjectID::LENGTH;
+#[serde_as]
 #[derive(Eq, Default, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Serialize, Deserialize)]
-pub struct SuiAddress(#[serde(with = "BytesOrHex")] [u8; SUI_ADDRESS_LENGTH]);
+pub struct SuiAddress(#[serde_as(as = "Readable<Hex, _>")] [u8; SUI_ADDRESS_LENGTH]);
 
 impl SuiAddress {
     pub fn to_vec(&self) -> Vec<u8> {
@@ -133,11 +135,15 @@ pub const TRANSACTION_DIGEST_LENGTH: usize = 32;
 pub const OBJECT_DIGEST_LENGTH: usize = 32;
 
 /// A transaction will have a (unique) digest.
+#[serde_as]
 #[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Serialize, Deserialize)]
-pub struct TransactionDigest(#[serde(with = "BytesOrBase64")] [u8; TRANSACTION_DIGEST_LENGTH]);
+pub struct TransactionDigest(
+    #[serde_as(as = "Readable<Base64, Bytes>")] [u8; TRANSACTION_DIGEST_LENGTH],
+);
 // Each object has a unique digest
+#[serde_as]
 #[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Serialize, Deserialize)]
-pub struct ObjectDigest(#[serde(with = "BytesOrBase64")] pub [u8; 32]); // We use SHA3-256 hence 32 bytes here
+pub struct ObjectDigest(#[serde_as(as = "Readable<Base64, Bytes>")] pub [u8; 32]); // We use SHA3-256 hence 32 bytes here
 
 pub const TX_CONTEXT_MODULE_NAME: &IdentStr = ident_str!("TxContext");
 pub const TX_CONTEXT_STRUCT_NAME: &IdentStr = TX_CONTEXT_MODULE_NAME;
@@ -393,7 +399,7 @@ impl TryFrom<&[u8]> for ObjectDigest {
 
 impl std::fmt::Debug for TransactionDigest {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> {
-        let s = Base64::encode_string(&self.0);
+        let s = base64ct::Base64::encode_string(&self.0);
         write!(f, "{}", s)?;
         Ok(())
     }
@@ -639,39 +645,3 @@ impl std::str::FromStr for ObjectID {
         Self::from_hex(s).or_else(|_| Self::from_hex_literal(s))
     }
 }
-
-impl<'de> Deserialize<'de> for ObjectID {
-    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        if deserializer.is_human_readable() {
-            let s = <String>::deserialize(deserializer)?;
-            ObjectID::from_hex(s).map_err(D::Error::custom)
-        } else {
-            // In order to preserve the Serde data model and help analysis tools,
-            // make sure to wrap our value in a container with the same name
-            // as the original type.
-            #[derive(::serde::Deserialize)]
-            #[serde(rename = "ObjectID")]
-            struct Value([u8; ObjectID::LENGTH]);
-
-            let value = Value::deserialize(deserializer)?;
-            Ok(ObjectID::new(value.0))
-        }
-    }
-}
-
-impl Serialize for ObjectID {
-    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        if serializer.is_human_readable() {
-            self.to_hex().serialize(serializer)
-        } else {
-            // See comment in deserialize.
-            serializer.serialize_newtype_struct("ObjectID", &self.0)
-        }
-    }
-}

sui_types/src/crypto.rs

@@ -1,18 +1,21 @@
 // Copyright (c) 2022, Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 use anyhow::Error;
-use base64ct::{Base64, Encoding};
+use base64ct::Encoding;
 use std::borrow::Borrow;
 use std::collections::HashMap;
 
 use crate::base_types::{AuthorityName, SuiAddress};
 use crate::error::{SuiError, SuiResult};
-use crate::readable_serde::BytesOrBase64;
+use crate::readable_serde::encoding::Base64;
+use crate::readable_serde::Readable;
 use ed25519_dalek as dalek;
 use ed25519_dalek::{Digest, PublicKey, Verifier};
 use once_cell::sync::OnceCell;
 use rand::rngs::OsRng;
 use serde::{Deserialize, Serialize};
+use serde_with::serde_as;
+use serde_with::Bytes;
 use sha3::Sha3_256;
 
 // TODO: Make sure secrets are not copyable and movable to control where they are in memory
@@ -49,7 +52,7 @@ impl Serialize for KeyPair {
     where
         S: serde::ser::Serializer,
     {
-        serializer.serialize_str(&Base64::encode_string(&self.key_pair.to_bytes()))
+        serializer.serialize_str(&base64ct::Base64::encode_string(&self.key_pair.to_bytes()))
     }
 }
 
@@ -59,8 +62,8 @@ impl<'de> Deserialize<'de> for KeyPair {
         D: serde::de::Deserializer<'de>,
     {
         let s = String::deserialize(deserializer)?;
-        let value =
-            Base64::decode_vec(&s).map_err(|err| serde::de::Error::custom(err.to_string()))?;
+        let value = base64ct::Base64::decode_vec(&s)
+            .map_err(|err| serde::de::Error::custom(err.to_string()))?;
         let key = dalek::Keypair::from_bytes(&value)
             .map_err(|err| serde::de::Error::custom(err.to_string()))?;
         Ok(KeyPair {
@@ -88,8 +91,11 @@ impl signature::Signer<AuthoritySignature> for KeyPair {
     }
 }
 
+#[serde_as]
 #[derive(Eq, Default, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Serialize, Deserialize)]
-pub struct PublicKeyBytes(#[serde(with = "BytesOrBase64")] [u8; dalek::PUBLIC_KEY_LENGTH]);
+pub struct PublicKeyBytes(
+    #[serde_as(as = "Readable<Base64, Bytes>")] [u8; dalek::PUBLIC_KEY_LENGTH],
+);
 
 impl PublicKeyBytes {
     pub fn to_vec(&self) -> Vec<u8> {
@@ -165,8 +171,9 @@ pub fn get_key_pair_from_bytes(bytes: &[u8]) -> (SuiAddress, KeyPair) {
 pub const SUI_SIGNATURE_LENGTH: usize =
     ed25519_dalek::PUBLIC_KEY_LENGTH + ed25519_dalek::SIGNATURE_LENGTH;
 
+#[serde_as]
 #[derive(Eq, PartialEq, Copy, Clone, Serialize, Deserialize)]
-pub struct Signature(#[serde(with = "BytesOrBase64")] [u8; SUI_SIGNATURE_LENGTH]);
+pub struct Signature(#[serde_as(as = "Readable<Base64, Bytes>")] [u8; SUI_SIGNATURE_LENGTH]);
 
 impl AsRef<[u8]> for Signature {
     fn as_ref(&self) -> &[u8] {
@@ -184,8 +191,8 @@ impl signature::Signature for Signature {
 
 impl std::fmt::Debug for Signature {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> {
-        let s = Base64::encode_string(self.signature_bytes());
-        let p = Base64::encode_string(self.public_key_bytes());
+        let s = base64ct::Base64::encode_string(self.signature_bytes());
+        let p = base64ct::Base64::encode_string(self.public_key_bytes());
         write!(f, "{s}@{p}")?;
         Ok(())
     }
@@ -297,8 +304,9 @@ impl Signature {
 
 /// A signature emitted by an authority. It's useful to decouple this from user signatures,
 /// as their set of supported schemes will probably diverge
+#[serde_as]
 #[derive(Debug, Eq, PartialEq, Copy, Clone, Serialize, Deserialize)]
-pub struct AuthoritySignature(#[serde(with = "BytesOrBase64")] pub dalek::Signature);
+pub struct AuthoritySignature(#[serde_as(as = "Readable<Base64, _>")] pub dalek::Signature);
 impl AsRef<[u8]> for AuthoritySignature {
     fn as_ref(&self) -> &[u8] {
         self.0.as_ref()

sui_types/src/messages.rs

@@ -2,14 +2,16 @@
 // Copyright (c) 2022, Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use super::{base_types::*, batch::*, committee::Committee, error::*, event::Event};
 use crate::crypto::{
     sha3_hash, AuthoritySignInfo, AuthoritySignInfoTrait, AuthoritySignature, BcsSignable,
     EmptySignInfo, Signable, Signature, VerificationObligation,
 };
 use crate::gas::GasCostSummary;
 use crate::object::{Object, ObjectFormatOptions, Owner, OBJECT_START_VERSION};
-use crate::readable_serde::Base64OrDefault;
-use base64ct::{Base64, Encoding};
+use crate::readable_serde::encoding::Base64;
+use crate::readable_serde::Readable;
+use base64ct::Encoding;
 use itertools::Either;
 use move_binary_format::{access::ModuleAccess, CompiledModule};
 use move_core_types::{
@@ -21,15 +23,13 @@ use once_cell::sync::OnceCell;
 use serde::{Deserialize, Serialize};
 use serde_name::{DeserializeNameAdapter, SerializeNameAdapter};
 use serde_with::serde_as;
+use serde_with::Bytes;
 use std::fmt::Write;
 use std::fmt::{Display, Formatter};
 use std::{
     collections::{BTreeSet, HashSet},
     hash::{Hash, Hasher},
 };
-
-use super::{base_types::*, batch::*, committee::Committee, error::*, event::Event};
-
 #[cfg(test)]
 #[path = "unit_tests/messages_tests.rs"]
 mod messages_tests;
@@ -59,7 +59,7 @@ pub struct MoveCall {
 #[serde_as]
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)]
 pub struct MoveModulePublish {
-    #[serde_as(as = "Vec<Base64OrDefault>")]
+    #[serde_as(as = "Vec<Readable<Base64, Bytes>>")]
     pub modules: Vec<Vec<u8>>,
 }
 
@@ -319,7 +319,7 @@ where
     }
 
     pub fn to_base64(&self) -> String {
-        Base64::encode_string(&self.to_bytes())
+        base64ct::Base64::encode_string(&self.to_bytes())
     }
 }
 

sui_types/src/move_package.rs

@@ -1,17 +1,19 @@
 // Copyright (c) 2022, Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::readable_serde::Base64OrDefault;
+use crate::readable_serde::encoding::Base64;
+use crate::readable_serde::Readable;
 use crate::{
     base_types::ObjectID,
     error::{SuiError, SuiResult},
 };
 use move_binary_format::file_format::CompiledModule;
 use move_core_types::identifier::Identifier;
 use serde::{Deserialize, Serialize};
-use serde_bytes::ByteBuf;
 use serde_with::serde_as;
+use serde_with::Bytes;
 use std::collections::BTreeMap;
+
 // TODO: robust MovePackage tests
 // #[cfg(test)]
 // #[path = "unit_tests/move_package.rs"]
@@ -22,13 +24,13 @@ use std::collections::BTreeMap;
 #[derive(Eq, PartialEq, Debug, Clone, Deserialize, Serialize, Hash)]
 pub struct MovePackage {
     id: ObjectID,
-    #[serde_as(as = "BTreeMap<_, Base64OrDefault>")]
     // TODO use session cache
-    module_map: BTreeMap<String, ByteBuf>,
+    #[serde_as(as = "BTreeMap<_, Readable<Base64, Bytes>>")]
+    module_map: BTreeMap<String, Vec<u8>>,
 }
 
 impl MovePackage {
-    pub fn new(id: ObjectID, module_map: &BTreeMap<String, ByteBuf>) -> Self {
+    pub fn new(id: ObjectID, module_map: &BTreeMap<String, Vec<u8>>) -> Self {
         Self {
             id,
             module_map: module_map.clone(),
@@ -39,7 +41,7 @@ impl MovePackage {
         self.id
     }
 
-    pub fn serialized_module_map(&self) -> &BTreeMap<String, ByteBuf> {
+    pub fn serialized_module_map(&self) -> &BTreeMap<String, Vec<u8>> {
         &self.module_map
     }
 
@@ -67,7 +69,7 @@ impl From<&Vec<CompiledModule>> for MovePackage {
                 .map(|module| {
                     let mut bytes = Vec::new();
                     module.serialize(&mut bytes).unwrap();
-                    (module.self_id().name().to_string(), ByteBuf::from(bytes))
+                    (module.self_id().name().to_string(), bytes)
                 })
                 .collect(),
         )