From 0caeecb33c129f05c7f9ec06cf58e4b538d6db56 Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 27 Feb 2024 02:30:49 +0800 Subject: [PATCH 1/7] Process only the first bad ER in submit_fraud_proof and lazily process the descendants ER Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 52 +++++++---- crates/pallet-domains/src/lib.rs | 112 +++++++++++------------- crates/pallet-domains/src/staking.rs | 49 ++++------- 3 files changed, 106 insertions(+), 107 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index cf3ce89977..3c4814326e 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/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, + 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( execution_receipt: ExecutionReceiptOf, receipt_type: AcceptedReceiptType, ) -> ProcessExecutionReceiptResult { + 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::(domain_id, submitter, execution_receipt); + add_new_receipt_to_block_tree::(domain_id, submitter, execution_receipt)?; // Update the head receipt number - HeadReceiptNumber::::insert(domain_id, domain_block_number); + HeadReceiptNumber::::insert(domain_id, receipt_block_number); HeadReceiptExtended::::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::::take(domain_id, to_prune) { - Some(h) => h, + let BlockTreeNode { + execution_receipt, + operator_ids, + } = match prune_receipt::(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::::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(); @@ -458,17 +457,24 @@ fn add_new_receipt_to_block_tree( domain_id: DomainId, submitter: OperatorId, execution_receipt: ExecutionReceiptOf, -) { +) -> Result<(), Error> { // Construct and add a new domain block to the block tree let er_hash = execution_receipt.hash::>(); let domain_block_number = execution_receipt.domain_block_number; + ensure!( + !BlockTree::::contains_key(domain_id, domain_block_number), + Error::OverwritingER, + ); + BlockTree::::insert(domain_id, domain_block_number, er_hash); let block_tree_node = BlockTreeNode { execution_receipt, operator_ids: sp_std::vec![submitter], }; BlockTreeNodes::::insert(er_hash, block_tree_node); + + Ok(()) } /// Import the genesis receipt to the block tree @@ -499,6 +505,20 @@ pub(crate) fn import_genesis_receipt( BlockTreeNodes::::insert(er_hash, block_tree_node); } +pub(crate) fn prune_receipt( + domain_id: DomainId, + receipt_number: DomainBlockNumberFor, +) -> Result>, Error> { + let receipt_hash = match BlockTree::::take(domain_id, receipt_number) { + Some(er_hash) => er_hash, + None => return Ok(None), + }; + let block_tree_node = + BlockTreeNodes::::take(receipt_hash).ok_or(Error::MissingDomainBlock)?; + + Ok(Some(block_tree_node)) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 65bd4b0a64..2250d36cce 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -116,6 +116,14 @@ pub type DomainBlockNumberFor = <::DomainHeader as Header>::Numb pub type DomainHashingFor = <::DomainHeader as Header>::Hashing; pub type ReceiptHashFor = <::DomainHeader as Header>::Hash; +pub type BlockTreeNodeFor = crate::block_tree::BlockTreeNode< + BlockNumberFor, + ::Hash, + DomainBlockNumberFor, + ::DomainHash, + BalanceOf, +>; + /// The current storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); @@ -124,8 +132,8 @@ mod pallet { #![allow(clippy::large_enum_variant)] use crate::block_tree::{ - execution_receipt_type, process_execution_receipt, BlockTreeNode, Error as BlockTreeError, - ReceiptType, + execution_receipt_type, process_execution_receipt, prune_receipt, AcceptedReceiptType, + Error as BlockTreeError, ReceiptType, }; #[cfg(not(feature = "runtime-benchmarks"))] use crate::bundle_storage_fund::refund_storage_fee; @@ -150,8 +158,9 @@ mod pallet { use crate::staking_epoch::{do_finalize_domain_current_epoch, Error as StakingEpochError}; use crate::weights::WeightInfo; use crate::{ - BalanceOf, BlockSlot, DomainBlockNumberFor, ElectionVerificationParams, HoldIdentifier, - NominatorId, OpaqueBundleOf, ReceiptHashFor, STORAGE_VERSION, + BalanceOf, BlockSlot, BlockTreeNodeFor, DomainBlockNumberFor, DomainHashingFor, + ElectionVerificationParams, HoldIdentifier, NominatorId, OpaqueBundleOf, ReceiptHashFor, + STORAGE_VERSION, }; use alloc::string::String; use codec::FullCodec; @@ -177,7 +186,6 @@ mod pallet { }; use sp_runtime::Saturating; use sp_std::boxed::Box; - use sp_std::collections::btree_map::BTreeMap; use sp_std::collections::btree_set::BTreeSet; use sp_std::fmt::Debug; use sp_std::vec; @@ -496,19 +504,8 @@ mod pallet { /// Mapping of block tree node hash to the node, each node represent a domain block #[pallet::storage] - pub(super) type BlockTreeNodes = StorageMap< - _, - Identity, - ReceiptHashFor, - BlockTreeNode< - BlockNumberFor, - T::Hash, - DomainBlockNumberFor, - T::DomainHash, - BalanceOf, - >, - OptionQuery, - >; + pub(super) type BlockTreeNodes = + StorageMap<_, Identity, ReceiptHashFor, BlockTreeNodeFor, OptionQuery>; /// The head receipt number of each domain #[pallet::storage] @@ -861,6 +858,7 @@ mod pallet { let operator_id = opaque_bundle.operator_id(); let bundle_size = opaque_bundle.size(); let receipt = opaque_bundle.into_receipt(); + let receipt_block_number = receipt.domain_block_number; #[cfg(not(feature = "runtime-benchmarks"))] let mut epoch_transitted = false; @@ -873,6 +871,24 @@ mod pallet { } // Add the exeuctione receipt to the block tree ReceiptType::Accepted(accepted_receipt_type) => { + // Before adding the new head receipt to the block tree, try to prune any previous + // bad ER at the same domain block and slash the submitter. + if accepted_receipt_type == AcceptedReceiptType::NewHead { + if let Some(block_tree_node) = + prune_receipt::(domain_id, receipt_block_number) + .map_err(Error::::from)? + { + let bad_receipt_hash = block_tree_node + .execution_receipt + .hash::>(); + do_slash_operators::( + block_tree_node.operator_ids.into_iter(), + SlashedReason::BadExecutionReceipt(bad_receipt_hash), + ) + .map_err(Error::::from)?; + } + } + #[cfg_attr(feature = "runtime-benchmarks", allow(unused_variables))] let maybe_confirmed_domain_block_info = process_execution_receipt::( domain_id, @@ -905,17 +921,9 @@ mod pallet { ) .map_err(Error::::from)?; - do_slash_operators::( - confirmed_block_info.invalid_bundle_authors.into_iter().map( - |operator_id| { - ( - operator_id, - SlashedReason::InvalidBundle( - confirmed_block_info.domain_block_number, - ), - ) - }, - ), + do_slash_operators::( + confirmed_block_info.invalid_bundle_authors.into_iter(), + SlashedReason::InvalidBundle(confirmed_block_info.domain_block_number), ) .map_err(Error::::from)?; @@ -992,7 +1000,6 @@ mod pallet { let domain_id = fraud_proof.domain_id(); if let Some(bad_receipt_hash) = fraud_proof.targeted_bad_receipt_hash() { - let mut operators_to_slash = BTreeMap::new(); let head_receipt_number = HeadReceiptNumber::::get(domain_id); let bad_receipt_number = BlockTreeNodes::::get(bad_receipt_hash) .ok_or::>(FraudProofError::BadReceiptNotFound.into())? @@ -1006,27 +1013,17 @@ mod pallet { Error::::from(FraudProofError::BadReceiptNotFound), ); - // Starting from the head receipt, prune all ER between [bad_receipt_number..head_receipt_number] - let mut to_prune = head_receipt_number; - - while to_prune >= bad_receipt_number { - let receipt_hash = BlockTree::::take(domain_id, to_prune) - .ok_or::>(FraudProofError::BadReceiptNotFound.into())?; - - let BlockTreeNode { operator_ids, .. } = - BlockTreeNodes::::take(receipt_hash) - .ok_or::>(FraudProofError::BadReceiptNotFound.into())?; - - // NOTE: the operator id will be deduplicated since we are using `BTreeMap` - // and slashed reason will hold earliest bad execution receipt hash which this - // operator submitted. - operator_ids.into_iter().for_each(|id| { - operators_to_slash - .insert(id, SlashedReason::BadExecutionReceipt(receipt_hash)); - }); - - to_prune -= One::one(); - } + // Prune the bad ER and slash the submitter, the descendants of the bad ER (i.e. all ERs in + // `[bad_receipt_number + 1..head_receipt_number]` ) and the corresponding submitter will be + // pruned/slashed lazily as the domain progressed. + let block_tree_node = prune_receipt::(domain_id, bad_receipt_number) + .map_err(Error::::from)? + .ok_or::>(FraudProofError::BadReceiptNotFound.into())?; + do_slash_operators::( + block_tree_node.operator_ids.into_iter(), + SlashedReason::BadExecutionReceipt(bad_receipt_hash), + ) + .map_err(Error::::from)?; // Update the head receipt number to `bad_receipt_number - 1` let new_head_receipt_number = bad_receipt_number.saturating_sub(One::one()); @@ -1036,10 +1033,6 @@ mod pallet { domain_id, new_head_receipt_number: Some(new_head_receipt_number), }); - - // Slash bad operators - do_slash_operators::(operators_to_slash.into_iter()) - .map_err(Error::::from)?; } else if let Some((targeted_bad_operator, slot)) = fraud_proof.targeted_bad_operator_and_slot_for_bundle_equivocation() { @@ -1048,12 +1041,9 @@ mod pallet { new_head_receipt_number: None, }); - do_slash_operators::( - vec![( - targeted_bad_operator, - SlashedReason::BundleEquivocation(slot), - )] - .into_iter(), + do_slash_operators::( + vec![targeted_bad_operator].into_iter(), + SlashedReason::BundleEquivocation(slot), ) .map_err(Error::::from)?; } diff --git a/crates/pallet-domains/src/staking.rs b/crates/pallet-domains/src/staking.rs index c3930d4ff9..7a3660101c 100644 --- a/crates/pallet-domains/src/staking.rs +++ b/crates/pallet-domains/src/staking.rs @@ -1170,16 +1170,11 @@ pub(crate) fn do_reward_operators( /// 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(operator_ids: Iter) -> Result<(), Error> -where - Iter: Iterator< - Item = ( - OperatorId, - SlashedReason, ReceiptHashFor>, - ), - >, -{ - for (operator_id, reason) in operator_ids { +pub(crate) fn do_slash_operators( + operator_ids: impl AsRef<[OperatorId]>, + slash_reason: SlashedReason, ReceiptHashFor>, +) -> Result<(), Error> { + for operator_id in operator_ids.as_ref() { Operators::::try_mutate(operator_id, |maybe_operator| { let operator = match maybe_operator.as_mut() { // If the operator is already slashed and removed due to fraud proof, when the operator @@ -1191,7 +1186,7 @@ where let mut pending_slashes = PendingSlashes::::get(operator.current_domain_id).unwrap_or_default(); - if pending_slashes.contains(&operator_id) { + if pending_slashes.contains(operator_id) { return Ok(()); } @@ -1203,24 +1198,24 @@ where .ok_or(Error::DomainNotInitialized)?; // slash and remove operator from next epoch set - operator.status = OperatorStatus::Slashed; - stake_summary.next_operators.remove(&operator_id); + operator.update_status(OperatorStatus::Slashed); + stake_summary.next_operators.remove(operator_id); // remove any current operator switches PendingOperatorSwitches::::mutate( operator.current_domain_id, |maybe_switching_operators| { if let Some(switching_operators) = maybe_switching_operators.as_mut() { - switching_operators.remove(&operator_id); + switching_operators.remove(operator_id); } }, ); - pending_slashes.insert(operator_id); + pending_slashes.insert(*operator_id); PendingSlashes::::insert(operator.current_domain_id, pending_slashes); Pallet::::deposit_event(Event::OperatorSlashed { - operator_id, - reason, + operator_id: *operator_id, + reason: slash_reason.clone(), }); Ok(()) }, @@ -2636,10 +2631,7 @@ pub(crate) mod tests { do_nominate_operator::(operator_id, deposit.0, deposit.1).unwrap(); } - do_slash_operators::( - vec![(operator_id, SlashedReason::InvalidBundle(1))].into_iter(), - ) - .unwrap(); + do_slash_operators::(vec![operator_id], SlashedReason::InvalidBundle(1)).unwrap(); let domain_stake_summary = DomainStakingSummary::::get(domain_id).unwrap(); assert!(!domain_stake_summary.next_operators.contains(&operator_id)); @@ -2741,15 +2733,12 @@ pub(crate) mod tests { ); } - do_slash_operators::( - vec![ - (operator_id_1, SlashedReason::InvalidBundle(1)), - (operator_id_2, SlashedReason::InvalidBundle(2)), - (operator_id_3, SlashedReason::InvalidBundle(3)), - ] - .into_iter(), - ) - .unwrap(); + do_slash_operators::(vec![operator_id_1], SlashedReason::InvalidBundle(1)) + .unwrap(); + do_slash_operators::(vec![operator_id_2], SlashedReason::InvalidBundle(2)) + .unwrap(); + do_slash_operators::(vec![operator_id_3], SlashedReason::InvalidBundle(3)) + .unwrap(); let domain_stake_summary = DomainStakingSummary::::get(domain_id).unwrap(); assert!(!domain_stake_summary.next_operators.contains(&operator_id_1)); From 5132c2b2078ff4d6e9e0a3d7bbe34732636aaaaf Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 27 Feb 2024 02:35:37 +0800 Subject: [PATCH 2/7] Introduce the LatestSubmittedER storage item for tracking the latest er submitted by an operator for a specific domain Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 30 ++++++++++++++++++++++++- crates/pallet-domains/src/lib.rs | 11 +++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 3c4814326e..5e7108ce32 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -4,7 +4,7 @@ use crate::{ BalanceOf, BlockTree, BlockTreeNodeFor, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockNumberFor, DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptExtended, HeadReceiptNumber, InboxedBundleAuthor, LatestConfirmedDomainBlock, - Pallet, ReceiptHashFor, + LatestSubmittedER, Pallet, ReceiptHashFor, }; use codec::{Decode, Encode}; use frame_support::{ensure, PalletError}; @@ -41,6 +41,7 @@ pub enum Error { BalanceOverflow, DomainTransfersTracking, InvalidDomainTransfers, + LatestSubmittedERFallback, OverwritingER, } @@ -397,6 +398,16 @@ pub(crate) fn process_execution_receipt( }); } } + + // Update the `LatestSubmittedER` for the operator + let key = (domain_id, submitter); + match receipt_block_number.cmp(&Pallet::::latest_submitted_er(key)) { + Ordering::Equal => {} + Ordering::Greater => LatestSubmittedER::::insert(key, receipt_block_number), + // The `LatestSubmittedER` should never fallback there must be something wrong + Ordering::Less => return Err(Error::LatestSubmittedERFallback), + } + Ok(None) } @@ -516,6 +527,23 @@ pub(crate) fn prune_receipt( let block_tree_node = BlockTreeNodes::::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::::latest_submitted_er(key); + if block_tree_node.execution_receipt.domain_block_number == latest_submitted_er { + LatestSubmittedER::::remove(key); + } + } + Ok(Some(block_tree_node)) } diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 2250d36cce..629014b7a2 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -582,6 +582,17 @@ mod pallet { OptionQuery, >; + /// The latest ER submitted by the operator for a given domain. It is used to determine if the operator + /// has submitted bad ER and is pending to slash. + /// + /// The storage item of a given `(domain_id, operator_id)` will be pruned after either: + /// - All the ERs submitted by the operator for this domain are confirmed and pruned + /// - All the bad ERs submitted by the operator for this domain are pruned and the operator is slashed + #[pallet::storage] + #[pallet::getter(fn latest_submitted_er)] + pub(super) type LatestSubmittedER = + StorageMap<_, Identity, (DomainId, OperatorId), DomainBlockNumberFor, ValueQuery>; + #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)] pub enum BundleError { /// Can not find the operator for given operator id. From d717fa842e75e817b4f823dd29d5d8bc44bfa485 Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 27 Feb 2024 02:42:45 +0800 Subject: [PATCH 3/7] Add checks to reject the bundle/staking request of operator that is pending to slash Signed-off-by: linning --- crates/pallet-domains/src/benchmarking.rs | 2 +- crates/pallet-domains/src/lib.rs | 19 ++++- crates/pallet-domains/src/staking.rs | 99 ++++++++++++++++++---- crates/pallet-domains/src/staking_epoch.rs | 15 +++- crates/pallet-domains/src/tests.rs | 22 +---- 5 files changed, 117 insertions(+), 40 deletions(-) diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index 251370dcbd..c5557c258e 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -343,7 +343,7 @@ mod benchmarks { let operator = Operators::::get(operator_id).expect("operator must exist"); assert_eq!( - operator.status, + *operator.status::(operator_id), OperatorStatus::Deregistered((domain_id, 0, DomainBlockNumberFor::::zero()).into()) ); } diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 629014b7a2..ac945c34b6 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -1619,7 +1619,8 @@ impl Pallet { let operator = Operators::::get(operator_id).ok_or(BundleError::InvalidOperatorId)?; ensure!( - operator.status != OperatorStatus::Slashed, + *operator.status::(operator_id) != OperatorStatus::Slashed + && *operator.status::(operator_id) != OperatorStatus::PendingSlash, BundleError::BadOperator ); @@ -2044,6 +2045,22 @@ impl Pallet { pub fn confirmed_domain_block_storage_key(domain_id: DomainId) -> Vec { LatestConfirmedDomainBlock::::hashed_key_for(domain_id) } + + pub fn is_operator_pending_to_slash(domain_id: DomainId, operator_id: OperatorId) -> bool { + let latest_submitted_er = LatestSubmittedER::::get((domain_id, operator_id)); + + // The genesis receipt is always valid + if latest_submitted_er.is_zero() { + return false; + } + + let head_receipt_number = HeadReceiptNumber::::get(domain_id); + + // If the operator have submitted an ER greater than the current `head_receipt_number` + // meaning the ER is a bad ER and the `head_receipt_number` is previously reverted by + // a fraud proof + head_receipt_number < latest_submitted_er + } } impl Pallet diff --git a/crates/pallet-domains/src/staking.rs b/crates/pallet-domains/src/staking.rs index 7a3660101c..a03f6e3071 100644 --- a/crates/pallet-domains/src/staking.rs +++ b/crates/pallet-domains/src/staking.rs @@ -2,9 +2,9 @@ use crate::bundle_storage_fund::{self, deposit_reserve_for_storage_fund}; use crate::pallet::{ - Deposits, DomainRegistry, DomainStakingSummary, NextOperatorId, NominatorCount, - OperatorIdOwner, OperatorSigningKey, Operators, PendingOperatorSwitches, PendingSlashes, - PendingStakingOperationCount, Withdrawals, + Deposits, DomainRegistry, DomainStakingSummary, LatestSubmittedER, NextOperatorId, + NominatorCount, OperatorIdOwner, OperatorSigningKey, Operators, PendingOperatorSwitches, + PendingSlashes, PendingStakingOperationCount, Withdrawals, }; use crate::staking_epoch::mint_funds; use crate::{ @@ -164,6 +164,7 @@ pub enum OperatorStatus { /// De-registered at given domain epoch. Deregistered(OperatorDeregisteredInfo), Slashed, + PendingSlash, } /// Type that represents an operator details. @@ -180,7 +181,11 @@ pub struct Operator { pub current_epoch_rewards: Balance, /// Total shares of all the nominators under this operator. pub current_total_shares: Share, - pub status: OperatorStatus, + /// The status of the operator, it may be stale due to the `OperatorStatus::PendingSlash` is + /// not assigned to this field directlt, thus MUST use the `status()` method to query the status + /// instead. + /// TODO: update the filed to `_status` to avoid accidental access in next network reset + status: OperatorStatus, /// Total deposits during the previous epoch pub deposits_in_epoch: Balance, /// Total withdrew shares during the previous epoch @@ -189,6 +194,46 @@ pub struct Operator { pub total_storage_fee_deposit: Balance, } +impl Operator { + pub fn status(&self, operator_id: OperatorId) -> &OperatorStatus { + if matches!(self.status, OperatorStatus::Slashed) { + &OperatorStatus::Slashed + } else if Pallet::::is_operator_pending_to_slash(self.current_domain_id, operator_id) { + &OperatorStatus::PendingSlash + } else { + &self.status + } + } + + pub fn update_status(&mut self, new_status: OperatorStatus) { + self.status = new_status; + } +} + +#[cfg(test)] +impl Operator { + pub(crate) fn dummy( + domain_id: DomainId, + signing_key: OperatorPublicKey, + minimum_nominator_stake: Balance, + ) -> Self { + Operator { + signing_key, + current_domain_id: domain_id, + next_domain_id: domain_id, + minimum_nominator_stake, + nomination_tax: Default::default(), + current_total_stake: Zero::zero(), + current_epoch_rewards: Zero::zero(), + current_total_shares: Zero::zero(), + status: OperatorStatus::Registered, + deposits_in_epoch: Zero::zero(), + withdrawals_in_epoch: Zero::zero(), + total_storage_fee_deposit: Zero::zero(), + } + } +} + #[derive(TypeInfo, Debug, Encode, Decode, Clone, PartialEq, Eq)] pub struct StakingSummary { /// Current epoch index for the domain. @@ -245,6 +290,7 @@ pub enum Error { UnlockPeriodNotComplete, OperatorNotDeregistered, BundleStorageFund(bundle_storage_fund::Error), + UnconfirmedER, } // Increase `PendingStakingOperationCount` by one and check if the `MaxPendingStakingOperation` @@ -509,7 +555,7 @@ pub(crate) fn do_nominate_operator( let operator = maybe_operator.as_mut().ok_or(Error::UnknownOperator)?; ensure!( - operator.status == OperatorStatus::Registered, + *operator.status::(operator_id) == OperatorStatus::Registered, Error::OperatorNotRegistered ); @@ -621,10 +667,17 @@ pub(crate) fn do_switch_operator_domain( note_pending_staking_operation::(operator.current_domain_id)?; ensure!( - operator.status == OperatorStatus::Registered, + *operator.status::(operator_id) == OperatorStatus::Registered, Error::OperatorNotRegistered ); + // Reject switching domain if there is unconfirmed ER submitted by this operator + // on the `current_domain_id` + ensure!( + !LatestSubmittedER::::contains_key((operator.current_domain_id, operator_id)), + Error::UnconfirmedER + ); + // noop when switch is for same domain if operator.current_domain_id == new_domain_id { return Ok(operator.current_domain_id); @@ -673,7 +726,7 @@ pub(crate) fn do_deregister_operator( note_pending_staking_operation::(operator.current_domain_id)?; ensure!( - operator.status == OperatorStatus::Registered, + *operator.status::(operator_id) == OperatorStatus::Registered, Error::OperatorNotRegistered ); @@ -696,7 +749,7 @@ pub(crate) fn do_deregister_operator( ) .into(); - operator.status = OperatorStatus::Deregistered(operator_deregister_info); + operator.update_status(OperatorStatus::Deregistered(operator_deregister_info)); stake_summary.next_operators.remove(&operator_id); Ok(()) @@ -713,7 +766,7 @@ pub(crate) fn do_withdraw_stake( Operators::::try_mutate(operator_id, |maybe_operator| { let operator = maybe_operator.as_mut().ok_or(Error::UnknownOperator)?; ensure!( - operator.status == OperatorStatus::Registered, + *operator.status::(operator_id) == OperatorStatus::Registered, Error::OperatorNotRegistered ); @@ -907,7 +960,7 @@ pub(crate) fn do_unlock_funds( ) -> Result, Error> { let operator = Operators::::get(operator_id).ok_or(Error::UnknownOperator)?; ensure!( - operator.status == OperatorStatus::Registered, + *operator.status::(operator_id) == OperatorStatus::Registered, Error::OperatorNotRegistered ); @@ -996,7 +1049,7 @@ pub(crate) fn do_unlock_operator(operator_id: OperatorId) -> Result<( let OperatorDeregisteredInfo { domain_epoch, unlock_at_confirmed_domain_block_number, - } = match operator.status { + } = match operator.status::(operator_id) { OperatorStatus::Deregistered(operator_deregistered_info) => operator_deregistered_info, _ => return Err(Error::OperatorNotDeregistered), }; @@ -1005,7 +1058,7 @@ pub(crate) fn do_unlock_operator(operator_id: OperatorId) -> Result<( let latest_confirmed_block_number = Pallet::::latest_confirmed_domain_block_number(domain_id); ensure!( - unlock_at_confirmed_domain_block_number <= latest_confirmed_block_number, + *unlock_at_confirmed_domain_block_number <= latest_confirmed_block_number, Error::UnlockPeriodNotComplete ); @@ -1773,7 +1826,7 @@ pub(crate) mod tests { let operator = Operators::::get(operator_id).unwrap(); assert_eq!( - operator.status, + *operator.status::(operator_id), OperatorStatus::Deregistered( ( domain_id, @@ -2637,7 +2690,10 @@ pub(crate) mod tests { assert!(!domain_stake_summary.next_operators.contains(&operator_id)); let operator = Operators::::get(operator_id).unwrap(); - assert_eq!(operator.status, OperatorStatus::Slashed); + assert_eq!( + *operator.status::(operator_id), + OperatorStatus::Slashed + ); let pending_slashes = PendingSlashes::::get(domain_id).unwrap(); assert!(pending_slashes.contains(&operator_id)); @@ -2746,13 +2802,22 @@ pub(crate) mod tests { assert!(!domain_stake_summary.next_operators.contains(&operator_id_3)); let operator = Operators::::get(operator_id_1).unwrap(); - assert_eq!(operator.status, OperatorStatus::Slashed); + assert_eq!( + *operator.status::(operator_id_1), + OperatorStatus::Slashed + ); let operator = Operators::::get(operator_id_2).unwrap(); - assert_eq!(operator.status, OperatorStatus::Slashed); + assert_eq!( + *operator.status::(operator_id_2), + OperatorStatus::Slashed + ); let operator = Operators::::get(operator_id_3).unwrap(); - assert_eq!(operator.status, OperatorStatus::Slashed); + assert_eq!( + *operator.status::(operator_id_3), + OperatorStatus::Slashed + ); assert_eq!( Balances::total_balance(&crate::tests::TreasuryAccount::get()), diff --git a/crates/pallet-domains/src/staking_epoch.rs b/crates/pallet-domains/src/staking_epoch.rs index c07e7a1d1a..c240709a57 100644 --- a/crates/pallet-domains/src/staking_epoch.rs +++ b/crates/pallet-domains/src/staking_epoch.rs @@ -22,6 +22,7 @@ use sp_domains::{DomainId, EpochIndex, OperatorId}; use sp_runtime::traits::{CheckedAdd, CheckedSub, One, Zero}; use sp_runtime::Saturating; use sp_std::collections::btree_map::BTreeMap; +use sp_std::collections::btree_set::BTreeSet; #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)] pub enum Error { @@ -168,7 +169,7 @@ fn switch_operator( .ok_or(TransitionError::UnknownOperator)?; // operator is not registered, just no-op - if operator.status != OperatorStatus::Registered { + if *operator.status::(operator_id) != OperatorStatus::Registered { return Ok(()); } @@ -200,7 +201,15 @@ pub(crate) fn do_finalize_domain_epoch_staking( let mut total_domain_stake = BalanceOf::::zero(); let mut current_operators = BTreeMap::new(); + let mut next_operators = BTreeSet::new(); for next_operator_id in &stake_summary.next_operators { + // If an operator is pending to slash then similar to the slashed operator it should not be added + // into the `next_operators/current_operators` and we should not `do_finalize_operator_epoch_staking` + // for it. + if Pallet::::is_operator_pending_to_slash(domain_id, *next_operator_id) { + continue; + } + let operator_stake = do_finalize_operator_epoch_staking::( domain_id, *next_operator_id, @@ -211,6 +220,7 @@ pub(crate) fn do_finalize_domain_epoch_staking( .checked_add(&operator_stake) .ok_or(TransitionError::BalanceOverflow)?; current_operators.insert(*next_operator_id, operator_stake); + next_operators.insert(*next_operator_id); } let election_verification_params = ElectionVerificationParams { @@ -224,6 +234,7 @@ pub(crate) fn do_finalize_domain_epoch_staking( stake_summary.current_epoch_index = next_epoch; stake_summary.current_total_stake = total_domain_stake; stake_summary.current_operators = current_operators; + stake_summary.next_operators = next_operators; Ok(previous_epoch) }) @@ -240,7 +251,7 @@ pub(crate) fn do_finalize_operator_epoch_staking( .as_mut() .ok_or(TransitionError::UnknownOperator)?; - if operator.status != OperatorStatus::Registered { + if *operator.status::(operator_id) != OperatorStatus::Registered { return Err(TransitionError::OperatorNotRegistered); } diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index 02a4633787..7309d2ef1f 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -5,7 +5,7 @@ use crate::{ self as pallet_domains, BalanceOf, BlockSlot, BlockTree, BlockTreeNodes, BundleError, Config, ConsensusBlockHash, DomainBlockNumberFor, DomainHashingFor, DomainRegistry, ExecutionInbox, ExecutionReceiptOf, FraudProofError, FungibleHoldId, HeadReceiptNumber, NextDomainId, - OperatorStatus, Operators, ReceiptHashFor, + Operators, ReceiptHashFor, }; use codec::{Decode, Encode, MaxEncodedLen}; use core::mem; @@ -39,7 +39,7 @@ use sp_domains_fraud_proof::{ FraudProofVerificationInfoResponse, SetCodeExtrinsic, }; use sp_runtime::traits::{ - AccountIdConversion, BlakeTwo256, BlockNumberProvider, Hash as HashT, IdentityLookup, One, Zero, + AccountIdConversion, BlakeTwo256, BlockNumberProvider, Hash as HashT, IdentityLookup, One, }; use sp_runtime::{BuildStorage, Digest, OpaqueExtrinsic, Saturating}; use sp_state_machine::backend::AsTrieBackend; @@ -618,23 +618,7 @@ pub(crate) fn register_genesis_domain(creator: u128, operator_ids: Vec::insert( - operator_id, - Operator { - signing_key: pair.public(), - current_domain_id: domain_id, - next_domain_id: domain_id, - minimum_nominator_stake: SSC, - nomination_tax: Default::default(), - current_total_stake: Zero::zero(), - current_epoch_rewards: Zero::zero(), - current_total_shares: Zero::zero(), - status: OperatorStatus::Registered, - deposits_in_epoch: Zero::zero(), - withdrawals_in_epoch: Zero::zero(), - total_storage_fee_deposit: Zero::zero(), - }, - ); + Operators::::insert(operator_id, Operator::dummy(domain_id, pair.public(), SSC)); } domain_id From c1c11edbb50aeaf31207091405702055116c2efa Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 27 Feb 2024 02:44:32 +0800 Subject: [PATCH 4/7] Add check to reject fraud proof that targetting pending to prune bad ER Signed-off-by: linning --- crates/pallet-domains/src/lib.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index ac945c34b6..96373b9303 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -656,6 +656,8 @@ mod pallet { UnexpectedFraudProof, /// Bad/Invalid bundle equivocation fraud proof. BadBundleEquivocationFraudProof, + /// The bad receipt already reported by a previous fraud proof + BadReceiptAlreadyReported, } impl From for Error { @@ -1705,6 +1707,14 @@ impl Pallet { FraudProofError::ChallengingGenesisReceipt ); + ensure!( + !Self::is_bad_er_pending_to_prune( + fraud_proof.domain_id(), + bad_receipt.domain_block_number + ), + FraudProofError::BadReceiptAlreadyReported, + ); + match fraud_proof { FraudProof::InvalidBlockFees(InvalidBlockFeesProof { storage_proof, .. }) => { verify_invalid_block_fees_fraud_proof::< @@ -1970,8 +1980,11 @@ impl Pallet { ) -> Option> { let oldest_nonconfirmed_er_number = Self::latest_confirmed_domain_block_number(domain_id).saturating_add(One::one()); + let is_er_exist = BlockTree::::get(domain_id, oldest_nonconfirmed_er_number).is_some(); + let is_pending_to_prune = + Self::is_bad_er_pending_to_prune(domain_id, oldest_nonconfirmed_er_number); - if BlockTree::::get(domain_id, oldest_nonconfirmed_er_number).is_some() { + if is_er_exist && !is_pending_to_prune { Some(oldest_nonconfirmed_er_number) } else { // The `oldest_nonconfirmed_er_number` ER may not exist if @@ -2046,6 +2059,22 @@ impl Pallet { LatestConfirmedDomainBlock::::hashed_key_for(domain_id) } + pub fn is_bad_er_pending_to_prune( + domain_id: DomainId, + receipt_number: DomainBlockNumberFor, + ) -> bool { + // The genesis receipt is always valid + if receipt_number.is_zero() { + return false; + } + + let head_receipt_number = HeadReceiptNumber::::get(domain_id); + + // If `receipt_number` is greater than the current `head_receipt_number` meaning it is a + // bad ER and the `head_receipt_number` is previously reverted by a fraud proof + head_receipt_number < receipt_number + } + pub fn is_operator_pending_to_slash(domain_id: DomainId, operator_id: OperatorId) -> bool { let latest_submitted_er = LatestSubmittedER::::get((domain_id, operator_id)); From 3e4e6b8d8e6b8f3a5447fd206e2ccd6962ff522a Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 27 Feb 2024 02:45:30 +0800 Subject: [PATCH 5/7] Add test cases for the pending to the prune bad ER Signed-off-by: linning --- crates/pallet-domains/src/tests.rs | 57 +++++++++-- crates/sp-domains/src/lib.rs | 3 + crates/subspace-runtime/src/lib.rs | 7 ++ domains/client/domain-operator/src/tests.rs | 100 +++++++++++--------- test/subspace-test-runtime/src/lib.rs | 7 ++ 5 files changed, 125 insertions(+), 49 deletions(-) diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index 7309d2ef1f..0c53f1a366 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -1247,7 +1247,8 @@ fn generate_invalid_domain_block_hash_fraud_proof( #[test] fn test_basic_fraud_proof_processing() { let creator = 0u128; - let operator_id = 1u64; + let malicious_operator = 1u64; + let honest_operator = 2u64; let head_domain_number = BlockTreePruningDepth::get() - 1; let test_cases = vec![ 1, @@ -1259,8 +1260,9 @@ fn test_basic_fraud_proof_processing() { for bad_receipt_at in test_cases { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { - let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); + let domain_id = + register_genesis_domain(creator, vec![malicious_operator, honest_operator]); + extend_block_tree_from_zero(domain_id, malicious_operator, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -1282,8 +1284,24 @@ fn test_basic_fraud_proof_processing() { assert_eq!(head_receipt_number_after_fraud_proof, bad_receipt_at - 1); for block_number in bad_receipt_at..=head_domain_number { - // The targetted ER and all its descendants should be removed from the block tree - assert!(BlockTree::::get(domain_id, block_number).is_none()); + if block_number == bad_receipt_at { + // The targetted ER should be removed from the block tree + assert!(BlockTree::::get(domain_id, block_number).is_none()); + } else { + // All the bad ER's descendants should be marked as pending to prune and the submitter + // should be marked as pending to slash + assert!(BlockTree::::get(domain_id, block_number).is_some()); + assert!(Domains::is_bad_er_pending_to_prune(domain_id, block_number)); + let submitter = get_block_tree_node_at::(domain_id, block_number) + .unwrap() + .operator_ids; + for operator_id in submitter { + assert!(Domains::is_operator_pending_to_slash( + domain_id, + operator_id + )); + } + } // The other data that used to verify ER should not be removed, such that the honest // operator can re-submit the valid ER @@ -1300,7 +1318,7 @@ fn test_basic_fraud_proof_processing() { let resubmit_receipt = bad_receipt; let bundle = create_dummy_bundle_with_receipts( domain_id, - operator_id, + honest_operator, H256::random(), resubmit_receipt, ); @@ -1309,6 +1327,33 @@ fn test_basic_fraud_proof_processing() { HeadReceiptNumber::::get(domain_id), head_receipt_number_after_fraud_proof + 1 ); + + // Submit one more ER, the bad ER at the same domain block should be pruned + let next_block_number = frame_system::Pallet::::current_block_number() + 1; + run_to_block::(next_block_number, H256::random()); + if let Some(receipt_hash) = BlockTree::::get(domain_id, bad_receipt_at + 1) { + let mut receipt = BlockTreeNodes::::get(receipt_hash) + .unwrap() + .execution_receipt; + receipt.final_state_root = H256::random(); + let bundle = create_dummy_bundle_with_receipts( + domain_id, + honest_operator, + H256::random(), + receipt.clone(), + ); + assert_ok!(Domains::submit_bundle(RawOrigin::None.into(), bundle)); + + assert_eq!( + HeadReceiptNumber::::get(domain_id), + head_receipt_number_after_fraud_proof + 2 + ); + assert!(BlockTreeNodes::::get(receipt_hash).is_none()); + assert!(!Domains::is_bad_er_pending_to_prune( + domain_id, + receipt.domain_block_number + )); + } }); } } diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index 71803689b2..5dd49eaca2 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -1248,6 +1248,9 @@ sp_api::decl_runtime_apis! { /// Reture the consensus chain byte fee that will used to charge the domain transaction for consensus /// chain storage fee fn consensus_chain_byte_fee() -> Balance; + + /// Return if the receipt is exist and pending to prune + fn is_bad_er_pending_to_prune(domain_id: DomainId, receipt_hash: HeaderHashFor) -> bool; } pub trait BundleProducerElectionApi { diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index b2be9576ca..3353f0c65c 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -1104,6 +1104,13 @@ impl_runtime_apis! { fn consensus_chain_byte_fee() -> Balance { DOMAIN_STORAGE_FEE_MULTIPLIER * TransactionFees::transaction_byte_fee() } + + fn is_bad_er_pending_to_prune(domain_id: DomainId, receipt_hash: DomainHash) -> bool { + Domains::execution_receipt(receipt_hash).map( + |er| Domains::is_bad_er_pending_to_prune(domain_id, er.domain_block_number) + ) + .unwrap_or(false) + } } impl sp_domains::BundleProducerElectionApi for Runtime { diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index a7371fc009..1e94de550c 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -3924,61 +3924,75 @@ async fn test_bad_receipt_chain() { // Remove the fraud proof from tx pool ferdie.clear_tx_pool().await.unwrap(); - // Produce a bundle with another bad ER that use previous bad ER as parent - let parent_bad_receipt_hash = bad_receipt_hash; - let slot = ferdie.produce_slot(); - let bundle = bundle_producer - .produce_bundle( - 0, - OperatorSlotInfo { - slot: slot.0, - proof_of_time: slot.1, - }, - ) - .await - .expect("produce bundle must success") - .expect("must win the challenge"); - let (bad_receipt_hash, bad_submit_bundle_tx) = { - let mut opaque_bundle = bundle; - let receipt = &mut opaque_bundle.sealed_header.header.receipt; - receipt.parent_domain_block_receipt_hash = parent_bad_receipt_hash; - receipt.domain_block_hash = Default::default(); - opaque_bundle.sealed_header.signature = Sr25519Keyring::Alice - .pair() - .sign(opaque_bundle.sealed_header.pre_hash().as_ref()) - .into(); - ( - opaque_bundle.receipt().hash::(), - bundle_to_tx(opaque_bundle), - ) - }; - ferdie - .submit_transaction(bad_submit_bundle_tx) - .await - .unwrap(); - // Wait for a fraud proof that target the first bad ER let wait_for_fraud_proof_fut = ferdie.wait_for_fraud_proof(move |fp| { matches!( fp, FraudProof::InvalidDomainBlockHash(InvalidDomainBlockHashProof { .. }) - ) && fp.targeted_bad_receipt_hash() == Some(parent_bad_receipt_hash) + ) && fp.targeted_bad_receipt_hash() == Some(bad_receipt_hash) }); - // Produce a consensus block that contains the bad receipt and it should - // be added to the consensus chain block tree - produce_block_with!(ferdie.produce_block_with_slot(slot), alice) - .await - .unwrap(); - assert!(ferdie.does_receipt_exist(bad_receipt_hash).unwrap()); + // Produce more bundle with bad ER that use previous bad ER as parent + let mut parent_bad_receipt_hash = bad_receipt_hash; + let mut bad_receipt_descendants = vec![]; + for _ in 0..10 { + let slot = ferdie.produce_slot(); + let bundle = bundle_producer + .produce_bundle( + 0, + OperatorSlotInfo { + slot: slot.0, + proof_of_time: slot.1, + }, + ) + .await + .expect("produce bundle must success") + .expect("must win the challenge"); + let (receipt_hash, bad_submit_bundle_tx) = { + let mut opaque_bundle = bundle; + let receipt = &mut opaque_bundle.sealed_header.header.receipt; + receipt.parent_domain_block_receipt_hash = parent_bad_receipt_hash; + receipt.domain_block_hash = Default::default(); + opaque_bundle.sealed_header.signature = Sr25519Keyring::Alice + .pair() + .sign(opaque_bundle.sealed_header.pre_hash().as_ref()) + .into(); + ( + opaque_bundle.receipt().hash::(), + bundle_to_tx(opaque_bundle), + ) + }; + parent_bad_receipt_hash = receipt_hash; + bad_receipt_descendants.push(receipt_hash); + ferdie + .produce_block_with_slot_at( + slot, + ferdie.client.info().best_hash, + Some(vec![bad_submit_bundle_tx]), + ) + .await + .unwrap(); + } + + // All bad ERs should be added to the consensus chain block tree + for receipt_hash in bad_receipt_descendants.iter().chain(&[bad_receipt_hash]) { + assert!(ferdie.does_receipt_exist(*receipt_hash).unwrap()); + } // The fraud proof should be submitted let _ = wait_for_fraud_proof_fut.await; - // Both bad ER should be pruned + // The first bad ER should be pruned and its descendants are marked as pending to prune ferdie.produce_blocks(1).await.unwrap(); - for er_hash in [parent_bad_receipt_hash, bad_receipt_hash] { - assert!(!ferdie.does_receipt_exist(er_hash).unwrap()); + assert!(!ferdie.does_receipt_exist(bad_receipt_hash).unwrap()); + + let ferdie_best_hash = ferdie.client.info().best_hash; + let runtime_api = ferdie.client.runtime_api(); + for receipt_hash in bad_receipt_descendants { + assert!(ferdie.does_receipt_exist(receipt_hash).unwrap()); + assert!(runtime_api + .is_bad_er_pending_to_prune(ferdie_best_hash, GENESIS_DOMAIN_ID, receipt_hash) + .unwrap()); } } diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index cc9d1f24ae..57b5edd8b9 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1297,6 +1297,13 @@ impl_runtime_apis! { fn consensus_chain_byte_fee() -> Balance { DOMAIN_STORAGE_FEE_MULTIPLIER * TransactionFees::transaction_byte_fee() } + + fn is_bad_er_pending_to_prune(domain_id: DomainId, receipt_hash: DomainHash) -> bool { + Domains::execution_receipt(receipt_hash).map( + |er| Domains::is_bad_er_pending_to_prune(domain_id, er.domain_block_number) + ) + .unwrap_or(false) + } } impl sp_domains::BundleProducerElectionApi for Runtime { From 918a72f16cc1f4fa4ebbb6be0cb8268f205207a8 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 1 Mar 2024 19:25:32 +0800 Subject: [PATCH 6/7] Remove the unnecessary LatestSubmittedERFallback error from pallet-domains Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 5e7108ce32..e95eaea575 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -41,7 +41,6 @@ pub enum Error { BalanceOverflow, DomainTransfersTracking, InvalidDomainTransfers, - LatestSubmittedERFallback, OverwritingER, } @@ -401,11 +400,8 @@ pub(crate) fn process_execution_receipt( // Update the `LatestSubmittedER` for the operator let key = (domain_id, submitter); - match receipt_block_number.cmp(&Pallet::::latest_submitted_er(key)) { - Ordering::Equal => {} - Ordering::Greater => LatestSubmittedER::::insert(key, receipt_block_number), - // The `LatestSubmittedER` should never fallback there must be something wrong - Ordering::Less => return Err(Error::LatestSubmittedERFallback), + if receipt_block_number > Pallet::::latest_submitted_er(key) { + LatestSubmittedER::::insert(key, receipt_block_number) } Ok(None) From 812f9f188928eb1d544ad9f383c99813ad2796de Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 1 Mar 2024 20:07:40 +0800 Subject: [PATCH 7/7] Remove the switch_domain extrinsic from pallet-domains switch_domain is not supported currently due to incompatible with lazily slashing, this commit only remove the entry of switch_domain and comment out related tests/benchmark to avoid invasive change to the rest of the code and also we may support it in the future. Signed-off-by: linning --- crates/pallet-domains/src/benchmarking.rs | 46 ++--- crates/pallet-domains/src/lib.rs | 25 +-- crates/pallet-domains/src/staking.rs | 200 ++++++++++----------- crates/pallet-domains/src/staking_epoch.rs | 161 ++++++++--------- 4 files changed, 202 insertions(+), 230 deletions(-) diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index c5557c258e..5be290a692 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -308,28 +308,30 @@ mod benchmarks { ); } - #[benchmark] - fn switch_domain() { - let domain1_id = register_domain::(); - let domain2_id = register_domain::(); - - let (operator_owner, operator_id) = - register_helper_operator::(domain1_id, T::Currency::minimum_balance()); - - #[extrinsic_call] - _( - RawOrigin::Signed(operator_owner.clone()), - operator_id, - domain2_id, - ); - - let operator = Operators::::get(operator_id).expect("operator must exist"); - assert_eq!(operator.next_domain_id, domain2_id); - - let pending_switch = - PendingOperatorSwitches::::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::(); + // let domain2_id = register_domain::(); + + // let (operator_owner, operator_id) = + // register_helper_operator::(domain1_id, T::Currency::minimum_balance()); + + // #[extrinsic_call] + // _( + // RawOrigin::Signed(operator_owner.clone()), + // operator_id, + // domain2_id, + // ); + + // let operator = Operators::::get(operator_id).expect("operator must exist"); + // assert_eq!(operator.next_domain_id, domain2_id); + + // let pending_switch = + // PendingOperatorSwitches::::get(domain1_id).expect("pending switch must exist"); + // assert!(pending_switch.contains(&operator_id)); + // } #[benchmark] fn deregister_operator() { diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 96373b9303..4ab8fc047f 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -151,9 +151,8 @@ mod pallet { use crate::staking::do_reward_operators; use crate::staking::{ do_deregister_operator, do_nominate_operator, do_register_operator, do_slash_operators, - do_switch_operator_domain, do_unlock_funds, do_unlock_operator, do_withdraw_stake, Deposit, - DomainEpoch, Error as StakingError, Operator, OperatorConfig, SharePrice, StakingSummary, - Withdrawal, + do_unlock_funds, do_unlock_operator, do_withdraw_stake, Deposit, DomainEpoch, + Error as StakingError, Operator, OperatorConfig, SharePrice, StakingSummary, Withdrawal, }; use crate::staking_epoch::{do_finalize_domain_current_epoch, Error as StakingEpochError}; use crate::weights::WeightInfo; @@ -1186,26 +1185,6 @@ mod pallet { Ok(()) } - #[pallet::call_index(7)] - #[pallet::weight(T::WeightInfo::switch_domain())] - pub fn switch_domain( - origin: OriginFor, - operator_id: OperatorId, - new_domain_id: DomainId, - ) -> DispatchResult { - let who = ensure_signed(origin)?; - - let old_domain_id = do_switch_operator_domain::(who, operator_id, new_domain_id) - .map_err(Error::::from)?; - - Self::deposit_event(Event::OperatorSwitchedDomain { - old_domain_id, - new_domain_id, - }); - - Ok(()) - } - #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::deregister_operator())] pub fn deregister_operator( diff --git a/crates/pallet-domains/src/staking.rs b/crates/pallet-domains/src/staking.rs index a03f6e3071..84e94b92f7 100644 --- a/crates/pallet-domains/src/staking.rs +++ b/crates/pallet-domains/src/staking.rs @@ -637,7 +637,9 @@ pub(crate) fn hold_deposit( Ok(()) } -pub(crate) fn do_switch_operator_domain( +// TODO: `switch_domain` is not supported currently due to incompatible with lazily slashing +#[allow(dead_code)] +fn do_switch_operator_domain( operator_owner: T::AccountId, operator_id: OperatorId, new_domain_id: DomainId, @@ -1284,8 +1286,7 @@ pub(crate) mod tests { use crate::domain_registry::{DomainConfig, DomainObject}; use crate::pallet::{ Config, Deposits, DomainRegistry, DomainStakingSummary, LatestConfirmedDomainBlock, - NextOperatorId, NominatorCount, OperatorIdOwner, Operators, PendingOperatorSwitches, - PendingSlashes, Withdrawals, + NextOperatorId, NominatorCount, OperatorIdOwner, Operators, PendingSlashes, Withdrawals, }; use crate::staking::{ do_convert_previous_epoch_withdrawal, do_nominate_operator, do_reward_operators, @@ -1705,97 +1706,99 @@ pub(crate) mod tests { }); } - #[test] - fn switch_domain_operator() { - let old_domain_id = DomainId::new(0); - let new_domain_id = DomainId::new(1); - let operator_account = 1; - let operator_free_balance = 250 * SSC; - let operator_stake = 200 * SSC; - let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - - let mut ext = new_test_ext(); - ext.execute_with(|| { - let (operator_id, _) = register_operator( - old_domain_id, - operator_account, - operator_free_balance, - operator_stake, - SSC, - pair.public(), - BTreeMap::new(), - ); - - let domain_config = DomainConfig { - domain_name: String::from_utf8(vec![0; 1024]).unwrap(), - runtime_id: 0, - max_block_size: u32::MAX, - max_block_weight: Weight::MAX, - bundle_slot_probability: (0, 0), - target_bundles_per_block: 0, - operator_allow_list: OperatorAllowList::Anyone, - initial_balances: Default::default(), - }; - - let domain_obj = DomainObject { - owner_account_id: 0, - created_at: 0, - genesis_receipt_hash: Default::default(), - domain_config, - domain_runtime_info: Default::default(), - }; - - DomainRegistry::::insert(new_domain_id, domain_obj); - - DomainStakingSummary::::insert( - new_domain_id, - StakingSummary { - current_epoch_index: 0, - current_total_stake: 0, - current_operators: BTreeMap::new(), - next_operators: BTreeSet::new(), - current_epoch_rewards: BTreeMap::new(), - }, - ); - - let res = Domains::switch_domain( - RuntimeOrigin::signed(operator_account), - operator_id, - new_domain_id, - ); - assert_ok!(res); - - let old_domain_stake_summary = - DomainStakingSummary::::get(old_domain_id).unwrap(); - assert!(!old_domain_stake_summary - .next_operators - .contains(&operator_id)); - - let new_domain_stake_summary = - DomainStakingSummary::::get(new_domain_id).unwrap(); - assert!(!new_domain_stake_summary - .next_operators - .contains(&operator_id)); - - let operator = Operators::::get(operator_id).unwrap(); - assert_eq!(operator.current_domain_id, old_domain_id); - assert_eq!(operator.next_domain_id, new_domain_id); - assert_eq!( - PendingOperatorSwitches::::get(old_domain_id).unwrap(), - BTreeSet::from_iter(vec![operator_id]) - ); - - let res = Domains::switch_domain( - RuntimeOrigin::signed(operator_account), - operator_id, - new_domain_id, - ); - assert_err!( - res, - Error::::Staking(crate::staking::Error::PendingOperatorSwitch) - ) - }); - } + // TODO: `switch_domain` is not supported currently due to incompatible with lazily slashing + // enable this test when `switch_domain` is ready + // #[test] + // fn switch_domain_operator() { + // let old_domain_id = DomainId::new(0); + // let new_domain_id = DomainId::new(1); + // let operator_account = 1; + // let operator_free_balance = 250 * SSC; + // let operator_stake = 200 * SSC; + // let pair = OperatorPair::from_seed(&U256::from(0u32).into()); + + // let mut ext = new_test_ext(); + // ext.execute_with(|| { + // let (operator_id, _) = register_operator( + // old_domain_id, + // operator_account, + // operator_free_balance, + // operator_stake, + // SSC, + // pair.public(), + // BTreeMap::new(), + // ); + + // let domain_config = DomainConfig { + // domain_name: String::from_utf8(vec![0; 1024]).unwrap(), + // runtime_id: 0, + // max_block_size: u32::MAX, + // max_block_weight: Weight::MAX, + // bundle_slot_probability: (0, 0), + // target_bundles_per_block: 0, + // operator_allow_list: OperatorAllowList::Anyone, + // initial_balances: Default::default(), + // }; + + // let domain_obj = DomainObject { + // owner_account_id: 0, + // created_at: 0, + // genesis_receipt_hash: Default::default(), + // domain_config, + // domain_runtime_info: Default::default(), + // }; + + // DomainRegistry::::insert(new_domain_id, domain_obj); + + // DomainStakingSummary::::insert( + // new_domain_id, + // StakingSummary { + // current_epoch_index: 0, + // current_total_stake: 0, + // current_operators: BTreeMap::new(), + // next_operators: BTreeSet::new(), + // current_epoch_rewards: BTreeMap::new(), + // }, + // ); + + // let res = Domains::switch_domain( + // RuntimeOrigin::signed(operator_account), + // operator_id, + // new_domain_id, + // ); + // assert_ok!(res); + + // let old_domain_stake_summary = + // DomainStakingSummary::::get(old_domain_id).unwrap(); + // assert!(!old_domain_stake_summary + // .next_operators + // .contains(&operator_id)); + + // let new_domain_stake_summary = + // DomainStakingSummary::::get(new_domain_id).unwrap(); + // assert!(!new_domain_stake_summary + // .next_operators + // .contains(&operator_id)); + + // let operator = Operators::::get(operator_id).unwrap(); + // assert_eq!(operator.current_domain_id, old_domain_id); + // assert_eq!(operator.next_domain_id, new_domain_id); + // assert_eq!( + // PendingOperatorSwitches::::get(old_domain_id).unwrap(), + // BTreeSet::from_iter(vec![operator_id]) + // ); + + // let res = Domains::switch_domain( + // RuntimeOrigin::signed(operator_account), + // operator_id, + // new_domain_id, + // ); + // assert_err!( + // res, + // Error::::Staking(crate::staking::Error::PendingOperatorSwitch) + // ) + // }); + // } #[test] fn operator_deregistration() { @@ -1870,15 +1873,6 @@ pub(crate) mod tests { current_epoch_rewards: BTreeMap::new(), }, ); - let res = Domains::switch_domain( - RuntimeOrigin::signed(operator_account), - operator_id, - new_domain_id, - ); - assert_err!( - res, - Error::::Staking(crate::staking::Error::OperatorNotRegistered) - ); // nominations will not work since the is frozen let nominator_account = 100; diff --git a/crates/pallet-domains/src/staking_epoch.rs b/crates/pallet-domains/src/staking_epoch.rs index c240709a57..c1700dc346 100644 --- a/crates/pallet-domains/src/staking_epoch.rs +++ b/crates/pallet-domains/src/staking_epoch.rs @@ -489,106 +489,103 @@ pub(crate) fn do_finalize_slashed_operators( #[cfg(test)] mod tests { use crate::bundle_storage_fund::STORAGE_FEE_RESERVE; - use crate::domain_registry::{DomainConfig, DomainObject}; use crate::pallet::{ - Deposits, DomainRegistry, DomainStakingSummary, LastEpochStakingDistribution, - LatestConfirmedDomainBlock, NominatorCount, OperatorIdOwner, OperatorSigningKey, Operators, - PendingOperatorSwitches, Withdrawals, + Deposits, DomainStakingSummary, LastEpochStakingDistribution, LatestConfirmedDomainBlock, + NominatorCount, OperatorIdOwner, OperatorSigningKey, Operators, Withdrawals, }; use crate::staking::tests::{register_operator, Share}; use crate::staking::{ do_deregister_operator, do_nominate_operator, do_reward_operators, do_unlock_operator, - do_withdraw_stake, StakingSummary, + do_withdraw_stake, }; use crate::staking_epoch::{ - do_finalize_domain_current_epoch, do_finalize_switch_operator_domain, - operator_take_reward_tax_and_stake, + do_finalize_domain_current_epoch, operator_take_reward_tax_and_stake, }; - use crate::tests::{new_test_ext, RuntimeOrigin, Test}; + use crate::tests::{new_test_ext, Test}; use crate::{BalanceOf, Config, HoldIdentifier, NominatorId}; use frame_support::assert_ok; use frame_support::traits::fungible::InspectHold; - use frame_support::weights::Weight; use sp_core::{Pair, U256}; - use sp_domains::{ConfirmedDomainBlock, DomainId, OperatorAllowList, OperatorPair}; + use sp_domains::{ConfirmedDomainBlock, DomainId, OperatorPair}; use sp_runtime::traits::Zero; use sp_runtime::{PerThing, Percent}; - use std::collections::{BTreeMap, BTreeSet}; + use std::collections::BTreeMap; use subspace_runtime_primitives::SSC; type Balances = pallet_balances::Pallet; - type Domains = crate::Pallet; - #[test] - fn finalize_operator_domain_switch() { - let old_domain_id = DomainId::new(0); - let new_domain_id = DomainId::new(1); - let operator_account = 1; - let operator_free_balance = 200 * SSC; - let operator_stake = 100 * SSC; - let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - - let mut ext = new_test_ext(); - ext.execute_with(|| { - let (operator_id, _) = register_operator( - old_domain_id, - operator_account, - operator_free_balance, - operator_stake, - 100 * SSC, - pair.public(), - BTreeMap::new(), - ); - - let domain_config = DomainConfig { - domain_name: String::from_utf8(vec![0; 1024]).unwrap(), - runtime_id: 0, - max_block_size: u32::MAX, - max_block_weight: Weight::MAX, - bundle_slot_probability: (0, 0), - target_bundles_per_block: 0, - operator_allow_list: OperatorAllowList::Anyone, - initial_balances: Default::default(), - }; - - let domain_obj = DomainObject { - owner_account_id: 0, - created_at: 0, - genesis_receipt_hash: Default::default(), - domain_config, - domain_runtime_info: Default::default(), - }; - - DomainRegistry::::insert(new_domain_id, domain_obj); - - DomainStakingSummary::::insert( - new_domain_id, - StakingSummary { - current_epoch_index: 0, - current_total_stake: 0, - current_operators: BTreeMap::new(), - next_operators: BTreeSet::new(), - current_epoch_rewards: BTreeMap::new(), - }, - ); - let res = Domains::switch_domain( - RuntimeOrigin::signed(operator_account), - operator_id, - new_domain_id, - ); - assert_ok!(res); - - assert!(do_finalize_switch_operator_domain::(old_domain_id).is_ok()); - - let operator = Operators::::get(operator_id).unwrap(); - assert_eq!(operator.current_domain_id, new_domain_id); - assert_eq!(operator.next_domain_id, new_domain_id); - assert_eq!(PendingOperatorSwitches::::get(old_domain_id), None); - - let domain_stake_summary = DomainStakingSummary::::get(new_domain_id).unwrap(); - assert!(domain_stake_summary.next_operators.contains(&operator_id)); - }); - } + // TODO: `switch_domain` is not supported currently due to incompatible with lazily slashing + // enable this test when `switch_domain` is ready + // #[test] + // fn finalize_operator_domain_switch() { + // let old_domain_id = DomainId::new(0); + // let new_domain_id = DomainId::new(1); + // let operator_account = 1; + // let operator_free_balance = 200 * SSC; + // let operator_stake = 100 * SSC; + // let pair = OperatorPair::from_seed(&U256::from(0u32).into()); + + // let mut ext = new_test_ext(); + // ext.execute_with(|| { + // let (operator_id, _) = register_operator( + // old_domain_id, + // operator_account, + // operator_free_balance, + // operator_stake, + // 100 * SSC, + // pair.public(), + // BTreeMap::new(), + // ); + + // let domain_config = DomainConfig { + // domain_name: String::from_utf8(vec![0; 1024]).unwrap(), + // runtime_id: 0, + // max_block_size: u32::MAX, + // max_block_weight: Weight::MAX, + // bundle_slot_probability: (0, 0), + // target_bundles_per_block: 0, + // operator_allow_list: OperatorAllowList::Anyone, + // initial_balances: Default::default(), + // }; + + // let domain_obj = DomainObject { + // owner_account_id: 0, + // created_at: 0, + // genesis_receipt_hash: Default::default(), + // domain_config, + // domain_runtime_info: Default::default(), + // }; + + // DomainRegistry::::insert(new_domain_id, domain_obj); + + // DomainStakingSummary::::insert( + // new_domain_id, + // StakingSummary { + // current_epoch_index: 0, + // current_total_stake: 0, + // current_operators: BTreeMap::new(), + // next_operators: BTreeSet::new(), + // current_epoch_rewards: BTreeMap::new(), + // }, + // ); + // let res = Domains::switch_domain( + // RuntimeOrigin::signed(operator_account), + // operator_id, + // new_domain_id, + // ); + // assert_ok!(res); + + // assert!(do_finalize_switch_operator_domain::(old_domain_id).is_ok()); + + // let operator = Operators::::get(operator_id).unwrap(); + // assert_eq!(operator.current_domain_id, new_domain_id); + // assert_eq!(operator.next_domain_id, new_domain_id); + // assert_eq!(PendingOperatorSwitches::::get(old_domain_id), None); + + // let domain_stake_summary = DomainStakingSummary::::get(new_domain_id).unwrap(); + // assert!(domain_stake_summary.next_operators.contains(&operator_id)); + // }); + // } fn unlock_operator( nominators: Vec<(NominatorId, BalanceOf)>,