Re-define the invalid bundle type checking order to resolve the confl…

…ict extrinsic index usage for invalid bundle weight FP

Signed-off-by: linning <[email protected]>

crates/sp-domains-fraud-proof/src/verification.rs

@@ -456,13 +456,9 @@ where
             // think it is valid
             BundleValidity::Valid(_) => true,
             BundleValidity::Invalid(invalid_type) => {
-                // The proof trying to prove there is an invalid extrinsic that the `bad_receipt_bundle` think is valid,
-                // so the proof should point to an extrinsic that in front of the `bad_receipt_bundle`'s
-                invalid_bundle_type.extrinsic_index() < invalid_type.extrinsic_index() ||
-                    // The proof trying to prove the invalid extrinsic can not pass a check that the `bad_receipt_bundle` think it can,
-                    // so the proof should point to the same extrinsic and a check that perform before the `bad_receipt_bundle`'s
-                    (invalid_bundle_type.extrinsic_index() == invalid_type.extrinsic_index()
-                        && invalid_bundle_type.checking_order() < invalid_type.checking_order())
+                // The proof trying to prove there is a check failed while the `bad_receipt` think is pass,
+                // so the proof should point to a check that is performed before the `bad_receipt`'s
+                invalid_bundle_type.checking_order() < invalid_type.checking_order()
             }
         }
     };

crates/sp-domains/src/lib.rs

@@ -1077,20 +1077,10 @@ pub enum InvalidBundleType {
 
 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 changing the order accidentally
-        match self {
-            Self::UndecodableTx(_) => 1,
-            Self::OutOfRangeTx(_) => 2,
-            Self::InherentExtrinsic(_) => 3,
-            Self::IllegalTx(_) => 5,
-            Self::InvalidBundleWeight => 6,
-        }
-    }
-
-    pub fn extrinsic_index(&self) -> u32 {
-        match self {
+    pub fn checking_order(&self) -> u64 {
+        // A bundle can contains multiple invalid extrinsics thus consider the first invalid extrinsic
+        // as the invalid type
+        let extrinsic_order = match self {
             Self::UndecodableTx(i) => *i,
             Self::OutOfRangeTx(i) => *i,
             Self::IllegalTx(i) => *i,
@@ -1100,6 +1090,40 @@ impl InvalidBundleType {
             // in the bundle returning `u32::MAX` indicate `InvalidBundleWeight` is checked after
             // all the extrinsic in the bundle is checked.
             Self::InvalidBundleWeight => u32::MAX,
+        };
+
+        // The extrinsic can be considered as invalid due to multiple `invalid_type` (i.e. an extrinsic
+        // can be `OutOfRangeTx` and `IllegalTx` at the same time) thus use the following checking order
+        // and consider the first check as the invalid type
+        //
+        // NOTE: Use explicit number as the order instead of the enum discriminant
+        // to avoid changing the order accidentally
+        let rule_order = match self {
+            Self::UndecodableTx(_) => 1,
+            Self::OutOfRangeTx(_) => 2,
+            Self::InherentExtrinsic(_) => 3,
+            Self::IllegalTx(_) => 5,
+            Self::InvalidBundleWeight => 6,
+        };
+
+        // The checking order is a combination of the `extrinsic_order` and `rule_order`
+        // it presents as an `u64` where the first 32 bits is the `extrinsic_order` and
+        // last 32 bits is the `rule_order` meaning the `extrinsic_order` is checked first
+        // then the `rule_order`.
+        ((extrinsic_order as u64) << 32) | (rule_order as u64)
+    }
+
+    // Return the index of the extrinsic that the invalid type points to
+    //
+    // NOTE: `InvalidBundleWeight` will return `None` since it is targetting the whole bundle not a
+    // specific single extrinsic
+    pub fn extrinsic_index(&self) -> Option<u32> {
+        match self {
+            Self::UndecodableTx(i) => Some(*i),
+            Self::OutOfRangeTx(i) => Some(*i),
+            Self::IllegalTx(i) => Some(*i),
+            Self::InherentExtrinsic(i) => Some(*i),
+            Self::InvalidBundleWeight => None,
         }
     }
 }
@@ -1144,9 +1168,7 @@ impl<Hash> InboxedBundle<Hash> {
 
     pub fn invalid_extrinsic_index(&self) -> Option<u32> {
         match &self.bundle {
-            BundleValidity::Invalid(invalid_bundle_type) => {
-                Some(invalid_bundle_type.extrinsic_index())
-            }
+            BundleValidity::Invalid(invalid_bundle_type) => invalid_bundle_type.extrinsic_index(),
             BundleValidity::Valid(_) => None,
         }
     }

crates/sp-domains/src/tests.rs

@@ -1,4 +1,4 @@
-use crate::{signer_in_tx_range, EMPTY_EXTRINSIC_ROOT};
+use crate::{signer_in_tx_range, InvalidBundleType, EMPTY_EXTRINSIC_ROOT};
 use num_traits::ops::wrapping::{WrappingAdd, WrappingSub};
 use parity_scale_codec::Encode;
 use sp_runtime::traits::{BlakeTwo256, Hash};
@@ -222,3 +222,58 @@ fn test_empty_extrinsic_root() {
     );
     assert_eq!(root, EMPTY_EXTRINSIC_ROOT);
 }
+
+#[test]
+fn test_invalid_bundle_type_checking_order() {
+    fn invalid_type(extrinsic_index: u32, rule_order: u32) -> InvalidBundleType {
+        match rule_order {
+            1 => InvalidBundleType::UndecodableTx(extrinsic_index),
+            2 => InvalidBundleType::OutOfRangeTx(extrinsic_index),
+            3 => InvalidBundleType::InherentExtrinsic(extrinsic_index),
+            5 => InvalidBundleType::IllegalTx(extrinsic_index),
+            6 => InvalidBundleType::InvalidBundleWeight,
+            _ => unreachable!(),
+        }
+    }
+
+    // The checking order is a combination of the `extrinsic_order` and `rule_order`
+    // it presents as an `u64` where the first 32 bits is the `extrinsic_order` and
+    // last 32 bits is the `rule_order` meaning the `extrinsic_order` is checked first
+    // then the `rule_order`.
+    assert_eq!(invalid_type(0, 1).checking_order(), 1);
+    assert_eq!(invalid_type(0, 2).checking_order(), 2);
+    assert_eq!(invalid_type(0, 3).checking_order(), 3);
+    assert_eq!(invalid_type(0, 5).checking_order(), 5);
+    assert_eq!(
+        invalid_type(1, 1).checking_order(),
+        (u32::MAX as u64 + 1) + 1
+    );
+    assert_eq!(
+        invalid_type(12345, 2).checking_order(),
+        (12345u64 * (u32::MAX as u64 + 1)) + 2
+    );
+    assert_eq!(
+        invalid_type(u32::MAX - 1, 3).checking_order(),
+        ((u32::MAX - 1) as u64 * (u32::MAX as u64 + 1)) + 3
+    );
+    assert_eq!(
+        invalid_type(u32::MAX, 5).checking_order(),
+        (u32::MAX as u64 * (u32::MAX as u64 + 1)) + 5
+    );
+    assert_eq!(
+        InvalidBundleType::InvalidBundleWeight.checking_order(),
+        // The extrinsic index of `InvalidBundleWeight` is `u32::MAX`
+        (u32::MAX as u64 * (u32::MAX as u64 + 1)) + 6
+    );
+
+    // The `extrinsic_order` is checked first then the `rule_order`
+    assert!(invalid_type(0, 1).checking_order() < invalid_type(0, 2).checking_order());
+    assert!(invalid_type(1, 1).checking_order() < invalid_type(1, 2).checking_order());
+    assert!(invalid_type(0, 1).checking_order() < invalid_type(1, 1).checking_order());
+    assert!(invalid_type(0, 2).checking_order() < invalid_type(1, 1).checking_order());
+    assert!(invalid_type(0, 1).checking_order() < invalid_type(1, 2).checking_order());
+    assert_eq!(
+        invalid_type(9876, 5).checking_order(),
+        invalid_type(9876, 5).checking_order()
+    );
+}

domains/client/domain-operator/src/domain_block_processor.rs

@@ -644,28 +644,19 @@ where
             BundleValidity::Invalid(external_invalid_type),
         ) => {
             match local_invalid_type
-                .extrinsic_index()
-                .cmp(&external_invalid_type.extrinsic_index())
+                .checking_order()
+                .cmp(&external_invalid_type.checking_order())
             {
-                // A bundle can contains multiple invalid extrinsics thus consider the first invalid extrinsic
-                // as the mismatch.
+                // The `external_invalid_type` claim a prior check is pass while the `local_invalid_type` think
+                // it is failed, so generate a fraud proof to prove that check is truely invalid
                 Ordering::Less => BundleMismatchType::TrueInvalid(local_invalid_type),
+                // The `external_invalid_type` claim a prior check is failed while the `local_invalid_type` think
+                // it is pass, so generate a fraud proof to prove that check is falsely invalid
                 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 `IllegalTx` 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`"
-                    ),
-                },
+                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`"
+                ),
             }
         }
         (BundleValidity::Valid(_), BundleValidity::Valid(_)) => BundleMismatchType::Valid,

domains/client/domain-operator/src/fraud_proof.rs

@@ -552,8 +552,10 @@ where
             }
         };
 
-        let extrinsic_index = invalid_type.extrinsic_index();
-        let proof_data = if bundle.extrinsics.len() as u32 <= extrinsic_index {
+        let proof_data = if invalid_type
+            .extrinsic_index()
+            .map_or(false, |idx| bundle.extrinsics.len() as u32 <= idx)
+        {
             // The bad receipt claims a non-exist extrinsic is invalid, in this case, generate a
             // `bundle_with_proof` as proof data is enough
             let bundle_with_proof = OpaqueBundleWithProof::generate(
@@ -673,7 +675,8 @@ where
 
                     InvalidBundlesProofData::Bundle(bundle_with_proof)
                 }
-                InvalidBundleType::UndecodableTx(_) | InvalidBundleType::InherentExtrinsic(_) => {
+                InvalidBundleType::UndecodableTx(extrinsic_index)
+                | InvalidBundleType::InherentExtrinsic(extrinsic_index) => {
                     let encoded_extrinsics: Vec<_> =
                         bundle.extrinsics.iter().map(Encode::encode).collect();