Skip to content

Commit

Permalink
[CLI] Remove reference to --legacy-digest (#13312)
Browse files Browse the repository at this point in the history
## Description

A long overdue clean-up PR to get rid of the `--legacy-digest` flag on
`sui client upgrade` and `sui move build`. This flag was responsible for
changing the way package digests were calculated, as the algorithm
changed around protocol version 7.

## Test Plan

```
$ cargo simtest
$ env SUI_SKIP_SIMTESTS=1 cargo nextest run
```

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

Removes the `--legacy-digest` flag from the `sui client upgrade` and
`sui move build` CLI commands, as no Sui networks require package
digests to be calculated using the legacy algorithm anymore.
  • Loading branch information
amnn authored and ebmifa committed Aug 10, 2023
1 parent d353795 commit f325c3c
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 42 deletions.
5 changes: 1 addition & 4 deletions crates/sui-core/src/authority/authority_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,11 +442,8 @@ pub async fn upgrade_package_on_single_authority(
let package = build_test_modules_with_dep_addr(path, dep_original_addresses, dep_id_mapping);

let with_unpublished_deps = false;
let hash_modules = true;
let modules = package.get_package_bytes(with_unpublished_deps);
let digest = package
.get_package_digest(with_unpublished_deps, hash_modules)
.to_vec();
let digest = package.get_package_digest(with_unpublished_deps).to_vec();

let rgp = state.epoch_store_for_testing().reference_gas_price();
let data = TransactionData::new_upgrade(
Expand Down
8 changes: 3 additions & 5 deletions crates/sui-core/src/unit_tests/move_package_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,15 +330,13 @@ fn package_digest_changes_with_dep_upgrades_and_in_sync_with_move_package_digest
.unwrap();

let b_pkg = MovePackage::new_initial(&build_test_modules("B"), u64::MAX, [&c_v1]).unwrap();

let b_v2 = MovePackage::new_initial(&build_test_modules("Bv2"), u64::MAX, [&c_v2]).unwrap();

let with_unpublished_deps = false;
let hash_modules = true;
let local_v1 = build_test_package("B").get_package_digest(with_unpublished_deps, hash_modules);
let local_v2 =
build_test_package("Bv2").get_package_digest(with_unpublished_deps, hash_modules);
let local_v1 = build_test_package("B").get_package_digest(with_unpublished_deps);
let local_v2 = build_test_package("Bv2").get_package_digest(with_unpublished_deps);

let hash_modules = true;
assert_ne!(b_pkg.digest(hash_modules), b_v2.digest(hash_modules));
assert_eq!(b_pkg.digest(hash_modules), local_v1);
assert_eq!(b_v2.digest(hash_modules), local_v2);
Expand Down
10 changes: 2 additions & 8 deletions crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,9 @@ fn build_upgrade_test_modules(test_dir: &str) -> (Vec<u8>, Vec<Vec<u8>>) {
let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
path.extend(["src", "unit_tests", "data", "move_upgrade", test_dir]);
let with_unpublished_deps = false;
let hash_modules = true;
let package = BuildConfig::new_for_testing().build(path).unwrap();
(
package
.get_package_digest(with_unpublished_deps, hash_modules)
.to_vec(),
package.get_package_digest(with_unpublished_deps).to_vec(),
package.get_package_bytes(with_unpublished_deps),
)
}
Expand All @@ -63,11 +60,8 @@ pub fn build_upgrade_test_modules_with_dep_addr(
path.extend(["src", "unit_tests", "data", "move_upgrade", test_dir]);
let package = build_test_modules_with_dep_addr(path, dep_original_addresses, dep_ids);
let with_unpublished_deps = false;
let hash_modules = true;
(
package
.get_package_digest(with_unpublished_deps, hash_modules)
.to_vec(),
package.get_package_digest(with_unpublished_deps).to_vec(),
package.get_package_bytes(with_unpublished_deps),
package.dependency_ids.published.values().cloned().collect(),
)
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-move-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,8 @@ impl CompiledPackage {
ids.into_iter().collect()
}

pub fn get_package_digest(&self, with_unpublished_deps: bool, hash_modules: bool) -> [u8; 32] {
pub fn get_package_digest(&self, with_unpublished_deps: bool) -> [u8; 32] {
let hash_modules = true;
MovePackage::compute_digest_for_modules_and_deps(
&self.get_package_bytes(with_unpublished_deps),
self.dependency_ids.published.values(),
Expand Down
7 changes: 1 addition & 6 deletions crates/sui-move/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ pub struct Build {
/// when dumping bytecode as base64)
#[clap(long, global = true)]
pub with_unpublished_dependencies: bool,
/// Use the legacy digest calculation algorithm
#[clap(long)]
legacy_digest: bool,
/// Whether we are printing in base64.
#[clap(long, global = true)]
pub dump_bytecode_as_base64: bool,
Expand Down Expand Up @@ -49,7 +46,6 @@ impl Build {
rerooted_path,
build_config,
self.with_unpublished_dependencies,
self.legacy_digest,
self.dump_bytecode_as_base64,
self.generate_struct_layouts,
self.lint,
Expand All @@ -60,7 +56,6 @@ impl Build {
rerooted_path: PathBuf,
config: MoveBuildConfig,
with_unpublished_deps: bool,
legacy_digest: bool,
dump_bytecode_as_base64: bool,
generate_struct_layouts: bool,
lint: bool,
Expand All @@ -84,7 +79,7 @@ impl Build {
json!({
"modules": pkg.get_package_base64(with_unpublished_deps),
"dependencies": json!(package_dependencies),
"digest": pkg.get_package_digest(with_unpublished_deps, !legacy_digest),
"digest": pkg.get_package_digest(with_unpublished_deps),
})
)
}
Expand Down
2 changes: 0 additions & 2 deletions crates/sui-move/src/unit_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ impl Test {
let rerooted_path = base::reroot_path(path)?;
// pre build for Sui-specific verifications
let with_unpublished_deps = false;
let legacy_digest = false;
let dump_bytecode_as_base64 = false;
let generate_struct_layouts: bool = false;
build::Build::execute_internal(
Expand All @@ -53,7 +52,6 @@ impl Test {
..build_config.clone()
},
with_unpublished_deps,
legacy_digest,
dump_bytecode_as_base64,
generate_struct_layouts,
self.lint,
Expand Down
5 changes: 1 addition & 4 deletions crates/sui-source-validation/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,8 @@ async fn upgrade_package(
) -> ObjectRef {
let package = compile_package(package);
let with_unpublished_deps = false;
let hash_modules = true;
let package_bytes = package.get_package_bytes(with_unpublished_deps);
let package_digest = package
.get_package_digest(with_unpublished_deps, hash_modules)
.to_vec();
let package_digest = package.get_package_digest(with_unpublished_deps).to_vec();
let package_deps = package.dependency_ids.published.into_values().collect();

upgrade_package_with_wallet(
Expand Down
9 changes: 2 additions & 7 deletions crates/sui/src/client_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,6 @@ pub enum SuiClientCommands {
#[clap(long)]
with_unpublished_dependencies: bool,

/// Use the legacy digest calculation algorithm
#[clap(long)]
legacy_digest: bool,

/// Instead of executing the transaction, serialize the bcs bytes of the unsigned transaction data
/// (TransactionData) using base64 encoding, and print out the string.
#[clap(long, required = false)]
Expand Down Expand Up @@ -641,7 +637,6 @@ impl SuiClientCommands {
gas_budget,
skip_dependency_verification,
with_unpublished_dependencies,
legacy_digest,
serialize_unsigned_transaction,
serialize_signed_transaction,
lint,
Expand Down Expand Up @@ -697,8 +692,8 @@ impl SuiClientCommands {
// policy at the moment. To change the policy you can call a Move function in the
// `package` module to change this policy.
let upgrade_policy = upgrade_cap.policy;
let package_digest = compiled_package
.get_package_digest(with_unpublished_dependencies, !legacy_digest);
let package_digest =
compiled_package.get_package_digest(with_unpublished_dependencies);

let data = client
.transaction_builder()
Expand Down
1 change: 0 additions & 1 deletion crates/sui/tests/cli_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,6 @@ async fn test_package_upgrade_command() -> Result<(), anyhow::Error> {
gas_budget: rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH,
skip_dependency_verification: false,
with_unpublished_dependencies: false,
legacy_digest: false,
serialize_unsigned_transaction: false,
serialize_signed_transaction: false,
lint: false,
Expand Down
5 changes: 1 addition & 4 deletions crates/transaction-fuzzer/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@ fn build_test_modules(test_dir: &str) -> (Vec<u8>, Vec<Vec<u8>>) {
let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
path.extend(["data", test_dir]);
let with_unpublished_deps = false;
let hash_modules = true;
let config = BuildConfig::new_for_testing();
let package = config.build(path).unwrap();
(
package
.get_package_digest(with_unpublished_deps, hash_modules)
.to_vec(),
package.get_package_digest(with_unpublished_deps).to_vec(),
package.get_package_bytes(with_unpublished_deps),
)
}
Expand Down

0 comments on commit f325c3c

Please sign in to comment.