source validation: emit multiple errors (MystenLabs#7971)

crates/sui-source-validation/fixture/c/Move.toml

@@ -0,0 +1,3 @@
+[package]
+name = "c"
+version = "0.0.1"

crates/sui-source-validation/fixture/c/sources/b.move

@@ -0,0 +1,8 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module c::b {
+    public fun b(): u64 {
+        42
+    }
+}

crates/sui-source-validation/fixture/c/sources/c.move

@@ -0,0 +1,8 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module c::c {
+    public fun c(): u64 {
+        43
+    }
+}

crates/sui-source-validation/fixture/c/sources/d.move

@@ -0,0 +1,8 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module c::d {
+    public fun d(): u64 {
+        44
+    }
+}

crates/sui-source-validation/fixture/d/Move.toml

@@ -0,0 +1,7 @@
+[package]
+name = "d"
+version = "0.0.1"
+
+[dependencies]
+b = { local = "../b" }
+c = { local = "../c" }

crates/sui-source-validation/fixture/d/sources/a.move

@@ -0,0 +1,13 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module d::d {
+    use b::b::b;
+    use c::c::c;
+
+    public fun d(): u64 {
+        let var = 123;
+        let _ = var + 4;
+        b() + c()
+    }
+}

crates/sui-source-validation/src/lib.rs

@@ -1,6 +1,7 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use core::fmt;
 use futures::future;
 use move_binary_format::access::ModuleAccess;
 use move_binary_format::CompiledModule;
@@ -56,6 +57,33 @@ pub enum SourceVerificationError {
     InvalidModuleFailure { name: String, message: String },
 }
 
+#[derive(Debug, Error)]
+pub struct AggregateSourceVerificationError(Vec<SourceVerificationError>);
+
+impl From<SourceVerificationError> for AggregateSourceVerificationError {
+    fn from(error: SourceVerificationError) -> Self {
+        AggregateSourceVerificationError(vec![error])
+    }
+}
+
+impl fmt::Display for AggregateSourceVerificationError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let AggregateSourceVerificationError(errors) = self;
+        match &errors[..] {
+            [] => unreachable!("Aggregate error with no errors"),
+            [error] => write!(f, "{}", error)?,
+            errors => {
+                writeln!(f, "Multiple source verification errors found:")?;
+                for error in errors {
+                    write!(f, "\n- {}", error)?;
+                }
+                return Ok(());
+            }
+        };
+        Ok(())
+    }
+}
+
 /// How to handle package source during bytecode verification.
 #[derive(PartialEq, Eq)]
 pub enum SourceMode {
@@ -94,7 +122,7 @@ impl<'a> BytecodeSourceVerifier<'a> {
         &self,
         compiled_package: &CompiledPackage,
         root_on_chain_address: AccountAddress,
-    ) -> Result<(), SourceVerificationError> {
+    ) -> Result<(), AggregateSourceVerificationError> {
         self.verify_package(
             compiled_package,
             /* verify_deps */ true,
@@ -109,7 +137,7 @@ impl<'a> BytecodeSourceVerifier<'a> {
         &self,
         compiled_package: &CompiledPackage,
         root_on_chain_address: AccountAddress,
-    ) -> Result<(), SourceVerificationError> {
+    ) -> Result<(), AggregateSourceVerificationError> {
         self.verify_package(
             compiled_package,
             /* verify_deps */ false,
@@ -123,7 +151,7 @@ impl<'a> BytecodeSourceVerifier<'a> {
     pub async fn verify_package_deps(
         &self,
         compiled_package: &CompiledPackage,
-    ) -> Result<(), SourceVerificationError> {
+    ) -> Result<(), AggregateSourceVerificationError> {
         self.verify_package(
             compiled_package,
             /* verify_deps */ true,
@@ -141,11 +169,11 @@ impl<'a> BytecodeSourceVerifier<'a> {
         compiled_package: &CompiledPackage,
         verify_deps: bool,
         source_mode: SourceMode,
-    ) -> Result<(), SourceVerificationError> {
+    ) -> Result<(), AggregateSourceVerificationError> {
         // On-chain address for matching root package cannot be zero
         if let SourceMode::VerifyAt(root_address) = &source_mode {
             if *root_address == AccountAddress::ZERO {
-                return Err(SourceVerificationError::ZeroOnChainAddresSpecifiedFailure);
+                return Err(SourceVerificationError::ZeroOnChainAddresSpecifiedFailure.into());
             }
         }
 
@@ -154,17 +182,19 @@ impl<'a> BytecodeSourceVerifier<'a> {
             .on_chain_bytes(local_modules.keys().map(|(addr, _)| *addr))
             .await?;
 
+        let mut errors = Vec::new();
         for ((address, module), (package, local_bytes)) in local_modules {
             let Some(on_chain_bytes) = on_chain_modules.remove(&(address, module)) else {
-                return Err(SourceVerificationError::OnChainDependencyNotFound {
+                errors.push(SourceVerificationError::OnChainDependencyNotFound {
                     package, module,
-                })
+                });
+		continue;
             };
 
             // compare local bytecode to on-chain bytecode to ensure integrity of our
             // dependencies
             if local_bytes != on_chain_bytes {
-                return Err(SourceVerificationError::ModuleBytecodeMismatch {
+                errors.push(SourceVerificationError::ModuleBytecodeMismatch {
                     address,
                     package,
                     module,
@@ -182,7 +212,11 @@ impl<'a> BytecodeSourceVerifier<'a> {
         }
 
         if let Some(((address, module), _)) = on_chain_modules.into_iter().next() {
-            return Err(SourceVerificationError::LocalDependencyNotFound { address, module });
+            errors.push(SourceVerificationError::LocalDependencyNotFound { address, module });
+        }
+
+        if !errors.is_empty() {
+            return Err(AggregateSourceVerificationError(errors));
         }
 
         Ok(())

crates/sui-source-validation/src/tests.rs

@@ -453,6 +453,66 @@ async fn module_bytecode_mismatch() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn multiple_failures() -> anyhow::Result<()> {
+    let mut cluster = TestClusterBuilder::new().build().await?;
+    let sender = cluster.get_address_0();
+    let context = &mut cluster.wallet;
+    let mut stable_addrs = HashMap::new();
+
+    // Publish package `b::b` on-chain without c.move.
+    let b_ref = {
+        let fixtures = tempfile::tempdir()?;
+        let b_src = copy_package(&fixtures, "b", [("b", SuiAddress::ZERO)]).await?;
+        tokio::fs::remove_file(b_src.join("sources").join("c.move")).await?;
+        publish_package(context, sender, b_src).await
+    };
+
+    // Publish package `c::c` on-chain, unmodified.
+    let c_ref = {
+        let fixtures = tempfile::tempdir()?;
+        let c_src = copy_package(&fixtures, "c", [("c", SuiAddress::ZERO)]).await?;
+        publish_package(context, sender, c_src).await
+    };
+
+    // Compile local package `d` that references:
+    // - `b::b` (c.move exists locally but not on chain => error)
+    // - `c::c` (d.move exists on-chain but we delete it locally before compiling => error)
+    let d_pkg = {
+        let fixtures = tempfile::tempdir()?;
+        let b_id = b_ref.0.into();
+        let c_id = c_ref.0.into();
+        stable_addrs.insert(b_id, "<b_id>");
+        stable_addrs.insert(c_id, "<c_id>");
+        copy_package(&fixtures, "b", [("b", b_id)]).await?;
+        let c_src = copy_package(&fixtures, "c", [("c", c_id)]).await?;
+        let d_src = copy_package(
+            &fixtures,
+            "d",
+            [("d", SuiAddress::ZERO), ("b", b_id), ("c", c_id)],
+        )
+        .await?;
+        tokio::fs::remove_file(c_src.join("sources").join("d.move")).await?; // delete local module in `c`
+        compile_package(d_src)
+    };
+
+    let client = context.get_client().await?;
+    let verifier = BytecodeSourceVerifier::new(client.read_api(), false);
+
+    let Err(err) = verifier.verify_package_deps(&d_pkg.package).await else {
+        panic!("Expected verification to fail");
+    };
+
+    let expected = expect![[r#"
+        Multiple source verification errors found:
+
+        - On-chain version of dependency b::c was not found.
+        - Local version of dependency <c_id>::d was not found."#]];
+    expected.assert_eq(&sanitize_id(err.to_string(), &stable_addrs));
+
+    Ok(())
+}
+
 /// Compile the package at absolute path `package`.
 fn compile_package(package: impl AsRef<Path>) -> CompiledPackage {
     sui_framework::build_move_package(package.as_ref(), BuildConfig::new_for_testing()).unwrap()