From c5609de0e8dbc70faeb096e2f53c8c17db705532 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 14 Mar 2023 15:39:43 -0500 Subject: [PATCH 01/12] Add self-bond filter condition when computing new set of collators Signed-off-by: Adam Reif --- pallets/parachain-staking/src/lib.rs | 9 ++- .../integrations_mock/integration_tests.rs | 67 +++++++++++++++++++ .../calamari/tests/integrations_mock/mod.rs | 1 + 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index c072e195e..5275f2101 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -1688,7 +1688,14 @@ pub mod pallet { .into_iter() .rev() .take(top_n) - .filter(|x| x.amount >= T::MinCollatorStk::get()) + .filter(|x| { + // Only consider collators above minimum total stake and self-bond + x.amount >= T::MinCollatorStk::get() + && Self::candidate_info(x.owner.clone()) + .expect("looping candidates. therefore canidateinfo exists. qed") + .bond + > T::MinCandidateStk::get() + }) .map(|x| x.owner) .collect::>(); collators.sort(); diff --git a/runtime/calamari/tests/integrations_mock/integration_tests.rs b/runtime/calamari/tests/integrations_mock/integration_tests.rs index d9fdeb85e..03e75527a 100644 --- a/runtime/calamari/tests/integrations_mock/integration_tests.rs +++ b/runtime/calamari/tests/integrations_mock/integration_tests.rs @@ -604,6 +604,73 @@ fn collator_cant_join_below_standard_bond() { }); } +#[test] +fn collator_with_large_stake_but_too_low_self_bond_not_selected_for_block_production() { + ExtBuilder::default() + .with_balances(vec![ + (ALICE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (BOB.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (CHARLIE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (DAVE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (EVE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (FERDIE.clone(), EARLY_COLLATOR_MINIMUM_STAKE + 100), + (USER.clone(), 40_000_000 * KMA), + ]) + .with_invulnerables(vec![]) + .with_authorities(vec![ + (ALICE.clone(), ALICE_SESSION_KEYS.clone()), + (BOB.clone(), BOB_SESSION_KEYS.clone()), + (CHARLIE.clone(), CHARLIE_SESSION_KEYS.clone()), + (DAVE.clone(), DAVE_SESSION_KEYS.clone()), + (EVE.clone(), EVE_SESSION_KEYS.clone()), + (FERDIE.clone(), FERDIE_SESSION_KEYS.clone()), + ]) + .build() + .execute_with(|| { + initialize_collators_through_whitelist(vec![ + ALICE.clone(), + BOB.clone(), + CHARLIE.clone(), + DAVE.clone(), + EVE.clone(), + FERDIE.clone(), + ]); + // Increase bond for everyone but FERDIE + for collator in vec![ + ALICE.clone(), + BOB.clone(), + CHARLIE.clone(), + DAVE.clone(), + EVE.clone(), + ] { + assert_ok!(ParachainStaking::candidate_bond_more( + Origin::signed(collator.clone()), + MIN_BOND_TO_BE_CONSIDERED_COLLATOR - EARLY_COLLATOR_MINIMUM_STAKE + )); + } + + // Delegate a large amount of tokens + for collator in vec![EVE.clone(), USER.clone()] { + assert_ok!(ParachainStaking::delegate( + Origin::signed(USER.clone()), + collator, + 10_000_000 * KMA, + 50, + 50 + )); + } + + // Ensure FERDIE is not selected despite having a large stake + // NOTE: Must use 6 or more collators because 5 is the minimum on calamari + assert!(ParachainStaking::compute_top_candidates().contains(&ALICE)); + assert!(ParachainStaking::compute_top_candidates().contains(&BOB)); + assert!(ParachainStaking::compute_top_candidates().contains(&CHARLIE)); + assert!(ParachainStaking::compute_top_candidates().contains(&DAVE)); + assert!(ParachainStaking::compute_top_candidates().contains(&EVE)); + assert!(!ParachainStaking::compute_top_candidates().contains(&FERDIE)); + }); +} + #[test] fn collator_can_leave_if_below_standard_bond() { ExtBuilder::default() diff --git a/runtime/calamari/tests/integrations_mock/mod.rs b/runtime/calamari/tests/integrations_mock/mod.rs index 8a7b126aa..e6dce9072 100644 --- a/runtime/calamari/tests/integrations_mock/mod.rs +++ b/runtime/calamari/tests/integrations_mock/mod.rs @@ -44,6 +44,7 @@ lazy_static! { pub(crate) static ref DAVE: AccountId = unchecked_account_id::("Dave"); pub(crate) static ref EVE: AccountId = unchecked_account_id::("Eve"); pub(crate) static ref FERDIE: AccountId = unchecked_account_id::("Ferdie"); + pub(crate) static ref USER: AccountId = unchecked_account_id::("User"); pub(crate) static ref ALICE_SESSION_KEYS: SessionKeys = SessionKeys::from_seed_unchecked("Alice"); pub(crate) static ref BOB_SESSION_KEYS: SessionKeys = SessionKeys::from_seed_unchecked("Bob"); From bc17a566ca3ff45e8aa61cbd275c2c1112fb0330 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 14 Mar 2023 15:44:49 -0500 Subject: [PATCH 02/12] fix Signed-off-by: Adam Reif --- runtime/calamari/tests/integrations_mock/integration_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/calamari/tests/integrations_mock/integration_tests.rs b/runtime/calamari/tests/integrations_mock/integration_tests.rs index 03e75527a..b5ac5782a 100644 --- a/runtime/calamari/tests/integrations_mock/integration_tests.rs +++ b/runtime/calamari/tests/integrations_mock/integration_tests.rs @@ -650,7 +650,7 @@ fn collator_with_large_stake_but_too_low_self_bond_not_selected_for_block_produc } // Delegate a large amount of tokens - for collator in vec![EVE.clone(), USER.clone()] { + for collator in vec![EVE.clone(), FERDIE.clone()] { assert_ok!(ParachainStaking::delegate( Origin::signed(USER.clone()), collator, From a33edac8f3fbb6946f9dd3fd8963a9768ec9e571 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 14 Mar 2023 16:19:24 -0500 Subject: [PATCH 03/12] fix2 Signed-off-by: Adam Reif --- pallets/parachain-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 5275f2101..1983087b0 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -1694,7 +1694,7 @@ pub mod pallet { && Self::candidate_info(x.owner.clone()) .expect("looping candidates. therefore canidateinfo exists. qed") .bond - > T::MinCandidateStk::get() + >= T::MinCandidateStk::get() }) .map(|x| x.owner) .collect::>(); From f23bf8fee90bc0359e4cfc29924a10083c5e3adf Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Wed, 15 Mar 2023 10:16:07 -0500 Subject: [PATCH 04/12] fix: sequence bug in parachain_staking::compute_top_candidates() Signed-off-by: Adam Reif --- pallets/parachain-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 1983087b0..907fdf3c5 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -1687,7 +1687,6 @@ pub mod pallet { let mut collators = candidates .into_iter() .rev() - .take(top_n) .filter(|x| { // Only consider collators above minimum total stake and self-bond x.amount >= T::MinCollatorStk::get() @@ -1696,6 +1695,7 @@ pub mod pallet { .bond >= T::MinCandidateStk::get() }) + .take(top_n) .map(|x| x.owner) .collect::>(); collators.sort(); From 906cb4a9242097e4d97de116ba5cb7d5a1841ccf Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Wed, 15 Mar 2023 10:47:53 -0500 Subject: [PATCH 05/12] fix testing, collator 6 from whitelist was never selected Signed-off-by: Adam Reif --- .../integrations_mock/integration_tests.rs | 32 +++++++++---------- .../calamari/tests/integrations_mock/mod.rs | 1 + 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/runtime/calamari/tests/integrations_mock/integration_tests.rs b/runtime/calamari/tests/integrations_mock/integration_tests.rs index b5ac5782a..f39fb231a 100644 --- a/runtime/calamari/tests/integrations_mock/integration_tests.rs +++ b/runtime/calamari/tests/integrations_mock/integration_tests.rs @@ -608,13 +608,13 @@ fn collator_cant_join_below_standard_bond() { fn collator_with_large_stake_but_too_low_self_bond_not_selected_for_block_production() { ExtBuilder::default() .with_balances(vec![ - (ALICE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (ALICE.clone(), EARLY_COLLATOR_MINIMUM_STAKE + 100), (BOB.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), (CHARLIE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), (DAVE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), (EVE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), - (FERDIE.clone(), EARLY_COLLATOR_MINIMUM_STAKE + 100), - (USER.clone(), 40_000_000 * KMA), + (FERDIE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (USER.clone(), 400_000_000 * KMA), ]) .with_invulnerables(vec![]) .with_authorities(vec![ @@ -635,13 +635,13 @@ fn collator_with_large_stake_but_too_low_self_bond_not_selected_for_block_produc EVE.clone(), FERDIE.clone(), ]); - // Increase bond for everyone but FERDIE + // Increase self-bond for everyone but ALICE for collator in vec![ - ALICE.clone(), BOB.clone(), CHARLIE.clone(), DAVE.clone(), EVE.clone(), + FERDIE.clone(), ] { assert_ok!(ParachainStaking::candidate_bond_more( Origin::signed(collator.clone()), @@ -649,25 +649,25 @@ fn collator_with_large_stake_but_too_low_self_bond_not_selected_for_block_produc )); } - // Delegate a large amount of tokens - for collator in vec![EVE.clone(), FERDIE.clone()] { + // Delegate a large amount of tokens to EVE and ALICE + for collator in vec![EVE.clone(), ALICE.clone()] { assert_ok!(ParachainStaking::delegate( Origin::signed(USER.clone()), collator, - 10_000_000 * KMA, + 100_000_000 * KMA, 50, 50 )); } - // Ensure FERDIE is not selected despite having a large stake + // Ensure ALICE is not selected despite having a large stake although it has a large total stake // NOTE: Must use 6 or more collators because 5 is the minimum on calamari - assert!(ParachainStaking::compute_top_candidates().contains(&ALICE)); + assert!(!ParachainStaking::compute_top_candidates().contains(&ALICE)); assert!(ParachainStaking::compute_top_candidates().contains(&BOB)); assert!(ParachainStaking::compute_top_candidates().contains(&CHARLIE)); assert!(ParachainStaking::compute_top_candidates().contains(&DAVE)); assert!(ParachainStaking::compute_top_candidates().contains(&EVE)); - assert!(!ParachainStaking::compute_top_candidates().contains(&FERDIE)); + assert!(ParachainStaking::compute_top_candidates().contains(&FERDIE)); }); } @@ -713,12 +713,12 @@ fn collator_can_leave_if_below_standard_bond() { fn collator_with_400k_not_selected_for_block_production() { ExtBuilder::default() .with_balances(vec![ - (ALICE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), + (ALICE.clone(), EARLY_COLLATOR_MINIMUM_STAKE + 100), (BOB.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), (CHARLIE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), (DAVE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), (EVE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), - (FERDIE.clone(), EARLY_COLLATOR_MINIMUM_STAKE + 100), + (FERDIE.clone(), MIN_BOND_TO_BE_CONSIDERED_COLLATOR + 100), ]) .with_invulnerables(vec![]) .with_authorities(vec![ @@ -741,11 +741,11 @@ fn collator_with_400k_not_selected_for_block_production() { ]); // Increase bond for everyone but FERDIE for collator in vec![ - ALICE.clone(), BOB.clone(), CHARLIE.clone(), DAVE.clone(), EVE.clone(), + FERDIE.clone(), ] { assert_ok!(ParachainStaking::candidate_bond_more( Origin::signed(collator.clone()), @@ -755,12 +755,12 @@ fn collator_with_400k_not_selected_for_block_production() { // Ensure CHARLIE and later are not selected // NOTE: Must use 6 or more collators because 5 is the minimum on calamari - assert!(ParachainStaking::compute_top_candidates().contains(&ALICE)); + assert!(!ParachainStaking::compute_top_candidates().contains(&ALICE)); assert!(ParachainStaking::compute_top_candidates().contains(&BOB)); assert!(ParachainStaking::compute_top_candidates().contains(&CHARLIE)); assert!(ParachainStaking::compute_top_candidates().contains(&DAVE)); assert!(ParachainStaking::compute_top_candidates().contains(&EVE)); - assert!(!ParachainStaking::compute_top_candidates().contains(&FERDIE)); + assert!(ParachainStaking::compute_top_candidates().contains(&FERDIE)); }); } diff --git a/runtime/calamari/tests/integrations_mock/mod.rs b/runtime/calamari/tests/integrations_mock/mod.rs index e6dce9072..6ba009796 100644 --- a/runtime/calamari/tests/integrations_mock/mod.rs +++ b/runtime/calamari/tests/integrations_mock/mod.rs @@ -97,4 +97,5 @@ pub fn initialize_collators_through_whitelist(collators: Vec) { ParachainStaking::candidate_pool().len(), candidate_count as usize ); + assert_ok!(ParachainStaking::set_total_selected(root_origin(), candidate_count)); } From c986f9b362164f8edd6cbeed8f003bf862d52b0d Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Wed, 15 Mar 2023 10:48:21 -0500 Subject: [PATCH 06/12] fmt Signed-off-by: Adam Reif --- runtime/calamari/tests/integrations_mock/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/runtime/calamari/tests/integrations_mock/mod.rs b/runtime/calamari/tests/integrations_mock/mod.rs index 6ba009796..2a01ad17f 100644 --- a/runtime/calamari/tests/integrations_mock/mod.rs +++ b/runtime/calamari/tests/integrations_mock/mod.rs @@ -97,5 +97,8 @@ pub fn initialize_collators_through_whitelist(collators: Vec) { ParachainStaking::candidate_pool().len(), candidate_count as usize ); - assert_ok!(ParachainStaking::set_total_selected(root_origin(), candidate_count)); + assert_ok!(ParachainStaking::set_total_selected( + root_origin(), + candidate_count + )); } From 298276d9babe4ae05490bc31ff858ffd4d8fddde Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Thu, 16 Mar 2023 09:24:38 -0500 Subject: [PATCH 07/12] fix comment Signed-off-by: Adam Reif --- runtime/calamari/tests/integrations_mock/integration_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/calamari/tests/integrations_mock/integration_tests.rs b/runtime/calamari/tests/integrations_mock/integration_tests.rs index f39fb231a..a312b7910 100644 --- a/runtime/calamari/tests/integrations_mock/integration_tests.rs +++ b/runtime/calamari/tests/integrations_mock/integration_tests.rs @@ -660,7 +660,7 @@ fn collator_with_large_stake_but_too_low_self_bond_not_selected_for_block_produc )); } - // Ensure ALICE is not selected despite having a large stake although it has a large total stake + // Ensure ALICE is not selected despite having a large total stake through delegation // NOTE: Must use 6 or more collators because 5 is the minimum on calamari assert!(!ParachainStaking::compute_top_candidates().contains(&ALICE)); assert!(ParachainStaking::compute_top_candidates().contains(&BOB)); From 7fcb00d1a8e293c213361da0b0e230e88ff4f004 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Thu, 16 Mar 2023 09:42:33 -0500 Subject: [PATCH 08/12] explicit is_candidate check Signed-off-by: Adam Reif --- pallets/parachain-staking/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 907fdf3c5..6001ef461 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -1690,8 +1690,9 @@ pub mod pallet { .filter(|x| { // Only consider collators above minimum total stake and self-bond x.amount >= T::MinCollatorStk::get() + && Self::is_candidate(x.owner.clone()) && Self::candidate_info(x.owner.clone()) - .expect("looping candidates. therefore canidateinfo exists. qed") + .expect("is_candidate => canidateinfo exists. qed") .bond >= T::MinCandidateStk::get() }) From b6ae25caae6abab616c8e1aaacc14f324f5350f4 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Thu, 16 Mar 2023 09:49:57 -0500 Subject: [PATCH 09/12] syntax Signed-off-by: Adam Reif --- pallets/parachain-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 6001ef461..45e77a38b 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -1690,7 +1690,7 @@ pub mod pallet { .filter(|x| { // Only consider collators above minimum total stake and self-bond x.amount >= T::MinCollatorStk::get() - && Self::is_candidate(x.owner.clone()) + && Self::is_candidate(&x.owner) && Self::candidate_info(x.owner.clone()) .expect("is_candidate => canidateinfo exists. qed") .bond From 25761ffbcc43f985959698ddea71abefbf517359 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Thu, 16 Mar 2023 10:13:56 -0500 Subject: [PATCH 10/12] don't panic in compute Signed-off-by: Adam Reif --- pallets/parachain-staking/src/lib.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 45e77a38b..7e7b281d5 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -1689,12 +1689,13 @@ pub mod pallet { .rev() .filter(|x| { // Only consider collators above minimum total stake and self-bond - x.amount >= T::MinCollatorStk::get() - && Self::is_candidate(&x.owner) - && Self::candidate_info(x.owner.clone()) - .expect("is_candidate => canidateinfo exists. qed") - .bond - >= T::MinCandidateStk::get() + x.amount >= T::MinCollatorStk::get() && + if let Some(info) = Self::candidate_info(x.owner.clone()) { + info.bond >= T::MinCandidateStk::get() + } else { + log::error!("Candidate did not have CandidateInfo in storage, this should not happen"); + false + } }) .take(top_n) .map(|x| x.owner) From 3ae54d018f122c2749bad4716b4a50f1b1781577 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Thu, 16 Mar 2023 11:23:21 -0500 Subject: [PATCH 11/12] Use nightly-2023-03-13 for fmt/clippy Signed-off-by: Adam Reif --- .github/workflows/run_linters.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run_linters.yml b/.github/workflows/run_linters.yml index 0dbff7c60..8f3ce4764 100644 --- a/.github/workflows/run_linters.yml +++ b/.github/workflows/run_linters.yml @@ -72,8 +72,8 @@ jobs: run: | curl -s https://sh.rustup.rs -sSf | sh -s -- -y source ${HOME}/.cargo/env - rustup toolchain install nightly - rustup default nightly + rustup toolchain install nightly-2023-03-13 + rustup default nightly-2023-03-13 cargo install taplo-cli - name: Check Formatting env: From 7f1042f1aa3e0354483a695f71dc27b2bd5058a4 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Thu, 16 Mar 2023 12:44:46 -0500 Subject: [PATCH 12/12] apt-get install yarn Signed-off-by: Adam Reif --- .github/workflows/integration_test_calamari.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration_test_calamari.yml b/.github/workflows/integration_test_calamari.yml index a8d4b4a4e..6ce37d0a1 100644 --- a/.github/workflows/integration_test_calamari.yml +++ b/.github/workflows/integration_test_calamari.yml @@ -76,7 +76,7 @@ jobs: - name: init run: | sudo apt update - sudo apt install -y pkg-config libssl-dev + sudo apt install -y pkg-config libssl-dev yarn curl -s https://sh.rustup.rs -sSf | sh -s -- -y source ${HOME}/.cargo/env rustup toolchain install stable