From dcf6de44df8478777b499022c4ca4ffced45c617 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 15 Mar 2024 07:13:06 +0800 Subject: [PATCH 1/3] Attach provides tag and priority to fraud proof extrinsic The provides tag ensure for a given domain there is only at most one fraud proof acceptted by the tx pool at a time. The priority ensure more emergency fraud proof have higher priority to be acceptted by tx pool and included in the block Signed-off-by: linning --- crates/pallet-domains/src/lib.rs | 86 ++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 25 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index abdcd40b91..24eea7b702 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -70,6 +70,7 @@ use sp_domains_fraud_proof::verification::{ verify_invalid_transfers_fraud_proof, verify_valid_bundle_fraud_proof, }; use sp_runtime::traits::{BlockNumberProvider, CheckedSub, Hash, Header, One, Zero}; +use sp_runtime::transaction_validity::TransactionPriority; use sp_runtime::{RuntimeAppPublic, SaturatedConversion, Saturating}; pub use staking::OperatorConfig; use subspace_core_primitives::{BlockHash, PotOutput, SlotNumber, U256}; @@ -119,6 +120,12 @@ pub(crate) struct ElectionVerificationParams { total_domain_stake: Balance, } +#[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] +pub(crate) enum FraudProofTag { + BadER(DomainId), + BundleEquivocation(OperatorId), +} + pub type DomainBlockNumberFor = <::DomainHeader as Header>::Number; pub type DomainHashingFor = <::DomainHeader as Header>::Hashing; pub type ReceiptHashFor = <::DomainHeader as Header>::Hash; @@ -1399,17 +1406,6 @@ mod pallet { } } - /// Constructs a `TransactionValidity` with pallet-executor specific defaults. - fn unsigned_validity(prefix: &'static str, tag: impl Encode) -> TransactionValidity { - ValidTransaction::with_tag_prefix(prefix) - .priority(TransactionPriority::MAX) - .and_provides(tag) - .longevity(TransactionLongevity::MAX) - // We need this extrinsic to be propagated to the farmer nodes. - .propagate(true) - .build() - } - #[pallet::validate_unsigned] impl ValidateUnsigned for Pallet { type Call = Call; @@ -1425,6 +1421,7 @@ mod pallet { .map_err(|_| InvalidTransaction::Call.into()) }), Call::submit_fraud_proof { fraud_proof } => Self::validate_fraud_proof(fraud_proof) + .map(|_| ()) .map_err(|_| InvalidTransaction::Call.into()), _ => Err(InvalidTransaction::Call.into()), } @@ -1490,16 +1487,24 @@ mod pallet { .build() } Call::submit_fraud_proof { fraud_proof } => { - if let Err(e) = Self::validate_fraud_proof(fraud_proof) { - log::warn!( - target: "runtime::domains", - "Bad fraud proof {:?}, error: {e:?}", fraud_proof.domain_id(), - ); - return InvalidTransactionCode::FraudProof.into(); - } - - // TODO: proper tag value. - unsigned_validity("SubspaceSubmitFraudProof", fraud_proof) + let (tag, priority) = match Self::validate_fraud_proof(fraud_proof) { + Err(e) => { + log::warn!( + target: "runtime::domains", + "Bad fraud proof {:?}, error: {e:?}", fraud_proof.domain_id(), + ); + return InvalidTransactionCode::FraudProof.into(); + } + Ok(tp) => tp, + }; + + ValidTransaction::with_tag_prefix("SubspaceSubmitFraudProof") + .priority(priority) + .and_provides(tag) + .longevity(TransactionLongevity::MAX) + // We need this extrinsic to be propagated to the farmer nodes. + .propagate(true) + .build() } _ => InvalidTransaction::Call.into(), @@ -1753,11 +1758,14 @@ impl Pallet { fn validate_fraud_proof( fraud_proof: &FraudProof, T::Hash, T::DomainHeader>, - ) -> Result<(), FraudProofError> { - if let Some(bad_receipt_hash) = fraud_proof.targeted_bad_receipt_hash() { + ) -> Result<(FraudProofTag, TransactionPriority), FraudProofError> { + let tag_and_priority = if let Some(bad_receipt_hash) = + fraud_proof.targeted_bad_receipt_hash() + { let bad_receipt = BlockTreeNodes::::get(bad_receipt_hash) .ok_or(FraudProofError::BadReceiptNotFound)? .execution_receipt; + let domain_block_number = bad_receipt.domain_block_number; ensure!( !bad_receipt.domain_block_number.is_zero(), @@ -1898,6 +1906,20 @@ impl Pallet { })?, _ => return Err(FraudProofError::UnexpectedFraudProof), } + + // The priority of fraud proof is determined by how many blocks left before the bad ER + // is confirmed, the less the more emergency it is, thus give a higher priority. + let block_before_bad_er_confirm = domain_block_number.saturating_sub( + Self::latest_confirmed_domain_block_number(fraud_proof.domain_id()), + ); + let priority = + TransactionPriority::MAX - block_before_bad_er_confirm.saturated_into::(); + + // Use the domain id as tag thus the consensus node only accept one fraud proof for a + // specific domain at a time + let tag = FraudProofTag::BadER(fraud_proof.domain_id()); + + (tag, priority) } else if let Some((bad_operator_id, _)) = fraud_proof.targeted_bad_operator_and_slot_for_bundle_equivocation() { @@ -1922,9 +1944,23 @@ impl Pallet { _ => return Err(FraudProofError::UnexpectedFraudProof), } - } - Ok(()) + // Bundle equivacotion fraud proof doesn't target bad ER thus we give it the lowest priority + // compared to other fraud proofs + let priority = TransactionPriority::MAX + - T::BlockTreePruningDepth::get().saturated_into::() + - 1; + + // Use the operator id as tag thus the consensus node only accept one bundle equivacotion fraud proof + // for a specific operator at a time + let tag = FraudProofTag::BundleEquivocation(bad_operator_id); + + (tag, priority) + } else { + return Err(FraudProofError::UnexpectedFraudProof); + }; + + Ok(tag_and_priority) } /// Return operators specific election verification params for Proof of Election verification. From db827f35ebb417b044f7e280a59b195c50786812 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 15 Mar 2024 07:28:29 +0800 Subject: [PATCH 2/3] Reduce the priority of bundle and XDM from MAX to 1 Bundle and XDM have a bit higher priority than normal extrinsic but must less than the fraud proof Signed-off-by: linning --- crates/pallet-domains/src/lib.rs | 4 +++- domains/pallets/messenger/src/lib.rs | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 24eea7b702..028c0590a5 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -1478,7 +1478,9 @@ mod pallet { } ValidTransaction::with_tag_prefix("SubspaceSubmitBundle") - .priority(TransactionPriority::MAX) + // Bundle 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") })) diff --git a/domains/pallets/messenger/src/lib.rs b/domains/pallets/messenger/src/lib.rs index 71ff264b46..9580e44192 100644 --- a/domains/pallets/messenger/src/lib.rs +++ b/domains/pallets/messenger/src/lib.rs @@ -436,7 +436,9 @@ mod pallet { )); }; valid_tx_builder - .priority(TransactionPriority::MAX) + // XDM have a bit higher priority than normal extrinsic but must less than + // fraud proof + .priority(1) .longevity(TransactionLongevity::MAX) .and_provides((msg.dst_chain_id, msg.channel_id, msg.nonce)) .propagate(true) @@ -456,7 +458,9 @@ mod pallet { )); }; valid_tx_builder - .priority(TransactionPriority::MAX) + // XDM have a bit higher priority than normal extrinsic but must less than + // fraud proof + .priority(1) .longevity(TransactionLongevity::MAX) .and_provides((msg.dst_chain_id, msg.channel_id, msg.nonce)) .propagate(true) From d2d8ae1f11c9f5c87f92b92d3933b58d291e988e Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 15 Mar 2024 07:29:13 +0800 Subject: [PATCH 3/3] Add TODO to check fraud proof priority before validating it Signed-off-by: linning --- crates/subspace-service/src/transaction_pool.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/subspace-service/src/transaction_pool.rs b/crates/subspace-service/src/transaction_pool.rs index 0228699ba1..14872fe563 100644 --- a/crates/subspace-service/src/transaction_pool.rs +++ b/crates/subspace-service/src/transaction_pool.rs @@ -161,6 +161,10 @@ where let slot_probability = self.chain_constants.slot_probability(); let fraud_proof_submit_sink = self.fraud_proof_submit_sink.clone(); async move { + // TODO: after https://github.com/paritytech/polkadot-sdk/issues/3705 is resolved, check if + // there is already a fraud proof with the same tag and higher priority in the tx pool, if so + // drop the incoming fraud proof before validating it. + let uxt_validity = chain_api .validate_transaction(at, source, uxt.clone()) .await?;