Repository: aptos-labs/aptos-core (language: python)

Issue:
[move e2] Better serialization format for enums
The current serialization of enums needs to use `serialize_seq` instead of `serialize_tuple` which implies that a size field needs to be written., which is inefficient. This is because the static size of enum variants is unknown.

We should investigate to take advantage of serde's builtin support for enum types. That support also would allow to marshall enums over into other languages, f.i. Rust. Unfortunately, there is zero to little documentation of how to use the serde lower-level enum API.s -- only for attributes.

Before enums land in production, we likely want to solve this problem.




Discussion / hints:

Fix patch:
diff --git a/third_party/move/move-binary-format/src/file_format_common.rs b/third_party/move/move-binary-format/src/file_format_common.rs
index c15ce25a90ec2..3a85904097856 100644
--- a/third_party/move/move-binary-format/src/file_format_common.rs
+++ b/third_party/move/move-binary-format/src/file_format_common.rs
@@ -13,6 +13,7 @@
 //! It's used to compress mostly indexes into the main binary tables.
 use crate::file_format::Bytecode;
 use anyhow::{bail, Result};
+use move_core_types::value;
 use std::{
     io::{Cursor, Read},
     mem::size_of,
@@ -76,7 +77,7 @@ pub const ACQUIRES_COUNT_MAX: u64 = 255;
 
 pub const FIELD_COUNT_MAX: u64 = 255;
 pub const FIELD_OFFSET_MAX: u64 = 255;
-pub const VARIANT_COUNT_MAX: u64 = 127;
+pub const VARIANT_COUNT_MAX: u64 = value::VARIANT_COUNT_MAX;
 pub const VARIANT_OFFSET_MAX: u64 = 127;
 
 pub const TYPE_PARAMETER_COUNT_MAX: u64 = 255;
diff --git a/third_party/move/move-core/types/src/value.rs b/third_party/move/move-core/types/src/value.rs
index 57be18fd08f0d..49a4d4eada8df 100644
--- a/third_party/move/move-core/types/src/value.rs
+++ b/third_party/move/move-core/types/src/value.rs
@@ -9,19 +9,29 @@ use crate::{
     u256,
 };
 use anyhow::{anyhow, bail, Result as AResult};
+use once_cell::sync::Lazy;
 use serde::{
-    de::{Error as DeError, Unexpected},
-    ser::{SerializeMap, SerializeSeq, SerializeStruct, SerializeTuple},
+    de::{EnumAccess, Error as DeError, VariantAccess},
+    ser::{SerializeMap, SerializeSeq, SerializeStruct, SerializeTuple, SerializeTupleVariant},
     Deserialize, Serialize,
 };
 use std::{
+    collections::{btree_map::Entry, BTreeMap},
     convert::TryInto,
     fmt::{self, Debug},
+    sync::Mutex,
 };
 
+/// The maximal number of enum variants which are supported in values. This must align with
+/// the configuration in the binary format, so the bytecode verifier checks its validness.
+pub const VARIANT_COUNT_MAX: u64 = 127;
+
 /// In the `WithTypes` configuration, a Move struct gets serialized into a Serde struct with this name
 pub const MOVE_STRUCT_NAME: &str = "struct";
 
+/// A Move enum gets serialized into a Serde struct with this name
+pub const MOVE_ENUM_NAME: &str = "enum";
+
 /// In the `WithTypes` configuration, a Move struct gets serialized into a Serde struct with this as the first field
 pub const MOVE_STRUCT_TYPE: &str = "type";
 
@@ -34,6 +44,37 @@ pub const MOVE_VARIANT_NAME: &str = "variant";
 /// In the `WithVariant` configuration, a Move enum variant gets serialized into a Serde struct with this as the first field
 pub const MOVE_VARIANT_NAME_FIELD: &str = "name";
 
+/// In order to serialize enums with serde, we have to provide `&'static str` names for
+/// variants. This static cache is used to generate those names, and contains
+/// signatures of the form `["0", "1", "2", ..]`. The size of this cache
+/// is bound by `VARIANT_COUNT_MAX * VARIANT_COUNT_MAX / 2` (8064 with max 127)
+static VARIANT_NAME_PLACEHOLDER_CACHE: Lazy<Mutex<BTreeMap<usize, &'static [&'static str]>>> =
+    Lazy::new(|| Mutex::new(Default::default()));
+
+/// Returns variant name placeholders for providing dummy names in serde serialization.
+pub fn variant_name_placeholder(len: usize) -> &'static [&'static str] {
+    assert!(
+        len < VARIANT_COUNT_MAX as usize,
+        "variant count is restricted to {}",
+        VARIANT_COUNT_MAX
+    );
+    let mutex = &VARIANT_NAME_PLACEHOLDER_CACHE;
+    let mut lock = mutex.lock().expect("acquire index name lock");
+    match lock.entry(len) {
+        Entry::Vacant(e) => {
+            let signature = Box::new(
+                (0..len)
+                    .map(|idx| Box::new(format!("{}", idx)).leak() as &str)
+                    .collect::<Vec<_>>(),
+            )
+            .leak();
+            e.insert(signature);
+            signature
+        },
+        Entry::Occupied(e) => e.get(),
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[cfg_attr(any(test, feature = "fuzzing"), derive(arbitrary::Arbitrary))]
 pub enum MoveStruct {
@@ -525,32 +566,30 @@ impl<'d, 'a> serde::de::Visitor<'d> for StructVariantVisitor<'a> {
         formatter.write_str("Variant")
     }
 
-    fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
+    fn visit_enum<A>(self, data: A) -> Result<Self::Value, A::Error>
     where
-        A: serde::de::SeqAccess<'d>,
+        A: EnumAccess<'d>,
     {
-        let mut val = Vec::new();
-
-        // First deserialize the variant tag
-        let variant_tag = match seq.next_element::<u16>()? {
-            Some(tag) => {
-                let tag = tag as usize;
-                if tag >= self.0.len() {
-                    return Err(A::Error::invalid_value(Unexpected::StructVariant, &self));
-                }
-                tag
-            },
-            None => return Err(A::Error::invalid_length(0, &self)),
-        };
-
-        // Based on the validated variant tag, we know the field types
-        for (i, field_type) in self.0[variant_tag].iter().enumerate() {
-            match seq.next_element_seed(field_type)? {
-                Some(elem) => val.push(elem),
-                None => return Err(A::Error::invalid_length(i, &self)),
+        let (tag, rest) = data.variant()?;
+        if tag as usize >= self.0.len() {
+            Err(A::Error::invalid_length(0, &self))
+        } else {
+            let fields = &self.0[tag as usize];
+            match fields.len() {
+                0 => {
+                    rest.unit_variant()?;
+                    Ok((tag, vec![]))
+                },
+                1 => {
+                    let value = rest.newtype_variant_seed(&fields[0])?;
+                    Ok((tag, vec![value]))
+                },
+                _ => {
+                    let values = rest.tuple_variant(fields.len(), StructFieldVisitor(fields))?;
+                    Ok((tag, values))
+                },
             }
         }
-        Ok((variant_tag as u16, val))
     }
 }
 
@@ -579,7 +618,15 @@ impl<'d> serde::de::DeserializeSeed<'d> for &MoveStructLayout {
                 Ok(MoveStruct::Runtime(fields))
             },
             MoveStructLayout::RuntimeVariants(variants) => {
-                let (tag, fields) = deserializer.deserialize_seq(StructVariantVisitor(variants))?;
+                if variants.len() > (u16::MAX as usize) {
+                    return Err(D::Error::custom("variant count out of range"));
+                }
+                let variant_names = variant_name_placeholder(variants.len());
+                let (tag, fields) = deserializer.deserialize_enum(
+                    MOVE_ENUM_NAME,
+                    variant_names,
+                    StructVariantVisitor(variants),
+                )?;
                 Ok(MoveStruct::RuntimeVariant(tag, fields))
             },
             MoveStructLayout::WithFields(layout) => {
@@ -600,14 +647,19 @@ impl<'d> serde::de::DeserializeSeed<'d> for &MoveStructLayout {
             },
             MoveStructLayout::WithVariants(decorated_variants) => {
                 // Downgrade the decorated variants to simple layouts to deserialize the fields.
-                let (tag, fields) = deserializer.deserialize_seq(StructVariantVisitor(
-                    &decorated_variants
-                        .iter()
-                        .map(|v| v.fields.iter().map(|f| f.layout.clone()).collect())
-                        .collect::<Vec<_>>(),
-                ))?;
+                let variant_names = variant_name_placeholder(decorated_variants.len());
+                let (tag, fields) = deserializer.deserialize_enum(
+                    MOVE_ENUM_NAME,
+                    variant_names,
+                    StructVariantVisitor(
+                        &decorated_variants
+                            .iter()
+                            .map(|v| v.fields.iter().map(|f| f.layout.clone()).collect())
+                            .collect::<Vec<_>>(),
+                    ),
+                )?;
                 // Now decorate the raw value. This is not optimally efficient but
-                // decorated values should not be in the serving path.
+                // decorated values should not be in the execution path.
                 Ok(MoveStruct::RuntimeVariant(tag, fields).decorate(self))
             },
         }
@@ -653,21 +705,41 @@ impl<'a> serde::Serialize for MoveFields<'a> {
 impl serde::Serialize for MoveStruct {
     fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
         match self {
-            Self::Runtime(s) => {
-                let mut t = serializer.serialize_tuple(s.len())?;
-                for v in s.iter() {
+            Self::Runtime(values) => {
+                let mut t = serializer.serialize_tuple(values.len())?;
+                for v in values.iter() {
                     t.serialize_element(v)?;
                 }
                 t.end()
             },
-            Self::RuntimeVariant(tag, s) => {
+            Self::RuntimeVariant(tag, values) => {
                 // Variants need to be serialized as sequences, as the size is not statically known.
-                let mut t = serializer.serialize_seq(Some(s.len() + 1))?;
-                t.serialize_element(tag)?;
-                for v in s.iter() {
-                    t.serialize_element(v)?;
+                let tag_idx = *tag as usize;
+                let variant_tag = tag_idx as u32;
+                let variant_name = variant_name_placeholder((tag + 1) as usize)[tag_idx];
+                match values.len() {
+                    0 => {
+                        serializer.serialize_unit_variant(MOVE_ENUM_NAME, variant_tag, variant_name)
+                    },
+                    1 => serializer.serialize_newtype_variant(
+                        MOVE_ENUM_NAME,
+                        variant_tag,
+                        variant_name,
+                        &values[0],
+                    ),
+                    _ => {
+                        let mut t = serializer.serialize_tuple_variant(
+                            MOVE_ENUM_NAME,
+                            variant_tag,
+                            variant_name,
+                            values.len(),
+                        )?;
+                        for v in values {
+                            t.serialize_field(v)?
+                        }
+                        t.end()
+                    },
                 }
-                t.end()
             },
             Self::WithFields(fields) => MoveFields(fields).serialize(serializer),
             Self::WithTypes { type_, fields } => {
diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs
index 2de96efc5e855..006c16d6b9dd1 100644
--- a/third_party/move/move-vm/types/src/values/values_impl.rs
+++ b/third_party/move/move-vm/types/src/values/values_impl.rs
@@ -17,7 +17,7 @@ use move_core_types::{
     account_address::AccountAddress,
     effects::Op,
     gas_algebra::AbstractMemorySize,
-    u256,
+    u256, value,
     value::{MoveStructLayout, MoveTypeLayout},
     vm_status::{sub_status::NFE_VECTOR_ERROR_BASE, StatusCode},
 };
@@ -3104,8 +3104,8 @@ pub mod debug {
 use crate::value_serde::{CustomDeserializer, CustomSerializer, RelaxedCustomSerDe};
 use move_binary_format::file_format::VariantIndex;
 use serde::{
-    de::{Error as DeError, Unexpected},
-    ser::{Error as SerError, SerializeSeq, SerializeTuple},
+    de::{EnumAccess, Error as DeError, Unexpected, VariantAccess},
+    ser::{Error as SerError, SerializeSeq, SerializeTuple, SerializeTupleVariant},
     Deserialize,
 };
 
@@ -3271,30 +3271,51 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize
     for SerializationReadyValue<'c, 'l, 'v, MoveStructLayout, Vec<ValueImpl>, C>
 {
     fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-        let values = self.value.as_slice();
+        let mut values = self.value.as_slice();
         if let Some((tag, variant_layouts)) = try_get_variant_field_layouts(self.layout, values) {
-            // Variants need to be serialized as sequences, as the deserializer cannot know
-            // their static size.
-            let mut t = serializer.serialize_seq(Some(values.len()))?;
-            t.serialize_element(&SerializationReadyValue {
-                custom_serializer: self.custom_serializer,
-                layout: &MoveTypeLayout::U16,
-                value: &ValueImpl::U16(tag),
-            })?;
-            if variant_layouts.len() != values.len() - 1 {
+            let tag_idx = tag as usize;
+            let variant_tag = tag_idx as u32;
+            let variant_name = value::variant_name_placeholder((tag + 1) as usize)[tag_idx];
+            values = &values[1..];
+            if variant_layouts.len() != values.len() {
                 return Err(invariant_violation::<S>(format!(
-                    "cannot serialize variant value {:?} as {:?} -- number of fields mismatch",
+                    "cannot serialize struct value {:?} as {:?} -- number of fields mismatch",
                     self.value, self.layout
                 )));
             }
-            for (field_layout, value) in variant_layouts.iter().zip(values.iter().skip(1)) {
-                t.serialize_element(&SerializationReadyValue {
-                    custom_serializer: self.custom_serializer,
-                    layout: field_layout,
-                    value,
-                })?;
+            match values.len() {
+                0 => serializer.serialize_unit_variant(
+                    value::MOVE_ENUM_NAME,
+                    variant_tag,
+                    variant_name,
+                ),
+                1 => serializer.serialize_newtype_variant(
+                    value::MOVE_ENUM_NAME,
+                    variant_tag,
+                    variant_name,
+                    &SerializationReadyValue {
+                        custom_serializer: self.custom_serializer,
+                        layout: &variant_layouts[0],
+                        value: &values[0],
+                    },
+                ),
+                _ => {
+                    let mut t = serializer.serialize_tuple_variant(
+                        value::MOVE_ENUM_NAME,
+                        variant_tag,
+                        variant_name,
+                        values.len(),
+                    )?;
+                    for (layout, value) in variant_layouts.iter().zip(values) {
+                        t.serialize_field(&SerializationReadyValue {
+                            custom_serializer: self.custom_serializer,
+                            layout,
+                            value,
+                        })?
+                    }
+                    t.end()
+                },
             }
-            t.end()
         } else {
             let field_layouts = self.layout.fields(None);
             let mut t = serializer.serialize_tuple(values.len())?;
@@ -3421,11 +3442,16 @@ impl<'d, C: CustomDeserializer> serde::de::DeserializeSeed<'d>
                 )?;
                 Ok(Struct::pack(fields))
             },
-            MoveStructLayout::RuntimeVariants(variant_layouts) => {
-                let fields = deserializer.deserialize_seq(StructVariantVisitor(
-                    self.custom_deserializer,
-                    variant_layouts,
-                ))?;
+            MoveStructLayout::RuntimeVariants(variants) => {
+                if variants.len() > (u16::MAX as usize) {
+                    return Err(D::Error::custom("variant count out of range"));
+                }
+                let variant_names = value::variant_name_placeholder(variants.len());
+                let fields = deserializer.deserialize_enum(
+                    value::MOVE_ENUM_NAME,
+                    variant_names,
+                    StructVariantVisitor(self.custom_deserializer, variants),
+                )?;
                 Ok(Struct::pack(fields))
             },
             MoveStructLayout::WithFields(_)
@@ -3498,6 +3524,39 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructVariant
         formatter.write_str("Variant")
     }
 
+    fn visit_enum<A>(self, data: A) -> Result<Self::Value, A::Error>
+    where
+        A: EnumAccess<'d>,
+    {
+        let (tag, rest) = data.variant()?;
+        if tag as usize >= self.1.len() {
+            Err(A::Error::invalid_length(0, &self))
+        } else {
+            let mut values = vec![Value::u16(tag)];
+            let fields = &self.1[tag as usize];
+            match fields.len() {
+                0 => {
+                    rest.unit_variant()?;
+                    Ok(values)
+                },
+                1 => {
+                    values.push(rest.newtype_variant_seed(DeserializationSeed {
+                        custom_deserializer: self.0,
+                        layout: &fields[0],
+                    })?);
+                    Ok(values)
+                },
+                _ => {
+                    values.append(
+                        &mut rest
+                            .tuple_variant(fields.len(), StructFieldVisitor(self.0, fields))?,
+                    );
+                    Ok(values)
+                },
+            }
+        }
+    }
+
     fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
     where
         A: serde::de::SeqAccess<'d>,


Test patch:
diff --git a/aptos-move/e2e-benchmark/src/main.rs b/aptos-move/e2e-benchmark/src/main.rs
index fc5600133bfb9..a00e511eaea89 100644
--- a/aptos-move/e2e-benchmark/src/main.rs
+++ b/aptos-move/e2e-benchmark/src/main.rs
@@ -123,8 +123,8 @@ fn main() {
         (29000, EntryPoints::InitializeVectorPicture {
             length: 30 * 1024,
         }),
-        (4510, EntryPoints::VectorPicture { length: 30 * 1024 }),
-        (4400, EntryPoints::VectorPictureRead { length: 30 * 1024 }),
+        (5900, EntryPoints::VectorPicture { length: 30 * 1024 }),
+        (5870, EntryPoints::VectorPictureRead { length: 30 * 1024 }),
         (33580, EntryPoints::SmartTablePicture {
             length: 30 * 1024,
             num_points_per_txn: 200,
diff --git a/testsuite/single_node_performance.py b/testsuite/single_node_performance.py
index 647974673b4bf..92dd74775d400 100755
--- a/testsuite/single_node_performance.py
+++ b/testsuite/single_node_performance.py
@@ -153,7 +153,7 @@ class RunGroupConfig:
 mix_publish_transfer	1	VM	0.967	1.163	2173.8
 batch100-transfer	1	VM	0.859	1.021	667.8
 batch100-transfer	1	native	0.784	1.222	1862.1
-vector-picture30k	1	VM	0.935	1.023	138.5
+vector-picture30k	1	VM	0.935	1.023	110.0
 vector-picture30k	20	VM	0.819	1.038	1347.9
 smart-table-picture30-k-with200-change	1	VM	0.959	1.054	21.4
 smart-table-picture30-k-with200-change	20	VM	0.923	1.057	182.6
diff --git a/third_party/move/move-vm/types/src/values/serialization_tests.rs b/third_party/move/move-vm/types/src/values/serialization_tests.rs
index d3223a32fc6b9..6085a50183e2c 100644
--- a/third_party/move/move-vm/types/src/values/serialization_tests.rs
+++ b/third_party/move/move-vm/types/src/values/serialization_tests.rs
@@ -2,11 +2,11 @@
 // SPDX-License-Identifier: Apache-2.0
 
 //! Contains tests for serialization
-//!
+
 use crate::{
     delayed_values::delayed_field_id::DelayedFieldID,
     value_serde::{serialize_and_allow_delayed_values, serialized_size_allowing_delayed_values},
-    values::{Struct as StructValue, Value},
+    values::{values_impl, Struct, Value},
 };
 use claims::{assert_err, assert_ok, assert_some};
 use move_core_types::{
@@ -14,14 +14,23 @@ use move_core_types::{
     u256,
     value::{IdentifierMappingKind, MoveStruct, MoveStructLayout, MoveTypeLayout, MoveValue},
 };
+use serde::{Deserialize, Serialize};
+use std::iter;
 
-#[test]
-fn enum_round_trip() {
-    let layout = MoveTypeLayout::Struct(MoveStructLayout::RuntimeVariants(vec![
+fn test_layout() -> MoveTypeLayout {
+    MoveTypeLayout::Struct(MoveStructLayout::RuntimeVariants(vec![
         vec![MoveTypeLayout::U64],
         vec![],
         vec![MoveTypeLayout::Bool, MoveTypeLayout::U32],
-    ]));
+    ]))
+}
+
+// ---------------------------------------------------------------------------
+// Serialization round trip tests
+
+#[test]
+fn enum_round_trip_move_value() {
+    let layout = test_layout();
     let good_values = vec![
         MoveValue::Struct(MoveStruct::RuntimeVariant(0, vec![MoveValue::U64(42)])),
         MoveValue::Struct(MoveStruct::RuntimeVariant(1, vec![])),
@@ -43,7 +52,7 @@ fn enum_round_trip() {
     MoveValue::simple_deserialize(&blob, &layout)
         .inspect_err(|e| {
             assert!(
-                e.to_string().contains("invalid value"),
+                e.to_string().contains("invalid length"),
                 "unexpected error message: {}",
                 e
             );
@@ -56,7 +65,7 @@ fn enum_round_trip() {
     MoveValue::simple_deserialize(&blob, &layout)
         .inspect_err(|e| {
             assert!(
-                e.to_string().contains("end of input"),
+                e.to_string().contains("invalid length"),
                 "unexpected error message: {}",
                 e
             );
@@ -64,6 +73,111 @@ fn enum_round_trip() {
         .expect_err("bad struct value deserialization fails");
 }
 
+#[test]
+fn enum_round_trip_vm_value() {
+    let layout = test_layout();
+    let good_values = vec![
+        Value::struct_(Struct::pack_variant(0, iter::once(Value::u64(42)))),
+        Value::struct_(Struct::pack_variant(1, iter::empty())),
+        Value::struct_(Struct::pack_variant(
+            2,
+            [Value::bool(true), Value::u32(13)].into_iter(),
+        )),
+    ];
+    for value in good_values {
+        let blob = value
+            .simple_serialize(&layout)
+            .expect("serialization succeeds");
+        let de_value = Value::simple_deserialize(&blob, &layout).expect("deserialization succeeds");
+        assert!(
+            value.equals(&de_value).unwrap(),
+            "roundtrip serialization succeeds"
+        )
+    }
+    let bad_tag_value = Value::struct_(Struct::pack_variant(3, [Value::u64(42)]));
+    assert!(
+        bad_tag_value.simple_serialize(&layout).is_none(),
+        "serialization fails"
+    );
+    let bad_struct_value = Value::struct_(Struct::pack([Value::u64(42)]));
+    assert!(
+        bad_struct_value.simple_serialize(&layout).is_none(),
+        "serialization fails"
+    );
+}
+
+// ---------------------------------------------------------------------------
+// Rust cross-serialization tests
+
+#[derive(Serialize, Deserialize, PartialEq, Eq, Debug)]
+pub enum RustEnum {
+    Number(u64),
+    Empty,
+    BoolNumber(bool, u32),
+}
+
+#[test]
+fn enum_rust_round_trip_move_value() {
+    let layout = test_layout();
+    let move_values = vec![
+        MoveValue::Struct(MoveStruct::RuntimeVariant(0, vec![MoveValue::U64(42)])),
+        MoveValue::Struct(MoveStruct::RuntimeVariant(1, vec![])),
+        MoveValue::Struct(MoveStruct::RuntimeVariant(2, vec![
+            MoveValue::Bool(true),
+            MoveValue::U32(13),
+        ])),
+    ];
+    let rust_values = vec![
+        RustEnum::Number(42),
+        RustEnum::Empty,
+        RustEnum::BoolNumber(true, 13),
+    ];
+    for (move_value, rust_value) in move_values.into_iter().zip(rust_values) {
+        let from_move = move_value.simple_serialize().expect("from move succeeds");
+        let to_rust = bcs::from_bytes::<RustEnum>(&from_move).expect("to rust successful");
+        assert_eq!(to_rust, rust_value);
+
+        let from_rust = bcs::to_bytes(&rust_value).expect("from rust succeeds");
+        let to_move = MoveValue::simple_deserialize(&from_rust, &layout).expect("to move succeeds");
+        assert_eq!(to_move, move_value)
+    }
+}
+
+#[test]
+fn enum_rust_round_trip_vm_value() {
+    let layout = test_layout();
+    let move_values = vec![
+        Value::struct_(Struct::pack_variant(0, iter::once(Value::u64(42)))),
+        Value::struct_(Struct::pack_variant(1, iter::empty())),
+        Value::struct_(Struct::pack_variant(
+            2,
+            [Value::bool(true), Value::u32(13)].into_iter(),
+        )),
+    ];
+    let rust_values = vec![
+        RustEnum::Number(42),
+        RustEnum::Empty,
+        RustEnum::BoolNumber(true, 13),
+    ];
+    for (move_value, rust_value) in move_values.into_iter().zip(rust_values) {
+        let from_move = move_value
+            .simple_serialize(&layout)
+            .expect("from move succeeds");
+        let to_rust = bcs::from_bytes::<RustEnum>(&from_move).expect("to rust successful");
+        assert_eq!(to_rust, rust_value);
+
+        let from_rust = bcs::to_bytes(&rust_value).expect("from rust succeeds");
+        let to_move = Value::simple_deserialize(&from_rust, &layout).expect("to move succeeds");
+        assert!(
+            to_move.equals(&move_value).unwrap(),
+            "from rust to move failed"
+        )
+    }
+}
+
+// --------------------------------------------------------------------------
+// Serialization size tests
+
 #[test]
 fn test_serialized_size() {
     use IdentifierMappingKind::*;
@@ -102,7 +216,7 @@ fn test_serialized_size() {
             Vector(Box::new(Address)),
         ),
         (
-            Value::struct_(StructValue::pack(vec![
+            Value::struct_(values_impl::Struct::pack(vec![
                 Value::bool(true),
                 Value::vector_u32(vec![1, 2, 3, 4, 5]),
             ])),