Merge pull request #2768 from subspace/bundle-weight-fp

Introduce the invalid bundle weight fraud proof

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

@@ -339,6 +339,9 @@ pub enum VerificationError<DomainHash> {
         error("Failed to get domain runtime call response")
     )]
     FailedToGetDomainRuntimeCallResponse,
+    /// Failed to get bundle weight
+    #[cfg_attr(feature = "thiserror", error("Failed to get bundle weight"))]
+    FailedToGetBundleWeight,
 }
 
 impl<DomainHash> From<storage_proof::VerificationError> for VerificationError<DomainHash> {

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

@@ -160,7 +160,7 @@ pub trait FraudProofRuntimeInterface {
             .domain_runtime_call(domain_runtime_code, call)
     }
 
-    #[version(1, register_only)]
+    #[version(1)]
     fn bundle_weight(
         &mut self,
         domain_runtime_code: Vec<u8>,

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()
             }
         }
     };
@@ -694,5 +690,26 @@ where
             }
             Ok(())
         }
+        InvalidBundleType::InvalidBundleWeight => {
+            let bundle = match proof_data {
+                InvalidBundlesProofData::Bundle(bundle_with_proof) => {
+                    bundle_with_proof.bundle.clone()
+                }
+                _ => return Err(VerificationError::UnexpectedInvalidBundleProofData),
+            };
+            let bundle_header_weight = bundle.estimated_weight();
+            let estimated_bundle_weight = fraud_proof_runtime_interface::bundle_weight(
+                domain_runtime_code,
+                bundle.extrinsics,
+            )
+            .ok_or(VerificationError::FailedToGetBundleWeight)?;
+
+            let is_bundle_weight_correct = estimated_bundle_weight == bundle_header_weight;
+
+            if is_bundle_weight_correct == is_true_invalid_fraud_proof {
+                return Err(VerificationError::InvalidProof);
+            }
+            Ok(())
+        }
     }
 }

crates/sp-domains/src/lib.rs

@@ -1092,27 +1092,60 @@ pub enum InvalidBundleType {
     /// Transaction is an inherent extrinsic.
     #[codec(index = 4)]
     InherentExtrinsic(u32),
+    /// The `estimated_bundle_weight` in the bundle header is invalid
+    #[codec(index = 5)]
+    InvalidBundleWeight,
 }
 
 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
+    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,
+            Self::InherentExtrinsic(i) => *i,
+            // NOTE: the `InvalidBundleWeight` is targetting the whole bundle not a specific
+            // single extrinsic, as `extrinsic_index` is used as the order to check the extrinsic
+            // 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
-        match self {
+        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)
     }
 
-    pub fn extrinsic_index(&self) -> u32 {
+    // 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) => *i,
-            Self::OutOfRangeTx(i) => *i,
-            Self::IllegalTx(i) => *i,
-            Self::InherentExtrinsic(i) => *i,
+            Self::UndecodableTx(i) => Some(*i),
+            Self::OutOfRangeTx(i) => Some(*i),
+            Self::IllegalTx(i) => Some(*i),
+            Self::InherentExtrinsic(i) => Some(*i),
+            Self::InvalidBundleWeight => None,
         }
     }
 }
@@ -1157,9 +1190,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/block-preprocessor/src/lib.rs

@@ -30,6 +30,7 @@ use sp_domains::{
 use sp_messenger::MessengerApi;
 use sp_runtime::traits::{Block as BlockT, Hash as HashT, NumberFor};
 use sp_state_machine::LayoutV1;
+use sp_weights::Weight;
 use std::collections::VecDeque;
 use std::marker::PhantomData;
 use std::sync::Arc;
@@ -251,6 +252,7 @@ where
             U256::from_be_bytes(bundle.sealed_header.header.proof_of_election.vrf_hash());
 
         let mut extrinsics = Vec::with_capacity(bundle.extrinsics.len());
+        let mut estimated_bundle_weight = Weight::default();
 
         let domain_block_number = self
             .client
@@ -316,9 +318,18 @@ where
                 )));
             }
 
+            let tx_weight = runtime_api.extrinsic_weight(at, &extrinsic)?;
+            estimated_bundle_weight = estimated_bundle_weight.saturating_add(tx_weight);
+
             extrinsics.push(extrinsic);
         }
 
+        if estimated_bundle_weight != bundle.estimated_weight() {
+            return Ok(BundleValidity::Invalid(
+                InvalidBundleType::InvalidBundleWeight,
+            ));
+        }
+
         Ok(BundleValidity::Valid(extrinsics))
     }
 }

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(
@@ -661,7 +663,7 @@ where
                         execution_proof,
                     }
                 }
-                InvalidBundleType::OutOfRangeTx(_) => {
+                InvalidBundleType::OutOfRangeTx(_) | InvalidBundleType::InvalidBundleWeight => {
                     let bundle_with_proof = OpaqueBundleWithProof::generate(
                         &self.storage_key_provider,
                         self.consensus_client.as_ref(),
@@ -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();