Merge pull request #2388 from subspace/fix-invalid-state-transition-p…

…roof

BugFix: Fix invalid state transition fraud proof generation

crates/pallet-domains/src/tests.rs

@@ -363,7 +363,7 @@ impl FraudProofHostFunctions for MockDomainFraudProofExtension {
         &self,
         _pre_state_root: H256,
         _encoded_proof: Vec<u8>,
-        _verifying_method: &str,
+        _execution_method: &str,
         _call_data: &[u8],
         _domain_runtime_code: Vec<u8>,
     ) -> Option<Vec<u8>> {

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

@@ -57,15 +57,15 @@ where
             .runtime_code()
             .map_err(sp_blockchain::Error::RuntimeCode)?;
 
-        // TODO: avoid using the String API specified by `proving_method()`
+        // TODO: avoid using the String API specified by `execution_method()`
         // https://github.com/paritytech/substrate/discussions/11095
         if let Some((delta, post_delta_root)) = delta_changes {
             let delta_backend = create_delta_backend(trie_backend, delta, post_delta_root);
             sp_state_machine::prove_execution_on_trie_backend(
                 &delta_backend,
                 &mut Default::default(),
                 &*self.executor,
-                execution_phase.proving_method(),
+                execution_phase.execution_method(),
                 call_data,
                 &runtime_code,
                 &mut Default::default(),
@@ -77,7 +77,7 @@ where
                 trie_backend,
                 &mut Default::default(),
                 &*self.executor,
-                execution_phase.proving_method(),
+                execution_phase.execution_method(),
                 call_data,
                 &runtime_code,
                 &mut Default::default(),
@@ -114,7 +114,7 @@ where
             proof,
             &mut Default::default(),
             &*self.executor,
-            execution_phase.verifying_method(),
+            execution_phase.execution_method(),
             call_data,
             &runtime_code,
         )

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

@@ -32,23 +32,10 @@ pub enum ExecutionPhase {
 
 impl ExecutionPhase {
     /// Returns the method for generating the proof.
-    pub fn proving_method(&self) -> &'static str {
+    pub fn execution_method(&self) -> &'static str {
         match self {
             // TODO: Replace `DomainCoreApi_initialize_block_with_post_state_root` with `Core_initalize_block`
             // Should be a same issue with https://github.com/paritytech/substrate/pull/10922#issuecomment-1068997467
-            Self::InitializeBlock => "DomainCoreApi_initialize_block_with_post_state_root",
-            Self::ApplyExtrinsic { .. } => "BlockBuilder_apply_extrinsic",
-            Self::FinalizeBlock => "BlockBuilder_finalize_block",
-        }
-    }
-
-    /// Returns the method for verifying the proof.
-    ///
-    /// The difference with [`Self::proving_method`] is that the return value of verifying method
-    /// must contain the post state root info so that it can be used to compare whether the
-    /// result of execution reported in [`FraudProof`] is expected or not.
-    pub fn verifying_method(&self) -> &'static str {
-        match self {
             Self::InitializeBlock => "DomainCoreApi_initialize_block_with_post_state_root",
             Self::ApplyExtrinsic { .. } => "DomainCoreApi_apply_extrinsic_with_post_state_root",
             Self::FinalizeBlock => "BlockBuilder_finalize_block",

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

@@ -56,7 +56,7 @@ pub trait FraudProofHostFunctions: Send + Sync {
         pre_state_root: H256,
         // TODO: implement `PassBy` for `sp_trie::StorageProof` in upstream to pass it directly here
         encoded_proof: Vec<u8>,
-        verifying_method: &str,
+        execution_method: &str,
         call_data: &[u8],
         domain_runtime_code: Vec<u8>,
     ) -> Option<Vec<u8>>;
@@ -467,7 +467,7 @@ where
         &self,
         pre_state_root: H256,
         encoded_proof: Vec<u8>,
-        verifying_method: &str,
+        execution_method: &str,
         call_data: &[u8],
         domain_runtime_code: Vec<u8>,
     ) -> Option<Vec<u8>> {
@@ -486,7 +486,7 @@ where
             proof,
             &mut Default::default(),
             self.executor.as_ref(),
-            verifying_method,
+            execution_method,
             call_data,
             &runtime_code,
         )

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

@@ -40,7 +40,7 @@ pub trait FraudProofRuntimeInterface {
         &mut self,
         pre_state_root: H256,
         encoded_proof: Vec<u8>,
-        verifying_method: &str,
+        execution_method: &str,
         call_data: &[u8],
         domain_runtime_code: Vec<u8>,
     ) -> Option<Vec<u8>> {
@@ -49,7 +49,7 @@ pub trait FraudProofRuntimeInterface {
             .execution_proof_check(
                 pre_state_root,
                 encoded_proof,
-                verifying_method,
+                execution_method,
                 call_data,
                 domain_runtime_code,
             )

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

@@ -228,7 +228,7 @@ where
     let execution_result = fraud_proof_runtime_interface::execution_proof_check(
         pre_state_root,
         proof.encode(),
-        execution_phase.verifying_method(),
+        execution_phase.execution_method(),
         call_data.as_ref(),
         domain_runtime_code,
     )

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

@@ -1,6 +1,8 @@
 use crate::domain_block_processor::{DomainBlockProcessor, PendingConsensusBlocks};
 use crate::domain_bundle_producer::DomainBundleProducer;
 use crate::domain_bundle_proposer::DomainBundleProposer;
+use crate::fraud_proof::FraudProofGenerator;
+use crate::tests::TxPoolError::InvalidTransaction as TxPoolInvalidTransaction;
 use crate::utils::OperatorSlotInfo;
 use codec::{Decode, Encode};
 use domain_runtime_primitives::Hash;
@@ -13,6 +15,7 @@ use futures::StreamExt;
 use sc_client_api::{Backend, BlockBackend, BlockchainEvents, HeaderBackend};
 use sc_consensus::SharedBlockImport;
 use sc_service::{BasePath, Role};
+use sc_transaction_pool::error::Error as PoolError;
 use sc_transaction_pool_api::error::Error as TxPoolError;
 use sc_transaction_pool_api::TransactionPool;
 use sp_api::{AsTrieBackend, HashT, ProvideRuntimeApi};
@@ -32,6 +35,7 @@ use sp_domains_fraud_proof::fraud_proof::{
 use sp_domains_fraud_proof::InvalidTransactionCode;
 use sp_runtime::generic::{BlockId, DigestItem};
 use sp_runtime::traits::{BlakeTwo256, Block as BlockT, Header as HeaderT};
+use sp_runtime::transaction_validity::InvalidTransaction;
 use sp_runtime::OpaqueExtrinsic;
 use std::sync::Arc;
 use subspace_core_primitives::Randomness;
@@ -808,6 +812,104 @@ async fn test_finalize_block_proof_creation_and_verification_should_work() {
     test_invalid_state_transition_proof_creation_and_verification(3).await
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_bad_invalid_state_transition_proof_is_rejected() {
+    let directory = TempDir::new().expect("Must be able to create temporary directory");
+
+    let mut builder = sc_cli::LoggerBuilder::new("");
+    builder.with_colors(false);
+    let _ = builder.init();
+
+    let tokio_handle = tokio::runtime::Handle::current();
+
+    // Start Ferdie
+    let mut ferdie = MockConsensusNode::run(
+        tokio_handle.clone(),
+        Ferdie,
+        BasePath::new(directory.path().join("ferdie")),
+    );
+
+    // Run Alice (a evm domain authority node)
+    let mut alice = domain_test_service::DomainNodeBuilder::new(
+        tokio_handle.clone(),
+        Alice,
+        BasePath::new(directory.path().join("alice")),
+    )
+    .build_evm_node(Role::Authority, GENESIS_DOMAIN_ID, &mut ferdie)
+    .await;
+
+    let fraud_proof_generator = FraudProofGenerator::new(
+        alice.client.clone(),
+        ferdie.client.clone(),
+        alice.backend.clone(),
+        alice.code_executor.clone(),
+    );
+
+    produce_blocks!(ferdie, alice, 5).await.unwrap();
+
+    alice
+        .construct_and_send_extrinsic(pallet_balances::Call::transfer_allow_death {
+            dest: Bob.to_account_id(),
+            value: 1,
+        })
+        .await
+        .expect("Failed to send extrinsic");
+
+    // Produce a bundle that contains the previously sent extrinsic and record that bundle for later use
+    let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await;
+    let target_bundle = bundle.unwrap();
+    assert_eq!(target_bundle.extrinsics.len(), 1);
+    produce_block_with!(ferdie.produce_block_with_slot(slot), alice)
+        .await
+        .unwrap();
+
+    // We get the receipt of target bundle
+    let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await;
+    let valid_receipt = bundle.unwrap().into_receipt();
+    assert_eq!(valid_receipt.execution_trace.len(), 4);
+    let valid_receipt_hash = valid_receipt.hash::<BlakeTwo256>();
+
+    produce_block_with!(ferdie.produce_block_with_slot(slot), alice)
+        .await
+        .unwrap();
+
+    // trace index 0 is the index of the state root after applying DomainCoreApi::initialize_block_with_post_state_root
+    // trace index 1 is the index of the state root after applying inherent timestamp extrinsic
+    // trace index 2 is the index of the state root after applying above balance transfer extrinsic
+    // trace index 3 is the index of the state root after applying BlockBuilder::finalize_block
+    for mismatch_index in 0..valid_receipt.execution_trace.len() {
+        let fraud_proof = fraud_proof_generator
+            .generate_invalid_state_transition_proof(
+                GENESIS_DOMAIN_ID,
+                mismatch_index.try_into().expect(
+                    "execution trace index should always be convertible into u32 from usize; qed",
+                ),
+                &valid_receipt,
+                valid_receipt_hash,
+            )
+            .unwrap();
+
+        let submit_fraud_proof_extrinsic = subspace_test_runtime::UncheckedExtrinsic::new_unsigned(
+            pallet_domains::Call::submit_fraud_proof {
+                fraud_proof: Box::new(fraud_proof),
+            }
+            .into(),
+        )
+        .into();
+
+        let response = ferdie
+            .submit_transaction(submit_fraud_proof_extrinsic)
+            .await;
+
+        assert!(matches!(
+            response,
+            Err(PoolError::Pool(TxPoolInvalidTransaction(
+                InvalidTransaction::Custom(105)
+            )))
+        ));
+    }
+}
+
 /// This test will create and verify a invalid state transition proof that targets the receipt of
 /// a bundle that contains 2 extrinsic (including 1 inherent timestamp extrinsic), thus there are
 /// 4 trace roots in total, passing the `mismatch_trace_index` as: