From 2ed50162af7bc2929bbd8debb3c3f79de33db3a8 Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Thu, 8 May 2025 12:47:25 -0700 Subject: [PATCH 1/2] [3/n][object runtime type tags] Add type tags to object runtime update adapter to handle them. --- .../tests/init/create_object.move | 17 +++ .../tests/init/create_object.snap | 22 ++++ .../tests/init/emit_event.move | 17 +++ .../tests/init/emit_event.snap | 15 +++ crates/sui-open-rpc/spec/openrpc.json | 1 + crates/sui-protocol-config/src/lib.rs | 9 ++ ...ocol_config__test__Mainnet_version_83.snap | 1 + ...ocol_config__test__Testnet_version_83.snap | 1 + ...sui_protocol_config__test__version_83.snap | 1 + .../move-vm-runtime/src/native_functions.rs | 43 ++++++- .../src/programmable_transactions/context.rs | 98 ++++++++++------ .../programmable_transactions/data_store.rs | 31 +++++- .../programmable_transactions/linkage_view.rs | 46 ++++---- .../latest/sui-move-natives/src/config.rs | 3 - .../sui-move-natives/src/dynamic_field.rs | 9 +- .../latest/sui-move-natives/src/event.rs | 10 +- .../src/object_runtime/mod.rs | 40 +++---- .../src/object_runtime/object_store.rs | 58 ++++------ .../sui-move-natives/src/test_scenario.rs | 105 ++++++++++++++---- .../latest/sui-move-natives/src/transfer.rs | 18 +-- 20 files changed, 388 insertions(+), 157 deletions(-) create mode 100644 crates/sui-adapter-transactional-tests/tests/init/create_object.move create mode 100644 crates/sui-adapter-transactional-tests/tests/init/create_object.snap create mode 100644 crates/sui-adapter-transactional-tests/tests/init/emit_event.move create mode 100644 crates/sui-adapter-transactional-tests/tests/init/emit_event.snap diff --git a/crates/sui-adapter-transactional-tests/tests/init/create_object.move b/crates/sui-adapter-transactional-tests/tests/init/create_object.move new file mode 100644 index 0000000000000..d52277e565478 --- /dev/null +++ b/crates/sui-adapter-transactional-tests/tests/init/create_object.move @@ -0,0 +1,17 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//# init --addresses Test=0x0 + +//# publish +module Test::M1 { + public struct X has key { + id: UID, + } + + fun init(ctx: &mut TxContext) { + sui::transfer::transfer(X { id: object::new(ctx) }, ctx.sender()); + } +} + +//# view-object 1,1 diff --git a/crates/sui-adapter-transactional-tests/tests/init/create_object.snap b/crates/sui-adapter-transactional-tests/tests/init/create_object.snap new file mode 100644 index 0000000000000..3d5738abf32bf --- /dev/null +++ b/crates/sui-adapter-transactional-tests/tests/init/create_object.snap @@ -0,0 +1,22 @@ +--- +source: external-crates/move/crates/move-transactional-test-runner/src/framework.rs +--- +processed 3 tasks + +task 1, lines 6-15: +//# publish +created: object(1,0), object(1,1) +mutated: object(0,0) +gas summary: computation_cost: 1000000, storage_cost: 6216800, storage_rebate: 0, non_refundable_storage_fee: 0 + +task 2, line 17: +//# view-object 1,1 +Owner: Account Address ( _ ) +Version: 2 +Contents: Test::M1::X { + id: sui::object::UID { + id: sui::object::ID { + bytes: fake(1,1), + }, + }, +} diff --git a/crates/sui-adapter-transactional-tests/tests/init/emit_event.move b/crates/sui-adapter-transactional-tests/tests/init/emit_event.move new file mode 100644 index 0000000000000..9a80413696c81 --- /dev/null +++ b/crates/sui-adapter-transactional-tests/tests/init/emit_event.move @@ -0,0 +1,17 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//# init --addresses Test=0x0 + +//# publish +module Test::M1 { + public struct Event has copy, drop, store { + x: u64, + } + + fun init(_ctx: &mut TxContext) { + sui::event::emit(Event { x: 1 }); + } +} + +//# view-object 1,0 diff --git a/crates/sui-adapter-transactional-tests/tests/init/emit_event.snap b/crates/sui-adapter-transactional-tests/tests/init/emit_event.snap new file mode 100644 index 0000000000000..4f07553555d33 --- /dev/null +++ b/crates/sui-adapter-transactional-tests/tests/init/emit_event.snap @@ -0,0 +1,15 @@ +--- +source: external-crates/move/crates/move-transactional-test-runner/src/framework.rs +--- +processed 3 tasks + +task 1, lines 6-15: +//# publish +events: Event { package_id: Test, transaction_module: Identifier("M1"), sender: _, type_: StructTag { address: Test, module: Identifier("M1"), name: Identifier("Event"), type_params: [] }, contents: [1, 0, 0, 0, 0, 0, 0, 0] } +created: object(1,0) +mutated: object(0,0) +gas summary: computation_cost: 1000000, storage_cost: 4689200, storage_rebate: 0, non_refundable_storage_fee: 0 + +task 2, line 17: +//# view-object 1,0 +1,0::M1 diff --git a/crates/sui-open-rpc/spec/openrpc.json b/crates/sui-open-rpc/spec/openrpc.json index d1313b4fc6cba..c7511e48393c5 100644 --- a/crates/sui-open-rpc/spec/openrpc.json +++ b/crates/sui-open-rpc/spec/openrpc.json @@ -1375,6 +1375,7 @@ "soft_bundle": false, "throughput_aware_consensus_submission": false, "txn_base_cost_as_multiplier": false, + "type_tags_in_object_runtime": false, "uncompressed_g1_group_elements": false, "upgraded_multisig_supported": false, "validate_identifier_inputs": false, diff --git a/crates/sui-protocol-config/src/lib.rs b/crates/sui-protocol-config/src/lib.rs index f86f59eec7e50..142566448ae53 100644 --- a/crates/sui-protocol-config/src/lib.rs +++ b/crates/sui-protocol-config/src/lib.rs @@ -694,6 +694,10 @@ struct FeatureFlags { // Enable native function for party transfer #[serde(skip_serializing_if = "is_false")] enable_party_transfer: bool, + + // Signifies the cut-over of using type tags instead of `Type`s in the object runtime. + #[serde(skip_serializing_if = "is_false")] + type_tags_in_object_runtime: bool, } fn is_false(b: &bool) -> bool { @@ -1978,6 +1982,10 @@ impl ProtocolConfig { pub fn enable_party_transfer(&self) -> bool { self.feature_flags.enable_party_transfer } + + pub fn type_tags_in_object_runtime(&self) -> bool { + self.feature_flags.type_tags_in_object_runtime + } } #[cfg(not(msim))] @@ -3539,6 +3547,7 @@ impl ProtocolConfig { // native function on mainnet. cfg.feature_flags.enable_nitro_attestation_upgraded_parsing = true; cfg.feature_flags.enable_nitro_attestation = true; + cfg.feature_flags.type_tags_in_object_runtime = true; } // Use this template when making changes: // diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_83.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_83.snap index f52381ce56bf9..d6b5eca879815 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_83.snap +++ b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_83.snap @@ -90,6 +90,7 @@ feature_flags: enforce_checkpoint_timestamp_monotonicity: true max_ptb_value_size_v2: true resolve_type_input_ids_to_defining_id: true + type_tags_in_object_runtime: true max_tx_size_bytes: 131072 max_input_objects: 2048 max_size_written_objects: 5000000 diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_83.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_83.snap index 80ede717745ca..41ca2d7c24a77 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_83.snap +++ b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_83.snap @@ -92,6 +92,7 @@ feature_flags: enforce_checkpoint_timestamp_monotonicity: true max_ptb_value_size_v2: true resolve_type_input_ids_to_defining_id: true + type_tags_in_object_runtime: true max_tx_size_bytes: 131072 max_input_objects: 2048 max_size_written_objects: 5000000 diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_83.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_83.snap index 61e56feb43550..2a41b93d04944 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_83.snap +++ b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_83.snap @@ -97,6 +97,7 @@ feature_flags: enforce_checkpoint_timestamp_monotonicity: true max_ptb_value_size_v2: true resolve_type_input_ids_to_defining_id: true + type_tags_in_object_runtime: true max_tx_size_bytes: 131072 max_input_objects: 2048 max_size_written_objects: 5000000 diff --git a/external-crates/move/crates/move-vm-runtime/src/native_functions.rs b/external-crates/move/crates/move-vm-runtime/src/native_functions.rs index 7a3b2774043b0..f3d67ef741140 100644 --- a/external-crates/move/crates/move-vm-runtime/src/native_functions.rs +++ b/external-crates/move/crates/move-vm-runtime/src/native_functions.rs @@ -20,7 +20,8 @@ use move_core_types::{ }; use move_vm_config::runtime::VMRuntimeLimitsConfig; use move_vm_types::{ - loaded_data::runtime_types::Type, natives::function::NativeResult, values::Value, + data_store::DataStore, loaded_data::runtime_types::Type, natives::function::NativeResult, + values::Value, }; use std::{ cell::RefCell, @@ -158,6 +159,46 @@ impl<'b> NativeContext<'_, 'b> { } } + // TODO: This is a bit hacky right now since we need to pass the store, however this is only + // used in test scenarios so we have some special knowledge that makes this work. In the new VM + // however this is _MUCH_ nicer as we don't need to pass the datastore as the VM's linkage + // tables must have the type present. + pub fn type_tag_to_fully_annotated_layout( + &self, + tag: &TypeTag, + store: &impl DataStore, + ) -> PartialVMResult> { + match self + .resolver + .loader() + .get_fully_annotated_type_layout(tag, store) + { + Ok(ty_layout) => Ok(Some(ty_layout)), + Err(e) if e.major_status().status_type() == StatusType::InvariantViolation => { + Err(e.to_partial()) + } + Err(_) => Ok(None), + } + } + + // TODO: This is a bit hacky right now since we need to pass the store, however this is only + // used in test scenarios so we have some special knowledge that makes this work. In the new VM + // however this is _MUCH_ nicer as we don't need to pass the datastore as the VM's linkage + // tables must have the type present. + pub fn type_tag_to_layout( + &self, + tag: &TypeTag, + store: &impl DataStore, + ) -> PartialVMResult> { + match self.resolver.loader().get_type_layout(tag, store) { + Ok(ty_layout) => Ok(Some(ty_layout)), + Err(e) if e.major_status().status_type() == StatusType::InvariantViolation => { + Err(e.to_partial()) + } + Err(_) => Ok(None), + } + } + pub fn type_to_abilities(&self, ty: &Type) -> PartialVMResult { self.resolver.loader().abilities(ty) } diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs index 432e51dac3f36..d9160d2fa9d4c 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs @@ -15,7 +15,10 @@ mod checked { }, gas_charger::GasCharger, gas_meter::SuiGasMeter, - programmable_transactions::{data_store::SuiDataStore, linkage_view::LinkageView}, + programmable_transactions::{ + data_store::{PackageStore, SuiDataStore}, + linkage_view::LinkageView, + }, type_resolver::TypeTagResolver, }; use move_binary_format::{ @@ -59,7 +62,7 @@ mod checked { metrics::LimitsMetrics, move_package::MovePackage, object::{Authenticator, Data, MoveObject, Object, ObjectInner, Owner}, - storage::{BackingPackageStore, DenyListResult, PackageObject}, + storage::DenyListResult, transaction::{Argument, CallArg, ObjectArg}, }; use tracing::instrument; @@ -285,10 +288,10 @@ mod checked { .unwrap_or(*package_id)); } - let package = package_for_linkage(&self.linkage_view, package_id) + let move_package = get_package(&self.linkage_view, package_id) .map_err(|e| self.convert_vm_error(e))?; - self.linkage_view.set_linkage(package.move_package()) + self.linkage_view.set_linkage(&move_package) } /// Load a type using the context's current session. @@ -341,7 +344,12 @@ mod checked { } let new_events = events .into_iter() - .map(|(ty, tag, value)| { + .map(|(tag, value)| { + let ty = unwrap_type_tag_load( + self.protocol_config, + self.load_type_from_struct(&tag) + .map_err(|e| self.convert_vm_error(e)), + )?; let layout = self .vm .get_runtime() @@ -704,7 +712,7 @@ mod checked { let Self { protocol_config, vm, - linkage_view, + mut linkage_view, mut native_extensions, tx_context, gas_charger, @@ -839,12 +847,6 @@ mod checked { loaded_runtime_objects.extend(loaded_child_objects); let mut written_objects = BTreeMap::new(); - for package in new_packages { - let package_obj = Object::new_from_package(package, tx_digest); - let id = package_obj.id(); - created_object_ids.insert(id); - written_objects.insert(id, package_obj); - } for (id, additional_write) in additional_writes { let AdditionalWrite { recipient, @@ -872,7 +874,24 @@ mod checked { } } - for (id, (recipient, ty, value)) in writes { + for (id, (recipient, tag, value)) in writes { + let ty = unwrap_type_tag_load( + protocol_config, + load_type_from_struct( + vm, + &mut linkage_view, + &new_packages, + &StructTag::from(tag.clone()), + ) + .map_err(|e| { + convert_vm_error( + e, + vm, + &linkage_view, + protocol_config.resolve_abort_locations_to_package_id(), + ) + }), + )?; let abilities = vm.get_runtime().get_type_abilities(&ty).map_err(|e| { convert_vm_error( e, @@ -910,6 +929,13 @@ mod checked { written_objects.insert(id, object); } + for package in new_packages { + let package_obj = Object::new_from_package(package, tx_digest); + let id = package_obj.id(); + created_object_ids.insert(id); + written_objects.insert(id, package_obj); + } + // Before finishing, ensure that any shared object taken by value by the transaction is either: // 1. Mutated (and still has a shared ownership); or // 2. Deleted. @@ -1023,7 +1049,6 @@ mod checked { /// Special case errors for type arguments to Move functions pub fn convert_type_argument_error(&self, idx: usize, error: VMError) -> ExecutionError { - use move_core_types::vm_status::StatusCode; use sui_types::execution_status::TypeArgumentError; match error.major_status() { StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH => { @@ -1284,22 +1309,17 @@ mod checked { /// Fetch the package at `package_id` with a view to using it as a link context. Produces an error /// if the object at that ID does not exist, or is not a package. - fn package_for_linkage( - linkage_view: &LinkageView, + fn get_package( + package_store: &dyn PackageStore, package_id: ObjectID, - ) -> VMResult { - use move_binary_format::errors::PartialVMError; - use move_core_types::vm_status::StatusCode; - - match linkage_view.get_package_object(&package_id) { + ) -> VMResult> { + match package_store.get_package(&package_id) { Ok(Some(package)) => Ok(package), Ok(None) => Err(PartialVMError::new(StatusCode::LINKER_ERROR) .with_message(format!("Cannot find link context {package_id} in store")) .finish(Location::Undefined)), Err(err) => Err(PartialVMError::new(StatusCode::LINKER_ERROR) - .with_message(format!( - "Error loading link context {package_id} from store: {err}" - )) + .with_message(format!("Error loading {package_id} from store: {err}")) .finish(Location::Undefined)), } } @@ -1323,22 +1343,27 @@ mod checked { // Load the package that the struct is defined in, in storage let defining_id = ObjectID::from_address(*address); - let package = package_for_linkage(linkage_view, defining_id)?; + + let data_store = SuiDataStore::new(linkage_view, new_packages); + let move_package = get_package(&data_store, defining_id)?; + + // Save the link context as we need to set it while loading the struct and we don't want to + // clobber it. + let saved_linkage = linkage_view.steal_linkage(); // Set the defining package as the link context while loading the // struct let original_address = linkage_view - .set_linkage(package.move_package()) - .map_err(|e| { - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message(e.to_string()) - .finish(Location::Undefined) - })?; + .set_linkage(&move_package) + .expect("Linkage context was just stolen. Therefore must be empty"); let runtime_id = ModuleId::new(original_address, module.clone()); let data_store = SuiDataStore::new(linkage_view, new_packages); let res = vm.get_runtime().load_type(&runtime_id, name, &data_store); linkage_view.reset_linkage(); + linkage_view + .restore_linkage(saved_linkage) + .expect("Linkage context was just reset. Therefore must be empty"); let (idx, struct_type) = res?; // Recursively load type parameters, if necessary @@ -1707,6 +1732,17 @@ mod checked { } } + fn unwrap_type_tag_load( + protocol_config: &ProtocolConfig, + ty: Result, + ) -> Result { + if ty.is_err() && !protocol_config.type_tags_in_object_runtime() { + panic!("Failed to load a type tag from the object runtime -- this shouldn't happen") + } else { + ty + } + } + enum EitherError { CommandArgument(CommandArgumentError), Execution(ExecutionError), diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/data_store.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/data_store.rs index ca3bd37c605d8..5732153bb9ac3 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/data_store.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/data_store.rs @@ -8,7 +8,10 @@ use move_core_types::{ resolver::ModuleResolver, vm_status::StatusCode, }; use move_vm_types::data_store::DataStore; -use sui_types::move_package::MovePackage; +use std::rc::Rc; +use sui_types::{ + base_types::ObjectID, error::SuiResult, move_package::MovePackage, storage::BackingPackageStore, +}; // Implementation of the `DataStore` trait for the Move VM. // When used during execution it may have a list of new packages that have @@ -97,3 +100,29 @@ impl DataStore for SuiDataStore<'_, '_> { Ok(()) } } + +// A unifying trait that allows us to load move packages that may not be objects just yet (e.g., if +// they were published in the current transaction). Note that this needs to load `MovePackage`s and +// not `MovePackageObject`s. +pub trait PackageStore { + fn get_package(&self, id: &ObjectID) -> SuiResult>>; +} + +impl PackageStore for T { + fn get_package(&self, id: &ObjectID) -> SuiResult>> { + Ok(self + .get_package_object(id)? + .map(|x| Rc::new(x.move_package().clone()))) + } +} + +impl PackageStore for SuiDataStore<'_, '_> { + fn get_package(&self, id: &ObjectID) -> SuiResult>> { + for package in self.new_packages { + if package.id() == *id { + return Ok(Some(Rc::new(package.clone()))); + } + } + self.linkage_view.get_package(id) + } +} diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/linkage_view.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/linkage_view.rs index 443ea70379bd2..2abc378ea7f22 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/linkage_view.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/linkage_view.rs @@ -1,32 +1,31 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 -use std::{ - cell::RefCell, - collections::{BTreeMap, HashMap, HashSet, hash_map::Entry}, - str::FromStr, -}; - -use crate::execution_value::SuiResolver; +use crate::programmable_transactions::data_store::PackageStore; use move_core_types::{ account_address::AccountAddress, identifier::{IdentStr, Identifier}, language_storage::ModuleId, resolver::{LinkageResolver, ModuleResolver}, }; -use sui_types::storage::{PackageObject, get_module}; +use std::{ + cell::RefCell, + collections::{BTreeMap, HashMap, HashSet, hash_map::Entry}, + rc::Rc, + str::FromStr, +}; use sui_types::{ base_types::ObjectID, error::{ExecutionError, SuiError, SuiResult}, move_package::{MovePackage, TypeOrigin, UpgradeInfo}, - storage::BackingPackageStore, }; /// Exposes module and linkage resolution to the Move runtime. The first by delegating to /// `resolver` and the second via linkage information that is loaded from a move package. pub struct LinkageView<'state> { - /// Interface to resolve packages, modules and resources directly from the store. - resolver: Box, + /// Interface to resolve packages, modules and resources directly from the store, and possibly + /// from other sources (e.g., packages just published). + resolver: Box, /// Information used to change module and type identities during linkage. linkage_info: Option, /// Cache containing the type origin information from every package that has been set as the @@ -52,7 +51,7 @@ pub struct LinkageInfo { pub struct SavedLinkage(LinkageInfo); impl<'state> LinkageView<'state> { - pub fn new(resolver: Box) -> Self { + pub fn new(resolver: Box) -> Self { Self { resolver, linkage_info: None, @@ -239,7 +238,7 @@ impl<'state> LinkageView<'state> { } let storage_id = ObjectID::from(*self.relocate(runtime_id)?.address()); - let Some(package) = self.resolver.get_package_object(&storage_id)? else { + let Some(package) = self.resolver.get_package(&storage_id)? else { invariant_violation!("Missing dependent package in store: {storage_id}",) }; @@ -247,7 +246,7 @@ impl<'state> LinkageView<'state> { module_name, datatype_name: struct_name, package, - } in package.move_package().type_origin_table() + } in package.type_origin_table() { if module_name == runtime_id.name().as_str() && struct_name == struct_.as_str() { self.add_type_origin(runtime_id.clone(), struct_.to_owned(), *package)?; @@ -257,7 +256,7 @@ impl<'state> LinkageView<'state> { invariant_violation!( "{runtime_id}::{struct_} not found in type origin table in {storage_id} (v{})", - package.move_package().version(), + package.version(), ) } } @@ -292,18 +291,23 @@ impl LinkageResolver for LinkageView<'_> { } } -// Remaining implementations delegated to state_view - impl ModuleResolver for LinkageView<'_> { type Error = SuiError; fn get_module(&self, id: &ModuleId) -> Result>, Self::Error> { - get_module(self, id) + Ok(self + .get_package(&ObjectID::from(*id.address()))? + .and_then(|package| { + package + .serialized_module_map() + .get(id.name().as_str()) + .cloned() + })) } } -impl BackingPackageStore for LinkageView<'_> { - fn get_package_object(&self, package_id: &ObjectID) -> SuiResult> { - self.resolver.get_package_object(package_id) +impl PackageStore for LinkageView<'_> { + fn get_package(&self, package_id: &ObjectID) -> SuiResult>> { + self.resolver.get_package(package_id) } } diff --git a/sui-execution/latest/sui-move-natives/src/config.rs b/sui-execution/latest/sui-move-natives/src/config.rs index 5210d40b0aced..6fb7387f8dc8e 100644 --- a/sui-execution/latest/sui-move-natives/src/config.rs +++ b/sui-execution/latest/sui-move-natives/src/config.rs @@ -87,7 +87,6 @@ pub fn read_setting_impl( let read_value_opt = consistent_value_before_current_epoch( object_runtime, - &field_setting_ty, field_setting_tag, &field_setting_layout, &setting_value_ty, @@ -111,7 +110,6 @@ pub fn read_setting_impl( fn consistent_value_before_current_epoch( object_runtime: &mut ObjectRuntime, - field_setting_ty: &Type, field_setting_tag: StructTag, field_setting_layout: &R::MoveTypeLayout, _setting_value_ty: &Type, @@ -125,7 +123,6 @@ fn consistent_value_before_current_epoch( let Some(field) = object_runtime.config_setting_unsequenced_read( config_addr.into(), name_df_addr.into(), - field_setting_ty, field_setting_layout, &field_setting_obj_ty, ) else { diff --git a/sui-execution/latest/sui-move-natives/src/dynamic_field.rs b/sui-execution/latest/sui-move-natives/src/dynamic_field.rs index a8238e82b0168..012cafc1eb93f 100644 --- a/sui-execution/latest/sui-move-natives/src/dynamic_field.rs +++ b/sui-execution/latest/sui-move-natives/src/dynamic_field.rs @@ -54,7 +54,6 @@ macro_rules! get_or_fetch_object { object_runtime.get_or_fetch_child_object( $parent, $child_id, - &child_ty, &layout, &annotated_layout, MoveObjectType::from(tag), @@ -229,13 +228,7 @@ pub fn add_child_object( ); let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; - object_runtime.add_child_object( - parent, - child_id, - &child_ty, - MoveObjectType::from(tag), - child, - )?; + object_runtime.add_child_object(parent, child_id, MoveObjectType::from(tag), child)?; Ok(NativeResult::ok(context.gas_used(), smallvec![])) } diff --git a/sui-execution/latest/sui-move-natives/src/event.rs b/sui-execution/latest/sui-move-natives/src/event.rs index 80e1b58f80b2a..5389d114d3a37 100644 --- a/sui-execution/latest/sui-move-natives/src/event.rs +++ b/sui-execution/latest/sui-move-natives/src/event.rs @@ -116,7 +116,7 @@ pub fn emit( let obj_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; - obj_runtime.emit_event(ty, *tag, event_value)?; + obj_runtime.emit_event(*tag, event_value)?; Ok(NativeResult::ok(context.gas_used(), smallvec![])) } @@ -147,12 +147,16 @@ pub fn get_events_by_type( let specialization: VectorSpecialization = (&specified_ty).try_into()?; assert!(args.is_empty()); let object_runtime_ref: &ObjectRuntime = context.extensions().get()?; + let specified_type_tag = match context.type_to_type_tag(&specified_ty)? { + TypeTag::Struct(s) => *s, + _ => return Ok(NativeResult::ok(legacy_test_cost(), smallvec![])), + }; let matched_events = object_runtime_ref .state .events() .iter() - .filter_map(|(ty, _, event)| { - if specified_ty == *ty { + .filter_map(|(tag, event)| { + if &specified_type_tag == tag { Some(event.copy_value().unwrap()) } else { None diff --git a/sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs b/sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs index af13a8c296509..ce0eaec3a3b66 100644 --- a/sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs +++ b/sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs @@ -22,10 +22,7 @@ use move_core_types::{ vm_status::StatusCode, }; use move_vm_runtime::native_extensions::NativeExtensionMarker; -use move_vm_types::{ - loaded_data::runtime_types::Type, - values::{GlobalValue, Value}, -}; +use move_vm_types::values::{GlobalValue, Value}; use object_store::{ActiveChildObject, ChildObjectStore}; use std::{ collections::{BTreeMap, BTreeSet}, @@ -59,11 +56,11 @@ type Set = IndexSet; pub(crate) struct TestInventories { pub(crate) objects: BTreeMap, // address inventories. Most recent objects are at the back of the set - pub(crate) address_inventories: BTreeMap>>, + pub(crate) address_inventories: BTreeMap>>, // global inventories.Most recent objects are at the back of the set - pub(crate) shared_inventory: BTreeMap>, - pub(crate) immutable_inventory: BTreeMap>, - pub(crate) taken_immutable_values: BTreeMap>, + pub(crate) shared_inventory: BTreeMap>, + pub(crate) immutable_inventory: BTreeMap>, + pub(crate) taken_immutable_values: BTreeMap>, // object has been taken from the inventory pub(crate) taken: BTreeMap, // allocated receiving tickets @@ -76,8 +73,8 @@ pub struct LoadedRuntimeObject { } pub struct RuntimeResults { - pub writes: IndexMap, - pub user_events: Vec<(Type, StructTag, Value)>, + pub writes: IndexMap, + pub user_events: Vec<(StructTag, Value)>, // Loaded child objects, their loaded version/digest and whether they were modified. pub loaded_child_objects: BTreeMap, pub created_object_ids: Set, @@ -93,8 +90,8 @@ pub(crate) struct ObjectRuntimeState { deleted_ids: Set, // transfers to a new owner (shared, immutable, object, or account address) // TODO these struct tags can be removed if type_to_type_tag was exposed in the session - transfers: IndexMap, - events: Vec<(Type, StructTag, Value)>, + transfers: IndexMap, + events: Vec<(StructTag, Value)>, // total size of events emitted so far total_events_size: u64, received: IndexMap, @@ -246,7 +243,7 @@ impl<'a> ObjectRuntime<'a> { pub fn transfer( &mut self, owner: Owner, - ty: Type, + ty: MoveObjectType, obj: Value, ) -> PartialVMResult { let id: ObjectID = get_object_id(obj.copy_value()?)? @@ -316,15 +313,15 @@ impl<'a> ObjectRuntime<'a> { Ok(transfer_result) } - pub fn emit_event(&mut self, ty: Type, tag: StructTag, event: Value) -> PartialVMResult<()> { + pub fn emit_event(&mut self, tag: StructTag, event: Value) -> PartialVMResult<()> { if self.state.events.len() >= (self.protocol_config.max_num_event_emit() as usize) { return Err(max_event_error(self.protocol_config.max_num_event_emit())); } - self.state.events.push((ty, tag, event)); + self.state.events.push((tag, event)); Ok(()) } - pub fn take_user_events(&mut self) -> Vec<(Type, StructTag, Value)> { + pub fn take_user_events(&mut self) -> Vec<(StructTag, Value)> { std::mem::take(&mut self.state.events) } @@ -351,7 +348,6 @@ impl<'a> ObjectRuntime<'a> { parent: ObjectID, child: ObjectID, child_version: SequenceNumber, - child_ty: &Type, child_layout: &R::MoveTypeLayout, child_fully_annotated_layout: &MoveTypeLayout, child_move_type: MoveObjectType, @@ -360,7 +356,6 @@ impl<'a> ObjectRuntime<'a> { parent, child, child_version, - child_ty, child_layout, child_fully_annotated_layout, child_move_type, @@ -387,7 +382,6 @@ impl<'a> ObjectRuntime<'a> { &mut self, parent: ObjectID, child: ObjectID, - child_ty: &Type, child_layout: &R::MoveTypeLayout, child_fully_annotated_layout: &MoveTypeLayout, child_move_type: MoveObjectType, @@ -395,7 +389,6 @@ impl<'a> ObjectRuntime<'a> { let res = self.child_object_store.get_or_fetch_object( parent, child, - child_ty, child_layout, child_fully_annotated_layout, child_move_type, @@ -410,26 +403,23 @@ impl<'a> ObjectRuntime<'a> { &mut self, parent: ObjectID, child: ObjectID, - child_ty: &Type, child_move_type: MoveObjectType, child_value: Value, ) -> PartialVMResult<()> { self.child_object_store - .add_object(parent, child, child_ty, child_move_type, child_value) + .add_object(parent, child, child_move_type, child_value) } pub(crate) fn config_setting_unsequenced_read( &mut self, config_id: ObjectID, name_df_id: ObjectID, - field_setting_ty: &Type, field_setting_layout: &R::MoveTypeLayout, field_setting_object_type: &MoveObjectType, ) -> Option { match self.child_object_store.config_setting_unsequenced_read( config_id, name_df_id, - field_setting_ty, field_setting_layout, field_setting_object_type, ) { @@ -620,7 +610,7 @@ impl ObjectRuntimeState { }) } - pub fn events(&self) -> &[(Type, StructTag, Value)] { + pub fn events(&self) -> &[(StructTag, Value)] { &self.events } diff --git a/sui-execution/latest/sui-move-natives/src/object_runtime/object_store.rs b/sui-execution/latest/sui-move-natives/src/object_runtime/object_store.rs index acbb3884e32e3..bbdc291d53f56 100644 --- a/sui-execution/latest/sui-move-natives/src/object_runtime/object_store.rs +++ b/sui-execution/latest/sui-move-natives/src/object_runtime/object_store.rs @@ -6,10 +6,7 @@ use move_binary_format::errors::{PartialVMError, PartialVMResult}; use move_core_types::{ annotated_value as A, effects::Op, runtime_value as R, vm_status::StatusCode, }; -use move_vm_types::{ - loaded_data::runtime_types::Type, - values::{GlobalValue, StructRef, Value}, -}; +use move_vm_types::values::{GlobalValue, StructRef, Value}; use std::{ collections::{btree_map, BTreeMap}, sync::Arc, @@ -27,8 +24,7 @@ use sui_types::{ pub(super) struct ChildObject { pub(super) owner: ObjectID, - pub(super) ty: Type, - pub(super) move_type: MoveObjectType, + pub(super) ty: MoveObjectType, pub(super) value: GlobalValue, pub(super) fingerprint: ObjectFingerprint, } @@ -36,8 +32,7 @@ pub(super) struct ChildObject { pub(crate) struct ActiveChildObject<'a> { pub(crate) id: &'a ObjectID, pub(crate) owner: &'a ObjectID, - pub(crate) ty: &'a Type, - pub(crate) move_type: &'a MoveObjectType, + pub(crate) ty: &'a MoveObjectType, pub(crate) copied_value: Option, } @@ -51,14 +46,14 @@ struct ConfigSetting { #[derive(Debug)] pub(crate) struct ChildObjectEffectV0 { pub(super) owner: ObjectID, - pub(super) ty: Type, + pub(super) ty: MoveObjectType, pub(super) effect: Op, } #[derive(Debug)] pub(crate) struct ChildObjectEffectV1 { pub(super) owner: ObjectID, - pub(super) ty: Type, + pub(super) ty: MoveObjectType, pub(super) final_value: Option, // True if the value or the owner has changed pub(super) object_changed: bool, @@ -306,18 +301,17 @@ impl Inner<'_> { &mut self, parent: ObjectID, child: ObjectID, - child_ty: &Type, child_ty_layout: &R::MoveTypeLayout, child_ty_fully_annotated_layout: &A::MoveTypeLayout, child_move_type: &MoveObjectType, - ) -> PartialVMResult> { + ) -> PartialVMResult> { // we copy the reference to the protocol config ahead of time for lifetime reasons let protocol_config = self.protocol_config; // retrieve the object from storage if it exists let obj = match self.get_or_fetch_object_from_store(parent, child)? { None => { return Ok(ObjectResult::Loaded(( - child_ty.clone(), + child_move_type.clone(), GlobalValue::none(), ObjectFingerprint::none(protocol_config), ))) @@ -375,7 +369,7 @@ impl Inner<'_> { } } Ok(ObjectResult::Loaded(( - child_ty.clone(), + child_move_type.clone(), global_value, fingerprint, ))) @@ -384,10 +378,9 @@ impl Inner<'_> { fn deserialize_move_object( obj: &MoveObject, - child_ty: &Type, child_ty_layout: &R::MoveTypeLayout, child_move_type: MoveObjectType, -) -> PartialVMResult> { +) -> PartialVMResult> { let child_id = obj.id(); // object exists, but the type does not match if obj.type_() != &child_move_type { @@ -403,11 +396,7 @@ fn deserialize_move_object( ) } }; - Ok(ObjectResult::Loaded(( - child_ty.clone(), - child_move_type, - value, - ))) + Ok(ObjectResult::Loaded((child_move_type, value))) } impl<'a> ChildObjectStore<'a> { @@ -442,7 +431,6 @@ impl<'a> ChildObjectStore<'a> { parent: ObjectID, child: ObjectID, child_version: SequenceNumber, - child_ty: &Type, child_layout: &R::MoveTypeLayout, child_fully_annotated_layout: &A::MoveTypeLayout, child_move_type: MoveObjectType, @@ -455,9 +443,9 @@ impl<'a> ChildObjectStore<'a> { }; Ok(Some( - match deserialize_move_object(&obj, child_ty, child_layout, child_move_type)? { + match deserialize_move_object(&obj, child_layout, child_move_type)? { ObjectResult::MismatchedType => (ObjectResult::MismatchedType, obj_meta), - ObjectResult::Loaded((_, _, v)) => { + ObjectResult::Loaded((_, v)) => { // Find all UIDs inside of the value and update the object parent maps with the contained // UIDs in the received value. They should all have an upper bound version as the receiving object. // Only do this if we successfully load the object though. @@ -503,7 +491,7 @@ impl<'a> ChildObjectStore<'a> { ) -> PartialVMResult { if let Some(child_object) = self.store.get(&child) { // exists and has same type - return Ok(child_object.value.exists()? && &child_object.move_type == child_move_type); + return Ok(child_object.value.exists()? && &child_object.ty == child_move_type); } Ok(self .inner @@ -516,7 +504,6 @@ impl<'a> ChildObjectStore<'a> { &mut self, parent: ObjectID, child: ObjectID, - child_ty: &Type, child_layout: &R::MoveTypeLayout, child_fully_annotated_layout: &A::MoveTypeLayout, child_move_type: MoveObjectType, @@ -527,7 +514,6 @@ impl<'a> ChildObjectStore<'a> { let (ty, value, fingerprint) = match self.inner.fetch_object_impl( parent, child, - child_ty, child_layout, child_fully_annotated_layout, &child_move_type, @@ -558,17 +544,21 @@ impl<'a> ChildObjectStore<'a> { )); }; + debug_assert_eq!( + ty, child_move_type, + "Child object type mismatch. \ + Expected {child_move_type} but found {ty}" + ); e.insert(ChildObject { owner: parent, ty, - move_type: child_move_type, value, fingerprint, }) } btree_map::Entry::Occupied(e) => { let child_object = e.into_mut(); - if child_object.move_type != child_move_type { + if child_object.ty != child_move_type { return Ok(ObjectResult::MismatchedType); } child_object @@ -581,7 +571,6 @@ impl<'a> ChildObjectStore<'a> { &mut self, parent: ObjectID, child: ObjectID, - child_ty: &Type, child_move_type: MoveObjectType, child_value: Value, ) -> PartialVMResult<()> { @@ -636,8 +625,7 @@ impl<'a> ChildObjectStore<'a> { } let child_object = ChildObject { owner: parent, - ty: child_ty.clone(), - move_type: child_move_type, + ty: child_move_type, value, fingerprint, }; @@ -649,7 +637,6 @@ impl<'a> ChildObjectStore<'a> { &mut self, config_id: ObjectID, name_df_id: ObjectID, - _field_setting_ty: &Type, field_setting_layout: &R::MoveTypeLayout, field_setting_object_type: &MoveObjectType, ) -> PartialVMResult>> { @@ -752,7 +739,6 @@ impl<'a> ChildObjectStore<'a> { let ChildObject { owner, ty, - move_type, value, fingerprint, } = child_object; @@ -760,7 +746,7 @@ impl<'a> ChildObjectStore<'a> { let dirty_flag_mutated = value.is_mutated(); let final_value = value.into_value(); let object_changed = - fingerprint.object_has_changed(&owner, &move_type, &final_value)?; + fingerprint.object_has_changed(&owner, &ty, &final_value)?; // The old dirty flag was pessimistic in its tracking of mutations, meaning // it would mark mutations even if the value remained the same. // This means that if the object changed, the dirty flag must have been marked. @@ -785,7 +771,6 @@ impl<'a> ChildObjectStore<'a> { let ChildObject { owner, ty, - move_type: _, value, fingerprint: _fingerprint, } = child_object; @@ -821,7 +806,6 @@ impl<'a> ChildObjectStore<'a> { id, owner: &child_object.owner, ty: &child_object.ty, - move_type: &child_object.move_type, copied_value: copied_child_value, } }) diff --git a/sui-execution/latest/sui-move-natives/src/test_scenario.rs b/sui-execution/latest/sui-move-natives/src/test_scenario.rs index 122d7b3f9ef55..042df6ac4ab77 100644 --- a/sui-execution/latest/sui-move-natives/src/test_scenario.rs +++ b/sui-execution/latest/sui-move-natives/src/test_scenario.rs @@ -17,6 +17,7 @@ use move_core_types::{ }; use move_vm_runtime::{native_extensions::NativeExtensionMarker, native_functions::NativeContext}; use move_vm_types::{ + data_store::DataStore, loaded_data::runtime_types::Type, natives::function::NativeResult, pop_arg, @@ -30,7 +31,7 @@ use std::{ thread::LocalKey, }; use sui_types::{ - base_types::{ObjectID, SequenceNumber, SuiAddress}, + base_types::{MoveObjectType, ObjectID, SequenceNumber, SuiAddress}, config, digests::{ObjectDigest, TransactionDigest}, dynamic_field::DynamicFieldInfo, @@ -307,14 +308,14 @@ pub fn end_transaction( let object_runtime_ref: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let mut config_settings = vec![]; for child in object_runtime_ref.all_active_child_objects() { - let s: StructTag = child.move_type.clone().into(); + let s: StructTag = child.ty.clone().into(); let is_setting = DynamicFieldInfo::is_dynamic_field(&s) && matches!(&s.type_params[1], TypeTag::Struct(s) if config::is_setting(s)); if is_setting { config_settings.push(( *child.owner, *child.id, - child.move_type.clone(), + child.ty.clone(), child.copied_value, )); } @@ -380,6 +381,7 @@ pub fn take_from_address_by_id( let account: SuiAddress = pop_arg!(args, AccountAddress).into(); pop_arg!(args, StructRef); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let res = take_from_inventory( @@ -387,7 +389,7 @@ pub fn take_from_address_by_id( inventories .address_inventories .get(&account) - .and_then(|inv| inv.get(&specified_ty)) + .and_then(|inv| inv.get(&specified_obj_ty)) .map(|s| s.contains(x)) .unwrap_or(false) }, @@ -412,12 +414,13 @@ pub fn ids_for_address( let specified_ty = get_specified_ty(ty_args); let account: SuiAddress = pop_arg!(args, AccountAddress).into(); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let ids = inventories .address_inventories .get(&account) - .and_then(|inv| inv.get(&specified_ty)) + .and_then(|inv| inv.get(&specified_obj_ty)) .map(|s| s.iter().map(|id| pack_id(*id)).collect::>()) .unwrap_or_default(); let ids_vector = Vector::pack(VectorSpecialization::Container, ids).unwrap(); @@ -433,11 +436,12 @@ pub fn most_recent_id_for_address( let specified_ty = get_specified_ty(ty_args); let account: SuiAddress = pop_arg!(args, AccountAddress).into(); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let most_recent_id = match inventories.address_inventories.get(&account) { None => pack_option(vector_specialization(&specified_ty), None), - Some(inv) => most_recent_at_ty(&inventories.taken, inv, specified_ty), + Some(inv) => most_recent_at_ty(&inventories.taken, inv, &specified_ty, specified_obj_ty), }; Ok(NativeResult::ok( legacy_test_cost(), @@ -478,13 +482,14 @@ pub fn take_immutable_by_id( let id = pop_id(&mut args)?; pop_arg!(args, StructRef); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let res = take_from_inventory( |x| { inventories .immutable_inventory - .get(&specified_ty) + .get(&specified_obj_ty) .map(|s| s.contains(x)) .unwrap_or(false) }, @@ -498,7 +503,7 @@ pub fn take_immutable_by_id( Ok(value) => { inventories .taken_immutable_values - .entry(specified_ty) + .entry(specified_obj_ty) .or_default() .insert(id, value.copy_value().unwrap()); NativeResult::ok(legacy_test_cost(), smallvec![value]) @@ -515,12 +520,14 @@ pub fn most_recent_immutable_id( ) -> PartialVMResult { let specified_ty = get_specified_ty(ty_args); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let most_recent_id = most_recent_at_ty( &inventories.taken, &inventories.immutable_inventory, - specified_ty, + &specified_ty, + specified_obj_ty, ); Ok(NativeResult::ok( legacy_test_cost(), @@ -560,13 +567,14 @@ pub fn take_shared_by_id( let id = pop_id(&mut args)?; pop_arg!(args, StructRef); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let res = take_from_inventory( |x| { inventories .shared_inventory - .get(&specified_ty) + .get(&specified_obj_ty) .map(|s| s.contains(x)) .unwrap_or(false) }, @@ -590,12 +598,14 @@ pub fn most_recent_id_shared( ) -> PartialVMResult { let specified_ty = get_specified_ty(ty_args); assert!(args.is_empty()); + let specified_obj_ty = object_type_of_type(context, &specified_ty)?; let object_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; let inventories = &mut object_runtime.test_inventories; let most_recent_id = most_recent_at_ty( &inventories.taken, &inventories.shared_inventory, - specified_ty, + &specified_ty, + specified_obj_ty, ); Ok(NativeResult::ok( legacy_test_cost(), @@ -782,19 +792,20 @@ fn vector_specialization(ty: &Type) -> VectorSpecialization { fn most_recent_at_ty( taken: &BTreeMap, - inv: &BTreeMap>, - ty: Type, + inv: &BTreeMap>, + runtime_ty: &Type, + ty: MoveObjectType, ) -> Value { pack_option( - vector_specialization(&ty), + vector_specialization(runtime_ty), most_recent_at_ty_opt(taken, inv, ty), ) } fn most_recent_at_ty_opt( taken: &BTreeMap, - inv: &BTreeMap>, - ty: Type, + inv: &BTreeMap>, + ty: MoveObjectType, ) -> Option { let s = inv.get(&ty)?; let most_recent_id = s.iter().filter(|id| !taken.contains_key(id)).last()?; @@ -891,6 +902,15 @@ fn transaction_effects( ])) } +fn object_type_of_type(context: &NativeContext, ty: &Type) -> PartialVMResult { + let TypeTag::Struct(s_tag) = context.type_to_type_tag(ty)? else { + return Err(PartialVMError::new( + StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, + )); + }; + Ok(MoveObjectType::from(*s_tag)) +} + fn pack_option(specialization: VectorSpecialization, opt: Option) -> Value { let item = match opt { Some(v) => vec![v], @@ -906,7 +926,7 @@ fn pack_option(specialization: VectorSpecialization, opt: Option) -> Valu fn find_all_wrapped_objects<'a, 'i>( context: &NativeContext, ids: &'i mut BTreeSet, - new_object_values: impl IntoIterator)>, + new_object_values: impl IntoIterator)>, ) { #[derive(Copy, Clone)] enum LookingFor { @@ -982,12 +1002,19 @@ fn find_all_wrapped_objects<'a, 'i>( let uid = UID::layout(); for (_id, ty, value) in new_object_values { - let Ok(Some(layout)) = context.type_to_type_layout(ty) else { + let type_tag = TypeTag::from(ty.clone()); + // NB: We can get the layout with the `EmptyDataStore` since the types and modules + // associated with all of these types must be in the type/module cache in the VM -- THIS IS + // BECAUSE WE ARE IN TEST SCENARIO ONLY AND THIS DOES NOT GENERALLY HOLD IN A + // MULTI-TRANSACTION SETTING. + let Ok(Some(layout)) = context.type_tag_to_layout(&type_tag, &EmptyDataStore) else { debug_assert!(false); continue; }; - let Ok(Some(annotated_layout)) = context.type_to_fully_annotated_layout(ty) else { + let Ok(Some(annotated_layout)) = + context.type_tag_to_fully_annotated_layout(&type_tag, &EmptyDataStore) + else { debug_assert!(false); continue; }; @@ -1005,3 +1032,43 @@ fn find_all_wrapped_objects<'a, 'i>( .unwrap(); } } + +// TODO: This can be removed in the new VM as we do not need a "fake" datastore here in order to +// get the type layout. +struct EmptyDataStore; +// All of these should be unreachable +impl DataStore for EmptyDataStore { + fn link_context(&self) -> AccountAddress { + AccountAddress::ZERO + } + + fn relocate( + &self, + _module_id: &move_core_types::language_storage::ModuleId, + ) -> PartialVMResult { + unreachable!("All types must be in the cache since we're in test scenario") + } + + fn defining_module( + &self, + _module_id: &move_core_types::language_storage::ModuleId, + _struct_: &move_core_types::identifier::IdentStr, + ) -> PartialVMResult { + unreachable!("All types must be in the cache since we're in test scenario") + } + + fn load_module( + &self, + _module_id: &move_core_types::language_storage::ModuleId, + ) -> move_binary_format::errors::VMResult> { + unreachable!("All types must be in the cache since we're in test scenario") + } + + fn publish_module( + &mut self, + _module_id: &move_core_types::language_storage::ModuleId, + _blob: Vec, + ) -> move_binary_format::errors::VMResult<()> { + unreachable!("All types must be in the cache since we're in test scenario") + } +} diff --git a/sui-execution/latest/sui-move-natives/src/transfer.rs b/sui-execution/latest/sui-move-natives/src/transfer.rs index 70bdcb14c604e..8e18a7c9232e2 100644 --- a/sui-execution/latest/sui-move-natives/src/transfer.rs +++ b/sui-execution/latest/sui-move-natives/src/transfer.rs @@ -81,7 +81,6 @@ pub fn receive_object_internal( parent, child_id, child_receiver_sequence_number, - &child_ty, &layout, &annotated_layout, MoveObjectType::from(tag), @@ -331,13 +330,16 @@ fn object_runtime_transfer( ty: Type, obj: Value, ) -> PartialVMResult { - if !matches!(context.type_to_type_tag(&ty)?, TypeTag::Struct(_)) { - return Err( - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message("Sui verifier guarantees this is a struct".to_string()), - ); - } + let object_type = match context.type_to_type_tag(&ty)? { + TypeTag::Struct(s) => MoveObjectType::from(*s), + _ => { + return Err( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message("Sui verifier guarantees this is a struct".to_string()), + ); + } + }; let obj_runtime: &mut ObjectRuntime = context.extensions_mut().get_mut()?; - obj_runtime.transfer(owner, ty, obj) + obj_runtime.transfer(owner, object_type, obj) } From 9b50333313e7942d05de54011aa8a6fc4c7d7dff Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Wed, 14 May 2025 15:24:55 -0700 Subject: [PATCH 2/2] respond to comments and bump for ci --- .../move-vm-runtime/src/native_functions.rs | 30 +++++++------------ .../sui-move-natives/src/test_scenario.rs | 8 +++-- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/external-crates/move/crates/move-vm-runtime/src/native_functions.rs b/external-crates/move/crates/move-vm-runtime/src/native_functions.rs index f3d67ef741140..d51754def5bd7 100644 --- a/external-crates/move/crates/move-vm-runtime/src/native_functions.rs +++ b/external-crates/move/crates/move-vm-runtime/src/native_functions.rs @@ -163,40 +163,30 @@ impl<'b> NativeContext<'_, 'b> { // used in test scenarios so we have some special knowledge that makes this work. In the new VM // however this is _MUCH_ nicer as we don't need to pass the datastore as the VM's linkage // tables must have the type present. - pub fn type_tag_to_fully_annotated_layout( + pub fn type_tag_to_fully_annotated_layout_for_test_scenario_only( &self, tag: &TypeTag, store: &impl DataStore, - ) -> PartialVMResult> { - match self - .resolver + ) -> PartialVMResult { + self.resolver .loader() .get_fully_annotated_type_layout(tag, store) - { - Ok(ty_layout) => Ok(Some(ty_layout)), - Err(e) if e.major_status().status_type() == StatusType::InvariantViolation => { - Err(e.to_partial()) - } - Err(_) => Ok(None), - } + .map_err(|e| e.to_partial()) } // TODO: This is a bit hacky right now since we need to pass the store, however this is only // used in test scenarios so we have some special knowledge that makes this work. In the new VM // however this is _MUCH_ nicer as we don't need to pass the datastore as the VM's linkage // tables must have the type present. - pub fn type_tag_to_layout( + pub fn type_tag_to_layout_for_test_scenario_only( &self, tag: &TypeTag, store: &impl DataStore, - ) -> PartialVMResult> { - match self.resolver.loader().get_type_layout(tag, store) { - Ok(ty_layout) => Ok(Some(ty_layout)), - Err(e) if e.major_status().status_type() == StatusType::InvariantViolation => { - Err(e.to_partial()) - } - Err(_) => Ok(None), - } + ) -> PartialVMResult { + self.resolver + .loader() + .get_type_layout(tag, store) + .map_err(|e| e.to_partial()) } pub fn type_to_abilities(&self, ty: &Type) -> PartialVMResult { diff --git a/sui-execution/latest/sui-move-natives/src/test_scenario.rs b/sui-execution/latest/sui-move-natives/src/test_scenario.rs index 042df6ac4ab77..78df24c11781c 100644 --- a/sui-execution/latest/sui-move-natives/src/test_scenario.rs +++ b/sui-execution/latest/sui-move-natives/src/test_scenario.rs @@ -1007,13 +1007,15 @@ fn find_all_wrapped_objects<'a, 'i>( // associated with all of these types must be in the type/module cache in the VM -- THIS IS // BECAUSE WE ARE IN TEST SCENARIO ONLY AND THIS DOES NOT GENERALLY HOLD IN A // MULTI-TRANSACTION SETTING. - let Ok(Some(layout)) = context.type_tag_to_layout(&type_tag, &EmptyDataStore) else { + let Ok(layout) = + context.type_tag_to_layout_for_test_scenario_only(&type_tag, &EmptyDataStore) + else { debug_assert!(false); continue; }; - let Ok(Some(annotated_layout)) = - context.type_tag_to_fully_annotated_layout(&type_tag, &EmptyDataStore) + let Ok(annotated_layout) = context + .type_tag_to_fully_annotated_layout_for_test_scenario_only(&type_tag, &EmptyDataStore) else { debug_assert!(false); continue;