From 39761dafe6993dd7880188a69999a6cc567f4413 Mon Sep 17 00:00:00 2001 From: b-yap <2826165+b-yap@users.noreply.github.com> Date: Mon, 12 Jun 2023 16:15:33 +0800 Subject: [PATCH] fix test build issues of parachain-staking. The order of the pallets during runtime construction has changed. see https://github.com/KILTprotocol/kilt-node/pull/416 --- pallets/parachain-staking/src/lib.rs | 3 + pallets/parachain-staking/src/mock.rs | 16 +- pallets/parachain-staking/src/set.rs | 15 +- pallets/parachain-staking/src/tests.rs | 4 +- pallets/parachain-staking/src/try_state.rs | 244 +++++++++++++++++++++ runtime/amplitude/src/lib.rs | 10 +- runtime/development/src/lib.rs | 4 +- runtime/foucoco/src/lib.rs | 10 +- runtime/pendulum/src/lib.rs | 10 +- 9 files changed, 289 insertions(+), 27 deletions(-) create mode 100644 pallets/parachain-staking/src/try_state.rs diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 0716fd80b..fce694cab 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -125,6 +125,9 @@ pub(crate) mod mock; #[cfg(test)] pub(crate) mod tests; +#[cfg(any(feature = "try-runtime", test))] +mod try_state; + mod inflation; mod set; mod types; diff --git a/pallets/parachain-staking/src/mock.rs b/pallets/parachain-staking/src/mock.rs index 73c9aecb0..7f9199c1f 100644 --- a/pallets/parachain-staking/src/mock.rs +++ b/pallets/parachain-staking/src/mock.rs @@ -24,7 +24,6 @@ use crate::{self as stake, types::NegativeImbalanceOf}; use frame_support::{ assert_ok, construct_runtime, parameter_types, traits::{Currency, GenesisBuild, OnFinalize, OnInitialize, OnUnbalanced}, - weights::Weight, }; use pallet_authorship::EventHandler; use sp_consensus_aura::sr25519::AuthorityId; @@ -58,16 +57,15 @@ construct_runtime!( { System: frame_system::{Pallet, Call, Config, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Authorship: pallet_authorship::{Pallet, Storage}, - StakePallet: stake::{Pallet, Call, Storage, Config, Event}, - Session: pallet_session::{Pallet, Call, Storage, Event, Config}, Aura: pallet_aura::{Pallet, Storage}, + Session: pallet_session::{Pallet, Call, Storage, Event, Config}, + StakePallet: stake::{Pallet, Call, Storage, Config, Event}, + Authorship: pallet_authorship::{Pallet, Storage}, } ); parameter_types! { pub const BlockHashCount: u64 = 250; - pub const MaximumBlockWeight: Weight = Weight::from_parts(1024,0); pub const MaximumBlockLength: u32 = 2 * 1024; pub const AvailableBlockRatio: Perbill = Perbill::one(); pub const SS58Prefix: u8 = 42; @@ -341,6 +339,13 @@ impl ExtBuilder { ext.execute_with(|| System::set_block_number(1)); ext } + pub fn build_and_execute_with_sanity_tests(self, test: impl FnOnce()) { + self.build().execute_with(|| { + test(); + crate::try_state::do_try_state::() + .expect("Sanity test for parachain staking failed."); + }) + } } /// Compare whether the difference of both sides is at most `precision * left`. @@ -384,6 +389,7 @@ pub(crate) fn roll_to_claim_rewards(n: BlockNumber, authors: Vec>(BoundedVec); -impl> OrderedSet { +impl> OrderedSet { /// Create a new empty set. pub fn new() -> Self { Self(BoundedVec::default()) } + pub fn iter(&self) -> sp_std::slice::Iter<'_, T> { + self.0.iter() + } + /// Creates an ordered set from a `BoundedVec`. /// /// The vector will be sorted reversily (from greatest to lowest) and @@ -52,7 +55,7 @@ impl> OrderedSet { let mut v = bv.into_inner(); v.sort_by(|a, b| b.cmp(a)); v.dedup(); - Self::from_sorted_set(v.try_into().expect("No values were added")) + Self::from_sorted_set(v.try_into().map_err(|_| ()).expect("No values were added")) } /// Create a set from a `BoundedVec`. @@ -233,9 +236,9 @@ impl> OrderedSet { } } -impl> From> for OrderedSet { +impl> From> for OrderedSet { fn from(bv: BoundedVec) -> Self { - OrderedSet::::from(bv) + Self::from(bv) } } diff --git a/pallets/parachain-staking/src/tests.rs b/pallets/parachain-staking/src/tests.rs index e3cf82641..2b471019f 100644 --- a/pallets/parachain-staking/src/tests.rs +++ b/pallets/parachain-staking/src/tests.rs @@ -3143,8 +3143,7 @@ fn authorities_per_round() { (11, 100 * stake), ]) .with_collators(vec![(1, stake), (2, stake), (3, stake), (4, stake)]) - .build() - .execute_with(|| { + .build_and_execute_with_sanity_tests(|| { assert_eq!(StakePallet::selected_candidates().into_inner(), vec![1, 2]); // reward 1 once per round let authors: Vec> = @@ -3168,6 +3167,7 @@ fn authorities_per_round() { // roll to last block of round 2 // should multiply with 4 because there are only 4 candidates roll_to_claim_rewards(14, authors.clone()); + let reward_2 = inflation.collator.reward_rate.per_block * stake * 4; assert_eq!(Balances::free_balance(1), stake + reward_0 + reward_1 + reward_2); diff --git a/pallets/parachain-staking/src/try_state.rs b/pallets/parachain-staking/src/try_state.rs new file mode 100644 index 000000000..c2e58bb85 --- /dev/null +++ b/pallets/parachain-staking/src/try_state.rs @@ -0,0 +1,244 @@ +// KILT Blockchain – https://botlabs.org +// Copyright (C) 2019-2023 BOTLabs GmbH + +// The KILT Blockchain is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// The KILT Blockchain is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +// If you feel like getting in touch with us, you can do so at info@botlabs.org + +use frame_support::{ensure, traits::Get}; +use scale_info::prelude::format; +use sp_runtime::{ + traits::{CheckedAdd, Zero}, + SaturatedConversion, Saturating, +}; + +use crate::{ + set::OrderedSet, + types::{BalanceOf, Candidate, Stake}, + CandidatePool, Config, DelegatorState, LastDelegation, MaxCollatorCandidateStake, + MaxSelectedCandidates, Pallet, Round, TopCandidates, TotalCollatorStake, +}; + +pub fn log_and_return_error_message(error_message: String) -> &'static str { + log::error!("{}", error_message); + "Sanity test error" +} + +pub(crate) fn do_try_state() -> Result<(), &'static str> { + validate_candiate_pool::()?; + validate_delegators::()?; + validate_top_candidates::()?; + validate_stake::() +} + +fn validate_candiate_pool() -> Result<(), &'static str> { + // check if enough collators are set. + ensure!( + CandidatePool::::count() >= T::MinCollators::get(), + log_and_return_error_message(format!( + "Insufficient collators. Collators count: {:?}. Min required collators: {:?}", + CandidatePool::::count(), + T::MinCollators::get() + )) + ); + + CandidatePool::::iter_values().try_for_each( + |candidate: Candidate, _>| -> Result<(), &'static str> { + let sum_delegations: BalanceOf = candidate + .delegators + .iter() + .fold(Zero::zero(), |acc, stake| acc.saturating_add(stake.amount)); + + // total stake should be the sum of delegators stake + colator stake. + let stake_total = sum_delegations.checked_add(&candidate.stake); + ensure!( + stake_total == Some(candidate.total), + log_and_return_error_message(format!( + "Total stake of collator {:?} does not match. Saved stake: {:?}. Calculated stake: {:?}", + candidate.id, candidate.stake, stake_total + )) + ); + + // Min required stake should be set + ensure!( + candidate.stake >= T::MinCollatorCandidateStake::get(), + log_and_return_error_message(format!( + "Stake of collator {:?} insufficient. Required stake: {:?}. Owned Stake: {:?} ", + candidate.id, + T::MinCollatorCandidateStake::get(), + candidate.stake + )) + ); + + validate_delegators_from_collator::(candidate.delegators)?; + + // check min and max stake for each candidate + ensure!( + candidate.stake <= MaxCollatorCandidateStake::::get(), + log_and_return_error_message(format!( + "Candidate {:?} exceeded stake. Allowed stake: {:?}. Owned Stake: {:?}", + candidate.id, + MaxCollatorCandidateStake::::get(), + candidate.stake + )) + ); + + Ok(()) + }, + ) +} + +fn validate_top_candidates() -> Result<(), &'static str> { + let top_candidates = TopCandidates::::get(); + + // check if enough top candidates are set. + ensure!( + top_candidates.len() >= T::MinRequiredCollators::get().saturated_into(), + log_and_return_error_message(format!( + "Not enough candidates are set. Candidate count: {:?}. Required: {:?}", + top_candidates.len(), + T::MinRequiredCollators::get() + )) + ); + + top_candidates.iter().try_for_each(|stake| -> Result<(), &'static str> { + // top candidates should be part of the candidate pool. + ensure!( + CandidatePool::::contains_key(&stake.owner), + log_and_return_error_message(format!( + "Unknown candidate {:?} in top candidates.", + stake.owner + )) + ); + + // an account can not be candidate and delegator. + ensure!( + DelegatorState::::get(&stake.owner).is_none(), + log_and_return_error_message(format!( + "Account {:?} is delegator and candidate.", + stake.owner + )) + ); + + // a top candidate should be active. + ensure!( + Pallet::::is_active_candidate(&stake.owner).unwrap(), + log_and_return_error_message(format!("Top candidate {:?} is inactive", stake.owner)) + ); + + Ok(()) + }) +} + +fn validate_delegators_from_collator( + delegators: OrderedSet>, T::MaxDelegatorsPerCollator>, +) -> Result<(), &'static str> { + delegators + .iter() + .try_for_each(|delegator_stake| -> Result<(), &'static str> { + let last_delegation = LastDelegation::::get(&delegator_stake.owner); + let round = Round::::get(); + let counter = if last_delegation.round < round.current { + 0u32 + } else { + last_delegation.counter + }; + + // each delegator should not exceed the [MaxDelegationsPerRound] + ensure!( + counter <= T::MaxDelegationsPerRound::get(), + log_and_return_error_message(format!( + "Delegator {:?} exceeded delegations per round. Allowed delegations {:?}. Confirmed delegations {:?}", + delegator_stake.owner, T::MaxDelegationsPerRound::get(), counter + )) + ); + + // each delegator should have the min required stake + ensure!( + delegator_stake.amount >= T::MinDelegatorStake::get(), + log_and_return_error_message(format!( + "Delegator {:?} insufficient stake. Required stake: {:?}. Owned stake: {:?}", + delegator_stake.owner, + T::MinDelegatorStake::get(), + delegator_stake.amount + )) + ); + + ensure!( + DelegatorState::::get(&delegator_stake.owner).is_some(), + log_and_return_error_message(format!("Unknown delegator {:?}", delegator_stake.owner)) + ); + + Ok(()) + }) +} + +fn validate_stake() -> Result<(), &'static str> { + // the total fund has to be the sum over the first [MaxSelectedCandidates] of + // [TopCandidates]. + let top_candidates = TopCandidates::::get(); + let top_n = MaxSelectedCandidates::::get().saturated_into::(); + + let total_stake = TotalCollatorStake::::get(); + + let collator_delegator_stake = top_candidates + .iter() + .take(top_n) + .fold(Zero::zero(), |acc: BalanceOf, details| acc.saturating_add(details.amount)); + + let collator_stake = top_candidates + .iter() + .take(top_n) + .filter_map(|stake| CandidatePool::::get(&stake.owner)) + .fold(Zero::zero(), |acc: BalanceOf, candidate| acc.saturating_add(candidate.stake)); + + let delegator_state = collator_delegator_stake.saturating_sub(collator_stake); + + ensure!( + total_stake.collators == collator_stake, + log_and_return_error_message(format!( + "Corrupted total collator stake. Saved total stake: {:?}. Calculated stake: {:?}", + total_stake.collators, collator_stake + )) + ); + + ensure!( + total_stake.delegators == delegator_state, + log_and_return_error_message(format!( + "Corrupted total delegator stake. Saved total stake: {:?}. Calculated stake: {:?}", + total_stake.delegators, delegator_state + )) + ); + + Ok(()) +} + +fn validate_delegators() -> Result<(), &'static str> { + DelegatorState::::iter_values().try_for_each( + |delegator_details| -> Result<(), &'static str> { + let Some(owner) = &delegator_details.owner else { + return Err("owner not found"); + }; + + ensure!( + CandidatePool::::contains_key(owner), + log_and_return_error_message(format!( + "Collator {:?} not found", + delegator_details.owner + )) + ); + Ok(()) + }, + ) +} diff --git a/runtime/amplitude/src/lib.rs b/runtime/amplitude/src/lib.rs index ebbdd3992..f11278f4e 100644 --- a/runtime/amplitude/src/lib.rs +++ b/runtime/amplitude/src/lib.rs @@ -1185,12 +1185,14 @@ construct_runtime!( Bounties: pallet_bounties::{Pallet, Call, Storage, Event} = 20, ChildBounties: pallet_child_bounties::{Pallet, Call, Storage, Event} = 21, - // Collator support. The order of these 4 are important and shall not change. - Authorship: pallet_authorship::{Pallet, Storage} = 30, - Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 32, + // Consensus support. + // The following order MUST NOT be changed: Aura -> Session -> Staking -> Authorship -> AuraExt + // Dependencies: AuraExt on Aura, Authorship and Session on ParachainStaking Aura: pallet_aura::{Pallet, Storage, Config} = 33, - AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 34, + Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 32, ParachainStaking: parachain_staking::{Pallet, Call, Storage, Event, Config} = 35, + Authorship: pallet_authorship::{Pallet, Storage} = 30, + AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 34, // XCM helpers. XcmpQueue: cumulus_pallet_xcmp_queue::{Pallet, Call, Storage, Event} = 40, diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index 36ddb8214..50fa79b40 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -435,10 +435,10 @@ construct_runtime!( TransactionPayment: pallet_transaction_payment::{Pallet, Storage, Event} = 11, // Collator support. The order of these 4 are important and shall not change. - Authorship: pallet_authorship::{Pallet, Storage} = 20, CollatorSelection: pallet_collator_selection::{Pallet, Call, Storage, Event, Config} = 21, - Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 22, Aura: pallet_aura::{Pallet, Storage, Config} = 23, + Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 22, + Authorship: pallet_authorship::{Pallet, Storage} = 20, AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 24, // XCM helpers. diff --git a/runtime/foucoco/src/lib.rs b/runtime/foucoco/src/lib.rs index f3e01aaf6..1c02508c1 100644 --- a/runtime/foucoco/src/lib.rs +++ b/runtime/foucoco/src/lib.rs @@ -1470,12 +1470,14 @@ construct_runtime!( Bounties: pallet_bounties::{Pallet, Call, Storage, Event} = 20, ChildBounties: pallet_child_bounties::{Pallet, Call, Storage, Event} = 21, - // Collator support. The order of these 4 are important and shall not change. - Authorship: pallet_authorship::{Pallet, Storage} = 30, - Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 32, + // Consensus support. + // The following order MUST NOT be changed: Aura -> Session -> Staking -> Authorship -> AuraExt + // Dependencies: AuraExt on Aura, Authorship and Session on ParachainStaking Aura: pallet_aura::{Pallet, Storage, Config} = 33, - AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 34, + Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 32, ParachainStaking: parachain_staking::{Pallet, Call, Storage, Event, Config} = 35, + Authorship: pallet_authorship::{Pallet, Storage} = 30, + AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 34, // XCM helpers. XcmpQueue: cumulus_pallet_xcmp_queue::{Pallet, Call, Storage, Event} = 40, diff --git a/runtime/pendulum/src/lib.rs b/runtime/pendulum/src/lib.rs index 7b00e5c9c..611a9eae8 100644 --- a/runtime/pendulum/src/lib.rs +++ b/runtime/pendulum/src/lib.rs @@ -973,12 +973,14 @@ construct_runtime!( Bounties: pallet_bounties::{Pallet, Call, Storage, Event} = 20, ChildBounties: pallet_child_bounties::{Pallet, Call, Storage, Event} = 21, - // Collator support. The order of these 4 are important and shall not change. - Authorship: pallet_authorship::{Pallet, Storage} = 30, - Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 32, + // Consensus support. + // The following order MUST NOT be changed: Aura -> Session -> Staking -> Authorship -> AuraExt + // Dependencies: AuraExt on Aura, Authorship and Session on ParachainStaking Aura: pallet_aura::{Pallet, Storage, Config} = 33, - AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 34, + Session: pallet_session::{Pallet, Call, Storage, Event, Config} = 32, ParachainStaking: parachain_staking::{Pallet, Call, Storage, Event, Config} = 35, + Authorship: pallet_authorship::{Pallet, Storage} = 30, + AuraExt: cumulus_pallet_aura_ext::{Pallet, Storage, Config} = 34, // XCM helpers. XcmpQueue: cumulus_pallet_xcmp_queue::{Pallet, Call, Storage, Event} = 40,