diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index ba9a41d0cb..415da78faa 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -3,7 +3,7 @@ use crate::pallet::StateRoots; use crate::{ BalanceOf, BlockTree, Config, ConsensusBlockInfo, DomainBlockDescendants, DomainBlocks, - ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber, InboxedBundle, + ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber, InboxedBundleAuthor, }; use codec::{Decode, Encode}; use frame_support::{ensure, PalletError}; @@ -123,7 +123,7 @@ pub(crate) fn verify_execution_receipt( consensus_block_number, consensus_block_hash, domain_block_number, - block_extrinsics_roots, + inboxed_bundles, parent_domain_block_receipt_hash, execution_trace, execution_trace_root, @@ -138,15 +138,15 @@ pub(crate) fn verify_execution_receipt( Error::BadGenesisReceipt ); } else { + let bundles_extrinsics_roots: Vec<_> = + inboxed_bundles.iter().map(|b| b.extrinsics_root).collect(); let execution_inbox = ExecutionInbox::::get((domain_id, domain_block_number, consensus_block_number)); - let expected_extrinsics_roots: Vec<_> = execution_inbox - .into_iter() - .map(|b| b.extrinsics_root) - .collect(); + let expected_extrinsics_roots: Vec<_> = + execution_inbox.iter().map(|b| b.extrinsics_root).collect(); ensure!( - !block_extrinsics_roots.is_empty() - && *block_extrinsics_roots == expected_extrinsics_roots, + !bundles_extrinsics_roots.is_empty() + && bundles_extrinsics_roots == expected_extrinsics_roots, Error::InvalidExtrinsicsRoots ); @@ -297,12 +297,10 @@ pub(crate) fn process_execution_receipt( execution_receipt.consensus_block_number, )); for (index, bd) in bundle_digests.into_iter().enumerate() { - if let Some(bundle_author) = InboxedBundle::::take(bd.header_hash) { - if execution_receipt - .invalid_bundles - .iter() - .any(|ib| ib.bundle_index == index as u32) - { + if let Some(bundle_author) = InboxedBundleAuthor::::take(bd.header_hash) { + // It is okay to index `ER::bundles` here since `verify_execution_receipt` have checked + // the `ER::bundles` have the same length of `ExecutionInbox` + if execution_receipt.inboxed_bundles[index].is_invalid() { invalid_bundle_authors.push(bundle_author); } } @@ -409,7 +407,7 @@ mod tests { use frame_support::dispatch::RawOrigin; use frame_support::{assert_err, assert_ok}; use sp_core::H256; - use sp_domains::{BundleDigest, InvalidBundle, InvalidBundleType}; + use sp_domains::{BundleDigest, InboxedBundle, InvalidBundleType}; use sp_runtime::traits::BlockNumberProvider; #[test] @@ -506,7 +504,9 @@ mod tests { extrinsics_root: bundle_extrinsics_root, }] ); - assert!(InboxedBundle::::contains_key(bundle_header_hash)); + assert!(InboxedBundleAuthor::::contains_key( + bundle_header_hash + )); // Head receipt number should be updated let head_receipt_number = HeadReceiptNumber::::get(domain_id); @@ -545,7 +545,7 @@ mod tests { let pruned_bundle = bundle_header_hash_of_block_1.unwrap(); assert!(BlockTree::::get(domain_id, 1).is_empty()); assert!(ExecutionInbox::::get((domain_id, 1, 1)).is_empty()); - assert!(!InboxedBundle::::contains_key(pruned_bundle)); + assert!(!InboxedBundleAuthor::::contains_key(pruned_bundle)); assert_eq!( execution_receipt_type::(domain_id, &pruned_receipt), ReceiptType::Rejected(RejectedReceiptType::Pruned) @@ -741,12 +741,12 @@ mod tests { future_receipt.consensus_block_number, ), future_receipt - .block_extrinsics_roots + .inboxed_bundles .clone() .into_iter() - .map(|er| BundleDigest { + .map(|b| BundleDigest { header_hash: H256::random(), - extrinsics_root: er, + extrinsics_root: b.extrinsics_root, }) .collect::>(), ); @@ -774,7 +774,8 @@ mod tests { // Receipt with unknown extrinsics roots let mut unknown_extrinsics_roots_receipt = current_head_receipt.clone(); - unknown_extrinsics_roots_receipt.block_extrinsics_roots = vec![H256::random()]; + unknown_extrinsics_roots_receipt.inboxed_bundles = + vec![InboxedBundle::valid(H256::random(), H256::random())]; assert_err!( verify_execution_receipt::(domain_id, &unknown_extrinsics_roots_receipt), Error::InvalidExtrinsicsRoots @@ -881,36 +882,38 @@ mod tests { let head_node = get_block_tree_node_at::(domain_id, head_receipt_number).unwrap(); assert_eq!(head_node.operator_ids, operator_set); - // Prepare the inbainvalid bundles and bundle authors - let invalid_bundle_authors: Vec<_> = operator_set - .clone() - .into_iter() - .filter(|i| i % 2 == 0) - .collect(); - let invalid_bundles = invalid_bundle_authors - .iter() - .map(|i| InvalidBundle { - // operator id start at 1 while bundle_index start at 0, thus minus 1 here - bundle_index: *i as u32 - 1, - invalid_bundle_type: InvalidBundleType::OutOfRangeTx, - }) - .collect(); - - // Get the `bunde_extrinsics_roots` that contains all the submitted bundles + // Get the `bundles_extrinsics_roots` that contains all the submitted bundles let current_block_number = frame_system::Pallet::::current_block_number(); let execution_inbox = ExecutionInbox::::get(( domain_id, current_block_number, current_block_number, )); - let bunde_extrinsics_roots: Vec<_> = execution_inbox + let bundles_extrinsics_roots: Vec<_> = execution_inbox .into_iter() .map(|b| b.extrinsics_root) .collect(); - assert_eq!(bunde_extrinsics_roots.len(), operator_set.len()); + assert_eq!(bundles_extrinsics_roots.len(), operator_set.len()); - target_receipt.invalid_bundles = invalid_bundles; - target_receipt.block_extrinsics_roots = bunde_extrinsics_roots; + // Prepare the invalid bundles and invalid bundle authors + let mut bundles = vec![]; + let mut invalid_bundle_authors = vec![]; + for (i, (operator, extrinsics_root)) in operator_set + .iter() + .zip(bundles_extrinsics_roots) + .enumerate() + { + if i % 2 == 0 { + invalid_bundle_authors.push(*operator); + bundles.push(InboxedBundle::invalid( + InvalidBundleType::OutOfRangeTx(0), + extrinsics_root, + )); + } else { + bundles.push(InboxedBundle::valid(H256::random(), extrinsics_root)); + } + } + target_receipt.inboxed_bundles = bundles; // Run into next block run_to_block::( diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index e0215f80f3..45a4f7b0ff 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -518,7 +518,7 @@ mod pallet { /// these bundles will be used to construct the domain block and `ExecutionInbox` is used to: /// /// 1. Ensure subsequent ERs of that domain block include all pre-validated extrinsic bundles - /// 2. Index the `InboxedBundle` and pruned its value when the corresponding `ExecutionInbox` is pruned + /// 2. Index the `InboxedBundleAuthor` and pruned its value when the corresponding `ExecutionInbox` is pruned #[pallet::storage] pub type ExecutionInbox = StorageNMap< _, @@ -535,7 +535,7 @@ mod pallet { /// the last `BlockTreePruningDepth` domain blocks. Used to verify the invalid bundle fraud proof and /// slash malicious operator who have submitted invalid bundle. #[pallet::storage] - pub(super) type InboxedBundle = + pub(super) type InboxedBundleAuthor = StorageMap<_, Identity, H256, OperatorId, OptionQuery>; /// The block number of the best domain block, increase by one when the first bundle of the domain is @@ -843,7 +843,7 @@ mod pallet { }, ); - InboxedBundle::::insert(bundle_header_hash, operator_id); + InboxedBundleAuthor::::insert(bundle_header_hash, operator_id); SuccessfulBundles::::append(domain_id, bundle_hash); @@ -1443,7 +1443,7 @@ impl Pallet { // and sign an existing bundle thus creating a duplicated bundle and pass the check. let bundle_header_hash = opaque_bundle.sealed_header.pre_hash(); ensure!( - !InboxedBundle::::contains_key(bundle_header_hash), + !InboxedBundleAuthor::::contains_key(bundle_header_hash), BundleError::DuplicatedBundle ); Ok(()) diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index 11ed4e3f31..a555940998 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -23,8 +23,8 @@ use sp_domains::fraud_proof::{ use sp_domains::merkle_tree::MerkleTree; use sp_domains::storage::RawGenesis; use sp_domains::{ - BundleHeader, DomainId, DomainsHoldIdentifier, ExecutionReceipt, OpaqueBundle, OperatorId, - OperatorPair, ProofOfElection, ReceiptHash, RuntimeType, SealedBundleHeader, + BundleHeader, DomainId, DomainsHoldIdentifier, ExecutionReceipt, InboxedBundle, OpaqueBundle, + OperatorId, OperatorPair, ProofOfElection, ReceiptHash, RuntimeType, SealedBundleHeader, StakingHoldIdentifier, }; use sp_runtime::traits::{AccountIdConversion, BlakeTwo256, Hash as HashT, IdentityLookup, Zero}; @@ -300,6 +300,10 @@ pub(crate) fn create_dummy_receipt( .into(); (execution_trace, execution_trace_root) }; + let inboxed_bundles = block_extrinsics_roots + .into_iter() + .map(InboxedBundle::dummy) + .collect(); ExecutionReceipt { domain_block_number: block_number, domain_block_hash: H256::random(), @@ -307,9 +311,7 @@ pub(crate) fn create_dummy_receipt( parent_domain_block_receipt_hash, consensus_block_number: block_number, consensus_block_hash, - valid_bundles: Vec::new(), - invalid_bundles: Vec::new(), - block_extrinsics_roots, + inboxed_bundles, final_state_root: Default::default(), execution_trace, execution_trace_root, @@ -856,18 +858,33 @@ fn test_invalid_domain_extrinsic_root_proof() { ); let bad_receipt_at = 8; - let domain_block = get_block_tree_node_at::(domain_id, bad_receipt_at).unwrap(); + let valid_bundle_digests = vec![ValidBundleDigest { + bundle_index: 0, + bundle_digest: vec![(Some(vec![1, 2, 3]), ExtrinsicDigest::Data(vec![4, 5, 6]))], + }]; + let mut domain_block = get_block_tree_node_at::(domain_id, bad_receipt_at).unwrap(); + let bad_receipt = &mut domain_block.execution_receipt; + bad_receipt.inboxed_bundles = { + valid_bundle_digests + .iter() + .map(|vbd| { + InboxedBundle::valid(BlakeTwo256::hash_of(&vbd.bundle_digest), H256::random()) + }) + .collect() + }; + bad_receipt.domain_block_extrinsic_root = H256::random(); - let bad_receipt_hash = domain_block.execution_receipt.hash(); + let bad_receipt_hash = bad_receipt.hash(); let (fraud_proof, root) = generate_invalid_domain_extrinsic_root_fraud_proof::( domain_id, bad_receipt_hash, + valid_bundle_digests, Randomness::from([1u8; 32]), 1000, ); let (consensus_block_number, consensus_block_hash) = ( - domain_block.execution_receipt.consensus_block_number, - domain_block.execution_receipt.consensus_block_hash, + bad_receipt.consensus_block_number, + bad_receipt.consensus_block_hash, ); ConsensusBlockInfo::::insert( domain_id, @@ -882,6 +899,7 @@ fn test_invalid_domain_extrinsic_root_proof() { fn generate_invalid_domain_extrinsic_root_fraud_proof( domain_id: DomainId, bad_receipt_hash: ReceiptHash, + valid_bundle_digests: Vec, randomness: Randomness, moment: Moment, ) -> (FraudProof, T::Hash>, T::Hash) { @@ -905,10 +923,6 @@ fn generate_invalid_domain_extrinsic_root_fraud_proof(backend.clone(), StorageKey(randomness_storage_key)); let (root, timestamp_storage_proof) = storage_proof_for_key::(backend, StorageKey(timestamp_storage_key)); - let valid_bundle_digests = vec![ValidBundleDigest { - bundle_index: 0, - bundle_digest: vec![(Some(vec![1, 2, 3]), ExtrinsicDigest::Data(vec![4, 5, 6]))], - }]; ( FraudProof::InvalidExtrinsicsRoot(InvalidExtrinsicsRootProof { domain_id, diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index b8315d1de4..c2162923ca 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -363,14 +363,8 @@ pub struct ExecutionReceipt { pub consensus_block_number: Number, /// The block hash corresponding to `consensus_block_number`. pub consensus_block_hash: Hash, - /// All the bundles that are included in the domain block building. - pub valid_bundles: Vec, - /// Potential bundles that are excluded from the domain block building. - pub invalid_bundles: Vec, - /// All `extrinsics_roots` for all bundles being executed by this block. - /// - /// Used to ensure these are contained within the state of the `execution_inbox`. - pub block_extrinsics_roots: Vec, + /// All the bundles that being included in the consensus block. + pub inboxed_bundles: Vec, /// The final state root for the current domain block reflected by this ER. /// /// Used for verifying storage proofs for domains. @@ -385,6 +379,38 @@ pub struct ExecutionReceipt { pub total_rewards: Balance, } +impl + ExecutionReceipt +{ + pub fn bundles_extrinsics_roots(&self) -> Vec<&ExtrinsicsRoot> { + self.inboxed_bundles + .iter() + .map(|b| &b.extrinsics_root) + .collect() + } + + pub fn valid_bundle_digests(&self) -> Vec { + self.inboxed_bundles + .iter() + .filter_map(|b| match b.bundle { + BundleValidity::Valid(bundle_digest_hash) => Some(bundle_digest_hash), + BundleValidity::Invalid(_) => None, + }) + .collect() + } + + pub fn valid_bundle_indexes(&self) -> Vec { + self.inboxed_bundles + .iter() + .enumerate() + .filter_map(|(index, b)| match b.bundle { + BundleValidity::Valid(_) => Some(index as u32), + BundleValidity::Invalid(_) => None, + }) + .collect() + } +} + impl< Number: Encode + Zero, Hash: Encode + Default, @@ -406,9 +432,7 @@ impl< parent_domain_block_receipt_hash: Default::default(), consensus_block_hash: Default::default(), consensus_block_number: Zero::zero(), - valid_bundles: Vec::new(), - invalid_bundles: Vec::new(), - block_extrinsics_roots: sp_std::vec![], + inboxed_bundles: Vec::new(), final_state_root: genesis_state_root.clone(), execution_trace: sp_std::vec![genesis_state_root], execution_trace_root: Default::default(), @@ -441,9 +465,7 @@ impl< parent_domain_block_receipt_hash, consensus_block_number, consensus_block_hash, - valid_bundles: Vec::new(), - invalid_bundles: Vec::new(), - block_extrinsics_roots: sp_std::vec![Default::default()], + inboxed_bundles: sp_std::vec![InboxedBundle::dummy(Default::default())], final_state_root: Default::default(), execution_trace, execution_trace_root, @@ -662,45 +684,90 @@ pub enum ReceiptValidity { Invalid(InvalidReceipt), } -/// Bundle invalidity type. +/// Bundle invalidity type +/// +/// Each type contains the index of the first invalid extrinsic within the bundle #[derive(Debug, Decode, Encode, TypeInfo, Clone, PartialEq, Eq)] pub enum InvalidBundleType { /// Failed to decode the opaque extrinsic. - UndecodableTx, + UndecodableTx(u32), /// Transaction is out of the tx range. - OutOfRangeTx, + OutOfRangeTx(u32), /// Transaction is illegal (unable to pay the fee, etc). - IllegalTx, + IllegalTx(u32), /// Transaction is an invalid XDM - InvalidXDM, - /// Receipt is invalid. - InvalidReceipt(InvalidReceipt), + InvalidXDM(u32), +} + +impl InvalidBundleType { + // Return the checking order of the invalid type + pub fn checking_order(&self) -> u8 { + // Use explicit number as the order instead of the enum discriminant + // to avoid chenging the order accidentally + match self { + Self::UndecodableTx(_) => 1, + Self::OutOfRangeTx(_) => 2, + Self::IllegalTx(_) => 3, + Self::InvalidXDM(_) => 4, + } + } + + pub fn extrinsic_index(&self) -> u32 { + match self { + Self::UndecodableTx(i) => *i, + Self::OutOfRangeTx(i) => *i, + Self::IllegalTx(i) => *i, + Self::InvalidXDM(i) => *i, + } + } } -/// [`InvalidBundle`] represents a bundle that was originally included in the consensus -/// block but subsequently excluded from the corresponding domain block by operator due -/// to being flagged as invalid. -#[derive(Debug, Decode, Encode, TypeInfo, Clone, PartialEq, Eq)] -pub struct InvalidBundle { - /// Index of this bundle in the original list of bundles in the consensus block. - pub bundle_index: u32, - /// Specific type of invalidity. - pub invalid_bundle_type: InvalidBundleType, +#[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] +pub enum BundleValidity { + // The invalid bundle was originally included in the consensus block but subsequently + // excluded from execution as invalid and holds the `InvalidBundleType` + Invalid(InvalidBundleType), + // The valid bundle's hash of `Vec<(tx_signer, tx_hash)>` of all domain extrinsic being + // included in the bundle. + Valid(H256), } -/// [`ValidBundle`] represents a bundle that was used when building the domain block. -#[derive(Debug, Decode, Encode, TypeInfo, Clone, PartialEq, Eq)] -pub struct ValidBundle { - /// Index of this bundle in the original list of bundles in the consensus block. - pub bundle_index: u32, - /// Hash of `Vec<(tx_signer, tx_hash)>` of all domain extrinsic being included in the bundle. - pub bundle_digest_hash: H256, +/// [`InboxedBundle`] represents a bundle that was successfully submitted to the consensus chain +#[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] +pub struct InboxedBundle { + pub bundle: BundleValidity, + pub extrinsics_root: ExtrinsicsRoot, } -#[derive(Debug, Decode, Encode, TypeInfo, Clone, PartialEq, Eq)] -pub enum BundleValidity { - Valid(Vec), - Invalid(InvalidBundleType), +impl InboxedBundle { + pub fn valid(bundle_digest_hash: H256, extrinsics_root: ExtrinsicsRoot) -> Self { + InboxedBundle { + bundle: BundleValidity::Valid(bundle_digest_hash), + extrinsics_root, + } + } + + pub fn invalid( + invalid_bundle_type: InvalidBundleType, + extrinsics_root: ExtrinsicsRoot, + ) -> Self { + InboxedBundle { + bundle: BundleValidity::Invalid(invalid_bundle_type), + extrinsics_root, + } + } + + pub fn is_invalid(&self) -> bool { + matches!(self.bundle, BundleValidity::Invalid(_)) + } + + #[cfg(any(feature = "std", feature = "runtime-benchmarks"))] + pub fn dummy(extrinsics_root: ExtrinsicsRoot) -> Self { + InboxedBundle { + bundle: BundleValidity::Valid(H256::random()), + extrinsics_root, + } + } } /// Empty extrinsics root diff --git a/crates/sp-domains/src/verification.rs b/crates/sp-domains/src/verification.rs index 8bb55aaee9..bcc0f1f46e 100644 --- a/crates/sp-domains/src/verification.rs +++ b/crates/sp-domains/src/verification.rs @@ -133,13 +133,13 @@ where } = fraud_proof; let mut bundle_extrinsics_digests = Vec::new(); - for (valid_bundle, bundle_digest) in bad_receipt - .valid_bundles + for (bad_receipt_valid_bundle_digest, bundle_digest) in bad_receipt + .valid_bundle_digests() .into_iter() .zip(valid_bundle_digests) { let bundle_digest_hash = BlakeTwo256::hash_of(&bundle_digest.bundle_digest); - if bundle_digest_hash != valid_bundle.bundle_digest_hash { + if bundle_digest_hash != bad_receipt_valid_bundle_digest { return Err(VerificationError::InvalidBundleDigest); } diff --git a/domains/client/block-preprocessor/src/lib.rs b/domains/client/block-preprocessor/src/lib.rs index 6a713bd569..d5d4f7c781 100644 --- a/domains/client/block-preprocessor/src/lib.rs +++ b/domains/client/block-preprocessor/src/lib.rs @@ -28,8 +28,8 @@ use sp_api::{HashT, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_domains::verification::deduplicate_and_shuffle_extrinsics; use sp_domains::{ - BundleValidity, DomainId, DomainsApi, DomainsDigestItem, ExecutionReceipt, ExtrinsicsRoot, - InvalidBundle, InvalidBundleType, OpaqueBundle, OpaqueBundles, ReceiptValidity, ValidBundle, + DomainId, DomainsApi, DomainsDigestItem, ExecutionReceipt, InboxedBundle, InvalidBundleType, + OpaqueBundle, OpaqueBundles, ReceiptValidity, }; use sp_messenger::MessengerApi; use sp_runtime::traits::{BlakeTwo256, Block as BlockT, Header as HeaderT, NumberFor}; @@ -47,6 +47,11 @@ type DomainBlockElements = ( MaybeNewRuntime, ); +enum BundleValidity { + Valid(Vec), + Invalid(InvalidBundleType), +} + /// Extracts the raw materials for building a new domain block from the primary block. fn prepare_domain_block_elements( domain_id: DomainId, @@ -106,9 +111,7 @@ where pub struct PreprocessResult { pub extrinsics: Vec, - pub extrinsics_roots: Vec, - pub valid_bundles: Vec, - pub invalid_bundles: Vec, + pub bundles: Vec, } pub struct DomainBlockPreprocessor { @@ -213,17 +216,12 @@ where return Ok(None); } - let extrinsics_roots = bundles - .iter() - .map(|bundle| bundle.extrinsics_root()) - .collect(); - let tx_range = self .consensus_client .runtime_api() .domain_tx_range(consensus_block_hash, self.domain_id)?; - let (valid_bundles, invalid_bundles, extrinsics) = + let (inboxed_bundles, extrinsics) = self.compile_bundles_to_extrinsics(bundles, tx_range, domain_hash)?; let extrinsics_in_bundle = deduplicate_and_shuffle_extrinsics::<::Extrinsic>( @@ -255,9 +253,7 @@ where } Ok(Some(PreprocessResult { extrinsics, - extrinsics_roots, - valid_bundles, - invalid_bundles, + bundles: inboxed_bundles, })) } @@ -270,15 +266,14 @@ where tx_range: U256, at: Block::Hash, ) -> sp_blockchain::Result<( - Vec, - Vec, + Vec, Vec<(Option, Block::Extrinsic)>, )> { - let mut invalid_bundles = Vec::with_capacity(bundles.len()); - let mut valid_bundles = Vec::with_capacity(bundles.len()); + let mut inboxed_bundles = Vec::with_capacity(bundles.len()); let mut valid_extrinsics = Vec::new(); - for (index, bundle) in bundles.into_iter().enumerate() { + for bundle in bundles { + let extrinsic_root = bundle.extrinsics_root(); match self.check_bundle_validity(&bundle, &tx_range, at)? { BundleValidity::Valid(extrinsics) => { let extrinsics: Vec<_> = match self.runtime_api.extract_signer(at, extrinsics) { @@ -292,22 +287,20 @@ where .iter() .map(|(signer, tx)| (signer.clone(), BlakeTwo256::hash_of(tx))) .collect(); - valid_bundles.push(ValidBundle { - bundle_index: index as u32, - bundle_digest_hash: BlakeTwo256::hash_of(&bundle_digest), - }); + inboxed_bundles.push(InboxedBundle::valid( + BlakeTwo256::hash_of(&bundle_digest), + extrinsic_root, + )); valid_extrinsics.extend(extrinsics); } BundleValidity::Invalid(invalid_bundle_type) => { - invalid_bundles.push(InvalidBundle { - bundle_index: index as u32, - invalid_bundle_type, - }); + inboxed_bundles + .push(InboxedBundle::invalid(invalid_bundle_type, extrinsic_root)); } } } - Ok((valid_bundles, invalid_bundles, valid_extrinsics)) + Ok((inboxed_bundles, valid_extrinsics)) } fn check_bundle_validity( @@ -322,21 +315,15 @@ where tx_range: &U256, at: Block::Hash, ) -> sp_blockchain::Result> { - // Bundles with incorrect ER are considered invalid. - if let ReceiptValidity::Invalid(invalid_receipt) = - self.receipt_validator.validate_receipt(bundle.receipt())? - { - return Ok(BundleValidity::Invalid(InvalidBundleType::InvalidReceipt( - invalid_receipt, - ))); - } - let bundle_vrf_hash = U256::from_be_bytes(bundle.sealed_header.header.proof_of_election.vrf_hash()); let mut extrinsics = Vec::with_capacity(bundle.extrinsics.len()); - for opaque_extrinsic in &bundle.extrinsics { + // Check the validity of each extrinsic + // + // NOTE: for each extrinsic the checking order must follow `InvalidBundleType::checking_order` + for (index, opaque_extrinsic) in bundle.extrinsics.iter().enumerate() { let Ok(extrinsic) = <::Extrinsic>::decode(&mut opaque_extrinsic.encode().as_slice()) else { @@ -345,7 +332,9 @@ where "Undecodable extrinsic in bundle({})", bundle.hash() ); - return Ok(BundleValidity::Invalid(InvalidBundleType::UndecodableTx)); + return Ok(BundleValidity::Invalid(InvalidBundleType::UndecodableTx( + index as u32, + ))); }; let is_within_tx_range = self.client.runtime_api().is_within_tx_range( @@ -357,7 +346,9 @@ where if !is_within_tx_range { // TODO: Generate a fraud proof for this invalid bundle - return Ok(BundleValidity::Invalid(InvalidBundleType::OutOfRangeTx)); + return Ok(BundleValidity::Invalid(InvalidBundleType::OutOfRangeTx( + index as u32, + ))); } // TODO: the `check_transaction_validity` is unimplemented @@ -369,7 +360,9 @@ where if !is_legal_tx { // TODO: Generate a fraud proof for this invalid bundle - return Ok(BundleValidity::Invalid(InvalidBundleType::IllegalTx)); + return Ok(BundleValidity::Invalid(InvalidBundleType::IllegalTx( + index as u32, + ))); } // TODO: the behavior is changed, as before invalid XDM will be dropped silently, @@ -382,7 +375,9 @@ where &extrinsic, )? { // TODO: Generate a fraud proof for this invalid bundle - return Ok(BundleValidity::Invalid(InvalidBundleType::InvalidXDM)); + return Ok(BundleValidity::Invalid(InvalidBundleType::InvalidXDM( + index as u32, + ))); } extrinsics.push(extrinsic); diff --git a/domains/client/domain-operator/src/aux_schema.rs b/domains/client/domain-operator/src/aux_schema.rs index ca143f78bc..e972c9fc77 100644 --- a/domains/client/domain-operator/src/aux_schema.rs +++ b/domains/client/domain-operator/src/aux_schema.rs @@ -6,6 +6,7 @@ use sc_client_api::backend::AuxStore; use sc_client_api::HeaderBackend; use sp_blockchain::{Error as ClientError, Result as ClientResult}; use sp_core::H256; +use sp_domains::InvalidBundleType; use sp_runtime::traits::{Block as BlockT, NumberFor, One, SaturatedConversion}; use subspace_core_primitives::BlockNumber; @@ -230,11 +231,15 @@ pub(super) fn target_receipt_is_pruned( head_receipt_number.saturating_sub(target_block) >= PRUNING_DEPTH } -// TODO: Naming could use some work #[derive(Encode, Decode, Debug, PartialEq)] -pub(super) enum InvalidBundlesMismatchType { - ValidAsInvalid, - InvalidAsValid, +pub(super) enum BundleMismatchType { + // The invalid bundle is mismatch + // For `TrueInvalid`, the fraud proof need to prove the bundle is indees invalid due to `InvalidBundleType` + // For `FalseInvalid`, the fraud proof need to prove the bundle is not invalid due to `InvalidBundleType` + TrueInvalid(InvalidBundleType), + FalseInvalid(InvalidBundleType), + // The valid bundle is mismatch + Valid, } #[derive(Encode, Decode, Debug, PartialEq)] @@ -249,8 +254,8 @@ pub(super) enum ReceiptMismatchInfo { DomainExtrinsicsRoot { consensus_block_hash: CHash, }, - InvalidBundles { - mismatch_type: InvalidBundlesMismatchType, + Bundles { + mismatch_type: BundleMismatchType, bundle_index: u32, consensus_block_hash: CHash, }, @@ -275,7 +280,7 @@ impl ReceiptMismatchInfo { consensus_block_hash, .. } => consensus_block_hash.clone(), - ReceiptMismatchInfo::InvalidBundles { + ReceiptMismatchInfo::Bundles { consensus_block_hash, .. } => consensus_block_hash.clone(), @@ -515,9 +520,7 @@ mod tests { parent_domain_block_receipt_hash: H256::random(), consensus_block_number, consensus_block_hash: H256::random(), - valid_bundles: Vec::new(), - invalid_bundles: Vec::new(), - block_extrinsics_roots: Default::default(), + inboxed_bundles: Vec::new(), final_state_root: Default::default(), execution_trace: Default::default(), execution_trace_root: Default::default(), diff --git a/domains/client/domain-operator/src/bundle_processor.rs b/domains/client/domain-operator/src/bundle_processor.rs index 5fd67adaa0..f76c5fce7e 100644 --- a/domains/client/domain-operator/src/bundle_processor.rs +++ b/domains/client/domain-operator/src/bundle_processor.rs @@ -13,7 +13,7 @@ use sp_consensus::BlockOrigin; use sp_core::traits::CodeExecutor; use sp_core::H256; use sp_domain_digests::AsPredigest; -use sp_domains::{DomainId, DomainsApi, InvalidReceipt, ReceiptValidity}; +use sp_domains::{DomainId, DomainsApi, ReceiptValidity}; use sp_keystore::KeystorePtr; use sp_messenger::MessengerApi; use sp_runtime::traits::{Block as BlockT, Zero}; @@ -108,7 +108,7 @@ where } let consensus_block_hash = receipt.consensus_block_hash; - let local_receipt = crate::aux_schema::load_execution_receipt::<_, Block, CBlock>( + let _local_receipt = crate::aux_schema::load_execution_receipt::<_, Block, CBlock>( &*self.client, consensus_block_hash, )? @@ -118,11 +118,6 @@ where )) })?; - if local_receipt.invalid_bundles != receipt.invalid_bundles { - // TODO: Generate fraud proof - return Ok(ReceiptValidity::Invalid(InvalidReceipt::InvalidBundles)); - } - Ok(ReceiptValidity::Valid) } } diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index 75cc234e48..7749030ce7 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/domains/client/domain-operator/src/domain_block_processor.rs @@ -1,4 +1,4 @@ -use crate::aux_schema::{InvalidBundlesMismatchType, ReceiptMismatchInfo}; +use crate::aux_schema::{BundleMismatchType, ReceiptMismatchInfo}; use crate::fraud_proof::{find_trace_mismatch, FraudProofGenerator}; use crate::parent_chain::ParentChainInterface; use crate::utils::{DomainBlockImportNotification, DomainImportNotificationSinks}; @@ -19,7 +19,7 @@ use sp_core::traits::CodeExecutor; use sp_core::H256; use sp_domains::fraud_proof::FraudProof; use sp_domains::merkle_tree::MerkleTree; -use sp_domains::{DomainId, DomainsApi, ExecutionReceipt}; +use sp_domains::{BundleValidity, DomainId, DomainsApi, ExecutionReceipt}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, One, Zero}; use sp_runtime::Digest; use std::cmp::Ordering; @@ -275,9 +275,7 @@ where ) -> Result, sp_blockchain::Error> { let PreprocessResult { extrinsics, - extrinsics_roots, - valid_bundles, - invalid_bundles, + bundles, } = preprocess_result; // Although the domain block intuitively ought to use the same fork choice @@ -374,9 +372,7 @@ where parent_domain_block_receipt_hash: parent_receipt.hash(), consensus_block_number, consensus_block_hash, - valid_bundles, - invalid_bundles, - block_extrinsics_roots: extrinsics_roots, + inboxed_bundles: bundles, final_state_root: state_root, execution_trace: trace, execution_trace_root: sp_core::H256(trace_root), @@ -548,87 +544,87 @@ where } } -/// Verifies invalid_bundle field in the ER and initializes receipt mismatch info accordingly. The `ReceiptMismatchInfo` -/// refers to the first mismatch -/// CONTRACT: It will return None if the field is valid, otherwise it will return `Some` with receipt mismatch info -/// pointing to first mismatch in invalid bundles array. -pub(crate) fn verify_invalid_bundles_field( +// Find the first mismatch of the `InboxedBundle` in the `ER::bundles` list +pub(crate) fn find_inboxed_bundles_mismatch( local_receipt: &ExecutionReceiptFor, external_receipt: &ExecutionReceiptFor, -) -> Option> +) -> Result>, sp_blockchain::Error> where Block: BlockT, CBlock: BlockT, { - if local_receipt.invalid_bundles == external_receipt.invalid_bundles { - return None; + if local_receipt.inboxed_bundles == external_receipt.inboxed_bundles { + return Ok(None); } - for (local_invalid_bundle, external_invalid_bundle) in local_receipt - .invalid_bundles + + // The `bundles_extrinsics_roots` should be checked in the runtime when the consensus block is + // constructed/imported thus the `external_receipt` must have the same `bundles_extrinsics_roots` + // + // NOTE: this also check `local_receipt.inboxed_bundles` and `external_receipt.inboxed_bundles` + // have the same length + if local_receipt.bundles_extrinsics_roots() != external_receipt.bundles_extrinsics_roots() { + return Err(sp_blockchain::Error::Application(format!( + "Found mismatch of `ER::bundles_extrinsics_roots`, this should not happen, local: {:?}, external: {:?}", + local_receipt.bundles_extrinsics_roots(), + external_receipt.bundles_extrinsics_roots(), + ).into())); + } + + // Get the first mismatch of `ER::bundles` + let (bundle_index, (local_bundle, external_bundle)) = local_receipt + .inboxed_bundles .iter() - .zip(external_receipt.invalid_bundles.iter()) - { - if local_invalid_bundle != external_invalid_bundle { - if local_invalid_bundle.invalid_bundle_type - != external_invalid_bundle.invalid_bundle_type - { - // Missing invalid bundle entry fraud proof can work for invalid bundle type mismatch - // as the proof can prove that particular bundle is invalid as well as type of invalidation. - return Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::InvalidAsValid, - bundle_index: local_invalid_bundle.bundle_index, - consensus_block_hash: local_receipt.consensus_block_hash, - }); - } - // FIXME: we need to add a check to the consensus chain runtime to ensure for all the ER included in the consensus block - // the `bundle_index` field of `ER.invalid_bundles` must be strictly increasing - match local_invalid_bundle - .bundle_index - .cmp(&external_invalid_bundle.bundle_index) + .zip(external_receipt.inboxed_bundles.iter()) + .enumerate() + .find(|(_, (local_bundle, external_bundle))| local_bundle != external_bundle) + .expect( + "The `local_receipt.inboxed_bundles` and `external_receipt.inboxed_bundles` are checked to have the \ + same length and being non-equal; qed", + ); + + // The `local_bundle` and `external_bundle` are checked to have the same `extrinsic_root` + // thus they must being mismatch due to bundle validity + let mismatch_type = match (local_bundle.bundle.clone(), external_bundle.bundle.clone()) { + ( + BundleValidity::Invalid(local_invalid_type), + BundleValidity::Invalid(external_invalid_type), + ) => { + match local_invalid_type + .extrinsic_index() + .cmp(&external_invalid_type.extrinsic_index()) { - Ordering::Greater => { - return Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::ValidAsInvalid, - bundle_index: external_invalid_bundle.bundle_index, - consensus_block_hash: local_receipt.consensus_block_hash, - }); - } - Ordering::Less => { - return Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::InvalidAsValid, - bundle_index: local_invalid_bundle.bundle_index, - consensus_block_hash: local_receipt.consensus_block_hash, - }); - } - Ordering::Equal => unreachable!("checked in this block's if condition; qed"), + // A bundle can contains multiple invalid extrinsics thus consider the first invalid extrinsic + // as the mismatch. + Ordering::Less => BundleMismatchType::TrueInvalid(local_invalid_type), + Ordering::Greater => BundleMismatchType::FalseInvalid(external_invalid_type), + // If both the `local_invalid_type` and `external_invalid_type` point to the same extrinsic, + // the extrinsic can be considered as invalid due to multiple `invalid_type` (i.e. an extrinsic + // can be `OutOfRangeTx` and `InvalidXDM` at the same time) thus use the checking order and + // consider the first check as the mismatch. + Ordering::Equal => match local_invalid_type.checking_order().cmp(&external_invalid_type.checking_order()) { + Ordering::Less => BundleMismatchType::TrueInvalid(local_invalid_type), + Ordering::Greater => BundleMismatchType::FalseInvalid(external_invalid_type), + Ordering::Equal => unreachable!( + "bundle validity must be different as the local/external bundle are checked to be different \ + and they have the same `extrinsic_root`" + ), + }, } } - } - match local_receipt - .invalid_bundles - .len() - .cmp(&external_receipt.invalid_bundles.len()) - { - Ordering::Greater => { - let invalid_bundle = - &local_receipt.invalid_bundles[external_receipt.invalid_bundles.len()]; - Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::InvalidAsValid, - bundle_index: invalid_bundle.bundle_index, - consensus_block_hash: local_receipt.consensus_block_hash - }) + (BundleValidity::Valid(_), BundleValidity::Valid(_)) => BundleMismatchType::Valid, + (BundleValidity::Valid(_), BundleValidity::Invalid(invalid_type)) => { + BundleMismatchType::FalseInvalid(invalid_type) } - Ordering::Less => { - let valid_bundle = - &external_receipt.invalid_bundles[local_receipt.invalid_bundles.len()]; - Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::ValidAsInvalid, - bundle_index: valid_bundle.bundle_index, - consensus_block_hash: local_receipt.consensus_block_hash - }) + (BundleValidity::Invalid(invalid_type), BundleValidity::Valid(_)) => { + BundleMismatchType::TrueInvalid(invalid_type) } - Ordering::Equal => unreachable!("already checked for vector equality and since the zipped elements are equal, length cannot be equal; qed"), - } + }; + + Ok(Some(ReceiptMismatchInfo::Bundles { + mismatch_type, + bundle_index: bundle_index as u32, + consensus_block_hash: local_receipt.consensus_block_hash, + })) } pub(crate) struct ReceiptsChecker< @@ -764,10 +760,10 @@ where execution_receipt.consensus_block_number )))?; - if let Some(receipt_mismatch_info) = verify_invalid_bundles_field::< + if let Some(receipt_mismatch_info) = find_inboxed_bundles_mismatch::< Block, ParentChainBlock, - >(&local_receipt, execution_receipt) + >(&local_receipt, execution_receipt)? { bad_receipts_to_write.push(( execution_receipt.consensus_block_number, @@ -921,24 +917,32 @@ where "Failed to generate invalid block rewards fraud proof: {err}" ))) })?, - ReceiptMismatchInfo::InvalidBundles { + ReceiptMismatchInfo::Bundles { mismatch_type, bundle_index, .. - } => self - .fraud_proof_generator - .generate_invalid_bundle_field_proof::( - self.domain_id, - &local_receipt, - mismatch_type, - bundle_index, - bad_receipt_hash, - ) - .map_err(|err| { - sp_blockchain::Error::Application(Box::from(format!( - "Failed to generate invalid bundles field fraud proof: {err}" - ))) - })?, + } => { + match mismatch_type { + BundleMismatchType::Valid => { + // TODO: generate valid bundle fraud proof + return Ok(None); + } + _ => self + .fraud_proof_generator + .generate_invalid_bundle_field_proof::( + self.domain_id, + &local_receipt, + mismatch_type, + bundle_index, + bad_receipt_hash, + ) + .map_err(|err| { + sp_blockchain::Error::Application(Box::from(format!( + "Failed to generate invalid bundles field fraud proof: {err}" + ))) + })?, + } + } ReceiptMismatchInfo::DomainExtrinsicsRoot { .. } => self .fraud_proof_generator .generate_invalid_domain_extrinsics_root_proof::( @@ -964,12 +968,12 @@ where mod tests { use super::*; use domain_test_service::evm_domain_test_runtime::Block; - use sp_core::sp_std; - use sp_domains::{ExecutionReceipt, InvalidBundle, InvalidBundleType}; + use sp_core::{sp_std, H256}; + use sp_domains::{ExecutionReceipt, InboxedBundle, InvalidBundleType}; use subspace_test_runtime::Block as CBlock; fn create_test_execution_receipt( - invalid_bundles: Vec, + inboxed_bundles: Vec, ) -> ExecutionReceiptFor where Block: BlockT, @@ -982,130 +986,214 @@ mod tests { parent_domain_block_receipt_hash: Default::default(), consensus_block_hash: Default::default(), consensus_block_number: Zero::zero(), - invalid_bundles, - block_extrinsics_roots: sp_std::vec![], + inboxed_bundles, final_state_root: Default::default(), execution_trace: sp_std::vec![], execution_trace_root: Default::default(), total_rewards: Zero::zero(), - valid_bundles: vec![], } } #[test] - fn invalid_bundles_fraud_proof_detection() { + fn er_bundles_mismatch_detection() { // If empty invalid receipt field on both should result in no fraud proof assert_eq!( - verify_invalid_bundles_field::( + find_inboxed_bundles_mismatch::( &create_test_execution_receipt(vec![]), &create_test_execution_receipt(vec![]), - ), + ) + .unwrap(), None ); assert_eq!( - verify_invalid_bundles_field::( - &create_test_execution_receipt(vec![InvalidBundle { - bundle_index: 3, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }]), - &create_test_execution_receipt(vec![InvalidBundle { - bundle_index: 3, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }]), - ), + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![InboxedBundle::invalid( + InvalidBundleType::UndecodableTx(0), + Default::default() + )]), + &create_test_execution_receipt(vec![InboxedBundle::invalid( + InvalidBundleType::UndecodableTx(0), + Default::default() + )]), + ) + .unwrap(), None ); - // Mismatch in invalid bundle type + // Mismatch in valid bundle assert_eq!( - verify_invalid_bundles_field::( + find_inboxed_bundles_mismatch::( &create_test_execution_receipt(vec![ - InvalidBundle { - bundle_index: 3, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }, - InvalidBundle { - bundle_index: 4, - invalid_bundle_type: InvalidBundleType::UndecodableTx - } + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(0), Default::default()), + InboxedBundle::valid(H256::random(), Default::default()), ]), &create_test_execution_receipt(vec![ - InvalidBundle { - bundle_index: 3, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }, - InvalidBundle { - bundle_index: 4, - invalid_bundle_type: InvalidBundleType::IllegalTx - } + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(0), Default::default()), + InboxedBundle::valid(H256::random(), Default::default()), ]), - ), - Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::InvalidAsValid, - bundle_index: 4, + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::Valid, + bundle_index: 1, consensus_block_hash: Default::default() }) ); - // Only first mismatch is detected + // Mismatch in invalid extrinsic index assert_eq!( - verify_invalid_bundles_field::( + find_inboxed_bundles_mismatch::( &create_test_execution_receipt(vec![ - InvalidBundle { - bundle_index: 1, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }, - InvalidBundle { - bundle_index: 4, - invalid_bundle_type: InvalidBundleType::UndecodableTx - } + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(1), Default::default()), ]), &create_test_execution_receipt(vec![ - InvalidBundle { - bundle_index: 3, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }, - InvalidBundle { - bundle_index: 4, - invalid_bundle_type: InvalidBundleType::IllegalTx - } + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(2), Default::default()), + ]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::TrueInvalid(InvalidBundleType::UndecodableTx(1)), + bundle_index: 1, + consensus_block_hash: Default::default() + }) + ); + assert_eq!( + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![ + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(4), Default::default()), + ]), + &create_test_execution_receipt(vec![ + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(3), Default::default()), + ]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::FalseInvalid(InvalidBundleType::UndecodableTx( + 3 + )), + bundle_index: 1, + consensus_block_hash: Default::default() + }) + ); + // Even the invalid type is mismatch, the extrinsic index mismatch should be considered first + assert_eq!( + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![ + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::UndecodableTx(4), Default::default()), + ]), + &create_test_execution_receipt(vec![ + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::IllegalTx(3), Default::default()), ]), - ), - Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::InvalidAsValid, + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::FalseInvalid(InvalidBundleType::IllegalTx(3)), bundle_index: 1, consensus_block_hash: Default::default() }) ); - // Valid bundle as invalid + // Mismatch in invalid type assert_eq!( - verify_invalid_bundles_field::( + find_inboxed_bundles_mismatch::( &create_test_execution_receipt(vec![ - InvalidBundle { - bundle_index: 5, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }, - InvalidBundle { - bundle_index: 6, - invalid_bundle_type: InvalidBundleType::UndecodableTx - } + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::IllegalTx(3), Default::default()), ]), &create_test_execution_receipt(vec![ - InvalidBundle { - bundle_index: 3, - invalid_bundle_type: InvalidBundleType::UndecodableTx - }, - InvalidBundle { - bundle_index: 4, - invalid_bundle_type: InvalidBundleType::IllegalTx - } + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::InvalidXDM(3), Default::default()), + ]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::TrueInvalid(InvalidBundleType::IllegalTx(3)), + bundle_index: 1, + consensus_block_hash: Default::default() + }) + ); + assert_eq!( + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![ + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::InvalidXDM(3), Default::default()), ]), - ), - Some(ReceiptMismatchInfo::InvalidBundles { - mismatch_type: InvalidBundlesMismatchType::ValidAsInvalid, - bundle_index: 3, + &create_test_execution_receipt(vec![ + InboxedBundle::valid(Default::default(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::IllegalTx(3), Default::default()), + ]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::FalseInvalid(InvalidBundleType::IllegalTx(3)), + bundle_index: 1, + consensus_block_hash: Default::default() + }) + ); + + // Only first mismatch is detected + assert_eq!( + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![ + InboxedBundle::valid(H256::random(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::InvalidXDM(3), Default::default()), + ]), + &create_test_execution_receipt(vec![ + InboxedBundle::valid(H256::random(), Default::default()), + InboxedBundle::invalid(InvalidBundleType::IllegalTx(3), Default::default()), + ]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::Valid, + bundle_index: 0, + consensus_block_hash: Default::default() + }) + ); + + // Taking valid bundle as invalid + assert_eq!( + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![InboxedBundle::valid( + H256::random(), + Default::default() + ),]), + &create_test_execution_receipt(vec![InboxedBundle::invalid( + InvalidBundleType::IllegalTx(3), + Default::default() + ),]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::FalseInvalid(InvalidBundleType::IllegalTx(3)), + bundle_index: 0, + consensus_block_hash: Default::default() + }) + ); + + // Taking invalid as valid + assert_eq!( + find_inboxed_bundles_mismatch::( + &create_test_execution_receipt(vec![InboxedBundle::invalid( + InvalidBundleType::IllegalTx(3), + Default::default() + ),]), + &create_test_execution_receipt(vec![InboxedBundle::valid( + H256::random(), + Default::default() + ),]), + ) + .unwrap(), + Some(ReceiptMismatchInfo::Bundles { + mismatch_type: BundleMismatchType::TrueInvalid(InvalidBundleType::IllegalTx(3)), + bundle_index: 0, consensus_block_hash: Default::default() }) ); diff --git a/domains/client/domain-operator/src/fraud_proof.rs b/domains/client/domain-operator/src/fraud_proof.rs index f07ec09286..d44254a870 100644 --- a/domains/client/domain-operator/src/fraud_proof.rs +++ b/domains/client/domain-operator/src/fraud_proof.rs @@ -1,4 +1,4 @@ -use crate::aux_schema::InvalidBundlesMismatchType; +use crate::aux_schema::BundleMismatchType; use crate::utils::to_number_primitive; use crate::ExecutionReceiptFor; use codec::{Decode, Encode}; @@ -126,7 +126,7 @@ where &self, domain_id: DomainId, _local_receipt: &ExecutionReceiptFor, - mismatch_type: InvalidBundlesMismatchType, + mismatch_type: BundleMismatchType, bundle_index: u32, _bad_receipt_hash: H256, ) -> Result, PCB::Hash>, FraudProofError> @@ -135,18 +135,24 @@ where { match mismatch_type { // TODO: Generate a proper proof once fields are in place - InvalidBundlesMismatchType::ValidAsInvalid => Ok(FraudProof::InvalidBundles( + BundleMismatchType::TrueInvalid(_invalid_type) => Ok(FraudProof::InvalidBundles( InvalidBundlesFraudProof::ValidAsInvalid(ValidAsInvalidBundleEntryFraudProof::new( domain_id, bundle_index, )), )), // TODO: Generate a proper proof once fields are in place - InvalidBundlesMismatchType::InvalidAsValid => Ok(FraudProof::InvalidBundles( + BundleMismatchType::FalseInvalid(_invalid_type) => Ok(FraudProof::InvalidBundles( InvalidBundlesFraudProof::MissingInvalidBundleEntry( MissingInvalidBundleEntryFraudProof::new(domain_id, bundle_index), ), )), + BundleMismatchType::Valid => Err(sp_blockchain::Error::Application( + "Unexpected bundle mismatch type, this should not happen" + .to_string() + .into(), + ) + .into()), } } @@ -175,9 +181,8 @@ where .extract_successful_bundles(consensus_block_hash, domain_id, consensus_extrinsics)?; let domain_runtime_api = self.client.runtime_api(); - let mut valid_bundle_digests = Vec::with_capacity(local_receipt.valid_bundles.len()); - for valid_bundle in local_receipt.valid_bundles.iter() { - let bundle_index = valid_bundle.bundle_index; + let mut valid_bundle_digests = Vec::new(); + for bundle_index in local_receipt.valid_bundle_indexes() { let bundle = bundles .get(bundle_index as usize)