From bbefef33382fcab2e24b883841b75040c4088f42 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 6 Jan 2026 10:47:55 +0200 Subject: [PATCH 1/3] MAX_CALL_SIZE -> CALL_SIZE_LIMIT --- .../src/generic/unchecked_extrinsic.rs | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs b/substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs index 1b7d8d082b7c..99927bba9c04 100644 --- a/substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs +++ b/substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs @@ -66,7 +66,7 @@ pub const LEGACY_EXTRINSIC_FORMAT_VERSION: ExtrinsicVersion = 4; const EXTENSION_VERSION: ExtensionVersion = 0; /// Maximum decoded heap size for a runtime call (in bytes). -pub const DEFAULT_MAX_CALL_SIZE: usize = 16 * 1024 * 1024; // 16 MiB +pub const DEFAULT_CALL_SIZE_LIMIT: usize = 16 * 1024 * 1024 + 1; // 16 MiB /// The `SignaturePayload` of `UncheckedExtrinsic`. pub type UncheckedSignaturePayload = (Address, Signature, Extension); @@ -240,7 +240,7 @@ pub struct UncheckedExtrinsic< Call, Signature, Extension, - const MAX_CALL_SIZE: usize = DEFAULT_MAX_CALL_SIZE, + const CALL_SIZE_LIMIT: usize = DEFAULT_CALL_SIZE_LIMIT, > { /// Information regarding the type of extrinsic this is (inherent or transaction) as well as /// associated extension (`Extension`) data if it's a transaction and a possible signature. @@ -262,8 +262,8 @@ impl< Call: Debug, Signature: Debug, Extension: Debug, - const MAX_CALL_SIZE: usize, - > Debug for UncheckedExtrinsic + const CALL_SIZE_LIMIT: usize, + > Debug for UncheckedExtrinsic { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("UncheckedExtrinsic") @@ -278,8 +278,8 @@ impl< Call: PartialEq, Signature: PartialEq, Extension: PartialEq, - const MAX_CALL_SIZE: usize, - > PartialEq for UncheckedExtrinsic + const CALL_SIZE_LIMIT: usize, + > PartialEq for UncheckedExtrinsic { fn eq(&self, other: &Self) -> bool { self.preamble == other.preamble && self.function == other.function @@ -320,8 +320,8 @@ where } } -impl - UncheckedExtrinsic +impl + UncheckedExtrinsic { /// New instance of a bare (ne unsigned) extrinsic. This could be used for an inherent or an /// old-school "unsigned transaction" (which are new being deprecated in favour of general @@ -409,10 +409,7 @@ impl let mut clone_bytes = CloneBytes(&mut input, Vec::new()); - // Adds 1 byte to the `MAX_CALL_SIZE` as the decoding fails exactly at the given value and - // the maximum should be allowed to fit in. - let function = - Call::decode_with_mem_limit(&mut clone_bytes, MAX_CALL_SIZE.saturating_add(1))?; + let function = Call::decode_with_mem_limit(&mut clone_bytes, CALL_SIZE_LIMIT)?; let encoded_call = Some(clone_bytes.1); @@ -553,8 +550,8 @@ impl Decode - for UncheckedExtrinsic +impl Decode + for UncheckedExtrinsic where Address: Decode, Signature: Decode, @@ -734,8 +731,8 @@ where } } -impl LazyExtrinsic - for UncheckedExtrinsic +impl LazyExtrinsic + for UncheckedExtrinsic where Preamble: Decode, Call: DecodeWithMemTracking, @@ -1217,15 +1214,15 @@ mod tests { } #[test] - fn max_call_heap_size_should_be_checked() { + fn call_size_limit_should_be_checked() { // Should be able to decode an `UncheckedExtrinsic` that contains a call with - // heap size < `MAX_CALL_HEAP_SIZE` - let ux = Ex::new_bare(Call::Raw(vec![0u8; DEFAULT_MAX_CALL_SIZE]).into()); + // heap size < `CALL_SIZE_LIMIT` + let ux = Ex::new_bare(Call::Raw(vec![0u8; DEFAULT_CALL_SIZE_LIMIT - 1]).into()); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Ok(ux)); // Otherwise should fail - let ux = Ex::new_bare(Call::Raw(vec![0u8; DEFAULT_MAX_CALL_SIZE + 1]).into()); + let ux = Ex::new_bare(Call::Raw(vec![0u8; DEFAULT_CALL_SIZE_LIMIT]).into()); let encoded = ux.encode(); assert_eq!( Ex::decode(&mut &encoded[..]).unwrap_err().to_string(), From 1b5e4637d9b94d1b3d9cdccf14937501783d89f7 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 30 Oct 2025 10:50:07 +0200 Subject: [PATCH 2/3] DoubleEncoded: simplifications --- .../src/v2/converter/convert.rs | 5 +-- polkadot/xcm/src/double_encoded.rs | 37 +++---------------- polkadot/xcm/src/v3/mod.rs | 2 +- polkadot/xcm/src/v4/mod.rs | 6 +-- polkadot/xcm/src/v5/mod.rs | 2 +- polkadot/xcm/xcm-executor/src/lib.rs | 6 +-- 6 files changed, 15 insertions(+), 43 deletions(-) diff --git a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs index f64554e42756..bf4cbe12077f 100644 --- a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs +++ b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs @@ -284,9 +284,8 @@ where let transact_call = match_expression!(self.peek(), Ok(Transact { call, .. }), call); if let Some(transact_call) = transact_call { let _ = self.next(); - let transact = - ContractCall::decode_all(&mut transact_call.clone().into_encoded().as_slice()) - .map_err(|_| TransactDecodeFailed)?; + let transact = ContractCall::decode_all(&mut transact_call.encoded()) + .map_err(|_| TransactDecodeFailed)?; match transact { ContractCall::V1 { target, calldata, gas, value } => commands .push(Command::CallContract { target: target.into(), calldata, gas, value }), diff --git a/polkadot/xcm/src/double_encoded.rs b/polkadot/xcm/src/double_encoded.rs index 5efb637b9075..b2aab7985d5c 100644 --- a/polkadot/xcm/src/double_encoded.rs +++ b/polkadot/xcm/src/double_encoded.rs @@ -58,23 +58,13 @@ impl From> for DoubleEncoded { } impl DoubleEncoded { - pub fn into(self) -> DoubleEncoded { - DoubleEncoded::from(self) + pub fn encoded(&self) -> &[u8] { + &self.encoded } - pub fn from(e: DoubleEncoded) -> Self { - Self { encoded: e.encoded, decoded: None } - } - - /// Provides an API similar to `AsRef` that provides access to the inner value. - /// `AsRef` implementation would expect an `&Option` return type. - pub fn as_ref(&self) -> Option<&T> { - self.decoded.as_ref() - } - - /// Access the encoded data. - pub fn into_encoded(self) -> Vec { - self.encoded + /// Converts a `DoubleEncoded` into a `DoubleEncoded`, dropping the decoded value. + pub fn transmute_encoded(self) -> DoubleEncoded { + DoubleEncoded { encoded: self.encoded, decoded: None } } } @@ -90,16 +80,6 @@ impl DoubleEncoded { self.decoded.as_ref().ok_or(()) } - /// Move the decoded value out or (if not present) decode `encoded`. - pub fn take_decoded(&mut self) -> Result { - self.decoded - .take() - .or_else(|| { - T::decode_all_with_depth_limit(MAX_XCM_DECODE_DEPTH, &mut &self.encoded[..]).ok() - }) - .ok_or(()) - } - /// Provides an API similar to `TryInto` that allows fallible conversion to the inner value /// type. `TryInto` implementation would collide with std blanket implementation based on /// `TryFrom`. @@ -120,13 +100,6 @@ mod tests { assert_eq!(encoded.ensure_decoded(), Ok(&val)); } - #[test] - fn take_decoded_works() { - let val: u64 = 42; - let mut encoded: DoubleEncoded<_> = Encode::encode(&val).into(); - assert_eq!(encoded.take_decoded(), Ok(val)); - } - #[test] fn try_into_works() { let val: u64 = 42; diff --git a/polkadot/xcm/src/v3/mod.rs b/polkadot/xcm/src/v3/mod.rs index c6f3c7254ff6..e21e4df96f21 100644 --- a/polkadot/xcm/src/v3/mod.rs +++ b/polkadot/xcm/src/v3/mod.rs @@ -1161,7 +1161,7 @@ impl Instruction { HrmpChannelClosing { initiator, sender, recipient } => HrmpChannelClosing { initiator, sender, recipient }, Transact { origin_kind, require_weight_at_most, call } => - Transact { origin_kind, require_weight_at_most, call: call.into() }, + Transact { origin_kind, require_weight_at_most, call: call.transmute_encoded() }, ReportError(response_info) => ReportError(response_info), DepositAsset { assets, beneficiary } => DepositAsset { assets, beneficiary }, DepositReserveAsset { assets, dest, xcm } => DepositReserveAsset { assets, dest, xcm }, diff --git a/polkadot/xcm/src/v4/mod.rs b/polkadot/xcm/src/v4/mod.rs index 502200e84940..5bbe7d9c7846 100644 --- a/polkadot/xcm/src/v4/mod.rs +++ b/polkadot/xcm/src/v4/mod.rs @@ -1122,7 +1122,7 @@ impl Instruction { HrmpChannelClosing { initiator, sender, recipient } => HrmpChannelClosing { initiator, sender, recipient }, Transact { origin_kind, require_weight_at_most, call } => - Transact { origin_kind, require_weight_at_most, call: call.into() }, + Transact { origin_kind, require_weight_at_most, call: call.transmute_encoded() }, ReportError(response_info) => ReportError(response_info), DepositAsset { assets, beneficiary } => DepositAsset { assets, beneficiary }, DepositReserveAsset { assets, dest, xcm } => DepositReserveAsset { assets, dest, xcm }, @@ -1306,7 +1306,7 @@ impl TryFrom> for Instructi Transact { origin_kind, mut call, fallback_max_weight } => { // We first try to decode the call, if we can't, we use the fallback weight, // if there's no fallback, we just return `Weight::MAX`. - let require_weight_at_most = match call.take_decoded() { + let require_weight_at_most = match call.ensure_decoded() { Ok(decoded) => decoded.get_dispatch_info().call_weight, Err(error) => { let fallback_weight = fallback_max_weight.unwrap_or(Weight::MAX); @@ -1319,7 +1319,7 @@ impl TryFrom> for Instructi fallback_weight }, }; - Self::Transact { origin_kind, require_weight_at_most, call: call.into() } + Self::Transact { origin_kind, require_weight_at_most, call } }, ReportError(response_info) => Self::ReportError(QueryResponseInfo { query_id: response_info.query_id, diff --git a/polkadot/xcm/src/v5/mod.rs b/polkadot/xcm/src/v5/mod.rs index 0caf7d0c581f..300638f5b03a 100644 --- a/polkadot/xcm/src/v5/mod.rs +++ b/polkadot/xcm/src/v5/mod.rs @@ -1190,7 +1190,7 @@ impl Instruction { HrmpChannelClosing { initiator, sender, recipient } => HrmpChannelClosing { initiator, sender, recipient }, Transact { origin_kind, call, fallback_max_weight } => - Transact { origin_kind, call: call.into(), fallback_max_weight }, + Transact { origin_kind, call: call.transmute_encoded(), fallback_max_weight }, ReportError(response_info) => ReportError(response_info), DepositAsset { assets, beneficiary } => DepositAsset { assets, beneficiary }, DepositReserveAsset { assets, dest, xcm } => DepositReserveAsset { assets, dest, xcm }, diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index 1c569225ce2b..0163f3cb389b 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -1033,7 +1033,7 @@ impl XcmExecutor { }) }, // `fallback_max_weight` is not used in the executor, it's only for conversions. - Transact { origin_kind, mut call, .. } => { + Transact { origin_kind, call, .. } => { let origin = self.cloned_origin().ok_or_else(|| { tracing::trace!( target: "xcm::process_instruction::transact", @@ -1043,7 +1043,7 @@ impl XcmExecutor { XcmError::BadOrigin })?; - let message_call = call.take_decoded().map_err(|_| { + let message_call = call.try_into().map_err(|_| { tracing::trace!( target: "xcm::process_instruction::transact", "Failed to decode call", @@ -1054,7 +1054,7 @@ impl XcmExecutor { tracing::trace!( target: "xcm::process_instruction::transact", - ?call, + ?message_call, "Processing call", ); From ea4fbebce46a0378c6c7729deefb423dd0044eab Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 29 Oct 2025 18:12:42 +0200 Subject: [PATCH 3/3] Track nested memory used by `xcm::DoubleEncoded` --- Cargo.lock | 1 + polkadot/xcm/src/double_encoded.rs | 84 ++++++-- polkadot/xcm/xcm-executor/src/lib.rs | 188 ++++++++-------- substrate/frame/executive/src/lib.rs | 201 +++++++++--------- substrate/primitives/runtime/Cargo.toml | 2 + .../primitives/runtime/src/generic/mod.rs | 2 +- substrate/primitives/runtime/src/lib.rs | 1 + .../primitives/runtime/src/nested_mem.rs | 100 +++++++++ 8 files changed, 377 insertions(+), 202 deletions(-) create mode 100644 substrate/primitives/runtime/src/nested_mem.rs diff --git a/Cargo.lock b/Cargo.lock index cc7df710575a..743ac9e84eec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -23330,6 +23330,7 @@ dependencies = [ "bytes", "docify", "either", + "environmental", "hash256-std-hasher", "impl-trait-for-tuples", "log", diff --git a/polkadot/xcm/src/double_encoded.rs b/polkadot/xcm/src/double_encoded.rs index b2aab7985d5c..04d5b3db9ce3 100644 --- a/polkadot/xcm/src/double_encoded.rs +++ b/polkadot/xcm/src/double_encoded.rs @@ -16,7 +16,11 @@ use crate::MAX_XCM_DECODE_DEPTH; use alloc::vec::Vec; -use codec::{Decode, DecodeLimit, DecodeWithMemTracking, Encode}; +use codec::{Decode, DecodeLimit, DecodeWithMemTracking, Encode, Input}; + +use sp_runtime::nested_mem; + +const DECODE_ALL_ERR_MSG: &str = "Input buffer has still data left after decoding!"; /// Wrapper around the encoded and decoded versions of a value. /// Caches the decoded value once computed. @@ -29,7 +33,8 @@ use codec::{Decode, DecodeLimit, DecodeWithMemTracking, Encode}; pub struct DoubleEncoded { encoded: Vec, #[codec(skip)] - decoded: Option, + #[cfg_attr(feature = "json-schema", schemars(skip))] + decoded: Option<(T, Option)>, } impl Clone for DoubleEncoded { @@ -69,23 +74,39 @@ impl DoubleEncoded { } impl DoubleEncoded { + fn try_decode(&self) -> Result<(T, Option), codec::Error> { + nested_mem::decode_with_limiter(&mut &self.encoded[..], |mem_tracking_input| { + let decoded = T::decode_with_depth_limit(MAX_XCM_DECODE_DEPTH, mem_tracking_input)?; + if mem_tracking_input.remaining_len() != Ok(Some(0)) { + return Err(DECODE_ALL_ERR_MSG.into()); + } + Ok(decoded) + }) + } + /// Decode the inner encoded value and store it. /// Returns a reference to the value in case of success and `Err(())` in case the decoding /// fails. - pub fn ensure_decoded(&mut self) -> Result<&T, ()> { + pub fn ensure_decoded(&mut self) -> Result<&T, codec::Error> { if self.decoded.is_none() { - self.decoded = - T::decode_all_with_depth_limit(MAX_XCM_DECODE_DEPTH, &mut &self.encoded[..]).ok(); + self.decoded = Some(self.try_decode()?); } - self.decoded.as_ref().ok_or(()) + Ok(self + .decoded + .as_ref() + .map(|(decoded, _deallocation_reminder)| decoded) + .expect("The value has just been decoded")) } - /// Provides an API similar to `TryInto` that allows fallible conversion to the inner value - /// type. `TryInto` implementation would collide with std blanket implementation based on - /// `TryFrom`. - pub fn try_into(mut self) -> Result { + /// Do something with the decoded value, consuming `self`. + pub fn try_using_decoded(mut self, f: F) -> Result + where + F: FnOnce(T) -> R, + { self.ensure_decoded()?; - self.decoded.ok_or(()) + let (decoded, _deallocation_reminder) = + self.decoded.expect("The value has just been decoded"); + Ok(f(decoded)) } } @@ -93,6 +114,10 @@ impl DoubleEncoded { mod tests { use super::*; + use sp_runtime::generic::DEFAULT_CALL_SIZE_LIMIT; + + const DECODE_OOM_MSG: &str = "Heap memory limit exceeded while decoding"; + #[test] fn ensure_decoded_works() { let val: u64 = 42; @@ -101,9 +126,38 @@ mod tests { } #[test] - fn try_into_works() { - let val: u64 = 42; - let encoded: DoubleEncoded<_> = Encode::encode(&val).into(); - assert_eq!(encoded.try_into(), Ok(val)); + fn try_using_decoded_works() { + let val_1 = vec![1; DEFAULT_CALL_SIZE_LIMIT - 1000]; + let encoded_val_1: DoubleEncoded> = Encode::encode(&val_1).into(); + + assert_eq!(nested_mem::get_current_limit(), None); + nested_mem::using_limiter_once(|| { + assert_eq!(nested_mem::get_current_limit(), Some(DEFAULT_CALL_SIZE_LIMIT)); + encoded_val_1 + .try_using_decoded(|decoded_val| { + assert_eq!(nested_mem::get_current_limit(), Some(1000)); + assert_eq!(decoded_val, val_1); + + let val_2 = vec![2; 999]; + let encoded_val_2: DoubleEncoded> = Encode::encode(&val_2).into(); + let res = encoded_val_2.try_using_decoded(|decoded_val| { + assert_eq!(decoded_val, val_2); + assert_eq!(nested_mem::get_current_limit(), Some(1)); + }); + assert_eq!(res, Ok(())); + assert_eq!(nested_mem::get_current_limit(), Some(1000)); + + let val_2 = vec![2; 1000]; + let encoded_val_2: DoubleEncoded> = Encode::encode(&val_2).into(); + let res = encoded_val_2.try_using_decoded(|decoded_val| { + assert_eq!(decoded_val, val_2); + }); + assert_eq!(res, Err(DECODE_OOM_MSG.into())); + assert_eq!(nested_mem::get_current_limit(), Some(1000)); + }) + .unwrap(); + assert_eq!(nested_mem::get_current_limit(), Some(DEFAULT_CALL_SIZE_LIMIT)); + }); + assert_eq!(nested_mem::get_current_limit(), None); } } diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index 0163f3cb389b..0e00a1c43f21 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -850,8 +850,8 @@ impl XcmExecutor { for (i, mut instr) in xcm.0.into_iter().enumerate() { match &mut result { r @ Ok(()) => { - // Initialize the recursion count only the first time we hit this code in our - // potential recursive execution. + // Initialize the recursion count only the first time we hit this code in + // our potential recursive execution. let inst_res = recursion_count::using_once(&mut 1, || { recursion_count::with(|count| { if *count > RECURSION_LIMIT { @@ -863,8 +863,8 @@ impl XcmExecutor { .flatten() .ok_or(XcmError::ExceedsStackLimit)?; - // Ensure that we always decrement the counter whenever we finish processing - // the instruction. + // Ensure that we always decrement the counter whenever we finish + // processing the instruction. defer! { recursion_count::with(|count| { *count = count.saturating_sub(1); @@ -899,6 +899,89 @@ impl XcmExecutor { result } + fn process_transact( + &mut self, + origin_kind: OriginKind, + message_call: Config::RuntimeCall, + ) -> Result<(), XcmError> { + let origin = self.cloned_origin().ok_or_else(|| { + tracing::trace!( + target: "xcm::process_transact", + "No origin provided", + ); + + XcmError::BadOrigin + })?; + + tracing::trace!( + target: "xcm::process_transact", + ?message_call, + "Processing call", + ); + + if !Config::SafeCallFilter::contains(&message_call) { + tracing::trace!( + target: "xcm::process_transact", + "Call filtered by `SafeCallFilter`", + ); + + return Err(XcmError::NoPermission) + } + + let dispatch_origin = Config::OriginConverter::convert_origin(origin.clone(), origin_kind) + .map_err(|_| { + tracing::trace!( + target: "xcm::process_transact", + ?origin, + ?origin_kind, + "Failed to convert origin to a local origin." + ); + + XcmError::BadOrigin + })?; + + tracing::trace!( + target: "xcm::process_transact", + origin = ?dispatch_origin, + call = ?message_call, + "Dispatching call with origin", + ); + + let weight = message_call.get_dispatch_info().call_weight; + let maybe_actual_weight = + match Config::CallDispatcher::dispatch(message_call, dispatch_origin) { + Ok(post_info) => { + tracing::trace!( + target: "xcm::process_transact", + ?post_info, + "Dispatch successful" + ); + self.transact_status = MaybeErrorCode::Success; + post_info.actual_weight + }, + Err(error_and_info) => { + tracing::trace!( + target: "xcm::process_transact", + ?error_and_info, + "Dispatch failed" + ); + + self.transact_status = error_and_info.error.encode().into(); + error_and_info.post_info.actual_weight + }, + }; + let actual_weight = maybe_actual_weight.unwrap_or(weight); + let surplus = weight.saturating_sub(actual_weight); + // If the actual weight of the call was less than the specified weight, we credit it. + // + // We make the adjustment for the total surplus, which is used eventually + // reported back to the caller and this ensures that they account for the total + // weight consumed correctly (potentially allowing them to do more operations in a + // block than they otherwise would). + self.total_surplus.saturating_accrue(surplus); + Ok(()) + } + /// Process a single XCM instruction, mutating the state of the XCM virtual machine. fn process_instruction( &mut self, @@ -1034,93 +1117,18 @@ impl XcmExecutor { }, // `fallback_max_weight` is not used in the executor, it's only for conversions. Transact { origin_kind, call, .. } => { - let origin = self.cloned_origin().ok_or_else(|| { - tracing::trace!( - target: "xcm::process_instruction::transact", - "No origin provided", - ); - - XcmError::BadOrigin - })?; - - let message_call = call.try_into().map_err(|_| { - tracing::trace!( - target: "xcm::process_instruction::transact", - "Failed to decode call", - ); - - XcmError::FailedToDecode - })?; - - tracing::trace!( - target: "xcm::process_instruction::transact", - ?message_call, - "Processing call", - ); - - if !Config::SafeCallFilter::contains(&message_call) { - tracing::trace!( - target: "xcm::process_instruction::transact", - "Call filtered by `SafeCallFilter`", - ); - - return Err(XcmError::NoPermission) - } - - let dispatch_origin = - Config::OriginConverter::convert_origin(origin.clone(), origin_kind).map_err( - |_| { - tracing::trace!( - target: "xcm::process_instruction::transact", - ?origin, - ?origin_kind, - "Failed to convert origin to a local origin." - ); - - XcmError::BadOrigin - }, - )?; - - tracing::trace!( - target: "xcm::process_instruction::transact", - origin = ?dispatch_origin, - call = ?message_call, - "Dispatching call with origin", - ); - - let weight = message_call.get_dispatch_info().call_weight; - let maybe_actual_weight = - match Config::CallDispatcher::dispatch(message_call, dispatch_origin) { - Ok(post_info) => { - tracing::trace!( - target: "xcm::process_instruction::transact", - ?post_info, - "Dispatch successful" - ); - self.transact_status = MaybeErrorCode::Success; - post_info.actual_weight - }, - Err(error_and_info) => { - tracing::trace!( - target: "xcm::process_instruction::transact", - ?error_and_info, - "Dispatch failed" - ); + sp_runtime::nested_mem::using_limiter_once(|| { + call.try_using_decoded(|message_call| { + self.process_transact(origin_kind, message_call) + }).map_err(|_| { + tracing::trace!( + target: "xcm::process_instruction::transact", + "Failed to decode call", + ); - self.transact_status = error_and_info.error.encode().into(); - error_and_info.post_info.actual_weight - }, - }; - let actual_weight = maybe_actual_weight.unwrap_or(weight); - let surplus = weight.saturating_sub(actual_weight); - // If the actual weight of the call was less than the specified weight, we credit it. - // - // We make the adjustment for the total surplus, which is used eventually - // reported back to the caller and this ensures that they account for the total - // weight consumed correctly (potentially allowing them to do more operations in a - // block than they otherwise would). - self.total_surplus.saturating_accrue(surplus); - Ok(()) + XcmError::FailedToDecode + })? + }) }, QueryResponse { query_id, response, max_weight, querier } => { let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?; diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 4446ecf1e3d9..2cda2e670293 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -133,6 +133,7 @@ use frame_support::{ use frame_system::pallet_prelude::BlockNumberFor; use sp_runtime::{ generic::Digest, + nested_mem, traits::{ self, Applyable, CheckEqual, Checkable, Dispatchable, Header, LazyBlock, NumberFor, One, ValidateUnsigned, Zero, @@ -333,88 +334,94 @@ where signature_check: bool, select: frame_try_runtime::TryStateSelect, ) -> Result { - log::info!( - target: LOG_TARGET, - "try-runtime: executing block #{:?} / state root check: {:?} / signature check: {:?} / try-state-select: {:?}", - block.header().number(), - state_root_check, - signature_check, - select, - ); + nested_mem::using_limiter_once(|| { + log::info!( + target: LOG_TARGET, + "try-runtime: executing block #{:?} / state root check: {:?} / signature check: {:?} / try-state-select: {:?}", + block.header().number(), + state_root_check, + signature_check, + select, + ); - let mode = Self::initialize_block(block.header()); - Self::initial_checks(block.header()); + let mode = Self::initialize_block(block.header()); + Self::initial_checks(block.header()); - // Apply extrinsics: - let signature_check = if signature_check { - Block::Extrinsic::check - } else { - Block::Extrinsic::unchecked_into_checked_i_know_what_i_am_doing - }; - Self::apply_extrinsics(mode, block.extrinsics(), |uxt, is_inherent| { - Self::do_apply_extrinsic(uxt, is_inherent, signature_check) - })?; + // Apply extrinsics: + let signature_check = if signature_check { + Block::Extrinsic::check + } else { + Block::Extrinsic::unchecked_into_checked_i_know_what_i_am_doing + }; + Self::apply_extrinsics(mode, block.extrinsics(), |uxt, is_inherent| { + Self::do_apply_extrinsic(uxt, is_inherent, signature_check) + })?; - // In this case there were no transactions to trigger this state transition: - if !>::inherents_applied() { - Self::inherents_applied(); - } + // In this case there were no transactions to trigger this state transition: + if !>::inherents_applied() { + Self::inherents_applied(); + } - // post-extrinsics book-keeping - >::note_finished_extrinsics(); - ::PostTransactions::post_transactions(); + // post-extrinsics book-keeping + >::note_finished_extrinsics(); + ::PostTransactions::post_transactions(); - let header = block.header(); - Self::on_idle_hook(*header.number()); - Self::on_finalize_hook(*header.number()); - - // run the try-state checks of all pallets, ensuring they don't alter any state. - let _guard = frame_support::StorageNoopGuard::default(); - , - >>::try_state(*header.number(), select.clone()) - .map_err(|e| { - log::error!(target: LOG_TARGET, "failure: {:?}", e); - ExecutiveError::Custom(e.into()) - })?; - if select.any() { - let res = AllPalletsWithSystem::try_decode_entire_state(); - Self::log_decode_result(res).map_err(|e| ExecutiveError::Custom(e.into()))?; - } - drop(_guard); - - // do some of the checks that would normally happen in `final_checks`, but perhaps skip - // the state root check. - { - let new_header = >::finalize(); - let items_zip = header.digest().logs().iter().zip(new_header.digest().logs().iter()); - for (header_item, computed_item) in items_zip { - header_item.check_equal(computed_item); - assert!(header_item == computed_item, "Digest item must match that calculated."); + let header = block.header(); + Self::on_idle_hook(*header.number()); + Self::on_finalize_hook(*header.number()); + + // run the try-state checks of all pallets, ensuring they don't alter any state. + let _guard = frame_support::StorageNoopGuard::default(); + , + >>::try_state(*header.number(), select.clone()) + .map_err(|e| { + log::error!(target: LOG_TARGET, "failure: {:?}", e); + ExecutiveError::Custom(e.into()) + })?; + if select.any() { + let res = AllPalletsWithSystem::try_decode_entire_state(); + Self::log_decode_result(res).map_err(|e| ExecutiveError::Custom(e.into()))?; } + drop(_guard); + + // do some of the checks that would normally happen in `final_checks`, but perhaps skip + // the state root check. + { + let new_header = >::finalize(); + let items_zip = + header.digest().logs().iter().zip(new_header.digest().logs().iter()); + for (header_item, computed_item) in items_zip { + header_item.check_equal(computed_item); + assert!( + header_item == computed_item, + "Digest item must match that calculated." + ); + } + + if state_root_check { + let storage_root = new_header.state_root(); + header.state_root().check_equal(storage_root); + assert!( + header.state_root() == storage_root, + "Storage root must match that calculated." + ); + } - if state_root_check { - let storage_root = new_header.state_root(); - header.state_root().check_equal(storage_root); assert!( - header.state_root() == storage_root, - "Storage root must match that calculated." + header.extrinsics_root() == new_header.extrinsics_root(), + "Transaction trie root must be valid.", ); } - assert!( - header.extrinsics_root() == new_header.extrinsics_root(), - "Transaction trie root must be valid.", + log::info!( + target: LOG_TARGET, + "try-runtime: Block #{:?} successfully executed", + header.number(), ); - } - log::info!( - target: LOG_TARGET, - "try-runtime: Block #{:?} successfully executed", - header.number(), - ); - - Ok(frame_system::Pallet::::block_weight().total()) + Ok(frame_system::Pallet::::block_weight().total()) + }) } /// Execute all Migrations of this runtime. @@ -690,37 +697,39 @@ where /// Actually execute all transitions for `block`. pub fn execute_block(block: Block::LazyBlock) { - sp_io::init_tracing(); - sp_tracing::within_span! { - sp_tracing::info_span!("execute_block", ?block); - // Execute `on_runtime_upgrade` and `on_initialize`. - let mode = Self::initialize_block(block.header()); - Self::initial_checks(block.header()); - - let extrinsics = block.extrinsics(); - if let Err(e) = Self::apply_extrinsics( - mode, - extrinsics, - |uxt, is_inherent| { - Self::do_apply_extrinsic(uxt, is_inherent, Block::Extrinsic::check) + nested_mem::using_limiter_once(|| { + sp_io::init_tracing(); + sp_tracing::within_span! { + sp_tracing::info_span!("execute_block", ?block); + // Execute `on_runtime_upgrade` and `on_initialize`. + let mode = Self::initialize_block(block.header()); + Self::initial_checks(block.header()); + + let extrinsics = block.extrinsics(); + if let Err(e) = Self::apply_extrinsics( + mode, + extrinsics, + |uxt, is_inherent| { + Self::do_apply_extrinsic(uxt, is_inherent, Block::Extrinsic::check) + } + ) { + panic!("{:?}", e) } - ) { - panic!("{:?}", e) - } - // In this case there were no transactions to trigger this state transition: - if !>::inherents_applied() { - Self::inherents_applied(); - } + // In this case there were no transactions to trigger this state transition: + if !>::inherents_applied() { + Self::inherents_applied(); + } - >::note_finished_extrinsics(); - ::PostTransactions::post_transactions(); + >::note_finished_extrinsics(); + ::PostTransactions::post_transactions(); - let header = block.header(); - Self::on_idle_hook(*header.number()); - Self::on_finalize_hook(*header.number()); - Self::final_checks(&header); - } + let header = block.header(); + Self::on_idle_hook(*header.number()); + Self::on_finalize_hook(*header.number()); + Self::final_checks(&header); + } + }) } /// Logic that runs directly after inherent application. diff --git a/substrate/primitives/runtime/Cargo.toml b/substrate/primitives/runtime/Cargo.toml index 8d2fa76d1eeb..df078051fd1a 100644 --- a/substrate/primitives/runtime/Cargo.toml +++ b/substrate/primitives/runtime/Cargo.toml @@ -22,6 +22,7 @@ bytes = { workspace = true } codec = { features = ["derive", "max-encoded-len"], workspace = true } docify = { workspace = true } either = { workspace = true } +environmental = { workspace = true } hash256-std-hasher = { workspace = true } impl-trait-for-tuples = { workspace = true } log = { workspace = true } @@ -63,6 +64,7 @@ std = [ "codec/std", "either/std", "either/use_std", + "environmental/std", "hash256-std-hasher/std", "log/std", "num-traits/std", diff --git a/substrate/primitives/runtime/src/generic/mod.rs b/substrate/primitives/runtime/src/generic/mod.rs index 9e9da2643494..3126a94c8492 100644 --- a/substrate/primitives/runtime/src/generic/mod.rs +++ b/substrate/primitives/runtime/src/generic/mod.rs @@ -35,7 +35,7 @@ pub use self::{ header::Header, unchecked_extrinsic::{ CallAndMaybeEncoded, ExtensionVersion, Preamble, SignedPayload, UncheckedExtrinsic, - EXTRINSIC_FORMAT_VERSION, + DEFAULT_CALL_SIZE_LIMIT, EXTRINSIC_FORMAT_VERSION, }, }; pub use unchecked_extrinsic::UncheckedSignaturePayload; diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index 0f4b043a454e..330aaf70b2f2 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/substrate/primitives/runtime/src/lib.rs @@ -87,6 +87,7 @@ pub mod curve; pub mod generic; pub mod legacy; mod multiaddress; +pub mod nested_mem; pub mod offchain; pub mod proving_trie; pub mod runtime_logger; diff --git a/substrate/primitives/runtime/src/nested_mem.rs b/substrate/primitives/runtime/src/nested_mem.rs new file mode 100644 index 000000000000..edd8871f9de0 --- /dev/null +++ b/substrate/primitives/runtime/src/nested_mem.rs @@ -0,0 +1,100 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Utils for keeping track of the heap memory allocated for decoding structures within nested +//! contexts during runtime code execution. +//! +//! For example an object that is double-encoded within an extrinsic would trigger such a scenario. +//! In this case the object will be decoded once together with the extrinsic and at that point its +//! heap memory use will be accounted for by the extrinsic decoding logic. Then it will be decoded +//! once more while executing the extrinsic, and we will need to separately keep track of the heap +//! memory used during this step as well. +//! +//! Another example would be a double-encoded object that we read from the storage while executing +//! a hook (e.g. `on_idle()`). +//! +//! There are also cases where there can be multiple nested double-encoded layers. + +use crate::generic::DEFAULT_CALL_SIZE_LIMIT; +use codec::{Decode, Error as CodecError, Input, MemTrackingInput}; + +// Global variable for keeping track of the heap memory allocated for decoding objects within +// nested contexts. +environmental::environmental!(limiter: usize); + +/// Get the current value stored in the `limiter` global variable. +pub fn get_current_limit() -> Option { + limiter::with(|mem_limit| *mem_limit) +} + +/// Initialize the `limiter` global variable. +/// +/// Any runtime logic that may lead to decoding double encoded objects while executing runtime code +/// should be called within this context. +pub fn using_limiter_once(f: F) -> R +where + F: FnOnce() -> R, +{ + let mut mem_limit = DEFAULT_CALL_SIZE_LIMIT; + limiter::using_once(&mut mem_limit, f) +} + +/// Helper struct used to update the `limiter` when some of the tracked memory is deallocated. +pub struct DeallocationReminder { + mem_size: usize, +} + +impl DeallocationReminder { + /// Create a new instance of `DeallocationReminder`. + pub fn new(mem_size: usize) -> Self { + Self { mem_size } + } +} + +impl Drop for DeallocationReminder { + fn drop(&mut self) { + let _ = limiter::with(|mem_limit| { + *mem_limit = mem_limit.saturating_add(self.mem_size); + *mem_limit = core::cmp::min(*mem_limit, DEFAULT_CALL_SIZE_LIMIT); + }); + } +} + +/// Helper function used to decode an object within a nested context. +/// +/// Apart from decoding the object, this method also returns a `DeallocationReminder` in order to +/// keep track of the heap memory that was allocated in the nested context. +pub fn decode_with_limiter< + I: Input, + T: Decode, + F: FnOnce(&mut MemTrackingInput) -> Result + Clone, +>( + input: &mut I, + decode_fn: F, +) -> Result<(T, Option), CodecError> { + limiter::with(|mem_limit| { + let mut mem_tracking_input = MemTrackingInput::new(input, *mem_limit); + let decoded = decode_fn.clone()(&mut mem_tracking_input)?; + let used_mem = mem_tracking_input.used_mem(); + *mem_limit = mem_limit.saturating_sub(used_mem); + Ok((decoded, Some(DeallocationReminder::new(used_mem)))) + }) + .unwrap_or_else(|| { + let mut mem_tracking_input = MemTrackingInput::new(input, DEFAULT_CALL_SIZE_LIMIT); + Ok((decode_fn(&mut mem_tracking_input)?, None)) + }) +}