diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index a9e93a16f0713..96345456fbfc7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1206,7 +1206,7 @@ impl pallet_contracts::Config for Runtime { type CallFilter = Nothing; type DepositPerItem = DepositPerItem; type DepositPerByte = DepositPerByte; - type CallStack = [pallet_contracts::Frame; 31]; + type CallStack = [pallet_contracts::Frame; 5]; type WeightPrice = pallet_transaction_payment::Pallet; type WeightInfo = pallet_contracts::weights::SubstrateWeight; type ChainExtension = (); @@ -1214,7 +1214,7 @@ impl pallet_contracts::Config for Runtime { type DeletionWeightLimit = DeletionWeightLimit; type Schedule = Schedule; type AddressGenerator = pallet_contracts::DefaultAddressGenerator; - type MaxCodeLen = ConstU32<{ 128 * 1024 }>; + type MaxCodeLen = ConstU32<{ 123 * 1024 }>; type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = ConstBool; type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 8ae70b804d69b..b5b630653667b 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -1336,7 +1336,18 @@ where fn append_debug_buffer(&mut self, msg: &str) -> bool { if let Some(buffer) = &mut self.debug_message { - let mut msg = msg.bytes(); + let err_msg = scale_info::prelude::format!( + "Debug message too big (size={}) for debug buffer (bound={})", + msg.len(), + DebugBufferVec::::bound(), + ); + + let mut msg = if msg.len() > DebugBufferVec::::bound() { + err_msg.bytes() + } else { + msg.bytes() + }; + let num_drain = { let capacity = DebugBufferVec::::bound().checked_sub(buffer.len()).expect( " @@ -1349,16 +1360,7 @@ where msg.len().saturating_sub(capacity).min(buffer.len()) }; buffer.drain(0..num_drain); - buffer - .try_extend(&mut msg) - .map_err(|_| { - log::debug!( - target: "runtime::contracts", - "Debug message to big (size={}) for debug buffer (bound={})", - msg.len(), DebugBufferVec::::bound(), - ); - }) - .ok(); + buffer.try_extend(&mut msg).ok(); true } else { false diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 9efeec65f51f5..870bd2ab28581 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -123,6 +123,7 @@ use pallet_contracts_primitives::{ StorageDeposit, }; use scale_info::TypeInfo; +use smallvec::Array; use sp_runtime::traits::{Convert, Hash, Saturating, StaticLookup, TrailingZeroInput}; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; @@ -272,7 +273,10 @@ pub mod pallet { /// The allowed depth is `CallStack::size() + 1`. /// Therefore a size of `0` means that a contract cannot use call or instantiate. /// In other words only the origin called "root contract" is allowed to execute then. - type CallStack: smallvec::Array>; + /// + /// This setting along with [`MaxCodeLen`](#associatedtype.MaxCodeLen) directly affects + /// memory usage of your runtime. + type CallStack: Array>; /// The maximum number of contracts that can be pending for deletion. /// @@ -323,6 +327,10 @@ pub mod pallet { /// 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. + /// + /// The value should be chosen carefully taking into the account the overall memory limit + /// your runtime has, as well as the [maximum allowed callstack + /// depth](#associatedtype.CallStack). Look into the `integrity_test()` for some insights. #[pallet::constant] type MaxCodeLen: Get; @@ -372,6 +380,71 @@ pub mod pallet { T::WeightInfo::on_process_deletion_queue_batch() } } + + fn integrity_test() { + // Total runtime memory is expected to have 128Mb upper limit + const MAX_RUNTIME_MEM: u32 = 1024 * 1024 * 128; + // Memory limits for a single contract: + // Value stack size: 1Mb per contract, default defined in wasmi + const MAX_STACK_SIZE: u32 = 1024 * 1024; + // Heap limit is normally 16 mempages of 64kb each = 1Mb per contract + let max_heap_size = T::Schedule::get().limits.max_memory_size(); + // Max call depth is CallStack::size() + 1 + let max_call_depth = u32::try_from(T::CallStack::size().saturating_add(1)) + .expect("CallStack size is too big"); + + // Check that given configured `MaxCodeLen`, runtime heap memory limit can't be broken. + // + // In worst case, the decoded wasm contract code would be `x16` times larger than the + // encoded one. This is because even a single-byte wasm instruction has 16-byte size in + // wasmi. This gives us `MaxCodeLen*16` safety margin. + // + // Next, the pallet keeps both the original and instrumented wasm blobs for each + // contract, hence we add up `MaxCodeLen*2` more to the safety margin. + // + // Finally, the inefficiencies of the freeing-bump allocator + // being used in the client for the runtime memory allocations, could lead to possible + // memory allocations for contract code grow up to `x4` times in some extreme cases, + // which gives us total multiplier of `18*4` for `MaxCodeLen`. + // + // That being said, for every contract executed in runtime, at least `MaxCodeLen*18*4` + // memory should be available. Note that maximum allowed heap memory and stack size per + // each contract (stack frame) should also be counted. + // + // Finally, we allow 50% of the runtime memory to be utilized by the contracts call + // stack, keeping the rest for other facilities, such as PoV, etc. + // + // This gives us the following formula: + // + // `(MaxCodeLen * 18 * 4 + MAX_STACK_SIZE + max_heap_size) * max_call_depth < + // MAX_RUNTIME_MEM/2` + // + // Hence the upper limit for the `MaxCodeLen` can be defined as follows: + let code_len_limit = MAX_RUNTIME_MEM + .saturating_div(2) + .saturating_div(max_call_depth) + .saturating_sub(max_heap_size) + .saturating_sub(MAX_STACK_SIZE) + .saturating_div(18 * 4); + + assert!( + T::MaxCodeLen::get() < code_len_limit, + "Given `CallStack` height {:?}, `MaxCodeLen` should be set less than {:?} \ + (current value is {:?}), to avoid possible runtime oom issues.", + max_call_depth, + code_len_limit, + T::MaxCodeLen::get(), + ); + + // Debug buffer should at least be large enough to accomodate a simple error message + const MIN_DEBUG_BUF_SIZE: u32 = 256; + assert!( + T::MaxDebugBufferLen::get() > MIN_DEBUG_BUF_SIZE, + "Debug buffer should have minimum size of {} (current setting is {})", + MIN_DEBUG_BUF_SIZE, + T::MaxDebugBufferLen::get(), + ) + } } #[pallet::call] diff --git a/frame/contracts/src/schedule.rs b/frame/contracts/src/schedule.rs index 912e58b048ff4..0d180e1bf5cef 100644 --- a/frame/contracts/src/schedule.rs +++ b/frame/contracts/src/schedule.rs @@ -134,9 +134,6 @@ pub struct Limits { /// The maximum length of a subject in bytes used for PRNG generation. pub subject_len: u32, - /// The maximum nesting level of the call stack. - pub call_depth: u32, - /// The maximum size of a storage value and event payload in bytes. pub payload_len: u32, } @@ -526,7 +523,6 @@ impl Default for Limits { table_size: 4096, br_table_size: 256, subject_len: 32, - call_depth: 32, payload_len: 16 * 1024, } } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 7a0736d1ed61a..698e47b026967 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -394,7 +394,7 @@ impl Config for Test { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; type CallFilter = TestFilter; - type CallStack = [Frame; 31]; + type CallStack = [Frame; 5]; type WeightPrice = Self; type WeightInfo = (); type ChainExtension = @@ -405,7 +405,7 @@ impl Config for Test { type DepositPerByte = DepositPerByte; type DepositPerItem = DepositPerItem; type AddressGenerator = DefaultAddressGenerator; - type MaxCodeLen = ConstU32<{ 128 * 1024 }>; + type MaxCodeLen = ConstU32<{ 123 * 1024 }>; type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = UnstableInterface; type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;