diff --git a/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs b/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs index 64627c634797d..40eec5457f640 100644 --- a/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs +++ b/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs @@ -15,17 +15,20 @@ use move_binary_format::{ CompiledModule, }; use move_core_types::{ - account_address::AccountAddress, identifier::IdentStr, language_storage::ModuleId, + account_address::AccountAddress, + identifier::IdentStr, + language_storage::{ModuleId, TypeTag}, metadata::Metadata, }; use move_vm_runtime::{ ambassador_impl_CodeStorage, ambassador_impl_ModuleStorage, ambassador_impl_WithRuntimeEnvironment, AsUnsyncCodeStorage, BorrowedOrOwned, CodeStorage, - Module, ModuleStorage, RuntimeEnvironment, Script, UnsyncCodeStorage, UnsyncModuleStorage, - WithRuntimeEnvironment, + Function, Module, ModuleStorage, RuntimeEnvironment, Script, UnsyncCodeStorage, + UnsyncModuleStorage, WithRuntimeEnvironment, }; use move_vm_types::{ code::{ModuleBytesStorage, ModuleCode}, + loaded_data::runtime_types::{StructType, Type}, module_storage_error, }; use std::{ops::Deref, sync::Arc}; diff --git a/aptos-move/aptos-vm-types/src/resolver.rs b/aptos-move/aptos-vm-types/src/resolver.rs index 99e72df4c8140..38bcef0f69a24 100644 --- a/aptos-move/aptos-vm-types/src/resolver.rs +++ b/aptos-move/aptos-vm-types/src/resolver.rs @@ -203,33 +203,27 @@ pub trait StateStorageView { /// TODO: audit and reconsider the default implementation (e.g. should not /// resolve AggregatorV2 via the state-view based default implementation, as it /// doesn't provide a value exchange functionality). -pub trait TExecutorView: +pub trait TExecutorView: TResourceView + TModuleView + TAggregatorV1View - + TDelayedFieldView + + TDelayedFieldView + StateStorageView { } -impl TExecutorView for A where +impl TExecutorView for A where A: TResourceView + TModuleView + TAggregatorV1View - + TDelayedFieldView + + TDelayedFieldView + StateStorageView { } -pub trait ExecutorView: - TExecutorView -{ -} +pub trait ExecutorView: TExecutorView {} -impl ExecutorView for T where - T: TExecutorView -{ -} +impl ExecutorView for T where T: TExecutorView {} pub trait ResourceGroupView: TResourceGroupView diff --git a/aptos-move/aptos-vm/src/block_executor/mod.rs b/aptos-move/aptos-vm/src/block_executor/mod.rs index 168b1bb4a3719..ad3918657a568 100644 --- a/aptos-move/aptos-vm/src/block_executor/mod.rs +++ b/aptos-move/aptos-vm/src/block_executor/mod.rs @@ -354,7 +354,7 @@ impl BlockExecutorTransactionOutput for AptosTransactionOutput { .materialized_size() } - fn get_write_summary(&self) -> HashSet> { + fn get_write_summary(&self) -> HashSet> { let vm_output = self.vm_output.lock(); let output = vm_output .as_ref() diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs b/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs index e20b94c89c68c..736e2327f953d 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs @@ -38,10 +38,10 @@ use move_core_types::{ vm_status::StatusCode, }; use move_vm_runtime::{ - move_vm::MoveVM, native_extensions::NativeContextExtensions, session::Session, ModuleStorage, - VerifiedModuleBundle, + move_vm::MoveVM, native_extensions::NativeContextExtensions, session::Session, + AsFunctionValueExtension, ModuleStorage, VerifiedModuleBundle, }; -use move_vm_types::{value_serde::serialize_and_allow_delayed_values, values::Value}; +use move_vm_types::{value_serde::ValueSerDeContext, values::Value}; use std::{ collections::BTreeMap, ops::{Deref, DerefMut}, @@ -127,6 +127,7 @@ impl<'r, 'l> SessionExt<'r, 'l> { module_storage: &impl ModuleStorage, ) -> VMResult<(VMChangeSet, ModuleWriteSet)> { let move_vm = self.inner.get_move_vm(); + let function_extension = module_storage.as_function_value_extension(); let resource_converter = |value: Value, layout: MoveTypeLayout, @@ -136,13 +137,17 @@ impl<'r, 'l> SessionExt<'r, 'l> { // We allow serialization of native values here because we want to // temporarily store native values (via encoding to ensure deterministic // gas charging) in block storage. - serialize_and_allow_delayed_values(&value, &layout)? + ValueSerDeContext::new() + .with_delayed_fields_serde() + .with_func_args_deserialization(&function_extension) + .serialize(&value, &layout)? .map(|bytes| (bytes.into(), Some(Arc::new(layout)))) } else { // Otherwise, there should be no native values so ensure // serialization fails here if there are any. - value - .simple_serialize(&layout) + ValueSerDeContext::new() + .with_func_args_deserialization(&function_extension) + .serialize(&value, &layout)? .map(|bytes| (bytes.into(), None)) }; serialization_result.ok_or_else(|| { @@ -165,7 +170,7 @@ impl<'r, 'l> SessionExt<'r, 'l> { let table_context: NativeTableContext = extensions.remove(); let table_change_set = table_context - .into_change_set() + .into_change_set(&function_extension) .map_err(|e| e.finish(Location::Undefined))?; let aggregator_context: NativeAggregatorContext = extensions.remove(); diff --git a/aptos-move/block-executor/src/captured_reads.rs b/aptos-move/block-executor/src/captured_reads.rs index 7a61f5e3fc16c..4835eaa2c4b63 100644 --- a/aptos-move/block-executor/src/captured_reads.rs +++ b/aptos-move/block-executor/src/captured_reads.rs @@ -1,10 +1,7 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use crate::{ - code_cache_global::GlobalModuleCache, types::InputOutputKey, - value_exchange::filter_value_for_exchange, -}; +use crate::{code_cache_global::GlobalModuleCache, types::InputOutputKey, view::LatestView}; use anyhow::bail; use aptos_aggregator::{ delta_math::DeltaHistory, @@ -21,15 +18,18 @@ use aptos_mvhashmap::{ }; use aptos_types::{ error::{code_invariant_error, PanicError, PanicOr}, - executable::ModulePath, - state_store::state_value::StateValueMetadata, + executable::{Executable, ModulePath}, + state_store::{state_value::StateValueMetadata, TStateView}, transaction::BlockExecutableTransaction as Transaction, write_set::TransactionWrite, }; use aptos_vm_types::resolver::ResourceGroupSize; use derivative::Derivative; use move_core_types::value::MoveTypeLayout; -use move_vm_types::code::{ModuleCode, SyncModuleCache, WithAddress, WithName, WithSize}; +use move_vm_types::{ + code::{ModuleCode, SyncModuleCache, WithAddress, WithName, WithSize}, + delayed_values::delayed_field_id::DelayedFieldID, +}; use std::{ collections::{ hash_map::{ @@ -325,7 +325,7 @@ pub enum CacheRead { pub(crate) struct CapturedReads { data_reads: HashMap>, group_reads: HashMap>, - delayed_field_reads: HashMap, + delayed_field_reads: HashMap, #[deprecated] pub(crate) deprecated_module_reads: Vec, @@ -359,9 +359,13 @@ where S: WithSize, { // Return an iterator over the captured reads. - pub(crate) fn get_read_values_with_delayed_fields( + pub(crate) fn get_read_values_with_delayed_fields< + SV: TStateView, + X: Executable, + >( &self, - delayed_write_set_ids: &HashSet, + view: &LatestView, + delayed_write_set_ids: &HashSet, skip: &HashSet, ) -> Result)>, PanicError> { self.data_reads @@ -372,7 +376,7 @@ where } if let DataRead::Versioned(_version, value, Some(layout)) = data_read { - filter_value_for_exchange::(value, layout, delayed_write_set_ids, key) + view.filter_value_for_exchange(value, layout, delayed_write_set_ids, key) } else { None } @@ -511,7 +515,7 @@ where pub(crate) fn capture_delayed_field_read( &mut self, - id: T::Identifier, + id: DelayedFieldID, update: bool, read: DelayedFieldRead, ) -> Result<(), PanicOr> { @@ -571,7 +575,7 @@ where pub(crate) fn get_delayed_field_by_kind( &self, - id: &T::Identifier, + id: &DelayedFieldID, min_kind: DelayedFieldReadKind, ) -> Option { self.delayed_field_reads @@ -718,7 +722,7 @@ where // (as it internally uses read_latest_predicted_value to get the current value). pub(crate) fn validate_delayed_field_reads( &self, - delayed_fields: &dyn TVersionedDelayedFieldView, + delayed_fields: &dyn TVersionedDelayedFieldView, idx_to_validate: TxnIndex, ) -> Result { if self.delayed_field_speculative_failure { @@ -779,9 +783,7 @@ where K: Hash + Eq + Ord + Clone + WithAddress + WithName, VC: Deref>, { - pub(crate) fn get_read_summary( - &self, - ) -> HashSet> { + pub(crate) fn get_read_summary(&self) -> HashSet> { let mut ret = HashSet::new(); for (key, read) in &self.data_reads { if let DataRead::Versioned(_, _, _) = read { @@ -822,7 +824,7 @@ where pub(crate) struct UnsyncReadSet { pub(crate) resource_reads: HashSet, pub(crate) group_reads: HashMap>, - pub(crate) delayed_field_reads: HashSet, + pub(crate) delayed_field_reads: HashSet, #[deprecated] pub(crate) deprecated_module_reads: HashSet, @@ -839,9 +841,7 @@ where self.module_reads.insert(key); } - pub(crate) fn get_read_summary( - &self, - ) -> HashSet> { + pub(crate) fn get_read_summary(&self) -> HashSet> { let mut ret = HashSet::new(); for key in &self.resource_reads { ret.insert(InputOutputKey::Resource(key.clone())); @@ -1067,7 +1067,6 @@ mod test { impl Transaction for TestTransactionType { type Event = MockEvent; - type Identifier = DelayedFieldID; type Key = KeyType; type Tag = u32; type Value = ValueType; diff --git a/aptos-move/block-executor/src/executor.rs b/aptos-move/block-executor/src/executor.rs index c0bd5ec56967e..b98b68559583c 100644 --- a/aptos-move/block-executor/src/executor.rs +++ b/aptos-move/block-executor/src/executor.rs @@ -59,7 +59,7 @@ use fail::fail_point; use move_binary_format::CompiledModule; use move_core_types::{language_storage::ModuleId, value::MoveTypeLayout, vm_status::StatusCode}; use move_vm_runtime::{Module, RuntimeEnvironment, WithRuntimeEnvironment}; -use move_vm_types::code::ModuleCache; +use move_vm_types::{code::ModuleCache, delayed_values::delayed_field_id::DelayedFieldID}; use num_cpus; use rayon::ThreadPool; use std::{ @@ -115,7 +115,7 @@ where incarnation: Incarnation, signature_verified_block: &TP, last_input_output: &TxnLastInputOutput, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, executor: &E, base_view: &S, global_module_cache: &GlobalModuleCache< @@ -406,7 +406,7 @@ where Module, AptosModuleExtension, >, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, scheduler: &Scheduler, ) -> bool { let _timer = TASK_VALIDATE_SECONDS.start_timer(); @@ -436,7 +436,7 @@ where fn update_transaction_on_abort( txn_idx: TxnIndex, last_input_output: &TxnLastInputOutput, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, runtime_environment: &RuntimeEnvironment, ) { counters::SPECULATIVE_ABORT_COUNT.inc(); @@ -490,7 +490,7 @@ where valid: bool, validation_wave: Wave, last_input_output: &TxnLastInputOutput, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, scheduler: &Scheduler, runtime_environment: &RuntimeEnvironment, ) -> Result { @@ -520,7 +520,7 @@ where /// returns false (indicating that transaction needs to be re-executed). fn validate_and_commit_delayed_fields( txn_idx: TxnIndex, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, last_input_output: &TxnLastInputOutput, ) -> Result { let read_set = last_input_output @@ -563,7 +563,7 @@ where &self, block_gas_limit_type: &BlockGasLimitType, scheduler: &Scheduler, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, scheduler_task: &mut SchedulerTask, last_input_output: &TxnLastInputOutput, shared_commit_state: &ExplicitSyncWrapper>, @@ -766,7 +766,7 @@ where Module, AptosModuleExtension, >, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, scheduler: &Scheduler, runtime_environment: &RuntimeEnvironment, ) -> Result<(), PanicError> { @@ -788,7 +788,7 @@ where fn materialize_aggregator_v1_delta_writes( txn_idx: TxnIndex, last_input_output: &TxnLastInputOutput, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, base_view: &S, ) -> Vec<(T::Key, WriteOp)> { // Materialize all the aggregator v1 deltas. @@ -840,7 +840,7 @@ where fn materialize_txn_commit( &self, txn_idx: TxnIndex, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, scheduler: &Scheduler, start_shared_counter: u32, shared_counter: &AtomicU32, @@ -953,7 +953,7 @@ where environment: &AptosEnvironment, block: &TP, last_input_output: &TxnLastInputOutput, - versioned_cache: &MVHashMap, + versioned_cache: &MVHashMap, scheduler: &Scheduler, // TODO: should not need to pass base view. base_view: &S, @@ -1274,7 +1274,7 @@ where Module, AptosModuleExtension, >, - unsync_map: &UnsyncMap, + unsync_map: &UnsyncMap, output: &E::Output, resource_write_set: Vec<(T::Key, Arc, Option>)>, ) -> Result<(), SequentialBlockExecutionError> { diff --git a/aptos-move/block-executor/src/limit_processor.rs b/aptos-move/block-executor/src/limit_processor.rs index b687da43e1e39..1b60348d45462 100644 --- a/aptos-move/block-executor/src/limit_processor.rs +++ b/aptos-move/block-executor/src/limit_processor.rs @@ -281,7 +281,6 @@ mod test { proptest_types::types::{KeyType, MockEvent, MockTransaction}, types::InputOutputKey, }; - use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use std::collections::HashSet; // TODO: add tests for accumulate_fee_statement / compute_conflict_multiplier for different BlockGasLimitType configs @@ -363,15 +362,13 @@ mod test { assert!(processor.should_end_block_parallel()); } - fn to_map( - reads: &[InputOutputKey], - ) -> HashSet, u32, DelayedFieldID>> { + fn to_map(reads: &[InputOutputKey]) -> HashSet, u32>> { reads .iter() .map(|key| match key { InputOutputKey::Resource(k) => InputOutputKey::Resource(KeyType(*k, false)), InputOutputKey::Group(k, t) => InputOutputKey::Group(KeyType(*k, false), *t), - InputOutputKey::DelayedField(i) => InputOutputKey::DelayedField((*i).into()), + InputOutputKey::DelayedField(i) => InputOutputKey::DelayedField(*i), }) .collect() } diff --git a/aptos-move/block-executor/src/proptest_types/types.rs b/aptos-move/block-executor/src/proptest_types/types.rs index d594f134c2829..bc5ee4395e11d 100644 --- a/aptos-move/block-executor/src/proptest_types/types.rs +++ b/aptos-move/block-executor/src/proptest_types/types.rs @@ -440,7 +440,6 @@ impl< > Transaction for MockTransaction { type Event = E; - type Identifier = DelayedFieldID; type Key = K; type Tag = u32; type Value = ValueType; @@ -848,7 +847,7 @@ where fn execute_transaction( &self, - view: &(impl TExecutorView + view: &(impl TExecutorView + TResourceGroupView + AptosCodeStorage), txn: &Self::Txn, @@ -1112,12 +1111,7 @@ where self.deltas.clone() } - fn delayed_field_change_set( - &self, - ) -> BTreeMap< - ::Identifier, - DelayedChange<::Identifier>, - > { + fn delayed_field_change_set(&self) -> BTreeMap> { // TODO[agg_v2](tests): add aggregators V2 to the proptest? BTreeMap::new() } @@ -1263,7 +1257,6 @@ where crate::types::InputOutputKey< ::Key, ::Tag, - ::Identifier, >, > { HashSet::new() diff --git a/aptos-move/block-executor/src/task.rs b/aptos-move/block-executor/src/task.rs index 3599420e28c3f..4efdc14d7160a 100644 --- a/aptos-move/block-executor/src/task.rs +++ b/aptos-move/block-executor/src/task.rs @@ -21,6 +21,7 @@ use aptos_vm_types::{ resolver::{ResourceGroupSize, TExecutorView, TResourceGroupView}, }; use move_core_types::{value::MoveTypeLayout, vm_status::StatusCode}; +use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use std::{ collections::{BTreeMap, HashSet}, fmt::Debug, @@ -77,7 +78,6 @@ pub trait ExecutorTask: Sync { ::Key, ::Tag, MoveTypeLayout, - ::Identifier, ::Value, > + TResourceGroupView< GroupKey = ::Key, @@ -118,12 +118,7 @@ pub trait TransactionOutput: Send + Sync + Debug { fn aggregator_v1_delta_set(&self) -> Vec<(::Key, DeltaOp)>; /// Get the delayed field changes of a transaction from its output. - fn delayed_field_change_set( - &self, - ) -> BTreeMap< - ::Identifier, - DelayedChange<::Identifier>, - >; + fn delayed_field_change_set(&self) -> BTreeMap>; fn reads_needing_delayed_field_exchange( &self, @@ -204,11 +199,5 @@ pub trait TransactionOutput: Send + Sync + Debug { fn get_write_summary( &self, - ) -> HashSet< - InputOutputKey< - ::Key, - ::Tag, - ::Identifier, - >, - >; + ) -> HashSet::Key, ::Tag>>; } diff --git a/aptos-move/block-executor/src/txn_last_input_output.rs b/aptos-move/block-executor/src/txn_last_input_output.rs index fcfbde7452aa7..0907d81202c72 100644 --- a/aptos-move/block-executor/src/txn_last_input_output.rs +++ b/aptos-move/block-executor/src/txn_last_input_output.rs @@ -25,6 +25,7 @@ use dashmap::DashSet; use move_binary_format::CompiledModule; use move_core_types::{language_storage::ModuleId, value::MoveTypeLayout}; use move_vm_runtime::{Module, RuntimeEnvironment}; +use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use std::{ collections::{BTreeMap, HashSet}, fmt::Debug, @@ -356,7 +357,7 @@ impl, E: Debug + Send + Clone> pub(crate) fn delayed_field_keys( &self, txn_idx: TxnIndex, - ) -> Option> { + ) -> Option> { self.outputs[txn_idx as usize] .load() .as_ref() @@ -462,7 +463,7 @@ impl, E: Debug + Send + Clone> pub(crate) fn get_write_summary( &self, txn_idx: TxnIndex, - ) -> HashSet> { + ) -> HashSet> { match self.outputs[txn_idx as usize] .load_full() .expect("Output must exist") diff --git a/aptos-move/block-executor/src/types.rs b/aptos-move/block-executor/src/types.rs index 8e4c987d91fe5..cd162d271925b 100644 --- a/aptos-move/block-executor/src/types.rs +++ b/aptos-move/block-executor/src/types.rs @@ -2,24 +2,25 @@ // SPDX-License-Identifier: Apache-2.0 use aptos_types::transaction::BlockExecutableTransaction as Transaction; +use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use std::{collections::HashSet, fmt}; #[derive(Eq, Hash, PartialEq, Debug)] -pub enum InputOutputKey { +pub enum InputOutputKey { Resource(K), Group(K, T), - DelayedField(I), + DelayedField(DelayedFieldID), } pub struct ReadWriteSummary { - reads: HashSet>, - writes: HashSet>, + reads: HashSet>, + writes: HashSet>, } impl ReadWriteSummary { pub fn new( - reads: HashSet>, - writes: HashSet>, + reads: HashSet>, + writes: HashSet>, ) -> Self { Self { reads, writes } } @@ -29,7 +30,7 @@ impl ReadWriteSummary { } pub fn collapse_resource_group_conflicts(self) -> Self { - let collapse = |k: InputOutputKey| match k { + let collapse = |k: InputOutputKey| match k { InputOutputKey::Resource(k) => InputOutputKey::Resource(k), InputOutputKey::Group(k, _) => InputOutputKey::Resource(k), InputOutputKey::DelayedField(id) => InputOutputKey::DelayedField(id), diff --git a/aptos-move/block-executor/src/value_exchange.rs b/aptos-move/block-executor/src/value_exchange.rs index e28ec35dddd03..0855164565ee3 100644 --- a/aptos-move/block-executor/src/value_exchange.rs +++ b/aptos-move/block-executor/src/value_exchange.rs @@ -17,9 +17,10 @@ use aptos_types::{ use bytes::Bytes; use move_binary_format::errors::PartialVMResult; use move_core_types::value::{IdentifierMappingKind, MoveTypeLayout}; +use move_vm_runtime::AsFunctionValueExtension; use move_vm_types::{ - delayed_values::delayed_field_id::{ExtractWidth, TryFromMoveValue}, - value_serde::{deserialize_and_allow_delayed_values, ValueToIdentifierMapping}, + delayed_values::delayed_field_id::{DelayedFieldID, ExtractWidth, TryFromMoveValue}, + value_serde::{ValueSerDeContext, ValueToIdentifierMapping}, value_traversal::find_identifiers_in_value, values::Value, }; @@ -35,7 +36,7 @@ pub(crate) struct TemporaryValueToIdentifierMapping< txn_idx: TxnIndex, // These are the delayed field keys that were touched when utilizing this mapping // to replace ids with values or values with ids - delayed_field_ids: RefCell>, + delayed_field_ids: RefCell>, } impl<'a, T: Transaction, S: TStateView, X: Executable> @@ -49,11 +50,11 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> } } - fn generate_delayed_field_id(&self, width: u32) -> T::Identifier { + fn generate_delayed_field_id(&self, width: u32) -> DelayedFieldID { self.latest_view.generate_delayed_field_id(width) } - pub fn into_inner(self) -> HashSet { + pub fn into_inner(self) -> HashSet { self.delayed_field_ids.into_inner() } } @@ -64,14 +65,12 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> impl<'a, T: Transaction, S: TStateView, X: Executable> ValueToIdentifierMapping for TemporaryValueToIdentifierMapping<'a, T, S, X> { - type Identifier = T::Identifier; - fn value_to_identifier( &self, kind: &IdentifierMappingKind, layout: &MoveTypeLayout, value: Value, - ) -> PartialVMResult { + ) -> PartialVMResult { let (base_value, width) = DelayedFieldValue::try_from_move_value(layout, value, kind)?; let id = self.generate_delayed_field_id(width); match &self.latest_view.latest_view { @@ -85,7 +84,7 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> ValueToIden fn identifier_to_value( &self, layout: &MoveTypeLayout, - identifier: Self::Identifier, + identifier: DelayedFieldID, ) -> PartialVMResult { self.delayed_field_ids.borrow_mut().insert(identifier); let delayed_field = match &self.latest_view.latest_view { @@ -106,68 +105,87 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> ValueToIden } } -// Given bytes, where values were already exchanged with identifiers, -// return a list of identifiers present in it. -fn extract_identifiers_from_value( - bytes: &Bytes, - layout: &MoveTypeLayout, -) -> anyhow::Result> { - // TODO[agg_v2](optimize): this performs 2 traversals of a value: - // 1) deserialize, - // 2) find identifiers to populate the set. - // See if can cache identifiers in advance, or combine it with - // deserialization. - let value = deserialize_and_allow_delayed_values(bytes, layout) - .ok_or_else(|| anyhow::anyhow!("Failed to deserialize resource during id replacement"))?; - - let mut identifiers = HashSet::new(); - find_identifiers_in_value(&value, &mut identifiers)?; - // TODO[agg_v2](cleanup): ugly way of converting delayed ids to generic type params. - Ok(identifiers.into_iter().map(T::Identifier::from).collect()) -} +impl<'a, T, S, X> LatestView<'a, T, S, X> +where + T: Transaction, + S: TStateView, + X: Executable, +{ + /// Given bytes, where values were already exchanged with identifiers, returns a list of + /// identifiers present in it. + fn extract_identifiers_from_value( + &self, + bytes: &Bytes, + layout: &MoveTypeLayout, + ) -> anyhow::Result> { + // TODO[agg_v2](optimize): this performs 2 traversals of a value: + // 1) deserialize, + // 2) find identifiers to populate the set. + // See if can cache identifiers in advance, or combine it with + // deserialization. + let function_value_extension = self.as_function_value_extension(); + let value = ValueSerDeContext::new() + .with_func_args_deserialization(&function_value_extension) + .with_delayed_fields_serde() + .deserialize(bytes, layout) + .ok_or_else(|| { + anyhow::anyhow!("Failed to deserialize resource during id replacement") + })?; -// Deletion returns a PanicError. -pub(crate) fn does_value_need_exchange( - value: &T::Value, - layout: &MoveTypeLayout, - delayed_write_set_ids: &HashSet, -) -> Result { - if let Some(bytes) = value.bytes() { - extract_identifiers_from_value::(bytes, layout) - .map(|identifiers_in_read| !delayed_write_set_ids.is_disjoint(&identifiers_in_read)) - .map_err(|e| code_invariant_error(format!("Identifier extraction failed with {:?}", e))) - } else { - // Deletion returns an error. - Err(code_invariant_error( - "Delete shouldn't be in values considered for exchange", - )) + let mut identifiers = HashSet::new(); + find_identifiers_in_value(&value, &mut identifiers)?; + // TODO[agg_v2](cleanup): ugly way of converting delayed ids to generic type params. + Ok(identifiers.into_iter().map(DelayedFieldID::from).collect()) } -} -// Exclude deletions, and values that do not contain any delayed field IDs that were written to. -pub(crate) fn filter_value_for_exchange( - value: &T::Value, - layout: &Arc, - delayed_write_set_ids: &HashSet, - key: &T::Key, -) -> Option)), PanicError>> { - if value.is_deletion() { - None - } else { - does_value_need_exchange::(value, layout, delayed_write_set_ids).map_or_else( - |e| Some(Err(e)), - |needs_exchange| { - needs_exchange.then(|| { - Ok(( - key.clone(), - ( - value.as_state_value_metadata().unwrap().clone(), - value.write_op_size().write_len().unwrap(), - layout.clone(), - ), - )) + // Deletion returns a PanicError. + pub(crate) fn does_value_need_exchange( + &self, + value: &T::Value, + layout: &MoveTypeLayout, + delayed_write_set_ids: &HashSet, + ) -> Result { + if let Some(bytes) = value.bytes() { + self.extract_identifiers_from_value(bytes, layout) + .map(|identifiers_in_read| !delayed_write_set_ids.is_disjoint(&identifiers_in_read)) + .map_err(|e| { + code_invariant_error(format!("Identifier extraction failed with {:?}", e)) }) - }, - ) + } else { + // Deletion returns an error. + Err(code_invariant_error( + "Delete shouldn't be in values considered for exchange", + )) + } + } + + // Exclude deletions, and values that do not contain any delayed field IDs that were written to. + pub(crate) fn filter_value_for_exchange( + &self, + value: &T::Value, + layout: &Arc, + delayed_write_set_ids: &HashSet, + key: &T::Key, + ) -> Option)), PanicError>> { + if value.is_deletion() { + None + } else { + self.does_value_need_exchange(value, layout, delayed_write_set_ids) + .map_or_else( + |e| Some(Err(e)), + |needs_exchange| { + needs_exchange.then(|| { + Ok(( + key.clone(), + ( + value.as_state_value_metadata().unwrap().clone(), + value.write_op_size().write_len().unwrap(), + layout.clone(), + ), + )) + }) + }, + ) + } } } diff --git a/aptos-move/block-executor/src/view.rs b/aptos-move/block-executor/src/view.rs index 7f4b3f3daa053..38d9ef7d30647 100644 --- a/aptos-move/block-executor/src/view.rs +++ b/aptos-move/block-executor/src/view.rs @@ -11,9 +11,7 @@ use crate::{ code_cache_global::GlobalModuleCache, counters, scheduler::{DependencyResult, DependencyStatus, Scheduler, TWaitForDependency}, - value_exchange::{ - does_value_need_exchange, filter_value_for_exchange, TemporaryValueToIdentifierMapping, - }, + value_exchange::TemporaryValueToIdentifierMapping, }; use aptos_aggregator::{ bounded_math::{ok_overflow, BoundedMath, SignedU128}, @@ -57,13 +55,10 @@ use move_binary_format::{ CompiledModule, }; use move_core_types::{language_storage::ModuleId, value::MoveTypeLayout, vm_status::StatusCode}; -use move_vm_runtime::{Module, RuntimeEnvironment}; +use move_vm_runtime::{AsFunctionValueExtension, Module, RuntimeEnvironment}; use move_vm_types::{ - delayed_values::delayed_field_id::ExtractUniqueIndex, - value_serde::{ - deserialize_and_allow_delayed_values, deserialize_and_replace_values_with_ids, - serialize_and_allow_delayed_values, serialize_and_replace_ids_with_values, - }, + delayed_values::delayed_field_id::{DelayedFieldID, ExtractUniqueIndex}, + value_serde::ValueSerDeContext, }; use std::{ cell::RefCell, @@ -163,7 +158,7 @@ trait ResourceGroupState { } pub(crate) struct ParallelState<'a, T: Transaction, X: Executable> { - pub(crate) versioned_map: &'a MVHashMap, + pub(crate) versioned_map: &'a MVHashMap, scheduler: &'a Scheduler, start_counter: u32, counter: &'a AtomicU32, @@ -175,9 +170,9 @@ fn get_delayed_field_value_impl( captured_reads: &RefCell< CapturedReads, >, - versioned_delayed_fields: &dyn TVersionedDelayedFieldView, + versioned_delayed_fields: &dyn TVersionedDelayedFieldView, wait_for: &dyn TWaitForDependency, - id: &T::Identifier, + id: &DelayedFieldID, txn_idx: TxnIndex, ) -> Result> { // We expect only DelayedFieldReadKind::Value (which is set from this function), @@ -315,9 +310,9 @@ fn delayed_field_try_add_delta_outcome_impl( captured_reads: &RefCell< CapturedReads, >, - versioned_delayed_fields: &dyn TVersionedDelayedFieldView, + versioned_delayed_fields: &dyn TVersionedDelayedFieldView, wait_for: &dyn TWaitForDependency, - id: &T::Identifier, + id: &DelayedFieldID, base_delta: &SignedU128, delta: &SignedU128, max_value: u128, @@ -451,7 +446,7 @@ fn wait_for_dependency( impl<'a, T: Transaction, X: Executable> ParallelState<'a, T, X> { pub(crate) fn new( - shared_map: &'a MVHashMap, + shared_map: &'a MVHashMap, shared_scheduler: &'a Scheduler, start_shared_counter: u32, shared_counter: &'a AtomicU32, @@ -465,7 +460,11 @@ impl<'a, T: Transaction, X: Executable> ParallelState<'a, T, X> { } } - pub(crate) fn set_delayed_field_value(&self, id: T::Identifier, base_value: DelayedFieldValue) { + pub(crate) fn set_delayed_field_value( + &self, + id: DelayedFieldID, + base_value: DelayedFieldValue, + ) { self.versioned_map .delayed_fields() .set_base_value(id, base_value) @@ -787,7 +786,7 @@ impl<'a, T: Transaction, X: Executable> ResourceGroupState for ParallelState< } pub(crate) struct SequentialState<'a, T: Transaction> { - pub(crate) unsync_map: &'a UnsyncMap, + pub(crate) unsync_map: &'a UnsyncMap, pub(crate) read_set: RefCell>, pub(crate) start_counter: u32, pub(crate) counter: &'a RefCell, @@ -797,7 +796,7 @@ pub(crate) struct SequentialState<'a, T: Transaction> { impl<'a, T: Transaction> SequentialState<'a, T> { pub fn new( - unsync_map: &'a UnsyncMap, + unsync_map: &'a UnsyncMap, start_counter: u32, counter: &'a RefCell, ) -> Self { @@ -810,11 +809,15 @@ impl<'a, T: Transaction> SequentialState<'a, T> { } } - pub(crate) fn set_delayed_field_value(&self, id: T::Identifier, base_value: DelayedFieldValue) { + pub(crate) fn set_delayed_field_value( + &self, + id: DelayedFieldID, + base_value: DelayedFieldValue, + ) { self.unsync_map.set_base_delayed_field(id, base_value) } - pub(crate) fn read_delayed_field(&self, id: T::Identifier) -> Option { + pub(crate) fn read_delayed_field(&self, id: DelayedFieldID) -> Option { self.unsync_map.fetch_delayed_field(&id) } } @@ -1020,7 +1023,7 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< } #[cfg(test)] - fn get_read_summary(&self) -> HashSet> { + fn get_read_summary(&self) -> HashSet> { match &self.latest_view { ViewState::Sync(state) => state.captured_reads.borrow().get_read_summary(), ViewState::Unsync(state) => state.read_set.borrow().get_read_summary(), @@ -1130,20 +1133,28 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< &self, state_value: StateValue, layout: &MoveTypeLayout, - ) -> anyhow::Result<(StateValue, HashSet)> { + ) -> anyhow::Result<(StateValue, HashSet)> { let mapping = TemporaryValueToIdentifierMapping::new(self, self.txn_idx); + let function_value_extension = self.as_function_value_extension(); + state_value .map_bytes(|bytes| { // This call will replace all occurrences of aggregator / snapshot // values with unique identifiers with the same type layout. // The values are stored in aggregators multi-version data structure, // see the actual trait implementation for more details. - let patched_value = - deserialize_and_replace_values_with_ids(bytes.as_ref(), layout, &mapping) - .ok_or_else(|| { - anyhow::anyhow!("Failed to deserialize resource during id replacement") - })?; - serialize_and_allow_delayed_values(&patched_value, layout)? + let patched_value = ValueSerDeContext::new() + .with_delayed_fields_replacement(&mapping) + .with_func_args_deserialization(&function_value_extension) + .deserialize(bytes.as_ref(), layout) + .ok_or_else(|| { + anyhow::anyhow!("Failed to deserialize resource during id replacement") + })?; + + ValueSerDeContext::new() + .with_delayed_fields_serde() + .with_func_args_deserialization(&function_value_extension) + .serialize(&patched_value, layout)? .ok_or_else(|| { anyhow::anyhow!( "Failed to serialize value {} after id replacement", @@ -1161,17 +1172,26 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< &self, bytes: &Bytes, layout: &MoveTypeLayout, - ) -> anyhow::Result<(Bytes, HashSet)> { + ) -> anyhow::Result<(Bytes, HashSet)> { // This call will replace all occurrences of aggregator / snapshot // identifiers with values with the same type layout. - let value = deserialize_and_allow_delayed_values(bytes, layout).ok_or_else(|| { - anyhow::anyhow!( - "Failed to deserialize resource during id replacement: {:?}", - bytes - ) - })?; + let function_value_extension = self.as_function_value_extension(); + let value = ValueSerDeContext::new() + .with_func_args_deserialization(&function_value_extension) + .with_delayed_fields_serde() + .deserialize(bytes, layout) + .ok_or_else(|| { + anyhow::anyhow!( + "Failed to deserialize resource during id replacement: {:?}", + bytes + ) + })?; + let mapping = TemporaryValueToIdentifierMapping::new(self, self.txn_idx); - let patched_bytes = serialize_and_replace_ids_with_values(&value, layout, &mapping) + let patched_bytes = ValueSerDeContext::new() + .with_delayed_fields_replacement(&mapping) + .with_func_args_deserialization(&function_value_extension) + .serialize(&value, layout)? .ok_or_else(|| anyhow::anyhow!("Failed to serialize resource during id replacement"))? .into(); Ok((patched_bytes, mapping.into_inner())) @@ -1180,8 +1200,8 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< fn get_reads_needing_exchange_sequential( &self, read_set: &HashSet, - unsync_map: &UnsyncMap, - delayed_write_set_ids: &HashSet, + unsync_map: &UnsyncMap, + delayed_write_set_ids: &HashSet, skip: &HashSet, ) -> Result)>, PanicError> { read_set @@ -1192,14 +1212,13 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< } match unsync_map.fetch_data(key) { - Some(ValueWithLayout::Exchanged(value, Some(layout))) => { - filter_value_for_exchange::( + Some(ValueWithLayout::Exchanged(value, Some(layout))) => self + .filter_value_for_exchange( value.as_ref(), &layout, delayed_write_set_ids, key, - ) - }, + ), Some(ValueWithLayout::Exchanged(_, None)) => None, Some(ValueWithLayout::RawFromStorage(_)) => Some(Err(code_invariant_error( "Cannot exchange value that was not exchanged before", @@ -1213,7 +1232,7 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< fn get_group_reads_needing_exchange_parallel( &self, parallel_state: &ParallelState<'a, T, X>, - delayed_write_set_ids: &HashSet, + delayed_write_set_ids: &HashSet, skip: &HashSet, ) -> PartialVMResult> { let reads_with_delayed_fields = parallel_state @@ -1233,12 +1252,9 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< let mut resources_needing_delayed_field_exchange = false; for data_read in inner_reads.values() { if let DataRead::Versioned(_version, value, Some(layout)) = data_read { - let needs_exchange = does_value_need_exchange::( - value, - layout.as_ref(), - delayed_write_set_ids, - ) - .map_err(PartialVMError::from)?; + let needs_exchange = self + .does_value_need_exchange(value, layout.as_ref(), delayed_write_set_ids) + .map_err(PartialVMError::from)?; if needs_exchange { resources_needing_delayed_field_exchange = true; @@ -1277,8 +1293,8 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< fn get_group_reads_needing_exchange_sequential( &self, group_read_set: &HashMap>, - unsync_map: &UnsyncMap, - delayed_write_set_ids: &HashSet, + unsync_map: &UnsyncMap, + delayed_write_set_ids: &HashSet, skip: &HashSet, ) -> PartialVMResult> { group_read_set @@ -1293,7 +1309,7 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< if let ValueWithLayout::Exchanged(value, Some(layout)) = value_with_layout { - let needs_exchange = does_value_need_exchange::( + let needs_exchange = self.does_value_need_exchange( &value, layout.as_ref(), delayed_write_set_ids, @@ -1659,7 +1675,7 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> TAggregator impl<'a, T: Transaction, S: TStateView, X: Executable> TDelayedFieldView for LatestView<'a, T, S, X> { - type Identifier = T::Identifier; + type Identifier = DelayedFieldID; type ResourceGroupTag = T::Tag; type ResourceKey = T::Key; @@ -1770,7 +1786,7 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> TDelayedFie ViewState::Sync(state) => state .captured_reads .borrow() - .get_read_values_with_delayed_fields(delayed_write_set_ids, skip), + .get_read_values_with_delayed_fields(self, delayed_write_set_ids, skip), ViewState::Unsync(state) => { let read_set = state.read_set.borrow(); self.get_reads_needing_exchange_sequential( @@ -1836,7 +1852,6 @@ mod test { write_set::TransactionWrite, }; use aptos_vm_types::resolver::TResourceView; - use bytes::Bytes; use claims::{assert_err_eq, assert_none, assert_ok_eq, assert_some_eq}; use move_core_types::value::{IdentifierMappingKind, MoveStructLayout, MoveTypeLayout}; use move_vm_types::{ @@ -1902,7 +1917,6 @@ mod test { impl BlockExecutableTransaction for TestTransactionType { type Event = MockEvent; - type Identifier = DelayedFieldID; type Key = KeyType; type Tag = u32; type Value = ValueType; @@ -2479,7 +2493,13 @@ mod test { } fn create_state_value(value: &Value, layout: &MoveTypeLayout) -> StateValue { - StateValue::new_legacy(value.simple_serialize(layout).unwrap().into()) + StateValue::new_legacy( + ValueSerDeContext::new() + .serialize(value, layout) + .unwrap() + .unwrap() + .into(), + ) } #[derive(Clone)] @@ -2520,7 +2540,7 @@ mod test { Value::u64(2), Value::u64(3), ])); - let state_value = StateValue::new_legacy(value.simple_serialize(&layout).unwrap().into()); + let state_value = create_state_value(&value, &layout); let (patched_state_value, identifiers) = latest_view .replace_values_with_identifiers(state_value.clone(), &layout) .unwrap(); @@ -2546,8 +2566,7 @@ mod test { let storage_layout = create_struct_layout(create_aggregator_storage_layout(MoveTypeLayout::U64)); let value = create_struct_value(create_aggregator_value_u64(25, 30)); - let state_value = - StateValue::new_legacy(value.simple_serialize(&storage_layout).unwrap().into()); + let state_value = create_state_value(&value, &storage_layout); let layout = create_struct_layout(create_aggregator_layout_u64()); let (patched_state_value, identifiers) = latest_view @@ -2585,8 +2604,7 @@ mod test { create_aggregator_value_u64(35, 65), create_aggregator_value_u64(0, 20), ])); - let state_value = - StateValue::new_legacy(value.simple_serialize(&storage_layout).unwrap().into()); + let state_value = create_state_value(&value, &storage_layout); let layout = create_struct_layout(create_vector_layout(create_aggregator_layout_u64())); let (patched_state_value, identifiers) = latest_view @@ -2619,12 +2637,7 @@ mod test { ])])); assert_eq!( patched_state_value, - StateValue::new_legacy( - patched_value - .simple_serialize(&storage_layout) - .unwrap() - .into() - ) + create_state_value(&patched_value, &storage_layout), ); let (final_state_value, identifiers) = latest_view .replace_identifiers_with_values(patched_state_value.bytes(), &layout) @@ -2649,8 +2662,7 @@ mod test { create_snapshot_value(Value::u128(35)), create_snapshot_value(Value::u128(0)), ])); - let state_value = - StateValue::new_legacy(value.simple_serialize(&storage_layout).unwrap().into()); + let state_value = create_state_value(&value, &storage_layout); let layout = create_struct_layout(create_vector_layout(create_snapshot_layout( MoveTypeLayout::U128, @@ -2682,12 +2694,7 @@ mod test { ])])); assert_eq!( patched_state_value, - StateValue::new_legacy( - patched_value - .simple_serialize(&storage_layout) - .unwrap() - .into() - ) + create_state_value(&patched_value, &storage_layout), ); let (final_state_value, identifiers2) = latest_view .replace_identifiers_with_values(patched_state_value.bytes(), &layout) @@ -2712,8 +2719,7 @@ mod test { create_derived_value("ab", 55), create_derived_value("c", 50), ])); - let state_value = - StateValue::new_legacy(value.simple_serialize(&storage_layout).unwrap().into()); + let state_value = create_state_value(&value, &storage_layout); let layout = create_struct_layout(create_vector_layout(create_derived_string_layout())); let (patched_state_value, identifiers) = latest_view @@ -2744,12 +2750,7 @@ mod test { ])])); assert_eq!( patched_state_value, - StateValue::new_legacy( - patched_value - .simple_serialize(&storage_layout) - .unwrap() - .into() - ) + create_state_value(&patched_value, &storage_layout), ); let (final_state_value, identifiers2) = latest_view .replace_identifiers_with_values(patched_state_value.bytes(), &layout) @@ -3100,18 +3101,13 @@ mod test { #[test] fn test_read_operations() { - let state_value_3 = StateValue::new_legacy(Bytes::from( - Value::u64(12321) - .simple_serialize(&MoveTypeLayout::U64) - .unwrap(), - )); + let state_value_3 = create_state_value(&Value::u64(12321), &MoveTypeLayout::U64); let mut data = HashMap::new(); data.insert(KeyType::(3, false), state_value_3.clone()); let storage_layout = create_struct_layout(create_aggregator_storage_layout(MoveTypeLayout::U64)); let value = create_struct_value(create_aggregator_value_u64(25, 30)); - let state_value_4 = - StateValue::new_legacy(value.simple_serialize(&storage_layout).unwrap().into()); + let state_value_4 = create_state_value(&value, &storage_layout); data.insert(KeyType::(4, false), state_value_4); let start_counter = 1000; @@ -3150,12 +3146,7 @@ mod test { ); let patched_value = create_struct_value(create_aggregator_value_u64(id.as_u64(), 30)); - let state_value_4 = StateValue::new_legacy( - patched_value - .simple_serialize(&storage_layout) - .unwrap() - .into(), - ); + let state_value_4 = create_state_value(&patched_value, &storage_layout); assert_eq!( views .get_resource_state_value(&KeyType::(4, false), Some(&layout)) @@ -3177,8 +3168,11 @@ mod test { let captured_reads = views.latest_view_par.take_parallel_reads(); assert!(captured_reads.validate_data_reads(holder.versioned_map.data(), 1)); // TODO(aggr_v2): what's up with this test case? - let _read_set_with_delayed_fields = - captured_reads.get_read_values_with_delayed_fields(&HashSet::new(), &HashSet::new()); + let _read_set_with_delayed_fields = captured_reads.get_read_values_with_delayed_fields( + &views.latest_view_par, + &HashSet::new(), + &HashSet::new(), + ); // TODO[agg_v2](test): This prints // read: (KeyType(4, false), Versioned(Err(StorageVersion), Some(Struct(Runtime([Struct(Runtime([Tagged(IdentifierMapping(Aggregator), U64), U64]))]))))) diff --git a/aptos-move/framework/move-stdlib/src/natives/bcs.rs b/aptos-move/framework/move-stdlib/src/natives/bcs.rs index 1ba7a7f17ca3f..d7cd0e9d258c4 100644 --- a/aptos-move/framework/move-stdlib/src/natives/bcs.rs +++ b/aptos-move/framework/move-stdlib/src/natives/bcs.rs @@ -21,7 +21,7 @@ use move_vm_runtime::native_functions::NativeFunction; use move_vm_types::{ loaded_data::runtime_types::Type, natives::function::{PartialVMError, PartialVMResult}, - value_serde::serialized_size_allowing_delayed_values, + value_serde::ValueSerDeContext, values::{values_impl::Reference, Struct, Value}, }; use smallvec::{smallvec, SmallVec}; @@ -69,7 +69,10 @@ fn native_to_bytes( // implement it in a more efficient way. let val = ref_to_val.read_ref()?; - let serialized_value = match val.simple_serialize(&layout) { + let serialized_value = match ValueSerDeContext::new() + .with_func_args_deserialization(context.function_value_extension()) + .serialize(&val, &layout)? + { Some(serialized_value) => serialized_value, None => { context.charge(BCS_TO_BYTES_FAILURE)?; @@ -131,7 +134,11 @@ fn serialized_size_impl( // implement it in a more efficient way. let value = reference.read_ref()?; let ty_layout = context.type_to_type_layout(ty)?; - serialized_size_allowing_delayed_values(&value, &ty_layout) + + ValueSerDeContext::new() + .with_func_args_deserialization(context.function_value_extension()) + .with_delayed_fields_serde() + .serialized_size(&value, &ty_layout) } fn native_constant_serialized_size( diff --git a/aptos-move/framework/src/natives/event.rs b/aptos-move/framework/src/natives/event.rs index 01de485b3f4fc..f970335350490 100644 --- a/aptos-move/framework/src/natives/event.rs +++ b/aptos-move/framework/src/natives/event.rs @@ -16,12 +16,9 @@ use move_binary_format::errors::PartialVMError; use move_core_types::{language_storage::TypeTag, value::MoveTypeLayout, vm_status::StatusCode}; use move_vm_runtime::native_functions::NativeFunction; #[cfg(feature = "testing")] -use move_vm_types::value_serde::deserialize_and_allow_delayed_values; -#[cfg(feature = "testing")] use move_vm_types::values::{Reference, Struct, StructRef}; use move_vm_types::{ - loaded_data::runtime_types::Type, value_serde::serialize_and_allow_delayed_values, - values::Value, + loaded_data::runtime_types::Type, value_serde::ValueSerDeContext, values::Value, }; use smallvec::{smallvec, SmallVec}; use std::collections::VecDeque; @@ -92,11 +89,16 @@ fn native_write_to_event_store( let ty_tag = context.type_to_type_tag(&ty)?; let (layout, has_aggregator_lifting) = context.type_to_type_layout_with_identifier_mappings(&ty)?; - let blob = serialize_and_allow_delayed_values(&msg, &layout)?.ok_or_else(|| { - SafeNativeError::InvariantViolation(PartialVMError::new( - StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, - )) - })?; + + let blob = ValueSerDeContext::new() + .with_delayed_fields_serde() + .with_func_args_deserialization(context.function_value_extension()) + .serialize(&msg, &layout)? + .ok_or_else(|| { + SafeNativeError::InvariantViolation(PartialVMError::new( + StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, + )) + })?; let key = bcs::from_bytes(guid.as_slice()).map_err(|_| { SafeNativeError::InvariantViolation(PartialVMError::new(StatusCode::EVENT_KEY_MISMATCH)) })?; @@ -147,16 +149,19 @@ fn native_emitted_events_by_handle( let key = EventKey::new(creation_num, addr); let ty_tag = context.type_to_type_tag(&ty)?; let ty_layout = context.type_to_type_layout(&ty)?; - let ctx = context.extensions_mut().get_mut::(); + let ctx = context.extensions().get::(); let events = ctx .emitted_v1_events(&key, &ty_tag) .into_iter() .map(|blob| { - Value::simple_deserialize(blob, &ty_layout).ok_or_else(|| { - SafeNativeError::InvariantViolation(PartialVMError::new( - StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, - )) - }) + ValueSerDeContext::new() + .with_func_args_deserialization(context.function_value_extension()) + .deserialize(blob, &ty_layout) + .ok_or_else(|| { + SafeNativeError::InvariantViolation(PartialVMError::new( + StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, + )) + }) }) .collect::>>()?; Ok(smallvec![Value::vector_for_testing_only(events)]) @@ -175,16 +180,21 @@ fn native_emitted_events( let ty_tag = context.type_to_type_tag(&ty)?; let ty_layout = context.type_to_type_layout(&ty)?; - let ctx = context.extensions_mut().get_mut::(); + let ctx = context.extensions().get::(); + let events = ctx .emitted_v2_events(&ty_tag) .into_iter() .map(|blob| { - deserialize_and_allow_delayed_values(blob, &ty_layout).ok_or_else(|| { - SafeNativeError::InvariantViolation(PartialVMError::new( - StatusCode::VALUE_DESERIALIZATION_ERROR, - )) - }) + ValueSerDeContext::new() + .with_func_args_deserialization(context.function_value_extension()) + .with_delayed_fields_serde() + .deserialize(blob, &ty_layout) + .ok_or_else(|| { + SafeNativeError::InvariantViolation(PartialVMError::new( + StatusCode::VALUE_DESERIALIZATION_ERROR, + )) + }) }) .collect::>>()?; Ok(smallvec![Value::vector_for_testing_only(events)]) @@ -234,12 +244,16 @@ fn native_write_module_event_to_store( } let (layout, has_identifier_mappings) = context.type_to_type_layout_with_identifier_mappings(&ty)?; - let blob = serialize_and_allow_delayed_values(&msg, &layout)?.ok_or_else(|| { - SafeNativeError::InvariantViolation( - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message("Event serialization failure".to_string()), - ) - })?; + let blob = ValueSerDeContext::new() + .with_delayed_fields_serde() + .with_func_args_deserialization(context.function_value_extension()) + .serialize(&msg, &layout)? + .ok_or_else(|| { + SafeNativeError::InvariantViolation( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message("Event serialization failure".to_string()), + ) + })?; let ctx = context.extensions_mut().get_mut::(); ctx.events.push(( ContractEvent::new_v2(type_tag, blob), diff --git a/aptos-move/framework/src/natives/util.rs b/aptos-move/framework/src/natives/util.rs index da30858dc4edf..f39239491fae1 100644 --- a/aptos-move/framework/src/natives/util.rs +++ b/aptos-move/framework/src/natives/util.rs @@ -8,7 +8,9 @@ use aptos_native_interface::{ }; use move_core_types::gas_algebra::NumBytes; use move_vm_runtime::native_functions::NativeFunction; -use move_vm_types::{loaded_data::runtime_types::Type, values::Value}; +use move_vm_types::{ + loaded_data::runtime_types::Type, value_serde::ValueSerDeContext, values::Value, +}; use smallvec::{smallvec, SmallVec}; use std::collections::VecDeque; @@ -40,7 +42,10 @@ fn native_from_bytes( context.charge( UTIL_FROM_BYTES_BASE + UTIL_FROM_BYTES_PER_BYTE * NumBytes::new(bytes.len() as u64), )?; - let val = match Value::simple_deserialize(&bytes, &layout) { + let val = match ValueSerDeContext::new() + .with_func_args_deserialization(context.function_value_extension()) + .deserialize(&bytes, &layout) + { Some(val) => val, None => { return Err(SafeNativeError::Abort { diff --git a/aptos-move/framework/table-natives/src/lib.rs b/aptos-move/framework/table-natives/src/lib.rs index d306b7a025890..ab1e0a6dd9190 100644 --- a/aptos-move/framework/table-natives/src/lib.rs +++ b/aptos-move/framework/table-natives/src/lib.rs @@ -28,7 +28,7 @@ pub use move_table_extension::{TableHandle, TableInfo, TableResolver}; use move_vm_runtime::native_functions::NativeFunctionTable; use move_vm_types::{ loaded_data::runtime_types::Type, - value_serde::{deserialize_and_allow_delayed_values, serialize_and_allow_delayed_values}, + value_serde::{FunctionValueExtension, ValueSerDeContext}, values::{GlobalValue, Reference, StructRef, Value}, }; use sha3::{Digest, Sha3_256}; @@ -118,7 +118,10 @@ impl<'a> NativeTableContext<'a> { } /// Computes the change set from a NativeTableContext. - pub fn into_change_set(self) -> PartialVMResult { + pub fn into_change_set( + self, + function_value_extension: &impl FunctionValueExtension, + ) -> PartialVMResult { let NativeTableContext { table_data, .. } = self; let TableData { new_tables, @@ -141,10 +144,24 @@ impl<'a> NativeTableContext<'a> { match op { Op::New(val) => { - entries.insert(key, Op::New(serialize_value(&value_layout_info, &val)?)); + entries.insert( + key, + Op::New(serialize_value( + function_value_extension, + &value_layout_info, + &val, + )?), + ); }, Op::Modify(val) => { - entries.insert(key, Op::Modify(serialize_value(&value_layout_info, &val)?)); + entries.insert( + key, + Op::Modify(serialize_value( + function_value_extension, + &value_layout_info, + &val, + )?), + ); }, Op::Delete => { entries.insert(key, Op::Delete); @@ -204,26 +221,33 @@ impl LayoutInfo { impl Table { fn get_or_create_global_value( &mut self, - context: &NativeTableContext, + function_value_extension: &dyn FunctionValueExtension, + table_context: &NativeTableContext, key: Vec, ) -> PartialVMResult<(&mut GlobalValue, Option>)> { Ok(match self.content.entry(key) { Entry::Vacant(entry) => { // If there is an identifier mapping, we need to pass layout to // ensure it gets recorded. - let data = context.resolver.resolve_table_entry_bytes_with_layout( - &self.handle, - entry.key(), - if self.value_layout_info.has_identifier_mappings { - Some(&self.value_layout_info.layout) - } else { - None - }, - )?; + let data = table_context + .resolver + .resolve_table_entry_bytes_with_layout( + &self.handle, + entry.key(), + if self.value_layout_info.has_identifier_mappings { + Some(&self.value_layout_info.layout) + } else { + None + }, + )?; let (gv, loaded) = match data { Some(val_bytes) => { - let val = deserialize_value(&self.value_layout_info, &val_bytes)?; + let val = deserialize_value( + function_value_extension, + &val_bytes, + &self.value_layout_info, + )?; ( GlobalValue::cached(val)?, Some(NumBytes::new(val_bytes.len() as u64)), @@ -341,6 +365,7 @@ fn native_add_box( context.charge(ADD_BOX_BASE)?; + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -350,10 +375,11 @@ fn native_add_box( let table = table_data.get_or_create_table(context, handle, &ty_args[0], &ty_args[2])?; - let key_bytes = serialize_key(&table.key_layout, &key)?; + let key_bytes = serialize_key(function_value_extension, &table.key_layout, &key)?; let key_cost = ADD_BOX_PER_BYTE_SERIALIZED * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; let res = match gv.move_to(val) { Ok(_) => Ok(smallvec![]), @@ -381,6 +407,7 @@ fn native_borrow_box( context.charge(BORROW_BOX_BASE)?; + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -389,10 +416,11 @@ fn native_borrow_box( let table = table_data.get_or_create_table(context, handle, &ty_args[0], &ty_args[2])?; - let key_bytes = serialize_key(&table.key_layout, &key)?; + let key_bytes = serialize_key(function_value_extension, &table.key_layout, &key)?; let key_cost = BORROW_BOX_PER_BYTE_SERIALIZED * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; let res = match gv.borrow_global() { Ok(ref_val) => Ok(smallvec![ref_val]), @@ -420,6 +448,7 @@ fn native_contains_box( context.charge(CONTAINS_BOX_BASE)?; + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -428,10 +457,11 @@ fn native_contains_box( let table = table_data.get_or_create_table(context, handle, &ty_args[0], &ty_args[2])?; - let key_bytes = serialize_key(&table.key_layout, &key)?; + let key_bytes = serialize_key(function_value_extension, &table.key_layout, &key)?; let key_cost = CONTAINS_BOX_PER_BYTE_SERIALIZED * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; let exists = Value::bool(gv.exists()?); drop(table_data); @@ -453,6 +483,7 @@ fn native_remove_box( context.charge(REMOVE_BOX_BASE)?; + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -461,10 +492,11 @@ fn native_remove_box( let table = table_data.get_or_create_table(context, handle, &ty_args[0], &ty_args[2])?; - let key_bytes = serialize_key(&table.key_layout, &key)?; + let key_bytes = serialize_key(function_value_extension, &table.key_layout, &key)?; let key_cost = REMOVE_BOX_PER_BYTE_SERIALIZED * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; let res = match gv.move_from() { Ok(val) => Ok(smallvec![val]), Err(_) => Err(SafeNativeError::Abort { @@ -528,34 +560,55 @@ fn get_table_handle(table: &StructRef) -> PartialVMResult { Ok(TableHandle(handle)) } -fn serialize_key(layout: &MoveTypeLayout, key: &Value) -> PartialVMResult> { - key.simple_serialize(layout) +fn serialize_key( + function_value_extension: &dyn FunctionValueExtension, + layout: &MoveTypeLayout, + key: &Value, +) -> PartialVMResult> { + ValueSerDeContext::new() + .with_func_args_deserialization(function_value_extension) + .serialize(key, layout)? .ok_or_else(|| partial_extension_error("cannot serialize table key")) } fn serialize_value( + function_value_extension: &dyn FunctionValueExtension, layout_info: &LayoutInfo, val: &Value, ) -> PartialVMResult<(Bytes, Option>)> { let serialization_result = if layout_info.has_identifier_mappings { // Value contains delayed fields, so we should be able to serialize it. - serialize_and_allow_delayed_values(val, layout_info.layout.as_ref())? + ValueSerDeContext::new() + .with_delayed_fields_serde() + .with_func_args_deserialization(function_value_extension) + .serialize(val, layout_info.layout.as_ref())? .map(|bytes| (bytes.into(), Some(layout_info.layout.clone()))) } else { // No delayed fields, make sure serialization fails if there are any // native values. - val.simple_serialize(layout_info.layout.as_ref()) + ValueSerDeContext::new() + .with_func_args_deserialization(function_value_extension) + .serialize(val, layout_info.layout.as_ref())? .map(|bytes| (bytes.into(), None)) }; serialization_result.ok_or_else(|| partial_extension_error("cannot serialize table value")) } -fn deserialize_value(layout_info: &LayoutInfo, bytes: &[u8]) -> PartialVMResult { +fn deserialize_value( + function_value_extension: &dyn FunctionValueExtension, + bytes: &[u8], + layout_info: &LayoutInfo, +) -> PartialVMResult { let layout = layout_info.layout.as_ref(); let deserialization_result = if layout_info.has_identifier_mappings { - deserialize_and_allow_delayed_values(bytes, layout) + ValueSerDeContext::new() + .with_func_args_deserialization(function_value_extension) + .with_delayed_fields_serde() + .deserialize(bytes, layout) } else { - Value::simple_deserialize(bytes, layout) + ValueSerDeContext::new() + .with_func_args_deserialization(function_value_extension) + .deserialize(bytes, layout) }; deserialization_result.ok_or_else(|| partial_extension_error("cannot deserialize table value")) } diff --git a/testsuite/fuzzer/fuzz/fuzz_targets/move/value_deserialize.rs b/testsuite/fuzzer/fuzz/fuzz_targets/move/value_deserialize.rs index 3cca926099b3b..fcd5cf6ebb645 100644 --- a/testsuite/fuzzer/fuzz/fuzz_targets/move/value_deserialize.rs +++ b/testsuite/fuzzer/fuzz/fuzz_targets/move/value_deserialize.rs @@ -5,7 +5,7 @@ use arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; use move_core_types::value::MoveTypeLayout; -use move_vm_types::values::Value; +use move_vm_types::value_serde::ValueSerDeContext; mod utils; use utils::helpers::is_valid_layout; @@ -19,5 +19,6 @@ fuzz_target!(|fuzz_data: FuzzData| { if fuzz_data.data.is_empty() || !is_valid_layout(&fuzz_data.layout) { return; } - let _ = Value::simple_deserialize(&fuzz_data.data, &fuzz_data.layout); + // TODO: How do we fuzz function resolution? + let _ = ValueSerDeContext::new().deserialize(&fuzz_data.data, &fuzz_data.layout); }); diff --git a/third_party/move/extensions/move-table-extension/src/lib.rs b/third_party/move/extensions/move-table-extension/src/lib.rs index b9235d3e71de9..3c876513723d1 100644 --- a/third_party/move/extensions/move-table-extension/src/lib.rs +++ b/third_party/move/extensions/move-table-extension/src/lib.rs @@ -26,6 +26,7 @@ use move_vm_types::{ loaded_data::runtime_types::Type, natives::function::NativeResult, pop_arg, + value_serde::{FunctionValueExtension, ValueSerDeContext}, values::{GlobalValue, Reference, StructRef, Value}, }; use sha3::{Digest, Sha3_256}; @@ -154,7 +155,10 @@ impl<'a> NativeTableContext<'a> { } /// Computes the change set from a NativeTableContext. - pub fn into_change_set(self) -> PartialVMResult { + pub fn into_change_set( + self, + function_value_extension: &impl FunctionValueExtension, + ) -> PartialVMResult { let NativeTableContext { table_data, .. } = self; let TableData { new_tables, @@ -177,11 +181,11 @@ impl<'a> NativeTableContext<'a> { match op { Op::New(val) => { - let bytes = serialize(&value_layout, &val)?; + let bytes = serialize(function_value_extension, &value_layout, &val)?; entries.insert(key, Op::New(bytes.into())); }, Op::Modify(val) => { - let bytes = serialize(&value_layout, &val)?; + let bytes = serialize(function_value_extension, &value_layout, &val)?; entries.insert(key, Op::Modify(bytes.into())); }, Op::Delete => { @@ -231,18 +235,19 @@ impl TableData { impl Table { fn get_or_create_global_value( &mut self, - context: &NativeTableContext, + function_value_extension: &dyn FunctionValueExtension, + table_context: &NativeTableContext, key: Vec, ) -> PartialVMResult<(&mut GlobalValue, Option>)> { Ok(match self.content.entry(key) { Entry::Vacant(entry) => { - let (gv, loaded) = match context.resolver.resolve_table_entry_bytes_with_layout( - &self.handle, - entry.key(), - None, - )? { + let (gv, loaded) = match table_context + .resolver + .resolve_table_entry_bytes_with_layout(&self.handle, entry.key(), None)? + { Some(val_bytes) => { - let val = deserialize(&self.value_layout, &val_bytes)?; + let val = + deserialize(function_value_extension, &val_bytes, &self.value_layout)?; ( GlobalValue::cached(val)?, Some(NumBytes::new(val_bytes.len() as u64)), @@ -390,6 +395,7 @@ fn native_add_box( assert_eq!(ty_args.len(), 3); assert_eq!(args.len(), 3); + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -401,10 +407,11 @@ fn native_add_box( let table = table_data.get_or_create_table(context, handle, &ty_args[0], &ty_args[2])?; - let key_bytes = serialize(&table.key_layout, &key)?; + let key_bytes = serialize(function_value_extension, &table.key_layout, &key)?; cost += gas_params.per_byte_serialized * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; cost += common_gas_params.calculate_load_cost(loaded); match gv.move_to(val) { @@ -440,6 +447,7 @@ fn native_borrow_box( assert_eq!(ty_args.len(), 3); assert_eq!(args.len(), 2); + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -450,10 +458,11 @@ fn native_borrow_box( let mut cost = gas_params.base; - let key_bytes = serialize(&table.key_layout, &key)?; + let key_bytes = serialize(function_value_extension, &table.key_layout, &key)?; cost += gas_params.per_byte_serialized * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; cost += common_gas_params.calculate_load_cost(loaded); match gv.borrow_global() { @@ -489,6 +498,7 @@ fn native_contains_box( assert_eq!(ty_args.len(), 3); assert_eq!(args.len(), 2); + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -499,10 +509,11 @@ fn native_contains_box( let mut cost = gas_params.base; - let key_bytes = serialize(&table.key_layout, &key)?; + let key_bytes = serialize(function_value_extension, &table.key_layout, &key)?; cost += gas_params.per_byte_serialized * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; cost += common_gas_params.calculate_load_cost(loaded); let exists = Value::bool(gv.exists()?); @@ -537,6 +548,7 @@ fn native_remove_box( assert_eq!(ty_args.len(), 3); assert_eq!(args.len(), 2); + let function_value_extension = context.function_value_extension(); let table_context = context.extensions().get::(); let mut table_data = table_context.table_data.borrow_mut(); @@ -547,10 +559,11 @@ fn native_remove_box( let mut cost = gas_params.base; - let key_bytes = serialize(&table.key_layout, &key)?; + let key_bytes = serialize(function_value_extension, &table.key_layout, &key)?; cost += gas_params.per_byte_serialized * NumBytes::new(key_bytes.len() as u64); - let (gv, loaded) = table.get_or_create_global_value(table_context, key_bytes)?; + let (gv, loaded) = + table.get_or_create_global_value(function_value_extension, table_context, key_bytes)?; cost += common_gas_params.calculate_load_cost(loaded); match gv.move_from() { @@ -685,13 +698,25 @@ fn get_table_handle(table: &StructRef) -> PartialVMResult { Ok(TableHandle(handle)) } -fn serialize(layout: &MoveTypeLayout, val: &Value) -> PartialVMResult> { - val.simple_serialize(layout) +fn serialize( + function_value_extension: &dyn FunctionValueExtension, + layout: &MoveTypeLayout, + val: &Value, +) -> PartialVMResult> { + ValueSerDeContext::new() + .with_func_args_deserialization(function_value_extension) + .serialize(val, layout)? .ok_or_else(|| partial_extension_error("cannot serialize table key or value")) } -fn deserialize(layout: &MoveTypeLayout, bytes: &[u8]) -> PartialVMResult { - Value::simple_deserialize(bytes, layout) +fn deserialize( + function_value_extension: &dyn FunctionValueExtension, + bytes: &[u8], + layout: &MoveTypeLayout, +) -> PartialVMResult { + ValueSerDeContext::new() + .with_func_args_deserialization(function_value_extension) + .deserialize(bytes, layout) .ok_or_else(|| partial_extension_error("cannot deserialize table key or value")) } diff --git a/third_party/move/move-stdlib/src/natives/bcs.rs b/third_party/move/move-stdlib/src/natives/bcs.rs index a9062a4cbbfad..c1722c0165fa1 100644 --- a/third_party/move/move-stdlib/src/natives/bcs.rs +++ b/third_party/move/move-stdlib/src/natives/bcs.rs @@ -13,6 +13,7 @@ use move_vm_types::{ loaded_data::runtime_types::Type, natives::function::NativeResult, pop_arg, + value_serde::ValueSerDeContext, values::{values_impl::Reference, Value}, }; use smallvec::smallvec; @@ -62,7 +63,10 @@ fn native_to_bytes( }; // serialize value let val = ref_to_val.read_ref()?; - let serialized_value = match val.simple_serialize(&layout) { + let serialized_value = match ValueSerDeContext::new() + .with_func_args_deserialization(context.function_value_extension()) + .serialize(&val, &layout)? + { Some(serialized_value) => serialized_value, None => { cost += gas_params.failure; diff --git a/third_party/move/move-vm/integration-tests/src/tests/mod.rs b/third_party/move/move-vm/integration-tests/src/tests/mod.rs index 23ca581ad7e71..18caef5ac3192 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/mod.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/mod.rs @@ -11,6 +11,7 @@ mod instantiation_tests; mod invariant_violation_tests; mod leak_tests; mod loader_tests; +mod module_storage_tests; mod mutated_accounts_tests; mod native_tests; mod nested_loop_tests; diff --git a/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs new file mode 100644 index 0000000000000..d5a1f80b228fd --- /dev/null +++ b/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs @@ -0,0 +1,137 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use crate::compiler::{as_module, compile_units}; +use bytes::Bytes; +use move_binary_format::file_format::{Ability, AbilitySet}; +use move_core_types::{ + account_address::AccountAddress, + ident_str, + identifier::Identifier, + language_storage::{ModuleId, TypeTag}, +}; +use move_vm_runtime::{ + AsFunctionValueExtension, AsUnsyncModuleStorage, RuntimeEnvironment, WithRuntimeEnvironment, +}; +use move_vm_test_utils::InMemoryStorage; +use move_vm_types::{ + loaded_data::runtime_types::{AbilityInfo, StructIdentifier, StructNameIndex, TypeBuilder}, + value_serde::FunctionValueExtension, +}; +use std::str::FromStr; + +#[cfg(test)] +fn module_bytes(module_code: &str) -> Bytes { + let compiled_module = as_module(compile_units(module_code).unwrap().pop().unwrap()); + let mut bytes = vec![]; + compiled_module.serialize(&mut bytes).unwrap(); + bytes.into() +} + +#[test] +fn test_function_value_extension() { + let mut module_bytes_storage = InMemoryStorage::new(); + + let code = r#" + module 0x1::test { + struct Foo {} + + fun b() { } + fun c(a: u64, _foo: &Foo): u64 { a } + fun d(_a: A, b: u64, _c: C): u64 { b } + } + "#; + let bytes = module_bytes(code); + let test_id = ModuleId::new(AccountAddress::ONE, Identifier::new("test").unwrap()); + module_bytes_storage.add_module_bytes(&AccountAddress::ONE, ident_str!("test"), bytes); + + let code = r#" + module 0x1::other_test { + struct Bar has drop {} + } + "#; + let bytes = module_bytes(code); + let other_test_id = ModuleId::new(AccountAddress::ONE, Identifier::new("other_test").unwrap()); + module_bytes_storage.add_module_bytes(&AccountAddress::ONE, ident_str!("other_test"), bytes); + + let runtime_environment = RuntimeEnvironment::new(vec![]); + let module_storage = module_bytes_storage.into_unsync_module_storage(runtime_environment); + let function_value_extension = module_storage.as_function_value_extension(); + + let result = function_value_extension.get_function_arg_tys( + &ModuleId::new(AccountAddress::ONE, Identifier::new("test").unwrap()), + ident_str!("a"), + vec![], + ); + assert!(result.is_err()); + + let mut types = function_value_extension + .get_function_arg_tys(&test_id, ident_str!("c"), vec![]) + .unwrap(); + assert_eq!(types.len(), 2); + + let ty_builder = TypeBuilder::with_limits(100, 100); + let foo_ty = types.pop().unwrap(); + let name = module_storage + .runtime_environment() + .idx_to_struct_name_for_test(StructNameIndex(0)) + .unwrap(); + assert_eq!(name, StructIdentifier { + module: test_id.clone(), + name: Identifier::new("Foo").unwrap(), + }); + assert_eq!( + foo_ty, + ty_builder + .create_ref_ty( + &ty_builder + .create_struct_ty(StructNameIndex(0), AbilityInfo::struct_(AbilitySet::EMPTY)), + false + ) + .unwrap() + ); + let u64_ty = types.pop().unwrap(); + assert_eq!(u64_ty, ty_builder.create_u64_ty()); + + // Generic function without type parameters should fail. + let result = function_value_extension.get_function_arg_tys( + &ModuleId::new(AccountAddress::ONE, Identifier::new("test").unwrap()), + ident_str!("d"), + vec![], + ); + assert!(result.is_err()); + + let mut types = function_value_extension + .get_function_arg_tys(&test_id, ident_str!("d"), vec![ + TypeTag::from_str("0x1::other_test::Bar").unwrap(), + TypeTag::Vector(Box::new(TypeTag::U8)), + ]) + .unwrap(); + assert_eq!(types.len(), 3); + + let vec_ty = types.pop().unwrap(); + assert_eq!( + vec_ty, + ty_builder + .create_vec_ty(&ty_builder.create_u8_ty()) + .unwrap() + ); + let u64_ty = types.pop().unwrap(); + assert_eq!(u64_ty, ty_builder.create_u64_ty()); + let bar_ty = types.pop().unwrap(); + let name = module_storage + .runtime_environment() + .idx_to_struct_name_for_test(StructNameIndex(1)) + .unwrap(); + assert_eq!(name, StructIdentifier { + module: other_test_id, + name: Identifier::new("Bar").unwrap(), + }); + assert_eq!( + bar_ty, + ty_builder.create_struct_ty( + StructNameIndex(1), + AbilityInfo::struct_(AbilitySet::from_u8(Ability::Drop as u8).unwrap()) + ) + ); +} diff --git a/third_party/move/move-vm/runtime/src/data_cache.rs b/third_party/move/move-vm/runtime/src/data_cache.rs index 0038b783d457b..197516b5d1dcd 100644 --- a/third_party/move/move-vm/runtime/src/data_cache.rs +++ b/third_party/move/move-vm/runtime/src/data_cache.rs @@ -5,6 +5,7 @@ use crate::{ loader::{LegacyModuleStorageAdapter, Loader}, logging::expect_no_verification_errors, + storage::module_storage::FunctionValueExtensionAdapter, ModuleStorage, }; use bytes::Bytes; @@ -26,7 +27,7 @@ use move_core_types::{ use move_vm_types::{ loaded_data::runtime_types::Type, resolver::MoveResolver, - value_serde::deserialize_and_allow_delayed_values, + value_serde::ValueSerDeContext, values::{GlobalValue, Value}, }; use sha3::{Digest, Sha3_256}; @@ -118,8 +119,10 @@ impl<'r> TransactionDataCache<'r> { ) -> PartialVMResult { let resource_converter = |value: Value, layout: MoveTypeLayout, _: bool| -> PartialVMResult { - value - .simple_serialize(&layout) + let function_value_extension = FunctionValueExtensionAdapter { module_storage }; + ValueSerDeContext::new() + .with_func_args_deserialization(&function_value_extension) + .serialize(&value, &layout)? .map(Into::into) .ok_or_else(|| { PartialVMError::new(StatusCode::INTERNAL_TYPE_ERROR) @@ -275,9 +278,14 @@ impl<'r> TransactionDataCache<'r> { }; load_res = Some(NumBytes::new(bytes_loaded as u64)); + let function_value_extension = FunctionValueExtensionAdapter { module_storage }; let gv = match data { Some(blob) => { - let val = match deserialize_and_allow_delayed_values(&blob, &ty_layout) { + let val = match ValueSerDeContext::new() + .with_func_args_deserialization(&function_value_extension) + .with_delayed_fields_serde() + .deserialize(&blob, &ty_layout) + { Some(val) => val, None => { let msg = diff --git a/third_party/move/move-vm/runtime/src/lib.rs b/third_party/move/move-vm/runtime/src/lib.rs index e9dd6f69a1f20..b4e7641d52cfe 100644 --- a/third_party/move/move-vm/runtime/src/lib.rs +++ b/third_party/move/move-vm/runtime/src/lib.rs @@ -34,7 +34,7 @@ mod frame_type_cache; mod runtime_type_checks; mod storage; -pub use loader::{LoadedFunction, Module, Script}; +pub use loader::{Function, LoadedFunction, Module, Script}; #[cfg(any(test, feature = "testing"))] pub use storage::implementations::unreachable_code_storage; pub use storage::{ @@ -46,6 +46,6 @@ pub use storage::{ unsync_code_storage::{AsUnsyncCodeStorage, UnsyncCodeStorage}, unsync_module_storage::{AsUnsyncModuleStorage, BorrowedOrOwned, UnsyncModuleStorage}, }, - module_storage::{ambassador_impl_ModuleStorage, ModuleStorage}, + module_storage::{ambassador_impl_ModuleStorage, AsFunctionValueExtension, ModuleStorage}, publishing::{StagingModuleStorage, VerifiedModuleBundle}, }; diff --git a/third_party/move/move-vm/runtime/src/loader/mod.rs b/third_party/move/move-vm/runtime/src/loader/mod.rs index 1777fd5ac9b9b..0bafef00c1f06 100644 --- a/third_party/move/move-vm/runtime/src/loader/mod.rs +++ b/third_party/move/move-vm/runtime/src/loader/mod.rs @@ -54,11 +54,12 @@ use crate::{ loader::modules::{StructVariantInfo, VariantFieldInfo}, native_functions::NativeFunctions, storage::{ - loader::LoaderV2, struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache, + loader::LoaderV2, module_storage::FunctionValueExtensionAdapter, + struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache, }, }; -pub use function::LoadedFunction; -pub(crate) use function::{Function, FunctionHandle, FunctionInstantiation, LoadedFunctionOwner}; +pub use function::{Function, LoadedFunction}; +pub(crate) use function::{FunctionHandle, FunctionInstantiation, LoadedFunctionOwner}; pub use modules::Module; pub(crate) use modules::{LegacyModuleCache, LegacyModuleStorage, LegacyModuleStorageAdapter}; use move_binary_format::file_format::{ @@ -66,7 +67,10 @@ use move_binary_format::file_format::{ VariantFieldInstantiationIndex, VariantIndex, }; use move_vm_metrics::{Timer, VM_TIMER}; -use move_vm_types::loaded_data::runtime_types::{StructLayout, TypeBuilder}; +use move_vm_types::{ + loaded_data::runtime_types::{StructLayout, TypeBuilder}, + value_serde::FunctionValueExtension, +}; pub use script::Script; pub(crate) use script::ScriptCache; use type_loader::intern_type; @@ -308,8 +312,7 @@ impl Loader { .resolve_module_and_function_by_name(module_id, function_name) .map_err(|err| err.finish(Location::Undefined)) }, - Loader::V2(loader) => loader.load_function_without_ty_args( - module_storage, + Loader::V2(_) => module_storage.fetch_function_definition( module_id.address(), module_id.name(), function_name, @@ -330,8 +333,8 @@ impl Loader { ) -> VMResult { match self { Self::V1(loader) => loader.load_type(ty_tag, data_store, module_store), - Self::V2(loader) => loader - .load_ty(module_storage, ty_tag) + Self::V2(_) => module_storage + .fetch_ty(ty_tag) .map_err(|e| e.finish(Location::Undefined)), } } @@ -356,8 +359,7 @@ impl Loader { Loader::V1(_) => { module_store.get_struct_type_by_identifier(&struct_name.name, &struct_name.module) }, - Loader::V2(loader) => loader.load_struct_ty( - module_storage, + Loader::V2(_) => module_storage.fetch_struct_ty( struct_name.module.address(), struct_name.module.name(), struct_name.name.as_ident_str(), @@ -1295,13 +1297,9 @@ impl<'a> Resolver<'a> { Loader::V1(_) => self .module_store .resolve_module_and_function_by_name(module_id, function_name)?, - Loader::V2(loader) => loader - .load_function_without_ty_args( - self.module_storage, - module_id.address(), - module_id.name(), - function_name, - ) + Loader::V2(_) => self + .module_storage + .fetch_function_definition(module_id.address(), module_id.name(), function_name) .map_err(|_| { // Note: legacy loader implementation used this error, so we need to remap. PartialVMError::new(StatusCode::FUNCTION_RESOLUTION_FAILURE).with_message( @@ -1652,6 +1650,20 @@ impl<'a> Resolver<'a> { } } +impl<'a> FunctionValueExtension for Resolver<'a> { + fn get_function_arg_tys( + &self, + module_id: &ModuleId, + function_name: &IdentStr, + ty_arg_tags: Vec, + ) -> PartialVMResult> { + let function_value_extension = FunctionValueExtensionAdapter { + module_storage: self.module_storage, + }; + function_value_extension.get_function_arg_tys(module_id, function_name, ty_arg_tags) + } +} + /// Maximal depth of a value in terms of type depth. pub const VALUE_DEPTH_MAX: u64 = 128; diff --git a/third_party/move/move-vm/runtime/src/native_functions.rs b/third_party/move/move-vm/runtime/src/native_functions.rs index 8cce249a6ebdd..214115c30b4e5 100644 --- a/third_party/move/move-vm/runtime/src/native_functions.rs +++ b/third_party/move/move-vm/runtime/src/native_functions.rs @@ -21,7 +21,8 @@ use move_core_types::{ vm_status::StatusCode, }; use move_vm_types::{ - loaded_data::runtime_types::Type, natives::function::NativeResult, values::Value, + loaded_data::runtime_types::Type, natives::function::NativeResult, + value_serde::FunctionValueExtension, values::Value, }; use std::{ collections::{HashMap, VecDeque}, @@ -197,6 +198,10 @@ impl<'a, 'b, 'c> NativeContext<'a, 'b, 'c> { self.traversal_context } + pub fn function_value_extension(&self) -> &dyn FunctionValueExtension { + self.resolver + } + pub fn load_function( &mut self, module_id: &ModuleId, @@ -220,13 +225,10 @@ impl<'a, 'b, 'c> NativeContext<'a, 'b, 'c> { .module_store() .resolve_module_and_function_by_name(module_id, function_name)? }, - Loader::V2(loader) => loader - .load_function_without_ty_args( - self.resolver.module_storage(), - module_id.address(), - module_id.name(), - function_name, - ) + Loader::V2(_) => self + .resolver + .module_storage() + .fetch_function_definition(module_id.address(), module_id.name(), function_name) // TODO(loader_v2): // Keeping this consistent with loader V1 implementation which returned that // error. Check if we can avoid remapping by replaying transactions. diff --git a/third_party/move/move-vm/runtime/src/runtime.rs b/third_party/move/move-vm/runtime/src/runtime.rs index 0ad885bca6c11..d33a49edfd645 100644 --- a/third_party/move/move-vm/runtime/src/runtime.rs +++ b/third_party/move/move-vm/runtime/src/runtime.rs @@ -12,7 +12,7 @@ use crate::{ native_extensions::NativeContextExtensions, session::SerializedReturnValues, storage::{code_storage::CodeStorage, module_storage::ModuleStorage}, - RuntimeEnvironment, + AsFunctionValueExtension, RuntimeEnvironment, }; use move_binary_format::{ access::ModuleAccess, @@ -29,6 +29,7 @@ use move_vm_metrics::{Timer, VM_TIMER}; use move_vm_types::{ gas::GasMeter, loaded_data::runtime_types::Type, + value_serde::ValueSerDeContext, values::{Locals, Reference, VMValueCast, Value}, }; use std::{borrow::Borrow, collections::BTreeSet, sync::Arc}; @@ -270,7 +271,11 @@ impl VMRuntime { return Err(deserialization_error()); } - match Value::simple_deserialize(arg.borrow(), &layout) { + let function_value_extension = module_storage.as_function_value_extension(); + match ValueSerDeContext::new() + .with_func_args_deserialization(&function_value_extension) + .deserialize(arg.borrow(), &layout) + { Some(val) => Ok(val), None => Err(deserialization_error()), } @@ -354,8 +359,10 @@ impl VMRuntime { return Err(serialization_error()); } - let bytes = value - .simple_serialize(&layout) + let function_value_extension = module_storage.as_function_value_extension(); + let bytes = ValueSerDeContext::new() + .with_func_args_deserialization(&function_value_extension) + .serialize(&value, &layout)? .ok_or_else(serialization_error)?; Ok((bytes, layout)) } diff --git a/third_party/move/move-vm/runtime/src/storage/code_storage.rs b/third_party/move/move-vm/runtime/src/storage/code_storage.rs index 5274a52543186..8afcef7777d32 100644 --- a/third_party/move/move-vm/runtime/src/storage/code_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/code_storage.rs @@ -1,12 +1,12 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use crate::{loader::Script, logging::expect_no_verification_errors, ModuleStorage}; +use crate::{loader::Script, ModuleStorage}; use ambassador::delegatable_trait; use move_binary_format::{errors::VMResult, file_format::CompiledScript}; use move_vm_types::{ code::{Code, ScriptCache}, - module_linker_error, sha3_256, + sha3_256, }; use std::sync::Arc; @@ -72,9 +72,7 @@ where .immediate_dependencies_iter() .map(|(addr, name)| { // Since module is stored on-chain, we should not see any verification errors here. - self.fetch_verified_module(addr, name) - .map_err(expect_no_verification_errors)? - .ok_or_else(|| module_linker_error!(addr, name)) + self.fetch_existing_verified_module(addr, name) }) .collect::>>()?; let verified_script = self diff --git a/third_party/move/move-vm/runtime/src/storage/environment.rs b/third_party/move/move-vm/runtime/src/storage/environment.rs index 7d5ae85f0640e..9a813576dbdd4 100644 --- a/third_party/move/move-vm/runtime/src/storage/environment.rs +++ b/third_party/move/move-vm/runtime/src/storage/environment.rs @@ -295,6 +295,15 @@ impl RuntimeEnvironment { ) -> PartialVMResult { self.struct_name_index_map.struct_name_to_idx(&struct_name) } + + /// Test-only function to be able to check cached struct names. + #[cfg(any(test, feature = "testing"))] + pub fn idx_to_struct_name_for_test( + &self, + idx: StructNameIndex, + ) -> PartialVMResult { + self.struct_name_index_map.idx_to_struct_name(idx) + } } impl Clone for RuntimeEnvironment { diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs index 19f6f1ae55fee..b559a6c6ab12f 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - loader::{Module, Script}, + loader::{Function, Module, Script}, storage::{ code_storage::{ambassador_impl_CodeStorage, CodeStorage}, environment::{ @@ -14,10 +14,18 @@ use crate::{ }; use ambassador::Delegate; use bytes::Bytes; -use move_binary_format::{errors::VMResult, file_format::CompiledScript, CompiledModule}; -use move_core_types::{account_address::AccountAddress, identifier::IdentStr, metadata::Metadata}; -use move_vm_types::code::{ - ambassador_impl_ScriptCache, Code, ModuleBytesStorage, ScriptCache, UnsyncScriptCache, +use move_binary_format::{ + errors::{PartialVMResult, VMResult}, + file_format::CompiledScript, + CompiledModule, +}; +use move_core_types::{ + account_address::AccountAddress, identifier::IdentStr, language_storage::TypeTag, + metadata::Metadata, +}; +use move_vm_types::{ + code::{ambassador_impl_ScriptCache, Code, ModuleBytesStorage, ScriptCache, UnsyncScriptCache}, + loaded_data::runtime_types::{StructType, Type}, }; use std::sync::Arc; diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs index fba7581d9d341..b5543e0bb4527 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - loader::Module, + loader::{Function, Module}, storage::{ environment::{ ambassador_impl_WithRuntimeEnvironment, RuntimeEnvironment, WithRuntimeEnvironment, @@ -12,9 +12,14 @@ use crate::{ }; use ambassador::Delegate; use bytes::Bytes; -use move_binary_format::{errors::VMResult, CompiledModule}; +use move_binary_format::{ + errors::{PartialVMResult, VMResult}, + CompiledModule, +}; use move_core_types::{ - account_address::AccountAddress, identifier::IdentStr, language_storage::ModuleId, + account_address::AccountAddress, + identifier::IdentStr, + language_storage::{ModuleId, TypeTag}, metadata::Metadata, }; use move_vm_types::{ @@ -22,6 +27,7 @@ use move_vm_types::{ ambassador_impl_ModuleCache, ModuleBytesStorage, ModuleCache, ModuleCode, ModuleCodeBuilder, UnsyncModuleCache, WithBytes, WithHash, }, + loaded_data::runtime_types::{StructType, Type}, sha3_256, }; use std::{borrow::Borrow, ops::Deref, sync::Arc}; diff --git a/third_party/move/move-vm/runtime/src/storage/loader.rs b/third_party/move/move-vm/runtime/src/storage/loader.rs index 539a439c0c4f5..3e4d3cfbcd9ca 100644 --- a/third_party/move/move-vm/runtime/src/storage/loader.rs +++ b/third_party/move/move-vm/runtime/src/storage/loader.rs @@ -2,16 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - config::VMConfig, - loader::{Function, LoadedFunctionOwner, Module}, - logging::expect_no_verification_errors, - module_traversal::TraversalContext, - storage::module_storage::ModuleStorage, - CodeStorage, LoadedFunction, + config::VMConfig, loader::LoadedFunctionOwner, module_traversal::TraversalContext, + storage::module_storage::ModuleStorage, CodeStorage, LoadedFunction, }; use move_binary_format::{ access::{ModuleAccess, ScriptAccess}, - errors::{Location, PartialVMError, PartialVMResult, VMResult}, + errors::{Location, PartialVMResult, VMResult}, CompiledModule, }; use move_core_types::{ @@ -19,11 +15,10 @@ use move_core_types::{ gas_algebra::NumBytes, identifier::IdentStr, language_storage::{ModuleId, TypeTag}, - vm_status::StatusCode, }; use move_vm_types::{ gas::GasMeter, - loaded_data::runtime_types::{StructType, Type, TypeBuilder}, + loaded_data::runtime_types::{Type, TypeBuilder}, module_linker_error, }; use std::{collections::BTreeMap, sync::Arc}; @@ -137,7 +132,7 @@ impl LoaderV2 { // arguments for scripts are verified on the client side. let ty_args = ty_tag_args .iter() - .map(|ty_tag| self.load_ty(code_storage, ty_tag)) + .map(|ty_tag| code_storage.fetch_ty(ty_tag)) .collect::>>() .map_err(|err| err.finish(Location::Script))?; @@ -151,95 +146,6 @@ impl LoaderV2 { function: main, }) } - - /// Returns a loaded & verified module corresponding to the specified name. - pub(crate) fn load_module( - &self, - module_storage: &dyn ModuleStorage, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - module_storage - .fetch_verified_module(address, module_name) - .map_err(expect_no_verification_errors)? - .ok_or_else(|| module_linker_error!(address, module_name)) - } - - /// Returns the function definition corresponding to the specified name, as well as the module - /// where this function is defined. - pub(crate) fn load_function_without_ty_args( - &self, - module_storage: &dyn ModuleStorage, - address: &AccountAddress, - module_name: &IdentStr, - function_name: &IdentStr, - ) -> VMResult<(Arc, Arc)> { - let module = self.load_module(module_storage, address, module_name)?; - let function = module - .function_map - .get(function_name) - .and_then(|idx| module.function_defs.get(*idx)) - .ok_or_else(|| { - PartialVMError::new(StatusCode::FUNCTION_RESOLUTION_FAILURE) - .with_message(format!( - "Function {}::{}::{} does not exist", - address, module_name, function_name - )) - .finish(Location::Undefined) - })? - .clone(); - Ok((module, function)) - } - - /// Returns a struct type corresponding to the specified name. The module - /// containing the struct is loaded. - pub(crate) fn load_struct_ty( - &self, - module_storage: &dyn ModuleStorage, - address: &AccountAddress, - module_name: &IdentStr, - struct_name: &IdentStr, - ) -> PartialVMResult> { - let module = self - .load_module(module_storage, address, module_name) - .map_err(|err| err.to_partial())?; - Ok(module - .struct_map - .get(struct_name) - .and_then(|idx| module.structs.get(*idx)) - .ok_or_else(|| { - PartialVMError::new(StatusCode::TYPE_RESOLUTION_FAILURE).with_message(format!( - "Struct {}::{}::{} does not exist", - address, module_name, struct_name - )) - })? - .definition_struct_type - .clone()) - } - - /// Returns a runtime type corresponding to the specified type tag (file format type - /// representation). In case struct types are transitively loaded, the module containing - /// the struct definition is also loaded. - pub(crate) fn load_ty( - &self, - module_storage: &impl ModuleStorage, - ty_tag: &TypeTag, - ) -> PartialVMResult { - // TODO(loader_v2): Loader V1 uses VMResults everywhere, but partial VM errors - // seem better fit. Here we map error to VMError to reuse existing - // type builder implementation, and then strip the location info. - self.ty_builder() - .create_ty(ty_tag, |st| { - self.load_struct_ty( - module_storage, - &st.address, - st.module.as_ident_str(), - st.name.as_ident_str(), - ) - .map_err(|err| err.finish(Location::Undefined)) - }) - .map_err(|err| err.to_partial()) - } } impl Clone for LoaderV2 { diff --git a/third_party/move/move-vm/runtime/src/storage/module_storage.rs b/third_party/move/move-vm/runtime/src/storage/module_storage.rs index da64fc6dbd001..c96571b046cd1 100644 --- a/third_party/move/move-vm/runtime/src/storage/module_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/module_storage.rs @@ -1,19 +1,31 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use crate::{loader::Module, WithRuntimeEnvironment}; +use crate::{ + loader::{Function, Module}, + logging::expect_no_verification_errors, + WithRuntimeEnvironment, +}; use ambassador::delegatable_trait; use bytes::Bytes; use hashbrown::HashSet; -use move_binary_format::{errors::VMResult, CompiledModule}; +use move_binary_format::{ + errors::{Location, PartialVMError, PartialVMResult, VMResult}, + CompiledModule, +}; use move_core_types::{ - account_address::AccountAddress, identifier::IdentStr, language_storage::ModuleId, + account_address::AccountAddress, + identifier::IdentStr, + language_storage::{ModuleId, TypeTag}, metadata::Metadata, + vm_status::StatusCode, }; use move_vm_metrics::{Timer, VM_TIMER}; use move_vm_types::{ code::{ModuleCache, ModuleCode, ModuleCodeBuilder, WithBytes, WithHash, WithSize}, + loaded_data::runtime_types::{StructType, Type}, module_cyclic_dependency_error, module_linker_error, + value_serde::FunctionValueExtension, }; use std::sync::Arc; @@ -101,6 +113,91 @@ pub trait ModuleStorage: WithRuntimeEnvironment { address: &AccountAddress, module_name: &IdentStr, ) -> VMResult>>; + + /// Returns the verified module. If it does not exist, a linker error is returned. All other + /// errors are mapped using [expect_no_verification_errors] - since on-chain code should not + /// fail bytecode verification. + fn fetch_existing_verified_module( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> VMResult> { + self.fetch_verified_module(address, module_name) + .map_err(expect_no_verification_errors)? + .ok_or_else(|| module_linker_error!(address, module_name)) + } + + /// Returns a struct type corresponding to the specified name. The module containing the struct + /// will be fetched and cached beforehand. + fn fetch_struct_ty( + &self, + address: &AccountAddress, + module_name: &IdentStr, + struct_name: &IdentStr, + ) -> PartialVMResult> { + let module = self + .fetch_existing_verified_module(address, module_name) + .map_err(|err| err.to_partial())?; + Ok(module + .struct_map + .get(struct_name) + .and_then(|idx| module.structs.get(*idx)) + .ok_or_else(|| { + PartialVMError::new(StatusCode::TYPE_RESOLUTION_FAILURE).with_message(format!( + "Struct {}::{}::{} does not exist", + address, module_name, struct_name + )) + })? + .definition_struct_type + .clone()) + } + + /// Returns a runtime type corresponding to the specified type tag (file format type + /// representation). If a struct type is constructed, the module containing the struct + /// definition is fetched and cached. + fn fetch_ty(&self, ty_tag: &TypeTag) -> PartialVMResult { + // TODO(loader_v2): Loader V1 uses VMResults everywhere, but partial VM errors + // seem better fit. Here we map error to VMError to reuse existing + // type builder implementation, and then strip the location info. + self.runtime_environment() + .vm_config() + .ty_builder + .create_ty(ty_tag, |st| { + self.fetch_struct_ty( + &st.address, + st.module.as_ident_str(), + st.name.as_ident_str(), + ) + .map_err(|err| err.finish(Location::Undefined)) + }) + .map_err(|err| err.to_partial()) + } + + /// Returns the function definition corresponding to the specified name, as well as the module + /// where this function is defined. The returned function can contain uninstantiated generic + /// types and its signature. The returned module is verified. + fn fetch_function_definition( + &self, + address: &AccountAddress, + module_name: &IdentStr, + function_name: &IdentStr, + ) -> VMResult<(Arc, Arc)> { + let module = self.fetch_existing_verified_module(address, module_name)?; + let function = module + .function_map + .get(function_name) + .and_then(|idx| module.function_defs.get(*idx)) + .ok_or_else(|| { + PartialVMError::new(StatusCode::FUNCTION_RESOLUTION_FAILURE) + .with_message(format!( + "Function {}::{}::{} does not exist", + address, module_name, function_name + )) + .finish(Location::Undefined) + })? + .clone(); + Ok((module, function)) + } } impl ModuleStorage for T @@ -302,3 +399,53 @@ where )?; Ok(module.code().verified().clone()) } + +/// Avoids the orphan rule to implement external [FunctionValueExtension] for any generic type that +/// implements [ModuleStorage]. +pub struct FunctionValueExtensionAdapter<'a> { + pub(crate) module_storage: &'a dyn ModuleStorage, +} + +pub trait AsFunctionValueExtension { + fn as_function_value_extension(&self) -> FunctionValueExtensionAdapter; +} + +impl AsFunctionValueExtension for T { + fn as_function_value_extension(&self) -> FunctionValueExtensionAdapter { + FunctionValueExtensionAdapter { + module_storage: self, + } + } +} + +impl<'a> FunctionValueExtension for FunctionValueExtensionAdapter<'a> { + fn get_function_arg_tys( + &self, + module_id: &ModuleId, + function_name: &IdentStr, + substitution_ty_arg_tags: Vec, + ) -> PartialVMResult> { + let substitution_ty_args = substitution_ty_arg_tags + .into_iter() + .map(|tag| self.module_storage.fetch_ty(&tag)) + .collect::>>()?; + + let (_, function) = self + .module_storage + .fetch_function_definition(module_id.address(), module_id.name(), function_name) + .map_err(|err| err.to_partial())?; + + let ty_builder = &self + .module_storage + .runtime_environment() + .vm_config() + .ty_builder; + function + .param_tys() + .iter() + .map(|ty_to_substitute| { + ty_builder.create_ty_with_subst(ty_to_substitute, &substitution_ty_args) + }) + .collect::>>() + } +} diff --git a/third_party/move/move-vm/runtime/src/storage/publishing.rs b/third_party/move/move-vm/runtime/src/storage/publishing.rs index d6d3320e9c54a..b59001ba45897 100644 --- a/third_party/move/move-vm/runtime/src/storage/publishing.rs +++ b/third_party/move/move-vm/runtime/src/storage/publishing.rs @@ -2,25 +2,30 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - ambassador_impl_ModuleStorage, ambassador_impl_WithRuntimeEnvironment, AsUnsyncModuleStorage, - Module, ModuleStorage, RuntimeEnvironment, UnsyncModuleStorage, WithRuntimeEnvironment, + ambassador_impl_ModuleStorage, ambassador_impl_WithRuntimeEnvironment, loader::Function, + AsUnsyncModuleStorage, Module, ModuleStorage, RuntimeEnvironment, UnsyncModuleStorage, + WithRuntimeEnvironment, }; use ambassador::Delegate; use bytes::Bytes; use move_binary_format::{ access::ModuleAccess, compatibility::Compatibility, - errors::{verification_error, Location, PartialVMError, VMResult}, + errors::{verification_error, Location, PartialVMError, PartialVMResult, VMResult}, normalized, CompiledModule, IndexKind, }; use move_core_types::{ account_address::AccountAddress, identifier::{IdentStr, Identifier}, - language_storage::ModuleId, + language_storage::{ModuleId, TypeTag}, metadata::Metadata, vm_status::StatusCode, }; -use move_vm_types::{code::ModuleBytesStorage, module_linker_error}; +use move_vm_types::{ + code::ModuleBytesStorage, + loaded_data::runtime_types::{StructType, Type}, + module_linker_error, +}; use std::{ collections::{btree_map, BTreeMap}, sync::Arc, diff --git a/third_party/move/move-vm/types/src/value_serde.rs b/third_party/move/move-vm/types/src/value_serde.rs index 5f17bfe9b3848..c8f390096f9ac 100644 --- a/third_party/move/move-vm/types/src/value_serde.rs +++ b/third_party/move/move-vm/types/src/value_serde.rs @@ -2,182 +2,183 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - delayed_values::delayed_field_id::{ - DelayedFieldID, ExtractUniqueIndex, ExtractWidth, TryFromMoveValue, TryIntoMoveValue, - }, + delayed_values::delayed_field_id::DelayedFieldID, + loaded_data::runtime_types::Type, values::{DeserializationSeed, SerializationReadyValue, Value}, }; use move_binary_format::errors::{PartialVMError, PartialVMResult}; use move_core_types::{ + identifier::IdentStr, + language_storage::{ModuleId, TypeTag}, value::{IdentifierMappingKind, MoveTypeLayout}, vm_status::StatusCode, }; -use serde::{ - de::{DeserializeSeed, Error as DeError}, - ser::Error as SerError, - Deserializer, Serialize, Serializer, -}; use std::cell::RefCell; -pub trait CustomDeserializer { - fn custom_deserialize<'d, D: Deserializer<'d>>( - &self, - deserializer: D, - kind: &IdentifierMappingKind, - layout: &MoveTypeLayout, - ) -> Result; -} - -pub trait CustomSerializer { - fn custom_serialize( +/// An extension to (de)serializer to lookup information about function values. +pub trait FunctionValueExtension { + /// Given the module's id and the function name, returns the parameter types of the + /// corresponding function, instantiated with the provided set of type tags. + fn get_function_arg_tys( &self, - serializer: S, - kind: &IdentifierMappingKind, - layout: &MoveTypeLayout, - id: DelayedFieldID, - ) -> Result; + module_id: &ModuleId, + function_name: &IdentStr, + ty_arg_tags: Vec, + ) -> PartialVMResult>; +} + +/// An extension to (de)serializer to lookup information about delayed fields. +pub(crate) struct DelayedFieldsExtension<'a> { + /// Number of delayed fields (de)serialized, capped. + pub(crate) delayed_fields_count: RefCell, + /// Optional mapping to ids/values. The mapping is used to replace ids with values at + /// serialization time and values with ids at deserialization time. If [None], ids and values + /// are serialized as is. + pub(crate) mapping: Option<&'a dyn ValueToIdentifierMapping>, +} + +impl<'a> DelayedFieldsExtension<'a> { + // Temporarily limit the number of delayed fields per resource, until proper charges are + // implemented. + // TODO[agg_v2](clean): + // Propagate up, so this value is controlled by the gas schedule version. + const MAX_DELAYED_FIELDS_PER_RESOURCE: usize = 10; + + /// Increments the delayed fields count, and checks if there are too many of them. If so, an + /// error is returned. + pub(crate) fn inc_and_check_delayed_fields_count(&self) -> PartialVMResult<()> { + *self.delayed_fields_count.borrow_mut() += 1; + if *self.delayed_fields_count.borrow() > Self::MAX_DELAYED_FIELDS_PER_RESOURCE { + return Err(PartialVMError::new(StatusCode::TOO_MANY_DELAYED_FIELDS) + .with_message("Too many Delayed fields in a single resource.".to_string())); + } + Ok(()) + } } -/// Custom (de)serializer which allows delayed values to be (de)serialized as -/// is. This means that when a delayed value is serialized, the deserialization -/// must construct the delayed value back. -pub struct RelaxedCustomSerDe { - delayed_fields_count: RefCell, +/// A (de)serializer context for a single Move [Value], containing optional extensions. If +/// extension is not provided, but required at (de)serialization time, (de)serialization fails. +pub struct ValueSerDeContext<'a> { + #[allow(dead_code)] + pub(crate) function_extension: Option<&'a dyn FunctionValueExtension>, + pub(crate) delayed_fields_extension: Option>, } -impl RelaxedCustomSerDe { +impl<'a> ValueSerDeContext<'a> { + /// Default (de)serializer that disallows delayed fields. #[allow(clippy::new_without_default)] pub fn new() -> Self { Self { - delayed_fields_count: RefCell::new(0), + function_extension: None, + delayed_fields_extension: None, } } -} -// TODO[agg_v2](clean): propagate up, so this value is controlled by the gas schedule version. -// Temporarily limit the number of delayed fields per resource, -// until proper charges are implemented. -pub const MAX_DELAYED_FIELDS_PER_RESOURCE: usize = 10; - -impl CustomDeserializer for RelaxedCustomSerDe { - fn custom_deserialize<'d, D: Deserializer<'d>>( - &self, - deserializer: D, - kind: &IdentifierMappingKind, - layout: &MoveTypeLayout, - ) -> Result { - *self.delayed_fields_count.borrow_mut() += 1; + /// Custom (de)serializer such that supports lookup of the argument types of a function during + /// function value deserialization. + pub fn with_func_args_deserialization( + mut self, + function_extension: &'a dyn FunctionValueExtension, + ) -> Self { + self.function_extension = Some(function_extension); + self + } - let value = DeserializationSeed { - custom_deserializer: None::<&RelaxedCustomSerDe>, - layout, + /// Returns the same extension but without allowing the delayed fields. + pub(crate) fn clone_without_delayed_fields(&self) -> Self { + Self { + function_extension: self.function_extension, + delayed_fields_extension: None, } - .deserialize(deserializer)?; - let (id, _width) = - DelayedFieldID::try_from_move_value(layout, value, &()).map_err(|_| { - D::Error::custom(format!( - "Custom deserialization failed for {:?} with layout {}", - kind, layout - )) - })?; - Ok(Value::delayed_value(id)) } -} -impl CustomSerializer for RelaxedCustomSerDe { - fn custom_serialize( - &self, - serializer: S, - kind: &IdentifierMappingKind, - layout: &MoveTypeLayout, - id: DelayedFieldID, - ) -> Result { - *self.delayed_fields_count.borrow_mut() += 1; + /// Custom (de)serializer such that: + /// 1. when serializing, the delayed value is replaced with a concrete value instance and + /// serialized instead; + /// 2. when deserializing, the concrete value instance is replaced with a delayed id. + pub fn with_delayed_fields_replacement( + mut self, + mapping: &'a dyn ValueToIdentifierMapping, + ) -> Self { + self.delayed_fields_extension = Some(DelayedFieldsExtension { + delayed_fields_count: RefCell::new(0), + mapping: Some(mapping), + }); + self + } - let value = id.try_into_move_value(layout).map_err(|_| { - S::Error::custom(format!( - "Custom serialization failed for {:?} with layout {}", - kind, layout - )) - })?; - SerializationReadyValue { - custom_serializer: None::<&RelaxedCustomSerDe>, + /// Custom (de)serializer that allows delayed values to be (de)serialized as is. This means + /// that when a delayed value is serialized, the deserialization must construct the delayed + /// value back. + pub fn with_delayed_fields_serde(mut self) -> Self { + self.delayed_fields_extension = Some(DelayedFieldsExtension { + delayed_fields_count: RefCell::new(0), + mapping: None, + }); + self + } + + /// Serializes a [Value] based on the provided layout. For legacy reasons, all serialization + /// errors are mapped to [None]. If [DelayedFieldsExtension] is set, and there are too many + /// delayed fields, an error may be returned. + pub fn serialize( + self, + value: &Value, + layout: &MoveTypeLayout, + ) -> PartialVMResult>> { + let value = SerializationReadyValue { + ctx: &self, layout, value: &value.0, + }; + + match bcs::to_bytes(&value).ok() { + Some(bytes) => Ok(Some(bytes)), + None => { + // Check if the error is due to too many delayed fields. If so, to be compatible + // with the older implementation return an error. + if let Some(delayed_fields_extension) = self.delayed_fields_extension { + if delayed_fields_extension.delayed_fields_count.into_inner() + > DelayedFieldsExtension::MAX_DELAYED_FIELDS_PER_RESOURCE + { + return Err(PartialVMError::new(StatusCode::TOO_MANY_DELAYED_FIELDS) + .with_message( + "Too many Delayed fields in a single resource.".to_string(), + )); + } + } + Ok(None) + }, } - .serialize(serializer) } -} -pub fn deserialize_and_allow_delayed_values( - bytes: &[u8], - layout: &MoveTypeLayout, -) -> Option { - let native_deserializer = RelaxedCustomSerDe::new(); - let seed = DeserializationSeed { - custom_deserializer: Some(&native_deserializer), - layout, - }; - bcs::from_bytes_seed(seed, bytes).ok().filter(|_| { - // Should never happen, it should always fail first in serialize_and_allow_delayed_values - // so we can treat it as regular deserialization error. - native_deserializer.delayed_fields_count.into_inner() <= MAX_DELAYED_FIELDS_PER_RESOURCE - }) -} - -pub fn serialize_and_allow_delayed_values( - value: &Value, - layout: &MoveTypeLayout, -) -> PartialVMResult>> { - let native_serializer = RelaxedCustomSerDe::new(); - let value = SerializationReadyValue { - custom_serializer: Some(&native_serializer), - layout, - value: &value.0, - }; - bcs::to_bytes(&value) - .ok() - .map(|v| { - if native_serializer.delayed_fields_count.into_inner() - <= MAX_DELAYED_FIELDS_PER_RESOURCE - { - Ok(v) - } else { - Err(PartialVMError::new(StatusCode::TOO_MANY_DELAYED_FIELDS) - .with_message("Too many Delayed fields in a single resource.".to_string())) - } + /// Returns the serialized size of a [Value] with the associated layout. All errors are mapped + /// to [StatusCode::VALUE_SERIALIZATION_ERROR]. + pub fn serialized_size(self, value: &Value, layout: &MoveTypeLayout) -> PartialVMResult { + let value = SerializationReadyValue { + ctx: &self, + layout, + value: &value.0, + }; + bcs::serialized_size(&value).map_err(|e| { + PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!( + "failed to compute serialized size of a value: {:?}", + e + )) }) - .transpose() -} + } -/// Returns the serialized size in bytes of a Move value, with compatible layout. -/// Note that the layout should match, as otherwise serialization fails. This -/// method explicitly allows having delayed values inside the passed in Move value -/// because their size is fixed and cannot change. -pub fn serialized_size_allowing_delayed_values( - value: &Value, - layout: &MoveTypeLayout, -) -> PartialVMResult { - let native_serializer = RelaxedCustomSerDe::new(); - let value = SerializationReadyValue { - custom_serializer: Some(&native_serializer), - layout, - value: &value.0, - }; - bcs::serialized_size(&value).map_err(|e| { - PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!( - "failed to compute serialized size of a value: {:?}", - e - )) - }) + /// Deserializes the bytes using the provided layout into a Move [Value]. + pub fn deserialize(self, bytes: &[u8], layout: &MoveTypeLayout) -> Option { + let seed = DeserializationSeed { ctx: &self, layout }; + bcs::from_bytes_seed(seed, bytes).ok() + } } /// Allow conversion between values and identifiers (delayed values). For example, /// this trait can be implemented to fetch a concrete Move value from the global /// state based on the identifier stored inside a delayed value. pub trait ValueToIdentifierMapping { - type Identifier; - fn value_to_identifier( &self, // We need kind to distinguish between aggregators and snapshots @@ -185,116 +186,11 @@ pub trait ValueToIdentifierMapping { kind: &IdentifierMappingKind, layout: &MoveTypeLayout, value: Value, - ) -> PartialVMResult; + ) -> PartialVMResult; fn identifier_to_value( &self, layout: &MoveTypeLayout, - identifier: Self::Identifier, + identifier: DelayedFieldID, ) -> PartialVMResult; } - -/// Custom (de)serializer such that: -/// 1. when encountering a delayed value, ir uses its id to replace it with a concrete -/// value instance and serialize it instead; -/// 2. when deserializing, the concrete value instance is replaced with a delayed value. -pub struct CustomSerDeWithExchange<'a, I: From + ExtractWidth + ExtractUniqueIndex> { - mapping: &'a dyn ValueToIdentifierMapping, - delayed_fields_count: RefCell, -} - -impl<'a, I: From + ExtractWidth + ExtractUniqueIndex> CustomSerDeWithExchange<'a, I> { - pub fn new(mapping: &'a dyn ValueToIdentifierMapping) -> Self { - Self { - mapping, - delayed_fields_count: RefCell::new(0), - } - } -} - -impl<'a, I: From + ExtractWidth + ExtractUniqueIndex> CustomSerializer - for CustomSerDeWithExchange<'a, I> -{ - fn custom_serialize( - &self, - serializer: S, - _kind: &IdentifierMappingKind, - layout: &MoveTypeLayout, - sized_id: DelayedFieldID, - ) -> Result { - *self.delayed_fields_count.borrow_mut() += 1; - - let value = self - .mapping - .identifier_to_value(layout, sized_id.as_u64().into()) - .map_err(|e| S::Error::custom(format!("{}", e)))?; - SerializationReadyValue { - custom_serializer: None::<&RelaxedCustomSerDe>, - layout, - value: &value.0, - } - .serialize(serializer) - } -} - -impl<'a, I: From + ExtractWidth + ExtractUniqueIndex> CustomDeserializer - for CustomSerDeWithExchange<'a, I> -{ - fn custom_deserialize<'d, D: Deserializer<'d>>( - &self, - deserializer: D, - kind: &IdentifierMappingKind, - layout: &MoveTypeLayout, - ) -> Result { - *self.delayed_fields_count.borrow_mut() += 1; - - let value = DeserializationSeed { - custom_deserializer: None::<&RelaxedCustomSerDe>, - layout, - } - .deserialize(deserializer)?; - let id = self - .mapping - .value_to_identifier(kind, layout, value) - .map_err(|e| D::Error::custom(format!("{}", e)))?; - Ok(Value::delayed_value(DelayedFieldID::new_with_width( - id.extract_unique_index(), - id.extract_width(), - ))) - } -} - -pub fn deserialize_and_replace_values_with_ids + ExtractWidth + ExtractUniqueIndex>( - bytes: &[u8], - layout: &MoveTypeLayout, - mapping: &impl ValueToIdentifierMapping, -) -> Option { - let custom_deserializer = CustomSerDeWithExchange::new(mapping); - let seed = DeserializationSeed { - custom_deserializer: Some(&custom_deserializer), - layout, - }; - bcs::from_bytes_seed(seed, bytes).ok().filter(|_| { - // Should never happen, it should always fail first in serialize_and_allow_delayed_values - // so we can treat it as regular deserialization error. - custom_deserializer.delayed_fields_count.into_inner() <= MAX_DELAYED_FIELDS_PER_RESOURCE - }) -} - -pub fn serialize_and_replace_ids_with_values + ExtractWidth + ExtractUniqueIndex>( - value: &Value, - layout: &MoveTypeLayout, - mapping: &impl ValueToIdentifierMapping, -) -> Option> { - let custom_serializer = CustomSerDeWithExchange::new(mapping); - let value = SerializationReadyValue { - custom_serializer: Some(&custom_serializer), - layout, - value: &value.0, - }; - bcs::to_bytes(&value).ok().filter(|_| { - // Should never happen, it should always fail first in serialize_and_allow_delayed_values - // so we can treat it as regular deserialization error. - custom_serializer.delayed_fields_count.into_inner() <= MAX_DELAYED_FIELDS_PER_RESOURCE - }) -} diff --git a/third_party/move/move-vm/types/src/values/serialization_tests.rs b/third_party/move/move-vm/types/src/values/serialization_tests.rs index 6085a50183e2c..e1c48c082fcbe 100644 --- a/third_party/move/move-vm/types/src/values/serialization_tests.rs +++ b/third_party/move/move-vm/types/src/values/serialization_tests.rs @@ -5,7 +5,7 @@ use crate::{ delayed_values::delayed_field_id::DelayedFieldID, - value_serde::{serialize_and_allow_delayed_values, serialized_size_allowing_delayed_values}, + value_serde::ValueSerDeContext, values::{values_impl, Struct, Value}, }; use claims::{assert_err, assert_ok, assert_some}; @@ -85,10 +85,13 @@ fn enum_round_trip_vm_value() { )), ]; for value in good_values { - let blob = value - .simple_serialize(&layout) + let blob = ValueSerDeContext::new() + .serialize(&value, &layout) + .unwrap() .expect("serialization succeeds"); - let de_value = Value::simple_deserialize(&blob, &layout).expect("deserialization succeeds"); + let de_value = ValueSerDeContext::new() + .deserialize(&blob, &layout) + .expect("deserialization succeeds"); assert!( value.equals(&de_value).unwrap(), "roundtrip serialization succeeds" @@ -96,12 +99,18 @@ fn enum_round_trip_vm_value() { } let bad_tag_value = Value::struct_(Struct::pack_variant(3, [Value::u64(42)])); assert!( - bad_tag_value.simple_serialize(&layout).is_none(), + ValueSerDeContext::new() + .serialize(&bad_tag_value, &layout) + .unwrap() + .is_none(), "serialization fails" ); let bad_struct_value = Value::struct_(Struct::pack([Value::u64(42)])); assert!( - bad_struct_value.simple_serialize(&layout).is_none(), + ValueSerDeContext::new() + .serialize(&bad_struct_value, &layout) + .unwrap() + .is_none(), "serialization fails" ); } @@ -160,14 +169,17 @@ fn enum_rust_round_trip_vm_value() { RustEnum::BoolNumber(true, 13), ]; for (move_value, rust_value) in move_values.into_iter().zip(rust_values) { - let from_move = move_value - .simple_serialize(&layout) + let from_move = ValueSerDeContext::new() + .serialize(&move_value, &layout) + .unwrap() .expect("from move succeeds"); let to_rust = bcs::from_bytes::(&from_move).expect("to rust successful"); assert_eq!(to_rust, rust_value); let from_rust = bcs::to_bytes(&rust_value).expect("from rust succeeds"); - let to_move = Value::simple_deserialize(&from_rust, &layout).expect("to move succeeds"); + let to_move = ValueSerDeContext::new() + .deserialize(&from_rust, &layout) + .expect("to move succeeds"); assert!( to_move.equals(&move_value).unwrap(), "from rust to move failed" @@ -224,10 +236,13 @@ fn test_serialized_size() { ), ]; for (value, layout) in good_values_layouts_sizes { - let bytes = assert_some!(assert_ok!(serialize_and_allow_delayed_values( - &value, &layout - ))); - let size = assert_ok!(serialized_size_allowing_delayed_values(&value, &layout)); + let bytes = assert_some!(assert_ok!(ValueSerDeContext::new() + .with_delayed_fields_serde() + .serialize(&value, &layout))); + + let size = assert_ok!(ValueSerDeContext::new() + .with_delayed_fields_serde() + .serialized_size(&value, &layout)); assert_eq!(size, bytes.len()); } @@ -241,6 +256,8 @@ fn test_serialized_size() { (Value::u64(12), Native(Aggregator, Box::new(U64))), ]; for (value, layout) in bad_values_layouts_sizes { - assert_err!(serialized_size_allowing_delayed_values(&value, &layout)); + assert_err!(ValueSerDeContext::new() + .with_delayed_fields_serde() + .serialized_size(&value, &layout)); } } diff --git a/third_party/move/move-vm/types/src/values/value_prop_tests.rs b/third_party/move/move-vm/types/src/values/value_prop_tests.rs index cd612bac743df..24e2ed8b3422d 100644 --- a/third_party/move/move-vm/types/src/values/value_prop_tests.rs +++ b/third_party/move/move-vm/types/src/values/value_prop_tests.rs @@ -2,15 +2,15 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use crate::values::{prop::layout_and_value_strategy, Value}; +use crate::{value_serde::ValueSerDeContext, values::prop::layout_and_value_strategy}; use move_core_types::value::MoveValue; use proptest::prelude::*; proptest! { #[test] fn serializer_round_trip((layout, value) in layout_and_value_strategy()) { - let blob = value.simple_serialize(&layout).expect("must serialize"); - let value_deserialized = Value::simple_deserialize(&blob, &layout).expect("must deserialize"); + let blob = ValueSerDeContext::new().serialize(&value, &layout).unwrap().expect("must serialize"); + let value_deserialized = ValueSerDeContext::new().deserialize(&blob, &layout).expect("must deserialize"); assert!(value.equals(&value_deserialized).unwrap()); let move_value = value.as_move_value(&layout); diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs index 42fbe5d9278c3..fae84d1694a85 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -5,22 +5,29 @@ #![allow(clippy::arc_with_non_send_sync)] use crate::{ + delayed_values::delayed_field_id::{DelayedFieldID, TryFromMoveValue, TryIntoMoveValue}, loaded_data::runtime_types::Type, + value_serde::ValueSerDeContext, views::{ValueView, ValueVisitor}, }; use itertools::Itertools; use move_binary_format::{ errors::*, - file_format::{Constant, SignatureToken}, + file_format::{Constant, SignatureToken, VariantIndex}, }; use move_core_types::{ account_address::AccountAddress, effects::Op, gas_algebra::AbstractMemorySize, u256, value, - value::{MoveStructLayout, MoveTypeLayout}, + value::{MoveStruct, MoveStructLayout, MoveTypeLayout, MoveValue}, vm_status::{sub_status::NFE_VECTOR_ERROR_BASE, StatusCode}, }; +use serde::{ + de::{EnumAccess, Error as DeError, Unexpected, VariantAccess}, + ser::{Error as SerError, SerializeSeq, SerializeTuple, SerializeTupleVariant}, + Deserialize, +}; use std::{ cell::RefCell, cmp::Ordering, @@ -3625,57 +3632,12 @@ pub mod debug { * is to involve an explicit representation of the type layout. * **************************************************************************************/ -use crate::value_serde::{CustomDeserializer, CustomSerializer, RelaxedCustomSerDe}; -use move_binary_format::file_format::VariantIndex; -use serde::{ - de::{EnumAccess, Error as DeError, Unexpected, VariantAccess}, - ser::{Error as SerError, SerializeSeq, SerializeTuple, SerializeTupleVariant}, - Deserialize, -}; - -impl Value { - pub fn simple_deserialize(blob: &[u8], layout: &MoveTypeLayout) -> Option { - let seed = DeserializationSeed { - custom_deserializer: None::<&RelaxedCustomSerDe>, - layout, - }; - bcs::from_bytes_seed(seed, blob).ok() - } - - pub fn simple_serialize(&self, layout: &MoveTypeLayout) -> Option> { - bcs::to_bytes(&SerializationReadyValue { - custom_serializer: None::<&RelaxedCustomSerDe>, - layout, - value: &self.0, - }) - .ok() - } -} - -impl Struct { - pub fn simple_deserialize(blob: &[u8], layout: &MoveStructLayout) -> Option { - let seed = DeserializationSeed { - custom_deserializer: None::<&RelaxedCustomSerDe>, - layout, - }; - bcs::from_bytes_seed(seed, blob).ok() - } - - pub fn simple_serialize(&self, layout: &MoveStructLayout) -> Option> { - bcs::to_bytes(&SerializationReadyValue { - custom_serializer: None::<&RelaxedCustomSerDe>, - layout, - value: &self.fields, - }) - .ok() - } -} // Wrapper around value with additional information which can be used by the // serializer. -pub(crate) struct SerializationReadyValue<'c, 'l, 'v, L, V, C> { - // Allows to perform a custom serialization for delayed values. - pub(crate) custom_serializer: Option<&'c C>, +pub(crate) struct SerializationReadyValue<'c, 'l, 'v, L, V> { + // Contains the current (possibly custom) serialization context. + pub(crate) ctx: &'c ValueSerDeContext<'c>, // Layout for guiding serialization. pub(crate) layout: &'l L, // Value to serialize. @@ -3688,8 +3650,8 @@ fn invariant_violation(message: String) -> S::Error { ) } -impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize - for SerializationReadyValue<'c, 'l, 'v, MoveTypeLayout, ValueImpl, C> +impl<'c, 'l, 'v> serde::Serialize + for SerializationReadyValue<'c, 'l, 'v, MoveTypeLayout, ValueImpl> { fn serialize(&self, serializer: S) -> Result { use MoveTypeLayout as L; @@ -3708,7 +3670,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize // Structs. (L::Struct(struct_layout), ValueImpl::Container(Container::Struct(r))) => { (SerializationReadyValue { - custom_serializer: self.custom_serializer, + ctx: self.ctx, layout: struct_layout, value: &*r.borrow(), }) @@ -3732,7 +3694,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize let mut t = serializer.serialize_seq(Some(v.len()))?; for value in v.iter() { t.serialize_element(&SerializationReadyValue { - custom_serializer: self.custom_serializer, + ctx: self.ctx, layout, value, })?; @@ -3756,7 +3718,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize ))); } (SerializationReadyValue { - custom_serializer: self.custom_serializer, + ctx: self.ctx, layout: &L::Address, value: &v[0], }) @@ -3766,14 +3728,37 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize // Delayed values. For their serialization, we must have custom // serialization available, otherwise an error is returned. (L::Native(kind, layout), ValueImpl::DelayedFieldID { id }) => { - match self.custom_serializer { - Some(custom_serializer) => { - custom_serializer.custom_serialize(serializer, kind, layout, *id) + match &self.ctx.delayed_fields_extension { + Some(delayed_fields_extension) => { + delayed_fields_extension + .inc_and_check_delayed_fields_count() + .map_err(S::Error::custom)?; + + let value = match delayed_fields_extension.mapping { + Some(mapping) => mapping + .identifier_to_value(layout, *id) + .map_err(|e| S::Error::custom(format!("{}", e)))?, + None => id.try_into_move_value(layout).map_err(|_| { + S::Error::custom(format!( + "Custom serialization failed for {:?} with layout {}", + kind, layout + )) + })?, + }; + + // The resulting value should not contain any delayed fields, we disallow + // this by using a context without the delayed field extension. + let ctx = self.ctx.clone_without_delayed_fields(); + let value = SerializationReadyValue { + ctx: &ctx, + layout: layout.as_ref(), + value: &value.0, + }; + value.serialize(serializer) }, None => { - // If no custom serializer, it is not known how the - // delayed value should be serialized. So, just return - // an error. + // If no delayed field extension, it is not known how the delayed value + // should be serialized. So, just return an error. Err(invariant_violation::(format!( "no custom serializer for delayed value ({:?}) with layout {}", kind, layout @@ -3791,8 +3776,8 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize } } -impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize - for SerializationReadyValue<'c, 'l, 'v, MoveStructLayout, Vec, C> +impl<'c, 'l, 'v> serde::Serialize + for SerializationReadyValue<'c, 'l, 'v, MoveStructLayout, Vec> { fn serialize(&self, serializer: S) -> Result { let mut values = self.value.as_slice(); @@ -3818,7 +3803,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize variant_tag, variant_name, &SerializationReadyValue { - custom_serializer: self.custom_serializer, + ctx: self.ctx, layout: &variant_layouts[0], value: &values[0], }, @@ -3832,7 +3817,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize )?; for (layout, value) in variant_layouts.iter().zip(values) { t.serialize_field(&SerializationReadyValue { - custom_serializer: self.custom_serializer, + ctx: self.ctx, layout, value, })? @@ -3851,7 +3836,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize } for (field_layout, value) in field_layouts.iter().zip(values.iter()) { t.serialize_element(&SerializationReadyValue { - custom_serializer: self.custom_serializer, + ctx: self.ctx, layout: field_layout, value, })?; @@ -3863,17 +3848,14 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize // Seed used by deserializer to ensure there is information about the value // being deserialized. -pub(crate) struct DeserializationSeed<'c, L, C> { - // Allows to deserialize delayed values in the custom format using external - // deserializer. - pub(crate) custom_deserializer: Option<&'c C>, +pub(crate) struct DeserializationSeed<'c, L> { + // Holds extensions external to the deserializer. + pub(crate) ctx: &'c ValueSerDeContext<'c>, // Layout to guide deserialization. pub(crate) layout: L, } -impl<'d, 'c, C: CustomDeserializer> serde::de::DeserializeSeed<'d> - for DeserializationSeed<'c, &MoveTypeLayout, C> -{ +impl<'d, 'c> serde::de::DeserializeSeed<'d> for DeserializationSeed<'c, &MoveTypeLayout> { type Value = Value; fn deserialize>( @@ -3897,7 +3879,7 @@ impl<'d, 'c, C: CustomDeserializer> serde::de::DeserializeSeed<'d> // Structs. L::Struct(struct_layout) => { let seed = DeserializationSeed { - custom_deserializer: self.custom_deserializer, + ctx: self.ctx, layout: struct_layout, }; Ok(Value::struct_(seed.deserialize(deserializer)?)) @@ -3915,7 +3897,7 @@ impl<'d, 'c, C: CustomDeserializer> serde::de::DeserializeSeed<'d> L::Address => Value::vector_address(Vec::deserialize(deserializer)?), layout => { let seed = DeserializationSeed { - custom_deserializer: self.custom_deserializer, + ctx: self.ctx, layout, }; let vector = deserializer.deserialize_seq(VectorElementVisitor(seed))?; @@ -3927,9 +3909,34 @@ impl<'d, 'c, C: CustomDeserializer> serde::de::DeserializeSeed<'d> // Delayed values should always use custom deserialization. L::Native(kind, layout) => { - match self.custom_deserializer { - Some(native_deserializer) => { - native_deserializer.custom_deserialize(deserializer, kind, layout) + match &self.ctx.delayed_fields_extension { + Some(delayed_fields_extension) => { + delayed_fields_extension + .inc_and_check_delayed_fields_count() + .map_err(D::Error::custom)?; + + let value = DeserializationSeed { + ctx: &self.ctx.clone_without_delayed_fields(), + layout: layout.as_ref(), + } + .deserialize(deserializer)?; + let id = match delayed_fields_extension.mapping { + Some(mapping) => mapping + .value_to_identifier(kind, layout, value) + .map_err(|e| D::Error::custom(format!("{}", e)))?, + None => { + let (id, _) = + DelayedFieldID::try_from_move_value(layout, value, &()) + .map_err(|_| { + D::Error::custom(format!( + "Custom deserialization failed for {:?} with layout {}", + kind, layout + )) + })?; + id + }, + }; + Ok(Value::delayed_value(id)) }, None => { // If no custom deserializer, it is not known how the @@ -3949,9 +3956,7 @@ impl<'d, 'c, C: CustomDeserializer> serde::de::DeserializeSeed<'d> } } -impl<'d, C: CustomDeserializer> serde::de::DeserializeSeed<'d> - for DeserializationSeed<'_, &MoveStructLayout, C> -{ +impl<'d> serde::de::DeserializeSeed<'d> for DeserializationSeed<'_, &MoveStructLayout> { type Value = Struct; fn deserialize>( @@ -3962,7 +3967,7 @@ impl<'d, C: CustomDeserializer> serde::de::DeserializeSeed<'d> MoveStructLayout::Runtime(field_layouts) => { let fields = deserializer.deserialize_tuple( field_layouts.len(), - StructFieldVisitor(self.custom_deserializer, field_layouts), + StructFieldVisitor(self.ctx, field_layouts), )?; Ok(Struct::pack(fields)) }, @@ -3974,7 +3979,7 @@ impl<'d, C: CustomDeserializer> serde::de::DeserializeSeed<'d> let fields = deserializer.deserialize_enum( value::MOVE_ENUM_NAME, variant_names, - StructVariantVisitor(self.custom_deserializer, variants), + StructVariantVisitor(self.ctx, variants), )?; Ok(Struct::pack(fields)) }, @@ -3987,9 +3992,9 @@ impl<'d, C: CustomDeserializer> serde::de::DeserializeSeed<'d> } } -struct VectorElementVisitor<'c, 'l, C>(DeserializationSeed<'c, &'l MoveTypeLayout, C>); +struct VectorElementVisitor<'c, 'l>(DeserializationSeed<'c, &'l MoveTypeLayout>); -impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for VectorElementVisitor<'c, 'l, C> { +impl<'d, 'c, 'l> serde::de::Visitor<'d> for VectorElementVisitor<'c, 'l> { type Value = Vec; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -4002,7 +4007,7 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for VectorElement { let mut vals = Vec::new(); while let Some(elem) = seq.next_element_seed(DeserializationSeed { - custom_deserializer: self.0.custom_deserializer, + ctx: self.0.ctx, layout: self.0.layout, })? { vals.push(elem.0) @@ -4011,9 +4016,9 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for VectorElement } } -struct StructFieldVisitor<'c, 'l, C>(Option<&'c C>, &'l [MoveTypeLayout]); +struct StructFieldVisitor<'c, 'l>(&'c ValueSerDeContext<'c>, &'l [MoveTypeLayout]); -impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructFieldVisitor<'c, 'l, C> { +impl<'d, 'c, 'l> serde::de::Visitor<'d> for StructFieldVisitor<'c, 'l> { type Value = Vec; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -4027,7 +4032,7 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructFieldVi let mut val = Vec::new(); for (i, field_layout) in self.1.iter().enumerate() { if let Some(elem) = seq.next_element_seed(DeserializationSeed { - custom_deserializer: self.0, + ctx: self.0, layout: field_layout, })? { val.push(elem) @@ -4039,9 +4044,9 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructFieldVi } } -struct StructVariantVisitor<'c, 'l, C>(Option<&'c C>, &'l [Vec]); +struct StructVariantVisitor<'c, 'l>(&'c ValueSerDeContext<'c>, &'l [Vec]); -impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructVariantVisitor<'c, 'l, C> { +impl<'d, 'c, 'l> serde::de::Visitor<'d> for StructVariantVisitor<'c, 'l> { type Value = Vec; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -4065,7 +4070,7 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructVariant }, 1 => { values.push(rest.newtype_variant_seed(DeserializationSeed { - custom_deserializer: self.0, + ctx: self.0, layout: &fields[0], })?); Ok(values) @@ -4091,7 +4096,7 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructVariant // Note this is actually directly serialized as u16, but this is equivalent // to MoveTypeLayout::U16, so we can reuse the custom deserializer seed. let variant_tag = match seq.next_element_seed(DeserializationSeed { - custom_deserializer: self.0, + ctx: self.0, layout: &MoveTypeLayout::U16, })? { Some(elem) => { @@ -4117,7 +4122,7 @@ impl<'d, 'c, 'l, C: CustomDeserializer> serde::de::Visitor<'d> for StructVariant // Based on the validated variant tag, we know the field types for (i, field_layout) in self.1[variant_tag].iter().enumerate() { if let Some(elem) = seq.next_element_seed(DeserializationSeed { - custom_deserializer: self.0, + ctx: self.0, layout: field_layout, })? { val.push(elem) @@ -4162,7 +4167,7 @@ impl Value { pub fn deserialize_constant(constant: &Constant) -> Option { let layout = Self::constant_sig_token_to_layout(&constant.type_)?; - Value::simple_deserialize(&constant.data, &layout) + ValueSerDeContext::new().deserialize(&constant.data, &layout) } } @@ -4583,9 +4588,6 @@ pub mod prop { } } -use crate::delayed_values::delayed_field_id::DelayedFieldID; -use move_core_types::value::{MoveStruct, MoveValue}; - impl ValueImpl { pub fn as_move_value(&self, layout: &MoveTypeLayout) -> MoveValue { use MoveTypeLayout as L; diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs index be05bd2c0e333..9fcf18dc3f188 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs @@ -50,6 +50,7 @@ use move_model::{ }; use move_symbol_pool::Symbol; use move_vm_runtime::session::SerializedReturnValues; +use move_vm_types::value_serde::ValueSerDeContext; use once_cell::sync::Lazy; use regex::Regex; use std::{ @@ -621,8 +622,7 @@ fn display_return_values(return_values: SerializedReturnValues) -> Option>(); @@ -641,7 +641,8 @@ fn display_return_values(return_values: SerializedReturnValues) -> Option>(); let printed = values diff --git a/third_party/move/tools/move-unit-test/src/extensions.rs b/third_party/move/tools/move-unit-test/src/extensions.rs index f9fd6d78203c4..05b1058540ef4 100644 --- a/third_party/move/tools/move-unit-test/src/extensions.rs +++ b/third_party/move/tools/move-unit-test/src/extensions.rs @@ -13,6 +13,7 @@ use move_table_extension::NativeTableContext; use move_vm_runtime::native_extensions::NativeContextExtensions; #[cfg(feature = "table-extension")] use move_vm_test_utils::BlankStorage; +use move_vm_types::value_serde::FunctionValueExtension; use once_cell::sync::Lazy; use std::{fmt::Write, sync::Mutex}; @@ -50,22 +51,32 @@ pub(crate) fn new_extensions<'a>() -> NativeContextExtensions<'a> { /// Print the change sets for available native context extensions. #[allow(unused)] -pub(crate) fn print_change_sets(_w: &mut W, extensions: &mut NativeContextExtensions) { +pub(crate) fn print_change_sets( + w: &mut W, + native_extensions: &mut NativeContextExtensions, + function_value_extension: &impl FunctionValueExtension, +) { #[cfg(feature = "table-extension")] - print_table_extension(_w, extensions); + print_table_extension(w, native_extensions, function_value_extension); } // ============================================================================================= // Table Extensions #[cfg(feature = "table-extension")] -fn create_table_extension(extensions: &mut NativeContextExtensions) { - extensions.add(NativeTableContext::new([0u8; 32], &*DUMMY_RESOLVER)); +fn create_table_extension(native_extensions: &mut NativeContextExtensions) { + native_extensions.add(NativeTableContext::new([0u8; 32], &*DUMMY_RESOLVER)); } #[cfg(feature = "table-extension")] -fn print_table_extension(w: &mut W, extensions: &mut NativeContextExtensions) { - let cs = extensions.remove::().into_change_set(); +fn print_table_extension( + w: &mut W, + native_extensions: &mut NativeContextExtensions, + function_value_extension: &impl FunctionValueExtension, +) { + let cs = native_extensions + .remove::() + .into_change_set(function_value_extension); if let Ok(cs) = cs { if !cs.new_tables.is_empty() { writeln!( diff --git a/third_party/move/tools/move-unit-test/src/test_runner.rs b/third_party/move/tools/move-unit-test/src/test_runner.rs index 6c5e839411fc6..03ea88601103e 100644 --- a/third_party/move/tools/move-unit-test/src/test_runner.rs +++ b/third_party/move/tools/move-unit-test/src/test_runner.rs @@ -27,7 +27,7 @@ use move_vm_runtime::{ move_vm::MoveVM, native_extensions::NativeContextExtensions, native_functions::NativeFunctionTable, - AsUnsyncModuleStorage, RuntimeEnvironment, + AsFunctionValueExtension, AsUnsyncModuleStorage, RuntimeEnvironment, }; use move_vm_test_utils::InMemoryStorage; use rayon::prelude::*; @@ -86,6 +86,7 @@ fn print_resources_and_extensions( cs: &ChangeSet, extensions: &mut NativeContextExtensions, storage: &InMemoryStorage, + natives: &NativeFunctionTable, ) -> Result { use std::fmt::Write; let mut buf = String::new(); @@ -104,7 +105,10 @@ fn print_resources_and_extensions( } } - extensions::print_change_sets(&mut buf, extensions); + let runtime_environment = RuntimeEnvironment::new(natives.clone()); + let module_storage = storage.as_unsync_module_storage(runtime_environment); + let function_value_extension = module_storage.as_function_value_extension(); + extensions::print_change_sets(&mut buf, extensions, &function_value_extension); Ok(buf) } @@ -339,6 +343,7 @@ impl SharedTestingConfig { &changeset, &mut extensions, &self.starting_storage_state, + &self.native_function_table, ) .ok() }) diff --git a/types/src/transaction/mod.rs b/types/src/transaction/mod.rs index 138dd8e81496c..5c2560926d9bc 100644 --- a/types/src/transaction/mod.rs +++ b/types/src/transaction/mod.rs @@ -68,9 +68,6 @@ pub use change_set::ChangeSet; pub use module::{Module, ModuleBundle}; pub use move_core_types::transaction_argument::TransactionArgument; use move_core_types::vm_status::AbortLocation; -use move_vm_types::delayed_values::delayed_field_id::{ - ExtractUniqueIndex, ExtractWidth, TryFromMoveValue, TryIntoMoveValue, -}; pub use multisig::{ExecutionError, Multisig, MultisigTransactionPayload}; use once_cell::sync::OnceCell; pub use script::{ @@ -2063,22 +2060,6 @@ pub trait BlockExecutableTransaction: Sync + Send + Clone + 'static { + Debug + DeserializeOwned + Serialize; - /// Delayed field identifier type. - type Identifier: PartialOrd - + Ord - + Send - + Sync - + Clone - + Hash - + Eq - + Debug - + Copy - + From - + From<(u32, u32)> - + ExtractUniqueIndex - + ExtractWidth - + TryIntoMoveValue - + TryFromMoveValue; type Value: Send + Sync + Debug + Clone + TransactionWrite; type Event: Send + Sync + Debug + Clone + TransactionEvent; diff --git a/types/src/transaction/signature_verified_transaction.rs b/types/src/transaction/signature_verified_transaction.rs index cd35b573b42c1..6d405aa54d8fb 100644 --- a/types/src/transaction/signature_verified_transaction.rs +++ b/types/src/transaction/signature_verified_transaction.rs @@ -9,7 +9,6 @@ use crate::{ }; use aptos_crypto::{hash::CryptoHash, HashValue}; use move_core_types::{account_address::AccountAddress, language_storage::StructTag}; -use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use serde::{Deserialize, Serialize}; use std::fmt::Debug; @@ -61,7 +60,6 @@ impl SignatureVerifiedTransaction { impl BlockExecutableTransaction for SignatureVerifiedTransaction { type Event = ContractEvent; - type Identifier = DelayedFieldID; type Key = StateKey; type Tag = StructTag; type Value = WriteOp;