From c69e26dfd6b82f3d694e1c09f4401b9678fbdcd3 Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Fri, 19 Apr 2024 14:33:10 -0700 Subject: [PATCH] [7/n][enums/Sui] Misc Sui changes for enums and enums in events --- crates/data-transform/src/main.rs | 14 ++-- .../src/handlers/event_handler.rs | 21 +++--- .../sui-analytics-indexer/src/handlers/mod.rs | 66 ++++++++++++++++++- crates/sui-core/src/authority.rs | 21 +++--- .../src/unit_tests/move_package_tests.rs | 2 +- crates/sui-e2e-tests/tests/full_node_tests.rs | 15 +++-- crates/sui-framework/src/lib.rs | 7 +- crates/sui-indexer/src/models/events.rs | 16 ++--- crates/sui-indexer/src/models/transactions.rs | 15 +++-- crates/sui-move-build/src/lib.rs | 4 +- .../src/displays/transaction_displays.rs | 5 +- crates/sui-tool/src/pkg_dump.rs | 2 +- .../tests/tests.rs | 31 ++++++--- crates/sui/src/client_ptb/builder.rs | 2 +- 14 files changed, 153 insertions(+), 68 deletions(-) diff --git a/crates/data-transform/src/main.rs b/crates/data-transform/src/main.rs index d41f3905f6592c..e4137ba2cda76f 100644 --- a/crates/data-transform/src/main.rs +++ b/crates/data-transform/src/main.rs @@ -9,10 +9,11 @@ use once_cell::sync::Lazy; use std::process::exit; use std::str::FromStr; use std::sync::Arc; +use sui_json_rpc_types::type_and_fields_from_move_event_data; use sui_types::object::bounded_visitor::BoundedVisitor; +use sui_types::type_resolver::get_layout_from_struct_tag; use move_bytecode_utils::module_cache::SyncModuleCache; -use sui_types::object::MoveObject; use self::models::*; use std::env; @@ -23,7 +24,6 @@ use sui_indexer::store::package_resolver::IndexerStorePackageResolver; use move_core_types::language_storage::ModuleId; use move_core_types::resolver::ModuleResolver; use std::collections::HashMap; -use sui_json_rpc_types::SuiMoveStruct; use sui_types::parse_sui_struct_tag; use tracing::debug; @@ -214,16 +214,18 @@ fn main() { // JSON parsing starts here let type_ = parse_sui_struct_tag(&event.event_type).expect("cannot load StructTag"); - let layout = MoveObject::get_layout_from_struct_tag(type_.clone(), &module_cache); + let layout = get_layout_from_struct_tag(type_.clone(), &module_cache); match layout { Ok(l) => { - let move_object = BoundedVisitor::deserialize_struct(&event.event_bcs, &l) - .map_err(|e| IndexerError::SerdeError(e.to_string())); + let move_object = + BoundedVisitor::deserialize_value(&event.event_bcs, &l.into_layout()) + .map_err(|e| IndexerError::SerdeError(e.to_string())); match move_object { Ok(m) => { - let parsed_json = SuiMoveStruct::from(m).to_json_value(); + let (parsed_json, _) = + type_and_fields_from_move_event_data(m).unwrap(); let final_result = serde_json::to_string_pretty(&parsed_json).unwrap(); println!("event json = {}", final_result); diff --git a/crates/sui-analytics-indexer/src/handlers/event_handler.rs b/crates/sui-analytics-indexer/src/handlers/event_handler.rs index d6b514bb4bf835..dd1ff1b9b1bafa 100644 --- a/crates/sui-analytics-indexer/src/handlers/event_handler.rs +++ b/crates/sui-analytics-indexer/src/handlers/event_handler.rs @@ -3,16 +3,17 @@ use anyhow::Result; use fastcrypto::encoding::{Base64, Encoding}; +use move_core_types::annotated_value::MoveValue; use sui_types::SYSTEM_PACKAGE_ADDRESSES; use std::path::Path; -use crate::handlers::{get_move_struct, AnalyticsHandler}; +use crate::handlers::AnalyticsHandler; use crate::package_store::{LocalDBPackageStore, PackageCache}; use crate::tables::EventEntry; use crate::FileType; use sui_indexer::framework::Handler; -use sui_json_rpc_types::SuiMoveStruct; +use sui_json_rpc_types::type_and_fields_from_move_event_data; use sui_package_resolver::Resolver; use sui_rest_api::CheckpointData; use sui_types::digests::TransactionDigest; @@ -98,14 +99,14 @@ impl EventHandler { type_, contents, } = event; - let move_struct = get_move_struct(type_, contents, &self.resolver).await?; - let (_struct_tag, sui_move_struct) = match move_struct.into() { - SuiMoveStruct::WithTypes { type_, fields } => { - (type_, SuiMoveStruct::WithFields(fields)) - } - fields => (type_.clone(), fields), - }; - let event_json = sui_move_struct.to_json_value(); + let layout = self + .resolver + .type_layout(move_core_types::language_storage::TypeTag::Struct( + Box::new(type_.clone()), + )) + .await?; + let move_value = MoveValue::simple_deserialize(contents, &layout)?; + let (_, event_json) = type_and_fields_from_move_event_data(move_value)?; let entry = EventEntry { transaction_digest: digest.base58_encode(), event_index: idx as u64, diff --git a/crates/sui-analytics-indexer/src/handlers/mod.rs b/crates/sui-analytics-indexer/src/handlers/mod.rs index 3c212db4985c15..7419d89174c95c 100644 --- a/crates/sui-analytics-indexer/src/handlers/mod.rs +++ b/crates/sui-analytics-indexer/src/handlers/mod.rs @@ -257,6 +257,16 @@ fn parse_struct_field( parse_struct(path, move_struct, all_structs) } } + MoveValue::Variant(v) => { + for (k, field) in v.fields.iter() { + parse_struct_field( + &format!("{}.{}", path, k), + field.clone(), + curr_struct, + all_structs, + ); + } + } MoveValue::Vector(fields) => { for (index, field) in fields.iter().enumerate() { parse_struct_field( @@ -275,7 +285,7 @@ fn parse_struct_field( mod tests { use crate::handlers::parse_struct; use move_core_types::account_address::AccountAddress; - use move_core_types::annotated_value::{MoveStruct, MoveValue}; + use move_core_types::annotated_value::{MoveStruct, MoveValue, MoveVariant}; use move_core_types::identifier::Identifier; use move_core_types::language_storage::StructTag; use std::collections::BTreeMap; @@ -320,4 +330,58 @@ mod tests { ); Ok(()) } + + #[tokio::test] + async fn test_wrapped_object_parsing_within_enum() -> anyhow::Result<()> { + let uid_field = MoveValue::Struct(MoveStruct { + type_: StructTag::from_str("0x2::object::UID")?, + fields: vec![( + Identifier::from_str("id")?, + MoveValue::Struct(MoveStruct { + type_: StructTag::from_str("0x2::object::ID")?, + fields: vec![( + Identifier::from_str("bytes")?, + MoveValue::Signer(AccountAddress::from_hex_literal("0x300")?), + )], + }), + )], + }); + let balance_field = MoveValue::Struct(MoveStruct { + type_: StructTag::from_str("0x2::balance::Balance")?, + fields: vec![(Identifier::from_str("value")?, MoveValue::U32(10))], + }); + let move_enum = MoveVariant { + type_: StructTag::from_str("0x2::test::TestEnum")?, + variant_name: Identifier::from_str("TestVariant")?, + tag: 0, + fields: vec![ + (Identifier::from_str("field0")?, MoveValue::U64(10)), + (Identifier::from_str("principal")?, balance_field), + ], + }; + let move_struct = MoveStruct { + type_: StructTag::from_str("0x2::test::Test")?, + fields: vec![ + (Identifier::from_str("id")?, uid_field), + ( + Identifier::from_str("enum_field")?, + MoveValue::Variant(move_enum), + ), + ], + }; + let mut all_structs = BTreeMap::new(); + parse_struct("$", move_struct, &mut all_structs); + assert_eq!( + all_structs.get("$").unwrap().object_id, + Some(ObjectID::from_hex_literal("0x300")?) + ); + assert_eq!( + all_structs + .get("$.enum_field.principal") + .unwrap() + .struct_tag, + Some(StructTag::from_str("0x2::balance::Balance")?) + ); + Ok(()) + } } diff --git a/crates/sui-core/src/authority.rs b/crates/sui-core/src/authority.rs index 69e98ca3ca2fb5..7a14a7c0733143 100644 --- a/crates/sui-core/src/authority.rs +++ b/crates/sui-core/src/authority.rs @@ -44,6 +44,7 @@ use sui_config::node::{AuthorityOverloadConfig, StateDebugDumpConfig}; use sui_config::NodeConfig; use sui_types::crypto::RandomnessRound; use sui_types::execution_status::ExecutionStatus; +use sui_types::type_resolver::into_struct_layout; use sui_types::type_resolver::LayoutResolver; use tap::{TapFallible, TapOptional}; use tokio::sync::mpsc::unbounded_channel; @@ -2286,7 +2287,9 @@ impl AuthorityState { return Ok(None); } - let layout = resolver.get_annotated_layout(&move_object.type_().clone().into())?; + let layout = into_struct_layout( + resolver.get_annotated_layout(&move_object.type_().clone().into())?, + )?; let move_struct = move_object.to_move_struct(&layout)?; let (name_value, type_, object_id) = @@ -2501,12 +2504,12 @@ impl AuthorityState { let layout = if let (LayoutGenerationOption::Generate, Some(move_obj)) = (request.generate_layout, object.data.try_as_move()) { - Some( + Some(into_struct_layout( self.load_epoch_store_one_call_per_task() .executor() .type_layout_resolver(Box::new(self.get_backing_package_store().as_ref())) .get_annotated_layout(&move_obj.type_().clone().into())?, - ) + )?) } else { None }; @@ -3299,11 +3302,13 @@ impl AuthorityState { .data .try_as_move() .map(|object| { - self.load_epoch_store_one_call_per_task() - .executor() - // TODO(cache) - must read through cache - .type_layout_resolver(Box::new(self.execution_cache.as_ref())) - .get_annotated_layout(&object.type_().clone().into()) + into_struct_layout( + self.load_epoch_store_one_call_per_task() + .executor() + // TODO(cache) - must read through cache + .type_layout_resolver(Box::new(self.execution_cache.as_ref())) + .get_annotated_layout(&object.type_().clone().into())?, + ) }) .transpose()?; Ok(layout) diff --git a/crates/sui-core/src/unit_tests/move_package_tests.rs b/crates/sui-core/src/unit_tests/move_package_tests.rs index 3fde3ad747e29b..c0ff7d49344c85 100644 --- a/crates/sui-core/src/unit_tests/move_package_tests.rs +++ b/crates/sui-core/src/unit_tests/move_package_tests.rs @@ -20,7 +20,7 @@ macro_rules! type_origin_table { {$($module:ident :: $type:ident => $pkg:expr),* $(,)?} => {{ vec![$(TypeOrigin { module_name: stringify!($module).to_string(), - struct_name: stringify!($type).to_string(), + datatype_name: stringify!($type).to_string(), package: $pkg, },)*] }} diff --git a/crates/sui-e2e-tests/tests/full_node_tests.rs b/crates/sui-e2e-tests/tests/full_node_tests.rs index d123a1a36555f6..a80fedc1e8eede 100644 --- a/crates/sui-e2e-tests/tests/full_node_tests.rs +++ b/crates/sui-e2e-tests/tests/full_node_tests.rs @@ -14,7 +14,7 @@ use sui::client_commands::{SuiClientCommandResult, SuiClientCommands}; use sui_config::node::RunWithRange; use sui_core::authority::EffectsNotifyRead; use sui_json_rpc_types::{ - type_and_fields_from_move_struct, EventPage, SuiEvent, SuiExecutionStatus, + type_and_fields_from_move_event_data, EventPage, SuiEvent, SuiExecutionStatus, SuiTransactionBlockEffectsAPI, SuiTransactionBlockResponse, SuiTransactionBlockResponseOptions, }; use sui_json_rpc_types::{EventFilter, TransactionFilter}; @@ -37,7 +37,7 @@ use sui_types::error::{SuiError, UserInputError}; use sui_types::event::{Event, EventID}; use sui_types::message_envelope::Message; use sui_types::messages_grpc::TransactionInfoRequest; -use sui_types::object::{MoveObject, Object, ObjectRead, Owner, PastObjectRead}; +use sui_types::object::{Object, ObjectRead, Owner, PastObjectRead}; use sui_types::programmable_transaction_builder::ProgrammableTransactionBuilder; use sui_types::quorum_driver_types::{ ExecuteTransactionRequest, ExecuteTransactionRequestType, ExecuteTransactionResponse, @@ -48,6 +48,7 @@ use sui_types::transaction::{ CallArg, GasData, TransactionData, TransactionKind, TEST_ONLY_GAS_UNIT_FOR_OBJECT_BASICS, TEST_ONLY_GAS_UNIT_FOR_SPLIT_COIN, TEST_ONLY_GAS_UNIT_FOR_TRANSFER, }; +use sui_types::type_resolver::get_layout_from_struct_tag; use sui_types::utils::{ to_sender_signed_transaction, to_sender_signed_transaction_with_multi_signers, }; @@ -693,14 +694,14 @@ async fn test_full_node_sub_and_query_move_event_ok() -> Result<(), anyhow::Erro other => panic!("Failed to get SuiEvent, but {:?}", other), }; let struct_tag = parse_struct_tag(&struct_tag_str).unwrap(); - let layout = MoveObject::get_layout_from_struct_tag( + let layout = get_layout_from_struct_tag( struct_tag.clone(), &**node.state().epoch_store_for_testing().module_cache(), )?; - let expected_parsed_event = Event::move_event_to_move_struct(&bcs, layout).unwrap(); - let (_, expected_parsed_event) = - type_and_fields_from_move_struct(&struct_tag, expected_parsed_event); + let expected_parsed_event = Event::move_event_to_move_value(&bcs, layout).unwrap(); + let (_, expected_parsed_events) = + type_and_fields_from_move_event_data(expected_parsed_event).unwrap(); let expected_event = SuiEvent { id: EventID { tx_digest: digest, @@ -710,7 +711,7 @@ async fn test_full_node_sub_and_query_move_event_ok() -> Result<(), anyhow::Erro transaction_module: ident_str!("devnet_nft").into(), sender, type_: struct_tag, - parsed_json: expected_parsed_event.to_json_value(), + parsed_json: expected_parsed_events, bcs, timestamp_ms: None, }; diff --git a/crates/sui-framework/src/lib.rs b/crates/sui-framework/src/lib.rs index 6a686a04a19a14..90d7582dd28a96 100644 --- a/crates/sui-framework/src/lib.rs +++ b/crates/sui-framework/src/lib.rs @@ -215,8 +215,8 @@ pub async fn compare_system_package( } let compatibility = Compatibility { - check_struct_and_pub_function_linking: true, - check_struct_layout: true, + check_datatype_and_pub_function_linking: true, + check_datatype_layout: true, check_friend_linking: false, // Checking `entry` linkage is required because system packages are updated in-place, and a // transaction that was rolled back to make way for reconfiguration should still be runnable @@ -228,7 +228,8 @@ pub async fn compare_system_package( // fail if one of its mutable inputs was being used in another private `entry` function. check_private_entry_linking: true, disallowed_new_abilities: AbilitySet::singleton(Ability::Key), - disallow_change_struct_type_params: true, + disallow_change_datatype_type_params: true, + disallow_new_variants: true, }; let new_pkg = new_object diff --git a/crates/sui-indexer/src/models/events.rs b/crates/sui-indexer/src/models/events.rs index f780a7d14429bd..9e66e16e998b40 100644 --- a/crates/sui-indexer/src/models/events.rs +++ b/crates/sui-indexer/src/models/events.rs @@ -5,10 +5,9 @@ use std::str::FromStr; use std::sync::Arc; use diesel::prelude::*; - -use move_core_types::annotated_value::MoveTypeLayout; use move_core_types::identifier::Identifier; -use sui_json_rpc_types::{SuiEvent, SuiMoveStruct}; + +use sui_json_rpc_types::{type_and_fields_from_move_event_data, SuiEvent}; use sui_package_resolver::{PackageStore, Resolver}; use sui_types::base_types::{ObjectID, SuiAddress}; use sui_types::digests::TransactionDigest; @@ -115,15 +114,10 @@ impl StoredEvent { "Failed to convert to sui event with Error: {e}", )) })?; - let move_struct_layout = match move_type_layout { - MoveTypeLayout::Struct(s) => Ok(s), - _ => Err(IndexerError::ResolveMoveStructError( - "MoveTypeLayout is not Struct".to_string(), - )), - }?; - let move_object = BoundedVisitor::deserialize_struct(&self.bcs, &move_struct_layout) + let move_object = BoundedVisitor::deserialize_value(&self.bcs, &move_type_layout) + .map_err(|e| IndexerError::SerdeError(e.to_string()))?; + let (_, parsed_json) = type_and_fields_from_move_event_data(move_object) .map_err(|e| IndexerError::SerdeError(e.to_string()))?; - let parsed_json = SuiMoveStruct::from(move_object).to_json_value(); let tx_digest = TransactionDigest::try_from(self.transaction_digest.as_slice()).map_err(|e| { IndexerError::SerdeError(format!( diff --git a/crates/sui-indexer/src/models/transactions.rs b/crates/sui-indexer/src/models/transactions.rs index eb144da54fe773..9f4d9f6d40a4c0 100644 --- a/crates/sui-indexer/src/models/transactions.rs +++ b/crates/sui-indexer/src/models/transactions.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use diesel::prelude::*; -use move_core_types::annotated_value::MoveTypeLayout; +use move_core_types::annotated_value::{MoveDatatypeLayout, MoveTypeLayout}; use move_core_types::language_storage::TypeTag; use sui_json_rpc_types::{ BalanceChange, ObjectChange, SuiEvent, SuiTransactionBlock, SuiTransactionBlockEffects, @@ -273,26 +273,27 @@ pub async fn tx_events_to_sui_tx_events( "Failed to convert to sui event with Error: {e}", )) })?; - let event_move_struct_layouts = event_move_type_layouts + let event_move_datatype_layouts = event_move_type_layouts .into_iter() .filter_map(|move_type_layout| match move_type_layout { - MoveTypeLayout::Struct(s) => Some(s), + MoveTypeLayout::Struct(s) => Some(MoveDatatypeLayout::Struct(s)), + MoveTypeLayout::Enum(e) => Some(MoveDatatypeLayout::Enum(e)), _ => None, }) .collect::>(); - assert!(tx_events_data_len == event_move_struct_layouts.len()); + assert!(tx_events_data_len == event_move_datatype_layouts.len()); let sui_events = tx_events .data .into_iter() .enumerate() - .zip(event_move_struct_layouts) - .map(|((seq, tx_event), move_struct_layout)| { + .zip(event_move_datatype_layouts) + .map(|((seq, tx_event), move_datatype_layout)| { SuiEvent::try_from( tx_event, tx_digest, seq as u64, Some(timestamp), - move_struct_layout, + move_datatype_layout, ) }) .collect::, _>>()?; diff --git a/crates/sui-move-build/src/lib.rs b/crates/sui-move-build/src/lib.rs index 3b742cda3dbbc6..bbf8810dfcbfd4 100644 --- a/crates/sui-move-build/src/lib.rs +++ b/crates/sui-move-build/src/lib.rs @@ -475,7 +475,7 @@ impl CompiledPackage { } let mut layout_builder = SerdeLayoutBuilder::new(self); for typ in &package_types { - layout_builder.build_struct_layout(typ).unwrap(); + layout_builder.build_data_layout(typ).unwrap(); } layout_builder.into_registry() } @@ -608,7 +608,7 @@ impl PackageHooks for SuiPackageHooks { &self, manifest: &SourceManifest, ) -> anyhow::Result { - if manifest.package.edition == Some(Edition::DEVELOPMENT) { + if !cfg!(debug_assertions) && manifest.package.edition == Some(Edition::DEVELOPMENT) { return Err(Edition::DEVELOPMENT.unknown_edition_error()); } Ok(manifest.package.name) diff --git a/crates/sui-replay/src/displays/transaction_displays.rs b/crates/sui-replay/src/displays/transaction_displays.rs index 1371df39498bc8..845e797fb4fe5f 100644 --- a/crates/sui-replay/src/displays/transaction_displays.rs +++ b/crates/sui-replay/src/displays/transaction_displays.rs @@ -302,7 +302,10 @@ fn resolve_to_layout( } TypeTag::Struct(inner) => { let mut layout_resolver = executor.type_layout_resolver(Box::new(store_factory)); - MoveTypeLayout::Struct(layout_resolver.get_annotated_layout(inner).unwrap()) + layout_resolver + .get_annotated_layout(inner) + .unwrap() + .into_layout() } TypeTag::Bool => MoveTypeLayout::Bool, TypeTag::U8 => MoveTypeLayout::U8, diff --git a/crates/sui-tool/src/pkg_dump.rs b/crates/sui-tool/src/pkg_dump.rs index e9bc0e4bd81462..bd78cbf2b4b877 100644 --- a/crates/sui-tool/src/pkg_dump.rs +++ b/crates/sui-tool/src/pkg_dump.rs @@ -94,7 +94,7 @@ fn dump_package(output_dir: &Path, id: SuiAddress, pkg: &[u8]) -> Result<()> { .iter() .map(|o| { ( - format!("{}::{}", o.module_name, o.struct_name), + format!("{}::{}", o.module_name, o.datatype_name), o.package.to_string(), ) }) diff --git a/crates/sui-upgrade-compatibility-transactional-tests/tests/tests.rs b/crates/sui-upgrade-compatibility-transactional-tests/tests/tests.rs index 70028ae00f2386..5b5e9307ebcef4 100644 --- a/crates/sui-upgrade-compatibility-transactional-tests/tests/tests.rs +++ b/crates/sui-upgrade-compatibility-transactional-tests/tests/tests.rs @@ -58,30 +58,43 @@ fn check_all_compatibilities( Compatibility::full_check(), // Full compat but allow private entry functions to change Compatibility { - check_struct_and_pub_function_linking: true, - check_struct_layout: true, + check_datatype_and_pub_function_linking: true, + check_datatype_layout: true, check_friend_linking: true, check_private_entry_linking: false, disallowed_new_abilities: AbilitySet::ALL, - disallow_change_struct_type_params: true, + disallow_change_datatype_type_params: true, + disallow_new_variants: true, }, // Full compat but allow private entry functions and friends to change Compatibility { - check_struct_and_pub_function_linking: true, - check_struct_layout: true, + check_datatype_and_pub_function_linking: true, + check_datatype_layout: true, check_friend_linking: false, check_private_entry_linking: false, disallowed_new_abilities: AbilitySet::ALL, - disallow_change_struct_type_params: true, + disallow_change_datatype_type_params: true, + disallow_new_variants: true, }, // Full compat but allow friends to change Compatibility { - check_struct_and_pub_function_linking: true, - check_struct_layout: true, + check_datatype_and_pub_function_linking: true, + check_datatype_layout: true, check_friend_linking: false, check_private_entry_linking: true, disallowed_new_abilities: AbilitySet::ALL, - disallow_change_struct_type_params: true, + disallow_change_datatype_type_params: true, + disallow_new_variants: true, + }, + // Full compat but allow new enum variants to be added + Compatibility { + check_datatype_and_pub_function_linking: true, + check_datatype_layout: true, + check_friend_linking: true, + check_private_entry_linking: true, + disallowed_new_abilities: AbilitySet::ALL, + disallow_change_datatype_type_params: true, + disallow_new_variants: true, }, Compatibility::no_check(), ]; diff --git a/crates/sui/src/client_ptb/builder.rs b/crates/sui/src/client_ptb/builder.rs index b67a417ac259dc..2e05deaa0c19a8 100644 --- a/crates/sui/src/client_ptb/builder.rs +++ b/crates/sui/src/client_ptb/builder.rs @@ -453,7 +453,7 @@ impl<'a> PTBBuilder<'a> { // and also determine if it's a receiving argument or not. for tok in param.preorder_traversal() { match tok { - SignatureToken::Struct(..) | SignatureToken::StructInstantiation(..) => { + SignatureToken::Datatype(..) | SignatureToken::DatatypeInstantiation(..) => { is_receiving |= is_receiving_argument(view, tok); } SignatureToken::TypeParameter(idx) => {