[cli] fix errors caused by mishandling of bytecode version (#5900)

aptos-labs · Dec 18, 2022 · 0ef3bf2 · 0ef3bf2
1 parent 2bab2aa
commit 0ef3bf2
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 3 deletions.
diff --git a/aptos-move/framework/src/aptos.rs b/aptos-move/framework/src/aptos.rs
@@ -115,6 +115,7 @@ impl ReleaseTarget {
                     references_file: Some("doc_template/references.md".to_string()),
                 }),
                 skip_fetch_latest_git_deps: false,
+                bytecode_version: None,
             },
             packages: packages.iter().map(|(path, _)| path.to_owned()).collect(),
             rust_bindings: packages

diff --git a/aptos-move/framework/src/built_package.rs b/aptos-move/framework/src/built_package.rs
@@ -57,6 +57,8 @@ pub struct BuildOptions {
     pub docgen_options: Option<DocgenOptions>,
     #[clap(long)]
     pub skip_fetch_latest_git_deps: bool,
+    #[clap(long)]
+    pub bytecode_version: Option<u32>,
 }
 
 // Because named_addresses has no parser, we can't use clap's default impl. This must be aligned
@@ -75,6 +77,7 @@ impl Default for BuildOptions {
             // This is false by default, because it could accidentally pull new dependencies
             // while in a test (and cause some havoc)
             skip_fetch_latest_git_deps: false,
+            bytecode_version: None,
         }
     }
 }
@@ -155,6 +158,7 @@ impl BuiltPackage {
                 .join(package.compiled_package_info.package_name.as_str()),
             &mut package,
             runtime_metadata,
+            options.bytecode_version,
         )?;
 
         // If enabled generate docs.
@@ -208,7 +212,11 @@ impl BuiltPackage {
     pub fn extract_code(&self) -> Vec<Vec<u8>> {
         self.package
             .root_modules()
-            .map(|unit_with_source| unit_with_source.unit.serialize(None))
+            .map(|unit_with_source| {
+                unit_with_source
+                    .unit
+                    .serialize(self.options.bytecode_version)
+            })
             .collect()
     }
 
@@ -240,7 +248,11 @@ impl BuiltPackage {
     pub fn extract_script_code(&self) -> Vec<Vec<u8>> {
         self.package
             .scripts()
-            .map(|unit_with_source| unit_with_source.unit.serialize(None))
+            .map(|unit_with_source| {
+                unit_with_source
+                    .unit
+                    .serialize(self.options.bytecode_version)
+            })
             .collect()
     }
 
@@ -334,6 +346,7 @@ fn inject_runtime_metadata(
     package_path: PathBuf,
     pack: &mut CompiledPackage,
     metadata: BTreeMap<ModuleId, RuntimeModuleMetadataV1>,
+    bytecode_version: Option<u32>,
 ) -> anyhow::Result<()> {
     for unit_with_source in pack.root_compiled_units.iter_mut() {
         match &mut unit_with_source.unit {
@@ -353,7 +366,7 @@ fn inject_runtime_metadata(
                             .join(named_module.name.as_str())
                             .with_extension(MOVE_COMPILED_EXTENSION);
                         if path.is_file() {
-                            let bytes = unit_with_source.unit.serialize(None);
+                            let bytes = unit_with_source.unit.serialize(bytecode_version);
                             std::fs::write(path, &bytes)?;
                         }
                     }

diff --git a/crates/aptos/src/common/types.rs b/crates/aptos/src/common/types.rs
@@ -949,6 +949,10 @@ impl MovePackageDir {
             .collect()
     }
 
+    pub fn bytecode_version_or_detault(&self) -> u32 {
+        self.bytecode_version.unwrap_or(5)
+    }
+
     pub fn add_named_address(&mut self, key: String, value: String) {
         self.named_addresses
             .insert(key, AccountAddressWrapper::from_str(&value).unwrap());

diff --git a/crates/aptos/src/governance/mod.rs b/crates/aptos/src/governance/mod.rs
@@ -552,6 +552,7 @@ fn compile_in_temp_dir(
     script_path: &Path,
     framework_package_args: &FrameworkPackageArgs,
     prompt_options: PromptOptions,
+    bytecode_version: u32,
 ) -> CliTypedResult<(Vec<u8>, HashValue)> {
     // Make a temporary directory for compilation
     let temp_dir = TempDir::new().map_err(|err| {
@@ -590,19 +591,22 @@ fn compile_in_temp_dir(
     compile_script(
         framework_package_args.skip_fetch_latest_git_deps,
         package_dir,
+        bytecode_version,
     )
 }
 
 fn compile_script(
     skip_fetch_latest_git_deps: bool,
     package_dir: &Path,
+    bytecode_version: u32,
 ) -> CliTypedResult<(Vec<u8>, HashValue)> {
     let build_options = BuildOptions {
         with_srcs: false,
         with_abis: false,
         with_source_maps: false,
         with_error_map: false,
         skip_fetch_latest_git_deps,
+        bytecode_version: Some(bytecode_version),
         ..BuildOptions::default()
     };
 
@@ -671,6 +675,9 @@ pub struct CompileScriptFunction {
 
     #[clap(flatten)]
     pub(crate) framework_package_args: FrameworkPackageArgs,
+
+    #[clap(long)]
+    pub(crate) bytecode_version: Option<u32>,
 }
 
 impl CompileScriptFunction {
@@ -715,6 +722,7 @@ impl CompileScriptFunction {
             script_path,
             &self.framework_package_args,
             prompt_options,
+            self.bytecode_version.unwrap_or(5),
         )
     }
 }
@@ -770,6 +778,7 @@ impl CliCommand<()> for GenerateUpgradeProposal {
         let options = included_artifacts.build_options(
             move_options.skip_fetch_latest_git_deps,
             move_options.named_addresses(),
+            move_options.bytecode_version_or_detault(),
         );
         let package = BuiltPackage::build(package_path, options)?;
         let release = ReleasePackage::new(package)?;

diff --git a/crates/aptos/src/move_tool/mod.rs b/crates/aptos/src/move_tool/mod.rs
@@ -304,6 +304,7 @@ impl CliCommand<Vec<String>> for CompilePackage {
                 .build_options(
                     self.move_options.skip_fetch_latest_git_deps,
                     self.move_options.named_addresses(),
+                    self.move_options.bytecode_version_or_detault(),
                 )
         };
         let pack = BuiltPackage::build(self.move_options.get_package_path()?, build_options)
@@ -481,6 +482,7 @@ impl CliCommand<&'static str> for DocumentPackage {
             named_addresses: move_options.named_addresses(),
             docgen_options: Some(docgen_options),
             skip_fetch_latest_git_deps: move_options.skip_fetch_latest_git_deps,
+            bytecode_version: Some(move_options.bytecode_version_or_detault()),
         };
         BuiltPackage::build(move_options.get_package_path()?, build_options)?;
         Ok("succeeded")
@@ -554,6 +556,7 @@ impl IncludedArtifacts {
         self,
         skip_fetch_latest_git_deps: bool,
         named_addresses: BTreeMap<String, AccountAddress>,
+        bytecode_version: u32,
     ) -> BuildOptions {
         use IncludedArtifacts::*;
         match self {
@@ -565,6 +568,7 @@ impl IncludedArtifacts {
                 with_error_map: true,
                 named_addresses,
                 skip_fetch_latest_git_deps,
+                bytecode_version: Some(bytecode_version),
                 ..BuildOptions::default()
             },
             Sparse => BuildOptions {
@@ -574,6 +578,7 @@ impl IncludedArtifacts {
                 with_error_map: true,
                 named_addresses,
                 skip_fetch_latest_git_deps,
+                bytecode_version: Some(bytecode_version),
                 ..BuildOptions::default()
             },
             All => BuildOptions {
@@ -583,6 +588,7 @@ impl IncludedArtifacts {
                 with_error_map: true,
                 named_addresses,
                 skip_fetch_latest_git_deps,
+                bytecode_version: Some(bytecode_version),
                 ..BuildOptions::default()
             },
         }
@@ -609,6 +615,7 @@ impl CliCommand<TransactionSummary> for PublishPackage {
         let options = included_artifacts_args.included_artifacts.build_options(
             move_options.skip_fetch_latest_git_deps,
             move_options.named_addresses(),
+            move_options.bytecode_version_or_detault(),
         );
         let package = BuiltPackage::build(package_path, options)?;
         let compiled_units = package.extract_code();
@@ -695,6 +702,7 @@ impl CliCommand<TransactionSummary> for CreateResourceAccountAndPublishPackage {
         let options = included_artifacts_args.included_artifacts.build_options(
             move_options.skip_fetch_latest_git_deps,
             move_options.named_addresses(),
+            move_options.bytecode_version_or_detault(),
         );
         let package = BuiltPackage::build(package_path, options)?;
         let compiled_units = package.extract_code();
@@ -819,9 +827,11 @@ impl CliCommand<&'static str> for VerifyPackage {
         // First build the package locally to get the package metadata
         let build_options = BuildOptions {
             install_dir: self.move_options.output_dir.clone(),
+            bytecode_version: Some(self.move_options.bytecode_version_or_detault()),
             ..self.included_artifacts.build_options(
                 self.move_options.skip_fetch_latest_git_deps,
                 self.move_options.named_addresses(),
+                self.move_options.bytecode_version_or_detault(),
             )
         };
         let pack = BuiltPackage::build(self.move_options.get_package_path()?, build_options)

diff --git a/crates/aptos/src/test/mod.rs b/crates/aptos/src/test/mod.rs
@@ -928,6 +928,7 @@ impl CliTestFramework {
                     framework_local_dir: Some(Self::aptos_framework_dir()),
                     skip_fetch_latest_git_deps: false,
                 },
+                bytecode_version: None,
             },
             args: Vec::new(),
             type_args: Vec::new(),