clean up pallet executive, CoreApi, and fraud proof verification

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

@@ -53,10 +53,8 @@ impl ExecutionPhase {
     /// Returns the method for generating the proof.
     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 { .. } => "DomainCoreApi_apply_extrinsic_with_post_state_root",
+            Self::InitializeBlock => "Core_initialize_block",
+            Self::ApplyExtrinsic { .. } => "BlockBuilder_apply_extrinsic",
             Self::FinalizeBlock { .. } => "BlockBuilder_finalize_block",
         }
     }
@@ -76,25 +74,6 @@ impl ExecutionPhase {
                 }
         )
     }
-    /// Returns the post state root for the given execution result.
-    pub fn decode_execution_result<Header: HeaderT>(
-        &self,
-        execution_result: Vec<u8>,
-    ) -> Result<Header::Hash, VerificationError<Header::Hash>> {
-        match self {
-            Self::InitializeBlock | Self::ApplyExtrinsic { .. } => {
-                let encoded_storage_root = Vec::<u8>::decode(&mut execution_result.as_slice())
-                    .map_err(VerificationError::InitializeBlockOrApplyExtrinsicDecode)?;
-                Header::Hash::decode(&mut encoded_storage_root.as_slice())
-                    .map_err(VerificationError::StorageRootDecode)
-            }
-            Self::FinalizeBlock { .. } => {
-                let new_header = Header::decode(&mut execution_result.as_slice())
-                    .map_err(VerificationError::HeaderDecode)?;
-                Ok(*new_header.state_root())
-            }
-        }
-    }
 
     pub fn pre_post_state_root<CBlock, DomainHeader, Balance>(
         &self,
@@ -307,6 +286,9 @@ pub enum VerificationError<DomainHash> {
     FailedToGetExtractXdmMmrProof,
     #[error("Failed to decode xdm mmr proof")]
     FailedToDecodeXdmMmrProof,
+    /// Failed to decode the execution proof check result.
+    #[error("Failed to decode execution proof check")]
+    ExecutionProofResultDecode(codec::Error),
 }
 
 impl<DomainHash> From<storage_proof::VerificationError> for VerificationError<DomainHash> {

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

@@ -3,7 +3,7 @@ extern crate alloc;
 
 use crate::{
     DomainInherentExtrinsic, DomainInherentExtrinsicData, DomainStorageKeyRequest,
-    StatelessDomainRuntimeCall,
+    ExecutionProofCheckResult, StatelessDomainRuntimeCall,
 };
 #[cfg(not(feature = "std"))]
 use alloc::vec::Vec;
@@ -12,6 +12,7 @@ use domain_block_preprocessor::stateless_runtime::StatelessRuntime;
 use domain_runtime_primitives::{
     BlockNumber, CheckExtrinsicsValidityError, CHECK_EXTRINSICS_AND_DO_PRE_DISPATCH_METHOD_NAME,
 };
+use frame_support::__private::StateVersion;
 use hash_db::{HashDB, Hasher};
 use sc_client_api::execution_extensions::ExtensionsFactory;
 use sc_client_api::BlockBackend;
@@ -216,13 +217,13 @@ where
         execution_proof_check::<<DomainBlock::Header as HeaderT>::Hashing, _>(
             pre_state_root.into(),
             proof,
-            &mut Default::default(),
             self.domain_executor.as_ref(),
             execution_method,
             call_data,
             &runtime_code,
             &mut domain_extensions,
         )
+        .map(|res| res.encode())
         .ok()
     }
 
@@ -247,8 +248,9 @@ where
             domain_runtime_code,
         )?;
 
+        let check_response = ExecutionProofCheckResult::decode(&mut raw_response.as_ref()).ok()?;
         let bundle_extrinsics_validity_response: Result<(), CheckExtrinsicsValidityError> =
-            Decode::decode(&mut raw_response.as_slice()).ok()?;
+            Decode::decode(&mut check_response.execution_result.as_slice()).ok()?;
         if let Err(bundle_extrinsic_validity_error) = bundle_extrinsics_validity_response {
             Some(Some(bundle_extrinsic_validity_error.extrinsic_index))
         } else {
@@ -457,6 +459,8 @@ pub(crate) enum ExecutionProofCheckError {
     ExecutionError(Box<dyn sp_state_machine::Error>),
     /// Error when storage proof contains unused node keys.
     UnusedNodes,
+    // /// Error when trying to get storage root over given overlay.
+    // StorageChanges(sp_state_machine::DefaultError),
 }
 
 impl From<Box<dyn sp_state_machine::Error>> for ExecutionProofCheckError {
@@ -466,28 +470,28 @@ impl From<Box<dyn sp_state_machine::Error>> for ExecutionProofCheckError {
 }
 
 #[allow(clippy::too_many_arguments)]
-/// Executes the given proof using the runtime
-/// The only difference between sp_state_machine::execution_proof_check is Extensions
+/// Executes the given proof using the runtime and returns the post state root.
+/// The only difference between sp_state_machine::execution_proof_check is Extensions and return result.
 pub(crate) fn execution_proof_check<H, Exec>(
     root: H::Out,
     proof: StorageProof,
-    overlay: &mut OverlayedChanges<H>,
     exec: &Exec,
     method: &str,
     call_data: &[u8],
     runtime_code: &RuntimeCode,
     extensions: &mut Extensions,
-) -> Result<Vec<u8>, ExecutionProofCheckError>
+) -> Result<ExecutionProofCheckResult, ExecutionProofCheckError>
 where
     H: Hasher,
     H::Out: Ord + 'static + Codec,
     Exec: CodeExecutor + Clone + 'static,
 {
     let expected_nodes_to_be_read = proof.iter_nodes().count();
     let (trie_backend, recorder) = create_proof_check_backend_with_recorder::<H>(root, proof)?;
+    let mut overlayed_changes = OverlayedChanges::<H>::default();
     let result = StateMachine::<_, H, Exec>::new(
         &trie_backend,
-        overlay,
+        &mut overlayed_changes,
         exec,
         method,
         call_data,
@@ -502,5 +506,12 @@ where
         return Err(ExecutionProofCheckError::UnusedNodes);
     }
 
-    Ok(result)
+    let changes = overlayed_changes
+        .drain_storage_changes(&trie_backend, StateVersion::V1)
+        .map_err(|err| ExecutionProofCheckError::ExecutionError(Box::new(err)))?;
+
+    Ok(ExecutionProofCheckResult {
+        execution_result: result,
+        post_execution_state_root: changes.transaction_storage_root.encode(),
+    })
 }

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

@@ -124,6 +124,15 @@ impl PassBy for DomainInherentExtrinsicData {
     type PassBy = pass_by::Codec<Self>;
 }
 
+/// Type that holds the result and post execution state root of the given execution.
+#[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
+pub struct ExecutionProofCheckResult {
+    /// Execution result.
+    pub execution_result: Vec<u8>,
+    /// Post state root.
+    pub post_execution_state_root: Vec<u8>,
+}
+
 #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
 pub struct DomainInherentExtrinsic {
     domain_timestamp_extrinsic: Vec<u8>,

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

@@ -8,7 +8,7 @@ use crate::fraud_proof::{
 use crate::storage_proof::{self, *};
 use crate::{
     fraud_proof_runtime_interface, DomainInherentExtrinsic, DomainInherentExtrinsicData,
-    DomainStorageKeyRequest, StatelessDomainRuntimeCall,
+    DomainStorageKeyRequest, ExecutionProofCheckResult, StatelessDomainRuntimeCall,
 };
 #[cfg(not(feature = "std"))]
 use alloc::vec::Vec;
@@ -283,7 +283,7 @@ where
     let call_data = execution_phase
         .call_data::<CBlock, DomainHeader, Balance>(&bad_receipt, &bad_receipt_parent)?;
 
-    let execution_result = fraud_proof_runtime_interface::execution_proof_check(
+    let execution_proof_check_result = fraud_proof_runtime_interface::execution_proof_check(
         (
             bad_receipt_parent.domain_block_number.saturated_into(),
             bad_receipt_parent.domain_block_hash.into(),
@@ -296,9 +296,14 @@ where
     )
     .ok_or(VerificationError::BadExecutionProof)?;
 
-    let valid_post_state_root = execution_phase
-        .decode_execution_result::<DomainHeader>(execution_result)?
-        .into();
+    let execution_result =
+        ExecutionProofCheckResult::decode(&mut execution_proof_check_result.as_slice())
+            .map_err(VerificationError::ExecutionProofResultDecode)?;
+
+    let valid_post_state_root =
+        DomainHeader::Hash::decode(&mut execution_result.post_execution_state_root.as_slice())
+            .map_err(VerificationError::StorageRootDecode)?
+            .into();
 
     let is_mismatch = valid_post_state_root != post_state_root;
 

crates/sp-domains/src/core_api.rs

@@ -28,15 +28,6 @@ sp_api::decl_runtime_apis! {
             tx_range: &U256,
         ) -> bool;
 
-        /// Returns the intermediate storage roots in an encoded form.
-        fn intermediate_roots() -> Vec<[u8; 32]>;
-
-        /// Returns the storage root after initializing the block.
-        fn initialize_block_with_post_state_root(header: &Block::Header) -> Vec<u8>;
-
-        /// Returns the storage root after applying the extrinsic.
-        fn apply_extrinsic_with_post_state_root(extrinsic: Block::Extrinsic) -> Vec<u8>;
-
         /// Returns an encoded extrinsic aiming to upgrade the runtime using given code.
         fn construct_set_code_extrinsic(code: Vec<u8>) -> Vec<u8>;
 

domains/client/block-builder/src/custom_api.rs

@@ -1,8 +1,5 @@
 //! Custom API that is efficient to collect storage roots.
 
-// TODO: remove in later commits
-#![allow(dead_code)]
-
 use codec::{Codec, Decode, Encode};
 use hash_db::{HashDB, Hasher, Prefix};
 use sc_client_api::{backend, ExecutorProvider, StateBackend};
@@ -26,9 +23,6 @@ pub(crate) type TrieDeltaBackendFor<'a, State, Block> = TrieBackend<
     HashingFor<Block>,
 >;
 
-type TrieBackendFor<State, Block> =
-    TrieBackend<TrieBackendStorageFor<State, Block>, HashingFor<Block>>;
-
 /// Storage changes are the collected throughout the execution.
 pub struct CollectedStorageChanges<H: Hasher> {
     /// Storage changes that are captured during the execution.
@@ -102,22 +96,6 @@ where
     }
 }
 
-impl<'a, S, H> DeltaBackend<'a, S, H>
-where
-    S: 'a + TrieBackendStorage<H>,
-    H: 'a + Hasher,
-{
-    fn consolidate_delta(&mut self, db: BackendTransaction<H>) {
-        let delta = if let Some(mut delta) = self.delta.take() {
-            delta.consolidate(db);
-            delta
-        } else {
-            db
-        };
-        self.delta = Some(delta);
-    }
-}
-
 pub(crate) struct TrieBackendApi<Client, Block: BlockT, Backend: backend::Backend<Block>, Exec> {
     parent_hash: Block::Hash,
     parent_number: NumberFor<Block>,

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

@@ -863,7 +863,7 @@ async fn test_bad_invalid_state_transition_proof_is_rejected() {
         .await
         .unwrap();
 
-    // trace index 0 is the index of the state root after applying DomainCoreApi::initialize_block_with_post_state_root
+    // trace index 0 is the index of the state root after applying DomainCoreApi::initialize_block
     // 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

domains/pallets/executive/src/lib.rs

@@ -44,8 +44,8 @@ use frame_support::storage::with_storage_layer;
 use frame_support::traits::fungible::{Inspect, Mutate};
 use frame_support::traits::tokens::{Fortitude, Precision, Preservation};
 use frame_support::traits::{
-    BeforeAllRuntimeMigrations, EnsureInherentsAreFirst, ExecuteBlock, Get, OffchainWorker,
-    OnFinalize, OnIdle, OnInitialize, OnPoll, OnRuntimeUpgrade, PostTransactions,
+    BeforeAllRuntimeMigrations, EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker, OnFinalize,
+    OnIdle, OnInitialize, OnPoll, OnRuntimeUpgrade, PostTransactions,
 };
 use frame_support::weights::{Weight, WeightToFee};
 use frame_system::pallet_prelude::*;
@@ -182,26 +182,10 @@ mod pallet {
     #[pallet::hooks]
     impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
         fn on_initialize(_block_number: BlockNumberFor<T>) -> Weight {
-            // Reset the intermediate storage roots from last block.
-            IntermediateRoots::<T>::kill();
             // TODO: Probably needs a different value
             Weight::from_parts(1, 0)
         }
     }
-
-    /// Intermediate storage roots collected during the block execution.
-    #[pallet::storage]
-    #[pallet::getter(fn intermediate_roots)]
-    pub(super) type IntermediateRoots<T: Config> = StorageValue<_, Vec<[u8; 32]>, ValueQuery>;
-}
-
-impl<T: Config> Pallet<T> {
-    pub(crate) fn push_root(root: Vec<u8>) {
-        IntermediateRoots::<T>::append(
-            TryInto::<[u8; 32]>::try_into(root)
-                .expect("root is a SCALE encoded hash which uses H256; qed"),
-        );
-    }
 }
 
 /// Same semantics with `frame_executive::Executive`.
@@ -285,12 +269,6 @@ where
     OriginOf<ExtrinsicOf<ExecutiveConfig>, Context>: From<Option<AccountIdOf<ExecutiveConfig>>>,
     UnsignedValidator: ValidateUnsigned<Call = CallOf<ExtrinsicOf<ExecutiveConfig>, Context>>,
 {
-    /// Returns the latest storage root.
-    pub fn storage_root() -> Vec<u8> {
-        let version = <ExecutiveConfig as frame_system::Config>::Version::get().state_version();
-        sp_io::storage::root(version)
-    }
-
     /// Wrapped `frame_executive::Executive::execute_on_runtime_upgrade`.
     pub fn execute_on_runtime_upgrade() -> Weight {
         frame_executive::Executive::<
@@ -388,16 +366,10 @@ where
                 panic!("{}", err)
             }
         });
-
-        // Note the storage root before finalizing the block so that the block imported during the
-        // syncing process produces the same storage root with the one processed based on
-        // the consensus block.
-        Pallet::<ExecutiveConfig>::push_root(Self::storage_root());
     }
 
     /// Wrapped `frame_executive::Executive::finalize_block`.
     pub fn finalize_block() -> HeaderFor<ExecutiveConfig> {
-        Pallet::<ExecutiveConfig>::push_root(Self::storage_root());
         frame_executive::Executive::<
             ExecutiveConfig,
             BlockOf<ExecutiveConfig>,
@@ -429,8 +401,6 @@ where
     ///
     /// Note the storage root in the beginning.
     pub fn apply_extrinsic(uxt: ExtrinsicOf<ExecutiveConfig>) -> ApplyExtrinsicResult {
-        Pallet::<ExecutiveConfig>::push_root(Self::storage_root());
-
         // apply the extrinsic within another transaction so that changes can be reverted.
         let res = with_storage_layer(|| {
             frame_executive::Executive::<
@@ -455,7 +425,7 @@ where
         //      - Pre and Post dispatch fails. Check the test `test_domain_block_builder_include_ext_with_failed_predispatch`
         //        why this could happen. If it fail due to this, then we revert the inner storage changes
         //        but still include extrinsic so that we can clear inconsistency between block body and trace roots.
-        let res = match res {
+        match res {
             Ok(dispatch_outcome) => Ok(dispatch_outcome),
             Err(err) => {
                 let encoded = uxt.encode();
@@ -509,18 +479,7 @@ where
                 <frame_system::Pallet<ExecutiveConfig>>::note_applied_extrinsic(&r, dispatch_info);
                 Ok(Err(err))
             }
-        };
-
-        // TODO: Critical!!! https://github.com/paritytech/substrate/pull/10922#issuecomment-1068997467
-        log::debug!(
-            target: "domain::runtime::executive",
-            "[apply_extrinsic] after: {:?}",
-            {
-                use codec::Decode;
-                BlockHashOf::<ExecutiveConfig>::decode(&mut Self::storage_root().as_slice()).unwrap()
-            }
-        );
-        res
+        }
     }
 
     // TODO: https://github.com/paritytech/substrate/issues/10711

domains/runtime/auto-id/src/lib.rs

@@ -758,20 +758,6 @@ impl_runtime_apis! {
             }
         }
 
-        fn intermediate_roots() -> Vec<[u8; 32]> {
-            ExecutivePallet::intermediate_roots()
-        }
-
-        fn initialize_block_with_post_state_root(header: &<Block as BlockT>::Header) -> Vec<u8> {
-            Executive::initialize_block(header);
-            Executive::storage_root()
-        }
-
-        fn apply_extrinsic_with_post_state_root(extrinsic: <Block as BlockT>::Extrinsic) -> Vec<u8> {
-            let _ = Executive::apply_extrinsic(extrinsic);
-            Executive::storage_root()
-        }
-
         fn construct_set_code_extrinsic(code: Vec<u8>) -> Vec<u8> {
             UncheckedExtrinsic::new_unsigned(
                 domain_pallet_executive::Call::set_code {