From 1a343dc165e27004279146f38c03ef6a7e914238 Mon Sep 17 00:00:00 2001 From: Tim Zakian <2895723+tzakian@users.noreply.github.com> Date: Fri, 17 Jan 2025 16:06:20 -0800 Subject: [PATCH] [vm-rewrite] Fix tests (#20917) ## Description Update how we handle deserialization errors so they maintain their specificity. Also updates a bad merge. ## Test plan CI --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .../src/execution/dispatch_tables.rs | 30 +++++++++++++++---- .../move-vm-runtime/src/execution/vm.rs | 14 +++------ .../crates/move-vm-runtime/src/runtime/mod.rs | 4 +-- .../move-vm-runtime/src/shared/constants.rs | 2 +- .../src/shared/serialization.rs | 8 ++--- .../validation/deserialization/translate.rs | 7 +---- 6 files changed, 35 insertions(+), 30 deletions(-) diff --git a/external-crates/move/crates/move-vm-runtime/src/execution/dispatch_tables.rs b/external-crates/move/crates/move-vm-runtime/src/execution/dispatch_tables.rs index eaceed50ecc23..569ed2ab76ff7 100644 --- a/external-crates/move/crates/move-vm-runtime/src/execution/dispatch_tables.rs +++ b/external-crates/move/crates/move-vm-runtime/src/execution/dispatch_tables.rs @@ -11,7 +11,9 @@ use crate::{ cache::identifier_interner::IdentifierKey, jit::execution::ast::{Datatype, EnumType, Function, Module, Package, StructType, Type}, shared::{ - constants::{MAX_TYPE_INSTANTIATION_NODES, MAX_TYPE_TO_LAYOUT_NODES, VALUE_DEPTH_MAX}, + constants::{ + HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES, MAX_TYPE_INSTANTIATION_NODES, VALUE_DEPTH_MAX, + }, types::RuntimePackageId, vm_pointer::VMPointer, }, @@ -30,6 +32,7 @@ use move_core_types::{ use move_binary_format::file_format::DatatypeTyParameter; use move_core_types::{annotated_value as A, identifier::Identifier, runtime_value as R}; +use move_vm_config::runtime::VMConfig; use parking_lot::RwLock; use std::{collections::BTreeMap, sync::Arc}; @@ -52,6 +55,7 @@ use tracing::error; /// vtable/cross-package function resolution but we will keep it simple for now. #[derive(Debug, Clone)] pub struct VMDispatchTables { + pub(crate) vm_config: Arc, pub(crate) loaded_packages: BTreeMap>, } @@ -142,8 +146,14 @@ pub struct DepthFormula { impl VMDispatchTables { /// Create a new RuntimeVTables instance. /// NOTE: This assumes linkage has already occured. - pub fn new(loaded_packages: BTreeMap>) -> VMResult { - Ok(Self { loaded_packages }) + pub fn new( + vm_config: Arc, + loaded_packages: BTreeMap>, + ) -> VMResult { + Ok(Self { + vm_config, + loaded_packages, + }) } pub fn get_package(&self, id: &RuntimePackageId) -> PartialVMResult> { @@ -654,7 +664,12 @@ impl VMDispatchTables { count: &mut u64, depth: u64, ) -> PartialVMResult { - if *count > MAX_TYPE_TO_LAYOUT_NODES { + if *count + > self + .vm_config + .max_type_to_layout_nodes + .unwrap_or(HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES) + { return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES)); } if depth > VALUE_DEPTH_MAX { @@ -795,7 +810,12 @@ impl VMDispatchTables { count: &mut u64, depth: u64, ) -> PartialVMResult { - if *count > MAX_TYPE_TO_LAYOUT_NODES { + if *count + > self + .vm_config + .max_type_to_layout_nodes + .unwrap_or(HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES) + { return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES)); } if depth > VALUE_DEPTH_MAX { diff --git a/external-crates/move/crates/move-vm-runtime/src/execution/vm.rs b/external-crates/move/crates/move-vm-runtime/src/execution/vm.rs index 8abf8b8e7d39b..f765977e0283b 100644 --- a/external-crates/move/crates/move-vm-runtime/src/execution/vm.rs +++ b/external-crates/move/crates/move-vm-runtime/src/execution/vm.rs @@ -325,7 +325,6 @@ impl<'extensions> MoveVM<'extensions> { // deserialization should be done outside of the VM calls. let (ref_ids, deserialized_args) = deserialize_args( &self.virtual_tables, - &self.vm_config, &mut self.base_heap, arg_types, serialized_args, @@ -348,13 +347,9 @@ impl<'extensions> MoveVM<'extensions> { deserialized_args, )?; - let serialized_return_values = serialize_return_values( - &self.virtual_tables, - &self.vm_config, - &return_types, - return_values, - ) - .map_err(|e| e.finish(Location::Undefined))?; + let serialized_return_values = + serialize_return_values(&self.virtual_tables, &return_types, return_values) + .map_err(|e| e.finish(Location::Undefined))?; let serialized_mut_ref_outputs = mut_ref_args .into_iter() .map(|(ndx, ty)| { @@ -362,8 +357,7 @@ impl<'extensions> MoveVM<'extensions> { // take the value of each reference; return values first in the case that a value // points into this local let local_val = self.base_heap.take_loc(*heap_ref_id)?; - let (bytes, layout) = - serialize_return_value(&self.virtual_tables, &self.vm_config, &ty, local_val)?; + let (bytes, layout) = serialize_return_value(&self.virtual_tables, &ty, local_val)?; Ok((ndx as LocalIndex, bytes, layout)) }) .collect::>() diff --git a/external-crates/move/crates/move-vm-runtime/src/runtime/mod.rs b/external-crates/move/crates/move-vm-runtime/src/runtime/mod.rs index 8970c8cfcf96e..98f0e67dbfb5e 100644 --- a/external-crates/move/crates/move-vm-runtime/src/runtime/mod.rs +++ b/external-crates/move/crates/move-vm-runtime/src/runtime/mod.rs @@ -120,7 +120,7 @@ impl MoveRuntime { .map(|pkg| (pkg.runtime.runtime_id, Arc::clone(&pkg.runtime))) .collect::>>(); - let virtual_tables = VMDispatchTables::new(runtime_packages)?; + let virtual_tables = VMDispatchTables::new(self.vm_config.clone(), runtime_packages)?; let base_heap = BaseHeap::new(); @@ -200,7 +200,7 @@ impl MoveRuntime { .map(|pkg| (pkg.runtime.runtime_id, Arc::clone(&pkg.runtime))) .collect::>>(); - let virtual_tables = VMDispatchTables::new(runtime_packages)?; + let virtual_tables = VMDispatchTables::new(self.vm_config.clone(), runtime_packages)?; let base_heap = BaseHeap::new(); diff --git a/external-crates/move/crates/move-vm-runtime/src/shared/constants.rs b/external-crates/move/crates/move-vm-runtime/src/shared/constants.rs index b11c38fb27b1e..cc4c219e58335 100644 --- a/external-crates/move/crates/move-vm-runtime/src/shared/constants.rs +++ b/external-crates/move/crates/move-vm-runtime/src/shared/constants.rs @@ -22,7 +22,7 @@ pub const VALUE_DEPTH_MAX: u64 = 128; /// fields for struct types. /// Maximal nodes which are allowed when converting to layout. This includes the types of /// fields for datatypes. -pub const MAX_TYPE_TO_LAYOUT_NODES: u64 = 256; +pub const HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES: u64 = 256; /// Maximal nodes which are all allowed when instantiating a generic type. This does not include /// field types of datatypes. diff --git a/external-crates/move/crates/move-vm-runtime/src/shared/serialization.rs b/external-crates/move/crates/move-vm-runtime/src/shared/serialization.rs index 74a1f7a9973c6..c9336e437c5a0 100644 --- a/external-crates/move/crates/move-vm-runtime/src/shared/serialization.rs +++ b/external-crates/move/crates/move-vm-runtime/src/shared/serialization.rs @@ -14,7 +14,6 @@ use move_binary_format::{ file_format::LocalIndex, }; use move_core_types::{runtime_value::MoveTypeLayout, vm_status::StatusCode}; -use move_vm_config::runtime::VMConfig; use tracing::warn; @@ -68,7 +67,6 @@ pub fn deserialize_value( /// Returns the list of mutable references plus the vector of values. pub fn deserialize_args( vtables: &VMDispatchTables, - _vm_config: &VMConfig, heap: &mut BaseHeap, arg_tys: Vec, serialized_args: Vec>, @@ -106,7 +104,6 @@ pub fn deserialize_args( pub fn serialize_return_value( vtables: &VMDispatchTables, - vm_config: &VMConfig, ty: &Type, value: Value, ) -> PartialVMResult<(Vec, MoveTypeLayout)> { @@ -123,7 +120,7 @@ pub fn serialize_return_value( _ => (ty, value), }; - let layout = if vm_config.rethrow_serialization_type_layout_errors { + let layout = if vtables.vm_config.rethrow_serialization_type_layout_errors { vtables.type_to_type_layout(ty)? } else { vtables.type_to_type_layout(ty).map_err(|_err| { @@ -142,7 +139,6 @@ pub fn serialize_return_value( pub fn serialize_return_values( vtables: &VMDispatchTables, - vm_config: &VMConfig, return_types: &[Type], return_values: Vec, ) -> PartialVMResult, MoveTypeLayout)>> { @@ -161,6 +157,6 @@ pub fn serialize_return_values( return_types .iter() .zip(return_values) - .map(|(ty, value)| serialize_return_value(vtables, vm_config, ty, value)) + .map(|(ty, value)| serialize_return_value(vtables, ty, value)) .collect() } diff --git a/external-crates/move/crates/move-vm-runtime/src/validation/deserialization/translate.rs b/external-crates/move/crates/move-vm-runtime/src/validation/deserialization/translate.rs index 93eca56188fde..3f6e43ee11f3b 100644 --- a/external-crates/move/crates/move-vm-runtime/src/validation/deserialization/translate.rs +++ b/external-crates/move/crates/move-vm-runtime/src/validation/deserialization/translate.rs @@ -13,12 +13,7 @@ pub(crate) fn package(vm_config: &VMConfig, pkg: SerializedPackage) -> VMResult< let mut modules = BTreeMap::new(); for module in pkg.modules.iter() { let module = CompiledModule::deserialize_with_config(module, &vm_config.binary_config) - .map_err(|err| -> move_binary_format::errors::VMError { - let msg = format!("Deserialization error: {:?}", err); - PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) - .with_message(msg) - .finish(Location::Undefined) // TODO(tzakian): add Location::Package - })?; + .map_err(|err| err.finish(Location::Undefined))?; // TODO: add Location::Package modules.insert(module.self_id(), module); }