[move cli] doctor command for detecting inconsistencies in storage

This command does a bunch of sanity checks to ensure that the storage directory is well-formed. For now:
- All modules verify
- All modules link
- All resources deserialize w.r.t the current version of their declaring module

It does this in the obvious way: iterating through the global storage, collecting each module/resource, and doing the check.
This is useful for detecting breaking changes (both code and data layout) after-the-fact, as well as bugs in the CLI.

I was motivated to create this check when I noticed a bug where `move publish` sometimes publishes a module `M` without publishing library modules.
If you run `move doctor` on such a state, it would catch the issue. I am planning to add `move doctor` to a bunch of the publish tests once I fix this bug.
It will also be useful for testing the `--ignore-breaking-changes` flag in diem#6753.

Closes: diem#6971

language/tools/move-cli/Cargo.toml

@@ -16,6 +16,8 @@ include_dir = { version = "0.6.0", features = ["search"] }
 once_cell = "1.4.1"
 structopt = "0.3.21"
 
+bcs = "0.1.2"
+bytecode-verifier = { path = "../../bytecode-verifier", version = "0.1.0" }
 compiled-stdlib = { path = "../../stdlib/compiled", version = "0.1.0" }
 disassembler = { path = "../disassembler", version = "0.1.0" }
 errmapgen = { path = "../../move-prover/errmapgen", version = "0.1.0" }
@@ -32,7 +34,7 @@ resource-viewer = { path = "../resource-viewer", version = "0.1.0" }
 stdlib = { path = "../../stdlib", version = "0.1.0" }
 vm = { path = "../../vm", version = "0.1.0" }
 vm-genesis = { path = "../vm-genesis", version = "0.1.0" }
-bcs = "0.1.2"
+walkdir = "2.3.1"
 
 [dev-dependencies]
 datatest-stable = { path = "../../../common/datatest-stable", version = "0.1.0" }

language/tools/move-cli/src/lib.rs

@@ -24,7 +24,7 @@ use vm::{
 
 use anyhow::{anyhow, bail, Result};
 use std::{
-    collections::HashMap,
+    collections::{BTreeMap, HashMap},
     convert::TryFrom,
     fs,
     path::{Path, PathBuf},
@@ -187,6 +187,8 @@ impl OnDiskStateView {
         })
     }
 
+    /// Returns a deserialized representation of the resource value stored at `resource_path`.
+    /// Returns Err if the path does not hold a resource value or the resource cannot be deserialized
     pub fn view_resource(&self, resource_path: &Path) -> Result<Option<AnnotatedMoveStruct>> {
         if resource_path.is_dir() {
             bail!("Bad resource path {:?}. Needed file, found directory")
@@ -325,6 +327,44 @@ impl OnDiskStateView {
         }
         Ok(!self.modules.is_empty())
     }
+
+    fn iter_paths<F>(&self, f: F) -> impl Iterator<Item = PathBuf>
+    where
+        F: FnOnce(&Path) -> bool + Copy,
+    {
+        walkdir::WalkDir::new(&self.storage_dir)
+            .into_iter()
+            .filter_map(|e| e.ok())
+            .map(|e| e.path().to_path_buf())
+            .filter(move |path| f(path))
+    }
+
+    pub fn resource_paths(&self) -> impl Iterator<Item = PathBuf> + '_ {
+        self.iter_paths(move |p| self.is_resource_path(p))
+    }
+
+    pub fn module_paths(&self) -> impl Iterator<Item = PathBuf> + '_ {
+        self.iter_paths(move |p| self.is_module_path(p))
+    }
+
+    pub fn event_paths(&self) -> impl Iterator<Item = PathBuf> + '_ {
+        self.iter_paths(move |p| self.is_event_path(p))
+    }
+
+    /// Return a map of module ID -> module for all modules in the self.storage_dir.
+    /// Returns an Err if a module does not deserialize
+    pub fn get_all_modules(&self) -> Result<BTreeMap<ModuleId, CompiledModule>> {
+        let mut modules = BTreeMap::new();
+        for path in self.module_paths() {
+            let module = CompiledModule::deserialize(&Self::get_bytes(&path)?.unwrap())
+                .map_err(|e| anyhow!("Failed to deserialized module: {:?}", e))?;
+            let id = module.self_id();
+            if modules.insert(id.clone(), module).is_some() {
+                bail!("Duplicate module {:?}", id)
+            }
+        }
+        Ok(modules)
+    }
 }
 
 impl RemoteCache for OnDiskStateView {

language/tools/move-cli/src/main.rs

@@ -153,6 +153,9 @@ enum Command {
     /// Delete all resources, events, and modules stored on disk under `storage`.
     /// Does *not* delete anything in `src`.
     Clean {},
+    /// Run well-formedness checks on the `storage` and `build` directories.
+    #[structopt(name = "doctor")]
+    Doctor {},
 }
 
 impl Move {
@@ -772,6 +775,47 @@ fn view(args: &Move, file: &str) -> Result<()> {
     Ok(())
 }
 
+/// Run sanity checks on storage and build dirs. This is primarily intended for testing the CLI;
+/// doctor should never fail unless `publish --ignore-breaking changes` is used or files under
+/// `storage` or `build` are modified manually. This runs the following checks:
+/// (1) all modules pass the bytecode verifier
+/// (2) all modules pass the linker
+/// (3) all resources can be deserialized
+/// (4) all events can be deserialized (TODO)
+/// (5) build/mv_interfaces is consistent with the global storage (TODO)
+fn doctor(args: &Move) -> Result<()> {
+    let storage_dir = maybe_create_dir(&args.storage_dir)?.canonicalize()?;
+    let stdlib_modules = vec![];
+    let state = OnDiskStateView::create(storage_dir, &stdlib_modules)?;
+    let modules = state.get_all_modules()?;
+    // verify and link each module
+    for module in modules.values() {
+        if bytecode_verifier::verify_module(module).is_err() {
+            bail!("Failed to verify module {:?}", module.self_id())
+        }
+        if bytecode_verifier::DependencyChecker::verify_module(module, modules.values()).is_err() {
+            bail!(
+                "Failed to link module {:?} against its dependencies",
+                module.self_id()
+            )
+        }
+    }
+    // deserialize each resource
+    for resource_path in state.resource_paths() {
+        let resource = state.view_resource(&resource_path);
+        if resource.is_err() {
+            let parent_addr = resource_path.parent().unwrap().parent().unwrap();
+            bail!(
+                "Failed to deserialize resource {:?} stored under address {:?}",
+                resource_path.file_name().unwrap(),
+                parent_addr.file_name().unwrap()
+            )
+        }
+    }
+
+    Ok(())
+}
+
 fn main() -> Result<()> {
     let move_args = Move::from_args();
 
@@ -836,5 +880,6 @@ fn main() -> Result<()> {
             }
             Ok(())
         }
+        Command::Doctor {} => doctor(&move_args),
     }
 }

language/tools/move-cli/tests/testsuite/data_breaking_change/args.exp

@@ -2,4 +2,8 @@ Command `publish src/modules/`:
 Command `publish M.move`:
 Breaking change detected--publishing aborted. Re-run with --ignore-breaking-changes to publish anyway.
 Error: Layout API for structs of module 00000000000000000000000000000002::M has changed. Need to do a data migration of published structs
+Command `run src/scripts/publish.move --signers 0xA`:
+Command `doctor`:
 Command `publish --ignore-breaking-changes M.move`:
+Command `doctor`:
+Error: Failed to deserialize resource "0x00000000000000000000000000000002::M::R.bcs" stored under address "0x0000000000000000000000000000000A"

language/tools/move-cli/tests/testsuite/data_breaking_change/args.txt

@@ -1,3 +1,6 @@
 publish src/modules/
 publish M.move
+run src/scripts/publish.move --signers 0xA
+doctor
 publish --ignore-breaking-changes M.move
+doctor

language/tools/move-cli/tests/testsuite/data_breaking_change/src/modules/M.move

@@ -1,5 +1,9 @@
 address 0x2 {
 module M {
     resource struct R { f: u64 }
+
+    public fun publish(account: &signer) {
+        move_to(account, R { f: 10 })
+    }
 }
 }

language/tools/move-cli/tests/testsuite/data_breaking_change/src/scripts/publish.move

@@ -0,0 +1,6 @@
+script {
+use 0x2::M;
+fun publish(account: &signer) {
+    M::publish(account)
+}
+}

language/tools/move-cli/tests/testsuite/linking_breaking_change/args.exp

@@ -2,4 +2,7 @@ Command `publish src/modules/`:
 Command `publish M.move`:
 Breaking change detected--publishing aborted. Re-run with --ignore-breaking-changes to publish anyway.
 Error: Linking API for structs/functions of module 00000000000000000000000000000002::M has changed. Need to redeploy all dependent modules.
+Command `doctor`:
 Command `publish --ignore-breaking-changes M.move`:
+Command `doctor`:
+Error: Failed to link module ModuleId { address: 00000000000000000000000000000002, name: Identifier("M2") } against its dependencies

language/tools/move-cli/tests/testsuite/linking_breaking_change/args.txt

@@ -1,3 +1,5 @@
 publish src/modules/
 publish M.move
+doctor
 publish --ignore-breaking-changes M.move
+doctor

language/tools/move-cli/tests/testsuite/linking_breaking_change/src/modules/M2.move

@@ -0,0 +1,10 @@
+address 0x2 {
+module M2 {
+  use 0x2::M;
+
+  public fun h() {
+      M::f();
+      M::g();
+  }
+}
+}