Merge the valid_bundles, invalid_bundles, and block_extrinsics_roots fields of the ER into one bundles field and add more precise fraud detection

crates/pallet-domains/src/block_tree.rs

@@ -123,7 +123,7 @@ pub(crate) fn verify_execution_receipt<T: Config>(
         consensus_block_number,
         consensus_block_hash,
         domain_block_number,
-        block_extrinsics_roots,
+        bundles,
         parent_domain_block_receipt_hash,
         execution_trace,
         execution_trace_root,
@@ -138,15 +138,14 @@ pub(crate) fn verify_execution_receipt<T: Config>(
             Error::BadGenesisReceipt
         );
     } else {
+        let bundles_extrinsics_roots: Vec<_> = bundles.iter().map(|b| b.extrinsics_root).collect();
         let execution_inbox =
             ExecutionInbox::<T>::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
         );
 
@@ -298,11 +297,9 @@ pub(crate) fn process_execution_receipt<T: Config>(
                 ));
                 for (index, bd) in bundle_digests.into_iter().enumerate() {
                     if let Some(bundle_author) = InboxedBundleAuthor::<T>::take(bd.header_hash) {
-                        if execution_receipt
-                            .invalid_bundles
-                            .iter()
-                            .any(|ib| ib.bundle_index == index as u32)
-                        {
+                        // 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.bundles[index].is_invalid() {
                             invalid_bundle_authors.push(bundle_author);
                         }
                     }
@@ -409,7 +406,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]
@@ -743,12 +740,12 @@ mod tests {
                     future_receipt.consensus_block_number,
                 ),
                 future_receipt
-                    .block_extrinsics_roots
+                    .bundles
                     .clone()
                     .into_iter()
-                    .map(|er| BundleDigest {
+                    .map(|b| BundleDigest {
                         header_hash: H256::random(),
-                        extrinsics_root: er,
+                        extrinsics_root: b.extrinsics_root,
                     })
                     .collect::<Vec<_>>(),
             );
@@ -776,7 +773,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.bundles =
+                vec![InboxedBundle::valid(H256::random(), H256::random())];
             assert_err!(
                 verify_execution_receipt::<Test>(domain_id, &unknown_extrinsics_roots_receipt),
                 Error::InvalidExtrinsicsRoots
@@ -883,36 +881,38 @@ mod tests {
             let head_node = get_block_tree_node_at::<Test>(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::<Test>::current_block_number();
             let execution_inbox = ExecutionInbox::<Test>::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.bundles = bundles;
 
             // Run into next block
             run_to_block::<Test>(

crates/pallet-domains/src/tests.rs

@@ -23,8 +23,9 @@ use sp_domains::fraud_proof::{
 use sp_domains::merkle_tree::MerkleTree;
 use sp_domains::{
     BundleHeader, DomainId, DomainInstanceData, DomainsHoldIdentifier, ExecutionReceipt,
-    GenerateGenesisStateRoot, GenesisReceiptExtension, OpaqueBundle, OperatorId, OperatorPair,
-    ProofOfElection, ReceiptHash, RuntimeType, SealedBundleHeader, StakingHoldIdentifier,
+    GenerateGenesisStateRoot, GenesisReceiptExtension, InboxedBundle, OpaqueBundle, OperatorId,
+    OperatorPair, ProofOfElection, ReceiptHash, RuntimeType, SealedBundleHeader,
+    StakingHoldIdentifier,
 };
 use sp_runtime::traits::{AccountIdConversion, BlakeTwo256, Hash as HashT, IdentityLookup, Zero};
 use sp_runtime::{BuildStorage, OpaqueExtrinsic};
@@ -304,16 +305,18 @@ pub(crate) fn create_dummy_receipt(
             .into();
         (execution_trace, execution_trace_root)
     };
+    let bundles = block_extrinsics_roots
+        .into_iter()
+        .map(InboxedBundle::dummy)
+        .collect();
     ExecutionReceipt {
         domain_block_number: block_number,
         domain_block_hash: H256::random(),
         domain_block_extrinsic_root: Default::default(),
         parent_domain_block_receipt_hash,
         consensus_block_number: block_number,
         consensus_block_hash,
-        valid_bundles: Vec::new(),
-        invalid_bundles: Vec::new(),
-        block_extrinsics_roots,
+        bundles,
         final_state_root: Default::default(),
         execution_trace,
         execution_trace_root,

crates/sp-domains/src/lib.rs

@@ -364,14 +364,8 @@ pub struct ExecutionReceipt<Number, Hash, DomainNumber, DomainHash, Balance> {
     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<ValidBundle>,
-    /// Potential bundles that are excluded from the domain block building.
-    pub invalid_bundles: Vec<InvalidBundle>,
-    /// 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<ExtrinsicsRoot>,
+    /// All the bundles that being included in the consensus block.
+    pub bundles: Vec<InboxedBundle>,
     /// The final state root for the current domain block reflected by this ER.
     ///
     /// Used for verifying storage proofs for domains.
@@ -386,6 +380,35 @@ pub struct ExecutionReceipt<Number, Hash, DomainNumber, DomainHash, Balance> {
     pub total_rewards: Balance,
 }
 
+impl<Number, Hash, DomainNumber, DomainHash, Balance>
+    ExecutionReceipt<Number, Hash, DomainNumber, DomainHash, Balance>
+{
+    pub fn bundles_extrinsics_roots(&self) -> Vec<&ExtrinsicsRoot> {
+        self.bundles.iter().map(|b| &b.extrinsics_root).collect()
+    }
+
+    pub fn valid_bundle_digests(&self) -> Vec<H256> {
+        self.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<u32> {
+        self.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,
@@ -407,9 +430,7 @@ impl<
             parent_domain_block_receipt_hash: Default::default(),
             consensus_block_hash: consensus_genesis_hash,
             consensus_block_number: Zero::zero(),
-            valid_bundles: Vec::new(),
-            invalid_bundles: Vec::new(),
-            block_extrinsics_roots: sp_std::vec![],
+            bundles: Vec::new(),
             final_state_root: genesis_state_root.clone(),
             execution_trace: sp_std::vec![genesis_state_root],
             execution_trace_root: Default::default(),
@@ -442,9 +463,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()],
+            bundles: sp_std::vec![InboxedBundle::dummy(Default::default())],
             final_state_root: Default::default(),
             execution_trace,
             execution_trace_root,
@@ -701,39 +720,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 due to being flagged as invalid of `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,
+}
+
+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

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);
         }