diff --git a/aptos-move/block-executor/src/captured_reads.rs b/aptos-move/block-executor/src/captured_reads.rs index 4eba9c9bac437..f1117594b1892 100644 --- a/aptos-move/block-executor/src/captured_reads.rs +++ b/aptos-move/block-executor/src/captured_reads.rs @@ -621,13 +621,12 @@ impl CapturedReads { return false; } + // TODO(loader_v2): + // Should we also check state value metadata? If so, using transaction indices to decide + // the result of validations might be easier. self.module_cache_reads.iter().all(|(id, previous_read)| { - let previous_hash_and_state_value_metadata = previous_read - .as_ref() - .map(|e| (e.hash(), e.state_value_metadata())); - code_cache - .module_cache() - .check_hash_and_state_value_metadata(id, previous_hash_and_state_value_metadata) + let previous_hash = previous_read.as_ref().map(|e| e.hash()); + code_cache.module_cache().check_hash(id, previous_hash) }) } diff --git a/aptos-move/mvhashmap/src/code_cache/sync.rs b/aptos-move/mvhashmap/src/code_cache/sync.rs index 7bcb6bda01213..02570770a7112 100644 --- a/aptos-move/mvhashmap/src/code_cache/sync.rs +++ b/aptos-move/mvhashmap/src/code_cache/sync.rs @@ -1,10 +1,7 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use aptos_types::{ - state_store::state_value::StateValueMetadata, - vm::{modules::ModuleCacheEntry, scripts::ScriptCacheEntry}, -}; +use aptos_types::vm::{modules::ModuleCacheEntry, scripts::ScriptCacheEntry}; use crossbeam::utils::CachePadded; use dashmap::{mapref::entry::Entry, DashMap}; use hashbrown::HashMap; @@ -79,33 +76,9 @@ impl ModuleCache { /// same state value metadata as previously, or does not contain the cached entry in case it /// did not contain it before. Used to validate module storage reads when there are modules /// published. - /// Note that [StateValueMetadata] comparison is required because it is currently possible to - /// publish modules with the same code (e.g., changing a single module as part of a package, - /// but publishing the whole package), and hence the same cache. For those, the state value - /// metadata changes, at least because we no longer have a creation but a modification op. - pub fn check_hash_and_state_value_metadata( - &self, - module_id: &ModuleId, - previous_hash_and_state_value_metadata: Option<(&[u8; 32], &StateValueMetadata)>, - ) -> bool { - let current_hash_and_state_value_metadata = self - .cache - .get(module_id) - .map(|e| (*e.hash(), e.state_value_metadata().clone())); - match ( - ¤t_hash_and_state_value_metadata, - previous_hash_and_state_value_metadata, - ) { - ( - Some((current_hash, current_state_value_metadata)), - Some((previous_hash, previous_state_value_metadata)), - ) => { - current_hash == previous_hash - && current_state_value_metadata == previous_state_value_metadata - }, - (None, None) => true, - (Some(..), None) | (None, Some(..)) => false, - } + pub fn check_hash(&self, module_id: &ModuleId, previous_hash: Option<&[u8; 32]>) -> bool { + let current_hash = self.cache.get(module_id).map(|e| *e.hash()); + current_hash.as_ref() == previous_hash } /// Return the cached module from the module cache. If it is not cached, use the passed