diff --git a/crates/sui-adapter/src/adapter.rs b/crates/sui-adapter/src/adapter.rs index 851368bfb68d6..cd12753f9ee5f 100644 --- a/crates/sui-adapter/src/adapter.rs +++ b/crates/sui-adapter/src/adapter.rs @@ -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, diff --git a/crates/sui-adapter/src/genesis.rs b/crates/sui-adapter/src/genesis.rs index 72898b4de70b8..e0a61175ce56c 100644 --- a/crates/sui-adapter/src/genesis.rs +++ b/crates/sui-adapter/src/genesis.rs @@ -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 } diff --git a/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs b/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs index e30b967e3f299..ac2793f23456b 100644 --- a/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs +++ b/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs @@ -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); @@ -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); diff --git a/crates/sui-core/src/unit_tests/authority_tests.rs b/crates/sui-core/src/unit_tests/authority_tests.rs index 0c8658723640d..62b4754f56a50 100644 --- a/crates/sui-core/src/unit_tests/authority_tests.rs +++ b/crates/sui-core/src/unit_tests/authority_tests.rs @@ -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, }; @@ -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(); @@ -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) diff --git a/crates/sui-core/tests/staged/sui.yaml b/crates/sui-core/tests/staged/sui.yaml index 443a616a38860..7d3a443f8e2db 100644 --- a/crates/sui-core/tests/staged/sui.yaml +++ b/crates/sui-core/tests/staged/sui.yaml @@ -154,58 +154,63 @@ 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: @@ -213,32 +218,32 @@ ExecutionFailureStatus: - 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: diff --git a/crates/sui-json-rpc-types/src/lib.rs b/crates/sui-json-rpc-types/src/lib.rs index f3c1e71393461..5e92a6b2f7280 100644 --- a/crates/sui-json-rpc-types/src/lib.rs +++ b/crates/sui-json-rpc-types/src/lib.rs @@ -544,7 +544,7 @@ impl TryInto for SuiObject { )? }) } - 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, diff --git a/crates/sui-json/src/tests.rs b/crates/sui-json/src/tests.rs index fd2c640619b26..5b9e3058c09e1 100644 --- a/crates/sui-json/src/tests.rs +++ b/crates/sui-json/src/tests.rs @@ -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(); @@ -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(); @@ -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(); diff --git a/crates/sui-transaction-builder/src/lib.rs b/crates/sui-transaction-builder/src/lib.rs index 20f7074c6f6ae..21530bf1d9111 100644 --- a/crates/sui-transaction-builder/src/lib.rs +++ b/crates/sui-transaction-builder/src/lib.rs @@ -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, diff --git a/crates/sui-types/src/messages.rs b/crates/sui-types/src/messages.rs index dd7991b6b6265..c2ce96818e026 100644 --- a/crates/sui-types/src/messages.rs +++ b/crates/sui-types/src/messages.rs @@ -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, }, // @@ -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!( diff --git a/crates/sui-types/src/move_package.rs b/crates/sui-types/src/move_package.rs index 4e0d88bf15334..da97f3974cf44 100644 --- a/crates/sui-types/src/move_package.rs +++ b/crates/sui-types/src/move_package.rs @@ -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; @@ -34,11 +35,53 @@ pub struct MovePackage { } impl MovePackage { - pub fn new(id: ObjectID, module_map: &BTreeMap>) -> Self { - Self { + pub fn new( + id: ObjectID, + module_map: &BTreeMap>, + ) -> Result { + 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>( + iter: T, + ) -> Result { + 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 { @@ -113,27 +156,3 @@ where } Ok(normalized_modules) } - -impl FromIterator for MovePackage { - fn from_iter>(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(), - ) - } -} diff --git a/crates/sui-types/src/object.rs b/crates/sui-types/src/object.rs index b7d0e0c6e261c..7adda41d5bc41 100644 --- a/crates/sui-types/src/object.rs +++ b/crates/sui-types/src/object.rs @@ -32,11 +32,11 @@ pub const GAS_VALUE_FOR_TESTING: u64 = 1_000_000_u64; pub const OBJECT_START_VERSION: SequenceNumber = SequenceNumber::from_u64(1); /// Maximum size of the `contents` part of an object, in bytes -pub const MAX_MOVE_OBJECT_SIZE: usize = 250 * 1024; // 250 KB +pub const MAX_MOVE_OBJECT_SIZE: u64 = 250 * 1024; // 250 KB -// TODO: enforce this limit +// TODO: increase to 500 KB. currently, publishing a package > 500 KB exceeds the max computation gas cost /// Maximum size of a Move package object, in bytes -pub const MAX_MOVE_PACKAGE_SIZE: usize = 500 * 1024; // 500 KB +pub const MAX_MOVE_PACKAGE_SIZE: u64 = 100 * 1024; // 100 KB /// Packages are immutable, version is always 1 pub const PACKAGE_VERSION: SequenceNumber = OBJECT_START_VERSION; @@ -86,11 +86,11 @@ impl MoveObject { // coins should always have public transfer, as they always should have store. // Thus, type_ == GasCoin::type_() ==> has_public_transfer debug_assert!(type_ != GasCoin::type_() || has_public_transfer); - if contents.len() > MAX_MOVE_OBJECT_SIZE { + if contents.len() as u64 > MAX_MOVE_OBJECT_SIZE { return Err(ExecutionError::from_kind( ExecutionErrorKind::MoveObjectTooBig { - object_size: contents.len() as u32, - max_object_size: MAX_MOVE_OBJECT_SIZE as u32, + object_size: contents.len() as u64, + max_object_size: MAX_MOVE_OBJECT_SIZE, }, )); } @@ -168,11 +168,11 @@ impl MoveObject { &mut self, new_contents: Vec, ) -> Result<(), ExecutionError> { - if new_contents.len() > MAX_MOVE_OBJECT_SIZE { + if new_contents.len() as u64 > MAX_MOVE_OBJECT_SIZE { return Err(ExecutionError::from_kind( ExecutionErrorKind::MoveObjectTooBig { - object_size: new_contents.len() as u32, - max_object_size: MAX_MOVE_OBJECT_SIZE as u32, + object_size: new_contents.len() as u64, + max_object_size: MAX_MOVE_OBJECT_SIZE, }, )); } @@ -423,13 +423,13 @@ impl Object { pub fn new_package( modules: Vec, previous_transaction: TransactionDigest, - ) -> Self { - Object { - data: Data::Package(MovePackage::from_iter(modules)), + ) -> Result { + Ok(Object { + data: Data::Package(MovePackage::from_module_iter(modules)?), owner: Owner::Immutable, previous_transaction, storage_rebate: 0, - } + }) } pub fn is_immutable(&self) -> bool { diff --git a/crates/sui-types/src/unit_tests/base_types_tests.rs b/crates/sui-types/src/unit_tests/base_types_tests.rs index 36e9fa65a0499..d55ac9be8a497 100644 --- a/crates/sui-types/src/unit_tests/base_types_tests.rs +++ b/crates/sui-types/src/unit_tests/base_types_tests.rs @@ -333,7 +333,7 @@ fn test_move_object_size_for_gas_metering() { #[test] fn test_move_package_size_for_gas_metering() { let module = file_format::empty_module(); - let package = Object::new_package(vec![module], TransactionDigest::genesis()); + let package = Object::new_package(vec![module], TransactionDigest::genesis()).unwrap(); let size = package.object_size_for_gas_metering(); let serialized = bcs::to_bytes(&package).unwrap(); // If the following assertion breaks, it's likely you have changed MovePackage's fields.