From 466bc0457e60238f8800e4ac817e5fd9bae4bd57 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Thu, 31 Mar 2022 09:26:10 +0800 Subject: [PATCH 1/5] Fetch the execution runtime properly --- crates/sp-executor/src/lib.rs | 5 +- crates/subspace-fraud-proof/src/lib.rs | 106 +++++++++++++++---------- crates/subspace-service/src/lib.rs | 5 +- 3 files changed, 69 insertions(+), 47 deletions(-) diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index d7bbf7f88e2..0e8daf537da 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -197,10 +197,6 @@ impl ExecutionPhase { /// Error type of fraud proof verification on primary node. #[derive(RuntimeDebug)] pub enum VerificationError { - /// Runtime code backend unavailable. - RuntimeCodeBackend, - /// Runtime code can not be fetched from the backend. - RuntimeCode(&'static str), /// Failed to pass the execution proof check. BadProof(sp_std::boxed::Box), /// The `post_state_root` calculated by farmer does not match the one declared in [`FraudProof`]. @@ -312,6 +308,7 @@ sp_api::decl_runtime_apis! { } } +// TODO: remove once the fraud proof verification is moved into the client. pub mod fraud_proof_ext { use sp_externalities::ExternalitiesExt; use sp_runtime_interface::runtime_interface; diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 3e99df3b490..619691ca2cc 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -11,12 +11,14 @@ use codec::{Codec, Decode, Encode}; use hash_db::{HashDB, Hasher, Prefix}; use sc_client_api::backend; use sc_client_api::execution_extensions::ExtensionsFactory; -use sp_api::{StateBackend, StorageProof}; +use sp_api::{ProvideRuntimeApi, StateBackend, StorageProof}; use sp_core::{ - traits::{CodeExecutor, SpawnNamed}, + traits::{CodeExecutor, FetchRuntimeCode, RuntimeCode, SpawnNamed}, H256, }; -use sp_executor::{fraud_proof_ext::FraudProofExt, ExecutionPhase, FraudProof, VerificationError}; +use sp_executor::{ + fraud_proof_ext::FraudProofExt, ExecutionPhase, ExecutorApi, FraudProof, VerificationError, +}; use sp_externalities::Extensions; use sp_runtime::{ generic::BlockId, @@ -167,17 +169,38 @@ impl<'a, S: 'a + TrieBackendStorage, H: 'a + Hasher, DB: HashDB> } } +struct RuntimCodeFetcher { + client: Arc, + at: Block::Hash, +} + +impl FetchRuntimeCode for RuntimCodeFetcher +where + Block: BlockT, + C: ProvideRuntimeApi + Send + Sync, + C::Api: ExecutorApi, +{ + fn fetch_runtime_code(&self) -> Option> { + self.client + .runtime_api() + .execution_wasm_bundle(&BlockId::Hash(self.at)) + .ok() + } +} + /// Fraud proof verifier. -pub struct ProofVerifier { +pub struct ProofVerifier { + client: Arc, backend: Arc, executor: Exec, spawn_handle: Spawn, _phantom: PhantomData, } -impl Clone for ProofVerifier { +impl Clone for ProofVerifier { fn clone(&self) -> Self { Self { + client: self.client.clone(), backend: self.backend.clone(), executor: self.executor.clone(), spawn_handle: self.spawn_handle.clone(), @@ -186,16 +209,19 @@ impl Clone for ProofVerifier, - Exec: CodeExecutor + Clone + 'static, - Spawn: SpawnNamed + Clone + Send + 'static, - > ProofVerifier +impl ProofVerifier +where + Block: BlockT, + C: ProvideRuntimeApi + Send + Sync, + C::Api: ExecutorApi, + B: backend::Backend, + Exec: CodeExecutor + Clone + 'static, + Spawn: SpawnNamed + Clone + Send + 'static, { /// Constructs a new instance of [`ProofVerifier`]. - pub fn new(backend: Arc, executor: Exec, spawn_handle: Spawn) -> Self { + pub fn new(client: Arc, backend: Arc, executor: Exec, spawn_handle: Spawn) -> Self { Self { + client, backend, executor, spawn_handle, @@ -212,25 +238,18 @@ impl< execution_phase, } = proof; - let at = BlockId::Hash( - Block::Hash::decode(&mut parent_hash.encode().as_slice()) + let code_fetcher = RuntimCodeFetcher { + client: self.client.clone(), + at: Block::Hash::decode(&mut parent_hash.encode().as_slice()) .expect("Block Hash must be H256; qed"), - ); - - // TODO: ensure the fetched runtime code works as expected. - let state = self - .backend - .state_at(at) - .map_err(|_| VerificationError::RuntimeCodeBackend)?; + }; - let trie_backend = state - .as_trie_backend() - .ok_or(VerificationError::RuntimeCodeBackend)?; - - let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend); - let runtime_code = state_runtime_code - .runtime_code() - .map_err(VerificationError::RuntimeCode)?; + let runtime_code = RuntimeCode { + code_fetcher: &code_fetcher, + hash: b"Hash of the code does not matter in terms of the execution proof check" + .to_vec(), + heap_pages: None, + }; let execution_result = sp_state_machine::execution_proof_check::( *pre_state_root, @@ -260,12 +279,15 @@ impl< } } -impl< - Block: BlockT, - B: backend::Backend, - Exec: CodeExecutor + Clone + 'static, - Spawn: SpawnNamed + Clone + Send + 'static, - > sp_executor::fraud_proof_ext::Externalities for ProofVerifier +impl sp_executor::fraud_proof_ext::Externalities + for ProofVerifier +where + Block: BlockT, + C: ProvideRuntimeApi + Send + Sync, + C::Api: ExecutorApi, + B: backend::Backend, + Exec: CodeExecutor + Clone + 'static, + Spawn: SpawnNamed + Clone + Send + 'static, { fn verify_fraud_proof(&self, proof: &sp_executor::FraudProof) -> bool { match self.verify(proof) { @@ -278,12 +300,14 @@ impl< } } -impl< - Block: BlockT, - B: backend::Backend + 'static, - Exec: CodeExecutor + Clone + Send + Sync, - Spawn: SpawnNamed + Clone + Send + Sync + 'static, - > ExtensionsFactory for ProofVerifier +impl ExtensionsFactory for ProofVerifier +where + Block: BlockT, + C: ProvideRuntimeApi + Send + Sync + 'static, + C::Api: ExecutorApi, + B: backend::Backend + 'static, + Exec: CodeExecutor + Clone + Send + Sync, + Spawn: SpawnNamed + Clone + Send + Sync + 'static, { fn extensions_for(&self, _: sp_core::offchain::Capabilities) -> Extensions { let mut exts = Extensions::new(); diff --git a/crates/subspace-service/src/lib.rs b/crates/subspace-service/src/lib.rs index 7339812ed75..d0f75a4d515 100644 --- a/crates/subspace-service/src/lib.rs +++ b/crates/subspace-service/src/lib.rs @@ -181,7 +181,10 @@ where executor.clone(), )?; + let client = Arc::new(client); + let proof_verifier = subspace_fraud_proof::ProofVerifier::new( + client.clone(), backend.clone(), executor, task_manager.spawn_handle(), @@ -190,8 +193,6 @@ where .execution_extensions() .set_extensions_factory(Box::new(proof_verifier)); - let client = Arc::new(client); - let telemetry = telemetry.map(|(worker, telemetry)| { task_manager .spawn_handle() From d932b704d8e2312ad4671065d1e9c1e4d4212422 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Thu, 31 Mar 2022 09:53:54 +0800 Subject: [PATCH 2/5] Expose necessary APIs for testing ProofVerifier --- Cargo.lock | 1 + crates/subspace-fraud-proof/src/lib.rs | 3 +- test/subspace-test-client/src/lib.rs | 3 ++ test/subspace-test-service/Cargo.toml | 1 + test/subspace-test-service/src/lib.rs | 42 +++++++++++++++++++------- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8922a5eddfb..ec3f337242b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9035,6 +9035,7 @@ dependencies = [ "rand 0.8.5", "sc-cli", "sc-client-api", + "sc-executor", "sc-network", "sc-service", "sc-tracing", diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 619691ca2cc..d5c6acd4556 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -229,7 +229,8 @@ where } } - fn verify(&self, proof: &FraudProof) -> Result<(), VerificationError> { + /// Verifies the fraud proof. + pub fn verify(&self, proof: &FraudProof) -> Result<(), VerificationError> { let FraudProof { parent_hash, pre_state_root, diff --git a/test/subspace-test-client/src/lib.rs b/test/subspace-test-client/src/lib.rs index 890d68ce6b7..049dcfbd4d1 100644 --- a/test/subspace-test-client/src/lib.rs +++ b/test/subspace-test-client/src/lib.rs @@ -57,6 +57,9 @@ impl sc_executor::NativeExecutionDispatch for TestExecutorDispatch { /// The client type being used by the test service. pub type Client = FullClient; +/// The backend type being used by the test service. +pub type Backend = sc_service::TFullBackend; + /// Run a farmer. pub fn start_farmer(new_full: &NewFull>) { let client = new_full.client.clone(); diff --git a/test/subspace-test-service/Cargo.toml b/test/subspace-test-service/Cargo.toml index dbde569b3b5..56ca09dfce3 100644 --- a/test/subspace-test-service/Cargo.toml +++ b/test/subspace-test-service/Cargo.toml @@ -22,6 +22,7 @@ pallet-balances = { git = "https://github.com/paritytech/substrate", rev = "c364 pallet-transaction-payment = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } polkadot-overseer = { path = "../../polkadot/node/overseer" } sc-client-api = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sc-executor = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sc-network = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sc-service = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998", features = ["wasmtime"] } sc-tracing = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } diff --git a/test/subspace-test-service/src/lib.rs b/test/subspace-test-service/src/lib.rs index dd95db5610d..5014d77befb 100644 --- a/test/subspace-test-service/src/lib.rs +++ b/test/subspace-test-service/src/lib.rs @@ -21,6 +21,7 @@ use futures::future::Future; use polkadot_overseer::Handle; use sc_client_api::execution_extensions::ExecutionStrategies; +use sc_executor::NativeElseWasmExecutor; use sc_network::{ config::{NetworkConfiguration, TransportConfig}, multiaddr, @@ -36,7 +37,7 @@ use sp_runtime::{codec::Encode, generic, traits::IdentifyAccount, MultiSigner}; use std::sync::Arc; use subspace_runtime_primitives::Balance; use subspace_service::NewFull; -use subspace_test_client::{chain_spec, start_farmer, Client, TestExecutorDispatch}; +use subspace_test_client::{chain_spec, start_farmer, Backend, Client, TestExecutorDispatch}; use subspace_test_runtime::{ BlockHashCount, Runtime, SignedExtra, SignedPayload, UncheckedExtrinsic, VERSION, }; @@ -50,7 +51,16 @@ pub fn new_full( config: Configuration, enable_rpc_extensions: bool, run_farmer: bool, -) -> NewFull> { +) -> ( + NewFull>, + NativeElseWasmExecutor, +) { + let executor = NativeElseWasmExecutor::::new( + config.wasm_method, + config.default_heap_pages, + config.max_runtime_instances, + config.runtime_cache_size, + ); let new_full = futures::executor::block_on(subspace_service::new_full::< subspace_test_runtime::RuntimeApi, TestExecutorDispatch, @@ -59,7 +69,7 @@ pub fn new_full( if run_farmer { start_farmer(&new_full); } - new_full + (new_full, executor) } /// Create a Subspace `Configuration`. @@ -165,14 +175,18 @@ pub fn run_validator_node( ) -> SubspaceTestNode { let config = node_config(tokio_handle, key, boot_nodes, is_validator); let multiaddr = config.network.listen_addresses[0].clone(); - let NewFull { - task_manager, - client, - network, - rpc_handlers, - overseer_handle, - .. - } = new_full(config, is_validator, true); + let ( + NewFull { + task_manager, + client, + backend, + network, + rpc_handlers, + overseer_handle, + .. + }, + executor, + ) = new_full(config, is_validator, true); let peer_id = *network.local_peer_id(); let addr = MultiaddrWithPeerId { multiaddr, peer_id }; @@ -180,6 +194,8 @@ pub fn run_validator_node( SubspaceTestNode { task_manager, client, + backend, + executor, overseer_handle, addr, rpc_handlers, @@ -192,6 +208,10 @@ pub struct SubspaceTestNode { pub task_manager: TaskManager, /// Client's instance. pub client: Arc, + /// Backend. + pub backend: Arc, + /// Code executor. + pub executor: NativeElseWasmExecutor, /// A handle to Overseer. pub overseer_handle: Option, /// The `MultiaddrWithPeerId` to this node. This is useful if you want to pass it as "boot node" to other nodes. From 2d70418ff37f109a03f5a3fda447f929531d97d2 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Thu, 31 Mar 2022 10:48:25 +0800 Subject: [PATCH 3/5] Test ProofVerifier --- cumulus/client/cirrus-executor/src/tests.rs | 107 +++++++++++++++++--- 1 file changed, 92 insertions(+), 15 deletions(-) diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index 9f9edf96075..d4836fdd1ff 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -8,7 +8,7 @@ use cirrus_test_service::{ use codec::Encode; use sc_client_api::{HeaderBackend, StorageProof}; use sp_api::ProvideRuntimeApi; -use sp_executor::ExecutionPhase; +use sp_executor::{ExecutionPhase, FraudProof}; use sp_runtime::{ generic::BlockId, traits::{BlakeTwo256, Header as HeaderT}, @@ -185,6 +185,7 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Create `initialize_block` proof"); + // Test `initialize_block` verification on executor. let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, @@ -192,14 +193,33 @@ async fn execution_proof_creation_and_verification_should_work() { &BlockId::Hash(parent_header.hash()), &execution_phase, *parent_header.state_root(), - storage_proof, + storage_proof.clone(), ) .expect("Check `initialize_block` proof"); - let post_execution_root = execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[0].into()); + // Test `initialize_block` verification on farmer. + let proof_verifier = subspace_fraud_proof::ProofVerifier::new( + alice.client.clone(), + alice.backend.clone(), + alice.executor.clone(), + alice.task_manager.spawn_handle(), + ); + + // Incorrect but it's fine for the test purpose. + let parent_hash_alice = alice.client.info().best_hash; + + let fraud_proof = FraudProof { + parent_hash: parent_hash_alice, + pre_state_root: *parent_header.state_root(), + post_state_root: intermediate_roots[0].into(), + proof: storage_proof, + execution_phase, + }; + assert!(proof_verifier.verify(&fraud_proof).is_ok()); + // Test extrinsic execution. for (target_extrinsic_index, xt) in test_txs.clone().into_iter().enumerate() { let storage_changes = create_block_builder() @@ -226,6 +246,7 @@ async fn execution_proof_creation_and_verification_should_work() { let target_trace_root: Hash = intermediate_roots[target_extrinsic_index].into(); assert_eq!(target_trace_root, post_delta_root); + // Test `apply_extrinsic` verification on executor. let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, @@ -233,13 +254,22 @@ async fn execution_proof_creation_and_verification_should_work() { &BlockId::Hash(parent_header.hash()), &execution_phase, post_delta_root, - storage_proof, + storage_proof.clone(), ) .expect("Check extrinsic execution proof"); - let post_execution_root = execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[target_extrinsic_index + 1].into()); + + // Test `apply_extrinsic` verification on farmer. + let fraud_proof = FraudProof { + parent_hash: parent_hash_alice, + pre_state_root: intermediate_roots[target_extrinsic_index].into(), + post_state_root: intermediate_roots[target_extrinsic_index + 1].into(), + proof: storage_proof, + execution_phase, + }; + assert!(proof_verifier.verify(&fraud_proof).is_ok()); } // Test `finalize_block` @@ -264,6 +294,7 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Create `finalize_block` proof"); + // Test `finalize_block` verification on executor. let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, @@ -271,13 +302,22 @@ async fn execution_proof_creation_and_verification_should_work() { &BlockId::Hash(parent_header.hash()), &execution_phase, post_delta_root, - storage_proof, + storage_proof.clone(), ) .expect("Check `finalize_block` proof"); - let post_execution_root = execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, *header.state_root()); + + // Test `finalize_block` verification on farmer. + let fraud_proof = FraudProof { + parent_hash: parent_hash_alice, + pre_state_root: intermediate_roots.last().unwrap().into(), + post_state_root: post_execution_root.into(), + proof: storage_proof, + execution_phase, + }; + assert!(proof_verifier.verify(&fraud_proof).is_ok()); } #[substrate_test_utils::test] @@ -377,13 +417,13 @@ async fn invalid_execution_proof_should_not_work() { ) .expect("Create extrinsic execution proof"); - (proof, delta, post_delta_root) + (proof, post_delta_root, execution_phase) }; - let (proof0, _delta0, post_delta_root0) = create_extrinsic_proof(0); - let (proof1, _delta1, post_delta_root1) = create_extrinsic_proof(1); + let (proof0, post_delta_root0, execution_phase0) = create_extrinsic_proof(0); + let (proof1, post_delta_root1, execution_phase1) = create_extrinsic_proof(1); - let check_proof = |post_delta_root: Hash, proof: StorageProof| { + let check_proof_executor = |post_delta_root: Hash, proof: StorageProof| { let execution_phase = ExecutionPhase::ApplyExtrinsic { call_data: transfer_to_charlie_again.encode() }; subspace_fraud_proof::check_execution_proof( @@ -397,8 +437,45 @@ async fn invalid_execution_proof_should_not_work() { ) }; - assert!(check_proof(post_delta_root1, proof0.clone()).is_err()); - assert!(check_proof(post_delta_root0, proof1.clone()).is_err()); - assert!(check_proof(post_delta_root0, proof0).is_ok()); - assert!(check_proof(post_delta_root1, proof1).is_ok()); + assert!(check_proof_executor(post_delta_root1, proof0.clone()).is_err()); + assert!(check_proof_executor(post_delta_root0, proof1.clone()).is_err()); + assert!(check_proof_executor(post_delta_root0, proof0.clone()).is_ok()); + assert!(check_proof_executor(post_delta_root1, proof1.clone()).is_ok()); + + let proof_verifier = subspace_fraud_proof::ProofVerifier::new( + alice.client.clone(), + alice.backend.clone(), + alice.executor.clone(), + alice.task_manager.spawn_handle(), + ); + + // Incorrect but it's fine for the test purpose. + let parent_hash_alice = alice.client.info().best_hash; + + let fraud_proof = FraudProof { + parent_hash: parent_hash_alice, + pre_state_root: post_delta_root0, + post_state_root: post_delta_root1, + proof: proof1, + execution_phase: execution_phase0.clone(), + }; + assert!(proof_verifier.verify(&fraud_proof).is_err()); + + let fraud_proof = FraudProof { + parent_hash: parent_hash_alice, + pre_state_root: post_delta_root0, + post_state_root: post_delta_root1, + proof: proof0.clone(), + execution_phase: execution_phase1, + }; + assert!(proof_verifier.verify(&fraud_proof).is_err()); + + let fraud_proof = FraudProof { + parent_hash: parent_hash_alice, + pre_state_root: post_delta_root0, + post_state_root: post_delta_root1, + proof: proof0, + execution_phase: execution_phase0, + }; + assert!(proof_verifier.verify(&fraud_proof).is_ok()); } From 0f43d8981314701519caed1fc026916fa51917c1 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Thu, 31 Mar 2022 11:56:39 +0800 Subject: [PATCH 4/5] Extract `ExecutionProver` --- crates/subspace-fraud-proof/src/lib.rs | 184 +++++++++++--------- cumulus/client/cirrus-executor/src/lib.rs | 35 ++-- cumulus/client/cirrus-executor/src/tests.rs | 143 +++++++-------- 3 files changed, 178 insertions(+), 184 deletions(-) diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index d5c6acd4556..d0e6bc40102 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -29,106 +29,118 @@ use sp_trie::DBValue; use std::marker::PhantomData; use std::sync::Arc; -/// Returns a storage proof which can be used to reconstruct a partial state trie to re-run -/// the execution by someone who does not own the whole state. -pub fn prove_execution< +/// Creates storage proof for verifying an execution without owning the whole state. +pub struct ExecutionProver { + backend: Arc, + executor: Arc, + spawn_handle: Box, + _phantom: PhantomData, +} + +impl ExecutionProver +where Block: BlockT, B: backend::Backend, Exec: CodeExecutor + 'static, - Spawn: SpawnNamed + Send + 'static, - DB: HashDB, DBValue>, ->( - backend: &Arc, - executor: &Exec, - spawn_handle: Spawn, - at: &BlockId, - execution_phase: &ExecutionPhase, - delta_changes: Option<(DB, Block::Hash)>, -) -> sp_blockchain::Result { - let state = backend.state_at(*at)?; +{ + /// Constructs a new instance of [`ExecutionProver`]. + pub fn new(backend: Arc, executor: Arc, spawn_handle: Box) -> Self { + Self { + backend, + executor, + spawn_handle, + _phantom: PhantomData::, + } + } - let trie_backend = state.as_trie_backend().ok_or_else(|| { - Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof) - as Box - })?; + /// Returns a storage proof which can be used to reconstruct a partial state trie to re-run + /// the execution by someone who does not own the whole state. + pub fn prove_execution, DBValue>>( + &self, + at: BlockId, + execution_phase: &ExecutionPhase, + delta_changes: Option<(DB, Block::Hash)>, + ) -> sp_blockchain::Result { + // TODO: fetch the runtime code properly. + let state = self.backend.state_at(at)?; - let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend); - let runtime_code = state_runtime_code - .runtime_code() - .map_err(sp_blockchain::Error::RuntimeCode)?; + let trie_backend = state.as_trie_backend().ok_or_else(|| { + Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof) + as Box + })?; - 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(), - executor, - spawn_handle, - execution_phase.proving_method(), - execution_phase.call_data(), - &runtime_code, - ) - .map(|(_ret, proof)| proof) - .map_err(Into::into) - } else { - sp_state_machine::prove_execution_on_trie_backend( - trie_backend, + let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend); + let runtime_code = state_runtime_code + .runtime_code() + .map_err(sp_blockchain::Error::RuntimeCode)?; + + 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, + self.spawn_handle.clone(), + execution_phase.proving_method(), + execution_phase.call_data(), + &runtime_code, + ) + .map(|(_ret, proof)| proof) + .map_err(Into::into) + } else { + sp_state_machine::prove_execution_on_trie_backend( + trie_backend, + &mut Default::default(), + &*self.executor, + self.spawn_handle.clone(), + execution_phase.proving_method(), + execution_phase.call_data(), + &runtime_code, + ) + .map(|(_ret, proof)| proof) + .map_err(Into::into) + } + } + + /// Runs the execution using the partial state constructed from the given storage proof and + /// returns the execution result. + /// + /// The execution result contains the information of state root after applying the execution + /// so that it can be used to compare with the one specified in the fraud proof. + pub fn check_execution_proof( + &self, + at: BlockId, + execution_phase: &ExecutionPhase, + pre_execution_root: H256, + proof: StorageProof, + ) -> sp_blockchain::Result> { + // TODO: fetch the runtime code properly. + let state = self.backend.state_at(at)?; + + let trie_backend = state.as_trie_backend().ok_or_else(|| { + Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof) + as Box + })?; + + let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend); + let runtime_code = state_runtime_code + .runtime_code() + .map_err(sp_blockchain::Error::RuntimeCode)?; + + sp_state_machine::execution_proof_check::( + pre_execution_root, + proof, &mut Default::default(), - executor, - spawn_handle, - execution_phase.proving_method(), + &*self.executor, + self.spawn_handle.clone(), + execution_phase.verifying_method(), execution_phase.call_data(), &runtime_code, ) - .map(|(_ret, proof)| proof) .map_err(Into::into) } } -/// Runs the execution using the partial state constructed from the given storage proof and -/// returns the execution result. -/// -/// The execution result contains the information of state root after applying the execution -/// so that it can be used to compare with the one specified in the fraud proof. -pub fn check_execution_proof< - Block: BlockT, - B: backend::Backend, - Exec: CodeExecutor + 'static, - Spawn: SpawnNamed + Send + 'static, ->( - backend: &Arc, - executor: &Exec, - spawn_handle: Spawn, - at: &BlockId, - execution_phase: &ExecutionPhase, - pre_execution_root: H256, - proof: StorageProof, -) -> sp_blockchain::Result> { - let state = backend.state_at(*at)?; - - let trie_backend = state.as_trie_backend().ok_or_else(|| { - Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof) - as Box - })?; - - let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend); - let runtime_code = state_runtime_code - .runtime_code() - .map_err(sp_blockchain::Error::RuntimeCode)?; - - sp_state_machine::execution_proof_check::( - pre_execution_root, - proof, - &mut Default::default(), - executor, - spawn_handle, - execution_phase.verifying_method(), - execution_phase.call_data(), - &runtime_code, - ) - .map_err(Into::into) -} - /// Create a new trie backend with memory DB delta changes. /// /// This can be used to verify any extrinsic-specific execution on the combined state of `backend` diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index 84551df62f1..9030a63d660 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -314,6 +314,7 @@ where extrinsic_index: usize, parent_header: &Block::Header, current_hash: Block::Hash, + prover: &subspace_fraud_proof::ExecutionProver, ) -> Result<(StorageProof, ExecutionPhase), GossipMessageError> { let extrinsics = self.block_body(current_hash)?; @@ -342,11 +343,8 @@ where let post_delta_root = storage_changes.transaction_storage_root; // TODO: way to call some runtime api against any specific state instead of having // to work with String API directly. - let execution_proof = subspace_fraud_proof::prove_execution( - &self.backend, - &*self.code_executor, - self.spawner.clone() as Box, - &BlockId::Hash(parent_header.hash()), + let execution_proof = prover.prove_execution( + BlockId::Hash(parent_header.hash()), &execution_phase, Some((delta, post_delta_root)), )?; @@ -594,6 +592,12 @@ where .map_err(|_| Self::Error::InvalidStateRootType) }; + let prover = subspace_fraud_proof::ExecutionProver::new( + self.backend.clone(), + self.code_executor.clone(), + self.spawner.clone() as Box, + ); + // TODO: abstract the execution proof impl to be reusable in the test. let fraud_proof = if local_trace_idx == 0 { // `initialize_block` execution proof. @@ -612,17 +616,8 @@ where // TODO: way to call some runtime api against any specific state instead of having // to work with String API directly. - let proof = subspace_fraud_proof::prove_execution::< - _, - _, - _, - _, - TransactionFor, - >( - &self.backend, - &*self.code_executor, - self.spawner.clone() as Box, - &BlockId::Hash(parent_header.hash()), + let proof = prover.prove_execution::>( + BlockId::Hash(parent_header.hash()), &execution_phase, None, )?; @@ -657,11 +652,8 @@ where // TODO: way to call some runtime api against any specific state instead of having // to work with String API directly. - let proof = subspace_fraud_proof::prove_execution( - &self.backend, - &*self.code_executor, - self.spawner.clone() as Box, - &BlockId::Hash(parent_header.hash()), + let proof = prover.prove_execution( + BlockId::Hash(parent_header.hash()), &execution_phase, Some((delta, post_delta_root)), )?; @@ -682,6 +674,7 @@ where local_trace_idx - 1, &parent_header, execution_receipt.secondary_hash, + &prover, )?; // TODO: proof should be a CompactProof. diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index d4836fdd1ff..d9b2edd261e 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -168,34 +168,30 @@ async fn execution_proof_creation_and_verification_should_work() { ); let execution_phase = ExecutionPhase::InitializeBlock { call_data: new_header.encode() }; + let prover = subspace_fraud_proof::ExecutionProver::new( + charlie.backend.clone(), + charlie.code_executor.clone(), + Box::new(charlie.task_manager.spawn_handle()), + ); + // Test `initialize_block`. - let storage_proof = subspace_fraud_proof::prove_execution::< - _, - _, - _, - _, - sp_trie::PrefixedMemoryDB, - >( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - None, - ) - .expect("Create `initialize_block` proof"); + let storage_proof = prover + .prove_execution::>( + BlockId::Hash(parent_header.hash()), + &execution_phase, + None, + ) + .expect("Create `initialize_block` proof"); // Test `initialize_block` verification on executor. - let execution_result = subspace_fraud_proof::check_execution_proof( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - *parent_header.state_root(), - storage_proof.clone(), - ) - .expect("Check `initialize_block` proof"); + let execution_result = prover + .check_execution_proof( + BlockId::Hash(parent_header.hash()), + &execution_phase, + *parent_header.state_root(), + storage_proof.clone(), + ) + .expect("Check `initialize_block` proof"); let post_execution_root = execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[0].into()); @@ -233,30 +229,26 @@ async fn execution_proof_creation_and_verification_should_work() { let execution_phase = ExecutionPhase::ApplyExtrinsic { call_data: xt.encode() }; - let storage_proof = subspace_fraud_proof::prove_execution( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - Some((delta, post_delta_root)), - ) - .expect("Create extrinsic execution proof"); + let storage_proof = prover + .prove_execution( + BlockId::Hash(parent_header.hash()), + &execution_phase, + Some((delta, post_delta_root)), + ) + .expect("Create extrinsic execution proof"); let target_trace_root: Hash = intermediate_roots[target_extrinsic_index].into(); assert_eq!(target_trace_root, post_delta_root); // Test `apply_extrinsic` verification on executor. - let execution_result = subspace_fraud_proof::check_execution_proof( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - post_delta_root, - storage_proof.clone(), - ) - .expect("Check extrinsic execution proof"); + let execution_result = prover + .check_execution_proof( + BlockId::Hash(parent_header.hash()), + &execution_phase, + post_delta_root, + storage_proof.clone(), + ) + .expect("Check extrinsic execution proof"); let post_execution_root = execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[target_extrinsic_index + 1].into()); @@ -284,27 +276,23 @@ async fn execution_proof_creation_and_verification_should_work() { let execution_phase = ExecutionPhase::FinalizeBlock; - let storage_proof = subspace_fraud_proof::prove_execution( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - Some((delta, post_delta_root)), - ) - .expect("Create `finalize_block` proof"); + let storage_proof = prover + .prove_execution( + BlockId::Hash(parent_header.hash()), + &execution_phase, + Some((delta, post_delta_root)), + ) + .expect("Create `finalize_block` proof"); // Test `finalize_block` verification on executor. - let execution_result = subspace_fraud_proof::check_execution_proof( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - post_delta_root, - storage_proof.clone(), - ) - .expect("Check `finalize_block` proof"); + let execution_result = prover + .check_execution_proof( + BlockId::Hash(parent_header.hash()), + &execution_phase, + post_delta_root, + storage_proof.clone(), + ) + .expect("Check `finalize_block` proof"); let post_execution_root = execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, *header.state_root()); @@ -396,6 +384,12 @@ async fn invalid_execution_proof_should_not_work() { .unwrap() }; + let prover = subspace_fraud_proof::ExecutionProver::new( + charlie.backend.clone(), + charlie.code_executor.clone(), + Box::new(charlie.task_manager.spawn_handle()), + ); + let create_extrinsic_proof = |extrinsic_index: usize| { let storage_changes = create_block_builder() .prepare_storage_changes_before(extrinsic_index) @@ -407,15 +401,13 @@ async fn invalid_execution_proof_should_not_work() { let execution_phase = ExecutionPhase::ApplyExtrinsic { call_data: test_txs[extrinsic_index].encode() }; - let proof = subspace_fraud_proof::prove_execution( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - &execution_phase, - Some((delta.clone(), post_delta_root.clone())), - ) - .expect("Create extrinsic execution proof"); + let proof = prover + .prove_execution( + BlockId::Hash(parent_header.hash()), + &execution_phase, + Some((delta.clone(), post_delta_root.clone())), + ) + .expect("Create extrinsic execution proof"); (proof, post_delta_root, execution_phase) }; @@ -426,11 +418,8 @@ async fn invalid_execution_proof_should_not_work() { let check_proof_executor = |post_delta_root: Hash, proof: StorageProof| { let execution_phase = ExecutionPhase::ApplyExtrinsic { call_data: transfer_to_charlie_again.encode() }; - subspace_fraud_proof::check_execution_proof( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), + prover.check_execution_proof( + BlockId::Hash(parent_header.hash()), &execution_phase, post_delta_root, proof, From 256b421e469f68984c926dbfb14c76a29b60e4d5 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Thu, 31 Mar 2022 13:42:21 +0800 Subject: [PATCH 5/5] Update some comments --- crates/subspace-fraud-proof/src/lib.rs | 6 ++++-- cumulus/client/cirrus-executor/src/lib.rs | 6 ------ cumulus/client/cirrus-executor/src/tests.rs | 9 +++------ 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index d0e6bc40102..361cbf85395 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -61,7 +61,7 @@ where execution_phase: &ExecutionPhase, delta_changes: Option<(DB, Block::Hash)>, ) -> sp_blockchain::Result { - // TODO: fetch the runtime code properly. + // TODO: fetch the runtime code from the primary chain instead of the local state. let state = self.backend.state_at(at)?; let trie_backend = state.as_trie_backend().ok_or_else(|| { @@ -74,6 +74,8 @@ where .runtime_code() .map_err(sp_blockchain::Error::RuntimeCode)?; + // TODO: avoid using the String API specified by `proving_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( @@ -114,7 +116,7 @@ where pre_execution_root: H256, proof: StorageProof, ) -> sp_blockchain::Result> { - // TODO: fetch the runtime code properly. + // TODO: fetch the runtime code from the primary chain instead of the local state. let state = self.backend.state_at(at)?; let trie_backend = state.as_trie_backend().ok_or_else(|| { diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index 9030a63d660..f61072e692c 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -341,8 +341,6 @@ where let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; - // TODO: way to call some runtime api against any specific state instead of having - // to work with String API directly. let execution_proof = prover.prove_execution( BlockId::Hash(parent_header.hash()), &execution_phase, @@ -614,8 +612,6 @@ where let execution_phase = ExecutionPhase::InitializeBlock { call_data: new_header.encode() }; - // TODO: way to call some runtime api against any specific state instead of having - // to work with String API directly. let proof = prover.prove_execution::>( BlockId::Hash(parent_header.hash()), &execution_phase, @@ -650,8 +646,6 @@ where let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; - // TODO: way to call some runtime api against any specific state instead of having - // to work with String API directly. let proof = prover.prove_execution( BlockId::Hash(parent_header.hash()), &execution_phase, diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index d9b2edd261e..148bfda7acc 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -183,7 +183,7 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Create `initialize_block` proof"); - // Test `initialize_block` verification on executor. + // Test `initialize_block` verification. let execution_result = prover .check_execution_proof( BlockId::Hash(parent_header.hash()), @@ -196,7 +196,6 @@ async fn execution_proof_creation_and_verification_should_work() { execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[0].into()); - // Test `initialize_block` verification on farmer. let proof_verifier = subspace_fraud_proof::ProofVerifier::new( alice.client.clone(), alice.backend.clone(), @@ -240,7 +239,7 @@ async fn execution_proof_creation_and_verification_should_work() { let target_trace_root: Hash = intermediate_roots[target_extrinsic_index].into(); assert_eq!(target_trace_root, post_delta_root); - // Test `apply_extrinsic` verification on executor. + // Test `apply_extrinsic` verification. let execution_result = prover .check_execution_proof( BlockId::Hash(parent_header.hash()), @@ -253,7 +252,6 @@ async fn execution_proof_creation_and_verification_should_work() { execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[target_extrinsic_index + 1].into()); - // Test `apply_extrinsic` verification on farmer. let fraud_proof = FraudProof { parent_hash: parent_hash_alice, pre_state_root: intermediate_roots[target_extrinsic_index].into(), @@ -284,7 +282,7 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Create `finalize_block` proof"); - // Test `finalize_block` verification on executor. + // Test `finalize_block` verification. let execution_result = prover .check_execution_proof( BlockId::Hash(parent_header.hash()), @@ -297,7 +295,6 @@ async fn execution_proof_creation_and_verification_should_work() { execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, *header.state_root()); - // Test `finalize_block` verification on farmer. let fraud_proof = FraudProof { parent_hash: parent_hash_alice, pre_state_root: intermediate_roots.last().unwrap().into(),