Skip to content

Commit

Permalink
Remove enforce_subcall_limit and associated logic
Browse files Browse the repository at this point in the history
  • Loading branch information
rockbmb committed Jan 10, 2025
1 parent 3e4bbe8 commit 7c6aedb
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 53 deletions.
18 changes: 4 additions & 14 deletions substrate/frame/revive/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
41 changes: 4 additions & 37 deletions substrate/frame/revive/src/storage/meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand All @@ -125,10 +121,8 @@ pub struct RawMeter<T: Config, E, S: State + Default + Debug> {
/// We only have one charge per contract hence the size of this vector is
/// limited by the maximum call depth.
charges: Vec<Charge<T>>,
/// 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<E>,
_phantom: PhantomData<(E, S)>,
}

/// This type is used to describe a storage change when charging from the meter.
Expand Down Expand Up @@ -289,15 +283,7 @@ where
pub fn nested(&self, limit: BalanceOf<T>) -> RawMeter<T, E, Nested> {
debug_assert!(matches!(self.contract_state(), ContractState::Alive));

if limit == BalanceOf::<T>::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.
Expand Down Expand Up @@ -479,13 +465,6 @@ impl<T: Config, E: Ext<T>> RawMeter<T, E, Nested> {

/// [`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<T>>,
Expand All @@ -504,18 +483,6 @@ impl<T: Config, E: Ext<T>> RawMeter<T, E, Nested> {
}
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<T>>,
) -> Result<(), DispatchError> {
match self.nested {
Nested::OwnLimit => self.enforce_limit(info),
Nested::DerivedLimit => Ok(()),
}
}
}

impl<T: Config> Ext<T> for ReservingExt {
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/revive/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down

0 comments on commit 7c6aedb

Please sign in to comment.