From 35a2e17acd7dacdefbe14ce62ec5b7ff64666e86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 5 Sep 2022 11:48:34 +0200 Subject: [PATCH 1/2] Use WeakBoundedVec for instrumented code --- frame/contracts/src/lib.rs | 13 ++----------- frame/contracts/src/tests.rs | 1 - frame/contracts/src/wasm/code_cache.rs | 14 ++++++++++---- frame/contracts/src/wasm/mod.rs | 12 +----------- 4 files changed, 13 insertions(+), 27 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 3c63ad86016f3..34c38fc966840 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -111,7 +111,7 @@ use frame_support::{ ensure, traits::{ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time}, weights::{DispatchClass, GetDispatchInfo, Pays, PostDispatchInfo, Weight}, - BoundedVec, + BoundedVec, WeakBoundedVec, }; use frame_system::{limits::BlockWeights, Pallet as System}; use pallet_contracts_primitives::{ @@ -135,7 +135,7 @@ type TrieId = BoundedVec>; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type CodeVec = BoundedVec::MaxCodeLen>; -type RelaxedCodeVec = BoundedVec::RelaxedMaxCodeLen>; +type RelaxedCodeVec = WeakBoundedVec::MaxCodeLen>; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; /// Used as a sentinel value when reading and writing contract memory. @@ -366,15 +366,6 @@ pub mod pallet { /// 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; - /// The maximum allowable length in bytes for storage keys. type MaxStorageKeyLen: Get; } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index f65f845f7b9fe..216ce63257af2 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -384,7 +384,6 @@ impl Config for Test { type AddressGenerator = DefaultAddressGenerator; type ContractAccessWeight = DefaultContractAccessWeight; type MaxCodeLen = ConstU32<{ 128 * 1024 }>; - type RelaxedMaxCodeLen = ConstU32<{ 256 * 1024 }>; type MaxStorageKeyLen = ConstU32<128>; } diff --git a/frame/contracts/src/wasm/code_cache.rs b/frame/contracts/src/wasm/code_cache.rs index eb5551e342bca..137eccf3db686 100644 --- a/frame/contracts/src/wasm/code_cache.rs +++ b/frame/contracts/src/wasm/code_cache.rs @@ -39,6 +39,7 @@ use frame_support::{ dispatch::{DispatchError, DispatchResult}, ensure, traits::{Get, ReservableCurrency}, + WeakBoundedVec, }; use sp_core::crypto::UncheckedFrom; use sp_runtime::traits::BadOrigin; @@ -195,10 +196,15 @@ 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) - .map_err(|_| >::CodeRejected)? - .try_into() - .map_err(|_| >::CodeTooLarge)?; + // We need to allow contracts growing too big after re-instrumentation. Otherwise + // the contract can become inaccessible. The user has no influence over this size + // as the contract is already deployed and every change in size would be the result + // of changes in the instrumentation algorithm controlled by the chain authors. + prefab_module.code = WeakBoundedVec::force_from( + prepare::reinstrument_contract::(&original_code, schedule) + .map_err(|_| >::CodeRejected)?, + Some("Contract exceeds limit after re-instrumentation."), + ); 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 f989c21b00ffc..ee5d7de5ab9e7 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -35,11 +35,7 @@ use crate::{ Schedule, }; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, - traits::Get, -}; +use frame_support::dispatch::{DispatchError, DispatchResult}; use sp_core::crypto::UncheckedFrom; use sp_sandbox::{SandboxEnvironmentBuilder, SandboxInstance, SandboxMemory}; use sp_std::prelude::*; @@ -134,12 +130,6 @@ where 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) } From d221615082ee14be87b3e1d6e2c4f00091e6af18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 5 Sep 2022 12:13:55 +0200 Subject: [PATCH 2/2] Remove `RelaxedMaxCodeLen` from kitchensink --- bin/node/runtime/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index fa0f877c5938d..9aada2f4d915e 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1157,7 +1157,6 @@ impl pallet_contracts::Config for Runtime { type AddressGenerator = pallet_contracts::DefaultAddressGenerator; type ContractAccessWeight = pallet_contracts::DefaultContractAccessWeight; type MaxCodeLen = ConstU32<{ 128 * 1024 }>; - type RelaxedMaxCodeLen = ConstU32<{ 256 * 1024 }>; type MaxStorageKeyLen = ConstU32<128>; }