From f57daeb16c588e03eda3c0dc7479413af9c008c5 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 14:53:01 -0400 Subject: [PATCH 01/21] prune nimbus-primitives --- Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index eb4ee77..a510b24 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,6 @@ pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "p pallet-utility = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.8", default-features = false } frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.8", default-features = false, optional = true } cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } -nimbus-primitives = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true } @@ -46,7 +45,6 @@ std = [ "sp-io/std", "cumulus-pallet-parachain-system/std", "cumulus-primitives-core/std", - "nimbus-primitives/std", "cumulus-primitives-parachain-inherent/std", ] runtime-benchmarks = [ From 5fead18a42af602d491129e51e534281424b1f8e Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 15:53:19 -0400 Subject: [PATCH 02/21] update branches and toolchain --- Cargo.toml | 28 ++++++++++++++-------------- rust-toolchain | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a510b24..7e397bc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,23 +10,23 @@ parity-scale-codec = { version = "2.1.1", default-features = false} serde = { version = "1.0.101", optional = true, features = ["derive"], default-features = false } log = { version = "0.4", default-features = false } -frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.8" } -frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.8" } -sp-core = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.8" } -sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.8" } -sp-std = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.8" } -sp-io = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.8" } -pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.8", default-features = false } -pallet-utility = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.8", default-features = false } -frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.8", default-features = false, optional = true } -cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } -cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } -cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } +frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9" } +frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9" } +sp-core = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9" } +sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9" } +sp-std = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9" } +sp-io = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9" } +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.8", optional = true } +sp-trie = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.9", optional = true } [dev-dependencies] -cumulus-test-relay-sproof-builder = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "joshy-np098" } +cumulus-test-relay-sproof-builder = { git = "https://github.com/purestake/cumulus", default-features = false, branch = "nimbus-polkadot-v0.9.9" } [features] default = ["std"] diff --git a/rust-toolchain b/rust-toolchain index d52cfca..15ea0d2 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,5 +1,5 @@ [toolchain] -channel = "nightly-2021-02-21" +channel = "nightly-2021-08-01" components = [ "rustfmt", "clippy" ] targets = [ "wasm32-unknown-unknown" ] profile = "minimal" From 6238fc7d67327b62a650f913c3abfd855b7d4f35 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 15:54:12 -0400 Subject: [PATCH 03/21] reflect new events from https://github.com/paritytech/substrate/pull/9462 --- src/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tests.rs b/src/tests.rs index be087a4..1d7557c 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -632,6 +632,8 @@ fn initialize_new_addresses_with_batch() { ); let expected = vec![ + pallet_utility::Event::ItemCompleted, + pallet_utility::Event::ItemCompleted, pallet_utility::Event::BatchCompleted, pallet_utility::Event::BatchInterrupted( 0, From bc276342f5c594c2c086c5134a58411f65f12eb4 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:26:28 -0400 Subject: [PATCH 04/21] core change. pallet compiles. --- src/lib.rs | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9158878..1a8dc6a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -74,7 +74,6 @@ 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, @@ -84,7 +83,7 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; use sp_core::crypto::AccountId32; - use sp_runtime::traits::{AccountIdConversion, Saturating, Verify}; + use sp_runtime::traits::{AccountIdConversion, BlockNumberProvider, Saturating, Verify}; use sp_runtime::{MultiSignature, Perbill}; use sp_std::vec::Vec; use sp_std::vec; @@ -99,7 +98,7 @@ pub mod pallet { /// 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? @@ -125,6 +124,20 @@ pub mod pallet { + Debug + Into + From; + + //TODO do we need at least 32 bit unsigned + /// The type that will be used to track vesting progress + type VestingBlockNumber: sp_runtime::traits::AtLeast32BitUnsigned + Parameter + + Member + + MaybeSerializeDeserialize + + Ord + + Default + + Debug + + 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; } @@ -149,10 +162,7 @@ pub mod pallet { fn on_finalize(n: ::BlockNumber) { // In the first block of the parachain we need to introduce the relay 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 +269,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 { @@ -343,7 +351,7 @@ pub mod pallet { #[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)?; @@ -357,7 +365,7 @@ pub mod pallet { // This ensures the lease ending block is bigger than the init relay block ensure!( - lease_ending_block > InitRelayBlock::::get(), + lease_ending_block > InitVestingBlock::::get(), Error::::VestingPeriodNonValid ); @@ -381,7 +389,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); @@ -602,12 +610,12 @@ pub mod pallet { #[pallet::storage] #[pallet::getter(fn init_relay_block)] /// Relay block height at the initialization of the pallet - type InitRelayBlock = StorageValue<_, relay_chain::BlockNumber, ValueQuery>; + type InitVestingBlock = StorageValue<_, T::VestingBlockNumber, ValueQuery>; #[pallet::storage] #[pallet::getter(fn end_relay_block)] /// Relay block height at the initialization of the pallet - type EndRelayBlock = StorageValue<_, relay_chain::BlockNumber, ValueQuery>; + type EndVestingBlock = StorageValue<_, T::VestingBlockNumber, ValueQuery>; #[pallet::storage] #[pallet::getter(fn init_reward_amount)] From 15f1f93c9a2d0c3dbc7add23bd13d852d13538b5 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:31:48 -0400 Subject: [PATCH 05/21] remove some seemingly-unnecessary trait bounds --- src/lib.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1a8dc6a..418b0b5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,14 +76,13 @@ pub mod pallet { use crate::weights::WeightInfo; 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, BlockNumberProvider, Saturating, Verify}; + use sp_runtime::traits::{AccountIdConversion, AtLeast32BitUnsigned, BlockNumberProvider, Saturating, Verify}; use sp_runtime::{MultiSignature, Perbill}; use sp_std::vec::Vec; use sp_std::vec; @@ -113,26 +112,16 @@ 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; - //TODO do we need at least 32 bit unsigned /// The type that will be used to track vesting progress - type VestingBlockNumber: sp_runtime::traits::AtLeast32BitUnsigned + Parameter - + Member - + MaybeSerializeDeserialize - + Ord + type VestingBlockNumber: AtLeast32BitUnsigned + + Parameter + Default - + Debug + Into>; /// The notion of time that will be used for vesting. Probably From c88c9da4840932b296b7f4b0ace4b4a38ed9e0bd Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:36:23 -0400 Subject: [PATCH 06/21] fix test mock --- src/mock.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mock.rs b/src/mock.rs index 2934bcd..b1e870c 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -134,6 +134,8 @@ 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 = (); } From f6b10a85ea777b7d020d17401c441b8b889413d8 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:40:14 -0400 Subject: [PATCH 07/21] make cumulus stuff dev-dependencies --- Cargo.toml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7e397bc..154357b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,14 +19,14 @@ 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 } [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" } +cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } +cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } +cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } [features] default = ["std"] @@ -43,9 +43,6 @@ std = [ "log/std", "sp-std/std", "sp-io/std", - "cumulus-pallet-parachain-system/std", - "cumulus-primitives-core/std", - "cumulus-primitives-parachain-inherent/std", ] runtime-benchmarks = [ "frame-benchmarking", From 6d6230e57a5273d4e00fce932db6e2593fffd5a4 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:56:29 -0400 Subject: [PATCH 08/21] Tests compile, but 7 fail --- src/mock.rs | 84 +++++++++++++++++++++------------------------------- src/tests.rs | 2 +- 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/src/mock.rs b/src/mock.rs index b1e870c..4a3759f 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -51,26 +51,10 @@ construct_runtime!( System: frame_system::{Pallet, Call, Config, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, Crowdloan: pallet_crowdloan_rewards::{Pallet, Call, Storage, Event}, - ParachainSystem: cumulus_pallet_parachain_system::{Pallet, Call, Storage, Inherent, Event}, Utility: pallet_utility::{Pallet, Call, Storage, Event}, } ); -parameter_types! { - pub ParachainId: cumulus_primitives_core::ParaId = 100.into(); -} - -impl cumulus_pallet_parachain_system::Config for Test { - type SelfParaId = ParachainId; - type Event = Event; - type OnValidationData = (); - type OutboundXcmpMessageSource = (); - type XcmpMessageHandler = (); - type ReservedXcmpWeight = (); - type DmpMessageHandler = (); - type ReservedDmpWeight = (); -} - parameter_types! { pub const BlockHashCount: u64 = 250; pub BlockWeights: frame_system::limits::BlockWeights = @@ -98,7 +82,7 @@ impl frame_system::Config for Test { type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (); - type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode; + type OnSetCode = (); type SystemWeightInfo = (); type SS58Prefix = (); } @@ -134,8 +118,8 @@ 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 VestingBlockNumber = ::BlockNumber; + type VestingBlockProvider = System; type WeightInfo = (); } @@ -210,37 +194,37 @@ pub(crate) fn batch_events() -> Vec { pub(crate) fn roll_to(n: u64) { while System::block_number() < n { // Relay chain Stuff. I might actually set this to a number different than N - let sproof_builder = RelayStateSproofBuilder::default(); - let (relay_parent_storage_root, relay_chain_state) = - sproof_builder.into_state_root_and_proof(); - let vfp = PersistedValidationData { - relay_parent_number: (System::block_number() + 1u64) as RelayChainBlockNumber, - relay_parent_storage_root, - ..Default::default() - }; - let inherent_data = { - let mut inherent_data = InherentData::default(); - let system_inherent_data = ParachainInherentData { - validation_data: vfp.clone(), - relay_chain_state, - downward_messages: Default::default(), - horizontal_messages: Default::default(), - }; - inherent_data - .put_data( - cumulus_primitives_parachain_inherent::INHERENT_IDENTIFIER, - &system_inherent_data, - ) - .expect("failed to put VFP inherent"); - inherent_data - }; - - ParachainSystem::on_initialize(System::block_number()); - ParachainSystem::create_inherent(&inherent_data) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); - ParachainSystem::on_finalize(System::block_number()); + // let sproof_builder = RelayStateSproofBuilder::default(); + // let (relay_parent_storage_root, relay_chain_state) = + // sproof_builder.into_state_root_and_proof(); + // let vfp = PersistedValidationData { + // relay_parent_number: (System::block_number() + 1u64) as RelayChainBlockNumber, + // relay_parent_storage_root, + // ..Default::default() + // }; + // let inherent_data = { + // let mut inherent_data = InherentData::default(); + // let system_inherent_data = ParachainInherentData { + // validation_data: vfp.clone(), + // relay_chain_state, + // downward_messages: Default::default(), + // horizontal_messages: Default::default(), + // }; + // inherent_data + // .put_data( + // cumulus_primitives_parachain_inherent::INHERENT_IDENTIFIER, + // &system_inherent_data, + // ) + // .expect("failed to put VFP inherent"); + // inherent_data + // }; + + // ParachainSystem::on_initialize(System::block_number()); + // ParachainSystem::create_inherent(&inherent_data) + // .expect("got an inherent") + // .dispatch_bypass_filter(RawOrigin::None.into()) + // .expect("dispatch succeeded"); + // ParachainSystem::on_finalize(System::block_number()); Crowdloan::on_finalize(System::block_number()); Balances::on_finalize(System::block_number()); diff --git a/src/tests.rs b/src/tests.rs index 1d7557c..5ec2c8a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -25,7 +25,7 @@ use sp_runtime::MultiSignature; // Constant that reflects the desired vesting period for the tests // Most tests complete initialization passing initRelayBlock + VESTING as the endRelayBlock -const VESTING: u32 = 8; +const VESTING: u64 = 8; #[test] fn geneses() { From ad451b6ccb0e3fa9988492caf55a3ea8ada6ca25 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:58:01 -0400 Subject: [PATCH 09/21] unused imports --- src/mock.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/mock.rs b/src/mock.rs index 4a3759f..6a04aa3 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -16,18 +16,11 @@ //! Test utilities use crate::{self as pallet_crowdloan_rewards, Config}; -use cumulus_primitives_core::relay_chain::BlockNumber as RelayChainBlockNumber; -use cumulus_primitives_core::PersistedValidationData; -use cumulus_primitives_parachain_inherent::ParachainInherentData; -use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; use frame_support::{ construct_runtime, - dispatch::UnfilteredDispatchable, - inherent::{InherentData, ProvideInherent}, parameter_types, traits::{GenesisBuild, OnFinalize, OnInitialize}, }; -use frame_system::RawOrigin; use sp_core::{ed25519, Pair, H256}; use sp_io; use sp_runtime::{ From a43227dae868055b555b03bd524b991647f9b3f2 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 16:58:52 -0400 Subject: [PATCH 10/21] totally prune the dependencies --- Cargo.toml | 6 ------ src/mock.rs | 33 --------------------------------- 2 files changed, 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 154357b..9a91749 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,12 +22,6 @@ frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = 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 } -[dev-dependencies] -cumulus-test-relay-sproof-builder = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } -cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } -cumulus-primitives-core = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } -cumulus-primitives-parachain-inherent = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" } - [features] default = ["std"] std = [ diff --git a/src/mock.rs b/src/mock.rs index 6a04aa3..33c113d 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -186,39 +186,6 @@ pub(crate) fn batch_events() -> Vec { pub(crate) fn roll_to(n: u64) { while System::block_number() < n { - // Relay chain Stuff. I might actually set this to a number different than N - // let sproof_builder = RelayStateSproofBuilder::default(); - // let (relay_parent_storage_root, relay_chain_state) = - // sproof_builder.into_state_root_and_proof(); - // let vfp = PersistedValidationData { - // relay_parent_number: (System::block_number() + 1u64) as RelayChainBlockNumber, - // relay_parent_storage_root, - // ..Default::default() - // }; - // let inherent_data = { - // let mut inherent_data = InherentData::default(); - // let system_inherent_data = ParachainInherentData { - // validation_data: vfp.clone(), - // relay_chain_state, - // downward_messages: Default::default(), - // horizontal_messages: Default::default(), - // }; - // inherent_data - // .put_data( - // cumulus_primitives_parachain_inherent::INHERENT_IDENTIFIER, - // &system_inherent_data, - // ) - // .expect("failed to put VFP inherent"); - // inherent_data - // }; - - // ParachainSystem::on_initialize(System::block_number()); - // ParachainSystem::create_inherent(&inherent_data) - // .expect("got an inherent") - // .dispatch_bypass_filter(RawOrigin::None.into()) - // .expect("dispatch succeeded"); - // ParachainSystem::on_finalize(System::block_number()); - Crowdloan::on_finalize(System::block_number()); Balances::on_finalize(System::block_number()); System::on_finalize(System::block_number()); From 9970ba1c913a03c063c1294dbe6aee7eacc8ae0b Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 17:05:51 -0400 Subject: [PATCH 11/21] fix one test --- src/tests.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index 5ec2c8a..1564a67 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -188,7 +188,8 @@ fn initializing_multi_relay_to_single_native_address_works() { roll_to(4); assert_ok!(Crowdloan::claim(Origin::signed(1))); - assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 400); + // Expect 500 clained (200 immediately + 100 per block for 3 blocks) + assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 500); assert_noop!( Crowdloan::claim(Origin::signed(3)), Error::::NoAssociatedClaim @@ -197,7 +198,7 @@ fn initializing_multi_relay_to_single_native_address_works() { let expected = vec![ crate::Event::InitialPaymentMade(1, 100), crate::Event::InitialPaymentMade(1, 100), - crate::Event::RewardsPaid(1, 200), + crate::Event::RewardsPaid(1, 300), ]; assert_eq!(events(), expected); }); From 7f16a85f726f560c79c11b16da5865256bc49c54 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 17:13:13 -0400 Subject: [PATCH 12/21] another one fixed --- src/tests.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index 1564a67..5bf0ced 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -650,7 +650,7 @@ fn initialize_new_addresses_with_batch() { } #[test] -fn floating_point_arithmetic_works() { +fn fixed_point_arithmetic_works() { empty().execute_with(|| { // The init relay block gets inserted roll_to(2); @@ -666,7 +666,8 @@ fn floating_point_arithmetic_works() { Some(2), 1185 )])), - // We will work with this. This has 100/8=12.5 payable per block + // We will work with this contribution. + // This has 25 paid immediately and 100/8=12.5 payable per block mock::Call::Crowdloan(crate::Call::initialize_reward_vec(vec![( [3u8; 32].into(), Some(3), @@ -685,29 +686,28 @@ fn floating_point_arithmetic_works() { 25u128 ); - // Block relay number is 2 post init initialization + // Vesting block number is 2 post init initialization // In this case there is no problem. Here we pay 12.5*2=25 // Total claimed reward: 25+25 = 50 - roll_to(4); - + roll_to(3); assert_ok!(Crowdloan::claim(Origin::signed(3))); - assert_eq!( Crowdloan::accounts_payable(&3).unwrap().claimed_reward, 50u128 ); - roll_to(5); + // If we claim now we have to pay 12.5. 12 will be paid. + roll_to(4); assert_ok!(Crowdloan::claim(Origin::signed(3))); - assert_eq!( Crowdloan::accounts_payable(&3).unwrap().claimed_reward, 62u128 ); - roll_to(6); + // Now we should pay 12.5. However the calculus will be: // Account 3 should have claimed 50 + 25 at this block, but // he only claimed 62. The payment is 13 + roll_to(5); assert_ok!(Crowdloan::claim(Origin::signed(3))); assert_eq!( Crowdloan::accounts_payable(&3).unwrap().claimed_reward, From 99fb15cfa32f0d022710db50801ff8ae34fc140a Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 17:15:12 -0400 Subject: [PATCH 13/21] step_by_step works --- src/tests.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index 5bf0ced..520c9fe 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -228,32 +228,40 @@ fn paying_works_step_by_step() { )); // 1 is payable assert!(Crowdloan::accounts_payable(&1).is_some()); - roll_to(4); + + roll_to(3); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 200); assert_noop!( Crowdloan::claim(Origin::signed(3)), Error::::NoAssociatedClaim ); - roll_to(5); + + roll_to(4); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 250); - roll_to(6); + + roll_to(5); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 300); - roll_to(7); + + roll_to(6); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 350); - roll_to(8); + + roll_to(7); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 400); - roll_to(9); + + roll_to(8); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 450); - roll_to(10); + + roll_to(9); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 500); - roll_to(11); + + roll_to(10); assert_noop!( Crowdloan::claim(Origin::signed(1)), Error::::RewardsAlreadyClaimed From f8c4f4a945f2873afaee47cb9384790706898d90 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 17:16:38 -0400 Subject: [PATCH 14/21] after_unclaimed_period --- src/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index 520c9fe..b632329 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -307,20 +307,20 @@ fn paying_works_after_unclaimed_period() { // 1 is payable assert!(Crowdloan::accounts_payable(&1).is_some()); - roll_to(4); + roll_to(3); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 200); assert_noop!( Crowdloan::claim(Origin::signed(3)), Error::::NoAssociatedClaim ); - roll_to(5); + roll_to(4); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 250); - roll_to(6); + roll_to(5); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 300); - roll_to(7); + roll_to(6); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_eq!(Crowdloan::accounts_payable(&1).unwrap().claimed_reward, 350); roll_to(11); From f09e84b1b3003ed3b146ab6093d09f4eb047d3f6 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 17:17:50 -0400 Subject: [PATCH 15/21] below vesting period --- src/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index b632329..b5f2db0 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -772,7 +772,7 @@ fn reward_below_vesting_period_works() { // Block relay number is 2 post init initialization // Here we should pay floor(0.625*2)=1 // Total claimed reward: 1+1 = 2 - roll_to(4); + roll_to(3); assert_ok!(Crowdloan::claim(Origin::signed(3))); @@ -780,7 +780,7 @@ fn reward_below_vesting_period_works() { Crowdloan::accounts_payable(&3).unwrap().claimed_reward, 2u128 ); - roll_to(5); + roll_to(4); // If we claim now we have to pay floor(0.625) = 0 assert_ok!(Crowdloan::claim(Origin::signed(3))); @@ -788,7 +788,7 @@ fn reward_below_vesting_period_works() { Crowdloan::accounts_payable(&3).unwrap().claimed_reward, 2u128 ); - roll_to(6); + roll_to(5); // Now we should pay 1 again. The claimer should have claimed floor(0.625*4) + 1 // but he only claimed 2 assert_ok!(Crowdloan::claim(Origin::signed(3))); From d0085b218d4ce3dbdf00f5d92a992e27ea52ef95 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 23 Aug 2021 17:19:43 -0400 Subject: [PATCH 16/21] tests pass --- src/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index b5f2db0..ac9d8d8 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -411,7 +411,7 @@ fn update_address_works() { init_block + VESTING )); - roll_to(4); + roll_to(3); assert_ok!(Crowdloan::claim(Origin::signed(1))); assert_noop!( Crowdloan::claim(Origin::signed(8)), @@ -419,7 +419,7 @@ fn update_address_works() { ); assert_ok!(Crowdloan::update_reward_address(Origin::signed(1), 8)); assert_eq!(Crowdloan::accounts_payable(&8).unwrap().claimed_reward, 200); - roll_to(6); + roll_to(5); assert_ok!(Crowdloan::claim(Origin::signed(8))); assert_eq!(Crowdloan::accounts_payable(&8).unwrap().claimed_reward, 300); // The initial payment is not @@ -490,7 +490,7 @@ fn update_address_with_existing_with_multi_address_works() { init_block + VESTING )); - roll_to(4); + roll_to(3); assert_ok!(Crowdloan::claim(Origin::signed(1))); // We make sure all rewards go to the new address From bd005fe008ce000395dd99fb9d418ce5a34ea49e Mon Sep 17 00:00:00 2001 From: gorka Date: Wed, 1 Sep 2021 09:53:28 +0200 Subject: [PATCH 17/21] Add custom trait for block provider, with a setter for benchmarking --- src/lib.rs | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 418b0b5..74b6018 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,10 +82,10 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; use sp_core::crypto::AccountId32; - use sp_runtime::traits::{AccountIdConversion, AtLeast32BitUnsigned, BlockNumberProvider, Saturating, Verify}; + use sp_runtime::traits::{AccountIdConversion, AtLeast32BitUnsigned, Saturating, Verify}; use sp_runtime::{MultiSignature, Perbill}; - use sp_std::vec::Vec; use sp_std::vec; + use sp_std::vec::Vec; #[pallet::pallet] // The crowdloan rewards pallet @@ -95,6 +95,26 @@ pub mod pallet { pub struct RelayChainBeacon(PhantomData); + /// Something that can provide a block number + pub trait BlockProvider { + /// Type of `BlockNumber` to provide. + type BlockNumber: parity_scale_codec::Codec + Clone + Ord + Eq + AtLeast32BitUnsigned; + + /// Returns the current block number. + /// + /// Provides an abstraction over an arbitrary way of providing the + /// current block number. + /// + fn current_block_number() -> Self::BlockNumber; + + /// Utility function only to be used in benchmarking scenarios, to be implemented optionally, + /// else a noop. + /// + /// It allows for setting the block number that will later be fetched + #[cfg(any(feature = "runtime-benchmarks", test))] + fn set_block_number(_block: Self::BlockNumber) {} + } + /// Configuration trait of this pallet. #[pallet::config] pub trait Config: frame_system::Config { @@ -117,16 +137,13 @@ pub mod pallet { //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>; + 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 VestingBlockProvider: BlockProvider; type WeightInfo: WeightInfo; } @@ -312,8 +329,7 @@ pub mod pallet { let signer = ensure_signed(origin)?; // Calculate the veted amount on demand. - let info = - AccountsPayable::::get(&signer).ok_or(Error::::NoAssociatedClaim)?; + let info = AccountsPayable::::get(&signer).ok_or(Error::::NoAssociatedClaim)?; // For now I prefer that we dont support providing an existing account here ensure!( From 2500bbf4a20528d30e5525e87eab0b48a8471ccd Mon Sep 17 00:00:00 2001 From: gorka Date: Wed, 1 Sep 2021 09:53:43 +0200 Subject: [PATCH 18/21] Adapt mock runtime to new trait --- src/mock.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/mock.rs b/src/mock.rs index 33c113d..488eb81 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -15,10 +15,9 @@ // along with Moonbeam. If not, see . //! Test utilities -use crate::{self as pallet_crowdloan_rewards, Config}; +use crate::{self as pallet_crowdloan_rewards, BlockProvider, Config}; use frame_support::{ - construct_runtime, - parameter_types, + construct_runtime, parameter_types, traits::{GenesisBuild, OnFinalize, OnInitialize}, }; use sp_core::{ed25519, Pair, H256}; @@ -96,6 +95,19 @@ impl pallet_balances::Config for Test { type WeightInfo = (); } +use sp_runtime::traits::BlockNumberProvider; +pub struct MockedBlockProvider; +impl BlockProvider for MockedBlockProvider { + type BlockNumber = u64; + + fn current_block_number() -> Self::BlockNumber { + System::current_block_number() + } + + #[cfg(feature = "runtime-benchmarks")] + fn set_block_number(_block: Self::BlockNumber) {} +} + parameter_types! { pub const TestMaxInitContributors: u32 = 8; pub const TestMinimumReward: u128 = 0; @@ -112,7 +124,7 @@ impl Config for Test { type RewardCurrency = Balances; type RelayChainAccountId = [u8; 32]; type VestingBlockNumber = ::BlockNumber; - type VestingBlockProvider = System; + type VestingBlockProvider = MockedBlockProvider; type WeightInfo = (); } From 999dbbe7567e2d66f5a73e3b226748d45522a8b6 Mon Sep 17 00:00:00 2001 From: gorka Date: Wed, 1 Sep 2021 09:54:11 +0200 Subject: [PATCH 19/21] Adapt benchmarks to new setter --- src/benchmarks.rs | 148 +++++----------------------------------------- 1 file changed, 14 insertions(+), 134 deletions(-) diff --git a/src/benchmarks.rs b/src/benchmarks.rs index 761b05a..19c576f 100644 --- a/src/benchmarks.rs +++ b/src/benchmarks.rs @@ -1,19 +1,10 @@ #![cfg(feature = "runtime-benchmarks")] +use crate::pallet::BlockProvider; use crate::{BalanceOf, Call, Config, Pallet}; -use cumulus_pallet_parachain_system::Pallet as RelayPallet; -use cumulus_primitives_core::{ - relay_chain::{v1::HeadData, BlockNumber as RelayChainBlockNumber}, - PersistedValidationData, -}; -use cumulus_primitives_parachain_inherent::ParachainInherentData; use ed25519_dalek::Signer; use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite}; -use frame_support::{ - dispatch::UnfilteredDispatchable, - inherent::{InherentData, ProvideInherent}, - traits::{Currency, Get, OnFinalize, OnInitialize}, -}; +use frame_support::traits::{Currency, Get, OnFinalize}; use frame_system::RawOrigin; use parity_scale_codec::Encode; use sp_core::{ @@ -23,21 +14,6 @@ use sp_core::{ use sp_runtime::{traits::One, MultiSignature}; use sp_std::vec; use sp_std::vec::Vec; -use sp_trie::StorageProof; -// This is a fake proof that emulates a storage proof inserted as the validation data -// We avoid using the sproof builder here because it generates an issue when compiling without std -// Fake storage proof -const MOCK_PROOF: [u8; 71] = [ - 127, 1, 6, 222, 61, 138, 84, 210, 126, 68, 169, 213, 206, 24, 150, 24, 242, 45, 180, 180, 157, - 149, 50, 13, 144, 33, 153, 76, 133, 15, 37, 184, 227, 133, 144, 0, 0, 32, 0, 0, 0, 16, 0, 8, 0, - 0, 0, 0, 4, 0, 0, 0, 1, 0, 0, 5, 0, 0, 0, 5, 0, 0, 0, 6, 0, 0, 0, 6, 0, 0, 0, -]; - -// fake storage root. This is valid with the previous proof -const MOCK_PROOF_HASH: [u8; 32] = [ - 216, 6, 227, 175, 180, 211, 98, 117, 202, 245, 206, 51, 21, 143, 100, 232, 96, 217, 14, 71, - 243, 146, 7, 202, 245, 129, 165, 70, 72, 184, 130, 141, -]; /// Default balance amount is minimum contribution fn default_balance() -> BalanceOf { @@ -67,33 +43,6 @@ fn create_funded_user( user } -fn create_inherent_data(block_number: u32) -> InherentData { - //let (relay_parent_storage_root, relay_chain_state) = create_fake_valid_proof(); - let vfp = PersistedValidationData { - relay_parent_number: block_number as RelayChainBlockNumber, - relay_parent_storage_root: MOCK_PROOF_HASH.into(), - max_pov_size: 1000u32, - parent_head: HeadData(vec![1, 1, 1]), - }; - let inherent_data = { - let mut inherent_data = InherentData::default(); - let system_inherent_data = ParachainInherentData { - validation_data: vfp.clone(), - relay_chain_state: StorageProof::new(vec![MOCK_PROOF.to_vec()]), - downward_messages: Default::default(), - horizontal_messages: Default::default(), - }; - inherent_data - .put_data( - cumulus_primitives_parachain_inherent::INHERENT_IDENTIFIER, - &system_inherent_data, - ) - .expect("failed to put VFP inherent"); - inherent_data - }; - inherent_data -} - /// Create contributors. fn create_contributors( total_number: u32, @@ -135,7 +84,7 @@ fn insert_contributors( } /// Create a Contributor. -fn close_initialization(end_relay: RelayChainBlockNumber) -> Result<(), &'static str> { +fn close_initialization(end_relay: T::VestingBlockNumber) -> Result<(), &'static str> { Pallet::::complete_initialization(RawOrigin::Root.into(), end_relay)?; Ok(()) } @@ -197,16 +146,10 @@ benchmarks! { insert_contributors::(contributors)?; // We need to create the first block inherent, to initialize the initRelayBlock - let first_block_inherent = create_inherent_data::(1u32); - RelayPallet::::on_initialize(T::BlockNumber::one()); - RelayPallet::::create_inherent(&first_block_inherent) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); - RelayPallet::::on_finalize(T::BlockNumber::one()); + T::VestingBlockProvider::set_block_number(1u32.into()); Pallet::::on_finalize(T::BlockNumber::one()); - }: _(RawOrigin::Root, 10u32) + }: _(RawOrigin::Root, 10u32.into()) verify { assert!(Pallet::::initialized()); } @@ -231,29 +174,19 @@ benchmarks! { close_initialization::(10u32.into())?; // First inherent - let first_block_inherent = create_inherent_data::(1u32); - RelayPallet::::on_initialize(T::BlockNumber::one()); - RelayPallet::::create_inherent(&first_block_inherent) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); - RelayPallet::::on_finalize(T::BlockNumber::one()); + T::VestingBlockProvider::set_block_number(1u32.into()); Pallet::::on_finalize(T::BlockNumber::one()); - // Create 4th relay block, by now the user should have vested some amount - RelayPallet::::on_initialize(4u32.into()); + // Claimed + let claimed_reward = Pallet::::accounts_payable(&caller).unwrap().claimed_reward; - let last_block_inherent = create_inherent_data::(4u32); - RelayPallet::::create_inherent(&last_block_inherent) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); + // Create 4th relay block, by now the user should have vested some amount + T::VestingBlockProvider::set_block_number(4u32.into()); - RelayPallet::::on_finalize(4u32.into()); }: _(RawOrigin::Signed(caller.clone())) verify { - assert_eq!(Pallet::::accounts_payable(&caller).unwrap().total_reward, (100u32.into())); + assert!(Pallet::::accounts_payable(&caller).unwrap().claimed_reward > claimed_reward); } update_reward_address { @@ -277,26 +210,12 @@ benchmarks! { close_initialization::(10u32.into())?; // First inherent - let first_block_inherent = create_inherent_data::(1u32); - RelayPallet::::on_initialize(T::BlockNumber::one()); - RelayPallet::::create_inherent(&first_block_inherent) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); - RelayPallet::::on_finalize(T::BlockNumber::one()); + T::VestingBlockProvider::set_block_number(1u32.into()); Pallet::::on_finalize(T::BlockNumber::one()); // Let's advance the relay so that the vested amount get transferred - - RelayPallet::::on_initialize(4u32.into()); - let last_block_inherent = create_inherent_data::(4u32); - RelayPallet::::create_inherent(&last_block_inherent) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); - - RelayPallet::::on_finalize(4u32.into()); + T::VestingBlockProvider::set_block_number(4u32.into()); // The new user let new_user = create_funded_user::("user", SEED+1, 0u32.into()); @@ -330,13 +249,7 @@ benchmarks! { close_initialization::(10u32.into())?; // First inherent - let first_block_inherent = create_inherent_data::(1u32); - RelayPallet::::on_initialize(T::BlockNumber::one()); - RelayPallet::::create_inherent(&first_block_inherent) - .expect("got an inherent") - .dispatch_bypass_filter(RawOrigin::None.into()) - .expect("dispatch succeeded"); - RelayPallet::::on_finalize(T::BlockNumber::one()); + T::VestingBlockProvider::set_block_number(1u32.into()); Pallet::::on_finalize(T::BlockNumber::one()); }: _(RawOrigin::Signed(caller.clone()), caller.clone(), relay_account.into(), signature) @@ -347,9 +260,7 @@ benchmarks! { } #[cfg(test)] mod tests { - use super::*; use crate::mock::Test; - use frame_support::assert_ok; use sp_io::TestExternalities; pub fn new_test_ext() -> TestExternalities { @@ -358,37 +269,6 @@ mod tests { .unwrap(); TestExternalities::new(t) } - - #[test] - fn bench_init_reward_vec() { - new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_initialize_reward_vec::()); - }); - } - #[test] - fn bench_complete_initialization() { - new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_complete_initialization::()); - }); - } - #[test] - fn bench_claim() { - new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_claim::()); - }); - } - #[test] - fn bench_update_reward_address() { - new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_update_reward_address::()); - }); - } - #[test] - fn bench_associate_native_identity() { - new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_associate_native_identity::()); - }); - } } impl_benchmark_test_suite!( From f79ddb96a4ca6b3d2a093a3d3522534c253bd7a6 Mon Sep 17 00:00:00 2001 From: gorka Date: Wed, 1 Sep 2021 16:06:54 +0200 Subject: [PATCH 20/21] Add comment to not forget the removal of the trait once the substrate PR is merged --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 74b6018..d6e7bd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -96,6 +96,7 @@ pub mod pallet { pub struct RelayChainBeacon(PhantomData); /// Something that can provide a block number + /// To be removed once https://github.com/paritytech/substrate/pull/9668 is merged pub trait BlockProvider { /// Type of `BlockNumber` to provide. type BlockNumber: parity_scale_codec::Codec + Clone + Ord + Eq + AtLeast32BitUnsigned; From 722da9a3671bac44d4b02f51226f1a34fb54a816 Mon Sep 17 00:00:00 2001 From: gorka Date: Wed, 1 Sep 2021 16:10:08 +0200 Subject: [PATCH 21/21] Add another comment so that we ensure we dont forget to change the provider --- src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index d6e7bd0..e9c18e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -144,6 +144,8 @@ pub mod pallet { /// The notion of time that will be used for vesting. Probably /// either the relay chain or sovereign chain block number. + /// This should change to use sp_runtime::traits::BlockNumberProvider once + /// https://github.com/paritytech/substrate/pull/9668 is merged type VestingBlockProvider: BlockProvider; type WeightInfo: WeightInfo;