diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 4b7aa2b0f9..ab553feb7f 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -217,15 +217,17 @@ pub(crate) fn verify_execution_receipt( } } -/// Details of the pruned domain block such as operators, rewards they would receive. -pub(crate) struct PrunedDomainBlockInfo { +/// Details of the confirmed domain block such as operators, rewards they would receive. +#[derive(Debug, PartialEq)] +pub(crate) struct ConfirmedDomainBlockInfo { pub domain_block_number: DomainNumber, pub operator_ids: Vec, pub rewards: Balance, + pub invalid_bundle_authors: Vec, } pub(crate) type ProcessExecutionReceiptResult = - Result::DomainNumber, BalanceOf>>, Error>; + Result::DomainNumber, BalanceOf>>, 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( execution_receipt: ExecutionReceiptOf, receipt_type: AcceptedReceiptType, ) -> ProcessExecutionReceiptResult { - let mut pruned_domain_block_info = None; match receipt_type { AcceptedReceiptType::NewBranch => { add_new_receipt_to_block_tree::(domain_id, submitter, execution_receipt); @@ -257,39 +258,56 @@ pub(crate) fn process_execution_receipt( 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::::take(receipt).ok_or(Error::MissingDomainBlock)?; + let DomainBlock { + execution_receipt, + operator_ids, + } = DomainBlocks::::take(receipt_hash).ok_or(Error::MissingDomainBlock)?; + _ = StateRoots::::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::::iter_prefix_values((domain_id, to_prune)) - { - for bd in bundle_digests { - InboxedBundle::::remove(bd.header_hash); + // Collect the invalid bundle author + let mut invalid_bundle_authors = Vec::new(); + let bundle_digests = ExecutionInbox::::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::::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::::clear_prefix((domain_id, to_prune), u32::MAX, None); ConsensusBlockHash::::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( }); } } - Ok(pruned_domain_block_info) + Ok(None) } fn add_new_receipt_to_block_tree( @@ -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 { let head_receipt_number = HeadReceiptNumber::::get(domain_id); assert!(head_receipt_number < to); let head_node = get_block_tree_node_at::(domain_id, head_receipt_number).unwrap(); let mut receipt = head_node.execution_receipt; - assert_eq!( - receipt.consensus_block_number, - frame_system::Pallet::::current_block_number() - ); for block_number in (head_receipt_number + 1)..=to { // Finilize parent block and initialize block at `block_number` run_to_block::(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::::get(domain_id); + let current_head_receipt = + get_block_tree_node_at::(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::::submit_bundle( + RawOrigin::None.into(), + bundle, + )); + } + let head_node = get_block_tree_node_at::(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::::current_block_number(); + let execution_inbox = ExecutionInbox::::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::( + current_block_number + 1, + target_receipt.consensus_block_hash, + ); + let current_block_number = current_block_number + 1; + + // Submit `target_receipt` + assert_ok!(crate::Pallet::::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::( + 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 + ); + }); + } } diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index a974355322..e47168295a 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/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,7 +709,7 @@ mod pallet { } // Add the exeuctione receipt to the block tree ReceiptType::Accepted(accepted_receipt_type) => { - let maybe_pruned_domain_block_info = process_execution_receipt::( + let maybe_confirmed_domain_block_info = process_execution_receipt::( domain_id, operator_id, receipt, @@ -716,26 +717,31 @@ mod pallet { ) .map_err(Error::::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::( 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::::from)?; - if pruned_block_info.domain_block_number % T::StakeEpochDuration::get() + do_slash_operators::( + confirmed_block_info.invalid_bundle_authors.into_iter(), + ) + .map_err(Error::::from)?; + + if confirmed_block_info.domain_block_number % T::StakeEpochDuration::get() == Zero::zero() { let completed_epoch_index = do_finalize_domain_current_epoch::( domain_id, - pruned_block_info.domain_block_number, + confirmed_block_info.domain_block_number, ) .map_err(Error::::from)?; @@ -747,7 +753,7 @@ mod pallet { do_unlock_pending_withdrawals::( domain_id, - pruned_block_info.domain_block_number, + confirmed_block_info.domain_block_number, ) .map_err(Error::::from)?; } diff --git a/crates/pallet-domains/src/staking.rs b/crates/pallet-domains/src/staking.rs index 6c3c506c8c..a6ed759507 100644 --- a/crates/pallet-domains/src/staking.rs +++ b/crates/pallet-domains/src/staking.rs @@ -501,8 +501,6 @@ pub(crate) fn do_auto_stake_block_rewards( 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(