diff --git a/crates/sp-domains-fraud-proof/src/fraud_proof.rs b/crates/sp-domains-fraud-proof/src/fraud_proof.rs index 149a960d6b..1542c0627f 100644 --- a/crates/sp-domains-fraud-proof/src/fraud_proof.rs +++ b/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( - &self, - execution_result: Vec, - ) -> Result> { - match self { - Self::InitializeBlock | Self::ApplyExtrinsic { .. } => { - let encoded_storage_root = Vec::::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( &self, @@ -307,6 +286,9 @@ pub enum VerificationError { 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 From for VerificationError { diff --git a/crates/sp-domains-fraud-proof/src/host_functions.rs b/crates/sp-domains-fraud-proof/src/host_functions.rs index eede593383..8cb8855e15 100644 --- a/crates/sp-domains-fraud-proof/src/host_functions.rs +++ b/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::<::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), /// 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> for ExecutionProofCheckError { @@ -466,18 +470,17 @@ impl From> 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( root: H::Out, proof: StorageProof, - overlay: &mut OverlayedChanges, exec: &Exec, method: &str, call_data: &[u8], runtime_code: &RuntimeCode, extensions: &mut Extensions, -) -> Result, ExecutionProofCheckError> +) -> Result where H: Hasher, H::Out: Ord + 'static + Codec, @@ -485,9 +488,10 @@ where { let expected_nodes_to_be_read = proof.iter_nodes().count(); let (trie_backend, recorder) = create_proof_check_backend_with_recorder::(root, proof)?; + let mut overlayed_changes = OverlayedChanges::::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(), + }) } diff --git a/crates/sp-domains-fraud-proof/src/lib.rs b/crates/sp-domains-fraud-proof/src/lib.rs index e62b88e220..3192e65077 100644 --- a/crates/sp-domains-fraud-proof/src/lib.rs +++ b/crates/sp-domains-fraud-proof/src/lib.rs @@ -124,6 +124,15 @@ impl PassBy for DomainInherentExtrinsicData { type PassBy = pass_by::Codec; } +/// 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, + /// Post state root. + pub post_execution_state_root: Vec, +} + #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] pub struct DomainInherentExtrinsic { domain_timestamp_extrinsic: Vec, diff --git a/crates/sp-domains-fraud-proof/src/verification.rs b/crates/sp-domains-fraud-proof/src/verification.rs index 6c4a624258..bb122c04e6 100644 --- a/crates/sp-domains-fraud-proof/src/verification.rs +++ b/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::(&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::(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; diff --git a/crates/sp-domains/src/core_api.rs b/crates/sp-domains/src/core_api.rs index 90a07c3a76..df44c3b71f 100644 --- a/crates/sp-domains/src/core_api.rs +++ b/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; - - /// Returns the storage root after applying the extrinsic. - fn apply_extrinsic_with_post_state_root(extrinsic: Block::Extrinsic) -> Vec; - /// Returns an encoded extrinsic aiming to upgrade the runtime using given code. fn construct_set_code_extrinsic(code: Vec) -> Vec; diff --git a/domains/client/block-builder/src/custom_api.rs b/domains/client/block-builder/src/custom_api.rs index ec9533db94..ae73f735ab 100644 --- a/domains/client/block-builder/src/custom_api.rs +++ b/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, >; -type TrieBackendFor = - TrieBackend, HashingFor>; - /// Storage changes are the collected throughout the execution. pub struct CollectedStorageChanges { /// 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: 'a + Hasher, -{ - fn consolidate_delta(&mut self, db: BackendTransaction) { - let delta = if let Some(mut delta) = self.delta.take() { - delta.consolidate(db); - delta - } else { - db - }; - self.delta = Some(delta); - } -} - pub(crate) struct TrieBackendApi, Exec> { parent_hash: Block::Hash, parent_number: NumberFor, diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index 08f93dbe20..43ddcb8a0c 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/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 diff --git a/domains/pallets/executive/src/lib.rs b/domains/pallets/executive/src/lib.rs index f4ebe82486..225b112cad 100644 --- a/domains/pallets/executive/src/lib.rs +++ b/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 Hooks> for Pallet { fn on_initialize(_block_number: BlockNumberFor) -> Weight { - // Reset the intermediate storage roots from last block. - IntermediateRoots::::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 = StorageValue<_, Vec<[u8; 32]>, ValueQuery>; -} - -impl Pallet { - pub(crate) fn push_root(root: Vec) { - IntermediateRoots::::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, Context>: From>>, UnsignedValidator: ValidateUnsigned, Context>>, { - /// Returns the latest storage root. - pub fn storage_root() -> Vec { - let version = ::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::::push_root(Self::storage_root()); } /// Wrapped `frame_executive::Executive::finalize_block`. pub fn finalize_block() -> HeaderFor { - Pallet::::push_root(Self::storage_root()); frame_executive::Executive::< ExecutiveConfig, BlockOf, @@ -429,8 +401,6 @@ where /// /// Note the storage root in the beginning. pub fn apply_extrinsic(uxt: ExtrinsicOf) -> ApplyExtrinsicResult { - Pallet::::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 >::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::::decode(&mut Self::storage_root().as_slice()).unwrap() - } - ); - res + } } // TODO: https://github.com/paritytech/substrate/issues/10711 diff --git a/domains/runtime/auto-id/src/lib.rs b/domains/runtime/auto-id/src/lib.rs index ade1200f0c..fd5c32ea24 100644 --- a/domains/runtime/auto-id/src/lib.rs +++ b/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: &::Header) -> Vec { - Executive::initialize_block(header); - Executive::storage_root() - } - - fn apply_extrinsic_with_post_state_root(extrinsic: ::Extrinsic) -> Vec { - let _ = Executive::apply_extrinsic(extrinsic); - Executive::storage_root() - } - fn construct_set_code_extrinsic(code: Vec) -> Vec { UncheckedExtrinsic::new_unsigned( domain_pallet_executive::Call::set_code { diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index 965573b918..d4a8c5a3f3 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -1169,20 +1169,6 @@ impl_runtime_apis! { } } - fn intermediate_roots() -> Vec<[u8; 32]> { - ExecutivePallet::intermediate_roots() - } - - fn initialize_block_with_post_state_root(header: &::Header) -> Vec { - Executive::initialize_block(header); - Executive::storage_root() - } - - fn apply_extrinsic_with_post_state_root(extrinsic: ::Extrinsic) -> Vec { - let _ = Executive::apply_extrinsic(extrinsic); - Executive::storage_root() - } - fn construct_set_code_extrinsic(code: Vec) -> Vec { UncheckedExtrinsic::new_unsigned( domain_pallet_executive::Call::set_code { diff --git a/domains/test/runtime/auto-id/src/lib.rs b/domains/test/runtime/auto-id/src/lib.rs index 0f77507876..5ea25fe4b0 100644 --- a/domains/test/runtime/auto-id/src/lib.rs +++ b/domains/test/runtime/auto-id/src/lib.rs @@ -749,20 +749,6 @@ impl_runtime_apis! { } } - fn intermediate_roots() -> Vec<[u8; 32]> { - ExecutivePallet::intermediate_roots() - } - - fn initialize_block_with_post_state_root(header: &::Header) -> Vec { - Executive::initialize_block(header); - Executive::storage_root() - } - - fn apply_extrinsic_with_post_state_root(extrinsic: ::Extrinsic) -> Vec { - let _ = Executive::apply_extrinsic(extrinsic); - Executive::storage_root() - } - fn construct_set_code_extrinsic(code: Vec) -> Vec { UncheckedExtrinsic::new_unsigned( domain_pallet_executive::Call::set_code { diff --git a/domains/test/runtime/evm/src/lib.rs b/domains/test/runtime/evm/src/lib.rs index 26dce324a9..e48eee62d8 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -1124,20 +1124,6 @@ impl_runtime_apis! { } } - fn intermediate_roots() -> Vec<[u8; 32]> { - ExecutivePallet::intermediate_roots() - } - - fn initialize_block_with_post_state_root(header: &::Header) -> Vec { - Executive::initialize_block(header); - Executive::storage_root() - } - - fn apply_extrinsic_with_post_state_root(extrinsic: ::Extrinsic) -> Vec { - let _ = Executive::apply_extrinsic(extrinsic); - Executive::storage_root() - } - fn construct_set_code_extrinsic(code: Vec) -> Vec { UncheckedExtrinsic::new_unsigned( domain_pallet_executive::Call::set_code {