diff --git a/crates/sp-domains-fraud-proof/src/verification.rs b/crates/sp-domains-fraud-proof/src/verification.rs index 9d145869ad..95a58b19f5 100644 --- a/crates/sp-domains-fraud-proof/src/verification.rs +++ b/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() } } }; diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index ceba256df9..535b677999 100644 --- a/crates/sp-domains/src/lib.rs +++ b/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 { + 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 InboxedBundle { pub fn invalid_extrinsic_index(&self) -> Option { 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, } } diff --git a/crates/sp-domains/src/tests.rs b/crates/sp-domains/src/tests.rs index 091571beb5..e58687401c 100644 --- a/crates/sp-domains/src/tests.rs +++ b/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() + ); +} diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index 634faa58b6..9ca84c266e 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/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, diff --git a/domains/client/domain-operator/src/fraud_proof.rs b/domains/client/domain-operator/src/fraud_proof.rs index 01d5367172..ec0abe8ed5 100644 --- a/domains/client/domain-operator/src/fraud_proof.rs +++ b/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();