[loader-v2] Remove extra hash calculation from runtime environment (#…

…15256)
aptos-labs · Nov 14, 2024 · 817bf70 · 817bf70
1 parent cb4dd96
commit 817bf70
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 20 deletions.
diff --git a/aptos-move/block-executor/src/code_cache.rs b/aptos-move/block-executor/src/code_cache.rs
@@ -54,9 +54,7 @@ impl<'a, T: Transaction, S: TStateView<Key = T::Key>, X: Executable> ModuleCodeB
             .map_err(|err| err.finish(Location::Undefined))?
             .map(|state_value| {
                 let extension = Arc::new(AptosModuleExtension::new(state_value));
-
-                // TODO(loader_v2): This recomputes module hash twice, we should avoid it.
-                let (compiled_module, _, _) = self
+                let compiled_module = self
                     .runtime_environment()
                     .deserialize_into_compiled_module(extension.bytes())?;
                 Ok(ModuleCode::from_deserialized(compiled_module, extension))

diff --git a/aptos-move/block-executor/src/executor.rs b/aptos-move/block-executor/src/executor.rs
@@ -1239,7 +1239,7 @@ where
 
         // Since we have successfully serialized the module when converting into this transaction
         // write, the deserialization should never fail.
-        let (compiled_module, _, _) = runtime_environment
+        let compiled_module = runtime_environment
             .deserialize_into_compiled_module(state_value.bytes())
             .map_err(|err| {
                 let msg = format!("Failed to construct the module from state value: {:?}", err);

diff --git a/third_party/move/move-vm/runtime/src/storage/environment.rs b/third_party/move/move-vm/runtime/src/storage/environment.rs
@@ -27,7 +27,6 @@ use move_core_types::{
 };
 #[cfg(any(test, feature = "testing"))]
 use move_vm_types::loaded_data::runtime_types::{StructIdentifier, StructNameIndex};
-use move_vm_types::sha3_256;
 use std::sync::Arc;
 
 /// [MoveVM] runtime environment encapsulating different configurations. Shared between the VM and
@@ -192,21 +191,15 @@ impl RuntimeEnvironment {
         result.map_err(|e| e.finish(Location::Undefined))
     }
 
-    /// Deserializes bytes into a compiled module, also returning its size and hash.
-    pub fn deserialize_into_compiled_module(
-        &self,
-        bytes: &Bytes,
-    ) -> VMResult<(CompiledModule, usize, [u8; 32])> {
-        let compiled_module =
-            CompiledModule::deserialize_with_config(bytes, &self.vm_config().deserializer_config)
-                .map_err(|err| {
+    /// Deserializes bytes into a compiled module.
+    pub fn deserialize_into_compiled_module(&self, bytes: &Bytes) -> VMResult<CompiledModule> {
+        CompiledModule::deserialize_with_config(bytes, &self.vm_config().deserializer_config)
+            .map_err(|err| {
                 let msg = format!("Deserialization error: {:?}", err);
                 PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR)
                     .with_message(msg)
                     .finish(Location::Undefined)
-            })?;
-
-        Ok((compiled_module, bytes.len(), sha3_256(bytes)))
+            })
     }
 
     /// Deserializes bytes into a compiled script.

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
@@ -17,9 +17,12 @@ use move_core_types::{
     account_address::AccountAddress, identifier::IdentStr, language_storage::ModuleId,
     metadata::Metadata,
 };
-use move_vm_types::code::{
-    ambassador_impl_ModuleCache, ModuleBytesStorage, ModuleCache, ModuleCode, ModuleCodeBuilder,
-    UnsyncModuleCache, WithBytes, WithHash,
+use move_vm_types::{
+    code::{
+        ambassador_impl_ModuleCache, ModuleBytesStorage, ModuleCache, ModuleCode,
+        ModuleCodeBuilder, UnsyncModuleCache, WithBytes, WithHash,
+    },
+    sha3_256,
 };
 use std::{borrow::Borrow, ops::Deref, sync::Arc};
 
@@ -137,9 +140,10 @@ impl<'s, S: ModuleBytesStorage, E: WithRuntimeEnvironment> ModuleCodeBuilder
             Some(bytes) => bytes,
             None => return Ok(None),
         };
-        let (compiled_module, _, hash) = self
+        let compiled_module = self
             .runtime_environment()
             .deserialize_into_compiled_module(&bytes)?;
+        let hash = sha3_256(&bytes);
         let extension = Arc::new(BytesWithHash::new(bytes, hash));
         let module = ModuleCode::from_deserialized(compiled_module, extension);
         Ok(Some(module))