Make Pallet ModuleId and LockIdentifier Configurable (#5695)

* transition treasury to configurable moduleids

* make election module id configurable

* convert runtime and pallet to accept module id config elections-phragmen

* add ModuleId to evm pallet

* change society pallet to configurable module id

* delete commented out module_id

* delete commented out code and merge in upstream  master

* try and convert 4 whitespace to tab

* fix remaining space to tab conversions

* trivial cleaning

* delete comment from elections-phragrems tests

* trivial

* Update frame/elections-phragmen/src/lib.rs

* add docs for elections and elections phragmen

* make has_lock test get moduleid dynamically

* Apply suggestions from code review

Co-Authored-By: Amar Singh <asinghchrony@protonmail.com>

* make sure get is imported to evm

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Amar Singh <asinghchrony@protonmail.com>
Co-authored-by: Benjamin Kampmann <ben@gnunicorn.org>

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

@@ -12,7 +12,7 @@
 // GNU General Public License for more details.
 
 // You should have received a copy of the GNU General Public License
-// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+// along with Substrate. If not, see <http://www.gnu.org/licenses/>.
 
 //! The Substrate runtime. This can be compiled with ``#[no_std]`, ready for Wasm.
 
@@ -24,7 +24,7 @@ use sp_std::prelude::*;
 use frame_support::{
 	construct_runtime, parameter_types, debug,
 	weights::{Weight, RuntimeDbWeight},
-	traits::{Currency, Randomness, OnUnbalanced, Imbalance},
+	traits::{Currency, Randomness, OnUnbalanced, Imbalance, LockIdentifier},
 };
 use sp_core::u32_trait::{_1, _2, _3, _4};
 pub use node_primitives::{AccountId, Signature};
@@ -32,7 +32,7 @@ use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment};
 use sp_api::impl_runtime_apis;
 use sp_runtime::{
 	Permill, Perbill, Perquintill, Percent, ApplyExtrinsicResult,
-	impl_opaque_keys, generic, create_runtime_str,
+	impl_opaque_keys, generic, create_runtime_str, ModuleId,
 };
 use sp_runtime::curve::PiecewiseLinear;
 use sp_runtime::transaction_validity::{TransactionValidity, TransactionSource, TransactionPriority};
@@ -387,9 +387,11 @@ parameter_types! {
 	pub const TermDuration: BlockNumber = 7 * DAYS;
 	pub const DesiredMembers: u32 = 13;
 	pub const DesiredRunnersUp: u32 = 7;
+	pub const ElectionsPhragmenModuleId: LockIdentifier = *b"phrelect";
 }
 
 impl pallet_elections_phragmen::Trait for Runtime {
+	type ModuleId = ElectionsPhragmenModuleId;
 	type Event = Event;
 	type Currency = Balances;
 	type ChangeMembers = Council;
@@ -439,6 +441,7 @@ parameter_types! {
 	pub const TipFindersFee: Percent = Percent::from_percent(20);
 	pub const TipReportDepositBase: Balance = 1 * DOLLARS;
 	pub const TipReportDepositPerByte: Balance = 1 * CENTS;
+	pub const TreasuryModuleId: ModuleId = ModuleId(*b"py/trsry");
 }
 
 impl pallet_treasury::Trait for Runtime {
@@ -456,6 +459,7 @@ impl pallet_treasury::Trait for Runtime {
 	type ProposalBondMinimum = ProposalBondMinimum;
 	type SpendPeriod = SpendPeriod;
 	type Burn = Burn;
+	type ModuleId = TreasuryModuleId;
 }
 
 parameter_types! {
@@ -625,6 +629,7 @@ parameter_types! {
 	pub const PeriodSpend: Balance = 500 * DOLLARS;
 	pub const MaxLockDuration: BlockNumber = 36 * 30 * DAYS;
 	pub const ChallengePeriod: BlockNumber = 7 * DAYS;
+	pub const SocietyModuleId: ModuleId = ModuleId(*b"py/socie");
 }
 
 impl pallet_society::Trait for Runtime {
@@ -641,6 +646,7 @@ impl pallet_society::Trait for Runtime {
 	type FounderSetOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>;
 	type SuspensionJudgementOrigin = pallet_society::EnsureFounder<Runtime>;
 	type ChallengePeriod = ChallengePeriod;
+	type ModuleId = SocietyModuleId;
 }
 
 parameter_types! {

substrate/frame/elections-phragmen/src/lib.rs

@@ -98,8 +98,6 @@ use frame_support::{
 use sp_phragmen::{build_support_map, ExtendedBalance, VoteWeight, PhragmenResult};
 use frame_system::{self as system, ensure_signed, ensure_root};
 
-const MODULE_ID: LockIdentifier = *b"phrelect";
-
 /// The maximum votes allowed per voter.
 pub const MAXIMUM_VOTE: usize = 16;
 
@@ -111,6 +109,9 @@ pub trait Trait: frame_system::Trait {
 	/// The overarching event type.c
 	type Event: From<Event<Self>> + Into<<Self as frame_system::Trait>::Event>;
 
+	/// Identifier for the elections-phragmen pallet's lock
+	type ModuleId: Get<LockIdentifier>;
+
 	/// The currency that people are electing with.
 	type Currency:
 		LockableCurrency<Self::AccountId, Moment=Self::BlockNumber> +
@@ -276,6 +277,7 @@ decl_module! {
 		const DesiredMembers: u32 = T::DesiredMembers::get();
 		const DesiredRunnersUp: u32 = T::DesiredRunnersUp::get();
 		const TermDuration: T::BlockNumber = T::TermDuration::get();
+		const ModuleId: LockIdentifier  = T::ModuleId::get();
 
 		/// Vote for a set of candidates for the upcoming round of election.
 		///
@@ -321,7 +323,7 @@ decl_module! {
 
 			// lock
 			T::Currency::set_lock(
-				MODULE_ID,
+				T::ModuleId::get(),
 				&who,
 				locked_balance,
 				WithdrawReasons::except(WithdrawReason::TransactionPayment),
@@ -650,7 +652,7 @@ impl<T: Trait> Module<T> {
 	fn do_remove_voter(who: &T::AccountId, unreserve: bool) {
 		// remove storage and lock.
 		Voting::<T>::remove(who);
-		T::Currency::remove_lock(MODULE_ID, who);
+		T::Currency::remove_lock(T::ModuleId::get(), who);
 
 		if unreserve {
 			T::Currency::unreserve(who, T::VotingBond::get());
@@ -924,7 +926,7 @@ mod tests {
 
 	parameter_types! {
 		pub const ExistentialDeposit: u64 = 1;
-}
+	}
 
 	impl pallet_balances::Trait for Test {
 		type Balance = u64;
@@ -1021,7 +1023,12 @@ mod tests {
 		}
 	}
 
+	parameter_types!{
+		pub const ElectionsPhragmenModuleId: LockIdentifier = *b"phrelect";
+	}
+
 	impl Trait for Test {
+		type ModuleId = ElectionsPhragmenModuleId;
 		type Event = Event;
 		type Currency = Balances;
 		type CurrencyToVote = CurrencyToVoteHandler;
@@ -1125,7 +1132,7 @@ mod tests {
 
 	fn has_lock(who: &u64) -> u64 {
 		let lock = Balances::locks(who)[0].clone();
-		assert_eq!(lock.id, MODULE_ID);
+		assert_eq!(lock.id, ElectionsPhragmenModuleId::get());
 		lock.amount
 	}
 

substrate/frame/elections/src/lib.rs

@@ -33,7 +33,7 @@ use frame_support::{
 	weights::{Weight, MINIMUM_WEIGHT, DispatchClass},
 	traits::{
 		Currency, ExistenceRequirement, Get, LockableCurrency, LockIdentifier, BalanceStatus,
-		OnUnbalanced, ReservableCurrency, WithdrawReason, WithdrawReasons, ChangeMembers
+		OnUnbalanced, ReservableCurrency, WithdrawReason, WithdrawReasons, ChangeMembers,
 	}
 };
 use codec::{Encode, Decode};
@@ -126,8 +126,6 @@ pub enum CellStatus {
 	Hole,
 }
 
-const MODULE_ID: LockIdentifier = *b"py/elect";
-
 /// Number of voters grouped in one chunk.
 pub const VOTER_SET_SIZE: usize = 64;
 /// NUmber of approvals grouped in one chunk.
@@ -149,6 +147,9 @@ const APPROVAL_FLAG_LEN: usize = 32;
 pub trait Trait: frame_system::Trait {
 	type Event: From<Event<Self>> + Into<<Self as frame_system::Trait>::Event>;
 
+	/// Identifier for the elections pallet's lock
+	type ModuleId: Get<LockIdentifier>;
+
 	/// The currency that people are electing with.
 	type Currency:
 		LockableCurrency<Self::AccountId, Moment=Self::BlockNumber>
@@ -379,6 +380,8 @@ decl_module! {
 		/// The chunk size of the approval vector.
 		const APPROVAL_SET_SIZE: u32 = APPROVAL_SET_SIZE as u32;
 
+		const ModuleId: LockIdentifier = T::ModuleId::get();
+
 		fn deposit_event() = default;
 
 		/// Set candidate approvals. Approval slots stay valid as long as candidates in those slots
@@ -494,7 +497,7 @@ decl_module! {
 			);
 
 			T::Currency::remove_lock(
-				MODULE_ID,
+				T::ModuleId::get(),
 				if valid { &who } else { &reporter }
 			);
 
@@ -532,7 +535,7 @@ decl_module! {
 
 			Self::remove_voter(&who, index);
 			T::Currency::unreserve(&who, T::VotingBond::get());
-			T::Currency::remove_lock(MODULE_ID, &who);
+			T::Currency::remove_lock(T::ModuleId::get(), &who);
 		}
 
 		/// Submit oneself for candidacy.
@@ -892,7 +895,7 @@ impl<T: Trait> Module<T> {
 		}
 
 		T::Currency::set_lock(
-			MODULE_ID,
+			T::ModuleId::get(),
 			&who,
 			locked_balance,
 			WithdrawReasons::all(),

substrate/frame/elections/src/mock.rs

@@ -21,7 +21,7 @@
 use std::cell::RefCell;
 use frame_support::{
 	StorageValue, StorageMap, parameter_types, assert_ok,
-	traits::{Get, ChangeMembers, Currency},
+	traits::{Get, ChangeMembers, Currency, LockIdentifier},
 	weights::Weight,
 };
 use sp_core::H256;
@@ -122,6 +122,10 @@ impl ChangeMembers<u64> for TestChangeMembers {
 	}
 }
 
+parameter_types!{
+	pub const ElectionModuleId: LockIdentifier = *b"py/elect"; 
+}
+
 impl elections::Trait for Test {
 	type Event = Event;
 	type Currency = Balances;
@@ -139,6 +143,7 @@ impl elections::Trait for Test {
 	type InactiveGracePeriod = InactiveGracePeriod;
 	type VotingPeriod = VotingPeriod;
 	type DecayRatio = DecayRatio;
+	type ModuleId = ElectionModuleId;
 }
 
 pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;

substrate/frame/evm/src/lib.rs

@@ -26,7 +26,7 @@ pub use crate::backend::{Account, Log, Vicinity, Backend};
 use sp_std::{vec::Vec, marker::PhantomData};
 use frame_support::{ensure, decl_module, decl_storage, decl_event, decl_error};
 use frame_support::weights::{Weight, MINIMUM_WEIGHT, DispatchClass, FunctionOf, Pays};
-use frame_support::traits::{Currency, WithdrawReason, ExistenceRequirement};
+use frame_support::traits::{Currency, WithdrawReason, ExistenceRequirement, Get};
 use frame_system::{self as system, ensure_signed};
 use sp_runtime::ModuleId;
 use sp_core::{U256, H256, H160, Hasher};
@@ -38,8 +38,6 @@ use evm::{ExitReason, ExitSucceed, ExitError, Config};
 use evm::executor::StackExecutor;
 use evm::backend::ApplyBackend;
 
-const MODULE_ID: ModuleId = ModuleId(*b"py/ethvm");
-
 /// Type alias for currency balance.
 pub type BalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::Balance;
 
@@ -119,6 +117,8 @@ static ISTANBUL_CONFIG: Config = Config::istanbul();
 
 /// EVM module trait
 pub trait Trait: frame_system::Trait + pallet_timestamp::Trait {
+	/// The EVM's module id
+	type ModuleId: Get<ModuleId>;
 	/// Calculator for current gas price.
 	type FeeCalculator: FeeCalculator;
 	/// Convert account ID to H160;
@@ -188,6 +188,8 @@ decl_module! {
 		type Error = Error<T>;
 
 		fn deposit_event() = default;
+		
+		const ModuleId: ModuleId = T::ModuleId::get();
 
 		/// Deposit balance from currency/balances module into EVM.
 		#[weight = MINIMUM_WEIGHT]
@@ -361,7 +363,7 @@ impl<T: Trait> Module<T> {
 	/// This actually does computation. If you need to keep using it, then make sure you cache the
 	/// value and only call this once.
 	pub fn account_id() -> T::AccountId {
-		MODULE_ID.into_account()
+		T::ModuleId::get().into_account()
 	}
 
 	/// Check whether an account is empty.

substrate/frame/society/src/lib.rs

@@ -269,13 +269,14 @@ use frame_system::{self as system, ensure_signed, ensure_root};
 
 type BalanceOf<T, I> = <<T as Trait<I>>::Currency as Currency<<T as system::Trait>::AccountId>>::Balance;
 
-const MODULE_ID: ModuleId = ModuleId(*b"py/socie");
-
 /// The module's configuration trait.
 pub trait Trait<I=DefaultInstance>: system::Trait {
 	/// The overarching event type.
 	type Event: From<Event<Self, I>> + Into<<Self as system::Trait>::Event>;
 
+	/// The societies's module id
+	type ModuleId: Get<ModuleId>;
+
 	/// The currency type used for bidding.
 	type Currency: ReservableCurrency<Self::AccountId>;
 
@@ -491,6 +492,9 @@ decl_module! {
 		/// The number of blocks between membership challenges.
 		const ChallengePeriod: T::BlockNumber = T::ChallengePeriod::get();
 
+		/// The societies's module id
+		const ModuleId: ModuleId = T::ModuleId::get();
+
 		// Used for handling module events.
 		fn deposit_event() = default;
 
@@ -1570,7 +1574,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
 	/// This actually does computation. If you need to keep using it, then make sure you cache the
 	/// value and only call this once.
 	pub fn account_id() -> T::AccountId {
-		MODULE_ID.into_account()
+		T::ModuleId::get().into_account()
 	}
 
 	/// The account ID of the payouts pot. This is where payouts are made from.
@@ -1578,7 +1582,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
 	/// This actually does computation. If you need to keep using it, then make sure you cache the
 	/// value and only call this once.
 	pub fn payouts() -> T::AccountId {
-		MODULE_ID.into_sub_account(b"payouts")
+		T::ModuleId::get().into_sub_account(b"payouts")
 	}
 
 	/// Return the duration of the lock, in blocks, with the given number of members.

substrate/frame/society/src/mock.rs

@@ -55,6 +55,7 @@ parameter_types! {
 	pub const AvailableBlockRatio: Perbill = Perbill::one();
 
 	pub const ExistentialDeposit: u64 = 1;
+	pub const SocietyModuleId: ModuleId = ModuleId(*b"py/socie");
 }
 
 ord_parameter_types! {
@@ -107,6 +108,7 @@ impl Trait for Test {
 	type FounderSetOrigin = EnsureSignedBy<FounderSetAccount, u128>;
 	type SuspensionJudgementOrigin = EnsureSignedBy<SuspensionJudgementSetAccount, u128>;
 	type ChallengePeriod = ChallengePeriod;
+	type ModuleId = SocietyModuleId;
 }
 
 pub type Society = Module<Test>;

substrate/frame/treasury/src/lib.rs

@@ -110,10 +110,10 @@ type BalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trai
 type PositiveImbalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::PositiveImbalance;
 type NegativeImbalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::NegativeImbalance;
 
-/// The treasury's module id, used for deriving its sovereign account ID.
-const MODULE_ID: ModuleId = ModuleId(*b"py/trsry");
-
 pub trait Trait: frame_system::Trait {
+	/// The treasury's module id, used for deriving its sovereign account ID.
+	type ModuleId: Get<ModuleId>;
+
 	/// The staking balance.
 	type Currency: Currency<Self::AccountId> + ReservableCurrency<Self::AccountId>;
 
@@ -313,6 +313,9 @@ decl_module! {
 
 		/// The amount held on deposit per byte within the tip report reason.
 		const TipReportDepositPerByte: BalanceOf<T> = T::TipReportDepositPerByte::get();
+		
+		/// The treasury's module id, used for deriving its sovereign account ID.
+		const ModuleId: ModuleId = T::ModuleId::get();
 
 		type Error = Error<T>;
 
@@ -571,7 +574,7 @@ impl<T: Trait> Module<T> {
 	/// This actually does computation. If you need to keep using it, then make sure you cache the
 	/// value and only call this once.
 	pub fn account_id() -> T::AccountId {
-		MODULE_ID.into_account()
+		T::ModuleId::get().into_account()
 	}
 
 	/// The needed bond for a proposal whose spend is `value`.

substrate/frame/treasury/src/tests.rs

@@ -26,7 +26,7 @@ use frame_support::{
 };
 use sp_core::H256;
 use sp_runtime::{
-	Perbill,
+	Perbill, ModuleId,
 	testing::Header,
 	traits::{BlakeTwo256, IdentityLookup, BadOrigin},
 };
@@ -118,8 +118,10 @@ parameter_types! {
 	pub const TipFindersFee: Percent = Percent::from_percent(20);
 	pub const TipReportDepositBase: u64 = 1;
 	pub const TipReportDepositPerByte: u64 = 1;
+	pub const TreasuryModuleId: ModuleId = ModuleId(*b"py/trsry");
 }
 impl Trait for Test {
+	type ModuleId = TreasuryModuleId;
 	type Currency = pallet_balances::Module<Test>;
 	type ApproveOrigin = frame_system::EnsureRoot<u64>;
 	type RejectOrigin = frame_system::EnsureRoot<u64>;