addressing comments

aptos-move/aptos-gas-schedule/src/gas_schedule/move_stdlib.rs

@@ -8,7 +8,8 @@ use crate::{
     gas_schedule::NativeGasParameters,
 };
 use aptos_gas_algebra::{
-    InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerTypeNode, InternalGasPerArg, InternalGasPerByte,
+    InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerArg, InternalGasPerByte,
+    InternalGasPerTypeNode,
 };
 
 crate::gas_schedule::macros::define_gas_parameters!(

aptos-move/framework/move-stdlib/Cargo.toml

@@ -14,6 +14,7 @@ publish = false
 aptos-gas-schedule = { workspace = true }
 aptos-native-interface = { workspace = true }
 aptos-types = { workspace = true }
+bcs = { workspace = true }
 move-core-types = { path = "../../../third_party/move/move-core/types" }
 move-vm-runtime = { path = "../../../third_party/move/move-vm/runtime" }
 move-vm-types = { path = "../../../third_party/move/move-vm/types" }

aptos-move/framework/move-stdlib/doc/bcs.md

@@ -80,7 +80,7 @@ Aborts with <code>0x1c5</code> error code if there is a failure when calculating
 Note:
 For some types it might not be known they have constant size, and function might return None.
 For example, signer appears to have constant size, but it's size might change.
-If this function returned Some() for some type before - it is guaranteed to continue returning Some()
+If this function returned Some() for some type before - it is guaranteed to continue returning Some().
 On the other hand, if function has returned None for some type,
 it might change in the future to return Some() instead, if size becomes "known".
 
@@ -94,7 +94,7 @@ it might change in the future to return Some() instead, if size becomes "known".
 <summary>Implementation</summary>
 
 
-<pre><code><b>native</b> <b>public</b>(<b>friend</b>) <b>fun</b> <a href="bcs.md#0x1_bcs_constant_serialized_size">constant_serialized_size</a>&lt;MoveValue&gt;(): std::option::Option&lt;u64&gt;;
+<pre><code><b>native</b> <b>public</b>(<b>friend</b>) <b>fun</b> <a href="bcs.md#0x1_bcs_constant_serialized_size">constant_serialized_size</a>&lt;MoveValue&gt;(): Option&lt;u64&gt;;
 </code></pre>
 
 

aptos-move/framework/move-stdlib/sources/bcs.move

@@ -3,6 +3,8 @@
 /// published on-chain. See https://github.com/aptos-labs/bcs#binary-canonical-serialization-bcs for more
 /// details on BCS.
 module std::bcs {
+    use std::option::Option;
+
     /// Returns the binary representation of `v` in BCS (Binary Canonical Serialization) format.
     /// Aborts with `0x1c5` error code if serialization fails.
     native public fun to_bytes<MoveValue>(v: &MoveValue): vector<u8>;
@@ -23,10 +25,10 @@ module std::bcs {
     /// Note:
     /// For some types it might not be known they have constant size, and function might return None.
     /// For example, signer appears to have constant size, but it's size might change.
-    /// If this function returned Some() for some type before - it is guaranteed to continue returning Some()
+    /// If this function returned Some() for some type before - it is guaranteed to continue returning Some().
     /// On the other hand, if function has returned None for some type,
     /// it might change in the future to return Some() instead, if size becomes "known".
-    native public(friend) fun constant_serialized_size<MoveValue>(): std::option::Option<u64>;
+    native public(friend) fun constant_serialized_size<MoveValue>(): Option<u64>;
 
     // ==============================
     // Module Specification

aptos-move/framework/move-stdlib/src/natives/bcs.rs

@@ -11,17 +11,17 @@ use aptos_native_interface::{
     SafeNativeResult,
 };
 use move_core_types::{
+    account_address::AccountAddress,
     gas_algebra::{NumBytes, NumTypeNodes},
-    vm_status::sub_status::NFE_BCS_SERIALIZATION_FAILURE,
+    u256,
+    value::{MoveStructLayout, MoveTypeLayout},
+    vm_status::{sub_status::NFE_BCS_SERIALIZATION_FAILURE, StatusCode},
 };
 use move_vm_runtime::native_functions::NativeFunction;
 use move_vm_types::{
     loaded_data::runtime_types::Type,
-    natives::function::PartialVMResult,
-    value_serde::{
-        constant_serialized_size, serialized_size_allowing_delayed_values,
-        type_visit_count_for_constant_serialized_size,
-    },
+    natives::function::{PartialVMError, PartialVMResult},
+    value_serde::serialized_size_allowing_delayed_values,
     values::{values_impl::Reference, Struct, Value},
 };
 use smallvec::{smallvec, SmallVec};
@@ -166,6 +166,106 @@ fn native_constant_serialized_size(
     Ok(smallvec![result])
 }
 
+/// Count upper limit on the number of types constant_serialized_size would visit,
+/// which is then used for gas charging, before performing the operation.
+/// This is different done type.num_nodes(), as some types are not traversed (i.e. vector),
+/// and for structs types and number of fields matter as well.
+///
+/// Unclear if type_visit_count would be the same for other usages
+/// (for example, whether vector types need to be traversed),
+/// so name it very specifically, and on future usages see how it generalizes.
+fn type_visit_count_for_constant_serialized_size(ty_layout: &MoveTypeLayout) -> u64 {
+    match ty_layout {
+        MoveTypeLayout::Bool
+        | MoveTypeLayout::U8
+        | MoveTypeLayout::U16
+        | MoveTypeLayout::U32
+        | MoveTypeLayout::U128
+        | MoveTypeLayout::U256
+        | MoveTypeLayout::U64
+        | MoveTypeLayout::Address
+        | MoveTypeLayout::Signer => 1,
+        // non-recursed:
+        MoveTypeLayout::Struct(
+            MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_),
+        )
+        | MoveTypeLayout::Vector(_) => 1,
+        // recursed:
+        MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => {
+            let mut total = 1; // Count the current visit, and aggregate all children
+            for field in fields {
+                total += type_visit_count_for_constant_serialized_size(field);
+            }
+            total
+        },
+        MoveTypeLayout::Struct(MoveStructLayout::WithFields(fields))
+        | MoveTypeLayout::Struct(MoveStructLayout::WithTypes { fields, .. }) => {
+            let mut total = 1; // Count the current visit, and aggregate all children
+            for field in fields {
+                total += type_visit_count_for_constant_serialized_size(&field.layout);
+            }
+            total
+        },
+        // Count the current visit, and inner visits
+        MoveTypeLayout::Native(_, inner) => {
+            1 + type_visit_count_for_constant_serialized_size(inner)
+        },
+    }
+}
+
+/// If given type has a constant serialized size (irrespective of the instance), it returns the serialized
+/// size in bytes any value would have.
+/// Otherwise it returns None.
+fn constant_serialized_size(ty_layout: &MoveTypeLayout) -> PartialVMResult<Option<usize>> {
+    let bcs_size_result = match ty_layout {
+        MoveTypeLayout::Bool => bcs::serialized_size(&false).map(Some),
+        MoveTypeLayout::U8 => bcs::serialized_size(&0u8).map(Some),
+        MoveTypeLayout::U16 => bcs::serialized_size(&0u16).map(Some),
+        MoveTypeLayout::U32 => bcs::serialized_size(&0u32).map(Some),
+        MoveTypeLayout::U64 => bcs::serialized_size(&0u64).map(Some),
+        MoveTypeLayout::U128 => bcs::serialized_size(&0u128).map(Some),
+        MoveTypeLayout::U256 => bcs::serialized_size(&u256::U256::zero()).map(Some),
+        MoveTypeLayout::Address => bcs::serialized_size(&AccountAddress::ZERO).map(Some),
+        // signer's size is VM implementation detail, and can change at will.
+        MoveTypeLayout::Signer => Ok(None),
+        // vectors have no constant size
+        MoveTypeLayout::Vector(_) => Ok(None),
+        // enums have no constant size
+        MoveTypeLayout::Struct(
+            MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_),
+        ) => Ok(None),
+        MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => {
+            let mut total = Some(0);
+            for field in fields {
+                let cur = constant_serialized_size(field)?;
+                match cur {
+                    Some(cur_value) => total = total.map(|v| v + cur_value),
+                    None => {
+                        total = None;
+                        break;
+                    },
+                }
+            }
+            Ok(total)
+        },
+        MoveTypeLayout::Struct(MoveStructLayout::WithFields(_))
+        | MoveTypeLayout::Struct(MoveStructLayout::WithTypes { .. }) => {
+            return Err(
+                PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(
+                    "Only runtime types expected, but found WithFields/WithTypes".to_string(),
+                ),
+            )
+        },
+        MoveTypeLayout::Native(_, inner) => Ok(constant_serialized_size(inner)?),
+    };
+    bcs_size_result.map_err(|e| {
+        PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!(
+            "failed to compute serialized size of a value: {:?}",
+            e
+        ))
+    })
+}
+
 /***************************************************************************************************
  * module
  **************************************************************************************************/

third_party/move/move-vm/types/src/value_serde.rs

@@ -9,9 +9,7 @@ use crate::{
 };
 use move_binary_format::errors::{PartialVMError, PartialVMResult};
 use move_core_types::{
-    account_address::AccountAddress,
-    u256,
-    value::{IdentifierMappingKind, MoveStructLayout, MoveTypeLayout},
+    value::{IdentifierMappingKind, MoveTypeLayout},
     vm_status::StatusCode,
 };
 use serde::{
@@ -174,112 +172,6 @@ pub fn serialized_size_allowing_delayed_values(
     })
 }
 
-/// Count number of types constant_serialized_size would visit, used for gas charging.
-/// This is different done type.num_nodes(), as some types are not traversed (i.e. vector),
-/// and for structs types and number of fields matter as well.
-///
-/// Unclear if type_visit_count would be the same for other usages
-/// (for example, whether vector types need to be traversed),
-/// so name it very specifically, and on future usages see how it generalizes.
-pub fn type_visit_count_for_constant_serialized_size(ty_layout: &MoveTypeLayout) -> u64 {
-    match ty_layout {
-        MoveTypeLayout::Bool
-        | MoveTypeLayout::U8
-        | MoveTypeLayout::U16
-        | MoveTypeLayout::U32
-        | MoveTypeLayout::U128
-        | MoveTypeLayout::U256
-        | MoveTypeLayout::U64
-        | MoveTypeLayout::Address
-        | MoveTypeLayout::Signer => 1,
-        // non-recursed:
-        MoveTypeLayout::Struct(
-            MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_),
-        )
-        | MoveTypeLayout::Vector(_) => 1,
-        // recursed:
-        MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => {
-            let mut total = 1; // Count the current visit, and aggregate all children
-            for field in fields {
-                total += type_visit_count_for_constant_serialized_size(field);
-            }
-            total
-        },
-        MoveTypeLayout::Struct(MoveStructLayout::WithFields(fields))
-        | MoveTypeLayout::Struct(MoveStructLayout::WithTypes { fields, .. }) => {
-            let mut total = 1; // Count the current visit, and aggregate all children
-            for field in fields {
-                total += type_visit_count_for_constant_serialized_size(&field.layout);
-            }
-            total
-        },
-        // Count the current visit, and inner visits
-        MoveTypeLayout::Native(_, inner) => {
-            1 + type_visit_count_for_constant_serialized_size(inner)
-        },
-    }
-}
-
-/// If given type has a constant serialized size (irrespective of the instance), it returns the serialized
-/// size in bytes any value would have.
-/// Otherwise it returns None.
-pub fn constant_serialized_size(ty_layout: &MoveTypeLayout) -> PartialVMResult<Option<usize>> {
-    let bcs_size_result = match ty_layout {
-        MoveTypeLayout::Bool => bcs::serialized_size(&false).map(Some),
-        MoveTypeLayout::U8 => bcs::serialized_size(&0u8).map(Some),
-        MoveTypeLayout::U16 => bcs::serialized_size(&0u16).map(Some),
-        MoveTypeLayout::U32 => bcs::serialized_size(&0u32).map(Some),
-        MoveTypeLayout::U64 => bcs::serialized_size(&0u64).map(Some),
-        MoveTypeLayout::U128 => bcs::serialized_size(&0u128).map(Some),
-        MoveTypeLayout::U256 => bcs::serialized_size(&u256::U256::zero()).map(Some),
-        MoveTypeLayout::Address => bcs::serialized_size(&AccountAddress::ZERO).map(Some),
-        // signer's size is VM implementation detail, and can change at will.
-        MoveTypeLayout::Signer => Ok(None),
-        // vectors have no constant size
-        MoveTypeLayout::Vector(_) => Ok(None),
-        // enums have no constant size
-        MoveTypeLayout::Struct(
-            MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_),
-        ) => Ok(None),
-        MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => {
-            let mut total = Some(0);
-            for field in fields {
-                let cur = constant_serialized_size(field)?;
-                match cur {
-                    Some(cur_value) => total = total.map(|v| v + cur_value),
-                    None => {
-                        total = None;
-                        break;
-                    },
-                }
-            }
-            Ok(total)
-        },
-        MoveTypeLayout::Struct(MoveStructLayout::WithFields(fields))
-        | MoveTypeLayout::Struct(MoveStructLayout::WithTypes { fields, .. }) => {
-            let mut total = Some(0);
-            for field in fields {
-                let cur = constant_serialized_size(&field.layout)?;
-                match cur {
-                    Some(cur_value) => total = total.map(|v| v + cur_value),
-                    None => {
-                        total = None;
-                        break;
-                    },
-                }
-            }
-            Ok(total)
-        },
-        MoveTypeLayout::Native(_, inner) => Ok(constant_serialized_size(inner)?),
-    };
-    bcs_size_result.map_err(|e| {
-        PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!(
-            "failed to compute serialized size of a value: {:?}",
-            e
-        ))
-    })
-}
-
 /// Allow conversion between values and identifiers (delayed values). For example,
 /// this trait can be implemented to fetch a concrete Move value from the global
 /// state based on the identifier stored inside a delayed value.