address comments from Igor & fix rebase

aptos-move/aptos-vm/src/errors.rs

@@ -138,7 +138,6 @@ pub fn convert_prologue_error(
                     let err_msg = format!("[aptos_vm] Unexpected prologue Move abort: {:?}::{:?} (Category: {:?} Reason: {:?})",
                     location, code, category, reason);
                     speculative_error!(log_context, err_msg.clone());
-                    println!("B: {:?}", err_msg);
                     return Err(VMStatus::Error {
                         status_code: StatusCode::UNEXPECTED_ERROR_FROM_KNOWN_MOVE_FUNCTION,
                         sub_status: None,
@@ -160,7 +159,6 @@ pub fn convert_prologue_error(
                 log_context,
                 format!("[aptos_vm] Unexpected prologue error: {:?}", status),
             );
-            println!("A: {:?}", status);
             VMStatus::Error {
                 status_code: StatusCode::UNEXPECTED_ERROR_FROM_KNOWN_MOVE_FUNCTION,
                 sub_status: status.sub_status(),
@@ -215,7 +213,6 @@ pub fn convert_epilogue_error(
         status => {
             let err_msg = format!("[aptos_vm] Unexpected success epilogue error: {:?}", status);
             speculative_error!(log_context, err_msg.clone());
-            println!("C: {:?}", err_msg);
             VMStatus::Error {
                 status_code: StatusCode::UNEXPECTED_ERROR_FROM_KNOWN_MOVE_FUNCTION,
                 sub_status: status.sub_status(),

aptos-move/block-executor/src/captured_reads.rs

@@ -620,8 +620,8 @@ impl<T: Transaction> CapturedReads<T> {
         // and fetching it returned an error, the error is not recorded. Hence, it is safe here
         // to simply treat such errors as a non-existent value.
         self.module_storage_reads.iter().all(|(k, previous_read)| {
-            let new_read = module_storage.get(k, idx_to_validate);
-            previous_read == &new_read
+            let current_read = module_storage.get(k, idx_to_validate);
+            previous_read == &current_read
         })
     }
 
@@ -736,10 +736,13 @@ impl<T: Transaction> CapturedReads<T> {
             }
         }
 
-        // TODO(loader_v2): We have a new set of reads now. We probably need to feature gate here as well?
+        // TODO(loader_v2): Test summaries are the same.
         for key in &self.module_reads {
             ret.insert(InputOutputKey::Resource(key.clone()));
         }
+        for key in self.module_storage_reads.keys() {
+            ret.insert(InputOutputKey::Resource(key.clone()));
+        }
 
         for (key, read) in &self.delayed_field_reads {
             if let DelayedFieldRead::Value { .. } = read {
@@ -784,7 +787,7 @@ impl<T: Transaction> UnsyncReadSet<T> {
             }
         }
 
-        // TODO(loader_v2): Should we be using a feature flag here?
+        // TODO(loader_v2): Test summaries are the same if we switch.
         for key in &self.module_reads {
             ret.insert(InputOutputKey::Resource(key.clone()));
         }

aptos-move/block-executor/src/executor.rs

@@ -542,6 +542,7 @@ where
                     ),
                 )?;
 
+                // TODO(loader_v2): Remove duplication here.
                 // Make sure we publish modules here before we wake up subsequent dependent
                 // transactions and decrease the validation index.
                 if runtime_environment.vm_config().use_loader_v2 {

third_party/move/move-vm/integration-tests/src/tests/bad_storage_tests.rs

@@ -635,14 +635,6 @@ impl ModuleStorage for BogusModuleStorage {
         Err(PartialVMError::new(self.bad_status_code).finish(Location::Undefined))
     }
 
-    fn fetch_module_bytes(
-        &self,
-        _address: &AccountAddress,
-        _module_name: &IdentStr,
-    ) -> PartialVMResult<Option<Bytes>> {
-        Err(PartialVMError::new(self.bad_status_code))
-    }
-
     fn fetch_module_size_in_bytes(
         &self,
         _address: &AccountAddress,

third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs

@@ -173,7 +173,6 @@ impl<M: ModuleStorage + WithRuntimeEnvironment> CodeStorage for UnsyncCodeStorag
     }
 }
 
-
 #[cfg(test)]
 impl<M: ModuleStorage + WithRuntimeEnvironment> UnsyncCodeStorage<M> {
     fn matches<P: Fn(&ScriptStorageEntry) -> bool>(

third_party/move/move-vm/runtime/src/storage/module_storage.rs

@@ -74,13 +74,3 @@ pub trait ModuleBytesStorage {
         module_name: &IdentStr,
     ) -> VMResult<Option<Bytes>>;
 }
-
-/// Storage that contains serialized modules. Clients can implement this trait
-/// for their own backends, so that [ModuleStorage] can be built on top of it.
-pub trait ModuleBytesStorage {
-    fn fetch_module_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> PartialVMResult<Option<Bytes>>;
-}