Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[utils] add logic for serializing Move value with both type and field… #44

Merged
merged 1 commit into from
Feb 9, 2022
Merged

[utils] add logic for serializing Move value with both type and field… #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion language/move-binary-format/src/normalized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl Struct {
pub fn new(m: &CompiledModule, def: &StructDefinition) -> (Identifier, Self) {
let handle = m.struct_handle_at(def.struct_handle);
let fields = match &def.field_information {
StructFieldInformation::Native => panic!("Can't extract for native struct"),
StructFieldInformation::Native => panic!("Can't extract for native struct"),
StructFieldInformation::Declared(fields) => {
fields.iter().map(|f| Field::new(m, f)).collect()
}
Expand Down
9 changes: 9 additions & 0 deletions language/move-core/types/src/language_storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@ pub const CORE_CODE_ADDRESS: AccountAddress = AccountAddress::ONE;

#[derive(Serialize, Deserialize, Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)]
pub enum TypeTag {
#[serde(rename = "bool")]
Bool,
#[serde(rename = "u8")]
U8,
#[serde(rename = "u64")]
U64,
#[serde(rename = "u128")]
U128,
#[serde(rename = "address")]
Address,
#[serde(rename = "signer")]
Signer,
#[serde(rename = "vector")]
Vector(Box<TypeTag>),
#[serde(rename = "struct")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sblackshear could I change this serde rename to "Struct" as well as other rename to capital case? Since "struct" is a reserved keyword in rust, the code deserialized back to RUST won't compile.

Struct(StructTag),
}

Expand All @@ -34,6 +42,7 @@ pub struct StructTag {
pub module: Identifier,
pub name: Identifier,
// TODO: rename to "type_args" (or better "ty_args"?)
#[serde(rename = "type_args")]
pub type_params: Vec<TypeTag>,
}

Expand Down
1 change: 1 addition & 0 deletions language/move-core/types/src/unit_tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

mod identifier_test;
mod language_storage_test;
mod value_test;
103 changes: 103 additions & 0 deletions language/move-core/types/src/unit_tests/value_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright (c) The Diem Core Contributors
// SPDX-License-Identifier: Apache-2.0

use crate::{
account_address::AccountAddress,
ident_str,
identifier::Identifier,
language_storage::{StructTag, TypeTag},
value::{MoveStruct, MoveValue},
};
use serde_json::json;

#[test]
fn struct_deserialization() {
let struct_type = StructTag {
address: AccountAddress::ZERO,
name: ident_str!("MyStruct").to_owned(),
module: ident_str!("MyModule").to_owned(),
type_params: vec![],
};
let values = vec![MoveValue::U64(7), MoveValue::Bool(true)];
let fields = vec![ident_str!("f").to_owned(), ident_str!("g").to_owned()];
let field_values: Vec<(Identifier, MoveValue)> =
fields.into_iter().zip(values.clone()).collect();

// test each deserialization scheme
let runtime_value = MoveStruct::Runtime(values);
assert_eq!(
serde_json::to_value(&runtime_value).unwrap(),
json!([7, true])
);

let fielded_value = MoveStruct::WithFields(field_values.clone());
assert_eq!(
serde_json::to_value(&fielded_value).unwrap(),
json!({ "f": 7, "g": true })
);

let typed_value = MoveStruct::with_types(struct_type, field_values);
assert_eq!(
serde_json::to_value(&typed_value).unwrap(),
json!({
"fields": { "f": 7, "g": true },
"type": "0x0::MyModule::MyStruct"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this type should be even further deconstructed possibly. e.g.,

"type": {
    "address": "0x0",
    "module_name": "MyModule",
    "type_name": "MyStruct"
}

In the monomorphic case I don't think there is much of a difference, but in the generic case it may make it easier to distinguish/find multiple instantiations of the same generic type:

[{
  "fields": { ... },
  "type": {
     "address": "0x0",
     "module_name": "MyModule",
     "type_name": "GenericStruct",
     "instantiation": [ "u8" ]
   }
},
{
  "fields": { ... },
  "type": {
     "address": "0x0",
     "module_name": "MyModule",
     "type_name": "GenericStruct",
     "instantiation": [ "u64" ]
   }
}]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good question. I actually did it this way initially, but ended up changing my mind and going with the current approach. My reasoning:

  • I think clients of the code will most commonly want to use the fully qualified type directly rather than breaking it into its components. E.g.: map the type to its Python or JS equivalent + create a Python or JS object for the value, index the struct value by its type, pipe it into a query like "show me other values of this type", copy the type and paste it into a block explorer. or just view the type in a human-readable format.
  • This way ends up being quite a bit more human-readable, especially considering that every struct in a (potentially deep) nesting has an attached type. If you use "type as object representation", I think any nested struct quickly becomes quite difficult to grok.

With that said, I think we should probably support multiple ways of printing the types just as we support multiple ways of printing struct values. Some refactoring of value.rs is probably needed to support this, the current approach is already becoming a bit unwieldy...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points, and agreed that this is becoming a bit unwieldy. Eventually I think it would be best to have a config where these types of things can be set ("packed" vs "unpacked" type representations in the output).

}
)
);
}

#[test]
fn nested_typed_struct_deserialization() {
let struct_type = StructTag {
address: AccountAddress::ZERO,
name: ident_str!("MyStruct").to_owned(),
module: ident_str!("MyModule").to_owned(),
type_params: vec![],
};
let nested_struct_type = StructTag {
address: AccountAddress::ZERO,
name: ident_str!("NestedStruct").to_owned(),
module: ident_str!("NestedModule").to_owned(),
type_params: vec![TypeTag::U8],
};

// test each deserialization scheme
let nested_runtime_struct = MoveValue::Struct(MoveStruct::Runtime(vec![MoveValue::U64(7)]));
let runtime_value = MoveStruct::Runtime(vec![nested_runtime_struct]);
assert_eq!(serde_json::to_value(&runtime_value).unwrap(), json!([[7]]));

let nested_fielded_struct = MoveValue::Struct(MoveStruct::with_fields(vec![(
ident_str!("f").to_owned(),
MoveValue::U64(7),
)]));
let fielded_value = MoveStruct::with_fields(vec![(
ident_str!("inner").to_owned(),
nested_fielded_struct,
)]);
assert_eq!(
serde_json::to_value(&fielded_value).unwrap(),
json!({ "inner": { "f": 7 } })
);

let nested_typed_struct = MoveValue::Struct(MoveStruct::with_types(
nested_struct_type,
vec![(ident_str!("f").to_owned(), MoveValue::U64(7))],
));
let typed_value = MoveStruct::with_types(
struct_type,
vec![(ident_str!("inner").to_owned(), nested_typed_struct)],
);
assert_eq!(
serde_json::to_value(&typed_value).unwrap(),
json!({
"fields": {
"inner": {
"fields": { "f": 7},
"type": "0x0::NestedModule::NestedStruct<u8>",
}
},
"type": "0x0::MyModule::MyStruct"
})
);
}