Skip to content

Commit

Permalink
[sui-execution] Test to detect breaking encapsulation
Browse files Browse the repository at this point in the history
Adds a test that checks that the "sui-execution" crate dominates all
execution layer crates in the dependency graphs of binaries where the
protocol config should determine which version of the execution layer
is used.

This helps to make sure that a crate does not inadvertantly bypass
versioning.  The test caught some existing discrepencies in the
dependency graph:

- `sui-core` depending on `sui-test-transaction-builder` (which
  depends on `sui-move-build`) but only using it for tests.  Fixed by
  making `sui-test-transaction-builder` a `dev-dependency`.
- `sui-storage` depending on `sui-simulator` (which in turn depends on
  `sui-move-build`) but only to use `fastcrypto` which it re-exports.
  Fixed by changing `sui-storage` to depend on `fastcrypto` instead of
  `sui-move-build`.
- `sui-sdk` depending on `sui-test-transaction-builder` to implement
  test-specific things, which probably should not be part of our
  public-facing SDK.  Fixed by moving internal test helpers out of the
  public SDK, back into `sui-test-transaction-builder`. This is the
  majority of the change.

Test Plan:

Run the newly introduced test:

```
sui-execution$ cargo nextest run
```

Run all the other tests, for the refactorings:

```
sui$ cargo simtest
sui$ env SUI_SKIP_SIMTESTS=1 cargo nextest run
```
  • Loading branch information
amnn committed Jul 7, 2023
1 parent be8ae5e commit 9f777e6
Show file tree
Hide file tree
Showing 33 changed files with 554 additions and 374 deletions.
14 changes: 10 additions & 4 deletions Cargo.lock

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

5 changes: 2 additions & 3 deletions crates/sui-cluster-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use sui_json_rpc_types::{
SuiTransactionBlockResponseOptions, TransactionBlockBytes,
};
use sui_sdk::wallet_context::WalletContext;
use sui_test_transaction_builder::batch_make_transfer_transactions;
use sui_types::base_types::TransactionDigest;
use sui_types::object::Owner;
use sui_types::quorum_driver_types::ExecuteTransactionRequestType;
Expand Down Expand Up @@ -128,9 +129,7 @@ impl TestContext {
/// See `make_transactions_with_wallet_context` for potential caveats
/// of this helper function.
pub async fn make_transactions(&self, max_txn_num: usize) -> Vec<Transaction> {
self.get_wallet()
.batch_make_transfer_transactions(max_txn_num)
.await
batch_make_transfer_transactions(self.get_wallet(), max_txn_num).await
}

pub async fn build_transaction_remotely(
Expand Down
6 changes: 2 additions & 4 deletions crates/sui-cluster-test/src/test_case/coin_index_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use sui_json::SuiJsonValue;
use sui_json_rpc_types::ObjectChange;
use sui_json_rpc_types::SuiTransactionBlockResponse;
use sui_json_rpc_types::{Balance, SuiTransactionBlockResponseOptions};
use sui_test_transaction_builder::make_staking_transaction;
use sui_types::base_types::{ObjectID, ObjectRef};
use sui_types::gas_coin::GAS;
use sui_types::object::Owner;
Expand Down Expand Up @@ -103,10 +104,7 @@ impl TestCaseImpl for CoinIndexTest {
.get(0)
.unwrap()
.sui_address;
let txn = ctx
.get_wallet()
.make_staking_transaction(validator_addr)
.await;
let txn = make_staking_transaction(ctx.get_wallet(), validator_addr).await;

let response = client
.quorum_driver_api()
Expand Down
24 changes: 12 additions & 12 deletions crates/sui-cluster-test/src/test_case/shared_object_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{helper::ObjectChecker, TestCaseImpl, TestContext};
use async_trait::async_trait;
use sui_json_rpc_types::{SuiExecutionStatus, SuiTransactionBlockEffectsAPI};
use sui_sdk::wallet_context::WalletContext;
use sui_test_transaction_builder::{increment_counter, publish_basics_package_and_make_counter};
use sui_types::object::Owner;
use tracing::info;

Expand All @@ -28,18 +29,17 @@ impl TestCaseImpl for SharedCounterTest {

let wallet_context: &WalletContext = ctx.get_wallet();
let address = ctx.get_wallet_address();
let (package_ref, (counter_id, initial_counter_version, _)) = wallet_context
.publish_basics_package_and_make_counter()
.await;
let response = wallet_context
.increment_counter(
address,
None,
package_ref.0,
counter_id,
initial_counter_version,
)
.await;
let (package_ref, (counter_id, initial_counter_version, _)) =
publish_basics_package_and_make_counter(wallet_context).await;
let response = increment_counter(
wallet_context,
address,
None,
package_ref.0,
counter_id,
initial_counter_version,
)
.await;
assert_eq!(
*response.effects.as_ref().unwrap().status(),
SuiExecutionStatus::Success,
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ sui-network.workspace = true
sui-protocol-config.workspace = true
sui-simulator.workspace = true
sui-storage.workspace = true
sui-test-transaction-builder.workspace = true
sui-types.workspace = true
workspace-hack = { version = "0.1", path = "../workspace-hack" }

Expand All @@ -92,6 +91,7 @@ serde_yaml.workspace = true
move-symbol-pool.workspace = true

narwhal-test-utils.workspace = true
sui-test-transaction-builder.workspace = true
sui-types = { workspace = true, features = ["test-utils"] }

[target.'cfg(not(target_env = "msvc"))'.dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-cost/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ strum.workspace = true
strum_macros.workspace = true
bcs.workspace = true
sui-core.workspace = true
sui-test-transaction-builder.workspace = true

[dev-dependencies]
insta.workspace = true
test-cluster.workspace = true
sui-config.workspace = true
sui-swarm-config.workspace = true
sui-test-transaction-builder.workspace = true
sui-move-build.workspace = true
move-cli.workspace = true
move-disassembler.workspace = true
Expand Down
7 changes: 3 additions & 4 deletions crates/sui-cost/tests/empirical_transaction_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use strum_macros::Display;
use strum_macros::EnumString;
use sui_json_rpc_types::SuiTransactionBlockEffectsAPI;
use sui_swarm_config::genesis_config::{AccountConfig, DEFAULT_GAS_AMOUNT};
use sui_test_transaction_builder::publish_basics_package_and_make_counter;
use sui_test_transaction_builder::TestTransactionBuilder;
use sui_types::base_types::{ObjectRef, SuiAddress};
use sui_types::coin::PAY_JOIN_FUNC_NAME;
Expand Down Expand Up @@ -96,10 +97,8 @@ async fn create_txes(
) -> BTreeMap<CommonTransactionCosts, TransactionData> {
// Initial preparations to create a shared counter. This needs to be done first to not interfere
// with the use of gas objects in the rest of this function.
let (counter_package, counter) = test_cluster
.wallet
.publish_basics_package_and_make_counter()
.await;
let (counter_package, counter) =
publish_basics_package_and_make_counter(&test_cluster.wallet).await;
let counter_package_id = counter_package.0;
let (counter_id, counter_initial_shared_version) = (counter.0, counter.1);

Expand Down
6 changes: 2 additions & 4 deletions crates/sui-e2e-tests/tests/checkpoint_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@

use std::time::Duration;
use sui_macros::sim_test;
use sui_test_transaction_builder::make_transfer_sui_transaction;
use test_cluster::TestClusterBuilder;

#[sim_test]
async fn basic_checkpoints_integration_test() {
let test_cluster = TestClusterBuilder::new().build().await;
let tx = test_cluster
.wallet
.make_transfer_sui_transaction(None, None)
.await;
let tx = make_transfer_sui_transaction(&test_cluster.wallet, None, None).await;
let digest = *tx.digest();
test_cluster.execute_transaction(tx).await;

Expand Down
Loading

0 comments on commit 9f777e6

Please sign in to comment.