From 1697f865e983ad9d3c0e591d5f82ac6b54a8eb74 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 8 Jul 2024 14:41:27 +0200 Subject: [PATCH 01/10] Hardcode ref_time per byte for deposit_event hostfn --- substrate/frame/contracts/src/wasm/runtime.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index 5f50dbf391a2..cb8dcefd9d0d 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -287,7 +287,10 @@ impl Token for RuntimeCosts { WeightToFee => T::WeightInfo::seal_weight_to_fee(), Terminate(locked_dependencies) => T::WeightInfo::seal_terminate(locked_dependencies), Random => T::WeightInfo::seal_random(), - DepositEvent { num_topic, len } => T::WeightInfo::seal_deposit_event(num_topic, len), + // Given a 2-second block time and hardcoding a `ref_time` of 60,000 picoseconds per + // byte, the max allocation size is 32MB per block. + DepositEvent { num_topic, len } => T::WeightInfo::seal_deposit_event(num_topic, len) + .saturating_add(Weight::from_parts(60_000u64.saturating_mul(len.into()), 0)), DebugMessage(len) => T::WeightInfo::seal_debug_message(len), SetStorage { new_bytes, old_bytes } => T::WeightInfo::seal_set_storage(new_bytes, old_bytes), From 314cf42325a0c1697d28dc37d03e3a83c43cb4a0 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Fri, 19 Jul 2024 17:12:11 +0200 Subject: [PATCH 02/10] Add memory check --- substrate/frame/contracts/src/lib.rs | 63 +++++++++++++++++++++-- substrate/frame/contracts/src/schedule.rs | 5 ++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 47772e0a5a0b..30dc84214467 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -110,9 +110,9 @@ use crate::{ exec::{ AccountIdOf, ErrorOrigin, ExecError, Executable, Ext, Key, MomentOf, Stack as ExecStack, }, - gas::GasMeter, + gas::{GasMeter, Token}, storage::{meter::Meter as StorageMeter, ContractInfo, DeletionQueueManager}, - wasm::{CodeInfo, WasmBlob}, + wasm::{CodeInfo, RuntimeCosts, WasmBlob}, }; use codec::{Codec, Decode, Encode, HasCompact, MaxEncodedLen}; use environmental::*; @@ -656,7 +656,64 @@ pub mod pallet { "Debug buffer should have minimum size of {} (current setting is {})", MIN_DEBUG_BUF_SIZE, T::MaxDebugBufferLen::get(), - ) + ); + + // Validators have configured runtime memory for 512MB. They need to keep in it + // max_runtime_mem, PoV in multiple copies (1x encoded + 2x decoded), and storage + // including events. The assumption is that max_runtime_mem + storage/events can be + // a maximum of half of the validator runtime memory. + let max_block_ref_time = T::BlockWeights::get() + .get(DispatchClass::Normal) + .max_total + .unwrap_or_else(|| T::BlockWeights::get().max_block) + .ref_time(); + let max_payload_size = T::Schedule::get().limits.payload_len; + // Blake2_128Concat is being used. + let max_key_size = T::MaxStorageKeyLen::get().saturating_add(128); + + // We can use storage to store items using the available block ref_time with the + // `set_storage` host function. + let max_storage_size: u32 = ((max_block_ref_time / + (>::weight(&RuntimeCosts::SetStorage { + new_bytes: max_payload_size, + old_bytes: 0, + }) + .ref_time() + .saturating_add(max_key_size as u64))) + .saturating_mul(max_payload_size as u64)) + .try_into() + .expect("Storage size too big"); + + let max_validator_runtime_mem: u32 = T::Schedule::get().limits.validator_runtime_memory; + let storage_size_limit = + (max_validator_runtime_mem / 2).saturating_sub(max_runtime_mem); + + assert!( + max_storage_size < storage_size_limit, + "Maximal storage size {} exceeds the storage limit {}", + max_storage_size, + storage_size_limit + ); + + // We can use storage to store events using the available block ref_time with the + // `deposit_event` host function. + let max_events_size: u32 = ((max_block_ref_time / + (>::weight(&RuntimeCosts::DepositEvent { + num_topic: 0, + len: max_payload_size, + }) + .ref_time() + .saturating_add(max_key_size as u64))) + .saturating_mul(max_payload_size as u64)) + .try_into() + .expect("Events size too big"); + + assert!( + max_events_size < storage_size_limit, + "Maximal events size {} exceeds the events limit {}", + max_events_size, + storage_size_limit + ); } } diff --git a/substrate/frame/contracts/src/schedule.rs b/substrate/frame/contracts/src/schedule.rs index 60c9520677eb..29a771e038ad 100644 --- a/substrate/frame/contracts/src/schedule.rs +++ b/substrate/frame/contracts/src/schedule.rs @@ -89,6 +89,10 @@ pub struct Limits { /// The maximum node runtime memory. This is for integrity checks only and does not affect the /// real setting. pub runtime_memory: u32, + + /// The maximum validator node runtime memory. This is for integrity checks only and does not + /// affect the real setting. + pub validator_runtime_memory: u32, } impl Limits { @@ -121,6 +125,7 @@ impl Default for Limits { subject_len: 32, payload_len: 16 * 1024, runtime_memory: 1024 * 1024 * 128, + validator_runtime_memory: 1024 * 1024 * 512, } } } From 276a1d917ee16c019251d5977931d7a362b8680c Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Sat, 20 Jul 2024 00:20:31 +0200 Subject: [PATCH 03/10] Fix calculations --- substrate/frame/contracts/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 30dc84214467..af9dcb3473b2 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -110,7 +110,7 @@ use crate::{ exec::{ AccountIdOf, ErrorOrigin, ExecError, Executable, Ext, Key, MomentOf, Stack as ExecStack, }, - gas::{GasMeter, Token}, + gas::GasMeter, storage::{meter::Meter as StorageMeter, ContractInfo, DeletionQueueManager}, wasm::{CodeInfo, RuntimeCosts, WasmBlob}, }; @@ -678,9 +678,8 @@ pub mod pallet { new_bytes: max_payload_size, old_bytes: 0, }) - .ref_time() - .saturating_add(max_key_size as u64))) - .saturating_mul(max_payload_size as u64)) + .ref_time())) + .saturating_mul(max_payload_size.saturating_add(max_key_size) as u64)) .try_into() .expect("Storage size too big"); @@ -696,14 +695,14 @@ pub mod pallet { ); // We can use storage to store events using the available block ref_time with the - // `deposit_event` host function. + // `deposit_event` host function. The overhead of stored events, which is around 100B, + // is not taken into account to simplify calculations, as it does not change much. let max_events_size: u32 = ((max_block_ref_time / (>::weight(&RuntimeCosts::DepositEvent { num_topic: 0, len: max_payload_size, }) - .ref_time() - .saturating_add(max_key_size as u64))) + .ref_time())) .saturating_mul(max_payload_size as u64)) .try_into() .expect("Events size too big"); From 5cf2ef25b50de16d815e325fb8c63c62fd40274c Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 22 Jul 2024 09:33:04 +0200 Subject: [PATCH 04/10] Add prdoc --- prdoc/pr_4973.prdoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 prdoc/pr_4973.prdoc diff --git a/prdoc/pr_4973.prdoc b/prdoc/pr_4973.prdoc new file mode 100644 index 000000000000..1afbd3afbf7f --- /dev/null +++ b/prdoc/pr_4973.prdoc @@ -0,0 +1,15 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "[pallet_contracts] Increase the weight of the deposit_event host function to limit the memory used by events." + +doc: + - audience: Runtime User + description: | + This PR updates the weight of the deposit_event host function by adding + a fixed ref_time of 60,000 picoseconds per byte. Given a block time of 2 seconds + and this specified ref_time, the total allocation size is 32MB. + +crates: + - name: pallet-contracts + bump: patch From 3a0e7ef1fe5d3c6897bf1ac93fbd44aa9cff89d9 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Fri, 26 Jul 2024 14:20:57 +0200 Subject: [PATCH 05/10] Update substrate/frame/contracts/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Theißen --- substrate/frame/contracts/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index af9dcb3473b2..d48ee4ad9072 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -658,9 +658,9 @@ pub mod pallet { T::MaxDebugBufferLen::get(), ); - // Validators have configured runtime memory for 512MB. They need to keep in it - // max_runtime_mem, PoV in multiple copies (1x encoded + 2x decoded), and storage - // including events. The assumption is that max_runtime_mem + storage/events can be + // Validators are configured to be able to use more memory than block builders. This is because in addition + // to `max_runtime_mem` they need to hold additional data in memory: PoV in multiple copies (1x encoded + 2x decoded) and all storage which includes emitted events. + // The assumption is that max_runtime_mem + storage/events can be // a maximum of half of the validator runtime memory. let max_block_ref_time = T::BlockWeights::get() .get(DispatchClass::Normal) From a3e68b92e8b70ad1a063150eb9e0e2eacd1c04c2 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 5 Aug 2024 10:37:24 +0200 Subject: [PATCH 06/10] Add hash size calculation --- substrate/frame/contracts/src/lib.rs | 7 +++++-- substrate/frame/contracts/src/schedule.rs | 5 +++++ substrate/frame/contracts/src/wasm/runtime.rs | 7 +++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index d48ee4ad9072..bb0299c56045 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -668,8 +668,11 @@ pub mod pallet { .unwrap_or_else(|| T::BlockWeights::get().max_block) .ref_time(); let max_payload_size = T::Schedule::get().limits.payload_len; - // Blake2_128Concat is being used. - let max_key_size = T::MaxStorageKeyLen::get().saturating_add(128); + let max_key_size = + Key::::try_from_var(vec![0u8; T::MaxStorageKeyLen::get() as usize]) + .expect("Key of maximal size shall be created") + .hash() + .len() as u32; // We can use storage to store items using the available block ref_time with the // `set_storage` host function. diff --git a/substrate/frame/contracts/src/schedule.rs b/substrate/frame/contracts/src/schedule.rs index 29a771e038ad..80b8c54b1e1d 100644 --- a/substrate/frame/contracts/src/schedule.rs +++ b/substrate/frame/contracts/src/schedule.rs @@ -93,6 +93,10 @@ pub struct Limits { /// The maximum validator node runtime memory. This is for integrity checks only and does not /// affect the real setting. pub validator_runtime_memory: u32, + + /// The additional ref_time added to the `deposit_event` host function call per event data + /// byte. + pub event_ref_time: u64, } impl Limits { @@ -126,6 +130,7 @@ impl Default for Limits { payload_len: 16 * 1024, runtime_memory: 1024 * 1024 * 128, validator_runtime_memory: 1024 * 1024 * 512, + event_ref_time: 60_000, } } } diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index cb8dcefd9d0d..55fdb2be147a 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -288,9 +288,12 @@ impl Token for RuntimeCosts { Terminate(locked_dependencies) => T::WeightInfo::seal_terminate(locked_dependencies), Random => T::WeightInfo::seal_random(), // Given a 2-second block time and hardcoding a `ref_time` of 60,000 picoseconds per - // byte, the max allocation size is 32MB per block. + // byte (event_ref_time), the max allocation size is 32MB per block. DepositEvent { num_topic, len } => T::WeightInfo::seal_deposit_event(num_topic, len) - .saturating_add(Weight::from_parts(60_000u64.saturating_mul(len.into()), 0)), + .saturating_add(Weight::from_parts( + T::Schedule::get().limits.event_ref_time.saturating_mul(len.into()), + 0, + )), DebugMessage(len) => T::WeightInfo::seal_debug_message(len), SetStorage { new_bytes, old_bytes } => T::WeightInfo::seal_set_storage(new_bytes, old_bytes), From 907b81e735ee7319736ba81f84354546dbdf29c8 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 5 Aug 2024 10:38:55 +0200 Subject: [PATCH 07/10] Fmt --- substrate/frame/contracts/src/lib.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index bb0299c56045..b8bcdfe87d8d 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -658,10 +658,11 @@ pub mod pallet { T::MaxDebugBufferLen::get(), ); - // Validators are configured to be able to use more memory than block builders. This is because in addition - // to `max_runtime_mem` they need to hold additional data in memory: PoV in multiple copies (1x encoded + 2x decoded) and all storage which includes emitted events. - // The assumption is that max_runtime_mem + storage/events can be - // a maximum of half of the validator runtime memory. + // Validators are configured to be able to use more memory than block builders. This is + // because in addition to `max_runtime_mem` they need to hold additional data in + // memory: PoV in multiple copies (1x encoded + 2x decoded) and all storage which + // includes emitted events. The assumption is that max_runtime_mem + storage/events + // can be a maximum of half of the validator runtime memory. let max_block_ref_time = T::BlockWeights::get() .get(DispatchClass::Normal) .max_total From ef118d6200ffc992f97871f645fa267437e2703c Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 5 Aug 2024 11:00:42 +0200 Subject: [PATCH 08/10] Fix compilation --- substrate/frame/contracts/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index cb894e325b96..af3cfb41638c 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -684,7 +684,7 @@ pub mod pallet { .ref_time(); let max_payload_size = T::Schedule::get().limits.payload_len; let max_key_size = - Key::::try_from_var(vec![0u8; T::MaxStorageKeyLen::get() as usize]) + Key::::try_from_var(alloc::vec![0u8; T::MaxStorageKeyLen::get() as usize]) .expect("Key of maximal size shall be created") .hash() .len() as u32; From 0ee8f3763e24667f83c5326f116abe599c561722 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 5 Aug 2024 13:47:15 +0200 Subject: [PATCH 09/10] Update pr severity --- prdoc/pr_4973.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_4973.prdoc b/prdoc/pr_4973.prdoc index 1afbd3afbf7f..20b8c94dd8a9 100644 --- a/prdoc/pr_4973.prdoc +++ b/prdoc/pr_4973.prdoc @@ -12,4 +12,4 @@ doc: crates: - name: pallet-contracts - bump: patch + bump: major From 77d019a1fd043f7c340b71d55811cf8e021beea3 Mon Sep 17 00:00:00 2001 From: Sebastian Miasojed Date: Mon, 5 Aug 2024 15:12:40 +0200 Subject: [PATCH 10/10] Add less restrictive check --- substrate/frame/contracts/src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index af3cfb41638c..7bb5b46cf527 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -675,8 +675,8 @@ pub mod pallet { // Validators are configured to be able to use more memory than block builders. This is // because in addition to `max_runtime_mem` they need to hold additional data in // memory: PoV in multiple copies (1x encoded + 2x decoded) and all storage which - // includes emitted events. The assumption is that max_runtime_mem + storage/events - // can be a maximum of half of the validator runtime memory. + // includes emitted events. The assumption is that storage/events size + // can be a maximum of half of the validator runtime memory - max_runtime_mem. let max_block_ref_time = T::BlockWeights::get() .get(DispatchClass::Normal) .max_total @@ -702,8 +702,7 @@ pub mod pallet { .expect("Storage size too big"); let max_validator_runtime_mem: u32 = T::Schedule::get().limits.validator_runtime_memory; - let storage_size_limit = - (max_validator_runtime_mem / 2).saturating_sub(max_runtime_mem); + let storage_size_limit = max_validator_runtime_mem.saturating_sub(max_runtime_mem) / 2; assert!( max_storage_size < storage_size_limit,