move: dump-bytecode-as-base64 uses Move.lock addresses (#18794)

## Description `sui move build` does not ordinarily require a network connection because it doesn't need to know about a chain's ID. The exception is when `--dump-bytecode-as-base64` is specified: In this case, we should resolve the correct addresses for the respective chain (e.g., testnet, mainnet) from the `Move.lock` under automated address management. Two options to fix `sui move build --dump-bytecode-as-base64` to work with automated addresses / by resolving from the `Move.lock`: 1. Require an extra `--chain-id` flag on `sui move build`, which a user must specify along with `--dump-bytecode-as-base64`. E.g., ``` sui move build --dump-bytecode-as-base64 --chain-id "$(sui client chain-identifier)" ``` OR 2. Require a network connection _only when_ `--dump-bytecode-as-base64` is set and and resolve the chain-id without requiring a flag. This PR opts for **(2)**, but it is trivial to change the implementation and test to do **(1)** instead. **(1)** should come with an accompanying doc change though. Context: encountered when running, e.g., ``` execSync(`${SUI} move build --dump-bytecode-as-base64 --path ${packagePath}` ``` ## Test plan Added test --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [x] CLI: Fixed an issue where `--dump-bytecode-as-base64` did not work as expected if [package addresses are automatically managed](https://docs.sui.io/concepts/sui-move-concepts/packages/automated-address-management#adopting-automated-address-management-for-published-packages). - [ ] Rust SDK: - [ ] REST API:
MystenLabs · Jul 27, 2024 · 2501733 · 2501733
1 parent db844c3
commit 2501733
Show file tree

Hide file tree

Showing 10 changed files with 140 additions and 11 deletions.
diff --git a/crates/sui-move/src/build.rs b/crates/sui-move/src/build.rs
@@ -29,6 +29,11 @@ pub struct Build {
     /// and events.
     #[clap(long, global = true)]
     pub generate_struct_layouts: bool,
+    /// The chain ID, if resolved. Required when the dump_bytecode_as_base64 is true,
+    /// for automated address management, where package addresses are resolved for the
+    /// respective chain in the Move.lock file.
+    #[clap(skip)]
+    pub chain_id: Option<String>,
 }
 
 impl Build {
@@ -45,6 +50,7 @@ impl Build {
             self.with_unpublished_dependencies,
             self.dump_bytecode_as_base64,
             self.generate_struct_layouts,
+            self.chain_id.clone(),
         )
     }
 
@@ -54,12 +60,13 @@ impl Build {
         with_unpublished_deps: bool,
         dump_bytecode_as_base64: bool,
         generate_struct_layouts: bool,
+        chain_id: Option<String>,
     ) -> anyhow::Result<()> {
         let pkg = BuildConfig {
             config,
             run_bytecode_verifier: true,
             print_diags_to_stderr: true,
-            chain_id: None,
+            chain_id,
         }
         .build(rerooted_path)?;
         if dump_bytecode_as_base64 {

diff --git a/crates/sui/src/sui_commands.rs b/crates/sui/src/sui_commands.rs
@@ -287,6 +287,10 @@ pub enum SuiCommand {
         /// Path to a package which the command should be run with respect to.
         #[clap(long = "path", short = 'p', global = true)]
         package_path: Option<PathBuf>,
+        /// Sets the file storing the state of our user accounts (an empty one will be created if missing)
+        /// Only used when the `--dump-bytecode-as-base64` is set.
+        #[clap(long = "client.config")]
+        config: Option<PathBuf>,
         /// Package build options
         #[clap(flatten)]
         build_config: BuildConfig,
@@ -448,8 +452,27 @@ impl SuiCommand {
             SuiCommand::Move {
                 package_path,
                 build_config,
-                cmd,
-            } => execute_move_command(package_path.as_deref(), build_config, cmd),
+                mut cmd,
+                config: client_config,
+            } => {
+                match &mut cmd {
+                    sui_move::Command::Build(build) if build.dump_bytecode_as_base64 => {
+                        // `sui move build` does not ordinarily require a network connection.
+                        // The exception is when --dump-bytecode-as-base64 is specified: In this
+                        // case, we should resolve the correct addresses for the respective chain
+                        // (e.g., testnet, mainnet) from the Move.lock under automated address management.
+                        let config =
+                            client_config.unwrap_or(sui_config_dir()?.join(SUI_CLIENT_CONFIG));
+                        prompt_if_no_config(&config, false).await?;
+                        let context = WalletContext::new(&config, None, None)?;
+                        let client = context.get_client().await?;
+                        let chain_id = client.read_api().get_chain_identifier().await.ok();
+                        build.chain_id = chain_id.clone();
+                    }
+                    _ => (),
+                };
+                execute_move_command(package_path.as_deref(), build_config, cmd)
+            }
             SuiCommand::BridgeInitialize {
                 network_config,
                 client_config,

diff --git a/crates/sui/tests/cli_tests.rs b/crates/sui/tests/cli_tests.rs
@@ -7,6 +7,7 @@ use std::net::SocketAddr;
 use std::os::unix::prelude::FileExt;
 use std::{fmt::Write, fs::read_dir, path::PathBuf, str, thread, time::Duration};
 
+use std::env;
 #[cfg(not(msim))]
 use std::str::FromStr;
 
@@ -3934,6 +3935,59 @@ async fn test_clever_errors() -> Result<(), anyhow::Error> {
     Ok(())
 }
 
+#[tokio::test]
+async fn test_move_build_bytecode_with_address_resolution() -> Result<(), anyhow::Error> {
+    let test_cluster = TestClusterBuilder::new().build().await;
+    let config_path = test_cluster.swarm.dir().join(SUI_CLIENT_CONFIG);
+
+    // Package setup: a simple package depends on another and copied to tmpdir
+    let mut simple_package_path = PathBuf::from(TEST_DATA_DIR);
+    simple_package_path.push("simple");
+
+    let mut depends_on_simple_package_path = PathBuf::from(TEST_DATA_DIR);
+    depends_on_simple_package_path.push("depends_on_simple");
+
+    let tmp_dir = tempfile::tempdir().unwrap();
+
+    fs_extra::dir::copy(
+        &simple_package_path,
+        &tmp_dir,
+        &fs_extra::dir::CopyOptions::default(),
+    )?;
+
+    fs_extra::dir::copy(
+        &depends_on_simple_package_path,
+        &tmp_dir,
+        &fs_extra::dir::CopyOptions::default(),
+    )?;
+
+    // Publish simple package.
+    let simple_tmp_dir = tmp_dir.path().join("simple");
+    test_with_sui_binary(&[
+        "client",
+        "--client.config",
+        config_path.to_str().unwrap(),
+        "publish",
+        simple_tmp_dir.to_str().unwrap(),
+    ])
+    .await?;
+
+    // Build the package that depends on 'simple' package. Addresses must resolve successfully
+    // from the `Move.lock` for this command to succeed at all.
+    let depends_on_simple_tmp_dir = tmp_dir.path().join("depends_on_simple");
+    test_with_sui_binary(&[
+        "move",
+        "--client.config",
+        config_path.to_str().unwrap(),
+        "build",
+        "--dump-bytecode-as-base64",
+        "--path",
+        depends_on_simple_tmp_dir.to_str().unwrap(),
+    ])
+    .await?;
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_parse_host_port() {
     let input = "127.0.0.0";

diff --git a/crates/sui/tests/data/depends_on_simple/Move.toml b/crates/sui/tests/data/depends_on_simple/Move.toml
@@ -0,0 +1,9 @@
+[package]
+name = "depends_on_simple"
+edition = "2024.beta"
+
+[dependencies]
+simple = { local = "../simple" }
+
+[addresses]
+depends_on_simple = "0x0"
diff --git a/crates/sui/tests/data/depends_on_simple/sources/depends_on_simple.move b/crates/sui/tests/data/depends_on_simple/sources/depends_on_simple.move
@@ -0,0 +1,4 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module depends_on_simple::depends_on_simple {}
diff --git a/crates/sui/tests/data/simple/Move.toml b/crates/sui/tests/data/simple/Move.toml
@@ -0,0 +1,8 @@
+[package]
+name = "simple"
+edition = "2024.beta"
+
+[dependencies]
+
+[addresses]
+simple = "0x0"
diff --git a/crates/sui/tests/data/simple/sources/simple.move b/crates/sui/tests/data/simple/sources/simple.move
@@ -0,0 +1,4 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module simple::simple {}
diff --git a/sdk/deepbook/test/e2e/setup.ts b/sdk/deepbook/test/e2e/setup.ts
@@ -2,6 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { execSync } from 'child_process';
+import { mkdtemp } from 'fs/promises';
+import { tmpdir } from 'os';
+import path from 'path';
 import type {
 	DevInspectResults,
 	SuiObjectChangeCreated,
@@ -30,10 +33,12 @@ export const DEFAULT_LOT_SIZE = 1n;
 export class TestToolbox {
 	keypair: Ed25519Keypair;
 	client: SuiClient;
+	configPath: string;
 
-	constructor(keypair: Ed25519Keypair, client: SuiClient) {
+	constructor(keypair: Ed25519Keypair, client: SuiClient, configPath: string) {
 		this.keypair = keypair;
 		this.client = client;
+		this.configPath = configPath;
 	}
 
 	address() {
@@ -64,7 +69,12 @@ export async function setupSuiClient() {
 		retryIf: (error: any) => !(error instanceof FaucetRateLimitError),
 		logger: (msg) => console.warn('Retrying requesting from faucet: ' + msg),
 	});
-	return new TestToolbox(keypair, client);
+
+	const tmpDirPath = path.join(tmpdir(), 'config-');
+	const tmpDir = await mkdtemp(tmpDirPath);
+	const configPath = path.join(tmpDir, 'client.yaml');
+	execSync(`${SUI_BIN} client --yes --client.config ${configPath}`, { encoding: 'utf-8' });
+	return new TestToolbox(keypair, client, configPath);
 }
 
 // TODO: expose these testing utils from @mysten/sui
@@ -81,7 +91,7 @@ export async function publishPackage(packagePath: string, toolbox?: TestToolbox)
 
 	const { modules, dependencies } = JSON.parse(
 		execSync(
-			`${SUI_BIN} move build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
+			`${SUI_BIN} move move --client.config ${toolbox.configPath} build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
 			{ encoding: 'utf-8' },
 		),
 	);

diff --git a/sdk/kiosk/test/e2e/setup.ts b/sdk/kiosk/test/e2e/setup.ts
@@ -2,6 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { execSync } from 'child_process';
+import { mkdtemp } from 'fs/promises';
+import { tmpdir } from 'os';
+import path from 'path';
 import type {
 	DevInspectResults,
 	SuiObjectChangePublished,
@@ -28,10 +31,12 @@ const SUI_BIN = import.meta.env.VITE_SUI_BIN ?? 'cargo run --bin sui';
 export class TestToolbox {
 	keypair: Ed25519Keypair;
 	client: SuiClient;
+	configPath: string;
 
-	constructor(keypair: Ed25519Keypair, client: SuiClient) {
+	constructor(keypair: Ed25519Keypair, client: SuiClient, configPath: string) {
 		this.keypair = keypair;
 		this.client = client;
+		this.configPath = configPath;
 	}
 
 	address() {
@@ -62,7 +67,12 @@ export async function setupSuiClient() {
 		retryIf: (error: any) => !(error instanceof FaucetRateLimitError),
 		logger: (msg) => console.warn('Retrying requesting from faucet: ' + msg),
 	});
-	return new TestToolbox(keypair, client);
+
+	const tmpDirPath = path.join(tmpdir(), 'config-');
+	const tmpDir = await mkdtemp(tmpDirPath);
+	const configPath = path.join(tmpDir, 'client.yaml');
+	execSync(`${SUI_BIN} client --yes --client.config ${configPath}`, { encoding: 'utf-8' });
+	return new TestToolbox(keypair, client, configPath);
 }
 
 // TODO: expose these testing utils from @mysten/sui
@@ -79,7 +89,7 @@ export async function publishPackage(packagePath: string, toolbox?: TestToolbox)
 
 	const { modules, dependencies } = JSON.parse(
 		execSync(
-			`${SUI_BIN} move build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
+			`${SUI_BIN} move --client.config ${toolbox.configPath} build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
 			{ encoding: 'utf-8' },
 		),
 	);

diff --git a/sdk/typescript/test/e2e/utils/setup.ts b/sdk/typescript/test/e2e/utils/setup.ts
@@ -174,7 +174,7 @@ export async function publishPackage(packagePath: string, toolbox?: TestToolbox)
 
 	const { modules, dependencies } = JSON.parse(
 		execSync(
-			`${SUI_BIN} move build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
+			`${SUI_BIN} move --client.config ${toolbox.configPath} build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
 			{ encoding: 'utf-8' },
 		),
 	);
@@ -228,7 +228,7 @@ export async function upgradePackage(
 
 	const { modules, dependencies, digest } = JSON.parse(
 		execSync(
-			`${SUI_BIN} move build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
+			`${SUI_BIN} move --client.config ${toolbox.configPath} build --dump-bytecode-as-base64 --path ${packagePath} --install-dir ${tmpobj.name}`,
 			{ encoding: 'utf-8' },
 		),
 	);