Starlight: Wire ContainerRegistrar and Registrar pallets

pallets/registrar/src/lib.rs

@@ -232,6 +232,27 @@ pub mod pallet {
         ValueQuery,
     >;
 
+    /// This storage aims to act as a 'buffer' for paraIds that must be deregistered at the 
+    /// end of the block execution by calling 'T::InnerRegistrar::deregister()' implementation.
+    ///
+    /// We need this buffer because when we are using this pallet on a relay-chain environment
+    /// like Starlight (where 'T::InnerRegistrar' implementation is usually the
+    /// 'paras_registrar' pallet) we need to deregister (via 'paras_registrar::deregister')
+    /// the same paraIds we have in 'PendingToRemove<T>', and we need to do this deregistration
+    /// process inside 'on_finalize' hook.
+    ///
+    /// It can be the case that some paraIds need to be downgraded to a parathread before
+    /// deregistering on 'paras_registrar'. This process usually takes 2 sessions,
+    /// and the actual downgrade happens when the block finalizes.
+    ///
+    /// Therefore, if we tried to perform this relay deregistration process at the beginning
+    /// of the session/block inside ('on_initialize') initializer_on_new_session() as we do
+    /// for this pallet, it would fail due to the downgrade process could have not taken
+    /// place yet.  
+    #[pallet::storage]
+    pub type BufferedParasToDeregister<T: Config> =
+        StorageValue<_, BoundedVec<ParaId, T::MaxLengthParaIds>, ValueQuery>;
+
     pub type DepositBalanceOf<T> =
         <<T as Config>::Currency as Inspect<<T as frame_system::Config>::AccountId>>::Balance;
 
@@ -315,6 +336,11 @@ pub mod pallet {
 
     #[pallet::hooks]
     impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+        fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
+            // Account for on_finalize weight
+            Weight::zero().saturating_add(T::DbWeight::get().reads(1))
+        }
+
         #[cfg(feature = "try-runtime")]
         fn try_state(_n: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
             use {scale_info::prelude::format, sp_std::collections::btree_set::BTreeSet};
@@ -416,6 +442,12 @@ pub mod pallet {
 
             Ok(())
         }
+
+        fn on_finalize(_: BlockNumberFor<T>) {
+            if let Some(para_id) = BufferedParasToDeregister::<T>::take().pop() {
+                T::InnerRegistrar::deregister(para_id);
+            }
+        }
     }
 
     #[pallet::call]
@@ -825,6 +857,14 @@ pub mod pallet {
                 Self::deposit_event(Event::ParaIdDeregistered { para_id });
                 // Cleanup immediately
                 Self::cleanup_deregistered_para_id(para_id);
+                if let Err(id) = BufferedParasToDeregister::<T>::try_mutate(|v| v.try_push(para_id))
+                {
+                    log::error!(
+                        target: LOG_TARGET,
+                        "Failed to add paraId {:?} to deregistration list",
+                        id
+                    );
+                }
             } else {
                 Self::schedule_paused_parachain_change(|para_ids, paused| {
                     // We have to find out where, in the sorted vec the para id is, if anywhere.
@@ -1168,6 +1208,15 @@ pub mod pallet {
                         for para_id in new_paras {
                             Self::cleanup_deregistered_para_id(*para_id);
                             removed_para_ids.insert(*para_id);
+                            if let Err(id) =
+                                BufferedParasToDeregister::<T>::try_mutate(|v| v.try_push(*para_id))
+                            {
+                                log::error!(
+                                    target: LOG_TARGET,
+                                    "Failed to add paraId {:?} to deregistration list",
+                                    id
+                                );
+                            }
                         }
                     }
 
@@ -1210,7 +1259,6 @@ pub mod pallet {
             ParaManager::<T>::remove(para_id);
 
             T::RegistrarHooks::para_deregistered(para_id);
-            T::InnerRegistrar::deregister(para_id);
         }
 
         fn schedule_parachain_cleanup(para_id: ParaId) -> DispatchResult {

primitives/traits/src/lib.rs

@@ -376,7 +376,7 @@ pub trait RegistrarHandler<AccountId> {
         id: ParaId,
         genesis_storage: Vec<ContainerChainGenesisDataItem>,
     ) -> DispatchResult;
-    
+
     fn schedule_para_upgrade(id: ParaId) -> DispatchResult;
     fn schedule_para_downgrade(id: ParaId) -> DispatchResult;
     fn deregister(id: ParaId) -> Weight;

solo-chains/runtime/starlight/src/lib.rs

@@ -1490,7 +1490,7 @@ parameter_types! {
     pub const DepositAmount: Balance = 100 * UNITS;
     #[derive(Clone)]
     pub const MaxLengthParaIds: u32 = 100u32;
-    pub const MaxEncodedGenesisDataSize: u32 = 5_000_000u32; // 5MB
+    pub const MaxEncodedGenesisDataSize: u32 = 1_000_000u32; // 1MB
 }
 
 pub struct InnerStarlightRegistrar<AccountId, RegistrarManager>(
@@ -1526,11 +1526,17 @@ where
     }
 
     fn schedule_para_upgrade(id: ParaId) -> DispatchResult {
-        RegistrarManager::make_parachain(id)
+        if !RegistrarManager::is_parachain(id) {
+            return RegistrarManager::make_parachain(id);
+        }
+        Ok(())
     }
 
     fn schedule_para_downgrade(id: ParaId) -> DispatchResult {
-        RegistrarManager::make_parathread(id)
+        if !RegistrarManager::is_parathread(id) {
+            return RegistrarManager::make_parathread(id);
+        }
+        Ok(())
     }
 
     fn deregister(id: ParaId) -> Weight {

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

@@ -58,7 +58,7 @@ use {
 
 pub use crate::{
     genesis_config_presets::get_authority_keys_from_seed, AccountId, AuthorNoting, Babe, Balance,
-    Grandpa, Initializer, Runtime, Session, System, TanssiAuthorityAssignment,
+    ContainerRegistrar, Grandpa, Initializer, Runtime, Session, System, TanssiAuthorityAssignment,
     TanssiCollatorAssignment, TransactionPayment,
 };
 
@@ -221,6 +221,7 @@ pub fn end_block() {
     Session::on_finalize(System::block_number());
     Initializer::on_finalize(System::block_number());
     TransactionPayment::on_finalize(System::block_number());
+    ContainerRegistrar::on_finalize(System::block_number());
 }
 
 pub fn run_block() {

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

@@ -17,8 +17,7 @@
 #![cfg(test)]
 
 use {
-    crate::tests::common::*,
-    crate::{Configuration, ContainerRegistrar, Paras, Registrar, System},
+    crate::{tests::common::*, Configuration, ContainerRegistrar, Paras, Registrar, System},
     frame_support::{assert_noop, assert_ok},
     pallet_registrar::Event as ContainerRegistrarEvent,
     pallet_registrar_runtime_api::{
@@ -351,6 +350,13 @@ fn deregister_calls_schedule_para_cleanup() {
                 Configuration::set_max_head_data_size(root_origin(), 20500),
                 ()
             );
+
+            // Call end_block() to ensure that parachains_shared::CurrentSessionIndex
+            // storage gets updated properly inside on_finalize() and
+            // matches the one inside pallet_session::CurrentIndex.
+            end_block();
+            start_block();
+
             run_to_session(4u32);
             assert_eq!(
                 parachains_configuration::ActiveConfig::<Runtime>::get().max_head_data_size,
@@ -376,6 +382,8 @@ fn deregister_calls_schedule_para_cleanup() {
                 ),
                 ()
             );
+            end_block();
+            start_block();
 
             // Now let's check if the para was preoperly registered in the relay.
             // Run to next session.
@@ -389,6 +397,9 @@ fn deregister_calls_schedule_para_cleanup() {
                 root_origin(),
                 validation_code.into()
             ));
+            end_block();
+            start_block();
+
             run_to_session(7);
 
             // Now the para should be a parathread.
@@ -401,6 +412,8 @@ fn deregister_calls_schedule_para_cleanup() {
                 ContainerRegistrar::mark_valid_for_collating(root_origin(), 1003.into()),
                 ()
             );
+            end_block();
+            start_block();
 
             // The change should be applied after 2 sessions.
             run_to_session(9);
@@ -422,8 +435,12 @@ fn deregister_calls_schedule_para_cleanup() {
                 }
                 .into(),
             );
+            end_block();
+            start_block();
 
             run_to_session(11);
+            end_block();
+
             assert_eq!(Runtime::genesis_data(1003.into()).as_ref(), None);
 
             // Para should be offboarding after 2 sessions.

test/suites/dev-tanssi-relay/registrar/test_registrars.ts

@@ -4,7 +4,7 @@
 import { jumpSessions } from "../../../util/block";
 
 describeSuite({
-    id: "DT0107",
+    id: "DT0102",
     title: "ContainerRegistrar <> relay Registrar",
     foundationMethods: "dev",
     testCases: ({ it, context }) => {
@@ -114,7 +114,7 @@
 
                 // Check we can't register via relay Registrar
                 const tx2 = polkadotJs.tx.registrar.register(2002, "0x", "0x0102030405060708091011").signAsync(alice);
                 let { result: result2 } = await context.createBlock([tx2]);
                 expect(result2[0].successful).to.be.false;
                 expect(result2[0].error.section).to.eq("registrar");
                 expect(result2[0].error.name).to.eq("AlreadyRegistered");
@@ -129,23 +129,18 @@
                 const isParachain = await polkadotJs.query.paras.paraLifecycles(2002);
                 expect(isParachain.toString()).to.eq("Parachain");
 
-                const onChainGenesisDataBefore = await polkadotJs.query.containerRegistrar.paraGenesisData(2002);
-                console.log("ON CHAIN GEN: ", onChainGenesisDataBefore.toHuman());
-
                 const tx = polkadotJs.tx.containerRegistrar.deregister(2002);
                 await context.createBlock([await polkadotJs.tx.sudo.sudo(tx).signAsync(alice)], {
                     allowFailures: false,
                 });
 
                 await jumpSessions(context, 2);
-                await context.createBlock();
 
                 // Check that the on chain genesis data is now empty
-                const onChainGenesisData = await polkadotJs.query.containerRegistrar.paraGenesisData(2002);
-                expect(onChainGenesisData).to.be.null;
+                const onChainGenesisDataAfter = await polkadotJs.query.containerRegistrar.paraGenesisData(2002);
+                expect(onChainGenesisDataAfter.toHuman()).to.be.null;
 
                 // Para should be offboarding
-                // TODO: check why this is failing and not behaving the same as in rust tests
                 const isOffboarding = await polkadotJs.query.paras.paraLifecycles(2002);
                 expect(isOffboarding.toString()).to.eq("OffboardingParathread");
             },