Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve sc-service API #5364

Merged
merged 15 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
2 changes: 1 addition & 1 deletion cumulus/client/relay-chain-minimal-node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async fn new_minimal_relay_chain<Block: BlockT, Network: NetworkBackend<RelayBlo
collator_pair: CollatorPair,
relay_chain_rpc_client: Arc<BlockChainRpcClient>,
) -> Result<NewMinimalNode, RelayChainError> {
let role = config.role.clone();
let role = config.role;
let mut net_config = sc_network::config::FullNetworkConfiguration::<_, _, Network>::new(
&config.network,
config.prometheus_config.as_ref().map(|cfg| cfg.registry.clone()),
Expand Down
2 changes: 1 addition & 1 deletion cumulus/client/relay-chain-minimal-node/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(crate) fn build_collator_network<Network: NetworkBackend<Block, Hash>>(
spawn_handle.spawn("peer-store", Some("networking"), peer_store.run());

let network_params = sc_network::config::Params::<Block, Hash, Network> {
role: config.role.clone(),
role: config.role,
executor: {
let spawn_handle = Clone::clone(&spawn_handle);
Box::new(move |fut| {
Expand Down
3 changes: 1 addition & 2 deletions cumulus/polkadot-parachain/polkadot-parachain-lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,9 @@ impl<Config: CliConfig> CliConfiguration<Self> for RelayChainCli<Config> {
_support_url: &String,
_impl_version: &String,
_logger_hook: F,
_config: &sc_service::Configuration,
) -> sc_cli::Result<()>
where
F: FnOnce(&mut sc_cli::LoggerBuilder, &sc_service::Configuration),
F: FnOnce(&mut sc_cli::LoggerBuilder),
{
unreachable!("PolkadotCli is never initialized; qed");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,15 @@ pub(crate) trait NodeSpec {
})
.transpose()?;

let heap_pages = config.default_heap_pages.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |h| {
HeapAllocStrategy::Static { extra_pages: h as _ }
});
let heap_pages =
config.executor.default_heap_pages.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |h| {
HeapAllocStrategy::Static { extra_pages: h as _ }
});

let executor = sc_executor::WasmExecutor::<ParachainHostFunctions>::builder()
.with_execution_method(config.wasm_method)
.with_max_runtime_instances(config.max_runtime_instances)
.with_runtime_cache_size(config.runtime_cache_size)
.with_execution_method(config.executor.wasm_method)
.with_max_runtime_instances(config.executor.max_runtime_instances)
.with_runtime_cache_size(config.executor.runtime_cache_size)
.with_onchain_heap_alloc_strategy(heap_pages)
.with_offchain_heap_alloc_strategy(heap_pages)
.build();
Expand Down
3 changes: 1 addition & 2 deletions cumulus/test/service/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,9 @@ impl CliConfiguration<Self> for RelayChainCli {
_support_url: &String,
_impl_version: &String,
_logger_hook: F,
_config: &sc_service::Configuration,
) -> CliResult<()>
where
F: FnOnce(&mut sc_cli::LoggerBuilder, &sc_service::Configuration),
F: FnOnce(&mut sc_cli::LoggerBuilder),
{
unreachable!("PolkadotCli is never initialized; qed");
}
Expand Down
77 changes: 41 additions & 36 deletions cumulus/test/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ use sc_network::{
};
use sc_service::{
config::{
BlocksPruning, DatabaseSource, KeystoreConfig, MultiaddrWithPeerId, NetworkConfiguration,
OffchainWorkerConfig, PruningMode, RpcBatchRequestConfig, RpcEndpoint, WasmExecutionMethod,
BlocksPruning, DatabaseSource, ExecutorConfiguration, KeystoreConfig, MultiaddrWithPeerId,
NetworkConfiguration, OffchainWorkerConfig, PruningMode, RpcBatchRequestConfig,
RpcConfiguration, RpcEndpoint, WasmExecutionMethod,
},
BasePath, ChainSpec as ChainSpecService, Configuration, Error as ServiceError,
PartialComponents, Role, RpcHandlers, TFullBackend, TFullClient, TaskManager,
Expand Down Expand Up @@ -194,15 +195,16 @@ pub fn new_partial(
enable_import_proof_record: bool,
) -> Result<Service, sc_service::Error> {
let heap_pages = config
.executor
.default_heap_pages
.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |h| HeapAllocStrategy::Static { extra_pages: h as _ });

let executor = WasmExecutor::builder()
.with_execution_method(config.wasm_method)
.with_execution_method(config.executor.wasm_method)
.with_onchain_heap_alloc_strategy(heap_pages)
.with_offchain_heap_alloc_strategy(heap_pages)
.with_max_runtime_instances(config.max_runtime_instances)
.with_runtime_cache_size(config.runtime_cache_size)
.with_max_runtime_instances(config.executor.max_runtime_instances)
.with_runtime_cache_size(config.executor.runtime_cache_size)
.build();

let (client, backend, keystore_container, task_manager) =
Expand Down Expand Up @@ -863,38 +865,41 @@ pub fn node_config(
state_pruning: Some(PruningMode::ArchiveAll),
blocks_pruning: BlocksPruning::KeepAll,
chain_spec: spec,
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy: sc_executor_wasmtime::InstantiationStrategy::PoolingCopyOnWrite,
executor: ExecutorConfiguration {
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy:
sc_executor_wasmtime::InstantiationStrategy::PoolingCopyOnWrite,
},
..ExecutorConfiguration::default()
},
rpc: RpcConfiguration {
addr: None,
max_connections: Default::default(),
cors: None,
methods: Default::default(),
max_request_size: Default::default(),
max_response_size: Default::default(),
id_provider: None,
max_subs_per_conn: Default::default(),
port: 9945,
message_buffer_capacity: Default::default(),
batch_config: RpcBatchRequestConfig::Unlimited,
rate_limit: None,
rate_limit_whitelisted_ips: Default::default(),
rate_limit_trust_proxy_headers: Default::default(),
},
rpc_addr: None,
rpc_max_connections: Default::default(),
rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_request_size: Default::default(),
rpc_max_response_size: Default::default(),
rpc_id_provider: None,
rpc_max_subs_per_conn: Default::default(),
rpc_port: 9945,
rpc_message_buffer_capacity: Default::default(),
rpc_batch_config: RpcBatchRequestConfig::Unlimited,
rpc_rate_limit: None,
rpc_rate_limit_whitelisted_ips: Default::default(),
rpc_rate_limit_trust_proxy_headers: Default::default(),
prometheus_config: None,
telemetry_endpoints: None,
default_heap_pages: None,
offchain_worker: OffchainWorkerConfig { enabled: true, indexing_enabled: false },
force_authoring: false,
disable_grandpa: false,
dev_key_seed: Some(key_seed),
tracing_targets: None,
tracing_receiver: Default::default(),
max_runtime_instances: 8,
announce_block: true,
data_path: root,
base_path,
wasm_runtime_overrides: None,
runtime_cache_size: 2,
})
}

Expand Down Expand Up @@ -1006,19 +1011,19 @@ pub fn run_relay_chain_validator_node(
);

if let Some(port) = port {
config.rpc_addr = Some(vec![RpcEndpoint {
batch_config: config.rpc_batch_config,
cors: config.rpc_cors.clone(),
config.rpc.addr = Some(vec![RpcEndpoint {
batch_config: config.rpc.batch_config,
cors: config.rpc.cors.clone(),
listen_addr: SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::LOCALHOST, port)),
max_connections: config.rpc_max_connections,
max_payload_in_mb: config.rpc_max_request_size,
max_payload_out_mb: config.rpc_max_response_size,
max_subscriptions_per_connection: config.rpc_max_subs_per_conn,
max_buffer_capacity_per_connection: config.rpc_message_buffer_capacity,
rpc_methods: config.rpc_methods,
rate_limit: config.rpc_rate_limit,
rate_limit_trust_proxy_headers: config.rpc_rate_limit_trust_proxy_headers,
rate_limit_whitelisted_ips: config.rpc_rate_limit_whitelisted_ips.clone(),
max_connections: config.rpc.max_connections,
max_payload_in_mb: config.rpc.max_request_size,
max_payload_out_mb: config.rpc.max_response_size,
max_subscriptions_per_connection: config.rpc.max_subs_per_conn,
max_buffer_capacity_per_connection: config.rpc.message_buffer_capacity,
rpc_methods: config.rpc.methods,
rate_limit: config.rpc.rate_limit,
rate_limit_trust_proxy_headers: config.rpc.rate_limit_trust_proxy_headers,
rate_limit_whitelisted_ips: config.rpc.rate_limit_whitelisted_ips.clone(),
retry_random_port: true,
is_optional: false,
}]);
Expand Down
9 changes: 5 additions & 4 deletions polkadot/node/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,15 +437,16 @@ fn new_partial_basics(
.transpose()?;

let heap_pages = config
.executor
.default_heap_pages
.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |h| HeapAllocStrategy::Static { extra_pages: h as _ });

let executor = WasmExecutor::builder()
.with_execution_method(config.wasm_method)
.with_execution_method(config.executor.wasm_method)
.with_onchain_heap_alloc_strategy(heap_pages)
.with_offchain_heap_alloc_strategy(heap_pages)
.with_max_runtime_instances(config.max_runtime_instances)
.with_runtime_cache_size(config.runtime_cache_size)
.with_max_runtime_instances(config.executor.max_runtime_instances)
.with_runtime_cache_size(config.executor.runtime_cache_size)
.build();

let (client, backend, keystore_container, task_manager) =
Expand Down Expand Up @@ -764,7 +765,7 @@ pub fn new_full<
use sc_network_sync::WarpSyncConfig;

let is_offchain_indexing_enabled = config.offchain_worker.indexing_enabled;
let role = config.role.clone();
let role = config.role;
let force_authoring = config.force_authoring;
let backoff_authoring_blocks = if !force_authoring_backoff &&
(config.chain_spec.is_polkadot() || config.chain_spec.is_kusama())
Expand Down
41 changes: 22 additions & 19 deletions polkadot/node/test/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use substrate_test_client::{
pub type Client = FullClient;

pub use polkadot_service::{FullBackend, GetLastTimestamp};
use sc_service::config::{ExecutorConfiguration, RpcConfiguration};

/// Create a new full node.
#[sc_tracing::logging::prefix_logs_with(config.network.node_name.as_str())]
Expand Down Expand Up @@ -200,35 +201,37 @@ pub fn node_config(
state_pruning: Default::default(),
blocks_pruning: BlocksPruning::KeepFinalized,
chain_spec: Box::new(spec),
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy: WasmtimeInstantiationStrategy::PoolingCopyOnWrite,
executor: ExecutorConfiguration {
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy: WasmtimeInstantiationStrategy::PoolingCopyOnWrite,
},
..ExecutorConfiguration::default()
},
wasm_runtime_overrides: Default::default(),
rpc_addr: Default::default(),
rpc_max_request_size: Default::default(),
rpc_max_response_size: Default::default(),
rpc_max_connections: Default::default(),
rpc_cors: None,
rpc_methods: Default::default(),
rpc_id_provider: None,
rpc_max_subs_per_conn: Default::default(),
rpc_port: 9944,
rpc_message_buffer_capacity: Default::default(),
rpc_batch_config: RpcBatchRequestConfig::Unlimited,
rpc_rate_limit: None,
rpc_rate_limit_whitelisted_ips: Default::default(),
rpc_rate_limit_trust_proxy_headers: Default::default(),
rpc: RpcConfiguration {
addr: Default::default(),
max_request_size: Default::default(),
max_response_size: Default::default(),
max_connections: Default::default(),
cors: None,
methods: Default::default(),
id_provider: None,
max_subs_per_conn: Default::default(),
port: 9944,
message_buffer_capacity: Default::default(),
batch_config: RpcBatchRequestConfig::Unlimited,
rate_limit: None,
rate_limit_whitelisted_ips: Default::default(),
rate_limit_trust_proxy_headers: Default::default(),
},
prometheus_config: None,
telemetry_endpoints: None,
default_heap_pages: None,
offchain_worker: Default::default(),
force_authoring: false,
disable_grandpa: false,
dev_key_seed: Some(key_seed),
tracing_targets: None,
tracing_receiver: Default::default(),
max_runtime_instances: 8,
runtime_cache_size: 2,
announce_block: true,
data_path: root,
base_path,
Expand Down
39 changes: 39 additions & 0 deletions prdoc/pr_5364.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
title: Improve `sc-service` API

doc:
- audience: Node Dev
description: |
This improves `sc-service` API by not requiring the whole `&Configuration`, using specific configuration options
instead. `RpcConfiguration` and `ExecutorConfiguration` were also extracted from `Configuration` to group all RPC
and executor options together.
If `sc-service` is used as a library with lower-level APIs, `Configuration` can now be avoided in most cases.
nazar-pc marked this conversation as resolved.
Show resolved Hide resolved

This mainly impacts you on your node implementation. There you need to change this:
```
with_execution_method(config.wasm_method)
```

to this:
```
with_execution_method(config.executor.wasm_method)
```

There are similar changes required as well, but all are around the initialization of the wasm executor.

crates:
- name: sc-service
bump: major
- name: sc-network-common
bump: patch
- name: sc-cli
bump: major
- name: polkadot-service
bump: patch
- name: cumulus-relay-chain-minimal-node
bump: none
- name: polkadot-parachain-bin
bump: major
- name: polkadot-parachain-lib
bump: major
- name: staging-node-inspect
bump: major
41 changes: 22 additions & 19 deletions substrate/bin/node/cli/benches/block_production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughpu

use kitchensink_runtime::{constants::currency::*, BalancesCall};
use node_cli::service::{create_extrinsic, FullClient};
use polkadot_sdk::sc_service::config::{ExecutorConfiguration, RpcConfiguration};
use sc_block_builder::{BlockBuilderBuilder, BuiltBlock};
use sc_consensus::{
block_import::{BlockImportParams, ForkChoiceStrategy},
Expand Down Expand Up @@ -73,34 +74,36 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
state_pruning: Some(PruningMode::ArchiveAll),
blocks_pruning: BlocksPruning::KeepAll,
chain_spec: spec,
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy: WasmtimeInstantiationStrategy::PoolingCopyOnWrite,
executor: ExecutorConfiguration {
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy: WasmtimeInstantiationStrategy::PoolingCopyOnWrite,
},
..ExecutorConfiguration::default()
},
rpc: RpcConfiguration {
addr: None,
max_connections: Default::default(),
cors: None,
methods: Default::default(),
max_request_size: Default::default(),
max_response_size: Default::default(),
id_provider: Default::default(),
max_subs_per_conn: Default::default(),
port: 9944,
message_buffer_capacity: Default::default(),
batch_config: RpcBatchRequestConfig::Unlimited,
rate_limit: None,
rate_limit_whitelisted_ips: Default::default(),
rate_limit_trust_proxy_headers: Default::default(),
},
rpc_addr: None,
rpc_max_connections: Default::default(),
rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_request_size: Default::default(),
rpc_max_response_size: Default::default(),
rpc_id_provider: Default::default(),
rpc_max_subs_per_conn: Default::default(),
rpc_port: 9944,
rpc_message_buffer_capacity: Default::default(),
rpc_batch_config: RpcBatchRequestConfig::Unlimited,
rpc_rate_limit: None,
rpc_rate_limit_whitelisted_ips: Default::default(),
rpc_rate_limit_trust_proxy_headers: Default::default(),
prometheus_config: None,
telemetry_endpoints: None,
default_heap_pages: None,
offchain_worker: OffchainWorkerConfig { enabled: true, indexing_enabled: false },
force_authoring: false,
disable_grandpa: false,
dev_key_seed: Some(Sr25519Keyring::Alice.to_seed()),
tracing_targets: None,
tracing_receiver: Default::default(),
max_runtime_instances: 8,
runtime_cache_size: 2,
announce_block: true,
data_path: base_path.path().into(),
base_path,
Expand Down
Loading
Loading