Merge pull request #2959 from autonomys/fix-submit-receipt

Fix submit receipt issues

crates/pallet-domains/src/block_tree.rs

@@ -51,7 +51,6 @@ pub enum Error {
     RuntimeNotFound,
     LastBlockNotFound,
     UnmatchedNewHeadReceipt,
-    UnexpectedConfirmedDomainBlock,
 }
 
 #[derive(TypeInfo, Debug, Encode, Decode, Clone, PartialEq, Eq)]

crates/pallet-domains/src/lib.rs

@@ -654,6 +654,10 @@ mod pallet {
     /// successfully submitted to current consensus block, which mean a new domain block with this block
     /// number will be produce. Used as a pointer in `ExecutionInbox` to identify the current under building
     /// domain block, also used as a mapping of consensus block number to domain block number.
+    //
+    // NOTE: the `HeadDomainNumber` is lazily updated for the domain runtime upgrade block (which only include
+    // the runtime upgrade tx from the consensus chain and no any user submitted tx from the bundle), use
+    // `domain_best_number` for the actual best domain block
     #[pallet::storage]
     pub(super) type HeadDomainNumber<T: Config> =
         StorageMap<_, Identity, DomainId, DomainBlockNumberFor<T>, ValueQuery>;
@@ -763,6 +767,8 @@ mod pallet {
         UnexpectedReceiptGap,
         /// Expecting receipt gap when validating `submit_receipt`
         ExpectingReceiptGap,
+        /// Failed to get missed domain runtime upgrade count
+        FailedToGetMissedUpgradeCount,
     }
 
     #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)]
@@ -1720,12 +1726,35 @@ mod pallet {
                     )
                     .map_err(Error::<T>::from)?;
 
-                    // Singleton receipt are used to fill up the receipt gap and there should be no
-                    // new domain block being confirmed before the gap is fill up
-                    ensure!(
-                        maybe_confirmed_domain_block_info.is_none(),
-                        Error::<T>::BlockTree(BlockTreeError::UnexpectedConfirmedDomainBlock),
-                    );
+                    // NOTE: Skip the following staking related operations when benchmarking the
+                    // `submit_receipt` call, these operations will be benchmarked separately.
+                    #[cfg(not(feature = "runtime-benchmarks"))]
+                    if let Some(confirmed_block_info) = maybe_confirmed_domain_block_info {
+                        actual_weight =
+                            actual_weight.saturating_add(T::WeightInfo::confirm_domain_block(
+                                confirmed_block_info.operator_ids.len() as u32,
+                                confirmed_block_info.invalid_bundle_authors.len() as u32,
+                            ));
+
+                        refund_storage_fee::<T>(
+                            confirmed_block_info.total_storage_fee,
+                            confirmed_block_info.paid_bundle_storage_fees,
+                        )
+                        .map_err(Error::<T>::from)?;
+
+                        do_reward_operators::<T>(
+                            domain_id,
+                            confirmed_block_info.operator_ids.into_iter(),
+                            confirmed_block_info.rewards,
+                        )
+                        .map_err(Error::<T>::from)?;
+
+                        do_mark_operators_as_slashed::<T>(
+                            confirmed_block_info.invalid_bundle_authors.into_iter(),
+                            SlashedReason::InvalidBundle(confirmed_block_info.domain_block_number),
+                        )
+                        .map_err(Error::<T>::from)?;
+                    }
                 }
             }
 
@@ -2020,8 +2049,13 @@ impl<T: Config> Pallet<T> {
             .and_then(|mut runtime_object| runtime_object.raw_genesis.take_runtime_code())
     }
 
-    pub fn domain_best_number(domain_id: DomainId) -> Option<DomainBlockNumberFor<T>> {
-        Some(HeadDomainNumber::<T>::get(domain_id))
+    pub fn domain_best_number(domain_id: DomainId) -> Result<DomainBlockNumberFor<T>, BundleError> {
+        // The missed domain runtime upgrades will derive domain blocks thus should be accountted
+        // into the domain best number
+        let missed_upgrade = Self::missed_domain_runtime_upgrade(domain_id)
+            .map_err(|_| BundleError::FailedToGetMissedUpgradeCount)?;
+
+        Ok(HeadDomainNumber::<T>::get(domain_id) + missed_upgrade.into())
     }
 
     pub fn runtime_id(domain_id: DomainId) -> Option<RuntimeId> {
@@ -2265,7 +2299,7 @@ impl<T: Config> Pallet<T> {
         // derived from the latest domain block, and the stale bundle (that verified against an old
         // domain block) produced by a lagging honest operator will be rejected.
         ensure!(
-            Self::receipt_gap(domain_id) <= One::one(),
+            Self::receipt_gap(domain_id)? <= One::one(),
             BundleError::UnexpectedReceiptGap,
         );
 
@@ -2301,7 +2335,7 @@ impl<T: Config> Pallet<T> {
 
         // Singleton receipt is only allowed when there is a receipt gap
         ensure!(
-            Self::receipt_gap(domain_id) > One::one(),
+            Self::receipt_gap(domain_id)? > One::one(),
             BundleError::ExpectingReceiptGap,
         );
 
@@ -2675,6 +2709,11 @@ impl<T: Config> Pallet<T> {
         // Start from the oldest non-confirmed ER to the head domain number
         let mut to_check =
             Self::latest_confirmed_domain_block_number(domain_id).saturating_add(One::one());
+
+        // NOTE: we use the `HeadDomainNumber` here instead of the `domain_best_number`, which include the
+        // missed domain runtime upgrade block, because we don't want to trigger empty bundle production
+        // for confirming these blocks since they only include runtime upgrade extrinsic and no any user
+        // submitted extrinsic.
         let head_number = HeadDomainNumber::<T>::get(domain_id);
 
         while to_check <= head_number {
@@ -2750,7 +2789,7 @@ impl<T: Config> Pallet<T> {
             .saturating_add(
                 // NOTE: within `submit_bundle`, only one of (or none) `handle_bad_receipt` and
                 // `confirm_domain_block` can happen, thus we use the `max` of them
-
+                //
                 // We use `MAX_BUNDLE_PER_BLOCK` number to assume the number of slashed operators.
                 // We do not expect so many operators to be slashed but nontheless, if it did happen
                 // we will limit the weight to 100 operators.
@@ -2765,10 +2804,15 @@ impl<T: Config> Pallet<T> {
     pub fn max_submit_receipt_weight() -> Weight {
         T::WeightInfo::submit_bundle()
             .saturating_add(
+                // NOTE: within `submit_bundle`, only one of (or none) `handle_bad_receipt` and
+                // `confirm_domain_block` can happen, thus we use the `max` of them
+                //
                 // We use `MAX_BUNDLE_PER_BLOCK` number to assume the number of slashed operators.
                 // We do not expect so many operators to be slashed but nontheless, if it did happen
                 // we will limit the weight to 100 operators.
-                T::WeightInfo::handle_bad_receipt(MAX_BUNDLE_PER_BLOCK),
+                T::WeightInfo::handle_bad_receipt(MAX_BUNDLE_PER_BLOCK).max(
+                    T::WeightInfo::confirm_domain_block(MAX_BUNDLE_PER_BLOCK, MAX_BUNDLE_PER_BLOCK),
+                ),
             )
             .saturating_add(T::WeightInfo::slash_operator(MAX_NOMINATORS_TO_SLASH))
     }
@@ -2942,13 +2986,13 @@ impl<T: Config> Pallet<T> {
         DomainSudoCalls::<T>::get(domain_id).maybe_call
     }
 
-    // The gap between `HeadDomainNumber` and `HeadReceiptNumber` represent the number
+    // The gap between `domain_best_number` and `HeadReceiptNumber` represent the number
     // of receipt to be submitted
-    pub fn receipt_gap(domain_id: DomainId) -> DomainBlockNumberFor<T> {
-        let head_domain_number = HeadDomainNumber::<T>::get(domain_id);
+    pub fn receipt_gap(domain_id: DomainId) -> Result<DomainBlockNumberFor<T>, BundleError> {
+        let domain_best_number = Self::domain_best_number(domain_id)?;
         let head_receipt_number = HeadReceiptNumber::<T>::get(domain_id);
 
-        head_domain_number.saturating_sub(head_receipt_number)
+        Ok(domain_best_number.saturating_sub(head_receipt_number))
     }
 }
 

crates/sp-domains/src/lib.rs

@@ -780,7 +780,7 @@ impl<CHash: Default> ProofOfElection<CHash> {
     }
 }
 
-/// Singleton receipt submit along when there is a gap between `HeadDomainNumber`
+/// Singleton receipt submit along when there is a gap between `domain_best_number`
 /// and `HeadReceiptNumber`
 #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
 pub struct SingletonReceipt<Number, Hash, DomainHeader: HeaderT, Balance> {

crates/subspace-runtime/src/lib.rs

@@ -118,7 +118,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("subspace"),
     impl_name: create_runtime_str!("subspace"),
     authoring_version: 0,
-    spec_version: 6,
+    spec_version: 7,
     impl_version: 0,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 0,
@@ -1231,7 +1231,7 @@ impl_runtime_apis! {
         }
 
         fn domain_best_number(domain_id: DomainId) -> Option<DomainNumber> {
-            Domains::domain_best_number(domain_id)
+            Domains::domain_best_number(domain_id).ok()
         }
 
         fn execution_receipt(receipt_hash: DomainHash) -> Option<ExecutionReceiptFor<DomainHeader, Block, Balance>> {

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

@@ -183,7 +183,7 @@ where
 
         let domain_best_number = self.client.info().best_number;
         let consensus_chain_best_hash = self.consensus_client.info().best_hash;
-        let head_domain_number = self
+        let domain_best_number_onchain = self
             .consensus_client
             .runtime_api()
             .domain_best_number(consensus_chain_best_hash, self.domain_id)?
@@ -239,7 +239,7 @@ where
 
             let receipt = self
                 .domain_bundle_proposer
-                .load_next_receipt(head_domain_number, head_receipt_number)?;
+                .load_next_receipt(domain_best_number_onchain, head_receipt_number)?;
 
             let api_version = self
                 .consensus_client
@@ -256,10 +256,10 @@ where
             // instead of bundle
             // TODO: remove api runtime version check before next network
             if api_version >= 5
-                && head_domain_number.saturating_sub(head_receipt_number) > 1u32.into()
+                && domain_best_number_onchain.saturating_sub(head_receipt_number) > 1u32.into()
             {
                 info!(
-                    ?head_domain_number,
+                    ?domain_best_number_onchain,
                     ?head_receipt_number,
                     "🔖 Producing singleton receipt at slot {:?}",
                     slot_info.slot

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

@@ -397,18 +397,18 @@ where
     /// Returns the receipt in the next domain bundle.
     pub fn load_next_receipt(
         &self,
-        head_domain_number: NumberFor<Block>,
+        domain_best_number_onchain: NumberFor<Block>,
         head_receipt_number: NumberFor<Block>,
     ) -> sp_blockchain::Result<ExecutionReceiptFor<Block, CBlock>> {
         tracing::trace!(
-            ?head_domain_number,
+            ?domain_best_number_onchain,
             ?head_receipt_number,
             "Collecting receipt"
         );
 
-        // Both `head_domain_number` and `head_receipt_number` are zero means the domain just
+        // Both `domain_best_number_onchain` and `head_receipt_number` are zero means the domain just
         // instantiated and nothing have submitted yet so submit the genesis receipt
-        if head_domain_number.is_zero() && head_receipt_number.is_zero() {
+        if domain_best_number_onchain.is_zero() && head_receipt_number.is_zero() {
             let genesis_hash = self.client.info().genesis_hash;
             let genesis_header = self.client.header(genesis_hash)?.ok_or_else(|| {
                 sp_blockchain::Error::Backend(format!(

test/subspace-test-runtime/src/lib.rs

@@ -1428,7 +1428,7 @@ impl_runtime_apis! {
         }
 
         fn domain_best_number(domain_id: DomainId) -> Option<DomainNumber> {
-            Domains::domain_best_number(domain_id)
+            Domains::domain_best_number(domain_id).ok()
         }
 
         fn execution_receipt(receipt_hash: DomainHash) -> Option<ExecutionReceiptFor<DomainHeader, Block, Balance>> {