Starlight collator rotation (#668)

* Rotation working

* Fixed should_end_session

* Integration tests

* Switch to Babe::AuthorVrfRandomness

* Cleanup

* clippy

* cleanup

* TS test for rotation

* reviews

* lint

runtime/dancebox/src/lib.rs

@@ -768,11 +768,10 @@ impl GetRandomnessForNextBlock<u32> 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]
         };
 

solo-chains/client/cli/src/command.rs

@@ -77,7 +77,17 @@ impl SubstrateCli for Cli {
     }
 
     fn load_spec(&self, id: &str) -> std::result::Result<Box<dyn sc_service::ChainSpec>, 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(),
+            ]),
+        )
     }
 }
 

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},
@@ -74,6 +75,7 @@ use {
     },
     scale_info::TypeInfo,
     serde::{Deserialize, Serialize},
+    sp_core::Get,
     sp_genesis_builder::PresetId,
     sp_runtime::traits::BlockNumberProvider,
     sp_std::{
@@ -113,8 +115,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,
@@ -2747,6 +2749,88 @@ impl tanssi_initializer::Config for Runtime {
     type SessionHandler = OwnApplySession;
 }
 
+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()
+    }
+
+    fn get_block_randomness_mixed(subject: &[u8]) -> Option<Hash> {
+        Self::get_block_randomness()
+            .map(|random_hash| mix_randomness::<Runtime>(random_hash, 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.
+pub fn mix_randomness<T: frame_system::Config>(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);
+
+    T::Hashing::hash(digest.as_slice())
+}
+
+/// Read full_rotation_period from pallet_configuration
+pub struct ConfigurationCollatorRotationSessionPeriod;
+
+impl Get<u32> for ConfigurationCollatorRotationSessionPeriod {
+    fn get() -> u32 {
+        CollatorConfiguration::config().full_rotation_period
+    }
+}
+
+// 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<u32> 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)
+                .saturating_sub(Babe::current_epoch_start());
+            *diff >= Babe::current_epoch().duration
+        }
+    }
+
+    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 return [0; 32]
+                [0; 32]
+            }
+        } else {
+            // In block 0 (genesis) there is no randomness
+            [0; 32]
+        };
+
+        random_seed
+    }
+}
+
+// Randomness trait
+impl frame_support::traits::Randomness<Hash, BlockNumber> for BabeCurrentBlockRandomnessGetter {
+    fn random(subject: &[u8]) -> (Hash, BlockNumber) {
+        let block_number = frame_system::Pallet::<Runtime>::block_number();
+        let randomness = Self::get_block_randomness_mixed(subject).unwrap_or_default();
+
+        (randomness, block_number)
+    }
+}
+
 pub struct RemoveParaIdsWithNoCreditsImpl;
 
 impl RemoveParaIdsWithNoCredits for RemoveParaIdsWithNoCreditsImpl {
@@ -2830,8 +2914,9 @@ impl pallet_collator_assignment::Config for Runtime {
     type ContainerChains = ContainerRegistrar;
     type SessionIndex = u32;
     type SelfParaId = MockParaId;
-    type ShouldRotateAllCollators = ();
-    type GetRandomnessForNextBlock = ();
+    type ShouldRotateAllCollators =
+        RotateCollatorsEveryNSessions<ConfigurationCollatorRotationSessionPeriod>;
+    type GetRandomnessForNextBlock = BabeGetRandomnessForNextBlock;
     type RemoveInvulnerables = ();
     type RemoveParaIdsWithNoCredits = RemoveParaIdsWithNoCreditsImpl;
     type CollatorAssignmentHook = ServicesPayment;

solo-chains/runtime/starlight/src/tests/collator_assignment_tests.rs

@@ -17,10 +17,9 @@
 #![cfg(test)]
 
 use {
-    crate::tests::common::*,
     crate::{
-        Balances, CollatorConfiguration, ContainerRegistrar, ServicesPayment,
-        TanssiAuthorityMapping, TanssiInvulnerables,
+        tests::common::*, BabeCurrentBlockRandomnessGetter, Balances, CollatorConfiguration,
+        ContainerRegistrar, ServicesPayment, TanssiAuthorityMapping, TanssiInvulnerables,
     },
     cumulus_primitives_core::ParaId,
     frame_support::assert_ok,
@@ -31,6 +30,73 @@ 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: 0,
+            max_orchestrator_collators: 0,
+            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]));
+
+            assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_none());
+
+            run_to_session(rotation_period - 1);
+            assert_eq!(
+                TanssiCollatorAssignment::collator_container_chain(),
+                initial_assignment,
+            );
+            assert!(TanssiCollatorAssignment::pending_collator_container_chain().is_some());
+
+            // 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(),
+                initial_assignment,
+            );
+        });
+}
+
 #[test]
 fn test_author_collation_aura_change_of_authorities_on_session() {
     ExtBuilder::default()
@@ -1939,3 +2005,104 @@ 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),
+        ])
+        .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_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()]
+            );
+        });
+}

solo-chains/runtime/starlight/src/tests/common/mod.rs

@@ -206,6 +206,7 @@ pub fn start_block() {
     Babe::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);
@@ -217,10 +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());
     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() {
@@ -646,6 +648,10 @@ 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]>) {
+    pallet_babe::AuthorVrfRandomness::<Runtime>::set(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.