From 07fb77e36123e303c14043d607fc2395bb40f355 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sun, 27 Mar 2022 22:10:05 +0800 Subject: [PATCH 1/9] Move cirrus-fraud-proof to subspace-fraud-proof --- Cargo.lock | 32 +++++++++---------- Cargo.toml | 1 - .../subspace-fraud-proof}/Cargo.toml | 2 +- .../subspace-fraud-proof}/src/lib.rs | 0 cumulus/client/cirrus-executor/Cargo.toml | 2 +- cumulus/client/cirrus-executor/src/lib.rs | 6 ++-- cumulus/client/cirrus-executor/src/tests.rs | 16 +++++----- 7 files changed, 29 insertions(+), 30 deletions(-) rename {cumulus/client/fraud-proof => crates/subspace-fraud-proof}/Cargo.toml (97%) rename {cumulus/client/fraud-proof => crates/subspace-fraud-proof}/src/lib.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index db155579402..bc9f067e0b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -840,7 +840,6 @@ version = "0.1.0" dependencies = [ "cirrus-block-builder", "cirrus-client-executor-gossip", - "cirrus-fraud-proof", "cirrus-node-primitives", "cirrus-primitives", "cirrus-test-service", @@ -874,6 +873,7 @@ dependencies = [ "sp-runtime", "sp-trie", "subspace-core-primitives", + "subspace-fraud-proof", "subspace-runtime-primitives", "substrate-test-runtime", "substrate-test-runtime-client", @@ -931,21 +931,6 @@ dependencies = [ "tracing", ] -[[package]] -name = "cirrus-fraud-proof" -version = "0.1.0" -dependencies = [ - "hash-db", - "parity-scale-codec", - "sc-client-api", - "sp-api", - "sp-blockchain", - "sp-core", - "sp-runtime", - "sp-state-machine", - "sp-trie", -] - [[package]] name = "cirrus-node-primitives" version = "0.1.0" @@ -8756,6 +8741,21 @@ dependencies = [ "zeroize", ] +[[package]] +name = "subspace-fraud-proof" +version = "0.1.0" +dependencies = [ + "hash-db", + "parity-scale-codec", + "sc-client-api", + "sp-api", + "sp-blockchain", + "sp-core", + "sp-runtime", + "sp-state-machine", + "sp-trie", +] + [[package]] name = "subspace-networking" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 6143231c9fe..5d1d6b2228a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,6 @@ members = [ "cumulus/client/consensus/common", "cumulus/client/consensus/relay-chain", "cumulus/client/executor-gossip", - "cumulus/client/fraud-proof", "cumulus/pallets/executive", "cumulus/parachain-template/node", "cumulus/parachain-template/runtime", diff --git a/cumulus/client/fraud-proof/Cargo.toml b/crates/subspace-fraud-proof/Cargo.toml similarity index 97% rename from cumulus/client/fraud-proof/Cargo.toml rename to crates/subspace-fraud-proof/Cargo.toml index d7a3c406fb3..d646fd95464 100644 --- a/cumulus/client/fraud-proof/Cargo.toml +++ b/crates/subspace-fraud-proof/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "cirrus-fraud-proof" +name = "subspace-fraud-proof" version = "0.1.0" authors = ["Liu-Cheng Xu "] edition = "2021" diff --git a/cumulus/client/fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs similarity index 100% rename from cumulus/client/fraud-proof/src/lib.rs rename to crates/subspace-fraud-proof/src/lib.rs diff --git a/cumulus/client/cirrus-executor/Cargo.toml b/cumulus/client/cirrus-executor/Cargo.toml index 7328a2631a0..014614d63dd 100644 --- a/cumulus/client/cirrus-executor/Cargo.toml +++ b/cumulus/client/cirrus-executor/Cargo.toml @@ -42,9 +42,9 @@ polkadot-node-subsystem = { path = "../../../polkadot/node/subsystem" } cirrus-block-builder = { path = "../block-builder" } cirrus-client-executor-gossip = { path = "../executor-gossip" } cirrus-node-primitives = { path = "../../../crates/cirrus-node-primitives" } -cirrus-fraud-proof = { path = "../fraud-proof" } cirrus-primitives = { path = "../../primitives" } sp-executor = { path = "../../../crates/sp-executor" } +subspace-fraud-proof = { path = "../../../crates/subspace-fraud-proof" } subspace-core-primitives = { path = "../../../crates/subspace-core-primitives" } subspace-runtime-primitives = { path = "../../../crates/subspace-runtime-primitives" } diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index 2014cca215e..d5f865162a1 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -340,7 +340,7 @@ 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 = cirrus_fraud_proof::prove_execution( + let execution_proof = subspace_fraud_proof::prove_execution( &self.backend, &*self.code_executor, self.spawner.clone() as Box, @@ -609,7 +609,7 @@ where // TODO: way to call some runtime api against any specific state instead of having // to work with String API directly. - let proof = cirrus_fraud_proof::prove_execution::< + let proof = subspace_fraud_proof::prove_execution::< _, _, _, @@ -648,7 +648,7 @@ where // TODO: way to call some runtime api against any specific state instead of having // to work with String API directly. - let proof = cirrus_fraud_proof::prove_execution( + let proof = subspace_fraud_proof::prove_execution( &self.backend, &*self.code_executor, self.spawner.clone() as Box, diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index 69cdf93672b..79a5bf6a1a7 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -168,7 +168,7 @@ async fn execution_proof_creation_and_verification_should_work() { Default::default(), ); - cirrus_fraud_proof::prove_execution::<_, _, _, _, sp_trie::PrefixedMemoryDB>( + subspace_fraud_proof::prove_execution::<_, _, _, _, sp_trie::PrefixedMemoryDB>( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -180,7 +180,7 @@ async fn execution_proof_creation_and_verification_should_work() { .expect("Create `initialize_block` proof") }; - let execution_result = cirrus_fraud_proof::check_execution_proof( + let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -207,7 +207,7 @@ async fn execution_proof_creation_and_verification_should_work() { let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; - let storage_proof = cirrus_fraud_proof::prove_execution( + let storage_proof = subspace_fraud_proof::prove_execution( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -221,7 +221,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); - let execution_result = cirrus_fraud_proof::check_execution_proof( + let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -248,7 +248,7 @@ async fn execution_proof_creation_and_verification_should_work() { assert_eq!(post_delta_root, intermediate_roots.last().unwrap().into()); - let storage_proof = cirrus_fraud_proof::prove_execution( + let storage_proof = subspace_fraud_proof::prove_execution( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -259,7 +259,7 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Create `finalize_block` proof"); - let execution_result = cirrus_fraud_proof::check_execution_proof( + let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -360,7 +360,7 @@ async fn invalid_execution_proof_should_not_work() { let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; - let proof = cirrus_fraud_proof::prove_execution( + let proof = subspace_fraud_proof::prove_execution( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), @@ -378,7 +378,7 @@ async fn invalid_execution_proof_should_not_work() { let (proof1, _delta1, post_delta_root1) = create_extrinsic_proof(1); let check_proof = |post_delta_root: Hash, proof: StorageProof| { - cirrus_fraud_proof::check_execution_proof( + subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), From a29e7bbee88737f4c2423489bab8fb684b88e0d9 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sun, 27 Mar 2022 22:38:35 +0800 Subject: [PATCH 2/9] Add version info to subspace-fraud-proof deps --- crates/subspace-fraud-proof/Cargo.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/subspace-fraud-proof/Cargo.toml b/crates/subspace-fraud-proof/Cargo.toml index d646fd95464..4431798d52a 100644 --- a/crates/subspace-fraud-proof/Cargo.toml +++ b/crates/subspace-fraud-proof/Cargo.toml @@ -14,10 +14,10 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.1.2", features = ["derive"] } hash-db = "0.15.2" -sc-client-api = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } -sp-api = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } -sp-blockchain = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } -sp-core = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } -sp-runtime = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } -sp-state-machine = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } -sp-trie = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sc-client-api = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-api = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-blockchain = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-core = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-runtime = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-state-machine = { version = "0.12.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-trie = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } From 0474277d9b6d65d4ee32494308f97425daa33273 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sun, 27 Mar 2022 23:09:55 +0800 Subject: [PATCH 3/9] Introduce FraudProof externalities This externalities extension will allow the fraud proof verification function implemented in the std environment to be called within the runtime. --- Cargo.lock | 9 + crates/pallet-executor/src/lib.rs | 9 +- crates/sp-executor/Cargo.toml | 6 + crates/sp-executor/src/lib.rs | 43 ++++ crates/subspace-fraud-proof/Cargo.toml | 3 + crates/subspace-fraud-proof/src/lib.rs | 300 ++++++++++++++++--------- crates/subspace-node/Cargo.toml | 1 + crates/subspace-node/src/lib.rs | 7 +- crates/subspace-service/Cargo.toml | 1 + crates/subspace-service/src/lib.rs | 12 +- test/subspace-test-client/Cargo.toml | 1 + test/subspace-test-client/src/lib.rs | 2 +- 12 files changed, 273 insertions(+), 121 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc9f067e0b3..7c9da9554fe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8203,7 +8203,10 @@ dependencies = [ "sp-api", "sp-consensus-slots", "sp-core", + "sp-externalities", "sp-runtime", + "sp-runtime-interface", + "sp-state-machine", "sp-std", "sp-trie", "subspace-core-primitives", @@ -8751,9 +8754,12 @@ dependencies = [ "sp-api", "sp-blockchain", "sp-core", + "sp-executor", + "sp-externalities", "sp-runtime", "sp-state-machine", "sp-trie", + "tracing", ] [[package]] @@ -8795,6 +8801,7 @@ dependencies = [ "serde", "sp-consensus", "sp-core", + "sp-executor", "sp-runtime", "subspace-archiving", "subspace-core-primitives", @@ -8922,6 +8929,7 @@ dependencies = [ "sp-timestamp", "sp-transaction-pool", "sp-trie", + "subspace-fraud-proof", "subspace-runtime-primitives", "substrate-frame-rpc-system", "substrate-prometheus-endpoint", @@ -8958,6 +8966,7 @@ dependencies = [ "sp-api", "sp-consensus-subspace", "sp-core", + "sp-executor", "sp-runtime", "subspace-archiving", "subspace-core-primitives", diff --git a/crates/pallet-executor/src/lib.rs b/crates/pallet-executor/src/lib.rs index ec3c12216db..a5e1563241c 100644 --- a/crates/pallet-executor/src/lib.rs +++ b/crates/pallet-executor/src/lib.rs @@ -212,8 +212,8 @@ mod pallet { } Call::submit_fraud_proof { fraud_proof } => { // TODO: prevent the spamming of fraud proof transaction. - if let Err(e) = Self::check_fraud_proof(fraud_proof) { - log::error!(target: "runtime::subspace::executor", "Invalid fraud proof: {:?}", e); + if !sp_executor::fraud_proof_ext::fraud_proof::verify(fraud_proof) { + log::error!(target: "runtime::subspace::executor", "Invalid fraud proof: {:?}", fraud_proof); return InvalidTransaction::Custom(INVALID_FRAUD_PROOF).into(); } // TODO: proper tag value. @@ -264,11 +264,6 @@ mod pallet { } impl Pallet { - // TODO: Checks if the fraud proof is valid. - fn check_fraud_proof(_fraud_proof: &FraudProof) -> Result<(), Error> { - Ok(()) - } - // TODO: Checks if the bundle equivocation proof is valid. fn check_bundle_equivocation_proof( _bundle_equivocation_proof: &BundleEquivocationProof, diff --git a/crates/sp-executor/Cargo.toml b/crates/sp-executor/Cargo.toml index b317f68db34..a164cd24b4e 100644 --- a/crates/sp-executor/Cargo.toml +++ b/crates/sp-executor/Cargo.toml @@ -17,7 +17,10 @@ scale-info = { version = "2.0.1", default-features = false, features = ["derive" sp-api = { version = "4.0.0-dev", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-consensus-slots = { version = "0.10.0-dev", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-core = { version = "6.0.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-externalities = { version = "0.12.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-runtime = { version = "6.0.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-runtime-interface = { version = "6.0.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-state-machine = { version = "0.12.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-std = { version = "4.0.0-dev", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-trie = { version = "6.0.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } subspace-core-primitives = { version = "0.1.0", default-features = false, path = "../subspace-core-primitives" } @@ -31,7 +34,10 @@ std = [ "sp-api/std", "sp-consensus-slots/std", "sp-core/std", + "sp-externalities/std", "sp-runtime/std", + "sp-runtime-interface/std", + "sp-state-machine/std", "sp-std/std", "sp-trie/std", "subspace-core-primitives/std", diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index 62782ddfb23..d96a3d4cd81 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -23,6 +23,7 @@ use sp_consensus_slots::Slot; use sp_core::H256; use sp_runtime::traits::{BlakeTwo256, Hash as HashT}; use sp_runtime::{OpaqueExtrinsic, RuntimeDebug}; +use sp_runtime_interface::pass_by::PassBy; use sp_std::borrow::Cow; use sp_std::vec::Vec; use sp_trie::StorageProof; @@ -125,6 +126,10 @@ impl From> for OpaqueExecutionReceipt { } } +/// Error type of fraud proof verification on primary node. +#[derive(RuntimeDebug)] +pub struct VerificationError; + /// Fraud proof for the state computation. #[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone, RuntimeDebug)] pub struct FraudProof { @@ -136,6 +141,10 @@ pub struct FraudProof { pub proof: StorageProof, } +impl PassBy for FraudProof { + type PassBy = sp_runtime_interface::pass_by::Codec; +} + /// Represents a bundle equivocation proof. An equivocation happens when an executor /// produces more than one bundle on the same slot. The proof of equivocation /// are the given distinct bundle headers that were signed by the validator and which @@ -213,3 +222,37 @@ sp_api::decl_runtime_apis! { fn execution_wasm_bundle() -> Cow<'static, [u8]>; } } + +pub mod fraud_proof_ext { + use sp_externalities::ExternalitiesExt; + use sp_runtime_interface::runtime_interface; + + /// Externalities for verifying fraud proof. + pub trait Externalities: Send { + /// Returns `true` when the proof is valid. + fn verify_fraud_proof(&self, proof: &crate::FraudProof) -> bool; + } + + #[cfg(feature = "std")] + sp_externalities::decl_extension! { + /// An extension to verify the fraud proof. + pub struct FraudProofExt(Box); + } + + #[cfg(feature = "std")] + impl FraudProofExt { + pub fn new(fraud_proof: E) -> Self { + Self(Box::new(fraud_proof)) + } + } + + #[runtime_interface] + pub trait FraudProof { + /// Verify fraud proof. + fn verify(&mut self, proof: &crate::FraudProof) -> bool { + self.extension::() + .expect("No `FraudProof` associated for the current context!") + .verify_fraud_proof(proof) + } + } +} diff --git a/crates/subspace-fraud-proof/Cargo.toml b/crates/subspace-fraud-proof/Cargo.toml index 4431798d52a..7e29b63dd05 100644 --- a/crates/subspace-fraud-proof/Cargo.toml +++ b/crates/subspace-fraud-proof/Cargo.toml @@ -18,6 +18,9 @@ sc-client-api = { version = "4.0.0-dev", git = "https://github.com/paritytech/su sp-api = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-blockchain = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-core = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-executor = { version = "0.1.0", path = "../sp-executor" } +sp-externalities = { version = "0.12.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-runtime = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-state-machine = { version = "0.12.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-trie = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +tracing = "0.1.23" diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 67f940ab3a7..bade7020e52 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -10,17 +10,21 @@ use codec::Codec; 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_core::{ - traits::{CodeExecutor, SpawnNamed}, - H256, + traits::{CodeExecutor, SpawnNamed}, + H256, }; +use sp_executor::{fraud_proof_ext::FraudProofExt, FraudProof, VerificationError}; +use sp_externalities::Extensions; use sp_runtime::{ - generic::BlockId, - traits::{BlakeTwo256, Block as BlockT, HashFor}, + generic::BlockId, + traits::{BlakeTwo256, Block as BlockT, HashFor}, }; use sp_state_machine::{TrieBackend, TrieBackendStorage}; 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 @@ -30,57 +34,58 @@ use std::sync::Arc; // another way. #[allow(clippy::too_many_arguments)] pub fn prove_execution< - Block: BlockT, - B: backend::Backend, - Exec: CodeExecutor + 'static, - Spawn: SpawnNamed + Send + 'static, - DB: HashDB, DBValue>, + Block: BlockT, + B: backend::Backend, + Exec: CodeExecutor + 'static, + Spawn: SpawnNamed + Send + 'static, + DB: HashDB, DBValue>, >( - backend: &Arc, - executor: &Exec, - spawn_handle: Spawn, - at: &BlockId, - method: &str, - call_data: &[u8], - delta_changes: Option<(DB, Block::Hash)>, + backend: &Arc, + executor: &Exec, + spawn_handle: Spawn, + at: &BlockId, + method: &str, + call_data: &[u8], + delta_changes: Option<(DB, Block::Hash)>, ) -> 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)?; - - 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, - method, - 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(), - executor, - spawn_handle, - method, - call_data, - &runtime_code, - ) - .map(|(_ret, proof)| proof) - .map_err(Into::into) - } + 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)?; + + 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, + method, + 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(), + executor, + spawn_handle, + method, + 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 @@ -91,42 +96,43 @@ pub fn prove_execution< // TODO: too many arguments. #[allow(clippy::too_many_arguments)] pub fn check_execution_proof< - Block: BlockT, - B: backend::Backend, - Exec: CodeExecutor + 'static, - Spawn: SpawnNamed + Send + 'static, + Block: BlockT, + B: backend::Backend, + Exec: CodeExecutor + 'static, + Spawn: SpawnNamed + Send + 'static, >( - backend: &Arc, - executor: &Exec, - spawn_handle: Spawn, - at: &BlockId, - method: &str, - call_data: &[u8], - pre_execution_root: H256, - proof: StorageProof, + backend: &Arc, + executor: &Exec, + spawn_handle: Spawn, + at: &BlockId, + method: &str, + call_data: &[u8], + 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, - method, - call_data, - &runtime_code, - ) - .map_err(Into::into) + 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, + method, + call_data, + &runtime_code, + ) + .map_err(Into::into) } /// Create a new trie backend with memory DB delta changes. @@ -134,37 +140,111 @@ pub fn check_execution_proof< /// This can be used to verify any extrinsic-specific execution on the combined state of `backend` /// and `delta`. fn create_delta_backend<'a, S: 'a + TrieBackendStorage, H: 'a + Hasher, DB: HashDB>( - backend: &'a TrieBackend, - delta: DB, - post_delta_root: H::Out, + backend: &'a TrieBackend, + delta: DB, + post_delta_root: H::Out, ) -> TrieBackend, H> where - H::Out: Codec, + H::Out: Codec, { - let essence = backend.essence(); - let delta_backend = DeltaBackend { - backend: essence.backend_storage(), - delta, - _phantom: std::marker::PhantomData::, - }; - TrieBackend::new(delta_backend, post_delta_root) + let essence = backend.essence(); + let delta_backend = DeltaBackend { + backend: essence.backend_storage(), + delta, + _phantom: std::marker::PhantomData::, + }; + TrieBackend::new(delta_backend, post_delta_root) } struct DeltaBackend<'a, S: 'a + TrieBackendStorage, H: 'a + Hasher, DB: HashDB> { - backend: &'a S, - delta: DB, - _phantom: std::marker::PhantomData, + backend: &'a S, + delta: DB, + _phantom: std::marker::PhantomData, } impl<'a, S: 'a + TrieBackendStorage, H: 'a + Hasher, DB: HashDB> - TrieBackendStorage for DeltaBackend<'a, S, H, DB> + TrieBackendStorage for DeltaBackend<'a, S, H, DB> +{ + type Overlay = S::Overlay; + + fn get(&self, key: &H::Out, prefix: Prefix) -> Result, String> { + match HashDB::get(&self.delta, key, prefix) { + Some(v) => Ok(Some(v)), + None => Ok(self.backend.get(key, prefix)?), + } + } +} + +/// Fraud proof verifier. +pub struct ProofVerifier { + backend: Arc, + executor: Exec, + spawn_handle: Spawn, + _phantom: PhantomData, +} + +impl Clone for ProofVerifier { + fn clone(&self) -> Self { + Self { + backend: self.backend.clone(), + executor: self.executor.clone(), + spawn_handle: self.spawn_handle.clone(), + _phantom: self._phantom, + } + } +} + +impl< + Block: BlockT, + B: backend::Backend, + Exec: CodeExecutor + Clone + 'static, + Spawn: SpawnNamed + Clone + Send + 'static, + > ProofVerifier +{ + /// Constructs a new instance of [`ProofVerifier`]. + pub fn new(backend: Arc, executor: Exec, spawn_handle: Spawn) -> Self { + Self { + backend, + executor, + spawn_handle, + _phantom: PhantomData::, + } + } + + fn verify(&self, proof: &FraudProof) -> Result<(), VerificationError> { + // TODO: impl + Ok(()) + } +} + +impl< + Block: BlockT, + B: backend::Backend, + Exec: CodeExecutor + Clone + 'static, + Spawn: SpawnNamed + Clone + Send + 'static, + > sp_executor::fraud_proof_ext::Externalities for ProofVerifier +{ + fn verify_fraud_proof(&self, proof: &sp_executor::FraudProof) -> bool { + match self.verify(proof) { + Ok(()) => true, + Err(e) => { + tracing::debug!(target: "fraud_proof", error = ?e, "Fraud proof verification failure"); + false + } + } + } +} + +impl< + Block: BlockT, + B: backend::Backend + 'static, + Exec: CodeExecutor + Clone + Send + Sync, + Spawn: SpawnNamed + Clone + Send + Sync + 'static, + > ExtensionsFactory for ProofVerifier { - type Overlay = S::Overlay; - - fn get(&self, key: &H::Out, prefix: Prefix) -> Result, String> { - match HashDB::get(&self.delta, key, prefix) { - Some(v) => Ok(Some(v)), - None => Ok(self.backend.get(key, prefix)?), - } - } + fn extensions_for(&self, _: sp_core::offchain::Capabilities) -> Extensions { + let mut exts = Extensions::new(); + exts.register(FraudProofExt::new(self.clone())); + exts + } } diff --git a/crates/subspace-node/Cargo.toml b/crates/subspace-node/Cargo.toml index 3d9f6d3f090..e7dec00d349 100644 --- a/crates/subspace-node/Cargo.toml +++ b/crates/subspace-node/Cargo.toml @@ -34,6 +34,7 @@ sc-telemetry = { version = "4.0.0-dev", git = "https://github.com/paritytech/sub serde = "1.0.136" sp-consensus = { version = "0.10.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-core = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-executor = { version = "0.1.0", path = "../sp-executor" } sp-runtime = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } subspace-archiving = { version = "0.1.0", path = "../subspace-archiving" } subspace-core-primitives = { version = "0.1.0", path = "../subspace-core-primitives" } diff --git a/crates/subspace-node/src/lib.rs b/crates/subspace-node/src/lib.rs index 95bab260243..08768a509ab 100644 --- a/crates/subspace-node/src/lib.rs +++ b/crates/subspace-node/src/lib.rs @@ -31,10 +31,13 @@ pub struct ExecutorDispatch; impl NativeExecutionDispatch for ExecutorDispatch { /// Only enable the benchmarking host functions when we actually want to benchmark. #[cfg(feature = "runtime-benchmarks")] - type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions; + type ExtendHostFunctions = ( + sp_executor::fraud_proof_ext::fraud_proof::HostFunctions, + frame_benchmarking::benchmarking::HostFunctions, + ); /// Otherwise we only use the default Substrate host functions. #[cfg(not(feature = "runtime-benchmarks"))] - type ExtendHostFunctions = (); + type ExtendHostFunctions = sp_executor::fraud_proof_ext::fraud_proof::HostFunctions; fn dispatch(method: &str, data: &[u8]) -> Option> { subspace_runtime::api::dispatch(method, data) diff --git a/crates/subspace-service/Cargo.toml b/crates/subspace-service/Cargo.toml index 4a65e8c91ea..6d145907af8 100644 --- a/crates/subspace-service/Cargo.toml +++ b/crates/subspace-service/Cargo.toml @@ -55,6 +55,7 @@ sp-runtime = { version = "6.0.0", git = "https://github.com/paritytech/substrate sp-timestamp = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-transaction-pool = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-trie = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +subspace-fraud-proof = { version = "0.1.0", path = "../subspace-fraud-proof" } subspace-runtime-primitives = { version = "0.1.0", path = "../subspace-runtime-primitives" } substrate-frame-rpc-system = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } substrate-prometheus-endpoint = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } diff --git a/crates/subspace-service/src/lib.rs b/crates/subspace-service/src/lib.rs index 0099757d23b..f53e9a35c79 100644 --- a/crates/subspace-service/src/lib.rs +++ b/crates/subspace-service/src/lib.rs @@ -175,8 +175,18 @@ where sc_service::new_full_parts::( config, telemetry.as_ref().map(|(_, telemetry)| telemetry.handle()), - executor, + executor.clone(), )?; + + let proof_verifier = subspace_fraud_proof::ProofVerifier::new( + backend.clone(), + executor, + task_manager.spawn_handle(), + ); + client + .execution_extensions() + .set_extensions_factory(Box::new(proof_verifier)); + let client = Arc::new(client); let telemetry = telemetry.map(|(worker, telemetry)| { diff --git a/test/subspace-test-client/Cargo.toml b/test/subspace-test-client/Cargo.toml index 84e71fe7a65..fda9e7a83e2 100644 --- a/test/subspace-test-client/Cargo.toml +++ b/test/subspace-test-client/Cargo.toml @@ -26,6 +26,7 @@ sc-service = { git = "https://github.com/paritytech/substrate", rev = "c364008a6 sp-api = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } sp-consensus-subspace = { version = "0.1.0", path = "../../crates/sp-consensus-subspace" } sp-core = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } +sp-executor = { version = "0.1.0", path = "../../crates/sp-executor" } sp-runtime = { git = "https://github.com/paritytech/substrate", rev = "c364008a6c7da8456e17967f55edf51e45146998" } subspace-archiving = { path = "../../crates/subspace-archiving" } subspace-core-primitives = { path = "../../crates/subspace-core-primitives" } diff --git a/test/subspace-test-client/src/lib.rs b/test/subspace-test-client/src/lib.rs index f75ba3fc3d2..890d68ce6b7 100644 --- a/test/subspace-test-client/src/lib.rs +++ b/test/subspace-test-client/src/lib.rs @@ -43,7 +43,7 @@ pub struct TestExecutorDispatch; impl sc_executor::NativeExecutionDispatch for TestExecutorDispatch { /// Otherwise we only use the default Substrate host functions. - type ExtendHostFunctions = (); + type ExtendHostFunctions = sp_executor::fraud_proof_ext::fraud_proof::HostFunctions; fn dispatch(method: &str, data: &[u8]) -> Option> { subspace_test_runtime::api::dispatch(method, data) From b72e200e066c0bd115315ea41005aab46536a2a9 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 28 Mar 2022 00:02:00 +0800 Subject: [PATCH 4/9] Introduce ExecutionArguments and add it to FraudProof Primary node needs the method and call data while verifying the fraud proof which are wrapped in ExecutionArguments. --- crates/sp-executor/src/lib.rs | 73 +++++++++++++++- cumulus/client/cirrus-executor/src/lib.rs | 32 +++---- cumulus/client/cirrus-executor/src/tests.rs | 93 ++++++++++++--------- 3 files changed, 142 insertions(+), 56 deletions(-) diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index d96a3d4cd81..1b4570aa922 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -21,7 +21,7 @@ use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; use sp_consensus_slots::Slot; use sp_core::H256; -use sp_runtime::traits::{BlakeTwo256, Hash as HashT}; +use sp_runtime::traits::{BlakeTwo256, Hash as HashT, Header as HeaderT}; use sp_runtime::{OpaqueExtrinsic, RuntimeDebug}; use sp_runtime_interface::pass_by::PassBy; use sp_std::borrow::Cow; @@ -126,6 +126,75 @@ impl From> for OpaqueExecutionReceipt { } } +/// Execution phase along with an optional encoded call data. +/// +/// Each execution phase has a different method for the runtime call. +#[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone, RuntimeDebug)] +pub enum ExecutionArguments { + /// Executes the `initialize_block` hook. + InitializeBlock(Vec), + /// Executes some extrinsic. + /// TODO: maybe optimized to not include the whole extrinsic blob in the future. + ApplyExtrinsic(Vec), + /// Executes the `finalize_block` hook. + FinalizeBlock, +} + +impl ExecutionArguments { + /// Returns the method for generating the proof. + pub fn proving_method(&self) -> &'static str { + match self { + // TODO: Replace `SecondaryApi_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(_) => "SecondaryApi_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(_) => "SecondaryApi_initialize_block_with_post_state_root", + Self::ApplyExtrinsic(_) => "SecondaryApi_apply_extrinsic_with_post_state_root", + Self::FinalizeBlock => "BlockBuilder_finalize_block", + } + } + + /// Returns the call data used to generate and verify the proof. + pub fn call_data(&self) -> &[u8] { + match self { + Self::InitializeBlock(call_data) | Self::ApplyExtrinsic(call_data) => call_data, + Self::FinalizeBlock => Default::default(), + } + } + + /// Returns the post state root for the given execution result. + pub fn decode_execution_result( + &self, + execution_result: Vec, + ) -> Header::Hash { + match self { + ExecutionArguments::InitializeBlock(_) | ExecutionArguments::ApplyExtrinsic(_) => { + let encoded_storage_root = Vec::::decode(&mut execution_result.as_slice()) + .expect("The return value of verifying `initialize_block` and `apply_extrinsic` must be an encoded storage root; qed"); + Header::Hash::decode(&mut encoded_storage_root.as_slice()) + .expect("storage root must use the same Header Hash type; qed") + } + ExecutionArguments::FinalizeBlock => { + let new_header = Header::decode(&mut execution_result.as_slice()).expect( + "The return value of `BlockBuilder_finalize_block` must be a Header; qed", + ); + *new_header.state_root() + } + } + } +} + /// Error type of fraud proof verification on primary node. #[derive(RuntimeDebug)] pub struct VerificationError; @@ -139,6 +208,8 @@ pub struct FraudProof { pub post_state_root: H256, /// Proof recorded during the computation. pub proof: StorageProof, + /// Execution arguments. + pub execution_args: ExecutionArguments, } impl PassBy for FraudProof { diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index d5f865162a1..25195a8ee13 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -53,8 +53,8 @@ use cirrus_node_primitives::{ }; use cirrus_primitives::{AccountId, Hash, SecondaryApi}; use sp_executor::{ - Bundle, BundleEquivocationProof, ExecutionReceipt, FraudProof, InvalidTransactionProof, - OpaqueBundle, + Bundle, BundleEquivocationProof, ExecutionArguments, ExecutionReceipt, FraudProof, + InvalidTransactionProof, OpaqueBundle, }; use subspace_core_primitives::Randomness; use subspace_runtime_primitives::Hash as PHash; @@ -314,7 +314,7 @@ where extrinsic_index: usize, parent_header: &Block::Header, current_hash: Block::Hash, - ) -> Result { + ) -> Result<(StorageProof, ExecutionArguments), GossipMessageError> { let extrinsics = self.block_body(current_hash)?; let encoded_extrinsic = extrinsics @@ -325,6 +325,8 @@ where })? .encode(); + let execution_args = ExecutionArguments::ApplyExtrinsic(encoded_extrinsic); + let block_builder = BlockBuilder::with_extrinsics( &*self.client, parent_header.hash(), @@ -345,12 +347,12 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - "BlockBuilder_apply_extrinsic", - &encoded_extrinsic, + execution_args.proving_method(), + execution_args.call_data(), Some((delta, post_delta_root)), )?; - Ok(execution_proof) + Ok((execution_proof, execution_args)) } async fn wait_for_local_receipt( @@ -606,6 +608,7 @@ where parent_header.hash(), Default::default(), ); + let execution_args = ExecutionArguments::InitializeBlock(new_header.encode()); // TODO: way to call some runtime api against any specific state instead of having // to work with String API directly. @@ -620,16 +623,17 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - "SecondaryApi_initialize_block_with_post_state_root", // TODO: "Core_initalize_block" - &new_header.encode(), + execution_args.proving_method(), + execution_args.call_data(), None, )?; - FraudProof { pre_state_root, post_state_root, proof } + FraudProof { pre_state_root, post_state_root, proof, execution_args } } else if local_trace_idx == local_receipt.trace.len() - 1 { // `finalize_block` execution proof. let pre_state_root = as_h256(&execution_receipt.trace[local_trace_idx - 1])?; let post_state_root = as_h256(local_root)?; + let execution_args = ExecutionArguments::FinalizeBlock; let block_builder = BlockBuilder::with_extrinsics( &*self.client, @@ -653,25 +657,25 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - "BlockBuilder_finalize_block", - Default::default(), + execution_args.proving_method(), + execution_args.call_data(), Some((delta, post_delta_root)), )?; - FraudProof { pre_state_root, post_state_root, proof } + FraudProof { pre_state_root, post_state_root, proof, execution_args } } else { // Regular extrinsic execution proof. let pre_state_root = as_h256(&execution_receipt.trace[local_trace_idx - 1])?; let post_state_root = as_h256(local_root)?; - let proof = self.create_extrinsic_execution_proof( + let (proof, execution_args) = self.create_extrinsic_execution_proof( local_trace_idx - 1, &parent_header, execution_receipt.secondary_hash, )?; // TODO: proof should be a CompactProof. - FraudProof { pre_state_root, post_state_root, proof } + FraudProof { pre_state_root, post_state_root, proof, execution_args } }; self.submit_fraud_proof(fraud_proof); diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index 79a5bf6a1a7..72f52621d93 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -5,9 +5,10 @@ use cirrus_test_service::{ runtime::Header, Keyring::{Alice, Charlie, Dave}, }; -use codec::{Decode, Encode}; +use codec::Encode; use sc_client_api::{HeaderBackend, StorageProof}; use sp_api::ProvideRuntimeApi; +use sp_executor::ExecutionArguments; use sp_runtime::{ generic::BlockId, traits::{BlakeTwo256, Header as HeaderT}, @@ -158,42 +159,46 @@ async fn execution_proof_creation_and_verification_should_work() { return } - // Test `initialize_block`. - let storage_proof = { - let new_header = Header::new( - *header.number(), - Default::default(), - Default::default(), - parent_header.hash(), - Default::default(), - ); + let new_header = Header::new( + *header.number(), + Default::default(), + Default::default(), + parent_header.hash(), + Default::default(), + ); + let execution_args = ExecutionArguments::InitializeBlock(new_header.encode()); - subspace_fraud_proof::prove_execution::<_, _, _, _, sp_trie::PrefixedMemoryDB>( - &charlie.backend, - &*charlie.code_executor, - charlie.task_manager.spawn_handle(), - &BlockId::Hash(parent_header.hash()), - "SecondaryApi_initialize_block_with_post_state_root", // TODO: "Core_initalize_block" - &new_header.encode(), - None, - ) - .expect("Create `initialize_block` proof") - }; + // 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_args.proving_method(), + execution_args.call_data(), + None, + ) + .expect("Create `initialize_block` proof"); let execution_result = subspace_fraud_proof::check_execution_proof( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - "SecondaryApi_initialize_block_with_post_state_root", - &header.encode(), + execution_args.verifying_method(), + execution_args.call_data(), *parent_header.state_root(), storage_proof, ) .expect("Check `initialize_block` proof"); - let res = Vec::::decode(&mut execution_result.as_slice()).unwrap(); - let post_execution_root = Hash::decode(&mut res.as_slice()).unwrap(); + let post_execution_root = execution_args.decode_execution_result::
(execution_result); assert_eq!(post_execution_root, intermediate_roots[0].into()); // Test extrinsic execution. @@ -207,13 +212,15 @@ async fn execution_proof_creation_and_verification_should_work() { let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; + let execution_args = ExecutionArguments::ApplyExtrinsic(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()), - "BlockBuilder_apply_extrinsic", - &xt.encode(), + execution_args.proving_method(), + execution_args.call_data(), Some((delta, post_delta_root)), ) .expect("Create extrinsic execution proof"); @@ -226,15 +233,15 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - "SecondaryApi_apply_extrinsic_with_post_state_root", - &xt.encode(), + execution_args.verifying_method(), + execution_args.call_data(), post_delta_root, storage_proof, ) .expect("Check extrinsic execution proof"); - let res = Vec::::decode(&mut execution_result.as_slice()).unwrap(); - let post_execution_root = Hash::decode(&mut res.as_slice()).unwrap(); + let post_execution_root = + execution_args.decode_execution_result::
(execution_result); assert_eq!(post_execution_root, intermediate_roots[target_extrinsic_index + 1].into()); } @@ -248,13 +255,15 @@ async fn execution_proof_creation_and_verification_should_work() { assert_eq!(post_delta_root, intermediate_roots.last().unwrap().into()); + let execution_args = ExecutionArguments::FinalizeBlock; + let storage_proof = subspace_fraud_proof::prove_execution( &charlie.backend, &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - "BlockBuilder_finalize_block", - Default::default(), + execution_args.proving_method(), + execution_args.call_data(), Some((delta, post_delta_root)), ) .expect("Create `finalize_block` proof"); @@ -264,15 +273,14 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - "BlockBuilder_finalize_block", - Default::default(), + execution_args.verifying_method(), + execution_args.call_data(), post_delta_root, storage_proof, ) .expect("Check `finalize_block` proof"); - let new_header = Header::decode(&mut execution_result.as_slice()).unwrap(); - let post_execution_root = *new_header.state_root(); + let post_execution_root = execution_args.decode_execution_result::
(execution_result); assert_eq!(post_execution_root, *header.state_root()); } @@ -360,13 +368,15 @@ async fn invalid_execution_proof_should_not_work() { let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; + let execution_args = ExecutionArguments::ApplyExtrinsic(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()), - "BlockBuilder_apply_extrinsic", - &test_txs[extrinsic_index].encode(), + execution_args.proving_method(), + execution_args.call_data(), Some((delta.clone(), post_delta_root.clone())), ) .expect("Create extrinsic execution proof"); @@ -378,13 +388,14 @@ async fn invalid_execution_proof_should_not_work() { let (proof1, _delta1, post_delta_root1) = create_extrinsic_proof(1); let check_proof = |post_delta_root: Hash, proof: StorageProof| { + let execution_args = ExecutionArguments::ApplyExtrinsic(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()), - "SecondaryApi_apply_extrinsic_with_post_state_root", - &transfer_to_charlie_again.encode(), + execution_args.verifying_method(), + execution_args.call_data(), post_delta_root, proof, ) From beaa5f1f2793adb9bf49b92d57804eb5fa2fc075 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 28 Mar 2022 00:09:28 +0800 Subject: [PATCH 5/9] Reduce the number of arguments in prove_execution and check_execution_proof --- crates/subspace-fraud-proof/src/lib.rs | 28 ++++++++------------- cumulus/client/cirrus-executor/src/lib.rs | 9 +++---- cumulus/client/cirrus-executor/src/tests.rs | 24 ++++++------------ 3 files changed, 22 insertions(+), 39 deletions(-) diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index bade7020e52..08ad93a59df 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -16,7 +16,9 @@ use sp_core::{ traits::{CodeExecutor, SpawnNamed}, H256, }; -use sp_executor::{fraud_proof_ext::FraudProofExt, FraudProof, VerificationError}; +use sp_executor::{ + fraud_proof_ext::FraudProofExt, ExecutionArguments, FraudProof, VerificationError, +}; use sp_externalities::Extensions; use sp_runtime::{ generic::BlockId, @@ -29,10 +31,6 @@ 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. -// TODO: too many arguments, but no need to refactor it right now as the API of execution proof -// on the primary node might have some other considerations, e.g., RuntimeCode will be fetched -// another way. -#[allow(clippy::too_many_arguments)] pub fn prove_execution< Block: BlockT, B: backend::Backend, @@ -44,8 +42,7 @@ pub fn prove_execution< executor: &Exec, spawn_handle: Spawn, at: &BlockId, - method: &str, - call_data: &[u8], + execution_args: &ExecutionArguments, delta_changes: Option<(DB, Block::Hash)>, ) -> sp_blockchain::Result { let state = backend.state_at(*at)?; @@ -67,8 +64,8 @@ pub fn prove_execution< &mut Default::default(), executor, spawn_handle, - method, - call_data, + execution_args.proving_method(), + execution_args.call_data(), &runtime_code, ) .map(|(_ret, proof)| proof) @@ -79,8 +76,8 @@ pub fn prove_execution< &mut Default::default(), executor, spawn_handle, - method, - call_data, + execution_args.proving_method(), + execution_args.call_data(), &runtime_code, ) .map(|(_ret, proof)| proof) @@ -93,8 +90,6 @@ pub fn prove_execution< /// /// 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. -// TODO: too many arguments. -#[allow(clippy::too_many_arguments)] pub fn check_execution_proof< Block: BlockT, B: backend::Backend, @@ -105,8 +100,7 @@ pub fn check_execution_proof< executor: &Exec, spawn_handle: Spawn, at: &BlockId, - method: &str, - call_data: &[u8], + execution_args: &ExecutionArguments, pre_execution_root: H256, proof: StorageProof, ) -> sp_blockchain::Result> { @@ -128,8 +122,8 @@ pub fn check_execution_proof< &mut Default::default(), executor, spawn_handle, - method, - call_data, + execution_args.verifying_method(), + execution_args.call_data(), &runtime_code, ) .map_err(Into::into) diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index 25195a8ee13..ca37e6c817f 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -347,8 +347,7 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, Some((delta, post_delta_root)), )?; @@ -623,8 +622,7 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, None, )?; @@ -657,8 +655,7 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, Some((delta, post_delta_root)), )?; diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index 72f52621d93..58fcee24390 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -180,8 +180,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, None, ) .expect("Create `initialize_block` proof"); @@ -191,8 +190,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.verifying_method(), - execution_args.call_data(), + &execution_args, *parent_header.state_root(), storage_proof, ) @@ -219,8 +217,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, Some((delta, post_delta_root)), ) .expect("Create extrinsic execution proof"); @@ -233,8 +230,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.verifying_method(), - execution_args.call_data(), + &execution_args, post_delta_root, storage_proof, ) @@ -262,8 +258,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, Some((delta, post_delta_root)), ) .expect("Create `finalize_block` proof"); @@ -273,8 +268,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.verifying_method(), - execution_args.call_data(), + &execution_args, post_delta_root, storage_proof, ) @@ -375,8 +369,7 @@ async fn invalid_execution_proof_should_not_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.proving_method(), - execution_args.call_data(), + &execution_args, Some((delta.clone(), post_delta_root.clone())), ) .expect("Create extrinsic execution proof"); @@ -394,8 +387,7 @@ async fn invalid_execution_proof_should_not_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - execution_args.verifying_method(), - execution_args.call_data(), + &execution_args, post_delta_root, proof, ) From e5ebb3d4a03cb62a300c91ef4c0780f1064b4641 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 28 Mar 2022 00:27:31 +0800 Subject: [PATCH 6/9] Implement fraud proof verify on primary node --- crates/sp-executor/src/lib.rs | 11 +++++- crates/subspace-fraud-proof/src/lib.rs | 53 ++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index 1b4570aa922..53a764a1913 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -197,7 +197,16 @@ impl ExecutionArguments { /// Error type of fraud proof verification on primary node. #[derive(RuntimeDebug)] -pub struct VerificationError; +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`]. + BadPostStateRoot { expected: H256, got: H256 }, +} /// Fraud proof for the state computation. #[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone, RuntimeDebug)] diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 08ad93a59df..9ee5271f569 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -7,7 +7,7 @@ #![warn(missing_docs)] -use codec::Codec; +use codec::{Codec, Decode, Encode}; use hash_db::{HashDB, Hasher, Prefix}; use sc_client_api::backend; use sc_client_api::execution_extensions::ExtensionsFactory; @@ -206,8 +206,55 @@ impl< } fn verify(&self, proof: &FraudProof) -> Result<(), VerificationError> { - // TODO: impl - Ok(()) + let FraudProof { + pre_state_root, + post_state_root, + proof, + execution_args, + } = proof; + + // TODO: we should use parent_hash. + let at = BlockId::Hash(Block::Hash::default()); + + 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 execution_result = sp_state_machine::execution_proof_check::( + *pre_state_root, + proof.clone(), + &mut Default::default(), + &self.executor, + self.spawn_handle.clone(), + execution_args.verifying_method(), + execution_args.call_data(), + &runtime_code, + ) + .map_err(VerificationError::BadProof)?; + + let new_post_state_root = + execution_args.decode_execution_result::(execution_result); + let new_post_state_root = H256::decode(&mut new_post_state_root.encode().as_slice()) + .expect("Block Hash must be H256; qed"); + + if new_post_state_root == *post_state_root { + Ok(()) + } else { + Err(VerificationError::BadPostStateRoot { + expected: new_post_state_root, + got: *post_state_root, + }) + } } } From e770d1559d2c3e337bd4088bdcb66a2684e463c3 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 28 Mar 2022 00:51:35 +0800 Subject: [PATCH 7/9] Add parent_hash to FraudProof `parent_hash` will be used to fetch the runtime code for running the execution. --- crates/sp-executor/src/lib.rs | 4 ++++ crates/subspace-fraud-proof/src/lib.rs | 7 +++++-- cumulus/client/cirrus-executor/src/lib.rs | 24 ++++++++++++++++++++--- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index 53a764a1913..5abfb029e59 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -211,6 +211,10 @@ pub enum VerificationError { /// Fraud proof for the state computation. #[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone, RuntimeDebug)] pub struct FraudProof { + /// Parent hash of the block at which the invalid execution occurred. + /// + /// Runtime code for this block's execution is retrieved on top of the parent block. + pub parent_hash: H256, /// State root before the fraudulent transaction. pub pre_state_root: H256, /// State root after the fraudulent transaction. diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 9ee5271f569..baba6275064 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -207,14 +207,17 @@ impl< fn verify(&self, proof: &FraudProof) -> Result<(), VerificationError> { let FraudProof { + parent_hash, pre_state_root, post_state_root, proof, execution_args, } = proof; - // TODO: we should use parent_hash. - let at = BlockId::Hash(Block::Hash::default()); + let at = BlockId::Hash( + Block::Hash::decode(&mut parent_hash.encode().as_slice()) + .expect("Block Hash must be H256; qed"), + ); let state = self .backend diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index ca37e6c817f..9787998d3f8 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -626,7 +626,13 @@ where None, )?; - FraudProof { pre_state_root, post_state_root, proof, execution_args } + FraudProof { + parent_hash: as_h256(&parent_header.hash())?, + pre_state_root, + post_state_root, + proof, + execution_args, + } } else if local_trace_idx == local_receipt.trace.len() - 1 { // `finalize_block` execution proof. let pre_state_root = as_h256(&execution_receipt.trace[local_trace_idx - 1])?; @@ -659,7 +665,13 @@ where Some((delta, post_delta_root)), )?; - FraudProof { pre_state_root, post_state_root, proof, execution_args } + FraudProof { + parent_hash: as_h256(&parent_header.hash())?, + pre_state_root, + post_state_root, + proof, + execution_args, + } } else { // Regular extrinsic execution proof. let pre_state_root = as_h256(&execution_receipt.trace[local_trace_idx - 1])?; @@ -672,7 +684,13 @@ where )?; // TODO: proof should be a CompactProof. - FraudProof { pre_state_root, post_state_root, proof, execution_args } + FraudProof { + parent_hash: as_h256(&parent_header.hash())?, + pre_state_root, + post_state_root, + proof, + execution_args, + } }; self.submit_fraud_proof(fraud_proof); From b03b3be83837a05375a11fba58f55239d655d912 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Tue, 29 Mar 2022 11:56:54 +0800 Subject: [PATCH 8/9] Rename `ExecutionArguments` to `ExecutionPhase` --- crates/sp-executor/src/lib.rs | 26 +++++++-------- crates/subspace-fraud-proof/src/lib.rs | 28 ++++++++-------- cumulus/client/cirrus-executor/src/lib.rs | 27 ++++++++-------- cumulus/client/cirrus-executor/src/tests.rs | 36 +++++++++++---------- 4 files changed, 59 insertions(+), 58 deletions(-) diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index 5abfb029e59..2c1a59272f3 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -130,24 +130,24 @@ impl From> for OpaqueExecutionReceipt { /// /// Each execution phase has a different method for the runtime call. #[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone, RuntimeDebug)] -pub enum ExecutionArguments { +pub enum ExecutionPhase { /// Executes the `initialize_block` hook. - InitializeBlock(Vec), + InitializeBlock { call_data: Vec }, /// Executes some extrinsic. /// TODO: maybe optimized to not include the whole extrinsic blob in the future. - ApplyExtrinsic(Vec), + ApplyExtrinsic { call_data: Vec }, /// Executes the `finalize_block` hook. FinalizeBlock, } -impl ExecutionArguments { +impl ExecutionPhase { /// Returns the method for generating the proof. pub fn proving_method(&self) -> &'static str { match self { // TODO: Replace `SecondaryApi_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(_) => "SecondaryApi_initialize_block_with_post_state_root", - Self::ApplyExtrinsic(_) => "BlockBuilder_apply_extrinsic", + Self::InitializeBlock { .. } => "SecondaryApi_initialize_block_with_post_state_root", + Self::ApplyExtrinsic { .. } => "BlockBuilder_apply_extrinsic", Self::FinalizeBlock => "BlockBuilder_finalize_block", } } @@ -159,8 +159,8 @@ impl ExecutionArguments { /// result of execution reported in [`FraudProof`] is expected or not. pub fn verifying_method(&self) -> &'static str { match self { - Self::InitializeBlock(_) => "SecondaryApi_initialize_block_with_post_state_root", - Self::ApplyExtrinsic(_) => "SecondaryApi_apply_extrinsic_with_post_state_root", + Self::InitializeBlock { .. } => "SecondaryApi_initialize_block_with_post_state_root", + Self::ApplyExtrinsic { .. } => "SecondaryApi_apply_extrinsic_with_post_state_root", Self::FinalizeBlock => "BlockBuilder_finalize_block", } } @@ -168,7 +168,7 @@ impl ExecutionArguments { /// Returns the call data used to generate and verify the proof. pub fn call_data(&self) -> &[u8] { match self { - Self::InitializeBlock(call_data) | Self::ApplyExtrinsic(call_data) => call_data, + Self::InitializeBlock { call_data } | Self::ApplyExtrinsic { call_data } => call_data, Self::FinalizeBlock => Default::default(), } } @@ -179,13 +179,13 @@ impl ExecutionArguments { execution_result: Vec, ) -> Header::Hash { match self { - ExecutionArguments::InitializeBlock(_) | ExecutionArguments::ApplyExtrinsic(_) => { + ExecutionPhase::InitializeBlock { .. } | ExecutionPhase::ApplyExtrinsic { .. } => { let encoded_storage_root = Vec::::decode(&mut execution_result.as_slice()) .expect("The return value of verifying `initialize_block` and `apply_extrinsic` must be an encoded storage root; qed"); Header::Hash::decode(&mut encoded_storage_root.as_slice()) .expect("storage root must use the same Header Hash type; qed") } - ExecutionArguments::FinalizeBlock => { + ExecutionPhase::FinalizeBlock => { let new_header = Header::decode(&mut execution_result.as_slice()).expect( "The return value of `BlockBuilder_finalize_block` must be a Header; qed", ); @@ -221,8 +221,8 @@ pub struct FraudProof { pub post_state_root: H256, /// Proof recorded during the computation. pub proof: StorageProof, - /// Execution arguments. - pub execution_args: ExecutionArguments, + /// Execution phase. + pub execution_phase: ExecutionPhase, } impl PassBy for FraudProof { diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index baba6275064..023f46443f0 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -16,9 +16,7 @@ use sp_core::{ traits::{CodeExecutor, SpawnNamed}, H256, }; -use sp_executor::{ - fraud_proof_ext::FraudProofExt, ExecutionArguments, FraudProof, VerificationError, -}; +use sp_executor::{fraud_proof_ext::FraudProofExt, ExecutionPhase, FraudProof, VerificationError}; use sp_externalities::Extensions; use sp_runtime::{ generic::BlockId, @@ -42,7 +40,7 @@ pub fn prove_execution< executor: &Exec, spawn_handle: Spawn, at: &BlockId, - execution_args: &ExecutionArguments, + execution_phase: &ExecutionPhase, delta_changes: Option<(DB, Block::Hash)>, ) -> sp_blockchain::Result { let state = backend.state_at(*at)?; @@ -64,8 +62,8 @@ pub fn prove_execution< &mut Default::default(), executor, spawn_handle, - execution_args.proving_method(), - execution_args.call_data(), + execution_phase.proving_method(), + execution_phase.call_data(), &runtime_code, ) .map(|(_ret, proof)| proof) @@ -76,8 +74,8 @@ pub fn prove_execution< &mut Default::default(), executor, spawn_handle, - execution_args.proving_method(), - execution_args.call_data(), + execution_phase.proving_method(), + execution_phase.call_data(), &runtime_code, ) .map(|(_ret, proof)| proof) @@ -100,7 +98,7 @@ pub fn check_execution_proof< executor: &Exec, spawn_handle: Spawn, at: &BlockId, - execution_args: &ExecutionArguments, + execution_phase: &ExecutionPhase, pre_execution_root: H256, proof: StorageProof, ) -> sp_blockchain::Result> { @@ -122,8 +120,8 @@ pub fn check_execution_proof< &mut Default::default(), executor, spawn_handle, - execution_args.verifying_method(), - execution_args.call_data(), + execution_phase.verifying_method(), + execution_phase.call_data(), &runtime_code, ) .map_err(Into::into) @@ -211,7 +209,7 @@ impl< pre_state_root, post_state_root, proof, - execution_args, + execution_phase, } = proof; let at = BlockId::Hash( @@ -239,14 +237,14 @@ impl< &mut Default::default(), &self.executor, self.spawn_handle.clone(), - execution_args.verifying_method(), - execution_args.call_data(), + execution_phase.verifying_method(), + execution_phase.call_data(), &runtime_code, ) .map_err(VerificationError::BadProof)?; let new_post_state_root = - execution_args.decode_execution_result::(execution_result); + execution_phase.decode_execution_result::(execution_result); let new_post_state_root = H256::decode(&mut new_post_state_root.encode().as_slice()) .expect("Block Hash must be H256; qed"); diff --git a/cumulus/client/cirrus-executor/src/lib.rs b/cumulus/client/cirrus-executor/src/lib.rs index 9787998d3f8..84551df62f1 100644 --- a/cumulus/client/cirrus-executor/src/lib.rs +++ b/cumulus/client/cirrus-executor/src/lib.rs @@ -53,7 +53,7 @@ use cirrus_node_primitives::{ }; use cirrus_primitives::{AccountId, Hash, SecondaryApi}; use sp_executor::{ - Bundle, BundleEquivocationProof, ExecutionArguments, ExecutionReceipt, FraudProof, + Bundle, BundleEquivocationProof, ExecutionPhase, ExecutionReceipt, FraudProof, InvalidTransactionProof, OpaqueBundle, }; use subspace_core_primitives::Randomness; @@ -314,7 +314,7 @@ where extrinsic_index: usize, parent_header: &Block::Header, current_hash: Block::Hash, - ) -> Result<(StorageProof, ExecutionArguments), GossipMessageError> { + ) -> Result<(StorageProof, ExecutionPhase), GossipMessageError> { let extrinsics = self.block_body(current_hash)?; let encoded_extrinsic = extrinsics @@ -325,7 +325,7 @@ where })? .encode(); - let execution_args = ExecutionArguments::ApplyExtrinsic(encoded_extrinsic); + let execution_phase = ExecutionPhase::ApplyExtrinsic { call_data: encoded_extrinsic }; let block_builder = BlockBuilder::with_extrinsics( &*self.client, @@ -347,11 +347,11 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, Some((delta, post_delta_root)), )?; - Ok((execution_proof, execution_args)) + Ok((execution_proof, execution_phase)) } async fn wait_for_local_receipt( @@ -607,7 +607,8 @@ where parent_header.hash(), Default::default(), ); - let execution_args = ExecutionArguments::InitializeBlock(new_header.encode()); + 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. @@ -622,7 +623,7 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, None, )?; @@ -631,13 +632,13 @@ where pre_state_root, post_state_root, proof, - execution_args, + execution_phase, } } else if local_trace_idx == local_receipt.trace.len() - 1 { // `finalize_block` execution proof. let pre_state_root = as_h256(&execution_receipt.trace[local_trace_idx - 1])?; let post_state_root = as_h256(local_root)?; - let execution_args = ExecutionArguments::FinalizeBlock; + let execution_phase = ExecutionPhase::FinalizeBlock; let block_builder = BlockBuilder::with_extrinsics( &*self.client, @@ -661,7 +662,7 @@ where &*self.code_executor, self.spawner.clone() as Box, &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, Some((delta, post_delta_root)), )?; @@ -670,14 +671,14 @@ where pre_state_root, post_state_root, proof, - execution_args, + execution_phase, } } else { // Regular extrinsic execution proof. let pre_state_root = as_h256(&execution_receipt.trace[local_trace_idx - 1])?; let post_state_root = as_h256(local_root)?; - let (proof, execution_args) = self.create_extrinsic_execution_proof( + let (proof, execution_phase) = self.create_extrinsic_execution_proof( local_trace_idx - 1, &parent_header, execution_receipt.secondary_hash, @@ -689,7 +690,7 @@ where pre_state_root, post_state_root, proof, - execution_args, + execution_phase, } }; diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index 58fcee24390..7ff74743f15 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::ExecutionArguments; +use sp_executor::ExecutionPhase; use sp_runtime::{ generic::BlockId, traits::{BlakeTwo256, Header as HeaderT}, @@ -166,7 +166,7 @@ async fn execution_proof_creation_and_verification_should_work() { parent_header.hash(), Default::default(), ); - let execution_args = ExecutionArguments::InitializeBlock(new_header.encode()); + let execution_phase = ExecutionPhase::InitializeBlock { call_data: new_header.encode() }; // Test `initialize_block`. let storage_proof = subspace_fraud_proof::prove_execution::< @@ -180,7 +180,7 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, None, ) .expect("Create `initialize_block` proof"); @@ -190,13 +190,13 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, *parent_header.state_root(), storage_proof, ) .expect("Check `initialize_block` proof"); - let post_execution_root = execution_args.decode_execution_result::
(execution_result); + let post_execution_root = execution_phase.decode_execution_result::
(execution_result); assert_eq!(post_execution_root, intermediate_roots[0].into()); // Test extrinsic execution. @@ -210,14 +210,14 @@ async fn execution_proof_creation_and_verification_should_work() { let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; - let execution_args = ExecutionArguments::ApplyExtrinsic(xt.encode()); + 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_args, + &execution_phase, Some((delta, post_delta_root)), ) .expect("Create extrinsic execution proof"); @@ -230,14 +230,14 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, post_delta_root, storage_proof, ) .expect("Check extrinsic execution proof"); let post_execution_root = - execution_args.decode_execution_result::
(execution_result); + execution_phase.decode_execution_result::
(execution_result); assert_eq!(post_execution_root, intermediate_roots[target_extrinsic_index + 1].into()); } @@ -251,14 +251,14 @@ async fn execution_proof_creation_and_verification_should_work() { assert_eq!(post_delta_root, intermediate_roots.last().unwrap().into()); - let execution_args = ExecutionArguments::FinalizeBlock; + 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_args, + &execution_phase, Some((delta, post_delta_root)), ) .expect("Create `finalize_block` proof"); @@ -268,13 +268,13 @@ async fn execution_proof_creation_and_verification_should_work() { &*charlie.code_executor, charlie.task_manager.spawn_handle(), &BlockId::Hash(parent_header.hash()), - &execution_args, + &execution_phase, post_delta_root, storage_proof, ) .expect("Check `finalize_block` proof"); - let post_execution_root = execution_args.decode_execution_result::
(execution_result); + let post_execution_root = execution_phase.decode_execution_result::
(execution_result); assert_eq!(post_execution_root, *header.state_root()); } @@ -362,14 +362,15 @@ async fn invalid_execution_proof_should_not_work() { let delta = storage_changes.transaction; let post_delta_root = storage_changes.transaction_storage_root; - let execution_args = ExecutionArguments::ApplyExtrinsic(test_txs[extrinsic_index].encode()); + 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_args, + &execution_phase, Some((delta.clone(), post_delta_root.clone())), ) .expect("Create extrinsic execution proof"); @@ -381,13 +382,14 @@ async fn invalid_execution_proof_should_not_work() { let (proof1, _delta1, post_delta_root1) = create_extrinsic_proof(1); let check_proof = |post_delta_root: Hash, proof: StorageProof| { - let execution_args = ExecutionArguments::ApplyExtrinsic(transfer_to_charlie_again.encode()); + 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()), - &execution_args, + &execution_phase, post_delta_root, proof, ) From a677dcb2663482561d0602179bdfc0a5ff070dd9 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Tue, 29 Mar 2022 12:23:42 +0800 Subject: [PATCH 9/9] Make `decode_execution_result` fallible Also added a TODO about fetching runtime code while verifying. --- crates/sp-executor/src/lib.rs | 19 ++++++++++++------- crates/subspace-fraud-proof/src/lib.rs | 3 ++- cumulus/client/cirrus-executor/src/tests.rs | 8 +++++--- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/sp-executor/src/lib.rs b/crates/sp-executor/src/lib.rs index 2c1a59272f3..d7bbf7f88e2 100644 --- a/crates/sp-executor/src/lib.rs +++ b/crates/sp-executor/src/lib.rs @@ -177,19 +177,18 @@ impl ExecutionPhase { pub fn decode_execution_result( &self, execution_result: Vec, - ) -> Header::Hash { + ) -> Result { match self { ExecutionPhase::InitializeBlock { .. } | ExecutionPhase::ApplyExtrinsic { .. } => { let encoded_storage_root = Vec::::decode(&mut execution_result.as_slice()) - .expect("The return value of verifying `initialize_block` and `apply_extrinsic` must be an encoded storage root; qed"); + .map_err(VerificationError::InitializeBlockOrApplyExtrinsicDecode)?; Header::Hash::decode(&mut encoded_storage_root.as_slice()) - .expect("storage root must use the same Header Hash type; qed") + .map_err(VerificationError::StorageRootDecode) } ExecutionPhase::FinalizeBlock => { - let new_header = Header::decode(&mut execution_result.as_slice()).expect( - "The return value of `BlockBuilder_finalize_block` must be a Header; qed", - ); - *new_header.state_root() + let new_header = Header::decode(&mut execution_result.as_slice()) + .map_err(VerificationError::HeaderDecode)?; + Ok(*new_header.state_root()) } } } @@ -206,6 +205,12 @@ pub enum VerificationError { BadProof(sp_std::boxed::Box), /// The `post_state_root` calculated by farmer does not match the one declared in [`FraudProof`]. BadPostStateRoot { expected: H256, got: H256 }, + /// Failed to decode the return value of `initialize_block` and `apply_extrinsic`. + InitializeBlockOrApplyExtrinsicDecode(parity_scale_codec::Error), + /// Failed to decode the storage root produced by verifying `initialize_block` or `apply_extrinsic`. + StorageRootDecode(parity_scale_codec::Error), + /// Failed to decode the header produced by `finalize_block`. + HeaderDecode(parity_scale_codec::Error), } /// Fraud proof for the state computation. diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 023f46443f0..3e99df3b490 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -217,6 +217,7 @@ impl< .expect("Block Hash must be H256; qed"), ); + // TODO: ensure the fetched runtime code works as expected. let state = self .backend .state_at(at) @@ -244,7 +245,7 @@ impl< .map_err(VerificationError::BadProof)?; let new_post_state_root = - execution_phase.decode_execution_result::(execution_result); + execution_phase.decode_execution_result::(execution_result)?; let new_post_state_root = H256::decode(&mut new_post_state_root.encode().as_slice()) .expect("Block Hash must be H256; qed"); diff --git a/cumulus/client/cirrus-executor/src/tests.rs b/cumulus/client/cirrus-executor/src/tests.rs index 7ff74743f15..9f9edf96075 100644 --- a/cumulus/client/cirrus-executor/src/tests.rs +++ b/cumulus/client/cirrus-executor/src/tests.rs @@ -196,7 +196,8 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Check `initialize_block` proof"); - let post_execution_root = execution_phase.decode_execution_result::
(execution_result); + let post_execution_root = + execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[0].into()); // Test extrinsic execution. @@ -237,7 +238,7 @@ async fn execution_proof_creation_and_verification_should_work() { .expect("Check extrinsic execution proof"); let post_execution_root = - execution_phase.decode_execution_result::
(execution_result); + execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, intermediate_roots[target_extrinsic_index + 1].into()); } @@ -274,7 +275,8 @@ async fn execution_proof_creation_and_verification_should_work() { ) .expect("Check `finalize_block` proof"); - let post_execution_root = execution_phase.decode_execution_result::
(execution_result); + let post_execution_root = + execution_phase.decode_execution_result::
(execution_result).unwrap(); assert_eq!(post_execution_root, *header.state_root()); }