Skip to content

Commit

Permalink
[client/publish] Add ability to publish --with-unpublished-dependenci…
Browse files Browse the repository at this point in the history
…es (MystenLabs#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 <[email protected]>
  • Loading branch information
amnn and bmwill authored Jan 20, 2023
1 parent c4307fc commit 6bfd48d
Show file tree
Hide file tree
Showing 17 changed files with 404 additions and 161 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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::<Vec<_>>();
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,
Expand Down
10 changes: 10 additions & 0 deletions crates/sui-core/src/unit_tests/data/depends_on_basics/Move.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "DependsOnBasics"
version = "0.0.0"

[dependencies]
Examples = { local = "../object_basics" }
Sui = { local = "../../../../../sui-framework" }

[addresses]
depends = "0x0"
Original file line number Diff line number Diff line change
@@ -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);
}
}
3 changes: 3 additions & 0 deletions crates/sui-core/src/unit_tests/gas_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
78 changes: 76 additions & 2 deletions crates/sui-core/src/unit_tests/move_integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<_>>(),
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() {
Expand Down Expand Up @@ -1845,14 +1917,15 @@ 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"));
path.push("src/unit_tests/data/");
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();
Expand Down Expand Up @@ -1887,6 +1960,7 @@ async fn build_and_publish_test_package(
gas_object_id,
test_dir,
MAX_GAS,
/* with_unpublished_deps */ false,
)
.await
.1
Expand Down
71 changes: 43 additions & 28 deletions crates/sui-framework-build/src/compiled_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CompiledModule> {
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<ModuleId> = 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<CompiledModule> {
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<Vec<u8>> {
self.get_dependency_sorted_modules()
pub fn get_package_bytes(&self, with_unpublished_deps: bool) -> Vec<Vec<u8>> {
self.get_dependency_sorted_modules(with_unpublished_deps)
.iter()
.map(|m| {
let mut bytes = Vec::new();
Expand All @@ -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<Base64> {
self.get_package_bytes()
pub fn get_package_base64(&self, with_unpublished_deps: bool) -> Vec<Base64> {
self.get_package_bytes(with_unpublished_deps)
.iter()
.map(|b| Base64::from_bytes(b))
.collect()
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-json-rpc/src/unit_tests/rpc_server_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions crates/sui-source-validation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
Loading

0 comments on commit 6bfd48d

Please sign in to comment.