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
amnn authored Aug 7, 2023
1 parent a880e50 commit d553135
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

2 comments on commit d553135

@vercel
Copy link

@vercel vercel bot commented on d553135 Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

sui-typescript-docs – ./sdk/docs

sui-typescript-docs.vercel.app
sui-typescript-docs-mysten-labs.vercel.app
sui-typescript-docs-git-main-mysten-labs.vercel.app

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 Validators 500/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 601 | 601 | 0      | 20            | 8043          | 12791         | 429,773,088,000       | 25,786,385,280,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 13  | 100 |

4 Validators 500/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 514 | 514 | 0      | 17            | 8719          | 13567         | 444,850,707,600       | 26,691,042,456,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 11  | 100 |

20 Validators 50/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 199 | 199 | 0      | 21            | 65            | 90            | 145,452,000,000       | 8,727,120,000,000          |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 26  | 50  |

20 Validators 50/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 194 | 194 | 0      | 60            | 1370          | 1973          | 175,772,925,600       | 10,546,375,536,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 26  | 54  |

Narwhal Benchmark Results

 SUMMARY:
-----------------------------------------
 + CONFIG:
 Faults: 0 node(s)
 Committee size: 4 node(s)
 Worker(s) per node: 1 worker(s)
 Collocate primary and workers: True
 Input rate: 50,000 tx/s
 Transaction size: 512 B
 Execution time: 0 s

 Header number of batches threshold: 32 digests
 Header maximum number of batches: 1,000 digests
 Max header delay: 2,000 ms
 GC depth: 50 round(s)
 Sync retry delay: 10,000 ms
 Sync retry nodes: 3 node(s)
 batch size: 500,000 B
 Max batch delay: 200 ms
 Max concurrent requests: 500,000 

 + RESULTS:
 Batch creation avg latency: 202 ms
 Header creation avg latency: -1 ms
 	Batch to header avg latency: -1 ms
 Header to certificate avg latency: 1 ms
 	Request vote outbound avg latency: 0 ms
 Certificate commit avg latency: 674 ms

 Consensus TPS: 0 tx/s
 Consensus BPS: 0 B/s
 Consensus latency: 0 ms

 End-to-end TPS: 0 tx/s
 End-to-end BPS: 0 B/s
 End-to-end latency: 0 ms
-----------------------------------------

Please sign in to comment.