Sensible scheduling for referenda (#2753)

* Nonlinear locking and cleanups

* Bump runtime version

* Minor cleanup

* Fix tests

* Fix council tests

* Fix flaw in turnout counting

* Initial work on referendum schedules

* Refactor council-democracy interface.

* Fix build

* Update srml/democracy/src/lib.rs

Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>

* Update srml/democracy/src/lib.rs

Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>

* Tests compile again

* Tests!

* Update todo

* Fix build

* Ensure votes arer not double-counted on member-transitions

* Extra logic for normal council changes

* Typo

* A few grumbles addressed.

substrate/srml/contract/src/lib.rs

@@ -506,10 +506,10 @@ decl_module! {
 		fn claim_surcharge(origin, dest: T::AccountId, aux_sender: Option<T::AccountId>) {
 			let origin = origin.into();
 			let (signed, rewarded) = match origin {
-				Some(system::RawOrigin::Signed(ref account)) if aux_sender.is_none() => {
+				Ok(system::RawOrigin::Signed(ref account)) if aux_sender.is_none() => {
 					(true, account)
 				},
-				Some(system::RawOrigin::None) if aux_sender.is_some() => {
+				Ok(system::RawOrigin::None) if aux_sender.is_some() => {
 					(false, aux_sender.as_ref().expect("checked above"))
 				},
 				_ => return Err("Invalid surcharge claim: origin must be signed or \

substrate/srml/council/src/lib.rs

@@ -18,34 +18,42 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-pub mod voting;
 pub mod motions;
 pub mod seats;
 
 pub use crate::seats::{Trait, Module, RawEvent, Event, VoteIndex};
 
+/// Trait for type that can handle incremental changes to a set of account IDs.
+pub trait OnMembersChanged<AccountId> {
+	/// A number of members `new` just joined the set and replaced some `old` ones.
+	fn on_members_changed(new: &[AccountId], old: &[AccountId]);
+}
+
+impl<T> OnMembersChanged<T> for () {
+	fn on_members_changed(_new: &[T], _old: &[T]) {}
+}
+
 #[cfg(test)]
 mod tests {
 	// These re-exports are here for a reason, edit with care
 	pub use super::*;
 	pub use runtime_io::with_externalities;
 	use srml_support::{impl_outer_origin, impl_outer_event, impl_outer_dispatch, parameter_types};
-	pub use substrate_primitives::H256;
-	pub use primitives::BuildStorage;
-	pub use primitives::traits::{BlakeTwo256, IdentityLookup};
-	pub use primitives::testing::{Digest, DigestItem, Header};
-	pub use substrate_primitives::{Blake2Hasher};
-	pub use {seats, motions, voting};
+	pub use substrate_primitives::{H256, Blake2Hasher, u32_trait::{_1, _2, _3, _4}};
+	pub use primitives::{
+		BuildStorage, traits::{BlakeTwo256, IdentityLookup}, testing::{Digest, DigestItem, Header}
+	};
+	pub use {seats, motions};
 
 	impl_outer_origin! {
 		pub enum Origin for Test {
-			motions
+			motions<T>
 		}
 	}
 
 	impl_outer_event! {
 		pub enum Event for Test {
-			balances<T>, democracy<T>, seats<T>, voting<T>, motions<T>,
+			balances<T>, democracy<T>, seats<T>, motions<T>,
 		}
 	}
 
@@ -86,6 +94,7 @@ mod tests {
 		pub const VotingPeriod: u64 = 3;
 		pub const MinimumDeposit: u64 = 1;
 		pub const EnactmentPeriod: u64 = 0;
+		pub const CooloffPeriod: u64 = 2;
 	}
 	impl democracy::Trait for Test {
 		type Proposal = Call;
@@ -93,22 +102,27 @@ mod tests {
 		type Currency = balances::Module<Self>;
 		type EnactmentPeriod = EnactmentPeriod;
 		type LaunchPeriod = LaunchPeriod;
+		type EmergencyVotingPeriod = VotingPeriod;
 		type VotingPeriod = VotingPeriod;
 		type MinimumDeposit = MinimumDeposit;
+		type ExternalOrigin = motions::EnsureProportionAtLeast<_1, _2, u64>;
+		type ExternalMajorityOrigin = motions::EnsureProportionAtLeast<_2, _3, u64>;
+		type EmergencyOrigin = motions::EnsureProportionAtLeast<_1, _1, u64>;
+		type CancellationOrigin = motions::EnsureProportionAtLeast<_2, _3, u64>;
+		type VetoOrigin = motions::EnsureMember<u64>;
+		type CooloffPeriod = CooloffPeriod;
 	}
 	impl seats::Trait for Test {
 		type Event = Event;
 		type BadPresentation = ();
 		type BadReaper = ();
+		type OnMembersChanged = CouncilMotions;
 	}
 	impl motions::Trait for Test {
 		type Origin = Origin;
 		type Proposal = Call;
 		type Event = Event;
 	}
-	impl voting::Trait for Test {
-		type Event = Event;
-	}
 
 	pub fn new_test_ext(with_council: bool) -> runtime_io::TestExternalities<Blake2Hasher> {
 		let mut t = system::GenesisConfig::<Test>::default().build_storage().unwrap().0;
@@ -138,11 +152,6 @@ mod tests {
 			desired_seats: 2,
 			term_duration: 5,
 		}.build_storage().unwrap().0);
-		t.extend(voting::GenesisConfig::<Test> {
-			cooloff_period: 2,
-			voting_period: 1,
-			enact_delay_period: 0,
-		}.build_storage().unwrap().0);
 		runtime_io::TestExternalities::new(t)
 	}
 
@@ -150,6 +159,5 @@ mod tests {
 	pub type Balances = balances::Module<Test>;
 	pub type Democracy = democracy::Module<Test>;
 	pub type Council = seats::Module<Test>;
-	pub type CouncilVoting = voting::Module<Test>;
 	pub type CouncilMotions = motions::Module<Test>;
 }

substrate/srml/council/src/motions.rs

@@ -16,21 +16,27 @@
 
 //! Council voting system.
 
-use rstd::prelude::*;
-use rstd::result;
+use rstd::{prelude::*, result};
 use substrate_primitives::u32_trait::Value as U32;
 use primitives::traits::{Hash, EnsureOrigin};
-use srml_support::dispatch::{Dispatchable, Parameter};
-use srml_support::{StorageValue, StorageMap, decl_module, decl_event, decl_storage, ensure};
-use super::{Trait as CouncilTrait, Module as Council};
+use srml_support::{
+	dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode},
+	StorageValue, StorageMap, decl_module, decl_event, decl_storage, ensure
+};
+use super::{Trait as CouncilTrait, Module as Council, OnMembersChanged};
 use system::{self, ensure_signed};
 
 /// Simple index type for proposal counting.
 pub type ProposalIndex = u32;
+/// A number of council members.
+///
+/// This also serves as a number of voting members, and since for motions, each council member may
+/// vote exactly once, therefore also the number of votes for any given motion.
+pub type MemberCount = u32;
 
 pub trait Trait: CouncilTrait {
 	/// The outer origin type.
-	type Origin: From<Origin>;
+	type Origin: From<RawOrigin<Self::AccountId>>;
 
 	/// The outer call dispatch type.
 	type Proposal: Parameter + Dispatchable<Origin=<Self as Trait>::Origin>;
@@ -42,31 +48,79 @@ pub trait Trait: CouncilTrait {
 /// Origin for the council module.
 #[derive(PartialEq, Eq, Clone)]
 #[cfg_attr(feature = "std", derive(Debug))]
-pub enum Origin {
-	/// It has been condoned by a given number of council members.
-	Members(u32),
+pub enum RawOrigin<AccountId> {
+	/// It has been condoned by a given number of council members from a given total.
+	Members(MemberCount, MemberCount),
+	/// It has been condoned by a single council member.
+	Member(AccountId),
+}
+
+/// Origin for the council module.
+pub type Origin<T> = RawOrigin<<T as system::Trait>::AccountId>;
+
+#[derive(PartialEq, Eq, Clone, Encode, Decode)]
+#[cfg_attr(feature = "std", derive(Debug))]
+/// Info for keeping track of a motion being voted on.
+pub struct Votes<AccountId> {
+	/// The proposal's unique index.
+	index: ProposalIndex,
+	/// The number of approval votes that are needed to pass the motion.
+	threshold: MemberCount,
+	/// The current set of voters that approved it.
+	ayes: Vec<AccountId>,
+	/// The current set of voters that rejected it.
+	nays: Vec<AccountId>,
+}
+
+decl_storage! {
+	trait Store for Module<T: Trait> as CouncilMotions {
+		/// The hashes of the active proposals.
+		pub Proposals get(proposals): Vec<T::Hash>;
+		/// Actual proposal for a given hash, if it's current.
+		pub ProposalOf get(proposal_of): map T::Hash => Option<<T as Trait>::Proposal>;
+		/// Votes on a given proposal, if it is ongoing.
+		pub Voting get(voting): map T::Hash => Option<Votes<T::AccountId>>;
+		/// Proposals so far.
+		pub ProposalCount get(proposal_count): u32;
+	}
 }
 
 decl_event!(
 	pub enum Event<T> where <T as system::Trait>::Hash, <T as system::Trait>::AccountId {
-		/// A motion (given hash) has been proposed (by given account) with a threshold (given u32).
-		Proposed(AccountId, ProposalIndex, Hash, u32),
+		/// A motion (given hash) has been proposed (by given account) with a threshold (given
+		/// `MemberCount`).
+		Proposed(AccountId, ProposalIndex, Hash, MemberCount),
 		/// A motion (given hash) has been voted on by given account, leaving
-		/// a tally (yes votes and no votes given as u32s respectively).
-		Voted(AccountId, Hash, bool, u32, u32),
+		/// a tally (yes votes and no votes given respectively as `MemberCount`).
+		Voted(AccountId, Hash, bool, MemberCount, MemberCount),
 		/// A motion was approved by the required threshold.
 		Approved(Hash),
 		/// A motion was not approved by the required threshold.
 		Disapproved(Hash),
 		/// A motion was executed; `bool` is true if returned without error.
 		Executed(Hash, bool),
+		/// A single councillor did some action; `bool` is true if returned without error.
+		MemberExecuted(Hash, bool),
 	}
 );
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: <T as system::Trait>::Origin {
 		fn deposit_event<T>() = default;
-		fn propose(origin, #[compact] threshold: u32, proposal: Box<<T as Trait>::Proposal>) {
+
+		/// Dispatch a proposal from a councilor using the `Member` origin.
+		///
+		/// Origin must be a council member.
+		fn execute(origin, proposal: Box<<T as Trait>::Proposal>) {
+			let who = ensure_signed(origin)?;
+			ensure!(Self::is_councillor(&who), "proposer not on council");
+
+			let proposal_hash = T::Hashing::hash_of(&proposal);
+			let ok = proposal.dispatch(RawOrigin::Member(who).into()).is_ok();
+			Self::deposit_event(RawEvent::MemberExecuted(proposal_hash, ok));
+		}
+
+		fn propose(origin, #[compact] threshold: MemberCount, proposal: Box<<T as Trait>::Proposal>) {
 			let who = ensure_signed(origin)?;
 
 			ensure!(Self::is_councillor(&who), "proposer not on council");
@@ -76,14 +130,16 @@ decl_module! {
 			ensure!(!<ProposalOf<T>>::exists(proposal_hash), "duplicate proposals not allowed");
 
 			if threshold < 2 {
-				let ok = proposal.dispatch(Origin::Members(1).into()).is_ok();
+				let seats = <Council<T>>::active_council().len() as MemberCount;
+				let ok = proposal.dispatch(RawOrigin::Members(1, seats).into()).is_ok();
 				Self::deposit_event(RawEvent::Executed(proposal_hash, ok));
 			} else {
 				let index = Self::proposal_count();
 				<ProposalCount<T>>::mutate(|i| *i += 1);
 				<Proposals<T>>::mutate(|proposals| proposals.push(proposal_hash));
 				<ProposalOf<T>>::insert(proposal_hash, *proposal);
-				<Voting<T>>::insert(proposal_hash, (index, threshold, vec![who.clone()], vec![]));
+				let votes = Votes { index, threshold, ayes: vec![who.clone()], nays: vec![] };
+				<Voting<T>>::insert(proposal_hash, votes);
 
 				Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold));
 			}
@@ -95,46 +151,46 @@ decl_module! {
 			ensure!(Self::is_councillor(&who), "voter not on council");
 
 			let mut voting = Self::voting(&proposal).ok_or("proposal must exist")?;
-			ensure!(voting.0 == index, "mismatched index");
+			ensure!(voting.index == index, "mismatched index");
 
-			let position_yes = voting.2.iter().position(|a| a == &who);
-			let position_no = voting.3.iter().position(|a| a == &who);
+			let position_yes = voting.ayes.iter().position(|a| a == &who);
+			let position_no = voting.nays.iter().position(|a| a == &who);
 
 			if approve {
 				if position_yes.is_none() {
-					voting.2.push(who.clone());
+					voting.ayes.push(who.clone());
 				} else {
 					return Err("duplicate vote ignored")
 				}
 				if let Some(pos) = position_no {
-					voting.3.swap_remove(pos);
+					voting.nays.swap_remove(pos);
 				}
 			} else {
 				if position_no.is_none() {
-					voting.3.push(who.clone());
+					voting.nays.push(who.clone());
 				} else {
 					return Err("duplicate vote ignored")
 				}
 				if let Some(pos) = position_yes {
-					voting.2.swap_remove(pos);
+					voting.ayes.swap_remove(pos);
 				}
 			}
 
-			let yes_votes = voting.2.len() as u32;
-			let no_votes = voting.3.len() as u32;
+			let yes_votes = voting.ayes.len() as MemberCount;
+			let no_votes = voting.nays.len() as MemberCount;
 			Self::deposit_event(RawEvent::Voted(who, proposal, approve, yes_votes, no_votes));
 
-			let threshold = voting.1;
-			let potential_votes = <Council<T>>::active_council().len() as u32;
-			let approved = yes_votes >= threshold;
-			let disapproved = potential_votes.saturating_sub(no_votes) < threshold;
+			let seats = <Council<T>>::active_council().len() as MemberCount;
+			let approved = yes_votes >= voting.threshold;
+			let disapproved = seats.saturating_sub(no_votes) < voting.threshold;
 			if approved || disapproved {
 				if approved {
 					Self::deposit_event(RawEvent::Approved(proposal));
 
 					// execute motion, assuming it exists.
 					if let Some(p) = <ProposalOf<T>>::take(&proposal) {
-						let ok = p.dispatch(Origin::Members(threshold).into()).is_ok();
+						let origin = RawOrigin::Members(voting.threshold, seats).into();
+						let ok = p.dispatch(origin).is_ok();
 						Self::deposit_event(RawEvent::Executed(proposal, ok));
 					}
 				} else {
@@ -153,22 +209,6 @@ decl_module! {
 	}
 }
 
-decl_storage! {
-	trait Store for Module<T: Trait> as CouncilMotions {
-		/// The (hashes of) the active proposals.
-		pub Proposals get(proposals): Vec<T::Hash>;
-		/// Actual proposal for a given hash, if it's current.
-		pub ProposalOf get(proposal_of): map T::Hash => Option< <T as Trait>::Proposal >;
-		/// Votes for a given proposal: (required_yes_votes, yes_voters, no_voters).
-		pub Voting get(voting): map T::Hash => Option<(ProposalIndex, u32, Vec<T::AccountId>, Vec<T::AccountId>)>;
-		/// Proposals so far.
-		pub ProposalCount get(proposal_count): u32;
-	}
-	add_extra_genesis {
-		build(|_, _, _| {});
-	}
-}
-
 impl<T: Trait> Module<T> {
 	pub fn is_councillor(who: &T::AccountId) -> bool {
 		<Council<T>>::active_council().iter()
@@ -176,24 +216,101 @@ impl<T: Trait> Module<T> {
 	}
 }
 
-/// Ensure that the origin `o` represents at least `n` council members. Returns
-/// `Ok` or an `Err` otherwise.
-pub fn ensure_council_members<OuterOrigin>(o: OuterOrigin, n: u32) -> result::Result<u32, &'static str>
-	where OuterOrigin: Into<Option<Origin>>
+impl<T: Trait> OnMembersChanged<T::AccountId> for Module<T> {
+	fn on_members_changed(_new: &[T::AccountId], old: &[T::AccountId]) {
+		// remove accounts from all current voting in motions.
+		let mut old = old.to_vec();
+		old.sort_unstable();
+		for h in Self::proposals().into_iter() {
+			<Voting<T>>::mutate(h, |v|
+				if let Some(mut votes) = v.take() {
+					votes.ayes = votes.ayes.into_iter()
+						.filter(|i| old.binary_search(i).is_err())
+						.collect();
+					votes.nays = votes.nays.into_iter()
+						.filter(|i| old.binary_search(i).is_err())
+						.collect();
+					*v = Some(votes);
+				}
+			);
+		}
+	}
+}
+
+/// Ensure that the origin `o` represents at least `n` council members. Returns `Ok` or an `Err`
+/// otherwise.
+pub fn ensure_council_members<OuterOrigin, AccountId>(o: OuterOrigin, n: MemberCount)
+	-> result::Result<MemberCount, &'static str>
+	where OuterOrigin: Into<result::Result<RawOrigin<AccountId>, OuterOrigin>>
 {
 	match o.into() {
-		Some(Origin::Members(x)) if x >= n => Ok(n),
+		Ok(RawOrigin::Members(x, _)) if x >= n => Ok(n),
 		_ => Err("bad origin: expected to be a threshold number of council members"),
 	}
 }
 
-pub struct EnsureMembers<N: U32>(::rstd::marker::PhantomData<N>);
-impl<O, N: U32> EnsureOrigin<O> for EnsureMembers<N>
-	where O: Into<Option<Origin>>
-{
-	type Success = u32;
-	fn ensure_origin(o: O) -> result::Result<Self::Success, &'static str> {
-		ensure_council_members(o, N::VALUE)
+pub struct EnsureMember<AccountId>(::rstd::marker::PhantomData<AccountId>);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	AccountId
+> EnsureOrigin<O> for EnsureMember<AccountId> {
+	type Success = AccountId;
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Member(id) => Ok(id),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureMembers<N: U32, AccountId>(::rstd::marker::PhantomData<(N, AccountId)>);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	N: U32,
+	AccountId,
+> EnsureOrigin<O> for EnsureMembers<N, AccountId> {
+	type Success = (MemberCount, MemberCount);
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Members(n, m) if n >= N::VALUE => Ok((n, m)),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureProportionMoreThan<N: U32, D: U32, AccountId>(
+	::rstd::marker::PhantomData<(N, D, AccountId)>
+);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	N: U32,
+	D: U32,
+	AccountId,
+> EnsureOrigin<O> for EnsureProportionMoreThan<N, D, AccountId> {
+	type Success = ();
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Members(n, m) if n * D::VALUE > N::VALUE * m => Ok(()),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureProportionAtLeast<N: U32, D: U32, AccountId>(
+	::rstd::marker::PhantomData<(N, D, AccountId)>
+);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	N: U32,
+	D: U32,
+	AccountId,
+> EnsureOrigin<O> for EnsureProportionAtLeast<N, D, AccountId> {
+	type Success = ();
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Members(n, m) if n * D::VALUE >= N::VALUE * m => Ok(()),
+			r => Err(O::from(r)),
+		})
 	}
 }
 
@@ -203,12 +320,13 @@ mod tests {
 	use super::RawEvent;
 	use crate::tests::*;
 	use crate::tests::{Call, Origin, Event as OuterEvent};
+	use primitives::traits::BlakeTwo256;
 	use srml_support::{Hashable, assert_ok, assert_noop};
 	use system::{EventRecord, Phase};
 	use hex_literal::hex;
 
 	#[test]
-	fn motions_basic_environment_works() {
+	fn basic_environment_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			assert_eq!(Balances::free_balance(&42), 0);
@@ -221,7 +339,41 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_propose_works() {
+	fn removal_of_old_voters_votes_works() {
+		with_externalities(&mut new_test_ext(true), || {
+			System::set_block_number(1);
+			let proposal = set_balance_proposal(42);
+			let hash = BlakeTwo256::hash_of(&proposal);
+			assert_ok!(CouncilMotions::propose(Origin::signed(1), 3, Box::new(proposal.clone())));
+			assert_ok!(CouncilMotions::vote(Origin::signed(2), hash.clone(), 0, true));
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![] })
+			);
+			CouncilMotions::on_members_changed(&[], &[1]);
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![] })
+			);
+
+			let proposal = set_balance_proposal(69);
+			let hash = BlakeTwo256::hash_of(&proposal);
+			assert_ok!(CouncilMotions::propose(Origin::signed(2), 2, Box::new(proposal.clone())));
+			assert_ok!(CouncilMotions::vote(Origin::signed(3), hash.clone(), 1, false));
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3] })
+			);
+			CouncilMotions::on_members_changed(&[], &[3]);
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![] })
+			);
+		});
+	}
+
+	#[test]
+	fn propose_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			let proposal = set_balance_proposal(42);
@@ -229,7 +381,10 @@ mod tests {
 			assert_ok!(CouncilMotions::propose(Origin::signed(1), 3, Box::new(proposal.clone())));
 			assert_eq!(CouncilMotions::proposals(), vec![hash]);
 			assert_eq!(CouncilMotions::proposal_of(&hash), Some(proposal));
-			assert_eq!(CouncilMotions::voting(&hash), Some((0, 3, vec![1], Vec::<u64>::new())));
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![] })
+			);
 
 			assert_eq!(System::events(), vec![
 				EventRecord {
@@ -242,7 +397,7 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_ignoring_non_council_proposals_works() {
+	fn ignoring_non_council_proposals_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			let proposal = set_balance_proposal(42);
@@ -251,7 +406,7 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_ignoring_non_council_votes_works() {
+	fn ignoring_non_council_votes_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			let proposal = set_balance_proposal(42);
@@ -262,7 +417,7 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_ignoring_bad_index_council_vote_works() {
+	fn ignoring_bad_index_council_vote_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(3);
 			let proposal = set_balance_proposal(42);
@@ -273,16 +428,22 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_revoting_works() {
+	fn revoting_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			let proposal = set_balance_proposal(42);
 			let hash: H256 = proposal.blake2_256().into();
 			assert_ok!(CouncilMotions::propose(Origin::signed(1), 2, Box::new(proposal.clone())));
-			assert_eq!(CouncilMotions::voting(&hash), Some((0, 2, vec![1], Vec::<u64>::new())));
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![] })
+			);
 			assert_noop!(CouncilMotions::vote(Origin::signed(1), hash.clone(), 0, true), "duplicate vote ignored");
 			assert_ok!(CouncilMotions::vote(Origin::signed(1), hash.clone(), 0, false));
-			assert_eq!(CouncilMotions::voting(&hash), Some((0, 2, Vec::<u64>::new(), vec![1])));
+			assert_eq!(
+				CouncilMotions::voting(&hash),
+				Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1] })
+			);
 			assert_noop!(CouncilMotions::vote(Origin::signed(1), hash.clone(), 0, false), "duplicate vote ignored");
 
 			assert_eq!(System::events(), vec![
@@ -301,7 +462,7 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_disapproval_works() {
+	fn disapproval_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			let proposal = set_balance_proposal(42);
@@ -330,7 +491,7 @@ mod tests {
 	}
 
 	#[test]
-	fn motions_approval_works() {
+	fn approval_works() {
 		with_externalities(&mut new_test_ext(true), || {
 			System::set_block_number(1);
 			let proposal = set_balance_proposal(42);

substrate/srml/council/src/seats.rs

@@ -25,6 +25,7 @@ use srml_support::{
 };
 use democracy;
 use system::{self, ensure_signed};
+use super::OnMembersChanged;
 
 // no polynomial attacks:
 //
@@ -95,6 +96,9 @@ pub trait Trait: democracy::Trait {
 
 	/// Handler for the unbalanced reduction when slashing an invalid reaping attempt.
 	type BadReaper: OnUnbalanced<NegativeImbalanceOf<Self>>;
+
+	/// What to do when the members change.
+	type OnMembersChanged: OnMembersChanged<Self::AccountId>;
 }
 
 decl_module! {
@@ -269,15 +273,16 @@ decl_module! {
 		}
 
 		/// Set the desired member count; if lower than the current count, then seats will not be up
-		/// election when they expire. If more, then a new vote will be started if one is not already
-		/// in progress.
+		/// election when they expire. If more, then a new vote will be started if one is not
+		/// already in progress.
 		fn set_desired_seats(#[compact] count: u32) {
 			<DesiredSeats<T>>::put(count);
 		}
 
-		/// Remove a particular member. A tally will happen instantly (if not already in a presentation
+		/// Remove a particular member from the council. This is effective immediately.
+		///
+		/// Note: A tally should happen instantly (if not already in a presentation
 		/// period) to fill the seat if removal means that the desired members are not met.
-		/// This is effective immediately.
 		fn remove_member(who: <T::Lookup as StaticLookup>::Source) {
 			let who = T::Lookup::lookup(who)?;
 			let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council()
@@ -285,6 +290,7 @@ decl_module! {
 				.filter(|i| i.0 != who)
 				.collect();
 			<ActiveCouncil<T>>::put(new_council);
+			T::OnMembersChanged::on_members_changed(&[], &[who]);
 		}
 
 		/// Set the presentation duration. If there is currently a vote being presented for, will
@@ -392,6 +398,14 @@ impl<T: Trait> Module<T> {
 		<RegisterInfoOf<T>>::exists(who)
 	}
 
+	/// Iff the councillor `who` still has a seat at blocknumber `n` returns `true`.
+	pub fn will_still_be_councillor_at(who: &T::AccountId, n: T::BlockNumber) -> bool {
+		Self::active_council().iter()
+			.find(|&&(ref a, _)| a == who)
+			.map(|&(_, expires)| expires > n)
+			.unwrap_or(false)
+	}
+
 	/// Determine the block that a vote can happen on which is no less than `n`.
 	pub fn next_vote_from(n: T::BlockNumber) -> T::BlockNumber {
 		let voting_period = Self::voting_period();
@@ -514,7 +528,7 @@ impl<T: Trait> Module<T> {
 
 		// return bond to winners.
 		let candidacy_bond = Self::candidacy_bond();
-		let incoming: Vec<T::AccountId> = leaderboard.iter()
+		let incoming: Vec<_> = leaderboard.iter()
 			.rev()
 			.take_while(|&&(b, _)| !b.is_zero())
 			.take(coming as usize)
@@ -523,7 +537,9 @@ impl<T: Trait> Module<T> {
 			.inspect(|a| {T::Currency::unreserve(a, candidacy_bond);})
 			.collect();
 		let active_council = Self::active_council();
-		let outgoing = active_council.iter().take(expiring.len()).map(|a| a.0.clone()).collect();
+		let outgoing: Vec<_> = active_council.iter()
+			.take(expiring.len())
+			.map(|a| a.0.clone()).collect();
 
 		// set the new council.
 		let mut new_council: Vec<_> = active_council
@@ -534,6 +550,8 @@ impl<T: Trait> Module<T> {
 		new_council.sort_by_key(|&(_, expiry)| expiry);
 		<ActiveCouncil<T>>::put(new_council);
 
+		T::OnMembersChanged::on_members_changed(&incoming, &outgoing);
+
 		// clear all except runners-up from candidate list.
 		let candidates = Self::candidates();
 		let mut new_candidates = vec![T::AccountId::default(); candidates.len()];	// shrink later.

substrate/srml/council/src/voting.rs

@@ -1,494 +0,0 @@
-// Copyright 2017-2019 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/>.
-
-//! Council voting system.
-
-use rstd::prelude::*;
-use rstd::borrow::Borrow;
-use primitives::traits::{Hash, Zero};
-use runtime_io::print;
-use srml_support::dispatch::Result;
-use srml_support::{StorageValue, StorageMap, IsSubType, decl_module, decl_storage, decl_event, ensure};
-use {system, democracy};
-use super::{Trait as CouncilTrait, Module as Council};
-use system::ensure_signed;
-
-pub trait Trait: CouncilTrait {
-	type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
-}
-
-decl_module! {
-	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn deposit_event<T>() = default;
-
-		fn propose(origin, proposal: Box<T::Proposal>) {
-			let who = ensure_signed(origin)?;
-
-			let expiry = <system::Module<T>>::block_number() + Self::voting_period();
-			ensure!(Self::will_still_be_councillor_at(&who, expiry), "proposer would not be on council");
-
-			let proposal_hash = T::Hashing::hash_of(&proposal);
-
-			ensure!(!<ProposalOf<T>>::exists(proposal_hash), "duplicate proposals not allowed");
-			ensure!(!Self::is_vetoed(&proposal_hash), "proposal is vetoed");
-
-			let mut proposals = Self::proposals();
-			proposals.push((expiry, proposal_hash));
-			proposals.sort_by_key(|&(expiry, _)| expiry);
-			Self::set_proposals(&proposals);
-
-			<ProposalOf<T>>::insert(proposal_hash, *proposal);
-			<ProposalVoters<T>>::insert(proposal_hash, vec![who.clone()]);
-			<CouncilVoteOf<T>>::insert((proposal_hash, who.clone()), true);
-		}
-
-		fn vote(origin, proposal: T::Hash, approve: bool) {
-			let who = ensure_signed(origin)?;
-
-			ensure!(Self::is_councillor(&who), "only councillors may vote on council proposals");
-
-			if Self::vote_of((proposal, who.clone())).is_none() {
-				<ProposalVoters<T>>::mutate(proposal, |voters| voters.push(who.clone()));
-			}
-			<CouncilVoteOf<T>>::insert((proposal, who), approve);
-		}
-
-		fn veto(origin, proposal_hash: T::Hash) {
-			let who = ensure_signed(origin)?;
-
-			ensure!(Self::is_councillor(&who), "only councillors may veto council proposals");
-			ensure!(<ProposalVoters<T>>::exists(&proposal_hash), "proposal must exist to be vetoed");
-
-			let mut existing_vetoers = Self::veto_of(&proposal_hash)
-				.map(|pair| pair.1)
-				.unwrap_or_else(Vec::new);
-			let insert_position = existing_vetoers.binary_search(&who)
-				.err().ok_or("a councillor may not veto a proposal twice")?;
-			existing_vetoers.insert(insert_position, who);
-			Self::set_veto_of(
-				&proposal_hash,
-				<system::Module<T>>::block_number() + Self::cooloff_period(),
-				existing_vetoers
-			);
-
-			Self::set_proposals(
-				&Self::proposals().into_iter().filter(|&(_, h)| h != proposal_hash
-			).collect::<Vec<_>>());
-			<ProposalVoters<T>>::remove(proposal_hash);
-			<ProposalOf<T>>::remove(proposal_hash);
-			for (c, _) in <Council<T>>::active_council() {
-				<CouncilVoteOf<T>>::remove((proposal_hash, c));
-			}
-		}
-
-		fn set_cooloff_period(#[compact] blocks: T::BlockNumber) {
-			<CooloffPeriod<T>>::put(blocks);
-		}
-
-		fn set_voting_period(#[compact] blocks: T::BlockNumber) {
-			<VotingPeriod<T>>::put(blocks);
-		}
-
-		fn on_finalize(n: T::BlockNumber) {
-			if let Err(e) = Self::end_block(n) {
-				print("Guru meditation");
-				print(e);
-			}
-		}
-	}
-}
-
-decl_storage! {
-	trait Store for Module<T: Trait> as CouncilVoting {
-		pub CooloffPeriod get(cooloff_period) config(): T::BlockNumber = 1000.into();
-		pub VotingPeriod get(voting_period) config(): T::BlockNumber = 3.into();
-		/// Number of blocks by which to delay enactment of successful, non-unanimous-council-instigated referendum proposals.
-		pub EnactDelayPeriod get(enact_delay_period) config(): T::BlockNumber = 0.into();
-		pub Proposals get(proposals) build(|_| vec![]): Vec<(T::BlockNumber, T::Hash)>; // ordered by expiry.
-		pub ProposalOf get(proposal_of): map T::Hash => Option<T::Proposal>;
-		pub ProposalVoters get(proposal_voters): map T::Hash => Vec<T::AccountId>;
-		pub CouncilVoteOf get(vote_of): map (T::Hash, T::AccountId) => Option<bool>;
-		pub VetoedProposal get(veto_of): map T::Hash => Option<(T::BlockNumber, Vec<T::AccountId>)>;
-	}
-}
-
-decl_event!(
-	pub enum Event<T> where <T as system::Trait>::Hash {
-		/// A voting tally has happened for a referendum cancellation vote.
-		/// Last three are yes, no, abstain counts.
-		TallyCancelation(Hash, u32, u32, u32),
-		/// A voting tally has happened for a referendum vote.
-		/// Last three are yes, no, abstain counts.
-		TallyReferendum(Hash, u32, u32, u32),
-	}
-);
-
-impl<T: Trait> Module<T> {
-	pub fn is_vetoed<B: Borrow<T::Hash>>(proposal: B) -> bool {
-		Self::veto_of(proposal.borrow())
-			.map(|(expiry, _): (T::BlockNumber, Vec<T::AccountId>)| <system::Module<T>>::block_number() < expiry)
-			.unwrap_or(false)
-	}
-
-	pub fn will_still_be_councillor_at(who: &T::AccountId, n: T::BlockNumber) -> bool {
-		<Council<T>>::active_council().iter()
-			.find(|&&(ref a, _)| a == who)
-			.map(|&(_, expires)| expires > n)
-			.unwrap_or(false)
-	}
-
-	pub fn is_councillor(who: &T::AccountId) -> bool {
-		<Council<T>>::active_council().iter()
-			.any(|&(ref a, _)| a == who)
-	}
-
-	pub fn tally(proposal_hash: &T::Hash) -> (u32, u32, u32) {
-		Self::generic_tally(proposal_hash, |w: &T::AccountId, p: &T::Hash| Self::vote_of((*p, w.clone())))
-	}
-
-	// Private
-	fn set_veto_of(proposal: &T::Hash, expiry: T::BlockNumber, vetoers: Vec<T::AccountId>) {
-		<VetoedProposal<T>>::insert(proposal, (expiry, vetoers));
-	}
-
-	fn kill_veto_of(proposal: &T::Hash) {
-		<VetoedProposal<T>>::remove(proposal);
-	}
-
-	fn take_tally(proposal_hash: &T::Hash) -> (u32, u32, u32) {
-		Self::generic_tally(proposal_hash, |w: &T::AccountId, p: &T::Hash| <CouncilVoteOf<T>>::take((*p, w.clone())))
-	}
-
-	fn generic_tally<F: Fn(&T::AccountId, &T::Hash) -> Option<bool>>(proposal_hash: &T::Hash, vote_of: F) -> (u32, u32, u32) {
-		let c = <Council<T>>::active_council();
-		let (approve, reject) = c.iter()
-			.filter_map(|&(ref a, _)| vote_of(a, proposal_hash))
-			.map(|approve| if approve { (1, 0) } else { (0, 1) })
-			.fold((0, 0), |(a, b), (c, d)| (a + c, b + d));
-		(approve, reject, c.len() as u32 - approve - reject)
-	}
-
-	fn set_proposals(p: &Vec<(T::BlockNumber, T::Hash)>) {
-		<Proposals<T>>::put(p);
-	}
-
-	fn take_proposal_if_expiring_at(n: T::BlockNumber) -> Option<(T::Proposal, T::Hash)> {
-		let proposals = Self::proposals();
-		match proposals.first() {
-			Some(&(expiry, hash)) if expiry == n => {
-				// yes this is horrible, but fixing it will need substantial work in storage.
-				Self::set_proposals(&proposals[1..].to_vec());
-				<ProposalOf<T>>::take(hash).map(|p| (p, hash))	/* defensive only: all queued proposal hashes must have associated proposals*/
-			}
-			_ => None,
-		}
-	}
-
-	fn end_block(now: T::BlockNumber) -> Result {
-		while let Some((proposal, proposal_hash)) = Self::take_proposal_if_expiring_at(now) {
-			let tally = Self::take_tally(&proposal_hash);
-			if let Some(&democracy::Call::cancel_referendum(ref_index)) = IsSubType::<democracy::Module<T>>::is_aux_sub_type(&proposal) {
-				Self::deposit_event(RawEvent::TallyCancelation(proposal_hash, tally.0, tally.1, tally.2));
-				if let (_, 0, 0) = tally {
-					<democracy::Module<T>>::internal_cancel_referendum(ref_index.into());
-				}
-			} else {
-				Self::deposit_event(RawEvent::TallyReferendum(proposal_hash.clone(), tally.0, tally.1, tally.2));
-				if tally.0 > tally.1 + tally.2 {
-					Self::kill_veto_of(&proposal_hash);
-					// If there were no nay-votes from the council, then it's weakly uncontroversial; we enact immediately.
-					let period = match tally.1 {
-						0 => Zero::zero(),
-						_ => Self::enact_delay_period(),
-					};
-					// If all council members voted yes, then it's strongly uncontroversial; we require a negative
-					// super-majority at referendum in order to defeat it.
-					let threshold = match tally {
-						(_, 0, 0) => democracy::VoteThreshold::SuperMajorityAgainst,
-						_ => democracy::VoteThreshold::SimpleMajority,
-					};
-					<democracy::Module<T>>::internal_start_referendum(proposal, threshold, period).map(|_| ())?;
-				}
-			}
-		}
-		Ok(())
-	}
-}
-
-#[cfg(test)]
-mod tests {
-	use super::*;
-	use crate::tests::*;
-	use crate::tests::{Call, Origin};
-	use srml_support::{Hashable, assert_ok, assert_noop};
-	use democracy::{ReferendumInfo, VoteThreshold};
-
-	#[test]
-	fn basic_environment_works() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			assert_eq!(Balances::free_balance(&42), 0);
-			assert_eq!(CouncilVoting::cooloff_period(), 2);
-			assert_eq!(CouncilVoting::voting_period(), 1);
-			assert_eq!(CouncilVoting::will_still_be_councillor_at(&1, 1), true);
-			assert_eq!(CouncilVoting::will_still_be_councillor_at(&1, 10), false);
-			assert_eq!(CouncilVoting::will_still_be_councillor_at(&4, 10), false);
-			assert_eq!(CouncilVoting::is_councillor(&1), true);
-			assert_eq!(CouncilVoting::is_councillor(&4), false);
-			assert_eq!(CouncilVoting::proposals(), Vec::<(u64, H256)>::new());
-			assert_eq!(CouncilVoting::proposal_voters(H256::default()), Vec::<u64>::new());
-			assert_eq!(CouncilVoting::is_vetoed(&H256::default()), false);
-			assert_eq!(CouncilVoting::vote_of((H256::default(), 1)), None);
-			assert_eq!(CouncilVoting::tally(&H256::default()), (0, 0, 3));
-		});
-	}
-
-	fn set_balance_proposal(value: u64) -> Call {
-		Call::Balances(balances::Call::set_balance(42, value.into(), 0))
-	}
-
-	fn cancel_referendum_proposal(id: u32) -> Call {
-		Call::Democracy(democracy::Call::cancel_referendum(id.into()))
-	}
-
-	#[test]
-	fn referendum_cancellation_should_work_when_unanimous() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove, 0), 0);
-			assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(4, proposal, VoteThreshold::SuperMajorityApprove, 0))]);
-
-			let cancellation = cancel_referendum_proposal(0);
-			let hash = cancellation.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(cancellation)));
-			assert_ok!(CouncilVoting::vote(Origin::signed(2), hash, true));
-			assert_ok!(CouncilVoting::vote(Origin::signed(3), hash, true));
-			assert_eq!(CouncilVoting::proposals(), vec![(2, hash)]);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(2);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(Democracy::active_referenda(), vec![]);
-			assert_eq!(Balances::free_balance(&42), 0);
-		});
-	}
-
-	#[test]
-	fn referendum_cancellation_should_fail_when_not_unanimous() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove, 0), 0);
-
-			let cancellation = cancel_referendum_proposal(0);
-			let hash = cancellation.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(cancellation)));
-			assert_ok!(CouncilVoting::vote(Origin::signed(2), hash, true));
-			assert_ok!(CouncilVoting::vote(Origin::signed(3), hash, false));
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(2);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(4, proposal, VoteThreshold::SuperMajorityApprove, 0))]);
-		});
-	}
-
-	#[test]
-	fn referendum_cancellation_should_fail_when_abstentions() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove, 0), 0);
-
-			let cancellation = cancel_referendum_proposal(0);
-			let hash = cancellation.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(cancellation)));
-			assert_ok!(CouncilVoting::vote(Origin::signed(2), hash, true));
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(2);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(4, proposal, VoteThreshold::SuperMajorityApprove, 0))]);
-		});
-	}
-
-	#[test]
-	fn veto_should_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			let hash = proposal.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::veto(Origin::signed(2), hash));
-			assert_eq!(CouncilVoting::proposals().len(), 0);
-			assert_eq!(Democracy::active_referenda().len(), 0);
-		});
-	}
-
-	#[test]
-	fn double_veto_should_not_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			let hash = proposal.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::veto(Origin::signed(2), hash));
-
-			System::set_block_number(3);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_noop!(CouncilVoting::veto(Origin::signed(2), hash), "a councillor may not veto a proposal twice");
-		});
-	}
-
-	#[test]
-	fn retry_in_cooloff_should_not_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			let hash = proposal.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::veto(Origin::signed(2), hash));
-
-			System::set_block_number(2);
-			assert_noop!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())), "proposal is vetoed");
-		});
-	}
-
-	#[test]
-	fn retry_after_cooloff_should_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			let hash = proposal.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::veto(Origin::signed(2), hash));
-
-			System::set_block_number(3);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::vote(Origin::signed(2), hash, false));
-			assert_ok!(CouncilVoting::vote(Origin::signed(3), hash, true));
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(4);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(CouncilVoting::proposals().len(), 0);
-			assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(7, set_balance_proposal(42), VoteThreshold::SimpleMajority, 0))]);
-		});
-	}
-
-	#[test]
-	fn alternative_double_veto_should_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			let hash = proposal.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::veto(Origin::signed(2), hash));
-
-			System::set_block_number(3);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::veto(Origin::signed(3), hash));
-			assert_eq!(CouncilVoting::proposals().len(), 0);
-			assert_eq!(Democracy::active_referenda().len(), 0);
-		});
-	}
-
-	#[test]
-	fn simple_propose_should_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			let hash = proposal.blake2_256().into();
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_eq!(CouncilVoting::proposals().len(), 1);
-			assert_eq!(CouncilVoting::proposal_voters(&hash), vec![1]);
-			assert_eq!(CouncilVoting::vote_of((hash, 1)), Some(true));
-			assert_eq!(CouncilVoting::tally(&hash), (1, 0, 2));
-		});
-	}
-
-	#[test]
-	fn unvoted_proposal_should_expire_without_action() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_eq!(CouncilVoting::tally(&proposal.blake2_256().into()), (1, 0, 2));
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(2);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(CouncilVoting::proposals().len(), 0);
-			assert_eq!(Democracy::active_referenda().len(), 0);
-		});
-	}
-
-	#[test]
-	fn unanimous_proposal_should_expire_with_biased_referendum() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::vote(Origin::signed(2), proposal.blake2_256().into(), true));
-			assert_ok!(CouncilVoting::vote(Origin::signed(3), proposal.blake2_256().into(), true));
-			assert_eq!(CouncilVoting::tally(&proposal.blake2_256().into()), (3, 0, 0));
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(2);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(CouncilVoting::proposals().len(), 0);
-			assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(5, proposal, VoteThreshold::SuperMajorityAgainst, 0))]);
-		});
-	}
-
-	#[test]
-	fn majority_proposal_should_expire_with_unbiased_referendum() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_ok!(CouncilVoting::vote(Origin::signed(2), proposal.blake2_256().into(), true));
-			assert_ok!(CouncilVoting::vote(Origin::signed(3), proposal.blake2_256().into(), false));
-			assert_eq!(CouncilVoting::tally(&proposal.blake2_256().into()), (2, 1, 0));
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-
-			System::set_block_number(2);
-			assert_ok!(CouncilVoting::end_block(System::block_number()));
-			assert_eq!(CouncilVoting::proposals().len(), 0);
-			assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(5, proposal, VoteThreshold::SimpleMajority, 0))]);
-		});
-	}
-
-	#[test]
-	fn propose_by_public_should_not_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_noop!(CouncilVoting::propose(Origin::signed(4), Box::new(proposal)), "proposer would not be on council");
-		});
-	}
-
-	#[test]
-	fn vote_by_public_should_not_work() {
-		with_externalities(&mut new_test_ext(true), || {
-			System::set_block_number(1);
-			let proposal = set_balance_proposal(42);
-			assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone())));
-			assert_noop!(CouncilVoting::vote(Origin::signed(4), proposal.blake2_256().into(), true), "only councillors may vote on council proposals");
-		});
-	}
-}

substrate/srml/support/src/metadata.rs

@@ -246,7 +246,8 @@ mod tests {
 
 	mod system {
 		pub trait Trait {
-			type Origin: Into<Option<RawOrigin<Self::AccountId>>> + From<RawOrigin<Self::AccountId>>;
+			type Origin: Into<Result<RawOrigin<Self::AccountId>, Self::Origin>>
+				+ From<RawOrigin<Self::AccountId>>;
 			type AccountId;
 			type BlockNumber;
 		}

substrate/srml/support/src/origin.rs

@@ -112,12 +112,12 @@ macro_rules! impl_outer_origin {
 				$name::system(x)
 			}
 		}
-		impl Into<Option<$system::Origin<$runtime>>> for $name {
-			fn into(self) -> Option<$system::Origin<$runtime>> {
+		impl Into<$crate::rstd::result::Result<$system::Origin<$runtime>, $name>> for $name {
+			fn into(self) -> $crate::rstd::result::Result<$system::Origin<$runtime>, Self> {
 				if let $name::system(l) = self {
-					Some(l)
+					Ok(l)
 				} else {
-					None
+					Err(self)
 				}
 			}
 		}
@@ -132,12 +132,18 @@ macro_rules! impl_outer_origin {
 					$name::$module(x)
 				}
 			}
-			impl Into<Option<$module::Origin $( <$generic_param $(, $generic_instance )? > )*>> for $name {
-				fn into(self) -> Option<$module::Origin $( <$generic_param $(, $generic_instance )? > )*> {
+			impl Into<$crate::rstd::result::Result<
+				$module::Origin $( <$generic_param $(, $generic_instance )? > )*,
+				$name
+			>> for $name {
+				fn into(self) -> $crate::rstd::result::Result<
+					$module::Origin $( <$generic_param $(, $generic_instance )? > )*,
+					Self
+				> {
 					if let $name::$module(l) = self {
-						Some(l)
+						Ok(l)
 					} else {
-						None
+						Err(self)
 					}
 				}
 			}

substrate/srml/support/src/traits.rs

@@ -26,12 +26,26 @@ use crate::runtime_primitives::traits::{
 
 use super::for_each_tuple;
 
-/// New trait for querying a single fixed value from a type.
+/// A trait for querying a single fixed value from a type.
 pub trait Get<T> {
 	/// Return a constant value.
 	fn get() -> T;
 }
 
+/// A trait for querying whether a type can be said to statically "contain" a value. Similar
+/// in nature to `Get`, except it is designed to be lazy rather than active (you can't ask it to
+/// enumerate all values that it contains) and work for multiple values rather than just one.
+pub trait Contains<T> {
+	/// Return `true` if this "contains" the given value `t`.
+	fn contains(t: &T) -> bool;
+}
+
+impl<V: PartialEq, T: Get<V>> Contains<V> for T {
+	fn contains(t: &V) -> bool {
+		&Self::get() == t
+	}
+}
+
 /// The account with the given id was killed.
 pub trait OnFreeBalanceZero<AccountId> {
 	/// The account was the given id was killed.

substrate/srml/support/test/tests/instance.rs

@@ -39,7 +39,8 @@ mod system {
 	use super::*;
 
 	pub trait Trait: 'static + Eq + Clone {
-		type Origin: Into<Option<RawOrigin<Self::AccountId>>> + From<RawOrigin<Self::AccountId>>;
+		type Origin: Into<Result<RawOrigin<Self::AccountId>, Self::Origin>>
+			+ From<RawOrigin<Self::AccountId>>;
 		type BlockNumber;
 		type Digest: Digest<Hash = H256>;
 		type Hash;
@@ -100,12 +101,9 @@ mod system {
 	}
 
 	pub fn ensure_root<OuterOrigin, AccountId>(o: OuterOrigin) -> Result<(), &'static str>
-		where OuterOrigin: Into<Option<RawOrigin<AccountId>>>
+		where OuterOrigin: Into<Result<RawOrigin<AccountId>, OuterOrigin>>
 	{
-		match o.into() {
-			Some(RawOrigin::Root) => Ok(()),
-			_ => Err("bad origin: expected to be a root origin"),
-		}
+		o.into().map(|_| ()).map_err(|_| "bad origin: expected to be a root origin")
 	}
 }
 

substrate/srml/system/src/lib.rs

@@ -78,14 +78,14 @@ use rstd::prelude::*;
 use rstd::map;
 use primitives::traits::{self, CheckEqual, SimpleArithmetic, SimpleBitOps, One, Bounded, Lookup,
 	Hash, Member, MaybeDisplay, EnsureOrigin, Digest as DigestT, CurrentHeight, BlockNumberToHash,
-	MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup,
+	MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup
 };
 #[cfg(any(feature = "std", test))]
 use primitives::traits::Zero;
 use substrate_primitives::storage::well_known_keys;
 use srml_support::{
 	storage, decl_module, decl_event, decl_storage, StorageDoubleMap, StorageValue,
-	StorageMap, Parameter, for_each_tuple,
+	StorageMap, Parameter, for_each_tuple, traits::Contains
 };
 use safe_mix::TripletMix;
 use parity_codec::{Encode, Decode};
@@ -145,7 +145,7 @@ pub fn extrinsics_data_root<H: Hash>(xts: Vec<Vec<u8>>) -> H::Output {
 
 pub trait Trait: 'static + Eq + Clone {
 	/// The aggregated `Origin` type used by dispatchable calls.
-	type Origin: Into<Option<RawOrigin<Self::AccountId>>> + From<RawOrigin<Self::AccountId>>;
+	type Origin: Into<Result<RawOrigin<Self::AccountId>, Self::Origin>> + From<RawOrigin<Self::AccountId>>;
 
 	/// Account index (aka nonce) type. This stores the number of previous transactions associated with a sender
 	/// account.
@@ -376,40 +376,97 @@ decl_storage! {
 }
 
 pub struct EnsureRoot<AccountId>(::rstd::marker::PhantomData<AccountId>);
-impl<O: Into<Option<RawOrigin<AccountId>>>, AccountId> EnsureOrigin<O> for EnsureRoot<AccountId> {
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	AccountId,
+> EnsureOrigin<O> for EnsureRoot<AccountId> {
 	type Success = ();
-	fn ensure_origin(o: O) -> Result<Self::Success, &'static str> {
-		ensure_root(o)
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Root => Ok(()),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureSigned<AccountId>(::rstd::marker::PhantomData<AccountId>);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	AccountId,
+> EnsureOrigin<O> for EnsureSigned<AccountId> {
+	type Success = AccountId;
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Signed(who) => Ok(who),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureSignedBy<Who, AccountId>(::rstd::marker::PhantomData<(Who, AccountId)>);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	Who: Contains<AccountId>,
+	AccountId: PartialEq + Clone,
+> EnsureOrigin<O> for EnsureSignedBy<Who, AccountId> {
+	type Success = AccountId;
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::Signed(ref who) if Who::contains(who) => Ok(who.clone()),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureNone<AccountId>(::rstd::marker::PhantomData<AccountId>);
+impl<
+	O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>,
+	AccountId,
+> EnsureOrigin<O> for EnsureNone<AccountId> {
+	type Success = ();
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		o.into().and_then(|o| match o {
+			RawOrigin::None => Ok(()),
+			r => Err(O::from(r)),
+		})
+	}
+}
+
+pub struct EnsureNever<T>(::rstd::marker::PhantomData<T>);
+impl<O, T> EnsureOrigin<O> for EnsureNever<T> {
+	type Success = T;
+	fn try_origin(o: O) -> Result<Self::Success, O> {
+		Err(o)
 	}
 }
 
 /// Ensure that the origin `o` represents a signed extrinsic (i.e. transaction).
 /// Returns `Ok` with the account that signed the extrinsic or an `Err` otherwise.
 pub fn ensure_signed<OuterOrigin, AccountId>(o: OuterOrigin) -> Result<AccountId, &'static str>
-	where OuterOrigin: Into<Option<RawOrigin<AccountId>>>
+	where OuterOrigin: Into<Result<RawOrigin<AccountId>, OuterOrigin>>
 {
 	match o.into() {
-		Some(RawOrigin::Signed(t)) => Ok(t),
+		Ok(RawOrigin::Signed(t)) => Ok(t),
 		_ => Err("bad origin: expected to be a signed origin"),
 	}
 }
 
 /// Ensure that the origin `o` represents the root. Returns `Ok` or an `Err` otherwise.
 pub fn ensure_root<OuterOrigin, AccountId>(o: OuterOrigin) -> Result<(), &'static str>
-	where OuterOrigin: Into<Option<RawOrigin<AccountId>>>
+	where OuterOrigin: Into<Result<RawOrigin<AccountId>, OuterOrigin>>
 {
 	match o.into() {
-		Some(RawOrigin::Root) => Ok(()),
+		Ok(RawOrigin::Root) => Ok(()),
 		_ => Err("bad origin: expected to be a root origin"),
 	}
 }
 
 /// Ensure that the origin `o` represents an unsigned extrinsic. Returns `Ok` or an `Err` otherwise.
 pub fn ensure_none<OuterOrigin, AccountId>(o: OuterOrigin) -> Result<(), &'static str>
-	where OuterOrigin: Into<Option<RawOrigin<AccountId>>>
+	where OuterOrigin: Into<Result<RawOrigin<AccountId>, OuterOrigin>>
 {
 	match o.into() {
-		Some(RawOrigin::None) => Ok(()),
+		Ok(RawOrigin::None) => Ok(()),
 		_ => Err("bad origin: expected to be no origin"),
 	}
 }
@@ -753,6 +810,13 @@ mod tests {
 		GenesisConfig::<Test>::default().build_storage().unwrap().0.into()
 	}
 
+	#[test]
+	fn origin_works() {
+		let o = Origin::from(RawOrigin::<u64>::Signed(1u64));
+		let x: Result<RawOrigin<u64>, Origin> = o.into();
+		assert_eq!(x, Ok(RawOrigin::<u64>::Signed(1u64)));
+	}
+
 	#[test]
 	fn deposit_event_should_work() {
 		with_externalities(&mut new_test_ext(), || {