Multiple improvements to the decl_module! macro

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)]

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.
 	///

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.