From 7c6aedbbd7de94d38ff806e3e67ea025a4e34d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Fri, 10 Jan 2025 14:56:01 +0000 Subject: [PATCH] Remove `enforce_subcall_limit` and associated logic --- substrate/frame/revive/src/exec.rs | 18 ++------- substrate/frame/revive/src/storage/meter.rs | 41 ++------------------- substrate/frame/revive/src/tests.rs | 4 +- 3 files changed, 10 insertions(+), 53 deletions(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index a8d0da88ff40..75db1bc7a4fe 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1117,25 +1117,15 @@ where return Ok(output); } - // Storage limit is normally enforced as late as possible (when the last frame returns) - // so that the ordering of storage accesses does not matter. - // (However, if a special limit was set for a sub-call, it should be enforced right - // after the sub-call returned. See below for this case of enforcement). - if self.frames.is_empty() { - let frame = &mut self.first_frame; - frame.contract_info.load(&frame.account_id); - let contract = frame.contract_info.as_contract(); - frame.nested_storage.enforce_limit(contract)?; - } - let frame = self.top_frame_mut(); - // If a special limit was set for the sub-call, we enforce it here. - // The sub-call will be rolled back in case the limit is exhausted. + // The storage deposit is only charged at the end of every call stack. + // To make sure that no sub call uses more than it is allowed to, + // the limit is manually enforced here. let contract = frame.contract_info.as_contract(); frame .nested_storage - .enforce_subcall_limit(contract) + .enforce_limit(contract) .map_err(|e| ExecError { error: e, origin: ErrorOrigin::Callee })?; let account_id = T::AddressMapper::to_address(&frame.account_id); diff --git a/substrate/frame/revive/src/storage/meter.rs b/substrate/frame/revive/src/storage/meter.rs index dbf78f2fc929..d61a6407307f 100644 --- a/substrate/frame/revive/src/storage/meter.rs +++ b/substrate/frame/revive/src/storage/meter.rs @@ -101,12 +101,8 @@ pub struct Root; /// State parameter that constitutes a meter that is in its nested state. /// Its value indicates whether the nested meter has its own limit. -#[derive(DefaultNoBound, RuntimeDebugNoBound)] -pub enum Nested { - #[default] - DerivedLimit, - OwnLimit, -} +#[derive(Default, Debug)] +pub struct Nested; impl State for Root {} impl State for Nested {} @@ -125,10 +121,8 @@ pub struct RawMeter { /// We only have one charge per contract hence the size of this vector is /// limited by the maximum call depth. charges: Vec>, - /// We store the nested state to determine if it has a special limit for sub-call. - nested: S, /// Type parameter only used in impls. - _phantom: PhantomData, + _phantom: PhantomData<(E, S)>, } /// This type is used to describe a storage change when charging from the meter. @@ -289,15 +283,7 @@ where pub fn nested(&self, limit: BalanceOf) -> RawMeter { debug_assert!(matches!(self.contract_state(), ContractState::Alive)); - if limit == BalanceOf::::max_value() { - RawMeter { limit: self.available(), ..Default::default() } - } else { - // If a special limit is specified higher than it is available, - // we want to enforce the lesser limit to the nested meter, to fail in the sub-call. - let limit = self.available().min(limit); - - RawMeter { limit, nested: Nested::OwnLimit, ..Default::default() } - } + RawMeter { limit: self.available().min(limit), ..Default::default() } } /// Absorb a child that was spawned to handle a sub call. @@ -479,13 +465,6 @@ impl> RawMeter { /// [`Self::charge`] does not enforce the storage limit since we want to do this check as late /// as possible to allow later refunds to offset earlier charges. - /// - /// # Note - /// - /// We normally need to call this **once** for every call stack and not for every cross contract - /// call. However, if a dedicated limit is specified for a sub-call, this needs to be called - /// once the sub-call has returned. For this, the [`Self::enforce_subcall_limit`] wrapper is - /// used. pub fn enforce_limit( &mut self, info: Option<&mut ContractInfo>, @@ -504,18 +483,6 @@ impl> RawMeter { } Ok(()) } - - /// This is a wrapper around [`Self::enforce_limit`] to use on the exit from a sub-call to - /// enforce its special limit if needed. - pub fn enforce_subcall_limit( - &mut self, - info: Option<&mut ContractInfo>, - ) -> Result<(), DispatchError> { - match self.nested { - Nested::OwnLimit => self.enforce_limit(info), - Nested::DerivedLimit => Ok(()), - } - } } impl Ext for ReservingExt { diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 04a6c4416062..6f08adc28baf 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -2265,8 +2265,8 @@ fn gas_estimation_for_subcalls() { GAS_LIMIT, GAS_LIMIT * 2, GAS_LIMIT / 5, - Weight::from_parts(0, GAS_LIMIT.proof_size()), - Weight::from_parts(GAS_LIMIT.ref_time(), 0), + Weight::from_parts(u64::MAX, GAS_LIMIT.proof_size()), + Weight::from_parts(GAS_LIMIT.ref_time(), u64::MAX), ]; // This call is passed to the sub call in order to create a large `required_weight`