Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow using any polkadot client instead of enum Client #1575

Merged
15 commits merged into from
Aug 13, 2020
Merged

Allow using any polkadot client instead of enum Client #1575

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ac4e6c4
WIP
cecton Aug 12, 2020
5527eaa
WIP
cecton Aug 12, 2020
47f184e
WIP
cecton Aug 12, 2020
f60375c
WIP
cecton Aug 12, 2020
99bbf74
WIP
cecton Aug 12, 2020
43512e8
WIP
cecton Aug 12, 2020
ae7082c
WIP
cecton Aug 12, 2020
fb78f0b
WIP
cecton Aug 12, 2020
846e23f
Apply suggestions from code review
cecton Aug 12, 2020
5caaaeb
WIP
cecton Aug 13, 2020
0070028
WIP
cecton Aug 13, 2020
b66861c
WIP
cecton Aug 13, 2020
96cdf51
CLEANUP
cecton Aug 13, 2020
5c4f5ca
link in doc
cecton Aug 13, 2020
12cbf02
doc
cecton Aug 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 28 additions & 15 deletions collator/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub use sc_network::PeerId;
pub use service::{RuntimeApiCollection, Client};
pub use sc_cli::SubstrateCli;
#[cfg(not(feature = "service-rewr"))]
use polkadot_service::{FullNodeHandles, AbstractClient};
use polkadot_service::{FullNodeHandles, AbstractClient, YaExecuteWithClient};
#[cfg(feature = "service-rewr")]
use polkadot_service_new::{
self as polkadot_service,
Expand All @@ -77,6 +77,7 @@ use sc_service::SpawnTaskHandle;
use sp_core::traits::SpawnNamed;
use sp_runtime::traits::BlakeTwo256;
use consensus_common::SyncOracle;
use sc_client_api::Backend as BackendT;

const COLLATION_TIMEOUT: Duration = Duration::from_secs(30);

Expand Down Expand Up @@ -117,14 +118,19 @@ pub trait BuildParachainContext {
type ParachainContext: self::ParachainContext;

/// Build the `ParachainContext`.
fn build<SP>(
fn build<SP, Client, Backend>(
self,
client: polkadot_service::Client,
client: Arc<Client>,
spawner: SP,
network: impl Network + SyncOracle + Clone + 'static,
) -> Result<Self::ParachainContext, ()>
where
SP: SpawnNamed + Clone + Send + Sync + 'static;
SP: SpawnNamed + Clone + Send + Sync + 'static,
Backend: BackendT<Block>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was sooo happy that we got rid off it :P But yeah, this will also make some code easier in Cumulus 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I AM SO SORRY

Backend::State: sp_api::StateBackend<BlakeTwo256>,
Client: polkadot_service::AbstractClient<Block, Backend> + 'static,
Client::Api: RuntimeApiCollection<StateBackend = Backend::State>,
;
}

/// Parachain context needed for collation.
Expand Down Expand Up @@ -194,11 +200,12 @@ pub async fn collate<P>(
Some(collation)
}

/// Build a collator service based on the `YaExecuteWithClient`.
#[cfg(feature = "service-rewr")]
fn build_collator_service<P>(
pub fn build_collator_service<P>(
spawner: SpawnTaskHandle,
handles: FullNodeHandles,
client: polkadot_service::Client,
client: impl YaExecuteWithClient,
para_id: ParaId,
key: Arc<CollatorPair>,
build_parachain_context: P,
Expand All @@ -217,7 +224,6 @@ struct BuildCollationWork<P> {
key: Arc<CollatorPair>,
build_parachain_context: P,
spawner: SpawnTaskHandle,
client: polkadot_service::Client,
}

impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
Expand All @@ -233,7 +239,7 @@ impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
Backend: sc_client_api::Backend<Block>,
Backend::State: sp_api::StateBackend<BlakeTwo256>,
Api: RuntimeApiCollection<StateBackend = Backend::State>,
Client: AbstractClient<Block, Backend, Api = Api> + 'static
Client: AbstractClient<Block, Backend, Api = Api> + 'static,
{
let polkadot_network = self.handles
.polkadot_network
Expand All @@ -246,7 +252,7 @@ impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
.ok_or_else(|| "Collator cannot run when validation networking has not been started")?;

let parachain_context = match self.build_parachain_context.build(
self.client,
client.clone(),
self.spawner.clone(),
polkadot_network.clone(),
) {
Expand Down Expand Up @@ -346,11 +352,12 @@ impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
}
}

/// Build a collator service based on the `YaExecuteWithClient`.
#[cfg(not(feature = "service-rewr"))]
fn build_collator_service<P>(
pub fn build_collator_service<P>(
spawner: SpawnTaskHandle,
handles: FullNodeHandles,
client: polkadot_service::Client,
client: impl YaExecuteWithClient,
para_id: ParaId,
key: Arc<CollatorPair>,
build_parachain_context: P,
Expand All @@ -366,7 +373,6 @@ fn build_collator_service<P>(
key,
build_parachain_context,
spawner,
client: client.clone(),
})
}

Expand Down Expand Up @@ -454,12 +460,19 @@ mod tests {
impl BuildParachainContext for BuildDummyParachainContext {
type ParachainContext = DummyParachainContext;

fn build<SP>(
fn build<SP, Client, Backend>(
self,
_: polkadot_service::Client,
_: Arc<Client>,
_: SP,
_: impl Network + Clone + 'static,
) -> Result<Self::ParachainContext, ()> {
) -> Result<Self::ParachainContext, ()>
where
SP: SpawnNamed + Clone + Send + Sync + 'static,
Backend: BackendT<Block>,
Backend::State: sp_api::StateBackend<BlakeTwo256>,
Client: polkadot_service::AbstractClient<Block, Backend> + 'static,
Client::Api: RuntimeApiCollection<StateBackend = Backend::State>,
{
Ok(DummyParachainContext)
}
}
Expand Down
13 changes: 11 additions & 2 deletions node/test-service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use polkadot_primitives::v0::{
Block, Hash, CollatorId, Id as ParaId,
};
use polkadot_runtime_common::{parachains, registrar, BlockHashCount};
use polkadot_service::{new_full, FullNodeHandles, AbstractClient};
use polkadot_service::{new_full, FullNodeHandles, AbstractClient, YaExecuteWithClient, ExecuteWithClient};
use polkadot_test_runtime::{RestrictFunctionality, Runtime, SignedExtra, SignedPayload, VERSION};
use sc_chain_spec::ChainSpec;
use sc_client_api::{execution_extensions::ExecutionStrategies, BlockchainEvents};
Expand Down Expand Up @@ -67,7 +67,7 @@ pub fn polkadot_test_new_full(
) -> Result<
(
TaskManager,
Arc<impl AbstractClient<Block, TFullBackend<Block>>>,
Arc<polkadot_service::FullClient<polkadot_test_runtime::RuntimeApi, PolkadotTestExecutor>>,
FullNodeHandles,
Arc<NetworkService<Block, Hash>>,
Arc<RpcHandlers>,
Expand All @@ -88,6 +88,15 @@ pub fn polkadot_test_new_full(
Ok((task_manager, client, handles, network, rpc_handlers))
}

/// A wrapper for the test client that implements `YaExecuteWithClient`.
pub struct TestClient(pub Arc<polkadot_service::FullClient<polkadot_test_runtime::RuntimeApi, PolkadotTestExecutor>>);

impl YaExecuteWithClient for TestClient {
fn execute_with<T: ExecuteWithClient>(&self, t: T) -> T::Output {
T::execute_with_client::<_, _, polkadot_service::FullBackend>(t, self.0.clone())
}
}

/// Create a Polkadot `Configuration`. By default an in-memory socket will be used, therefore you need to provide boot
/// nodes if you want the future node to be connected to other nodes. The `storage_update_func` can be used to make
/// adjustements to the runtime before the node starts.
Expand Down
3 changes: 3 additions & 0 deletions parachain/test-parachains/adder/collator/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ adder = { package = "test-parachain-adder", path = ".." }
parachain = { package = "polkadot-parachain", path = "../../.." }
collator = { package = "polkadot-collator", path = "../../../../collator" }
primitives = { package = "polkadot-primitives", path = "../../../../primitives" }
service = { package = "polkadot-service", path = "../../../../service" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
client-api = { package = "sc-client-api", git = "https://github.com/paritytech/substrate", branch = "master" }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
parking_lot = "0.10.0"
codec = { package = "parity-scale-codec", version = "1.3.4" }
futures = "0.3.4"
19 changes: 14 additions & 5 deletions parachain/test-parachains/adder/collator/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ use std::collections::HashMap;
use std::sync::Arc;

use adder::{HeadData as AdderHead, BlockData as AdderBody};
use sp_core::Pair;
use sp_core::{traits::SpawnNamed, Pair};
use codec::{Encode, Decode};
use primitives::v0::{
Hash, DownwardMessage,
Block, Hash, DownwardMessage,
HeadData, BlockData, Id as ParaId, LocalValidationData, GlobalValidationData,
};
use collator::{ParachainContext, Network, BuildParachainContext, Cli, SubstrateCli};
use parking_lot::Mutex;
use futures::future::{Ready, ready, FutureExt};
use sp_runtime::traits::BlakeTwo256;
use client_api::Backend as BackendT;

const GENESIS: AdderHead = AdderHead {
number: 0,
Expand Down Expand Up @@ -103,12 +105,19 @@ impl ParachainContext for AdderContext {
impl BuildParachainContext for AdderContext {
type ParachainContext = Self;

fn build<SP>(
fn build<SP, Client, Backend>(
self,
_: collator::Client,
_: Arc<Client>,
_: SP,
network: impl Network + Clone + 'static,
) -> Result<Self::ParachainContext, ()> {
) -> Result<Self::ParachainContext, ()>
where
SP: SpawnNamed + Clone + Send + Sync + 'static,
Backend: BackendT<Block>,
Backend::State: sp_api::StateBackend<BlakeTwo256>,
Client: service::AbstractClient<Block, Backend> + 'static,
Client::Api: service::RuntimeApiCollection<StateBackend = Backend::State>,
{
Ok(Self { _network: Some(Arc::new(network)), ..self })
}
}
Expand Down
11 changes: 8 additions & 3 deletions service/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ pub trait ExecuteWithClient {
Client: AbstractClient<Block, Backend, Api = Api> + 'static;
}

/// Yet Another ExecuteWithClient
pub trait YaExecuteWithClient {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ClientHandle?

With a much better documentation on what it does and why we need it? Something like:

/// A handle to a Polkadot client instance
///
/// The Polkadot service supports multiple different runtimes, like
/// Westend, Polkadot itself, etc. As each runtime has a specialized
/// client, we need to hide them behind a trait. This is this trait. 
///
/// When wanting to work with the inner client, you need to use [`execute_with`](ClientHandle::execute_with).
/// See [`ExecuteWithClient`] for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg I was not expecting that much 😆 🙏

/// Execute the given something with the client.
fn execute_with<T: ExecuteWithClient>(&self, t: T) -> T::Output;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to call it... ClientExecutor? ClientExecutionContext? @bkchr


/// A client instance of Polkadot.
///
/// See [`ExecuteWithClient`] for more information.
Expand All @@ -129,9 +135,8 @@ pub enum Client {
Rococo(Arc<crate::FullClient<rococo_runtime::RuntimeApi, crate::RococoExecutor>>),
}

impl Client {
/// Execute the given something with the client.
pub fn execute_with<T: ExecuteWithClient>(&self, t: T) -> T::Output {
impl YaExecuteWithClient for Client {
fn execute_with<T: ExecuteWithClient>(&self, t: T) -> T::Output {
match self {
Self::Polkadot(client) => {
T::execute_with_client::<_, _, crate::FullBackend>(t, client.clone())
Expand Down
12 changes: 6 additions & 6 deletions service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,16 @@ impl IdentifyVariant for Box<dyn ChainSpec> {
}
}

type FullBackend = service::TFullBackend<Block>;
type FullSelectChain = sc_consensus::LongestChain<FullBackend, Block>;
type FullClient<RuntimeApi, Executor> = service::TFullClient<Block, RuntimeApi, Executor>;
type FullGrandpaBlockImport<RuntimeApi, Executor> = grandpa::GrandpaBlockImport<
pub type FullBackend = service::TFullBackend<Block>;
pub type FullSelectChain = sc_consensus::LongestChain<FullBackend, Block>;
pub type FullClient<RuntimeApi, Executor> = service::TFullClient<Block, RuntimeApi, Executor>;
pub type FullGrandpaBlockImport<RuntimeApi, Executor> = grandpa::GrandpaBlockImport<
FullBackend, Block, FullClient<RuntimeApi, Executor>, FullSelectChain
>;

type LightBackend = service::TLightBackendWithHash<Block, sp_runtime::traits::BlakeTwo256>;
pub type LightBackend = service::TLightBackendWithHash<Block, sp_runtime::traits::BlakeTwo256>;

type LightClient<RuntimeApi, Executor> =
pub type LightClient<RuntimeApi, Executor> =
service::TLightClientWithBackend<Block, RuntimeApi, Executor, LightBackend>;

#[cfg(feature = "full-node")]
Expand Down