From 59c4be819112fd87bb434d692823264b60027aeb Mon Sep 17 00:00:00 2001 From: linning Date: Thu, 1 Aug 2024 05:06:54 +0800 Subject: [PATCH 1/3] Handle confirmed domain block within submit_receipt Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 1 - crates/pallet-domains/src/lib.rs | 44 ++++++++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index d4a5e01da6..d2f2f34212 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -51,7 +51,6 @@ pub enum Error { RuntimeNotFound, LastBlockNotFound, UnmatchedNewHeadReceipt, - UnexpectedConfirmedDomainBlock, } #[derive(TypeInfo, Debug, Encode, Decode, Clone, PartialEq, Eq)] diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index b7273e97db..5d23579a7d 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -1720,12 +1720,35 @@ mod pallet { ) .map_err(Error::::from)?; - // Singleton receipt are used to fill up the receipt gap and there should be no - // new domain block being confirmed before the gap is fill up - ensure!( - maybe_confirmed_domain_block_info.is_none(), - Error::::BlockTree(BlockTreeError::UnexpectedConfirmedDomainBlock), - ); + // NOTE: Skip the following staking related operations when benchmarking the + // `submit_receipt` call, these operations will be benchmarked separately. + #[cfg(not(feature = "runtime-benchmarks"))] + if let Some(confirmed_block_info) = maybe_confirmed_domain_block_info { + actual_weight = + actual_weight.saturating_add(T::WeightInfo::confirm_domain_block( + confirmed_block_info.operator_ids.len() as u32, + confirmed_block_info.invalid_bundle_authors.len() as u32, + )); + + refund_storage_fee::( + confirmed_block_info.total_storage_fee, + confirmed_block_info.paid_bundle_storage_fees, + ) + .map_err(Error::::from)?; + + do_reward_operators::( + domain_id, + confirmed_block_info.operator_ids.into_iter(), + confirmed_block_info.rewards, + ) + .map_err(Error::::from)?; + + do_mark_operators_as_slashed::( + confirmed_block_info.invalid_bundle_authors.into_iter(), + SlashedReason::InvalidBundle(confirmed_block_info.domain_block_number), + ) + .map_err(Error::::from)?; + } } } @@ -2750,7 +2773,7 @@ impl Pallet { .saturating_add( // NOTE: within `submit_bundle`, only one of (or none) `handle_bad_receipt` and // `confirm_domain_block` can happen, thus we use the `max` of them - + // // We use `MAX_BUNDLE_PER_BLOCK` number to assume the number of slashed operators. // We do not expect so many operators to be slashed but nontheless, if it did happen // we will limit the weight to 100 operators. @@ -2765,10 +2788,15 @@ impl Pallet { pub fn max_submit_receipt_weight() -> Weight { T::WeightInfo::submit_bundle() .saturating_add( + // NOTE: within `submit_bundle`, only one of (or none) `handle_bad_receipt` and + // `confirm_domain_block` can happen, thus we use the `max` of them + // // We use `MAX_BUNDLE_PER_BLOCK` number to assume the number of slashed operators. // We do not expect so many operators to be slashed but nontheless, if it did happen // we will limit the weight to 100 operators. - T::WeightInfo::handle_bad_receipt(MAX_BUNDLE_PER_BLOCK), + T::WeightInfo::handle_bad_receipt(MAX_BUNDLE_PER_BLOCK).max( + T::WeightInfo::confirm_domain_block(MAX_BUNDLE_PER_BLOCK, MAX_BUNDLE_PER_BLOCK), + ), ) .saturating_add(T::WeightInfo::slash_operator(MAX_NOMINATORS_TO_SLASH)) } From e6e7d4511b17fd3f9c40c7df02c924a46c3aac15 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 2 Aug 2024 19:07:25 +0800 Subject: [PATCH 2/3] Account the missed domain runtime upgrade into the domain best number onchain Signed-off-by: linning --- crates/pallet-domains/src/lib.rs | 32 ++++++++++++++----- crates/sp-domains/src/lib.rs | 2 +- crates/subspace-runtime/src/lib.rs | 2 +- .../src/domain_bundle_producer.rs | 8 ++--- .../src/domain_bundle_proposer.rs | 8 ++--- test/subspace-test-runtime/src/lib.rs | 2 +- 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 5d23579a7d..4a9d2e8554 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -654,6 +654,10 @@ mod pallet { /// successfully submitted to current consensus block, which mean a new domain block with this block /// number will be produce. Used as a pointer in `ExecutionInbox` to identify the current under building /// domain block, also used as a mapping of consensus block number to domain block number. + // + // NOTE: the `HeadDomainNumber` is lazily updated for the domain runtime upgrade block (which only include + // the runtime upgrade tx from the consensus chain and no any user submitted tx from the bundle), use + // `domain_best_number` for the actual best domain block #[pallet::storage] pub(super) type HeadDomainNumber = StorageMap<_, Identity, DomainId, DomainBlockNumberFor, ValueQuery>; @@ -763,6 +767,8 @@ mod pallet { UnexpectedReceiptGap, /// Expecting receipt gap when validating `submit_receipt` ExpectingReceiptGap, + /// Failed to get missed domain runtime upgrade count + FailedToGetMissedUpgradeCount, } #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)] @@ -2043,8 +2049,13 @@ impl Pallet { .and_then(|mut runtime_object| runtime_object.raw_genesis.take_runtime_code()) } - pub fn domain_best_number(domain_id: DomainId) -> Option> { - Some(HeadDomainNumber::::get(domain_id)) + pub fn domain_best_number(domain_id: DomainId) -> Result, BundleError> { + // The missed domain runtime upgrades will derive domain blocks thus should be accountted + // into the domain best number + let missed_upgrade = Self::missed_domain_runtime_upgrade(domain_id) + .map_err(|_| BundleError::FailedToGetMissedUpgradeCount)?; + + Ok(HeadDomainNumber::::get(domain_id) + missed_upgrade.into()) } pub fn runtime_id(domain_id: DomainId) -> Option { @@ -2288,7 +2299,7 @@ impl Pallet { // derived from the latest domain block, and the stale bundle (that verified against an old // domain block) produced by a lagging honest operator will be rejected. ensure!( - Self::receipt_gap(domain_id) <= One::one(), + Self::receipt_gap(domain_id)? <= One::one(), BundleError::UnexpectedReceiptGap, ); @@ -2324,7 +2335,7 @@ impl Pallet { // Singleton receipt is only allowed when there is a receipt gap ensure!( - Self::receipt_gap(domain_id) > One::one(), + Self::receipt_gap(domain_id)? > One::one(), BundleError::ExpectingReceiptGap, ); @@ -2698,6 +2709,11 @@ impl Pallet { // Start from the oldest non-confirmed ER to the head domain number let mut to_check = Self::latest_confirmed_domain_block_number(domain_id).saturating_add(One::one()); + + // NOTE: we use the `HeadDomainNumber` here instead of the `domain_best_number`, which include the + // missed domain runtime upgrade block, because we don't want to trigger empty bundle production + // for confirming these blocks since they only include runtime upgrade extrinsic and no any user + // submitted extrinsic. let head_number = HeadDomainNumber::::get(domain_id); while to_check <= head_number { @@ -2970,13 +2986,13 @@ impl Pallet { DomainSudoCalls::::get(domain_id).maybe_call } - // The gap between `HeadDomainNumber` and `HeadReceiptNumber` represent the number + // The gap between `domain_best_number` and `HeadReceiptNumber` represent the number // of receipt to be submitted - pub fn receipt_gap(domain_id: DomainId) -> DomainBlockNumberFor { - let head_domain_number = HeadDomainNumber::::get(domain_id); + pub fn receipt_gap(domain_id: DomainId) -> Result, BundleError> { + let domain_best_number = Self::domain_best_number(domain_id)?; let head_receipt_number = HeadReceiptNumber::::get(domain_id); - head_domain_number.saturating_sub(head_receipt_number) + Ok(domain_best_number.saturating_sub(head_receipt_number)) } } diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index b7014e2080..600c3a6987 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -780,7 +780,7 @@ impl ProofOfElection { } } -/// Singleton receipt submit along when there is a gap between `HeadDomainNumber` +/// Singleton receipt submit along when there is a gap between `domain_best_number` /// and `HeadReceiptNumber` #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] pub struct SingletonReceipt { diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index 6c372188e8..f5fd1ed3a2 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -1231,7 +1231,7 @@ impl_runtime_apis! { } fn domain_best_number(domain_id: DomainId) -> Option { - Domains::domain_best_number(domain_id) + Domains::domain_best_number(domain_id).ok() } fn execution_receipt(receipt_hash: DomainHash) -> Option> { diff --git a/domains/client/domain-operator/src/domain_bundle_producer.rs b/domains/client/domain-operator/src/domain_bundle_producer.rs index 2ebeb2c9f7..774f55ee55 100644 --- a/domains/client/domain-operator/src/domain_bundle_producer.rs +++ b/domains/client/domain-operator/src/domain_bundle_producer.rs @@ -183,7 +183,7 @@ where let domain_best_number = self.client.info().best_number; let consensus_chain_best_hash = self.consensus_client.info().best_hash; - let head_domain_number = self + let domain_best_number_onchain = self .consensus_client .runtime_api() .domain_best_number(consensus_chain_best_hash, self.domain_id)? @@ -239,7 +239,7 @@ where let receipt = self .domain_bundle_proposer - .load_next_receipt(head_domain_number, head_receipt_number)?; + .load_next_receipt(domain_best_number_onchain, head_receipt_number)?; let api_version = self .consensus_client @@ -256,10 +256,10 @@ where // instead of bundle // TODO: remove api runtime version check before next network if api_version >= 5 - && head_domain_number.saturating_sub(head_receipt_number) > 1u32.into() + && domain_best_number_onchain.saturating_sub(head_receipt_number) > 1u32.into() { info!( - ?head_domain_number, + ?domain_best_number_onchain, ?head_receipt_number, "🔖 Producing singleton receipt at slot {:?}", slot_info.slot diff --git a/domains/client/domain-operator/src/domain_bundle_proposer.rs b/domains/client/domain-operator/src/domain_bundle_proposer.rs index 3463559f3b..e6c05e7d09 100644 --- a/domains/client/domain-operator/src/domain_bundle_proposer.rs +++ b/domains/client/domain-operator/src/domain_bundle_proposer.rs @@ -397,18 +397,18 @@ where /// Returns the receipt in the next domain bundle. pub fn load_next_receipt( &self, - head_domain_number: NumberFor, + domain_best_number_onchain: NumberFor, head_receipt_number: NumberFor, ) -> sp_blockchain::Result> { tracing::trace!( - ?head_domain_number, + ?domain_best_number_onchain, ?head_receipt_number, "Collecting receipt" ); - // Both `head_domain_number` and `head_receipt_number` are zero means the domain just + // Both `domain_best_number_onchain` and `head_receipt_number` are zero means the domain just // instantiated and nothing have submitted yet so submit the genesis receipt - if head_domain_number.is_zero() && head_receipt_number.is_zero() { + if domain_best_number_onchain.is_zero() && head_receipt_number.is_zero() { let genesis_hash = self.client.info().genesis_hash; let genesis_header = self.client.header(genesis_hash)?.ok_or_else(|| { sp_blockchain::Error::Backend(format!( diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index 04b4877af2..1aa72cc470 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1428,7 +1428,7 @@ impl_runtime_apis! { } fn domain_best_number(domain_id: DomainId) -> Option { - Domains::domain_best_number(domain_id) + Domains::domain_best_number(domain_id).ok() } fn execution_receipt(receipt_hash: DomainHash) -> Option> { From 9f7225b824c41817fa978d2dae2dba8f9a419706 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 2 Aug 2024 19:08:01 +0800 Subject: [PATCH 3/3] Bump consensus runtime spec version Signed-off-by: linning --- crates/subspace-runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index f5fd1ed3a2..6c64505e67 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -118,7 +118,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("subspace"), impl_name: create_runtime_str!("subspace"), authoring_version: 0, - spec_version: 6, + spec_version: 7, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 0,