Skip to content

Commit

Permalink
fix(test): publish depending on upgrade
Browse files Browse the repository at this point in the history
## Description

Fix a bug in tests that publish packages where the publish transaction
was always constructed referring to dependencies at their original IDs,
and not their storage IDs.

This has not caused a problem to date, meaning these tests may not
publish a package that depends on an upgrade package, but to avoid
confusion `get_dependency_original_package_ids` has been replaced with
`get_dependency_storage_package_ids`.

Similarly, `get_package_dependencies_hex` has been replaced with an
invocation of `get_dependency_storage_package_ids`.

## Test plan

CI +:

```
sui$ cargo build --bin sui
sui$ cd tmp
sui$ ~/sui/target/debug/sui move new test
sui$ # make it possible to compile
sui$ ~/sui/target/debug/ui move build --dump-bytecode-as-base64
```
  • Loading branch information
amnn committed Aug 14, 2024
1 parent 545c16b commit 919ecb5
Show file tree
Hide file tree
Showing 9 changed files with 11 additions and 36 deletions.
2 changes: 1 addition & 1 deletion crates/sui-cluster-test/src/test_case/coin_index_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ async fn publish_managed_coin_package(
let compiled_package = compile_managed_coin_package();
let all_module_bytes =
compiled_package.get_package_base64(/* with_unpublished_deps */ false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let params = rpc_params![
ctx.get_wallet_address(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl TestCaseImpl for FullNodeBuildPublishTransactionTest {
let compiled_package = compile_basics_package();
let all_module_bytes =
compiled_package.get_package_base64(/* with_unpublished_deps */ false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let params = rpc_params![
ctx.get_wallet_address(),
Expand Down
4 changes: 2 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 @@ -2806,7 +2806,7 @@ pub fn build_package(
let compiled_package = BuildConfig::new_for_testing().build(&path).unwrap();
let digest = compiled_package.get_package_digest(with_unpublished_deps);
let modules = compiled_package.get_package_bytes(with_unpublished_deps);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();
(digest.to_vec(), modules, dependencies)
}

Expand All @@ -2826,7 +2826,7 @@ pub async fn build_and_try_publish_test_package(

let compiled_package = BuildConfig::new_for_testing().build(&path).unwrap();
let all_module_bytes = compiled_package.get_package_bytes(with_unpublished_deps);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let gas_object = authority.get_object(gas_object_id).await.unwrap();
let gas_object_ref = gas_object.unwrap().compute_object_reference();
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-json-rpc-tests/tests/balance_changes_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async fn test_dry_run_publish_with_mocked_coin() -> Result<(), anyhow::Error> {
.into_iter()
.map(|b| b.to_vec().unwrap())
.collect::<Vec<_>>();
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let mut builder = ProgrammableTransactionBuilder::new();
builder.publish_immutable(compiled_modules_bytes, dependencies);
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-json-rpc-tests/tests/rpc_server_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ async fn test_publish() -> Result<(), anyhow::Error> {
BuildConfig::new_for_testing().build(Path::new("../../examples/move/basics"))?;
let compiled_modules_bytes =
compiled_package.get_package_base64(/* with_unpublished_deps */ false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let transaction_bytes: TransactionBlockBytes = http_client
.publish(
Expand Down Expand Up @@ -453,7 +453,7 @@ async fn test_get_metadata() -> Result<(), anyhow::Error> {
let compiled_package = BuildConfig::new_for_testing().build(&path)?;
let compiled_modules_bytes =
compiled_package.get_package_base64(/* with_unpublished_deps */ false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let transaction_bytes: TransactionBlockBytes = http_client
.publish(
Expand Down Expand Up @@ -537,7 +537,7 @@ async fn test_get_total_supply() -> Result<(), anyhow::Error> {
let compiled_package = BuildConfig::default().build(&path)?;
let compiled_modules_bytes =
compiled_package.get_package_base64(/* with_unpublished_deps */ false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let transaction_bytes: TransactionBlockBytes = http_client
.publish(
Expand Down
24 changes: 0 additions & 24 deletions crates/sui-move-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,22 +352,6 @@ impl CompiledPackage {
self.dependency_ids.published.values().cloned().collect()
}

/// Return the set of Object IDs corresponding to this package's transitive dependencies'
/// original package IDs.
pub fn get_dependency_original_package_ids(&self) -> Vec<ObjectID> {
let mut ids: BTreeSet<_> = self
.package
.deps_compiled_units
.iter()
.map(|(_, m)| ObjectID::from(*m.unit.module.address()))
.collect();

// `0x0` is not a real dependency ID -- it means that the package has unpublished
// dependencies.
ids.remove(&ObjectID::ZERO);
ids.into_iter().collect()
}

pub fn get_package_digest(&self, with_unpublished_deps: bool) -> [u8; 32] {
let hash_modules = true;
MovePackage::compute_digest_for_modules_and_deps(
Expand Down Expand Up @@ -397,14 +381,6 @@ impl CompiledPackage {
.collect()
}

pub fn get_package_dependencies_hex(&self) -> Vec<String> {
self.dependency_ids
.published
.values()
.map(|object_id| object_id.to_hex_uncompressed())
.collect()
}

/// Get bytecode modules from DeepBook that are used by this package
pub fn get_deepbook_modules(&self) -> impl Iterator<Item = &CompiledModule> {
self.get_modules_and_deps()
Expand Down
3 changes: 1 addition & 2 deletions crates/sui-move/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,11 @@ impl Build {
check_unpublished_dependencies(&pkg.dependency_ids.unpublished)?;
}

let package_dependencies = pkg.get_package_dependencies_hex();
println!(
"{}",
json!({
"modules": pkg.get_package_base64(with_unpublished_deps),
"dependencies": json!(package_dependencies),
"dependencies": pkg.get_dependency_storage_package_ids(),
"digest": pkg.get_package_digest(with_unpublished_deps),
})
)
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-oracle/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ async fn publish_package(
) -> ObjectID {
let compiled_package = BuildConfig::new_for_testing().build(path).unwrap();
let all_module_bytes = compiled_package.get_package_bytes(false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();
let gas = client
.coin_read_api()
.get_coins(sender, None, None, Some(1))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ async fn test_publish_and_move_call() {
let compiled_package = BuildConfig::new_for_testing().build(&path).unwrap();
let compiled_modules_bytes =
compiled_package.get_package_bytes(/* with_unpublished_deps */ false);
let dependencies = compiled_package.get_dependency_original_package_ids();
let dependencies = compiled_package.get_dependency_storage_package_ids();

let pt = {
let mut builder = ProgrammableTransactionBuilder::new();
Expand Down

0 comments on commit 919ecb5

Please sign in to comment.