Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Optional PoV block limits #13164

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions frame/support/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,10 @@ impl PerDispatchClass<Weight> {

/// Add some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`.
pub fn add(&mut self, weight: Weight, class: DispatchClass) {
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
self.saturating_accrue(weight, class);
}

pub fn saturating_accrue(&mut self, weight: Weight, class: DispatchClass) {
let value = self.get_mut(class);
*value = value.saturating_add(weight);
}
Expand Down
72 changes: 39 additions & 33 deletions frame/system/src/extensions/check_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,10 @@ where
fn check_extrinsic_weight(
info: &DispatchInfoOf<T::RuntimeCall>,
) -> Result<(), TransactionValidityError> {
let max = T::BlockWeights::get().get(info.class).max_extrinsic;
match max {
Some(max) if info.weight.any_gt(max) =>
Err(InvalidTransaction::ExhaustsResources.into()),
_ => Ok(()),
}
let limit = T::BlockWeights::get().get(info.class).max_extrinsic;
limit
.check_within(info.weight)
.map_err(|_| InvalidTransaction::ExhaustsResources.into())
}

/// Checks if the current extrinsic can fit into the block with respect to block weight limits.
Expand Down Expand Up @@ -134,36 +132,40 @@ where
let limit_per_class = maximum_weight.get(info.class);

// add the weight. If class is unlimited, use saturating add instead of checked one.
if limit_per_class.max_total.is_none() && limit_per_class.reserved.is_none() {
all_weight.add(extrinsic_weight, info.class)
if limit_per_class.max_total.is_time_unlimited() && limit_per_class.reserved.is_time_unlimited()
{
all_weight.saturating_accrue(extrinsic_weight.without_proof_size(), info.class)
} else {
all_weight
.checked_add(extrinsic_weight, info.class)
.checked_add(extrinsic_weight.without_proof_size(), info.class)
.map_err(|_| InvalidTransaction::ExhaustsResources)?;
}

if limit_per_class.max_total.is_proof_unlimited() &&
limit_per_class.reserved.is_proof_unlimited()
{
all_weight.saturating_accrue(extrinsic_weight.without_ref_time(), info.class)
} else {
all_weight
.checked_add(extrinsic_weight.without_ref_time(), info.class)
.map_err(|_| InvalidTransaction::ExhaustsResources)?;
}

let per_class = *all_weight.get(info.class);

// Check if we don't exceed per-class allowance
match limit_per_class.max_total {
Some(max) if per_class.any_gt(max) =>
return Err(InvalidTransaction::ExhaustsResources.into()),
// There is no `max_total` limit (`None`),
// or we are below the limit.
_ => {},
}
limit_per_class
.max_total
.check_within(per_class)
.map_err(|_| InvalidTransaction::ExhaustsResources)?;

// In cases total block weight is exceeded, we need to fall back
// to `reserved` pool if there is any.
if all_weight.total().any_gt(maximum_weight.max_block) {
match limit_per_class.reserved {
// We are over the limit in reserved pool.
Some(reserved) if per_class.any_gt(reserved) =>
return Err(InvalidTransaction::ExhaustsResources.into()),
// There is either no limit in reserved pool (`None`),
// or we are below the limit.
_ => {},
}
limit_per_class
.reserved
.check_within(per_class)
.map_err(|_| InvalidTransaction::ExhaustsResources)?;
}

Ok(all_weight)
Expand Down Expand Up @@ -258,6 +260,7 @@ mod tests {
};
use frame_support::{assert_err, assert_ok, dispatch::Pays, weights::Weight};
use sp_std::marker::PhantomData;
use sp_weights::WeightLimit;

fn block_weights() -> crate::limits::BlockWeights {
<Test as crate::Config>::BlockWeights::get()
Expand All @@ -266,8 +269,8 @@ mod tests {
fn normal_weight_limit() -> Weight {
block_weights()
.get(DispatchClass::Normal)
.max_total
.unwrap_or_else(|| block_weights().max_block)
.max_total // if the class total limit is unlimited, then the block limit is used.
.chromatic_limited_or(block_weights().max_block)
}

fn block_weight_limit() -> Weight {
Expand Down Expand Up @@ -307,8 +310,11 @@ mod tests {
fn normal_extrinsic_limited_by_maximum_extrinsic_weight() {
new_test_ext().execute_with(|| {
let max = DispatchInfo {
weight: block_weights().get(DispatchClass::Normal).max_extrinsic.unwrap() +
Weight::from_ref_time(1),
weight: block_weights()
.get(DispatchClass::Normal)
.max_extrinsic
.exact_limits()
.unwrap() + Weight::from_ref_time(1), // FAIL-CI
class: DispatchClass::Normal,
..Default::default()
};
Expand All @@ -327,7 +333,7 @@ mod tests {
let operational_limit = weights
.get(DispatchClass::Operational)
.max_total
.unwrap_or_else(|| weights.max_block);
.chromatic_limited_or(weights.max_block);
let base_weight = weights.get(DispatchClass::Operational).base_extrinsic;

let weight = operational_limit - base_weight;
Expand Down Expand Up @@ -668,20 +674,20 @@ mod tests {
.base_block(Weight::zero())
.for_class(DispatchClass::non_mandatory(), |w| {
w.base_extrinsic = Weight::zero();
w.max_total = Some(Weight::from_ref_time(20).set_proof_size(u64::MAX));
w.max_total = Weight::from_ref_time(20).set_proof_size(20000).into();
})
.for_class(DispatchClass::Mandatory, |w| {
w.base_extrinsic = Weight::zero();
w.reserved = Some(Weight::from_ref_time(5).set_proof_size(u64::MAX));
w.max_total = None;
w.reserved = Weight::from_ref_time(5).set_proof_size(20000).into();
w.max_total = WeightLimit::UNLIMITED;
})
.build_or_panic();
let all_weight = crate::ConsumedWeight::new(|class| match class {
DispatchClass::Normal => Weight::from_ref_time(10),
DispatchClass::Operational => Weight::from_ref_time(10),
DispatchClass::Mandatory => Weight::zero(),
});
assert_eq!(maximum_weight.max_block, all_weight.total().set_proof_size(u64::MAX));
assert_eq!(maximum_weight.max_block, all_weight.total().set_proof_size(20000));

// fits into reserved
let mandatory1 = DispatchInfo {
Expand Down
45 changes: 38 additions & 7 deletions frame/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,17 +503,33 @@ pub mod pallet {
#[pallet::event]
pub enum Event<T: Config> {
/// An extrinsic completed successfully.
ExtrinsicSuccess { dispatch_info: DispatchInfo },
ExtrinsicSuccess {
dispatch_info: DispatchInfo,
},
/// An extrinsic failed.
ExtrinsicFailed { dispatch_error: DispatchError, dispatch_info: DispatchInfo },
ExtrinsicFailed {
dispatch_error: DispatchError,
dispatch_info: DispatchInfo,
},
/// `:code` was updated.
CodeUpdated,
/// A new account was created.
NewAccount { account: T::AccountId },
NewAccount {
account: T::AccountId,
},
/// An account was reaped.
KilledAccount { account: T::AccountId },
KilledAccount {
account: T::AccountId,
},
/// On on-chain remark happened.
Remarked { sender: T::AccountId, hash: T::Hash },
Remarked {
sender: T::AccountId,
hash: T::Hash,
},
PovSoftLimitExceeded {
limit: u64,
consumed: u64,
},
}

/// Error for the System pallet
Expand Down Expand Up @@ -1338,7 +1354,22 @@ impl<T: Config> Pallet<T> {
/// Remove temporary "environment" entries in storage, compute the storage root and return the
/// resulting header for this block.
pub fn finalize() -> T::Header {
log::debug!(
// Check the soft weight limit.
let consumed = BlockWeight::<T>::get();
if consumed.total().proof_size() > 5 * 1024 * 1024 {
log::warn!(
target: LOG_TARGET,
"Block {:?} consumed more than 5MiB of proof size",
Self::block_number()
);
// emit an event
Self::deposit_event(Event::PovSoftLimitExceeded {
limit: 5 * 1024 * 1024,
consumed: consumed.total().proof_size(),
});
}

/*log::debug!(
target: LOG_TARGET,
"[{:?}] {} extrinsics, length: {} (normal {}%, op: {}%, mandatory {}%) / normal weight:\
{} ({}%) op weight {} ({}%) / mandatory weight {} ({}%)",
Expand Down Expand Up @@ -1372,7 +1403,7 @@ impl<T: Config> Pallet<T> {
Self::block_weight().get(DispatchClass::Mandatory).ref_time(),
T::BlockWeights::get().get(DispatchClass::Mandatory).max_total.unwrap_or(Bounded::max_value()).ref_time()
).deconstruct(),
);
);*/
ExecutionPhase::<T>::kill();
AllExtrinsicsLen::<T>::kill();

Expand Down
62 changes: 35 additions & 27 deletions frame/system/src/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ use frame_support::{
weights::{constants, Weight},
};
use scale_info::TypeInfo;
use sp_runtime::{traits::Bounded, Perbill, RuntimeDebug};
use sp_runtime::{Perbill, RuntimeDebug};
use sp_weights::WeightLimit;

/// Block length limit configuration.
#[derive(RuntimeDebug, Clone, codec::Encode, codec::Decode, TypeInfo)]
Expand Down Expand Up @@ -101,24 +102,25 @@ pub struct WeightsPerClass {
pub base_extrinsic: Weight,
/// Maximal weight of single extrinsic. Should NOT include `base_extrinsic` cost.
///
/// `None` indicates that this class of extrinsics doesn't have a limit.
pub max_extrinsic: Option<Weight>,
/// FAIL-CI fix docs
/// `UNLIMITED` indicates that this class of extrinsics doesn't have a limit.
pub max_extrinsic: WeightLimit,
/// Block maximal total weight for all extrinsics of given class.
///
/// `None` indicates that weight sum of this class of extrinsics is not
/// `UNLIMITED` indicates that weight sum of this class of extrinsics is not
/// restricted. Use this value carefully, since it might produce heavily oversized
/// blocks.
///
/// In the worst case, the total weight consumed by the class is going to be:
/// `MAX(max_total) + MAX(reserved)`.
pub max_total: Option<Weight>,
pub max_total: WeightLimit,
/// Block reserved allowance for all extrinsics of a particular class.
///
/// Setting to `None` indicates that extrinsics of that class are allowed
/// to go over total block weight (but at most `max_total` for that class).
/// Setting to `Some(x)` guarantees that at least `x` weight of particular class
/// is processed in every block.
pub reserved: Option<Weight>,
pub reserved: WeightLimit,
}

/// Block weight limits & base values configuration.
Expand Down Expand Up @@ -222,16 +224,13 @@ impl BlockWeights {

/// Verifies correctness of this `BlockWeights` object.
pub fn validate(self) -> ValidationResult {
fn or_max(w: Option<Weight>) -> Weight {
w.unwrap_or_else(Weight::max_value)
}
let mut error = ValidationErrors::default();

for class in DispatchClass::all() {
let weights = self.per_class.get(*class);
let max_for_class = or_max(weights.max_total);
let max_for_class = weights.max_total.limited_or_max();
let base_for_class = weights.base_extrinsic;
let reserved = or_max(weights.reserved);
let reserved = weights.reserved.limited_or_max();
// Make sure that if total is set it's greater than base_block &&
// base_for_class
error_assert!(
Expand All @@ -245,17 +244,17 @@ impl BlockWeights {
error_assert!(
weights
.max_extrinsic
.unwrap_or(Weight::zero())
.limited_or_min()
.all_lte(max_for_class.saturating_sub(base_for_class)),
&mut error,
"[{:?}] {:?} (max_extrinsic) can't be greater than {:?} (max for class)",
class,
weights.max_extrinsic,
max_for_class.saturating_sub(base_for_class),
);
// Max extrinsic should not be 0
// Max extrinsic should have a value for each component.
error_assert!(
weights.max_extrinsic.unwrap_or_else(Weight::max_value).all_gt(Weight::zero()),
!weights.max_extrinsic.is_nothing(),
&mut error,
"[{:?}] {:?} (max_extrinsic) must not be 0. Check base cost and average initialization cost.",
class, weights.max_extrinsic,
Expand All @@ -271,7 +270,7 @@ impl BlockWeights {
);
// Make sure max block is greater than max_total if it's set.
error_assert!(
self.max_block.all_gte(weights.max_total.unwrap_or(Weight::zero())),
self.max_block.all_gte(weights.max_total.limited_or_min()),
&mut error,
"[{:?}] {:?} (max block) has to be greater than {:?} (max for class)",
class,
Expand Down Expand Up @@ -308,7 +307,7 @@ impl BlockWeights {
weights.base_extrinsic = Weight::zero();
})
.for_class(DispatchClass::non_mandatory(), |weights| {
weights.max_total = block_weight.into();
weights.max_total = WeightLimit::from_weight(block_weight);
})
.build()
.expect("We only specify max_total and leave base values as defaults; qed")
Expand Down Expand Up @@ -342,13 +341,16 @@ impl BlockWeights {
BlockWeightsBuilder {
weights: BlockWeights {
base_block: constants::BlockExecutionWeight::get(),
max_block: Weight::zero(),
max_block: Weight::zero(), // This will be set by `build`.
per_class: PerDispatchClass::new(|class| {
let initial =
if class == DispatchClass::Mandatory { None } else { Some(Weight::zero()) };
let initial = if class == DispatchClass::Mandatory {
WeightLimit::UNLIMITED
} else {
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
WeightLimit::NOTHING
};
WeightsPerClass {
base_extrinsic: constants::ExtrinsicBaseWeight::get(),
max_extrinsic: None,
max_extrinsic: WeightLimit::UNLIMITED,
max_total: initial,
reserved: initial,
}
Expand Down Expand Up @@ -407,20 +409,26 @@ impl BlockWeightsBuilder {

// compute max block size.
for class in DispatchClass::all() {
weights.max_block = match weights.per_class.get(*class).max_total {
Some(max) => max.max(weights.max_block),
_ => weights.max_block,
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
};
// FAIL-CI why is this not just maxing to unlimited aka `limited_or_max`?
weights.max_block =
weights.per_class.get(*class).max_total.limited_or_min().max(weights.max_block);
}
// compute max size of single extrinsic
if let Some(init_weight) = init_cost.map(|rate| rate * weights.max_block) {
for class in DispatchClass::all() {
let per_class = weights.per_class.get_mut(*class);
if per_class.max_extrinsic.is_none() && init_cost.is_some() {
// FAIL-CI do this per component?
if per_class.max_extrinsic.is_time_unlimited() && init_cost.is_some() {
per_class.max_extrinsic = per_class
.max_total
.saturating_sub(init_weight.without_proof_size())
.saturating_sub(per_class.base_extrinsic.without_proof_size());
}
if per_class.max_extrinsic.is_proof_unlimited() && init_cost.is_some() {
per_class.max_extrinsic = per_class
.max_total
.map(|x| x.saturating_sub(init_weight))
.map(|x| x.saturating_sub(per_class.base_extrinsic));
.saturating_sub(init_weight.without_ref_time())
.saturating_sub(per_class.base_extrinsic.without_ref_time());
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions frame/system/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ parameter_types! {
weights.base_extrinsic = Weight::from_ref_time(5);
})
.for_class(DispatchClass::Normal, |weights| {
weights.max_total = Some(NORMAL_DISPATCH_RATIO * MAX_BLOCK_WEIGHT);
weights.max_total = (NORMAL_DISPATCH_RATIO * MAX_BLOCK_WEIGHT).into();
})
.for_class(DispatchClass::Operational, |weights| {
weights.base_extrinsic = Weight::from_ref_time(10);
weights.max_total = Some(MAX_BLOCK_WEIGHT);
weights.reserved = Some(
weights.max_total = MAX_BLOCK_WEIGHT.into();
weights.reserved = (
MAX_BLOCK_WEIGHT - NORMAL_DISPATCH_RATIO * MAX_BLOCK_WEIGHT
);
).into();
})
.avg_block_initialization(Perbill::from_percent(0))
.build_or_panic();
Expand Down
Loading