From cc42220fd23ccb28e3294b5a61c765ca635ef887 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 21:04:23 +0800 Subject: [PATCH 1/8] Pure refactoring to pallet-domains This is for better code reusability in the upcoming commit Signed-off-by: linning --- .../pallet-domains/src/bundle_storage_fund.rs | 12 +- crates/pallet-domains/src/lib.rs | 200 ++++++++++-------- 2 files changed, 118 insertions(+), 94 deletions(-) diff --git a/crates/pallet-domains/src/bundle_storage_fund.rs b/crates/pallet-domains/src/bundle_storage_fund.rs index 1e0fa6b1de..2cf2c1f7d2 100644 --- a/crates/pallet-domains/src/bundle_storage_fund.rs +++ b/crates/pallet-domains/src/bundle_storage_fund.rs @@ -76,14 +76,20 @@ pub fn charge_bundle_storage_fee( let storage_fund_acc = storage_fund_account::(operator_id); let storage_fee = T::StorageFee::transaction_byte_fee() * bundle_size.into(); - T::Currency::burn_from( + if let Err(err) = T::Currency::burn_from( &storage_fund_acc, storage_fee, Preservation::Expendable, Precision::Exact, Fortitude::Polite, - ) - .map_err(|_| Error::BundleStorageFeePayment)?; + ) { + let total_balance = total_balance::(operator_id); + log::debug!( + target: "runtime::domains", + "Operator {operator_id:?} unable to pay for the bundle storage fee {storage_fee:?}, storage fund total balance {total_balance:?}, err {err:?}", + ); + return Err(Error::BundleStorageFeePayment); + } // Note the storage fee, it will go to the consensus block author T::StorageFee::note_storage_fees(storage_fee); diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 988499144f..33b3559a19 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -36,8 +36,8 @@ pub mod weights; extern crate alloc; use crate::block_tree::{verify_execution_receipt, Error as BlockTreeError}; -use crate::bundle_storage_fund::storage_fund_account; -use crate::domain_registry::Error as DomainRegistryError; +use crate::bundle_storage_fund::{charge_bundle_storage_fee, storage_fund_account}; +use crate::domain_registry::{DomainConfig, Error as DomainRegistryError}; use crate::runtime_registry::into_complete_raw_genesis; #[cfg(feature = "runtime-benchmarks")] pub use crate::staking::do_register_operator; @@ -175,7 +175,7 @@ mod pallet { }; #[cfg(not(feature = "runtime-benchmarks"))] use crate::bundle_storage_fund::refund_storage_fee; - use crate::bundle_storage_fund::{charge_bundle_storage_fee, Error as BundleStorageFundError}; + use crate::bundle_storage_fund::Error as BundleStorageFundError; use crate::domain_registry::{ do_instantiate_domain, do_update_domain_allow_list, DomainConfig, DomainObject, Error as DomainRegistryError, @@ -749,6 +749,8 @@ mod pallet { EquivocatedBundle, /// Domain is frozen and cannot accept new bundles DomainFrozen, + /// The operator's bundle storage fund unable to pay the storage fee + UnableToPayBundleStorageFee, } #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)] @@ -1770,15 +1772,10 @@ mod pallet { type Call = Call; fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { match call { - Call::submit_bundle { opaque_bundle } => Self::validate_bundle(opaque_bundle, true) - .map_err(|_| InvalidTransaction::Call.into()) - .and_then(|_| { - charge_bundle_storage_fee::( - opaque_bundle.operator_id(), - opaque_bundle.size(), - ) + Call::submit_bundle { opaque_bundle } => { + Self::validate_submit_bundle(opaque_bundle, true) .map_err(|_| InvalidTransaction::Call.into()) - }), + } Call::submit_fraud_proof { fraud_proof } => Self::validate_fraud_proof(fraud_proof) .map(|_| ()) .map_err(|_| InvalidTransaction::Call.into()), @@ -1789,54 +1786,21 @@ mod pallet { fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { match call { Call::submit_bundle { opaque_bundle } => { - if let Err(e) = Self::validate_bundle(opaque_bundle, false) { - match e { - // These errors are common due to networking delay or chain re-org, - // using a lower log level to avoid the noise. - BundleError::Receipt(BlockTreeError::InFutureReceipt) - | BundleError::Receipt(BlockTreeError::StaleReceipt) - | BundleError::Receipt(BlockTreeError::NewBranchReceipt) - | BundleError::Receipt(BlockTreeError::UnavailableConsensusBlockHash) - | BundleError::Receipt(BlockTreeError::BuiltOnUnknownConsensusBlock) - | BundleError::SlotInThePast - | BundleError::SlotInTheFuture - | BundleError::InvalidProofOfTime - | BundleError::SlotSmallerThanPreviousBlockBundle => { - log::debug!( - target: "runtime::domains", - "Bad bundle {:?}, error: {e:?}", opaque_bundle.domain_id(), - ); - } - _ => { - log::warn!( - target: "runtime::domains", - "Bad bundle {:?}, operator {}, error: {e:?}", - opaque_bundle.domain_id(), - opaque_bundle.operator_id(), - ); - } - } - if let BundleError::Receipt(_) = e { + let domain_id = opaque_bundle.domain_id(); + let operator_id = opaque_bundle.operator_id(); + let slot_number = opaque_bundle.slot_number(); + + if let Err(e) = Self::validate_submit_bundle(opaque_bundle, false) { + Self::log_bundle_error(&e, domain_id, operator_id); + if BundleError::UnableToPayBundleStorageFee == e { + return InvalidTransactionCode::BundleStorageFeePayment.into(); + } else if let BundleError::Receipt(_) = e { return InvalidTransactionCode::ExecutionReceipt.into(); } else { return InvalidTransactionCode::Bundle.into(); } } - if let Err(e) = charge_bundle_storage_fee::( - opaque_bundle.operator_id(), - opaque_bundle.size(), - ) { - log::debug!( - target: "runtime::domains", - "Operator {} unable to pay for the bundle storage fee, domain id {:?}, error: {e:?}", - opaque_bundle.operator_id(), - opaque_bundle.domain_id(), - ); - return InvalidTransactionCode::BundleStorageFeePayment.into(); - } - - let tag = (opaque_bundle.operator_id(), opaque_bundle.slot_number()); ValidTransaction::with_tag_prefix("SubspaceSubmitBundle") // Bundle have a bit higher priority than normal extrinsic but must less than // fraud proof @@ -1844,7 +1808,7 @@ mod pallet { .longevity(T::ConfirmationDepthK::get().try_into().unwrap_or_else(|_| { panic!("Block number always fits in TransactionLongevity; qed") })) - .and_provides(tag) + .and_provides((operator_id, slot_number)) .propagate(true) .build() } @@ -1876,6 +1840,33 @@ mod pallet { } impl Pallet { + fn log_bundle_error(err: &BundleError, domain_id: DomainId, operator_id: OperatorId) { + match err { + // These errors are common due to networking delay or chain re-org, + // using a lower log level to avoid the noise. + BundleError::Receipt(BlockTreeError::InFutureReceipt) + | BundleError::Receipt(BlockTreeError::StaleReceipt) + | BundleError::Receipt(BlockTreeError::NewBranchReceipt) + | BundleError::Receipt(BlockTreeError::UnavailableConsensusBlockHash) + | BundleError::Receipt(BlockTreeError::BuiltOnUnknownConsensusBlock) + | BundleError::SlotInThePast + | BundleError::SlotInTheFuture + | BundleError::InvalidProofOfTime + | BundleError::SlotSmallerThanPreviousBlockBundle => { + log::debug!( + target: "runtime::domains", + "Bad bundle, domain {domain_id:?}, operator {operator_id:?}, error: {err:?}", + ); + } + _ => { + log::warn!( + target: "runtime::domains", + "Bad bundle, domain {domain_id:?}, operator {operator_id:?}, error: {err:?}", + ); + } + } + } + pub fn successful_bundles(domain_id: DomainId) -> Vec { SuccessfulBundles::::get(domain_id) } @@ -2031,22 +2022,47 @@ impl Pallet { Ok(()) } - // TODO: as bundle equivocation fraud proof is removed, add check to rejected bundle with the - // same `(operator_id, slot)` fn validate_bundle( opaque_bundle: &OpaqueBundleOf, + domain_config: &DomainConfig>, + ) -> Result<(), BundleError> { + let domain_bundle_limit = domain_config + .calculate_bundle_limit::() + .map_err(|_| BundleError::UnableToCalculateBundleLimit)?; + + ensure!( + opaque_bundle.body_size() <= domain_bundle_limit.max_bundle_size, + BundleError::BundleTooLarge + ); + + ensure!( + opaque_bundle + .estimated_weight() + .all_lte(domain_bundle_limit.max_bundle_weight), + BundleError::BundleTooHeavy + ); + + Self::check_extrinsics_root(opaque_bundle)?; + + Ok(()) + } + + fn validate_eligibility( + to_sign: &[u8], + signature: &OperatorSignature, + proof_of_election: &ProofOfElection, + domain_config: &DomainConfig>, pre_dispatch: bool, ) -> Result<(), BundleError> { - let domain_id = opaque_bundle.domain_id(); + let domain_id = proof_of_election.domain_id; + let operator_id = proof_of_election.operator_id; + let slot_number = proof_of_election.slot_number; + ensure!( !FrozenDomains::::get().contains(&domain_id), BundleError::DomainFrozen ); - let operator_id = opaque_bundle.operator_id(); - let sealed_header = &opaque_bundle.sealed_header; - let slot_number = opaque_bundle.slot_number(); - let operator = Operators::::get(operator_id).ok_or(BundleError::InvalidOperatorId)?; let operator_status = operator.status::(operator_id); @@ -2056,10 +2072,7 @@ impl Pallet { BundleError::BadOperator ); - if !operator - .signing_key - .verify(&sealed_header.pre_hash(), &sealed_header.signature) - { + if !operator.signing_key.verify(&to_sign, signature) { return Err(BundleError::BadBundleSignature); } @@ -2076,34 +2089,11 @@ impl Pallet { BundleError::EquivocatedBundle, ); - let domain_config = DomainRegistry::::get(domain_id) - .ok_or(BundleError::InvalidDomainId)? - .domain_config; - - let domain_bundle_limit = domain_config - .calculate_bundle_limit::() - .map_err(|_| BundleError::UnableToCalculateBundleLimit)?; - - ensure!( - opaque_bundle.body_size() <= domain_bundle_limit.max_bundle_size, - BundleError::BundleTooLarge - ); - - ensure!( - opaque_bundle - .estimated_weight() - .all_lte(domain_bundle_limit.max_bundle_weight), - BundleError::BundleTooHeavy - ); - - Self::check_extrinsics_root(opaque_bundle)?; - - let proof_of_election = &sealed_header.header.proof_of_election; let (operator_stake, total_domain_stake) = Self::fetch_operator_stake_info(domain_id, &operator_id)?; Self::check_slot_and_proof_of_time( - proof_of_election.slot_number, + slot_number, proof_of_election.proof_of_time, pre_dispatch, )?; @@ -2116,8 +2106,36 @@ impl Pallet { total_domain_stake.saturated_into(), )?; - let receipt = &sealed_header.header.receipt; - verify_execution_receipt::(domain_id, receipt).map_err(BundleError::Receipt)?; + Ok(()) + } + + fn validate_submit_bundle( + opaque_bundle: &OpaqueBundleOf, + pre_dispatch: bool, + ) -> Result<(), BundleError> { + let domain_id = opaque_bundle.domain_id(); + let operator_id = opaque_bundle.operator_id(); + let sealed_header = &opaque_bundle.sealed_header; + + let domain_config = DomainRegistry::::get(domain_id) + .ok_or(BundleError::InvalidDomainId)? + .domain_config; + + Self::validate_bundle(opaque_bundle, &domain_config)?; + + Self::validate_eligibility( + sealed_header.pre_hash().as_ref(), + &sealed_header.signature, + &sealed_header.header.proof_of_election, + &domain_config, + pre_dispatch, + )?; + + verify_execution_receipt::(domain_id, &sealed_header.header.receipt) + .map_err(BundleError::Receipt)?; + + charge_bundle_storage_fee::(operator_id, opaque_bundle.size()) + .map_err(|_| BundleError::UnableToPayBundleStorageFee)?; Ok(()) } From 2055ad8c68bcb9d6a9a7cc5762977e7e450b7307 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:11:15 +0800 Subject: [PATCH 2/8] Add SingletonReceipt struct defination and helper functions It is used to submit receipt to fill up the gap between HeadDomainNumber and HeadReceiptNumber, it consist of the receipt, proof_of_election, and the signature. Signed-off-by: linning --- crates/sp-domains/src/lib.rs | 91 ++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index a6eaf713f3..4a9156cd9d 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -754,6 +754,10 @@ impl ProofOfElection { bytes.append(&mut self.vrf_signature.proof.encode()); blake3_hash(&bytes) } + + pub fn slot_number(&self) -> u64 { + self.slot_number + } } impl ProofOfElection { @@ -776,6 +780,93 @@ impl ProofOfElection { } } +/// Singleton receipt submit along when there is a gap between `HeadDomainNumber` +/// and `HeadReceiptNumber` +#[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] +pub struct SingletonReceipt { + /// Proof of receipt producer election. + pub proof_of_election: ProofOfElection, + /// The receipt to submit + pub receipt: ExecutionReceipt< + Number, + Hash, + HeaderNumberFor, + HeaderHashFor, + Balance, + >, +} + +impl + SingletonReceipt +{ + pub fn hash(&self) -> HeaderHashFor { + HeaderHashingFor::::hash_of(&self) + } +} + +#[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] +pub struct SealedSingletonReceipt { + /// A collection of the receipt. + pub singleton_receipt: SingletonReceipt, + /// Signature of the receipt bundle. + pub signature: OperatorSignature, +} + +impl + SealedSingletonReceipt +{ + /// Returns the `domain_id` + pub fn domain_id(&self) -> DomainId { + self.singleton_receipt.proof_of_election.domain_id + } + + /// Return the `operator_id` + pub fn operator_id(&self) -> OperatorId { + self.singleton_receipt.proof_of_election.operator_id + } + + /// Return the `slot_number` of the `proof_of_election` + pub fn slot_number(&self) -> u64 { + self.singleton_receipt.proof_of_election.slot_number + } + + /// Return the receipt + pub fn receipt( + &self, + ) -> &ExecutionReceipt< + Number, + Hash, + HeaderNumberFor, + HeaderHashFor, + Balance, + > { + &self.singleton_receipt.receipt + } + + /// Consume this `SealedSingletonReceipt` and return the receipt + pub fn into_receipt( + self, + ) -> ExecutionReceipt< + Number, + Hash, + HeaderNumberFor, + HeaderHashFor, + Balance, + > { + self.singleton_receipt.receipt + } + + /// Returns the hash of `SingletonReceipt` + pub fn pre_hash(&self) -> HeaderHashFor { + HeaderHashingFor::::hash_of(&self.singleton_receipt) + } + + /// Return the encode size of `SealedSingletonReceipt` + pub fn size(&self) -> u32 { + self.encoded_size() as u32 + } +} + /// Type that represents an operator allow list for Domains. #[derive(TypeInfo, Debug, Encode, Decode, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum OperatorAllowList { From 56b1d2712242a52986f72aff113774c73de96eca Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:18:41 +0800 Subject: [PATCH 3/8] Introduce the submit_receipt extrinsic Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 1 + crates/pallet-domains/src/lib.rs | 207 +++++++++++++++++++- crates/sp-domains/src/lib.rs | 3 + crates/subspace-fake-runtime-api/src/lib.rs | 6 + crates/subspace-runtime/src/lib.rs | 6 + test/subspace-test-runtime/src/lib.rs | 6 + 6 files changed, 224 insertions(+), 5 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 993bb5f63d..1bcfc9c49c 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -49,6 +49,7 @@ pub enum Error { OverwritingER, RuntimeNotFound, LastBlockNotFound, + 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 33b3559a19..72e69bd6fa 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -66,8 +66,8 @@ use sp_core::H256; use sp_domains::bundle_producer_election::BundleProducerElectionParams; use sp_domains::{ DomainBlockLimit, DomainBundleLimit, DomainId, DomainInstanceData, ExecutionReceipt, - OpaqueBundle, OperatorId, OperatorPublicKey, RuntimeId, - DOMAIN_EXTRINSICS_SHUFFLING_SEED_SUBJECT, EMPTY_EXTRINSIC_ROOT, + OpaqueBundle, OperatorId, OperatorPublicKey, OperatorSignature, ProofOfElection, RuntimeId, + SealedSingletonReceipt, DOMAIN_EXTRINSICS_SHUFFLING_SEED_SUBJECT, EMPTY_EXTRINSIC_ROOT, }; use sp_domains_fraud_proof::fraud_proof::{ DomainRuntimeCodeAt, FraudProof, FraudProofVariant, InvalidBlockFeesProof, @@ -126,6 +126,13 @@ pub type OpaqueBundleOf = OpaqueBundle< BalanceOf, >; +pub type SingletonReceiptOf = SealedSingletonReceipt< + BlockNumberFor, + ::Hash, + ::DomainHeader, + BalanceOf, +>; + pub type FraudProofFor = FraudProof< BlockNumberFor, ::Hash, @@ -204,7 +211,7 @@ mod pallet { use crate::{ BalanceOf, BlockSlot, BlockTreeNodeFor, DomainBlockNumberFor, ElectionVerificationParams, ExecutionReceiptOf, FraudProofFor, HoldIdentifier, NominatorId, OpaqueBundleOf, - ReceiptHashFor, StateRootOf, MAX_BUNLDE_PER_BLOCK, STORAGE_VERSION, + ReceiptHashFor, SingletonReceiptOf, StateRootOf, MAX_BUNLDE_PER_BLOCK, STORAGE_VERSION, }; #[cfg(not(feature = "std"))] use alloc::string::String; @@ -751,6 +758,10 @@ mod pallet { DomainFrozen, /// The operator's bundle storage fund unable to pay the storage fee UnableToPayBundleStorageFee, + /// Unexpected receipt gap when validating `submit_bundle` + UnexpectedReceiptGap, + /// Expecting receipt gap when validating `submit_receipt` + ExpectingReceiptGap, } #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)] @@ -1631,6 +1642,89 @@ mod pallet { Ok(Some(actual_weight).into()) } + + // FIXME: recorrect weight + #[pallet::call_index(20)] + #[pallet::weight(Pallet::::max_submit_bundle_weight())] + pub fn submit_receipt( + origin: OriginFor, + singleton_receipt: SingletonReceiptOf, + ) -> DispatchResultWithPostInfo { + ensure_none(origin)?; + + let domain_id = singleton_receipt.domain_id(); + let operator_id = singleton_receipt.operator_id(); + let receipt = singleton_receipt.into_receipt(); + + #[cfg(not(feature = "runtime-benchmarks"))] + let mut actual_weight = T::WeightInfo::submit_receipt(); + #[cfg(feature = "runtime-benchmarks")] + let actual_weight = T::WeightInfo::submit_receipt(); + + match execution_receipt_type::(domain_id, &receipt) { + ReceiptType::Rejected(rejected_receipt_type) => { + return Err(Error::::BlockTree(rejected_receipt_type.into()).into()); + } + // 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. + // + // 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 accepted_receipt_type == AcceptedReceiptType::NewHead { + if let Some(block_tree_node) = + prune_receipt::(domain_id, receipt.domain_block_number) + .map_err(Error::::from)? + { + actual_weight = + actual_weight.saturating_add(T::WeightInfo::handle_bad_receipt( + block_tree_node.operator_ids.len() as u32, + )); + + let bad_receipt_hash = block_tree_node + .execution_receipt + .hash::>(); + do_mark_operators_as_slashed::( + 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, + operator_id, + receipt, + accepted_receipt_type, + ) + .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), + ); + } + } + + // slash operator who are in pending slash + #[cfg(not(feature = "runtime-benchmarks"))] + { + let slashed_nominator_count = + do_slash_operator::(domain_id, MAX_NOMINATORS_TO_SLASH) + .map_err(Error::::from)?; + actual_weight = actual_weight + .saturating_add(T::WeightInfo::slash_operator(slashed_nominator_count)); + } + + // Ensure the returned weight not exceed the maximum weight in the `pallet::weight` + Ok(Some(actual_weight.min(Self::max_submit_bundle_weight())).into()) + } } #[pallet::genesis_config] @@ -1779,6 +1873,10 @@ mod pallet { Call::submit_fraud_proof { fraud_proof } => Self::validate_fraud_proof(fraud_proof) .map(|_| ()) .map_err(|_| InvalidTransaction::Call.into()), + Call::submit_receipt { singleton_receipt } => { + Self::validate_singleton_receipt(singleton_receipt, true) + .map_err(|_| InvalidTransaction::Call.into()) + } _ => Err(InvalidTransaction::Call.into()), } } @@ -1832,6 +1930,33 @@ mod pallet { .propagate(true) .build() } + Call::submit_receipt { singleton_receipt } => { + let domain_id = singleton_receipt.domain_id(); + let operator_id = singleton_receipt.operator_id(); + let slot_number = singleton_receipt.slot_number(); + + if let Err(e) = Self::validate_singleton_receipt(singleton_receipt, false) { + Self::log_bundle_error(&e, domain_id, operator_id); + if BundleError::UnableToPayBundleStorageFee == e { + return InvalidTransactionCode::BundleStorageFeePayment.into(); + } else if let BundleError::Receipt(_) = e { + return InvalidTransactionCode::ExecutionReceipt.into(); + } else { + return InvalidTransactionCode::Bundle.into(); + } + } + + ValidTransaction::with_tag_prefix("SubspaceSubmitReceipt") + // Receipt have a bit higher priority than normal extrinsic but must less than + // fraud proof + .priority(1) + .longevity(T::ConfirmationDepthK::get().try_into().unwrap_or_else(|_| { + panic!("Block number always fits in TransactionLongevity; qed") + })) + .and_provides((operator_id, slot_number)) + .propagate(true) + .build() + } _ => InvalidTransaction::Call.into(), } @@ -1855,13 +1980,13 @@ impl Pallet { | BundleError::SlotSmallerThanPreviousBlockBundle => { log::debug!( target: "runtime::domains", - "Bad bundle, domain {domain_id:?}, operator {operator_id:?}, error: {err:?}", + "Bad bundle/receipt, domain {domain_id:?}, operator {operator_id:?}, error: {err:?}", ); } _ => { log::warn!( target: "runtime::domains", - "Bad bundle, domain {domain_id:?}, operator {operator_id:?}, error: {err:?}", + "Bad bundle/receipt, domain {domain_id:?}, operator {operator_id:?}, error: {err:?}", ); } } @@ -2117,6 +2242,14 @@ impl Pallet { let operator_id = opaque_bundle.operator_id(); let sealed_header = &opaque_bundle.sealed_header; + // Ensure the receipt gap is <= 1 so that the bundle will only be acceptted if its receipt is + // 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(), + BundleError::UnexpectedReceiptGap, + ); + let domain_config = DomainRegistry::::get(domain_id) .ok_or(BundleError::InvalidDomainId)? .domain_config; @@ -2140,6 +2273,42 @@ impl Pallet { Ok(()) } + fn validate_singleton_receipt( + sealed_singleton_receipt: &SingletonReceiptOf, + pre_dispatch: bool, + ) -> Result<(), BundleError> { + let domain_id = sealed_singleton_receipt.domain_id(); + let operator_id = sealed_singleton_receipt.operator_id(); + + // Singleton receipt is only allowed when there is a receipt gap + ensure!( + Self::receipt_gap(domain_id) > One::one(), + BundleError::ExpectingReceiptGap, + ); + + let domain_config = DomainRegistry::::get(domain_id) + .ok_or(BundleError::InvalidDomainId)? + .domain_config; + Self::validate_eligibility( + sealed_singleton_receipt.pre_hash().as_ref(), + &sealed_singleton_receipt.signature, + &sealed_singleton_receipt.singleton_receipt.proof_of_election, + &domain_config, + pre_dispatch, + )?; + + verify_execution_receipt::( + domain_id, + &sealed_singleton_receipt.singleton_receipt.receipt, + ) + .map_err(BundleError::Receipt)?; + + charge_bundle_storage_fee::(operator_id, sealed_singleton_receipt.size()) + .map_err(|_| BundleError::UnableToPayBundleStorageFee)?; + + Ok(()) + } + fn validate_fraud_proof( fraud_proof: &FraudProofFor, ) -> Result<(DomainId, TransactionPriority), FraudProofError> { @@ -2747,6 +2916,15 @@ impl Pallet { pub fn domain_sudo_call(domain_id: DomainId) -> Option> { DomainSudoCalls::::get(domain_id).maybe_call } + + // The gap between `HeadDomainNumber` 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); + let head_receipt_number = HeadReceiptNumber::::get(domain_id); + + head_domain_number.saturating_sub(head_receipt_number) + } } impl sp_domains::DomainOwner for Pallet { @@ -2783,6 +2961,25 @@ where } } + /// Submits an unsigned extrinsic [`Call::submit_receipt`]. + pub fn submit_receipt_unsigned(singleton_receipt: SingletonReceiptOf) { + let slot = singleton_receipt.slot_number(); + let domain_block_number = singleton_receipt.receipt().domain_block_number; + + let call = Call::submit_receipt { singleton_receipt }; + match SubmitTransaction::>::submit_unsigned_transaction(call.into()) { + Ok(()) => { + log::info!( + target: "runtime::domains", + "Submitted singleton receipt from slot {slot}, domain_block_number: {domain_block_number:?}", + ); + } + Err(()) => { + log::error!(target: "runtime::domains", "Error submitting singleton receipt"); + } + } + } + /// Submits an unsigned extrinsic [`Call::submit_fraud_proof`]. pub fn submit_fraud_proof_unsigned(fraud_proof: FraudProofFor) { let call = Call::submit_fraud_proof { diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index 4a9156cd9d..2630118274 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -1511,6 +1511,9 @@ sp_api::decl_runtime_apis! { /// Submits the transaction bundle via an unsigned extrinsic. fn submit_bundle_unsigned(opaque_bundle: OpaqueBundle, Block::Hash, DomainHeader, Balance>); + // Submit singleton receipt via an unsigned extrinsic. + fn submit_receipt_unsigned(singleton_receipt: SealedSingletonReceipt, Block::Hash, DomainHeader, Balance>); + /// Extract the bundles stored successfully from the given extrinsics. fn extract_successful_bundles( domain_id: DomainId, diff --git a/crates/subspace-fake-runtime-api/src/lib.rs b/crates/subspace-fake-runtime-api/src/lib.rs index 76076b930c..74028f9344 100644 --- a/crates/subspace-fake-runtime-api/src/lib.rs +++ b/crates/subspace-fake-runtime-api/src/lib.rs @@ -199,6 +199,12 @@ sp_api::impl_runtime_apis! { unreachable!() } + fn submit_receipt_unsigned( + _singleton_receipt: sp_domains::SealedSingletonReceipt, ::Hash, DomainHeader, Balance>, + ) { + unreachable!() + } + fn extract_successful_bundles( _domain_id: DomainId, _extrinsics: Vec<::Extrinsic>, diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index b11e3e2590..3ddcbb76e7 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -1153,6 +1153,12 @@ impl_runtime_apis! { Domains::submit_bundle_unsigned(opaque_bundle) } + fn submit_receipt_unsigned( + singleton_receipt: sp_domains::SealedSingletonReceipt, ::Hash, DomainHeader, Balance>, + ) { + Domains::submit_receipt_unsigned(singleton_receipt) + } + fn extract_successful_bundles( domain_id: DomainId, extrinsics: Vec<::Extrinsic>, diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index 01a42e71e3..b9aa640332 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1349,6 +1349,12 @@ impl_runtime_apis! { Domains::submit_bundle_unsigned(opaque_bundle) } + fn submit_receipt_unsigned( + singleton_receipt: sp_domains::SealedSingletonReceipt, ::Hash, DomainHeader, Balance>, + ) { + Domains::submit_receipt_unsigned(singleton_receipt) + } + fn extract_successful_bundles( domain_id: DomainId, extrinsics: Vec<::Extrinsic>, From ed3a331bd8c1d7de56871832a1e50b3c275803ed Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:21:07 +0800 Subject: [PATCH 4/8] Add benchmark and weight for submit_receipt Signed-off-by: linning --- crates/pallet-domains/src/benchmarking.rs | 36 ++++++++++++++++++- crates/pallet-domains/src/lib.rs | 23 +++++++----- crates/pallet-domains/src/weights.rs | 43 +++++++++++++++++++++++ 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index 511e4ee828..105e98b63a 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -29,7 +29,8 @@ use sp_core::crypto::{Ss58Codec, UncheckedFrom}; use sp_core::ByteArray; use sp_domains::{ dummy_opaque_bundle, BlockFees, DomainId, ExecutionReceipt, OperatorAllowList, OperatorId, - OperatorPublicKey, OperatorSignature, PermissionedActionAllowedBy, RuntimeType, Transfers, + OperatorPublicKey, OperatorSignature, PermissionedActionAllowedBy, ProofOfElection, + RuntimeType, SealedSingletonReceipt, SingletonReceipt, Transfers, }; use sp_domains_fraud_proof::fraud_proof::FraudProof; use sp_runtime::traits::{CheckedAdd, One, Zero}; @@ -860,6 +861,36 @@ mod benchmarks { assert_eq!(domain_obj.domain_config.operator_allow_list, new_allow_list); } + #[benchmark] + fn submit_receipt() { + let domain_id = register_domain::(); + let (_, operator_id) = + register_helper_operator::(domain_id, T::MinNominatorStake::get()); + + assert_eq!(Domains::::head_receipt_number(domain_id), 0u32.into()); + + let receipt = { + let mut er = BlockTree::::get::<_, DomainBlockNumberFor>(domain_id, Zero::zero()) + .and_then(BlockTreeNodes::::get) + .expect("genesis receipt must exist") + .execution_receipt; + er.domain_block_number = One::one(); + er + }; + let sealed_singleton_receipt = SealedSingletonReceipt { + singleton_receipt: SingletonReceipt { + proof_of_election: ProofOfElection::dummy(domain_id, operator_id), + receipt, + }, + signature: OperatorSignature::unchecked_from([0u8; 64]), + }; + + #[extrinsic_call] + submit_receipt(RawOrigin::None, sealed_singleton_receipt); + + assert_eq!(Domains::::head_receipt_number(domain_id), 1u32.into()); + } + fn register_runtime() -> RuntimeId { let genesis_storage = include_bytes!("../res/evm-domain-genesis-storage").to_vec(); let runtime_id = NextRuntimeId::::get(); @@ -961,6 +992,9 @@ mod benchmarks { } fn run_to_block(block_number: BlockNumberFor, parent_hash: T::Hash) { + if let Some(parent_block_number) = block_number.checked_sub(&One::one()) { + as Hooks>>::on_finalize(parent_block_number); + } System::::set_block_number(block_number); System::::initialize(&block_number, &parent_hash, &Default::default()); as Hooks>>::on_initialize(block_number); diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 72e69bd6fa..3c00203373 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -1193,7 +1193,7 @@ mod pallet { do_slash_operator::(domain_id, MAX_NOMINATORS_TO_SLASH) .map_err(Error::::from)?; actual_weight = actual_weight - .saturating_add(Self::actual_slash_operator_weight(slashed_nominator_count)); + .saturating_add(T::WeightInfo::slash_operator(slashed_nominator_count)); } Self::deposit_event(Event::BundleStored { @@ -1643,9 +1643,8 @@ mod pallet { Ok(Some(actual_weight).into()) } - // FIXME: recorrect weight #[pallet::call_index(20)] - #[pallet::weight(Pallet::::max_submit_bundle_weight())] + #[pallet::weight(Pallet::::max_submit_receipt_weight())] pub fn submit_receipt( origin: OriginFor, singleton_receipt: SingletonReceiptOf, @@ -1723,7 +1722,7 @@ mod pallet { } // Ensure the returned weight not exceed the maximum weight in the `pallet::weight` - Ok(Some(actual_weight.min(Self::max_submit_bundle_weight())).into()) + Ok(Some(actual_weight.min(Self::max_submit_receipt_weight())).into()) } } @@ -2743,6 +2742,17 @@ impl Pallet { .saturating_add(T::WeightInfo::slash_operator(MAX_NOMINATORS_TO_SLASH)) } + pub fn max_submit_receipt_weight() -> Weight { + T::WeightInfo::submit_bundle() + .saturating_add( + // We use `MAX_BUNLDE_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_BUNLDE_PER_BLOCK), + ) + .saturating_add(T::WeightInfo::slash_operator(MAX_NOMINATORS_TO_SLASH)) + } + pub fn max_staking_epoch_transition() -> Weight { T::WeightInfo::operator_reward_tax_and_restake(MAX_BUNLDE_PER_BLOCK).saturating_add( T::WeightInfo::finalize_domain_epoch_staking(T::MaxPendingStakingOperation::get()), @@ -2778,11 +2788,6 @@ impl Pallet { } } - #[cfg(not(feature = "runtime-benchmarks"))] - fn actual_slash_operator_weight(slashed_nominators: u32) -> Weight { - T::WeightInfo::slash_operator(slashed_nominators) - } - pub fn storage_fund_account_balance(operator_id: OperatorId) -> BalanceOf { let storage_fund_acc = storage_fund_account::(operator_id); T::Currency::reducible_balance(&storage_fund_acc, Preservation::Preserve, Fortitude::Polite) diff --git a/crates/pallet-domains/src/weights.rs b/crates/pallet-domains/src/weights.rs index 3cd0dd1f45..6b531383d8 100644 --- a/crates/pallet-domains/src/weights.rs +++ b/crates/pallet-domains/src/weights.rs @@ -48,6 +48,7 @@ pub trait WeightInfo { fn unlock_funds() -> Weight; fn unlock_nominator() -> Weight; fn update_domain_operator_allow_list() -> Weight; + fn submit_receipt() -> Weight; } /// Weights for pallet_domains using the Substrate node and recommended hardware. @@ -478,6 +479,27 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } + /// Storage: `Domains::HeadReceiptNumber` (r:1 w:1) + /// Proof: `Domains::HeadReceiptNumber` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::NewAddedHeadReceipt` (r:1 w:1) + /// Proof: `Domains::NewAddedHeadReceipt` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::LatestConfirmedDomainExecutionReceipt` (r:1 w:0) + /// Proof: `Domains::LatestConfirmedDomainExecutionReceipt` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::BlockTree` (r:1 w:1) + /// Proof: `Domains::BlockTree` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::LatestSubmittedER` (r:1 w:1) + /// Proof: `Domains::LatestSubmittedER` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::BlockTreeNodes` (r:0 w:1) + /// Proof: `Domains::BlockTreeNodes` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn submit_receipt() -> Weight { + // Proof Size summary in bytes: + // Measured: `655` + // Estimated: `4120` + // Minimum execution time: 32_000_000 picoseconds. + Weight::from_parts(35_000_000, 4120) + .saturating_add(T::DbWeight::get().reads(5_u64)) + .saturating_add(T::DbWeight::get().writes(5_u64)) + } } // For backwards compatibility and tests @@ -907,4 +929,25 @@ impl WeightInfo for () { .saturating_add(ParityDbWeight::get().reads(1_u64)) .saturating_add(ParityDbWeight::get().writes(1_u64)) } + /// Storage: `Domains::HeadReceiptNumber` (r:1 w:1) + /// Proof: `Domains::HeadReceiptNumber` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::NewAddedHeadReceipt` (r:1 w:1) + /// Proof: `Domains::NewAddedHeadReceipt` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::LatestConfirmedDomainExecutionReceipt` (r:1 w:0) + /// Proof: `Domains::LatestConfirmedDomainExecutionReceipt` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::BlockTree` (r:1 w:1) + /// Proof: `Domains::BlockTree` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::LatestSubmittedER` (r:1 w:1) + /// Proof: `Domains::LatestSubmittedER` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Domains::BlockTreeNodes` (r:0 w:1) + /// Proof: `Domains::BlockTreeNodes` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn submit_receipt() -> Weight { + // Proof Size summary in bytes: + // Measured: `655` + // Estimated: `4120` + // Minimum execution time: 32_000_000 picoseconds. + Weight::from_parts(35_000_000, 4120) + .saturating_add(ParityDbWeight::get().reads(5_u64)) + .saturating_add(ParityDbWeight::get().writes(5_u64)) + } } From ea5f8553f936b9bfa7e8a45616bf8a0a863d6841 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:25:00 +0800 Subject: [PATCH 5/8] Update domain client to produce/submit singleton receipt when there is receipt gap Signed-off-by: linning --- .../src/malicious_bundle_producer.rs | 1 + domains/client/block-preprocessor/src/lib.rs | 64 +++++-- .../src/domain_bundle_producer.rs | 159 ++++++++++++++---- .../src/domain_bundle_proposer.rs | 40 ++--- .../domain-operator/src/domain_worker.rs | 18 +- domains/client/domain-operator/src/tests.rs | 3 + 6 files changed, 204 insertions(+), 81 deletions(-) diff --git a/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs b/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs index b381e548c2..95fc13df52 100644 --- a/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs +++ b/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs @@ -183,6 +183,7 @@ where None }) .await + .and_then(|res| res.into_opaque_bundle()) } pub async fn start + Send + 'static>( diff --git a/domains/client/block-preprocessor/src/lib.rs b/domains/client/block-preprocessor/src/lib.rs index 63e9cca2c0..356ce3672e 100644 --- a/domains/client/block-preprocessor/src/lib.rs +++ b/domains/client/block-preprocessor/src/lib.rs @@ -18,7 +18,7 @@ use crate::inherents::is_runtime_upgraded; use codec::Encode; use domain_runtime_primitives::opaque::AccountId; use sc_client_api::BlockBackend; -use sp_api::{ApiError, ApiExt, ProvideRuntimeApi}; +use sp_api::{ApiError, ApiExt, Core, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_core::H256; use sp_domains::core_api::DomainCoreApi; @@ -203,27 +203,59 @@ where &self, bundles: OpaqueBundles, tx_range: U256, - (domain_hash, domain_number): (Block::Hash, NumberFor), + (parent_domain_hash, parent_domain_number): (Block::Hash, NumberFor), at_consensus_hash: CBlock::Hash, ) -> sp_blockchain::Result<( Vec>, Vec<(Option, Block::Extrinsic)>, )> { + let consensus_spec_version = self + .consensus_client + .runtime_api() + .version(at_consensus_hash) + .map_err(sp_blockchain::Error::RuntimeApiError)? + .spec_version; + let mut inboxed_bundles = Vec::with_capacity(bundles.len()); let mut valid_extrinsics = Vec::new(); let runtime_api = self.client.runtime_api(); for bundle in bundles { + // For the honest operator the validity of the extrinsic of the bundle is committed + // to (or say verified against) the receipt that is submitted with the bundle, the + // consensus runtime should only accept the bundle if the receipt is derived from + // the parent domain block. If it is not then either there is a bug in the consensus + // runtime (for validating the bundle) or in the domain client (for finding the parent + // domain block). + // + // NOTE: The receipt's `domain_block_number` is verified by the consensus runtime while + // the `domain_block_hash` is not (which is take care of by the fraud proof) so we can't + // check the parent domain block hash here. + // TODO: remove consensus runtime version check before next network + if consensus_spec_version >= 6 + && bundle.receipt().domain_block_number != parent_domain_number + { + return Err(sp_blockchain::Error::RuntimeApiError( + ApiError::Application( + format!( + "Unexpected bundle in consensus block: {:?}, something must be wrong", + at_consensus_hash + ) + .into(), + ), + )); + } + let extrinsic_root = bundle.extrinsics_root(); match self.check_bundle_validity( &bundle, &tx_range, - (domain_hash, domain_number), + (parent_domain_hash, parent_domain_number), at_consensus_hash, )? { BundleValidity::Valid(extrinsics) => { let extrinsics: Vec<_> = match runtime_api - .extract_signer(domain_hash, extrinsics) + .extract_signer(parent_domain_hash, extrinsics) { Ok(res) => res, Err(e) => { @@ -262,7 +294,7 @@ where &self, bundle: &OpaqueBundle, CBlock::Hash, Block::Header, Balance>, tx_range: &U256, - (domain_hash, domain_number): (Block::Hash, NumberFor), + (parent_domain_hash, parent_domain_number): (Block::Hash, NumberFor), at_consensus_hash: CBlock::Hash, ) -> sp_blockchain::Result> { let bundle_vrf_hash = @@ -274,11 +306,13 @@ where let runtime_api = self.client.runtime_api(); let consensus_runtime_api = self.consensus_client.runtime_api(); let api_version = runtime_api - .api_version::, CBlock::Hash>>(domain_hash) + .api_version::, CBlock::Hash>>( + parent_domain_hash, + ) .map_err(sp_blockchain::Error::RuntimeApiError)? .ok_or_else(|| { sp_blockchain::Error::RuntimeApiError(ApiError::Application( - format!("MessengerApi not found at: {:?}", domain_hash).into(), + format!("MessengerApi not found at: {:?}", parent_domain_hash).into(), )) })?; @@ -287,7 +321,7 @@ where // NOTE: for each extrinsic the checking order must follow `InvalidBundleType::checking_order` for (index, opaque_extrinsic) in bundle.extrinsics.iter().enumerate() { let decode_result = - runtime_api.decode_extrinsic(domain_hash, opaque_extrinsic.clone())?; + runtime_api.decode_extrinsic(parent_domain_hash, opaque_extrinsic.clone())?; let extrinsic = match decode_result { Ok(extrinsic) => extrinsic, Err(err) => { @@ -304,7 +338,7 @@ where }; let is_within_tx_range = runtime_api.is_within_tx_range( - domain_hash, + parent_domain_hash, &extrinsic, &bundle_vrf_hash, tx_range, @@ -319,7 +353,7 @@ where // Check if this extrinsic is an inherent extrinsic. // If so, this is an invalid bundle since these extrinsics should not be included in the // bundle. Extrinsic is always decodable due to the check above. - if runtime_api.is_inherent_extrinsic(domain_hash, &extrinsic)? { + if runtime_api.is_inherent_extrinsic(parent_domain_hash, &extrinsic)? { return Ok(BundleValidity::Invalid( InvalidBundleType::InherentExtrinsic(index as u32), )); @@ -327,7 +361,7 @@ where if api_version >= 4 { if let Some(xdm_mmr_proof) = - runtime_api.extract_xdm_mmr_proof(domain_hash, &extrinsic)? + runtime_api.extract_xdm_mmr_proof(parent_domain_hash, &extrinsic)? { let ConsensusChainMmrLeafProof { opaque_mmr_leaf, @@ -352,10 +386,10 @@ where // to maintain side-effect in the storage buffer. let is_legal_tx = runtime_api .check_extrinsics_and_do_pre_dispatch( - domain_hash, + parent_domain_hash, vec![extrinsic.clone()], - domain_number, - domain_hash, + parent_domain_number, + parent_domain_hash, )? .is_ok(); @@ -365,7 +399,7 @@ where ))); } - let tx_weight = runtime_api.extrinsic_weight(domain_hash, &extrinsic)?; + let tx_weight = runtime_api.extrinsic_weight(parent_domain_hash, &extrinsic)?; estimated_bundle_weight = estimated_bundle_weight.saturating_add(tx_weight); extrinsics.push(extrinsic); diff --git a/domains/client/domain-operator/src/domain_bundle_producer.rs b/domains/client/domain-operator/src/domain_bundle_producer.rs index 8b8651e6f4..2ebeb2c9f7 100644 --- a/domains/client/domain-operator/src/domain_bundle_producer.rs +++ b/domains/client/domain-operator/src/domain_bundle_producer.rs @@ -4,19 +4,19 @@ use crate::utils::OperatorSlotInfo; use crate::BundleSender; use codec::Decode; use sc_client_api::{AuxStore, BlockBackend}; -use sp_api::ProvideRuntimeApi; +use sp_api::{ApiError, ApiExt, ProvideRuntimeApi}; use sp_block_builder::BlockBuilder; use sp_blockchain::HeaderBackend; use sp_consensus_slots::Slot; use sp_domains::core_api::DomainCoreApi; use sp_domains::{ Bundle, BundleProducerElectionApi, DomainId, DomainsApi, OperatorId, OperatorPublicKey, - OperatorSignature, SealedBundleHeader, + OperatorSignature, SealedBundleHeader, SealedSingletonReceipt, SingletonReceipt, }; use sp_keystore::KeystorePtr; use sp_messenger::MessengerApi; use sp_runtime::traits::{Block as BlockT, NumberFor, Zero}; -use sp_runtime::RuntimeAppPublic; +use sp_runtime::{RuntimeAppPublic, Saturating}; use sp_transaction_pool::runtime_api::TaggedTransactionQueue; use std::sync::Arc; use subspace_runtime_primitives::Balance; @@ -29,6 +29,27 @@ type OpaqueBundle = sp_domains::OpaqueBundle< Balance, >; +type SealedSingletonReceiptFor = SealedSingletonReceipt< + NumberFor, + ::Hash, + ::Header, + Balance, +>; + +pub enum DomainProposal { + Bundle(OpaqueBundle), + Receipt(SealedSingletonReceiptFor), +} + +impl DomainProposal { + pub fn into_opaque_bundle(self) -> Option> { + match self { + DomainProposal::Bundle(b) => Some(b), + DomainProposal::Receipt(_) => None, + } + } +} + pub struct DomainBundleProducer where Block: BlockT, @@ -124,11 +145,37 @@ where } } + fn sign( + &self, + operator_signing_key: &OperatorPublicKey, + msg: &[u8], + ) -> sp_blockchain::Result { + let signature = self + .keystore + .sr25519_sign(OperatorPublicKey::ID, operator_signing_key.as_ref(), msg) + .map_err(|error| { + sp_blockchain::Error::Application(Box::from(format!( + "Error occurred when signing the bundle: {error}" + ))) + })? + .ok_or_else(|| { + sp_blockchain::Error::Application(Box::from( + "This should not happen as the existence of key was just checked", + )) + })?; + + OperatorSignature::decode(&mut signature.as_ref()).map_err(|err| { + sp_blockchain::Error::Application(Box::from(format!( + "Failed to decode the signature of bundle: {err}" + ))) + }) + } + pub async fn produce_bundle( &mut self, operator_id: OperatorId, slot_info: OperatorSlotInfo, - ) -> sp_blockchain::Result>> { + ) -> sp_blockchain::Result>> { let OperatorSlotInfo { slot, proof_of_time, @@ -136,12 +183,25 @@ where let domain_best_number = self.client.info().best_number; let consensus_chain_best_hash = self.consensus_client.info().best_hash; - let should_skip_slot = { - let head_receipt_number = self - .consensus_client - .runtime_api() - .head_receipt_number(consensus_chain_best_hash, self.domain_id)?; + let head_domain_number = self + .consensus_client + .runtime_api() + .domain_best_number(consensus_chain_best_hash, self.domain_id)? + .ok_or_else(|| { + sp_blockchain::Error::Application( + format!( + "Failed to get the head domain number for domain {:?} at {:?}", + self.domain_id, consensus_chain_best_hash + ) + .into(), + ) + })?; + let head_receipt_number = self + .consensus_client + .runtime_api() + .head_receipt_number(consensus_chain_best_hash, self.domain_id)?; + let should_skip_slot = { // Operator is lagging behind the receipt chain on its parent chain as another operator // already processed a block higher than the local best and submitted the receipt to // the parent chain, we ought to catch up with the consensus block processing before @@ -175,7 +235,53 @@ where proof_of_time, )? { - tracing::info!("📦 Claimed bundle at slot {slot}"); + tracing::info!("📦 Claimed slot {slot}"); + + let receipt = self + .domain_bundle_proposer + .load_next_receipt(head_domain_number, head_receipt_number)?; + + let api_version = self + .consensus_client + .runtime_api() + .api_version::>(consensus_chain_best_hash) + .map_err(sp_blockchain::Error::RuntimeApiError)? + .ok_or_else(|| { + sp_blockchain::Error::RuntimeApiError(ApiError::Application( + format!("DomainsApi not found at: {:?}", consensus_chain_best_hash).into(), + )) + })?; + + // When the receipt gap is greater than one the operator need to produce receipt + // 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() + { + info!( + ?head_domain_number, + ?head_receipt_number, + "🔖 Producing singleton receipt at slot {:?}", + slot_info.slot + ); + + let singleton_receipt = SingletonReceipt { + proof_of_election, + receipt, + }; + + let signature = { + let to_sign: ::Hash = singleton_receipt.hash(); + self.sign(&operator_signing_key, to_sign.as_ref())? + }; + + let sealed_singleton_receipt: SealedSingletonReceiptFor = + SealedSingletonReceipt { + singleton_receipt, + signature, + }; + return Ok(Some(DomainProposal::Receipt(sealed_singleton_receipt))); + } let tx_range = self .consensus_client @@ -188,7 +294,7 @@ where })?; let (bundle_header, extrinsics) = self .domain_bundle_proposer - .propose_bundle_at(proof_of_election, tx_range, operator_id) + .propose_bundle_at(proof_of_election, tx_range, operator_id, receipt) .await?; // if there are no extrinsics and no receipts to confirm, skip the bundle @@ -210,31 +316,10 @@ where info!("🔖 Producing bundle at slot {:?}", slot_info.slot); - let to_sign = bundle_header.hash(); - - let signature = self - .keystore - .sr25519_sign( - OperatorPublicKey::ID, - operator_signing_key.as_ref(), - to_sign.as_ref(), - ) - .map_err(|error| { - sp_blockchain::Error::Application(Box::from(format!( - "Error occurred when signing the bundle: {error}" - ))) - })? - .ok_or_else(|| { - sp_blockchain::Error::Application(Box::from( - "This should not happen as the existence of key was just checked", - )) - })?; - - let signature = OperatorSignature::decode(&mut signature.as_ref()).map_err(|err| { - sp_blockchain::Error::Application(Box::from(format!( - "Failed to decode the signature of bundle: {err}" - ))) - })?; + let signature = { + let to_sign = bundle_header.hash(); + self.sign(&operator_signing_key, to_sign.as_ref())? + }; let bundle = Bundle { sealed_header: SealedBundleHeader::new(bundle_header, signature), @@ -246,7 +331,7 @@ where // tracing::error!(error = ?e, "Failed to send transaction bundle"); // } - Ok(Some(bundle.into_opaque_bundle())) + Ok(Some(DomainProposal::Bundle(bundle.into_opaque_bundle()))) } else { Ok(None) } diff --git a/domains/client/domain-operator/src/domain_bundle_proposer.rs b/domains/client/domain-operator/src/domain_bundle_proposer.rs index 7f39340282..3463559f3b 100644 --- a/domains/client/domain-operator/src/domain_bundle_proposer.rs +++ b/domains/client/domain-operator/src/domain_bundle_proposer.rs @@ -207,9 +207,11 @@ where proof_of_election: ProofOfElection, tx_range: U256, operator_id: OperatorId, + receipt: ExecutionReceiptFor, ) -> sp_blockchain::Result> { - let parent_number = self.client.info().best_number; - let parent_hash = self.client.info().best_hash; + // NOTE: use the domain block that derive the ER to validate the extrinsic to be included + // in the bundle, so the validity of the extrinsic is committed to the ER that submited together. + let (parent_number, parent_hash) = (receipt.domain_block_number, receipt.domain_block_hash); let mut t1 = self.transaction_pool.ready_at(parent_number).fuse(); // TODO: proper timeout @@ -232,8 +234,6 @@ where self.previous_bundled_tx .maybe_clear(self.consensus_client.info().best_hash); - let receipt = self.load_bundle_receipt(parent_number)?; - let bundle_vrf_hash = U256::from_be_bytes(proof_of_election.vrf_hash()); let domain_bundle_limit = self.fetch_domain_bundle_limit()?; @@ -395,33 +395,20 @@ where } /// Returns the receipt in the next domain bundle. - fn load_bundle_receipt( + pub fn load_next_receipt( &self, - header_number: NumberFor, + head_domain_number: NumberFor, + head_receipt_number: NumberFor, ) -> sp_blockchain::Result> { - let consensus_chain_block_hash = self.consensus_client.info().best_hash; - let head_receipt_number = self - .consensus_client - .runtime_api() - .head_receipt_number(consensus_chain_block_hash, self.domain_id)?; - - // TODO: the `receipt_number` may not be the best domain block number if there - // is fraud proof submitted and bad ERs pruned, thus the ER may not the one that - // derive from the latest domain block, which may cause the lagging operator able - // to submit invalid bundle accidentally. - // - // We need to resolve `https://github.com/subspace/subspace/issues/1673` to fix it - // completely. - let receipt_number = (head_receipt_number + One::one()).min(header_number); - tracing::trace!( - ?header_number, + ?head_domain_number, ?head_receipt_number, - ?receipt_number, - "Collecting receipts at {consensus_chain_block_hash:?}" + "Collecting receipt" ); - if receipt_number.is_zero() { + // Both `head_domain_number` 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() { let genesis_hash = self.client.info().genesis_hash; let genesis_header = self.client.header(genesis_hash)?.ok_or_else(|| { sp_blockchain::Error::Backend(format!( @@ -436,6 +423,9 @@ where )); } + // The next receipt must extend the current head receipt + let receipt_number = head_receipt_number + One::one(); + // Get the domain block hash corresponding to `receipt_number` in the domain canonical chain let domain_hash = self.client.hash(receipt_number)?.ok_or_else(|| { sp_blockchain::Error::Backend(format!( diff --git a/domains/client/domain-operator/src/domain_worker.rs b/domains/client/domain-operator/src/domain_worker.rs index 384e78a5b2..57cd581e46 100644 --- a/domains/client/domain-operator/src/domain_worker.rs +++ b/domains/client/domain-operator/src/domain_worker.rs @@ -15,7 +15,7 @@ // along with Polkadot. If not, see . use crate::bundle_processor::BundleProcessor; -use crate::domain_bundle_producer::DomainBundleProducer; +use crate::domain_bundle_producer::{DomainBundleProducer, DomainProposal}; use crate::utils::{BlockInfo, OperatorSlotInfo}; use crate::{NewSlotNotification, OperatorStreams}; use futures::channel::mpsc; @@ -149,12 +149,22 @@ pub(super) async fn start_worker< Err(err) => { tracing::error!(?slot, ?err, "Error at producing bundle."); } - Ok(Some(opaque_bundle)) => { + Ok(Some(domain_proposal)) => { let best_hash = consensus_client.info().best_hash; let mut runtime_api = consensus_client.runtime_api(); runtime_api.register_extension(consensus_offchain_tx_pool_factory.offchain_transaction_pool(best_hash)); - if let Err(err) = runtime_api.submit_bundle_unsigned(best_hash, opaque_bundle) { - tracing::error!(?slot, ?err, "Error at submitting bundle."); + + match domain_proposal { + DomainProposal::Bundle(opaque_bundle) => { + if let Err(err) = runtime_api.submit_bundle_unsigned(best_hash, opaque_bundle) { + tracing::error!(?slot, ?err, "Error at submitting bundle."); + } + }, + DomainProposal::Receipt(singleton_receipt) => { + if let Err(err) = runtime_api.submit_receipt_unsigned(best_hash, singleton_receipt) { + tracing::error!(?slot, ?err, "Error at submitting receipt."); + } + }, } } Ok(None) => {} diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index bc447f3de3..bf97f909b5 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -3273,16 +3273,19 @@ async fn stale_and_in_future_bundle_should_be_rejected() { .produce_bundle(operator_id, slot_info(valid_slot, valid_pot)) .await .unwrap() + .and_then(|res| res.into_opaque_bundle()) .unwrap(); let bundle_with_unknow_pot = bundle_producer .produce_bundle(operator_id, slot_info(valid_slot, unknow_pot)) .await .unwrap() + .and_then(|res| res.into_opaque_bundle()) .unwrap(); let bundle_with_slot_in_future = bundle_producer .produce_bundle(operator_id, slot_info(slot_in_future, valid_pot)) .await .unwrap() + .and_then(|res| res.into_opaque_bundle()) .unwrap(); for bundle in [ bundle_with_unknow_pot.clone(), From fedc400273e874af9edb36d8426fc993d328c281 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:25:41 +0800 Subject: [PATCH 6/8] Add test case for singleton receipt Signed-off-by: linning --- domains/client/domain-operator/src/tests.rs | 120 +++++++++++++++++++- domains/test/service/src/domain.rs | 15 ++- 2 files changed, 127 insertions(+), 8 deletions(-) diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index bf97f909b5..7bcf86da20 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -13,6 +13,7 @@ use domain_test_service::EcdsaKeyring::{Alice, Bob, Charlie, Eve}; use domain_test_service::Sr25519Keyring::{self, Alice as Sr25519Alice, Ferdie}; use domain_test_service::{construct_extrinsic_generic, AUTO_ID_DOMAIN_ID, EVM_DOMAIN_ID}; use futures::StreamExt; +use pallet_domains::OperatorConfig; use pallet_messenger::ChainAllowlistUpdate; use sc_client_api::{Backend, BlockBackend, BlockchainEvents, HeaderBackend}; use sc_consensus::SharedBlockImport; @@ -32,7 +33,7 @@ use sp_domains::core_api::DomainCoreApi; use sp_domains::merkle_tree::MerkleTree; use sp_domains::{ Bundle, BundleValidity, ChainId, ChannelId, DomainsApi, HeaderHashingFor, InboxedBundle, - InvalidBundleType, Transfers, + InvalidBundleType, OperatorSignature, OperatorSigningKeyProofOfOwnershipData, Transfers, }; use sp_domains_fraud_proof::fraud_proof::{ ApplyExtrinsicMismatch, ExecutionPhase, FinalizeBlockMismatch, FraudProofVariant, @@ -2868,6 +2869,8 @@ async fn test_valid_bundle_proof_generation_and_verification() { .build_evm_node(Role::Authority, Alice, &mut ferdie) .await; + produce_blocks!(ferdie, alice, 3).await.unwrap(); + for i in 0..3 { let tx = alice.construct_extrinsic( alice.account_nonce() + i, @@ -4328,7 +4331,7 @@ async fn test_bad_receipt_chain() { // Start Ferdie let mut ferdie = MockConsensusNode::run( tokio_handle.clone(), - Ferdie, + Sr25519Alice, BasePath::new(directory.path().join("ferdie")), ); @@ -4368,10 +4371,11 @@ async fn test_bad_receipt_chain() { ) }; - produce_blocks!(ferdie, alice, 5).await.unwrap(); + produce_blocks!(ferdie, alice, 15).await.unwrap(); // Get a bundle from the txn pool and modify the receipt of the target bundle to an invalid one let (slot, mut opaque_bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await; + let stale_bundle = opaque_bundle.clone(); let (bad_receipt_hash, bad_submit_bundle_tx) = { let receipt = &mut opaque_bundle.sealed_header.header.receipt; receipt.domain_block_hash = Default::default(); @@ -4425,6 +4429,7 @@ async fn test_bad_receipt_chain() { ) .await .expect("produce bundle must success") + .and_then(|res| res.into_opaque_bundle()) .expect("must win the challenge"); let (receipt_hash, bad_submit_bundle_tx) = { let mut opaque_bundle = bundle; @@ -4466,12 +4471,115 @@ async fn test_bad_receipt_chain() { 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()); + 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, EVM_DOMAIN_ID, receipt_hash) + .is_bad_er_pending_to_prune(ferdie_best_hash, EVM_DOMAIN_ID, *receipt_hash) .unwrap()); } + + // There should be a receipt gap + let head_domain_number = ferdie + .client + .runtime_api() + .domain_best_number(ferdie_best_hash, EVM_DOMAIN_ID) + .unwrap() + .unwrap(); + let head_receipt_number = ferdie + .client + .runtime_api() + .head_receipt_number(ferdie_best_hash, EVM_DOMAIN_ID) + .unwrap(); + assert_eq!(head_domain_number - head_receipt_number, 9); + // The previou bundle will be rejected as there is a receipt gap + match ferdie + .submit_transaction(bundle_to_tx(stale_bundle)) + .await + .unwrap_err() + { + sc_transaction_pool::error::Error::Pool(TxPoolError::InvalidTransaction(invalid_tx)) => { + assert_eq!(invalid_tx, InvalidTransactionCode::Bundle.into()) + } + e => panic!("Unexpected error: {e}"), + } + + // Register another operator as Alice is slashed + ferdie + .construct_and_send_extrinsic_with(pallet_domains::Call::register_operator { + domain_id: EVM_DOMAIN_ID, + amount: 1000 * SSC, + config: OperatorConfig { + signing_key: Sr25519Keyring::Charlie.public().into(), + minimum_nominator_stake: Balance::MAX, + nomination_tax: Default::default(), + }, + signing_key_proof_of_ownership: OperatorSignature::from( + OperatorSigningKeyProofOfOwnershipData { + operator_owner: Sr25519Alice.to_account_id(), + } + .using_encoded(|e| Sr25519Keyring::Charlie.sign(e)), + ), + }) + .await + .unwrap(); + ferdie.produce_blocks(1).await.unwrap(); + + ferdie + .construct_and_send_extrinsic_with(pallet_sudo::Call::sudo { + call: Box::new(subspace_test_runtime::RuntimeCall::Domains( + pallet_domains::Call::force_staking_epoch_transition { + domain_id: EVM_DOMAIN_ID, + }, + )), + }) + .await + .unwrap(); + ferdie.produce_blocks(1).await.unwrap(); + + let alice_best_number = alice.client.info().best_number; + drop(alice); + + // Start another operator node + let bob = domain_test_service::DomainNodeBuilder::new( + tokio_handle.clone(), + BasePath::new(directory.path().join("bob")), + ) + .operator_id(2) + .build_evm_node(Role::Authority, Charlie, &mut ferdie) + .await; + ferdie.produce_blocks(1).await.unwrap(); + + let bob_best_number = bob.client.info().best_number; + assert_eq!(alice_best_number, bob_best_number); + + // Bad receipt should be pruned as singletone receipt submitting + for receipt_hash in vec![bad_receipt_hash] + .into_iter() + .chain(bad_receipt_descendants) + { + let slot = ferdie.produce_slot(); + ferdie.notify_new_slot_and_wait_for_bundle(slot).await; + ferdie.produce_block_with_slot(slot).await.unwrap(); + assert!(!ferdie.does_receipt_exist(receipt_hash).unwrap()); + } + + // The receipt gap should be fill up + let ferdie_best_hash = ferdie.client.info().best_hash; + let head_domain_number = ferdie + .client + .runtime_api() + .domain_best_number(ferdie_best_hash, EVM_DOMAIN_ID) + .unwrap() + .unwrap(); + let head_receipt_number = ferdie + .client + .runtime_api() + .head_receipt_number(ferdie_best_hash, EVM_DOMAIN_ID) + .unwrap(); + assert_eq!(head_domain_number - head_receipt_number, 1); + assert_eq!(bob_best_number, bob.client.info().best_number); + + produce_blocks!(ferdie, bob, 15).await.unwrap(); } #[tokio::test(flavor = "multi_thread")] diff --git a/domains/test/service/src/domain.rs b/domains/test/service/src/domain.rs index bd485e0612..6636f6310b 100644 --- a/domains/test/service/src/domain.rs +++ b/domains/test/service/src/domain.rs @@ -29,7 +29,7 @@ use sp_block_builder::BlockBuilder; use sp_consensus_subspace::SubspaceApi; use sp_core::{Encode, H256}; use sp_domains::core_api::DomainCoreApi; -use sp_domains::DomainId; +use sp_domains::{DomainId, OperatorId}; use sp_messenger::messages::{ChainId, ChannelId}; use sp_messenger::{MessengerApi, RelayerApi}; use sp_offchain::OffchainWorkerApi; @@ -134,6 +134,7 @@ where domain_nodes: Vec, domain_nodes_exclusive: bool, skip_empty_bundle_production: bool, + maybe_operator_id: Option, role: Role, mock_consensus_node: &mut MockConsensusNode, ) -> Self { @@ -186,7 +187,7 @@ where let maybe_operator_id = role .is_authority() - .then_some(if domain_id == EVM_DOMAIN_ID { 0 } else { 1 }); + .then_some(maybe_operator_id.unwrap_or(if domain_id == EVM_DOMAIN_ID { 0 } else { 1 })); let consensus_best_hash = mock_consensus_node.client.info().best_hash; let chain_constants = mock_consensus_node @@ -409,6 +410,7 @@ pub struct DomainNodeBuilder { domain_nodes_exclusive: bool, skip_empty_bundle_production: bool, base_path: BasePath, + maybe_operator_id: Option, } impl DomainNodeBuilder { @@ -423,6 +425,7 @@ impl DomainNodeBuilder { domain_nodes_exclusive: false, skip_empty_bundle_production: false, base_path, + maybe_operator_id: None, } } @@ -449,6 +452,12 @@ impl DomainNodeBuilder { self } + /// Set the operator id + pub fn operator_id(mut self, operator_id: OperatorId) -> Self { + self.maybe_operator_id = Some(operator_id); + self + } + /// Build a evm domain node pub async fn build_evm_node( self, @@ -464,6 +473,7 @@ impl DomainNodeBuilder { self.domain_nodes, self.domain_nodes_exclusive, self.skip_empty_bundle_production, + self.maybe_operator_id, role, mock_consensus_node, ) @@ -485,6 +495,7 @@ impl DomainNodeBuilder { self.domain_nodes, self.domain_nodes_exclusive, self.skip_empty_bundle_production, + self.maybe_operator_id, role, mock_consensus_node, ) From b08793ec594ed414dac53e8816e6a55d249e049b Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:28:27 +0800 Subject: [PATCH 7/8] Fix receipt gap brings by the genesis receipt Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 1bcfc9c49c..b1994cb5c2 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -158,9 +158,11 @@ pub(crate) fn execution_receipt_type( } // Add confirm to the head receipt that added in the current block or it is - // the genesis receipt + // the first genesis receipt + let is_first_genesis_receipt = + receipt_number.is_zero() && HeadDomainNumber::::get(domain_id).is_zero(); if receipt_number == head_receipt_number - && (head_receipt_extended || receipt_number.is_zero()) + && (head_receipt_extended || is_first_genesis_receipt) { return ReceiptType::Accepted(AcceptedReceiptType::CurrentHead); } From 2a6e90024a4959b6e670fdf1438b44180837902e Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 19 Jul 2024 22:29:11 +0800 Subject: [PATCH 8/8] Add fast path to skip verifying the same receipt in the same block Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 31 ++++++++++++++++++------- crates/pallet-domains/src/lib.rs | 8 +++---- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index b1994cb5c2..d4a5e01da6 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -6,8 +6,9 @@ extern crate alloc; use crate::{ BalanceOf, BlockTree, BlockTreeNodeFor, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockNumberFor, DomainHashingFor, DomainRuntimeUpgradeRecords, ExecutionInbox, - ExecutionReceiptOf, HeadReceiptExtended, HeadReceiptNumber, InboxedBundleAuthor, - LatestConfirmedDomainExecutionReceipt, LatestSubmittedER, Pallet, ReceiptHashFor, + ExecutionReceiptOf, HeadDomainNumber, HeadReceiptNumber, InboxedBundleAuthor, + LatestConfirmedDomainExecutionReceipt, LatestSubmittedER, NewAddedHeadReceipt, Pallet, + ReceiptHashFor, }; #[cfg(not(feature = "std"))] use alloc::vec::Vec; @@ -49,6 +50,7 @@ pub enum Error { OverwritingER, RuntimeNotFound, LastBlockNotFound, + UnmatchedNewHeadReceipt, UnexpectedConfirmedDomainBlock, } @@ -122,7 +124,7 @@ pub(crate) fn execution_receipt_type( ) -> ReceiptType { let receipt_number = execution_receipt.domain_block_number; let head_receipt_number = HeadReceiptNumber::::get(domain_id); - let head_receipt_extended = HeadReceiptExtended::::get(domain_id); + let head_receipt_extended = NewAddedHeadReceipt::::get(domain_id).is_some(); let next_receipt_number = head_receipt_number.saturating_add(One::one()); let latest_confirmed_domain_block_number = Pallet::::latest_confirmed_domain_block_number(domain_id); @@ -197,6 +199,19 @@ pub(crate) fn verify_execution_receipt( return Err(rejected_receipt_type.into()); } + // If there is new head receipt added in the current block, as long as the incoming + // receipt is the same as the new head receipt we can safely skip the following checks, + // if they are not the same, we just reject the incoming receipt and expecting a fraud + // proof will be submit if the new head receipt is fraudulent and then the incoming + // receipt will be re-submit. + if let Some(new_added_head_receipt) = NewAddedHeadReceipt::::get(domain_id) { + ensure!( + new_added_head_receipt == execution_receipt.hash::>(), + Error::UnmatchedNewHeadReceipt, + ); + return Ok(()); + } + // The genesis receipt is generated and added to the block tree by the runtime upon domain // instantiation thus it is unchallengeable, we can safely skip other checks as long as we // can ensure it is always be the same. @@ -324,6 +339,7 @@ pub(crate) fn process_execution_receipt( execution_receipt: ExecutionReceiptOf, receipt_type: AcceptedReceiptType, ) -> ProcessExecutionReceiptResult { + let er_hash = execution_receipt.hash::>(); let receipt_block_number = execution_receipt.domain_block_number; match receipt_type { AcceptedReceiptType::NewHead => { @@ -331,7 +347,7 @@ pub(crate) fn process_execution_receipt( // Update the head receipt number HeadReceiptNumber::::insert(domain_id, receipt_block_number); - HeadReceiptExtended::::insert(domain_id, true); + NewAddedHeadReceipt::::insert(domain_id, er_hash); // Prune expired domain block if let Some(to_prune) = @@ -425,7 +441,6 @@ pub(crate) fn process_execution_receipt( } AcceptedReceiptType::CurrentHead => { // Add confirmation to the current head receipt - let er_hash = execution_receipt.hash::>(); BlockTreeNodes::::mutate(er_hash, |maybe_node| { let node = maybe_node.as_mut().expect( "The domain block of `CurrentHead` receipt is checked to be exist in `execution_receipt_type`; qed" @@ -883,7 +898,7 @@ mod tests { extend_block_tree_from_zero(domain_id, operator_id1, 3); // No new receipt submitted in current block - assert!(!HeadReceiptExtended::::get(domain_id)); + assert!(NewAddedHeadReceipt::::get(domain_id).is_none()); // Receipt that confirm a head receipt of the previous block is stale receipt let head_receipt_number = HeadReceiptNumber::::get(domain_id); @@ -1093,8 +1108,8 @@ mod tests { let head_receipt_number = HeadReceiptNumber::::get(domain_id); // reject extending receipt if the HeadReceiptNumber is already extended - assert!(!HeadReceiptExtended::::get(domain_id)); - HeadReceiptExtended::::set(domain_id, true); + assert!(NewAddedHeadReceipt::::get(domain_id).is_none()); + NewAddedHeadReceipt::::set(domain_id, Some(H256::random())); // Construct a future receipt let mut future_receipt = next_receipt.clone(); diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 3c00203373..f6ac44e672 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -606,12 +606,12 @@ mod pallet { pub(super) type HeadReceiptNumber = StorageMap<_, Identity, DomainId, DomainBlockNumberFor, ValueQuery>; - /// Whether the head receipt have extended in the current consensus block + /// The hash of the new head receipt added in the current consensus block /// /// Temporary storage only exist during block execution #[pallet::storage] - pub(super) type HeadReceiptExtended = - StorageMap<_, Identity, DomainId, bool, ValueQuery>; + pub(super) type NewAddedHeadReceipt = + StorageMap<_, Identity, DomainId, T::DomainHash, OptionQuery>; /// The consensus block hash used to verify ER, /// only store the consensus block hash for a domain @@ -1856,7 +1856,7 @@ mod pallet { fn on_finalize(_: BlockNumberFor) { let _ = LastEpochStakingDistribution::::clear(u32::MAX, None); - let _ = HeadReceiptExtended::::clear(u32::MAX, None); + let _ = NewAddedHeadReceipt::::clear(u32::MAX, None); } }