Slash invalid bundle authors when the corresponding ER is confirmed

crates/pallet-domains/src/block_tree.rs

@@ -217,15 +217,17 @@ pub(crate) fn verify_execution_receipt<T: Config>(
     }
 }
 
-/// Details of the pruned domain block such as operators, rewards they would receive.
-pub(crate) struct PrunedDomainBlockInfo<DomainNumber, Balance> {
+/// Details of the confirmed domain block such as operators, rewards they would receive.
+#[derive(Debug, PartialEq)]
+pub(crate) struct ConfirmedDomainBlockInfo<DomainNumber, Balance> {
     pub domain_block_number: DomainNumber,
     pub operator_ids: Vec<OperatorId>,
     pub rewards: Balance,
+    pub invalid_bundle_authors: Vec<OperatorId>,
 }
 
 pub(crate) type ProcessExecutionReceiptResult<T> =
-    Result<Option<PrunedDomainBlockInfo<<T as Config>::DomainNumber, BalanceOf<T>>>, Error>;
+    Result<Option<ConfirmedDomainBlockInfo<<T as Config>::DomainNumber, BalanceOf<T>>>, Error>;
 
 /// Process the execution receipt to add it to the block tree
 /// Returns the domain block number that was pruned, if any
@@ -235,7 +237,6 @@ pub(crate) fn process_execution_receipt<T: Config>(
     execution_receipt: ExecutionReceiptOf<T>,
     receipt_type: AcceptedReceiptType,
 ) -> ProcessExecutionReceiptResult<T> {
-    let mut pruned_domain_block_info = None;
     match receipt_type {
         AcceptedReceiptType::NewBranch => {
             add_new_receipt_to_block_tree::<T>(domain_id, submitter, execution_receipt);
@@ -257,39 +258,56 @@ pub(crate) fn process_execution_receipt<T: Config>(
                     return Err(Error::MultipleERsAfterChallengePeriod);
                 }
 
-                let receipt = receipts_at_number
+                let receipt_hash = receipts_at_number
                     .first()
                     .cloned()
                     .expect("should always have a value due to check above");
 
-                let domain_block =
-                    DomainBlocks::<T>::take(receipt).ok_or(Error::MissingDomainBlock)?;
+                let DomainBlock {
+                    execution_receipt,
+                    operator_ids,
+                } = DomainBlocks::<T>::take(receipt_hash).ok_or(Error::MissingDomainBlock)?;
+
                 _ = StateRoots::<T>::take((
                     domain_id,
-                    domain_block.execution_receipt.domain_block_number,
-                    domain_block.execution_receipt.domain_block_hash,
+                    to_prune,
+                    execution_receipt.domain_block_hash,
                 ));
 
-                // Remove the block's `ExecutionInbox` and `InboxedBundle` as the block is pruned and
-                // does not need to verify its receipt's `extrinsics_root` anymore.
-                for bundle_digests in ExecutionInbox::<T>::iter_prefix_values((domain_id, to_prune))
-                {
-                    for bd in bundle_digests {
-                        InboxedBundle::<T>::remove(bd.header_hash);
+                // Collect the invalid bundle author
+                let mut invalid_bundle_authors = Vec::new();
+                let bundle_digests = ExecutionInbox::<T>::get((
+                    domain_id,
+                    to_prune,
+                    execution_receipt.consensus_block_number,
+                ));
+                for (index, bd) in bundle_digests.into_iter().enumerate() {
+                    if let Some(bundle_author) = InboxedBundle::<T>::take(bd.header_hash) {
+                        if execution_receipt
+                            .invalid_bundles
+                            .iter()
+                            .any(|ib| ib.bundle_index == index as u32)
+                        {
+                            invalid_bundle_authors.push(bundle_author);
+                        }
                     }
                 }
+
+                // Remove the block's `ExecutionInbox` as the domain block is confirmed and no need to verify
+                // its receipt's `extrinsics_root` anymore.
                 let _ = ExecutionInbox::<T>::clear_prefix((domain_id, to_prune), u32::MAX, None);
 
                 ConsensusBlockHash::<T>::remove(
                     domain_id,
-                    domain_block.execution_receipt.consensus_block_number,
+                    execution_receipt.consensus_block_number,
                 );
 
-                pruned_domain_block_info = Some(PrunedDomainBlockInfo {
+                return Ok(Some(ConfirmedDomainBlockInfo {
                     domain_block_number: to_prune,
-                    operator_ids: domain_block.operator_ids,
-                    rewards: domain_block.execution_receipt.total_rewards,
-                })
+                    operator_ids,
+                    rewards: execution_receipt.total_rewards,
+                    invalid_bundle_authors,
+                }));
             }
         }
         AcceptedReceiptType::CurrentHead => {
@@ -303,7 +321,7 @@ pub(crate) fn process_execution_receipt<T: Config>(
             });
         }
     }
-    Ok(pruned_domain_block_info)
+    Ok(None)
 }
 
 fn add_new_receipt_to_block_tree<T: Config>(
@@ -366,7 +384,8 @@ mod tests {
     use crate::pallet::Operators;
     use crate::staking::{Operator, OperatorStatus};
     use crate::tests::{
-        create_dummy_bundle_with_receipts, create_dummy_receipt, new_test_ext_with_extensions, Test,
+        create_dummy_bundle_with_receipts, create_dummy_receipt, new_test_ext_with_extensions,
+        BlockTreePruningDepth, Test,
     };
     use crate::{BalanceOf, NextDomainId};
     use frame_support::dispatch::RawOrigin;
@@ -375,7 +394,7 @@ mod tests {
     use frame_support::{assert_err, assert_ok};
     use frame_system::Pallet as System;
     use sp_core::{Pair, H256, U256};
-    use sp_domains::{BundleDigest, OperatorPair, RuntimeType};
+    use sp_domains::{BundleDigest, InvalidBundle, InvalidBundleType, OperatorPair, RuntimeType};
     use sp_runtime::traits::BlockNumberProvider;
     use subspace_runtime_primitives::SSC;
 
@@ -436,16 +455,16 @@ mod tests {
     }
 
     // Submit new head receipt to extend the block tree
-    fn extend_block_tree(domain_id: DomainId, operator_id: u64, to: u64) {
+    fn extend_block_tree(
+        domain_id: DomainId,
+        operator_id: u64,
+        to: u64,
+    ) -> ExecutionReceiptOf<Test> {
         let head_receipt_number = HeadReceiptNumber::<Test>::get(domain_id);
         assert!(head_receipt_number < to);
 
         let head_node = get_block_tree_node_at::<Test>(domain_id, head_receipt_number).unwrap();
         let mut receipt = head_node.execution_receipt;
-        assert_eq!(
-            receipt.consensus_block_number,
-            frame_system::Pallet::<Test>::current_block_number()
-        );
         for block_number in (head_receipt_number + 1)..=to {
             // Finilize parent block and initialize block at `block_number`
             run_to_block::<Test>(block_number, receipt.consensus_block_hash);
@@ -474,6 +493,8 @@ mod tests {
                 vec![bundle_extrinsics_root],
             );
         }
+
+        receipt
     }
 
     #[allow(clippy::type_complexity)]
@@ -920,4 +941,108 @@ mod tests {
             );
         });
     }
+
+    #[test]
+    fn test_collect_invalid_bundle_author() {
+        let creator = 0u64;
+        let challenge_period = BlockTreePruningDepth::get() as u64;
+        let operator_set: Vec<_> = (1..15).collect();
+        let mut ext = new_test_ext_with_extensions();
+        ext.execute_with(|| {
+            let domain_id = register_genesis_domain(creator, operator_set.clone());
+            let mut target_receipt = extend_block_tree(domain_id, operator_set[0], 3);
+
+            // Submit bundle for every operator except operator 1 since it already submit one
+            let head_receipt_number = HeadReceiptNumber::<Test>::get(domain_id);
+            let current_head_receipt =
+                get_block_tree_node_at::<Test>(domain_id, head_receipt_number)
+                    .unwrap()
+                    .execution_receipt;
+            for operator_id in operator_set.iter().skip(1) {
+                let bundle = create_dummy_bundle_with_receipts(
+                    domain_id,
+                    *operator_id,
+                    H256::random(),
+                    current_head_receipt.clone(),
+                );
+                assert_ok!(crate::Pallet::<Test>::submit_bundle(
+                    RawOrigin::None.into(),
+                    bundle,
+                ));
+            }
+            let head_node = get_block_tree_node_at::<Test>(domain_id, head_receipt_number).unwrap();
+            assert_eq!(head_node.operator_ids, operator_set);
+
+            // Prepare the inbainvalid bundles and bundle authors
+            let invalid_bundle_authors: Vec<_> = operator_set
+                .clone()
+                .into_iter()
+                .filter(|i| i % 2 == 0)
+                .collect();
+            let invalid_bundles = invalid_bundle_authors
+                .iter()
+                .map(|i| InvalidBundle {
+                    // operator id start at 1 while bundle_index start at 0, thus minus 1 here
+                    bundle_index: *i as u32 - 1,
+                    invalid_bundle_type: InvalidBundleType::OutOfRangeTx,
+                })
+                .collect();
+
+            // Get the `bunde_extrinsics_roots` that contains all the submitted bundles
+            let current_block_number = frame_system::Pallet::<Test>::current_block_number();
+            let execution_inbox = ExecutionInbox::<Test>::get((
+                domain_id,
+                current_block_number,
+                current_block_number,
+            ));
+            let bunde_extrinsics_roots: Vec<_> = execution_inbox
+                .into_iter()
+                .map(|b| b.extrinsics_root)
+                .collect();
+            assert_eq!(bunde_extrinsics_roots.len(), operator_set.len());
+
+            target_receipt.invalid_bundles = invalid_bundles;
+            target_receipt.block_extrinsics_roots = bunde_extrinsics_roots;
+
+            // Run into next block
+            run_to_block::<Test>(
+                current_block_number + 1,
+                target_receipt.consensus_block_hash,
+            );
+            let current_block_number = current_block_number + 1;
+
+            // Submit `target_receipt`
+            assert_ok!(crate::Pallet::<Test>::submit_bundle(
+                RawOrigin::None.into(),
+                create_dummy_bundle_with_receipts(
+                    domain_id,
+                    operator_set[0],
+                    H256::random(),
+                    target_receipt,
+                ),
+            ));
+
+            // Extend the block tree by `challenge_period - 1` blocks
+            let next_receipt = extend_block_tree(
+                domain_id,
+                operator_set[0],
+                current_block_number + challenge_period - 1,
+            );
+            // Confirm `target_receipt`
+            let confirmed_domain_block = process_execution_receipt::<Test>(
+                domain_id,
+                operator_set[0],
+                next_receipt,
+                AcceptedReceiptType::NewHead,
+            )
+            .unwrap()
+            .unwrap();
+
+            // Invalid bundle authors should be collected correctly
+            assert_eq!(
+                confirmed_domain_block.invalid_bundle_authors,
+                invalid_bundle_authors
+            );
+        });
+    }
 }

crates/pallet-domains/src/lib.rs

@@ -110,8 +110,9 @@ mod pallet {
     };
     use crate::staking::{
         do_auto_stake_block_rewards, do_deregister_operator, do_nominate_operator,
-        do_register_operator, do_reward_operators, do_switch_operator_domain, do_withdraw_stake,
-        Error as StakingError, Nominator, Operator, OperatorConfig, StakingSummary, Withdraw,
+        do_register_operator, do_reward_operators, do_slash_operators, do_switch_operator_domain,
+        do_withdraw_stake, Error as StakingError, Nominator, Operator, OperatorConfig,
+        StakingSummary, Withdraw,
     };
     use crate::staking_epoch::{
         do_finalize_domain_current_epoch, do_unlock_pending_withdrawals,
@@ -708,34 +709,39 @@ mod pallet {
                 }
                 // Add the exeuctione receipt to the block tree
                 ReceiptType::Accepted(accepted_receipt_type) => {
-                    let maybe_pruned_domain_block_info = process_execution_receipt::<T>(
+                    let maybe_confirmed_domain_block_info = process_execution_receipt::<T>(
                         domain_id,
                         operator_id,
                         receipt,
                         accepted_receipt_type,
                     )
                     .map_err(Error::<T>::from)?;
 
-                    // If any domain block is pruned, then we have a new head added
+                    // If any domain block is confirmed, then we have a new head added
                     // so distribute the operator rewards and, if required, do epoch transition as well.
                     //
                     // NOTE: Skip the following staking related operations when benchmarking the
                     // `submit_bundle` call, these operations will be benchmarked separately.
                     #[cfg(not(feature = "runtime-benchmarks"))]
-                    if let Some(pruned_block_info) = maybe_pruned_domain_block_info {
+                    if let Some(confirmed_block_info) = maybe_confirmed_domain_block_info {
                         do_reward_operators::<T>(
                             domain_id,
-                            pruned_block_info.operator_ids.into_iter(),
-                            pruned_block_info.rewards,
+                            confirmed_block_info.operator_ids.into_iter(),
+                            confirmed_block_info.rewards,
                         )
                         .map_err(Error::<T>::from)?;
 
-                        if pruned_block_info.domain_block_number % T::StakeEpochDuration::get()
+                        do_slash_operators::<T>(
+                            confirmed_block_info.invalid_bundle_authors.into_iter(),
+                        )
+                        .map_err(Error::<T>::from)?;
+
+                        if confirmed_block_info.domain_block_number % T::StakeEpochDuration::get()
                             == Zero::zero()
                         {
                             let completed_epoch_index = do_finalize_domain_current_epoch::<T>(
                                 domain_id,
-                                pruned_block_info.domain_block_number,
+                                confirmed_block_info.domain_block_number,
                             )
                             .map_err(Error::<T>::from)?;
 
@@ -747,7 +753,7 @@ mod pallet {
 
                         do_unlock_pending_withdrawals::<T>(
                             domain_id,
-                            pruned_block_info.domain_block_number,
+                            confirmed_block_info.domain_block_number,
                         )
                         .map_err(Error::<T>::from)?;
                     }

crates/pallet-domains/src/staking.rs

@@ -501,8 +501,6 @@ pub(crate) fn do_auto_stake_block_rewards<T: Config>(
     Ok(())
 }
 
-#[allow(dead_code)]
-// TODO: remove once fraud proof is done
 /// Freezes the slashed operators and moves the operator to be removed once the domain they are
 /// operating finishes the epoch.
 pub(crate) fn do_slash_operators<T: Config>(