[Bugfix] Fix broken bytecode dependency (aptos-labs#14929)

* Add bytecode_deps to CompiledPackage

* fmt

* error message

* fixed issue

* fixed issue

* refactor

* bug fix

* resolve merge conflicts

* lint

* resolve comments

* linter

* config

* sort

---------

Co-authored-by: Hao Xia <hao@shinami.com>
Co-authored-by: Zekun Wang <zekun.wang@aptoslabs.com>

Author: 3 people

aptos-move/aptos-vm/Cargo.toml

@@ -63,6 +63,7 @@ aptos-aggregator = { workspace = true, features = ["testing"] }
 aptos-language-e2e-tests = { workspace = true }
 aptos-types = { workspace = true, features = ["fuzzing"] }
 claims = { workspace = true }
+move-vm-types = { workspace = true, features = ["testing"] }
 proptest = { workspace = true }
 rand_core = { workspace = true }
 

aptos-move/framework/src/built_package.rs

@@ -483,6 +483,15 @@ impl BuiltPackage {
                     package_name,
                 }
             })
+            .chain(
+                self.package
+                    .bytecode_deps
+                    .iter()
+                    .map(|(name, address)| PackageDep {
+                        account: address.into_inner(),
+                        package_name: name.as_str().to_string(),
+                    }),
+            )
             .collect::<BTreeSet<_>>()
             .into_iter()
             .collect();

third_party/move/tools/move-package/src/compilation/compiled_package.rs

@@ -11,7 +11,7 @@ use crate::{
     },
     Architecture, BuildConfig, CompilerConfig, CompilerVersion,
 };
-use anyhow::{bail, ensure, Result};
+use anyhow::{bail, ensure, Context, Result};
 use colored::Colorize;
 use itertools::{Either, Itertools};
 use move_abigen::{Abigen, AbigenOptions};
@@ -81,6 +81,8 @@ pub struct CompiledPackage {
     pub root_compiled_units: Vec<CompiledUnitWithSource>,
     /// The output compiled bytecode for dependencies
     pub deps_compiled_units: Vec<(PackageName, CompiledUnitWithSource)>,
+    /// Bytecode dependencies of this compiled package
+    pub bytecode_deps: BTreeMap<PackageName, NumericalAddress>,
 
     // Optional artifacts from compilation
     //
@@ -100,6 +102,7 @@ pub struct OnDiskPackage {
     pub compiled_package_info: CompiledPackageInfo,
     /// Dependency names for this package.
     pub dependencies: Vec<PackageName>,
+    pub bytecode_deps: Vec<PackageName>,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -198,6 +201,8 @@ impl OnDiskCompiledPackage {
             compiled_package_info: self.package.compiled_package_info.clone(),
             root_compiled_units,
             deps_compiled_units,
+            // TODO: support bytecode deps
+            bytecode_deps: BTreeMap::new(),
             compiled_docs,
             compiled_abis,
         })
@@ -631,72 +636,75 @@ impl CompiledPackage {
         let effective_language_version = config.language_version.unwrap_or_default();
         effective_compiler_version.check_language_support(effective_language_version)?;
 
-        let (file_map, all_compiled_units, optional_global_env) = match config
-            .compiler_version
-            .unwrap_or_default()
-        {
-            CompilerVersion::V1 => {
-                let mut paths = src_deps;
-                paths.push(sources_package_paths.clone());
-                let compiler =
-                    Compiler::from_package_paths(paths, bytecode_deps, flags, &known_attributes);
-                compiler_driver_v1(compiler)?
-            },
-            version @ CompilerVersion::V2_0 | version @ CompilerVersion::V2_1 => {
-                let to_str_vec = |ps: &[Symbol]| {
-                    ps.iter()
-                        .map(move |s| s.as_str().to_owned())
-                        .collect::<Vec<_>>()
-                };
-                let mut global_address_map = BTreeMap::new();
-                for pack in std::iter::once(&sources_package_paths)
-                    .chain(src_deps.iter())
-                    .chain(bytecode_deps.iter())
-                {
-                    for (name, val) in &pack.named_address_map {
-                        if let Some(old) = global_address_map.insert(name.as_str().to_owned(), *val)
-                        {
-                            if old != *val {
-                                let pack_name = pack
-                                    .name
-                                    .map(|s| s.as_str().to_owned())
-                                    .unwrap_or_else(|| "<unnamed>".to_owned());
-                                bail!(
+        let (file_map, all_compiled_units, optional_global_env) =
+            match config.compiler_version.unwrap_or_default() {
+                CompilerVersion::V1 => {
+                    let mut paths = src_deps;
+                    paths.push(sources_package_paths.clone());
+                    let compiler = Compiler::from_package_paths(
+                        paths,
+                        bytecode_deps.clone(),
+                        flags,
+                        &known_attributes,
+                    );
+                    compiler_driver_v1(compiler)?
+                },
+                version @ CompilerVersion::V2_0 | version @ CompilerVersion::V2_1 => {
+                    let to_str_vec = |ps: &[Symbol]| {
+                        ps.iter()
+                            .map(move |s| s.as_str().to_owned())
+                            .collect::<Vec<_>>()
+                    };
+                    let mut global_address_map = BTreeMap::new();
+                    for pack in std::iter::once(&sources_package_paths)
+                        .chain(src_deps.iter())
+                        .chain(bytecode_deps.iter())
+                    {
+                        for (name, val) in &pack.named_address_map {
+                            if let Some(old) =
+                                global_address_map.insert(name.as_str().to_owned(), *val)
+                            {
+                                if old != *val {
+                                    let pack_name = pack
+                                        .name
+                                        .map(|s| s.as_str().to_owned())
+                                        .unwrap_or_else(|| "<unnamed>".to_owned());
+                                    bail!(
                                     "found remapped address alias `{}` (`{} != {}`) in package `{}`\
                                     , please use unique address aliases across dependencies",
                                     name, old, val, pack_name
                                 )
+                                }
                             }
                         }
                     }
-                }
-                let mut options = move_compiler_v2::Options {
-                    sources: sources_package_paths
-                        .paths
-                        .iter()
-                        .map(|path| path.as_str().to_owned())
-                        .collect(),
-                    sources_deps: src_deps.iter().flat_map(|x| to_str_vec(&x.paths)).collect(),
-                    dependencies: bytecode_deps
-                        .iter()
-                        .flat_map(|x| to_str_vec(&x.paths))
-                        .collect(),
-                    named_address_mapping: global_address_map
-                        .into_iter()
-                        .map(|(k, v)| format!("{}={}", k, v))
-                        .collect(),
-                    skip_attribute_checks,
-                    known_attributes: known_attributes.clone(),
-                    language_version: Some(effective_language_version),
-                    compiler_version: Some(version),
-                    compile_test_code: flags.keep_testing_functions(),
-                    experiments: config.experiments.clone(),
-                    ..Default::default()
-                };
-                options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true);
-                compiler_driver_v2(options)?
-            },
-        };
+                    let mut options = move_compiler_v2::Options {
+                        sources: sources_package_paths
+                            .paths
+                            .iter()
+                            .map(|path| path.as_str().to_owned())
+                            .collect(),
+                        sources_deps: src_deps.iter().flat_map(|x| to_str_vec(&x.paths)).collect(),
+                        dependencies: bytecode_deps
+                            .iter()
+                            .flat_map(|x| to_str_vec(&x.paths))
+                            .collect(),
+                        named_address_mapping: global_address_map
+                            .into_iter()
+                            .map(|(k, v)| format!("{}={}", k, v))
+                            .collect(),
+                        skip_attribute_checks,
+                        known_attributes: known_attributes.clone(),
+                        language_version: Some(effective_language_version),
+                        compiler_version: Some(version),
+                        compile_test_code: flags.keep_testing_functions(),
+                        experiments: config.experiments.clone(),
+                        ..Default::default()
+                    };
+                    options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true);
+                    compiler_driver_v2(options)?
+                },
+            };
         let mut root_compiled_units = vec![];
         let mut deps_compiled_units = vec![];
         let obtain_package_name =
@@ -794,14 +802,24 @@ impl CompiledPackage {
         };
 
         let compiled_package = CompiledPackage {
+            root_compiled_units,
+            deps_compiled_units,
+            bytecode_deps: bytecode_deps
+                .iter()
+                .flat_map(|package| {
+                    let name = package.name.unwrap();
+                    package.paths.iter().map(move |pkg_path| {
+                        get_addr_from_module_in_package(name, pkg_path.as_str())
+                            .map(|addr| (name, addr))
+                    })
+                })
+                .try_collect()?,
             compiled_package_info: CompiledPackageInfo {
                 package_name: resolved_package.source_package.package.name,
                 address_alias_instantiation: resolved_package.resolution_table,
                 source_digest: Some(resolved_package.source_digest),
                 build_flags: resolution_graph.build_options.clone(),
             },
-            root_compiled_units,
-            deps_compiled_units,
             compiled_docs,
             compiled_abis,
         };
@@ -888,6 +906,13 @@ impl CompiledPackage {
                     .collect::<BTreeSet<_>>()
                     .into_iter()
                     .collect(),
+                bytecode_deps: self
+                    .bytecode_deps
+                    .keys()
+                    .copied()
+                    .collect::<BTreeSet<_>>()
+                    .into_iter()
+                    .collect(),
             },
         };
 
@@ -1138,3 +1163,23 @@ pub fn build_and_report_no_exit_v2_driver(
         Some(env),
     ))
 }
+
+/// Returns the address of the module
+fn get_addr_from_module_in_package(pkg_name: Symbol, pkg_path: &str) -> Result<NumericalAddress> {
+    // Read the bytecode file
+    let mut bytecode = Vec::new();
+    std::fs::File::open(pkg_path)
+        .context(format!("Failed to open bytecode file for {}", pkg_path))
+        .and_then(|mut file| {
+            // read contents of the file into bytecode
+            std::io::Read::read_to_end(&mut file, &mut bytecode)
+                .context(format!("Failed to read bytecode file {}", pkg_path))
+        })
+        .and_then(|_| {
+            CompiledModule::deserialize(&bytecode).context(format!(
+                "Failed to deserialize bytecode file for {}",
+                pkg_name
+            ))
+        })
+        .map(|module| NumericalAddress::from_account_address(*module.self_addr()))
+}