From 6bfd48d41271d486c15e7633ae1857b267e13331 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Fri, 20 Jan 2023 15:43:22 +0000 Subject: [PATCH] [client/publish] Add ability to publish --with-unpublished-dependencies (#7426) If a package depends on other packages that haven't been published yet, this flag offers a convenience to publish them as part of the same transaction (or to include them in the base64 dump when building). This is not ideal from the perspective of code re-use, but does help while wipes are more common. This also has implications for source validation, which needs to support validating packages that have other packages embedded in them: - Now, on-chain modules in the root package could be matched against multiple source packages, so they are fetched all in one go. - This also means there is no longer a 1:1 correspondence between numerical address and on-chain address, so the `LocalDependencyNotFound` error now identifies a package by its address. ## Test Plan New source-validation and move integration tests: ``` $ cargo simtest $ cargo nextest run ``` Co-authored-by: Brandon Williams --- Cargo.lock | 1 + ...fullnode_build_publish_transaction_test.rs | 9 +- .../data/depends_on_basics/Move.toml | 10 + .../sources/depends_on_basics.move | 13 + crates/sui-core/src/unit_tests/gas_tests.rs | 3 + .../src/unit_tests/move_integration_tests.rs | 78 ++++- .../src/compiled_package.rs | 71 +++-- .../src/unit_tests/rpc_server_tests.rs | 6 +- crates/sui-source-validation/Cargo.toml | 1 + crates/sui-source-validation/src/lib.rs | 268 ++++++++++-------- crates/sui-source-validation/src/tests.rs | 79 +++++- crates/sui/src/client_commands.rs | 8 +- crates/sui/src/sui_move/build.rs | 8 +- crates/sui/src/sui_move/unit_test.rs | 2 + crates/sui/src/unit_tests/cli_tests.rs | 2 + crates/test-utils/src/messages.rs | 4 +- crates/test-utils/src/transaction.rs | 2 +- 17 files changed, 404 insertions(+), 161 deletions(-) create mode 100644 crates/sui-core/src/unit_tests/data/depends_on_basics/Move.toml create mode 100644 crates/sui-core/src/unit_tests/data/depends_on_basics/sources/depends_on_basics.move diff --git a/Cargo.lock b/Cargo.lock index 7530fb35da417..0f166136b7335 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9231,6 +9231,7 @@ name = "sui-source-validation" version = "0.1.0" dependencies = [ "anyhow", + "futures", "move-binary-format", "move-bytecode-utils", "move-bytecode-verifier", diff --git a/crates/sui-cluster-test/src/test_case/fullnode_build_publish_transaction_test.rs b/crates/sui-cluster-test/src/test_case/fullnode_build_publish_transaction_test.rs index 4f1275da6a705..27d6ded0f1774 100644 --- a/crates/sui-cluster-test/src/test_case/fullnode_build_publish_transaction_test.rs +++ b/crates/sui-cluster-test/src/test_case/fullnode_build_publish_transaction_test.rs @@ -3,7 +3,6 @@ use crate::{TestCaseImpl, TestContext}; use async_trait::async_trait; -use fastcrypto::encoding::Base64; use jsonrpsee::rpc_params; use sui_core::test_utils::compile_basics_package; use sui_types::{base_types::ObjectID, object::Owner}; @@ -21,11 +20,9 @@ impl TestCaseImpl for FullNodeBuildPublishTransactionTest { } async fn run(&self, ctx: &mut TestContext) -> Result<(), anyhow::Error> { - let all_module_bytes = compile_basics_package() - .get_package_bytes() - .iter() - .map(|bytes| Base64::from_bytes(bytes)) - .collect::>(); + let all_module_bytes = + compile_basics_package().get_package_base64(/* with_unpublished_deps */ false); + let params = rpc_params![ ctx.get_wallet_address(), all_module_bytes, diff --git a/crates/sui-core/src/unit_tests/data/depends_on_basics/Move.toml b/crates/sui-core/src/unit_tests/data/depends_on_basics/Move.toml new file mode 100644 index 0000000000000..caf3eb8c038e0 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/depends_on_basics/Move.toml @@ -0,0 +1,10 @@ +[package] +name = "DependsOnBasics" +version = "0.0.0" + +[dependencies] +Examples = { local = "../object_basics" } +Sui = { local = "../../../../../sui-framework" } + +[addresses] +depends = "0x0" diff --git a/crates/sui-core/src/unit_tests/data/depends_on_basics/sources/depends_on_basics.move b/crates/sui-core/src/unit_tests/data/depends_on_basics/sources/depends_on_basics.move new file mode 100644 index 0000000000000..66739f3d18e5e --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/depends_on_basics/sources/depends_on_basics.move @@ -0,0 +1,13 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Test depending on another unpublished package, which is published +/// along with your own. +module depends::depends_on_basics { + use examples::object_basics; + use sui::tx_context::{Self, TxContext}; + + public entry fun delegate(ctx: &mut TxContext) { + object_basics::share(ctx); + } +} diff --git a/crates/sui-core/src/unit_tests/gas_tests.rs b/crates/sui-core/src/unit_tests/gas_tests.rs index 63780f2ea15e8..b2a3bc6bc26f8 100644 --- a/crates/sui-core/src/unit_tests/gas_tests.rs +++ b/crates/sui-core/src/unit_tests/gas_tests.rs @@ -264,6 +264,7 @@ async fn test_publish_gas() -> anyhow::Result<()> { &gas_object_id, "object_wrapping", GAS_VALUE_FOR_TESTING, + /* with_unpublished_deps */ false, ) .await; let effects = response.1.into_data(); @@ -337,6 +338,7 @@ async fn test_publish_gas() -> anyhow::Result<()> { &gas_object_id, "object_wrapping", budget, + /* with_unpublished_deps */ false, ) .await; let effects = response.1.into_data(); @@ -368,6 +370,7 @@ async fn test_publish_gas() -> anyhow::Result<()> { &gas_object_id, "object_wrapping", budget, + /* with_unpublished_deps */ false, ) .await; let effects = response.1.into_data(); diff --git a/crates/sui-core/src/unit_tests/move_integration_tests.rs b/crates/sui-core/src/unit_tests/move_integration_tests.rs index d3598c6117b77..48cb4295fd798 100644 --- a/crates/sui-core/src/unit_tests/move_integration_tests.rs +++ b/crates/sui-core/src/unit_tests/move_integration_tests.rs @@ -6,7 +6,7 @@ use super::*; use crate::authority::authority_tests::{ call_move, call_move_, init_state_with_ids, send_and_confirm_transaction, TestCallArg, }; -use sui_types::utils::to_sender_signed_transaction; +use sui_types::{object::Data, utils::to_sender_signed_transaction}; use move_core_types::{ language_storage::TypeTag, @@ -25,6 +25,78 @@ use std::{env, str::FromStr}; const MAX_GAS: u64 = 10000; +#[tokio::test] +#[cfg_attr(msim, ignore)] +async fn test_publishing_with_unpublished_deps() { + let (sender, sender_key): (_, AccountKeyPair) = get_key_pair(); + let gas = ObjectID::random(); + let authority = init_state_with_ids(vec![(sender, gas)]).await; + + let effects = build_and_try_publish_test_package( + &authority, + &sender, + &sender_key, + &gas, + "depends_on_basics", + MAX_GAS, + /* with_unpublished_deps */ true, + ) + .await + .1 + .into_data(); + + assert!(effects.status.is_ok()); + assert_eq!(effects.created.len(), 1); + let package = effects.created[0].0; + + let ObjectRead::Exists(read_ref, package_obj, _) = authority + .get_object_read(&package.0) + .await + .unwrap() + else { + panic!("Can't read package") + }; + + assert_eq!(package, read_ref); + let Data::Package(move_package) = package_obj.data else { + panic!("Not a package") + }; + + // Check that the published package includes its depended upon module. + assert_eq!( + move_package + .serialized_module_map() + .keys() + .map(String::as_str) + .collect::>(), + HashSet::from(["depends_on_basics", "object_basics"]), + ); + + let effects = call_move( + &authority, + &gas, + &sender, + &sender_key, + &package, + "depends_on_basics", + "delegate", + vec![], + vec![], + ) + .await + .unwrap(); + + assert!(effects.status.is_ok()); + assert_eq!(effects.created.len(), 1); + let ((_, v, _), owner) = effects.created[0]; + + // Check that calling the function does what we expect + assert!(matches!( + owner, + Owner::Shared { initial_shared_version: initial } if initial == v + )); +} + #[tokio::test] #[cfg_attr(msim, ignore)] async fn test_object_wrapping_unwrapping() { @@ -1845,6 +1917,7 @@ pub async fn build_and_try_publish_test_package( gas_object_id: &ObjectID, test_dir: &str, gas_budget: u64, + with_unpublished_deps: bool, ) -> (Transaction, SignedTransactionEffects) { let build_config = BuildConfig::default(); let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); @@ -1852,7 +1925,7 @@ pub async fn build_and_try_publish_test_package( path.push(test_dir); let all_module_bytes = sui_framework::build_move_package(&path, build_config) .unwrap() - .get_package_bytes(); + .get_package_bytes(with_unpublished_deps); let gas_object = authority.get_object(gas_object_id).await.unwrap(); let gas_object_ref = gas_object.unwrap().compute_object_reference(); @@ -1887,6 +1960,7 @@ async fn build_and_publish_test_package( gas_object_id, test_dir, MAX_GAS, + /* with_unpublished_deps */ false, ) .await .1 diff --git a/crates/sui-framework-build/src/compiled_package.rs b/crates/sui-framework-build/src/compiled_package.rs index 2823188ca3b92..3bc73d9c4cb60 100644 --- a/crates/sui-framework-build/src/compiled_package.rs +++ b/crates/sui-framework-build/src/compiled_package.rs @@ -130,35 +130,50 @@ impl CompiledPackage { }) } - /// Return the bytecode modules in this package, topologically sorted in dependency order - /// This is the function to call if you would like to publish or statically analyze the modules - pub fn get_dependency_sorted_modules(&self) -> Vec { - let compiled_modules = self.package.root_modules_map(); - // Collect all module IDs from the current package to be - // published (module names are not sufficient as we may - // have modules with the same names in user code and in - // Sui framework which would result in the latter being - // pulled into a set of modules to be published). - // For each transitive dependent module, if they are not to be published, - // they must have a non-zero address (meaning they are already published on-chain). - let self_modules: HashSet = compiled_modules - .iter_modules() - .iter() - .map(|m| m.self_id()) - .collect(); - self.package - .all_modules_map() - .compute_dependency_graph() - .compute_topological_order() - .unwrap() // safe because package built successfully - .filter(|m| self_modules.contains(&m.self_id())) - .cloned() - .collect() + /// Return the bytecode modules in this package, topologically sorted in dependency order. + /// Optionally include dependencies that have not been published (are at address 0x0), if + /// `with_unpublished_deps` is true. This is the function to call if you would like to publish + /// or statically analyze the modules. + pub fn get_dependency_sorted_modules( + &self, + with_unpublished_deps: bool, + ) -> Vec { + let all_modules = self.package.all_modules_map(); + let graph = all_modules.compute_dependency_graph(); + + // SAFETY: package built successfully + let modules = graph.compute_topological_order().unwrap(); + + if with_unpublished_deps { + // For each transitive dependent module, if they are not to be published, they must have + // a non-zero address (meaning they are already published on-chain). + modules + .filter(|module| module.address() == &AccountAddress::ZERO) + .cloned() + .collect() + } else { + // Collect all module IDs from the current package to be published (module names are not + // sufficient as we may have modules with the same names in user code and in Sui + // framework which would result in the latter being pulled into a set of modules to be + // published). + let self_modules: HashSet<_> = self + .package + .root_modules_map() + .iter_modules() + .iter() + .map(|m| m.self_id()) + .collect(); + + modules + .filter(|module| self_modules.contains(&module.self_id())) + .cloned() + .collect() + } } /// Return a serialized representation of the bytecode modules in this package, topologically sorted in dependency order - pub fn get_package_bytes(&self) -> Vec> { - self.get_dependency_sorted_modules() + pub fn get_package_bytes(&self, with_unpublished_deps: bool) -> Vec> { + self.get_dependency_sorted_modules(with_unpublished_deps) .iter() .map(|m| { let mut bytes = Vec::new(); @@ -169,8 +184,8 @@ impl CompiledPackage { } /// Return the base64-encoded representation of the bytecode modules in this package, topologically sorted in dependency order - pub fn get_package_base64(&self) -> Vec { - self.get_package_bytes() + pub fn get_package_base64(&self, with_unpublished_deps: bool) -> Vec { + self.get_package_bytes(with_unpublished_deps) .iter() .map(|b| Base64::from_bytes(b)) .collect() diff --git a/crates/sui-json-rpc/src/unit_tests/rpc_server_tests.rs b/crates/sui-json-rpc/src/unit_tests/rpc_server_tests.rs index b69a51d28af5d..d4ea9b36b36d5 100644 --- a/crates/sui-json-rpc/src/unit_tests/rpc_server_tests.rs +++ b/crates/sui-json-rpc/src/unit_tests/rpc_server_tests.rs @@ -99,7 +99,7 @@ async fn test_publish() -> Result<(), anyhow::Error> { let compiled_modules = BuildConfig::default() .build(Path::new("../../sui_programmability/examples/fungible_tokens").to_path_buf())? - .get_package_base64(); + .get_package_base64(/* with_unpublished_deps */ false); let transaction_bytes: TransactionBytes = http_client .publish(*address, compiled_modules, Some(gas.object_id), 10000) @@ -257,7 +257,7 @@ async fn test_get_metadata() -> Result<(), anyhow::Error> { // Publish test coin package let compiled_modules = BuildConfig::default() .build(Path::new("src/unit_tests/data/dummy_modules_publish").to_path_buf())? - .get_package_base64(); + .get_package_base64(/* with_unpublished_deps */ false); let transaction_bytes: TransactionBytes = http_client .publish(*address, compiled_modules, Some(gas.object_id), 10000) @@ -316,7 +316,7 @@ async fn test_get_total_supply() -> Result<(), anyhow::Error> { // Publish test coin package let compiled_modules = BuildConfig::default() .build(Path::new("src/unit_tests/data/dummy_modules_publish").to_path_buf())? - .get_package_base64(); + .get_package_base64(/* with_unpublished_deps */ false); let transaction_bytes: TransactionBytes = http_client .publish(*address, compiled_modules, Some(gas.object_id), 10000) diff --git a/crates/sui-source-validation/Cargo.toml b/crates/sui-source-validation/Cargo.toml index 489e2d8719c76..05d5be4fb8c4e 100644 --- a/crates/sui-source-validation/Cargo.toml +++ b/crates/sui-source-validation/Cargo.toml @@ -12,6 +12,7 @@ path = "src/lib.rs" [dependencies] anyhow = { version = "1.0.64", features = ["backtrace"] } thiserror = "1.0.34" +futures = "0.3.25" sui-types = { path = "../sui-types" } sui-sdk = { path = "../../crates/sui-sdk" } diff --git a/crates/sui-source-validation/src/lib.rs b/crates/sui-source-validation/src/lib.rs index 91c7343bfcb54..6873c58a95507 100644 --- a/crates/sui-source-validation/src/lib.rs +++ b/crates/sui-source-validation/src/lib.rs @@ -1,7 +1,9 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +use futures::future; use move_binary_format::access::ModuleAccess; +use move_binary_format::CompiledModule; use std::{collections::HashMap, fmt::Debug}; use thiserror::Error; @@ -32,8 +34,11 @@ pub enum SourceVerificationError { #[error("On-chain version of dependency {package}::{module} was not found.")] OnChainDependencyNotFound { package: Symbol, module: Symbol }, - #[error("Local version of dependency {package}::{module} was not found.")] - LocalDependencyNotFound { package: Symbol, module: String }, + #[error("Local version of dependency {address}::{module} was not found.")] + LocalDependencyNotFound { + address: AccountAddress, + module: Symbol, + }, #[error( "Local dependency did not match its on-chain version at {address}::{package}::{module}" @@ -69,9 +74,11 @@ pub struct BytecodeSourceVerifier<'a> { rpc_client: &'a ReadApi, } -/// Map the package's direct dependencies (keyed by their address and package name) to their module -/// bytecode (mapping from module name to byte array). -type ModuleBytesMap = HashMap<(AccountAddress, Symbol), HashMap>>; +/// Map package addresses and module names to package names and bytecode. +type LocalBytes = HashMap<(AccountAddress, Symbol), (Symbol, Vec)>; +/// Map package addresses and modules names to bytecode (package names are gone in the on-chain +/// representation). +type OnChainBytes = HashMap<(AccountAddress, Symbol), Vec>; impl<'a> BytecodeSourceVerifier<'a> { pub fn new(rpc_client: &'a ReadApi, verbose: bool) -> Self { @@ -142,64 +149,52 @@ impl<'a> BytecodeSourceVerifier<'a> { } } - let compiled_dep_map = get_module_bytes_map(compiled_package, verify_deps, source_mode)?; - - for ((address, package), local_modules) in compiled_dep_map { - // if `root_on_chain_address` is None, then Zero address is the package we're checking - // dependencies for, it does not need to (and cannot) be verified. - if address == AccountAddress::ZERO { - continue; - } + let local_modules = local_bytes(compiled_package, verify_deps, source_mode)?; + let mut on_chain_modules = self + .on_chain_bytes(local_modules.keys().map(|(addr, _)| *addr)) + .await?; - // fetch the Sui object at the address specified for the package in the local resolution - // table - let SuiRawMovePackage { - module_map: mut on_chain_modules, - .. - } = self.pkg_for_address(&address).await?; - - for (module, local_bytes) in local_modules { - let Some(on_chain_bytes) = on_chain_modules.remove(module.as_ref()) else { - return Err(SourceVerificationError::OnChainDependencyNotFound { - package, module, - }) - }; - - // compare local bytecode to on-chain bytecode to ensure integrity of our - // dependencies - if local_bytes != on_chain_bytes { - return Err(SourceVerificationError::ModuleBytecodeMismatch { - address, - package, - module, - }); - } + 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 { + package, module, + }) + }; - if self.verbose { - println!( - "{}::{} - {} bytes, code matches", - package.as_ref(), - module.as_ref(), - on_chain_bytes.len() - ); - } + // compare local bytecode to on-chain bytecode to ensure integrity of our + // dependencies + if local_bytes != on_chain_bytes { + return Err(SourceVerificationError::ModuleBytecodeMismatch { + address, + package, + module, + }); } - if let Some((module, _)) = on_chain_modules.into_iter().next() { - return Err(SourceVerificationError::LocalDependencyNotFound { package, module }); + if self.verbose { + println!( + "{}::{} - {} bytes, code matches", + package.as_ref(), + module.as_ref(), + on_chain_bytes.len() + ); } } + if let Some(((address, module), _)) = on_chain_modules.into_iter().next() { + return Err(SourceVerificationError::LocalDependencyNotFound { address, module }); + } + Ok(()) } async fn pkg_for_address( &self, - addr: &AccountAddress, + addr: AccountAddress, ) -> Result { // Move packages are specified with an AccountAddress, but are // fetched from a sui network via sui_getObject, which takes an object ID - let obj_id = ObjectID::from(*addr); + let obj_id = ObjectID::from(addr); // fetch the Sui object at the address specified for the package in the local resolution table // if future packages with a large set of dependency packages prove too slow to verify, @@ -221,90 +216,137 @@ impl<'a> BytecodeSourceVerifier<'a> { ), } } -} - -fn get_module_bytes_map( - compiled_package: &CompiledPackage, - include_deps: bool, - source_mode: SourceMode, -) -> Result { - let mut map: ModuleBytesMap = HashMap::new(); - - #[allow(clippy::type_complexity)] - fn make_map_entry( - package: &Symbol, - named_compiled_module: &NamedCompiledModule, - subst_addr: Option, - ) -> Result<((AccountAddress, Symbol), (Symbol, Vec)), SourceVerificationError> { - let module = named_compiled_module.name; - let address = subst_addr.unwrap_or_else(|| named_compiled_module.address.into_inner()); - let mut bytes = vec![]; - - // Replace the zero address entries in the module if needed - if let Some(new_address) = subst_addr { - let mut compiled_module = named_compiled_module.module.clone(); - let self_handle = compiled_module.self_handle().clone(); - let self_address_idx = self_handle.address; - - let addrs = &mut compiled_module.address_identifiers; - let Some(address_mut) = addrs.get_mut(self_address_idx.0 as usize) else { - let name = compiled_module.identifier_at(self_handle.name); - return Err(SourceVerificationError::InvalidModuleFailure { - name: name.to_string(), - message: "Self address field missing".to_string() - }); - }; - if *address_mut != AccountAddress::ZERO { - let name = compiled_module.identifier_at(self_handle.name); - return Err(SourceVerificationError::InvalidModuleFailure { - name: name.to_string(), - message: "Self address already populated".to_string(), - }); - }; + async fn on_chain_bytes( + &self, + addresses: impl Iterator + Clone, + ) -> Result { + let resp = future::join_all(addresses.clone().map(|addr| self.pkg_for_address(addr))).await; + let mut map = OnChainBytes::new(); + + for (addr, pkg) in addresses.zip(resp) { + let SuiRawMovePackage { module_map, .. } = pkg?; + map.extend( + module_map + .into_iter() + .map(move |(module, bytes)| ((addr, Symbol::from(module)), bytes)), + ) + } - *address_mut = new_address; + Ok(map) + } +} - // TODO: in the future, this probably needs to use `serialize_for_version`. - compiled_module.serialize(&mut bytes).unwrap(); - } else { - // TODO: in the future, this probably needs to use `serialize_for_version`. - named_compiled_module.module.serialize(&mut bytes).unwrap(); - } +fn substitute_root_address( + named_module: &NamedCompiledModule, + root: AccountAddress, +) -> Result { + let mut module = named_module.module.clone(); + let address_idx = module.self_handle().address; + + let Some(addr) = module.address_identifiers.get_mut(address_idx.0 as usize) else { + return Err(SourceVerificationError::InvalidModuleFailure { + name: named_module.name.to_string(), + message: "Self address field missing".into(), + }); + }; - Ok(((address, *package), (module, bytes))) + if *addr != AccountAddress::ZERO { + return Err(SourceVerificationError::InvalidModuleFailure { + name: named_module.name.to_string(), + message: "Self address already populated".to_string(), + }); } + *addr = root; + Ok(module) +} + +fn local_bytes( + compiled_package: &CompiledPackage, + include_deps: bool, + source_mode: SourceMode, +) -> Result { + let mut map = LocalBytes::new(); + if include_deps { for (package, local_unit) in &compiled_package.deps_compiled_units { let CompiledUnitEnum::Module(m) = &local_unit.unit else { continue; }; - let (k, v) = make_map_entry(package, m, None)?; + let module = m.name; + let address = m.address.into_inner(); + if address == AccountAddress::ZERO { + continue; + } - map.entry(k).or_default().insert(v.0, v.1); + let mut bytes = vec![]; + m.module.serialize(&mut bytes).unwrap(); + map.insert((address, module), (*package, bytes)); } } - if source_mode == SourceMode::Skip { - return Ok(map); - } + let root_package = compiled_package.compiled_package_info.package_name; + match source_mode { + SourceMode::Skip => { /* nop */ } + + // Include the root compiled units, at their current addresses. + SourceMode::Verify => { + for local_unit in &compiled_package.root_compiled_units { + let CompiledUnitEnum::Module(m) = &local_unit.unit else { + continue; + }; - let subst_addr = if let SourceMode::VerifyAt(root_address) = source_mode { - Some(root_address) - } else { - None - }; + let module = m.name; + let address = m.address.into_inner(); + if address == AccountAddress::ZERO { + return Err(SourceVerificationError::InvalidModuleFailure { + name: module.to_string(), + message: "Can't verify unpublished source".to_string(), + }); + } - let root_package = compiled_package.compiled_package_info.package_name; - for local_unit in &compiled_package.root_compiled_units { - let CompiledUnitEnum::Module(m) = &local_unit.unit else { - continue; - }; + let mut bytes = vec![]; + m.module.serialize(&mut bytes).unwrap(); + map.insert((address, module), (root_package, bytes)); + } + } + + // Include the root compiled units, and any unpublished dependencies with their + // addresses substituted + SourceMode::VerifyAt(root_address) => { + for local_unit in &compiled_package.root_compiled_units { + let CompiledUnitEnum::Module(m) = &local_unit.unit else { + continue; + }; + + let module = m.name; + let mut bytes = vec![]; + substitute_root_address(m, root_address)? + .serialize(&mut bytes) + .unwrap(); + map.insert((root_address, module), (root_package, bytes)); + } + + for (package, local_unit) in &compiled_package.deps_compiled_units { + let CompiledUnitEnum::Module(m) = &local_unit.unit else { + continue; + }; - let (package, (module, bytes)) = make_map_entry(&root_package, m, subst_addr)?; - map.entry(package).or_default().insert(module, bytes); + let module = m.name; + let address = m.address.into_inner(); + if address != AccountAddress::ZERO { + continue; + } + + let mut bytes = vec![]; + substitute_root_address(m, root_address)? + .serialize(&mut bytes) + .unwrap(); + map.insert((root_address, module), (*package, bytes)); + } + } } Ok(map) diff --git a/crates/sui-source-validation/src/tests.rs b/crates/sui-source-validation/src/tests.rs index e60d487f14983..3820e14b02ca9 100644 --- a/crates/sui-source-validation/src/tests.rs +++ b/crates/sui-source-validation/src/tests.rs @@ -86,6 +86,34 @@ async fn successful_verification() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn successful_verification_unpublished_deps() -> anyhow::Result<()> { + let mut cluster = TestClusterBuilder::new().build().await?; + let sender = cluster.get_address_0(); + let context = &mut cluster.wallet; + let fixtures = tempfile::tempdir()?; + + let a_src = { + let zero = SuiAddress::ZERO; + copy_package(&fixtures, "b", [("b", zero)]).await?; + copy_package(&fixtures, "a", [("a", zero), ("b", zero)]).await? + }; + + let a_pkg = compile_package(a_src.clone()); + let a_ref = publish_package_and_deps(context, sender, a_src).await; + + let client = context.get_client().await?; + let verifier = BytecodeSourceVerifier::new(client.read_api(), false); + + // Verify the root package which now includes dependency modules + verifier + .verify_package_root(&a_pkg.package, a_ref.0.into()) + .await + .unwrap(); + + Ok(()) +} + #[tokio::test] async fn fail_verification_bad_address() -> anyhow::Result<()> { let mut cluster = TestClusterBuilder::new().build().await?; @@ -121,6 +149,36 @@ async fn fail_verification_bad_address() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn fail_to_verify_unpublished_root() -> anyhow::Result<()> { + let mut cluster = TestClusterBuilder::new().build().await?; + let context = &mut cluster.wallet; + + let b_pkg = { + let fixtures = tempfile::tempdir()?; + let b_src = copy_package(&fixtures, "b", [("b", SuiAddress::ZERO)]).await?; + compile_package(&b_src) + }; + + let client = context.get_client().await?; + let verifier = BytecodeSourceVerifier::new(client.read_api(), false); + + // Trying to verify the root package, which hasn't been published -- this is going to fail + // because there is no on-chain package to verify against. + assert!(matches!( + verifier + .verify_package( + &b_pkg.package, + /* verify_deps */ false, + SourceMode::Verify + ) + .await, + Err(SourceVerificationError::InvalidModuleFailure { .. }), + )); + + Ok(()) +} + #[tokio::test] async fn rpc_call_failed_during_verify() -> anyhow::Result<()> { let mut cluster = TestClusterBuilder::new().build().await?; @@ -309,12 +367,12 @@ async fn module_not_found_locally() -> anyhow::Result<()> { panic!("Expected verification to fail"); }; - let SourceVerificationError::LocalDependencyNotFound { package, module } = err else { + let SourceVerificationError::LocalDependencyNotFound { address, module } = err else { panic!("Expected LocalDependencyNotFound, got: {:?}", err); }; - assert_eq!(package, "b".into()); - assert_eq!(module, "d"); + assert_eq!(address, b_ref.0.into()); + assert_eq!(module.as_ref(), "d"); Ok(()) } @@ -398,7 +456,20 @@ async fn publish_package( sender: SuiAddress, package: impl AsRef, ) -> ObjectRef { - let package_bytes = compile_package(package).get_package_bytes(); + let package_bytes = + compile_package(package).get_package_bytes(/* with_unpublished_deps */ false); + publish_package_with_wallet(context, sender, package_bytes).await +} + +/// Compile and publish package at absolute path `package` to chain, along with its unpublished +/// dependencies. +async fn publish_package_and_deps( + context: &WalletContext, + sender: SuiAddress, + package: impl AsRef, +) -> ObjectRef { + let package_bytes = + compile_package(package).get_package_bytes(/* with_unpublished_deps */ true); publish_package_with_wallet(context, sender, package_bytes).await } diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index 2a8bf1deb27ea..7cfaaf56fc98f 100644 --- a/crates/sui/src/client_commands.rs +++ b/crates/sui/src/client_commands.rs @@ -138,6 +138,10 @@ pub enum SuiClientCommands { /// dependency found on-chain. #[clap(long)] verify_dependencies: bool, + + /// Also publish transitive dependencies that have not already been published. + #[clap(long)] + with_unpublished_dependencies: bool, }, /// Verify local Move packages against on-chain packages, and optionally their dependencies. @@ -460,6 +464,7 @@ impl SuiClientCommands { build_config, gas_budget, verify_dependencies, + with_unpublished_dependencies, } => { let sender = context.try_get_object_owner(&gas).await?; let sender = sender.unwrap_or(context.active_address()?); @@ -486,7 +491,8 @@ impl SuiClientCommands { } } - let compiled_modules = compiled_package.get_package_bytes(); + let compiled_modules = + compiled_package.get_package_bytes(with_unpublished_dependencies); let client = context.get_client().await?; if verify_dependencies { diff --git a/crates/sui/src/sui_move/build.rs b/crates/sui/src/sui_move/build.rs index 6ae5d3235f472..da10b33d30d6f 100644 --- a/crates/sui/src/sui_move/build.rs +++ b/crates/sui/src/sui_move/build.rs @@ -18,6 +18,10 @@ const STRUCT_LAYOUTS_FILENAME: &str = "struct_layouts.yaml"; pub struct Build { #[clap(flatten)] pub build: build::Build, + /// Include the contents of packages in dependencies that haven't been published (only relevant + /// when dumping bytecode as base64) + #[clap(long, global = true)] + pub with_unpublished_dependencies: bool, /// Whether we are printing in base64. #[clap(long, global = true)] pub dump_bytecode_as_base64: bool, @@ -40,6 +44,7 @@ impl Build { Self::execute_internal( &rerooted_path, build_config, + self.with_unpublished_dependencies, self.dump_bytecode_as_base64, self.generate_struct_layouts, ) @@ -48,6 +53,7 @@ impl Build { pub fn execute_internal( rerooted_path: &Path, config: MoveBuildConfig, + with_unpublished_deps: bool, dump_bytecode_as_base64: bool, generate_struct_layouts: bool, ) -> anyhow::Result<()> { @@ -60,7 +66,7 @@ impl Build { }, )?; if dump_bytecode_as_base64 { - println!("{}", json!(pkg.get_package_base64())) + println!("{}", json!(pkg.get_package_base64(with_unpublished_deps))) } if generate_struct_layouts { diff --git a/crates/sui/src/sui_move/unit_test.rs b/crates/sui/src/sui_move/unit_test.rs index 93ef935830895..560aec02a1d83 100644 --- a/crates/sui/src/sui_move/unit_test.rs +++ b/crates/sui/src/sui_move/unit_test.rs @@ -26,6 +26,7 @@ impl Test { // find manifest file directory from a given path or (if missing) from current dir let rerooted_path = base::reroot_path(path)?; // pre build for Sui-specific verifications + let with_unpublished_deps = false; let dump_bytecode_as_base64 = false; let generate_struct_layouts: bool = false; build::Build::execute_internal( @@ -36,6 +37,7 @@ impl Test { test_mode: false, // make sure to verify tests ..build_config.clone() }, + with_unpublished_deps, dump_bytecode_as_base64, generate_struct_layouts, )?; diff --git a/crates/sui/src/unit_tests/cli_tests.rs b/crates/sui/src/unit_tests/cli_tests.rs index 926501f31b83b..b100ea7a079aa 100644 --- a/crates/sui/src/unit_tests/cli_tests.rs +++ b/crates/sui/src/unit_tests/cli_tests.rs @@ -356,6 +356,7 @@ async fn test_move_call_args_linter_command() -> Result<(), anyhow::Error> { gas: Some(gas_obj_id), gas_budget: 20_000, verify_dependencies: true, + with_unpublished_dependencies: false, } .execute(context) .await?; @@ -520,6 +521,7 @@ async fn test_package_publish_command() -> Result<(), anyhow::Error> { gas: Some(gas_obj_id), gas_budget: 20_000, verify_dependencies: true, + with_unpublished_dependencies: false, } .execute(context) .await?; diff --git a/crates/test-utils/src/messages.rs b/crates/test-utils/src/messages.rs index 293ce767e09a0..3ef672e91ad41 100644 --- a/crates/test-utils/src/messages.rs +++ b/crates/test-utils/src/messages.rs @@ -285,7 +285,7 @@ pub fn create_publish_move_package_transaction( let build_config = BuildConfig::default(); let all_module_bytes = sui_framework::build_move_package(&path, build_config) .unwrap() - .get_package_bytes(); + .get_package_bytes(/* with_unpublished_deps */ false); let data = TransactionData::new_module_with_dummy_gas_price( sender, gas_object_ref, @@ -315,7 +315,7 @@ pub fn make_publish_basics_transaction(gas_object: ObjectRef) -> VerifiedTransac let build_config = BuildConfig::default(); let all_module_bytes = sui_framework::build_move_package(&path, build_config) .unwrap() - .get_package_bytes(); + .get_package_bytes(/* with_unpublished_deps */ false); let data = TransactionData::new_module_with_dummy_gas_price( sender, gas_object, diff --git a/crates/test-utils/src/transaction.rs b/crates/test-utils/src/transaction.rs index fc4cd9953d474..a8bcd107247be 100644 --- a/crates/test-utils/src/transaction.rs +++ b/crates/test-utils/src/transaction.rs @@ -79,7 +79,7 @@ pub async fn publish_basics_package(context: &WalletContext, sender: SuiAddress) publish_package_with_wallet( context, sender, - compile_basics_package().get_package_bytes(), + compile_basics_package().get_package_bytes(/* with_unpublished_deps */ false), ) .await }