Skip to content

Commit

Permalink
fix(runtime): Correct normal dispatches length (#4166)
Browse files Browse the repository at this point in the history
  • Loading branch information
breathx committed Aug 22, 2024
1 parent e8e01a8 commit 33ff492
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 22 deletions.
16 changes: 8 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions common/src/pallet_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ macro_rules! impl_pallet_balances_inner {
};
}

pub const NORMAL_DISPATCH_RATIO: Perbill = Perbill::from_percent(75);
pub const NORMAL_DISPATCH_WEIGHT_RATIO: Perbill = Perbill::from_percent(75);
pub const MAX_BLOCK: u64 = 250_000_000_000;

frame_support::parameter_types! {
pub RuntimeBlockWeights: BlockWeights = BlockWeights::with_sensible_defaults(
Weight::from_parts(MAX_BLOCK, u64::MAX),
NORMAL_DISPATCH_RATIO,
NORMAL_DISPATCH_WEIGHT_RATIO,
);
pub const SS58Prefix: u8 = 42;
pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight { read: 1_110, write: 2_300 };
Expand Down
2 changes: 1 addition & 1 deletion images/time-weight-synchronization.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 3 additions & 3 deletions node/authorship/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ The finalization of the block is not supposed to be included in this time frame.

In Gear we have a special pseudo-inherent that is called `Gear::run()` and has to be added at the end of each block after all the normal (or operational) extrinsics have been pushed. This pseudo-inherent is responsible for processing the message queue.

The `Runtime` specifies the so-called `NORMAL_DISPATCH_RATIO` constant that defines the share of the `DispatchClass::Normal` extrinsics in the overall block weight. In Gear this constant is set to 25% (as opposed to the default 80% in Substrate). This means that the want the `DispatchClass::Normal` extrinsics to take up to 25% of the block time, leaving the rest to `DispatchClass::Mandatory` (including the `Gear::run()`) and `DispatchClass::Operational` extrinsics.
The `Runtime` specifies the so-called `NORMAL_DISPATCH_WEIGHT_RATIO` constant that defines the share of the `DispatchClass::Normal` extrinsics in the overall block weight. In Gear this constant is set to 25% (as opposed to the default 80% in Substrate). This means that the want the `DispatchClass::Normal` extrinsics to take up to 25% of the block time, leaving the rest to `DispatchClass::Mandatory` (including the `Gear::run()`) and `DispatchClass::Operational` extrinsics.

<p align="lift">
<img src="../../images/substrate-vs-gear-block.svg" width="80%" alt="Gear">
Expand Down Expand Up @@ -72,8 +72,8 @@ There are two main invariants the `BlockBuilder` implementation should enforce t
To guarantee these invariants, we suggest the following approach:
- In addition to the overall proposal "hard" deadline introduce another "hard" deadline (as opposed to the existing `soft_deadline` which has a specific meaning and will remain) to limit the normal extrinsics application time to protect ourselves from the situation when the weight for the `DispatchClass:Normal` extrinsics hasn't been exhausted while timing-wise this stage has taken more than expected. In most cases, however, this deadline is never supposed to be hit, provided the benchmarked weights of all the extrinsics are correct;
- In case the normal extrinsics application stage hit the deadline, we can't anymore rely on the remaining block `gas_allowance` in the `Runtime`; if we did, the `Gear::run()` would think it has more gas to spend when it actually does. Therefore, we need to adjust the gas budget for the `Gear::run()` by passing it an explicit `Some(max_gas)` parameter;
- To calculate the extrinsic stage deadline, we can use a "relaxed" version of the `NORMAL_DISPATCH_RATIO` constant, say, 35% instead of 25%. This means that in the worst case, if we have run the extrinsics application phase for the entire 35% of the block proposal duration, we can then still let the `Gear::run()` run for 3/4 of the original proposal duration thereby expecting to exceed the ultimate deadline by 10% tops, which is still affordable given we have 1/3 of the `SlotProportion` for the block finalization and are never supposed to use up all of that. This `max_slippage` percentage can be made configurable;
- To calculate the extrinsic stage deadline, we can use a "relaxed" version of the `NORMAL_DISPATCH_WEIGHT_RATIO` constant, say, 35% instead of 25%. This means that in the worst case, if we have run the extrinsics application phase for the entire 35% of the block proposal duration, we can then still let the `Gear::run()` run for 3/4 of the original proposal duration thereby expecting to exceed the ultimate deadline by 10% tops, which is still affordable given we have 1/3 of the `SlotProportion` for the block finalization and are never supposed to use up all of that. This `max_slippage` percentage can be made configurable;
- To calculate the `max_gas` for the `Gear::run()` we have to resort to heuristic knowledge of the `time` to `gas` conversion: 1 pico-second is equivalent to 1 unit of gas. Therefore, the reasonable expectation is that if we manually set the `max_gas` to `7.5*10^11`, the `Gear::run()` will run for `750 ms`, or 3/4 of the block proposal duration so that the ultimate "relaxed" deadline is not exceeded;
- Despite the `max_gas` provided explicitly, we still make the `Gear::run()` to execute against a timeout to avoid skipping the entire block. The timeout will be set to the same value in pico-seconds as the `max_gas` absolute value. If triggered, the `Gear::run()` will be dropped entirely and all the changes in the storage overlay will be reverted. However, this is a very much undesirable situation and should be avoidedm because it'll lead to the message queue inflation, higher latency and transaction cost.
- Despite the `max_gas` provided explicitly, we still make the `Gear::run()` to execute against a timeout to avoid skipping the entire block. The timeout will be set to the same value in pico-seconds as the `max_gas` absolute value. If triggered, the `Gear::run()` will be dropped entirely and all the changes in the storage overlay will be reverted. However, this is a very much undesirable situation and should be avoidedm because it'll lead to the message queue inflation, higher latency and transaction cost.

The `soft_deadline` for the extrinsics application phase can be left at `50%`, as it is in the default Substrate block authorship implementation to serve the same purpose.
2 changes: 1 addition & 1 deletion node/authorship/src/authorship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub const DEFAULT_DEADLINE_SLIPPAGE: DurationMultiplier = DurationMultiplier(0.1

/// Default extrinsics application deadline fraction used by [`Proposer`].
///
/// Equivalent to the `NORMAL_DISPATCH_RATIO` in `Runtime`
/// Equivalent to the `NORMAL_DISPATCH_WEIGHT_RATIO` in `Runtime`
/// Can be overwritten by [`ProposerFactory::set_deadline`].
pub const DEFAULT_DISPATCH_RATIO: DurationMultiplier = DurationMultiplier(0.25);

Expand Down
16 changes: 11 additions & 5 deletions runtime/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,18 @@ use sp_runtime::Perbill;
/// Mostly we don't produce any calculations in `on_initialize` hook,
/// so it's safe to reduce from default 10 to custom 3 percents.
pub const AVERAGE_ON_INITIALIZE_RATIO: Perbill = Perbill::from_percent(3);
pub const NORMAL_DISPATCH_RATIO_NUM: u8 = 25;
pub const GAS_LIMIT_MIN_PERCENTAGE_NUM: u8 = 100 - NORMAL_DISPATCH_RATIO_NUM;

pub const NORMAL_DISPATCH_LENGTH_RATIO_NUM: u8 = 50;
pub const NORMAL_DISPATCH_LENGTH_RATIO: Perbill =
Perbill::from_percent(NORMAL_DISPATCH_LENGTH_RATIO_NUM as u32);

pub const NORMAL_DISPATCH_WEIGHT_RATIO_NUM: u8 = 25;
pub const GAS_LIMIT_MIN_PERCENTAGE_NUM: u8 = 100 - NORMAL_DISPATCH_WEIGHT_RATIO_NUM;

// Extrinsics with DispatchClass::Normal only account for user messages
// TODO: consider making the normal extrinsics share adjustable in runtime
pub const NORMAL_DISPATCH_RATIO: Perbill = Perbill::from_percent(NORMAL_DISPATCH_RATIO_NUM as u32);
pub const NORMAL_DISPATCH_WEIGHT_RATIO: Perbill =
Perbill::from_percent(NORMAL_DISPATCH_WEIGHT_RATIO_NUM as u32);

/// Returns common for gear protocol `BlockWeights` depend on given max block weight.
pub fn block_weights_for(maximum_block_weight: Weight) -> BlockWeights {
Expand All @@ -56,14 +62,14 @@ pub fn block_weights_for(maximum_block_weight: Weight) -> BlockWeights {
weights.base_extrinsic = ExtrinsicBaseWeight::get();
})
.for_class(DispatchClass::Normal, |weights| {
weights.max_total = Some(NORMAL_DISPATCH_RATIO * maximum_block_weight);
weights.max_total = Some(NORMAL_DISPATCH_WEIGHT_RATIO * maximum_block_weight);
})
.for_class(DispatchClass::Operational, |weights| {
weights.max_total = Some(maximum_block_weight);
// Operational transactions have some extra reserved space, so that they
// are included even if block reached `MAXIMUM_BLOCK_WEIGHT`.
weights.reserved =
Some(maximum_block_weight - NORMAL_DISPATCH_RATIO * maximum_block_weight);
Some(maximum_block_weight - NORMAL_DISPATCH_WEIGHT_RATIO * maximum_block_weight);
})
.avg_block_initialization(AVERAGE_ON_INITIALIZE_RATIO)
.build_or_panic()
Expand Down
5 changes: 3 additions & 2 deletions runtime/vara/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ pub use runtime_common::{
RESUME_SESSION_DURATION_HOUR_FACTOR,
},
impl_runtime_apis_plus_common, BlockHashCount, DealWithFees, AVERAGE_ON_INITIALIZE_RATIO,
GAS_LIMIT_MIN_PERCENTAGE_NUM, NORMAL_DISPATCH_RATIO, VALUE_PER_GAS,
GAS_LIMIT_MIN_PERCENTAGE_NUM, NORMAL_DISPATCH_LENGTH_RATIO, NORMAL_DISPATCH_WEIGHT_RATIO,
VALUE_PER_GAS,
};
pub use runtime_primitives::{AccountId, Signature, VARA_SS58_PREFIX};
use runtime_primitives::{Balance, BlockNumber, Hash, Moment, Nonce};
Expand Down Expand Up @@ -215,7 +216,7 @@ parameter_types! {
pub const SS58Prefix: u8 = VARA_SS58_PREFIX;
pub RuntimeBlockWeights: BlockWeights = runtime_common::block_weights_for(MAXIMUM_BLOCK_WEIGHT);
pub RuntimeBlockLength: BlockLength =
BlockLength::max_with_normal_ratio(5 * 1024 * 1024, NORMAL_DISPATCH_RATIO);
BlockLength::max_with_normal_ratio(5 * 1024 * 1024, NORMAL_DISPATCH_LENGTH_RATIO);
}

// Configure FRAME pallets to include in runtime.
Expand Down
14 changes: 14 additions & 0 deletions runtime/vara/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ fn bridge_session_timer_is_correct() {
);
}

#[test]
fn normal_dispatch_length_suits_minimal() {
const MB: u32 = 1024 * 1024;

let block_length = <Runtime as frame_system::Config>::BlockLength::get();

// Normal dispatch class is bigger than 2 MB.
assert!(*block_length.max.get(DispatchClass::Normal) > 2 * MB);

// Others are on maximum.
assert_eq!(*block_length.max.get(DispatchClass::Operational), 5 * MB);
assert_eq!(*block_length.max.get(DispatchClass::Mandatory), 5 * MB);
}

#[test]
fn instruction_weights_heuristics_test() {
let weights = InstructionWeights::<Runtime>::default();
Expand Down

0 comments on commit 33ff492

Please sign in to comment.