Skip to content

Commit

Permalink
[data model] enforce max package size
Browse files Browse the repository at this point in the history
Enforce a max Move package size of 100 KB. Ideally, we would like to enforce a size limit of 500 KB, but the gas cost calculation for packages is quite conservative at the moment, and any attempt to set a more permissive limit goes over the max budget of 10,000 mist.
  • Loading branch information
sblackshear committed Nov 29, 2022
1 parent 7d5c7e4 commit e3f78e2
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 84 deletions.
2 changes: 1 addition & 1 deletion crates/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ pub fn store_package_and_init_modules<

// wrap the modules in an object, write it to the store
// The call to unwrap() will go away once we remove address owner from Immutable objects.
let package_object = Object::new_package(modules, ctx.digest());
let package_object = Object::new_package(modules, ctx.digest())?;
let id = package_object.id();
let changes = BTreeMap::from([(
id,
Expand Down
5 changes: 3 additions & 2 deletions crates/sui-adapter/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ pub fn get_genesis_context() -> TxContext {
fn create_genesis_module_objects() -> Genesis {
let sui_modules = sui_framework::get_sui_framework();
let std_modules = sui_framework::get_move_stdlib();
// unwraps safe because genesis packages should never exceed max size
let objects = vec![
Object::new_package(std_modules.clone(), TransactionDigest::genesis()),
Object::new_package(sui_modules.clone(), TransactionDigest::genesis()),
Object::new_package(std_modules.clone(), TransactionDigest::genesis()).unwrap(),
Object::new_package(sui_modules.clone(), TransactionDigest::genesis()).unwrap(),
];
let modules = vec![std_modules, sui_modules];
Genesis { objects, modules }
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-core/src/unit_tests/authority_aggregator_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub async fn init_local_authorities(
.into_iter()
.cloned()
.collect();
let pkg = Object::new_package(modules, TransactionDigest::genesis());
let pkg = Object::new_package(modules, TransactionDigest::genesis()).unwrap();
let pkg_ref = pkg.compute_object_reference();
genesis_objects.push(pkg);

Expand Down Expand Up @@ -443,7 +443,7 @@ async fn test_quorum_map_and_reduce_timeout() {
.into_iter()
.cloned()
.collect();
let pkg = Object::new_package(modules, TransactionDigest::genesis());
let pkg = Object::new_package(modules, TransactionDigest::genesis()).unwrap();
let pkg_ref = pkg.compute_object_reference();
let (addr1, key1): (_, AccountKeyPair) = get_key_pair();
let gas_object1 = Object::with_owner_for_testing(addr1);
Expand Down
44 changes: 42 additions & 2 deletions crates/sui-core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use sui_types::{
crypto::{get_key_pair, Signature},
crypto::{AccountKeyPair, AuthorityKeyPair, KeypairTraits},
messages::VerifiedTransaction,
object::{Owner, GAS_VALUE_FOR_TESTING, OBJECT_START_VERSION},
object::{Owner, GAS_VALUE_FOR_TESTING, MAX_MOVE_PACKAGE_SIZE, OBJECT_START_VERSION},
sui_system_state::SuiSystemState,
SUI_SYSTEM_STATE_OBJECT_ID,
};
Expand Down Expand Up @@ -771,6 +771,46 @@ async fn test_publish_non_existing_dependent_module() {
);
}

// make sure that publishing a package above the size limit fails
#[tokio::test]
async fn test_package_size_limit() {
let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
let gas_payment_object_id = ObjectID::random();
let gas_payment_object =
Object::with_id_owner_gas_for_testing(gas_payment_object_id, sender, u64::MAX);
let gas_payment_object_ref = gas_payment_object.compute_object_reference();
let mut package = Vec::new();
let mut package_size = 0;
// create a package larger than the max size
while package_size <= MAX_MOVE_PACKAGE_SIZE {
let mut module = file_format::empty_module();
// generate unique name
module.identifiers[0] = Identifier::new(format!("TestModule{:?}", package_size)).unwrap();
let module_bytes = {
let mut bytes = Vec::new();
module.serialize(&mut bytes).unwrap();
bytes
};
package_size += module_bytes.len() as u64;
package.push(module_bytes);
}
let authority = init_state_with_objects(vec![gas_payment_object]).await;
let data = TransactionData::new_module(sender, gas_payment_object_ref, package, MAX_GAS);
let transaction = to_sender_signed_transaction(data, &sender_key);
let response = send_and_confirm_transaction(&authority, transaction)
.await
.unwrap();
assert_eq!(
response.signed_effects.unwrap().status,
ExecutionStatus::Failure {
error: ExecutionFailureStatus::MovePackageTooBig {
object_size: package_size,
max_object_size: MAX_MOVE_PACKAGE_SIZE
}
}
)
}

#[tokio::test]
async fn test_handle_move_transaction() {
let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
Expand Down Expand Up @@ -2354,7 +2394,7 @@ pub async fn init_state_with_ids_and_object_basics<
.into_iter()
.cloned()
.collect();
let pkg = Object::new_package(modules, TransactionDigest::genesis());
let pkg = Object::new_package(modules, TransactionDigest::genesis()).unwrap();
let pkg_ref = pkg.compute_object_reference();
state.insert_genesis_object(pkg).await;
(state, pkg_ref)
Expand Down
63 changes: 34 additions & 29 deletions crates/sui-core/tests/staged/sui.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -154,91 +154,96 @@ ExecutionFailureStatus:
6:
MoveObjectTooBig:
STRUCT:
- object_size: U32
- max_object_size: U32
- object_size: U64
- max_object_size: U64
7:
InvalidTransferObject: UNIT
MovePackageTooBig:
STRUCT:
- object_size: U64
- max_object_size: U64
8:
InvalidTransferSui: UNIT
InvalidTransferObject: UNIT
9:
InvalidTransferSuiInsufficientBalance: UNIT
InvalidTransferSui: UNIT
10:
InvalidCoinObject: UNIT
InvalidTransferSuiInsufficientBalance: UNIT
11:
InvalidCoinMetadataObject: UNIT
InvalidCoinObject: UNIT
12:
EmptyInputCoins: UNIT
InvalidCoinMetadataObject: UNIT
13:
EmptyRecipients: UNIT
EmptyInputCoins: UNIT
14:
RecipientsAmountsArityMismatch: UNIT
EmptyRecipients: UNIT
15:
InsufficientBalance: UNIT
RecipientsAmountsArityMismatch: UNIT
16:
CoinTypeMismatch: UNIT
InsufficientBalance: UNIT
17:
NonEntryFunctionInvoked: UNIT
CoinTypeMismatch: UNIT
18:
EntryTypeArityMismatch: UNIT
NonEntryFunctionInvoked: UNIT
19:
EntryTypeArityMismatch: UNIT
20:
EntryArgumentError:
NEWTYPE:
TYPENAME: EntryArgumentError
20:
21:
EntryTypeArgumentError:
NEWTYPE:
TYPENAME: EntryTypeArgumentError
21:
22:
CircularObjectOwnership:
NEWTYPE:
TYPENAME: CircularObjectOwnership
22:
23:
InvalidChildObjectArgument:
NEWTYPE:
TYPENAME: InvalidChildObjectArgument
23:
24:
InvalidSharedByValue:
NEWTYPE:
TYPENAME: InvalidSharedByValue
24:
25:
TooManyChildObjects:
STRUCT:
- object:
TYPENAME: ObjectID
25:
26:
InvalidParentDeletion:
STRUCT:
- parent:
TYPENAME: ObjectID
- kind:
OPTION:
TYPENAME: DeleteKind
26:
27:
InvalidParentFreezing:
STRUCT:
- parent:
TYPENAME: ObjectID
27:
PublishErrorEmptyPackage: UNIT
28:
PublishErrorNonZeroAddress: UNIT
PublishErrorEmptyPackage: UNIT
29:
PublishErrorDuplicateModule: UNIT
PublishErrorNonZeroAddress: UNIT
30:
SuiMoveVerificationError: UNIT
PublishErrorDuplicateModule: UNIT
31:
SuiMoveVerificationError: UNIT
32:
MovePrimitiveRuntimeError:
NEWTYPE:
OPTION:
TYPENAME: MoveLocation
32:
33:
MoveAbort:
TUPLE:
- TYPENAME: MoveLocation
- U64
33:
VMVerificationOrDeserializationError: UNIT
34:
VMVerificationOrDeserializationError: UNIT
35:
VMInvariantViolation: UNIT
ExecutionStatus:
ENUM:
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-json-rpc-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ impl TryInto<Object> for SuiObject<SuiRawData> {
)?
})
}
SuiRawData::Package(p) => Data::Package(MovePackage::new(p.id, &p.module_map)),
SuiRawData::Package(p) => Data::Package(MovePackage::new(p.id, &p.module_map)?),
};
Ok(Object {
data,
Expand Down
9 changes: 6 additions & 3 deletions crates/sui-json/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,8 @@ fn test_basic_args_linter_top_level() {
let path =
Path::new(env!("CARGO_MANIFEST_DIR")).join("../../sui_programmability/examples/nfts");
let compiled_modules = BuildConfig::default().build(path).unwrap().into_modules();
let example_package = Object::new_package(compiled_modules, TransactionDigest::genesis());
let example_package =
Object::new_package(compiled_modules, TransactionDigest::genesis()).unwrap();
let example_package = example_package.data.try_as_package().unwrap();

let module = Identifier::new("geniteam").unwrap();
Expand Down Expand Up @@ -504,7 +505,8 @@ fn test_basic_args_linter_top_level() {
let path =
Path::new(env!("CARGO_MANIFEST_DIR")).join("../../sui_programmability/examples/basics");
let compiled_modules = BuildConfig::default().build(path).unwrap().into_modules();
let example_package = Object::new_package(compiled_modules, TransactionDigest::genesis());
let example_package =
Object::new_package(compiled_modules, TransactionDigest::genesis()).unwrap();
let framework_pkg = example_package.data.try_as_package().unwrap();

let module = Identifier::new("object_basics").unwrap();
Expand Down Expand Up @@ -583,7 +585,8 @@ fn test_basic_args_linter_top_level() {
let path = Path::new(env!("CARGO_MANIFEST_DIR"))
.join("../sui-core/src/unit_tests/data/entry_point_vector");
let compiled_modules = BuildConfig::default().build(path).unwrap().into_modules();
let example_package = Object::new_package(compiled_modules, TransactionDigest::genesis());
let example_package =
Object::new_package(compiled_modules, TransactionDigest::genesis()).unwrap();
let example_package = example_package.data.try_as_package().unwrap();

let module = Identifier::new("entry_point_vector").unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-transaction-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl TransactionBuilder {
.try_as_package()
.cloned()
.ok_or_else(|| anyhow!("Object [{}] is not a move package.", package_id))?;
let package: MovePackage = MovePackage::new(package.id, &package.module_map);
let package: MovePackage = MovePackage::new(package.id, &package.module_map)?;

let json_args = resolve_move_function_args(
&package,
Expand Down
9 changes: 7 additions & 2 deletions crates/sui-types/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1229,8 +1229,12 @@ pub enum ExecutionFailureStatus {
FunctionNotFound,
InvariantViolation,
MoveObjectTooBig {
object_size: u32,
max_object_size: u32,
object_size: u64,
max_object_size: u64,
},
MovePackageTooBig {
object_size: u64,
max_object_size: u64,
},

//
Expand Down Expand Up @@ -1408,6 +1412,7 @@ impl Display for ExecutionFailureStatus {
}
ExecutionFailureStatus::ModuleNotFound => write!(f, "Module Not Found."),
ExecutionFailureStatus::MoveObjectTooBig { object_size, max_object_size } => write!(f, "Move object with size {object_size} is larger than the maximum object size {max_object_size}"),
ExecutionFailureStatus::MovePackageTooBig { object_size, max_object_size } => write!(f, "Move package with size {object_size} is larger than the maximum object size {max_object_size}"),
ExecutionFailureStatus::FunctionNotFound => write!(f, "Function Not Found."),
ExecutionFailureStatus::InvariantViolation => write!(f, "INVARIANT VIOLATION."),
ExecutionFailureStatus::InvalidTransferObject => write!(
Expand Down
73 changes: 46 additions & 27 deletions crates/sui-types/src/move_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

use crate::{
base_types::ObjectID,
error::{SuiError, SuiResult},
error::{ExecutionError, ExecutionErrorKind, SuiError, SuiResult},
object::MAX_MOVE_PACKAGE_SIZE,
};
use move_binary_format::access::ModuleAccess;
use move_binary_format::binary_views::BinaryIndexedView;
Expand Down Expand Up @@ -34,11 +35,53 @@ pub struct MovePackage {
}

impl MovePackage {
pub fn new(id: ObjectID, module_map: &BTreeMap<String, Vec<u8>>) -> Self {
Self {
pub fn new(
id: ObjectID,
module_map: &BTreeMap<String, Vec<u8>>,
) -> Result<Self, ExecutionError> {
let pkg = Self {
id,
module_map: module_map.clone(),
};
let object_size = pkg.size() as u64;
if object_size > MAX_MOVE_PACKAGE_SIZE {
return Err(ExecutionErrorKind::MovePackageTooBig {
object_size,
max_object_size: MAX_MOVE_PACKAGE_SIZE,
}
.into());
}
Ok(pkg)
}

pub fn from_module_iter<T: IntoIterator<Item = CompiledModule>>(
iter: T,
) -> Result<Self, ExecutionError> {
let mut iter = iter.into_iter().peekable();
let id = ObjectID::from(
*iter
.peek()
.expect("Tried to build a Move package from an empty iterator of Compiled modules")
.self_id()
.address(),
);

Self::new(
id,
&iter
.map(|module| {
let mut bytes = Vec::new();
module.serialize(&mut bytes).unwrap();
(module.self_id().name().to_string(), bytes)
})
.collect(),
)
}

/// Return the size of the package in bytes. Only count the bytes of the modules themselves--the
/// fact that we store them in a map is an implementation detail
pub fn size(&self) -> usize {
self.module_map.values().map(|b| b.len()).sum()
}

pub fn id(&self) -> ObjectID {
Expand Down Expand Up @@ -113,27 +156,3 @@ where
}
Ok(normalized_modules)
}

impl FromIterator<CompiledModule> for MovePackage {
fn from_iter<T: IntoIterator<Item = CompiledModule>>(iter: T) -> Self {
let mut iter = iter.into_iter().peekable();
let id = ObjectID::from(
*iter
.peek()
.expect("Tried to build a Move package from an empty iterator of Compiled modules")
.self_id()
.address(),
);

Self::new(
id,
&iter
.map(|module| {
let mut bytes = Vec::new();
module.serialize(&mut bytes).unwrap();
(module.self_id().name().to_string(), bytes)
})
.collect(),
)
}
}
Loading

0 comments on commit e3f78e2

Please sign in to comment.