diff --git a/Cargo.lock b/Cargo.lock index 666fe6e451831..3434e01d13cd2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5180,6 +5180,16 @@ dependencies = [ "sp-tracing", ] +[[package]] +name = "pallet-bags-list-fuzzer" +version = "4.0.0-dev" +dependencies = [ + "frame-election-provider-support", + "honggfuzz", + "pallet-bags-list", + "rand 0.8.4", +] + [[package]] name = "pallet-balances" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 4a228203159eb..743e0f7066647 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,6 +135,7 @@ members = [ "frame/utility", "frame/vesting", "frame/bags-list", + "frame/bags-list/fuzzer", "primitives/api", "primitives/api/proc-macro", "primitives/api/test", diff --git a/frame/bags-list/Cargo.toml b/frame/bags-list/Cargo.toml index cd06ce4a69983..372dc87e212e2 100644 --- a/frame/bags-list/Cargo.toml +++ b/frame/bags-list/Cargo.toml @@ -63,4 +63,10 @@ runtime-benchmarks = [ "sp-tracing", "frame-election-provider-support/runtime-benchmarks", ] +fuzz = [ + "sp-core", + "sp-io", + "pallet-balances", + "sp-tracing", +] diff --git a/frame/bags-list/fuzzer/.gitignore b/frame/bags-list/fuzzer/.gitignore new file mode 100644 index 0000000000000..3ebcb104d4a50 --- /dev/null +++ b/frame/bags-list/fuzzer/.gitignore @@ -0,0 +1,2 @@ +hfuzz_target +hfuzz_workspace diff --git a/frame/bags-list/fuzzer/Cargo.toml b/frame/bags-list/fuzzer/Cargo.toml new file mode 100644 index 0000000000000..171e0e7af70cd --- /dev/null +++ b/frame/bags-list/fuzzer/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "pallet-bags-list-fuzzer" +version = "4.0.0-dev" +authors = ["Parity Technologies "] +edition = "2018" +license = "Apache-2.0" +homepage = "https://substrate.dev" +repository = "https://github.com/paritytech/substrate/" +description = "Fuzzer for FRAME pallet bags list" +readme = "README.md" +publish = false + +[dependencies] +honggfuzz = "0.5" +rand = { version = "0.8", features = ["std", "small_rng"] } + +pallet-bags-list = { version = "4.0.0-dev", features = ["fuzz"], path = ".." } +frame-election-provider-support = { version = "4.0.0-dev", path = "../../election-provider-support", features = ["runtime-benchmarks"] } + +[[bin]] +name = "bags-list" +path = "src/main.rs" diff --git a/frame/bags-list/fuzzer/src/main.rs b/frame/bags-list/fuzzer/src/main.rs new file mode 100644 index 0000000000000..02a2003b9a71f --- /dev/null +++ b/frame/bags-list/fuzzer/src/main.rs @@ -0,0 +1,88 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Running +//! Running this fuzzer can be done with `cargo hfuzz run bags-list`. `honggfuzz` CLI options can +//! be used by setting `HFUZZ_RUN_ARGS`, such as `-n 4` to use 4 threads. +//! +//! # Debugging a panic +//! Once a panic is found, it can be debugged with +//! `cargo hfuzz run-debug fixed_point hfuzz_workspace/bags_list/*.fuzz`. +//! +//! # More information +//! More information about `honggfuzz` can be found +//! [here](https://docs.rs/honggfuzz/). + +use frame_election_provider_support::{SortedListProvider, VoteWeight}; +use honggfuzz::fuzz; +use pallet_bags_list::mock::{AccountId, BagsList, ExtBuilder}; +use std::convert::From; + +const ID_RANGE: AccountId = 25_000; + +/// Actions of a `SortedListProvider` that we fuzz. +enum Action { + Insert, + Update, + Remove, +} + +impl From for Action { + fn from(v: u32) -> Self { + let num_variants = Self::Remove as u32 + 1; + match v % num_variants { + _x if _x == Action::Insert as u32 => Action::Insert, + _x if _x == Action::Update as u32 => Action::Update, + _x if _x == Action::Remove as u32 => Action::Remove, + _ => unreachable!(), + } + } +} + +fn main() { + ExtBuilder::default().build_and_execute(|| loop { + fuzz!(|data: (AccountId, VoteWeight, u32)| { + let (account_id_seed, vote_weight, action_seed) = data; + + let id = account_id_seed % ID_RANGE; + let action = Action::from(action_seed); + + match action { + Action::Insert => { + if BagsList::on_insert(id.clone(), vote_weight).is_err() { + // this was a duplicate id, which is ok. We can just update it. + BagsList::on_update(&id, vote_weight); + } + assert!(BagsList::contains(&id)); + }, + Action::Update => { + let already_contains = BagsList::contains(&id); + BagsList::on_update(&id, vote_weight); + if already_contains { + assert!(BagsList::contains(&id)); + } + }, + Action::Remove => { + BagsList::on_remove(&id); + assert!(!BagsList::contains(&id)); + }, + } + + assert!(BagsList::sanity_check().is_ok()); + }) + }); +} diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 4202a4d499895..10a692e8b3f95 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -59,8 +59,8 @@ use sp_std::prelude::*; mod benchmarks; mod list; -#[cfg(test)] -mod mock; +#[cfg(any(test, feature = "fuzz"))] +pub mod mock; #[cfg(test)] mod tests; pub mod weights; diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 3f55f22271910..057565e645f90 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -391,8 +391,8 @@ impl List { /// /// * there are no duplicate ids, /// * length of this list is in sync with `CounterForListNodes`, - /// * and sanity-checks all bags. This will cascade down all the checks and makes sure all bags - /// are checked per *any* update to `List`. + /// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure + /// all bags and nodes are checked per *any* update to `List`. #[cfg(feature = "std")] pub(crate) fn sanity_check() -> Result<(), &'static str> { use frame_support::ensure; @@ -414,7 +414,6 @@ impl List { let thresholds = T::BagThresholds::get().iter().copied(); let thresholds: Vec = if thresholds.clone().last() == Some(VoteWeight::MAX) { // in the event that they included it, we don't need to make any changes - // Box::new(thresholds.collect() thresholds.collect() } else { // otherwise, insert it here. @@ -774,10 +773,13 @@ impl Node { "node does not exist in the expected bag" ); + let non_terminal_check = !self.is_terminal() && + expected_bag.head.as_ref() != Some(id) && + expected_bag.tail.as_ref() != Some(id); + let terminal_check = + expected_bag.head.as_ref() == Some(id) || expected_bag.tail.as_ref() == Some(id); frame_support::ensure!( - !self.is_terminal() || - expected_bag.head.as_ref() == Some(id) || - expected_bag.tail.as_ref() == Some(id), + non_terminal_check || terminal_check, "a terminal node is neither its bag head or tail" ); diff --git a/frame/bags-list/src/mock.rs b/frame/bags-list/src/mock.rs index a6ab35896b1e7..45eb1d85abe3c 100644 --- a/frame/bags-list/src/mock.rs +++ b/frame/bags-list/src/mock.rs @@ -101,12 +101,13 @@ pub(crate) const GENESIS_IDS: [(AccountId, VoteWeight); 4] = [(1, 10), (2, 1_000), (3, 1_000), (4, 1_000)]; #[derive(Default)] -pub(crate) struct ExtBuilder { +pub struct ExtBuilder { ids: Vec<(AccountId, VoteWeight)>, } impl ExtBuilder { /// Add some AccountIds to insert into `List`. + #[cfg(test)] pub(crate) fn add_ids(mut self, ids: Vec<(AccountId, VoteWeight)>) -> Self { self.ids = ids; self @@ -126,18 +127,20 @@ impl ExtBuilder { ext } - pub(crate) fn build_and_execute(self, test: impl FnOnce() -> ()) { + pub fn build_and_execute(self, test: impl FnOnce() -> ()) { self.build().execute_with(|| { test(); List::::sanity_check().expect("Sanity check post condition failed") }) } + #[cfg(test)] pub(crate) fn build_and_execute_no_post_check(self, test: impl FnOnce() -> ()) { self.build().execute_with(test) } } +#[cfg(test)] pub(crate) mod test_utils { use super::*; use list::Bag;