Merge pull request #2560 from subspace/lazy-handling-bad-er

Lazily process bad ER after fraud proof is submitted

crates/pallet-domains/src/benchmarking.rs

@@ -308,28 +308,30 @@ mod benchmarks {
         );
     }
 
-    #[benchmark]
-    fn switch_domain() {
-        let domain1_id = register_domain::<T>();
-        let domain2_id = register_domain::<T>();
-
-        let (operator_owner, operator_id) =
-            register_helper_operator::<T>(domain1_id, T::Currency::minimum_balance());
-
-        #[extrinsic_call]
-        _(
-            RawOrigin::Signed(operator_owner.clone()),
-            operator_id,
-            domain2_id,
-        );
-
-        let operator = Operators::<T>::get(operator_id).expect("operator must exist");
-        assert_eq!(operator.next_domain_id, domain2_id);
-
-        let pending_switch =
-            PendingOperatorSwitches::<T>::get(domain1_id).expect("pending switch must exist");
-        assert!(pending_switch.contains(&operator_id));
-    }
+    // TODO: `switch_domain` is not supported currently due to incompatible with lazily slashing
+    // enable this test when `switch_domain` is ready
+    // #[benchmark]
+    // fn switch_domain() {
+    //     let domain1_id = register_domain::<T>();
+    //     let domain2_id = register_domain::<T>();
+
+    //     let (operator_owner, operator_id) =
+    //         register_helper_operator::<T>(domain1_id, T::Currency::minimum_balance());
+
+    //     #[extrinsic_call]
+    //     _(
+    //         RawOrigin::Signed(operator_owner.clone()),
+    //         operator_id,
+    //         domain2_id,
+    //     );
+
+    //     let operator = Operators::<T>::get(operator_id).expect("operator must exist");
+    //     assert_eq!(operator.next_domain_id, domain2_id);
+
+    //     let pending_switch =
+    //         PendingOperatorSwitches::<T>::get(domain1_id).expect("pending switch must exist");
+    //     assert!(pending_switch.contains(&operator_id));
+    // }
 
     #[benchmark]
     fn deregister_operator() {
@@ -343,7 +345,7 @@ mod benchmarks {
 
         let operator = Operators::<T>::get(operator_id).expect("operator must exist");
         assert_eq!(
-            operator.status,
+            *operator.status::<T>(operator_id),
             OperatorStatus::Deregistered((domain_id, 0, DomainBlockNumberFor::<T>::zero()).into())
         );
     }

crates/pallet-domains/src/block_tree.rs

@@ -1,9 +1,10 @@
 //! Domain block tree
 
 use crate::{
-    BalanceOf, BlockTree, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockNumberFor,
-    DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptExtended, HeadReceiptNumber,
-    InboxedBundleAuthor, LatestConfirmedDomainBlock, Pallet, ReceiptHashFor,
+    BalanceOf, BlockTree, BlockTreeNodeFor, BlockTreeNodes, Config, ConsensusBlockHash,
+    DomainBlockNumberFor, DomainHashingFor, ExecutionInbox, ExecutionReceiptOf,
+    HeadReceiptExtended, HeadReceiptNumber, InboxedBundleAuthor, LatestConfirmedDomainBlock,
+    LatestSubmittedER, Pallet, ReceiptHashFor,
 };
 use codec::{Decode, Encode};
 use frame_support::{ensure, PalletError};
@@ -40,6 +41,7 @@ pub enum Error {
     BalanceOverflow,
     DomainTransfersTracking,
     InvalidDomainTransfers,
+    OverwritingER,
 }
 
 #[derive(TypeInfo, Debug, Encode, Decode, Clone, PartialEq, Eq)]
@@ -292,32 +294,29 @@ pub(crate) fn process_execution_receipt<T: Config>(
     execution_receipt: ExecutionReceiptOf<T>,
     receipt_type: AcceptedReceiptType,
 ) -> ProcessExecutionReceiptResult<T> {
+    let receipt_block_number = execution_receipt.domain_block_number;
     match receipt_type {
         AcceptedReceiptType::NewHead => {
-            let domain_block_number = execution_receipt.domain_block_number;
-
-            add_new_receipt_to_block_tree::<T>(domain_id, submitter, execution_receipt);
+            add_new_receipt_to_block_tree::<T>(domain_id, submitter, execution_receipt)?;
 
             // Update the head receipt number
-            HeadReceiptNumber::<T>::insert(domain_id, domain_block_number);
+            HeadReceiptNumber::<T>::insert(domain_id, receipt_block_number);
             HeadReceiptExtended::<T>::insert(domain_id, true);
 
             // Prune expired domain block
             if let Some(to_prune) =
-                domain_block_number.checked_sub(&T::BlockTreePruningDepth::get())
+                receipt_block_number.checked_sub(&T::BlockTreePruningDepth::get())
             {
-                let receipt_hash = match BlockTree::<T>::take(domain_id, to_prune) {
-                    Some(h) => h,
+                let BlockTreeNode {
+                    execution_receipt,
+                    operator_ids,
+                } = match prune_receipt::<T>(domain_id, to_prune)? {
+                    Some(n) => n,
                     // The receipt at `to_prune` may already been pruned if there is fraud proof being
                     // processed previously and the `HeadReceiptNumber` is reverted.
                     None => return Ok(None),
                 };
 
-                let BlockTreeNode {
-                    execution_receipt,
-                    operator_ids,
-                } = BlockTreeNodes::<T>::take(receipt_hash).ok_or(Error::MissingDomainBlock)?;
-
                 // Collect the paid bundle storage fees and the invalid bundle author
                 let mut paid_bundle_storage_fees = BTreeMap::new();
                 let mut invalid_bundle_authors = Vec::new();
@@ -398,6 +397,13 @@ pub(crate) fn process_execution_receipt<T: Config>(
             });
         }
     }
+
+    // Update the `LatestSubmittedER` for the operator
+    let key = (domain_id, submitter);
+    if receipt_block_number > Pallet::<T>::latest_submitted_er(key) {
+        LatestSubmittedER::<T>::insert(key, receipt_block_number)
+    }
+
     Ok(None)
 }
 
@@ -458,17 +464,24 @@ fn add_new_receipt_to_block_tree<T: Config>(
     domain_id: DomainId,
     submitter: OperatorId,
     execution_receipt: ExecutionReceiptOf<T>,
-) {
+) -> Result<(), Error> {
     // Construct and add a new domain block to the block tree
     let er_hash = execution_receipt.hash::<DomainHashingFor<T>>();
     let domain_block_number = execution_receipt.domain_block_number;
 
+    ensure!(
+        !BlockTree::<T>::contains_key(domain_id, domain_block_number),
+        Error::OverwritingER,
+    );
+
     BlockTree::<T>::insert(domain_id, domain_block_number, er_hash);
     let block_tree_node = BlockTreeNode {
         execution_receipt,
         operator_ids: sp_std::vec![submitter],
     };
     BlockTreeNodes::<T>::insert(er_hash, block_tree_node);
+
+    Ok(())
 }
 
 /// Import the genesis receipt to the block tree
@@ -499,6 +512,37 @@ pub(crate) fn import_genesis_receipt<T: Config>(
     BlockTreeNodes::<T>::insert(er_hash, block_tree_node);
 }
 
+pub(crate) fn prune_receipt<T: Config>(
+    domain_id: DomainId,
+    receipt_number: DomainBlockNumberFor<T>,
+) -> Result<Option<BlockTreeNodeFor<T>>, Error> {
+    let receipt_hash = match BlockTree::<T>::take(domain_id, receipt_number) {
+        Some(er_hash) => er_hash,
+        None => return Ok(None),
+    };
+    let block_tree_node =
+        BlockTreeNodes::<T>::take(receipt_hash).ok_or(Error::MissingDomainBlock)?;
+
+    // If the pruned ER is the operator's `latest_submitted_er` for this domain, it means either:
+    //
+    // - All the ER the operator submitted for this domain are confirmed and pruned, so the operator
+    //   can't be targetted by fraud proof later unless it submit other new ERs.
+    //
+    // - All the bad ER the operator submitted for this domain are pruned and the operator is already
+    //   slashed, so wwe don't need `LatestSubmittedER` to determine if the operator is pending slash.
+    //
+    // In both cases, it is safe to remove the `LatestSubmittedER` for the operator in this domain
+    for operator_id in block_tree_node.operator_ids.iter() {
+        let key = (domain_id, operator_id);
+        let latest_submitted_er = Pallet::<T>::latest_submitted_er(key);
+        if block_tree_node.execution_receipt.domain_block_number == latest_submitted_er {
+            LatestSubmittedER::<T>::remove(key);
+        }
+    }
+
+    Ok(Some(block_tree_node))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;