Skip to content

Commit

Permalink
Limit number of pacakges/upgrades in a single transaction block (#13589)
Browse files Browse the repository at this point in the history
## Description 

There is no point in having a significant number of publish/upgrade in a
single transaction block.
Cost will be higher after a relatively low number because of memory
compound effects and tracking upgrade caps/packages will be harder.
So we are adding a protocol config controllable cap to the number of
publish/upgrades per PTB

## Test Plan 

New tests added for publish, upgrade and mixed

---
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
- [ ] 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
  • Loading branch information
dariorussi authored Sep 4, 2023
1 parent 0f595ec commit 3b1ea05
Show file tree
Hide file tree
Showing 10 changed files with 345 additions and 3 deletions.
118 changes: 118 additions & 0 deletions crates/sui-core/src/unit_tests/move_integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use sui_types::{
error::ExecutionErrorKind,
programmable_transaction_builder::ProgrammableTransactionBuilder,
utils::to_sender_signed_transaction,
SUI_FRAMEWORK_PACKAGE_ID,
};

use move_core_types::language_storage::TypeTag;
Expand All @@ -32,6 +33,7 @@ use sui_types::{
use std::{collections::HashSet, path::PathBuf};
use std::{env, str::FromStr};
use sui_types::execution_status::{CommandArgumentError, ExecutionFailureStatus, ExecutionStatus};
use sui_types::move_package::UpgradeCap;

#[tokio::test]
#[cfg_attr(msim, ignore)]
Expand Down Expand Up @@ -2784,6 +2786,7 @@ async fn test_object_no_id_error() {
err_str.contains("SuiMoveVerificationError")
&& err_str.contains("First field of struct NotObject must be 'id'"));
}

pub fn build_test_package(test_dir: &str, with_unpublished_deps: bool) -> Vec<Vec<u8>> {
let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
path.extend(["src", "unit_tests", "data", test_dir]);
Expand All @@ -2793,6 +2796,20 @@ pub fn build_test_package(test_dir: &str, with_unpublished_deps: bool) -> Vec<Ve
.get_package_bytes(with_unpublished_deps)
}

pub fn build_package(
code_dir: &str,
with_unpublished_deps: bool,
) -> (Vec<u8>, Vec<Vec<u8>>, Vec<ObjectID>) {
move_package::package_hooks::register_package_hooks(Box::new(SuiPackageHooks));
let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
path.extend(["src", "unit_tests", "data", code_dir]);
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();
(digest.to_vec(), modules, dependencies)
}

pub async fn build_and_try_publish_test_package(
authority: &AuthorityState,
sender: &SuiAddress,
Expand Down Expand Up @@ -2896,6 +2913,107 @@ pub async fn build_and_publish_test_package_with_upgrade_cap(
(package.0, upgrade_cap.0)
}

pub async fn collect_packages_and_upgrade_caps(
authority: &AuthorityState,
effects: &TransactionEffects,
) -> HashMap<ObjectID, ObjectRef> {
let packages: HashMap<_, _> = effects
.created()
.into_iter()
.filter(|(_, owner)| matches!(owner, Owner::Immutable))
.map(|(package, _)| (package.0, package))
.collect();
let mut caps = HashMap::new();
for (obj_ref, owner) in effects.created() {
if !matches!(owner, Owner::AddressOwner(_)) {
continue;
}
let cap = authority.get_object(&obj_ref.0).await.unwrap().unwrap();
let bcs = cap.data.try_as_move().unwrap().contents();
let obj: UpgradeCap = bcs::from_bytes(bcs).unwrap();
let pkg = packages.get(&obj.package.bytes).unwrap();
caps.insert(pkg.0, obj_ref);
}
caps
}

pub async fn run_multi_txns(
authority: &AuthorityState,
sender: SuiAddress,
sender_key: &AccountKeyPair,
gas_object_id: &ObjectID,
builder: ProgrammableTransactionBuilder,
) -> Result<(CertifiedTransaction, SignedTransactionEffects), SuiError> {
// build the transaction data
let pt = builder.finish();
let gas_object = authority.get_object(gas_object_id).await.unwrap();
let gas_object_ref = gas_object.unwrap().compute_object_reference();
let gas_price = authority.reference_gas_price_for_testing().unwrap();
let gas_budget = pt.non_system_packages_to_be_published().count() as u64
* TEST_ONLY_GAS_UNIT_FOR_PUBLISH
* gas_price;
let data =
TransactionData::new_programmable(sender, vec![gas_object_ref], pt, gas_budget, gas_price);
// run the transaction
let transaction = to_sender_signed_transaction(data, sender_key);
send_and_confirm_transaction(authority, transaction).await
}

pub fn build_multi_publish_txns(
builder: &mut ProgrammableTransactionBuilder,
sender: SuiAddress,
packages: Vec<(Vec<Vec<u8>>, Vec<ObjectID>)>,
) {
for (modules, dep_ids) in packages {
let upgrade_cap = builder.publish_upgradeable(modules, dep_ids);
builder.transfer_arg(sender, upgrade_cap);
}
}

pub struct UpgradeData {
pub package_id: ObjectID,
pub upgrade_cap: ObjectRef,
pub policy: u8,
pub digest: Vec<u8>,
pub dep_ids: Vec<ObjectID>,
pub modules: Vec<Vec<u8>>,
}

pub fn build_multi_upgrade_txns(
builder: &mut ProgrammableTransactionBuilder,
package_upgrades: Vec<UpgradeData>,
) {
// build the programmable transaction
for package_upgrade in package_upgrades {
let package_id = package_upgrade.package_id;
let cap = builder
.obj(ObjectArg::ImmOrOwnedObject(package_upgrade.upgrade_cap))
.unwrap();
let policy = builder.pure(package_upgrade.policy).unwrap();
let digest = builder.pure(package_upgrade.digest).unwrap();
let ticket = builder.programmable_move_call(
SUI_FRAMEWORK_PACKAGE_ID,
Identifier::new("package").unwrap(),
Identifier::new("authorize_upgrade").unwrap(),
vec![],
vec![cap, policy, digest],
);
let receipt = builder.upgrade(
package_id,
ticket,
package_upgrade.dep_ids,
package_upgrade.modules,
);
builder.programmable_move_call(
SUI_FRAMEWORK_PACKAGE_ID,
Identifier::new("package").unwrap(),
Identifier::new("commit_upgrade").unwrap(),
vec![],
vec![cap, receipt],
);
}
}

async fn check_latest_object_ref(
authority: &AuthorityState,
object_ref: &ObjectRef,
Expand Down
66 changes: 66 additions & 0 deletions crates/sui-core/src/unit_tests/move_package_publish_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ use sui_types::{
error::SuiError,
};

use crate::authority::move_integration_tests::{
build_multi_publish_txns, build_package, run_multi_txns,
};
use expect_test::expect;
use std::env;
use std::fs::File;
Expand All @@ -30,6 +33,7 @@ use std::{collections::HashSet, path::PathBuf};
use sui_framework::BuiltInFramework;
use sui_types::effects::TransactionEffectsAPI;
use sui_types::execution_status::{ExecutionFailureStatus, ExecutionStatus};
use sui_types::programmable_transaction_builder::ProgrammableTransactionBuilder;

#[tokio::test]
#[cfg_attr(msim, ignore)]
Expand Down Expand Up @@ -422,3 +426,65 @@ async fn test_publish_extraneous_bytes_modules() {
}
)
}

#[tokio::test]
#[cfg_attr(msim, ignore)]
async fn test_publish_max_packages() {
let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
let gas_object_id = ObjectID::random();
let authority = init_state_with_ids(vec![(sender, gas_object_id)]).await;

let (_, modules, dependencies) = build_package("object_basics", false);

// push max number of packages allowed to publish
let max_pub_cmd = authority
.epoch_store_for_testing()
.protocol_config()
.max_publish_or_upgrade_per_ptb_as_option()
.unwrap_or(0);
assert!(max_pub_cmd > 0);
let packages = vec![(modules, dependencies); max_pub_cmd as usize];

let mut builder = ProgrammableTransactionBuilder::new();
build_multi_publish_txns(&mut builder, sender, packages);
let result = run_multi_txns(&authority, sender, &sender_key, &gas_object_id, builder)
.await
.unwrap()
.1;
let effects = result.into_data();
assert_eq!(effects.status(), &ExecutionStatus::Success);
}

#[tokio::test]
#[cfg_attr(msim, ignore)]
async fn test_publish_more_than_max_packages_error() {
let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
let gas_object_id = ObjectID::random();
let authority = init_state_with_ids(vec![(sender, gas_object_id)]).await;

let (_, modules, dependencies) = build_package("object_basics", false);

// push max number of packages allowed to publish
let max_pub_cmd = authority
.epoch_store_for_testing()
.protocol_config()
.max_publish_or_upgrade_per_ptb_as_option()
.unwrap_or(0);
assert!(max_pub_cmd > 0);
let packages = vec![(modules, dependencies); (max_pub_cmd + 1) as usize];

let mut builder = ProgrammableTransactionBuilder::new();
build_multi_publish_txns(&mut builder, sender, packages);
let err = run_multi_txns(&authority, sender, &sender_key, &gas_object_id, builder)
.await
.unwrap_err();
assert_eq!(
err,
SuiError::UserInputError {
error: UserInputError::MaxPublishCountExceeded {
max_publish_commands: max_pub_cmd,
publish_count: max_pub_cmd + 1,
}
}
);
}
129 changes: 127 additions & 2 deletions crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@ use sui_types::{
};

use std::{collections::BTreeSet, path::PathBuf, str::FromStr, sync::Arc};
use sui_types::effects::{TransactionEffects, TransactionEffectsV1};
use sui_types::effects::{TransactionEffects, TransactionEffectsAPI, TransactionEffectsV1};
use sui_types::error::{SuiError, UserInputError};
use sui_types::execution_status::{
CommandArgumentError, ExecutionFailureStatus, PackageUpgradeError,
CommandArgumentError, ExecutionFailureStatus, ExecutionStatus, PackageUpgradeError,
};

use crate::authority::authority_tests::init_state_with_ids;
use crate::authority::move_integration_tests::{
build_multi_publish_txns, build_multi_upgrade_txns, build_package,
collect_packages_and_upgrade_caps, run_multi_txns, UpgradeData,
};
use crate::authority::test_authority_builder::TestAuthorityBuilder;
use crate::authority::{
authority_test_utils::build_test_modules_with_dep_addr,
Expand Down Expand Up @@ -1312,3 +1318,122 @@ async fn test_upgrade_cross_module_refs() {
assert!(effects.status.is_ok(), "{:#?}", effects.status);
assert_eq!(effects.created.len(), 6);
}

#[tokio::test]
async fn test_upgrade_max_packages() {
let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
let gas_object_id = ObjectID::random();
let authority = init_state_with_ids(vec![(sender, gas_object_id)]).await;

//
// Build and publich max number of packages allowed
let (_, modules, dependencies) = build_package("move_upgrade/base", false);

// push max number of packages allowed to publish
let max_pub_cmd = authority
.epoch_store_for_testing()
.protocol_config()
.max_publish_or_upgrade_per_ptb_as_option()
.unwrap_or(0);
assert!(max_pub_cmd > 0);
let packages = vec![(modules, dependencies); max_pub_cmd as usize];

let mut builder = ProgrammableTransactionBuilder::new();
build_multi_publish_txns(&mut builder, sender, packages);
let result = run_multi_txns(&authority, sender, &sender_key, &gas_object_id, builder)
.await
.unwrap()
.1;
let effects = result.into_data();
assert_eq!(effects.status(), &ExecutionStatus::Success);

// collect package and upgrade caps
let (digest, modules, dep_ids) = build_package("move_upgrade/base", false);
let packages_and_upgrades = collect_packages_and_upgrade_caps(&authority, &effects).await;
// (package id, upgrade cap ref, policy, digest, dep ids, modules)
let mut package_upgrades: Vec<UpgradeData> = vec![];
for (package_id, upgrade_cap) in packages_and_upgrades {
package_upgrades.push(UpgradeData {
package_id,
upgrade_cap,
policy: UpgradePolicy::COMPATIBLE,
digest: digest.clone(),
dep_ids: dep_ids.clone(),
modules: modules.clone(),
});
}

// Upgrade all packages
let mut builder = ProgrammableTransactionBuilder::new();
build_multi_upgrade_txns(&mut builder, package_upgrades);
let result = run_multi_txns(&authority, sender, &sender_key, &gas_object_id, builder)
.await
.unwrap()
.1;
let effects = result.into_data();
assert_eq!(effects.status(), &ExecutionStatus::Success);
}

#[tokio::test]
async fn test_upgrade_more_than_max_packages_error() {
let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
let gas_object_id = ObjectID::random();
let authority = init_state_with_ids(vec![(sender, gas_object_id)]).await;

//
// Build and publich max number of packages allowed
let (_, modules, dependencies) = build_package("move_upgrade/base", false);

// push max number of packages allowed to publish
let max_pub_cmd = authority
.epoch_store_for_testing()
.protocol_config()
.max_publish_or_upgrade_per_ptb_as_option()
.unwrap_or(0);
assert!(max_pub_cmd > 0);
let packages = vec![(modules, dependencies); max_pub_cmd as usize];

let mut builder = ProgrammableTransactionBuilder::new();
build_multi_publish_txns(&mut builder, sender, packages);
let result = run_multi_txns(&authority, sender, &sender_key, &gas_object_id, builder)
.await
.unwrap()
.1;
let effects = result.into_data();
assert_eq!(effects.status(), &ExecutionStatus::Success);

// collect package and upgrade caps
let (digest, modules, dep_ids) = build_package("move_upgrade/base", false);
let packages_and_upgrades = collect_packages_and_upgrade_caps(&authority, &effects).await;
// (package id, upgrade cap ref, policy, digest, dep ids, modules)
let mut package_upgrades: Vec<UpgradeData> = vec![];
for (package_id, upgrade_cap) in packages_and_upgrades {
package_upgrades.push(UpgradeData {
package_id,
upgrade_cap,
policy: UpgradePolicy::COMPATIBLE,
digest: digest.clone(),
dep_ids: dep_ids.clone(),
modules: modules.clone(),
});
}
let (_, modules, dependencies) = build_package("object_basics", false);
let packages = vec![(modules, dependencies); 2];

// Upgrade all packages
let mut builder = ProgrammableTransactionBuilder::new();
build_multi_upgrade_txns(&mut builder, package_upgrades);
build_multi_publish_txns(&mut builder, sender, packages);
let err = run_multi_txns(&authority, sender, &sender_key, &gas_object_id, builder)
.await
.unwrap_err();
assert_eq!(
err,
SuiError::UserInputError {
error: UserInputError::MaxPublishCountExceeded {
max_publish_commands: max_pub_cmd,
publish_count: max_pub_cmd + 2,
}
}
);
}
1 change: 1 addition & 0 deletions crates/sui-open-rpc/spec/openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -1718,6 +1718,7 @@
"max_programmable_tx_commands": {
"u32": "1024"
},
"max_publish_or_upgrade_per_ptb": null,
"max_pure_argument_size": {
"u32": "16384"
},
Expand Down
Loading

1 comment on commit 3b1ea05

@vercel
Copy link

@vercel vercel bot commented on 3b1ea05 Sep 4, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.