Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove tight coupling to parachain system #43

Merged
merged 15 commits into from
Sep 14, 2021
Merged

Remove tight coupling to parachain system #43

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f57daeb
prune nimbus-primitives
JoshOrndorff Aug 23, 2021
5fead18
update branches and toolchain
JoshOrndorff Aug 23, 2021
6238fc7
reflect new events from https://github.com/paritytech/substrate/pull/…
JoshOrndorff Aug 23, 2021
bc27634
core change. pallet compiles.
JoshOrndorff Aug 23, 2021
15f1f93
remove some seemingly-unnecessary trait bounds
JoshOrndorff Aug 23, 2021
c88c9da
fix test mock
JoshOrndorff Aug 23, 2021
f6b10a8
make cumulus stuff dev-dependencies
JoshOrndorff Aug 23, 2021
2b348a0
use old storage location so we don't need a migration
JoshOrndorff Sep 2, 2021
d04a121
update accessor names and coments
JoshOrndorff Sep 2, 2021
214824f
make cumulus dependencies optional and use them when building benchmarks
JoshOrndorff Sep 2, 2021
47af150
Merge branch 'main' into joshy-loose-coupling
JoshOrndorff Sep 2, 2021
87c6e88
cargo fmt
JoshOrndorff Sep 2, 2021
15bf43c
Make benchmarks build as long as parachan system is persent.
JoshOrndorff Sep 7, 2021
7d694cc
blanket impl should make it work
JoshOrndorff Sep 9, 2021
60003f9
fix no_std
JoshOrndorff Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +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" }
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" }
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 }
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", branch = "nimbus-polkadot-v0.9.9" }
cumulus-pallet-parachain-system = { git = "https://github.com/purestake/cumulus", branch = "nimbus-polkadot-v0.9.9" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go as optional depencencies (and enable them in runtime-benchmarks feature) so that benchmarking uses them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done this, but we still have the problem that we can't initialize pallet parachain system in the benchmarks. The problem is that even though our mock runtime does have parachain system installed, the benchmarks are generic over a runtime T and it is not guaranteed that every T will have that pallet installed. Previously this was guaranteed by the trait bound Config: cumulus_pallet_parachain_system::Config.

So I think we need paritytech/substrate#9668 even for this PR.

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"]
Expand All @@ -44,10 +43,6 @@ std = [
"log/std",
"sp-std/std",
"sp-io/std",
"cumulus-pallet-parachain-system/std",
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved
"cumulus-primitives-core/std",
"nimbus-primitives/std",
"cumulus-primitives-parachain-inherent/std",
]
runtime-benchmarks = [
"frame-benchmarking",
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[toolchain]
channel = "nightly-2021-02-21"
channel = "nightly-2021-08-01"
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved
components = [ "rustfmt", "clippy" ]
targets = [ "wasm32-unknown-unknown" ]
profile = "minimal"
51 changes: 24 additions & 27 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,15 @@ 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::Vec;
use sp_std::vec;
Expand All @@ -99,7 +97,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<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
/// Checker for the reward vec, is it initalized already?
Expand All @@ -114,17 +112,21 @@ 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.
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved
+ 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;
}
Expand All @@ -149,10 +151,7 @@ pub mod pallet {
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
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());
}
}
}
Expand Down Expand Up @@ -259,21 +258,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 {
Expand Down Expand Up @@ -343,7 +340,7 @@ pub mod pallet {
#[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)?;

Expand All @@ -357,7 +354,7 @@ pub mod pallet {

// This ensures the lease ending block is bigger than the init relay block
ensure!(
lease_ending_block > InitRelayBlock::<T>::get(),
lease_ending_block > InitVestingBlock::<T>::get(),
Error::<T>::VestingPeriodNonValid
);

Expand All @@ -381,7 +378,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);

Expand Down Expand Up @@ -602,12 +599,12 @@ pub mod pallet {
#[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>;
type InitVestingBlock<T: Config> = StorageValue<_, T::VestingBlockNumber, ValueQuery>;
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved

#[pallet::storage]
#[pallet::getter(fn end_relay_block)]
/// Relay block height at the initialization of the pallet
type EndRelayBlock<T: Config> = StorageValue<_, relay_chain::BlockNumber, ValueQuery>;
type EndVestingBlock<T: Config> = StorageValue<_, T::VestingBlockNumber, ValueQuery>;

#[pallet::storage]
#[pallet::getter(fn init_reward_amount)]
Expand Down
2 changes: 2 additions & 0 deletions src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self>;
type WeightInfo = ();
}

Expand Down
2 changes: 2 additions & 0 deletions src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down