Take into account size as well in weight limiting. (paritytech#7369)

* Take into account size as well in weight limiting. * Fix logging. * More logs. * Remove randomized selection in provisioner No longer supported by runtime. * Fix and simplify weight calculation. Random filtering of remote disputes got dropped. * Make existing tests pass. * Tests for size limiting. * Fix provisioner. * Remove rand dependency. * Better default block length for tests. * ".git/.scripts/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent * ".git/.scripts/commands/bench/bench.sh" runtime polkadot runtime_parachains::paras_inherent * ".git/.scripts/commands/bench/bench.sh" runtime westend runtime_parachains::paras_inherent * Update runtime/parachains/src/paras_inherent/mod.rs Co-authored-by: Tsvetomir Dimitrov <[email protected]> * Update runtime/parachains/src/paras_inherent/mod.rs Co-authored-by: Chris Sosnin <[email protected]> * Add back missing line. * Fix test. * fmt fix. * Add missing test annotation --------- Co-authored-by: eskimor <[email protected]> Co-authored-by: command-bot <> Co-authored-by: Tsvetomir Dimitrov <[email protected]> Co-authored-by: Chris Sosnin <[email protected]>
arturgontijo · Aug 1, 2023 · ff5f602 · ff5f602
1 parent c25980c
commit ff5f602
Show file tree

Hide file tree

Showing 13 changed files with 438 additions and 402 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/node/core/provisioner/Cargo.toml b/node/core/provisioner/Cargo.toml
@@ -13,7 +13,6 @@ polkadot-primitives = { path = "../../../primitives" }
 polkadot-node-primitives = { path = "../../primitives" }
 polkadot-node-subsystem = { path = "../../subsystem" }
 polkadot-node-subsystem-util = { path = "../../subsystem-util" }
-rand = "0.8.5"
 futures-timer = "3.0.2"
 fatality = "0.0.6"
 

diff --git a/node/core/provisioner/src/disputes/mod.rs b/node/core/provisioner/src/disputes/mod.rs
@@ -14,12 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
-//! The disputes module is responsible for selecting dispute votes to be sent with the inherent data. It contains two
-//! different implementations, extracted in two separate modules - `random_selection` and `prioritized_selection`. Which
-//! implementation will be executed depends on the version of the runtime. Runtime v2 supports `random_selection`. Runtime
-//! `v3` and above - `prioritized_selection`. The entrypoint to these implementations is the `select_disputes` function.
-//! `prioritized_selection` is considered superior and will be the default one in the future. Refer to the documentation of
-//! the modules for more details about each implementation.
+//! The disputes module is responsible for selecting dispute votes to be sent with the inherent data.
 
 use crate::LOG_TARGET;
 use futures::channel::oneshot;
@@ -49,5 +44,3 @@ async fn request_votes(
 }
 
 pub(crate) mod prioritized_selection;
-
-pub(crate) mod random_selection;
diff --git a/node/core/provisioner/src/disputes/random_selection/mod.rs b/node/core/provisioner/src/disputes/random_selection/mod.rs
diff --git a/node/core/provisioner/src/lib.rs b/node/core/provisioner/src/lib.rs
@@ -393,16 +393,17 @@ async fn send_inherent_data(
 		"Selecting disputes"
 	);
 
-	let disputes = match has_required_runtime(
-		from_job,
-		leaf.hash,
-		PRIORITIZED_SELECTION_RUNTIME_VERSION_REQUIREMENT,
-	)
-	.await
-	{
-		true => disputes::prioritized_selection::select_disputes(from_job, metrics, leaf).await,
-		false => disputes::random_selection::select_disputes(from_job, metrics).await,
-	};
+	debug_assert!(
+		has_required_runtime(
+			from_job,
+			leaf.hash,
+			PRIORITIZED_SELECTION_RUNTIME_VERSION_REQUIREMENT,
+		)
+		.await,
+		"randomized selection no longer supported, please upgrade your runtime!"
+	);
+
+	let disputes = disputes::prioritized_selection::select_disputes(from_job, metrics, leaf).await;
 
 	gum::trace!(
 		target: LOG_TARGET,

diff --git a/primitives/src/v5/mod.rs b/primitives/src/v5/mod.rs
@@ -1372,7 +1372,7 @@ impl AsRef<DisputeStatementSet> for DisputeStatementSet {
 pub type MultiDisputeStatementSet = Vec<DisputeStatementSet>;
 
 /// A _checked_ set of dispute statements.
-#[derive(Clone, PartialEq, RuntimeDebug)]
+#[derive(Clone, PartialEq, RuntimeDebug, Encode)]
 pub struct CheckedDisputeStatementSet(DisputeStatementSet);
 
 impl AsRef<DisputeStatementSet> for CheckedDisputeStatementSet {

diff --git a/runtime/kusama/src/weights/runtime_parachains_paras_inherent.rs b/runtime/kusama/src/weights/runtime_parachains_paras_inherent.rs
@@ -17,27 +17,26 @@
 //! Autogenerated weights for `runtime_parachains::paras_inherent`
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2023-06-19, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2023-06-28, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! WORST CASE MAP SIZE: `1000000`
-//! HOSTNAME: `runner-e8ezs4ez-project-163-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
+//! HOSTNAME: `runner-xerhrdyb-project-163-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
 //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("kusama-dev"), DB CACHE: 1024
 
 // Executed Command:
-// ./target/production/polkadot
+// target/production/polkadot
 // benchmark
 // pallet
-// --chain=kusama-dev
 // --steps=50
 // --repeat=20
-// --no-storage-info
-// --no-median-slopes
-// --no-min-squares
-// --pallet=runtime_parachains::paras_inherent
 // --extrinsic=*
 // --execution=wasm
 // --wasm-execution=compiled
+// --heap-pages=4096
+// --json-file=/builds/parity/mirrors/polkadot/.git/.artifacts/bench.json
+// --pallet=runtime_parachains::paras_inherent
+// --chain=kusama-dev
 // --header=./file_header.txt
-// --output=./runtime/kusama/src/weights/runtime_parachains_paras_inherent.rs
+// --output=./runtime/kusama/src/weights/
 
 #![cfg_attr(rustfmt, rustfmt_skip)]
 #![allow(unused_parens)]
@@ -115,11 +114,11 @@ impl<T: frame_system::Config> runtime_parachains::paras_inherent::WeightInfo for
 		// Proof Size summary in bytes:
 		//  Measured:  `50671`
 		//  Estimated: `56611 + v * (23 ±0)`
-		// Minimum execution time: 982_516_000 picoseconds.
-		Weight::from_parts(453_473_787, 0)
+		// Minimum execution time: 1_008_586_000 picoseconds.
+		Weight::from_parts(471_892_709, 0)
 			.saturating_add(Weight::from_parts(0, 56611))
-			// Standard Error: 21_034
-			.saturating_add(Weight::from_parts(57_212_691, 0).saturating_mul(v.into()))
+			// Standard Error: 15_634
+			.saturating_add(Weight::from_parts(56_433_120, 0).saturating_mul(v.into()))
 			.saturating_add(T::DbWeight::get().reads(27))
 			.saturating_add(T::DbWeight::get().writes(15))
 			.saturating_add(Weight::from_parts(0, 23).saturating_mul(v.into()))
@@ -186,8 +185,8 @@ impl<T: frame_system::Config> runtime_parachains::paras_inherent::WeightInfo for
 		// Proof Size summary in bytes:
 		//  Measured:  `42504`
 		//  Estimated: `48444`
-		// Minimum execution time: 465_674_000 picoseconds.
-		Weight::from_parts(483_678_000, 0)
+		// Minimum execution time: 469_409_000 picoseconds.
+		Weight::from_parts(487_865_000, 0)
 			.saturating_add(Weight::from_parts(0, 48444))
 			.saturating_add(T::DbWeight::get().reads(25))
 			.saturating_add(T::DbWeight::get().writes(16))
@@ -259,11 +258,11 @@ impl<T: frame_system::Config> runtime_parachains::paras_inherent::WeightInfo for
 		// Proof Size summary in bytes:
 		//  Measured:  `42540`
 		//  Estimated: `48480`
-		// Minimum execution time: 6_886_272_000 picoseconds.
-		Weight::from_parts(1_235_371_688, 0)
+		// Minimum execution time: 6_874_816_000 picoseconds.
+		Weight::from_parts(1_229_912_739, 0)
 			.saturating_add(Weight::from_parts(0, 48480))
-			// Standard Error: 28_012
-			.saturating_add(Weight::from_parts(56_395_511, 0).saturating_mul(v.into()))
+			// Standard Error: 27_352
+			.saturating_add(Weight::from_parts(56_137_302, 0).saturating_mul(v.into()))
 			.saturating_add(T::DbWeight::get().reads(28))
 			.saturating_add(T::DbWeight::get().writes(15))
 	}
@@ -337,8 +336,8 @@ impl<T: frame_system::Config> runtime_parachains::paras_inherent::WeightInfo for
 		// Proof Size summary in bytes:
 		//  Measured:  `42567`
 		//  Estimated: `48507`
-		// Minimum execution time: 42_215_280_000 picoseconds.
-		Weight::from_parts(43_255_598_000, 0)
+		// Minimum execution time: 41_075_073_000 picoseconds.
+		Weight::from_parts(43_753_587_000, 0)
 			.saturating_add(Weight::from_parts(0, 48507))
 			.saturating_add(T::DbWeight::get().reads(30))
 			.saturating_add(T::DbWeight::get().writes(15))

diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs
@@ -32,6 +32,7 @@ use frame_support::{
 	weights::{Weight, WeightMeter},
 };
 use frame_support_test::TestRandomness;
+use frame_system::limits;
 use parity_scale_codec::Decode;
 use primitives::{
 	AuthorityDiscoveryId, Balance, BlockNumber, CandidateHash, Moment, SessionIndex, UpwardMessage,
@@ -42,7 +43,7 @@ use sp_io::TestExternalities;
 use sp_runtime::{
 	traits::{AccountIdConversion, BlakeTwo256, IdentityLookup},
 	transaction_validity::TransactionPriority,
-	BuildStorage, Permill,
+	BuildStorage, Perbill, Permill,
 };
 use std::{cell::RefCell, collections::HashMap};
 
@@ -81,18 +82,19 @@ where
 
 parameter_types! {
 	pub const BlockHashCount: u32 = 250;
-	pub BlockWeights: frame_system::limits::BlockWeights =
+	pub static BlockWeights: frame_system::limits::BlockWeights =
 		frame_system::limits::BlockWeights::simple_max(
 			Weight::from_parts(4 * 1024 * 1024, u64::MAX),
 		);
+	pub static BlockLength: limits::BlockLength = limits::BlockLength::max_with_normal_ratio(u32::MAX, Perbill::from_percent(75));
 }
 
 pub type AccountId = u64;
 
 impl frame_system::Config for Test {
 	type BaseCallFilter = frame_support::traits::Everything;
 	type BlockWeights = BlockWeights;
-	type BlockLength = ();
+	type BlockLength = BlockLength;
 	type DbWeight = ();
 	type RuntimeOrigin = RuntimeOrigin;
 	type RuntimeCall = RuntimeCall;