Remove tight coupling to parachain system (#43)

Cargo.toml

@@ -19,12 +19,14 @@ sp-io = { git = "https://github.com/purestake/substrate", default-features = fal
 pallet-balances = { git = "https://github.com/purestake/substrate", branch = "crystalin-v0.9.9-block-response-length", default-features = false }
 pallet-utility = { git = "https://github.com/purestake/substrate", branch = "crystalin-v0.9.9-block-response-length", default-features = false }
 frame-benchmarking = { git = "https://github.com/purestake/substrate", branch = "crystalin-v0.9.9-block-response-length", default-features = false, optional = true }
-cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length" }
-cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length" }
-cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length" }
 ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true }
 sp-trie = { git = "https://github.com/purestake/substrate", default-features = false, branch = "crystalin-v0.9.9-block-response-length", optional = true }
 
+# These cumulus dependencies to be removed with https://github.com/PureStake/crowdloan-rewards/pull/44
+cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length", optional = true }
+cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length", optional = true }
+cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length", optional = true }
+
 [dev-dependencies]
 cumulus-test-relay-sproof-builder = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "notlesh-nimbus-v0.9.9-block-response-length" }
 
@@ -50,5 +52,8 @@ std = [
 runtime-benchmarks = [
     "frame-benchmarking",
     "sp-trie",
-    "ed25519-dalek"
+    "ed25519-dalek",
+    "cumulus-primitives-core",
+    "cumulus-pallet-parachain-system",
+    "cumulus-primitives-parachain-inherent",
 ]

src/benchmarks.rs

@@ -1,6 +1,6 @@
 #![cfg(feature = "runtime-benchmarks")]
 
-use crate::{BalanceOf, Call, Config, Pallet};
+use crate::{BalanceOf, Call, Pallet};
 use cumulus_pallet_parachain_system::Pallet as RelayPallet;
 use cumulus_primitives_core::{
 	relay_chain::{v1::HeadData, BlockNumber as RelayChainBlockNumber},
@@ -39,9 +39,14 @@ const MOCK_PROOF_HASH: [u8; 32] = [
 	243, 146, 7, 202, 245, 129, 165, 70, 72, 184, 130, 141,
 ];
 
+// These benchmarks only work with a Runtime that uses cumulus's RelayChainBlockNumberProvider.
+// This will improve once https://github.com/PureStake/crowdloan-rewards/pull/44 lands
+pub trait Config: crate::Config + cumulus_pallet_parachain_system::Config {}
+impl<T: crate::Config + cumulus_pallet_parachain_system::Config> Config for T {}
+
 /// Default balance amount is minimum contribution
 fn default_balance<T: Config>() -> BalanceOf<T> {
-	<<T as Config>::MinimumReward as Get<BalanceOf<T>>>::get()
+	T::MinimumReward::get()
 }
 
 /// Create a funded user.
@@ -135,8 +140,10 @@ fn insert_contributors<T: Config>(
 }
 
 /// Create a Contributor.
-fn close_initialization<T: Config>(end_relay: RelayChainBlockNumber) -> Result<(), &'static str> {
-	Pallet::<T>::complete_initialization(RawOrigin::Root.into(), end_relay)?;
+fn close_initialization<T: Config>(
+	end_vesting_block: T::VestingBlockNumber,
+) -> Result<(), &'static str> {
+	Pallet::<T>::complete_initialization(RawOrigin::Root.into(), end_vesting_block)?;
 	Ok(())
 }
 
@@ -154,7 +161,7 @@ fn create_sig<T: Config>(signed_account: T::AccountId) -> (AccountId32, MultiSig
 }
 
 fn max_batch_contributors<T: Config>() -> u32 {
-	<<T as Config>::MaxInitContributors as Get<u32>>::get()
+	T::MaxInitContributors::get()
 }
 
 // This is our current number of contributors
@@ -206,7 +213,7 @@ benchmarks! {
 		RelayPallet::<T>::on_finalize(T::BlockNumber::one());
 		Pallet::<T>::on_finalize(T::BlockNumber::one());
 
-	}:  _(RawOrigin::Root, 10u32)
+	}:  _(RawOrigin::Root, 10u32.into())
 	verify {
 	  assert!(Pallet::<T>::initialized());
 	}

src/lib.rs

@@ -74,17 +74,17 @@ pub mod weights;
 pub mod pallet {
 
 	use crate::weights::WeightInfo;
-	use cumulus_primitives_core::relay_chain;
 	use frame_support::traits::WithdrawReasons;
 	use frame_support::{
-		dispatch::fmt::Debug,
 		pallet_prelude::*,
 		traits::{Currency, ExistenceRequirement::AllowDeath},
 		PalletId,
 	};
 	use frame_system::pallet_prelude::*;
 	use sp_core::crypto::AccountId32;
-	use sp_runtime::traits::{AccountIdConversion, Saturating, Verify};
+	use sp_runtime::traits::{
+		AccountIdConversion, AtLeast32BitUnsigned, BlockNumberProvider, Saturating, Verify,
+	};
 	use sp_runtime::{MultiSignature, Perbill};
 	use sp_std::vec;
 	use sp_std::vec::Vec;
@@ -95,11 +95,9 @@ pub mod pallet {
 
 	pub const PALLET_ID: PalletId = PalletId(*b"Crowdloa");
 
-	pub struct RelayChainBeacon<T>(PhantomData<T>);
-
 	/// Configuration trait of this pallet.
 	#[pallet::config]
-	pub trait Config: cumulus_pallet_parachain_system::Config + frame_system::Config {
+	pub trait Config: frame_system::Config {
 		/// The overarching event type
 		type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
 		/// Checker for the reward vec, is it initalized already?
@@ -114,18 +112,19 @@ pub mod pallet {
 		type MinimumReward: Get<BalanceOf<Self>>;
 		/// The currency in which the rewards will be paid (probably the parachain native currency)
 		type RewardCurrency: Currency<Self::AccountId>;
-		// TODO What trait bounds do I need here? I think concretely we would
-		// be using MultiSigner? Or maybe MultiAccount? I copied these from frame_system
 		/// The AccountId type contributors used on the relay chain.
 		type RelayChainAccountId: Parameter
-			+ Member
-			+ MaybeSerializeDeserialize
-			+ Ord
-			+ Default
-			+ Debug
+			//TODO these AccountId32 bounds feel a little extraneous. I wonder if we can remove them.
 			+ Into<AccountId32>
 			+ From<AccountId32>;
 
+		/// The type that will be used to track vesting progress
+		type VestingBlockNumber: AtLeast32BitUnsigned + Parameter + Default + Into<BalanceOf<Self>>;
+
+		/// The notion of time that will be used for vesting. Probably
+		/// either the relay chain or sovereign chain block number.
+		type VestingBlockProvider: BlockNumberProvider<BlockNumber = Self::VestingBlockNumber>;
+
 		type WeightInfo: WeightInfo;
 	}
 
@@ -143,16 +142,13 @@ pub mod pallet {
 		pub contributed_relay_addresses: Vec<T::RelayChainAccountId>,
 	}
 
-	// This hook is in charge of initializing the relay chain height at the first block of the parachain
+	// This hook is in charge of initializing the vesting height at the first block of the parachain
 	#[pallet::hooks]
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		fn on_finalize(n: <T as frame_system::Config>::BlockNumber) {
-			// In the first block of the parachain we need to introduce the relay block related info
+			// In the first block of the parachain we need to introduce the vesting block related info
 			if n == 1u32.into() {
-				let slot = cumulus_pallet_parachain_system::Pallet::<T>::validation_data()
-					.expect("validation data was set in parachain system inherent")
-					.relay_parent_number;
-				<InitRelayBlock<T>>::put(slot);
+				<InitVestingBlock<T>>::put(T::VestingBlockProvider::current_block_number());
 			}
 		}
 	}
@@ -259,21 +255,19 @@ pub mod pallet {
 				Error::<T>::RewardsAlreadyClaimed
 			);
 
-			// Vesting is done in relation with the relay chain slot
-			let now = cumulus_pallet_parachain_system::Pallet::<T>::validation_data()
-				.expect("validation data was set in parachain system inherent")
-				.relay_parent_number;
+			// Get the current block used for vesting purposes
+			let now = T::VestingBlockProvider::current_block_number();
 
 			// Substract the first payment from the vested amount
 			let first_paid = T::InitializationPayment::get() * info.total_reward;
 
 			// To calculate how much could the user have claimed already
-			let payable_period = now.saturating_sub(<InitRelayBlock<T>>::get());
+			let payable_period = now.saturating_sub(<InitVestingBlock<T>>::get());
 
 			// How much should the contributor have already claimed by this block?
 			// By multiplying first we allow the conversion to integer done with the biggest number
-			let period = EndRelayBlock::<T>::get() - InitRelayBlock::<T>::get();
-			let should_have_claimed = if period == 0 {
+			let period = EndVestingBlock::<T>::get() - InitVestingBlock::<T>::get();
+			let should_have_claimed = if period == 0u32.into() {
 				// Pallet is configured with a zero vesting period.
 				info.total_reward - first_paid
 			} else {
@@ -337,12 +331,12 @@ pub mod pallet {
 
 		/// This extrinsic completes the initialization if some checks are fullfiled. These checks are:
 		///  -The reward contribution money matches the crowdloan pot
-		///  -The end relay block is higher than the init relay block
+		///  -The end vesting block is higher than the init vesting block
 		///  -The initialization has not complete yet
 		#[pallet::weight(T::WeightInfo::complete_initialization())]
 		pub fn complete_initialization(
 			origin: OriginFor<T>,
-			lease_ending_block: relay_chain::BlockNumber,
+			lease_ending_block: T::VestingBlockNumber,
 		) -> DispatchResultWithPostInfo {
 			ensure_root(origin)?;
 
@@ -354,9 +348,10 @@ pub mod pallet {
 				Error::<T>::RewardVecAlreadyInitialized
 			);
 
-			// This ensures the lease ending block is bigger than the init relay block
+			// This ensures the end vesting block (when all funds are fully vested)
+			// is bigger than the init vesting block
 			ensure!(
-				lease_ending_block > InitRelayBlock::<T>::get(),
+				lease_ending_block > InitVestingBlock::<T>::get(),
 				Error::<T>::VestingPeriodNonValid
 			);
 
@@ -380,7 +375,7 @@ pub mod pallet {
 			.expect("Shouldnt fail, as the fund should be enough to burn and nothing is locked");
 			drop(imbalance);
 
-			EndRelayBlock::<T>::put(lease_ending_block);
+			EndVestingBlock::<T>::put(lease_ending_block);
 
 			<Initialized<T>>::put(true);
 
@@ -440,7 +435,7 @@ pub mod pallet {
 				}
 
 				if *reward < T::MinimumReward::get() {
-					// Dont fail as this is supposed to be called with batch calls and we
+					// Don't fail as this is supposed to be called with batch calls and we
 					// dont want to stall the rest of the contributions
 					Self::deposit_event(Event::InitializedAccountWithNotEnoughContribution(
 						relay_account.clone(),
@@ -468,7 +463,7 @@ pub mod pallet {
 					0u32.into()
 				};
 
-				// We need to calculate the vesting based on the relay block number
+				// Calculate the reward info to store after the initial payment has been made.
 				let mut reward_info = RewardInfo {
 					total_reward: *reward,
 					claimed_reward: initial_payment,
@@ -599,14 +594,16 @@ pub mod pallet {
 	pub type Initialized<T: Config> = StorageValue<_, bool, ValueQuery, T::Initialized>;
 
 	#[pallet::storage]
-	#[pallet::getter(fn init_relay_block)]
-	/// Relay block height at the initialization of the pallet
-	type InitRelayBlock<T: Config> = StorageValue<_, relay_chain::BlockNumber, ValueQuery>;
+	#[pallet::storage_prefix = "InitRelayBlock"]
+	#[pallet::getter(fn init_vesting_block)]
+	/// Vesting block height at the initialization of the pallet
+	type InitVestingBlock<T: Config> = StorageValue<_, T::VestingBlockNumber, ValueQuery>;
 
 	#[pallet::storage]
-	#[pallet::getter(fn end_relay_block)]
-	/// Relay block height at which vesting will be complete and all rewards fully vested.
-	type EndRelayBlock<T: Config> = StorageValue<_, relay_chain::BlockNumber, ValueQuery>;
+	#[pallet::storage_prefix = "EndRelayBlock"]
+	#[pallet::getter(fn end_vesting_block)]
+	/// Vesting block height at the initialization of the pallet
+	type EndVestingBlock<T: Config> = StorageValue<_, T::VestingBlockNumber, ValueQuery>;
 
 	#[pallet::storage]
 	#[pallet::getter(fn init_reward_amount)]

src/mock.rs

@@ -134,6 +134,9 @@ impl Config for Test {
 	type MinimumReward = TestMinimumReward;
 	type RewardCurrency = Balances;
 	type RelayChainAccountId = [u8; 32];
+	type VestingBlockNumber = RelayChainBlockNumber;
+	type VestingBlockProvider =
+		cumulus_pallet_parachain_system::RelaychainBlockNumberProvider<Self>;
 	type WeightInfo = ();
 }