diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 40316cee9b2b7..de6f8eeb9f1c0 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1118,6 +1118,8 @@ impl pallet_contracts::Config for Runtime { type Schedule = Schedule; type AddressGenerator = pallet_contracts::DefaultAddressGenerator; type ContractAccessWeight = pallet_contracts::DefaultContractAccessWeight; + type MaxCodeLen = ConstU32<{ 128 * 1024 }>; + type RelaxedMaxCodeLen = ConstU32<{ 256 * 1024 }>; } impl pallet_sudo::Config for Runtime { diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 194f97f8c878d..01e62e07e1ce0 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -231,7 +231,7 @@ benchmarks! { // first time after a new schedule was deployed: For every new schedule a contract needs // to re-run the instrumentation once. reinstrument { - let c in 0 .. T::Schedule::get().limits.code_len; + let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get()); let WasmModule { code, hash, .. } = WasmModule::::sized(c, Location::Call); Contracts::::store_code_raw(code, whitelisted_caller())?; let schedule = T::Schedule::get(); @@ -247,7 +247,7 @@ benchmarks! { // which is in the wasm module but not executed on `call`. // The results are supposed to be used as `call_with_code_kb(c) - call_with_code_kb(0)`. call_with_code_per_byte { - let c in 0 .. T::Schedule::get().limits.code_len; + let c in 0 .. T::MaxCodeLen::get(); let instance = Contract::::with_caller( whitelisted_caller(), WasmModule::sized(c, Location::Deploy), vec![], )?; @@ -271,7 +271,7 @@ benchmarks! { // We cannot let `c` grow to the maximum code size because the code is not allowed // to be larger than the maximum size **after instrumentation**. instantiate_with_code { - let c in 0 .. Perbill::from_percent(49).mul_ceil(T::Schedule::get().limits.code_len); + let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get()); let s in 0 .. code::max_pages::() * 64 * 1024; let salt = vec![42u8; s as usize]; let value = T::Currency::minimum_balance(); @@ -360,7 +360,7 @@ benchmarks! { // We cannot let `c` grow to the maximum code size because the code is not allowed // to be larger than the maximum size **after instrumentation**. upload_code { - let c in 0 .. Perbill::from_percent(50).mul_ceil(T::Schedule::get().limits.code_len); + let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get()); let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, caller_funding::()); let WasmModule { code, hash, .. } = WasmModule::::sized(c, Location::Call); diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index ca639d7781ac2..ffac3e222bbce 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -114,8 +114,9 @@ use codec::{Encode, HasCompact}; use frame_support::{ dispatch::Dispatchable, ensure, - traits::{Contains, Currency, Get, Randomness, ReservableCurrency, Time}, + traits::{ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time}, weights::{DispatchClass, GetDispatchInfo, Pays, PostDispatchInfo, Weight}, + BoundedVec, }; use frame_system::{limits::BlockWeights, Pallet as System}; use pallet_contracts_primitives::{ @@ -129,9 +130,11 @@ use sp_runtime::traits::{Convert, Hash, Saturating, StaticLookup}; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; type CodeHash = ::Hash; -type TrieId = Vec; +type TrieId = BoundedVec>; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +type CodeVec = BoundedVec::MaxCodeLen>; +type RelaxedCodeVec = BoundedVec::RelaxedMaxCodeLen>; /// Used as a sentinel value when reading and writing contract memory. /// @@ -224,7 +227,6 @@ pub mod pallet { #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] - #[pallet::without_storage_info] pub struct Pallet(PhantomData); #[pallet::config] @@ -356,6 +358,20 @@ pub mod pallet { /// The address generator used to generate the addresses of contracts. type AddressGenerator: AddressGenerator; + + /// The maximum length of a contract code in bytes. This limit applies to the instrumented + /// version of the code. Therefore `instantiate_with_code` can fail even when supplying + /// a wasm binary below this maximum size. + type MaxCodeLen: Get; + + /// The maximum length of a contract code after reinstrumentation. + /// + /// When uploading a new contract the size defined by [`Self::MaxCodeLen`] is used for both + /// the pristine **and** the instrumented version. When a existing contract needs to be + /// reinstrumented after a runtime upgrade we apply this bound. The reason is that if the + /// new instrumentation increases the size beyond the limit it would make that contract + /// inaccessible until rectified by another runtime upgrade. + type RelaxedMaxCodeLen: Get; } #[pallet::hooks] @@ -698,7 +714,7 @@ pub mod pallet { /// A mapping from an original code hash to the original code, untouched by instrumentation. #[pallet::storage] - pub(crate) type PristineCode = StorageMap<_, Identity, CodeHash, Vec>; + pub(crate) type PristineCode = StorageMap<_, Identity, CodeHash, CodeVec>; /// A mapping between an original code hash and instrumented wasm code, ready for execution. #[pallet::storage] @@ -746,7 +762,8 @@ pub mod pallet { /// Child trie deletion is a heavy operation depending on the amount of storage items /// stored in said trie. Therefore this operation is performed lazily in `on_initialize`. #[pallet::storage] - pub(crate) type DeletionQueue = StorageValue<_, Vec, ValueQuery>; + pub(crate) type DeletionQueue = + StorageValue<_, BoundedVec, ValueQuery>; } /// Return type of the private [`Pallet::internal_call`] function. @@ -864,8 +881,8 @@ where storage_deposit_limit: Option>, ) -> CodeUploadResult, BalanceOf> { let schedule = T::Schedule::get(); - let module = PrefabWasmModule::from_code(code, &schedule, origin) - .map_err(|_| >::CodeRejected)?; + let module = + PrefabWasmModule::from_code(code, &schedule, origin).map_err(|(err, _)| err)?; let deposit = module.open_deposit(); if let Some(storage_deposit_limit) = storage_deposit_limit { ensure!(storage_deposit_limit >= deposit, >::StorageDepositLimitExhausted); @@ -971,19 +988,11 @@ where let schedule = T::Schedule::get(); let (extra_deposit, executable) = match code { Code::Upload(Bytes(binary)) => { - ensure!( - binary.len() as u32 <= schedule.limits.code_len, - >::CodeTooLarge - ); let executable = PrefabWasmModule::from_code(binary, &schedule, origin.clone()) - .map_err(|msg| { + .map_err(|(err, msg)| { debug_message.as_mut().map(|buffer| buffer.extend(msg.as_bytes())); - >::CodeRejected + err })?; - ensure!( - executable.code_len() <= schedule.limits.code_len, - >::CodeTooLarge - ); // The open deposit will be charged during execution when the // uploaded module does not already exist. This deposit is not part of the // storage meter because it is not transfered to the contract but diff --git a/frame/contracts/src/schedule.rs b/frame/contracts/src/schedule.rs index 9e9f213fabb76..907ce9e088648 100644 --- a/frame/contracts/src/schedule.rs +++ b/frame/contracts/src/schedule.rs @@ -33,7 +33,7 @@ use wasm_instrument::{gas_metering, parity_wasm::elements}; /// How many API calls are executed in a single batch. The reason for increasing the amount /// of API calls in batches (per benchmark component increase) is so that the linear regression /// has an easier time determining the contribution of that component. -pub const API_BENCHMARK_BATCH_SIZE: u32 = 100; +pub const API_BENCHMARK_BATCH_SIZE: u32 = 80; /// How many instructions are executed in a single batch. The reasoning is the same /// as for `API_BENCHMARK_BATCH_SIZE`. @@ -147,11 +147,6 @@ pub struct Limits { /// The maximum size of a storage value and event payload in bytes. pub payload_len: u32, - - /// The maximum length of a contract code in bytes. This limit applies to the instrumented - /// version of the code. Therefore `instantiate_with_code` can fail even when supplying - /// a wasm binary below this maximum size. - pub code_len: u32, } impl Limits { @@ -522,7 +517,6 @@ impl Default for Limits { subject_len: 32, call_depth: 32, payload_len: 16 * 1024, - code_len: 128 * 1024, } } } diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index 2bdacc15cb11f..a9a78f12581d8 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -24,11 +24,10 @@ use crate::{ weights::WeightInfo, BalanceOf, CodeHash, Config, ContractInfoOf, DeletionQueue, Error, TrieId, SENTINEL, }; -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch::{DispatchError, DispatchResult}, storage::child::{self, ChildInfo, KillStorageResult}, - traits::Get, weights::Weight, }; use scale_info::TypeInfo; @@ -44,7 +43,7 @@ pub type ContractInfo = RawContractInfo, BalanceOf>; /// Information for managing an account and its sub trie abstraction. /// This is the required info to cache for an account. -#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)] +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub struct RawContractInfo { /// Unique ID for the subtree encoded as a bytes vector. pub trie_id: TrieId, @@ -67,7 +66,7 @@ fn child_trie_info(trie_id: &[u8]) -> ChildInfo { ChildInfo::new_default(trie_id) } -#[derive(Encode, Decode, TypeInfo)] +#[derive(Encode, Decode, TypeInfo, MaxEncodedLen)] pub struct DeletedContract { pub(crate) trie_id: TrieId, } @@ -217,12 +216,8 @@ where /// /// You must make sure that the contract is also removed when queuing the trie for deletion. pub fn queue_trie_for_deletion(contract: &ContractInfo) -> DispatchResult { - if >::decode_len().unwrap_or(0) >= T::DeletionQueueDepth::get() as usize { - Err(Error::::DeletionQueueFull.into()) - } else { - >::append(DeletedContract { trie_id: contract.trie_id.clone() }); - Ok(()) - } + >::try_append(DeletedContract { trie_id: contract.trie_id.clone() }) + .map_err(|_| >::DeletionQueueFull.into()) } /// Calculates the weight that is necessary to remove one key from the trie and how many @@ -293,7 +288,11 @@ where /// Generates a unique trie id by returning `hash(account_id ++ nonce)`. pub fn generate_trie_id(account_id: &AccountIdOf, nonce: u64) -> TrieId { let buf: Vec<_> = account_id.as_ref().iter().chain(&nonce.to_le_bytes()).cloned().collect(); - T::Hashing::hash(&buf).as_ref().into() + T::Hashing::hash(&buf) + .as_ref() + .to_vec() + .try_into() + .expect("Runtime uses a reasonable hash size. Hence sizeof(T::Hash) <= 128; qed") } /// Returns the code hash of the contract specified by `account` ID. @@ -305,9 +304,11 @@ where /// Fill up the queue in order to exercise the limits during testing. #[cfg(test)] pub fn fill_queue_with_dummies() { - let queue: Vec<_> = (0..T::DeletionQueueDepth::get()) - .map(|_| DeletedContract { trie_id: vec![] }) + use frame_support::{traits::Get, BoundedVec}; + let queue: Vec = (0..T::DeletionQueueDepth::get()) + .map(|_| DeletedContract { trie_id: TrieId::default() }) .collect(); - >::put(queue); + let bounded: BoundedVec<_, _> = queue.try_into().unwrap(); + >::put(bounded); } } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 407e1e999dde1..e52ce5ca0e156 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -288,6 +288,8 @@ impl Config for Test { type DepositPerItem = DepositPerItem; type AddressGenerator = DefaultAddressGenerator; type ContractAccessWeight = DefaultContractAccessWeight; + type MaxCodeLen = ConstU32<{ 128 * 1024 }>; + type RelaxedMaxCodeLen = ConstU32<{ 256 * 1024 }>; } pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]); diff --git a/frame/contracts/src/wasm/code_cache.rs b/frame/contracts/src/wasm/code_cache.rs index b194c283e90cd..10de436bfb155 100644 --- a/frame/contracts/src/wasm/code_cache.rs +++ b/frame/contracts/src/wasm/code_cache.rs @@ -163,7 +163,8 @@ pub fn load( where T::AccountId: UncheckedFrom + AsRef<[u8]>, { - let charged = gas_meter.charge(CodeToken::Load(schedule.limits.code_len))?; + let max_code_len = T::MaxCodeLen::get(); + let charged = gas_meter.charge(CodeToken::Load(max_code_len))?; let mut prefab_module = >::get(code_hash).ok_or(Error::::CodeNotFound)?; gas_meter.adjust_gas(charged, CodeToken::Load(prefab_module.code.len() as u32)); @@ -172,7 +173,7 @@ where if prefab_module.instruction_weights_version < schedule.instruction_weights.version { // The instruction weights have changed. // We need to re-instrument the code with the new instruction weights. - let charged = gas_meter.charge(CodeToken::Reinstrument(schedule.limits.code_len))?; + let charged = gas_meter.charge(CodeToken::Reinstrument(max_code_len))?; let code_size = reinstrument(&mut prefab_module, schedule)?; gas_meter.adjust_gas(charged, CodeToken::Reinstrument(code_size)); } @@ -190,7 +191,10 @@ pub fn reinstrument( let original_code = >::get(&prefab_module.code_hash).ok_or(Error::::CodeNotFound)?; let original_code_len = original_code.len(); - prefab_module.code = prepare::reinstrument_contract::(original_code, schedule)?; + prefab_module.code = prepare::reinstrument_contract::(&original_code, schedule) + .map_err(|_| >::CodeRejected)? + .try_into() + .map_err(|_| >::CodeTooLarge)?; prefab_module.instruction_weights_version = schedule.instruction_weights.version; >::insert(&prefab_module.code_hash, &*prefab_module); Ok(original_code_len as u32) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index d710cad199f93..8764bf3690b82 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -31,10 +31,15 @@ use crate::{ exec::{ExecResult, Executable, ExportedFunction, Ext}, gas::GasMeter, wasm::env_def::FunctionImplProvider, - AccountIdOf, BalanceOf, CodeHash, CodeStorage, Config, Schedule, + AccountIdOf, BalanceOf, CodeHash, CodeStorage, CodeVec, Config, Error, RelaxedCodeVec, + Schedule, }; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::dispatch::{DispatchError, DispatchResult}; +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, + traits::Get, +}; use sp_core::crypto::UncheckedFrom; use sp_sandbox::{SandboxEnvironmentBuilder, SandboxInstance, SandboxMemory}; use sp_std::prelude::*; @@ -50,7 +55,8 @@ pub use tests::MockExt; /// `instruction_weights_version` and `code` change when a contract with an outdated instrumentation /// is called. Therefore one must be careful when holding any in-memory representation of this /// type while calling into a contract as those fields can get out of date. -#[derive(Clone, Encode, Decode, scale_info::TypeInfo)] +#[derive(Clone, Encode, Decode, scale_info::TypeInfo, MaxEncodedLen)] +#[codec(mel_bound())] #[scale_info(skip_type_params(T))] pub struct PrefabWasmModule { /// Version of the instruction weights with which the code was instrumented. @@ -63,14 +69,14 @@ pub struct PrefabWasmModule { #[codec(compact)] maximum: u32, /// Code instrumented with the latest schedule. - code: Vec, + code: RelaxedCodeVec, /// The uninstrumented, pristine version of the code. /// /// It is not stored because the pristine code has its own storage item. The value /// is only `Some` when this module was created from an `original_code` and `None` if /// it was loaded from storage. #[codec(skip)] - original_code: Option>, + original_code: Option>, /// The code hash of the stored code which is defined as the hash over the `original_code`. /// /// As the map key there is no need to store the hash in the value, too. It is set manually @@ -122,8 +128,19 @@ where original_code: Vec, schedule: &Schedule, owner: AccountIdOf, - ) -> Result { - prepare::prepare_contract(original_code, schedule, owner) + ) -> Result { + let module = prepare::prepare_contract( + original_code.try_into().map_err(|_| (>::CodeTooLarge.into(), ""))?, + schedule, + owner, + )?; + // When instrumenting a new code we apply a stricter limit than enforced by the + // `RelaxedCodeVec` in order to leave some headroom for reinstrumentation. + ensure!( + module.code.len() as u32 <= T::MaxCodeLen::get(), + (>::CodeTooLarge.into(), ""), + ); + Ok(module) } /// Store the code without instantiating it. diff --git a/frame/contracts/src/wasm/prepare.rs b/frame/contracts/src/wasm/prepare.rs index 6e9babe1264ed..f6fff20de6b1a 100644 --- a/frame/contracts/src/wasm/prepare.rs +++ b/frame/contracts/src/wasm/prepare.rs @@ -23,10 +23,10 @@ use crate::{ chain_extension::ChainExtension, storage::meter::Diff, wasm::{env_def::ImportSatisfyCheck, OwnerInfo, PrefabWasmModule}, - AccountIdOf, Config, Schedule, + AccountIdOf, CodeVec, Config, Error, Schedule, }; use codec::{Encode, MaxEncodedLen}; -use sp_runtime::traits::Hash; +use sp_runtime::{traits::Hash, DispatchError}; use sp_std::prelude::*; use wasm_instrument::parity_wasm::elements::{ self, External, Internal, MemoryType, Type, ValueType, @@ -401,19 +401,19 @@ fn check_and_instrument( } fn do_preparation( - original_code: Vec, + original_code: CodeVec, schedule: &Schedule, owner: AccountIdOf, -) -> Result, &'static str> { - let (code, (initial, maximum)) = - check_and_instrument::(original_code.as_ref(), schedule)?; +) -> Result, (DispatchError, &'static str)> { + let (code, (initial, maximum)) = check_and_instrument::(original_code.as_ref(), schedule) + .map_err(|msg| (>::CodeRejected.into(), msg))?; let original_code_len = original_code.len(); let mut module = PrefabWasmModule { instruction_weights_version: schedule.instruction_weights.version, initial, maximum, - code, + code: code.try_into().map_err(|_| (>::CodeTooLarge.into(), ""))?, code_hash: T::Hashing::hash(&original_code), original_code: Some(original_code), owner_info: None, @@ -446,10 +446,10 @@ fn do_preparation( /// /// The preprocessing includes injecting code for gas metering and metering the height of stack. pub fn prepare_contract( - original_code: Vec, + original_code: CodeVec, schedule: &Schedule, owner: AccountIdOf, -) -> Result, &'static str> { +) -> Result, (DispatchError, &'static str)> { do_preparation::(original_code, schedule, owner) } @@ -459,10 +459,10 @@ pub fn prepare_contract( /// /// Use this when an existing contract should be re-instrumented with a newer schedule version. pub fn reinstrument_contract( - original_code: Vec, + original_code: &[u8], schedule: &Schedule, ) -> Result, &'static str> { - Ok(check_and_instrument::(&original_code, schedule)?.0) + Ok(check_and_instrument::(original_code, schedule)?.0) } /// Alternate (possibly unsafe) preparation functions used only for benchmarking. @@ -493,9 +493,12 @@ pub mod benchmarking { instruction_weights_version: schedule.instruction_weights.version, initial: memory_limits.0, maximum: memory_limits.1, - code: contract_module.into_wasm_code()?, code_hash: T::Hashing::hash(&original_code), - original_code: Some(original_code), + original_code: Some(original_code.try_into().map_err(|_| "Original code too large")?), + code: contract_module + .into_wasm_code()? + .try_into() + .map_err(|_| "Instrumented code too large")?, owner_info: Some(OwnerInfo { owner, // this is a helper function for benchmarking which skips deposit collection @@ -546,7 +549,7 @@ mod tests { ($name:ident, $wat:expr, $($expected:tt)*) => { #[test] fn $name() { - let wasm = wat::parse_str($wat).unwrap(); + let wasm = wat::parse_str($wat).unwrap().try_into().unwrap(); let schedule = Schedule { limits: Limits { globals: 3, @@ -559,7 +562,7 @@ mod tests { .. Default::default() }; let r = do_preparation::(wasm, &schedule, ALICE); - assert_matches::assert_matches!(r, $($expected)*); + assert_matches::assert_matches!(r.map_err(|(_, msg)| msg), $($expected)*); } }; }