diff --git a/.github/workflows/check-workspace.yml b/.github/workflows/check-workspace.yml index 3dd812d7d9b3..81ec311ccce8 100644 --- a/.github/workflows/check-workspace.yml +++ b/.github/workflows/check-workspace.yml @@ -2,8 +2,6 @@ name: Check workspace on: pull_request: - paths: - - "*.toml" merge_group: jobs: @@ -19,5 +17,5 @@ jobs: run: > python3 .github/scripts/check-workspace.py . --exclude - "substrate/frame/contracts/fixtures/build" + "substrate/frame/contracts/fixtures/build" "substrate/frame/contracts/fixtures/contracts/common" diff --git a/Cargo.lock b/Cargo.lock index fc68a7b47d9c..49507ecd4f93 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -22193,8 +22193,10 @@ name = "xcm-simulator-fuzzer" version = "1.0.0" dependencies = [ "arbitrary", + "frame-executive", "frame-support", "frame-system", + "frame-try-runtime", "honggfuzz", "pallet-balances", "pallet-message-queue", diff --git a/polkadot/node/subsystem-bench/src/approval/message_generator.rs b/polkadot/node/subsystem-bench/src/approval/message_generator.rs index 4318dcdf8902..0d308a2fecc0 100644 --- a/polkadot/node/subsystem-bench/src/approval/message_generator.rs +++ b/polkadot/node/subsystem-bench/src/approval/message_generator.rs @@ -62,10 +62,11 @@ use crate::{ GeneratedState, BUFFER_FOR_GENERATION_MILLIS, LOG_TARGET, SLOT_DURATION_MILLIS, }, core::{ - configuration::{TestAuthorities, TestConfiguration, TestObjective}, + configuration::{TestAuthorities, TestConfiguration}, mock::session_info_for_peers, NODE_UNDER_TEST, }, + TestObjective, }; use polkadot_node_network_protocol::v3 as protocol_v3; use polkadot_primitives::Hash; diff --git a/polkadot/node/subsystem-bench/src/approval/mod.rs b/polkadot/node/subsystem-bench/src/approval/mod.rs index 055aeb193456..b1ab53638701 100644 --- a/polkadot/node/subsystem-bench/src/approval/mod.rs +++ b/polkadot/node/subsystem-bench/src/approval/mod.rs @@ -28,7 +28,7 @@ use crate::{ mock_chain_selection::MockChainSelection, }, core::{ - configuration::{TestAuthorities, TestConfiguration}, + configuration::TestAuthorities, environment::{ BenchmarkUsage, TestEnvironment, TestEnvironmentDependencies, MAX_TIME_OF_FLIGHT, }, @@ -43,6 +43,7 @@ use crate::{ }, NODE_UNDER_TEST, }, + TestConfiguration, }; use colored::Colorize; use futures::channel::oneshot; diff --git a/polkadot/node/subsystem-bench/src/availability/mod.rs b/polkadot/node/subsystem-bench/src/availability/mod.rs index 56ec6705b7e3..8ed39525a1e3 100644 --- a/polkadot/node/subsystem-bench/src/availability/mod.rs +++ b/polkadot/node/subsystem-bench/src/availability/mod.rs @@ -67,7 +67,7 @@ use super::core::{configuration::TestConfiguration, mock::dummy_builder, network const LOG_TARGET: &str = "subsystem-bench::availability"; -use super::{cli::TestObjective, core::mock::AlwaysSupportsParachains}; +use super::{core::mock::AlwaysSupportsParachains, TestObjective}; use polkadot_node_subsystem_test_helpers::{ derive_erasure_chunks_with_proofs_and_root, mock::new_block_import_info, }; diff --git a/polkadot/node/subsystem-bench/src/cli.rs b/polkadot/node/subsystem-bench/src/cli.rs deleted file mode 100644 index 21f5e6a85629..000000000000 --- a/polkadot/node/subsystem-bench/src/cli.rs +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright (C) Parity Technologies (UK) Ltd. -// This file is part of Polkadot. - -// Polkadot is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Polkadot is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Polkadot. If not, see . -use super::availability::DataAvailabilityReadOptions; -use crate::approval::ApprovalsOptions; -use serde::{Deserialize, Serialize}; - -#[derive(Debug, Clone, Serialize, Deserialize, clap::Parser)] -#[clap(rename_all = "kebab-case")] -#[allow(missing_docs)] -pub struct TestSequenceOptions { - #[clap(short, long, ignore_case = true)] - pub path: String, -} - -/// Supported test objectives -#[derive(Debug, Clone, clap::Parser, Serialize, Deserialize)] -#[command(rename_all = "kebab-case")] -pub enum TestObjective { - /// Benchmark availability recovery strategies. - DataAvailabilityRead(DataAvailabilityReadOptions), - /// Benchmark availability and bitfield distribution. - DataAvailabilityWrite, - /// Run a test sequence specified in a file - TestSequence(TestSequenceOptions), - /// Benchmark the approval-voting and approval-distribution subsystems. - ApprovalVoting(ApprovalsOptions), - Unimplemented, -} - -impl std::fmt::Display for TestObjective { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}", - match self { - Self::DataAvailabilityRead(_) => "DataAvailabilityRead", - Self::DataAvailabilityWrite => "DataAvailabilityWrite", - Self::TestSequence(_) => "TestSequence", - Self::ApprovalVoting(_) => "ApprovalVoting", - Self::Unimplemented => "Unimplemented", - } - ) - } -} - -#[derive(Debug, clap::Parser)] -#[clap(rename_all = "kebab-case")] -#[allow(missing_docs)] -pub struct StandardTestOptions { - #[clap(long, ignore_case = true, default_value_t = 100)] - /// Number of cores to fetch availability for. - pub n_cores: usize, - - #[clap(long, ignore_case = true, default_value_t = 500)] - /// Number of validators to fetch chunks from. - pub n_validators: usize, - - #[clap(long, ignore_case = true, default_value_t = 5120)] - /// The minimum pov size in KiB - pub min_pov_size: usize, - - #[clap(long, ignore_case = true, default_value_t = 5120)] - /// The maximum pov size bytes - pub max_pov_size: usize, - - #[clap(short, long, ignore_case = true, default_value_t = 1)] - /// The number of blocks the test is going to run. - pub num_blocks: usize, -} diff --git a/polkadot/node/subsystem-bench/src/core/configuration.rs b/polkadot/node/subsystem-bench/src/core/configuration.rs index 0c8a78c504c8..d9eec43873aa 100644 --- a/polkadot/node/subsystem-bench/src/core/configuration.rs +++ b/polkadot/node/subsystem-bench/src/core/configuration.rs @@ -22,7 +22,7 @@ use sc_network::PeerId; use sp_consensus_babe::AuthorityId; use std::{collections::HashMap, path::Path}; -pub use crate::cli::TestObjective; +use crate::TestObjective; use polkadot_primitives::{AssignmentId, AuthorityDiscoveryId, ValidatorId}; use rand::thread_rng; use rand_distr::{Distribution, Normal, Uniform}; @@ -240,95 +240,6 @@ impl TestConfiguration { peer_id_to_authority, } } - - /// An unconstrained standard configuration matching Polkadot/Kusama - pub fn ideal_network( - objective: TestObjective, - num_blocks: usize, - n_validators: usize, - n_cores: usize, - min_pov_size: usize, - max_pov_size: usize, - ) -> TestConfiguration { - Self { - objective, - n_cores, - n_validators, - max_validators_per_core: 5, - pov_sizes: generate_pov_sizes(n_cores, min_pov_size, max_pov_size), - bandwidth: 50 * 1024 * 1024, - peer_bandwidth: 50 * 1024 * 1024, - // No latency - latency: None, - num_blocks, - min_pov_size, - max_pov_size, - connectivity: 100, - needed_approvals: default_needed_approvals(), - n_delay_tranches: default_n_delay_tranches(), - no_show_slots: default_no_show_slots(), - relay_vrf_modulo_samples: default_relay_vrf_modulo_samples(), - zeroth_delay_tranche_width: default_zeroth_delay_tranche_width(), - } - } - - pub fn healthy_network( - objective: TestObjective, - num_blocks: usize, - n_validators: usize, - n_cores: usize, - min_pov_size: usize, - max_pov_size: usize, - ) -> TestConfiguration { - Self { - objective, - n_cores, - n_validators, - max_validators_per_core: 5, - pov_sizes: generate_pov_sizes(n_cores, min_pov_size, max_pov_size), - bandwidth: 50 * 1024 * 1024, - peer_bandwidth: 50 * 1024 * 1024, - latency: Some(PeerLatency { mean_latency_ms: 50, std_dev: 12.5 }), - num_blocks, - min_pov_size, - max_pov_size, - connectivity: 95, - needed_approvals: default_needed_approvals(), - n_delay_tranches: default_n_delay_tranches(), - no_show_slots: default_no_show_slots(), - relay_vrf_modulo_samples: default_relay_vrf_modulo_samples(), - zeroth_delay_tranche_width: default_zeroth_delay_tranche_width(), - } - } - - pub fn degraded_network( - objective: TestObjective, - num_blocks: usize, - n_validators: usize, - n_cores: usize, - min_pov_size: usize, - max_pov_size: usize, - ) -> TestConfiguration { - Self { - objective, - n_cores, - n_validators, - max_validators_per_core: 5, - pov_sizes: generate_pov_sizes(n_cores, min_pov_size, max_pov_size), - bandwidth: 50 * 1024 * 1024, - peer_bandwidth: 50 * 1024 * 1024, - latency: Some(PeerLatency { mean_latency_ms: 150, std_dev: 40.0 }), - num_blocks, - min_pov_size, - max_pov_size, - connectivity: 67, - needed_approvals: default_needed_approvals(), - n_delay_tranches: default_n_delay_tranches(), - no_show_slots: default_no_show_slots(), - relay_vrf_modulo_samples: default_relay_vrf_modulo_samples(), - zeroth_delay_tranche_width: default_zeroth_delay_tranche_width(), - } - } } /// Sample latency (in milliseconds) from a normal distribution with parameters diff --git a/polkadot/node/subsystem-bench/src/subsystem-bench.rs b/polkadot/node/subsystem-bench/src/subsystem-bench.rs index 433354f6525d..a5dfbc52d606 100644 --- a/polkadot/node/subsystem-bench/src/subsystem-bench.rs +++ b/polkadot/node/subsystem-bench/src/subsystem-bench.rs @@ -17,6 +17,7 @@ //! A tool for running subsystem benchmark tests designed for development and //! CI regression testing. use clap::Parser; +use serde::{Deserialize, Serialize}; use colored::Colorize; @@ -28,24 +29,23 @@ use std::path::Path; pub(crate) mod approval; pub(crate) mod availability; -pub(crate) mod cli; pub(crate) mod core; mod valgrind; const LOG_TARGET: &str = "subsystem-bench"; use availability::{prepare_test, NetworkEmulation, TestState}; -use cli::TestObjective; +use approval::{bench_approvals, ApprovalsOptions}; +use availability::DataAvailabilityReadOptions; use core::{ configuration::TestConfiguration, + display::display_configuration, environment::{TestEnvironment, GENESIS_HASH}, }; use clap_num::number_range; -use crate::{approval::bench_approvals, core::display::display_configuration}; - fn le_100(s: &str) -> Result { number_range(s, 0, 100) } @@ -54,6 +54,34 @@ fn le_5000(s: &str) -> Result { number_range(s, 0, 5000) } +/// Supported test objectives +#[derive(Debug, Clone, Parser, Serialize, Deserialize)] +#[command(rename_all = "kebab-case")] +pub enum TestObjective { + /// Benchmark availability recovery strategies. + DataAvailabilityRead(DataAvailabilityReadOptions), + /// Benchmark availability and bitfield distribution. + DataAvailabilityWrite, + /// Benchmark the approval-voting and approval-distribution subsystems. + ApprovalVoting(ApprovalsOptions), + Unimplemented, +} + +impl std::fmt::Display for TestObjective { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + Self::DataAvailabilityRead(_) => "DataAvailabilityRead", + Self::DataAvailabilityWrite => "DataAvailabilityWrite", + Self::ApprovalVoting(_) => "ApprovalVoting", + Self::Unimplemented => "Unimplemented", + } + ) + } +} + #[derive(Debug, Parser)] #[allow(missing_docs)] struct BenchCli { @@ -61,9 +89,6 @@ struct BenchCli { /// The type of network to be emulated pub network: NetworkEmulation, - #[clap(flatten)] - pub standard_configuration: cli::StandardTestOptions, - #[clap(short, long)] /// The bandwidth of emulated remote peers in KiB pub peer_bandwidth: Option, @@ -104,42 +129,12 @@ struct BenchCli { /// Shows the output in YAML format pub yaml_output: bool, - #[command(subcommand)] - pub objective: cli::TestObjective, + #[arg(required = true)] + /// Path to the test sequence configuration file + pub path: String, } impl BenchCli { - fn create_test_configuration(&self) -> TestConfiguration { - let configuration = &self.standard_configuration; - - match self.network { - NetworkEmulation::Healthy => TestConfiguration::healthy_network( - self.objective.clone(), - configuration.num_blocks, - configuration.n_validators, - configuration.n_cores, - configuration.min_pov_size, - configuration.max_pov_size, - ), - NetworkEmulation::Degraded => TestConfiguration::degraded_network( - self.objective.clone(), - configuration.num_blocks, - configuration.n_validators, - configuration.n_cores, - configuration.min_pov_size, - configuration.max_pov_size, - ), - NetworkEmulation::Ideal => TestConfiguration::ideal_network( - self.objective.clone(), - configuration.num_blocks, - configuration.n_validators, - configuration.n_cores, - configuration.min_pov_size, - configuration.max_pov_size, - ), - } - } - fn launch(self) -> eyre::Result<()> { let is_valgrind_running = valgrind::is_valgrind_running(); if !is_valgrind_running && self.cache_misses { @@ -156,125 +151,56 @@ impl BenchCli { None }; - let mut test_config = match self.objective { - TestObjective::TestSequence(options) => { - let test_sequence = - core::configuration::TestSequence::new_from_file(Path::new(&options.path)) - .expect("File exists") - .into_vec(); - let num_steps = test_sequence.len(); - gum::info!( - "{}", - format!("Sequence contains {} step(s)", num_steps).bright_purple() - ); - for (index, test_config) in test_sequence.into_iter().enumerate() { - let benchmark_name = - format!("{} #{} {}", &options.path, index + 1, test_config.objective); - gum::info!(target: LOG_TARGET, "{}", format!("Step {}/{}", index + 1, num_steps).bright_purple(),); - display_configuration(&test_config); - - let usage = match test_config.objective { - TestObjective::DataAvailabilityRead(ref _opts) => { - let mut state = TestState::new(&test_config); - let (mut env, _protocol_config) = prepare_test(test_config, &mut state); - env.runtime().block_on(availability::benchmark_availability_read( - &benchmark_name, - &mut env, - state, - )) - }, - TestObjective::ApprovalVoting(ref options) => { - let (mut env, state) = - approval::prepare_test(test_config.clone(), options.clone()); - env.runtime().block_on(bench_approvals( - &benchmark_name, - &mut env, - state, - )) - }, - TestObjective::DataAvailabilityWrite => { - let mut state = TestState::new(&test_config); - let (mut env, _protocol_config) = prepare_test(test_config, &mut state); - env.runtime().block_on(availability::benchmark_availability_write( - &benchmark_name, - &mut env, - state, - )) - }, - TestObjective::TestSequence(_) => todo!(), - TestObjective::Unimplemented => todo!(), - }; - - let output = if self.yaml_output { - serde_yaml::to_string(&vec![usage])? - } else { - usage.to_string() - }; - println!("{}", output); - } - - return Ok(()) - }, - TestObjective::DataAvailabilityRead(ref _options) => self.create_test_configuration(), - TestObjective::DataAvailabilityWrite => self.create_test_configuration(), - TestObjective::ApprovalVoting(_) => todo!(), - TestObjective::Unimplemented => todo!(), - }; - - let mut latency_config = test_config.latency.clone().unwrap_or_default(); - - if let Some(latency) = self.peer_mean_latency { - latency_config.mean_latency_ms = latency; + let test_sequence = core::configuration::TestSequence::new_from_file(Path::new(&self.path)) + .expect("File exists") + .into_vec(); + let num_steps = test_sequence.len(); + gum::info!("{}", format!("Sequence contains {} step(s)", num_steps).bright_purple()); + for (index, test_config) in test_sequence.into_iter().enumerate() { + let benchmark_name = format!("{} #{} {}", &self.path, index + 1, test_config.objective); + gum::info!(target: LOG_TARGET, "{}", format!("Step {}/{}", index + 1, num_steps).bright_purple(),); + display_configuration(&test_config); + + let usage = match test_config.objective { + TestObjective::DataAvailabilityRead(ref _opts) => { + let mut state = TestState::new(&test_config); + let (mut env, _protocol_config) = prepare_test(test_config, &mut state); + env.runtime().block_on(availability::benchmark_availability_read( + &benchmark_name, + &mut env, + state, + )) + }, + TestObjective::ApprovalVoting(ref options) => { + let (mut env, state) = + approval::prepare_test(test_config.clone(), options.clone()); + env.runtime().block_on(bench_approvals(&benchmark_name, &mut env, state)) + }, + TestObjective::DataAvailabilityWrite => { + let mut state = TestState::new(&test_config); + let (mut env, _protocol_config) = prepare_test(test_config, &mut state); + env.runtime().block_on(availability::benchmark_availability_write( + &benchmark_name, + &mut env, + state, + )) + }, + TestObjective::Unimplemented => todo!(), + }; + + let output = if self.yaml_output { + serde_yaml::to_string(&vec![usage])? + } else { + usage.to_string() + }; + println!("{}", output); } - if let Some(std_dev) = self.peer_latency_std_dev { - latency_config.std_dev = std_dev; - } - - // Write back the updated latency. - test_config.latency = Some(latency_config); - - if let Some(connectivity) = self.connectivity { - test_config.connectivity = connectivity; - } - - if let Some(bandwidth) = self.peer_bandwidth { - // CLI expects bw in KiB - test_config.peer_bandwidth = bandwidth * 1024; - } - - if let Some(bandwidth) = self.bandwidth { - // CLI expects bw in KiB - test_config.bandwidth = bandwidth * 1024; - } - - display_configuration(&test_config); - - let mut state = TestState::new(&test_config); - let (mut env, _protocol_config) = prepare_test(test_config, &mut state); - - let benchmark_name = format!("{}", self.objective); - let usage = match self.objective { - TestObjective::DataAvailabilityRead(_options) => env.runtime().block_on( - availability::benchmark_availability_read(&benchmark_name, &mut env, state), - ), - TestObjective::DataAvailabilityWrite => env.runtime().block_on( - availability::benchmark_availability_write(&benchmark_name, &mut env, state), - ), - TestObjective::TestSequence(_options) => todo!(), - TestObjective::ApprovalVoting(_) => todo!(), - TestObjective::Unimplemented => todo!(), - }; - if let Some(agent_running) = agent_running { let agent_ready = agent_running.stop()?; agent_ready.shutdown(); } - let output = - if self.yaml_output { serde_yaml::to_string(&vec![usage])? } else { usage.to_string() }; - println!("{}", output); - Ok(()) } } diff --git a/polkadot/xcm/xcm-simulator/fuzzer/Cargo.toml b/polkadot/xcm/xcm-simulator/fuzzer/Cargo.toml index 13b6e7b8652f..30644dc0e0a5 100644 --- a/polkadot/xcm/xcm-simulator/fuzzer/Cargo.toml +++ b/polkadot/xcm/xcm-simulator/fuzzer/Cargo.toml @@ -18,6 +18,8 @@ scale-info = { version = "2.10.0", features = ["derive"] } frame-system = { path = "../../../../substrate/frame/system" } frame-support = { path = "../../../../substrate/frame/support" } +frame-executive = { path = "../../../../substrate/frame/executive" } +frame-try-runtime = { path = "../../../../substrate/frame/try-runtime" } pallet-balances = { path = "../../../../substrate/frame/balances" } pallet-message-queue = { path = "../../../../substrate/frame/message-queue" } sp-std = { path = "../../../../substrate/primitives/std" } @@ -35,6 +37,17 @@ polkadot-runtime-parachains = { path = "../../../runtime/parachains" } polkadot-parachain-primitives = { path = "../../../parachain" } [features] +try-runtime = [ + "frame-executive/try-runtime", + "frame-support/try-runtime", + "frame-system/try-runtime", + "frame-try-runtime/try-runtime", + "pallet-balances/try-runtime", + "pallet-message-queue/try-runtime", + "pallet-xcm/try-runtime", + "polkadot-runtime-parachains/try-runtime", + "sp-runtime/try-runtime", +] runtime-benchmarks = [ "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", diff --git a/polkadot/xcm/xcm-simulator/fuzzer/README.md b/polkadot/xcm/xcm-simulator/fuzzer/README.md index 0b3fdd8ec776..9c15ee881c1b 100644 --- a/polkadot/xcm/xcm-simulator/fuzzer/README.md +++ b/polkadot/xcm/xcm-simulator/fuzzer/README.md @@ -14,7 +14,7 @@ cargo install honggfuzz In this directory, run this command: ``` -cargo hfuzz run xcm-fuzzer +HFUZZ_BUILD_ARGS="--features=try-runtime" cargo hfuzz run xcm-fuzzer ``` ## Run a single input @@ -22,7 +22,7 @@ cargo hfuzz run xcm-fuzzer In this directory, run this command: ``` -cargo hfuzz run-debug xcm-fuzzer hfuzz_workspace/xcm-fuzzer/fuzzer_input_file +cargo run --features=try-runtime -- hfuzz_workspace/xcm-fuzzer/fuzzer_input_file ``` ## Generate coverage @@ -31,7 +31,7 @@ In this directory, run these four commands: ``` RUSTFLAGS="-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort" \ -CARGO_INCREMENTAL=0 SKIP_WASM_BUILD=1 CARGO_HOME=./cargo cargo build +CARGO_INCREMENTAL=0 SKIP_WASM_BUILD=1 CARGO_HOME=./cargo cargo build --features=try-runtime ../../../target/debug/xcm-fuzzer hfuzz_workspace/xcm-fuzzer/input/ zip -0 ccov.zip `find ../../../target/ \( -name "*.gc*" -o -name "test-*.gc*" \) -print` grcov ccov.zip -s ../../../ -t html --llvm --branch --ignore-not-existing -o ./coverage diff --git a/polkadot/xcm/xcm-simulator/fuzzer/src/fuzz.rs b/polkadot/xcm/xcm-simulator/fuzzer/src/fuzz.rs index 7026d5467c8b..adf6cacd278b 100644 --- a/polkadot/xcm/xcm-simulator/fuzzer/src/fuzz.rs +++ b/polkadot/xcm/xcm-simulator/fuzzer/src/fuzz.rs @@ -23,7 +23,9 @@ use polkadot_parachain_primitives::primitives::Id as ParaId; use sp_runtime::{traits::AccountIdConversion, BuildStorage}; use xcm_simulator::{decl_test_network, decl_test_parachain, decl_test_relay_chain, TestExt}; -use frame_support::assert_ok; +#[cfg(feature = "try-runtime")] +use frame_support::traits::{TryState, TryStateSelect::All}; +use frame_support::{assert_ok, traits::IntegrityTest}; use xcm::{latest::prelude::*, MAX_XCM_DECODE_DEPTH}; use arbitrary::{Arbitrary, Error, Unstructured}; @@ -98,7 +100,7 @@ impl<'a> Arbitrary<'a> for XcmMessage { if let Ok(message) = DecodeLimit::decode_with_depth_limit(MAX_XCM_DECODE_DEPTH, &mut encoded_message) { - return Ok(XcmMessage { source, destination, message }) + return Ok(XcmMessage { source, destination, message }); } Err(Error::IncorrectFormat) } @@ -148,6 +150,21 @@ pub fn relay_ext() -> sp_io::TestExternalities { pub type RelayChainPalletXcm = pallet_xcm::Pallet; pub type ParachainPalletXcm = pallet_xcm::Pallet; +// We check XCM messages recursively for blocklisted messages +fn recursively_matches_blocklisted_messages(message: &Instruction<()>) -> bool { + match message { + DepositReserveAsset { xcm, .. } | + ExportMessage { xcm, .. } | + InitiateReserveWithdraw { xcm, .. } | + InitiateTeleport { xcm, .. } | + TransferReserveAsset { xcm, .. } | + SetErrorHandler(xcm) | + SetAppendix(xcm) => xcm.iter().any(recursively_matches_blocklisted_messages), + // The blocklisted message is the Transact instruction. + m => matches!(m, Transact { .. }), + } +} + fn run_input(xcm_messages: [XcmMessage; 5]) { MockNet::reset(); @@ -155,6 +172,11 @@ fn run_input(xcm_messages: [XcmMessage; 5]) { println!(); for xcm_message in xcm_messages { + if xcm_message.message.iter().any(recursively_matches_blocklisted_messages) { + println!(" skipping message\n"); + continue; + } + if xcm_message.source % 4 == 0 { // We get the destination for the message let parachain_id = (xcm_message.destination % 3) + 1; @@ -197,8 +219,22 @@ fn run_input(xcm_messages: [XcmMessage; 5]) { } #[cfg(not(fuzzing))] println!(); + // We run integrity tests and try_runtime invariants + [ParaA::execute_with, ParaB::execute_with, ParaC::execute_with].iter().for_each( + |execute_with| { + execute_with(|| { + #[cfg(feature = "try-runtime")] + parachain::AllPalletsWithSystem::try_state(Default::default(), All).unwrap(); + parachain::AllPalletsWithSystem::integrity_test(); + }); + }, + ); + Relay::execute_with(|| { + #[cfg(feature = "try-runtime")] + relay_chain::AllPalletsWithSystem::try_state(Default::default(), All).unwrap(); + relay_chain::AllPalletsWithSystem::integrity_test(); + }); } - Relay::execute_with(|| {}); } fn main() { diff --git a/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs b/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs index d8327c9b401d..fbb60a25f44a 100644 --- a/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs +++ b/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs @@ -24,10 +24,11 @@ use frame_support::{ }; use frame_system::EnsureRoot; -use sp_core::{ConstU32, H256}; +use sp_core::ConstU32; use sp_runtime::{ - traits::{Hash, IdentityLookup}, - AccountId32, + generic, + traits::{AccountIdLookup, BlakeTwo256, Hash, IdentifyAccount, Verify}, + MultiAddress, MultiSignature, }; use sp_std::prelude::*; @@ -47,38 +48,29 @@ use xcm_builder::{ }; use xcm_executor::{Config, XcmExecutor}; -pub type AccountId = AccountId32; +pub type SignedExtra = (frame_system::CheckNonZeroSender,); + +pub type BlockNumber = u64; +pub type Address = MultiAddress; +pub type Header = generic::Header; +pub type UncheckedExtrinsic = + generic::UncheckedExtrinsic; +pub type Block = generic::Block; + +pub type Signature = MultiSignature; +pub type AccountId = <::Signer as IdentifyAccount>::AccountId; pub type Balance = u128; parameter_types! { - pub const BlockHashCount: u64 = 250; + pub const BlockHashCount: u32 = 250; } #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Runtime { - type RuntimeOrigin = RuntimeOrigin; - type RuntimeCall = RuntimeCall; - type Nonce = u64; - type Hash = H256; - type Hashing = ::sp_runtime::traits::BlakeTwo256; type AccountId = AccountId; - type Lookup = IdentityLookup; + type Lookup = AccountIdLookup; type Block = Block; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = BlockHashCount; - type BlockWeights = (); - type BlockLength = (); - type Version = (); - type PalletInfo = PalletInfo; type AccountData = pallet_balances::AccountData; - type OnNewAccount = (); - type OnKilledAccount = (); - type DbWeight = (); - type BaseCallFilter = Everything; - type SystemWeightInfo = (); - type SS58Prefix = (); - type OnSetCode = (); - type MaxConsumers = frame_support::traits::ConstU32<16>; } parameter_types! { @@ -356,8 +348,6 @@ impl pallet_xcm::Config for Runtime { type AdminOrigin = EnsureRoot; } -type Block = frame_system::mocking::MockBlock; - construct_runtime!( pub enum Runtime { diff --git a/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs b/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs index 7e42f558dd6e..e8294560dfcc 100644 --- a/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs +++ b/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs @@ -23,8 +23,12 @@ use frame_support::{ }; use frame_system::EnsureRoot; -use sp_core::{ConstU32, H256}; -use sp_runtime::{traits::IdentityLookup, AccountId32}; +use sp_core::ConstU32; +use sp_runtime::{ + generic, + traits::{BlakeTwo256, IdentifyAccount, Verify}, + MultiAddress, MultiSignature, +}; use polkadot_parachain_primitives::primitives::Id as ParaId; use polkadot_runtime_parachains::{ @@ -43,38 +47,29 @@ use xcm_builder::{ }; use xcm_executor::{Config, XcmExecutor}; -pub type AccountId = AccountId32; +pub type SignedExtra = (frame_system::CheckNonZeroSender,); + +pub type BlockNumber = u64; +pub type Address = MultiAddress; +pub type Header = generic::Header; +pub type UncheckedExtrinsic = + generic::UncheckedExtrinsic; +pub type Block = generic::Block; + +pub type Signature = MultiSignature; +pub type AccountId = <::Signer as IdentifyAccount>::AccountId; pub type Balance = u128; parameter_types! { - pub const BlockHashCount: u64 = 250; + pub const BlockHashCount: u32 = 250; } #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Runtime { - type RuntimeOrigin = RuntimeOrigin; - type RuntimeCall = RuntimeCall; - type Nonce = u64; - type Hash = H256; - type Hashing = ::sp_runtime::traits::BlakeTwo256; type AccountId = AccountId; - type Lookup = IdentityLookup; + type Lookup = sp_runtime::traits::AccountIdLookup; type Block = Block; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = BlockHashCount; - type BlockWeights = (); - type BlockLength = (); - type Version = (); - type PalletInfo = PalletInfo; type AccountData = pallet_balances::AccountData; - type OnNewAccount = (); - type OnKilledAccount = (); - type DbWeight = (); - type BaseCallFilter = Everything; - type SystemWeightInfo = (); - type SS58Prefix = (); - type OnSetCode = (); - type MaxConsumers = ConstU32<16>; } parameter_types! { @@ -202,8 +197,6 @@ parameter_types! { impl origin::Config for Runtime {} -type Block = frame_system::mocking::MockBlock; - parameter_types! { /// Amount of weight that can be spent per block to service messages. pub MessageQueueServiceWeight: Weight = Weight::from_parts(1_000_000_000, 1_000_000); diff --git a/prdoc/pr_3007.prdoc b/prdoc/pr_3007.prdoc new file mode 100644 index 000000000000..17870469a219 --- /dev/null +++ b/prdoc/pr_3007.prdoc @@ -0,0 +1,16 @@ +title: Try State Hook for Ranked Collective. + +doc: + - audience: Runtime User + description: | + Invariants for storage items in the ranked collective pallet. Enforces the following Invariants: + 1. Total number of `Members` in storage should be >= [`MemberIndex`] of a [`Rank`] in `MemberCount`. + 2. `Rank` in Members should be in `MemberCount`. + 3.`Sum` of `MemberCount` index should be the same as the sum of all the index attained for + rank possessed by `Members` + 4. `Member` in storage of `IdToIndex` should be the same as `Member` in `IndexToId`. + 5. `Rank` in `IdToIndex` should be the same as the the `Rank` in `IndexToId`. + 6. `Rank` of the member `who` in `IdToIndex` should be the same as the `Rank` of + the member `who` in `Members` +crates: +- name: pallet-ranked-collective diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs index e5efb70ae8d5..3e1e36f7d448 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -863,6 +863,29 @@ fn cannot_set_expired_lease() { }); } +#[test] +fn short_leases_are_cleaned() { + TestExt::new().region_length(3).execute_with(|| { + assert_ok!(Broker::do_start_sales(200, 1)); + advance_to(2); + + // New leases are allowed to expire within this region given expiry > `current_timeslice`. + assert_noop!( + Broker::do_set_lease(1000, Broker::current_timeslice()), + Error::::AlreadyExpired + ); + assert_eq!(Leases::::get().len(), 0); + assert_ok!(Broker::do_set_lease(1000, Broker::current_timeslice().saturating_add(1))); + assert_eq!(Leases::::get().len(), 1); + + // But are cleaned up in the next rotate_sale. + let config = Configuration::::get().unwrap(); + let timeslice_period: u64 = ::TimeslicePeriod::get(); + advance_to(timeslice_period.saturating_mul(config.region_length.into())); + assert_eq!(Leases::::get().len(), 0); + }); +} + #[test] fn leases_are_limited() { TestExt::new().execute_with(|| { diff --git a/substrate/frame/broker/src/tick_impls.rs b/substrate/frame/broker/src/tick_impls.rs index 8b7860c8e3af..388370bce4d4 100644 --- a/substrate/frame/broker/src/tick_impls.rs +++ b/substrate/frame/broker/src/tick_impls.rs @@ -216,7 +216,9 @@ impl Pallet { let assignment = CoreAssignment::Task(task); let schedule = BoundedVec::truncate_from(vec![ScheduleItem { mask, assignment }]); Workplan::::insert((region_begin, first_core), &schedule); - let expiring = until >= region_begin && until < region_end; + // Separate these to avoid missed expired leases hanging around forever. + let expired = until < region_end; + let expiring = until >= region_begin && expired; if expiring { // last time for this one - make it renewable. let renewal_id = AllowedRenewalId { core: first_core, when: region_end }; @@ -231,7 +233,7 @@ impl Pallet { Self::deposit_event(Event::LeaseEnding { when: region_end, task }); } first_core.saturating_inc(); - !expiring + !expired }); Leases::::put(&leases); diff --git a/substrate/frame/ranked-collective/src/benchmarking.rs b/substrate/frame/ranked-collective/src/benchmarking.rs index f093fd93e6e2..462f55a238d2 100644 --- a/substrate/frame/ranked-collective/src/benchmarking.rs +++ b/substrate/frame/ranked-collective/src/benchmarking.rs @@ -181,5 +181,5 @@ benchmarks_instance_pallet! { assert_has_event::(Event::MemberExchanged { who, new_who }.into()); } - impl_benchmark_test_suite!(RankedCollective, crate::tests::new_test_ext(), crate::tests::Test); + impl_benchmark_test_suite!(RankedCollective, crate::tests::ExtBuilder::default().build(), crate::tests::Test); } diff --git a/substrate/frame/ranked-collective/src/lib.rs b/substrate/frame/ranked-collective/src/lib.rs index 65ea886acc4f..2b4fb2dbff47 100644 --- a/substrate/frame/ranked-collective/src/lib.rs +++ b/substrate/frame/ranked-collective/src/lib.rs @@ -716,6 +716,14 @@ pub mod pallet { } } + #[pallet::hooks] + impl, I: 'static> Hooks> for Pallet { + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + Self::do_try_state() + } + } + impl, I: 'static> Pallet { fn ensure_member(who: &T::AccountId) -> Result { Members::::get(who).ok_or(Error::::NotMember.into()) @@ -847,6 +855,132 @@ pub mod pallet { } } + #[cfg(any(feature = "try-runtime", test))] + impl, I: 'static> Pallet { + /// Ensure the correctness of the state of this pallet. + pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { + Self::try_state_members()?; + Self::try_state_index()?; + + Ok(()) + } + + /// ### Invariants of Member storage items + /// + /// Total number of [`Members`] in storage should be >= [`MemberIndex`] of a [`Rank`] in + /// [`MemberCount`]. + /// [`Rank`] in Members should be in [`MemberCount`] + /// [`Sum`] of [`MemberCount`] index should be the same as the sum of all the index attained + /// for rank possessed by [`Members`] + fn try_state_members() -> Result<(), sp_runtime::TryRuntimeError> { + MemberCount::::iter().try_for_each(|(_, member_index)| -> DispatchResult { + let total_members = Members::::iter().count(); + ensure!( + total_members as u32 >= member_index, + "Total count of `Members` should be greater than or equal to the number of `MemberIndex` of a particular `Rank` in `MemberCount`." + ); + + Ok(()) + })?; + + let mut sum_of_member_rank_indexes = 0; + Members::::iter().try_for_each(|(_, member_record)| -> DispatchResult { + ensure!( + Self::is_rank_in_member_count(member_record.rank.into()), + "`Rank` in Members should be in `MemberCount`" + ); + + sum_of_member_rank_indexes += Self::determine_index_of_a_rank(member_record.rank); + + Ok(()) + })?; + + let sum_of_all_member_count_indexes = + MemberCount::::iter_values().fold(0, |sum, index| sum + index); + ensure!( + sum_of_all_member_count_indexes == sum_of_member_rank_indexes as u32, + "Sum of `MemberCount` index should be the same as the sum of all the index attained for rank possessed by `Members`" + ); + Ok(()) + } + + /// ### Invariants of Index storage items + /// [`Member`] in storage of [`IdToIndex`] should be the same as [`Member`] in [`IndexToId`] + /// [`Rank`] in [`IdToIndex`] should be the same as the the [`Rank`] in [`IndexToId`] + /// [`Rank`] of the member [`who`] in [`IdToIndex`] should be the same as the [`Rank`] of + /// the member [`who`] in [`Members`] + fn try_state_index() -> Result<(), sp_runtime::TryRuntimeError> { + IdToIndex::::iter().try_for_each( + |(rank, who, member_index)| -> DispatchResult { + let who_from_index = IndexToId::::get(rank, member_index).unwrap(); + ensure!( + who == who_from_index, + "`Member` in storage of `IdToIndex` should be the same as `Member` in `IndexToId`." + ); + + ensure!( + Self::is_rank_in_index_to_id_storage(rank.into()), + "`Rank` in `IdToIndex` should be the same as the `Rank` in `IndexToId`" + ); + Ok(()) + }, + )?; + + Members::::iter().try_for_each(|(who, member_record)| -> DispatchResult { + ensure!( + Self::is_who_rank_in_id_to_index_storage(who, member_record.rank), + "`Rank` of the member `who` in `IdToIndex` should be the same as the `Rank` of the member `who` in `Members`" + ); + + Ok(()) + })?; + + Ok(()) + } + + /// Checks if a rank is part of the `MemberCount` + fn is_rank_in_member_count(rank: u32) -> bool { + for (r, _) in MemberCount::::iter() { + if r as u32 == rank { + return true; + } + } + + return false; + } + + /// Checks if a rank is the same as the rank `IndexToId` + fn is_rank_in_index_to_id_storage(rank: u32) -> bool { + for (r, _, _) in IndexToId::::iter() { + if r as u32 == rank { + return true; + } + } + + return false; + } + + /// Checks if a member(who) rank is the same as the rank of a member(who) in `IdToIndex` + fn is_who_rank_in_id_to_index_storage(who: T::AccountId, rank: u16) -> bool { + for (rank_, who_, _) in IdToIndex::::iter() { + if who == who_ && rank == rank_ { + return true; + } + } + + return false; + } + + /// Determines the total index for a rank + fn determine_index_of_a_rank(rank: u16) -> u16 { + let mut sum = 0; + for _ in 0..rank + 1 { + sum += 1; + } + sum + } + } + impl, I: 'static> RankedMembers for Pallet { type AccountId = T::AccountId; type Rank = Rank; diff --git a/substrate/frame/ranked-collective/src/tests.rs b/substrate/frame/ranked-collective/src/tests.rs index c5fccd3f7724..31add52d90af 100644 --- a/substrate/frame/ranked-collective/src/tests.rs +++ b/substrate/frame/ranked-collective/src/tests.rs @@ -183,11 +183,28 @@ impl Config for Test { type BenchmarkSetup = (); } -pub fn new_test_ext() -> sp_io::TestExternalities { - let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - let mut ext = sp_io::TestExternalities::new(t); - ext.execute_with(|| System::set_block_number(1)); - ext +pub struct ExtBuilder {} + +impl Default for ExtBuilder { + fn default() -> Self { + Self {} + } +} + +impl ExtBuilder { + pub fn build(self) -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(1)); + ext + } + + pub fn build_and_execute(self, test: impl FnOnce() -> ()) { + self.build().execute_with(|| { + test(); + Club::do_try_state().expect("All invariants must hold after a test"); + }) + } } fn next_block() { @@ -225,14 +242,14 @@ fn completed_poll_should_panic() { #[test] fn basic_stuff() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); }); } #[test] fn member_lifecycle_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); @@ -244,7 +261,7 @@ fn member_lifecycle_works() { #[test] fn add_remove_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_eq!(member_count(0), 1); @@ -274,7 +291,7 @@ fn add_remove_works() { #[test] fn promote_demote_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_eq!(member_count(0), 1); @@ -305,7 +322,7 @@ fn promote_demote_works() { #[test] fn promote_demote_by_rank_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); for _ in 0..7 { assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); @@ -372,7 +389,7 @@ fn promote_demote_by_rank_works() { #[test] fn voting_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 0)); assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); @@ -406,7 +423,7 @@ fn voting_works() { #[test] fn cleanup_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); @@ -433,7 +450,7 @@ fn cleanup_works() { #[test] fn remove_member_cleanup_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); @@ -459,7 +476,7 @@ fn remove_member_cleanup_works() { #[test] fn ensure_ranked_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); @@ -528,7 +545,7 @@ fn ensure_ranked_works() { #[test] fn do_add_member_to_rank_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { let max_rank = 9u16; assert_ok!(Club::do_add_member_to_rank(69, max_rank / 2, true)); assert_ok!(Club::do_add_member_to_rank(1337, max_rank, true)); @@ -545,7 +562,7 @@ fn do_add_member_to_rank_works() { #[test] fn tally_support_correct() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { // add members, // rank 1: accounts 1, 2, 3 // rank 2: accounts 2, 3 @@ -585,7 +602,7 @@ fn tally_support_correct() { #[test] fn exchange_member_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_eq!(member_count(0), 1); @@ -613,7 +630,7 @@ fn exchange_member_works() { #[test] fn exchange_member_same_noops() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build_and_execute(|| { assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); assert_ok!(Club::add_member(RuntimeOrigin::root(), 2));