From e59830c8d5a18380e07b78f9765b131d7e613551 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sun, 26 Mar 2023 22:08:18 +0800 Subject: [PATCH 1/7] Introduce `PreValidateTransaction` and merge generics `Verifier` and `BundleValidator` into `TxPreValidator` This commits introduces a trait `PreValidateTransaction` which is invoked before calling the normal `validate_transaction` function. `FraudProofVerifierAndBundleValidator` is separated out temperoarily, it's now used by both the primary chain and the system domain. However, the system domain does not need the bundle validator at all, which will be removed later, making it more explicit and easier to figure out the ingredients of the custom tx-pre-validator for primary_chain/system_domain/core_domain. --- Cargo.lock | 1 + crates/subspace-service/src/lib.rs | 48 ++-- crates/subspace-transaction-pool/Cargo.toml | 1 + crates/subspace-transaction-pool/src/lib.rs | 277 ++++++++++++-------- domains/service/src/system_domain.rs | 19 +- test/subspace-test-client/src/lib.rs | 12 +- test/subspace-test-service/src/lib.rs | 8 +- test/subspace-test-service/src/mock.rs | 22 +- 8 files changed, 250 insertions(+), 138 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7180dbfca71..21732eb370b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10496,6 +10496,7 @@ dependencies = [ name = "subspace-transaction-pool" version = "0.1.0" dependencies = [ + "async-trait", "domain-runtime-primitives", "futures 0.3.27", "jsonrpsee", diff --git a/crates/subspace-service/src/lib.rs b/crates/subspace-service/src/lib.rs index 3a1487b47ae..9151be51988 100644 --- a/crates/subspace-service/src/lib.rs +++ b/crates/subspace-service/src/lib.rs @@ -75,14 +75,15 @@ use std::num::NonZeroUsize; use std::sync::{Arc, Mutex}; use subspace_core_primitives::crypto::kzg::{embedded_kzg_settings, Kzg}; use subspace_core_primitives::PIECES_IN_SEGMENT; -use subspace_fraud_proof::VerifyFraudProof; use subspace_networking::libp2p::multiaddr::Protocol; use subspace_networking::libp2p::Multiaddr; use subspace_networking::{peer_id, Node}; use subspace_runtime_primitives::opaque::Block; use subspace_runtime_primitives::{AccountId, Balance, Hash, Index as Nonce}; -use subspace_transaction_pool::bundle_validator::{BundleValidator, ValidateBundle}; -use subspace_transaction_pool::FullPool; +use subspace_transaction_pool::bundle_validator::BundleValidator; +use subspace_transaction_pool::{ + FraudProofVerifierAndBundleValidator, FullPool, PreValidateTransaction, +}; use tracing::{debug, error, info, Instrument}; /// Error type for Subspace service. @@ -210,8 +211,12 @@ pub fn new_partial( FullPool< Block, FullClient, - FraudProofVerifier, - BundleValidator>, + FraudProofVerifierAndBundleValidator< + Block, + FullClient, + FraudProofVerifier, + BundleValidator>, + >, >, ( impl BlockImport< @@ -294,12 +299,17 @@ where task_manager.spawn_handle(), subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); + let tx_pre_validator = FraudProofVerifierAndBundleValidator::new( + client.clone(), + Box::new(task_manager.spawn_handle()), + proof_verifier.clone(), + bundle_validator.clone(), + ); let transaction_pool = subspace_transaction_pool::new_full( config, &task_manager, client.clone(), - proof_verifier.clone(), - bundle_validator.clone(), + tx_pre_validator, ); let fraud_proof_block_import = @@ -368,7 +378,7 @@ where } /// Full node along with some other components. -pub struct NewFull +pub struct NewFull where Client: ProvideRuntimeApi + BlockBackend @@ -379,9 +389,7 @@ where Client::Api: TaggedTransactionQueue + ExecutorApi + PreValidationObjectApi, - Validator: - ValidateBundle + Clone + Send + Sync + 'static, - Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, + TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { /// Task manager. pub task_manager: TaskManager, @@ -408,13 +416,17 @@ where /// Network starter. pub network_starter: NetworkStarter, /// Transaction pool. - pub transaction_pool: Arc>, + pub transaction_pool: Arc>, } type FullNode = NewFull< FullClient, - BundleValidator>, - FraudProofVerifier, + FraudProofVerifierAndBundleValidator< + Block, + FullClient, + FraudProofVerifier, + BundleValidator>, + >, >; /// Builds a new service for a full client. @@ -429,8 +441,12 @@ pub async fn new_full( FullPool< Block, FullClient, - FraudProofVerifier, - BundleValidator>, + FraudProofVerifierAndBundleValidator< + Block, + FullClient, + FraudProofVerifier, + BundleValidator>, + >, >, ( I, diff --git a/crates/subspace-transaction-pool/Cargo.toml b/crates/subspace-transaction-pool/Cargo.toml index 7052d69e470..cde406e9089 100644 --- a/crates/subspace-transaction-pool/Cargo.toml +++ b/crates/subspace-transaction-pool/Cargo.toml @@ -9,6 +9,7 @@ repository = "https://github.com/subspace/subspace" description = "Subspace transaction pool" [dependencies] +async-trait = "0.1.64" codec = { package = "parity-scale-codec", version = "3.4.0", default-features = false, features = ["derive"] } domain-runtime-primitives = { version = "0.1.0", path = "../../domains/primitives/runtime" } futures = "0.3.26" diff --git a/crates/subspace-transaction-pool/src/lib.rs b/crates/subspace-transaction-pool/src/lib.rs index b95855436df..8c62195a290 100644 --- a/crates/subspace-transaction-pool/src/lib.rs +++ b/crates/subspace-transaction-pool/src/lib.rs @@ -46,9 +46,9 @@ type ExtrinsicHash = <::Block as BlockT>::Hash; type ExtrinsicFor = <::Block as BlockT>::Extrinsic; /// A transaction pool for a full node. -pub type FullPool = BasicPoolWrapper< +pub type FullPool = BasicPoolWrapper< Block, - FullChainApiWrapper>, + FullChainApiWrapper>, >; /// A transaction pool with chain verifier. @@ -125,50 +125,57 @@ pub trait VerifyExtrinsic { ) -> ValidationFuture; } +/// This trait allows to perform some extra validation on the extrinsic before +/// doing the regular `validate_transaction` in the substrate transaction pool. +#[async_trait::async_trait] +pub trait PreValidateTransaction { + type Block: BlockT; + /// Returns `Ok(())` if the extrinsic passes the pre-validation. + async fn pre_validate_transaction( + &self, + at: ::Hash, + source: TransactionSource, + uxt: ::Extrinsic, + ) -> TxPoolResult<()>; +} + /// Verifier to verify the Fraud proofs and receipts. -pub struct FullChainVerifier { +pub struct FullChainVerifier { _phantom_data: PhantomData, client: Arc, - verifier: FraudProofVerifier, - bundle_validator: BundleValidator, + tx_pre_validator: TxPreValidator, } -impl Clone - for FullChainVerifier +impl Clone for FullChainVerifier where Block: Clone, - Verifier: Clone, - BundleValidator: Clone, + TxPreValidator: Clone, { fn clone(&self) -> Self { Self { _phantom_data: Default::default(), client: self.client.clone(), - verifier: self.verifier.clone(), - bundle_validator: self.bundle_validator.clone(), + tx_pre_validator: self.tx_pre_validator.clone(), } } } -impl - FullChainVerifier +impl FullChainVerifier where Block: BlockT, Client: HeaderBackend, { - pub fn new(client: Arc, verifier: Verifier, bundle_validator: BundleValidator) -> Self { + pub fn new(client: Arc, tx_pre_validator: TxPreValidator) -> Self { Self { _phantom_data: Default::default(), - verifier, client, - bundle_validator, + tx_pre_validator, } } } -impl - VerifyExtrinsic> - for FullChainVerifier +impl VerifyExtrinsic> + for FullChainVerifier where Block: BlockT, Client: ProvideRuntimeApi @@ -181,92 +188,27 @@ where + 'static, Client::Api: TaggedTransactionQueue + PreValidationObjectApi, - BundleValidator: - ValidateBundle + Clone + Send + Sync + 'static, - Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, + TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { fn verify_extrinsic( &self, at: Block::Hash, source: TransactionSource, uxt: BlockExtrinsicOf, - spawner: Box, + _spawner: Box, chain_api: Arc>, ) -> ValidationFuture { - match self - .client - .runtime_api() - .extract_pre_validation_object(at, uxt.clone()) - { - Ok(pre_validation_object) => { - match pre_validation_object { - PreValidationObject::Null => { - // No pre-validation is required. - } - PreValidationObject::Bundle(bundle) => { - if let Err(err) = self - .bundle_validator - .validate_bundle(&BlockId::Hash(at), &bundle) - { - tracing::trace!(target: "txpool", error = ?err, "Dropped `submit_bundle` extrinsic"); - return async move { Err(TxPoolError::ImmediatelyDropped.into()) } - .boxed(); - } - } - PreValidationObject::FraudProof(fraud_proof) => { - let inner = chain_api.clone(); - let spawner = spawner.clone(); - let fraud_proof_verifier = self.verifier.clone(); - - return async move { - let (verified_result_sender, verified_result_receiver) = oneshot::channel(); - - // Verify the fraud proof in another blocking task as it might be pretty heavy. - spawner.spawn_blocking( - "txpool-fraud-proof-verification", - None, - async move { - let verified_result = - fraud_proof_verifier.verify_fraud_proof(&fraud_proof); - verified_result_sender - .send(verified_result) - .expect("Failed to send the verified fraud proof result"); - } - .boxed(), - ); - - match verified_result_receiver.await { - Ok(verified_result) => { - match verified_result { - Ok(_) => inner.validate_transaction(&BlockId::Hash(at), source, uxt).await, - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); - Err(TxPoolError::InvalidTransaction( - InvalidTransactionCode::FraudProof.into(), - ) - .into()) - } - } - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); - Err(TxPoolError::UnknownTransaction(UnknownTransaction::CannotLookup).into()) - } - } - } - .boxed(); - } - } - } - Err(err) => { - return async move { - Err(sc_transaction_pool::error::Error::Blockchain(err.into())) - } - .boxed(); - } + let tx_pre_validator = self.tx_pre_validator.clone(); + async move { + tx_pre_validator + .pre_validate_transaction(at, source, uxt.clone()) + .await?; + + chain_api + .validate_transaction(&BlockId::Hash(at), source, uxt) + .await } - - chain_api.validate_transaction(&BlockId::Hash(at), source, uxt) + .boxed() } } @@ -537,13 +479,142 @@ where } } -pub fn new_full( +pub struct FraudProofVerifierAndBundleValidator { + client: Arc, + spawner: Box, + fraud_proof_verifier: Verifier, + bundle_validator: BundleValidator, + _phantom_data: PhantomData, +} + +impl Clone + for FraudProofVerifierAndBundleValidator +where + Verifier: Clone, + BundleValidator: Clone, +{ + fn clone(&self) -> Self { + Self { + client: self.client.clone(), + spawner: self.spawner.clone(), + fraud_proof_verifier: self.fraud_proof_verifier.clone(), + bundle_validator: self.bundle_validator.clone(), + _phantom_data: self._phantom_data, + } + } +} + +impl + FraudProofVerifierAndBundleValidator +{ + pub fn new( + client: Arc, + spawner: Box, + fraud_proof_verifier: Verifier, + bundle_validator: BundleValidator, + ) -> Self { + Self { + client, + spawner, + fraud_proof_verifier, + bundle_validator, + _phantom_data: Default::default(), + } + } +} + +#[async_trait::async_trait] +impl PreValidateTransaction + for FraudProofVerifierAndBundleValidator +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: PreValidationObjectApi, + Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, + BundleValidator: + ValidateBundle + Clone + Send + Sync + 'static, +{ + type Block = Block; + async fn pre_validate_transaction( + &self, + at: Block::Hash, + _source: TransactionSource, + uxt: Block::Extrinsic, + ) -> TxPoolResult<()> { + let pre_validation_object = self + .client + .runtime_api() + .extract_pre_validation_object(at, uxt.clone()) + .map_err(|err| sc_transaction_pool::error::Error::Blockchain(err.into()))?; + + match pre_validation_object { + PreValidationObject::Null => { + // No pre-validation is required. + } + PreValidationObject::Bundle(bundle) => { + if let Err(err) = self + .bundle_validator + .validate_bundle(&BlockId::Hash(at), &bundle) + { + tracing::trace!(target: "txpool", error = ?err, "Dropped `submit_bundle` extrinsic"); + return Err(TxPoolError::ImmediatelyDropped.into()); + } + } + PreValidationObject::FraudProof(fraud_proof) => { + let spawner = self.spawner.clone(); + let fraud_proof_verifier = self.fraud_proof_verifier.clone(); + + let (verified_result_sender, verified_result_receiver) = oneshot::channel(); + + // Verify the fraud proof in another blocking task as it might be pretty heavy. + spawner.spawn_blocking( + "txpool-fraud-proof-verification", + None, + async move { + let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); + verified_result_sender + .send(verified_result) + .expect("Failed to send the verified fraud proof result"); + } + .boxed(), + ); + + match verified_result_receiver.await { + Ok(verified_result) => { + match verified_result { + Ok(_) => { + // Continue the regular `validate_transaction` + } + Err(err) => { + tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); + return Err(TxPoolError::InvalidTransaction( + InvalidTransactionCode::FraudProof.into(), + ) + .into()); + } + } + } + Err(err) => { + tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); + return Err(TxPoolError::UnknownTransaction( + UnknownTransaction::CannotLookup, + ) + .into()); + } + } + } + } + + Ok(()) + } +} + +pub fn new_full( config: &Configuration, task_manager: &TaskManager, client: Arc, - verifier: Verifier, - bundle_validator: BundleValidator, -) -> Arc> + tx_pre_validator: TxPreValidator, +) -> Arc> where Block: BlockT, Client: ProvideRuntimeApi @@ -558,16 +629,14 @@ where + 'static, Client::Api: TaggedTransactionQueue + PreValidationObjectApi, - BundleValidator: - ValidateBundle + Clone + Send + Sync + 'static, - Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, + TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { let prometheus = config.prometheus_registry(); let pool_api = Arc::new(FullChainApiWrapper::new( client.clone(), prometheus, task_manager, - FullChainVerifier::new(client.clone(), verifier, bundle_validator), + FullChainVerifier::new(client.clone(), tx_pre_validator), )); let pool = Arc::new(BasicPoolWrapper::with_revalidation_type( config, diff --git a/domains/service/src/system_domain.rs b/domains/service/src/system_domain.rs index d310c079858..dee8e2a2292 100644 --- a/domains/service/src/system_domain.rs +++ b/domains/service/src/system_domain.rs @@ -41,7 +41,7 @@ use std::sync::Arc; use subspace_core_primitives::Blake2b256Hash; use subspace_runtime_primitives::Index as Nonce; use subspace_transaction_pool::bundle_validator::SkipBundleValidation; -use subspace_transaction_pool::FullChainVerifier; +use subspace_transaction_pool::{FraudProofVerifierAndBundleValidator, FullChainVerifier}; use substrate_frame_rpc_system::AccountNonceApi; use system_runtime_primitives::SystemDomainApi; @@ -114,8 +114,12 @@ pub type FullPool = FullChainVerifier< Block, FullClient, - FraudProofVerifier, - SkipBundleValidation, + FraudProofVerifierAndBundleValidator< + Block, + FullClient, + FraudProofVerifier, + SkipBundleValidation, + >, >, StateRootExtractorWithSystemDomainClient>, >, @@ -211,10 +215,17 @@ where subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); + let tx_pre_validator = FraudProofVerifierAndBundleValidator::new( + client.clone(), + Box::new(task_manager.spawn_handle()), + proof_verifier, + SkipBundleValidation, + ); + // Skip bundle validation here because for the system domain the bundle is extract from the // primary block thus it is already validated and accepted by the consensus chain. let system_domain_fraud_proof_verifier = - FullChainVerifier::new(client.clone(), proof_verifier, SkipBundleValidation); + FullChainVerifier::new(client.clone(), tx_pre_validator); let system_domain_xdm_verifier = SystemDomainXDMVerifier::new( primary_chain_client, StateRootExtractorWithSystemDomainClient::new(client.clone()), diff --git a/test/subspace-test-client/src/lib.rs b/test/subspace-test-client/src/lib.rs index 4134d3c85db..97741a79db4 100644 --- a/test/subspace-test-client/src/lib.rs +++ b/test/subspace-test-client/src/lib.rs @@ -49,6 +49,7 @@ use subspace_runtime_primitives::opaque::Block; use subspace_service::{FullClient, NewFull}; use subspace_solving::REWARD_SIGNING_CONTEXT; use subspace_transaction_pool::bundle_validator::BundleValidator; +use subspace_transaction_pool::FraudProofVerifierAndBundleValidator; use zeroize::Zeroizing; /// Subspace native executor instance. @@ -77,10 +78,15 @@ pub type Backend = sc_service::TFullBackend; pub type FraudProofVerifier = subspace_service::FraudProofVerifier; +type TxPreValidator = FraudProofVerifierAndBundleValidator< + Block, + Client, + FraudProofVerifier, + BundleValidator, +>; + /// Run a farmer. -pub fn start_farmer( - new_full: &NewFull, FraudProofVerifier>, -) { +pub fn start_farmer(new_full: &NewFull) { let client = new_full.client.clone(); let new_slot_notification_stream = new_full.new_slot_notification_stream.clone(); let reward_signing_notification_stream = new_full.reward_signing_notification_stream.clone(); diff --git a/test/subspace-test-service/src/lib.rs b/test/subspace-test-service/src/lib.rs index e193a2720e8..40801612ff0 100644 --- a/test/subspace-test-service/src/lib.rs +++ b/test/subspace-test-service/src/lib.rs @@ -44,13 +44,10 @@ use subspace_networking::libp2p::identity; use subspace_runtime_primitives::opaque::Block; use subspace_runtime_primitives::Balance; use subspace_service::{DsnConfig, NewFull, SubspaceConfiguration, SubspaceNetworking}; -use subspace_test_client::{ - chain_spec, start_farmer, Backend, Client, FraudProofVerifier, TestExecutorDispatch, -}; +use subspace_test_client::{chain_spec, start_farmer, Backend, Client, TestExecutorDispatch}; use subspace_test_runtime::{ BlockHashCount, Runtime, RuntimeApi, SignedExtra, SignedPayload, UncheckedExtrinsic, VERSION, }; -use subspace_transaction_pool::bundle_validator::BundleValidator; use subspace_transaction_pool::FullPool; use substrate_test_client::{ BlockchainEventsExt, RpcHandlersExt, RpcTransactionError, RpcTransactionOutput, @@ -284,8 +281,7 @@ pub struct PrimaryTestNode { /// `RPCHandlers` to make RPC queries. pub rpc_handlers: RpcHandlers, /// Transaction pool. - pub transaction_pool: - Arc>>, + pub transaction_pool: Arc>, } impl PrimaryTestNode { diff --git a/test/subspace-test-service/src/mock.rs b/test/subspace-test-service/src/mock.rs index f59b81951ae..0a047df99d4 100644 --- a/test/subspace-test-service/src/mock.rs +++ b/test/subspace-test-service/src/mock.rs @@ -35,10 +35,17 @@ use subspace_solving::create_chunk_signature; use subspace_test_client::{Backend, Client, FraudProofVerifier, TestExecutorDispatch}; use subspace_test_runtime::{RuntimeApi, SLOT_DURATION}; use subspace_transaction_pool::bundle_validator::BundleValidator; -use subspace_transaction_pool::FullPool; +use subspace_transaction_pool::{FraudProofVerifierAndBundleValidator, FullPool}; type StorageChanges = sp_api::StorageChanges, Block>; +pub(super) type TxPreValidator = FraudProofVerifierAndBundleValidator< + Block, + Client, + FraudProofVerifier, + BundleValidator, +>; + /// A mock Subspace primary node instance used for testing. pub struct MockPrimaryNode { /// `TaskManager`'s instance. @@ -50,8 +57,7 @@ pub struct MockPrimaryNode { /// Code executor. pub executor: NativeElseWasmExecutor, /// Transaction pool. - pub transaction_pool: - Arc>>, + pub transaction_pool: Arc>, /// The SelectChain Strategy pub select_chain: FullSelectChain, /// The next slot number @@ -97,12 +103,18 @@ impl MockPrimaryNode { task_manager.spawn_handle(), subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); + let tx_pre_validator = FraudProofVerifierAndBundleValidator::new( + client.clone(), + Box::new(task_manager.spawn_handle()), + proof_verifier.clone(), + bundle_validator, + ); + let transaction_pool = subspace_transaction_pool::new_full( &config, &task_manager, client.clone(), - proof_verifier.clone(), - bundle_validator, + tx_pre_validator, ); let fraud_proof_block_import = From 00ebf22c46b4a15ada9ce495fdbacd919d0a26c7 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sun, 26 Mar 2023 23:08:44 +0800 Subject: [PATCH 2/7] Move `PreValidateTransaction` impls from subspace-transaction-pool into service crates This commit introduces `PrimaryChainTxPreValidator` and `SystemDomainTxPreValidator`, moving the implementations of `PreValidateTransaction` from `subspace-transaction-pool` to the service crates where the transaction pool was constructed. - PrimaryChainTxPreValidator: FraudProofVerifierAndBundleValidator - SystemDomainTxPreValidator: only BundleValidator (xdm verifier will be integrated in later commits) --- Cargo.lock | 2 + crates/subspace-service/src/lib.rs | 14 +- .../subspace-service/src/tx_pre_validator.rs | 148 ++++++++++++++++++ .../src/bundle_validator.rs | 13 -- crates/subspace-transaction-pool/src/lib.rs | 141 +---------------- domains/service/Cargo.toml | 2 + domains/service/src/lib.rs | 1 + domains/service/src/system_domain.rs | 10 +- .../src/system_domain_tx_pre_validator.rs | 127 +++++++++++++++ test/subspace-test-client/src/lib.rs | 10 +- test/subspace-test-service/src/mock.rs | 13 +- 11 files changed, 301 insertions(+), 180 deletions(-) create mode 100644 crates/subspace-service/src/tx_pre_validator.rs create mode 100644 domains/service/src/system_domain_tx_pre_validator.rs diff --git a/Cargo.lock b/Cargo.lock index 21732eb370b..ab2806acb5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2330,6 +2330,7 @@ dependencies = [ name = "domain-service" version = "0.1.0" dependencies = [ + "async-trait", "clap 4.1.13", "cross-domain-message-gossip", "domain-client-consensus-relay-chain", @@ -2380,6 +2381,7 @@ dependencies = [ "substrate-build-script-utils", "substrate-frame-rpc-system", "system-runtime-primitives", + "tracing", ] [[package]] diff --git a/crates/subspace-service/src/lib.rs b/crates/subspace-service/src/lib.rs index 9151be51988..5659d09f0e5 100644 --- a/crates/subspace-service/src/lib.rs +++ b/crates/subspace-service/src/lib.rs @@ -21,11 +21,13 @@ pub mod dsn; pub mod piece_cache; pub mod root_blocks; pub mod rpc; +pub mod tx_pre_validator; use crate::dsn::import_blocks::import_blocks as import_blocks_from_dsn; use crate::dsn::{create_dsn_instance, DsnConfigurationError}; use crate::piece_cache::PieceCache; use crate::root_blocks::{start_root_block_archiver, RootBlockCache}; +use crate::tx_pre_validator::PrimaryChainTxPreValidator; use derive_more::{Deref, DerefMut, Into}; use domain_runtime_primitives::Hash as DomainHash; use dsn::start_dsn_archiver; @@ -81,9 +83,7 @@ use subspace_networking::{peer_id, Node}; use subspace_runtime_primitives::opaque::Block; use subspace_runtime_primitives::{AccountId, Balance, Hash, Index as Nonce}; use subspace_transaction_pool::bundle_validator::BundleValidator; -use subspace_transaction_pool::{ - FraudProofVerifierAndBundleValidator, FullPool, PreValidateTransaction, -}; +use subspace_transaction_pool::{FullPool, PreValidateTransaction}; use tracing::{debug, error, info, Instrument}; /// Error type for Subspace service. @@ -211,7 +211,7 @@ pub fn new_partial( FullPool< Block, FullClient, - FraudProofVerifierAndBundleValidator< + PrimaryChainTxPreValidator< Block, FullClient, FraudProofVerifier, @@ -299,7 +299,7 @@ where task_manager.spawn_handle(), subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); - let tx_pre_validator = FraudProofVerifierAndBundleValidator::new( + let tx_pre_validator = PrimaryChainTxPreValidator::new( client.clone(), Box::new(task_manager.spawn_handle()), proof_verifier.clone(), @@ -421,7 +421,7 @@ where type FullNode = NewFull< FullClient, - FraudProofVerifierAndBundleValidator< + PrimaryChainTxPreValidator< Block, FullClient, FraudProofVerifier, @@ -441,7 +441,7 @@ pub async fn new_full( FullPool< Block, FullClient, - FraudProofVerifierAndBundleValidator< + PrimaryChainTxPreValidator< Block, FullClient, FraudProofVerifier, diff --git a/crates/subspace-service/src/tx_pre_validator.rs b/crates/subspace-service/src/tx_pre_validator.rs new file mode 100644 index 00000000000..da4c4482639 --- /dev/null +++ b/crates/subspace-service/src/tx_pre_validator.rs @@ -0,0 +1,148 @@ +use futures::channel::oneshot; +use futures::future::FutureExt; +use sc_transaction_pool::error::Result as TxPoolResult; +use sc_transaction_pool_api::error::Error as TxPoolError; +use sc_transaction_pool_api::TransactionSource; +use sp_api::ProvideRuntimeApi; +use sp_core::traits::SpawnNamed; +use sp_domains::transaction::{ + InvalidTransactionCode, PreValidationObject, PreValidationObjectApi, +}; +use sp_runtime::generic::BlockId; +use sp_runtime::traits::Block as BlockT; +use sp_runtime::transaction_validity::UnknownTransaction; +use std::marker::PhantomData; +use std::sync::Arc; +use subspace_fraud_proof::VerifyFraudProof; +use subspace_transaction_pool::bundle_validator::ValidateBundle; +use subspace_transaction_pool::PreValidateTransaction; + +pub struct PrimaryChainTxPreValidator { + client: Arc, + spawner: Box, + fraud_proof_verifier: Verifier, + bundle_validator: BundleValidator, + _phantom_data: PhantomData, +} + +impl Clone + for PrimaryChainTxPreValidator +where + Verifier: Clone, + BundleValidator: Clone, +{ + fn clone(&self) -> Self { + Self { + client: self.client.clone(), + spawner: self.spawner.clone(), + fraud_proof_verifier: self.fraud_proof_verifier.clone(), + bundle_validator: self.bundle_validator.clone(), + _phantom_data: self._phantom_data, + } + } +} + +impl + PrimaryChainTxPreValidator +{ + pub fn new( + client: Arc, + spawner: Box, + fraud_proof_verifier: Verifier, + bundle_validator: BundleValidator, + ) -> Self { + Self { + client, + spawner, + fraud_proof_verifier, + bundle_validator, + _phantom_data: Default::default(), + } + } +} + +#[async_trait::async_trait] +impl PreValidateTransaction + for PrimaryChainTxPreValidator +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: PreValidationObjectApi, + Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, + BundleValidator: + ValidateBundle + Clone + Send + Sync + 'static, +{ + type Block = Block; + async fn pre_validate_transaction( + &self, + at: Block::Hash, + _source: TransactionSource, + uxt: Block::Extrinsic, + ) -> TxPoolResult<()> { + let pre_validation_object = self + .client + .runtime_api() + .extract_pre_validation_object(at, uxt.clone()) + .map_err(|err| sc_transaction_pool::error::Error::Blockchain(err.into()))?; + + match pre_validation_object { + PreValidationObject::Null => { + // No pre-validation is required. + } + PreValidationObject::Bundle(bundle) => { + if let Err(err) = self + .bundle_validator + .validate_bundle(&BlockId::Hash(at), &bundle) + { + tracing::trace!(target: "txpool", error = ?err, "Dropped `submit_bundle` extrinsic"); + return Err(TxPoolError::ImmediatelyDropped.into()); + } + } + PreValidationObject::FraudProof(fraud_proof) => { + let spawner = self.spawner.clone(); + let fraud_proof_verifier = self.fraud_proof_verifier.clone(); + + let (verified_result_sender, verified_result_receiver) = oneshot::channel(); + + // Verify the fraud proof in another blocking task as it might be pretty heavy. + spawner.spawn_blocking( + "txpool-fraud-proof-verification", + None, + async move { + let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); + verified_result_sender + .send(verified_result) + .expect("Failed to send the verified fraud proof result"); + } + .boxed(), + ); + + match verified_result_receiver.await { + Ok(verified_result) => { + match verified_result { + Ok(_) => { + // Continue the regular `validate_transaction` + } + Err(err) => { + tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); + return Err(TxPoolError::InvalidTransaction( + InvalidTransactionCode::FraudProof.into(), + ) + .into()); + } + } + } + Err(err) => { + tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); + return Err(TxPoolError::UnknownTransaction( + UnknownTransaction::CannotLookup, + ) + .into()); + } + } + } + } + + Ok(()) + } +} diff --git a/crates/subspace-transaction-pool/src/bundle_validator.rs b/crates/subspace-transaction-pool/src/bundle_validator.rs index 0baa564a5a7..236e42543a4 100644 --- a/crates/subspace-transaction-pool/src/bundle_validator.rs +++ b/crates/subspace-transaction-pool/src/bundle_validator.rs @@ -307,19 +307,6 @@ pub trait ValidateBundle { ) -> Result<(), BundleError>; } -#[derive(Clone)] -pub struct SkipBundleValidation; - -impl ValidateBundle for SkipBundleValidation { - fn validate_bundle( - &self, - _at: &BlockId, - _signed_opaque_bundle: &SignedOpaqueBundle, Block::Hash, DomainHash>, - ) -> Result<(), BundleError> { - Ok(()) - } -} - impl ValidateBundle for BundleValidator where Block: BlockT, diff --git a/crates/subspace-transaction-pool/src/lib.rs b/crates/subspace-transaction-pool/src/lib.rs index 8c62195a290..d8ecc7799be 100644 --- a/crates/subspace-transaction-pool/src/lib.rs +++ b/crates/subspace-transaction-pool/src/lib.rs @@ -1,5 +1,3 @@ -use bundle_validator::ValidateBundle; -use futures::channel::oneshot; use futures::future::{Future, FutureExt, Ready}; use jsonrpsee::core::async_trait; use sc_client_api::blockchain::HeaderBackend; @@ -18,20 +16,15 @@ use sc_transaction_pool_api::{ use sp_api::ProvideRuntimeApi; use sp_blockchain::{HeaderMetadata, TreeRoute}; use sp_core::traits::{SpawnEssentialNamed, SpawnNamed}; -use sp_domains::transaction::{ - InvalidTransactionCode, PreValidationObject, PreValidationObjectApi, -}; +use sp_domains::transaction::PreValidationObjectApi; use sp_runtime::generic::BlockId; use sp_runtime::traits::{Block as BlockT, BlockIdTo, NumberFor, SaturatedConversion}; -use sp_runtime::transaction_validity::{ - TransactionValidity, TransactionValidityError, UnknownTransaction, -}; +use sp_runtime::transaction_validity::{TransactionValidity, TransactionValidityError}; use sp_transaction_pool::runtime_api::TaggedTransactionQueue; use std::collections::HashMap; use std::marker::PhantomData; use std::pin::Pin; use std::sync::Arc; -use subspace_fraud_proof::VerifyFraudProof; use substrate_prometheus_endpoint::Registry as PrometheusRegistry; pub mod bundle_validator; @@ -479,136 +472,6 @@ where } } -pub struct FraudProofVerifierAndBundleValidator { - client: Arc, - spawner: Box, - fraud_proof_verifier: Verifier, - bundle_validator: BundleValidator, - _phantom_data: PhantomData, -} - -impl Clone - for FraudProofVerifierAndBundleValidator -where - Verifier: Clone, - BundleValidator: Clone, -{ - fn clone(&self) -> Self { - Self { - client: self.client.clone(), - spawner: self.spawner.clone(), - fraud_proof_verifier: self.fraud_proof_verifier.clone(), - bundle_validator: self.bundle_validator.clone(), - _phantom_data: self._phantom_data, - } - } -} - -impl - FraudProofVerifierAndBundleValidator -{ - pub fn new( - client: Arc, - spawner: Box, - fraud_proof_verifier: Verifier, - bundle_validator: BundleValidator, - ) -> Self { - Self { - client, - spawner, - fraud_proof_verifier, - bundle_validator, - _phantom_data: Default::default(), - } - } -} - -#[async_trait::async_trait] -impl PreValidateTransaction - for FraudProofVerifierAndBundleValidator -where - Block: BlockT, - Client: ProvideRuntimeApi + Send + Sync, - Client::Api: PreValidationObjectApi, - Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, - BundleValidator: - ValidateBundle + Clone + Send + Sync + 'static, -{ - type Block = Block; - async fn pre_validate_transaction( - &self, - at: Block::Hash, - _source: TransactionSource, - uxt: Block::Extrinsic, - ) -> TxPoolResult<()> { - let pre_validation_object = self - .client - .runtime_api() - .extract_pre_validation_object(at, uxt.clone()) - .map_err(|err| sc_transaction_pool::error::Error::Blockchain(err.into()))?; - - match pre_validation_object { - PreValidationObject::Null => { - // No pre-validation is required. - } - PreValidationObject::Bundle(bundle) => { - if let Err(err) = self - .bundle_validator - .validate_bundle(&BlockId::Hash(at), &bundle) - { - tracing::trace!(target: "txpool", error = ?err, "Dropped `submit_bundle` extrinsic"); - return Err(TxPoolError::ImmediatelyDropped.into()); - } - } - PreValidationObject::FraudProof(fraud_proof) => { - let spawner = self.spawner.clone(); - let fraud_proof_verifier = self.fraud_proof_verifier.clone(); - - let (verified_result_sender, verified_result_receiver) = oneshot::channel(); - - // Verify the fraud proof in another blocking task as it might be pretty heavy. - spawner.spawn_blocking( - "txpool-fraud-proof-verification", - None, - async move { - let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); - verified_result_sender - .send(verified_result) - .expect("Failed to send the verified fraud proof result"); - } - .boxed(), - ); - - match verified_result_receiver.await { - Ok(verified_result) => { - match verified_result { - Ok(_) => { - // Continue the regular `validate_transaction` - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); - return Err(TxPoolError::InvalidTransaction( - InvalidTransactionCode::FraudProof.into(), - ) - .into()); - } - } - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); - return Err(TxPoolError::UnknownTransaction( - UnknownTransaction::CannotLookup, - ) - .into()); - } - } - } - } - - Ok(()) - } -} - pub fn new_full( config: &Configuration, task_manager: &TaskManager, diff --git a/domains/service/Cargo.toml b/domains/service/Cargo.toml index acb41348a18..fc16387c69c 100644 --- a/domains/service/Cargo.toml +++ b/domains/service/Cargo.toml @@ -13,6 +13,7 @@ build = "build.rs" targets = ["x86_64-unknown-linux-gnu"] [dependencies] +async-trait = "0.1.64" clap = { version = "4.1.6", features = ["derive"] } cross-domain-message-gossip = { version = "0.1.0", path = "../client/cross-domain-message-gossip" } domain-client-consensus-relay-chain = { version = "0.1.0", path = "../client/consensus-relay-chain" } @@ -62,6 +63,7 @@ subspace-fraud-proof = { version = "0.1.0", path = "../../crates/subspace-fraud- subspace-runtime-primitives = { version = "0.1.0", path = "../../crates/subspace-runtime-primitives" } subspace-transaction-pool = { version = "0.1.0", path = "../../crates/subspace-transaction-pool" } substrate-frame-rpc-system = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } +tracing = "0.1.37" [build-dependencies] substrate-build-script-utils = { version = "3.0.0", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } diff --git a/domains/service/src/lib.rs b/domains/service/src/lib.rs index edd07ad4c1f..db72ca19bf2 100644 --- a/domains/service/src/lib.rs +++ b/domains/service/src/lib.rs @@ -3,6 +3,7 @@ mod core_domain; mod rpc; mod system_domain; +mod system_domain_tx_pre_validator; pub use self::core_domain::{new_full_core, CoreDomainParams, NewFullCore}; pub use self::system_domain::{new_full_system, FullPool, NewFullSystem}; diff --git a/domains/service/src/system_domain.rs b/domains/service/src/system_domain.rs index dee8e2a2292..ffa2fabf308 100644 --- a/domains/service/src/system_domain.rs +++ b/domains/service/src/system_domain.rs @@ -1,3 +1,4 @@ +use crate::system_domain_tx_pre_validator::SystemDomainTxPreValidator; use crate::{DomainConfiguration, FullBackend, FullClient}; use cross_domain_message_gossip::{DomainTxPoolSink, Message as GossipMessage}; use domain_client_executor::state_root_extractor::StateRootExtractorWithSystemDomainClient; @@ -40,8 +41,7 @@ use sp_transaction_pool::runtime_api::TaggedTransactionQueue; use std::sync::Arc; use subspace_core_primitives::Blake2b256Hash; use subspace_runtime_primitives::Index as Nonce; -use subspace_transaction_pool::bundle_validator::SkipBundleValidation; -use subspace_transaction_pool::{FraudProofVerifierAndBundleValidator, FullChainVerifier}; +use subspace_transaction_pool::FullChainVerifier; use substrate_frame_rpc_system::AccountNonceApi; use system_runtime_primitives::SystemDomainApi; @@ -114,11 +114,10 @@ pub type FullPool = FullChainVerifier< Block, FullClient, - FraudProofVerifierAndBundleValidator< + SystemDomainTxPreValidator< Block, FullClient, FraudProofVerifier, - SkipBundleValidation, >, >, StateRootExtractorWithSystemDomainClient>, @@ -215,11 +214,10 @@ where subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); - let tx_pre_validator = FraudProofVerifierAndBundleValidator::new( + let tx_pre_validator = SystemDomainTxPreValidator::new( client.clone(), Box::new(task_manager.spawn_handle()), proof_verifier, - SkipBundleValidation, ); // Skip bundle validation here because for the system domain the bundle is extract from the diff --git a/domains/service/src/system_domain_tx_pre_validator.rs b/domains/service/src/system_domain_tx_pre_validator.rs new file mode 100644 index 00000000000..8172dc5b2ad --- /dev/null +++ b/domains/service/src/system_domain_tx_pre_validator.rs @@ -0,0 +1,127 @@ +use futures::channel::oneshot; +use futures::future::FutureExt; +use sc_transaction_pool::error::Result as TxPoolResult; +use sc_transaction_pool_api::error::Error as TxPoolError; +use sc_transaction_pool_api::TransactionSource; +use sp_api::ProvideRuntimeApi; +use sp_core::traits::SpawnNamed; +use sp_domains::transaction::{ + InvalidTransactionCode, PreValidationObject, PreValidationObjectApi, +}; +use sp_runtime::traits::Block as BlockT; +use sp_runtime::transaction_validity::UnknownTransaction; +use std::marker::PhantomData; +use std::sync::Arc; +use subspace_fraud_proof::VerifyFraudProof; +use subspace_transaction_pool::PreValidateTransaction; + +pub struct SystemDomainTxPreValidator { + client: Arc, + spawner: Box, + fraud_proof_verifier: Verifier, + _phantom_data: PhantomData, +} + +impl Clone for SystemDomainTxPreValidator +where + Verifier: Clone, +{ + fn clone(&self) -> Self { + Self { + client: self.client.clone(), + spawner: self.spawner.clone(), + fraud_proof_verifier: self.fraud_proof_verifier.clone(), + _phantom_data: self._phantom_data, + } + } +} + +impl SystemDomainTxPreValidator { + pub fn new( + client: Arc, + spawner: Box, + fraud_proof_verifier: Verifier, + ) -> Self { + Self { + client, + spawner, + fraud_proof_verifier, + _phantom_data: Default::default(), + } + } +} + +#[async_trait::async_trait] +impl PreValidateTransaction + for SystemDomainTxPreValidator +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: PreValidationObjectApi, + Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, +{ + type Block = Block; + async fn pre_validate_transaction( + &self, + at: Block::Hash, + _source: TransactionSource, + uxt: Block::Extrinsic, + ) -> TxPoolResult<()> { + let pre_validation_object = self + .client + .runtime_api() + .extract_pre_validation_object(at, uxt.clone()) + .map_err(|err| sc_transaction_pool::error::Error::Blockchain(err.into()))?; + + match pre_validation_object { + PreValidationObject::Null | PreValidationObject::Bundle(_) => { + // No pre-validation is required. + } + PreValidationObject::FraudProof(fraud_proof) => { + let spawner = self.spawner.clone(); + let fraud_proof_verifier = self.fraud_proof_verifier.clone(); + + let (verified_result_sender, verified_result_receiver) = oneshot::channel(); + + // Verify the fraud proof in another blocking task as it might be pretty heavy. + spawner.spawn_blocking( + "txpool-fraud-proof-verification", + None, + async move { + let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); + verified_result_sender + .send(verified_result) + .expect("Failed to send the verified fraud proof result"); + } + .boxed(), + ); + + match verified_result_receiver.await { + Ok(verified_result) => { + match verified_result { + Ok(_) => { + // Continue the regular `validate_transaction` + } + Err(err) => { + tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); + return Err(TxPoolError::InvalidTransaction( + InvalidTransactionCode::FraudProof.into(), + ) + .into()); + } + } + } + Err(err) => { + tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); + return Err(TxPoolError::UnknownTransaction( + UnknownTransaction::CannotLookup, + ) + .into()); + } + } + } + } + + Ok(()) + } +} diff --git a/test/subspace-test-client/src/lib.rs b/test/subspace-test-client/src/lib.rs index 97741a79db4..85de1f77665 100644 --- a/test/subspace-test-client/src/lib.rs +++ b/test/subspace-test-client/src/lib.rs @@ -46,10 +46,10 @@ use subspace_farmer_components::farming::audit_sector; use subspace_farmer_components::plotting::{plot_sector, PieceGetter, PieceGetterRetryPolicy}; use subspace_farmer_components::{FarmerProtocolInfo, SectorMetadata}; use subspace_runtime_primitives::opaque::Block; +use subspace_service::tx_pre_validator::PrimaryChainTxPreValidator; use subspace_service::{FullClient, NewFull}; use subspace_solving::REWARD_SIGNING_CONTEXT; use subspace_transaction_pool::bundle_validator::BundleValidator; -use subspace_transaction_pool::FraudProofVerifierAndBundleValidator; use zeroize::Zeroizing; /// Subspace native executor instance. @@ -78,12 +78,8 @@ pub type Backend = sc_service::TFullBackend; pub type FraudProofVerifier = subspace_service::FraudProofVerifier; -type TxPreValidator = FraudProofVerifierAndBundleValidator< - Block, - Client, - FraudProofVerifier, - BundleValidator, ->; +type TxPreValidator = + PrimaryChainTxPreValidator>; /// Run a farmer. pub fn start_farmer(new_full: &NewFull) { diff --git a/test/subspace-test-service/src/mock.rs b/test/subspace-test-service/src/mock.rs index 0a047df99d4..697189d7a0c 100644 --- a/test/subspace-test-service/src/mock.rs +++ b/test/subspace-test-service/src/mock.rs @@ -30,21 +30,18 @@ use std::time; use subspace_core_primitives::{Blake2b256Hash, Solution}; use subspace_runtime_primitives::opaque::Block; use subspace_runtime_primitives::{AccountId, Hash}; +use subspace_service::tx_pre_validator::PrimaryChainTxPreValidator; use subspace_service::FullSelectChain; use subspace_solving::create_chunk_signature; use subspace_test_client::{Backend, Client, FraudProofVerifier, TestExecutorDispatch}; use subspace_test_runtime::{RuntimeApi, SLOT_DURATION}; use subspace_transaction_pool::bundle_validator::BundleValidator; -use subspace_transaction_pool::{FraudProofVerifierAndBundleValidator, FullPool}; +use subspace_transaction_pool::FullPool; type StorageChanges = sp_api::StorageChanges, Block>; -pub(super) type TxPreValidator = FraudProofVerifierAndBundleValidator< - Block, - Client, - FraudProofVerifier, - BundleValidator, ->; +pub(super) type TxPreValidator = + PrimaryChainTxPreValidator>; /// A mock Subspace primary node instance used for testing. pub struct MockPrimaryNode { @@ -103,7 +100,7 @@ impl MockPrimaryNode { task_manager.spawn_handle(), subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); - let tx_pre_validator = FraudProofVerifierAndBundleValidator::new( + let tx_pre_validator = PrimaryChainTxPreValidator::new( client.clone(), Box::new(task_manager.spawn_handle()), proof_verifier.clone(), From c02d6e10b307a706e7b120bef05aa92c9dbb9da0 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 27 Mar 2023 00:07:11 +0800 Subject: [PATCH 3/7] Remove needless trait bound `PreValidationObjectApi` in subspace-transaction-pool `subspace-transaction-pool` is now pretty flexible, all kinds of pre-validation requiremnts are actually handled by the `TxPreValidator`. --- crates/subspace-transaction-pool/src/lib.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/subspace-transaction-pool/src/lib.rs b/crates/subspace-transaction-pool/src/lib.rs index d8ecc7799be..f571c1c8d0f 100644 --- a/crates/subspace-transaction-pool/src/lib.rs +++ b/crates/subspace-transaction-pool/src/lib.rs @@ -16,7 +16,6 @@ use sc_transaction_pool_api::{ use sp_api::ProvideRuntimeApi; use sp_blockchain::{HeaderMetadata, TreeRoute}; use sp_core::traits::{SpawnEssentialNamed, SpawnNamed}; -use sp_domains::transaction::PreValidationObjectApi; use sp_runtime::generic::BlockId; use sp_runtime::traits::{Block as BlockT, BlockIdTo, NumberFor, SaturatedConversion}; use sp_runtime::transaction_validity::{TransactionValidity, TransactionValidityError}; @@ -179,8 +178,7 @@ where + Send + Sync + 'static, - Client::Api: TaggedTransactionQueue - + PreValidationObjectApi, + Client::Api: TaggedTransactionQueue, TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { fn verify_extrinsic( @@ -490,8 +488,7 @@ where + Send + Sync + 'static, - Client::Api: TaggedTransactionQueue - + PreValidationObjectApi, + Client::Api: TaggedTransactionQueue, TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { let prometheus = config.prometheus_registry(); From 182ddcc67469c310087f8bfed8f1d90fa52d6673 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 27 Mar 2023 00:08:32 +0800 Subject: [PATCH 4/7] Introduce `CoreDomainTxPreValidator` Replace `subspace_transaction_pool::new_full_with_verifier` with `subspace_transaction_pool::new_full` using the core domain specific tx pre-validator. The only requirement of pre-validation on the core domain is the xdm verification. --- .../domain-executor/src/xdm_verifier.rs | 2 +- domains/service/src/core_domain.rs | 27 ++++---- .../src/core_domain_tx_pre_validator.rs | 66 +++++++++++++++++++ domains/service/src/lib.rs | 1 + 4 files changed, 80 insertions(+), 16 deletions(-) create mode 100644 domains/service/src/core_domain_tx_pre_validator.rs diff --git a/domains/client/domain-executor/src/xdm_verifier.rs b/domains/client/domain-executor/src/xdm_verifier.rs index 2794ba60203..32386eacf8e 100644 --- a/domains/client/domain-executor/src/xdm_verifier.rs +++ b/domains/client/domain-executor/src/xdm_verifier.rs @@ -22,7 +22,7 @@ use system_runtime_primitives::SystemDomainApi; /// Core domains nodes use this to verify an XDM coming from other domains. /// Returns either true if the XDM is valid else false. /// Returns Error when required calls to fetch header info fails. -pub(crate) fn verify_xdm_with_system_domain_client( +pub fn verify_xdm_with_system_domain_client( system_domain_client: &Arc, extrinsic: &SBlock::Extrinsic, ) -> Result diff --git a/domains/service/src/core_domain.rs b/domains/service/src/core_domain.rs index 67ad4c486d3..4b80e472e12 100644 --- a/domains/service/src/core_domain.rs +++ b/domains/service/src/core_domain.rs @@ -1,6 +1,6 @@ +use crate::core_domain_tx_pre_validator::CoreDomainTxPreValidator; use crate::{DomainConfiguration, FullBackend, FullClient}; use cross_domain_message_gossip::{DomainTxPoolSink, Message as GossipMessage}; -use domain_client_executor::xdm_verifier::CoreDomainXDMVerifier; use domain_client_executor::{ CoreDomainParentChain, CoreExecutor, CoreGossipMessageValidator, EssentialExecutorParams, ExecutorStreams, @@ -40,6 +40,7 @@ use sp_transaction_pool::runtime_api::TaggedTransactionQueue; use std::sync::Arc; use subspace_core_primitives::{Blake2b256Hash, BlockNumber}; use subspace_runtime_primitives::Index as Nonce; +use subspace_transaction_pool::FullPool; use substrate_frame_rpc_system::AccountNonceApi; use system_runtime_primitives::SystemDomainApi; @@ -51,7 +52,11 @@ type CoreDomainExecutor, SClient, PClient, - FullPool>, + FullPool< + Block, + FullClient, + CoreDomainTxPreValidator, + >, FullBackend, NativeElseWasmExecutor, >; @@ -110,13 +115,6 @@ pub struct NewFullCore< pub tx_pool_sink: DomainTxPoolSink, } -pub type FullPool = - subspace_transaction_pool::FullPoolWithChainVerifier< - Block, - FullClient, - Verifier, - >; - /// Constructs a partial core domain node. #[allow(clippy::type_complexity)] fn new_partial( @@ -128,10 +126,10 @@ fn new_partial( TFullBackend, (), sc_consensus::DefaultImportQueue>, - subspace_transaction_pool::FullPoolWithChainVerifier< + FullPool< Block, FullClient, - CoreDomainXDMVerifier, + CoreDomainTxPreValidator, >, ( Option, @@ -188,13 +186,12 @@ where telemetry }); - let core_domain_xdm_verifier = - CoreDomainXDMVerifier::::new(system_domain_client); - let transaction_pool = subspace_transaction_pool::new_full_with_verifier( + let core_domain_tx_pre_validator = CoreDomainTxPreValidator::new(system_domain_client); + let transaction_pool = subspace_transaction_pool::new_full( config, &task_manager, client.clone(), - core_domain_xdm_verifier, + core_domain_tx_pre_validator, ); let import_queue = domain_client_consensus_relay_chain::import_queue( diff --git a/domains/service/src/core_domain_tx_pre_validator.rs b/domains/service/src/core_domain_tx_pre_validator.rs new file mode 100644 index 00000000000..cfaa5cd0f5c --- /dev/null +++ b/domains/service/src/core_domain_tx_pre_validator.rs @@ -0,0 +1,66 @@ +use domain_client_executor::xdm_verifier::verify_xdm_with_system_domain_client; +use sc_transaction_pool::error::Result as TxPoolResult; +use sc_transaction_pool_api::error::Error as TxPoolError; +use sc_transaction_pool_api::TransactionSource; +use sp_api::ProvideRuntimeApi; +use sp_blockchain::HeaderBackend; +use sp_messenger::MessengerApi; +use sp_runtime::traits::{Block as BlockT, NumberFor}; +use std::marker::PhantomData; +use std::sync::Arc; +use subspace_transaction_pool::PreValidateTransaction; +use system_runtime_primitives::SystemDomainApi; + +pub struct CoreDomainTxPreValidator { + system_domain_client: Arc, + _phantom_data: PhantomData<(Block, SBlock, PBlock)>, +} + +impl Clone + for CoreDomainTxPreValidator +{ + fn clone(&self) -> Self { + Self { + system_domain_client: self.system_domain_client.clone(), + _phantom_data: self._phantom_data, + } + } +} + +impl CoreDomainTxPreValidator { + pub fn new(system_domain_client: Arc) -> Self { + Self { + system_domain_client, + _phantom_data: Default::default(), + } + } +} + +#[async_trait::async_trait] +impl PreValidateTransaction + for CoreDomainTxPreValidator +where + Block: BlockT, + SBlock: BlockT, + PBlock: BlockT, + Block::Extrinsic: Into, + SClient: HeaderBackend + ProvideRuntimeApi + Send + Sync + 'static, + SClient::Api: MessengerApi> + + SystemDomainApi, PBlock::Hash>, +{ + type Block = Block; + async fn pre_validate_transaction( + &self, + _at: Block::Hash, + _source: TransactionSource, + uxt: Block::Extrinsic, + ) -> TxPoolResult<()> { + if !verify_xdm_with_system_domain_client::<_, Block, SBlock, PBlock>( + &self.system_domain_client, + &(uxt.into()), + )? { + return Err(TxPoolError::ImmediatelyDropped.into()); + } + Ok(()) + } +} diff --git a/domains/service/src/lib.rs b/domains/service/src/lib.rs index db72ca19bf2..05a74cb2938 100644 --- a/domains/service/src/lib.rs +++ b/domains/service/src/lib.rs @@ -1,6 +1,7 @@ //! Service and ServiceFactory implementation. Specialized wrapper over substrate service. mod core_domain; +mod core_domain_tx_pre_validator; mod rpc; mod system_domain; mod system_domain_tx_pre_validator; From 1ca93b334f2502f6ee0f36e419a65094cdaeb4d7 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 27 Mar 2023 00:44:44 +0800 Subject: [PATCH 5/7] Integrate xdm verifier into `SystemDomainTxPreValidator` Integrate the xdm verification in the system domain tx pre-validator and also replace `subspace_transaction_pool::new_full_with_verifier` with `subspace_transaction_pool::new_full`. --- .../domain-executor/src/xdm_verifier.rs | 2 +- domains/service/src/system_domain.rs | 45 ++++++------------- .../src/system_domain_tx_pre_validator.rs | 44 +++++++++++++++--- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/domains/client/domain-executor/src/xdm_verifier.rs b/domains/client/domain-executor/src/xdm_verifier.rs index 32386eacf8e..57a5f0b0894 100644 --- a/domains/client/domain-executor/src/xdm_verifier.rs +++ b/domains/client/domain-executor/src/xdm_verifier.rs @@ -86,7 +86,7 @@ where /// This is used by the System domain to validate Extrinsics. /// Returns either true if the XDM is valid else false. /// Returns Error when required calls to fetch header info fails. -pub(crate) fn verify_xdm_with_primary_chain_client( +pub fn verify_xdm_with_primary_chain_client( primary_chain_client: &Arc, state_root_extractor: &SRE, extrinsic: &SBlock::Extrinsic, diff --git a/domains/service/src/system_domain.rs b/domains/service/src/system_domain.rs index ffa2fabf308..39eaee1e124 100644 --- a/domains/service/src/system_domain.rs +++ b/domains/service/src/system_domain.rs @@ -2,7 +2,6 @@ use crate::system_domain_tx_pre_validator::SystemDomainTxPreValidator; use crate::{DomainConfiguration, FullBackend, FullClient}; use cross_domain_message_gossip::{DomainTxPoolSink, Message as GossipMessage}; use domain_client_executor::state_root_extractor::StateRootExtractorWithSystemDomainClient; -use domain_client_executor::xdm_verifier::SystemDomainXDMVerifier; use domain_client_executor::{ EssentialExecutorParams, ExecutorStreams, SystemDomainParentChain, SystemExecutor, SystemGossipMessageValidator, @@ -41,7 +40,6 @@ use sp_transaction_pool::runtime_api::TaggedTransactionQueue; use std::sync::Arc; use subspace_core_primitives::Blake2b256Hash; use subspace_runtime_primitives::Index as Nonce; -use subspace_transaction_pool::FullChainVerifier; use substrate_frame_rpc_system::AccountNonceApi; use system_runtime_primitives::SystemDomainApi; @@ -58,6 +56,7 @@ type SystemDomainExecutor = Syste /// System domain full node along with some other components. pub struct NewFullSystem where + Block: BlockT, PBlock: BlockT, NumberFor: From>, PBlock::Hash: From, @@ -103,26 +102,18 @@ where pub tx_pool_sink: DomainTxPoolSink, } -pub type FullPool = - subspace_transaction_pool::FullPoolWithChainVerifier< +pub type FullPool = subspace_transaction_pool::FullPool< + Block, + FullClient, + SystemDomainTxPreValidator< Block, + PBlock, FullClient, - SystemDomainXDMVerifier< - PClient, - PBlock, - Block, - FullChainVerifier< - Block, - FullClient, - SystemDomainTxPreValidator< - Block, - FullClient, - FraudProofVerifier, - >, - >, - StateRootExtractorWithSystemDomainClient>, - >, - >; + FraudProofVerifier, + PClient, + StateRootExtractorWithSystemDomainClient>, + >, +>; type FraudProofVerifier = subspace_fraud_proof::ProofVerifier< @@ -214,27 +205,19 @@ where subspace_fraud_proof::PrePostStateRootVerifier::new(client.clone()), ); - let tx_pre_validator = SystemDomainTxPreValidator::new( + let system_domain_tx_pre_validator = SystemDomainTxPreValidator::new( client.clone(), Box::new(task_manager.spawn_handle()), proof_verifier, - ); - - // Skip bundle validation here because for the system domain the bundle is extract from the - // primary block thus it is already validated and accepted by the consensus chain. - let system_domain_fraud_proof_verifier = - FullChainVerifier::new(client.clone(), tx_pre_validator); - let system_domain_xdm_verifier = SystemDomainXDMVerifier::new( primary_chain_client, StateRootExtractorWithSystemDomainClient::new(client.clone()), - system_domain_fraud_proof_verifier, ); - let transaction_pool = subspace_transaction_pool::new_full_with_verifier( + let transaction_pool = subspace_transaction_pool::new_full( config, &task_manager, client.clone(), - system_domain_xdm_verifier, + system_domain_tx_pre_validator, ); let import_queue = domain_client_consensus_relay_chain::import_queue( diff --git a/domains/service/src/system_domain_tx_pre_validator.rs b/domains/service/src/system_domain_tx_pre_validator.rs index 8172dc5b2ad..bd628c2589e 100644 --- a/domains/service/src/system_domain_tx_pre_validator.rs +++ b/domains/service/src/system_domain_tx_pre_validator.rs @@ -1,64 +1,86 @@ +use domain_client_executor::state_root_extractor::StateRootExtractor; +use domain_client_executor::xdm_verifier::verify_xdm_with_primary_chain_client; use futures::channel::oneshot; use futures::future::FutureExt; use sc_transaction_pool::error::Result as TxPoolResult; use sc_transaction_pool_api::error::Error as TxPoolError; use sc_transaction_pool_api::TransactionSource; use sp_api::ProvideRuntimeApi; +use sp_blockchain::HeaderBackend; use sp_core::traits::SpawnNamed; use sp_domains::transaction::{ InvalidTransactionCode, PreValidationObject, PreValidationObjectApi, }; -use sp_runtime::traits::Block as BlockT; +use sp_domains::ExecutorApi; +use sp_runtime::traits::{Block as BlockT, NumberFor}; use sp_runtime::transaction_validity::UnknownTransaction; use std::marker::PhantomData; use std::sync::Arc; use subspace_fraud_proof::VerifyFraudProof; use subspace_transaction_pool::PreValidateTransaction; -pub struct SystemDomainTxPreValidator { +pub struct SystemDomainTxPreValidator { client: Arc, spawner: Box, fraud_proof_verifier: Verifier, - _phantom_data: PhantomData, + primary_chain_client: Arc, + state_root_extractor: SRE, + _phantom_data: PhantomData<(Block, PBlock)>, } -impl Clone for SystemDomainTxPreValidator +impl Clone + for SystemDomainTxPreValidator where Verifier: Clone, + SRE: Clone, { fn clone(&self) -> Self { Self { client: self.client.clone(), spawner: self.spawner.clone(), fraud_proof_verifier: self.fraud_proof_verifier.clone(), + primary_chain_client: self.primary_chain_client.clone(), + state_root_extractor: self.state_root_extractor.clone(), _phantom_data: self._phantom_data, } } } -impl SystemDomainTxPreValidator { +impl + SystemDomainTxPreValidator +{ pub fn new( client: Arc, spawner: Box, fraud_proof_verifier: Verifier, + primary_chain_client: Arc, + state_root_extractor: SRE, ) -> Self { Self { client, spawner, fraud_proof_verifier, + primary_chain_client, + state_root_extractor, _phantom_data: Default::default(), } } } #[async_trait::async_trait] -impl PreValidateTransaction - for SystemDomainTxPreValidator +impl PreValidateTransaction + for SystemDomainTxPreValidator where Block: BlockT, + PBlock: BlockT, + PBlock::Hash: From, + NumberFor: From>, Client: ProvideRuntimeApi + Send + Sync, Client::Api: PreValidationObjectApi, Verifier: VerifyFraudProof + Clone + Send + Sync + 'static, + PClient: HeaderBackend + ProvideRuntimeApi + 'static, + PClient::Api: ExecutorApi, + SRE: StateRootExtractor + Send + Sync, { type Block = Block; async fn pre_validate_transaction( @@ -67,6 +89,14 @@ where _source: TransactionSource, uxt: Block::Extrinsic, ) -> TxPoolResult<()> { + if !verify_xdm_with_primary_chain_client::( + &self.primary_chain_client, + &self.state_root_extractor, + &uxt, + )? { + return Err(TxPoolError::ImmediatelyDropped.into()); + } + let pre_validation_object = self .client .runtime_api() From 4401624dc6615d78c643d875e6488873655ae0ab Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 27 Mar 2023 01:11:45 +0800 Subject: [PATCH 6/7] Remove legacy FullChainWithVerifier abstraction and remove unnecesary transaction pool deps from domain-executor --- Cargo.lock | 2 - crates/subspace-transaction-pool/src/lib.rs | 185 +++--------------- domains/client/domain-executor/Cargo.toml | 3 - .../domain-executor/src/xdm_verifier.rs | 177 +---------------- 4 files changed, 31 insertions(+), 336 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab2806acb5b..cdc83083760 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2235,14 +2235,12 @@ dependencies = [ "sp-messenger", "sp-runtime", "sp-state-machine", - "sp-transaction-pool", "sp-trie", "subspace-core-primitives", "subspace-fraud-proof", "subspace-runtime-primitives", "subspace-test-runtime", "subspace-test-service", - "subspace-transaction-pool", "subspace-wasm-tools", "substrate-test-runtime-client", "substrate-test-utils", diff --git a/crates/subspace-transaction-pool/src/lib.rs b/crates/subspace-transaction-pool/src/lib.rs index f571c1c8d0f..af46617b1cc 100644 --- a/crates/subspace-transaction-pool/src/lib.rs +++ b/crates/subspace-transaction-pool/src/lib.rs @@ -15,13 +15,12 @@ use sc_transaction_pool_api::{ }; use sp_api::ProvideRuntimeApi; use sp_blockchain::{HeaderMetadata, TreeRoute}; -use sp_core::traits::{SpawnEssentialNamed, SpawnNamed}; +use sp_core::traits::SpawnEssentialNamed; use sp_runtime::generic::BlockId; use sp_runtime::traits::{Block as BlockT, BlockIdTo, NumberFor, SaturatedConversion}; use sp_runtime::transaction_validity::{TransactionValidity, TransactionValidityError}; use sp_transaction_pool::runtime_api::TaggedTransactionQueue; use std::collections::HashMap; -use std::marker::PhantomData; use std::pin::Pin; use std::sync::Arc; use substrate_prometheus_endpoint::Registry as PrometheusRegistry; @@ -38,10 +37,8 @@ type ExtrinsicHash = <::Block as BlockT>::Hash; type ExtrinsicFor = <::Block as BlockT>::Extrinsic; /// A transaction pool for a full node. -pub type FullPool = BasicPoolWrapper< - Block, - FullChainApiWrapper>, ->; +pub type FullPool = + BasicPoolWrapper>; /// A transaction pool with chain verifier. pub type FullPoolWithChainVerifier = @@ -55,14 +52,13 @@ type ReadyIteratorFor = BoxedReadyIterator, Extr type PolledIterator = Pin> + Send>>; #[derive(Clone)] -pub struct FullChainApiWrapper { +pub struct FullChainApiWrapper { inner: Arc>, client: Arc, - verifier: Verifier, - spawner: Box, + tx_pre_validator: TxPreValidator, } -impl FullChainApiWrapper +impl FullChainApiWrapper where Block: BlockT, Client: ProvideRuntimeApi @@ -74,12 +70,13 @@ where + Sync + 'static, Client::Api: TaggedTransactionQueue, + TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { fn new( client: Arc, prometheus: Option<&PrometheusRegistry>, task_manager: &TaskManager, - verifier: Verifier, + tx_pre_validator: TxPreValidator, ) -> Self { Self { inner: Arc::new(FullChainApi::new( @@ -88,8 +85,7 @@ where &task_manager.spawn_essential_handle(), )), client, - verifier, - spawner: Box::new(task_manager.spawn_handle()), + tx_pre_validator, } } @@ -105,18 +101,6 @@ where pub type BlockExtrinsicOf = ::Extrinsic; -/// Abstracts and provides just the validation hook for a transaction pool. -pub trait VerifyExtrinsic { - fn verify_extrinsic( - &self, - at: Block::Hash, - source: TransactionSource, - uxt: BlockExtrinsicOf, - spawner: Box, - chain_api: Arc, - ) -> ValidationFuture; -} - /// This trait allows to perform some extra validation on the extrinsic before /// doing the regular `validate_transaction` in the substrate transaction pool. #[async_trait::async_trait] @@ -131,81 +115,9 @@ pub trait PreValidateTransaction { ) -> TxPoolResult<()>; } -/// Verifier to verify the Fraud proofs and receipts. -pub struct FullChainVerifier { - _phantom_data: PhantomData, - client: Arc, - tx_pre_validator: TxPreValidator, -} - -impl Clone for FullChainVerifier -where - Block: Clone, - TxPreValidator: Clone, -{ - fn clone(&self) -> Self { - Self { - _phantom_data: Default::default(), - client: self.client.clone(), - tx_pre_validator: self.tx_pre_validator.clone(), - } - } -} - -impl FullChainVerifier -where - Block: BlockT, - Client: HeaderBackend, -{ - pub fn new(client: Arc, tx_pre_validator: TxPreValidator) -> Self { - Self { - _phantom_data: Default::default(), - client, - tx_pre_validator, - } - } -} - -impl VerifyExtrinsic> - for FullChainVerifier -where - Block: BlockT, - Client: ProvideRuntimeApi - + BlockBackend - + BlockIdTo - + HeaderBackend - + HeaderMetadata - + Send - + Sync - + 'static, - Client::Api: TaggedTransactionQueue, - TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, -{ - fn verify_extrinsic( - &self, - at: Block::Hash, - source: TransactionSource, - uxt: BlockExtrinsicOf, - _spawner: Box, - chain_api: Arc>, - ) -> ValidationFuture { - let tx_pre_validator = self.tx_pre_validator.clone(); - async move { - tx_pre_validator - .pre_validate_transaction(at, source, uxt.clone()) - .await?; - - chain_api - .validate_transaction(&BlockId::Hash(at), source, uxt) - .await - } - .boxed() - } -} - pub type ValidationFuture = Pin> + Send>>; -impl ChainApi for FullChainApiWrapper +impl ChainApi for FullChainApiWrapper where Block: BlockT, Client: ProvideRuntimeApi @@ -217,8 +129,7 @@ where + Sync + 'static, Client::Api: TaggedTransactionQueue, - Verifier: - VerifyExtrinsic> + Clone + Send + Sync + 'static, + TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { type Block = Block; type Error = sc_transaction_pool::error::Error; @@ -247,8 +158,18 @@ where } }; - self.verifier - .verify_extrinsic(at, source, uxt, self.spawner.clone(), self.inner.clone()) + let tx_pre_validator = self.tx_pre_validator.clone(); + let chain_api = self.inner.clone(); + async move { + tx_pre_validator + .pre_validate_transaction(at, source, uxt.clone()) + .await?; + + chain_api + .validate_transaction(&BlockId::Hash(at), source, uxt) + .await + } + .boxed() } fn block_id_to_number( @@ -335,8 +256,8 @@ where } } -impl sc_transaction_pool_api::LocalTransactionPool - for BasicPoolWrapper> +impl sc_transaction_pool_api::LocalTransactionPool + for BasicPoolWrapper> where Block: BlockT, Client: ProvideRuntimeApi @@ -348,12 +269,11 @@ where + Sync + 'static, Client::Api: TaggedTransactionQueue, - Verifier: - VerifyExtrinsic> + Clone + Send + Sync + 'static, + TxPreValidator: PreValidateTransaction + Send + Sync + Clone + 'static, { type Block = Block; - type Hash = ExtrinsicHash>; - type Error = as ChainApi>::Error; + type Hash = ExtrinsicHash>; + type Error = as ChainApi>::Error; fn submit_local( &self, @@ -496,55 +416,8 @@ where client.clone(), prometheus, task_manager, - FullChainVerifier::new(client.clone(), tx_pre_validator), - )); - let pool = Arc::new(BasicPoolWrapper::with_revalidation_type( - config, - pool_api, - prometheus, - task_manager.spawn_essential_handle(), - client.clone(), - )); - - // make transaction pool available for off-chain runtime calls. - client - .execution_extensions() - .register_transaction_pool(&pool); - - pool -} - -/// Constructs a transaction pool with provided verifier. -pub fn new_full_with_verifier( - config: &Configuration, - task_manager: &TaskManager, - client: Arc, - verifier: Verifier, -) -> Arc> -where - Block: BlockT, - Client: ProvideRuntimeApi - + BlockBackend - + HeaderBackend - + HeaderMetadata - + ExecutorProvider - + UsageProvider - + BlockIdTo - + Send - + Sync - + 'static, - Client::Api: TaggedTransactionQueue, - Verifier: - VerifyExtrinsic> + Clone + Send + Sync + 'static, -{ - let prometheus = config.prometheus_registry(); - let pool_api = Arc::new(FullChainApiWrapper::new( - client.clone(), - prometheus, - task_manager, - verifier, + tx_pre_validator, )); - let pool = Arc::new(BasicPoolWrapper::with_revalidation_type( config, pool_api, diff --git a/domains/client/domain-executor/Cargo.toml b/domains/client/domain-executor/Cargo.toml index e3df3d16f6a..586501103a5 100644 --- a/domains/client/domain-executor/Cargo.toml +++ b/domains/client/domain-executor/Cargo.toml @@ -20,7 +20,6 @@ sc-client-api = { version = "4.0.0-dev", git = "https://github.com/subspace/subs sc-consensus = { version = "0.10.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sc-executor = { version = "0.10.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sc-network = { version = "0.10.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } -sc-transaction-pool = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sc-transaction-pool-api = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sc-utils = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sp-api = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } @@ -35,12 +34,10 @@ sp-keystore = { version = "0.13.0", git = "https://github.com/subspace/substrate sp-messenger = { version = "0.1.0", path = "../../primitives/messenger" } sp-runtime = { version = "7.0.0", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sp-state-machine = { version = "0.13.0", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } -sp-transaction-pool = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sp-trie = { version = "7.0.0", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } subspace-core-primitives = { version = "0.1.0", path = "../../../crates/subspace-core-primitives" } subspace-fraud-proof = { version = "0.1.0", path = "../../../crates/subspace-fraud-proof" } subspace-runtime-primitives = { version = "0.1.0", path = "../../../crates/subspace-runtime-primitives" } -subspace-transaction-pool = { version = "0.1.0", path = "../../../crates/subspace-transaction-pool" } subspace-wasm-tools = { version = "0.1.0", path = "../../../crates/subspace-wasm-tools" } system-runtime-primitives = { version = "0.1.0", path = "../../primitives/system-runtime" } tracing = "0.1.37" diff --git a/domains/client/domain-executor/src/xdm_verifier.rs b/domains/client/domain-executor/src/xdm_verifier.rs index 57a5f0b0894..de7d0853a4d 100644 --- a/domains/client/domain-executor/src/xdm_verifier.rs +++ b/domains/client/domain-executor/src/xdm_verifier.rs @@ -1,20 +1,10 @@ use crate::state_root_extractor::StateRootExtractor; -use futures::FutureExt; -use sc_client_api::BlockBackend; -use sc_transaction_pool::{ChainApi, FullChainApi}; -use sc_transaction_pool_api::error::Error as TxPoolError; use sp_api::ProvideRuntimeApi; -use sp_blockchain::{Error, HeaderBackend, HeaderMetadata}; -use sp_core::traits::SpawnNamed; +use sp_blockchain::{Error, HeaderBackend}; use sp_domains::ExecutorApi; use sp_messenger::MessengerApi; -use sp_runtime::generic::BlockId; -use sp_runtime::traits::{Block as BlockT, BlockIdTo, CheckedSub, Header, NumberFor}; -use sp_runtime::transaction_validity::TransactionSource; -use sp_transaction_pool::runtime_api::TaggedTransactionQueue; -use std::marker::PhantomData; +use sp_runtime::traits::{Block as BlockT, CheckedSub, Header, NumberFor}; use std::sync::Arc; -use subspace_transaction_pool::{BlockExtrinsicOf, ValidationFuture, VerifyExtrinsic}; use system_runtime_primitives::SystemDomainApi; /// Verifies if the xdm has the correct proof generated from known parent block. @@ -118,166 +108,3 @@ where Ok(true) } - -/// A verifier for XDM messages on System domain. -pub struct SystemDomainXDMVerifier { - _data: PhantomData<(PBlock, SBlock)>, - primary_chain_client: Arc, - state_root_extractor: SRE, - inner_verifier: Verifier, -} - -impl - SystemDomainXDMVerifier -{ - pub fn new( - primary_chain_client: Arc, - state_root_extractor: SRE, - inner_verifier: Verifier, - ) -> Self { - Self { - _data: Default::default(), - primary_chain_client, - state_root_extractor, - inner_verifier, - } - } -} - -impl Clone - for SystemDomainXDMVerifier -where - Verifier: Clone, - SRE: Clone, -{ - fn clone(&self) -> Self { - Self { - _data: Default::default(), - primary_chain_client: self.primary_chain_client.clone(), - state_root_extractor: self.state_root_extractor.clone(), - inner_verifier: self.inner_verifier.clone(), - } - } -} - -impl - VerifyExtrinsic> - for SystemDomainXDMVerifier -where - PClient: HeaderBackend + ProvideRuntimeApi + 'static, - PClient::Api: ExecutorApi, - SClient: HeaderBackend + ProvideRuntimeApi + 'static, - SClient::Api: MessengerApi> - + SystemDomainApi, PBlock::Hash>, - SBlock: BlockT, - PBlock: BlockT, - NumberFor: From>, - PBlock::Hash: From, - Verifier: VerifyExtrinsic>, - SRE: StateRootExtractor, -{ - fn verify_extrinsic( - &self, - at: SBlock::Hash, - source: TransactionSource, - uxt: BlockExtrinsicOf, - spawner: Box, - chain_api: Arc>, - ) -> ValidationFuture { - let result = verify_xdm_with_primary_chain_client::<_, _, _, _>( - &self.primary_chain_client, - &self.state_root_extractor, - &uxt, - ); - - match result { - Ok(valid) => { - if valid { - self.inner_verifier - .verify_extrinsic(at, source, uxt, spawner, chain_api) - } else { - tracing::trace!(target: "system_domain_xdm_validator", "Dropped invalid XDM extrinsic"); - async move { Err(TxPoolError::ImmediatelyDropped.into()) }.boxed() - } - } - Err(err) => { - tracing::trace!(target: "system_domain_xdm_validator", error = ?err, "Failed to verify XDM"); - async move { Err(TxPoolError::ImmediatelyDropped.into()) }.boxed() - } - } - } -} - -/// A Verifier for XDM messages on Core domains. -pub struct CoreDomainXDMVerifier { - _data: PhantomData<(PBlock, SBlock)>, - system_domain_client: Arc, -} - -impl CoreDomainXDMVerifier { - pub fn new(system_domain_client: Arc) -> Self { - Self { - _data: Default::default(), - system_domain_client, - } - } -} - -impl Clone for CoreDomainXDMVerifier { - fn clone(&self) -> Self { - Self { - _data: Default::default(), - system_domain_client: self.system_domain_client.clone(), - } - } -} - -impl VerifyExtrinsic> - for CoreDomainXDMVerifier -where - Block: BlockT, - SDC: HeaderBackend + ProvideRuntimeApi + 'static, - SDC::Api: MessengerApi> - + SystemDomainApi, PBlock::Hash>, - Block: BlockT, - SBlock: BlockT, - Block::Extrinsic: Into, - PBlock: BlockT, - Client: ProvideRuntimeApi - + BlockBackend - + BlockIdTo - + HeaderBackend - + HeaderMetadata - + Send - + Sync - + 'static, - Client::Api: TaggedTransactionQueue, -{ - fn verify_extrinsic( - &self, - at: Block::Hash, - source: TransactionSource, - uxt: BlockExtrinsicOf, - _spawner: Box, - chain_api: Arc>, - ) -> ValidationFuture { - let result = verify_xdm_with_system_domain_client::( - &self.system_domain_client, - &(uxt.clone().into()), - ); - match result { - Ok(valid) => { - if valid { - chain_api.validate_transaction(&BlockId::Hash(at), source, uxt) - } else { - tracing::trace!(target: "core_domain_xdm_validator", "Dropped invalid XDM extrinsic"); - async move { Err(TxPoolError::ImmediatelyDropped.into()) }.boxed() - } - } - Err(err) => { - tracing::trace!(target: "core_domain_xdm_validator", error = ?err, "Failed to verify XDM"); - async move { Err(TxPoolError::ImmediatelyDropped.into()) }.boxed() - } - } - } -} From 14af3d0a5c8e5d84af727b8b889ec73e4ce91b4e Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 27 Mar 2023 09:59:08 +0800 Subject: [PATCH 7/7] Extract `validate_fraud_proof_in_tx_pool` in subspace-fraud-proof and remove `subspace-fraud-proof` dep from `subspace-transaction-pool` --- Cargo.lock | 1 - crates/sp-domains/src/fraud_proof.rs | 6 +++ crates/subspace-fraud-proof/Cargo.toml | 2 +- crates/subspace-fraud-proof/src/lib.rs | 33 ++++++++++++ .../subspace-service/src/tx_pre_validator.rs | 54 ++++--------------- crates/subspace-transaction-pool/Cargo.toml | 1 - .../src/system_domain_tx_pre_validator.rs | 54 ++++--------------- 7 files changed, 60 insertions(+), 91 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cdc83083760..4f56c591610 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10514,7 +10514,6 @@ dependencies = [ "sp-domains", "sp-runtime", "sp-transaction-pool", - "subspace-fraud-proof", "substrate-prometheus-endpoint", "tracing", ] diff --git a/crates/sp-domains/src/fraud_proof.rs b/crates/sp-domains/src/fraud_proof.rs index 3ab629432b0..e67ac94d02b 100644 --- a/crates/sp-domains/src/fraud_proof.rs +++ b/crates/sp-domains/src/fraud_proof.rs @@ -128,6 +128,12 @@ pub enum VerificationError { #[cfg(feature = "std")] #[cfg_attr(feature = "thiserror", error("Failed to get runtime code: {0}"))] RuntimeCode(String), + #[cfg(feature = "std")] + #[cfg_attr( + feature = "thiserror", + error("Oneshot error when verifying fraud proof in tx pool: {0}") + )] + Oneshot(String), } /// Fraud proof. diff --git a/crates/subspace-fraud-proof/Cargo.toml b/crates/subspace-fraud-proof/Cargo.toml index 35b6cab3b38..69221f56b49 100644 --- a/crates/subspace-fraud-proof/Cargo.toml +++ b/crates/subspace-fraud-proof/Cargo.toml @@ -14,6 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.4.0", features = ["derive"] } domain-runtime-primitives = { version = "0.1.0", path = "../../domains/primitives/runtime" } +futures = "0.3.26" hash-db = "0.15.2" sc-client-api = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sp-api = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } @@ -30,7 +31,6 @@ tracing = "0.1.37" [dev-dependencies] domain-block-builder = { version = "0.1.0", path = "../../domains/client/block-builder" } domain-test-service = { version = "0.1.0", path = "../../domains/test/service" } -futures = "0.3.26" pallet-balances = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sc-cli = { version = "0.10.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232", default-features = false } sc-service = { version = "0.10.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232", default-features = false } diff --git a/crates/subspace-fraud-proof/src/lib.rs b/crates/subspace-fraud-proof/src/lib.rs index 669ca83fcf4..8109eaec5bb 100644 --- a/crates/subspace-fraud-proof/src/lib.rs +++ b/crates/subspace-fraud-proof/src/lib.rs @@ -12,6 +12,8 @@ mod invalid_state_transition_proof; mod tests; use codec::{Decode, Encode}; +use futures::channel::oneshot; +use futures::FutureExt; use invalid_state_transition_proof::InvalidStateTransitionProofVerifier; pub use invalid_state_transition_proof::{ ExecutionProver, PrePostStateRootVerifier, VerifyPrePostStateRoot, @@ -117,3 +119,34 @@ where self.verify(proof) } } + +/// Verifies the fraud proof extracted from extrinsic in the transaction pool. +pub async fn validate_fraud_proof_in_tx_pool( + spawner: &dyn SpawnNamed, + fraud_proof_verifier: Verifier, + fraud_proof: FraudProof, Block::Hash>, +) -> Result<(), VerificationError> +where + Block: BlockT, + Verifier: VerifyFraudProof + Send + 'static, +{ + let (verified_result_sender, verified_result_receiver) = oneshot::channel(); + + // Verify the fraud proof in another blocking task as it might be pretty heavy. + spawner.spawn_blocking( + "txpool-fraud-proof-verification", + None, + async move { + let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); + verified_result_sender + .send(verified_result) + .expect("Failed to send the verified fraud proof result"); + } + .boxed(), + ); + + match verified_result_receiver.await { + Ok(verified_result) => verified_result, + Err(err) => Err(VerificationError::Oneshot(err.to_string())), + } +} diff --git a/crates/subspace-service/src/tx_pre_validator.rs b/crates/subspace-service/src/tx_pre_validator.rs index da4c4482639..359eca44f37 100644 --- a/crates/subspace-service/src/tx_pre_validator.rs +++ b/crates/subspace-service/src/tx_pre_validator.rs @@ -1,5 +1,3 @@ -use futures::channel::oneshot; -use futures::future::FutureExt; use sc_transaction_pool::error::Result as TxPoolResult; use sc_transaction_pool_api::error::Error as TxPoolError; use sc_transaction_pool_api::TransactionSource; @@ -10,7 +8,6 @@ use sp_domains::transaction::{ }; use sp_runtime::generic::BlockId; use sp_runtime::traits::Block as BlockT; -use sp_runtime::transaction_validity::UnknownTransaction; use std::marker::PhantomData; use std::sync::Arc; use subspace_fraud_proof::VerifyFraudProof; @@ -99,47 +96,16 @@ where } } PreValidationObject::FraudProof(fraud_proof) => { - let spawner = self.spawner.clone(); - let fraud_proof_verifier = self.fraud_proof_verifier.clone(); - - let (verified_result_sender, verified_result_receiver) = oneshot::channel(); - - // Verify the fraud proof in another blocking task as it might be pretty heavy. - spawner.spawn_blocking( - "txpool-fraud-proof-verification", - None, - async move { - let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); - verified_result_sender - .send(verified_result) - .expect("Failed to send the verified fraud proof result"); - } - .boxed(), - ); - - match verified_result_receiver.await { - Ok(verified_result) => { - match verified_result { - Ok(_) => { - // Continue the regular `validate_transaction` - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); - return Err(TxPoolError::InvalidTransaction( - InvalidTransactionCode::FraudProof.into(), - ) - .into()); - } - } - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); - return Err(TxPoolError::UnknownTransaction( - UnknownTransaction::CannotLookup, - ) - .into()); - } - } + subspace_fraud_proof::validate_fraud_proof_in_tx_pool( + &self.spawner, + self.fraud_proof_verifier.clone(), + fraud_proof, + ) + .await + .map_err(|err| { + tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); + TxPoolError::InvalidTransaction(InvalidTransactionCode::FraudProof.into()) + })?; } } diff --git a/crates/subspace-transaction-pool/Cargo.toml b/crates/subspace-transaction-pool/Cargo.toml index cde406e9089..8a2409dca22 100644 --- a/crates/subspace-transaction-pool/Cargo.toml +++ b/crates/subspace-transaction-pool/Cargo.toml @@ -27,6 +27,5 @@ sp-consensus-subspace = { version = "0.1.0", path = "../sp-consensus-subspace" } sp-domains = { version = "0.1.0", path = "../sp-domains" } sp-runtime = { version = "7.0.0", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } sp-transaction-pool = { version = "4.0.0-dev", git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } -subspace-fraud-proof = { version = "0.1.0", path = "../subspace-fraud-proof" } substrate-prometheus-endpoint = { git = "https://github.com/subspace/substrate", rev = "456cfad45a178617f6886ec400c312f2fea59232" } tracing = "0.1.37" diff --git a/domains/service/src/system_domain_tx_pre_validator.rs b/domains/service/src/system_domain_tx_pre_validator.rs index bd628c2589e..8b332d10818 100644 --- a/domains/service/src/system_domain_tx_pre_validator.rs +++ b/domains/service/src/system_domain_tx_pre_validator.rs @@ -1,7 +1,5 @@ use domain_client_executor::state_root_extractor::StateRootExtractor; use domain_client_executor::xdm_verifier::verify_xdm_with_primary_chain_client; -use futures::channel::oneshot; -use futures::future::FutureExt; use sc_transaction_pool::error::Result as TxPoolResult; use sc_transaction_pool_api::error::Error as TxPoolError; use sc_transaction_pool_api::TransactionSource; @@ -13,7 +11,6 @@ use sp_domains::transaction::{ }; use sp_domains::ExecutorApi; use sp_runtime::traits::{Block as BlockT, NumberFor}; -use sp_runtime::transaction_validity::UnknownTransaction; use std::marker::PhantomData; use std::sync::Arc; use subspace_fraud_proof::VerifyFraudProof; @@ -108,47 +105,16 @@ where // No pre-validation is required. } PreValidationObject::FraudProof(fraud_proof) => { - let spawner = self.spawner.clone(); - let fraud_proof_verifier = self.fraud_proof_verifier.clone(); - - let (verified_result_sender, verified_result_receiver) = oneshot::channel(); - - // Verify the fraud proof in another blocking task as it might be pretty heavy. - spawner.spawn_blocking( - "txpool-fraud-proof-verification", - None, - async move { - let verified_result = fraud_proof_verifier.verify_fraud_proof(&fraud_proof); - verified_result_sender - .send(verified_result) - .expect("Failed to send the verified fraud proof result"); - } - .boxed(), - ); - - match verified_result_receiver.await { - Ok(verified_result) => { - match verified_result { - Ok(_) => { - // Continue the regular `validate_transaction` - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); - return Err(TxPoolError::InvalidTransaction( - InvalidTransactionCode::FraudProof.into(), - ) - .into()); - } - } - } - Err(err) => { - tracing::debug!(target: "txpool", error = ?err, "Failed to receive the fraud proof verified result"); - return Err(TxPoolError::UnknownTransaction( - UnknownTransaction::CannotLookup, - ) - .into()); - } - } + subspace_fraud_proof::validate_fraud_proof_in_tx_pool( + &self.spawner, + self.fraud_proof_verifier.clone(), + fraud_proof, + ) + .await + .map_err(|err| { + tracing::debug!(target: "txpool", error = ?err, "Invalid fraud proof"); + TxPoolError::InvalidTransaction(InvalidTransactionCode::FraudProof.into()) + })?; } }