Merge pull request #1716 from subspace/domain-instance-fix

Make the domain client instantiation-aware

crates/pallet-domains/src/lib.rs

@@ -665,11 +665,15 @@ mod pallet {
                         )
                         .map_err(Error::<T>::from)?;
 
-                        do_finalize_domain_current_epoch::<T>(
-                            domain_id,
-                            pruned_block_info.domain_block_number,
-                        )
-                        .map_err(Error::<T>::from)?;
+                        if pruned_block_info.domain_block_number % T::StakeEpochDuration::get()
+                            == Zero::zero()
+                        {
+                            do_finalize_domain_current_epoch::<T>(
+                                domain_id,
+                                pruned_block_info.domain_block_number,
+                            )
+                            .map_err(Error::<T>::from)?;
+                        }
 
                         do_unlock_pending_withdrawals::<T>(
                             domain_id,
@@ -936,19 +940,16 @@ mod pallet {
                         genesis_domain.runtime_type.clone(),
                         genesis_domain.runtime_version.clone(),
                         genesis_domain.code.clone(),
-                        Zero::zero(),
+                        One::one(),
                     )
                     .expect("Genesis runtime registration must always succeed");
 
                     // Instantiate the genesis domain
                     let domain_config = DomainConfig::from_genesis::<T>(genesis_domain, runtime_id);
                     let domain_owner = genesis_domain.owner_account_id.clone();
-                    let domain_id = do_instantiate_domain::<T>(
-                        domain_config,
-                        domain_owner.clone(),
-                        Zero::zero(),
-                    )
-                    .expect("Genesis domain instantiation must always succeed");
+                    let domain_id =
+                        do_instantiate_domain::<T>(domain_config, domain_owner.clone(), One::one())
+                            .expect("Genesis domain instantiation must always succeed");
 
                     // Register domain_owner as the genesis operator.
                     let operator_config = OperatorConfig {
@@ -967,7 +968,7 @@ mod pallet {
                     )
                     .expect("Genesis operator registration must succeed");
 
-                    do_finalize_domain_current_epoch::<T>(domain_id, Zero::zero())
+                    do_finalize_domain_current_epoch::<T>(domain_id, One::one())
                         .expect("Genesis epoch must succeed");
                 }
             }
@@ -1068,16 +1069,20 @@ impl<T: Config> Pallet<T> {
             .map(|domain_object| domain_object.domain_config.runtime_id)
     }
 
-    pub fn domain_instance_data(domain_id: DomainId) -> Option<DomainInstanceData> {
-        let runtime_id = DomainRegistry::<T>::get(domain_id)?
-            .domain_config
-            .runtime_id;
-        let (runtime_type, runtime_code) = RuntimeRegistry::<T>::get(runtime_id)
-            .map(|runtime_object| (runtime_object.runtime_type, runtime_object.code))?;
-        Some(DomainInstanceData {
-            runtime_type,
-            runtime_code,
-        })
+    pub fn domain_instance_data(
+        domain_id: DomainId,
+    ) -> Option<(DomainInstanceData, T::BlockNumber)> {
+        let domain_obj = DomainRegistry::<T>::get(domain_id)?;
+        let (runtime_type, runtime_code) =
+            RuntimeRegistry::<T>::get(domain_obj.domain_config.runtime_id)
+                .map(|runtime_object| (runtime_object.runtime_type, runtime_object.code))?;
+        Some((
+            DomainInstanceData {
+                runtime_type,
+                runtime_code,
+            },
+            domain_obj.created_at,
+        ))
     }
 
     pub fn genesis_state_root(domain_id: DomainId) -> Option<H256> {

crates/pallet-domains/src/staking.rs

@@ -914,7 +914,7 @@ pub(crate) mod tests {
                 nominators,
             );
 
-            assert!(do_finalize_domain_current_epoch::<Test>(domain_id, Zero::zero()).unwrap());
+            do_finalize_domain_current_epoch::<Test>(domain_id, Zero::zero()).unwrap();
 
             if !operator_reward.is_zero() {
                 do_reward_operators::<Test>(
@@ -1222,12 +1222,12 @@ pub(crate) mod tests {
                 BTreeMap::from_iter(nominators),
             );
 
-            assert!(do_finalize_domain_current_epoch::<Test>(domain_id, Zero::zero()).unwrap());
+            do_finalize_domain_current_epoch::<Test>(domain_id, Zero::zero()).unwrap();
 
             for unlock in &unlocking {
                 do_withdraw_stake::<Test>(operator_id, unlock.0, Withdraw::Some(unlock.1)).unwrap();
             }
-            assert!(do_finalize_domain_current_epoch::<Test>(domain_id, Zero::zero()).unwrap());
+            do_finalize_domain_current_epoch::<Test>(domain_id, Zero::zero()).unwrap();
 
             for deposit in deposits {
                 do_nominate_operator::<Test>(operator_id, deposit.0, deposit.1).unwrap();

crates/pallet-domains/src/staking_epoch.rs

@@ -38,11 +38,7 @@ pub enum Error {
 pub(crate) fn do_finalize_domain_current_epoch<T: Config>(
     domain_id: DomainId,
     domain_block_number: T::DomainNumber,
-) -> Result<bool, Error> {
-    if domain_block_number % T::StakeEpochDuration::get() != Zero::zero() {
-        return Ok(false);
-    }
-
+) -> Result<(), Error> {
     // slash the operators
     do_finalize_slashed_operators::<T>(domain_id).map_err(Error::SlashOperator)?;
 
@@ -57,7 +53,7 @@ pub(crate) fn do_finalize_domain_current_epoch<T: Config>(
 
     // finalize any withdrawals and then deposits
     do_finalize_domain_pending_transfers::<T>(domain_id, domain_block_number)?;
-    Ok(true)
+    Ok(())
 }
 
 /// Unlocks any operators who are de-registering or nominators who are withdrawing staked funds.

crates/sp-domains/src/lib.rs

@@ -615,7 +615,7 @@ sp_api::decl_runtime_apis! {
         fn runtime_id(domain_id: DomainId) -> Option<RuntimeId>;
 
         /// Returns the domain instance data for given `domain_id`.
-        fn domain_instance_data(domain_id: DomainId) -> Option<DomainInstanceData>;
+        fn domain_instance_data(domain_id: DomainId) -> Option<(DomainInstanceData, NumberFor<Block>)>;
 
         /// Returns the current timestamp at given height.
         fn timestamp() -> Moment;

crates/subspace-node/src/domain/domain_instance_starter.rs

@@ -43,6 +43,7 @@ impl DomainInstanceStarter {
     ) -> std::result::Result<(), Box<dyn std::error::Error>> {
         let BootstrapResult {
             domain_instance_data,
+            domain_created_at,
             imported_block_notification_stream,
         } = bootstrap_result;
 
@@ -142,6 +143,7 @@ impl DomainInstanceStarter {
                 let domain_params = domain_service::DomainParams {
                     domain_id,
                     domain_config,
+                    domain_created_at,
                     consensus_client,
                     consensus_network_sync_oracle: consensus_sync_service.clone(),
                     select_chain,

crates/subspace-runtime/src/lib.rs

@@ -871,7 +871,7 @@ impl_runtime_apis! {
             Domains::runtime_id(domain_id)
         }
 
-        fn domain_instance_data(domain_id: DomainId) -> Option<DomainInstanceData> {
+        fn domain_instance_data(domain_id: DomainId) -> Option<(DomainInstanceData, NumberFor<Block>)> {
             Domains::domain_instance_data(domain_id)
         }
 

domains/client/domain-operator/src/bootstrapper.rs

@@ -11,6 +11,8 @@ pub struct BootstrapResult<CBlock: BlockT> {
     // The `DomainInstanceData` used by the domain instance starter to
     // construct `RuntimeGenesisConfig` of the domain instance
     pub domain_instance_data: DomainInstanceData,
+    /// The consensus chain block number when the domain first instantiated.
+    pub domain_created_at: NumberFor<CBlock>,
     // The `imported_block_notification_stream` used by the bootstrapper
     //
     // NOTE: the domain instance starter must reuse this stream instead of
@@ -53,19 +55,20 @@ where
 
         // Check if the domain instance data already exist in the consensus chain's genesis state
         let best_hash = self.consensus_client.info().best_hash;
-        if let Some(domain_instance_data) = self
+        if let Some((domain_instance_data, domain_created_at)) = self
             .consensus_client
             .runtime_api()
             .domain_instance_data(best_hash, self_domain_id)?
         {
             return Ok(BootstrapResult {
                 domain_instance_data,
+                domain_created_at,
                 imported_block_notification_stream,
             });
         }
 
         // Check each imported consensus block to get the domain instance data
-        let domain_instance_data = 'outer: loop {
+        let (domain_instance_data, domain_created_at) = 'outer: loop {
             if let Some(block_imported) = imported_block_notification_stream.next().await {
                 let header = block_imported.header;
                 for item in header.digest().logs.iter() {
@@ -97,6 +100,7 @@ where
 
         Ok(BootstrapResult {
             domain_instance_data,
+            domain_created_at,
             imported_block_notification_stream,
         })
     }

domains/client/domain-operator/src/domain_block_processor.rs

@@ -18,6 +18,7 @@ use sp_domains::merkle_tree::MerkleTree;
 use sp_domains::{DomainId, DomainsApi, ExecutionReceipt, ExtrinsicsRoot};
 use sp_runtime::traits::{Block as BlockT, CheckedSub, HashFor, Header as HeaderT, One, Zero};
 use sp_runtime::Digest;
+use std::cmp::Ordering;
 use std::sync::Arc;
 
 pub(crate) struct DomainBlockResult<Block, CBlock>
@@ -37,6 +38,7 @@ where
     CBlock: BlockT,
 {
     pub(crate) domain_id: DomainId,
+    pub(crate) domain_created_at: NumberFor<CBlock>,
     pub(crate) client: Arc<Client>,
     pub(crate) consensus_client: Arc<CClient>,
     pub(crate) backend: Arc<Backend>,
@@ -54,6 +56,7 @@ where
     fn clone(&self) -> Self {
         Self {
             domain_id: self.domain_id,
+            domain_created_at: self.domain_created_at,
             client: self.client.clone(),
             consensus_client: self.consensus_client.clone(),
             backend: self.backend.clone(),
@@ -116,26 +119,46 @@ where
         consensus_block_hash: CBlock::Hash,
         consensus_block_number: NumberFor<CBlock>,
     ) -> sp_blockchain::Result<Option<PendingConsensusBlocks<Block, CBlock>>> {
-        if consensus_block_number == One::one() {
-            return Ok(Some(PendingConsensusBlocks {
-                initial_parent: (self.client.info().genesis_hash, Zero::zero()),
-                consensus_imports: vec![HashAndNumber {
-                    hash: consensus_block_hash,
-                    number: consensus_block_number,
-                }],
-            }));
+        match consensus_block_number.cmp(&(self.domain_created_at + One::one())) {
+            // Consensus block at `domain_created_at + 1` is the first block that possibly contains
+            // bundle, thus we can safely skip any consensus block that is smaller than this block.
+            Ordering::Less => return Ok(None),
+            // For consensus block at `domain_created_at + 1` use the genesis domain block as the
+            // initial parent block directly to avoid further processing.
+            Ordering::Equal => {
+                return Ok(Some(PendingConsensusBlocks {
+                    initial_parent: (self.client.info().genesis_hash, Zero::zero()),
+                    consensus_imports: vec![HashAndNumber {
+                        hash: consensus_block_hash,
+                        number: consensus_block_number,
+                    }],
+                }));
+            }
+            // For consensus block > `domain_created_at + 1`, there is potential existing fork
+            // thus we need to handle it carefully as following.
+            Ordering::Greater => {}
         }
 
         let best_hash = self.client.info().best_hash;
         let best_number = self.client.info().best_number;
 
-        let consensus_block_hash_for_best_domain_hash =
+        let consensus_block_hash_for_best_domain_hash = if best_number.is_zero() {
+            self.consensus_client
+                .hash(self.domain_created_at)?
+                .ok_or_else(|| {
+                    sp_blockchain::Error::Backend(format!(
+                        "Consensus hash for block #{} not found",
+                        self.domain_created_at
+                    ))
+                })?
+        } else {
             crate::aux_schema::latest_consensus_block_hash_for(&*self.backend, &best_hash)?
                 .ok_or_else(|| {
                     sp_blockchain::Error::Backend(format!(
                         "Consensus hash for domain hash #{best_number},{best_hash} not found"
                     ))
-                })?;
+                })?
+        };
 
         let consensus_from = consensus_block_hash_for_best_domain_hash;
         let consensus_to = consensus_block_hash;
@@ -181,6 +204,22 @@ where
                 let (common_block_number, common_block_hash) =
                     (route.common_block().number, route.common_block().hash);
 
+                // The `common_block` is smaller than the consensus block that the domain was created at, thus
+                // we can safely skip any consensus block that is smaller than `domain_created_at` and start at
+                // the consensus block `domain_created_at + 1`, which the first block that possibly contains bundle,
+                // and use the genesis domain block as the initial parent block.
+                if common_block_number <= self.domain_created_at {
+                    let consensus_imports = enacted
+                        .iter()
+                        .skip_while(|block| block.number <= self.domain_created_at)
+                        .cloned()
+                        .collect();
+                    return Ok(Some(PendingConsensusBlocks {
+                        initial_parent: (self.client.info().genesis_hash, Zero::zero()),
+                        consensus_imports,
+                    }));
+                }
+
                 // Get the domain block that is derived from the common consensus block and use it as
                 // the initial domain parent block
                 let domain_block_hash: Block::Hash = crate::aux_schema::best_domain_hash_for(
@@ -449,7 +488,7 @@ where
                             "Header for consensus block {consensus_block_hash:?} not found"
                         ))
                     })?;
-                if !consensus_header.number().is_one() {
+                if *consensus_header.number() > self.domain_created_at + One::one() {
                     let consensus_parent_hash = consensus_header.parent_hash();
                     crate::aux_schema::best_domain_hash_for(&*self.client, consensus_parent_hash)?
                         .ok_or_else(|| {

domains/client/domain-operator/src/lib.rs

@@ -164,6 +164,7 @@ pub struct OperatorParams<
     NSNS: Stream<Item = NewSlotNotification> + Send + 'static,
 {
     pub domain_id: DomainId,
+    pub domain_created_at: NumberFor<CBlock>,
     pub consensus_client: Arc<CClient>,
     pub consensus_network_sync_oracle: Arc<dyn SyncOracle + Send + Sync>,
     pub client: Arc<Client>,

domains/client/domain-operator/src/operator.rs

@@ -153,6 +153,7 @@ where
 
         let domain_block_processor = DomainBlockProcessor {
             domain_id: params.domain_id,
+            domain_created_at: params.domain_created_at,
             client: params.client.clone(),
             consensus_client: params.consensus_client.clone(),
             backend: params.backend.clone(),

domains/client/domain-operator/src/tests.rs

@@ -528,8 +528,12 @@ async fn test_executor_full_node_catching_up() {
         Ferdie,
         BasePath::new(directory.path().join("ferdie")),
     );
-    // Produce 1 consensus block to initialize genesis domain
-    ferdie.produce_block_with_slot(1.into()).await.unwrap();
+    // Produce 5 consensus block to initialize genesis domain
+    //
+    // This also make the first consensus block being processed by the alice's domain
+    // block processor be block #5, to ensure it can correctly handle the consensus
+    // block even it is out of order.
+    ferdie.produce_blocks(5).await.unwrap();
 
     // Run Alice (a evm domain authority node)
     let alice = domain_test_service::DomainNodeBuilder::new(

domains/service/src/domain.rs

@@ -244,6 +244,7 @@ where
 {
     pub domain_id: DomainId,
     pub domain_config: DomainConfiguration<AccountId>,
+    pub domain_created_at: NumberFor<CBlock>,
     pub consensus_client: Arc<CClient>,
     pub consensus_network_sync_oracle: Arc<dyn SyncOracle + Send + Sync>,
     pub select_chain: SC,
@@ -341,6 +342,7 @@ where
     let DomainParams {
         domain_id,
         domain_config,
+        domain_created_at,
         consensus_client,
         consensus_network_sync_oracle,
         select_chain,
@@ -440,6 +442,7 @@ where
         &select_chain,
         OperatorParams {
             domain_id,
+            domain_created_at,
             consensus_client: consensus_client.clone(),
             consensus_network_sync_oracle,
             client: client.clone(),