Multiple improvements to the decl_module! macro (#953)

* General `decl_module` improvements

* Make `deposit_event` implementable by `decl_module!`

* Make `decl_module!` implement calls directly

* Regenerate the wasm file after master rebase

substrate/srml/assets/src/lib.rs

@@ -69,16 +69,47 @@ type AssetId = u32;
 decl_module! {
 	// Simple declaration of the `Module` type. Lets the macro know what its working on.
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		fn deposit_event() = default;
 		/// Issue a new class of fungible assets. There are, and will only ever be, `total`
 		/// such assets and they'll all belong to the `origin` initially. It will have an
 		/// identifier `AssetId` instance: this will be specified in the `Issued` event.
-		fn issue(origin, total: T::Balance) -> Result;
+		fn issue(origin, total: T::Balance) -> Result {
+			let origin = ensure_signed(origin)?;
+
+			let id = Self::next_asset_id();
+			<NextAssetId<T>>::mutate(|id| *id += 1);
+
+			<Balances<T>>::insert((id, origin.clone()), total);
+
+			Self::deposit_event(RawEvent::Issued(id, origin, total));
+			Ok(())
+		}
 
 		/// Move some assets from one holder to another.
-		fn transfer(origin, id: AssetId, target: T::AccountId, total: T::Balance) -> Result;
+		fn transfer(origin, id: AssetId, target: T::AccountId, amount: T::Balance) -> Result {
+			let origin = ensure_signed(origin)?;
+			let origin_account = (id, origin.clone());
+			let origin_balance = <Balances<T>>::get(&origin_account);
+			ensure!(origin_balance >= amount, "origin account balance must be greater than amount");
+
+			Self::deposit_event(RawEvent::Transfered(id, origin, target.clone(), amount));
+			<Balances<T>>::insert(origin_account, origin_balance - amount);
+			<Balances<T>>::mutate((id, target), |balance| *balance += amount);
+
+			Ok(())
+		}
 
 		/// Destroy any assets of `id` owned by `origin`.
-		fn destroy(origin, id: AssetId) -> Result;
+		fn destroy(origin, id: AssetId) -> Result {
+			let origin = ensure_signed(origin)?;
+
+			let balance = <Balances<T>>::take((id, origin.clone()));
+			ensure!(!balance.is_zero(), "origin balance should be non-zero");
+
+			Self::deposit_event(RawEvent::Destroyed(id, origin, balance));
+
+			Ok(())
+		}
 	}
 }
 
@@ -107,57 +138,12 @@ decl_storage! {
 
 // The main implementation block for the module.
 impl<T: Trait> Module<T> {
-	/// Deposit one of this module's events.
-	// TODO: move into `decl_module` macro.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	// Public immutables
 
 	/// Get the asset `id` balance of `who`.
 	pub fn balance(id: AssetId, who: T::AccountId) -> T::Balance {
 		<Balances<T>>::get((id, who))
 	}
-
-	// Implement Calls and add public immutables and private mutables.
-
-	fn issue(origin: T::Origin, total: T::Balance) -> Result {
-		let origin = ensure_signed(origin)?;
-
-		let id = Self::next_asset_id();
-		<NextAssetId<T>>::mutate(|id| *id += 1);
-
-
-		<Balances<T>>::insert((id, origin.clone()), total);
-
-		Self::deposit_event(RawEvent::Issued(id, origin, total));
-		Ok(())
-	}
-
-	fn transfer(origin: T::Origin, id: AssetId, target: T::AccountId, amount: T::Balance) -> Result {
-		let origin = ensure_signed(origin)?;
-		let origin_account = (id, origin.clone());
-		let origin_balance = <Balances<T>>::get(&origin_account);
-		ensure!(origin_balance >= amount, "origin account balance must be greater than amount");
-
-		Self::deposit_event(RawEvent::Transfered(id, origin, target.clone(), amount));
-		<Balances<T>>::insert(origin_account, origin_balance - amount);
-		<Balances<T>>::mutate((id, target), |balance| *balance += amount);
-
-		Ok(())
-	}
-
-	fn destroy(origin: T::Origin, id: AssetId) -> Result {
-		let origin = ensure_signed(origin)?;
-
-		let balance = <Balances<T>>::take((id, origin.clone()));
-		ensure!(!balance.is_zero(), "origin balance should be non-zero");
-
-		Self::deposit_event(RawEvent::Destroyed(id, origin, balance));
-
-		Ok(())
-	}
 }
 
 #[cfg(test)]

substrate/srml/balances/src/lib.rs

@@ -125,8 +125,64 @@ pub trait Trait: system::Trait {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn transfer(origin, dest: RawAddress<T::AccountId, T::AccountIndex>, value: <T::Balance as HasCompact>::Type) -> Result;
-		fn set_balance(who: RawAddress<T::AccountId, T::AccountIndex>, free: <T::Balance as HasCompact>::Type, reserved: <T::Balance as HasCompact>::Type) -> Result;
+		fn deposit_event() = default;
+
+		/// Transfer some liquid free balance to another staker.
+		pub fn transfer(
+			origin,
+			dest: RawAddress<T::AccountId, T::AccountIndex>,
+			value: <T::Balance as HasCompact>::Type
+		) -> Result {
+			let transactor = ensure_signed(origin)?;
+
+			let dest = Self::lookup(dest)?;
+			let value = value.into();
+			let from_balance = Self::free_balance(&transactor);
+			let to_balance = Self::free_balance(&dest);
+			let would_create = to_balance.is_zero();
+			let fee = if would_create { Self::creation_fee() } else { Self::transfer_fee() };
+			let liability = match value.checked_add(&fee) {
+				Some(l) => l,
+				None => return Err("got overflow after adding a fee to value"),
+			};
+
+			let new_from_balance = match from_balance.checked_sub(&liability) {
+				Some(b) => b,
+				None => return Err("balance too low to send value"),
+			};
+			if would_create && value < Self::existential_deposit() {
+				return Err("value too low to create account");
+			}
+			T::EnsureAccountLiquid::ensure_account_liquid(&transactor)?;
+
+			// NOTE: total stake being stored in the same type means that this could never overflow
+			// but better to be safe than sorry.
+			let new_to_balance = match to_balance.checked_add(&value) {
+				Some(b) => b,
+				None => return Err("destination balance too high to receive value"),
+			};
+
+			if transactor != dest {
+				Self::set_free_balance(&transactor, new_from_balance);
+				Self::decrease_total_stake_by(fee);
+				Self::set_free_balance_creating(&dest, new_to_balance);
+				Self::deposit_event(RawEvent::Transfer(transactor, dest, value, fee));
+			}
+
+			Ok(())
+		}
+
+		/// Set the balances of a given account.
+		fn set_balance(
+			who: RawAddress<T::AccountId, T::AccountIndex>,
+			free: <T::Balance as HasCompact>::Type,
+			reserved: <T::Balance as HasCompact>::Type
+		) -> Result {
+			let who = Self::lookup(who)?;
+			Self::set_free_balance(&who, free.into());
+			Self::set_reserved_balance(&who, reserved.into());
+			Ok(())
+		}
 	}
 }
 
@@ -232,12 +288,6 @@ pub enum UpdateBalanceOutcome {
 }
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	// PUBLIC IMMUTABLES
 
 	/// The combined balance of `who`.
@@ -285,58 +335,7 @@ impl<T: Trait> Module<T> {
 		}
 	}
 
-	// PUBLIC DISPATCH
-
-	/// Transfer some liquid free balance to another staker.
-	pub fn transfer(origin: T::Origin, dest: Address<T>, value: <T::Balance as HasCompact>::Type) -> Result {
-		let transactor = ensure_signed(origin)?;
-
-		let dest = Self::lookup(dest)?;
-		let value = value.into();
-		let from_balance = Self::free_balance(&transactor);
-		let to_balance = Self::free_balance(&dest);
-		let would_create = to_balance.is_zero();
-		let fee = if would_create { Self::creation_fee() } else { Self::transfer_fee() };
-		let liability = match value.checked_add(&fee) {
-			Some(l) => l,
-			None => return Err("got overflow after adding a fee to value"),
-		};
-
-		let new_from_balance = match from_balance.checked_sub(&liability) {
-			Some(b) => b,
-			None => return Err("balance too low to send value"),
-		};
-		if would_create && value < Self::existential_deposit() {
-			return Err("value too low to create account");
-		}
-		T::EnsureAccountLiquid::ensure_account_liquid(&transactor)?;
-
-		// NOTE: total stake being stored in the same type means that this could never overflow
-		// but better to be safe than sorry.
-		let new_to_balance = match to_balance.checked_add(&value) {
-			Some(b) => b,
-			None => return Err("destination balance too high to receive value"),
-		};
-
-		if transactor != dest {
-			Self::set_free_balance(&transactor, new_from_balance);
-			Self::decrease_total_stake_by(fee);
-			Self::set_free_balance_creating(&dest, new_to_balance);
-			Self::deposit_event(RawEvent::Transfer(transactor, dest, value, fee));
-		}
-
-		Ok(())
-	}
-
-	/// Set the balances of a given account.
-	fn set_balance(who: Address<T>, free: <T::Balance as HasCompact>::Type, reserved: <T::Balance as HasCompact>::Type) -> Result {
-		let who = Self::lookup(who)?;
-		Self::set_free_balance(&who, free.into());
-		Self::set_reserved_balance(&who, reserved.into());
-		Ok(())
-	}
-
-	// PUBLIC MUTABLES (DANGEROUS)
+	//PUBLIC MUTABLES (DANGEROUS)
 
 	/// Set the free balance of an account to some new value.
 	///

substrate/srml/consensus/src/lib.rs

@@ -143,11 +143,51 @@ decl_storage! {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn report_misbehavior(origin, report: Vec<u8>) -> Result;
-		fn note_offline(origin, offline_val_indices: Vec<u32>) -> Result;
-		fn remark(origin, remark: Vec<u8>) -> Result;
-		fn set_code(new: Vec<u8>) -> Result;
-		fn set_storage(items: Vec<KeyValue>) -> Result;
+		/// Report some misbehaviour.
+		fn report_misbehavior(origin, _report: Vec<u8>) -> Result {
+			ensure_signed(origin)?;
+			// TODO.
+			Ok(())
+		}
+
+		/// Note the previous block's validator missed their opportunity to propose a block.
+		/// This only comes in if 2/3+1 of the validators agree that no proposal was submitted.
+		/// It's only relevant for the previous block.
+		fn note_offline(origin, offline_val_indices: Vec<u32>) -> Result {
+			ensure_inherent(origin)?;
+			assert!(
+				<system::Module<T>>::extrinsic_index() == Some(T::NOTE_OFFLINE_POSITION),
+				"note_offline extrinsic must be at position {} in the block",
+				T::NOTE_OFFLINE_POSITION
+			);
+
+			for validator_index in offline_val_indices.into_iter() {
+				T::OnOfflineValidator::on_offline_validator(validator_index as usize);
+			}
+
+			Ok(())
+		}
+
+		/// Make some on-chain remark.
+		fn remark(origin, _remark: Vec<u8>) -> Result {
+			ensure_signed(origin)?;
+			Ok(())
+		}
+
+		/// Set the new code.
+		fn set_code(new: Vec<u8>) -> Result {
+			storage::unhashed::put_raw(well_known_keys::CODE, &new);
+			Ok(())
+		}
+
+		/// Set some items of storage.
+		fn set_storage(items: Vec<KeyValue>) -> Result {
+			for i in &items {
+				storage::unhashed::put_raw(&i.0, &i.1);
+			}
+			Ok(())
+		}
+
 		fn on_finalise() {
 			if let Some(original_authorities) = <OriginalAuthorities<T>>::take() {
 				let current_authorities = AuthorityStorageVec::<T::SessionKey>::items();
@@ -165,51 +205,6 @@ impl<T: Trait> Module<T> {
 		AuthorityStorageVec::<T::SessionKey>::items()
 	}
 
-	/// Set the new code.
-	fn set_code(new: Vec<u8>) -> Result {
-		storage::unhashed::put_raw(well_known_keys::CODE, &new);
-		Ok(())
-	}
-
-	/// Set some items of storage.
-	fn set_storage(items: Vec<KeyValue>) -> Result {
-		for i in &items {
-			storage::unhashed::put_raw(&i.0, &i.1);
-		}
-		Ok(())
-	}
-
-	/// Report some misbehaviour.
-	fn report_misbehavior(origin: T::Origin, _report: Vec<u8>) -> Result {
-		ensure_signed(origin)?;
-		// TODO.
-		Ok(())
-	}
-
-	/// Note the previous block's validator missed their opportunity to propose a block. This only comes in
-	/// if 2/3+1 of the validators agree that no proposal was submitted. It's only relevant
-	/// for the previous block.
-	fn note_offline(origin: T::Origin, offline_val_indices: Vec<u32>) -> Result {
-		ensure_inherent(origin)?;
-		assert!(
-			<system::Module<T>>::extrinsic_index() == Some(T::NOTE_OFFLINE_POSITION),
-			"note_offline extrinsic must be at position {} in the block",
-			T::NOTE_OFFLINE_POSITION
-		);
-
-		for validator_index in offline_val_indices.into_iter() {
-			T::OnOfflineValidator::on_offline_validator(validator_index as usize);
-		}
-
-		Ok(())
-	}
-
-	/// Make some on-chain remark.
-	fn remark(origin: T::Origin, _remark: Vec<u8>) -> Result {
-		ensure_signed(origin)?;
-		Ok(())
-	}
-
 	/// Set the current set of authorities' session keys.
 	///
 	/// Called by `next_session` only.

substrate/srml/contract/src/lib.rs

@@ -151,22 +151,104 @@ where
 decl_module! {
 	/// Contracts module.
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		fn deposit_event() = default;
 		// TODO: Change AccountId to staking::Address
+		/// Make a call to a specified account, optionally transferring some balance.
 		fn call(
 			origin,
 			dest: T::AccountId,
 			value: <T::Balance as HasCompact>::Type,
 			gas_limit: <T::Gas as HasCompact>::Type,
 			data: Vec<u8>
-		) -> Result;
+		) -> Result {
+			let origin = ensure_signed(origin)?;
+			let value = value.into();
+			let gas_limit = gas_limit.into();
 
+			// Pay for the gas upfront.
+			//
+			// NOTE: it is very important to avoid any state changes before
+			// paying for the gas.
+			let mut gas_meter = gas::buy_gas::<T>(&origin, gas_limit)?;
+
+			let mut ctx = ExecutionContext {
+				self_account: origin.clone(),
+				depth: 0,
+				overlay: OverlayAccountDb::<T>::new(&account_db::DirectAccountDb),
+				events: Vec::new(),
+			};
+
+			let mut output_data = Vec::new();
+			let result = ctx.call(origin.clone(), dest, value, &mut gas_meter, &data, &mut output_data);
+
+			if let Ok(_) = result {
+				// Commit all changes that made it thus far into the persistant storage.
+				account_db::DirectAccountDb.commit(ctx.overlay.into_change_set());
+
+				// Then deposit all events produced.
+				ctx.events.into_iter().for_each(Self::deposit_event);
+			}
+
+			// Refund cost of the unused gas.
+			//
+			// NOTE: this should go after the commit to the storage, since the storage changes
+			// can alter the balance of the caller.
+			gas::refund_unused_gas::<T>(&origin, gas_meter);
+
+			result.map(|_| ())
+		}
+
+		/// Create a new contract, optionally transfering some balance to the created account.
+		///
+		/// Creation is executed as follows:
+		///
+		/// - the destination address is computed based on the sender and hash of the code.
+		/// - account is created at the computed address.
+		/// - the `ctor_code` is executed in the context of the newly created account. Buffer returned
+		///   after the execution is saved as the `code` of the account. That code will be invoked
+		///   upon any message received by this account.
 		fn create(
 			origin,
-			value: <T::Balance as HasCompact>::Type,
+			endowment: <T::Balance as HasCompact>::Type,
 			gas_limit: <T::Gas as HasCompact>::Type,
-			init_code: Vec<u8>,
+			ctor_code: Vec<u8>,
 			data: Vec<u8>
-		) -> Result;
+		) -> Result {
+			let origin = ensure_signed(origin)?;
+			let endowment = endowment.into();
+			let gas_limit = gas_limit.into();
+
+			// Pay for the gas upfront.
+			//
+			// NOTE: it is very important to avoid any state changes before
+			// paying for the gas.
+			let mut gas_meter = gas::buy_gas::<T>(&origin, gas_limit)?;
+
+			let mut ctx = ExecutionContext {
+				self_account: origin.clone(),
+				depth: 0,
+				overlay: OverlayAccountDb::<T>::new(&account_db::DirectAccountDb),
+				events: Vec::new(),
+			};
+			let result = ctx.create(origin.clone(), endowment, &mut gas_meter, &ctor_code, &data);
+
+			if let Ok(_) = result {
+				// Commit all changes that made it thus far into the persistant storage.
+				account_db::DirectAccountDb.commit(ctx.overlay.into_change_set());
+
+				// Then deposit all events produced.
+				ctx.events.into_iter().for_each(Self::deposit_event);
+			}
+
+			// Refund cost of the unused gas.
+			//
+			// NOTE: this should go after the commit to the storage, since the storage changes
+			// can alter the balance of the caller.
+			gas::refund_unused_gas::<T>(&origin, gas_meter);
+
+			result.map(|_| ())
+		}
+
 		fn on_finalise() {
 			<GasSpent<T>>::kill();
 		}
@@ -220,109 +302,6 @@ impl<T: Trait> double_map::StorageDoubleMap for StorageOf<T> {
 	type Value = Vec<u8>;
 }
 
-impl<T: Trait> Module<T> {
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
-	/// Make a call to a specified account, optionally transferring some balance.
-	fn call(
-		origin: <T as system::Trait>::Origin,
-		dest: T::AccountId,
-		value: <T::Balance as HasCompact>::Type,
-		gas_limit: <T::Gas as HasCompact>::Type,
-		data: Vec<u8>,
-	) -> Result {
-		let origin = ensure_signed(origin)?;
-		let value = value.into();
-		let gas_limit = gas_limit.into();
-
-		// Pay for the gas upfront.
-		//
-		// NOTE: it is very important to avoid any state changes before
-		// paying for the gas.
-		let mut gas_meter = gas::buy_gas::<T>(&origin, gas_limit)?;
-
-		let mut ctx = ExecutionContext {
-			self_account: origin.clone(),
-			depth: 0,
-			overlay: OverlayAccountDb::<T>::new(&account_db::DirectAccountDb),
-			events: Vec::new(),
-		};
-
-		let mut output_data = Vec::new();
-		let result = ctx.call(origin.clone(), dest, value, &mut gas_meter, &data, &mut output_data);
-
-		if let Ok(_) = result {
-			// Commit all changes that made it thus far into the persistant storage.
-			account_db::DirectAccountDb.commit(ctx.overlay.into_change_set());
-
-			// Then deposit all events produced.
-			ctx.events.into_iter().for_each(Self::deposit_event);
-		}
-
-		// Refund cost of the unused gas.
-		//
-		// NOTE: this should go after the commit to the storage, since the storage changes
-		// can alter the balance of the caller.
-		gas::refund_unused_gas::<T>(&origin, gas_meter);
-
-		result.map(|_| ())
-	}
-
-	/// Create a new contract, optionally transfering some balance to the created account.
-	///
-	/// Creation is executed as follows:
-	///
-	/// - the destination address is computed based on the sender and hash of the code.
-	/// - account is created at the computed address.
-	/// - the `ctor_code` is executed in the context of the newly created account. Buffer returned
-	///   after the execution is saved as the `code` of the account. That code will be invoked
-	///   upon any message received by this account.
-	fn create(
-		origin: <T as system::Trait>::Origin,
-		endowment: <T::Balance as HasCompact>::Type,
-		gas_limit: <T::Gas as HasCompact>::Type,
-		ctor_code: Vec<u8>,
-		data: Vec<u8>,
-	) -> Result {
-		let origin = ensure_signed(origin)?;
-		let endowment = endowment.into();
-		let gas_limit = gas_limit.into();
-
-		// Pay for the gas upfront.
-		//
-		// NOTE: it is very important to avoid any state changes before
-		// paying for the gas.
-		let mut gas_meter = gas::buy_gas::<T>(&origin, gas_limit)?;
-
-		let mut ctx = ExecutionContext {
-			self_account: origin.clone(),
-			depth: 0,
-			overlay: OverlayAccountDb::<T>::new(&account_db::DirectAccountDb),
-			events: Vec::new(),
-		};
-		let result = ctx.create(origin.clone(), endowment, &mut gas_meter, &ctor_code, &data);
-
-		if let Ok(_) = result {
-			// Commit all changes that made it thus far into the persistant storage.
-			account_db::DirectAccountDb.commit(ctx.overlay.into_change_set());
-
-			// Then deposit all events produced.
-			ctx.events.into_iter().for_each(Self::deposit_event);
-		}
-
-		// Refund cost of the unused gas.
-		//
-		// NOTE: this should go after the commit to the storage, since the storage changes
-		// can alter the balance of the caller.
-		gas::refund_unused_gas::<T>(&origin, gas_meter);
-
-		result.map(|_| ())
-	}
-}
-
 impl<T: Trait> balances::OnFreeBalanceZero<T::AccountId> for Module<T> {
 	fn on_free_balance_zero(who: &T::AccountId) {
 		<CodeOf<T>>::remove(who);

substrate/srml/council/src/motions.rs

@@ -68,8 +68,96 @@ decl_event!(
 decl_module! {
 	#[cfg_attr(feature = "std", serde(bound(deserialize = "<T as Trait>::Proposal: ::serde::de::DeserializeOwned")))]
 	pub struct Module<T: Trait> for enum Call where origin: <T as system::Trait>::Origin {
-		fn propose(origin, threshold: Compact<u32>, proposal: Box<<T as Trait>::Proposal>) -> Result;
-		fn vote(origin, proposal: T::Hash, index: Compact<ProposalIndex>, approve: bool) -> Result;
+		fn deposit_event() = default;
+		fn propose(origin, threshold: Compact<u32>, proposal: Box<<T as Trait>::Proposal>) -> Result {
+			let who = ensure_signed(origin)?;
+			let threshold = threshold.into();
+
+			ensure!(Self::is_councillor(&who), "proposer not on council");
+
+			let proposal_hash = T::Hashing::hash_of(&proposal);
+
+			ensure!(!<ProposalOf<T>>::exists(proposal_hash), "duplicate proposals not allowed");
+
+			if threshold < 2 {
+				let ok = proposal.dispatch(Origin::Members(1).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![]));
+
+				Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold));
+			}
+			Ok(())
+		}
+
+		fn vote(origin, proposal: T::Hash, index: Compact<ProposalIndex>, approve: bool) -> Result {
+			let who = ensure_signed(origin)?;
+			let index = index.into();
+
+			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");
+
+			let position_yes = voting.2.iter().position(|a| a == &who);
+			let position_no = voting.3.iter().position(|a| a == &who);
+
+			if approve {
+				if position_yes.is_none() {
+					voting.2.push(who.clone());
+				} else {
+					return Err("duplicate vote ignored")
+				}
+				if let Some(pos) = position_no {
+					voting.3.swap_remove(pos);
+				}
+			} else {
+				if position_no.is_none() {
+					voting.3.push(who.clone());
+				} else {
+					return Err("duplicate vote ignored")
+				}
+				if let Some(pos) = position_yes {
+					voting.2.swap_remove(pos);
+				}
+			}
+
+			let yes_votes = voting.2.len() as u32;
+			let no_votes = voting.3.len() as u32;
+			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;
+			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();
+						Self::deposit_event(RawEvent::Executed(proposal, ok));
+					}
+				} else {
+					// disapproved
+					Self::deposit_event(RawEvent::Disapproved(proposal));
+				}
+
+				// remove vote
+				<Voting<T>>::remove(&proposal);
+				<Proposals<T>>::mutate(|proposals| proposals.retain(|h| h != &proposal));
+			} else {
+				// update voting
+				<Voting<T>>::insert(&proposal, voting);
+			}
+
+			Ok(())
+		}
 	}
 }
 
@@ -91,107 +179,10 @@ decl_storage! {
 }
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	pub fn is_councillor(who: &T::AccountId) -> bool {
 		<Council<T>>::active_council().iter()
 			.any(|&(ref a, _)| a == who)
 	}
-
-	// Dispatch
-	fn propose(origin: <T as system::Trait>::Origin, threshold: Compact<u32>, proposal: Box<<T as Trait>::Proposal>) -> Result {
-		let who = ensure_signed(origin)?;
-		let threshold = threshold.into();
-
-		ensure!(Self::is_councillor(&who), "proposer not on council");
-
-		let proposal_hash = T::Hashing::hash_of(&proposal);
-
-		ensure!(!<ProposalOf<T>>::exists(proposal_hash), "duplicate proposals not allowed");
-
-		if threshold < 2 {
-			let ok = proposal.dispatch(Origin::Members(1).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![]));
-
-			Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold));
-		}
-		Ok(())
-	}
-
-	fn vote(origin: <T as system::Trait>::Origin, proposal: T::Hash, index: Compact<ProposalIndex>, approve: bool) -> Result {
-		let who = ensure_signed(origin)?;
-		let index = index.into();
-
-		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");
-
-		let position_yes = voting.2.iter().position(|a| a == &who);
-		let position_no = voting.3.iter().position(|a| a == &who);
-
-		if approve {
-			if position_yes.is_none() {
-				voting.2.push(who.clone());
-			} else {
-				return Err("duplicate vote ignored")
-			}
-			if let Some(pos) = position_no {
-				voting.3.swap_remove(pos);
-			}
-		} else {
-			if position_no.is_none() {
-				voting.3.push(who.clone());
-			} else {
-				return Err("duplicate vote ignored")
-			}
-			if let Some(pos) = position_yes {
-				voting.2.swap_remove(pos);
-			}
-		}
-
-		let yes_votes = voting.2.len() as u32;
-		let no_votes = voting.3.len() as u32;
-		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;
-		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();
-					Self::deposit_event(RawEvent::Executed(proposal, ok));
-				}
-			} else {
-				// disapproved
-				Self::deposit_event(RawEvent::Disapproved(proposal));
-			}
-
-			// remove vote
-			<Voting<T>>::remove(&proposal);
-			<Proposals<T>>::mutate(|proposals| proposals.retain(|h| h != &proposal));
-		} else {
-			// update voting
-			<Voting<T>>::insert(&proposal, voting);
-		}
-
-		Ok(())
-	}
 }
 
 /// Ensure that the origin `o` represents at least `n` council members. Returns

substrate/srml/council/src/seats.rs

@@ -87,16 +87,228 @@ pub trait Trait: democracy::Trait {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn set_approvals(origin, votes: Vec<bool>, index: Compact<VoteIndex>) -> Result;
-		fn reap_inactive_voter(origin, reporter_index: Compact<u32>, who: Address<T::AccountId, T::AccountIndex>, who_index: Compact<u32>, assumed_vote_index: Compact<VoteIndex>) -> Result;
-		fn retract_voter(origin, index: Compact<u32>) -> Result;
-		fn submit_candidacy(origin, slot: Compact<u32>) -> Result;
-		fn present_winner(origin, candidate: Address<T::AccountId, T::AccountIndex>, total: <T::Balance as HasCompact>::Type, index: Compact<VoteIndex>) -> Result;
+		fn deposit_event() = default;
+
+		/// Set candidate approvals. Approval slots stay valid as long as candidates in those slots
+		/// are registered.
+		fn set_approvals(origin, votes: Vec<bool>, index: Compact<VoteIndex>) -> Result {
+			let who = ensure_signed(origin)?;
+			let index: VoteIndex = index.into();
+
+			ensure!(!Self::presentation_active(), "no approval changes during presentation period");
+			ensure!(index == Self::vote_index(), "incorrect vote index");
+			if !<LastActiveOf<T>>::exists(&who) {
+				// not yet a voter - deduct bond.
+				// NOTE: this must be the last potential bailer, since it changes state.
+				<balances::Module<T>>::reserve(&who, Self::voting_bond())?;
+
+				<Voters<T>>::put({
+					let mut v = Self::voters();
+					v.push(who.clone());
+					v
+				});
+			}
+			<LastActiveOf<T>>::insert(&who, index);
+			<ApprovalsOf<T>>::insert(&who, votes);
+			Ok(())
+		}
+
+		/// Remove a voter. For it not to be a bond-consuming no-op, all approved candidate indices
+		/// must now be either unregistered or registered to a candidate that registered the slot after
+		/// the voter gave their last approval set.
+		///
+		/// May be called by anyone. Returns the voter deposit to `signed`.
+		fn reap_inactive_voter(
+			origin,
+			reporter_index: Compact<u32>,
+			who: Address<T::AccountId, T::AccountIndex>,
+			who_index: Compact<u32>,
+			assumed_vote_index: Compact<VoteIndex>
+		) -> Result {
+			let reporter = ensure_signed(origin)?;
+			let assumed_vote_index: VoteIndex = assumed_vote_index.into();
+
+			let who = <balances::Module<T>>::lookup(who)?;
+			ensure!(!Self::presentation_active(), "cannot reap during presentation period");
+			ensure!(Self::voter_last_active(&reporter).is_some(), "reporter must be a voter");
+			let last_active = Self::voter_last_active(&who).ok_or("target for inactivity cleanup must be active")?;
+			ensure!(assumed_vote_index == Self::vote_index(), "vote index not current");
+			ensure!(last_active < assumed_vote_index - Self::inactivity_grace_period(), "cannot reap during grace perid");
+			let voters = Self::voters();
+			let reporter_index: u32 = reporter_index.into();
+			let reporter_index = reporter_index as usize;
+			let who_index: u32 = who_index.into();
+			let who_index = who_index as usize;
+			ensure!(reporter_index < voters.len() && voters[reporter_index] == reporter, "bad reporter index");
+			ensure!(who_index < voters.len() && voters[who_index] == who, "bad target index");
+
+			// will definitely kill one of signed or who now.
+
+			let valid = !Self::approvals_of(&who).iter()
+				.zip(Self::candidates().iter())
+				.any(|(&appr, addr)|
+					 appr &&
+					 *addr != T::AccountId::default() &&
+					 Self::candidate_reg_info(addr).map_or(false, |x| x.0 <= last_active)/*defensive only: all items in candidates list are registered*/
+				);
+
+			Self::remove_voter(
+				if valid { &who } else { &reporter },
+				if valid { who_index } else { reporter_index },
+				voters
+			);
+			if valid {
+				// This only fails if `who` doesn't exist, which it clearly must do since its the origin.
+				// Still, it's no more harmful to propagate any error at this point.
+				<balances::Module<T>>::repatriate_reserved(&who, &reporter, Self::voting_bond())?;
+				Self::deposit_event(RawEvent::VoterReaped(who, reporter));
+			} else {
+				<balances::Module<T>>::slash_reserved(&reporter, Self::voting_bond());
+				Self::deposit_event(RawEvent::BadReaperSlashed(reporter));
+			}
+			Ok(())
+		}
+
+		/// Remove a voter. All votes are cancelled and the voter deposit is returned.
+		fn retract_voter(origin, index: Compact<u32>) -> Result {
+			let who = ensure_signed(origin)?;
+
+			ensure!(!Self::presentation_active(), "cannot retract when presenting");
+			ensure!(<LastActiveOf<T>>::exists(&who), "cannot retract non-voter");
+			let voters = Self::voters();
+			let index: u32 = index.into();
+			let index = index as usize;
+			ensure!(index < voters.len(), "retraction index invalid");
+			ensure!(voters[index] == who, "retraction index mismatch");
+
+			Self::remove_voter(&who, index, voters);
+			<balances::Module<T>>::unreserve(&who, Self::voting_bond());
+			Ok(())
+		}
+
+		/// Submit oneself for candidacy.
+		///
+		/// Account must have enough transferrable funds in it to pay the bond.
+		fn submit_candidacy(origin, slot: Compact<u32>) -> Result {
+			let who = ensure_signed(origin)?;
+
+			ensure!(!Self::is_a_candidate(&who), "duplicate candidate submission");
+			let slot: u32 = slot.into();
+			let slot = slot as usize;
+			let count = Self::candidate_count() as usize;
+			let candidates = Self::candidates();
+			ensure!(
+				(slot == count && count == candidates.len()) ||
+					(slot < candidates.len() && candidates[slot] == T::AccountId::default()),
+				"invalid candidate slot"
+			);
+			// NOTE: This must be last as it has side-effects.
+			<balances::Module<T>>::reserve(&who, Self::candidacy_bond())
+				.map_err(|_| "candidate has not enough funds")?;
+
+			<RegisterInfoOf<T>>::insert(&who, (Self::vote_index(), slot as u32));
+			let mut candidates = candidates;
+			if slot == candidates.len() {
+				candidates.push(who);
+			} else {
+				candidates[slot] = who;
+			}
+			<Candidates<T>>::put(candidates);
+			<CandidateCount<T>>::put(count as u32 + 1);
+			Ok(())
+		}
+
+		/// Claim that `signed` is one of the top Self::carry_count() + current_vote().1 candidates.
+		/// Only works if the `block_number >= current_vote().0` and `< current_vote().0 + presentation_duration()``
+		/// `signed` should have at least
+		fn present_winner(
+			origin,
+			candidate: Address<T::AccountId, T::AccountIndex>,
+			total: <T::Balance as HasCompact>::Type,
+			index: Compact<VoteIndex>
+		) -> Result {
+			let who = ensure_signed(origin)?;
+			let total = total.into();
+			let index: VoteIndex = index.into();
+
+			let candidate = <balances::Module<T>>::lookup(candidate)?;
+			ensure!(index == Self::vote_index(), "index not current");
+			let (_, _, expiring) = Self::next_finalise().ok_or("cannot present outside of presentation period")?;
+			let stakes = Self::snapshoted_stakes();
+			let voters = Self::voters();
+			let bad_presentation_punishment = Self::present_slash_per_voter() * T::Balance::sa(voters.len() as u64);
+			ensure!(<balances::Module<T>>::can_slash(&who, bad_presentation_punishment), "presenter must have sufficient slashable funds");
+
+			let mut leaderboard = Self::leaderboard().ok_or("leaderboard must exist while present phase active")?;
+			ensure!(total > leaderboard[0].0, "candidate not worthy of leaderboard");
+
+			if let Some(p) = Self::active_council().iter().position(|&(ref c, _)| c == &candidate) {
+				ensure!(p < expiring.len(), "candidate must not form a duplicated member if elected");
+			}
+
+			let (registered_since, candidate_index): (VoteIndex, u32) =
+				Self::candidate_reg_info(&candidate).ok_or("presented candidate must be current")?;
+			let actual_total = voters.iter()
+				.zip(stakes.iter())
+				.filter_map(|(voter, stake)|
+							match Self::voter_last_active(voter) {
+								Some(b) if b >= registered_since =>
+									Self::approvals_of(voter).get(candidate_index as usize)
+									.and_then(|approved| if *approved { Some(*stake) } else { None }),
+								_ => None,
+							})
+				.fold(Zero::zero(), |acc, n| acc + n);
+			let dupe = leaderboard.iter().find(|&&(_, ref c)| c == &candidate).is_some();
+			if total == actual_total && !dupe {
+				// insert into leaderboard
+				leaderboard[0] = (total, candidate);
+				leaderboard.sort_by_key(|&(t, _)| t);
+				<Leaderboard<T>>::put(leaderboard);
+				Ok(())
+			} else {
+				// we can rest assured it will be Ok since we checked `can_slash` earlier; still
+				// better safe than sorry.
+				let _ = <balances::Module<T>>::slash(&who, bad_presentation_punishment);
+				Err(if dupe { "duplicate presentation" } else { "incorrect total" })
+			}
+		}
+
+		/// 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.
+		fn set_desired_seats(count: Compact<u32>) -> Result {
+			let count: u32 = count.into();
+			<DesiredSeats<T>>::put(count);
+			Ok(())
+		}
+
+		/// Remove a particular member. A tally will 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: Address<T::AccountId, T::AccountIndex>) -> Result {
+			let who = <balances::Module<T>>::lookup(who)?;
+			let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council()
+				.into_iter()
+				.filter(|i| i.0 != who)
+				.collect();
+			<ActiveCouncil<T>>::put(new_council);
+			Ok(())
+		}
+
+		/// Set the presentation duration. If there is currently a vote being presented for, will
+		/// invoke `finalise_vote`.
+		fn set_presentation_duration(count: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<PresentationDuration<T>>::put(count.into());
+			Ok(())
+		}
+
+		/// Set the presentation duration. If there is current a vote being presented for, will
+		/// invoke `finalise_vote`.
+		fn set_term_duration(count: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<TermDuration<T>>::put(count.into());
+			Ok(())
+		}
 
-		fn set_desired_seats(count: Compact<u32>) -> Result;
-		fn remove_member(who: Address<T::AccountId, T::AccountIndex>) -> Result;
-		fn set_presentation_duration(count: <T::BlockNumber as HasCompact>::Type) -> Result;
-		fn set_term_duration(count: <T::BlockNumber as HasCompact>::Type) -> Result;
 		fn on_finalise(n: T::BlockNumber) {
 			if let Err(e) = Self::end_block(n) {
 				print("Guru meditation");
@@ -176,12 +388,6 @@ decl_event!(
 );
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	// exposed immutables.
 
 	/// True if we're currently in a presentation period.
@@ -230,230 +436,7 @@ impl<T: Trait> Module<T> {
 		}
 	}
 
-	// dispatch
-
-	/// Set candidate approvals. Approval slots stay valid as long as candidates in those slots
-	/// are registered.
-	fn set_approvals(origin: T::Origin, votes: Vec<bool>, index: Compact<VoteIndex>) -> Result {
-		let who = ensure_signed(origin)?;
-		let index: VoteIndex = index.into();
-
-		ensure!(!Self::presentation_active(), "no approval changes during presentation period");
-		ensure!(index == Self::vote_index(), "incorrect vote index");
-		if !<LastActiveOf<T>>::exists(&who) {
-			// not yet a voter - deduct bond.
-			// NOTE: this must be the last potential bailer, since it changes state.
-			<balances::Module<T>>::reserve(&who, Self::voting_bond())?;
-
-			<Voters<T>>::put({
-				let mut v = Self::voters();
-				v.push(who.clone());
-				v
-			});
-		}
-		<LastActiveOf<T>>::insert(&who, index);
-		<ApprovalsOf<T>>::insert(&who, votes);
-		Ok(())
-	}
-
-	/// Remove a voter. For it not to be a bond-consuming no-op, all approved candidate indices
-	/// must now be either unregistered or registered to a candidate that registered the slot after
-	/// the voter gave their last approval set.
-	///
-	/// May be called by anyone. Returns the voter deposit to `signed`.
-	fn reap_inactive_voter(
-		origin: T::Origin,
-		reporter_index: Compact<u32>,
-		who: Address<T::AccountId, T::AccountIndex>,
-		who_index: Compact<u32>,
-		assumed_vote_index: Compact<VoteIndex>
-	) -> Result {
-		let reporter = ensure_signed(origin)?;
-		let assumed_vote_index: VoteIndex = assumed_vote_index.into();
-
-		let who = <balances::Module<T>>::lookup(who)?;
-		ensure!(!Self::presentation_active(), "cannot reap during presentation period");
-		ensure!(Self::voter_last_active(&reporter).is_some(), "reporter must be a voter");
-		let last_active = Self::voter_last_active(&who).ok_or("target for inactivity cleanup must be active")?;
-		ensure!(assumed_vote_index == Self::vote_index(), "vote index not current");
-		ensure!(last_active < assumed_vote_index - Self::inactivity_grace_period(), "cannot reap during grace perid");
-		let voters = Self::voters();
-		let reporter_index: u32 = reporter_index.into();
-		let reporter_index = reporter_index as usize;
-		let who_index: u32 = who_index.into();
-		let who_index = who_index as usize;
-		ensure!(reporter_index < voters.len() && voters[reporter_index] == reporter, "bad reporter index");
-		ensure!(who_index < voters.len() && voters[who_index] == who, "bad target index");
-
-		// will definitely kill one of signed or who now.
-
-		let valid = !Self::approvals_of(&who).iter()
-			.zip(Self::candidates().iter())
-			.any(|(&appr, addr)|
-				appr &&
-				*addr != T::AccountId::default() &&
-				Self::candidate_reg_info(addr).map_or(false, |x| x.0 <= last_active)/*defensive only: all items in candidates list are registered*/
-			);
-
-		Self::remove_voter(
-			if valid { &who } else { &reporter },
-			if valid { who_index } else { reporter_index },
-			voters
-		);
-		if valid {
-			// This only fails if `who` doesn't exist, which it clearly must do since its the origin.
-			// Still, it's no more harmful to propagate any error at this point.
-			<balances::Module<T>>::repatriate_reserved(&who, &reporter, Self::voting_bond())?;
-			Self::deposit_event(RawEvent::VoterReaped(who, reporter));
-		} else {
-			<balances::Module<T>>::slash_reserved(&reporter, Self::voting_bond());
-			Self::deposit_event(RawEvent::BadReaperSlashed(reporter));
-		}
-		Ok(())
-	}
-
-	/// Remove a voter. All votes are cancelled and the voter deposit is returned.
-	fn retract_voter(origin: T::Origin, index: Compact<u32>) -> Result {
-		let who = ensure_signed(origin)?;
-
-		ensure!(!Self::presentation_active(), "cannot retract when presenting");
-		ensure!(<LastActiveOf<T>>::exists(&who), "cannot retract non-voter");
-		let voters = Self::voters();
-		let index: u32 = index.into();
-		let index = index as usize;
-		ensure!(index < voters.len(), "retraction index invalid");
-		ensure!(voters[index] == who, "retraction index mismatch");
-
-		Self::remove_voter(&who, index, voters);
-		<balances::Module<T>>::unreserve(&who, Self::voting_bond());
-		Ok(())
-	}
-
-	/// Submit oneself for candidacy.
-	///
-	/// Account must have enough transferrable funds in it to pay the bond.
-	fn submit_candidacy(origin: T::Origin, slot: Compact<u32>) -> Result {
-		let who = ensure_signed(origin)?;
-
-		ensure!(!Self::is_a_candidate(&who), "duplicate candidate submission");
-		let slot: u32 = slot.into();
-		let slot = slot as usize;
-		let count = Self::candidate_count() as usize;
-		let candidates = Self::candidates();
-		ensure!(
-			(slot == count && count == candidates.len()) ||
-			(slot < candidates.len() && candidates[slot] == T::AccountId::default()),
-			"invalid candidate slot"
-		);
-		// NOTE: This must be last as it has side-effects.
-		<balances::Module<T>>::reserve(&who, Self::candidacy_bond())
-			.map_err(|_| "candidate has not enough funds")?;
-
-		<RegisterInfoOf<T>>::insert(&who, (Self::vote_index(), slot as u32));
-		let mut candidates = candidates;
-		if slot == candidates.len() {
-			candidates.push(who);
-		} else {
-			candidates[slot] = who;
-		}
-		<Candidates<T>>::put(candidates);
-		<CandidateCount<T>>::put(count as u32 + 1);
-		Ok(())
-	}
-
-	/// Claim that `signed` is one of the top Self::carry_count() + current_vote().1 candidates.
-	/// Only works if the `block_number >= current_vote().0` and `< current_vote().0 + presentation_duration()``
-	/// `signed` should have at least
-	fn present_winner(
-		origin: T::Origin,
-		candidate: Address<T::AccountId, T::AccountIndex>,
-		total: <T::Balance as HasCompact>::Type,
-		index: Compact<VoteIndex>
-	) -> Result {
-		let who = ensure_signed(origin)?;
-		let total = total.into();
-		let index: VoteIndex = index.into();
-
-		let candidate = <balances::Module<T>>::lookup(candidate)?;
-		ensure!(index == Self::vote_index(), "index not current");
-		let (_, _, expiring) = Self::next_finalise().ok_or("cannot present outside of presentation period")?;
-		let stakes = Self::snapshoted_stakes();
-		let voters = Self::voters();
-		let bad_presentation_punishment = Self::present_slash_per_voter() * T::Balance::sa(voters.len() as u64);
-		ensure!(<balances::Module<T>>::can_slash(&who, bad_presentation_punishment), "presenter must have sufficient slashable funds");
-
-		let mut leaderboard = Self::leaderboard().ok_or("leaderboard must exist while present phase active")?;
-		ensure!(total > leaderboard[0].0, "candidate not worthy of leaderboard");
-
-		if let Some(p) = Self::active_council().iter().position(|&(ref c, _)| c == &candidate) {
-			ensure!(p < expiring.len(), "candidate must not form a duplicated member if elected");
-		}
-
-		let (registered_since, candidate_index): (VoteIndex, u32) =
-			Self::candidate_reg_info(&candidate).ok_or("presented candidate must be current")?;
-		let actual_total = voters.iter()
-			.zip(stakes.iter())
-			.filter_map(|(voter, stake)|
-				match Self::voter_last_active(voter) {
-					Some(b) if b >= registered_since =>
-						Self::approvals_of(voter).get(candidate_index as usize)
-							.and_then(|approved| if *approved { Some(*stake) } else { None }),
-					_ => None,
-				})
-			.fold(Zero::zero(), |acc, n| acc + n);
-		let dupe = leaderboard.iter().find(|&&(_, ref c)| c == &candidate).is_some();
-		if total == actual_total && !dupe {
-			// insert into leaderboard
-			leaderboard[0] = (total, candidate);
-			leaderboard.sort_by_key(|&(t, _)| t);
-			<Leaderboard<T>>::put(leaderboard);
-			Ok(())
-		} else {
-			// we can rest assured it will be Ok since we checked `can_slash` earlier; still
-			// better safe than sorry.
-			let _ = <balances::Module<T>>::slash(&who, bad_presentation_punishment);
-			Err(if dupe { "duplicate presentation" } else { "incorrect total" })
-		}
-	}
-
-	/// 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.
-	fn set_desired_seats(count: Compact<u32>) -> Result {
-		let count: u32 = count.into();
-		<DesiredSeats<T>>::put(count);
-		Ok(())
-	}
-
-	/// Remove a particular member. A tally will 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: Address<T::AccountId, T::AccountIndex>) -> Result {
-		let who = <balances::Module<T>>::lookup(who)?;
-		let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council()
-			.into_iter()
-			.filter(|i| i.0 != who)
-			.collect();
-		<ActiveCouncil<T>>::put(new_council);
-		Ok(())
-	}
-
-	/// Set the presentation duration. If there is currently a vote being presented for, will
-	/// invoke `finalise_vote`.
-	fn set_presentation_duration(count: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<PresentationDuration<T>>::put(count.into());
-		Ok(())
-	}
-
-	/// Set the presentation duration. If there is current a vote being presented for, will
-	/// invoke `finalise_vote`.
-	fn set_term_duration(count: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<TermDuration<T>>::put(count.into());
-		Ok(())
-	}
-
-	// private
-
+	// Private
 	/// Check there's nothing to do this block
 	fn end_block(block_number: T::BlockNumber) -> Result {
 		if (block_number % Self::voting_period()).is_zero() {

substrate/srml/council/src/voting.rs

@@ -33,12 +33,82 @@ pub trait Trait: CouncilTrait {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn propose(origin, proposal: Box<T::Proposal>) -> Result;
-		fn vote(origin, proposal: T::Hash, approve: bool) -> Result;
-		fn veto(origin, proposal_hash: T::Hash) -> Result;
+		fn deposit_event() = default;
+
+		fn propose(origin, proposal: Box<T::Proposal>) -> Result {
+			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);
+
+			Ok(())
+		}
+
+		fn vote(origin, proposal: T::Hash, approve: bool) -> Result {
+			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);
+			Ok(())
+		}
+
+		fn veto(origin, proposal_hash: T::Hash) -> Result {
+			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));
+			}
+			Ok(())
+		}
+
+		fn set_cooloff_period(blocks: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<CooloffPeriod<T>>::put(blocks.into());
+			Ok(())
+		}
+
+		fn set_voting_period(blocks: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<VotingPeriod<T>>::put(blocks.into());
+			Ok(())
+		}
 
-		fn set_cooloff_period(blocks: <T::BlockNumber as HasCompact>::Type) -> Result;
-		fn set_voting_period(blocks: <T::BlockNumber as HasCompact>::Type) -> Result;
 		fn on_finalise(n: T::BlockNumber) {
 			if let Err(e) = Self::end_block(n) {
 				print("Guru meditation");
@@ -73,12 +143,6 @@ decl_event!(
 );
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	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)
@@ -101,78 +165,7 @@ impl<T: Trait> Module<T> {
 		Self::generic_tally(proposal_hash, |w: &T::AccountId, p: &T::Hash| Self::vote_of((*p, w.clone())))
 	}
 
-	// Dispatch
-	fn propose(origin: T::Origin, proposal: Box<T::Proposal>) -> Result {
-		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);
-
-		Ok(())
-	}
-
-	fn vote(origin: T::Origin, proposal: T::Hash, approve: bool) -> Result {
-		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);
-		Ok(())
-	}
-
-	fn veto(origin: T::Origin, proposal_hash: T::Hash) -> Result {
-		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));
-		}
-		Ok(())
-	}
-
-	fn set_cooloff_period(blocks: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<CooloffPeriod<T>>::put(blocks.into());
-		Ok(())
-	}
-
-	fn set_voting_period(blocks: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<VotingPeriod<T>>::put(blocks.into());
-		Ok(())
-	}
-
-	// private
-
-
+	// Private
 	fn set_veto_of(proposal: &T::Hash, expiry: T::BlockNumber, vetoers: Vec<T::AccountId>) {
 		<VetoedProposal<T>>::insert(proposal, (expiry, vetoers));
 	}

substrate/srml/democracy/src/lib.rs

@@ -62,12 +62,74 @@ pub trait Trait: balances::Trait + Sized {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn propose(origin, proposal: Box<T::Proposal>, value: <T::Balance as HasCompact>::Type) -> Result;
-		fn second(origin, proposal: Compact<PropIndex>) -> Result;
-		fn vote(origin, ref_index: Compact<ReferendumIndex>, approve_proposal: bool) -> Result;
+		fn deposit_event() = default;
+
+		/// Propose a sensitive action to be taken.
+		fn propose(
+			origin,
+			proposal: Box<T::Proposal>,
+			value: <T::Balance as HasCompact>::Type
+		) -> Result {
+			let who = ensure_signed(origin)?;
+			let value = value.into();
+
+			ensure!(value >= Self::minimum_deposit(), "value too low");
+			<balances::Module<T>>::reserve(&who, value)
+				.map_err(|_| "proposer's balance too low")?;
+
+			let index = Self::public_prop_count();
+			<PublicPropCount<T>>::put(index + 1);
+			<DepositOf<T>>::insert(index, (value, vec![who.clone()]));
+
+			let mut props = Self::public_props();
+			props.push((index, (*proposal).clone(), who));
+			<PublicProps<T>>::put(props);
+			Ok(())
+		}
+
+		/// Propose a sensitive action to be taken.
+		fn second(origin, proposal: Compact<PropIndex>) -> Result {
+			let who = ensure_signed(origin)?;
+			let proposal: PropIndex = proposal.into();
+			let mut deposit = Self::deposit_of(proposal)
+				.ok_or("can only second an existing proposal")?;
+			<balances::Module<T>>::reserve(&who, deposit.0)
+				.map_err(|_| "seconder's balance too low")?;
+			deposit.1.push(who);
+			<DepositOf<T>>::insert(proposal, deposit);
+			Ok(())
+		}
+
+		/// Vote in a referendum. If `approve_proposal` is true, the vote is to enact the proposal;
+		/// false would be a vote to keep the status quo.
+		fn vote(origin, ref_index: Compact<ReferendumIndex>, approve_proposal: bool) -> Result {
+			let who = ensure_signed(origin)?;
+			let ref_index = ref_index.into();
+			ensure!(Self::is_active_referendum(ref_index), "vote given for invalid referendum.");
+			ensure!(!<balances::Module<T>>::total_balance(&who).is_zero(),
+					"transactor must have balance to signal approval.");
+			if !<VoteOf<T>>::exists(&(ref_index, who.clone())) {
+				<VotersFor<T>>::mutate(ref_index, |voters| voters.push(who.clone()));
+			}
+			<VoteOf<T>>::insert(&(ref_index, who), approve_proposal);
+			Ok(())
+		}
+
+		/// Start a referendum.
+		fn start_referendum(proposal: Box<T::Proposal>, vote_threshold: VoteThreshold) -> Result {
+			Self::inject_referendum(
+				<system::Module<T>>::block_number() + Self::voting_period(),
+				*proposal,
+				vote_threshold
+			).map(|_| ())
+		}
+
+		/// Remove a referendum.
+		fn cancel_referendum(ref_index: Compact<ReferendumIndex>) -> Result {
+			Self::clear_referendum(ref_index.into());
+			Ok(())
+		}
 
-		fn start_referendum(proposal: Box<T::Proposal>, vote_threshold: VoteThreshold) -> Result;
-		fn cancel_referendum(ref_index: Compact<ReferendumIndex>) -> Result;
 		fn on_finalise(n: T::BlockNumber) {
 			if let Err(e) = Self::end_block(n) {
 				runtime_io::print(e);
@@ -121,12 +183,6 @@ decl_event!(
 );
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	// exposed immutables.
 
 	/// Get the amount locked in support of `proposal`; `None` if proposal isn't a valid proposal
@@ -167,71 +223,7 @@ impl<T: Trait> Module<T> {
 			.fold((Zero::zero(), Zero::zero()), |(a, b), (c, d)| (a + c, b + d))
 	}
 
-	// dispatching.
-
-	/// Propose a sensitive action to be taken.
-	fn propose(origin: T::Origin, proposal: Box<T::Proposal>, value: <T::Balance as HasCompact>::Type) -> Result {
-		let who = ensure_signed(origin)?;
-		let value = value.into();
-
-		ensure!(value >= Self::minimum_deposit(), "value too low");
-		<balances::Module<T>>::reserve(&who, value)
-			.map_err(|_| "proposer's balance too low")?;
-
-		let index = Self::public_prop_count();
-		<PublicPropCount<T>>::put(index + 1);
-		<DepositOf<T>>::insert(index, (value, vec![who.clone()]));
-
-		let mut props = Self::public_props();
-		props.push((index, (*proposal).clone(), who));
-		<PublicProps<T>>::put(props);
-		Ok(())
-	}
-
-	/// Propose a sensitive action to be taken.
-	fn second(origin: T::Origin, proposal: Compact<PropIndex>) -> Result {
-		let who = ensure_signed(origin)?;
-		let proposal: PropIndex = proposal.into();
-		let mut deposit = Self::deposit_of(proposal)
-			.ok_or("can only second an existing proposal")?;
-		<balances::Module<T>>::reserve(&who, deposit.0)
-			.map_err(|_| "seconder's balance too low")?;
-		deposit.1.push(who);
-		<DepositOf<T>>::insert(proposal, deposit);
-		Ok(())
-	}
-
-	/// Vote in a referendum. If `approve_proposal` is true, the vote is to enact the proposal;
-	/// false would be a vote to keep the status quo.
-	fn vote(origin: T::Origin, ref_index: Compact<ReferendumIndex>, approve_proposal: bool) -> Result {
-		let who = ensure_signed(origin)?;
-		let ref_index = ref_index.into();
-		ensure!(Self::is_active_referendum(ref_index), "vote given for invalid referendum.");
-		ensure!(!<balances::Module<T>>::total_balance(&who).is_zero(),
-			"transactor must have balance to signal approval.");
-		if !<VoteOf<T>>::exists(&(ref_index, who.clone())) {
-			<VotersFor<T>>::mutate(ref_index, |voters| voters.push(who.clone()));
-		}
-		<VoteOf<T>>::insert(&(ref_index, who), approve_proposal);
-		Ok(())
-	}
-
-	/// Start a referendum.
-	fn start_referendum(proposal: Box<T::Proposal>, vote_threshold: VoteThreshold) -> Result {
-		Self::inject_referendum(
-			<system::Module<T>>::block_number() + Self::voting_period(),
-			*proposal,
-			vote_threshold
-		).map(|_| ())
-	}
-
-	/// Remove a referendum.
-	fn cancel_referendum(ref_index: Compact<ReferendumIndex>) -> Result {
-		Self::clear_referendum(ref_index.into());
-		Ok(())
-	}
-
-	// exposed mutables.
+	// Exposed mutables.
 
 	/// Start a referendum. Can be called directly by the council.
 	pub fn internal_start_referendum(proposal: T::Proposal, vote_threshold: VoteThreshold) -> result::Result<ReferendumIndex, &'static str> {

substrate/srml/example/src/lib.rs

@@ -103,13 +103,90 @@ pub trait Trait: balances::Trait {
 decl_module! {
 	// Simple declaration of the `Module` type. Lets the macro know what its working on.
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		/// Deposit one of this module's events by using the default implementation.
+		/// It is also possible to provide a custom implementation.
+		fn deposit_event() = default;
 		/// This is your public interface. Be extremely careful.
 		/// This is just a simple example of how to interact with the module from the external
 		/// world.
-		fn accumulate_dummy(origin, increase_by: T::Balance) -> Result;
+		// This just increases the value of `Dummy` by `increase_by`.
+		//
+		// Since this is a dispatched function there are two extremely important things to
+		// remember:
+		//
+		// - MUST NOT PANIC: Under no circumstances (save, perhaps, storage getting into an
+		// irreparably damaged state) must this function panic.
+		// - NO SIDE-EFFECTS ON ERROR: This function must either complete totally (and return
+		// `Ok(())` or it must have no side-effects on storage and return `Err('Some reason')`.
+		//
+		// The first is relatively easy to audit for - just ensure all panickers are removed from
+		// logic that executes in production (which you do anyway, right?!). To ensure the second
+		// is followed, you should do all tests for validity at the top of your function. This
+		// is stuff like checking the sender (`origin`) or that state is such that the operation
+		// makes sense.
+		//
+		// Once you've determined that it's all good, then enact the operation and change storage.
+		// If you can't be certain that the operation will succeed without substantial computation
+		// then you have a classic blockchain attack scenario. The normal way of managing this is
+		// to attach a bond to the operation. As the first major alteration of storage, reserve
+		// some value from the sender's account (`Balances` module has a `reserve` function for
+		// exactly this scenario). This amount should be enough to cover any costs of the
+		// substantial execution in case it turns out that you can't proceed with the operation.
+		//
+		// If it eventually transpires that the operation is fine and, therefore, that the
+		// expense of the checks should be borne by the network, then you can refund the reserved
+		// deposit. If, however, the operation turns out to be invalid and the computation is
+		// wasted, then you can burn it or repatriate elsewhere.
+		//
+		// Security bonds ensure that attackers can't game it by ensuring that anyone interacting
+		// with the system either progresses it or pays for the trouble of faffing around with
+		// no progress.
+		//
+		// If you don't respect these rules, it is likely that your chain will be attackable.
+		fn accumulate_dummy(origin, increase_by: T::Balance) -> Result {
+			// This is a public call, so we ensure that the origin is some signed account.
+			let _sender = ensure_signed(origin)?;
+
+			// Read the value of dummy from storage.
+			// let dummy = Self::dummy();
+			// Will also work using the `::get` on the storage item type itself:
+			// let dummy = <Dummy<T>>::get();
+
+			// Calculate the new value.
+			// let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by);
+
+			// Put the new value into storage.
+			// <Dummy<T>>::put(new_dummy);
+			// Will also work with a reference:
+			// <Dummy<T>>::put(&new_dummy);
+
+			// Here's the new one of read and then modify the value.
+			<Dummy<T>>::mutate(|dummy| {
+				let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by);
+				*dummy = Some(new_dummy);
+			});
+
+			// Let's deposit an event to let the outside world know this happened.
+			Self::deposit_event(RawEvent::Dummy(increase_by));
+
+			// All good.
+			Ok(())
+		}
 
 		/// A privileged call; in this case it resets our dummy value to something new.
-		fn set_dummy(new_dummy: T::Balance) -> Result;
+		// Implementation of a privileged call. This doesn't have an `origin` parameter because
+		// it's not (directly) from an extrinsic, but rather the system as a whole has decided
+		// to execute it. Different runtimes have different reasons for allow privileged
+		// calls to be executed - we don't need to care why. Because it's privileged, we can
+		// assume it's a one-off operation and substantial processing/storage/memory can be used
+		// without worrying about gameability or attack scenarios.
+		fn set_dummy(new_value: T::Balance) -> Result {
+			// Put the new value into storage.
+			<Dummy<T>>::put(new_value);
+
+			// All good.
+			Ok(())
+		}
 
 		// The signature could also look like: `fn on_finalise()`
 		fn on_finalise(_n: T::BlockNumber) {
@@ -168,85 +245,11 @@ decl_storage! {
 
 // The main implementation block for the module. Functions here fall into three broad
 // categories:
-// - Implementations of dispatch functions. The dispatch code generated by the module macro
-// expects each of its functions to be implemented.
 // - Public interface. These are functions that are `pub` and generally fall into inspector
 // functions that do not write to storage and operation functions that do.
 // - Private functions. These are your usual private utilities unavailable to other modules.
 impl<T: Trait> Module<T> {
-	/// Deposit one of this module's events.
-	// TODO: move into `decl_module` macro.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
-	// Implement Calls and add public immutables and private mutables.
-
-	// Implement dispatched function `accumulate_dummy`. This just increases the value
-	// of `Dummy` by `increase_by`.
-	//
-	// Since this is a dispatched function there are two extremely important things to
-	// remember:
-	//
-	// - MUST NOT PANIC: Under no circumstances (save, perhaps, storage getting into an
-	// irreparably damaged state) must this function panic.
-	// - NO SIDE-EFFECTS ON ERROR: This function must either complete totally (and return
-	// `Ok(())` or it must have no side-effects on storage and return `Err('Some reason')`.
-	//
-	// The first is relatively easy to audit for - just ensure all panickers are removed from
-	// logic that executes in production (which you do anyway, right?!). To ensure the second
-	// is followed, you should do all tests for validity at the top of your function. This
-	// is stuff like checking the sender (`origin`) or that state is such that the operation
-	// makes sense.
-	//
-	// Once you've determined that it's all good, then enact the operation and change storage.
-	// If you can't be certain that the operation will succeed without substantial computation
-	// then you have a classic blockchain attack scenario. The normal way of managing this is
-	// to attach a bond to the operation. As the first major alteration of storage, reserve
-	// some value from the sender's account (`Balances` module has a `reserve` function for
-	// exactly this scenario). This amount should be enough to cover any costs of the
-	// substantial execution in case it turns out that you can't proceed with the operation.
-	//
-	// If it eventually transpires that the operation is fine and, therefore, that the
-	// expense of the checks should be borne by the network, then you can refund the reserved
-	// deposit. If, however, the operation turns out to be invalid and the computation is
-	// wasted, then you can burn it or repatriate elsewhere.
-	//
-	// Security bonds ensure that attackers can't game it by ensuring that anyone interacting
-	// with the system either progresses it or pays for the trouble of faffing around with
-	// no progress.
-	//
-	// If you don't respect these rules, it is likely that your chain will be attackable.
-	fn accumulate_dummy(origin: T::Origin, increase_by: T::Balance) -> Result {
-		// This is a public call, so we ensure that the origin is some signed account.
-		let _sender = ensure_signed(origin)?;
-
-		// Read the value of dummy from storage.
-		// let dummy = Self::dummy();
-		// Will also work using the `::get` on the storage item type itself:
-		// let dummy = <Dummy<T>>::get();
-
-		// Calculate the new value.
-		// let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by);
-
-		// Put the new value into storage.
-		// <Dummy<T>>::put(new_dummy);
-		// Will also work with a reference:
-		// <Dummy<T>>::put(&new_dummy);
-
-		// Here's the new one of read and then modify the value.
-		<Dummy<T>>::mutate(|dummy| {
-			let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by);
-			*dummy = Some(new_dummy);
-		});
-
-		// Let's deposit an event to let the outside world know this happened.
-		Self::deposit_event(RawEvent::Dummy(increase_by));
-
-		// All good.
-		Ok(())
-	}
-
+	// Add public immutables and private mutables.
 	#[allow(dead_code)]
 	fn accumulate_foo(origin: T::Origin, increase_by: T::Balance) -> Result {
 		let _sender = ensure_signed(origin)?;
@@ -261,20 +264,6 @@ impl<T: Trait> Module<T> {
 
 		Ok(())
 	}
-
-	// Implementation of a privileged call. This doesn't have an `origin` parameter because
-	// it's not (directly) from an extrinsic, but rather the system as a whole has decided
-	// to execute it. Different runtimes have different reasons for allow privileged
-	// calls to be executed - we don't need to care why. Because it's privileged, we can
-	// assume it's a one-off operation and substantial processing/storage/memory can be used
-	// without worrying about gameability or attack scenarios.
-	fn set_dummy(new_value: T::Balance) -> Result {
-		// Put the new value into storage.
-		<Dummy<T>>::put(new_value);
-
-		// All good.
-		Ok(())
-	}
 }
 
 #[cfg(test)]

substrate/srml/session/src/lib.rs

@@ -67,10 +67,28 @@ pub trait Trait: timestamp::Trait {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn set_key(origin, key: T::SessionKey) -> Result;
+		fn deposit_event() = default;
+
+		/// Sets the session key of `_validator` to `_key`. This doesn't take effect until the next
+		/// session.
+		fn set_key(origin, key: T::SessionKey) -> Result {
+			let who = ensure_signed(origin)?;
+			// set new value for next session
+			<NextKeyFor<T>>::insert(who, key);
+			Ok(())
+		}
+
+		/// Set a new session length. Won't kick in until the next session change (at current length).
+		fn set_length(new: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<NextSessionLength<T>>::put(new.into());
+			Ok(())
+		}
+
+		/// Forces a new session.
+		fn force_new_session(apply_rewards: bool) -> Result {
+			Self::apply_force_new_session(apply_rewards)
+		}
 
-		fn set_length(new: <T::BlockNumber as HasCompact>::Type) -> Result;
-		fn force_new_session(apply_rewards: bool) -> Result;
 		fn on_finalise(n: T::BlockNumber) {
 			Self::check_rotate_session(n);
 		}
@@ -111,12 +129,6 @@ decl_storage! {
 }
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
 	/// The number of validators currently.
 	pub fn validator_count() -> u32 {
 		<Validators<T>>::get().len() as u32	// TODO: can probably optimised
@@ -127,28 +139,7 @@ impl<T: Trait> Module<T> {
 		<LastLengthChange<T>>::get().unwrap_or_else(T::BlockNumber::zero)
 	}
 
-	/// Sets the session key of `_validator` to `_key`. This doesn't take effect until the next
-	/// session.
-	fn set_key(origin: T::Origin, key: T::SessionKey) -> Result {
-		let who = ensure_signed(origin)?;
-		// set new value for next session
-		<NextKeyFor<T>>::insert(who, key);
-		Ok(())
-	}
-
-	/// Set a new session length. Won't kick in until the next session change (at current length).
-	fn set_length(new: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<NextSessionLength<T>>::put(new.into());
-		Ok(())
-	}
-
-	/// Forces a new session.
-	pub fn force_new_session(apply_rewards: bool) -> Result {
-		Self::apply_force_new_session(apply_rewards)
-	}
-
 	// INTERNAL API (available to other runtime modules)
-
 	/// Forces a new session, no origin.
 	pub fn apply_force_new_session(apply_rewards: bool) -> Result {
 		<ForcingNewSession<T>>::put(apply_rewards);

substrate/srml/staking/src/lib.rs

@@ -105,17 +105,140 @@ pub trait Trait: balances::Trait + session::Trait {
 decl_module! {
 	#[cfg_attr(feature = "std", serde(bound(deserialize = "T::Balance: ::serde::de::DeserializeOwned")))]
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn stake(origin) -> Result;
-		fn unstake(origin, intentions_index: Compact<u32>) -> Result;
-		fn nominate(origin, target: Address<T::AccountId, T::AccountIndex>) -> Result;
-		fn unnominate(origin, target_index: Compact<u32>) -> Result;
-		fn register_preferences(origin, intentions_index: Compact<u32>, prefs: ValidatorPrefs<T::Balance>) -> Result;
+		fn deposit_event() = default;
 
-		fn set_sessions_per_era(new: <T::BlockNumber as HasCompact>::Type) -> Result;
-		fn set_bonding_duration(new: <T::BlockNumber as HasCompact>::Type) -> Result;
-		fn set_validator_count(new: Compact<u32>) -> Result;
-		fn force_new_era(apply_rewards: bool) -> Result;
-		fn set_offline_slash_grace(new: Compact<u32>) -> Result;
+		/// Declare the desire to stake for the transactor.
+		///
+		/// Effects will be felt at the beginning of the next era.
+		fn stake(origin) -> Result {
+			let who = ensure_signed(origin)?;
+			ensure!(Self::nominating(&who).is_none(), "Cannot stake if already nominating.");
+			let mut intentions = <Intentions<T>>::get();
+			// can't be in the list twice.
+			ensure!(intentions.iter().find(|&t| t == &who).is_none(), "Cannot stake if already staked.");
+
+			<Bondage<T>>::insert(&who, T::BlockNumber::max_value());
+			intentions.push(who);
+			<Intentions<T>>::put(intentions);
+			Ok(())
+		}
+
+		/// Retract the desire to stake for the transactor.
+		///
+		/// Effects will be felt at the beginning of the next era.
+		fn unstake(origin, intentions_index: Compact<u32>) -> Result {
+			let who = ensure_signed(origin)?;
+			let intentions_index: u32 = intentions_index.into();
+			// unstake fails in degenerate case of having too few existing staked parties
+			if Self::intentions().len() <= Self::minimum_validator_count() as usize {
+				return Err("cannot unstake when there are too few staked participants")
+			}
+			Self::apply_unstake(&who, intentions_index as usize)
+		}
+
+		fn nominate(origin, target: Address<T::AccountId, T::AccountIndex>) -> Result {
+			let who = ensure_signed(origin)?;
+			let target = <balances::Module<T>>::lookup(target)?;
+
+			ensure!(Self::nominating(&who).is_none(), "Cannot nominate if already nominating.");
+			ensure!(Self::intentions().iter().find(|&t| t == &who).is_none(), "Cannot nominate if already staked.");
+
+			// update nominators_for
+			let mut t = Self::nominators_for(&target);
+			t.push(who.clone());
+			<NominatorsFor<T>>::insert(&target, t);
+
+			// update nominating
+			<Nominating<T>>::insert(&who, &target);
+
+			// Update bondage
+			<Bondage<T>>::insert(&who, T::BlockNumber::max_value());
+
+			Ok(())
+		}
+
+		/// Will panic if called when source isn't currently nominating target.
+		/// Updates Nominating, NominatorsFor and NominationBalance.
+		fn unnominate(origin, target_index: Compact<u32>) -> Result {
+			let source = ensure_signed(origin)?;
+			let target_index: u32 = target_index.into();
+			let target_index = target_index as usize;
+
+			let target = <Nominating<T>>::get(&source).ok_or("Account must be nominating")?;
+
+			let mut t = Self::nominators_for(&target);
+			if t.get(target_index) != Some(&source) {
+				return Err("Invalid target index")
+			}
+
+			// Ok - all valid.
+
+			// update nominators_for
+			t.swap_remove(target_index);
+			<NominatorsFor<T>>::insert(&target, t);
+
+			// update nominating
+			<Nominating<T>>::remove(&source);
+
+			// update bondage
+			<Bondage<T>>::insert(
+				source,
+				<system::Module<T>>::block_number() + Self::bonding_duration()
+			);
+			Ok(())
+		}
+
+		/// Set the given account's preference for slashing behaviour should they be a validator.
+		///
+		/// An error (no-op) if `Self::intentions()[intentions_index] != origin`.
+		fn register_preferences(
+			origin,
+			intentions_index: Compact<u32>,
+			prefs: ValidatorPrefs<T::Balance>
+		) -> Result {
+			let who = ensure_signed(origin)?;
+			let intentions_index: u32 = intentions_index.into();
+
+			if Self::intentions().get(intentions_index as usize) != Some(&who) {
+				return Err("Invalid index")
+			}
+
+			<ValidatorPreferences<T>>::insert(who, prefs);
+
+			Ok(())
+		}
+
+		/// Set the number of sessions in an era.
+		fn set_sessions_per_era(new: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<NextSessionsPerEra<T>>::put(new.into());
+			Ok(())
+		}
+
+		/// The length of the bonding duration in eras.
+		fn set_bonding_duration(new: <T::BlockNumber as HasCompact>::Type) -> Result {
+			<BondingDuration<T>>::put(new.into());
+			Ok(())
+		}
+
+		/// The length of a staking era in sessions.
+		fn set_validator_count(new: Compact<u32>) -> Result {
+			let new: u32 = new.into();
+			<ValidatorCount<T>>::put(new);
+			Ok(())
+		}
+
+		/// Force there to be a new era. This also forces a new session immediately after.
+		/// `apply_rewards` should be true for validators to get the session reward.
+		fn force_new_era(apply_rewards: bool) -> Result {
+			Self::apply_force_new_era(apply_rewards)
+		}
+
+		/// Set the offline slash grace period.
+		fn set_offline_slash_grace(new: Compact<u32>) -> Result {
+			let new: u32 = new.into();
+			<OfflineSlashGrace<T>>::put(new);
+			Ok(())
+		}
 	}
 }
 
@@ -189,10 +312,10 @@ decl_storage! {
 }
 
 impl<T: Trait> Module<T> {
-
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
+	// Just force_new_era without origin check.
+	fn apply_force_new_era(apply_rewards: bool) -> Result {
+		<ForcingNewEra<T>>::put(());
+		<session::Module<T>>::apply_force_new_session(apply_rewards)
 	}
 
 	// PUBLIC IMMUTABLES
@@ -225,147 +348,6 @@ impl<T: Trait> Module<T> {
 		}
 	}
 
-	// PUBLIC DISPATCH
-
-	/// Declare the desire to stake for the transactor.
-	///
-	/// Effects will be felt at the beginning of the next era.
-	fn stake(origin: T::Origin) -> Result {
-		let who = ensure_signed(origin)?;
-		ensure!(Self::nominating(&who).is_none(), "Cannot stake if already nominating.");
-		let mut intentions = <Intentions<T>>::get();
-		// can't be in the list twice.
-		ensure!(intentions.iter().find(|&t| t == &who).is_none(), "Cannot stake if already staked.");
-
-		<Bondage<T>>::insert(&who, T::BlockNumber::max_value());
-		intentions.push(who);
-		<Intentions<T>>::put(intentions);
-		Ok(())
-	}
-
-	/// Retract the desire to stake for the transactor.
-	///
-	/// Effects will be felt at the beginning of the next era.
-	fn unstake(origin: T::Origin, intentions_index: Compact<u32>) -> Result {
-		let who = ensure_signed(origin)?;
-		let intentions_index: u32 = intentions_index.into();
-		// unstake fails in degenerate case of having too few existing staked parties
-		if Self::intentions().len() <= Self::minimum_validator_count() as usize {
-			return Err("cannot unstake when there are too few staked participants")
-		}
-		Self::apply_unstake(&who, intentions_index as usize)
-	}
-
-	fn nominate(origin: T::Origin, target: Address<T::AccountId, T::AccountIndex>) -> Result {
-		let who = ensure_signed(origin)?;
-		let target = <balances::Module<T>>::lookup(target)?;
-
-		ensure!(Self::nominating(&who).is_none(), "Cannot nominate if already nominating.");
-		ensure!(Self::intentions().iter().find(|&t| t == &who).is_none(), "Cannot nominate if already staked.");
-
-		// update nominators_for
-		let mut t = Self::nominators_for(&target);
-		t.push(who.clone());
-		<NominatorsFor<T>>::insert(&target, t);
-
-		// update nominating
-		<Nominating<T>>::insert(&who, &target);
-
-		// Update bondage
-		<Bondage<T>>::insert(&who, T::BlockNumber::max_value());
-
-		Ok(())
-	}
-
-	/// Will panic if called when source isn't currently nominating target.
-	/// Updates Nominating, NominatorsFor and NominationBalance.
-	fn unnominate(origin: T::Origin, target_index: Compact<u32>) -> Result {
-		let source = ensure_signed(origin)?;
-		let target_index: u32 = target_index.into();
-		let target_index = target_index as usize;
-
-		let target = <Nominating<T>>::get(&source).ok_or("Account must be nominating")?;
-
-		let mut t = Self::nominators_for(&target);
-		if t.get(target_index) != Some(&source) {
-			return Err("Invalid target index")
-		}
-
-		// Ok - all valid.
-
-		// update nominators_for
-		t.swap_remove(target_index);
-		<NominatorsFor<T>>::insert(&target, t);
-
-		// update nominating
-		<Nominating<T>>::remove(&source);
-
-		// update bondage
-		<Bondage<T>>::insert(source, <system::Module<T>>::block_number() + Self::bonding_duration());
-		Ok(())
-	}
-
-	/// Set the given account's preference for slashing behaviour should they be a validator.
-	///
-	/// An error (no-op) if `Self::intentions()[intentions_index] != origin`.
-	fn register_preferences(
-		origin: T::Origin,
-		intentions_index: Compact<u32>,
-		prefs: ValidatorPrefs<T::Balance>
-	) -> Result {
-		let who = ensure_signed(origin)?;
-		let intentions_index: u32 = intentions_index.into();
-
-		if Self::intentions().get(intentions_index as usize) != Some(&who) {
-			return Err("Invalid index")
-		}
-
-		<ValidatorPreferences<T>>::insert(who, prefs);
-
-		Ok(())
-	}
-
-	// PRIV DISPATCH
-
-	/// Set the number of sessions in an era.
-	fn set_sessions_per_era(new: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<NextSessionsPerEra<T>>::put(new.into());
-		Ok(())
-	}
-
-	/// The length of the bonding duration in eras.
-	fn set_bonding_duration(new: <T::BlockNumber as HasCompact>::Type) -> Result {
-		<BondingDuration<T>>::put(new.into());
-		Ok(())
-	}
-
-	/// The length of a staking era in sessions.
-	fn set_validator_count(new: Compact<u32>) -> Result {
-		let new: u32 = new.into();
-		<ValidatorCount<T>>::put(new);
-		Ok(())
-	}
-
-	/// Force there to be a new era. This also forces a new session immediately after.
-	/// `apply_rewards` should be true for validators to get the session reward.
-	fn force_new_era(apply_rewards: bool) -> Result {
-		Self::apply_force_new_era(apply_rewards)
-	}
-
-	// Just force_new_era without origin check.
-	fn apply_force_new_era(apply_rewards: bool) -> Result {
-		<ForcingNewEra<T>>::put(());
-		<session::Module<T>>::apply_force_new_session(apply_rewards)
-	}
-
-
-	/// Set the offline slash grace period.
-	fn set_offline_slash_grace(new: Compact<u32>) -> Result {
-		let new: u32 = new.into();
-		<OfflineSlashGrace<T>>::put(new);
-		Ok(())
-	}
-
 	// PUBLIC MUTABLES (DANGEROUS)
 
 	/// Slash a given validator by a specific amount. Removes the slash from their balance by preference,

substrate/srml/support/src/dispatch.rs

@@ -75,7 +75,8 @@ macro_rules! decl_module {
 		decl_module!(@normalize
 			$(#[$attr])*
 			pub struct $mod_type<$trait_instance: $trait_name>
-			for enum $call_type where origin: $origin_type where system = system
+			for enum $call_type where origin: $origin_type, system = system
+			{}
 			{}
 			[]
 			$($t)*
@@ -84,14 +85,15 @@ macro_rules! decl_module {
 	(
 		$(#[$attr:meta])*
 		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
-		for enum $call_type:ident where origin: $origin_type:ty where system = $system:ident {
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident {
 			$($t:tt)*
 		}
 	) => {
 		decl_module!(@normalize
 			$(#[$attr])*
 			pub struct $mod_type<$trait_instance: $trait_name>
-			for enum $call_type where origin: $origin_type where system = $system
+			for enum $call_type where origin: $origin_type, system = $system
+			{}
 			{}
 			[]
 			$($t)*
@@ -101,17 +103,61 @@ macro_rules! decl_module {
 	(@normalize
 		$(#[$attr:meta])*
 		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
-		for enum $call_type:ident where origin: $origin_type:ty where system = $system:ident
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident
+		{}
 		{ $( $on_finalise:tt )* }
 		[ $($t:tt)* ]
 		$(#[doc = $doc_attr:tt])*
+		$vis:vis fn deposit_event() = default;
+		$($rest:tt)*
+	) => {
+		decl_module!(@normalize
+			$(#[$attr])*
+			pub struct $mod_type<$trait_instance: $trait_name>
+			for enum $call_type where origin: $origin_type, system = $system
+			{ $vis fn deposit_event() = default; }
+			{ $( $on_finalise )* }
+			[ $($t)* ]
+			$($rest)*
+		);
+	};
+	(@normalize
+		$(#[$attr:meta])*
+		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident
+		{}
+		{ $( $on_finalise:tt )* }
+		[ $($t:tt)* ]
+		$(#[doc = $doc_attr:tt])*
+		$vis:vis fn deposit_event($($param_name:ident : $param:ty),* ) { $( $impl:tt )* }
+		$($rest:tt)*
+	) => {
+		decl_module!(@normalize
+			$(#[$attr])*
+			pub struct $mod_type<$trait_instance: $trait_name>
+			for enum $call_type where origin: $origin_type, system = $system
+			{ $vis fn deposit_event($( $param_name: $param ),* ) { $( $impl )* } }
+			{ $( $on_finalise )* }
+			[ $($t)* ]
+			$($rest)*
+		);
+	};
+	(@normalize
+		$(#[$attr:meta])*
+		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident
+	    { $( $deposit_event:tt )* }
+		{}
+		[ $($t:tt)* ]
+		$(#[doc = $doc_attr:tt])*
 		fn on_finalise($($param_name:ident : $param:ty),* ) { $( $impl:tt )* }
 		$($rest:tt)*
 	) => {
 		decl_module!(@normalize
 			$(#[$attr])*
 			pub struct $mod_type<$trait_instance: $trait_name>
-			for enum $call_type where origin: $origin_type where system = $system
+			for enum $call_type where origin: $origin_type, system = $system
+			{ $( $deposit_event )* }
 			{ fn on_finalise( $( $param_name : $param ),* ) { $( $impl )* } }
 			[ $($t)* ]
 			$($rest)*
@@ -120,64 +166,72 @@ macro_rules! decl_module {
 	(@normalize
 		$(#[$attr:meta])*
 		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
-		for enum $call_type:ident where origin: $origin_type:ty where system = $system:ident
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident
+		{ $( $deposit_event:tt )* }
 		{ $( $on_finalise:tt )* }
 		[ $($t:tt)* ]
 		$(#[doc = $doc_attr:tt])*
-		fn $fn_name:ident(origin $(, $param_name:ident : $param:ty)* ) -> $result:ty ;
+		$fn_vis:vis fn $fn_name:ident($origin:ident $(, $param_name:ident : $param:ty)* ) -> $result:ty { $( $impl:tt )* }
 		$($rest:tt)*
 	) => {
 		decl_module!(@normalize
 			$(#[$attr])*
 			pub struct $mod_type<$trait_instance: $trait_name>
-			for enum $call_type where origin: $origin_type where system = $system
+			for enum $call_type where origin: $origin_type, system = $system
+			{ $( $deposit_event )* }
 			{ $( $on_finalise )* }
-			[ $($t)* $(#[doc = $doc_attr])* fn $fn_name(origin $( , $param_name : $param )* ) -> $result; ]
+			[
+				$($t)*
+				$(#[doc = $doc_attr])*
+				$fn_vis fn $fn_name($origin $( , $param_name : $param )* ) -> $result { $( $impl )* }
+			]
 			$($rest)*
 		);
 	};
 	(@normalize
 		$(#[$attr:meta])*
 		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
-		for enum $call_type:ident where origin: $origin_type:ty where system = $system:ident
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident
+		{ $( $deposit_event:tt )* }
 		{ $( $on_finalise:tt )* }
 		[ $($t:tt)* ]
 		$(#[doc = $doc_attr:tt])*
-		fn $fn_name:ident($( $param_name:ident : $param:ty),* ) -> $result:ty ;
+		$fn_vis:vis fn $fn_name:ident($( $param_name:ident : $param:ty),* ) -> $result:ty { $( $impl:tt )* }
 		$($rest:tt)*
 	) => {
 		decl_module!(@normalize
 			$(#[$attr])*
 			pub struct $mod_type<$trait_instance: $trait_name>
-			for enum $call_type where origin: $origin_type where system = $system
+			for enum $call_type where origin: $origin_type, system = $system
+			{ $( $deposit_event )* }
 			{ $( $on_finalise )* }
-			[ $($t)* $(#[doc = $doc_attr])* fn $fn_name(root $( , $param_name : $param )* ) -> $result; ]
+			[
+				$($t)*
+				$(#[doc = $doc_attr])*
+				fn $fn_name(root $( , $param_name : $param )* ) -> $result { $( $impl )* }
+			]
 			$($rest)*
 		);
 	};
 	(@normalize
 		$(#[$attr:meta])*
 		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
-		for enum $call_type:ident where origin: $origin_type:ty where system = $system:ident
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident
+		{ $( $deposit_event:tt )* }
 		{ $( $on_finalise:tt )* }
 		[ $($t:tt)* ]
 	) => {
 		decl_module!(@imp
 			$(#[$attr])*
 			pub struct $mod_type<$trait_instance: $trait_name>
-			for enum $call_type where origin: $origin_type where system = $system {
+			for enum $call_type where origin: $origin_type, system = $system {
 				$($t)*
 			}
+			{ $( $deposit_event )* }
 			{ $( $on_finalise )* }
 		);
 	};
 
-	(@call
-		origin
-		$mod_type:ident $trait_instance:ident $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ]
-	) => {
-		<$mod_type<$trait_instance>>::$fn_name( $origin $(, $param_name )* )
-	};
 	(@call
 		root
 		$mod_type:ident $trait_instance:ident $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ]
@@ -187,6 +241,44 @@ macro_rules! decl_module {
 			<$mod_type<$trait_instance>>::$fn_name( $( $param_name ),* )
 		}
 	};
+	(@call
+		$ingore:ident
+		$mod_type:ident $trait_instance:ident $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ]
+	) => {
+		<$mod_type<$trait_instance>>::$fn_name( $origin $(, $param_name )* )
+	};
+
+	// no `deposit_event` function wanted
+	(@impl_deposit_event
+		$module:ident<$trait_instance:ident: $trait_name:ident>;
+		$system:ident;
+	) => {};
+
+	(@impl_deposit_event
+		$module:ident<$trait_instance:ident: $trait_name:ident>;
+		$system:ident;
+		$vis:vis fn deposit_event() = default;
+	) => {
+		impl<$trait_instance: $trait_name> $module<$trait_instance> {
+			$vis fn deposit_event(event: Event<$trait_instance>) {
+				<$system::Module<$trait_instance>>::deposit_event(
+					<$trait_instance as $trait_name>::Event::from(event).into()
+				);
+			}
+		}
+	};
+
+	(@impl_deposit_event
+		$module:ident<$trait_instance:ident: $trait_name:ident>;
+		$system:ident;
+		$vis:vis fn deposit_event($param:ident : $param_ty:ty) { $( $impl:tt )* }
+	) => {
+		impl<$trait_instance: $trait_name> $module<$trait_instance> {
+			$vis fn deposit_event($param: $param_ty) {
+				$( $impl )*
+			}
+		}
+	};
 
 	(@impl_on_finalise
 		$module:ident<$trait_instance:ident: $trait_name:ident>;
@@ -218,15 +310,43 @@ macro_rules! decl_module {
 			for $module<$trait_instance> {}
 	};
 
+	(@impl_function
+		$module:ident<$trait_instance:ident: $trait_name:ident>;
+		$origin_ty:ty;
+		root;
+		$vis:vis fn $name:ident ( root $(, $param:ident : $param_ty:ty )* ) -> $result:ty { $( $impl:tt )* }
+	) => {
+		impl<$trait_instance: $trait_name> $module<$trait_instance> {
+			$vis fn $name($( $param: $param_ty ),* ) -> $result {
+				$( $impl )*
+			}
+		}
+	};
+	(@impl_function
+		$module:ident<$trait_instance:ident: $trait_name:ident>;
+		$origin_ty:ty;
+		$ignore:ident;
+		$vis:vis fn $name:ident ( $origin:ident $(, $param:ident : $param_ty:ty )* ) -> $result:ty { $( $impl:tt )* }
+	) => {
+		impl<$trait_instance: $trait_name> $module<$trait_instance> {
+			$vis fn $name($origin: $origin_ty $(, $param: $param_ty )* ) -> $result {
+				$( $impl )*
+			}
+		}
+	};
+
 	(@imp
 		$(#[$attr:meta])*
 		pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident>
-		for enum $call_type:ident where origin: $origin_type:ty where system = $system:ident {
+		for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident {
 			$(
 				$(#[doc = $doc_attr:tt])*
-				fn $fn_name:ident($from:ident $( , $param_name:ident : $param:ty)*) -> $result:ty;
+				$fn_vis:vis fn $fn_name:ident(
+					$from:ident $( , $param_name:ident : $param:ty)*
+				) -> $result:ty { $( $impl:tt )* }
 			)*
 		}
+		{ $( $deposit_event:tt )* }
 		{ $( $on_finalise:tt )* }
 	) => {
 		// Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted.
@@ -250,6 +370,23 @@ macro_rules! decl_module {
 			$( $on_finalise )*
 		}
 
+		decl_module! {
+			@impl_deposit_event
+			$mod_type<$trait_instance: $trait_name>;
+			$system;
+			$( $deposit_event )*
+		}
+
+		$(
+			decl_module! {
+				@impl_function
+				$mod_type<$trait_instance: $trait_name>;
+				$origin_type;
+				$from;
+				$fn_vis fn $fn_name ($from $(, $param_name : $param )* ) -> $result { $( $impl )* }
+			}
+		)*
+
 		#[cfg(feature = "std")]
 		$(#[$attr])*
 		#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
@@ -357,7 +494,11 @@ macro_rules! decl_module {
 				match self {
 					$(
 						$call_type::$fn_name( $( $param_name ),* ) => {
-							decl_module!(@call $from $mod_type $trait_instance $fn_name _origin $system [ $( $param_name ),* ])
+							decl_module!(
+								@call
+								$from
+								$mod_type $trait_instance $fn_name _origin $system [ $( $param_name ),* ]
+							)
 						},
 					)*
 					_ => { panic!("__PhantomItem should never be used.") },
@@ -694,11 +835,11 @@ mod tests {
 	decl_module! {
 		pub struct Module<T: Trait> for enum Call where origin: T::Origin {
 			/// Hi, this is a comment.
-			fn aux_0(origin) -> Result;
-			fn aux_1(origin, data: i32) -> Result;
-			fn aux_2(origin, data: i32, data2: String) -> Result;
-			fn aux_3() -> Result;
-			fn aux_4(data: i32) -> Result;
+			fn aux_0(_origin) -> Result { unreachable!() }
+			fn aux_1(_origin, _data: i32) -> Result { unreachable!() }
+			fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() }
+			fn aux_3() -> Result { unreachable!() }
+			fn aux_4(_data: i32) -> Result { unreachable!() }
 		}
 	}
 
@@ -720,7 +861,7 @@ mod tests {
 					name: DecodeDifferent::Encode("aux_1"),
 					arguments: DecodeDifferent::Encode(&[
 						FunctionArgumentMetadata {
-							name: DecodeDifferent::Encode("data"),
+							name: DecodeDifferent::Encode("_data"),
 							ty: DecodeDifferent::Encode("i32"),
 						}
 					]),
@@ -731,11 +872,11 @@ mod tests {
 					name: DecodeDifferent::Encode("aux_2"),
 					arguments: DecodeDifferent::Encode(&[
 						FunctionArgumentMetadata {
-							name: DecodeDifferent::Encode("data"),
+							name: DecodeDifferent::Encode("_data"),
 							ty: DecodeDifferent::Encode("i32"),
 						},
 						FunctionArgumentMetadata {
-							name: DecodeDifferent::Encode("data2"),
+							name: DecodeDifferent::Encode("_data2"),
 							ty: DecodeDifferent::Encode("String"),
 						}
 					]),
@@ -752,7 +893,7 @@ mod tests {
 					name: DecodeDifferent::Encode("aux_4"),
 					arguments: DecodeDifferent::Encode(&[
 						FunctionArgumentMetadata {
-							name: DecodeDifferent::Encode("data"),
+							name: DecodeDifferent::Encode("_data"),
 							ty: DecodeDifferent::Encode("i32"),
 						}
 					]),
@@ -762,28 +903,6 @@ mod tests {
 		},
 	};
 
-	impl<T: Trait> Module<T> {
-		fn aux_0(_: T::Origin) -> Result {
-			unreachable!()
-		}
-
-		fn aux_1(_: T::Origin, _: i32) -> Result {
-			unreachable!()
-		}
-
-		fn aux_2(_: T::Origin, _: i32, _: String) -> Result {
-			unreachable!()
-		}
-
-		fn aux_3() -> Result {
-			unreachable!()
-		}
-
-		fn aux_4(_: i32) -> Result {
-			unreachable!()
-		}
-	}
-
 	struct TraitImpl {}
 
 	impl Trait for TraitImpl {

substrate/srml/support/src/metadata.rs

@@ -160,13 +160,7 @@ mod tests {
 
 		decl_module! {
 			pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-				fn aux_0(origin) -> Result;
-			}
-		}
-
-		impl<T: Trait> Module<T> {
-			fn aux_0(_: T::Origin) -> Result {
-				unreachable!()
+				fn aux_0(_origin) -> Result { unreachable!() }
 			}
 		}
 	}

substrate/srml/system/src/lib.rs

@@ -86,7 +86,16 @@ pub trait Trait: Eq + Clone {
 pub type DigestItemOf<T> = <<T as Trait>::Digest as traits::Digest>::Item;
 
 decl_module! {
-	pub struct Module<T: Trait> for enum Call where origin: T::Origin {}
+	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		/// Deposits an event onto this block's event record.
+		pub fn deposit_event(event: T::Event) {
+			let extrinsic_index = Self::extrinsic_index();
+			let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c));
+			let mut events = Self::events();
+			events.push(EventRecord { phase, event });
+			<Events<T>>::put(events);
+		}
+	}
 }
 
 /// A phase of a block's execution.
@@ -301,15 +310,6 @@ impl<T: Trait> Module<T> {
 		<Digest<T>>::put(l);
 	}
 
-	/// Deposits an event onto this block's event record.
-	pub fn deposit_event(event: T::Event) {
-		let extrinsic_index = Self::extrinsic_index();
-		let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c));
-		let mut events = Self::events();
-		events.push(EventRecord { phase, event });
-		<Events<T>>::put(events);
-	}
-
 	/// Calculate the current block's random seed.
 	fn calculate_random() -> T::Hash {
 		assert!(Self::block_number() > Zero::zero(), "Block number may never be zero");

substrate/srml/timestamp/src/lib.rs

@@ -74,7 +74,33 @@ pub trait Trait: consensus::Trait + system::Trait {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		fn set(origin, now: <T::Moment as HasCompact>::Type) -> Result;
+		/// Set the current time.
+		///
+		/// Extrinsic with this call should be placed at the specific position in the each block
+		/// (specified by the Trait::TIMESTAMP_SET_POSITION) typically at the start of the each block.
+		/// This call should be invoked exactly once per block. It will panic at the finalization phase,
+		/// if this call hasn't been invoked by that time.
+		///
+		/// The timestamp should be greater than the previous one by the amount specified by `block_period`.
+		fn set(origin, now: <T::Moment as HasCompact>::Type) -> Result {
+			ensure_inherent(origin)?;
+			let now = now.into();
+
+			assert!(!<Self as Store>::DidUpdate::exists(), "Timestamp must be updated only once in the block");
+			assert!(
+				<system::Module<T>>::extrinsic_index() == Some(T::TIMESTAMP_SET_POSITION),
+				"Timestamp extrinsic must be at position {} in the block",
+				T::TIMESTAMP_SET_POSITION
+			);
+			assert!(
+				Self::now().is_zero() || now >= Self::now() + Self::block_period(),
+				"Timestamp must increment by at least <BlockPeriod> between sequential blocks"
+			);
+			<Self as Store>::Now::put(now);
+			<Self as Store>::DidUpdate::put(true);
+			Ok(())
+		}
+
 		fn on_finalise() {
 			assert!(<Self as Store>::DidUpdate::take(), "Timestamp must be updated once in the block");
 		}
@@ -103,33 +129,6 @@ impl<T: Trait> Module<T> {
 		Self::now()
 	}
 
-	/// Set the current time.
-	///
-	/// Extrinsic with this call should be placed at the specific position in the each block
-	/// (specified by the Trait::TIMESTAMP_SET_POSITION) typically at the start of the each block.
-	/// This call should be invoked exactly once per block. It will panic at the finalization phase,
-	/// if this call hasn't been invoked by that time.
-	///
-	/// The timestamp should be greater than the previous one by the amount specified by `block_period`.
-	fn set(origin: T::Origin, now: <T::Moment as HasCompact>::Type) -> Result {
-		ensure_inherent(origin)?;
-		let now = now.into();
-
-		assert!(!<Self as Store>::DidUpdate::exists(), "Timestamp must be updated only once in the block");
-		assert!(
-			<system::Module<T>>::extrinsic_index() == Some(T::TIMESTAMP_SET_POSITION),
-			"Timestamp extrinsic must be at position {} in the block",
-			T::TIMESTAMP_SET_POSITION
-		);
-		assert!(
-			Self::now().is_zero() || now >= Self::now() + Self::block_period(),
-			"Timestamp must increment by at least <BlockPeriod> between sequential blocks"
-		);
-		<Self as Store>::Now::put(now);
-		<Self as Store>::DidUpdate::put(true);
-		Ok(())
-	}
-
 	/// Set the timestamp to something in particular. Only used for tests.
 	#[cfg(feature = "std")]
 	pub fn set_timestamp(now: T::Moment) {

substrate/srml/treasury/src/lib.rs

@@ -73,23 +73,80 @@ type ProposalIndex = u32;
 decl_module! {
 	// Simple declaration of the `Module` type. Lets the macro know what its working on.
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-		// Put forward a suggestion for spending. A deposit proportional to the value
-		// is reserved and slashed if the proposal is rejected. It is returned once the
-		// proposal is awarded.
-		fn propose_spend(origin, value: <T::Balance as HasCompact>::Type, beneficiary: Address<T::AccountId, T::AccountIndex>) -> Result;
+		fn deposit_event() = default;
+		/// Put forward a suggestion for spending. A deposit proportional to the value
+		/// is reserved and slashed if the proposal is rejected. It is returned once the
+		/// proposal is awarded.
+		fn propose_spend(
+			origin,
+			value: <T::Balance as HasCompact>::Type,
+			beneficiary: Address<T::AccountId, T::AccountIndex>
+		) -> Result {
+			let proposer = ensure_signed(origin)?;
+			let beneficiary = <balances::Module<T>>::lookup(beneficiary)?;
+			let value = value.into();
 
-		// Set the balance of funds available to spend.
-		fn set_pot(new_pot: <T::Balance as HasCompact>::Type) -> Result;
+			let bond = Self::calculate_bond(value);
+			<balances::Module<T>>::reserve(&proposer, bond)
+				.map_err(|_| "Proposer's balance too low")?;
 
-		// (Re-)configure this module.
-		fn configure(proposal_bond: Permill, proposal_bond_minimum: <T::Balance as HasCompact>::Type, spend_period: <T::BlockNumber as HasCompact>::Type, burn: Permill) -> Result;
+			let c = Self::proposal_count();
+			<ProposalCount<T>>::put(c + 1);
+			<Proposals<T>>::insert(c, Proposal { proposer, value, beneficiary, bond });
 
-		// Reject a proposed spend. The original deposit will be slashed.
-		fn reject_proposal(origin, proposal_id: Compact<ProposalIndex>) -> Result;
+			Self::deposit_event(RawEvent::Proposed(c));
 
-		// Approve a proposal. At a later time, the proposal will be allocated to the beneficiary
-		// and the original deposit will be returned.
-		fn approve_proposal(origin, proposal_id: Compact<ProposalIndex>) -> Result;
+			Ok(())
+		}
+
+		/// Set the balance of funds available to spend.
+		fn set_pot(new_pot: <T::Balance as HasCompact>::Type) -> Result {
+			// Put the new value into storage.
+			<Pot<T>>::put(new_pot.into());
+
+			// All good.
+			Ok(())
+		}
+
+		/// (Re-)configure this module.
+		fn configure(
+			proposal_bond: Permill,
+			proposal_bond_minimum: <T::Balance as HasCompact>::Type,
+			spend_period: <T::BlockNumber as HasCompact>::Type,
+			burn: Permill
+		) -> Result {
+			<ProposalBond<T>>::put(proposal_bond);
+			<ProposalBondMinimum<T>>::put(proposal_bond_minimum.into());
+			<SpendPeriod<T>>::put(spend_period.into());
+			<Burn<T>>::put(burn);
+			Ok(())
+		}
+
+		/// Reject a proposed spend. The original deposit will be slashed.
+		fn reject_proposal(origin, proposal_id: Compact<ProposalIndex>) -> Result {
+			T::RejectOrigin::ensure_origin(origin)?;
+			let proposal_id: ProposalIndex = proposal_id.into();
+
+			let proposal = <Proposals<T>>::take(proposal_id).ok_or("No proposal at that index")?;
+
+			let value = proposal.bond;
+			let _ = <balances::Module<T>>::slash_reserved(&proposal.proposer, value);
+
+			Ok(())
+		}
+
+		/// Approve a proposal. At a later time, the proposal will be allocated to the beneficiary
+		/// and the original deposit will be returned.
+		fn approve_proposal(origin, proposal_id: Compact<ProposalIndex>) -> Result {
+			T::ApproveOrigin::ensure_origin(origin)?;
+			let proposal_id = proposal_id.into();
+
+			ensure!(<Proposals<T>>::exists(proposal_id), "No proposal at that index");
+
+			<Approvals<T>>::mutate(|v| v.push(proposal_id));
+
+			Ok(())
+		}
 
 		fn on_finalise(n: T::BlockNumber) {
 			// Check to see if we should spend some funds!
@@ -160,74 +217,7 @@ decl_event!(
 );
 
 impl<T: Trait> Module<T> {
-	/// Deposit one of this module's events.
-	fn deposit_event(event: Event<T>) {
-		<system::Module<T>>::deposit_event(<T as Trait>::Event::from(event).into());
-	}
-
-	// Implement Calls and add public immutables and private mutables.
-
-	fn propose_spend(origin: T::Origin, value: <T::Balance as HasCompact>::Type, beneficiary: Address<T::AccountId, T::AccountIndex>) -> Result {
-		let proposer = ensure_signed(origin)?;
-		let beneficiary = <balances::Module<T>>::lookup(beneficiary)?;
-		let value = value.into();
-
-		let bond = Self::calculate_bond(value);
-		<balances::Module<T>>::reserve(&proposer, bond)
-			.map_err(|_| "Proposer's balance too low")?;
-
-		let c = Self::proposal_count();
-		<ProposalCount<T>>::put(c + 1);
-		<Proposals<T>>::insert(c, Proposal { proposer, value, beneficiary, bond });
-
-		Self::deposit_event(RawEvent::Proposed(c));
-
-		Ok(())
-	}
-
-	fn reject_proposal(origin: T::Origin, proposal_id: Compact<ProposalIndex>) -> Result {
-		T::RejectOrigin::ensure_origin(origin)?;
-		let proposal_id: ProposalIndex = proposal_id.into();
-
-		let proposal = <Proposals<T>>::take(proposal_id).ok_or("No proposal at that index")?;
-
-		let value = proposal.bond;
-		let _ = <balances::Module<T>>::slash_reserved(&proposal.proposer, value);
-
-		Ok(())
-	}
-
-	fn approve_proposal(origin: T::Origin, proposal_id: Compact<ProposalIndex>) -> Result {
-		T::ApproveOrigin::ensure_origin(origin)?;
-		let proposal_id = proposal_id.into();
-
-		ensure!(<Proposals<T>>::exists(proposal_id), "No proposal at that index");
-
-		<Approvals<T>>::mutate(|v| v.push(proposal_id));
-
-		Ok(())
-	}
-
-	fn set_pot(new_pot: <T::Balance as HasCompact>::Type) -> Result {
-		// Put the new value into storage.
-		<Pot<T>>::put(new_pot.into());
-
-		// All good.
-		Ok(())
-	}
-
-	fn configure(
-		proposal_bond: Permill,
-		proposal_bond_minimum: <T::Balance as HasCompact>::Type,
-		spend_period: <T::BlockNumber as HasCompact>::Type,
-		burn: Permill
-	) -> Result {
-		<ProposalBond<T>>::put(proposal_bond);
-		<ProposalBondMinimum<T>>::put(proposal_bond_minimum.into());
-		<SpendPeriod<T>>::put(spend_period.into());
-		<Burn<T>>::put(burn);
-		Ok(())
-	}
+	// Add public immutables and private mutables.
 
 	/// The needed bond for a proposal whose spend is `value`.
 	fn calculate_bond(value: T::Balance) -> T::Balance {