[JSONRPC] - Use a more structured dynamic field name instead of MoveValue::to_string

.changeset/witty-bananas-yell.md

@@ -0,0 +1,5 @@
+---
+"@mysten/sui.js": minor
+---
+
+Use DynamicFieldName struct instead of string for dynamic field's name

crates/sui-core/src/authority.rs

@@ -46,7 +46,7 @@ use narwhal_config::{
 use sui_adapter::{adapter, execution_mode};
 use sui_config::genesis::Genesis;
 use sui_json_rpc_types::{
-    type_and_fields_from_move_struct, DevInspectResults, SuiEvent, SuiEventEnvelope,
+    type_and_fields_from_move_struct, DevInspectResults, SuiEvent, SuiEventEnvelope, SuiMoveValue,
     SuiTransactionEffects,
 };
 use sui_macros::nondeterministic;
@@ -60,7 +60,7 @@ use sui_storage::{
 };
 use sui_types::committee::{EpochId, ProtocolVersion};
 use sui_types::crypto::{sha3_hash, AuthorityKeyPair, NetworkKeyPair, Signer};
-use sui_types::dynamic_field::{DynamicFieldInfo, DynamicFieldType};
+use sui_types::dynamic_field::{DynamicFieldInfo, DynamicFieldName, DynamicFieldType};
 use sui_types::event::{Event, EventID};
 use sui_types::gas::{GasCostSummary, GasPrice, SuiCostTable, SuiGasStatus};
 use sui_types::messages_checkpoint::{
@@ -1273,9 +1273,16 @@ impl AuthorityState {
             self.module_cache.as_ref(),
         )?;
 
-        let (name, type_, object_id) =
+        let (name_value, type_, object_id) =
             DynamicFieldInfo::parse_move_object(&move_struct).tap_err(|e| warn!("{e}"))?;
 
+        let name_type = DynamicFieldInfo::try_extract_field_name(&move_object.type_, &type_)?;
+
+        let name = DynamicFieldName {
+            type_: name_type,
+            value: SuiMoveValue::from(name_value).to_json_value(),
+        };
+
         Ok(Some(match type_ {
             DynamicFieldType::DynamicObject => {
                 // Find the actual object from storage using the object id obtained from the wrapper.
@@ -2019,7 +2026,7 @@ impl AuthorityState {
     pub fn get_dynamic_field_object_id(
         &self,
         owner: ObjectID,
-        name: &str,
+        name: &DynamicFieldName,
     ) -> SuiResult<Option<ObjectID>> {
         if let Some(indexes) = &self.indexes {
             indexes.get_dynamic_field_object_id(owner, name)

crates/sui-core/src/event_handler.rs

@@ -128,11 +128,7 @@ impl EventHandler {
                     Event::move_event_to_move_struct(type_, contents, self.module_cache.as_ref())?;
                 // Convert into `SuiMoveStruct` which is a mirror of MoveStruct but with additional type supports, (e.g. ascii::String).
                 let sui_move_struct = SuiMoveStruct::from(move_struct);
-                Some(sui_move_struct.to_json_value().map_err(|e| {
-                    SuiError::ObjectSerializationError {
-                        error: e.to_string(),
-                    }
-                })?)
+                Some(sui_move_struct.to_json_value())
             }
             _ => None,
         };

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

@@ -17,6 +17,7 @@ use move_binary_format::{
     file_format::{self, AddressIdentifierIndex, IdentifierIndex, ModuleHandle},
     CompiledModule,
 };
+use move_core_types::identifier::IdentStr;
 use move_core_types::{
     account_address::AccountAddress, ident_str, identifier::Identifier, language_storage::TypeTag,
 };
@@ -25,6 +26,7 @@ use rand::{
     prelude::StdRng,
     Rng, SeedableRng,
 };
+use serde_json::json;
 use std::collections::HashSet;
 use std::fs;
 use std::future::Future;
@@ -38,6 +40,7 @@ use sui_types::utils::{
 use sui_types::{SUI_CLOCK_OBJECT_ID, SUI_CLOCK_OBJECT_SHARED_VERSION, SUI_FRAMEWORK_OBJECT_ID};
 
 use crate::epoch::epoch_metrics::EpochMetrics;
+use move_core_types::parser::parse_type_tag;
 use std::{convert::TryInto, env};
 use sui_macros::sim_test;
 use sui_protocol_config::{ProtocolConfig, SupportedProtocolVersions};
@@ -3183,6 +3186,22 @@ async fn test_store_revert_unwrap_move_call() {
 }
 #[tokio::test]
 async fn test_store_get_dynamic_object() {
+    let (_, fields) = create_and_retrieve_df_info(ident_str!("add_ofield")).await;
+    assert_eq!(fields.len(), 1);
+    assert_eq!(fields[0].type_, DynamicFieldType::DynamicObject);
+}
+
+#[tokio::test]
+async fn test_store_get_dynamic_field() {
+    let (_, fields) = create_and_retrieve_df_info(ident_str!("add_field")).await;
+
+    assert_eq!(fields.len(), 1);
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicField));
+    assert_eq!(json!(true), fields[0].name.value);
+    assert_eq!(TypeTag::Bool, fields[0].name.type_)
+}
+
+async fn create_and_retrieve_df_info(function: &IdentStr) -> (SuiAddress, Vec<DynamicFieldInfo>) {
     let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
     let gas_object_id = ObjectID::random();
     let (authority_state, object_basics) =
@@ -3222,7 +3241,7 @@ async fn test_store_get_dynamic_object() {
             sender,
             object_basics.0,
             ident_str!("object_basics").to_owned(),
-            ident_str!("add_ofield").to_owned(),
+            function.to_owned(),
             vec![],
             create_inner_effects.gas_object.0,
             vec![
@@ -3245,82 +3264,82 @@ async fn test_store_get_dynamic_object() {
     assert!(add_effects.status.is_ok());
     assert_eq!(add_effects.created.len(), 1);
 
-    let fields = authority_state
-        .get_dynamic_fields(outer_v0.0, None, usize::MAX)
-        .unwrap();
-    assert_eq!(fields.len(), 1);
-    assert_eq!(fields[0].type_, DynamicFieldType::DynamicObject);
+    (
+        sender,
+        authority_state
+            .get_dynamic_fields(outer_v0.0, None, usize::MAX)
+            .unwrap(),
+    )
 }
 
 #[tokio::test]
-async fn test_store_get_dynamic_field() {
-    let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
-    let gas_object_id = ObjectID::random();
-    let (authority_state, object_basics) =
-        init_state_with_ids_and_object_basics(vec![(sender, gas_object_id)]).await;
+async fn test_dynamic_field_struct_name_parsing() {
+    let (_, fields) = create_and_retrieve_df_info(ident_str!("add_field_with_struct_name")).await;
 
-    let create_outer_effects = create_move_object(
-        &object_basics.0,
-        &authority_state,
-        &gas_object_id,
-        &sender,
-        &sender_key,
+    assert_eq!(fields.len(), 1);
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicField));
+    assert_eq!(json!({"name_str": "Test Name"}), fields[0].name.value);
+    assert_eq!(
+        parse_type_tag("0x0::object_basics::Name").unwrap(),
+        fields[0].name.type_
     )
-    .await
-    .unwrap();
+}
 
-    assert!(create_outer_effects.status.is_ok());
-    assert_eq!(create_outer_effects.created.len(), 1);
+#[tokio::test]
+async fn test_dynamic_field_bytearray_name_parsing() {
+    let (_, fields) =
+        create_and_retrieve_df_info(ident_str!("add_field_with_bytearray_name")).await;
 
-    let create_inner_effects = create_move_object(
-        &object_basics.0,
-        &authority_state,
-        &gas_object_id,
-        &sender,
-        &sender_key,
-    )
-    .await
-    .unwrap();
+    assert_eq!(fields.len(), 1);
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicField));
+    assert_eq!(parse_type_tag("vector<u8>").unwrap(), fields[0].name.type_);
+    assert_eq!(json!("Test Name".as_bytes()), fields[0].name.value);
+}
 
-    assert!(create_inner_effects.status.is_ok());
-    assert_eq!(create_inner_effects.created.len(), 1);
+#[tokio::test]
+async fn test_dynamic_field_address_name_parsing() {
+    let (sender, fields) =
+        create_and_retrieve_df_info(ident_str!("add_field_with_address_name")).await;
 
-    let outer_v0 = create_outer_effects.created[0].0;
-    let inner_v0 = create_inner_effects.created[0].0;
+    assert_eq!(fields.len(), 1);
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicField));
+    assert_eq!(parse_type_tag("address").unwrap(), fields[0].name.type_);
+    assert_eq!(json!(sender), fields[0].name.value);
+}
 
-    let add_txn = to_sender_signed_transaction(
-        TransactionData::new_move_call_with_dummy_gas_price(
-            sender,
-            object_basics.0,
-            ident_str!("object_basics").to_owned(),
-            ident_str!("add_field").to_owned(),
-            vec![],
-            create_inner_effects.gas_object.0,
-            vec![
-                CallArg::Object(ObjectArg::ImmOrOwnedObject(outer_v0)),
-                CallArg::Object(ObjectArg::ImmOrOwnedObject(inner_v0)),
-            ],
-            MAX_GAS,
-        ),
-        &sender_key,
-    );
+#[tokio::test]
+async fn test_dynamic_object_field_struct_name_parsing() {
+    let (_, fields) = create_and_retrieve_df_info(ident_str!("add_ofield_with_struct_name")).await;
 
-    let add_cert = init_certified_transaction(add_txn, &authority_state);
+    assert_eq!(fields.len(), 1);
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicObject));
+    assert_eq!(json!({"name_str": "Test Name"}), fields[0].name.value);
+    assert_eq!(
+        parse_type_tag("0x0::object_basics::Name").unwrap(),
+        fields[0].name.type_
+    )
+}
 
-    let add_effects = authority_state
-        .try_execute_for_test(&add_cert)
-        .await
-        .unwrap()
-        .into_message();
+#[tokio::test]
+async fn test_dynamic_object_field_bytearray_name_parsing() {
+    let (_, fields) =
+        create_and_retrieve_df_info(ident_str!("add_ofield_with_bytearray_name")).await;
 
-    assert!(add_effects.status.is_ok());
-    assert_eq!(add_effects.created.len(), 1);
+    assert_eq!(fields.len(), 1);
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicObject));
+    assert_eq!(parse_type_tag("vector<u8>").unwrap(), fields[0].name.type_);
+    assert_eq!(json!("Test Name".as_bytes()), fields[0].name.value);
+}
+
+#[tokio::test]
+async fn test_dynamic_object_field_address_name_parsing() {
+    let (sender, fields) =
+        create_and_retrieve_df_info(ident_str!("add_ofield_with_address_name")).await;
 
-    let fields = authority_state
-        .get_dynamic_fields(outer_v0.0, None, usize::MAX)
-        .unwrap();
     assert_eq!(fields.len(), 1);
-    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicField));
+    assert!(matches!(fields[0].type_, DynamicFieldType::DynamicObject));
+    assert_eq!(parse_type_tag("address").unwrap(), fields[0].name.type_);
+    assert_eq!(json!(sender), fields[0].name.value);
 }
 
 #[tokio::test]

crates/sui-core/src/unit_tests/data/object_basics/sources/object_basics.move

@@ -107,6 +107,34 @@ module examples::object_basics {
         );
     }
 
+    struct Name has copy, drop, store {
+        name_str: std::string::String
+    }
+
+    public entry fun add_field_with_struct_name(o: &mut Object, v: Object) {
+        sui::dynamic_field::add(&mut o.id, Name {name_str: std::string::utf8(b"Test Name")}, v);
+    }
+
+    public entry fun add_ofield_with_struct_name(o: &mut Object, v: Object) {
+        ofield::add(&mut o.id, Name {name_str: std::string::utf8(b"Test Name")}, v);
+    }
+
+    public entry fun add_field_with_bytearray_name(o: &mut Object, v: Object) {
+        sui::dynamic_field::add(&mut o.id,b"Test Name", v);
+    }
+
+    public entry fun add_ofield_with_bytearray_name(o: &mut Object, v: Object) {
+        ofield::add(&mut o.id,b"Test Name", v);
+    }
+
+    public entry fun add_field_with_address_name(o: &mut Object, v: Object,  ctx: &mut TxContext) {
+        sui::dynamic_field::add(&mut o.id,tx_context::sender(ctx), v);
+    }
+
+    public entry fun add_ofield_with_address_name(o: &mut Object, v: Object,  ctx: &mut TxContext) {
+        ofield::add(&mut o.id,tx_context::sender(ctx), v);
+    }
+
     public entry fun generic_test<T>() {}
 
     public entry fun use_clock(_clock: &Clock) {}

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

@@ -37,7 +37,7 @@ fn test_to_json_value() {
         MoveStruct::simple_deserialize(&event_bytes, &TestEvent::layout())
             .unwrap()
             .into();
-    let json_value = sui_move_struct.to_json_value().unwrap();
+    let json_value = sui_move_struct.to_json_value();
     assert_eq!(
         Some(&json!("1000000")),
         json_value.pointer("/coins/0/balance")