Revert "[move] Visitor trait for annotated Move values (#16994) " (#1…

…7002)

Reverts #16995

crates/data-transform/src/main.rs

@@ -9,9 +9,9 @@ use once_cell::sync::Lazy;
 use std::process::exit;
 use std::str::FromStr;
 use std::sync::Arc;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 
 use move_bytecode_utils::module_cache::SyncModuleCache;
+use move_core_types::annotated_value::MoveStruct;
 use sui_types::object::MoveObject;
 
 use self::models::*;
@@ -286,7 +286,7 @@ fn main() {
 
                 match layout {
                     Ok(l) => {
-                        let move_object = BoundedVisitor::deserialize_struct(&event.event_bcs, &l)
+                        let move_object = MoveStruct::simple_deserialize(&event.event_bcs, &l)
                             .map_err(|e| IndexerError::SerdeError(e.to_string()));
 
                         match move_object {

crates/sui-analytics-indexer/src/handlers/mod.rs

@@ -12,7 +12,6 @@ use sui_package_resolver::{PackageStore, Resolver};
 use sui_types::base_types::ObjectID;
 use sui_types::effects::TransactionEffects;
 use sui_types::effects::TransactionEffectsAPI;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::object::{Object, Owner};
 use sui_types::transaction::TransactionData;
 use sui_types::transaction::TransactionDataAPI;
@@ -175,7 +174,7 @@ async fn get_move_struct<T: PackageStore>(
         .await?
     {
         MoveTypeLayout::Struct(move_struct_layout) => {
-            BoundedVisitor::deserialize_struct(contents, &move_struct_layout)
+            MoveStruct::simple_deserialize(contents, &move_struct_layout)
         }
         _ => Err(anyhow!("Object is not a move struct")),
     }?;

crates/sui-core/src/unit_tests/subscription_handler_tests.rs

@@ -5,7 +5,7 @@ use move_core_types::account_address::AccountAddress;
 use move_core_types::identifier::Identifier;
 
 use move_core_types::{
-    annotated_value::{MoveFieldLayout, MoveStructLayout, MoveTypeLayout},
+    annotated_value::{MoveFieldLayout, MoveStruct, MoveStructLayout, MoveTypeLayout},
     ident_str,
     language_storage::StructTag,
 };
@@ -17,7 +17,6 @@ use sui_json_rpc_types::SuiMoveStruct;
 
 use sui_types::base_types::ObjectID;
 use sui_types::gas_coin::GasCoin;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::{MOVE_STDLIB_ADDRESS, SUI_FRAMEWORK_ADDRESS};
 
 #[test]
@@ -34,7 +33,7 @@ fn test_to_json_value() {
     };
     let event_bytes = bcs::to_bytes(&move_event).unwrap();
     let sui_move_struct: SuiMoveStruct =
-        BoundedVisitor::deserialize_struct(&event_bytes, &TestEvent::layout())
+        MoveStruct::simple_deserialize(&event_bytes, &TestEvent::layout())
             .unwrap()
             .into();
     let json_value = sui_move_struct.to_json_value();

crates/sui-graphql-rpc/src/types/move_value.rs

@@ -10,7 +10,6 @@ use move_core_types::{
 };
 use serde::{Deserialize, Serialize};
 use sui_package_resolver::Resolver;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 
 use crate::context_data::package_cache::PackageCache;
 use crate::{error::Error, types::json::Json, types::move_type::unexpected_signer_error};
@@ -124,8 +123,9 @@ impl MoveValue {
     }
 
     fn value_impl(&self, layout: A::MoveTypeLayout) -> Result<A::MoveValue, Error> {
-        // TODO (annotated-visitor): deserializing directly using a custom visitor.
-        BoundedVisitor::deserialize_value(&self.bcs.0[..], &layout).map_err(|_| {
+        // TODO: If this becomes a performance bottleneck, it can be made more efficient by not
+        // deserializing via `value::MoveValue` (but this is significantly more code).
+        bcs::from_bytes_seed(&layout, &self.bcs.0[..]).map_err(|_| {
             let type_tag: TypeTag = (&layout).into();
             Error::Internal(format!(
                 "Failed to deserialize Move value for type: {}",

crates/sui-graphql-rpc/src/types/object.rs

@@ -41,7 +41,6 @@ use sui_indexer::schema::{objects, objects_history, objects_snapshot};
 use sui_indexer::types::ObjectStatus as NativeObjectStatus;
 use sui_indexer::types::OwnerType;
 use sui_package_resolver::Resolver;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::object::{
     MoveObject as NativeMoveObject, Object as NativeObject, Owner as NativeOwner,
 };
@@ -1353,9 +1352,7 @@ pub(crate) async fn deserialize_move_struct(
         return Err(Error::Internal("Object is not a move struct".to_string()));
     };
 
-    // TODO (annotated-visitor): Use custom visitors for extracting a dynamic field, and for
-    // creating a GraphQL MoveValue directly (not via an annotated visitor).
-    let move_struct = BoundedVisitor::deserialize_struct(contents, &layout).map_err(|e| {
+    let move_struct = MoveStruct::simple_deserialize(contents, &layout).map_err(|e| {
         Error::Internal(format!(
             "Error deserializing move struct for type {}: {e}",
             struct_tag.to_canonical_string(/* with_prefix */ true)

crates/sui-indexer/src/models/events.rs

@@ -5,13 +5,13 @@ use std::str::FromStr;
 
 use diesel::prelude::*;
 use move_bytecode_utils::module_cache::GetModule;
+use move_core_types::annotated_value::MoveStruct;
 use move_core_types::identifier::Identifier;
 
 use sui_json_rpc_types::{SuiEvent, SuiMoveStruct};
 use sui_types::base_types::{ObjectID, SuiAddress};
 use sui_types::digests::TransactionDigest;
 use sui_types::event::EventID;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::object::MoveObject;
 use sui_types::parse_sui_struct_tag;
 
@@ -108,7 +108,7 @@ impl StoredEvent {
         let type_ = parse_sui_struct_tag(&self.event_type)?;
 
         let layout = MoveObject::get_layout_from_struct_tag(type_.clone(), module_cache)?;
-        let move_object = BoundedVisitor::deserialize_struct(&self.bcs, &layout)
+        let move_object = MoveStruct::simple_deserialize(&self.bcs, &layout)
             .map_err(|e| IndexerError::SerdeError(e.to_string()))?;
         let parsed_json = SuiMoveStruct::from(move_object).to_json_value();
         let tx_digest =

crates/sui-json/src/lib.rs

@@ -35,7 +35,6 @@ use sui_types::base_types::{
 };
 use sui_types::id::{ID, RESOLVED_SUI_ID};
 use sui_types::move_package::MovePackage;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::transfer::RESOLVED_RECEIVING_STRUCT;
 use sui_types::MOVE_STDLIB_ADDRESS;
 
@@ -155,7 +154,7 @@ impl SuiJsonValue {
             if let Some(s) = try_parse_string(layout, bytes) {
                 json!(s)
             } else {
-                let result = BoundedVisitor::deserialize_value(bytes, layout).map_or_else(
+                let result = bcs::from_bytes_seed(layout, bytes).map_or_else(
                     |_| {
                         // fallback to array[u8] if fail to convert to json.
                         JsonValue::Array(

crates/sui-replay/src/displays/transaction_displays.rs

@@ -9,7 +9,6 @@ use std::fmt::{Display, Formatter};
 use std::sync::Arc;
 use sui_execution::Executor;
 use sui_types::execution_mode::ExecutionResult;
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::transaction::CallArg::Pure;
 use sui_types::transaction::{
     write_sep, Argument, CallArg, Command, ObjectArg, ProgrammableMoveCall, ProgrammableTransaction,
@@ -322,8 +321,26 @@ fn resolve_value(
     executor: &Arc<dyn Executor + Send + Sync>,
     store_factory: &LocalExec,
 ) -> anyhow::Result<MoveValue> {
-    let layout = resolve_to_layout(type_tag, executor, store_factory);
-    BoundedVisitor::deserialize_value(bytes, &layout)
+    let mut layout_resolver = executor.type_layout_resolver(Box::new(store_factory.clone()));
+    match type_tag {
+        TypeTag::Vector(inner_type_tag) => {
+            let inner_layout = resolve_to_layout(inner_type_tag, executor, store_factory);
+            MoveValue::simple_deserialize(bytes, &MoveTypeLayout::Vector(Box::from(inner_layout)))
+        }
+        TypeTag::Struct(struct_tag) => {
+            let annotated = layout_resolver.get_annotated_layout(struct_tag)?;
+            MoveValue::simple_deserialize(bytes, &MoveTypeLayout::Struct(annotated))
+        }
+        TypeTag::Bool => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::Bool),
+        TypeTag::U8 => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::U8),
+        TypeTag::U64 => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::U64),
+        TypeTag::U128 => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::U128),
+        TypeTag::Address => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::Address),
+        TypeTag::Signer => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::Signer),
+        TypeTag::U16 => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::U16),
+        TypeTag::U32 => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::U32),
+        TypeTag::U256 => MoveValue::simple_deserialize(bytes, &MoveTypeLayout::U256),
+    }
 }
 
 pub fn transform_command_results_to_annotated(

crates/sui-transactional-test-runner/src/test_adapter.rs

@@ -26,6 +26,7 @@ use move_compiler::{
 use move_core_types::ident_str;
 use move_core_types::{
     account_address::AccountAddress,
+    annotated_value::MoveStruct,
     identifier::IdentStr,
     language_storage::{ModuleId, TypeTag},
 };
@@ -68,7 +69,6 @@ use sui_types::effects::{TransactionEffects, TransactionEffectsAPI, TransactionE
 use sui_types::messages_checkpoint::{
     CheckpointContents, CheckpointContentsDigest, CheckpointSequenceNumber, VerifiedCheckpoint,
 };
-use sui_types::object::bounded_visitor::BoundedVisitor;
 use sui_types::storage::ObjectStore;
 use sui_types::storage::ReadStore;
 use sui_types::transaction::Command;
@@ -648,8 +648,7 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter {
                     object::Data::Move(move_obj) => {
                         let layout = move_obj.get_layout(&&*self).unwrap();
                         let move_struct =
-                            BoundedVisitor::deserialize_struct(move_obj.contents(), &layout)
-                                .unwrap();
+                            MoveStruct::simple_deserialize(move_obj.contents(), &layout).unwrap();
 
                         self.stabilize_str(format!(
                             "Owner: {}\nVersion: {}\nContents: {}",

crates/sui-types/src/event.rs

@@ -19,7 +19,6 @@ use serde_with::Bytes;
 
 use crate::base_types::{ObjectID, SuiAddress, TransactionDigest};
 use crate::error::{SuiError, SuiResult};
-use crate::object::bounded_visitor::BoundedVisitor;
 use crate::sui_serde::BigInt;
 use crate::sui_serde::Readable;
 use crate::SUI_SYSTEM_ADDRESS;
@@ -129,7 +128,7 @@ impl Event {
         contents: &[u8],
         layout: MoveStructLayout,
     ) -> SuiResult<MoveStruct> {
-        BoundedVisitor::deserialize_struct(contents, &layout).map_err(|e| {
+        MoveStruct::simple_deserialize(contents, &layout).map_err(|e| {
             SuiError::ObjectSerializationError {
                 error: e.to_string(),
             }

crates/sui-types/src/object.rs

@@ -10,14 +10,15 @@ use std::sync::Arc;
 use move_binary_format::CompiledModule;
 use move_bytecode_utils::layout::TypeLayoutBuilder;
 use move_bytecode_utils::module_cache::GetModule;
-use move_core_types::annotated_value::{MoveStruct, MoveStructLayout, MoveTypeLayout};
+use move_core_types::annotated_value::{MoveStruct, MoveStructLayout, MoveTypeLayout, MoveValue};
 use move_core_types::language_storage::StructTag;
 use move_core_types::language_storage::TypeTag;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
 use serde_with::Bytes;
 
+use crate::balance::Balance;
 use crate::base_types::{MoveObjectType, ObjectIDParseError};
 use crate::coin::{Coin, CoinMetadata, TreasuryCap};
 use crate::crypto::{default_hash, deterministic_random_account_key};
@@ -35,12 +36,6 @@ use crate::{
 };
 use sui_protocol_config::ProtocolConfig;
 
-use self::balance_traversal::BalanceTraversal;
-use self::bounded_visitor::BoundedVisitor;
-
-mod balance_traversal;
-pub mod bounded_visitor;
-
 pub const GAS_VALUE_FOR_TESTING: u64 = 300_000_000_000_000;
 pub const OBJECT_START_VERSION: SequenceNumber = SequenceNumber::from_u64(1);
 
@@ -323,7 +318,7 @@ impl MoveObject {
 
     /// Convert `self` to the JSON representation dictated by `layout`.
     pub fn to_move_struct(&self, layout: &MoveStructLayout) -> Result<MoveStruct, SuiError> {
-        BoundedVisitor::deserialize_struct(&self.contents, layout).map_err(|e| {
+        MoveStruct::simple_deserialize(&self.contents, layout).map_err(|e| {
             SuiError::ObjectSerializationError {
                 error: e.to_string(),
             }
@@ -363,31 +358,90 @@ impl MoveObject {
 
 // Helpers for extracting Coin<T> balances for all T
 impl MoveObject {
+    fn is_balance(s: &StructTag) -> Option<&TypeTag> {
+        (Balance::is_balance(s) && s.type_params.len() == 1).then(|| &s.type_params[0])
+    }
+
     /// Get the total balances for all `Coin<T>` embedded in `self`.
     pub fn get_coin_balances(
         &self,
         layout_resolver: &mut dyn LayoutResolver,
     ) -> Result<BTreeMap<TypeTag, u64>, SuiError> {
+        let mut balances = BTreeMap::default();
+
         // Fast path without deserialization.
         if let Some(type_tag) = self.type_.coin_type_maybe() {
             let balance = self.get_coin_value_unsafe();
-            Ok(if balance > 0 {
-                BTreeMap::from([(type_tag.clone(), balance)])
-            } else {
-                BTreeMap::default()
-            })
+            if balance > 0 {
+                *balances.entry(type_tag).or_insert(0) += balance;
+            }
         } else {
             let layout = layout_resolver.get_annotated_layout(&self.type_().clone().into())?;
+            let move_struct = self.to_move_struct(&layout)?;
+            Self::get_coin_balances_in_struct(&move_struct, &mut balances, 0)?;
+        }
 
-            let mut traversal = BalanceTraversal::default();
-            MoveStruct::visit_deserialize(&self.contents, &layout, &mut traversal).map_err(
-                |e| SuiError::ObjectSerializationError {
-                    error: e.to_string(),
-                },
-            )?;
+        Ok(balances)
+    }
+
+    /// Get the total balances for all `Coin<T>` embedded in `s`, eitehr directly or in its
+    /// (transitive fields).
+    fn get_coin_balances_in_struct(
+        s: &MoveStruct,
+        balances: &mut BTreeMap<TypeTag, u64>,
+        value_depth: u64,
+    ) -> Result<(), SuiError> {
+        let MoveStruct {
+            type_: struct_type,
+            fields,
+        } = s;
+
+        if let Some(type_tag) = Self::is_balance(struct_type) {
+            let balance = match fields[0].1 {
+                MoveValue::U64(n) => n,
+                _ => unreachable!(), // a Balance<T> object should have exactly one field, of type int
+            };
+
+            // Accumulate the found balance
+            if balance > 0 {
+                *balances.entry(type_tag.clone()).or_insert(0) += balance;
+            }
+        } else {
+            for field in fields {
+                Self::get_coin_balances_in_value(&field.1, balances, value_depth)?;
+            }
+        }
+
+        Ok(())
+    }
+
+    fn get_coin_balances_in_value(
+        v: &MoveValue,
+        balances: &mut BTreeMap<TypeTag, u64>,
+        value_depth: u64,
+    ) -> Result<(), SuiError> {
+        const MAX_MOVE_VALUE_DEPTH: u64 = 256; // This is 2x was the current value of
+                                               // `max_move_value_depth` is from protocol config
+
+        let value_depth = value_depth + 1;
 
-            Ok(traversal.finish())
+        if value_depth > MAX_MOVE_VALUE_DEPTH {
+            return Err(SuiError::GenericAuthorityError {
+                error: "exceeded max move value depth".to_owned(),
+            });
         }
+
+        match v {
+            MoveValue::Struct(s) => Self::get_coin_balances_in_struct(s, balances, value_depth)?,
+            MoveValue::Vector(vec) => {
+                for entry in vec {
+                    Self::get_coin_balances_in_value(entry, balances, value_depth)?;
+                }
+            }
+            _ => {}
+        }
+
+        Ok(())
     }
 }