From 5f4e78d15ae1f2ab15849be5b788125c7cb52072 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Tue, 14 Sep 2021 07:58:18 -0700 Subject: [PATCH] Remove tight coupling to parachain system (#43) --- Cargo.toml | 15 ++++++---- src/benchmarks.rs | 19 ++++++++---- src/lib.rs | 75 +++++++++++++++++++++++------------------------ src/mock.rs | 3 ++ src/tests.rs | 34 ++++++++++----------- 5 files changed, 79 insertions(+), 67 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7e397bc..44c9987 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,14 +19,16 @@ sp-io = { git = "https://github.com/paritytech/substrate", default-features = fa pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9", default-features = false } pallet-utility = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9", default-features = false } frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9", default-features = false, optional = true } -cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9" } -cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9" } -cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9" } ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true } sp-trie = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9", optional = true } +# These cumulus dependencies to be removed with https://github.com/PureStake/crowdloan-rewards/pull/44 +cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9", optional = true } +cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9", optional = true } +cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9", optional = true } + [dev-dependencies] -cumulus-test-relay-sproof-builder = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9" } +cumulus-test-relay-sproof-builder = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } [features] default = ["std"] @@ -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", ] diff --git a/src/benchmarks.rs b/src/benchmarks.rs index 3c34cff..ef9135a 100644 --- a/src/benchmarks.rs +++ b/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 Config for T {} + /// Default balance amount is minimum contribution fn default_balance() -> BalanceOf { - <::MinimumReward as Get>>::get() + T::MinimumReward::get() } /// Create a funded user. @@ -135,8 +140,10 @@ fn insert_contributors( } /// Create a Contributor. -fn close_initialization(end_relay: RelayChainBlockNumber) -> Result<(), &'static str> { - Pallet::::complete_initialization(RawOrigin::Root.into(), end_relay)?; +fn close_initialization( + end_vesting_block: T::VestingBlockNumber, +) -> Result<(), &'static str> { + Pallet::::complete_initialization(RawOrigin::Root.into(), end_vesting_block)?; Ok(()) } @@ -154,7 +161,7 @@ fn create_sig(signed_account: T::AccountId) -> (AccountId32, MultiSig } fn max_batch_contributors() -> u32 { - <::MaxInitContributors as Get>::get() + T::MaxInitContributors::get() } // This is our current number of contributors @@ -206,7 +213,7 @@ benchmarks! { RelayPallet::::on_finalize(T::BlockNumber::one()); Pallet::::on_finalize(T::BlockNumber::one()); - }: _(RawOrigin::Root, 10u32) + }: _(RawOrigin::Root, 10u32.into()) verify { assert!(Pallet::::initialized()); } diff --git a/src/lib.rs b/src/lib.rs index 1403033..0a556dc 100644 --- a/src/lib.rs +++ b/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(PhantomData); - /// 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> + IsType<::Event>; /// Checker for the reward vec, is it initalized already? @@ -114,18 +112,19 @@ pub mod pallet { type MinimumReward: Get>; /// The currency in which the rewards will be paid (probably the parachain native currency) type RewardCurrency: Currency; - // 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 + From; + /// The type that will be used to track vesting progress + type VestingBlockNumber: AtLeast32BitUnsigned + Parameter + Default + Into>; + + /// The notion of time that will be used for vesting. Probably + /// either the relay chain or sovereign chain block number. + type VestingBlockProvider: BlockNumberProvider; + type WeightInfo: WeightInfo; } @@ -143,16 +142,13 @@ pub mod pallet { pub contributed_relay_addresses: Vec, } - // 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 Hooks> for Pallet { fn on_finalize(n: ::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::::validation_data() - .expect("validation data was set in parachain system inherent") - .relay_parent_number; - >::put(slot); + >::put(T::VestingBlockProvider::current_block_number()); } } } @@ -259,21 +255,19 @@ pub mod pallet { Error::::RewardsAlreadyClaimed ); - // Vesting is done in relation with the relay chain slot - let now = cumulus_pallet_parachain_system::Pallet::::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(>::get()); + let payable_period = now.saturating_sub(>::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::::get() - InitRelayBlock::::get(); - let should_have_claimed = if period == 0 { + let period = EndVestingBlock::::get() - InitVestingBlock::::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, - lease_ending_block: relay_chain::BlockNumber, + lease_ending_block: T::VestingBlockNumber, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; @@ -354,9 +348,10 @@ pub mod pallet { Error::::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::::get(), + lease_ending_block > InitVestingBlock::::get(), Error::::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::::put(lease_ending_block); + EndVestingBlock::::put(lease_ending_block); >::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 = StorageValue<_, bool, ValueQuery, T::Initialized>; #[pallet::storage] - #[pallet::getter(fn init_relay_block)] - /// Relay block height at the initialization of the pallet - type InitRelayBlock = 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 = 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 = 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 = StorageValue<_, T::VestingBlockNumber, ValueQuery>; #[pallet::storage] #[pallet::getter(fn init_reward_amount)] diff --git a/src/mock.rs b/src/mock.rs index 2934bcd..ddfbe46 100644 --- a/src/mock.rs +++ b/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; type WeightInfo = (); } diff --git a/src/tests.rs b/src/tests.rs index 1d7557c..c43e3fd 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -33,7 +33,7 @@ fn geneses() { assert!(System::events().is_empty()); // Insert contributors let pairs = get_ed25519_pairs(3); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -80,7 +80,7 @@ fn proving_assignation_works() { empty().execute_with(|| { // Insert contributors let pairs = get_ed25519_pairs(3); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -162,7 +162,7 @@ fn initializing_multi_relay_to_single_native_address_works() { let pairs = get_ed25519_pairs(3); // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -210,7 +210,7 @@ fn paying_works_step_by_step() { let pairs = get_ed25519_pairs(3); // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -280,7 +280,7 @@ fn paying_works_after_unclaimed_period() { let pairs = get_ed25519_pairs(3); // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -343,7 +343,7 @@ fn paying_late_joiner_works() { empty().execute_with(|| { // Insert contributors let pairs = get_ed25519_pairs(3); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -386,7 +386,7 @@ fn update_address_works() { let pairs = get_ed25519_pairs(3); // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -432,7 +432,7 @@ fn update_address_with_existing_address_fails() { let pairs = get_ed25519_pairs(3); // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -465,7 +465,7 @@ fn update_address_with_existing_with_multi_address_works() { let pairs = get_ed25519_pairs(3); // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -503,7 +503,7 @@ fn initialize_new_addresses() { roll_to(2); // Insert contributors let pairs = get_ed25519_pairs(3); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -544,7 +544,7 @@ fn initialize_new_addresses_handle_dust() { roll_to(2); // Insert contributors let pairs = get_ed25519_pairs(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), vec![ @@ -578,7 +578,7 @@ fn initialize_new_addresses_not_matching_funds() { roll_to(2); // Insert contributors let pairs = get_ed25519_pairs(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); // Total supply is 2500.Lets ensure inserting 2495 is not working. assert_ok!(Crowdloan::initialize_reward_vec( Origin::root(), @@ -601,7 +601,7 @@ fn initialize_new_addresses_with_batch() { empty().execute_with(|| { // This time should succeed trully roll_to(10); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(mock::Call::Utility(UtilityCall::batch_all(vec![ mock::Call::Crowdloan(crate::Call::initialize_reward_vec(vec![( [4u8; 32].into(), @@ -621,7 +621,7 @@ fn initialize_new_addresses_with_batch() { )); assert_eq!(Crowdloan::total_contributors(), 2); // Verify that the second ending block provider had no effect - assert_eq!(Crowdloan::end_relay_block(), init_block + VESTING); + assert_eq!(Crowdloan::end_vesting_block(), init_block + VESTING); // Batch calls always succeed. We just need to check the inner event assert_ok!( @@ -653,7 +653,7 @@ fn floating_point_arithmetic_works() { empty().execute_with(|| { // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(mock::Call::Utility(UtilityCall::batch_all(vec![ mock::Call::Crowdloan(crate::Call::initialize_reward_vec(vec![( [4u8; 32].into(), @@ -729,7 +729,7 @@ fn reward_below_vesting_period_works() { empty().execute_with(|| { // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); assert_ok!(mock::Call::Utility(UtilityCall::batch_all(vec![ mock::Call::Crowdloan(crate::Call::initialize_reward_vec(vec![( [4u8; 32].into(), @@ -819,7 +819,7 @@ fn test_initialization_errors() { empty().execute_with(|| { // The init relay block gets inserted roll_to(2); - let init_block = Crowdloan::init_relay_block(); + let init_block = Crowdloan::init_vesting_block(); let pot = Crowdloan::pot();