From 43552f7dca8f357d38d387b9baea4b75870b76ba Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Tue, 27 Aug 2024 11:22:18 +0200 Subject: [PATCH 01/10] Rotation working --- solo-chains/runtime/starlight/src/lib.rs | 85 ++++++++++++++++++- .../src/tests/collator_assignment_tests.rs | 60 +++++++++++++ .../runtime/starlight/src/tests/common/mod.rs | 17 +++- 3 files changed, 157 insertions(+), 5 deletions(-) diff --git a/solo-chains/runtime/starlight/src/lib.rs b/solo-chains/runtime/starlight/src/lib.rs index 70e384365..05c21de37 100644 --- a/solo-chains/runtime/starlight/src/lib.rs +++ b/solo-chains/runtime/starlight/src/lib.rs @@ -36,6 +36,7 @@ use { }, frame_system::{pallet_prelude::BlockNumberFor, EnsureNever}, nimbus_primitives::NimbusId, + pallet_collator_assignment::{GetRandomnessForNextBlock, RotateCollatorsEveryNSessions}, pallet_initializer as tanssi_initializer, pallet_registrar_runtime_api::ContainerChainGenesisData, pallet_services_payment::{ProvideBlockProductionCost, ProvideCollatorAssignmentCost}, @@ -73,6 +74,7 @@ use { shared as parachains_shared, }, scale_info::TypeInfo, + sp_core::Get, sp_genesis_builder::PresetId, sp_runtime::traits::BlockNumberProvider, sp_std::{ @@ -109,8 +111,8 @@ use { sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, traits::{ - BlakeTwo256, Block as BlockT, ConstU32, Extrinsic as ExtrinsicT, IdentityLookup, - Keccak256, OpaqueKeys, SaturatedConversion, Verify, Zero, + BlakeTwo256, Block as BlockT, ConstU32, Extrinsic as ExtrinsicT, Hash as HashT, + IdentityLookup, Keccak256, OpaqueKeys, SaturatedConversion, Verify, Zero, }, transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity}, ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill, Percent, Permill, RuntimeDebug, @@ -2633,6 +2635,80 @@ impl tanssi_initializer::Config for Runtime { type SessionHandler = OwnApplySession; } +pub struct BabeCurrentBlockRandomnessGetter; +impl BabeCurrentBlockRandomnessGetter { + fn get_block_randomness() -> Option { + frame_support::storage::unhashed::get(primitives::well_known_keys::CURRENT_BLOCK_RANDOMNESS) + } + + fn get_block_randomness_mixed(subject: &[u8]) -> Option { + Self::get_block_randomness() + .map(|random_hash| mix_randomness::(random_hash, subject)) + } +} + +/// Combines the vrf output of the previous relay block with the provided subject. +/// This ensures that the randomness will be different on different pallets, as long as the subject is different. +fn mix_randomness(vrf_output: Hash, subject: &[u8]) -> T::Hash { + let mut digest = Vec::new(); + digest.extend_from_slice(vrf_output.as_ref()); + digest.extend_from_slice(subject); + + T::Hashing::hash(digest.as_slice()) +} + +/// Read full_rotation_period from pallet_configuration +pub struct ConfigurationCollatorRotationSessionPeriod; + +impl Get for ConfigurationCollatorRotationSessionPeriod { + fn get() -> u32 { + CollatorConfiguration::config().full_rotation_period + } +} + +pub struct BabeGetRandomnessForNextBlock; + +impl GetRandomnessForNextBlock for BabeGetRandomnessForNextBlock { + fn should_end_session(n: u32) -> bool { + ::ShouldEndSession::should_end_session(n) + } + + fn get_randomness() -> [u8; 32] { + let block_number = System::block_number(); + let random_seed = if block_number != 0 { + if let Some(random_hash) = { + BabeCurrentBlockRandomnessGetter::get_block_randomness_mixed(b"CollatorAssignment") + } { + // Return random_hash as a [u8; 32] instead of a Hash + let mut buf = [0u8; 32]; + let len = sp_std::cmp::min(32, random_hash.as_ref().len()); + buf[..len].copy_from_slice(&random_hash.as_ref()[..len]); + + buf + } else { + // If there is no randomness (e.g when running in dev mode), return [0; 32] + // TODO: smoke test to ensure this never happens in a live network + [0; 32] + } + } else { + // In block 0 (genesis) there is randomness + [0; 32] + }; + + random_seed + } +} + +// Randomness trait +impl frame_support::traits::Randomness for BabeCurrentBlockRandomnessGetter { + fn random(subject: &[u8]) -> (Hash, BlockNumber) { + let block_number = frame_system::Pallet::::block_number(); + let randomness = Self::get_block_randomness_mixed(subject).unwrap_or_default(); + + (randomness, block_number) + } +} + pub struct RemoveParaIdsWithNoCreditsImpl; impl RemoveParaIdsWithNoCredits for RemoveParaIdsWithNoCreditsImpl { @@ -2716,8 +2792,9 @@ impl pallet_collator_assignment::Config for Runtime { type ContainerChains = ContainerRegistrar; type SessionIndex = u32; type SelfParaId = MockParaId; - type ShouldRotateAllCollators = (); - type GetRandomnessForNextBlock = (); + type ShouldRotateAllCollators = + RotateCollatorsEveryNSessions; + type GetRandomnessForNextBlock = BabeGetRandomnessForNextBlock; type RemoveInvulnerables = (); type RemoveParaIdsWithNoCredits = RemoveParaIdsWithNoCreditsImpl; type CollatorAssignmentHook = ServicesPayment; diff --git a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs index dc0682a7d..aae556b03 100644 --- a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs +++ b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs @@ -31,6 +31,66 @@ use { test_relay_sproof_builder::{HeaderAs, ParaHeaderSproofBuilder, ParaHeaderSproofBuilderItem}, }; +#[test] +fn test_collator_assignment_rotation() { + ExtBuilder::default() + .with_balances(vec![ + // Alice gets 10k extra tokens for her mapping deposit + (AccountId::from(ALICE), 210_000 * UNIT), + (AccountId::from(BOB), 100_000 * UNIT), + (AccountId::from(CHARLIE), 100_000 * UNIT), + (AccountId::from(DAVE), 100_000 * UNIT), + ]) + .with_collators(vec![ + (AccountId::from(ALICE), 210 * UNIT), + (AccountId::from(BOB), 100 * UNIT), + (AccountId::from(CHARLIE), 100 * UNIT), + (AccountId::from(DAVE), 100 * UNIT), + ]) + .with_empty_parachains(vec![1001, 1002]) + .with_config(pallet_configuration::HostConfiguration { + max_collators: 100, + min_orchestrator_collators: 2, + max_orchestrator_collators: 5, + collators_per_container: 2, + full_rotation_period: 24, + ..Default::default() + }) + .build() + .execute_with(|| { + // Alice and Bob to 1001 + let assignment = TanssiCollatorAssignment::collator_container_chain(); + let initial_assignment = assignment.clone(); + assert_eq!( + assignment.container_chains[&1001u32.into()], + vec![ALICE.into(), BOB.into()] + ); + + let rotation_period = CollatorConfiguration::config().full_rotation_period; + run_to_session(rotation_period - 2); + set_new_randomness_data(Some([1; 32])); + // run_block(); + + assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_none()); + + run_to_session(rotation_period - 1); + run_block(); + assert_eq!( + TanssiCollatorAssignment::collator_container_chain(), + initial_assignment, + ); + assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_some()); + + run_to_session(rotation_period); + run_block(); + // Assignment changed + assert_ne!( + TanssiCollatorAssignment::collator_container_chain(), + initial_assignment, + ); + }); +} + #[test] fn test_author_collation_aura_change_of_authorities_on_session() { ExtBuilder::default() diff --git a/solo-chains/runtime/starlight/src/tests/common/mod.rs b/solo-chains/runtime/starlight/src/tests/common/mod.rs index ed9ffc3cf..302fc82bd 100644 --- a/solo-chains/runtime/starlight/src/tests/common/mod.rs +++ b/solo-chains/runtime/starlight/src/tests/common/mod.rs @@ -70,7 +70,7 @@ pub fn session_to_block(n: u32) -> u32 { // Add 1 because the block that emits the NewSession event cannot contain any extrinsics, // so this is the first block of the new session that can actually be used - block_number + 1 + block_number + 2 } pub fn babe_authorities() -> Vec { @@ -204,6 +204,7 @@ pub fn start_block() { // Initialize the new block Babe::on_initialize(System::block_number()); + TanssiCollatorAssignment::on_initialize(System::block_number()); Session::on_initialize(System::block_number()); Initializer::on_initialize(System::block_number()); let maybe_mock_inherent = take_new_inherent_data(); @@ -218,6 +219,7 @@ pub fn end_block() { // Finalize the block Babe::on_finalize(System::block_number()); Grandpa::on_finalize(System::block_number()); + TanssiCollatorAssignment::on_finalize(System::block_number()); Session::on_finalize(System::block_number()); Initializer::on_finalize(System::block_number()); TransactionPayment::on_finalize(System::block_number()); @@ -643,6 +645,13 @@ pub fn set_new_inherent_data(data: cumulus_primitives_core::relay_chain::Inheren frame_support::storage::unhashed::put(b"ParasInherent", &data); } +pub fn set_new_randomness_data(data: Option<[u8; 32]>) { + frame_support::storage::unhashed::put( + primitives::well_known_keys::CURRENT_BLOCK_RANDOMNESS, + &data, + ); +} + /// Mock the inherent that sets validation data in ParachainSystem, which /// contains the `relay_chain_block_number`, which is used in `collator-assignment` as a /// source of randomness. @@ -1059,3 +1068,9 @@ pub fn storage_map_final_key( final_key } + +// pub fn set_parachain_inherent_data_random_seed(random_seed: [u8; 32]) { +// set_new_inherent_data(MockInherentData { +// random_seed: Some(random_seed), +// }); +// } From bbb91370acf08687a925d954d874333babdeed0b Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Tue, 27 Aug 2024 16:36:35 +0200 Subject: [PATCH 02/10] Fixed should_end_session --- solo-chains/runtime/starlight/src/lib.rs | 7 ++++++- solo-chains/runtime/starlight/src/tests/common/mod.rs | 10 +++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/solo-chains/runtime/starlight/src/lib.rs b/solo-chains/runtime/starlight/src/lib.rs index 05c21de37..f3b3060b4 100644 --- a/solo-chains/runtime/starlight/src/lib.rs +++ b/solo-chains/runtime/starlight/src/lib.rs @@ -2670,7 +2670,12 @@ pub struct BabeGetRandomnessForNextBlock; impl GetRandomnessForNextBlock for BabeGetRandomnessForNextBlock { fn should_end_session(n: u32) -> bool { - ::ShouldEndSession::should_end_session(n) + n != 1 && { + let diff = Babe::current_slot() + .saturating_add(1u64) + .saturating_sub(Babe::current_epoch_start()); + *diff >= Babe::current_epoch().duration + } } fn get_randomness() -> [u8; 32] { diff --git a/solo-chains/runtime/starlight/src/tests/common/mod.rs b/solo-chains/runtime/starlight/src/tests/common/mod.rs index 302fc82bd..d66e15c4d 100644 --- a/solo-chains/runtime/starlight/src/tests/common/mod.rs +++ b/solo-chains/runtime/starlight/src/tests/common/mod.rs @@ -70,7 +70,7 @@ pub fn session_to_block(n: u32) -> u32 { // Add 1 because the block that emits the NewSession event cannot contain any extrinsics, // so this is the first block of the new session that can actually be used - block_number + 2 + block_number + 1 } pub fn babe_authorities() -> Vec { @@ -204,9 +204,9 @@ pub fn start_block() { // Initialize the new block Babe::on_initialize(System::block_number()); - TanssiCollatorAssignment::on_initialize(System::block_number()); Session::on_initialize(System::block_number()); Initializer::on_initialize(System::block_number()); + TanssiCollatorAssignment::on_initialize(System::block_number()); let maybe_mock_inherent = take_new_inherent_data(); if let Some(mock_inherent_data) = maybe_mock_inherent { set_paras_inherent(mock_inherent_data); @@ -218,11 +218,11 @@ pub fn end_block() { advance_block_state_machine(RunBlockState::End(block_number)); // Finalize the block Babe::on_finalize(System::block_number()); - Grandpa::on_finalize(System::block_number()); - TanssiCollatorAssignment::on_finalize(System::block_number()); Session::on_finalize(System::block_number()); - Initializer::on_finalize(System::block_number()); + Grandpa::on_finalize(System::block_number()); TransactionPayment::on_finalize(System::block_number()); + Initializer::on_finalize(System::block_number()); + TanssiCollatorAssignment::on_finalize(System::block_number()); } pub fn run_block() { From 90aae432e7f9949f6f8b4f95c57ce04bc09345ed Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Wed, 28 Aug 2024 10:30:11 +0200 Subject: [PATCH 03/10] Integration tests --- .../src/tests/collator_assignment_tests.rs | 108 +++++++++++++++++- 1 file changed, 102 insertions(+), 6 deletions(-) diff --git a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs index 8dc433387..006c8be2c 100644 --- a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs +++ b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs @@ -17,18 +17,14 @@ #![cfg(test)] use { - crate::tests::common::*, - crate::{ - Balances, CollatorConfiguration, ContainerRegistrar, ServicesPayment, - TanssiAuthorityMapping, TanssiInvulnerables, - }, + crate::{tests::common::*, Balances, CollatorConfiguration, ContainerRegistrar, ServicesPayment, TanssiAuthorityMapping, TanssiInvulnerables}, cumulus_primitives_core::ParaId, frame_support::assert_ok, parity_scale_codec::Encode, sp_consensus_aura::AURA_ENGINE_ID, sp_runtime::{traits::BlakeTwo256, DigestItem}, sp_std::vec, - test_relay_sproof_builder::{HeaderAs, ParaHeaderSproofBuilder, ParaHeaderSproofBuilderItem}, + test_relay_sproof_builder::{HeaderAs, ParaHeaderSproofBuilder, ParaHeaderSproofBuilderItem}, xcm_builder::AliasForeignAccountId32, }; #[test] @@ -1999,3 +1995,103 @@ fn test_collator_assignment_tip_withdraw_min_tip() { ); }); } + +#[test] +fn test_parachains_deregister_collators_re_assigned() { + ExtBuilder::default() + .with_balances(vec![ + // Alice gets 10k extra tokens for her mapping deposit + (AccountId::from(ALICE), 210_000 * UNIT), + (AccountId::from(BOB), 100_000 * UNIT), + (AccountId::from(CHARLIE), 100_000 * UNIT), + (AccountId::from(DAVE), 100_000 * UNIT), + ]) + .with_collators(vec![ + (AccountId::from(ALICE), 210 * UNIT), + (AccountId::from(BOB), 100 * UNIT), + // (AccountId::from(CHARLIE), 100 * UNIT), + // (AccountId::from(DAVE), 100 * UNIT), + ]) + .with_empty_parachains(vec![1001, 1002]) + .build() + .execute_with(|| { + // Alice and Bob to 1001 + let assignment = TanssiCollatorAssignment::collator_container_chain(); + assert_eq!( + assignment.container_chains[&1001u32.into()], + vec![ALICE.into(), BOB.into()] + ); + + assert_ok!(ContainerRegistrar::deregister(root_origin(), 1001.into()), ()); + + // Assignment should happen after 2 sessions + run_to_session(1u32); + + let assignment = TanssiCollatorAssignment::collator_container_chain(); + assert_eq!( + assignment.container_chains[&1001u32.into()], + vec![ALICE.into(), BOB.into()] + ); + + run_to_session(2u32); + + // Alice and Bob should be assigned to para 1002 this time + let assignment = TanssiCollatorAssignment::collator_container_chain(); + assert_eq!( + assignment.container_chains[&1002u32.into()], + vec![ALICE.into(), BOB.into()] + ); + }); +} + +#[test] +fn test_parachains_deregister_collators_config_change_reassigned() { + ExtBuilder::default() + .with_balances(vec![ + // Alice gets 10k extra tokens for her mapping deposit + (AccountId::from(ALICE), 210_000 * UNIT), + (AccountId::from(BOB), 100_000 * UNIT), + (AccountId::from(CHARLIE), 100_000 * UNIT), + (AccountId::from(DAVE), 100_000 * UNIT), + ]) + .with_collators(vec![ + (AccountId::from(ALICE), 210 * UNIT), + (AccountId::from(BOB), 100 * UNIT), + (AccountId::from(CHARLIE), 100 * UNIT), + (AccountId::from(DAVE), 100 * UNIT), + ]) + .with_empty_parachains(vec![1001, 1002]) + .build() + .execute_with(|| { + // Set container chain collators to 3 + assert_ok!( + CollatorConfiguration::set_collators_per_container(root_origin(), 3), + () + ); + + // Alice and Bob to 1001 + let assignment = TanssiCollatorAssignment::collator_container_chain(); + assert_eq!( + assignment.container_chains[&1001u32.into()], + vec![ALICE.into(), BOB.into()] + ); + + // Assignment should happen after 2 sessions + run_to_session(1u32); + + let assignment = TanssiCollatorAssignment::collator_container_chain(); + assert_eq!( + assignment.container_chains[&1001u32.into()], + vec![ALICE.into(), BOB.into()] + ); + + run_to_session(2u32); + + // Alice, Bob and Charlie should be assigned to para 1001 this time + let assignment = TanssiCollatorAssignment::collator_container_chain(); + assert_eq!( + assignment.container_chains[&1001u32.into()], + vec![ALICE.into(), BOB.into(), CHARLIE.into()] + ); + }); +} From b49a174ebcc91595629a269c35d174612fbc284b Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Wed, 28 Aug 2024 10:45:10 +0200 Subject: [PATCH 04/10] Switch to Babe::AuthorVrfRandomness --- solo-chains/runtime/starlight/src/lib.rs | 8 ++++---- .../starlight/src/tests/collator_assignment_tests.rs | 12 +++++++++--- .../runtime/starlight/src/tests/common/mod.rs | 5 +---- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/solo-chains/runtime/starlight/src/lib.rs b/solo-chains/runtime/starlight/src/lib.rs index a90c24b74..3cbc3a68b 100644 --- a/solo-chains/runtime/starlight/src/lib.rs +++ b/solo-chains/runtime/starlight/src/lib.rs @@ -2751,8 +2751,8 @@ impl tanssi_initializer::Config for Runtime { pub struct BabeCurrentBlockRandomnessGetter; impl BabeCurrentBlockRandomnessGetter { - fn get_block_randomness() -> Option { - frame_support::storage::unhashed::get(primitives::well_known_keys::CURRENT_BLOCK_RANDOMNESS) + fn get_block_randomness() -> Option<[u8; 32]> { + Babe::author_vrf_randomness() } fn get_block_randomness_mixed(subject: &[u8]) -> Option { @@ -2761,9 +2761,9 @@ impl BabeCurrentBlockRandomnessGetter { } } -/// Combines the vrf output of the previous relay block with the provided subject. +/// Combines the vrf output of the previous block with the provided subject. /// This ensures that the randomness will be different on different pallets, as long as the subject is different. -fn mix_randomness(vrf_output: Hash, subject: &[u8]) -> T::Hash { +fn mix_randomness(vrf_output: [u8; 32], subject: &[u8]) -> T::Hash { let mut digest = Vec::new(); digest.extend_from_slice(vrf_output.as_ref()); digest.extend_from_slice(subject); diff --git a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs index 006c8be2c..b252f2a08 100644 --- a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs +++ b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs @@ -17,14 +17,17 @@ #![cfg(test)] use { - crate::{tests::common::*, Balances, CollatorConfiguration, ContainerRegistrar, ServicesPayment, TanssiAuthorityMapping, TanssiInvulnerables}, + crate::{ + tests::common::*, Balances, CollatorConfiguration, ContainerRegistrar, ServicesPayment, + TanssiAuthorityMapping, TanssiInvulnerables, + }, cumulus_primitives_core::ParaId, frame_support::assert_ok, parity_scale_codec::Encode, sp_consensus_aura::AURA_ENGINE_ID, sp_runtime::{traits::BlakeTwo256, DigestItem}, sp_std::vec, - test_relay_sproof_builder::{HeaderAs, ParaHeaderSproofBuilder, ParaHeaderSproofBuilderItem}, xcm_builder::AliasForeignAccountId32, + test_relay_sproof_builder::{HeaderAs, ParaHeaderSproofBuilder, ParaHeaderSproofBuilderItem}, }; #[test] @@ -2022,7 +2025,10 @@ fn test_parachains_deregister_collators_re_assigned() { vec![ALICE.into(), BOB.into()] ); - assert_ok!(ContainerRegistrar::deregister(root_origin(), 1001.into()), ()); + assert_ok!( + ContainerRegistrar::deregister(root_origin(), 1001.into()), + () + ); // Assignment should happen after 2 sessions run_to_session(1u32); diff --git a/solo-chains/runtime/starlight/src/tests/common/mod.rs b/solo-chains/runtime/starlight/src/tests/common/mod.rs index 9760afd67..16739846d 100644 --- a/solo-chains/runtime/starlight/src/tests/common/mod.rs +++ b/solo-chains/runtime/starlight/src/tests/common/mod.rs @@ -646,10 +646,7 @@ pub fn set_new_inherent_data(data: cumulus_primitives_core::relay_chain::Inheren } pub fn set_new_randomness_data(data: Option<[u8; 32]>) { - frame_support::storage::unhashed::put( - primitives::well_known_keys::CURRENT_BLOCK_RANDOMNESS, - &data, - ); + pallet_babe::AuthorVrfRandomness::::set(data.into()); } /// Mock the inherent that sets validation data in ParachainSystem, which From edc6e2a7ad14d2dc75e8c1046826aaec212aae92 Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Wed, 28 Aug 2024 10:48:14 +0200 Subject: [PATCH 05/10] Cleanup --- solo-chains/runtime/starlight/src/tests/common/mod.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/solo-chains/runtime/starlight/src/tests/common/mod.rs b/solo-chains/runtime/starlight/src/tests/common/mod.rs index 16739846d..31a181bc4 100644 --- a/solo-chains/runtime/starlight/src/tests/common/mod.rs +++ b/solo-chains/runtime/starlight/src/tests/common/mod.rs @@ -1101,9 +1101,3 @@ pub fn set_dummy_boot_node(para_manager: RuntimeOrigin, para_id: ParaId) { "profile should be correctly assigned" ); } - -// pub fn set_parachain_inherent_data_random_seed(random_seed: [u8; 32]) { -// set_new_inherent_data(MockInherentData { -// random_seed: Some(random_seed), -// }); -// } From 076345abc2d29028aecbe85590c9497d0d434e43 Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Wed, 28 Aug 2024 10:55:18 +0200 Subject: [PATCH 06/10] clippy --- solo-chains/runtime/starlight/src/tests/common/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solo-chains/runtime/starlight/src/tests/common/mod.rs b/solo-chains/runtime/starlight/src/tests/common/mod.rs index 31a181bc4..e048f2b11 100644 --- a/solo-chains/runtime/starlight/src/tests/common/mod.rs +++ b/solo-chains/runtime/starlight/src/tests/common/mod.rs @@ -646,7 +646,7 @@ pub fn set_new_inherent_data(data: cumulus_primitives_core::relay_chain::Inheren } pub fn set_new_randomness_data(data: Option<[u8; 32]>) { - pallet_babe::AuthorVrfRandomness::::set(data.into()); + pallet_babe::AuthorVrfRandomness::::set(data); } /// Mock the inherent that sets validation data in ParachainSystem, which From a03eff758fc4f2b079889f4f90d3366457d16c26 Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Wed, 28 Aug 2024 11:02:11 +0200 Subject: [PATCH 07/10] cleanup --- .../runtime/starlight/src/tests/collator_assignment_tests.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs index b252f2a08..790536515 100644 --- a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs +++ b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs @@ -68,12 +68,10 @@ fn test_collator_assignment_rotation() { let rotation_period = CollatorConfiguration::config().full_rotation_period; run_to_session(rotation_period - 2); set_new_randomness_data(Some([1; 32])); - // run_block(); assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_none()); run_to_session(rotation_period - 1); - run_block(); assert_eq!( TanssiCollatorAssignment::collator_container_chain(), initial_assignment, @@ -81,7 +79,6 @@ fn test_collator_assignment_rotation() { assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_some()); run_to_session(rotation_period); - run_block(); // Assignment changed assert_ne!( TanssiCollatorAssignment::collator_container_chain(), From eea8cf4cfc49d5734590cdb8b135c0b95c5e0602 Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Wed, 28 Aug 2024 20:35:35 +0200 Subject: [PATCH 08/10] TS test for rotation --- solo-chains/client/cli/src/command.rs | 12 +++- .../test-collator-assignment.ts | 63 +++++++++++++++++++ .../test_pallet_data_preservers.ts | 2 +- 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts diff --git a/solo-chains/client/cli/src/command.rs b/solo-chains/client/cli/src/command.rs index 361b37e8a..cdde9654a 100644 --- a/solo-chains/client/cli/src/command.rs +++ b/solo-chains/client/cli/src/command.rs @@ -77,7 +77,17 @@ impl SubstrateCli for Cli { } fn load_spec(&self, id: &str) -> std::result::Result, String> { - load_spec(id, vec![], vec![2000, 2001], None) + load_spec( + id, + vec![], + vec![2000, 2001], + Some(vec![ + "Bob".to_string(), + "Charlie".to_string(), + "Dave".to_string(), + "Eve".to_string(), + ]), + ) } } diff --git a/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts b/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts new file mode 100644 index 000000000..ef8b23b98 --- /dev/null +++ b/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts @@ -0,0 +1,63 @@ +import "@tanssi/api-augment"; +import { describeSuite, expect, beforeAll } from "@moonwall/cli"; +import { ApiPromise } from "@polkadot/api"; +import { jumpSessions, jumpToSession } from "util/block"; + +describeSuite({ + id: "DTR0301", + title: "Collator assignment tests", + foundationMethods: "dev", + + testCases: ({ it, context }) => { + let polkadotJs: ApiPromise; + + beforeAll(async () => { + polkadotJs = context.polkadotJs(); + }); + + it({ + id: "E01", + title: "Collator should rotate", + test: async function () { + const fullRotationPeriod = (await context.polkadotJs().query.collatorConfiguration.activeConfig())[ + "fullRotationPeriod" + ].toString(); + const sessionIndex = (await polkadotJs.query.session.currentIndex()).toNumber(); + const remainingSessionsForRotation = + sessionIndex > fullRotationPeriod ? sessionIndex % fullRotationPeriod : fullRotationPeriod; + + await jumpToSession(context, remainingSessionsForRotation - 2); + + const initialAssignment = ( + await polkadotJs.query.tanssiCollatorAssignment.collatorContainerChain() + ).toJSON(); + + expect(initialAssignment.containerChains[2000].length).to.eq(2); + expect((await polkadotJs.query.tanssiCollatorAssignment.pendingCollatorContainerChain()).isNone); + + // remainingSessionsForRotation - 1 + await jumpSessions(context, 1); + const minusOneAssignment = ( + await polkadotJs.query.tanssiCollatorAssignment.collatorContainerChain() + ).toJSON(); + + expect((await polkadotJs.query.tanssiCollatorAssignment.pendingCollatorContainerChain()).isSome); + // Assignment shouldn't have changed yet + expect(initialAssignment.containerChains[2000].toSorted()).to.deep.eq( + minusOneAssignment.containerChains[2000].toSorted() + ); + + // remainingSessionsForRotation + await jumpSessions(context, 1); + + const finalAssignment = ( + await polkadotJs.query.tanssiCollatorAssignment.collatorContainerChain() + ).toJSON(); + // Assignment should have changed + expect(initialAssignment.containerChains[2000].toSorted()).to.not.deep.eq( + finalAssignment.containerChains[2000].toSorted() + ); + }, + }); + }, +}); diff --git a/test/suites/dev-tanssi-relay/pallet-data-preservers/test_pallet_data_preservers.ts b/test/suites/dev-tanssi-relay/pallet-data-preservers/test_pallet_data_preservers.ts index 68f9d633b..93e4c8762 100644 --- a/test/suites/dev-tanssi-relay/pallet-data-preservers/test_pallet_data_preservers.ts +++ b/test/suites/dev-tanssi-relay/pallet-data-preservers/test_pallet_data_preservers.ts @@ -4,7 +4,7 @@ import { ApiPromise } from "@polkadot/api"; import { KeyringPair } from "@moonwall/util"; describeSuite({ - id: "DTR0301", + id: "DTR0401", title: "Data preservers pallet relay test suite", foundationMethods: "dev", From 73d57ec49537de746c0f46056ae15fdec48964ac Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Thu, 29 Aug 2024 19:06:33 +0200 Subject: [PATCH 09/10] reviews --- runtime/dancebox/src/lib.rs | 3 +- solo-chains/runtime/starlight/src/lib.rs | 13 +++--- .../src/tests/collator_assignment_tests.rs | 24 +++++++---- .../test-collator-assignment.ts | 41 +++++++++++++------ 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/runtime/dancebox/src/lib.rs b/runtime/dancebox/src/lib.rs index c1ae94a88..93193e93d 100644 --- a/runtime/dancebox/src/lib.rs +++ b/runtime/dancebox/src/lib.rs @@ -768,11 +768,10 @@ impl GetRandomnessForNextBlock for BabeGetRandomnessForNextBlock { buf } else { // If there is no randomness (e.g when running in dev mode), return [0; 32] - // TODO: smoke test to ensure this never happens in a live network [0; 32] } } else { - // In block 0 (genesis) there is randomness + // In block 0 (genesis) there is no randomness [0; 32] }; diff --git a/solo-chains/runtime/starlight/src/lib.rs b/solo-chains/runtime/starlight/src/lib.rs index 3cbc3a68b..09a31ac7b 100644 --- a/solo-chains/runtime/starlight/src/lib.rs +++ b/solo-chains/runtime/starlight/src/lib.rs @@ -2752,6 +2752,7 @@ impl tanssi_initializer::Config for Runtime { pub struct BabeCurrentBlockRandomnessGetter; impl BabeCurrentBlockRandomnessGetter { fn get_block_randomness() -> Option<[u8; 32]> { + // In a relay context we get block randomness from Babe's AuthorVrfRandomness Babe::author_vrf_randomness() } @@ -2763,7 +2764,7 @@ impl BabeCurrentBlockRandomnessGetter { /// Combines the vrf output of the previous block with the provided subject. /// This ensures that the randomness will be different on different pallets, as long as the subject is different. -fn mix_randomness(vrf_output: [u8; 32], subject: &[u8]) -> T::Hash { +pub fn mix_randomness(vrf_output: [u8; 32], subject: &[u8]) -> T::Hash { let mut digest = Vec::new(); digest.extend_from_slice(vrf_output.as_ref()); digest.extend_from_slice(subject); @@ -2780,10 +2781,13 @@ impl Get for ConfigurationCollatorRotationSessionPeriod { } } +// CollatorAssignment expects to set up the rotation's randomness seed on the +// on_finalize hook of the block prior to the actual session change. +// So should_end_session should be true on the last block of the current session pub struct BabeGetRandomnessForNextBlock; - impl GetRandomnessForNextBlock for BabeGetRandomnessForNextBlock { fn should_end_session(n: u32) -> bool { + // Check if next slot there is a session change n != 1 && { let diff = Babe::current_slot() .saturating_add(1u64) @@ -2805,12 +2809,11 @@ impl GetRandomnessForNextBlock for BabeGetRandomnessForNextBlock { buf } else { - // If there is no randomness (e.g when running in dev mode), return [0; 32] - // TODO: smoke test to ensure this never happens in a live network + // If there is no randomness return [0; 32] [0; 32] } } else { - // In block 0 (genesis) there is randomness + // In block 0 (genesis) there is no randomness [0; 32] }; diff --git a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs index 790536515..f4f037c81 100644 --- a/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs +++ b/solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs @@ -18,8 +18,8 @@ use { crate::{ - tests::common::*, Balances, CollatorConfiguration, ContainerRegistrar, ServicesPayment, - TanssiAuthorityMapping, TanssiInvulnerables, + tests::common::*, BabeCurrentBlockRandomnessGetter, Balances, CollatorConfiguration, + ContainerRegistrar, ServicesPayment, TanssiAuthorityMapping, TanssiInvulnerables, }, cumulus_primitives_core::ParaId, frame_support::assert_ok, @@ -49,8 +49,8 @@ fn test_collator_assignment_rotation() { .with_empty_parachains(vec![1001, 1002]) .with_config(pallet_configuration::HostConfiguration { max_collators: 100, - min_orchestrator_collators: 2, - max_orchestrator_collators: 5, + min_orchestrator_collators: 0, + max_orchestrator_collators: 0, collators_per_container: 2, full_rotation_period: 24, ..Default::default() @@ -78,7 +78,17 @@ fn test_collator_assignment_rotation() { ); assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_some()); - run_to_session(rotation_period); + // Check that the randomness in CollatorAssignment is set + // in the block before the session change + run_to_block(session_to_block(rotation_period) - 1); + end_block(); + let expected_randomness: [u8; 32] = + BabeCurrentBlockRandomnessGetter::get_block_randomness_mixed(b"CollatorAssignment") + .unwrap() + .into(); + assert_eq!(TanssiCollatorAssignment::randomness(), expected_randomness); + start_block(); + // Assignment changed assert_ne!( TanssiCollatorAssignment::collator_container_chain(), @@ -2009,8 +2019,6 @@ fn test_parachains_deregister_collators_re_assigned() { .with_collators(vec![ (AccountId::from(ALICE), 210 * UNIT), (AccountId::from(BOB), 100 * UNIT), - // (AccountId::from(CHARLIE), 100 * UNIT), - // (AccountId::from(DAVE), 100 * UNIT), ]) .with_empty_parachains(vec![1001, 1002]) .build() @@ -2048,7 +2056,7 @@ fn test_parachains_deregister_collators_re_assigned() { } #[test] -fn test_parachains_deregister_collators_config_change_reassigned() { +fn test_parachains_collators_config_change_reassigned() { ExtBuilder::default() .with_balances(vec![ // Alice gets 10k extra tokens for her mapping deposit diff --git a/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts b/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts index ef8b23b98..3a78b8439 100644 --- a/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts +++ b/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts @@ -1,7 +1,10 @@ import "@tanssi/api-augment"; import { describeSuite, expect, beforeAll } from "@moonwall/cli"; import { ApiPromise } from "@polkadot/api"; -import { jumpSessions, jumpToSession } from "util/block"; +import { jumpBlocks, jumpSessions, jumpToSession } from "util/block"; +import { filterAndApply } from "@moonwall/util"; +import { EventRecord } from "@polkadot/types/interfaces"; +import { bool, u32, u8, Vec } from "@polkadot/types-codec"; describeSuite({ id: "DTR0301", @@ -23,6 +26,9 @@ describeSuite({ "fullRotationPeriod" ].toString(); const sessionIndex = (await polkadotJs.query.session.currentIndex()).toNumber(); + // Calculate the remaining sessions for next full rotation + // This is a workaround for running moonwall in run mode + // as it runs all tests in the same chain instance const remainingSessionsForRotation = sessionIndex > fullRotationPeriod ? sessionIndex % fullRotationPeriod : fullRotationPeriod; @@ -34,29 +40,40 @@ describeSuite({ expect(initialAssignment.containerChains[2000].length).to.eq(2); expect((await polkadotJs.query.tanssiCollatorAssignment.pendingCollatorContainerChain()).isNone); + const blockNumber = (await polkadotJs.rpc.chain.getHeader()).number.toNumber(); // remainingSessionsForRotation - 1 await jumpSessions(context, 1); - const minusOneAssignment = ( + const rotationEndAssignment = ( await polkadotJs.query.tanssiCollatorAssignment.collatorContainerChain() ).toJSON(); expect((await polkadotJs.query.tanssiCollatorAssignment.pendingCollatorContainerChain()).isSome); // Assignment shouldn't have changed yet expect(initialAssignment.containerChains[2000].toSorted()).to.deep.eq( - minusOneAssignment.containerChains[2000].toSorted() + rotationEndAssignment.containerChains[2000].toSorted() ); - // remainingSessionsForRotation - await jumpSessions(context, 1); - - const finalAssignment = ( - await polkadotJs.query.tanssiCollatorAssignment.collatorContainerChain() - ).toJSON(); - // Assignment should have changed - expect(initialAssignment.containerChains[2000].toSorted()).to.not.deep.eq( - finalAssignment.containerChains[2000].toSorted() + // As randomness isn't deterministic in starlight we can't be + // 100% certain that the assignation will indeed change. So the + // best we can do is verify that the pending rotation event for + // next session is emitted and is a full rotation as expected + const events = await polkadotJs.query.system.events(); + const filteredEvents = filterAndApply( + events, + "tanssiCollatorAssignment", + ["NewPendingAssignment"], + ({ event }: EventRecord) => + event.data as unknown as { randomSeed: Vec; fullRotation: bool; targetSession: u32 } ); + expect(filteredEvents[0].fullRotation.toJSON()).toBe(true); + + // Check that the randomness is set in CollatorAssignment the + // block previous to the full rotation + const sessionDuration = await polkadotJs.consts.babe.epochDuration.toNumber(); + await jumpBlocks(context, sessionDuration - 1); + const assignmentRandomness = await polkadotJs.query.tanssiCollatorAssignment.randomness(); + expect(assignmentRandomness.isEmpty).toBe(false); }, }); }, From a803b9f4db654a03fc8de8f6092b3b38e52a5efa Mon Sep 17 00:00:00 2001 From: Francisco Gamundi Date: Thu, 29 Aug 2024 19:13:51 +0200 Subject: [PATCH 10/10] lint --- .../collator-assignment/test-collator-assignment.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts b/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts index 3a78b8439..9796f813c 100644 --- a/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts +++ b/test/suites/dev-tanssi-relay/collator-assignment/test-collator-assignment.ts @@ -40,7 +40,6 @@ describeSuite({ expect(initialAssignment.containerChains[2000].length).to.eq(2); expect((await polkadotJs.query.tanssiCollatorAssignment.pendingCollatorContainerChain()).isNone); - const blockNumber = (await polkadotJs.rpc.chain.getHeader()).number.toNumber(); // remainingSessionsForRotation - 1 await jumpSessions(context, 1);