Benchmark Collective Pallet (#5343)

* Init

* Fix execute.

* Duplicate macro for instances temporarilly

* Add propose.

* Add vote, close.

* Propose from own module

* Add old members to set_members.

* Add previous proposals to propose.

* Compress a bit the macro.

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

substrate/Cargo.lock

@@ -4062,6 +4062,7 @@ dependencies = [
 name = "pallet-collective"
 version = "2.0.0-alpha.5"
 dependencies = [
+ "frame-benchmarking",
  "frame-support",
  "frame-system",
  "hex-literal",

substrate/bin/node/runtime/Cargo.toml

@@ -142,6 +142,7 @@ runtime-benchmarks = [
 	"pallet-session-benchmarking",
 	"pallet-staking/runtime-benchmarks",
 	"pallet-vesting/runtime-benchmarks",
+	"pallet-collective/runtime-benchmarks",
 	"pallet-session-benchmarking",
 	"pallet-staking/runtime-benchmarks",
 	"pallet-im-online/runtime-benchmarks",

substrate/bin/node/runtime/src/lib.rs

@@ -916,6 +916,13 @@ impl_runtime_apis! {
 					steps,
 					repeat,
 				),
+				b"pallet-collective" | b"collective" => Council::run_benchmark(
+					extrinsic,
+					lowest_range_values,
+					highest_range_values,
+					steps,
+					repeat,
+				),
 				_ => Err("Benchmark not found for this pallet."),
 			};
 

substrate/frame/benchmarking/src/lib.rs

@@ -134,7 +134,23 @@ macro_rules! benchmarks {
 		}
 		$( $rest:tt )*
 	) => {
-		$crate::benchmarks_iter!({
+		$crate::benchmarks_iter!(NO_INSTANCE {
+			$( { $common , $common_from , $common_to , $common_instancer } )*
+		} ( ) $( $rest )* );
+	}
+}
+
+#[macro_export]
+macro_rules! benchmarks_instance {
+	(
+		_ {
+			$(
+				let $common:ident in $common_from:tt .. $common_to:expr => $common_instancer:expr;
+			)*
+		}
+		$( $rest:tt )*
+	) => {
+		$crate::benchmarks_iter!(INSTANCE {
 			$( { $common , $common_from , $common_to , $common_instancer } )*
 		} ( ) $( $rest )* );
 	}
@@ -145,44 +161,63 @@ macro_rules! benchmarks {
 macro_rules! benchmarks_iter {
 	// mutation arm:
 	(
+		$instance:ident
 		{ $( $common:tt )* }
 		( $( $names:ident )* )
 		$name:ident { $( $code:tt )* }: _ ( $origin:expr $( , $arg:expr )* )
 		$( $rest:tt )*
 	) => {
 		$crate::benchmarks_iter! {
-			{ $( $common )* } ( $( $names )* ) $name { $( $code )* }: $name ( $origin $( , $arg )* ) $( $rest )*
+			$instance { $( $common )* } ( $( $names )* ) $name { $( $code )* }: $name ( $origin $( , $arg )* ) $( $rest )*
 		}
 	};
-	// mutation arm:
+	// no instance mutation arm:
 	(
+		NO_INSTANCE
 		{ $( $common:tt )* }
 		( $( $names:ident )* )
 		$name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* )
 		$( $rest:tt )*
 	) => {
 		$crate::benchmarks_iter! {
+			NO_INSTANCE
 			{ $( $common )* } ( $( $names )* ) $name { $( $code )* }: {
 				<Call<T> as $crate::Dispatchable>::dispatch(Call::<T>::$dispatch($($arg),*), $origin.into())?;
 			} $( $rest )*
 		}
 	};
+	// instance mutation arm:
+	(
+		INSTANCE
+		{ $( $common:tt )* }
+		( $( $names:ident )* )
+		$name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* )
+		$( $rest:tt )*
+	) => {
+		$crate::benchmarks_iter! {
+			INSTANCE
+			{ $( $common )* } ( $( $names )* ) $name { $( $code )* }: {
+				<Call<T, I> as $crate::Dispatchable>::dispatch(Call::<T, I>::$dispatch($($arg),*), $origin.into())?;
+			} $( $rest )*
+		}
+	};
 	// iteration arm:
 	(
+		$instance:ident
 		{ $( $common:tt )* }
 		( $( $names:ident )* )
 		$name:ident { $( $code:tt )* }: $eval:block
 		$( $rest:tt )*
 	) => {
 		$crate::benchmark_backend! {
-			$name { $( $common )* } { } { $eval } { $( $code )* }
+			$instance $name { $( $common )* } { } { $eval } { $( $code )* }
 		}
-		$crate::benchmarks_iter!( { $( $common )* } ( $( $names )* $name ) $( $rest )* );
+		$crate::benchmarks_iter!( $instance { $( $common )* } ( $( $names )* $name ) $( $rest )* );
 	};
 	// iteration-exit arm
-	( { $( $common:tt )* } ( $( $names:ident )* ) ) => {
-		$crate::selected_benchmark!( $( $names ),* );
-		$crate::impl_benchmark!( $( $names ),* );
+	( $instance:ident { $( $common:tt )* } ( $( $names:ident )* ) ) => {
+		$crate::selected_benchmark!( $instance $( $names ),* );
+		$crate::impl_benchmark!( $instance $( $names ),* );
 	}
 }
 
@@ -190,7 +225,7 @@ macro_rules! benchmarks_iter {
 #[allow(missing_docs)]
 macro_rules! benchmark_backend {
 	// parsing arms
-	($name:ident {
+	($instance:ident $name:ident {
 		$( $common:tt )*
 	} {
 		$( PRE { $( $pre_parsed:tt )* } )*
@@ -199,13 +234,13 @@ macro_rules! benchmark_backend {
 			$( $rest:tt )*
 	} ) => {
 		$crate::benchmark_backend! {
-			$name { $( $common )* } {
+			$instance $name { $( $common )* } {
 				$( PRE { $( $pre_parsed )* } )*
 				PRE { $pre_id , $pre_ty , $pre_ex }
 			} { $eval } { $( $rest )* }
 		}
 	};
-	($name:ident {
+	($instance:ident $name:ident {
 		$( $common:tt )*
 	} {
 		$( $parsed:tt )*
@@ -214,14 +249,14 @@ macro_rules! benchmark_backend {
 		$( $rest:tt )*
 	}) => {
 		$crate::benchmark_backend! {
-			$name { $( $common )* } {
+			$instance $name { $( $common )* } {
 				$( $parsed )*
 				PARAM { $param , $param_from , $param_to , $param_instancer }
 			} { $eval } { $( $rest )* }
 		}
 	};
 	// mutation arm to look after defaulting to a common param
-	($name:ident {
+	($instance:ident $name:ident {
 		$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
 	} {
 		$( $parsed:tt )*
@@ -230,7 +265,7 @@ macro_rules! benchmark_backend {
 		$( $rest:tt )*
 	}) => {
 		$crate::benchmark_backend! {
-			$name {
+			$instance $name {
 				$( { $common , $common_from , $common_to , $common_instancer } )*
 			} {
 				$( $parsed )*
@@ -244,7 +279,7 @@ macro_rules! benchmark_backend {
 		}
 	};
 	// mutation arm to look after defaulting only the range to common param
-	($name:ident {
+	($instance:ident $name:ident {
 		$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
 	} {
 		$( $parsed:tt )*
@@ -253,7 +288,7 @@ macro_rules! benchmark_backend {
 		$( $rest:tt )*
 	}) => {
 		$crate::benchmark_backend! {
-			$name {
+			$instance $name {
 				$( { $common , $common_from , $common_to , $common_instancer } )*
 			} {
 				$( $parsed )*
@@ -267,7 +302,7 @@ macro_rules! benchmark_backend {
 		}
 	};
 	// mutation arm to look after a single tt for param_from.
-	($name:ident {
+	($instance:ident $name:ident {
 		$( $common:tt )*
 	} {
 		$( $parsed:tt )*
@@ -276,14 +311,14 @@ macro_rules! benchmark_backend {
 		$( $rest:tt )*
 	}) => {
 		$crate::benchmark_backend! {
-			$name { $( $common )* } { $( $parsed )* } { $eval } {
+			$instance $name { $( $common )* } { $( $parsed )* } { $eval } {
 				let $param in ( $param_from ) .. $param_to => $param_instancer;
 				$( $rest )*
 			}
 		}
 	};
 	// mutation arm to look after the default tail of `=> ()`
-	($name:ident {
+	($instance:ident $name:ident {
 		$( $common:tt )*
 	} {
 		$( $parsed:tt )*
@@ -292,14 +327,14 @@ macro_rules! benchmark_backend {
 		$( $rest:tt )*
 	}) => {
 		$crate::benchmark_backend! {
-			$name { $( $common )* } { $( $parsed )* } { $eval } {
+			$instance $name { $( $common )* } { $( $parsed )* } { $eval } {
 				let $param in $param_from .. $param_to => ();
 				$( $rest )*
 			}
 		}
 	};
 	// mutation arm to look after `let _ =`
-	($name:ident {
+	($instance:ident $name:ident {
 		$( $common:tt )*
 	} {
 		$( $parsed:tt )*
@@ -308,14 +343,14 @@ macro_rules! benchmark_backend {
 		$( $rest:tt )*
 	}) => {
 		$crate::benchmark_backend! {
-			$name { $( $common )* } { $( $parsed )* } { $eval } {
+			$instance $name { $( $common )* } { $( $parsed )* } { $eval } {
 				let $pre_id : _ = $pre_ex;
 				$( $rest )*
 			}
 		}
 	};
-	// actioning arm
-	($name:ident {
+	// no instance actioning arm
+	(NO_INSTANCE $name:ident {
 		$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
 	} {
 		$( PRE { $pre_id:tt , $pre_ty:ty , $pre_ex:expr } )*
@@ -333,6 +368,45 @@ macro_rules! benchmark_backend {
 				]
 			}
 
+			fn instance(&self, components: &[($crate::BenchmarkParameter, u32)])
+				-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
+			{
+				$(
+					let $common = $common_from;
+				)*
+				$(
+					// Prepare instance
+					let $param = components.iter().find(|&c| c.0 == $crate::BenchmarkParameter::$param).unwrap().1;
+				)*
+				$(
+					let $pre_id : $pre_ty = $pre_ex;
+				)*
+				$( $param_instancer ; )*
+				$( $post )*
+
+				Ok(Box::new(move || -> Result<(), &'static str> { $eval; Ok(()) }))
+			}
+		}
+	};
+	// instance actioning arm
+	(INSTANCE $name:ident {
+		$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
+	} {
+		$( PRE { $pre_id:tt , $pre_ty:ty , $pre_ex:expr } )*
+		$( PARAM { $param:ident , $param_from:expr , $param_to:expr , $param_instancer:expr } )*
+	} { $eval:block } { $( $post:tt )* } ) => {
+		#[allow(non_camel_case_types)]
+		struct $name;
+		#[allow(unused_variables)]
+		impl<T: Trait<I>, I: Instance> $crate::BenchmarkingSetupInstance<T, I> for $name {
+			fn components(&self) -> Vec<($crate::BenchmarkParameter, u32, u32)> {
+				vec! [
+					$(
+						($crate::BenchmarkParameter::$param, $param_from, $param_to)
+					),*
+				]
+			}
+
 			fn instance(&self, components: &[($crate::BenchmarkParameter, u32)])
 				-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
 			{
@@ -372,7 +446,7 @@ macro_rules! benchmark_backend {
 #[macro_export]
 macro_rules! selected_benchmark {
 	(
-		$( $bench:ident ),*
+		NO_INSTANCE $( $bench:ident ),*
 	) => {
 		// The list of available benchmarks for this pallet.
 		#[allow(non_camel_case_types)]
@@ -397,12 +471,38 @@ macro_rules! selected_benchmark {
 			}
 		}
 	};
+	(
+		INSTANCE $( $bench:ident ),*
+	) => {
+		// The list of available benchmarks for this pallet.
+		#[allow(non_camel_case_types)]
+		enum SelectedBenchmark {
+			$( $bench, )*
+		}
+
+		// Allow us to select a benchmark from the list of available benchmarks.
+		impl<T: Trait<I>, I: Instance> $crate::BenchmarkingSetupInstance<T, I> for SelectedBenchmark {
+			fn components(&self) -> Vec<($crate::BenchmarkParameter, u32, u32)> {
+				match self {
+					$( Self::$bench => <$bench as $crate::BenchmarkingSetupInstance<T, I>>::components(&$bench), )*
+				}
+			}
+
+			fn instance(&self, components: &[($crate::BenchmarkParameter, u32)])
+				-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
+			{
+				match self {
+					$( Self::$bench => <$bench as $crate::BenchmarkingSetupInstance<T, I>>::instance(&$bench, components), )*
+				}
+			}
+		}
+	}
 }
 
 #[macro_export]
 macro_rules! impl_benchmark {
 	(
-		$( $name:ident ),*
+		NO_INSTANCE $( $name:ident ),*
 	) => {
 		impl<T: Trait> $crate::Benchmarking<$crate::BenchmarkResults> for Module<T> {
 			fn run_benchmark(
@@ -492,5 +592,97 @@ macro_rules! impl_benchmark {
 				return Ok(results);
 			}
 		}
+	};
+	(
+		INSTANCE $( $name:ident ),*
+	) => {
+		impl<T: Trait<I>, I: Instance> $crate::Benchmarking<$crate::BenchmarkResults> for Module<T, I> {
+			fn run_benchmark(
+				extrinsic: Vec<u8>,
+				lowest_range_values: Vec<u32>,
+				highest_range_values: Vec<u32>,
+				steps: Vec<u32>,
+				repeat: u32,
+			) -> Result<Vec<$crate::BenchmarkResults>, &'static str> {
+				// Map the input to the selected benchmark.
+				let extrinsic = sp_std::str::from_utf8(extrinsic.as_slice())
+					.map_err(|_| "`extrinsic` is not a valid utf8 string!")?;
+				let selected_benchmark = match extrinsic {
+					$( stringify!($name) => SelectedBenchmark::$name, )*
+					_ => return Err("Could not find extrinsic."),
+				};
+
+				// Warm up the DB
+				$crate::benchmarking::commit_db();
+				$crate::benchmarking::wipe_db();
+
+				let components = <SelectedBenchmark as $crate::BenchmarkingSetupInstance<T, I>>::components(&selected_benchmark);
+				let mut results: Vec<$crate::BenchmarkResults> = Vec::new();
+
+				// Default number of steps for a component.
+				let mut prev_steps = 10;
+
+				// Select the component we will be benchmarking. Each component will be benchmarked.
+				for (idx, (name, low, high)) in components.iter().enumerate() {
+					// Get the number of steps for this component.
+					let steps = steps.get(idx).cloned().unwrap_or(prev_steps);
+					prev_steps = steps;
+
+					let lowest = lowest_range_values.get(idx).cloned().unwrap_or(*low);
+					let highest = highest_range_values.get(idx).cloned().unwrap_or(*high);
+
+					let diff = highest - lowest;
+
+					// Create up to `STEPS` steps for that component between high and low.
+					let step_size = (diff / steps).max(1);
+					let num_of_steps = diff / step_size + 1;
+
+					for s in 0..num_of_steps {
+						// This is the value we will be testing for component `name`
+						let component_value = lowest + step_size * s;
+
+						// Select the max value for all the other components.
+						let c: Vec<($crate::BenchmarkParameter, u32)> = components.iter()
+							.enumerate()
+							.map(|(idx, (n, _, h))|
+								if n == name {
+									(*n, component_value)
+								} else {
+									(*n, *highest_range_values.get(idx).unwrap_or(h))
+								}
+							)
+							.collect();
+
+						// Run the benchmark `repeat` times.
+						for _ in 0..repeat {
+							// Set up the externalities environment for the setup we want to benchmark.
+							let closure_to_benchmark = <SelectedBenchmark as $crate::BenchmarkingSetupInstance<T, I>>::instance(&selected_benchmark, &c)?;
+
+							// Commit the externalities to the database, flushing the DB cache.
+							// This will enable worst case scenario for reading from the database.
+							$crate::benchmarking::commit_db();
+
+							// Time the extrinsic logic.
+							let start_extrinsic = $crate::benchmarking::current_time();
+							closure_to_benchmark()?;
+							let finish_extrinsic = $crate::benchmarking::current_time();
+							let elapsed_extrinsic = finish_extrinsic - start_extrinsic;
+
+							// Time the storage root recalculation.
+							let start_storage_root = $crate::benchmarking::current_time();
+							$crate::storage_root();
+							let finish_storage_root = $crate::benchmarking::current_time();
+							let elapsed_storage_root = finish_storage_root - start_storage_root;
+
+							results.push((c.clone(), elapsed_extrinsic, elapsed_storage_root));
+
+							// Wipe the DB back to the genesis state.
+							$crate::benchmarking::wipe_db();
+						}
+					}
+				}
+				return Ok(results);
+			}
+		}
 	}
 }

substrate/frame/benchmarking/src/utils.rs

@@ -101,6 +101,15 @@ pub trait BenchmarkingSetup<T> {
 	fn instance(&self, components: &[(BenchmarkParameter, u32)]) -> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>;
 }
 
+/// The required setup for creating a benchmark.
+pub trait BenchmarkingSetupInstance<T, I> {
+	/// Return the components and their ranges which should be tested in this benchmark.
+	fn components(&self) -> Vec<(BenchmarkParameter, u32, u32)>;
+
+	/// Set up the storage, and prepare a closure to test in a single run of the benchmark.
+	fn instance(&self, components: &[(BenchmarkParameter, u32)]) -> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>;
+}
+
 /// Grab an account, seeded by a name and index.
 pub fn account<AccountId: Decode + Default>(name: &'static str, index: u32, seed: u32) -> AccountId {
 	let entropy = (name, index, seed).using_encoded(blake2_256);

substrate/frame/collective/Cargo.toml

@@ -15,6 +15,7 @@ sp-core = { version = "2.0.0-alpha.5", default-features = false, path = "../../p
 sp-std = { version = "2.0.0-alpha.5", default-features = false, path = "../../primitives/std" }
 sp-io = { version = "2.0.0-alpha.5", default-features = false, path = "../../primitives/io" }
 sp-runtime = { version = "2.0.0-alpha.5", default-features = false, path = "../../primitives/runtime" }
+frame-benchmarking = { version = "2.0.0-alpha.5", default-features = false, path = "../benchmarking", optional = true }
 frame-support = { version = "2.0.0-alpha.5", default-features = false, path = "../support" }
 frame-system = { version = "2.0.0-alpha.5", default-features = false, path = "../system" }
 
@@ -30,7 +31,9 @@ std = [
 	"sp-std/std",
 	"serde",
 	"sp-io/std",
+	"frame-benchmarking/std",
 	"frame-support/std",
 	"sp-runtime/std",
 	"frame-system/std",
 ]
+runtime-benchmarks = ["frame-benchmarking"]

substrate/frame/collective/src/benchmarking.rs

@@ -0,0 +1,185 @@
+// Copyright 2020 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate 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.
+
+// Substrate 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 Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Staking pallet benchmarking.
+
+use super::*;
+
+use frame_system::RawOrigin as SystemOrigin;
+use frame_benchmarking::{benchmarks_instance, account};
+
+use frame_system::Module as System;
+use crate::Module as Collective;
+
+const SEED: u32 = 0;
+
+benchmarks_instance! {
+	_{
+		// User account seed.
+		let u in 1 .. 1000 => ();
+		// Old members.
+		let n in 1 .. 1000 => ();
+		// New members.
+		let m in 1 .. 1000 => ();
+		// Existing proposals.
+		let p in 1 .. 100 => ();
+	}
+
+	set_members {
+		let m in ...;
+		let n in ...;
+
+		// Construct `new_members`.
+		// It should influence timing since it will sort this vector.
+		let mut new_members = vec![];
+		for i in 0 .. m {
+			let member = account("member", i, SEED);
+			new_members.push(member);
+		}
+
+		// Set old members. 
+		// We compute the difference of old and new members, so it should influence timing.
+		let mut old_members = vec![];
+		for i in 0 .. n {
+			let old_member = account("old member", i, SEED);
+			old_members.push(old_member);
+		}
+
+		let prime = Some(account("prime", 0, SEED));
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), old_members, prime.clone())?;
+
+	}: _(SystemOrigin::Root, new_members, prime)
+
+	execute {
+		let u in ...;
+
+		let caller: T::AccountId = account("caller", u, SEED);
+		let proposal: T::Proposal = Call::<T, I>::close(Default::default(), Default::default()).into();
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller.clone()], None)?;
+
+	}: _(SystemOrigin::Signed(caller), Box::new(proposal))
+
+	propose {
+		let u in ...;
+
+		let caller: T::AccountId = account("caller", u, SEED);
+		let proposal: T::Proposal = Call::<T, I>::close(Default::default(), Default::default()).into();
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller.clone()], None)?;
+
+		let member_count = 0;
+
+	}: _(SystemOrigin::Signed(caller), member_count, Box::new(proposal.into()))
+
+	propose_else_branch {
+		let u in ...;
+		let p in ...;
+
+		let caller: T::AccountId = account("caller", u, SEED);
+		let proposal: T::Proposal = Call::<T, I>::close(Default::default(), Default::default()).into();
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller.clone()], None)?;
+
+		let member_count = 3;
+
+		// Add previous proposals.
+		for i in 0 .. p {
+			let proposal: T::Proposal = Call::<T, I>::close(Default::default(), (i + 1).into()).into();
+			Collective::<T, _>::propose(SystemOrigin::Signed(caller.clone()).into(), member_count.clone(), Box::new(proposal.into()))?;
+		}
+
+	}: propose(SystemOrigin::Signed(caller), member_count, Box::new(proposal.into()))
+
+	vote {
+		let u in ...;
+
+		let caller1: T::AccountId = account("caller1", u, SEED);
+		let caller2: T::AccountId = account("caller2", u, SEED);
+
+		let proposal: Box<T::Proposal> = Box::new(Call::<T, I>::close(Default::default(), Default::default()).into());
+		let proposal_hash = T::Hashing::hash_of(&proposal);
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller1.clone(), caller2.clone()], None)?;
+
+		let member_count = 3;
+		Collective::<T, _>::propose(SystemOrigin::Signed(caller1.clone()).into(), member_count, proposal)?;
+
+		let index = 0;
+		let approve = true;
+
+	}: _(SystemOrigin::Signed(caller2), proposal_hash, index, approve)
+
+	vote_not_approve {
+		let u in ...;
+
+		let caller1: T::AccountId = account("caller1", u, SEED);
+		let caller2: T::AccountId = account("caller2", u, SEED);
+
+		let proposal: Box<T::Proposal> = Box::new(Call::<T, I>::close(Default::default(), Default::default()).into());
+		let proposal_hash = T::Hashing::hash_of(&proposal);
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller1.clone(), caller2.clone()], None)?;
+
+		let member_count = 3;
+		Collective::<T, _>::propose(SystemOrigin::Signed(caller1.clone()).into(), member_count, proposal)?;
+
+		let index = 0;
+		let approve = false;
+
+	}: vote(SystemOrigin::Signed(caller2), proposal_hash, index, approve)
+
+	vote_approved {
+		let u in ...;
+
+		let caller1: T::AccountId = account("caller1", u, SEED);
+		let caller2: T::AccountId = account("caller2", u, SEED);
+		
+		let proposal: Box<T::Proposal> = Box::new(Call::<T, I>::close(Default::default(), Default::default()).into());
+		let proposal_hash = T::Hashing::hash_of(&proposal);
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller1.clone(), caller2.clone()], None)?;
+
+		let member_count = 2;
+		Collective::<T, _>::propose(SystemOrigin::Signed(caller1.clone()).into(), member_count, proposal)?;
+
+		let index = 0;
+		let approve = true;
+
+	}: vote(SystemOrigin::Signed(caller2), proposal_hash, index, approve)
+
+	close {
+		let u in ...;
+
+		let caller1: T::AccountId = account("caller1", u, SEED);
+		let caller2: T::AccountId = account("caller2", u, SEED);
+		
+		let proposal: Box<T::Proposal> = Box::new(Call::<T, I>::close(Default::default(), Default::default()).into());
+		let proposal_hash = T::Hashing::hash_of(&proposal);
+
+		Collective::<T, _>::set_members(SystemOrigin::Root.into(), vec![caller1.clone(), caller2.clone()], None)?;
+		let member_count = 2;
+		Collective::<T, _>::propose(SystemOrigin::Signed(caller1.clone()).into(), member_count, proposal)?;
+
+		let index = 0;
+		let approve = true;
+
+		let vote_end = T::MotionDuration::get() + 1u32.into();
+		System::<T>::set_block_number(vote_end);
+
+	}: _(SystemOrigin::Signed(caller2), proposal_hash, index)
+}

substrate/frame/collective/src/lib.rs

@@ -48,6 +48,9 @@ use frame_support::{
 };
 use frame_system::{self as system, ensure_signed, ensure_root};
 
+#[cfg(feature = "runtime-benchmarks")]
+mod benchmarking;
+
 /// Simple index type for proposal counting.
 pub type ProposalIndex = u32;
 
@@ -57,12 +60,12 @@ pub type ProposalIndex = u32;
 /// vote exactly once, therefore also the number of votes for any given motion.
 pub type MemberCount = u32;
 
-pub trait Trait<I=DefaultInstance>: frame_system::Trait {
+pub trait Trait<I: Instance=DefaultInstance>: frame_system::Trait {
 	/// The outer origin type.
 	type Origin: From<RawOrigin<Self::AccountId, I>>;
 
 	/// The outer call dispatch type.
-	type Proposal: Parameter + Dispatchable<Origin=<Self as Trait<I>>::Origin>;
+	type Proposal: Parameter + Dispatchable<Origin=<Self as Trait<I>>::Origin> + From<Call<Self, I>>;
 
 	/// The outer event type.
 	type Event: From<Event<Self, I>> + Into<<Self as frame_system::Trait>::Event>;