From 7f32fdf9453712f6db204bda5e0d8dc3596a5a72 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Mon, 9 Sep 2024 09:25:30 +0200 Subject: [PATCH 01/28] rpc v2: rely backpressure `Storage::query_iter` --- substrate/client/rpc-servers/src/lib.rs | 2 +- substrate/client/rpc-spec-v2/Cargo.toml | 2 +- .../client/rpc-spec-v2/src/archive/archive.rs | 56 +----- .../src/archive/archive_storage.rs | 80 +++----- .../client/rpc-spec-v2/src/archive/mod.rs | 2 +- .../client/rpc-spec-v2/src/archive/tests.rs | 58 +++--- .../rpc-spec-v2/src/chain_head/chain_head.rs | 74 ++----- .../src/chain_head/chain_head_storage.rs | 188 +++++++++--------- .../client/rpc-spec-v2/src/chain_head/mod.rs | 7 + .../src/chain_head/subscription/inner.rs | 173 ++++++---------- .../src/chain_head/subscription/mod.rs | 2 +- .../rpc-spec-v2/src/chain_head/test_utils.rs | 3 +- .../rpc-spec-v2/src/chain_head/tests.rs | 103 ++-------- .../client/rpc-spec-v2/src/common/storage.rs | 135 ++++++++----- substrate/client/service/src/builder.rs | 3 +- .../primitives/state-machine/src/backend.rs | 2 +- 16 files changed, 353 insertions(+), 537 deletions(-) diff --git a/substrate/client/rpc-servers/src/lib.rs b/substrate/client/rpc-servers/src/lib.rs index ca74c2371c25..95476ea76be3 100644 --- a/substrate/client/rpc-servers/src/lib.rs +++ b/substrate/client/rpc-servers/src/lib.rs @@ -225,7 +225,7 @@ where }; let rpc_middleware = - RpcServiceBuilder::new().option_layer(middleware_layer.clone()); + RpcServiceBuilder::new().rpc_logger(1024).option_layer(middleware_layer.clone()); let mut svc = service_builder .set_rpc_middleware(rpc_middleware) .build(methods, stop_handle); diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index ae21895de38d..5537ac045d77 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -28,7 +28,6 @@ sp-rpc = { workspace = true, default-features = true } sp-blockchain = { workspace = true, default-features = true } sp-version = { workspace = true, default-features = true } sc-client-api = { workspace = true, default-features = true } -sc-utils = { workspace = true, default-features = true } sc-rpc = { workspace = true, default-features = true } codec = { workspace = true, default-features = true } thiserror = { workspace = true } @@ -59,3 +58,4 @@ sc-service = { features = ["test-helpers"], workspace = true, default-features = assert_matches = { workspace = true } pretty_assertions = { workspace = true } sc-transaction-pool = { workspace = true, default-features = true } +sc-utils = { workspace = true, default-features = true } diff --git a/substrate/client/rpc-spec-v2/src/archive/archive.rs b/substrate/client/rpc-spec-v2/src/archive/archive.rs index 82c6b2cacc2f..745aced70fd3 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive.rs @@ -30,6 +30,7 @@ use sc_client_api::{ Backend, BlockBackend, BlockchainEvents, CallExecutor, ChildInfo, ExecutorProvider, StorageKey, StorageProvider, }; +use sc_rpc::SubscriptionTaskExecutor; use sp_api::{CallApiAt, CallContext}; use sp_blockchain::{ Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, @@ -43,36 +44,6 @@ use std::{collections::HashSet, marker::PhantomData, sync::Arc}; use super::archive_storage::ArchiveStorage; -/// The configuration of [`Archive`]. -pub struct ArchiveConfig { - /// The maximum number of items the `archive_storage` can return for a descendant query before - /// pagination is required. - pub max_descendant_responses: usize, - /// The maximum number of queried items allowed for the `archive_storage` at a time. - pub max_queried_items: usize, -} - -/// The maximum number of items the `archive_storage` can return for a descendant query before -/// pagination is required. -/// -/// Note: this is identical to the `chainHead` value. -const MAX_DESCENDANT_RESPONSES: usize = 5; - -/// The maximum number of queried items allowed for the `archive_storage` at a time. -/// -/// Note: A queried item can also be a descendant query which can return up to -/// `MAX_DESCENDANT_RESPONSES`. -const MAX_QUERIED_ITEMS: usize = 8; - -impl Default for ArchiveConfig { - fn default() -> Self { - Self { - max_descendant_responses: MAX_DESCENDANT_RESPONSES, - max_queried_items: MAX_QUERIED_ITEMS, - } - } -} - /// An API for archive RPC calls. pub struct Archive, Block: BlockT, Client> { /// Substrate client. @@ -81,13 +52,10 @@ pub struct Archive, Block: BlockT, Client> { backend: Arc, /// The hexadecimal encoded hash of the genesis block. genesis_hash: String, - /// The maximum number of items the `archive_storage` can return for a descendant query before - /// pagination is required. - storage_max_descendant_responses: usize, - /// The maximum number of queried items allowed for the `archive_storage` at a time. - storage_max_queried_items: usize, /// Phantom member to pin the block type. _phantom: PhantomData, + /// Subscription task executor. + executor: SubscriptionTaskExecutor, } impl, Block: BlockT, Client> Archive { @@ -96,17 +64,10 @@ impl, Block: BlockT, Client> Archive { client: Arc, backend: Arc, genesis_hash: GenesisHash, - config: ArchiveConfig, + executor: SubscriptionTaskExecutor, ) -> Self { let genesis_hash = hex_string(&genesis_hash.as_ref()); - Self { - client, - backend, - genesis_hash, - storage_max_descendant_responses: config.max_descendant_responses, - storage_max_queried_items: config.max_queried_items, - _phantom: PhantomData, - } + Self { client, backend, genesis_hash, _phantom: PhantomData, executor } } } @@ -270,11 +231,8 @@ where .transpose()? .map(ChildInfo::new_default_from_vec); - let storage_client = ArchiveStorage::new( - self.client.clone(), - self.storage_max_descendant_responses, - self.storage_max_queried_items, - ); + let storage_client = ArchiveStorage::new(self.client.clone(), self.executor.clone()); + Ok(storage_client.handle_query(hash, items, child_trie)) } } diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index 26e7c299de41..7e222fba2967 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -21,6 +21,7 @@ use std::sync::Arc; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; +use sc_rpc::SubscriptionTaskExecutor; use sp_runtime::traits::Block as BlockT; use crate::common::{ @@ -32,44 +33,31 @@ use crate::common::{ pub struct ArchiveStorage { /// Storage client. client: Storage, - /// The maximum number of responses the API can return for a descendant query at a time. - storage_max_descendant_responses: usize, - /// The maximum number of queried items allowed for the `archive_storage` at a time. - storage_max_queried_items: usize, } impl ArchiveStorage { /// Constructs a new [`ArchiveStorage`]. - pub fn new( - client: Arc, - storage_max_descendant_responses: usize, - storage_max_queried_items: usize, - ) -> Self { - Self { - client: Storage::new(client), - storage_max_descendant_responses, - storage_max_queried_items, - } + pub fn new(client: Arc, executor: SubscriptionTaskExecutor) -> Self { + Self { client: Storage::new(client, executor) } } } impl ArchiveStorage where - Block: BlockT + 'static, - BE: Backend + 'static, - Client: StorageProvider + 'static, + Block: BlockT + Send + 'static, + BE: Backend + Send + 'static, + Client: StorageProvider + Send + Sync + 'static, { /// Generate the response of the `archive_storage` method. pub fn handle_query( &self, hash: Block::Hash, - mut items: Vec>, + items: Vec>, child_key: Option, ) -> ArchiveStorageResult { - let discarded_items = items.len().saturating_sub(self.storage_max_queried_items); - items.truncate(self.storage_max_queried_items); - let mut storage_results = Vec::with_capacity(items.len()); + let mut query_iter = Vec::new(); + for item in items { match item.query_type { StorageQueryType::Value => { @@ -92,38 +80,34 @@ where Err(error) => return ArchiveStorageResult::err(error), }, StorageQueryType::DescendantsValues => { - match self.client.query_iter_pagination( - QueryIter { - query_key: item.key, - ty: IterQueryType::Value, - pagination_start_key: item.pagination_start_key, - }, - hash, - child_key.as_ref(), - self.storage_max_descendant_responses, - ) { - Ok((results, _)) => storage_results.extend(results), - Err(error) => return ArchiveStorageResult::err(error), - } + query_iter.push(QueryIter { + query_key: item.key, + ty: IterQueryType::Value, + pagination_start_key: item.pagination_start_key, + }); }, StorageQueryType::DescendantsHashes => { - match self.client.query_iter_pagination( - QueryIter { - query_key: item.key, - ty: IterQueryType::Hash, - pagination_start_key: item.pagination_start_key, - }, - hash, - child_key.as_ref(), - self.storage_max_descendant_responses, - ) { - Ok((results, _)) => storage_results.extend(results), - Err(error) => return ArchiveStorageResult::err(error), - } + query_iter.push(QueryIter { + query_key: item.key, + ty: IterQueryType::Hash, + pagination_start_key: item.pagination_start_key, + }); }, }; } - ArchiveStorageResult::ok(storage_results, discarded_items) + if !query_iter.is_empty() { + let mut rx = self.client.query_iter_pagination(query_iter, hash, child_key); + + while let Some(val) = rx.blocking_recv() { + match val { + Ok(Some(value)) => storage_results.push(value), + Ok(None) => continue, + Err(error) => return ArchiveStorageResult::err(error), + } + } + } + + ArchiveStorageResult::ok(storage_results, 0) } } diff --git a/substrate/client/rpc-spec-v2/src/archive/mod.rs b/substrate/client/rpc-spec-v2/src/archive/mod.rs index 5f020c203eab..14fa104c113a 100644 --- a/substrate/client/rpc-spec-v2/src/archive/mod.rs +++ b/substrate/client/rpc-spec-v2/src/archive/mod.rs @@ -32,4 +32,4 @@ pub mod archive; pub mod error; pub use api::ArchiveApiServer; -pub use archive::{Archive, ArchiveConfig}; +pub use archive::Archive; diff --git a/substrate/client/rpc-spec-v2/src/archive/tests.rs b/substrate/client/rpc-spec-v2/src/archive/tests.rs index 078016f5b3e2..460aa4916695 100644 --- a/substrate/client/rpc-spec-v2/src/archive/tests.rs +++ b/substrate/client/rpc-spec-v2/src/archive/tests.rs @@ -24,10 +24,7 @@ use crate::{ hex_string, MethodResult, }; -use super::{ - archive::{Archive, ArchiveConfig}, - *, -}; +use super::{archive::Archive, *}; use assert_matches::assert_matches; use codec::{Decode, Encode}; @@ -38,7 +35,7 @@ use sc_block_builder::BlockBuilderBuilder; use sc_client_api::ChildInfo; use sp_blockchain::HeaderBackend; use sp_consensus::BlockOrigin; -use sp_core::{Blake2Hasher, Hasher}; +use sp_core::{testing::TaskExecutor, Blake2Hasher, Hasher}; use sp_runtime::{ traits::{Block as BlockT, Header as HeaderT}, SaturatedConversion, @@ -51,8 +48,6 @@ use substrate_test_runtime_client::{ const CHAIN_GENESIS: [u8; 32] = [0; 32]; const INVALID_HASH: [u8; 32] = [1; 32]; -const MAX_PAGINATION_LIMIT: usize = 5; -const MAX_QUERIED_LIMIT: usize = 5; const KEY: &[u8] = b":mock"; const VALUE: &[u8] = b"hello world"; const CHILD_STORAGE_KEY: &[u8] = b"child"; @@ -61,10 +56,7 @@ const CHILD_VALUE: &[u8] = b"child value"; type Header = substrate_test_runtime_client::runtime::Header; type Block = substrate_test_runtime_client::runtime::Block; -fn setup_api( - max_descendant_responses: usize, - max_queried_items: usize, -) -> (Arc>, RpcModule>>) { +fn setup_api() -> (Arc>, RpcModule>>) { let child_info = ChildInfo::new_default(CHILD_STORAGE_KEY); let builder = TestClientBuilder::new().add_extra_child_storage( &child_info, @@ -74,20 +66,16 @@ fn setup_api( let backend = builder.backend(); let client = Arc::new(builder.build()); - let api = Archive::new( - client.clone(), - backend, - CHAIN_GENESIS, - ArchiveConfig { max_descendant_responses, max_queried_items }, - ) - .into_rpc(); + let api = + Archive::new(client.clone(), backend, CHAIN_GENESIS, Arc::new(TaskExecutor::default())) + .into_rpc(); (client, api) } #[tokio::test] async fn archive_genesis() { - let (_client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (_client, api) = setup_api(); let genesis: String = api.call("archive_unstable_genesisHash", EmptyParams::new()).await.unwrap(); @@ -96,7 +84,7 @@ async fn archive_genesis() { #[tokio::test] async fn archive_body() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); // Invalid block hash. let invalid_hash = hex_string(&INVALID_HASH); @@ -130,7 +118,7 @@ async fn archive_body() { #[tokio::test] async fn archive_header() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); // Invalid block hash. let invalid_hash = hex_string(&INVALID_HASH); @@ -164,7 +152,7 @@ async fn archive_header() { #[tokio::test] async fn archive_finalized_height() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); let client_height: u32 = client.info().finalized_number.saturated_into(); @@ -176,7 +164,7 @@ async fn archive_finalized_height() { #[tokio::test] async fn archive_hash_by_height() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); // Genesis height. let hashes: Vec = api.call("archive_unstable_hashByHeight", [0]).await.unwrap(); @@ -282,7 +270,7 @@ async fn archive_hash_by_height() { #[tokio::test] async fn archive_call() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); let invalid_hash = hex_string(&INVALID_HASH); // Invalid parameter (non-hex). @@ -341,7 +329,7 @@ async fn archive_call() { #[tokio::test] async fn archive_storage_hashes_values() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); let block = BlockBuilderBuilder::new(&*client) .on_parent_block(client.chain_info().genesis_hash) @@ -431,7 +419,7 @@ async fn archive_storage_hashes_values() { #[tokio::test] async fn archive_storage_closest_merkle_value() { - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); /// The core of this test. /// @@ -592,7 +580,7 @@ async fn archive_storage_closest_merkle_value() { #[tokio::test] async fn archive_storage_paginate_iterations() { // 1 iteration allowed before pagination kicks in. - let (client, api) = setup_api(1, MAX_QUERIED_LIMIT); + let (client, api) = setup_api(); // Import a new block with storage changes. let mut builder = BlockBuilderBuilder::new(&*client) @@ -647,7 +635,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 1); + assert_eq!(result.len(), 5); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":m")); @@ -673,7 +661,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 1); + assert_eq!(result.len(), 4); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":mo")); @@ -699,7 +687,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 1); + assert_eq!(result.len(), 3); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":moD")); @@ -725,7 +713,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 1); + assert_eq!(result.len(), 2); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":moc")); @@ -785,9 +773,9 @@ async fn archive_storage_paginate_iterations() { } #[tokio::test] -async fn archive_storage_discarded_items() { +async fn archive_storage_is_backpressured() { // One query at a time - let (client, api) = setup_api(MAX_PAGINATION_LIMIT, 1); + let (client, api) = setup_api(); // Import a new block with storage changes. let mut builder = BlockBuilderBuilder::new(&*client) @@ -829,8 +817,8 @@ async fn archive_storage_discarded_items() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 1); - assert_eq!(discarded_items, 2); + assert_eq!(result.len(), 3); + assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":m")); assert_eq!(result[0].result, StorageResultType::Value(hex_string(b"a"))); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index 1bc5cecb205b..6f94c62e449c 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -34,7 +34,7 @@ use crate::{ hex_string, SubscriptionTaskExecutor, }; use codec::Encode; -use futures::{channel::oneshot, future::FutureExt}; +use futures::{channel::oneshot, future::FutureExt, SinkExt}; use jsonrpsee::{ core::async_trait, server::ResponsePayload, types::SubscriptionId, ConnectionId, Extensions, MethodResponseFuture, PendingSubscriptionSink, @@ -65,9 +65,6 @@ pub struct ChainHeadConfig { /// Stop all subscriptions if the distance between the leaves and the current finalized /// block is larger than this value. pub max_lagging_distance: usize, - /// The maximum number of items reported by the `chainHead_storage` before - /// pagination is required. - pub operation_max_storage_items: usize, /// The maximum number of `chainHead_follow` subscriptions per connection. pub max_follow_subscriptions_per_connection: usize, } @@ -87,10 +84,6 @@ const MAX_PINNED_DURATION: Duration = Duration::from_secs(60); /// Note: The lower limit imposed by the spec is 16. const MAX_ONGOING_OPERATIONS: usize = 16; -/// The maximum number of items the `chainHead_storage` can return -/// before paginations is required. -const MAX_STORAGE_ITER_ITEMS: usize = 5; - /// Stop all subscriptions if the distance between the leaves and the current finalized /// block is larger than this value. const MAX_LAGGING_DISTANCE: usize = 128; @@ -105,7 +98,6 @@ impl Default for ChainHeadConfig { subscription_max_pinned_duration: MAX_PINNED_DURATION, subscription_max_ongoing_operations: MAX_ONGOING_OPERATIONS, max_lagging_distance: MAX_LAGGING_DISTANCE, - operation_max_storage_items: MAX_STORAGE_ITER_ITEMS, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, } } @@ -121,9 +113,6 @@ pub struct ChainHead, Block: BlockT, Client> { executor: SubscriptionTaskExecutor, /// Keep track of the pinned blocks for each subscription. subscriptions: SubscriptionManagement, - /// The maximum number of items reported by the `chainHead_storage` before - /// pagination is required. - operation_max_storage_items: usize, /// Stop all subscriptions if the distance between the leaves and the current finalized /// block is larger than this value. max_lagging_distance: usize, @@ -150,7 +139,6 @@ impl, Block: BlockT, Client> ChainHead { config.max_follow_subscriptions_per_connection, backend, ), - operation_max_storage_items: config.operation_max_storage_items, max_lagging_distance: config.max_lagging_distance, _phantom: PhantomData, } @@ -314,7 +302,7 @@ where }), }; - let (rp, rp_fut) = method_started_response(operation_id, None); + let (rp, rp_fut) = method_started_response(operation_id); let fut = async move { // Wait for the server to send out the response and if it produces an error no event // should be generated. @@ -322,7 +310,7 @@ where return; } - let _ = block_guard.response_sender().unbounded_send(event); + let _ = block_guard.response_sender().send(event).await; }; executor.spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed()); @@ -426,20 +414,12 @@ where Err(_) => return ResponsePayload::error(ChainHeadRpcError::InvalidBlock), }; - let mut storage_client = ChainHeadStorage::::new( - self.client.clone(), - self.operation_max_storage_items, - ); + let mut storage_client = + ChainHeadStorage::::new(self.client.clone(), self.executor.clone()); let operation = block_guard.operation(); let operation_id = operation.operation_id(); - // The number of operations we are allowed to execute. - let num_operations = operation.num_reserved(); - let discarded = items.len().saturating_sub(num_operations); - let mut items = items; - items.truncate(num_operations); - - let (rp, rp_fut) = method_started_response(operation_id, Some(discarded)); + let (rp, rp_fut) = method_started_response(operation_id); let fut = async move { // Wait for the server to send out the response and if it produces an error no event // should be generated. @@ -447,7 +427,7 @@ where return; } - storage_client.generate_events(block_guard, hash, items, child_trie).await; + _ = storage_client.generate_events(block_guard, hash, items, child_trie).await; }; self.executor .spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed()); @@ -503,7 +483,7 @@ where let operation_id = block_guard.operation().operation_id(); let client = self.client.clone(); - let (rp, rp_fut) = method_started_response(operation_id.clone(), None); + let (rp, rp_fut) = method_started_response(operation_id.clone()); let fut = async move { // Wait for the server to send out the response and if it produces an error no event // should be generated. @@ -527,7 +507,7 @@ where }) }); - let _ = block_guard.response_sender().unbounded_send(event); + let _ = block_guard.response_sender().send(event).await; }; self.executor .spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed()); @@ -575,30 +555,12 @@ where async fn chain_head_unstable_continue( &self, - ext: &Extensions, - follow_subscription: String, - operation_id: String, + _ext: &Extensions, + _follow_subscription: String, + _operation_id: String, ) -> Result<(), ChainHeadRpcError> { - let conn_id = ext - .get::() - .copied() - .expect("ConnectionId is always set by jsonrpsee; qed"); - - if !self.subscriptions.contains_subscription(conn_id, &follow_subscription) { - return Ok(()) - } - - let Some(operation) = self.subscriptions.get_operation(&follow_subscription, &operation_id) - else { - return Ok(()) - }; - - if !operation.submit_continue() { - // Continue called without generating a `WaitingForContinue` event. - Err(ChainHeadRpcError::InvalidContinue.into()) - } else { - Ok(()) - } + // `WaitingForContinue`` is never emitted by the server. + Ok(()) } async fn chain_head_unstable_stop_operation( @@ -616,12 +578,13 @@ where return Ok(()) } - let Some(operation) = self.subscriptions.get_operation(&follow_subscription, &operation_id) + let Some(mut operation) = + self.subscriptions.get_operation(&follow_subscription, &operation_id) else { return Ok(()) }; - operation.stop_operation(); + operation.stop(); Ok(()) } @@ -629,9 +592,8 @@ where fn method_started_response( operation_id: String, - discarded_items: Option, ) -> (ResponsePayload<'static, MethodResponse>, MethodResponseFuture) { - let rp = MethodResponse::Started(MethodResponseStarted { operation_id, discarded_items }); + let rp = MethodResponse::Started(MethodResponseStarted { operation_id, discarded_items: None }); ResponsePayload::success(rp).notify_on_completion() } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index ee39ec253a30..35f2331ce2fd 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -18,21 +18,24 @@ //! Implementation of the `chainHead_storage` method. -use std::{collections::VecDeque, marker::PhantomData, sync::Arc}; +use std::{marker::PhantomData, sync::Arc}; +use futures::SinkExt; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; -use sc_utils::mpsc::TracingUnboundedSender; +use sc_rpc::SubscriptionTaskExecutor; use sp_runtime::traits::Block as BlockT; +use tokio::sync::mpsc; use crate::{ chain_head::{ + chain_head::LOG_TARGET, event::{OperationError, OperationId, OperationStorageItems}, - subscription::BlockGuard, - FollowEvent, + subscription::{BlockGuard, StopHandle}, + FollowEvent, FollowEventSendError, FollowEventSender, }, common::{ events::{StorageQuery, StorageQueryType}, - storage::{IterQueryType, QueryIter, QueryIterResult, Storage}, + storage::{IterQueryType, QueryIter, QueryResult, Storage}, }, }; @@ -40,92 +43,57 @@ use crate::{ pub struct ChainHeadStorage { /// Storage client. client: Storage, - /// Queue of operations that may require pagination. - iter_operations: VecDeque, - /// The maximum number of items reported by the `chainHead_storage` before - /// pagination is required. - operation_max_storage_items: usize, _phandom: PhantomData<(BE, Block)>, } impl ChainHeadStorage { /// Constructs a new [`ChainHeadStorage`]. - pub fn new(client: Arc, operation_max_storage_items: usize) -> Self { - Self { - client: Storage::new(client), - iter_operations: VecDeque::new(), - operation_max_storage_items, - _phandom: PhantomData, - } + pub fn new(client: Arc, executor: SubscriptionTaskExecutor) -> Self { + Self { client: Storage::new(client, executor), _phandom: PhantomData } } } impl ChainHeadStorage where - Block: BlockT + 'static, - BE: Backend + 'static, - Client: StorageProvider + 'static, + Block: BlockT + Send + 'static, + BE: Backend + Send + 'static, + Client: StorageProvider + Send + Sync + 'static, { /// Iterate over (key, hash) and (key, value) generating the `WaitingForContinue` event if /// necessary. async fn generate_storage_iter_events( - &mut self, + &self, + query_iter: Vec, mut block_guard: BlockGuard, hash: Block::Hash, child_key: Option, - ) { - let sender = block_guard.response_sender(); - let operation = block_guard.operation(); + ) -> Result<(), FollowEventSendError> { + let stop_handle = block_guard.operation().stop_handle().clone(); - while let Some(query) = self.iter_operations.pop_front() { - if operation.was_stopped() { - return - } - - let result = self.client.query_iter_pagination( - query, - hash, - child_key.as_ref(), - self.operation_max_storage_items, - ); - let (events, maybe_next_query) = match result { - QueryIterResult::Ok(result) => result, - QueryIterResult::Err(error) => { - send_error::(&sender, operation.operation_id(), error.to_string()); - return - }, - }; + if stop_handle.is_stopped() { + return Ok(()); + } - if !events.is_empty() { - // Send back the results of the iteration produced so far. - let _ = sender.unbounded_send(FollowEvent::::OperationStorageItems( - OperationStorageItems { operation_id: operation.operation_id(), items: events }, - )); - } - - if let Some(next_query) = maybe_next_query { - let _ = - sender.unbounded_send(FollowEvent::::OperationWaitingForContinue( - OperationId { operation_id: operation.operation_id() }, - )); - - // The operation might be continued or cancelled only after the - // `OperationWaitingForContinue` is generated above. - operation.wait_for_continue().await; - - // Give a chance for the other items to advance next time. - self.iter_operations.push_back(next_query); - } + if !query_iter.is_empty() { + process_storage_iter_stream( + self.client.query_iter_pagination(query_iter, hash, child_key), + block_guard.response_sender(), + block_guard.operation().operation_id().to_owned(), + &stop_handle, + ) + .await?; } - if operation.was_stopped() { - return + if !stop_handle.is_stopped() { + block_guard + .response_sender() + .send(FollowEvent::OperationStorageDone(OperationId { + operation_id: block_guard.operation().operation_id().to_owned(), + })) + .await?; } - let _ = - sender.unbounded_send(FollowEvent::::OperationStorageDone(OperationId { - operation_id: operation.operation_id(), - })); + Ok(()) } /// Generate the block events for the `chainHead_storage` method. @@ -135,8 +103,11 @@ where hash: Block::Hash, items: Vec>, child_key: Option, - ) { - let sender = block_guard.response_sender(); + ) -> Result<(), FollowEventSendError> { + log::info!(target: LOG_TARGET, "generate_events={:?}", items.len()); + + let mut iter_ops = Vec::new(); + let mut sender = block_guard.response_sender(); let operation = block_guard.operation(); let mut storage_results = Vec::with_capacity(items.len()); @@ -147,8 +118,7 @@ where Ok(Some(value)) => storage_results.push(value), Ok(None) => continue, Err(error) => { - send_error::(&sender, operation.operation_id(), error); - return + return send_error(&mut sender, operation.operation_id(), error).await; }, } }, @@ -157,8 +127,7 @@ where Ok(Some(value)) => storage_results.push(value), Ok(None) => continue, Err(error) => { - send_error::(&sender, operation.operation_id(), error); - return + return send_error(&mut sender, operation.operation_id(), error).await; }, }, StorageQueryType::ClosestDescendantMerkleValue => @@ -166,16 +135,15 @@ where Ok(Some(value)) => storage_results.push(value), Ok(None) => continue, Err(error) => { - send_error::(&sender, operation.operation_id(), error); - return + return send_error(&mut sender, operation.operation_id(), error).await; }, }, - StorageQueryType::DescendantsValues => self.iter_operations.push_back(QueryIter { + StorageQueryType::DescendantsValues => iter_ops.push(QueryIter { query_key: item.key, ty: IterQueryType::Value, pagination_start_key: None, }), - StorageQueryType::DescendantsHashes => self.iter_operations.push_back(QueryIter { + StorageQueryType::DescendantsHashes => iter_ops.push(QueryIter { query_key: item.key, ty: IterQueryType::Hash, pagination_start_key: None, @@ -184,26 +152,66 @@ where } if !storage_results.is_empty() { - let _ = sender.unbounded_send(FollowEvent::::OperationStorageItems( - OperationStorageItems { + sender + .send(FollowEvent::::OperationStorageItems(OperationStorageItems { operation_id: operation.operation_id(), items: storage_results, - }, - )); + })) + .await?; } - self.generate_storage_iter_events(block_guard, hash, child_key).await + self.generate_storage_iter_events(iter_ops, block_guard, hash, child_key).await } } /// Build and send the opaque error back to the `chainHead_follow` method. -fn send_error( - sender: &TracingUnboundedSender>, +async fn send_error( + sender: &mut FollowEventSender, operation_id: String, error: String, -) { - let _ = sender.unbounded_send(FollowEvent::::OperationError(OperationError { - operation_id, - error, - })); +) -> Result<(), FollowEventSendError> { + sender + .send(FollowEvent::OperationError(OperationError { operation_id, error })) + .await +} + +async fn process_storage_iter_stream( + mut storage_query_stream: mpsc::Receiver, + mut sender: FollowEventSender, + operation_id: String, + stop_handle: &StopHandle, +) -> Result<(), FollowEventSendError> { + let mut buf = Vec::new(); + + loop { + tokio::select! { + _ = stop_handle.stopped() => return Ok(()), + len = storage_query_stream.recv_many(&mut buf, 1024) => { + if len == 0 { + break Ok(()); + } + + let mut items = Vec::with_capacity(buf.len()); + + for val in buf.drain(..) { + match val { + QueryResult::Err(error) => + return send_error(&mut sender, operation_id.clone(), error).await, + QueryResult::Ok(Some(v)) => { + items.push(v) + }, + QueryResult::Ok(None) => continue + } + } + + // Send back the results of the iteration produced so far. + sender + .send(FollowEvent::OperationStorageItems(OperationStorageItems { + operation_id: operation_id.clone(), + items, + })).await?; + + }, + } + } } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/mod.rs b/substrate/client/rpc-spec-v2/src/chain_head/mod.rs index c9fe19aca2b1..84b4055fa422 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/mod.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/mod.rs @@ -42,3 +42,10 @@ pub use event::{ BestBlockChanged, ErrorEvent, Finalized, FollowEvent, Initialized, NewBlock, RuntimeEvent, RuntimeVersionEvent, }; + +/// Follow event sender. +pub(crate) type FollowEventSender = futures::channel::mpsc::Sender>; +/// Follow event receiver. +pub(crate) type FollowEventReceiver = futures::channel::mpsc::Receiver>; +/// Follow event send error. +pub(crate) type FollowEventSendError = futures::channel::mpsc::SendError; \ No newline at end of file diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs index 14325b4fbb98..384ea02791e9 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs @@ -19,18 +19,19 @@ use futures::channel::oneshot; use parking_lot::Mutex; use sc_client_api::Backend; -use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use sp_runtime::traits::Block as BlockT; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, - sync::{atomic::AtomicBool, Arc}, + sync::Arc, time::{Duration, Instant}, }; -use crate::chain_head::{subscription::SubscriptionManagementError, FollowEvent}; +use crate::chain_head::{ + subscription::SubscriptionManagementError, FollowEventReceiver, FollowEventSender, +}; -/// The queue size after which the `sc_utils::mpsc::tracing_unbounded` would produce warnings. -const QUEUE_SIZE_WARNING: usize = 512; +type NotifyOnDrop = tokio::sync::mpsc::Receiver<()>; +type SharedOperations = Arc>>; /// The state machine of a block of a single subscription ID. /// @@ -155,72 +156,42 @@ struct PermitOperations { _permit: tokio::sync::OwnedSemaphorePermit, } -/// The state of one operation. -/// -/// This is directly exposed to users via `chain_head_unstable_continue` and -/// `chain_head_unstable_stop_operation`. +/// Stop handle for the operation. #[derive(Clone)] -pub struct OperationState { - /// The shared operation state that holds information about the - /// `waitingForContinue` event and cancellation. - shared_state: Arc, - /// Send notifications when the user calls `chainHead_continue` method. - send_continue: tokio::sync::mpsc::Sender<()>, -} - -impl OperationState { - /// Returns true if `chainHead_continue` is called after the - /// `waitingForContinue` event was emitted for the associated - /// operation ID. - pub fn submit_continue(&self) -> bool { - // `waitingForContinue` not generated. - if !self.shared_state.requested_continue.load(std::sync::atomic::Ordering::Acquire) { - return false - } +pub struct StopHandle(tokio::sync::mpsc::Sender<()>); - // Has enough capacity for 1 message. - // Can fail if the `stop_operation` propagated the stop first. - self.send_continue.try_send(()).is_ok() +impl StopHandle { + pub async fn stopped(&self) { + self.0.closed().await; } - /// Stops the operation if `waitingForContinue` event was emitted for the associated - /// operation ID. - /// - /// Returns nothing in accordance with `chainHead_v1_stopOperation`. - pub fn stop_operation(&self) { - // `waitingForContinue` not generated. - if !self.shared_state.requested_continue.load(std::sync::atomic::Ordering::Acquire) { - return - } - - self.shared_state - .operation_stopped - .store(true, std::sync::atomic::Ordering::Release); - - // Send might not have enough capacity if `submit_continue` was sent first. - // However, the `operation_stopped` boolean was set. - let _ = self.send_continue.try_send(()); + pub fn is_stopped(&self) -> bool { + self.0.is_closed() } } /// The shared operation state between the backend [`RegisteredOperation`] and frontend /// [`RegisteredOperation`]. -struct SharedOperationState { - /// True if the `chainHead` generated `waitingForContinue` event. - requested_continue: AtomicBool, - /// True if the operation was cancelled by the user. - operation_stopped: AtomicBool, +#[derive(Clone)] +pub struct OperationState { + stop: StopHandle, + operations: SharedOperations, + operation_id: String, } -impl SharedOperationState { - /// Constructs a new [`SharedOperationState`]. - /// - /// This is efficiently cloned under a single heap allocation. - fn new() -> Arc { - Arc::new(SharedOperationState { - requested_continue: AtomicBool::new(false), - operation_stopped: AtomicBool::new(false), - }) +impl OperationState { + pub fn stop(&mut self) { + if !self.stop.is_stopped() { + self.operations.lock().remove(&self.operation_id); + } + } + + pub fn is_stopped(&self) -> bool { + self.stop.is_stopped() + } + + pub async fn stopped(&self) { + self.stop.stopped().await; } } @@ -228,40 +199,20 @@ impl SharedOperationState { /// /// This is used internally by the `chainHead` methods. pub struct RegisteredOperation { - /// The shared operation state that holds information about the - /// `waitingForContinue` event and cancellation. - shared_state: Arc, - /// Receive notifications when the user calls `chainHead_continue` method. - recv_continue: tokio::sync::mpsc::Receiver<()>, + /// Stop handle for the operation. + stop_handle: StopHandle, + /// Track the operations ID of this subscription. + operations: SharedOperations, /// The operation ID of the request. operation_id: String, - /// Track the operations ID of this subscription. - operations: Arc>>, /// Permit a number of items to be executed by this operation. permit: PermitOperations, } impl RegisteredOperation { - /// Wait until the user calls `chainHead_continue` or the operation - /// is cancelled via `chainHead_stopOperation`. - pub async fn wait_for_continue(&mut self) { - self.shared_state - .requested_continue - .store(true, std::sync::atomic::Ordering::Release); - - // The sender part of this channel is around for as long as this object exists, - // because it is stored in the `OperationState` of the `operations` field. - // The sender part is removed from tracking when this object is dropped. - let _ = self.recv_continue.recv().await; - - self.shared_state - .requested_continue - .store(false, std::sync::atomic::Ordering::Release); - } - - /// Returns true if the current operation was stopped. - pub fn was_stopped(&self) -> bool { - self.shared_state.operation_stopped.load(std::sync::atomic::Ordering::Acquire) + /// Stop handle for the operation. + pub fn stop_handle(&self) -> &StopHandle { + &self.stop_handle } /// Get the operation ID. @@ -279,8 +230,7 @@ impl RegisteredOperation { impl Drop for RegisteredOperation { fn drop(&mut self) { - let mut operations = self.operations.lock(); - operations.remove(&self.operation_id); + self.operations.lock().remove(&self.operation_id); } } @@ -291,7 +241,7 @@ struct Operations { /// Limit the number of ongoing operations. limits: LimitOperations, /// Track the operations ID of this subscription. - operations: Arc>>, + operations: SharedOperations, } impl Operations { @@ -307,25 +257,25 @@ impl Operations { /// Register a new operation. pub fn register_operation(&mut self, to_reserve: usize) -> Option { let permit = self.limits.reserve_at_most(to_reserve)?; - let operation_id = self.next_operation_id(); - // At most one message can be sent. - let (send_continue, recv_continue) = tokio::sync::mpsc::channel(1); - let shared_state = SharedOperationState::new(); - - let state = OperationState { send_continue, shared_state: shared_state.clone() }; - - // Cloned operations for removing the current ID on drop. + let (tx, rx) = tokio::sync::mpsc::channel(1); + let stop_handle = StopHandle(tx); let operations = self.operations.clone(); - operations.lock().insert(operation_id.clone(), state); + operations.lock().insert(operation_id.clone(), (rx, stop_handle.clone())); - Some(RegisteredOperation { shared_state, operation_id, recv_continue, operations, permit }) + Some(RegisteredOperation { stop_handle, operation_id, operations, permit }) } /// Get the associated operation state with the ID. pub fn get_operation(&self, id: &str) -> Option { - self.operations.lock().get(id).map(|state| state.clone()) + let stop = self.operations.lock().get(id).map(|(_, stop)| stop.clone())?; + + Some(OperationState { + stop, + operations: self.operations.clone(), + operation_id: id.to_string(), + }) } /// Generate the next operation ID for this subscription. @@ -352,7 +302,7 @@ struct SubscriptionState { /// The sender of message responses to the `chainHead_follow` events. /// /// This object is cloned between methods. - response_sender: TracingUnboundedSender>, + response_sender: FollowEventSender, /// The ongoing operations of a subscription. operations: Operations, /// Track the block hashes available for this subscription. @@ -486,7 +436,7 @@ impl SubscriptionState { pub struct BlockGuard> { hash: Block::Hash, with_runtime: bool, - response_sender: TracingUnboundedSender>, + response_sender: FollowEventSender, operation: RegisteredOperation, backend: Arc, } @@ -504,7 +454,7 @@ impl> BlockGuard { fn new( hash: Block::Hash, with_runtime: bool, - response_sender: TracingUnboundedSender>, + response_sender: FollowEventSender, operation: RegisteredOperation, backend: Arc, ) -> Result { @@ -521,7 +471,7 @@ impl> BlockGuard { } /// Send message responses from the `chainHead` methods to `chainHead_follow`. - pub fn response_sender(&self) -> TracingUnboundedSender> { + pub fn response_sender(&self) -> FollowEventSender { self.response_sender.clone() } @@ -543,7 +493,7 @@ pub struct InsertedSubscriptionData { /// Signal that the subscription must stop. pub rx_stop: oneshot::Receiver<()>, /// Receive message responses from the `chainHead` methods. - pub response_receiver: TracingUnboundedReceiver>, + pub response_receiver: FollowEventReceiver, } pub struct SubscriptionsInner> { @@ -593,8 +543,9 @@ impl> SubscriptionsInner { ) -> Option> { if let Entry::Vacant(entry) = self.subs.entry(sub_id) { let (tx_stop, rx_stop) = oneshot::channel(); - let (response_sender, response_receiver) = - tracing_unbounded("chain-head-method-responses", QUEUE_SIZE_WARNING); + // We are relying on the backpressure from the underlying subscription and we shouldn't + // buffer more than the underlying client can handle. + let (response_sender, response_receiver) = futures::channel::mpsc::channel(1); let state = SubscriptionState:: { with_runtime, tx_stop: Some(tx_stop), @@ -972,8 +923,7 @@ mod tests { #[test] fn sub_state_register_twice() { - let (response_sender, _response_receiver) = - tracing_unbounded("test-chain-head-method-responses", QUEUE_SIZE_WARNING); + let (response_sender, _response_receiver) = futures::channel::mpsc::channel(1); let mut sub_state = SubscriptionState:: { with_runtime: false, tx_stop: None, @@ -1001,8 +951,7 @@ mod tests { #[test] fn sub_state_register_unregister() { - let (response_sender, _response_receiver) = - tracing_unbounded("test-chain-head-method-responses", QUEUE_SIZE_WARNING); + let (response_sender, _response_receiver) = futures::channel::mpsc::channel(1); let mut sub_state = SubscriptionState:: { with_runtime: false, tx_stop: None, diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/mod.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/mod.rs index f266c9d8b34f..84d1b8f8f9b7 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/mod.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/mod.rs @@ -34,7 +34,7 @@ use self::inner::SubscriptionsInner; pub use self::inner::OperationState; pub use error::SubscriptionManagementError; -pub use inner::{BlockGuard, InsertedSubscriptionData}; +pub use inner::{BlockGuard, InsertedSubscriptionData, StopHandle}; /// Manage block pinning / unpinning for subscription IDs. pub struct SubscriptionManagement> { diff --git a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs index 073ee34a79f3..fa10fde388f9 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs @@ -343,7 +343,8 @@ where fn number( &self, hash: Block::Hash, - ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> { + ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> + { self.client.number(hash) } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index a638a9c7ec54..13099603ef70 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -60,7 +60,6 @@ type Block = substrate_test_runtime_client::runtime::Block; const MAX_PINNED_BLOCKS: usize = 32; const MAX_PINNED_SECS: u64 = 60; const MAX_OPERATIONS: usize = 16; -const MAX_PAGINATION_LIMIT: usize = 5; const MAX_LAGGING_DISTANCE: usize = 128; const MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION: usize = 4; @@ -85,7 +84,6 @@ pub async fn run_server() -> std::net::SocketAddr { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, max_follow_subscriptions_per_connection: 1, max_lagging_distance: MAX_LAGGING_DISTANCE, }, @@ -147,7 +145,7 @@ async fn setup_api() -> ( global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, + max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -255,8 +253,6 @@ async fn follow_subscription_produces_blocks() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -326,8 +322,6 @@ async fn follow_with_runtime() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -636,8 +630,6 @@ async fn call_runtime_without_flag() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1297,8 +1289,6 @@ async fn separate_operation_ids_for_subscriptions() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1385,8 +1375,6 @@ async fn follow_generates_initial_blocks() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1543,8 +1531,6 @@ async fn follow_exceeding_pinned_blocks() { global_max_pinned_blocks: 2, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1622,8 +1608,6 @@ async fn follow_with_unpin() { global_max_pinned_blocks: 2, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1730,8 +1714,6 @@ async fn unpin_duplicate_hashes() { global_max_pinned_blocks: 3, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1835,8 +1817,6 @@ async fn follow_with_multiple_unpin_hashes() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -1982,8 +1962,6 @@ async fn follow_prune_best_block() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -2170,8 +2148,6 @@ async fn follow_forks_pruned_block() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -2332,8 +2308,6 @@ async fn follow_report_multiple_pruned_block() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -2580,8 +2554,6 @@ async fn pin_block_references() { global_max_pinned_blocks: 3, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -2717,8 +2689,6 @@ async fn follow_finalized_before_new_block() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -2834,8 +2804,6 @@ async fn ensure_operation_limits_works() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: 1, - operation_max_storage_items: MAX_PAGINATION_LIMIT, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -2887,7 +2855,7 @@ async fn ensure_operation_limits_works() { let operation_id = match response { MethodResponse::Started(started) => { // Check discarded items. - assert_eq!(started.discarded_items.unwrap(), 3); + assert!(started.discarded_items.is_none()); started.operation_id }, MethodResponse::LimitReached => panic!("Expected started response"), @@ -2922,7 +2890,7 @@ async fn ensure_operation_limits_works() { } #[tokio::test] -async fn check_continue_operation() { +async fn storage_is_backpressured() { let child_info = ChildInfo::new_default(CHILD_STORAGE_KEY); let builder = TestClientBuilder::new().add_extra_child_storage( &child_info, @@ -2941,8 +2909,6 @@ async fn check_continue_operation() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: 1, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -3021,18 +2987,6 @@ async fn check_continue_operation() { res.items[0].result == StorageResultType::Value(hex_string(b"a")) ); - // Pagination event. - assert_matches!( - get_next_event::>(&mut sub).await, - FollowEvent::OperationWaitingForContinue(res) if res.operation_id == operation_id - ); - - does_not_produce_event::>( - &mut sub, - std::time::Duration::from_secs(DOES_NOT_PRODUCE_EVENTS_SECONDS), - ) - .await; - let _res: () = api.call("chainHead_v1_continue", [&sub_id, &operation_id]).await.unwrap(); assert_matches!( get_next_event::>(&mut sub).await, FollowEvent::OperationStorageItems(res) if res.operation_id == operation_id && @@ -3041,17 +2995,6 @@ async fn check_continue_operation() { res.items[0].result == StorageResultType::Value(hex_string(b"ab")) ); - // Pagination event. - assert_matches!( - get_next_event::>(&mut sub).await, - FollowEvent::OperationWaitingForContinue(res) if res.operation_id == operation_id - ); - does_not_produce_event::>( - &mut sub, - std::time::Duration::from_secs(DOES_NOT_PRODUCE_EVENTS_SECONDS), - ) - .await; - let _res: () = api.call("chainHead_v1_continue", [&sub_id, &operation_id]).await.unwrap(); assert_matches!( get_next_event::>(&mut sub).await, FollowEvent::OperationStorageItems(res) if res.operation_id == operation_id && @@ -3060,18 +3003,6 @@ async fn check_continue_operation() { res.items[0].result == StorageResultType::Value(hex_string(b"abcmoD")) ); - // Pagination event. - assert_matches!( - get_next_event::>(&mut sub).await, - FollowEvent::OperationWaitingForContinue(res) if res.operation_id == operation_id - ); - - does_not_produce_event::>( - &mut sub, - std::time::Duration::from_secs(DOES_NOT_PRODUCE_EVENTS_SECONDS), - ) - .await; - let _res: () = api.call("chainHead_v1_continue", [&sub_id, &operation_id]).await.unwrap(); assert_matches!( get_next_event::>(&mut sub).await, FollowEvent::OperationStorageItems(res) if res.operation_id == operation_id && @@ -3080,17 +3011,6 @@ async fn check_continue_operation() { res.items[0].result == StorageResultType::Value(hex_string(b"abc")) ); - // Pagination event. - assert_matches!( - get_next_event::>(&mut sub).await, - FollowEvent::OperationWaitingForContinue(res) if res.operation_id == operation_id - ); - does_not_produce_event::>( - &mut sub, - std::time::Duration::from_secs(DOES_NOT_PRODUCE_EVENTS_SECONDS), - ) - .await; - let _res: () = api.call("chainHead_v1_continue", [&sub_id, &operation_id]).await.unwrap(); assert_matches!( get_next_event::>(&mut sub).await, FollowEvent::OperationStorageItems(res) if res.operation_id == operation_id && @@ -3126,8 +3046,6 @@ async fn stop_storage_operation() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: 1, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -3203,15 +3121,22 @@ async fn stop_storage_operation() { res.items[0].result == StorageResultType::Value(hex_string(b"a")) ); - // Pagination event. assert_matches!( get_next_event::>(&mut sub).await, - FollowEvent::OperationWaitingForContinue(res) if res.operation_id == operation_id + FollowEvent::OperationStorageItems(res) if res.operation_id == operation_id && + res.items.len() == 1 && + res.items[0].key == hex_string(b":mo") && + res.items[0].result == StorageResultType::Value(hex_string(b"ab")) ); // Stop the operation. let _res: () = api.call("chainHead_v1_stopOperation", [&sub_id, &operation_id]).await.unwrap(); + assert_matches!( + get_next_event::>(&mut sub).await, + FollowEvent::OperationStorageDone(done) if done.operation_id == operation_id + ); + does_not_produce_event::>( &mut sub, std::time::Duration::from_secs(DOES_NOT_PRODUCE_EVENTS_SECONDS), @@ -3425,7 +3350,6 @@ async fn chain_head_stop_all_subscriptions() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, max_lagging_distance: 5, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, @@ -3639,7 +3563,6 @@ async fn chain_head_limit_reached() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: 1, }, @@ -3680,7 +3603,6 @@ async fn follow_unique_pruned_blocks() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, max_lagging_distance: MAX_LAGGING_DISTANCE, }, @@ -3850,7 +3772,6 @@ async fn follow_report_best_block_of_a_known_block() { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - operation_max_storage_items: MAX_PAGINATION_LIMIT, max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index bd249e033f8f..4148b75f96c0 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -20,8 +20,11 @@ use std::{marker::PhantomData, sync::Arc}; +use futures::{stream::FuturesUnordered, FutureExt, StreamExt}; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; +use sc_rpc::SubscriptionTaskExecutor; use sp_runtime::traits::Block as BlockT; +use tokio::sync::mpsc; use super::events::{StorageResult, StorageResultType}; use crate::hex_string; @@ -30,17 +33,25 @@ use crate::hex_string; pub struct Storage { /// Substrate client. client: Arc, + executor: SubscriptionTaskExecutor, _phandom: PhantomData<(BE, Block)>, } +impl Clone for Storage { + fn clone(&self) -> Self { + Self { client: self.client.clone(), executor: self.executor.clone(), _phandom: PhantomData } + } +} + impl Storage { /// Constructs a new [`Storage`]. - pub fn new(client: Arc) -> Self { - Self { client, _phandom: PhantomData } + pub fn new(client: Arc, executor: SubscriptionTaskExecutor) -> Self { + Self { client, _phandom: PhantomData, executor } } } /// Query to iterate over storage. +#[derive(Debug)] pub struct QueryIter { /// The key from which the iteration was started. pub query_key: StorageKey, @@ -51,6 +62,7 @@ pub struct QueryIter { } /// The query type of an iteration. +#[derive(Debug)] pub enum IterQueryType { /// Iterating over (key, value) pairs. Value, @@ -61,14 +73,11 @@ pub enum IterQueryType { /// The result of making a query call. pub type QueryResult = Result, String>; -/// The result of iterating over keys. -pub type QueryIterResult = Result<(Vec, Option), String>; - impl Storage where - Block: BlockT + 'static, - BE: Backend + 'static, - Client: StorageProvider + 'static, + Block: BlockT + Send + 'static, + BE: Backend + Send + 'static, + Client: StorageProvider + Send + Sync + 'static, { /// Fetch the value from storage. pub fn query_value( @@ -123,7 +132,7 @@ where key: &StorageKey, child_key: Option<&ChildInfo>, ) -> QueryResult { - let result = if let Some(child_key) = child_key { + let result = if let Some(ref child_key) = child_key { self.client.child_closest_merkle_value(hash, child_key, key) } else { self.client.closest_merkle_value(hash, key) @@ -146,53 +155,83 @@ where .unwrap_or_else(|error| QueryResult::Err(error.to_string())) } - /// Iterate over at most the provided number of keys. + /// Iterate over the storage which returns a receiver that will receive the results of the + /// query. + /// + /// Internally this relies on a bounded channel which provides backpressure to the client + /// and that's why we don't need a static limit. /// - /// Returns the storage result with a potential next key to resume iteration. + /// Thus, if the client is slow then we will slow down the iteration. pub fn query_iter_pagination( &self, - query: QueryIter, + queries: Vec, hash: Block::Hash, - child_key: Option<&ChildInfo>, - count: usize, - ) -> QueryIterResult { - let QueryIter { ty, query_key, pagination_start_key } = query; - - let mut keys_iter = if let Some(child_key) = child_key { - self.client.child_storage_keys( - hash, - child_key.to_owned(), - Some(&query_key), - pagination_start_key.as_ref(), - ) - } else { - self.client.storage_keys(hash, Some(&query_key), pagination_start_key.as_ref()) - } - .map_err(|err| err.to_string())?; + child_key: Option, + ) -> mpsc::Receiver { + // NOTE: This is small because we rely on the buffer from the actual JSON-RPC connection. + let (tx, rx) = mpsc::channel(1); + let storage = self.clone(); + + let fut = async move { + let futs: FuturesUnordered<_> = queries + .into_iter() + .map(|query| { + query_iter_pagination_one(&storage, query, hash, child_key.as_ref(), &tx) + }) + .collect(); + + futs.for_each(|_| async {}).await; + }; - let mut ret = Vec::with_capacity(count); - let mut next_pagination_key = None; - for _ in 0..count { - let Some(key) = keys_iter.next() else { break }; + self.executor + .spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed()); - next_pagination_key = Some(key.clone()); + rx + } +} - let result = match ty { - IterQueryType::Value => self.query_value(hash, &key, child_key), - IterQueryType::Hash => self.query_hash(hash, &key, child_key), - }?; +async fn query_iter_pagination_one( + storage: &Storage, + query: QueryIter, + hash: Block::Hash, + child_key: Option<&ChildInfo>, + tx: &mpsc::Sender, +) where + Block: BlockT + Send + 'static, + BE: Backend + Send + 'static, + Client: StorageProvider + Send + Sync + 'static, +{ + let QueryIter { ty, query_key, pagination_start_key } = query; + + let maybe_storage = if let Some(child_key) = child_key { + storage.client.child_storage_keys( + hash, + child_key.to_owned(), + Some(&query_key), + pagination_start_key.as_ref(), + ) + } else { + storage + .client + .storage_keys(hash, Some(&query_key), pagination_start_key.as_ref()) + }; + + let mut keys_iter = match maybe_storage { + Ok(keys_iter) => keys_iter, + Err(error) => { + _ = tx.send(Err(error.to_string())).await; + return; + }, + }; + + while let Some(key) = keys_iter.next() { + let result = match ty { + IterQueryType::Value => storage.query_value(hash, &key, child_key), + IterQueryType::Hash => storage.query_hash(hash, &key, child_key), + }; - if let Some(value) = result { - ret.push(value); - } + if tx.send(result).await.is_err() { + break; } - - // Save the next key if any to continue the iteration. - let maybe_next_query = keys_iter.next().map(|_| QueryIter { - ty, - query_key, - pagination_start_key: next_pagination_key, - }); - Ok((ret, maybe_next_query)) } } diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index 0dc28d1361cb..448abf4830e9 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -712,8 +712,7 @@ where client.clone(), backend.clone(), genesis_hash, - // Defaults to sensible limits for the `Archive`. - sc_rpc_spec_v2::archive::ArchiveConfig::default(), + task_executor.clone(), ) .into_rpc(); rpc_api.merge(archive_v2).map_err(|e| Error::Application(e.into()))?; diff --git a/substrate/primitives/state-machine/src/backend.rs b/substrate/primitives/state-machine/src/backend.rs index 90be55d58a4e..13eccf22b46f 100644 --- a/substrate/primitives/state-machine/src/backend.rs +++ b/substrate/primitives/state-machine/src/backend.rs @@ -187,7 +187,7 @@ pub trait Backend: core::fmt::Debug { type TrieBackendStorage: TrieBackendStorage; /// Type of the raw storage iterator. - type RawIter: StorageIterator; + type RawIter: StorageIterator + Send; /// Get keyed storage or None if there is nothing associated. fn storage(&self, key: &[u8]) -> Result, Self::Error>; From c7d9a508f502967ec226084a28ae1f63f5da92dc Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 17 Sep 2024 17:35:14 +0200 Subject: [PATCH 02/28] Update substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs --- .../client/rpc-spec-v2/src/chain_head/chain_head_storage.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index 35f2331ce2fd..116a79612469 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -104,7 +104,6 @@ where items: Vec>, child_key: Option, ) -> Result<(), FollowEventSendError> { - log::info!(target: LOG_TARGET, "generate_events={:?}", items.len()); let mut iter_ops = Vec::new(); let mut sender = block_guard.response_sender(); From 540daa6dda658ce694afae9d8f582978cca0f999 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 17 Sep 2024 17:36:20 +0200 Subject: [PATCH 03/28] Update substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs --- .../client/rpc-spec-v2/src/chain_head/chain_head_storage.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index 116a79612469..02e266005398 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -104,7 +104,6 @@ where items: Vec>, child_key: Option, ) -> Result<(), FollowEventSendError> { - let mut iter_ops = Vec::new(); let mut sender = block_guard.response_sender(); let operation = block_guard.operation(); From 423bc3030922baa2f1432eaafc2a25a7726bc968 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 17 Sep 2024 17:37:18 +0200 Subject: [PATCH 04/28] Update substrate/client/rpc-spec-v2/src/chain_head/tests.rs --- substrate/client/rpc-spec-v2/src/chain_head/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index 13099603ef70..f30db423b4fd 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -145,7 +145,6 @@ async fn setup_api() -> ( global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), subscription_max_ongoing_operations: MAX_OPERATIONS, - max_lagging_distance: MAX_LAGGING_DISTANCE, max_follow_subscriptions_per_connection: MAX_FOLLOW_SUBSCRIPTIONS_PER_CONNECTION, }, From 73512068fad092bb79da03bde210ac613ba8584e Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 18 Sep 2024 11:29:23 +0200 Subject: [PATCH 05/28] cleanup --- substrate/client/rpc-servers/src/lib.rs | 5 +++-- .../src/chain_head/chain_head_storage.rs | 1 - .../client/rpc-spec-v2/src/chain_head/mod.rs | 2 +- .../src/chain_head/subscription/inner.rs | 18 +++++++++--------- .../rpc-spec-v2/src/chain_head/test_utils.rs | 3 +-- .../client/rpc-spec-v2/src/common/storage.rs | 11 ++++++++--- 6 files changed, 22 insertions(+), 18 deletions(-) diff --git a/substrate/client/rpc-servers/src/lib.rs b/substrate/client/rpc-servers/src/lib.rs index a31663b1bac9..733e3cb35ca5 100644 --- a/substrate/client/rpc-servers/src/lib.rs +++ b/substrate/client/rpc-servers/src/lib.rs @@ -256,8 +256,9 @@ where ), }; - let rpc_middleware = - RpcServiceBuilder::new().rpc_logger(1024).option_layer(middleware_layer.clone()); + let rpc_middleware = RpcServiceBuilder::new() + .rpc_logger(1024) + .option_layer(middleware_layer.clone()); let mut svc = service_builder .set_rpc_middleware(rpc_middleware) .build(methods, stop_handle); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index 02e266005398..be30b5f47cdc 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -28,7 +28,6 @@ use tokio::sync::mpsc; use crate::{ chain_head::{ - chain_head::LOG_TARGET, event::{OperationError, OperationId, OperationStorageItems}, subscription::{BlockGuard, StopHandle}, FollowEvent, FollowEventSendError, FollowEventSender, diff --git a/substrate/client/rpc-spec-v2/src/chain_head/mod.rs b/substrate/client/rpc-spec-v2/src/chain_head/mod.rs index 84b4055fa422..98ddfbbdc63f 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/mod.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/mod.rs @@ -48,4 +48,4 @@ pub(crate) type FollowEventSender = futures::channel::mpsc::Sender = futures::channel::mpsc::Receiver>; /// Follow event send error. -pub(crate) type FollowEventSendError = futures::channel::mpsc::SendError; \ No newline at end of file +pub(crate) type FollowEventSendError = futures::channel::mpsc::SendError; diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs index 384ea02791e9..f3f37db1a93d 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs @@ -33,6 +33,12 @@ use crate::chain_head::{ type NotifyOnDrop = tokio::sync::mpsc::Receiver<()>; type SharedOperations = Arc>>; +/// The buffer capacity for each subscription +/// +/// Beware of that the JSON-RPC server has a global +/// buffer per connection and this a extra buffer. +const BUF_CAP_PER_SUBSCRIPTION: usize = 16; + /// The state machine of a block of a single subscription ID. /// /// # Motivation @@ -185,14 +191,6 @@ impl OperationState { self.operations.lock().remove(&self.operation_id); } } - - pub fn is_stopped(&self) -> bool { - self.stop.is_stopped() - } - - pub async fn stopped(&self) { - self.stop.stopped().await; - } } /// The registered operation passed to the `chainHead` methods. @@ -223,6 +221,7 @@ impl RegisteredOperation { /// Returns the number of reserved elements for this permit. /// /// This can be smaller than the number of items requested via [`LimitOperations::reserve()`]. + #[allow(unused)] pub fn num_reserved(&self) -> usize { self.permit.num_ops } @@ -545,7 +544,8 @@ impl> SubscriptionsInner { let (tx_stop, rx_stop) = oneshot::channel(); // We are relying on the backpressure from the underlying subscription and we shouldn't // buffer more than the underlying client can handle. - let (response_sender, response_receiver) = futures::channel::mpsc::channel(1); + let (response_sender, response_receiver) = + futures::channel::mpsc::channel(BUF_CAP_PER_SUBSCRIPTION); let state = SubscriptionState:: { with_runtime, tx_stop: Some(tx_stop), diff --git a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs index fa10fde388f9..073ee34a79f3 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs @@ -343,8 +343,7 @@ where fn number( &self, hash: Block::Hash, - ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> - { + ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> { self.client.number(hash) } diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index 4148b75f96c0..10c5bdc4ac54 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -29,6 +29,12 @@ use tokio::sync::mpsc; use super::events::{StorageResult, StorageResultType}; use crate::hex_string; +/// The buffer capacity for `Storage::query_iter_pagination`. +/// +/// This is small because the underlying JSON-RPC server has +/// its down buffer capacity per connection as well. +const QUERY_ITER_PAGINATED_BUF_CAP: usize = 16; + /// Call into the storage of blocks. pub struct Storage { /// Substrate client. @@ -155,7 +161,7 @@ where .unwrap_or_else(|error| QueryResult::Err(error.to_string())) } - /// Iterate over the storage which returns a receiver that will receive the results of the + /// Iterate over the storage which returns a stream that receive the results of the /// query. /// /// Internally this relies on a bounded channel which provides backpressure to the client @@ -168,8 +174,7 @@ where hash: Block::Hash, child_key: Option, ) -> mpsc::Receiver { - // NOTE: This is small because we rely on the buffer from the actual JSON-RPC connection. - let (tx, rx) = mpsc::channel(1); + let (tx, rx) = mpsc::channel(QUERY_ITER_PAGINATED_BUF_CAP); let storage = self.clone(); let fut = async move { From b963c8b2d5e38d586a9e85a48d0d47098003ac94 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 18 Sep 2024 18:09:48 +0200 Subject: [PATCH 06/28] revert archive static limits --- .../client/rpc-spec-v2/src/archive/archive.rs | 53 ++++++++++++++++- .../src/archive/archive_storage.rs | 26 +++++++-- .../client/rpc-spec-v2/src/archive/mod.rs | 2 +- .../client/rpc-spec-v2/src/archive/tests.rs | 57 ++++++++++++------- .../src/chain_head/chain_head_storage.rs | 34 +++++------ .../client/rpc-spec-v2/src/common/storage.rs | 14 ++++- substrate/client/service/src/builder.rs | 2 + 7 files changed, 134 insertions(+), 54 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive.rs b/substrate/client/rpc-spec-v2/src/archive/archive.rs index 745aced70fd3..3fb45131d4e1 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive.rs @@ -44,6 +44,36 @@ use std::{collections::HashSet, marker::PhantomData, sync::Arc}; use super::archive_storage::ArchiveStorage; +/// The configuration of [`Archive`]. +pub struct ArchiveConfig { + /// The maximum number of items the `archive_storage` can return for a descendant query before + /// pagination is required. + pub max_descendant_responses: usize, + /// The maximum number of queried items allowed for the `archive_storage` at a time. + pub max_queried_items: usize, +} + +/// The maximum number of items the `archive_storage` can return for a descendant query before +/// pagination is required. +/// +/// Note: this is identical to the `chainHead` value. +const MAX_DESCENDANT_RESPONSES: usize = 5; + +/// The maximum number of queried items allowed for the `archive_storage` at a time. +/// +/// Note: A queried item can also be a descendant query which can return up to +/// `MAX_DESCENDANT_RESPONSES`. +const MAX_QUERIED_ITEMS: usize = 8; + +impl Default for ArchiveConfig { + fn default() -> Self { + Self { + max_descendant_responses: MAX_DESCENDANT_RESPONSES, + max_queried_items: MAX_QUERIED_ITEMS, + } + } +} + /// An API for archive RPC calls. pub struct Archive, Block: BlockT, Client> { /// Substrate client. @@ -52,6 +82,11 @@ pub struct Archive, Block: BlockT, Client> { backend: Arc, /// The hexadecimal encoded hash of the genesis block. genesis_hash: String, + /// The maximum number of items the `archive_storage` can return for a descendant query before + /// pagination is required. + storage_max_descendant_responses: usize, + /// The maximum number of queried items allowed for the `archive_storage` at a time. + storage_max_queried_items: usize, /// Phantom member to pin the block type. _phantom: PhantomData, /// Subscription task executor. @@ -64,10 +99,19 @@ impl, Block: BlockT, Client> Archive { client: Arc, backend: Arc, genesis_hash: GenesisHash, + config: ArchiveConfig, executor: SubscriptionTaskExecutor, ) -> Self { let genesis_hash = hex_string(&genesis_hash.as_ref()); - Self { client, backend, genesis_hash, _phantom: PhantomData, executor } + Self { + client, + backend, + genesis_hash, + storage_max_descendant_responses: config.max_descendant_responses, + storage_max_queried_items: config.max_queried_items, + _phantom: PhantomData, + executor, + } } } @@ -231,7 +275,12 @@ where .transpose()? .map(ChildInfo::new_default_from_vec); - let storage_client = ArchiveStorage::new(self.client.clone(), self.executor.clone()); + let storage_client = ArchiveStorage::new( + self.client.clone(), + self.storage_max_descendant_responses, + self.storage_max_queried_items, + self.executor.clone(), + ); Ok(storage_client.handle_query(hash, items, child_trie)) } diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index 7e222fba2967..7d6f14d0e02e 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -33,12 +33,25 @@ use crate::common::{ pub struct ArchiveStorage { /// Storage client. client: Storage, + /// The maximum number of responses the API can return for a descendant query at a time. + storage_max_descendant_responses: usize, + /// The maximum number of queried items allowed for the `archive_storage` at a time. + storage_max_queried_items: usize, } impl ArchiveStorage { /// Constructs a new [`ArchiveStorage`]. - pub fn new(client: Arc, executor: SubscriptionTaskExecutor) -> Self { - Self { client: Storage::new(client, executor) } + pub fn new( + client: Arc, + storage_max_descendant_responses: usize, + storage_max_queried_items: usize, + executor: SubscriptionTaskExecutor, + ) -> Self { + Self { + client: Storage::new(client, executor), + storage_max_descendant_responses, + storage_max_queried_items, + } } } @@ -52,9 +65,12 @@ where pub fn handle_query( &self, hash: Block::Hash, - items: Vec>, + mut items: Vec>, child_key: Option, ) -> ArchiveStorageResult { + let discarded_items = items.len().saturating_sub(self.storage_max_queried_items); + items.truncate(self.storage_max_queried_items); + let mut storage_results = Vec::with_capacity(items.len()); let mut query_iter = Vec::new(); @@ -97,7 +113,7 @@ where } if !query_iter.is_empty() { - let mut rx = self.client.query_iter_pagination(query_iter, hash, child_key); + let mut rx = self.client.query_iter_pagination(query_iter, hash, child_key, Some(self.storage_max_descendant_responses)); while let Some(val) = rx.blocking_recv() { match val { @@ -108,6 +124,6 @@ where } } - ArchiveStorageResult::ok(storage_results, 0) + ArchiveStorageResult::ok(storage_results, discarded_items) } } diff --git a/substrate/client/rpc-spec-v2/src/archive/mod.rs b/substrate/client/rpc-spec-v2/src/archive/mod.rs index 14fa104c113a..5f020c203eab 100644 --- a/substrate/client/rpc-spec-v2/src/archive/mod.rs +++ b/substrate/client/rpc-spec-v2/src/archive/mod.rs @@ -32,4 +32,4 @@ pub mod archive; pub mod error; pub use api::ArchiveApiServer; -pub use archive::Archive; +pub use archive::{Archive, ArchiveConfig}; diff --git a/substrate/client/rpc-spec-v2/src/archive/tests.rs b/substrate/client/rpc-spec-v2/src/archive/tests.rs index 460aa4916695..e14e6df76c45 100644 --- a/substrate/client/rpc-spec-v2/src/archive/tests.rs +++ b/substrate/client/rpc-spec-v2/src/archive/tests.rs @@ -24,7 +24,10 @@ use crate::{ hex_string, MethodResult, }; -use super::{archive::Archive, *}; +use super::{ + archive::{Archive, ArchiveConfig}, + *, +}; use assert_matches::assert_matches; use codec::{Decode, Encode}; @@ -48,6 +51,8 @@ use substrate_test_runtime_client::{ const CHAIN_GENESIS: [u8; 32] = [0; 32]; const INVALID_HASH: [u8; 32] = [1; 32]; +const MAX_PAGINATION_LIMIT: usize = 5; +const MAX_QUERIED_LIMIT: usize = 5; const KEY: &[u8] = b":mock"; const VALUE: &[u8] = b"hello world"; const CHILD_STORAGE_KEY: &[u8] = b"child"; @@ -56,7 +61,10 @@ const CHILD_VALUE: &[u8] = b"child value"; type Header = substrate_test_runtime_client::runtime::Header; type Block = substrate_test_runtime_client::runtime::Block; -fn setup_api() -> (Arc>, RpcModule>>) { +fn setup_api( + max_descendant_responses: usize, + max_queried_items: usize, +) -> (Arc>, RpcModule>>) { let child_info = ChildInfo::new_default(CHILD_STORAGE_KEY); let builder = TestClientBuilder::new().add_extra_child_storage( &child_info, @@ -66,16 +74,21 @@ fn setup_api() -> (Arc>, RpcModule = api.call("archive_unstable_hashByHeight", [0]).await.unwrap(); @@ -270,7 +283,7 @@ async fn archive_hash_by_height() { #[tokio::test] async fn archive_call() { - let (client, api) = setup_api(); + let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); let invalid_hash = hex_string(&INVALID_HASH); // Invalid parameter (non-hex). @@ -329,7 +342,7 @@ async fn archive_call() { #[tokio::test] async fn archive_storage_hashes_values() { - let (client, api) = setup_api(); + let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); let block = BlockBuilderBuilder::new(&*client) .on_parent_block(client.chain_info().genesis_hash) @@ -419,7 +432,7 @@ async fn archive_storage_hashes_values() { #[tokio::test] async fn archive_storage_closest_merkle_value() { - let (client, api) = setup_api(); + let (client, api) = setup_api(MAX_PAGINATION_LIMIT, MAX_QUERIED_LIMIT); /// The core of this test. /// @@ -580,7 +593,7 @@ async fn archive_storage_closest_merkle_value() { #[tokio::test] async fn archive_storage_paginate_iterations() { // 1 iteration allowed before pagination kicks in. - let (client, api) = setup_api(); + let (client, api) = setup_api(1, MAX_QUERIED_LIMIT); // Import a new block with storage changes. let mut builder = BlockBuilderBuilder::new(&*client) @@ -635,7 +648,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 5); + assert_eq!(result.len(), 1); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":m")); @@ -661,7 +674,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 1); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":mo")); @@ -687,7 +700,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 3); + assert_eq!(result.len(), 1); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":moD")); @@ -713,7 +726,7 @@ async fn archive_storage_paginate_iterations() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 2); + assert_eq!(result.len(), 1); assert_eq!(discarded_items, 0); assert_eq!(result[0].key, hex_string(b":moc")); @@ -773,9 +786,9 @@ async fn archive_storage_paginate_iterations() { } #[tokio::test] -async fn archive_storage_is_backpressured() { +async fn archive_storage_discarded_items() { // One query at a time - let (client, api) = setup_api(); + let (client, api) = setup_api(MAX_PAGINATION_LIMIT, 1); // Import a new block with storage changes. let mut builder = BlockBuilderBuilder::new(&*client) @@ -817,8 +830,8 @@ async fn archive_storage_is_backpressured() { .unwrap(); match result { ArchiveStorageResult::Ok(ArchiveStorageMethodOk { result, discarded_items }) => { - assert_eq!(result.len(), 3); - assert_eq!(discarded_items, 0); + assert_eq!(result.len(), 1); + assert_eq!(discarded_items, 2); assert_eq!(result[0].key, hex_string(b":m")); assert_eq!(result[0].result, StorageResultType::Value(hex_string(b"a"))); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index be30b5f47cdc..75f49badf923 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -75,7 +75,7 @@ where if !query_iter.is_empty() { process_storage_iter_stream( - self.client.query_iter_pagination(query_iter, hash, child_key), + self.client.query_iter_pagination(query_iter, hash, child_key, None), block_guard.response_sender(), block_guard.operation().operation_id().to_owned(), &stop_handle, @@ -178,34 +178,26 @@ async fn process_storage_iter_stream( operation_id: String, stop_handle: &StopHandle, ) -> Result<(), FollowEventSendError> { - let mut buf = Vec::new(); - loop { tokio::select! { _ = stop_handle.stopped() => return Ok(()), - len = storage_query_stream.recv_many(&mut buf, 1024) => { - if len == 0 { - break Ok(()); - } - - let mut items = Vec::with_capacity(buf.len()); - - for val in buf.drain(..) { - match val { - QueryResult::Err(error) => - return send_error(&mut sender, operation_id.clone(), error).await, - QueryResult::Ok(Some(v)) => { - items.push(v) - }, - QueryResult::Ok(None) => continue + maybe_storage = storage_query_stream.recv() => { + let Some(storage) = maybe_storage else { + return Ok(()); + }; + + let item = match storage { + QueryResult::Err(error) => { + return send_error(&mut sender, operation_id.clone(), error).await; } - } + QueryResult::Ok(Some(v)) => v, + QueryResult::Ok(None) => continue, + }; - // Send back the results of the iteration produced so far. sender .send(FollowEvent::OperationStorageItems(OperationStorageItems { operation_id: operation_id.clone(), - items, + items: vec![item], })).await?; }, diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index 10c5bdc4ac54..e44e4cdcf917 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -173,6 +173,7 @@ where queries: Vec, hash: Block::Hash, child_key: Option, + max_iterations: Option, ) -> mpsc::Receiver { let (tx, rx) = mpsc::channel(QUERY_ITER_PAGINATED_BUF_CAP); let storage = self.clone(); @@ -181,7 +182,7 @@ where let futs: FuturesUnordered<_> = queries .into_iter() .map(|query| { - query_iter_pagination_one(&storage, query, hash, child_key.as_ref(), &tx) + query_iter_pagination_one(&storage, query, hash, child_key.as_ref(), &tx, max_iterations) }) .collect(); @@ -201,6 +202,7 @@ async fn query_iter_pagination_one( hash: Block::Hash, child_key: Option<&ChildInfo>, tx: &mpsc::Sender, + max_iterations: Option, ) where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, @@ -221,7 +223,7 @@ async fn query_iter_pagination_one( .storage_keys(hash, Some(&query_key), pagination_start_key.as_ref()) }; - let mut keys_iter = match maybe_storage { + let keys_iter = match maybe_storage { Ok(keys_iter) => keys_iter, Err(error) => { _ = tx.send(Err(error.to_string())).await; @@ -229,7 +231,13 @@ async fn query_iter_pagination_one( }, }; - while let Some(key) = keys_iter.next() { + for (idx, key) in keys_iter.into_iter().enumerate() { + if let Some(max_iterations) = max_iterations { + if idx >= max_iterations { + break; + } + } + let result = match ty { IterQueryType::Value => storage.query_value(hash, &key, child_key), IterQueryType::Hash => storage.query_hash(hash, &key, child_key), diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index b4bc25a69a67..18153cf52bbb 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -728,6 +728,8 @@ where client.clone(), backend.clone(), genesis_hash, + // Defaults to sensible limits for the `Archive`. + sc_rpc_spec_v2::archive::ArchiveConfig::default(), task_executor.clone(), ) .into_rpc(); From 1cb17f345cc348c9cd255fb3356a52d92292bdee Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 18 Sep 2024 18:49:06 +0200 Subject: [PATCH 07/28] cargo fmt --- .../client/rpc-spec-v2/src/archive/archive_storage.rs | 7 ++++++- substrate/client/rpc-spec-v2/src/common/storage.rs | 9 ++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index 7d6f14d0e02e..c902c10f46a2 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -113,7 +113,12 @@ where } if !query_iter.is_empty() { - let mut rx = self.client.query_iter_pagination(query_iter, hash, child_key, Some(self.storage_max_descendant_responses)); + let mut rx = self.client.query_iter_pagination( + query_iter, + hash, + child_key, + Some(self.storage_max_descendant_responses), + ); while let Some(val) = rx.blocking_recv() { match val { diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index e44e4cdcf917..96638ba29df3 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -182,7 +182,14 @@ where let futs: FuturesUnordered<_> = queries .into_iter() .map(|query| { - query_iter_pagination_one(&storage, query, hash, child_key.as_ref(), &tx, max_iterations) + query_iter_pagination_one( + &storage, + query, + hash, + child_key.as_ref(), + &tx, + max_iterations, + ) }) .collect(); From 0c94a6b679ffa5e2d074990d60c0b62448d919dd Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 19 Sep 2024 09:59:59 +0200 Subject: [PATCH 08/28] add trait bound RawIter: Send --- .../client/rpc-spec-v2/src/archive/archive.rs | 1 + .../src/archive/archive_storage.rs | 3 +- .../rpc-spec-v2/src/chain_head/chain_head.rs | 3 +- .../src/chain_head/chain_head_storage.rs | 3 +- .../client/rpc-spec-v2/src/common/storage.rs | 29 ++++++++++++------- substrate/client/service/src/builder.rs | 4 ++- .../primitives/state-machine/src/backend.rs | 2 +- 7 files changed, 29 insertions(+), 16 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive.rs b/substrate/client/rpc-spec-v2/src/archive/archive.rs index 3fb45131d4e1..2e8555f85b80 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive.rs @@ -141,6 +141,7 @@ where + CallApiAt + StorageProvider + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send { fn archive_unstable_body(&self, hash: Block::Hash) -> RpcResult>> { let Ok(Some(signed_block)) = self.client.block(hash) else { return Ok(None) }; diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index c902c10f46a2..753b5413471c 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -22,7 +22,7 @@ use std::sync::Arc; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; use sc_rpc::SubscriptionTaskExecutor; -use sp_runtime::traits::Block as BlockT; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use crate::common::{ events::{ArchiveStorageResult, PaginatedStorageQuery, StorageQueryType}, @@ -60,6 +60,7 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send { /// Generate the response of the `archive_storage` method. pub fn handle_query( diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index 6f94c62e449c..e232dc94bb34 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -49,7 +49,7 @@ use sp_api::CallApiAt; use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use sp_core::{traits::CallContext, Bytes}; use sp_rpc::list::ListOrValue; -use sp_runtime::traits::Block as BlockT; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{marker::PhantomData, sync::Arc, time::Duration}; pub(crate) const LOG_TARGET: &str = "rpc-spec-v2"; @@ -182,6 +182,7 @@ where + CallApiAt + StorageProvider + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send { fn chain_head_unstable_follow(&self, pending: PendingSubscriptionSink, with_runtime: bool) { let subscriptions = self.subscriptions.clone(); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index 75f49badf923..bdc5f6b9068f 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -23,7 +23,7 @@ use std::{marker::PhantomData, sync::Arc}; use futures::SinkExt; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; use sc_rpc::SubscriptionTaskExecutor; -use sp_runtime::traits::Block as BlockT; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use tokio::sync::mpsc; use crate::{ @@ -57,6 +57,7 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send { /// Iterate over (key, hash) and (key, value) generating the `WaitingForContinue` event if /// necessary. diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index 96638ba29df3..08ee8e51fba0 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -23,7 +23,7 @@ use std::{marker::PhantomData, sync::Arc}; use futures::{stream::FuturesUnordered, FutureExt, StreamExt}; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; use sc_rpc::SubscriptionTaskExecutor; -use sp_runtime::traits::Block as BlockT; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use tokio::sync::mpsc; use super::events::{StorageResult, StorageResultType}; @@ -84,6 +84,7 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send, { /// Fetch the value from storage. pub fn query_value( @@ -164,10 +165,11 @@ where /// Iterate over the storage which returns a stream that receive the results of the /// query. /// - /// Internally this relies on a bounded channel which provides backpressure to the client - /// and that's why we don't need a static limit. + /// Internally this relies on a bounded channel which provides backpressure which needs + /// propagated down the underlying client. /// - /// Thus, if the client is slow then we will slow down the iteration. + /// For users of this API, if you can't rely on backpressure then you can + /// use `max_iterations` to limit the number of iterations. pub fn query_iter_pagination( &self, queries: Vec, @@ -178,8 +180,8 @@ where let (tx, rx) = mpsc::channel(QUERY_ITER_PAGINATED_BUF_CAP); let storage = self.clone(); - let fut = async move { - let futs: FuturesUnordered<_> = queries + let pending_queries = async move { + let queries: FuturesUnordered<_> = queries .into_iter() .map(|query| { query_iter_pagination_one( @@ -193,11 +195,14 @@ where }) .collect(); - futs.for_each(|_| async {}).await; + queries.for_each(|_| async {}).await; }; - self.executor - .spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed()); + self.executor.spawn_blocking( + "substrate-rpc-subscription", + Some("rpc"), + Box::pin(pending_queries), + ); rx } @@ -214,6 +219,8 @@ async fn query_iter_pagination_one( Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: std::marker::Send, + { let QueryIter { ty, query_key, pagination_start_key } = query; @@ -238,9 +245,9 @@ async fn query_iter_pagination_one( }, }; - for (idx, key) in keys_iter.into_iter().enumerate() { + for (count, key) in keys_iter.into_iter().enumerate() { if let Some(max_iterations) = max_iterations { - if idx >= max_iterations { + if count >= max_iterations { break; } } diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index 18153cf52bbb..ee569fd6b3a5 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -86,7 +86,7 @@ use sp_consensus::block_validation::{ }; use sp_core::traits::{CodeExecutor, SpawnNamed}; use sp_keystore::KeystorePtr; -use sp_runtime::traits::{Block as BlockT, BlockIdTo, NumberFor, Zero}; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT, BlockIdTo, NumberFor, Zero}; use std::{str::FromStr, sync::Arc, time::SystemTime}; /// Full client type. @@ -405,6 +405,7 @@ where TBl::Header: Unpin, TBackend: 'static + sc_client_api::backend::Backend + Send, TExPool: MaintainedTransactionPool::Hash> + 'static, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send { let SpawnTasksParams { mut config, @@ -669,6 +670,7 @@ where TExPool: MaintainedTransactionPool::Hash> + 'static, TBl::Hash: Unpin, TBl::Header: Unpin, + <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send { let system_info = sc_rpc::system::SystemInfo { chain_name: chain_spec.name().into(), diff --git a/substrate/primitives/state-machine/src/backend.rs b/substrate/primitives/state-machine/src/backend.rs index 13eccf22b46f..90be55d58a4e 100644 --- a/substrate/primitives/state-machine/src/backend.rs +++ b/substrate/primitives/state-machine/src/backend.rs @@ -187,7 +187,7 @@ pub trait Backend: core::fmt::Debug { type TrieBackendStorage: TrieBackendStorage; /// Type of the raw storage iterator. - type RawIter: StorageIterator + Send; + type RawIter: StorageIterator; /// Get keyed storage or None if there is nothing associated. fn storage(&self, key: &[u8]) -> Result, Self::Error>; From 583ea58a8bd19100bb1398b50502ecf74d194e14 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 19 Sep 2024 10:08:32 +0200 Subject: [PATCH 09/28] cargo fmt --- substrate/client/rpc-spec-v2/src/archive/archive.rs | 4 +++- .../client/rpc-spec-v2/src/archive/archive_storage.rs | 4 +++- .../client/rpc-spec-v2/src/chain_head/chain_head.rs | 4 +++- .../rpc-spec-v2/src/chain_head/chain_head_storage.rs | 4 +++- substrate/client/rpc-spec-v2/src/common/storage.rs | 9 ++++++--- substrate/client/service/src/builder.rs | 10 +++++++--- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive.rs b/substrate/client/rpc-spec-v2/src/archive/archive.rs index 2e8555f85b80..a30e6d419b5c 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive.rs @@ -141,7 +141,9 @@ where + CallApiAt + StorageProvider + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { fn archive_unstable_body(&self, hash: Block::Hash) -> RpcResult>> { let Ok(Some(signed_block)) = self.client.block(hash) else { return Ok(None) }; diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index 753b5413471c..b34dc44a7b2c 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -60,7 +60,9 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { /// Generate the response of the `archive_storage` method. pub fn handle_query( diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index e232dc94bb34..5c4cf082fe6b 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -182,7 +182,9 @@ where + CallApiAt + StorageProvider + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { fn chain_head_unstable_follow(&self, pending: PendingSubscriptionSink, with_runtime: bool) { let subscriptions = self.subscriptions.clone(); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index bdc5f6b9068f..3c0ecd63a1a5 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -57,7 +57,9 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { /// Iterate over (key, hash) and (key, value) generating the `WaitingForContinue` event if /// necessary. diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index 08ee8e51fba0..20097fb86c4f 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -84,7 +84,9 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send, + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { /// Fetch the value from storage. pub fn query_value( @@ -219,8 +221,9 @@ async fn query_iter_pagination_one( Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: std::marker::Send, - + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: std::marker::Send, { let QueryIter { ty, query_key, pagination_start_key } = query; diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index ee569fd6b3a5..2bd30eef4887 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -86,7 +86,7 @@ use sp_consensus::block_validation::{ }; use sp_core::traits::{CodeExecutor, SpawnNamed}; use sp_keystore::KeystorePtr; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT, BlockIdTo, NumberFor, Zero}; +use sp_runtime::traits::{Block as BlockT, BlockIdTo, Header as HeaderT, NumberFor, Zero}; use std::{str::FromStr, sync::Arc, time::SystemTime}; /// Full client type. @@ -405,7 +405,9 @@ where TBl::Header: Unpin, TBackend: 'static + sc_client_api::backend::Backend + Send, TExPool: MaintainedTransactionPool::Hash> + 'static, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { let SpawnTasksParams { mut config, @@ -670,7 +672,9 @@ where TExPool: MaintainedTransactionPool::Hash> + 'static, TBl::Hash: Unpin, TBl::Header: Unpin, - <>::State as sc_client_api::StateBackend<<::Header as HeaderT>::Hashing>>::RawIter: Send + <>::State as sc_client_api::StateBackend< + <::Header as HeaderT>::Hashing, + >>::RawIter: Send, { let system_info = sc_rpc::system::SystemInfo { chain_name: chain_spec.name().into(), From 486b246208ef411c74d207d7adf1891a16dabf44 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 19 Sep 2024 10:48:20 +0200 Subject: [PATCH 10/28] remove unused import --- substrate/client/rpc-spec-v2/src/common/storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index 20097fb86c4f..bd4dbb75cc19 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -20,7 +20,7 @@ use std::{marker::PhantomData, sync::Arc}; -use futures::{stream::FuturesUnordered, FutureExt, StreamExt}; +use futures::{stream::FuturesUnordered, StreamExt}; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; use sc_rpc::SubscriptionTaskExecutor; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; From 67a37ce02c0a0ac3f18bad5eb620a536c036d6e1 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 19 Sep 2024 11:06:54 +0200 Subject: [PATCH 11/28] Update substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs --- .../client/rpc-spec-v2/src/chain_head/subscription/inner.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs index f3f37db1a93d..5f70073e15a7 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs @@ -543,7 +543,6 @@ impl> SubscriptionsInner { if let Entry::Vacant(entry) = self.subs.entry(sub_id) { let (tx_stop, rx_stop) = oneshot::channel(); // We are relying on the backpressure from the underlying subscription and we shouldn't - // buffer more than the underlying client can handle. let (response_sender, response_receiver) = futures::channel::mpsc::channel(BUF_CAP_PER_SUBSCRIPTION); let state = SubscriptionState:: { From 05386e84d4677281ad12ca050b042b68fcee3196 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 19 Sep 2024 11:07:48 +0200 Subject: [PATCH 12/28] Update substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs --- .../client/rpc-spec-v2/src/chain_head/subscription/inner.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs index 5f70073e15a7..680c0b62433d 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs @@ -542,7 +542,6 @@ impl> SubscriptionsInner { ) -> Option> { if let Entry::Vacant(entry) = self.subs.entry(sub_id) { let (tx_stop, rx_stop) = oneshot::channel(); - // We are relying on the backpressure from the underlying subscription and we shouldn't let (response_sender, response_receiver) = futures::channel::mpsc::channel(BUF_CAP_PER_SUBSCRIPTION); let state = SubscriptionState:: { From ab88c05c6b804963905fbfff38d57d993594a8f5 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 24 Sep 2024 16:26:52 +0200 Subject: [PATCH 13/28] address grumbles --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 25 +++++++++++++++---- .../src/chain_head/chain_head_storage.rs | 3 +-- .../src/chain_head/subscription/inner.rs | 21 +++------------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index 5c4cf082fe6b..c5f45d54f0d7 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -558,12 +558,27 @@ where async fn chain_head_unstable_continue( &self, - _ext: &Extensions, - _follow_subscription: String, - _operation_id: String, + ext: &Extensions, + follow_subscription: String, + operation_id: String, ) -> Result<(), ChainHeadRpcError> { - // `WaitingForContinue`` is never emitted by the server. - Ok(()) + let conn_id = ext + .get::() + .copied() + .expect("ConnectionId is always set by jsonrpsee; qed"); + + if !self.subscriptions.contains_subscription(conn_id, &follow_subscription) { + return Ok(()) + } + + // WaitingForContinue event is never emitted, in such cases + // emit an `InvalidContinue error`. + if self.subscriptions.get_operation(&follow_subscription, &operation_id).is_some() { + Err(ChainHeadRpcError::InvalidContinue.into()) + } + else { + Ok(()) + } } async fn chain_head_unstable_stop_operation( diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index 3c0ecd63a1a5..d74e7985c6a9 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -61,8 +61,7 @@ where <::Header as HeaderT>::Hashing, >>::RawIter: Send, { - /// Iterate over (key, hash) and (key, value) generating the `WaitingForContinue` event if - /// necessary. + /// Iterate over (key, hash) and (key, value). async fn generate_storage_iter_events( &self, query_iter: Vec, diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs index 680c0b62433d..84fe478f6524 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs @@ -145,7 +145,7 @@ impl LimitOperations { .try_acquire_many_owned(num_ops.try_into().ok()?) .ok()?; - Some(PermitOperations { num_ops, _permit: permits }) + Some(permits) } } @@ -155,12 +155,7 @@ impl LimitOperations { /// to guarantee the RPC server can execute the number of operations. /// /// The number of reserved items are given back to the [`LimitOperations`] on drop. -struct PermitOperations { - /// The number of operations permitted (reserved). - num_ops: usize, - /// The permit for these operations. - _permit: tokio::sync::OwnedSemaphorePermit, -} +type PermitOperations = tokio::sync::OwnedSemaphorePermit; /// Stop handle for the operation. #[derive(Clone)] @@ -204,7 +199,7 @@ pub struct RegisteredOperation { /// The operation ID of the request. operation_id: String, /// Permit a number of items to be executed by this operation. - permit: PermitOperations, + _permit: PermitOperations, } impl RegisteredOperation { @@ -217,14 +212,6 @@ impl RegisteredOperation { pub fn operation_id(&self) -> String { self.operation_id.clone() } - - /// Returns the number of reserved elements for this permit. - /// - /// This can be smaller than the number of items requested via [`LimitOperations::reserve()`]. - #[allow(unused)] - pub fn num_reserved(&self) -> usize { - self.permit.num_ops - } } impl Drop for RegisteredOperation { @@ -263,7 +250,7 @@ impl Operations { let operations = self.operations.clone(); operations.lock().insert(operation_id.clone(), (rx, stop_handle.clone())); - Some(RegisteredOperation { stop_handle, operation_id, operations, permit }) + Some(RegisteredOperation { stop_handle, operation_id, operations, _permit: permit }) } /// Get the associated operation state with the ID. From accb056211bece374cde51ad80b48ff45cc34b4b Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 24 Sep 2024 16:59:16 +0200 Subject: [PATCH 14/28] add prdoc --- prdoc/pr_5741.prdoc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 prdoc/pr_5741.prdoc diff --git a/prdoc/pr_5741.prdoc b/prdoc/pr_5741.prdoc new file mode 100644 index 000000000000..45c229693517 --- /dev/null +++ b/prdoc/pr_5741.prdoc @@ -0,0 +1,20 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: make RPC endpoint `chainHead_v1_storage` faster + +doc: + - audience: Node Operator + description: | + The RPC endpoint `chainHead_v1_storage` has removed the static limit for the number + of storage queries instead it has removed all static limits and relies on backpressure. + This should improve the throughput for bigger storage queries significantly. + + Benchmarks using subxt on localhost: + - Iterate over 10 accounts on westend-dev -> ~2-3x faster + - Fetch 1024 storage values (i.e, not descedant values) -> ~50x faster + - Fetch 1024 descendant values -> ~500x faster + +crates: + - name: sc-rpc-spec-v2 + bump: patch From 7a7a75ed3dedfca3b3473fe144047bcf3566fdbd Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 24 Sep 2024 17:01:25 +0200 Subject: [PATCH 15/28] Update prdoc/pr_5741.prdoc --- prdoc/pr_5741.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5741.prdoc b/prdoc/pr_5741.prdoc index 45c229693517..bfac014b141b 100644 --- a/prdoc/pr_5741.prdoc +++ b/prdoc/pr_5741.prdoc @@ -17,4 +17,4 @@ doc: crates: - name: sc-rpc-spec-v2 - bump: patch + bump: major From 6fa42e46ba6fe0ebf4f0507b359be38da801375b Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 24 Sep 2024 17:20:59 +0200 Subject: [PATCH 16/28] prdoc fixes --- prdoc/pr_5741.prdoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/prdoc/pr_5741.prdoc b/prdoc/pr_5741.prdoc index bfac014b141b..3e676f924d56 100644 --- a/prdoc/pr_5741.prdoc +++ b/prdoc/pr_5741.prdoc @@ -18,3 +18,7 @@ doc: crates: - name: sc-rpc-spec-v2 bump: major + - name: sc-rpc-server + bump: patch + - name: sc-service + bump: major From 08699f6177b9562a6dfb08336db648768d04d000 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 24 Sep 2024 15:35:15 +0000 Subject: [PATCH 17/28] ".git/.scripts/commands/fmt/fmt.sh" --- substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index c5f45d54f0d7..da56774beaed 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -575,8 +575,7 @@ where // emit an `InvalidContinue error`. if self.subscriptions.get_operation(&follow_subscription, &operation_id).is_some() { Err(ChainHeadRpcError::InvalidContinue.into()) - } - else { + } else { Ok(()) } } From 9766eb3921cf8d84eee19a10f74939632a0197fc Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 25 Sep 2024 10:04:24 +0200 Subject: [PATCH 18/28] bump tokio for permit api --- Cargo.lock | 28 +++++++++---------- Cargo.toml | 2 +- .../src/chain_head/subscription/inner.rs | 6 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 18517f94788a..cb28fbf9b09f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7148,9 +7148,9 @@ dependencies = [ [[package]] name = "hermit-abi" -version = "0.3.2" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "443144c8cdadd93ebf52ddb4056d257f5b52c04d3c804e657d19eb73fc33668b" +checksum = "d231dfb89cfffdbc30e7fc41579ed6066ad03abda9e567ccafae602b97ec5024" [[package]] name = "hex" @@ -7752,7 +7752,7 @@ version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eae7b9aee968036d54dce06cebaefd919e4472e753296daccd6d344e3e2df0c2" dependencies = [ - "hermit-abi 0.3.2", + "hermit-abi 0.3.9", "libc", "windows-sys 0.48.0", ] @@ -7810,7 +7810,7 @@ version = "0.4.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" dependencies = [ - "hermit-abi 0.3.2", + "hermit-abi 0.3.9", "rustix 0.38.21", "windows-sys 0.48.0", ] @@ -9620,13 +9620,14 @@ dependencies = [ [[package]] name = "mio" -version = "0.8.11" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" +checksum = "80e04d1dcff3aae0704555fe5fee3bcfaf3d1fdf8a7e521d5b9d2b42acb52cec" dependencies = [ + "hermit-abi 0.3.9", "libc", "wasi", - "windows-sys 0.48.0", + "windows-sys 0.52.0", ] [[package]] @@ -10409,7 +10410,7 @@ version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4161fcb6d602d4d2081af7c3a45852d875a03dd337a6bfdd6e06407b61342a43" dependencies = [ - "hermit-abi 0.3.2", + "hermit-abi 0.3.9", "libc", ] @@ -24855,21 +24856,20 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.37.0" +version = "1.40.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1adbebffeca75fcfd058afa480fb6c0b81e165a0323f9c9d39c9697e37c46787" +checksum = "e2b070231665d27ad9ec9b8df639893f46727666c6767db40317fbe920a5d998" dependencies = [ "backtrace", "bytes", "libc", "mio", - "num_cpus", "parking_lot 0.12.3", "pin-project-lite", "signal-hook-registry", "socket2 0.5.7", "tokio-macros", - "windows-sys 0.48.0", + "windows-sys 0.52.0", ] [[package]] @@ -24884,9 +24884,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.2.0" +version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" +checksum = "693d596312e88961bc67d7f1f97af8a70227d9f90c31bba5806eec004978d752" dependencies = [ "proc-macro2 1.0.86", "quote 1.0.37", diff --git a/Cargo.toml b/Cargo.toml index 8e3330e4e614..4fe415a26b0a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1317,7 +1317,7 @@ tikv-jemalloc-ctl = { version = "0.5.0" } tikv-jemallocator = { version = "0.5.0" } time = { version = "0.3" } tiny-keccak = { version = "2.0.2" } -tokio = { version = "1.37.0", default-features = false } +tokio = { version = "1.40.0", default-features = false } tokio-retry = { version = "0.3.0" } tokio-stream = { version = "0.1.14" } tokio-test = { version = "0.4.2" } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs index 84fe478f6524..95a7c7fe1832 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/subscription/inner.rs @@ -1283,12 +1283,12 @@ mod tests { // One operation is reserved. let permit_one = ops.reserve_at_most(1).unwrap(); - assert_eq!(permit_one.num_ops, 1); + assert_eq!(permit_one.num_permits(), 1); // Request 2 operations, however there is capacity only for one. let permit_two = ops.reserve_at_most(2).unwrap(); // Number of reserved permits is smaller than provided. - assert_eq!(permit_two.num_ops, 1); + assert_eq!(permit_two.num_permits(), 1); // Try to reserve operations when there's no space. let permit = ops.reserve_at_most(1); @@ -1299,7 +1299,7 @@ mod tests { // Can reserve again let permit_three = ops.reserve_at_most(1).unwrap(); - assert_eq!(permit_three.num_ops, 1); + assert_eq!(permit_three.num_permits(), 1); } #[test] From af28077bb265f32d427f847f66ed1afe6925ee01 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 26 Sep 2024 16:36:33 +0200 Subject: [PATCH 19/28] simplify the code --- .../client/rpc-spec-v2/src/archive/archive.rs | 9 - .../src/archive/archive_storage.rs | 64 +++-- .../rpc-spec-v2/src/chain_head/chain_head.rs | 81 ++++++- .../src/chain_head/chain_head_storage.rs | 219 ++++++------------ .../rpc-spec-v2/src/chain_head/tests.rs | 33 ++- .../client/rpc-spec-v2/src/common/storage.rs | 188 +++++++-------- substrate/client/service/src/builder.rs | 1 - 7 files changed, 264 insertions(+), 331 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive.rs b/substrate/client/rpc-spec-v2/src/archive/archive.rs index a30e6d419b5c..dd6c566a76ed 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive.rs @@ -30,7 +30,6 @@ use sc_client_api::{ Backend, BlockBackend, BlockchainEvents, CallExecutor, ChildInfo, ExecutorProvider, StorageKey, StorageProvider, }; -use sc_rpc::SubscriptionTaskExecutor; use sp_api::{CallApiAt, CallContext}; use sp_blockchain::{ Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, @@ -89,8 +88,6 @@ pub struct Archive, Block: BlockT, Client> { storage_max_queried_items: usize, /// Phantom member to pin the block type. _phantom: PhantomData, - /// Subscription task executor. - executor: SubscriptionTaskExecutor, } impl, Block: BlockT, Client> Archive { @@ -100,7 +97,6 @@ impl, Block: BlockT, Client> Archive { backend: Arc, genesis_hash: GenesisHash, config: ArchiveConfig, - executor: SubscriptionTaskExecutor, ) -> Self { let genesis_hash = hex_string(&genesis_hash.as_ref()); Self { @@ -110,7 +106,6 @@ impl, Block: BlockT, Client> Archive { storage_max_descendant_responses: config.max_descendant_responses, storage_max_queried_items: config.max_queried_items, _phantom: PhantomData, - executor, } } } @@ -141,9 +136,6 @@ where + CallApiAt + StorageProvider + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { fn archive_unstable_body(&self, hash: Block::Hash) -> RpcResult>> { let Ok(Some(signed_block)) = self.client.block(hash) else { return Ok(None) }; @@ -282,7 +274,6 @@ where self.client.clone(), self.storage_max_descendant_responses, self.storage_max_queried_items, - self.executor.clone(), ); Ok(storage_client.handle_query(hash, items, child_trie)) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index b34dc44a7b2c..132b95623580 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -21,8 +21,7 @@ use std::sync::Arc; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; -use sc_rpc::SubscriptionTaskExecutor; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_runtime::traits::Block as BlockT; use crate::common::{ events::{ArchiveStorageResult, PaginatedStorageQuery, StorageQueryType}, @@ -45,10 +44,9 @@ impl ArchiveStorage { client: Arc, storage_max_descendant_responses: usize, storage_max_queried_items: usize, - executor: SubscriptionTaskExecutor, ) -> Self { Self { - client: Storage::new(client, executor), + client: Storage::new(client), storage_max_descendant_responses, storage_max_queried_items, } @@ -60,9 +58,6 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { /// Generate the response of the `archive_storage` method. pub fn handle_query( @@ -75,8 +70,6 @@ where items.truncate(self.storage_max_queried_items); let mut storage_results = Vec::with_capacity(items.len()); - let mut query_iter = Vec::new(); - for item in items { match item.query_type { StorageQueryType::Value => { @@ -99,39 +92,38 @@ where Err(error) => return ArchiveStorageResult::err(error), }, StorageQueryType::DescendantsValues => { - query_iter.push(QueryIter { - query_key: item.key, - ty: IterQueryType::Value, - pagination_start_key: item.pagination_start_key, - }); + match self.client.query_iter_pagination( + QueryIter { + query_key: item.key, + ty: IterQueryType::Value, + pagination_start_key: item.pagination_start_key, + }, + hash, + child_key.as_ref(), + self.storage_max_descendant_responses, + ) { + Ok((results, _)) => storage_results.extend(results), + Err(error) => return ArchiveStorageResult::err(error), + } }, StorageQueryType::DescendantsHashes => { - query_iter.push(QueryIter { - query_key: item.key, - ty: IterQueryType::Hash, - pagination_start_key: item.pagination_start_key, - }); + match self.client.query_iter_pagination( + QueryIter { + query_key: item.key, + ty: IterQueryType::Hash, + pagination_start_key: item.pagination_start_key, + }, + hash, + child_key.as_ref(), + self.storage_max_descendant_responses, + ) { + Ok((results, _)) => storage_results.extend(results), + Err(error) => return ArchiveStorageResult::err(error), + } }, }; } - if !query_iter.is_empty() { - let mut rx = self.client.query_iter_pagination( - query_iter, - hash, - child_key, - Some(self.storage_max_descendant_responses), - ); - - while let Some(val) = rx.blocking_recv() { - match val { - Ok(Some(value)) => storage_results.push(value), - Ok(None) => continue, - Err(error) => return ArchiveStorageResult::err(error), - } - } - } - ArchiveStorageResult::ok(storage_results, discarded_items) } } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index da56774beaed..3b0b9bb63b50 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -27,10 +27,11 @@ use crate::{ api::ChainHeadApiServer, chain_head_follow::ChainHeadFollower, error::Error as ChainHeadRpcError, - event::{FollowEvent, MethodResponse, OperationError}, - subscription::{SubscriptionManagement, SubscriptionManagementError}, + event::{FollowEvent, MethodResponse, OperationError, OperationId, OperationStorageItems}, + subscription::{StopHandle, SubscriptionManagement, SubscriptionManagementError}, + FollowEventSendError, FollowEventSender, }, - common::events::StorageQuery, + common::{events::StorageQuery, storage::QueryResult}, hex_string, SubscriptionTaskExecutor, }; use codec::Encode; @@ -51,9 +52,16 @@ use sp_core::{traits::CallContext, Bytes}; use sp_rpc::list::ListOrValue; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{marker::PhantomData, sync::Arc, time::Duration}; +use tokio::sync::mpsc; pub(crate) const LOG_TARGET: &str = "rpc-spec-v2"; +/// The buffer capacity for each storage query. +/// +/// This is small because the underlying JSON-RPC server has +/// its down buffer capacity per connection as well. +const STORAGE_QUERY_BUF: usize = 16; + /// The configuration of [`ChainHead`]. pub struct ChainHeadConfig { /// The maximum number of pinned blocks across all subscriptions. @@ -417,12 +425,10 @@ where Err(_) => return ResponsePayload::error(ChainHeadRpcError::InvalidBlock), }; - let mut storage_client = - ChainHeadStorage::::new(self.client.clone(), self.executor.clone()); - let operation = block_guard.operation(); - let operation_id = operation.operation_id(); + let mut storage_client = ChainHeadStorage::::new(self.client.clone()); + + let (rp, rp_fut) = method_started_response(block_guard.operation().operation_id()); - let (rp, rp_fut) = method_started_response(operation_id); let fut = async move { // Wait for the server to send out the response and if it produces an error no event // should be generated. @@ -430,10 +436,20 @@ where return; } - _ = storage_client.generate_events(block_guard, hash, items, child_trie).await; + let (tx, rx) = tokio::sync::mpsc::channel(STORAGE_QUERY_BUF); + let operation_id = block_guard.operation().operation_id(); + let stop_handle = block_guard.operation().stop_handle().clone(); + let response_sender = block_guard.response_sender(); + + // May fail if the channel is closed or the connection is closed. + // which is okay to ignore. + let _ = futures::future::join( + storage_client.generate_events(hash, items, child_trie, tx), + process_storage_items(rx, response_sender, operation_id, &stop_handle), + ) + .await; }; - self.executor - .spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed()); + self.executor.spawn("substrate-rpc-subscription", Some("rpc"), fut.boxed()); rp } @@ -636,3 +652,46 @@ where rx } + +async fn process_storage_items( + mut storage_query_stream: mpsc::Receiver, + mut sender: FollowEventSender, + operation_id: String, + stop_handle: &StopHandle, +) -> Result<(), FollowEventSendError> { + loop { + tokio::select! { + _ = stop_handle.stopped() => { + break; + }, + + maybe_storage = storage_query_stream.recv() => { + let Some(storage) = maybe_storage else { + break; + }; + + let item = match storage { + QueryResult::Err(error) => { + return sender + .send(FollowEvent::OperationError(OperationError { operation_id, error })) + .await + } + QueryResult::Ok(Some(v)) => v, + QueryResult::Ok(None) => continue, + }; + + sender + .send(FollowEvent::OperationStorageItems(OperationStorageItems { + operation_id: operation_id.clone(), + items: vec![item], + })).await?; + }, + } + } + + sender + .send(FollowEvent::OperationStorageDone(OperationId { operation_id })) + .await?; + + Ok(()) +} diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index d74e7985c6a9..f7e9e004f8b5 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -20,22 +20,13 @@ use std::{marker::PhantomData, sync::Arc}; -use futures::SinkExt; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; -use sc_rpc::SubscriptionTaskExecutor; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_runtime::traits::Block as BlockT; use tokio::sync::mpsc; -use crate::{ - chain_head::{ - event::{OperationError, OperationId, OperationStorageItems}, - subscription::{BlockGuard, StopHandle}, - FollowEvent, FollowEventSendError, FollowEventSender, - }, - common::{ - events::{StorageQuery, StorageQueryType}, - storage::{IterQueryType, QueryIter, QueryResult, Storage}, - }, +use crate::common::{ + events::{StorageQuery, StorageQueryType}, + storage::{IterQueryType, QueryIter, QueryResult, Storage}, }; /// Generates the events of the `chainHead_storage` method. @@ -45,10 +36,16 @@ pub struct ChainHeadStorage { _phandom: PhantomData<(BE, Block)>, } +impl Clone for ChainHeadStorage { + fn clone(&self) -> Self { + Self { client: self.client.clone(), _phandom: PhantomData } + } +} + impl ChainHeadStorage { /// Constructs a new [`ChainHeadStorage`]. - pub fn new(client: Arc, executor: SubscriptionTaskExecutor) -> Self { - Self { client: Storage::new(client, executor), _phandom: PhantomData } + pub fn new(client: Arc) -> Self { + Self { client: Storage::new(client), _phandom: PhantomData } } } @@ -57,152 +54,70 @@ where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { - /// Iterate over (key, hash) and (key, value). - async fn generate_storage_iter_events( - &self, - query_iter: Vec, - mut block_guard: BlockGuard, - hash: Block::Hash, - child_key: Option, - ) -> Result<(), FollowEventSendError> { - let stop_handle = block_guard.operation().stop_handle().clone(); - - if stop_handle.is_stopped() { - return Ok(()); - } - - if !query_iter.is_empty() { - process_storage_iter_stream( - self.client.query_iter_pagination(query_iter, hash, child_key, None), - block_guard.response_sender(), - block_guard.operation().operation_id().to_owned(), - &stop_handle, - ) - .await?; - } - - if !stop_handle.is_stopped() { - block_guard - .response_sender() - .send(FollowEvent::OperationStorageDone(OperationId { - operation_id: block_guard.operation().operation_id().to_owned(), - })) - .await?; - } - - Ok(()) - } - /// Generate the block events for the `chainHead_storage` method. pub async fn generate_events( &mut self, - mut block_guard: BlockGuard, hash: Block::Hash, items: Vec>, child_key: Option, - ) -> Result<(), FollowEventSendError> { - let mut iter_ops = Vec::new(); - let mut sender = block_guard.response_sender(); - let operation = block_guard.operation(); - - let mut storage_results = Vec::with_capacity(items.len()); - for item in items { - match item.query_type { - StorageQueryType::Value => { - match self.client.query_value(hash, &item.key, child_key.as_ref()) { - Ok(Some(value)) => storage_results.push(value), - Ok(None) => continue, - Err(error) => { - return send_error(&mut sender, operation.operation_id(), error).await; - }, - } - }, - StorageQueryType::Hash => - match self.client.query_hash(hash, &item.key, child_key.as_ref()) { - Ok(Some(value)) => storage_results.push(value), - Ok(None) => continue, - Err(error) => { - return send_error(&mut sender, operation.operation_id(), error).await; - }, + tx: mpsc::Sender, + ) -> Result<(), tokio::task::JoinError> { + let this = self.clone(); + + tokio::task::spawn_blocking(move || { + for item in items { + match item.query_type { + StorageQueryType::Value => { + let rp = this.client.query_value(hash, &item.key, child_key.as_ref()); + if tx.blocking_send(rp).is_err() { + break; + } }, - StorageQueryType::ClosestDescendantMerkleValue => - match self.client.query_merkle_value(hash, &item.key, child_key.as_ref()) { - Ok(Some(value)) => storage_results.push(value), - Ok(None) => continue, - Err(error) => { - return send_error(&mut sender, operation.operation_id(), error).await; - }, + StorageQueryType::Hash => { + let rp = this.client.query_hash(hash, &item.key, child_key.as_ref()); + if tx.blocking_send(rp).is_err() { + break; + } }, - StorageQueryType::DescendantsValues => iter_ops.push(QueryIter { - query_key: item.key, - ty: IterQueryType::Value, - pagination_start_key: None, - }), - StorageQueryType::DescendantsHashes => iter_ops.push(QueryIter { - query_key: item.key, - ty: IterQueryType::Hash, - pagination_start_key: None, - }), - }; - } - - if !storage_results.is_empty() { - sender - .send(FollowEvent::::OperationStorageItems(OperationStorageItems { - operation_id: operation.operation_id(), - items: storage_results, - })) - .await?; - } - - self.generate_storage_iter_events(iter_ops, block_guard, hash, child_key).await - } -} - -/// Build and send the opaque error back to the `chainHead_follow` method. -async fn send_error( - sender: &mut FollowEventSender, - operation_id: String, - error: String, -) -> Result<(), FollowEventSendError> { - sender - .send(FollowEvent::OperationError(OperationError { operation_id, error })) - .await -} - -async fn process_storage_iter_stream( - mut storage_query_stream: mpsc::Receiver, - mut sender: FollowEventSender, - operation_id: String, - stop_handle: &StopHandle, -) -> Result<(), FollowEventSendError> { - loop { - tokio::select! { - _ = stop_handle.stopped() => return Ok(()), - maybe_storage = storage_query_stream.recv() => { - let Some(storage) = maybe_storage else { - return Ok(()); - }; - - let item = match storage { - QueryResult::Err(error) => { - return send_error(&mut sender, operation_id.clone(), error).await; - } - QueryResult::Ok(Some(v)) => v, - QueryResult::Ok(None) => continue, - }; - - sender - .send(FollowEvent::OperationStorageItems(OperationStorageItems { - operation_id: operation_id.clone(), - items: vec![item], - })).await?; + StorageQueryType::ClosestDescendantMerkleValue => { + let rp = + this.client.query_merkle_value(hash, &item.key, child_key.as_ref()); + if tx.blocking_send(rp).is_err() { + break; + } + }, + StorageQueryType::DescendantsValues => { + let query = QueryIter { + query_key: item.key, + ty: IterQueryType::Value, + pagination_start_key: None, + }; + this.client.query_iter_pagination_with_producer( + query, + hash, + child_key.as_ref(), + &tx, + ) + }, + StorageQueryType::DescendantsHashes => { + let query = QueryIter { + query_key: item.key, + ty: IterQueryType::Hash, + pagination_start_key: None, + }; + this.client.query_iter_pagination_with_producer( + query, + hash, + child_key.as_ref(), + &tx, + ) + }, + } + } + }) + .await?; - }, - } + Ok(()) } } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index 4102ee8fcab4..7f81c267f4b6 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -3213,30 +3213,23 @@ async fn storage_closest_merkle_value() { MethodResponse::LimitReached => panic!("Expected started response"), }; - let event = get_next_event::>(&mut sub).await; - let merkle_values: HashMap<_, _> = match event { - FollowEvent::OperationStorageItems(res) => { - assert_eq!(res.operation_id, operation_id); - - res.items - .into_iter() - .map(|res| { + let mut merkle_values = HashMap::new(); + + loop { + match get_next_event::>(&mut sub).await { + FollowEvent::OperationStorageItems(res) if res.operation_id == operation_id => + for res in res.items { let value = match res.result { StorageResultType::ClosestDescendantMerkleValue(value) => value, _ => panic!("Unexpected StorageResultType"), }; - (res.key, value) - }) - .collect() - }, - _ => panic!("Expected OperationStorageItems event"), - }; - - // Finished. - assert_matches!( - get_next_event::>(&mut sub).await, - FollowEvent::OperationStorageDone(done) if done.operation_id == operation_id - ); + merkle_values.insert(res.key, value); + }, + FollowEvent::OperationStorageDone(done) if done.operation_id == operation_id => + break, + _ => panic!("Unexpected event"), + } + } // Response for AAAA, AAAB, A and AA. assert_eq!(merkle_values.len(), 4); diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index bd4dbb75cc19..0328f12abbd0 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -20,39 +20,30 @@ use std::{marker::PhantomData, sync::Arc}; -use futures::{stream::FuturesUnordered, StreamExt}; use sc_client_api::{Backend, ChildInfo, StorageKey, StorageProvider}; -use sc_rpc::SubscriptionTaskExecutor; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_runtime::traits::Block as BlockT; use tokio::sync::mpsc; use super::events::{StorageResult, StorageResultType}; use crate::hex_string; -/// The buffer capacity for `Storage::query_iter_pagination`. -/// -/// This is small because the underlying JSON-RPC server has -/// its down buffer capacity per connection as well. -const QUERY_ITER_PAGINATED_BUF_CAP: usize = 16; - /// Call into the storage of blocks. pub struct Storage { /// Substrate client. client: Arc, - executor: SubscriptionTaskExecutor, _phandom: PhantomData<(BE, Block)>, } impl Clone for Storage { fn clone(&self) -> Self { - Self { client: self.client.clone(), executor: self.executor.clone(), _phandom: PhantomData } + Self { client: self.client.clone(), _phandom: PhantomData } } } impl Storage { /// Constructs a new [`Storage`]. - pub fn new(client: Arc, executor: SubscriptionTaskExecutor) -> Self { - Self { client, _phandom: PhantomData, executor } + pub fn new(client: Arc) -> Self { + Self { client, _phandom: PhantomData } } } @@ -79,14 +70,14 @@ pub enum IterQueryType { /// The result of making a query call. pub type QueryResult = Result, String>; +/// The result of iterating over keys. +pub type QueryIterResult = Result<(Vec, Option), String>; + impl Storage where Block: BlockT + Send + 'static, BE: Backend + Send + 'static, Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { /// Fetch the value from storage. pub fn query_value( @@ -164,104 +155,97 @@ where .unwrap_or_else(|error| QueryResult::Err(error.to_string())) } - /// Iterate over the storage which returns a stream that receive the results of the - /// query. + /// Iterate over the storage keys and send the results to the provided sender. /// - /// Internally this relies on a bounded channel which provides backpressure which needs - /// propagated down the underlying client. - /// - /// For users of this API, if you can't rely on backpressure then you can - /// use `max_iterations` to limit the number of iterations. - pub fn query_iter_pagination( + /// Because this relies on a bounded channel, it will pause the storage iteration + // if the channel is becomes full which in turn provides backpressure. + pub fn query_iter_pagination_with_producer( &self, - queries: Vec, + query: QueryIter, hash: Block::Hash, - child_key: Option, - max_iterations: Option, - ) -> mpsc::Receiver { - let (tx, rx) = mpsc::channel(QUERY_ITER_PAGINATED_BUF_CAP); - let storage = self.clone(); - - let pending_queries = async move { - let queries: FuturesUnordered<_> = queries - .into_iter() - .map(|query| { - query_iter_pagination_one( - &storage, - query, - hash, - child_key.as_ref(), - &tx, - max_iterations, - ) - }) - .collect(); - - queries.for_each(|_| async {}).await; + child_key: Option<&ChildInfo>, + tx: &mpsc::Sender, + ) { + let QueryIter { ty, query_key, pagination_start_key } = query; + + let maybe_storage = if let Some(child_key) = child_key { + self.client.child_storage_keys( + hash, + child_key.to_owned(), + Some(&query_key), + pagination_start_key.as_ref(), + ) + } else { + self.client.storage_keys(hash, Some(&query_key), pagination_start_key.as_ref()) }; - self.executor.spawn_blocking( - "substrate-rpc-subscription", - Some("rpc"), - Box::pin(pending_queries), - ); + let keys_iter = match maybe_storage { + Ok(keys_iter) => keys_iter, + Err(error) => { + _ = tx.blocking_send(Err(error.to_string())); + return; + }, + }; - rx - } -} + for key in keys_iter { + let result = match ty { + IterQueryType::Value => self.query_value(hash, &key, child_key), + IterQueryType::Hash => self.query_hash(hash, &key, child_key), + }; -async fn query_iter_pagination_one( - storage: &Storage, - query: QueryIter, - hash: Block::Hash, - child_key: Option<&ChildInfo>, - tx: &mpsc::Sender, - max_iterations: Option, -) where - Block: BlockT + Send + 'static, - BE: Backend + Send + 'static, - Client: StorageProvider + Send + Sync + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: std::marker::Send, -{ - let QueryIter { ty, query_key, pagination_start_key } = query; - - let maybe_storage = if let Some(child_key) = child_key { - storage.client.child_storage_keys( - hash, - child_key.to_owned(), - Some(&query_key), - pagination_start_key.as_ref(), - ) - } else { - storage - .client - .storage_keys(hash, Some(&query_key), pagination_start_key.as_ref()) - }; - - let keys_iter = match maybe_storage { - Ok(keys_iter) => keys_iter, - Err(error) => { - _ = tx.send(Err(error.to_string())).await; - return; - }, - }; - - for (count, key) in keys_iter.into_iter().enumerate() { - if let Some(max_iterations) = max_iterations { - if count >= max_iterations { + if tx.blocking_send(result).is_err() { break; } } + } - let result = match ty { - IterQueryType::Value => storage.query_value(hash, &key, child_key), - IterQueryType::Hash => storage.query_hash(hash, &key, child_key), - }; + /// Iterate over at most the provided number of keys. + /// + /// Returns the storage result with a potential next key to resume iteration. + pub fn query_iter_pagination( + &self, + query: QueryIter, + hash: Block::Hash, + child_key: Option<&ChildInfo>, + count: usize, + ) -> QueryIterResult { + let QueryIter { ty, query_key, pagination_start_key } = query; + + let mut keys_iter = if let Some(child_key) = child_key { + self.client.child_storage_keys( + hash, + child_key.to_owned(), + Some(&query_key), + pagination_start_key.as_ref(), + ) + } else { + self.client.storage_keys(hash, Some(&query_key), pagination_start_key.as_ref()) + } + .map_err(|err| err.to_string())?; + + let mut ret = Vec::with_capacity(count); + let mut next_pagination_key = None; + for _ in 0..count { + let Some(key) = keys_iter.next() else { break }; + + next_pagination_key = Some(key.clone()); - if tx.send(result).await.is_err() { - break; + let result = match ty { + IterQueryType::Value => self.query_value(hash, &key, child_key), + IterQueryType::Hash => self.query_hash(hash, &key, child_key), + }?; + + if let Some(value) = result { + ret.push(value); + } } + + // Save the next key if any to continue the iteration. + let maybe_next_query = keys_iter.next().map(|_| QueryIter { + ty, + query_key, + pagination_start_key: next_pagination_key, + }); + Ok((ret, maybe_next_query)) } } diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index 2bd30eef4887..fa32749bbe22 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -736,7 +736,6 @@ where genesis_hash, // Defaults to sensible limits for the `Archive`. sc_rpc_spec_v2::archive::ArchiveConfig::default(), - task_executor.clone(), ) .into_rpc(); rpc_api.merge(archive_v2).map_err(|e| Error::Application(e.into()))?; From 737e8589387388d7548b9f4114c16122403b35eb Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 27 Sep 2024 10:53:02 +0200 Subject: [PATCH 20/28] fix tests --- substrate/client/rpc-spec-v2/Cargo.toml | 2 + .../client/rpc-spec-v2/src/archive/tests.rs | 3 +- .../rpc-spec-v2/src/chain_head/tests.rs | 50 +++++++++---------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index 5537ac045d77..644af61401c8 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -55,7 +55,9 @@ sp-externalities = { workspace = true, default-features = true } sp-maybe-compressed-blob = { workspace = true, default-features = true } sc-block-builder = { workspace = true, default-features = true } sc-service = { features = ["test-helpers"], workspace = true, default-features = true } +sc-rpc = { workspace = true, default-features = true, features = ["test-helpers"] } assert_matches = { workspace = true } pretty_assertions = { workspace = true } sc-transaction-pool = { workspace = true, default-features = true } sc-utils = { workspace = true, default-features = true } +#tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } diff --git a/substrate/client/rpc-spec-v2/src/archive/tests.rs b/substrate/client/rpc-spec-v2/src/archive/tests.rs index e14e6df76c45..078016f5b3e2 100644 --- a/substrate/client/rpc-spec-v2/src/archive/tests.rs +++ b/substrate/client/rpc-spec-v2/src/archive/tests.rs @@ -38,7 +38,7 @@ use sc_block_builder::BlockBuilderBuilder; use sc_client_api::ChildInfo; use sp_blockchain::HeaderBackend; use sp_consensus::BlockOrigin; -use sp_core::{testing::TaskExecutor, Blake2Hasher, Hasher}; +use sp_core::{Blake2Hasher, Hasher}; use sp_runtime::{ traits::{Block as BlockT, Header as HeaderT}, SaturatedConversion, @@ -79,7 +79,6 @@ fn setup_api( backend, CHAIN_GENESIS, ArchiveConfig { max_descendant_responses, max_queried_items }, - Arc::new(TaskExecutor::new()), ) .into_rpc(); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index 7f81c267f4b6..ac92800438ac 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -34,11 +34,11 @@ use jsonrpsee::{ use sc_block_builder::BlockBuilderBuilder; use sc_client_api::ChildInfo; use sc_service::client::new_in_mem; +use sc_rpc::testing::TokioTestExecutor; use sp_blockchain::HeaderBackend; use sp_consensus::BlockOrigin; use sp_core::{ storage::well_known_keys::{self, CODE}, - testing::TaskExecutor, Blake2Hasher, Hasher, }; use sp_runtime::traits::Block as BlockT; @@ -79,7 +79,7 @@ pub async fn run_server() -> std::net::SocketAddr { let api = ChainHead::new( client, backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -140,7 +140,7 @@ async fn setup_api() -> ( let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -247,7 +247,7 @@ async fn follow_subscription_produces_blocks() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -316,7 +316,7 @@ async fn follow_with_runtime() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -624,7 +624,7 @@ async fn call_runtime_without_flag() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1283,7 +1283,7 @@ async fn separate_operation_ids_for_subscriptions() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1369,7 +1369,7 @@ async fn follow_generates_initial_blocks() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1525,7 +1525,7 @@ async fn follow_exceeding_pinned_blocks() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: 2, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1602,7 +1602,7 @@ async fn follow_with_unpin() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: 2, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1708,7 +1708,7 @@ async fn unpin_duplicate_hashes() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: 3, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1811,7 +1811,7 @@ async fn follow_with_multiple_unpin_hashes() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -1956,7 +1956,7 @@ async fn follow_prune_best_block() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -2142,7 +2142,7 @@ async fn follow_forks_pruned_block() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -2302,7 +2302,7 @@ async fn follow_report_multiple_pruned_block() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -2539,7 +2539,7 @@ async fn pin_block_references() { genesis_block_builder, None, None, - Box::new(TaskExecutor::new()), + Box::new(TokioTestExecutor::default()), client_config, ) .unwrap(), @@ -2548,7 +2548,7 @@ async fn pin_block_references() { let api = ChainHead::new( client.clone(), backend.clone(), - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: 3, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -2683,7 +2683,7 @@ async fn follow_finalized_before_new_block() { let api = ChainHead::new( client_mock.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -2798,7 +2798,7 @@ async fn ensure_operation_limits_works() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -2903,7 +2903,7 @@ async fn storage_is_backpressured() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -3040,7 +3040,7 @@ async fn stop_storage_operation() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -3337,7 +3337,7 @@ async fn chain_head_stop_all_subscriptions() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -3550,7 +3550,7 @@ async fn chain_head_limit_reached() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -3590,7 +3590,7 @@ async fn follow_unique_pruned_blocks() { let api = ChainHead::new( client.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), @@ -3759,7 +3759,7 @@ async fn follow_report_best_block_of_a_known_block() { let api = ChainHead::new( client_mock.clone(), backend, - Arc::new(TaskExecutor::default()), + Arc::new(TokioTestExecutor::default()), ChainHeadConfig { global_max_pinned_blocks: MAX_PINNED_BLOCKS, subscription_max_pinned_duration: Duration::from_secs(MAX_PINNED_SECS), From 797f1da55042e8ad88ac213732e03ba3ff4a755a Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 27 Sep 2024 11:15:57 +0200 Subject: [PATCH 21/28] cargo fmt --- substrate/client/rpc-spec-v2/src/chain_head/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index ac92800438ac..44a2849d9153 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -33,8 +33,8 @@ use jsonrpsee::{ }; use sc_block_builder::BlockBuilderBuilder; use sc_client_api::ChildInfo; -use sc_service::client::new_in_mem; use sc_rpc::testing::TokioTestExecutor; +use sc_service::client::new_in_mem; use sp_blockchain::HeaderBackend; use sp_consensus::BlockOrigin; use sp_core::{ From 381fcc0a0dc4a9b5f1317031fe56cfee2cbc7a97 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Fri, 27 Sep 2024 09:56:37 +0000 Subject: [PATCH 22/28] ".git/.scripts/commands/fmt/fmt.sh" --- bridges/modules/xcm-bridge-hub/src/lib.rs | 37 +++++++++---------- .../src/cli/relay_headers_and_messages/mod.rs | 4 +- .../src/on_demand/parachains.rs | 3 +- .../messages/src/message_race_strategy.rs | 6 +-- .../primitives/router/src/inbound/mod.rs | 6 +-- .../primitives/router/src/outbound/mod.rs | 3 +- .../runtime/runtime-common/src/lib.rs | 3 +- cumulus/pallets/parachain-system/src/lib.rs | 3 +- .../asset-hub-rococo/src/tests/teleport.rs | 4 +- .../asset-hub-westend/src/tests/teleport.rs | 4 +- .../people-rococo/src/tests/teleport.rs | 4 +- .../people-westend/src/tests/teleport.rs | 4 +- .../assets/asset-hub-rococo/src/lib.rs | 3 +- .../assets/asset-hub-westend/src/lib.rs | 3 +- .../assets/test-utils/src/test_cases.rs | 3 +- .../bridge-hub-westend/tests/tests.rs | 32 ++++++---------- .../test-utils/src/test_cases/mod.rs | 11 +++--- cumulus/primitives/utility/src/lib.rs | 6 +-- .../core/approval-voting-parallel/src/lib.rs | 3 +- polkadot/node/core/approval-voting/src/lib.rs | 7 +--- .../node/core/approval-voting/src/tests.rs | 3 +- .../common/src/worker/security/change_root.rs | 3 +- .../approval-distribution/src/tests.rs | 3 +- .../src/task/strategy/chunks.rs | 7 ++-- .../subsystem-bench/src/lib/statement/mod.rs | 16 ++++---- .../subsystem-types/src/runtime_client.rs | 3 +- polkadot/runtime/parachains/src/hrmp.rs | 2 +- .../parachains/src/paras_inherent/mod.rs | 25 ++++++------- polkadot/runtime/parachains/src/ump_tests.rs | 7 ++-- polkadot/runtime/westend/src/lib.rs | 3 +- polkadot/statement-table/src/generic.rs | 6 +-- .../parachain/xcm_config.rs | 2 +- .../relay_chain/xcm_config.rs | 2 +- polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs | 2 +- .../xcm/xcm-builder/src/asset_conversion.rs | 8 +--- .../xcm-builder/src/nonfungibles_adapter.rs | 9 +---- .../xcm-runtime-apis/tests/fee_estimation.rs | 2 +- .../client/cli/src/params/node_key_params.rs | 4 +- .../consensus/grandpa/src/aux_schema.rs | 4 +- .../src/protocol/notifications/behaviour.rs | 3 +- substrate/client/network/sync/src/engine.rs | 3 +- .../rpc-spec-v2/src/chain_head/test_utils.rs | 3 +- substrate/frame/bags-list/src/list/tests.rs | 2 +- .../balances/src/tests/currency_tests.rs | 6 +-- substrate/frame/bounties/src/lib.rs | 12 +++--- substrate/frame/child-bounties/src/lib.rs | 13 +++---- .../examples/offchain-worker/src/tests.rs | 22 +++++------ substrate/frame/nis/src/lib.rs | 16 ++++---- substrate/frame/referenda/src/types.rs | 3 +- substrate/frame/revive/proc-macro/src/lib.rs | 25 ++++++------- substrate/frame/society/src/tests.rs | 2 +- substrate/frame/staking/src/tests.rs | 2 +- .../procedural/src/pallet/parse/call.rs | 23 ++++++------ .../support/src/storage/types/double_map.rs | 3 +- .../traits/try_runtime/decode_entire_state.rs | 6 +-- substrate/frame/support/test/tests/pallet.rs | 5 +-- .../frame/transaction-payment/src/tests.rs | 6 +-- substrate/frame/utility/src/lib.rs | 4 +- substrate/frame/vesting/src/tests.rs | 10 ++--- .../utils/wasm-builder/src/wasm_project.rs | 7 ++-- 60 files changed, 179 insertions(+), 257 deletions(-) diff --git a/bridges/modules/xcm-bridge-hub/src/lib.rs b/bridges/modules/xcm-bridge-hub/src/lib.rs index 1b2536598a20..b9a3f6a4d3c2 100644 --- a/bridges/modules/xcm-bridge-hub/src/lib.rs +++ b/bridges/modules/xcm-bridge-hub/src/lib.rs @@ -1455,26 +1455,25 @@ mod tests { let lane_id = TestLaneIdType::try_new(1, 2).unwrap(); let lane_id_mismatch = TestLaneIdType::try_new(3, 4).unwrap(); - let test_bridge_state = - |id, - bridge, - (lane_id, bridge_id), - (inbound_lane_id, outbound_lane_id), - expected_error: Option| { - Bridges::::insert(id, bridge); - LaneToBridge::::insert(lane_id, bridge_id); + let test_bridge_state = |id, + bridge, + (lane_id, bridge_id), + (inbound_lane_id, outbound_lane_id), + expected_error: Option| { + Bridges::::insert(id, bridge); + LaneToBridge::::insert(lane_id, bridge_id); - let lanes_manager = LanesManagerOf::::new(); - lanes_manager.create_inbound_lane(inbound_lane_id).unwrap(); - lanes_manager.create_outbound_lane(outbound_lane_id).unwrap(); - - let result = XcmOverBridge::do_try_state(); - if let Some(e) = expected_error { - assert_err!(result, e); - } else { - assert_ok!(result); - } - }; + let lanes_manager = LanesManagerOf::::new(); + lanes_manager.create_inbound_lane(inbound_lane_id).unwrap(); + lanes_manager.create_outbound_lane(outbound_lane_id).unwrap(); + + let result = XcmOverBridge::do_try_state(); + if let Some(e) = expected_error { + assert_err!(result, e); + } else { + assert_ok!(result); + } + }; let cleanup = |bridge_id, lane_ids| { Bridges::::remove(bridge_id); for lane_id in lane_ids { diff --git a/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs b/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs index e875c53a2e7e..9261dc437536 100644 --- a/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs @@ -325,9 +325,7 @@ where _, Self::Left, MessagesLaneIdOf, - >( - common.right.client.clone(), &common.metrics_params, &common.right.accounts, &lanes - ) + >(common.right.client.clone(), &common.metrics_params, &common.right.accounts, &lanes) .await?; } diff --git a/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs b/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs index 96eba0af988c..2ef86f48ecbe 100644 --- a/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -664,8 +664,7 @@ impl<'a, P: SubstrateParachainsPipeline, SourceRelayClnt, TargetClnt> for ( &'a OnDemandParachainsRelay, &'a ParachainsSource, - ) -where + ) where SourceRelayClnt: Client, TargetClnt: Client, { diff --git a/bridges/relays/messages/src/message_race_strategy.rs b/bridges/relays/messages/src/message_race_strategy.rs index 1303fcfedebd..3a532331d79d 100644 --- a/bridges/relays/messages/src/message_race_strategy.rs +++ b/bridges/relays/messages/src/message_race_strategy.rs @@ -67,8 +67,7 @@ impl< TargetHeaderHash, SourceNoncesRange, Proof, - > -where + > where SourceHeaderHash: Clone, SourceHeaderNumber: Clone + Ord, SourceNoncesRange: NoncesRange, @@ -190,8 +189,7 @@ impl< TargetHeaderHash, SourceNoncesRange, Proof, - > -where + > where SourceHeaderHash: Clone + Debug + Send + Sync, SourceHeaderNumber: Clone + Ord + Debug + Send + Sync, SourceNoncesRange: NoncesRange + Debug + Send + Sync, diff --git a/bridges/snowbridge/primitives/router/src/inbound/mod.rs b/bridges/snowbridge/primitives/router/src/inbound/mod.rs index fbfc52d01c83..a10884b45531 100644 --- a/bridges/snowbridge/primitives/router/src/inbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/inbound/mod.rs @@ -168,8 +168,7 @@ impl< ConvertAssetId, EthereumUniversalLocation, GlobalAssetHubLocation, - > -where + > where CreateAssetCall: Get, CreateAssetDeposit: Get, InboundQueuePalletInstance: Get, @@ -227,8 +226,7 @@ impl< ConvertAssetId, EthereumUniversalLocation, GlobalAssetHubLocation, - > -where + > where CreateAssetCall: Get, CreateAssetDeposit: Get, InboundQueuePalletInstance: Get, diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index b23bce0e2185..d3b6c116dd7a 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/mod.rs @@ -44,8 +44,7 @@ impl -where + > where UniversalLocation: Get, EthereumNetwork: Get, OutboundQueue: SendMessage, diff --git a/bridges/snowbridge/runtime/runtime-common/src/lib.rs b/bridges/snowbridge/runtime/runtime-common/src/lib.rs index 0b1a74b232a0..aae45520ff4b 100644 --- a/bridges/snowbridge/runtime/runtime-common/src/lib.rs +++ b/bridges/snowbridge/runtime/runtime-common/src/lib.rs @@ -50,8 +50,7 @@ impl -where + > where Balance: BaseArithmetic + Unsigned + Copy + From + Into + Debug, AccountId: Clone + FullCodec, FeeAssetLocation: Get, diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 21af35fe3de3..9dc41aa03d9b 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -390,8 +390,7 @@ pub mod pallet { let maximum_channels = host_config .hrmp_max_message_num_per_candidate - .min(>::take()) - as usize; + .min(>::take()) as usize; // Note: this internally calls the `GetChannelInfo` implementation for this // pallet, which draws on the `RelevantMessagingState`. That in turn has diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs index 470b4d0f389e..d6cf819118e9 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs @@ -265,9 +265,7 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = AssetHubRococo::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >( - test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest - ) + >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) }); // Sender's balance is reduced diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs index ee0f297792f8..ddb82a954a87 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs @@ -265,9 +265,7 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = AssetHubWestend::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >( - test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest - ) + >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) }); // Sender's balance is reduced diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs index 2619ca7591d0..44e6b3934f0e 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs @@ -107,9 +107,7 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = PeopleRococo::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >( - test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest - ) + >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) }); // Sender's balance is reduced diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs index d9a2c23ac0c6..83888031723f 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs @@ -107,9 +107,7 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = PeopleWestend::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >( - test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest - ) + >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) }); // Sender's balance is reduced diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 430c5bd18561..af5d1ce17bf3 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -545,8 +545,7 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | - RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 8565cd30f430..bd7780339267 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -543,8 +543,7 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | - RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( diff --git a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs index c80222142304..67b585ecfe86 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs @@ -1143,8 +1143,7 @@ pub fn create_and_manage_foreign_assets_for_local_consensus_parachain_assets_wor .with_balances(vec![( foreign_creator_as_account_id.clone(), existential_deposit + - asset_deposit + - metadata_deposit_base + + asset_deposit + metadata_deposit_base + metadata_deposit_per_byte_eta + buy_execution_fee_amount.into() + buy_execution_fee_amount.into(), diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs index d168d1b7a13c..4ff388f4ba2b 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs @@ -301,16 +301,12 @@ fn relayed_incoming_message_works() { XcmOverBridgeHubRococoInstance, LocationToAccountId, WestendLocation, - >( - SiblingParachainLocation::get(), - BridgedUniversalLocation::get(), - |locations, fee| { - bridge_hub_test_utils::open_bridge_with_storage::< - Runtime, - XcmOverBridgeHubRococoInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) - }, - ) + >(SiblingParachainLocation::get(), BridgedUniversalLocation::get(), |locations, fee| { + bridge_hub_test_utils::open_bridge_with_storage::< + Runtime, + XcmOverBridgeHubRococoInstance, + >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + }) .1 }, construct_and_apply_extrinsic, @@ -335,16 +331,12 @@ fn free_relay_extrinsic_works() { XcmOverBridgeHubRococoInstance, LocationToAccountId, WestendLocation, - >( - SiblingParachainLocation::get(), - BridgedUniversalLocation::get(), - |locations, fee| { - bridge_hub_test_utils::open_bridge_with_storage::< - Runtime, - XcmOverBridgeHubRococoInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) - }, - ) + >(SiblingParachainLocation::get(), BridgedUniversalLocation::get(), |locations, fee| { + bridge_hub_test_utils::open_bridge_with_storage::< + Runtime, + XcmOverBridgeHubRococoInstance, + >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + }) .1 }, construct_and_apply_extrinsic, diff --git a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs index 9c5d6269dc0e..663558f5fd5c 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs @@ -506,12 +506,11 @@ pub fn message_dispatch_routing_works< // 2. this message is sent from other global consensus with destination of this Runtime // sibling parachain (HRMP) - let bridging_message = - test_data::simulate_message_exporter_on_bridged_chain::< - BridgedNetwork, - NetworkWithParentCount, - AlwaysLatest, - >((RuntimeNetwork::get(), [Parachain(sibling_parachain_id)].into())); + let bridging_message = test_data::simulate_message_exporter_on_bridged_chain::< + BridgedNetwork, + NetworkWithParentCount, + AlwaysLatest, + >((RuntimeNetwork::get(), [Parachain(sibling_parachain_id)].into())); // 2.1. WITHOUT opened hrmp channel -> RoutingError let result = diff --git a/cumulus/primitives/utility/src/lib.rs b/cumulus/primitives/utility/src/lib.rs index 8530f5b87487..6bd14d136a62 100644 --- a/cumulus/primitives/utility/src/lib.rs +++ b/cumulus/primitives/utility/src/lib.rs @@ -385,8 +385,7 @@ impl< FungiblesAssetMatcher, OnUnbalanced, AccountId, - > -where + > where Fungibles::Balance: Into, { fn new() -> Self { @@ -546,8 +545,7 @@ impl< FungiblesAssetMatcher, OnUnbalanced, AccountId, - > -where + > where Fungibles::Balance: Into, { fn drop(&mut self) { diff --git a/polkadot/node/core/approval-voting-parallel/src/lib.rs b/polkadot/node/core/approval-voting-parallel/src/lib.rs index 1a7ef756bdfc..18c73cfba1fc 100644 --- a/polkadot/node/core/approval-voting-parallel/src/lib.rs +++ b/polkadot/node/core/approval-voting-parallel/src/lib.rs @@ -878,8 +878,7 @@ pub struct ApprovalVotingToApprovalDistribution> - overseer::SubsystemSender - for ApprovalVotingToApprovalDistribution + overseer::SubsystemSender for ApprovalVotingToApprovalDistribution { #[allow(clippy::type_complexity, clippy::type_repetition_in_bounds)] fn send_message<'life0, 'async_trait>( diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs index 835098293050..2a1861c72341 100644 --- a/polkadot/node/core/approval-voting/src/lib.rs +++ b/polkadot/node/core/approval-voting/src/lib.rs @@ -2465,12 +2465,7 @@ fn schedule_wakeup_action( last_assignment_tick.map(|l| l + APPROVAL_DELAY).filter(|t| t > &tick_now), next_no_show, ) - .map(|tick| Action::ScheduleWakeup { - block_hash, - block_number, - candidate_hash, - tick, - }) + .map(|tick| Action::ScheduleWakeup { block_hash, block_number, candidate_hash, tick }) }, RequiredTranches::Pending { considered, next_no_show, clock_drift, .. } => { // select the minimum of `next_no_show`, or the tick of the next non-empty tranche diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs index db5ffd441c0d..56e5ead0808f 100644 --- a/polkadot/node/core/approval-voting/src/tests.rs +++ b/polkadot/node/core/approval-voting/src/tests.rs @@ -254,8 +254,7 @@ where _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, _assignment: &polkadot_node_primitives::approval::v2::AssignmentCertV2, _backing_groups: Vec, - ) -> Result - { + ) -> Result { self.1(validator_index) } } diff --git a/polkadot/node/core/pvf/common/src/worker/security/change_root.rs b/polkadot/node/core/pvf/common/src/worker/security/change_root.rs index fcfaf6541c29..9ec66906819f 100644 --- a/polkadot/node/core/pvf/common/src/worker/security/change_root.rs +++ b/polkadot/node/core/pvf/common/src/worker/security/change_root.rs @@ -124,8 +124,7 @@ fn try_restrict(worker_info: &WorkerInfo) -> Result<()> { libc::MS_BIND | libc::MS_REC | libc::MS_NOEXEC | libc::MS_NODEV | libc::MS_NOSUID | - libc::MS_NOATIME | - additional_flags, + libc::MS_NOATIME | additional_flags, ptr::null(), // ignored when MS_BIND is used ) < 0 { diff --git a/polkadot/node/network/approval-distribution/src/tests.rs b/polkadot/node/network/approval-distribution/src/tests.rs index 063e71f2f528..068559dea762 100644 --- a/polkadot/node/network/approval-distribution/src/tests.rs +++ b/polkadot/node/network/approval-distribution/src/tests.rs @@ -535,8 +535,7 @@ impl AssignmentCriteria for MockAssignmentCriteria { _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, _assignment: &polkadot_node_primitives::approval::v2::AssignmentCertV2, _backing_groups: Vec, - ) -> Result - { + ) -> Result { self.tranche } } diff --git a/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs b/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs index 6b34538b6266..8ea229dbf021 100644 --- a/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs +++ b/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs @@ -107,10 +107,9 @@ impl FetchChunks { state: &mut State, common_params: &RecoveryParams, ) -> Result { - let recovery_duration = - common_params - .metrics - .time_erasure_recovery(RecoveryStrategy::::strategy_type(self)); + let recovery_duration = common_params + .metrics + .time_erasure_recovery(RecoveryStrategy::::strategy_type(self)); // Send request to reconstruct available data from chunks. let (avilable_data_tx, available_data_rx) = oneshot::channel(); diff --git a/polkadot/node/subsystem-bench/src/lib/statement/mod.rs b/polkadot/node/subsystem-bench/src/lib/statement/mod.rs index dd7095d3b00c..e2d50f285687 100644 --- a/polkadot/node/subsystem-bench/src/lib/statement/mod.rs +++ b/polkadot/node/subsystem-bench/src/lib/statement/mod.rs @@ -114,14 +114,14 @@ fn build_overseer( state.pvd.clone(), state.own_backing_group.clone(), ); - let (statement_req_receiver, statement_req_cfg) = - IncomingRequest::get_config_receiver::>( - &ReqProtocolNames::new(GENESIS_HASH, None), - ); - let (candidate_req_receiver, candidate_req_cfg) = - IncomingRequest::get_config_receiver::>( - &ReqProtocolNames::new(GENESIS_HASH, None), - ); + let (statement_req_receiver, statement_req_cfg) = IncomingRequest::get_config_receiver::< + Block, + sc_network::NetworkWorker, + >(&ReqProtocolNames::new(GENESIS_HASH, None)); + let (candidate_req_receiver, candidate_req_cfg) = IncomingRequest::get_config_receiver::< + Block, + sc_network::NetworkWorker, + >(&ReqProtocolNames::new(GENESIS_HASH, None)); let keystore = make_keystore(); let subsystem = StatementDistributionSubsystem::new( keystore.clone(), diff --git a/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index a8af8b7996f9..7938223df23b 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -665,8 +665,7 @@ where fn number( &self, hash: Block::Hash, - ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> - { + ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> { self.client.number(hash) } diff --git a/polkadot/runtime/parachains/src/hrmp.rs b/polkadot/runtime/parachains/src/hrmp.rs index 220543f00ec3..b149404b41b8 100644 --- a/polkadot/runtime/parachains/src/hrmp.rs +++ b/polkadot/runtime/parachains/src/hrmp.rs @@ -945,7 +945,7 @@ impl Pallet { outgoing_paras.len() as u32 )) .saturating_add(::WeightInfo::force_process_hrmp_close( - outgoing_paras.len() as u32, + outgoing_paras.len() as u32 )) } diff --git a/polkadot/runtime/parachains/src/paras_inherent/mod.rs b/polkadot/runtime/parachains/src/paras_inherent/mod.rs index 70c4ba72d58e..84d8299cd29c 100644 --- a/polkadot/runtime/parachains/src/paras_inherent/mod.rs +++ b/polkadot/runtime/parachains/src/paras_inherent/mod.rs @@ -1216,19 +1216,18 @@ fn filter_backed_statements_from_disabled_validators< // Get relay parent block number of the candidate. We need this to get the group index // assigned to this core at this block number - let relay_parent_block_number = match allowed_relay_parents - .acquire_info(bc.descriptor().relay_parent(), None) - { - Some((_, block_num)) => block_num, - None => { - log::debug!( - target: LOG_TARGET, - "Relay parent {:?} for candidate is not in the allowed relay parents. Dropping the candidate.", - bc.descriptor().relay_parent() - ); - return false - }, - }; + let relay_parent_block_number = + match allowed_relay_parents.acquire_info(bc.descriptor().relay_parent(), None) { + Some((_, block_num)) => block_num, + None => { + log::debug!( + target: LOG_TARGET, + "Relay parent {:?} for candidate is not in the allowed relay parents. Dropping the candidate.", + bc.descriptor().relay_parent() + ); + return false + }, + }; // Get the group index for the core let group_idx = match scheduler::Pallet::::group_assigned_to_core( diff --git a/polkadot/runtime/parachains/src/ump_tests.rs b/polkadot/runtime/parachains/src/ump_tests.rs index 91571859ecf0..d914bf8b6661 100644 --- a/polkadot/runtime/parachains/src/ump_tests.rs +++ b/polkadot/runtime/parachains/src/ump_tests.rs @@ -462,11 +462,10 @@ fn verify_relay_dispatch_queue_size_is_externally_accessible() { fn assert_queue_size(para: ParaId, count: u32, size: u32) { #[allow(deprecated)] - let raw_queue_size = sp_io::storage::get(&well_known_keys::relay_dispatch_queue_size(para)) - .expect( - "enqueuing a message should create the dispatch queue\ + let raw_queue_size = sp_io::storage::get(&well_known_keys::relay_dispatch_queue_size(para)).expect( + "enqueuing a message should create the dispatch queue\ and it should be accessible via the well known keys", - ); + ); let (c, s) = <(u32, u32)>::decode(&mut &raw_queue_size[..]) .expect("the dispatch queue size should be decodable into (u32, u32)"); assert_eq!((c, s), (count, size)); diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 576e297df31c..b9f04b440b5f 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1110,8 +1110,7 @@ impl InstanceFilter for ProxyType { matches!( c, RuntimeCall::Staking(..) | - RuntimeCall::Session(..) | - RuntimeCall::Utility(..) | + RuntimeCall::Session(..) | RuntimeCall::Utility(..) | RuntimeCall::FastUnstake(..) | RuntimeCall::VoterList(..) | RuntimeCall::NominationPools(..) diff --git a/polkadot/statement-table/src/generic.rs b/polkadot/statement-table/src/generic.rs index e3c470fcdeec..1e90338a0f18 100644 --- a/polkadot/statement-table/src/generic.rs +++ b/polkadot/statement-table/src/generic.rs @@ -245,8 +245,7 @@ impl CandidateData { pub fn attested( &self, validity_threshold: usize, - ) -> Option> - { + ) -> Option> { let valid_votes = self.validity_votes.len(); if valid_votes < validity_threshold { return None @@ -322,8 +321,7 @@ impl Table { digest: &Ctx::Digest, context: &Ctx, minimum_backing_votes: u32, - ) -> Option> - { + ) -> Option> { self.candidate_votes.get(digest).and_then(|data| { let v_threshold = context.get_group_size(&data.group_id).map_or(usize::MAX, |len| { effective_minimum_backing_votes(len, minimum_backing_votes) diff --git a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs index 7cb230f6e006..99f17693093e 100644 --- a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs +++ b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs @@ -152,7 +152,7 @@ impl pallet_xcm::Config for Runtime { // We turn off sending for these tests type SendXcmOrigin = EnsureXcmOrigin; type XcmRouter = super::super::network::ParachainXcmRouter; // Provided by xcm-simulator - // Anyone can execute XCM programs + // Anyone can execute XCM programs type ExecuteXcmOrigin = EnsureXcmOrigin; // We execute any type of program type XcmExecuteFilter = Everything; diff --git a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs index a31e664d8216..987bb3f9ab66 100644 --- a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs +++ b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs @@ -125,7 +125,7 @@ impl pallet_xcm::Config for Runtime { // No one can call `send` type SendXcmOrigin = EnsureXcmOrigin; type XcmRouter = super::super::network::RelayChainXcmRouter; // Provided by xcm-simulator - // Anyone can execute XCM programs + // Anyone can execute XCM programs type ExecuteXcmOrigin = EnsureXcmOrigin; // We execute any type of program type XcmExecuteFilter = Everything; diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs index 210b8f656377..4a12bb7f47c6 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs @@ -72,7 +72,7 @@ pub fn generate_holding_assets(max_assets: u32) -> Assets { let fungibles_amount: u128 = 100; let holding_fungibles = max_assets / 2; let holding_non_fungibles = max_assets - holding_fungibles - 1; // -1 because of adding `Here` asset - // add count of `holding_fungibles` + // add count of `holding_fungibles` (0..holding_fungibles) .map(|i| { Asset { diff --git a/polkadot/xcm/xcm-builder/src/asset_conversion.rs b/polkadot/xcm/xcm-builder/src/asset_conversion.rs index 6d090b04886c..16ae05c20795 100644 --- a/polkadot/xcm/xcm-builder/src/asset_conversion.rs +++ b/polkadot/xcm/xcm-builder/src/asset_conversion.rs @@ -137,13 +137,7 @@ impl< ConvertClassId: MaybeEquivalence, ConvertInstanceId: MaybeEquivalence, > MatchesNonFungibles - for MatchedConvertedConcreteId< - ClassId, - InstanceId, - MatchClassId, - ConvertClassId, - ConvertInstanceId, - > + for MatchedConvertedConcreteId { fn matches_nonfungibles(a: &Asset) -> result::Result<(ClassId, InstanceId), MatchError> { let (instance, class) = match (&a.fun, &a.id) { diff --git a/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs b/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs index 006c28954bce..b111a05a4f1f 100644 --- a/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs @@ -270,14 +270,7 @@ impl< CheckAsset: AssetChecking, CheckingAccount: Get>, > TransactAsset - for NonFungiblesAdapter< - Assets, - Matcher, - AccountIdConverter, - AccountId, - CheckAsset, - CheckingAccount, - > + for NonFungiblesAdapter { fn can_check_in(origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { NonFungiblesMutateAdapter::< diff --git a/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs b/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs index 2d14b4e571c6..889a50a2bab9 100644 --- a/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs +++ b/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs @@ -197,7 +197,7 @@ fn fee_estimation_for_teleport() { fn dry_run_reserve_asset_transfer() { sp_tracing::init_for_tests(); let who = 1; // AccountId = u64. - // Native token used for fees. + // Native token used for fees. let balances = vec![(who, DeliveryFees::get() + ExistentialDeposit::get())]; // Relay token is the one we want to transfer. let assets = vec![(1, who, 100)]; // id, account_id, balance. diff --git a/substrate/client/cli/src/params/node_key_params.rs b/substrate/client/cli/src/params/node_key_params.rs index 70671bff8c05..cdd637888114 100644 --- a/substrate/client/cli/src/params/node_key_params.rs +++ b/substrate/client/cli/src/params/node_key_params.rs @@ -116,8 +116,8 @@ impl NodeKeyParams { .clone() .unwrap_or_else(|| net_config_dir.join(NODE_KEY_ED25519_FILE)); if !self.unsafe_force_node_key_generation && - role.is_authority() && - !is_dev && !key_path.exists() + role.is_authority() && !is_dev && + !key_path.exists() { return Err(Error::NetworkKeyNotFound(key_path)) } diff --git a/substrate/client/consensus/grandpa/src/aux_schema.rs b/substrate/client/consensus/grandpa/src/aux_schema.rs index c42310dcd72c..8ec882591be9 100644 --- a/substrate/client/consensus/grandpa/src/aux_schema.rs +++ b/substrate/client/consensus/grandpa/src/aux_schema.rs @@ -743,9 +743,7 @@ mod test { substrate_test_runtime_client::runtime::Block, _, _, - >( - &client, H256::random(), 0, || unreachable!() - ) + >(&client, H256::random(), 0, || unreachable!()) .unwrap(); assert_eq!( diff --git a/substrate/client/network/src/protocol/notifications/behaviour.rs b/substrate/client/network/src/protocol/notifications/behaviour.rs index a562546145c8..5a4559db0d8d 100644 --- a/substrate/client/network/src/protocol/notifications/behaviour.rs +++ b/substrate/client/network/src/protocol/notifications/behaviour.rs @@ -2398,8 +2398,7 @@ mod tests { } fn development_notifs( - ) -> (Notifications, ProtocolController, Box) - { + ) -> (Notifications, ProtocolController, Box) { let (protocol_handle_pair, notif_service) = crate::protocol::notifications::service::notification_service("/proto/1".into()); let (to_notifications, from_controller) = diff --git a/substrate/client/network/sync/src/engine.rs b/substrate/client/network/sync/src/engine.rs index dceea9954c6e..96c1750b3116 100644 --- a/substrate/client/network/sync/src/engine.rs +++ b/substrate/client/network/sync/src/engine.rs @@ -812,8 +812,7 @@ where } if !self.default_peers_set_no_slot_connected_peers.remove(&peer_id) && - info.inbound && - info.info.roles.is_full() + info.inbound && info.info.roles.is_full() { match self.num_in_peers.checked_sub(1) { Some(value) => { diff --git a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs index fa10fde388f9..073ee34a79f3 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs @@ -343,8 +343,7 @@ where fn number( &self, hash: Block::Hash, - ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> - { + ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> { self.client.number(hash) } diff --git a/substrate/frame/bags-list/src/list/tests.rs b/substrate/frame/bags-list/src/list/tests.rs index fc4c4fbd088b..e5fff76d75c7 100644 --- a/substrate/frame/bags-list/src/list/tests.rs +++ b/substrate/frame/bags-list/src/list/tests.rs @@ -778,7 +778,7 @@ mod bags { assert_eq!(bag_1000.iter().count(), 3); bag_1000.insert_node_unchecked(node(4, None, None, bag_1000.bag_upper)); // panics in debug assert_eq!(bag_1000.iter().count(), 3); // in release we expect it to silently ignore the - // request. + // request. }); } diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index a4984b34f6db..2243859458be 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/substrate/frame/balances/src/tests/currency_tests.rs @@ -1017,7 +1017,7 @@ fn slash_consumed_slash_full_works() { ExtBuilder::default().existential_deposit(100).build_and_execute_with(|| { Balances::make_free_balance_be(&1, 1_000); assert_ok!(System::inc_consumers(&1)); // <-- Reference counter added here is enough for all tests - // Slashed completed in full + // Slashed completed in full assert_eq!(Balances::slash(&1, 900), (NegativeImbalance::new(900), 0)); // Account is still alive assert!(System::account_exists(&1)); @@ -1029,7 +1029,7 @@ fn slash_consumed_slash_over_works() { ExtBuilder::default().existential_deposit(100).build_and_execute_with(|| { Balances::make_free_balance_be(&1, 1_000); assert_ok!(System::inc_consumers(&1)); // <-- Reference counter added here is enough for all tests - // Slashed completed in full + // Slashed completed in full assert_eq!(Balances::slash(&1, 1_000), (NegativeImbalance::new(900), 100)); // Account is still alive assert!(System::account_exists(&1)); @@ -1041,7 +1041,7 @@ fn slash_consumed_slash_partial_works() { ExtBuilder::default().existential_deposit(100).build_and_execute_with(|| { Balances::make_free_balance_be(&1, 1_000); assert_ok!(System::inc_consumers(&1)); // <-- Reference counter added here is enough for all tests - // Slashed completed in full + // Slashed completed in full assert_eq!(Balances::slash(&1, 800), (NegativeImbalance::new(800), 0)); // Account is still alive assert!(System::account_exists(&1)); diff --git a/substrate/frame/bounties/src/lib.rs b/substrate/frame/bounties/src/lib.rs index e30d6fa2d143..7b89a6e3e76f 100644 --- a/substrate/frame/bounties/src/lib.rs +++ b/substrate/frame/bounties/src/lib.rs @@ -459,12 +459,12 @@ pub mod pallet { Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { let bounty = maybe_bounty.as_mut().ok_or(Error::::InvalidIndex)?; - let slash_curator = - |curator: &T::AccountId, curator_deposit: &mut BalanceOf| { - let imbalance = T::Currency::slash_reserved(curator, *curator_deposit).0; - T::OnSlash::on_unbalanced(imbalance); - *curator_deposit = Zero::zero(); - }; + let slash_curator = |curator: &T::AccountId, + curator_deposit: &mut BalanceOf| { + let imbalance = T::Currency::slash_reserved(curator, *curator_deposit).0; + T::OnSlash::on_unbalanced(imbalance); + *curator_deposit = Zero::zero(); + }; match bounty.status { BountyStatus::Proposed | BountyStatus::Approved | BountyStatus::Funded => { diff --git a/substrate/frame/child-bounties/src/lib.rs b/substrate/frame/child-bounties/src/lib.rs index 660a30ca5d26..911fd4c4c49f 100644 --- a/substrate/frame/child-bounties/src/lib.rs +++ b/substrate/frame/child-bounties/src/lib.rs @@ -473,13 +473,12 @@ pub mod pallet { let child_bounty = maybe_child_bounty.as_mut().ok_or(BountiesError::::InvalidIndex)?; - let slash_curator = - |curator: &T::AccountId, curator_deposit: &mut BalanceOf| { - let imbalance = - T::Currency::slash_reserved(curator, *curator_deposit).0; - T::OnSlash::on_unbalanced(imbalance); - *curator_deposit = Zero::zero(); - }; + let slash_curator = |curator: &T::AccountId, + curator_deposit: &mut BalanceOf| { + let imbalance = T::Currency::slash_reserved(curator, *curator_deposit).0; + T::OnSlash::on_unbalanced(imbalance); + *curator_deposit = Zero::zero(); + }; match child_bounty.status { ChildBountyStatus::Added => { diff --git a/substrate/frame/examples/offchain-worker/src/tests.rs b/substrate/frame/examples/offchain-worker/src/tests.rs index 741adbe6d26a..b665cbbb62ae 100644 --- a/substrate/frame/examples/offchain-worker/src/tests.rs +++ b/substrate/frame/examples/offchain-worker/src/tests.rs @@ -266,12 +266,11 @@ fn should_submit_unsigned_transaction_on_chain_for_any_account() { { assert_eq!(body, price_payload); - let signature_valid = ::Public, - frame_system::pallet_prelude::BlockNumberFor, - > as SignedPayload>::verify::( - &price_payload, signature - ); + let signature_valid = + ::Public, + frame_system::pallet_prelude::BlockNumberFor, + > as SignedPayload>::verify::(&price_payload, signature); assert!(signature_valid); } @@ -321,12 +320,11 @@ fn should_submit_unsigned_transaction_on_chain_for_all_accounts() { { assert_eq!(body, price_payload); - let signature_valid = ::Public, - frame_system::pallet_prelude::BlockNumberFor, - > as SignedPayload>::verify::( - &price_payload, signature - ); + let signature_valid = + ::Public, + frame_system::pallet_prelude::BlockNumberFor, + > as SignedPayload>::verify::(&price_payload, signature); assert!(signature_valid); } diff --git a/substrate/frame/nis/src/lib.rs b/substrate/frame/nis/src/lib.rs index 87e2276e768d..016daa4cb78b 100644 --- a/substrate/frame/nis/src/lib.rs +++ b/substrate/frame/nis/src/lib.rs @@ -756,13 +756,15 @@ pub mod pallet { .map(|_| ()) // We ignore this error as it just means the amount we're trying to deposit is // dust and the beneficiary account doesn't exist. - .or_else(|e| { - if e == TokenError::CannotCreate.into() { - Ok(()) - } else { - Err(e) - } - })?; + .or_else( + |e| { + if e == TokenError::CannotCreate.into() { + Ok(()) + } else { + Err(e) + } + }, + )?; summary.receipts_on_hold.saturating_reduce(on_hold); } T::Currency::release(&HoldReason::NftReceipt.into(), &who, amount, Exact)?; diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs index e83f28b472cd..1039b288b2ae 100644 --- a/substrate/frame/referenda/src/types.rs +++ b/substrate/frame/referenda/src/types.rs @@ -258,8 +258,7 @@ impl< Tally: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone, AccountId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone, ScheduleAddress: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone, - > - ReferendumInfo + > ReferendumInfo { /// Take the Decision Deposit from `self`, if there is one. Returns an `Err` if `self` is not /// in a valid state for the Decision Deposit to be refunded. diff --git a/substrate/frame/revive/proc-macro/src/lib.rs b/substrate/frame/revive/proc-macro/src/lib.rs index 012b4bfab9a9..95f4110a2d76 100644 --- a/substrate/frame/revive/proc-macro/src/lib.rs +++ b/substrate/frame/revive/proc-macro/src/lib.rs @@ -349,19 +349,18 @@ where let Some(ident) = path.path.get_ident() else { panic!("Type needs to be ident"); }; - let size = if ident == "i8" || - ident == "i16" || - ident == "i32" || - ident == "u8" || - ident == "u16" || - ident == "u32" - { - 1 - } else if ident == "i64" || ident == "u64" { - 2 - } else { - panic!("Pass by value only supports primitives"); - }; + let size = + if ident == "i8" || + ident == "i16" || ident == "i32" || + ident == "u8" || ident == "u16" || + ident == "u32" + { + 1 + } else if ident == "i64" || ident == "u64" { + 2 + } else { + panic!("Pass by value only supports primitives"); + }; registers_used += size; if registers_used > ALLOWED_REGISTERS { return quote! { diff --git a/substrate/frame/society/src/tests.rs b/substrate/frame/society/src/tests.rs index 2a13f99855b5..df8e844cdad9 100644 --- a/substrate/frame/society/src/tests.rs +++ b/substrate/frame/society/src/tests.rs @@ -281,7 +281,7 @@ fn bidding_works() { // No more candidates satisfy the requirements assert_eq!(candidacies(), vec![]); assert_ok!(Society::defender_vote(Origin::signed(10), true)); // Keep defender around - // Next period + // Next period run_to_block(16); // Same members assert_eq!(members(), vec![10, 30, 40, 50]); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index dd178a95bec5..ab2c00ca9ccc 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7995,7 +7995,7 @@ mod ledger_recovery { assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // OK assert_eq!(Bonded::::get(&333), Some(444)); // OK assert!(Payee::::get(&333).is_some()); // OK - // however, ledger associated with its controller was killed. + // however, ledger associated with its controller was killed. assert!(Ledger::::get(&444).is_none()); // NOK // side effects on 444 - ledger, bonded, payee, lock should be completely removed. diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 68ced1bc0ed3..346dff46f12e 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -400,19 +400,18 @@ impl CallDef { } for (feeless_arg, arg) in feeless_check.inputs.iter().skip(1).zip(args.iter()) { - let feeless_arg_type = if let syn::Pat::Type(syn::PatType { ty, .. }) = - feeless_arg.clone() - { - if let syn::Type::Reference(pat) = *ty { - pat.elem.clone() + let feeless_arg_type = + if let syn::Pat::Type(syn::PatType { ty, .. }) = feeless_arg.clone() { + if let syn::Type::Reference(pat) = *ty { + pat.elem.clone() + } else { + let msg = "Invalid pallet::call, feeless_if closure argument must be a reference"; + return Err(syn::Error::new(ty.span(), msg)); + } } else { - let msg = "Invalid pallet::call, feeless_if closure argument must be a reference"; - return Err(syn::Error::new(ty.span(), msg)); - } - } else { - let msg = "Invalid pallet::call, feeless_if closure argument must be a type ascription pattern"; - return Err(syn::Error::new(feeless_arg.span(), msg)); - }; + let msg = "Invalid pallet::call, feeless_if closure argument must be a type ascription pattern"; + return Err(syn::Error::new(feeless_arg.span(), msg)); + }; if feeless_arg_type != arg.2 { let msg = diff --git a/substrate/frame/support/src/storage/types/double_map.rs b/substrate/frame/support/src/storage/types/double_map.rs index 24aad3de0b33..c70d9de54467 100644 --- a/substrate/frame/support/src/storage/types/double_map.rs +++ b/substrate/frame/support/src/storage/types/double_map.rs @@ -129,8 +129,7 @@ impl OnEmpty, MaxValues, >, - > -where + > where Prefix: StorageInstance, Hasher1: crate::hash::StorageHasher, Hasher2: crate::hash::StorageHasher, diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index a7465c87fb27..8dbeecd8e860 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -197,8 +197,7 @@ impl TryDecodeEntireS QueryKind, OnEmpty, MaxValues, - > -where + > where Prefix: CountedStorageMapInstance, Hasher: StorageHasher, Key: FullCodec, @@ -230,8 +229,7 @@ impl QueryKind, OnEmpty, MaxValues, - > -where + > where Prefix: StorageInstance, Hasher1: StorageHasher, Key1: FullCodec, diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index 3d6aa1d83749..7f1ce0556eab 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -2431,10 +2431,9 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // any storage version "enabled". assert!( ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) - .unwrap_err() == - "On chain storage version set, while the pallet \ + .unwrap_err() == "On chain storage version set, while the pallet \ doesn't have the `#[pallet::storage_version(VERSION)]` attribute." - .into() + .into() ); }); } diff --git a/substrate/frame/transaction-payment/src/tests.rs b/substrate/frame/transaction-payment/src/tests.rs index bac89967d6af..35d5322a6f33 100644 --- a/substrate/frame/transaction-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/src/tests.rs @@ -273,10 +273,8 @@ fn signed_ext_length_fee_is_also_updated_per_congestion() { NextFeeMultiplier::::put(Multiplier::saturating_from_rational(3, 2)); let len = 10; - assert_ok!( - ChargeTransactionPayment::::from(10) // tipped - .pre_dispatch(&1, CALL, &info_from_weight(Weight::from_parts(3, 0)), len) - ); + assert_ok!(ChargeTransactionPayment::::from(10) // tipped + .pre_dispatch(&1, CALL, &info_from_weight(Weight::from_parts(3, 0)), len)); assert_eq!( Balances::free_balance(1), 100 // original diff --git a/substrate/frame/utility/src/lib.rs b/substrate/frame/utility/src/lib.rs index a4f66298f3fa..ed5544fe55ca 100644 --- a/substrate/frame/utility/src/lib.rs +++ b/substrate/frame/utility/src/lib.rs @@ -134,8 +134,8 @@ pub mod pallet { fn batched_calls_limit() -> u32 { let allocator_limit = sp_core::MAX_POSSIBLE_ALLOCATION; let call_size = ((core::mem::size_of::<::RuntimeCall>() as u32 + - CALL_ALIGN - 1) / - CALL_ALIGN) * CALL_ALIGN; + CALL_ALIGN - 1) / CALL_ALIGN) * + CALL_ALIGN; // The margin to take into account vec doubling capacity. let margin_factor = 3; diff --git a/substrate/frame/vesting/src/tests.rs b/substrate/frame/vesting/src/tests.rs index 57cb59f27a4d..004da0dfbfa1 100644 --- a/substrate/frame/vesting/src/tests.rs +++ b/substrate/frame/vesting/src/tests.rs @@ -182,7 +182,7 @@ fn unvested_balance_should_not_transfer() { ExtBuilder::default().existential_deposit(10).build().execute_with(|| { let user1_free_balance = Balances::free_balance(&1); assert_eq!(user1_free_balance, 100); // Account 1 has free balance - // Account 1 has only 5 units vested at block 1 (plus 50 unvested) + // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); // Account 1 cannot send more than vested amount... assert_noop!(Balances::transfer_allow_death(Some(1).into(), 2, 56), TokenError::Frozen); @@ -194,7 +194,7 @@ fn vested_balance_should_transfer() { ExtBuilder::default().existential_deposit(10).build().execute_with(|| { let user1_free_balance = Balances::free_balance(&1); assert_eq!(user1_free_balance, 100); // Account 1 has free balance - // Account 1 has only 5 units vested at block 1 (plus 50 unvested) + // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest(Some(1).into())); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 55)); @@ -232,7 +232,7 @@ fn vested_balance_should_transfer_using_vest_other() { ExtBuilder::default().existential_deposit(10).build().execute_with(|| { let user1_free_balance = Balances::free_balance(&1); assert_eq!(user1_free_balance, 100); // Account 1 has free balance - // Account 1 has only 5 units vested at block 1 (plus 50 unvested) + // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest_other(Some(2).into(), 1)); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 55)); @@ -286,7 +286,7 @@ fn extra_balance_should_transfer() { assert_eq!(Vesting::vesting_balance(&2), Some(200)); assert_ok!(Vesting::vest(Some(2).into())); assert_ok!(Balances::transfer_allow_death(Some(2).into(), 3, 100)); // Account 2 can send extra - // units gained + // units gained }); } @@ -296,7 +296,7 @@ fn liquid_funds_should_transfer_with_delayed_vesting() { let user12_free_balance = Balances::free_balance(&12); assert_eq!(user12_free_balance, 2560); // Account 12 has free balance - // Account 12 has liquid funds + // Account 12 has liquid funds assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - 256 * 5)); // Account 12 has delayed vesting diff --git a/substrate/utils/wasm-builder/src/wasm_project.rs b/substrate/utils/wasm-builder/src/wasm_project.rs index dcd5e6f1ade9..3fc8dfbc7f35 100644 --- a/substrate/utils/wasm-builder/src/wasm_project.rs +++ b/substrate/utils/wasm-builder/src/wasm_project.rs @@ -601,10 +601,9 @@ fn project_enabled_features( // We don't want to enable the `std`/`default` feature for the wasm build and // we need to check if the feature is enabled by checking the env variable. *f != "std" && - *f != "default" && - env::var(format!("CARGO_FEATURE_{feature_env}")) - .map(|v| v == "1") - .unwrap_or_default() + *f != "default" && env::var(format!("CARGO_FEATURE_{feature_env}")) + .map(|v| v == "1") + .unwrap_or_default() }) .map(|d| d.0.clone()) .collect::>(); From 6263c838c83c30979dece02308805a00c3d36faa Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 27 Sep 2024 14:14:42 +0200 Subject: [PATCH 23/28] Revert "".git/.scripts/commands/fmt/fmt.sh"" This reverts commit 381fcc0a0dc4a9b5f1317031fe56cfee2cbc7a97. --- bridges/modules/xcm-bridge-hub/src/lib.rs | 37 ++++++++++--------- .../src/cli/relay_headers_and_messages/mod.rs | 4 +- .../src/on_demand/parachains.rs | 3 +- .../messages/src/message_race_strategy.rs | 6 ++- .../primitives/router/src/inbound/mod.rs | 6 ++- .../primitives/router/src/outbound/mod.rs | 3 +- .../runtime/runtime-common/src/lib.rs | 3 +- cumulus/pallets/parachain-system/src/lib.rs | 3 +- .../asset-hub-rococo/src/tests/teleport.rs | 4 +- .../asset-hub-westend/src/tests/teleport.rs | 4 +- .../people-rococo/src/tests/teleport.rs | 4 +- .../people-westend/src/tests/teleport.rs | 4 +- .../assets/asset-hub-rococo/src/lib.rs | 3 +- .../assets/asset-hub-westend/src/lib.rs | 3 +- .../assets/test-utils/src/test_cases.rs | 3 +- .../bridge-hub-westend/tests/tests.rs | 32 ++++++++++------ .../test-utils/src/test_cases/mod.rs | 11 +++--- cumulus/primitives/utility/src/lib.rs | 6 ++- .../core/approval-voting-parallel/src/lib.rs | 3 +- polkadot/node/core/approval-voting/src/lib.rs | 7 +++- .../node/core/approval-voting/src/tests.rs | 3 +- .../common/src/worker/security/change_root.rs | 3 +- .../approval-distribution/src/tests.rs | 3 +- .../src/task/strategy/chunks.rs | 7 ++-- .../subsystem-bench/src/lib/statement/mod.rs | 16 ++++---- .../subsystem-types/src/runtime_client.rs | 3 +- polkadot/runtime/parachains/src/hrmp.rs | 2 +- .../parachains/src/paras_inherent/mod.rs | 25 +++++++------ polkadot/runtime/parachains/src/ump_tests.rs | 7 ++-- polkadot/runtime/westend/src/lib.rs | 3 +- polkadot/statement-table/src/generic.rs | 6 ++- .../parachain/xcm_config.rs | 2 +- .../relay_chain/xcm_config.rs | 2 +- polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs | 2 +- .../xcm/xcm-builder/src/asset_conversion.rs | 8 +++- .../xcm-builder/src/nonfungibles_adapter.rs | 9 ++++- .../xcm-runtime-apis/tests/fee_estimation.rs | 2 +- .../client/cli/src/params/node_key_params.rs | 4 +- .../consensus/grandpa/src/aux_schema.rs | 4 +- .../src/protocol/notifications/behaviour.rs | 3 +- substrate/client/network/sync/src/engine.rs | 3 +- .../rpc-spec-v2/src/chain_head/test_utils.rs | 3 +- substrate/frame/bags-list/src/list/tests.rs | 2 +- .../balances/src/tests/currency_tests.rs | 6 +-- substrate/frame/bounties/src/lib.rs | 12 +++--- substrate/frame/child-bounties/src/lib.rs | 13 ++++--- .../examples/offchain-worker/src/tests.rs | 22 ++++++----- substrate/frame/nis/src/lib.rs | 16 ++++---- substrate/frame/referenda/src/types.rs | 3 +- substrate/frame/revive/proc-macro/src/lib.rs | 25 +++++++------ substrate/frame/society/src/tests.rs | 2 +- substrate/frame/staking/src/tests.rs | 2 +- .../procedural/src/pallet/parse/call.rs | 23 ++++++------ .../support/src/storage/types/double_map.rs | 3 +- .../traits/try_runtime/decode_entire_state.rs | 6 ++- substrate/frame/support/test/tests/pallet.rs | 5 ++- .../frame/transaction-payment/src/tests.rs | 6 ++- substrate/frame/utility/src/lib.rs | 4 +- substrate/frame/vesting/src/tests.rs | 10 ++--- .../utils/wasm-builder/src/wasm_project.rs | 7 ++-- 60 files changed, 257 insertions(+), 179 deletions(-) diff --git a/bridges/modules/xcm-bridge-hub/src/lib.rs b/bridges/modules/xcm-bridge-hub/src/lib.rs index b9a3f6a4d3c2..1b2536598a20 100644 --- a/bridges/modules/xcm-bridge-hub/src/lib.rs +++ b/bridges/modules/xcm-bridge-hub/src/lib.rs @@ -1455,25 +1455,26 @@ mod tests { let lane_id = TestLaneIdType::try_new(1, 2).unwrap(); let lane_id_mismatch = TestLaneIdType::try_new(3, 4).unwrap(); - let test_bridge_state = |id, - bridge, - (lane_id, bridge_id), - (inbound_lane_id, outbound_lane_id), - expected_error: Option| { - Bridges::::insert(id, bridge); - LaneToBridge::::insert(lane_id, bridge_id); + let test_bridge_state = + |id, + bridge, + (lane_id, bridge_id), + (inbound_lane_id, outbound_lane_id), + expected_error: Option| { + Bridges::::insert(id, bridge); + LaneToBridge::::insert(lane_id, bridge_id); - let lanes_manager = LanesManagerOf::::new(); - lanes_manager.create_inbound_lane(inbound_lane_id).unwrap(); - lanes_manager.create_outbound_lane(outbound_lane_id).unwrap(); - - let result = XcmOverBridge::do_try_state(); - if let Some(e) = expected_error { - assert_err!(result, e); - } else { - assert_ok!(result); - } - }; + let lanes_manager = LanesManagerOf::::new(); + lanes_manager.create_inbound_lane(inbound_lane_id).unwrap(); + lanes_manager.create_outbound_lane(outbound_lane_id).unwrap(); + + let result = XcmOverBridge::do_try_state(); + if let Some(e) = expected_error { + assert_err!(result, e); + } else { + assert_ok!(result); + } + }; let cleanup = |bridge_id, lane_ids| { Bridges::::remove(bridge_id); for lane_id in lane_ids { diff --git a/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs b/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs index 9261dc437536..e875c53a2e7e 100644 --- a/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs @@ -325,7 +325,9 @@ where _, Self::Left, MessagesLaneIdOf, - >(common.right.client.clone(), &common.metrics_params, &common.right.accounts, &lanes) + >( + common.right.client.clone(), &common.metrics_params, &common.right.accounts, &lanes + ) .await?; } diff --git a/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs b/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs index 2ef86f48ecbe..96eba0af988c 100644 --- a/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -664,7 +664,8 @@ impl<'a, P: SubstrateParachainsPipeline, SourceRelayClnt, TargetClnt> for ( &'a OnDemandParachainsRelay, &'a ParachainsSource, - ) where + ) +where SourceRelayClnt: Client, TargetClnt: Client, { diff --git a/bridges/relays/messages/src/message_race_strategy.rs b/bridges/relays/messages/src/message_race_strategy.rs index 3a532331d79d..1303fcfedebd 100644 --- a/bridges/relays/messages/src/message_race_strategy.rs +++ b/bridges/relays/messages/src/message_race_strategy.rs @@ -67,7 +67,8 @@ impl< TargetHeaderHash, SourceNoncesRange, Proof, - > where + > +where SourceHeaderHash: Clone, SourceHeaderNumber: Clone + Ord, SourceNoncesRange: NoncesRange, @@ -189,7 +190,8 @@ impl< TargetHeaderHash, SourceNoncesRange, Proof, - > where + > +where SourceHeaderHash: Clone + Debug + Send + Sync, SourceHeaderNumber: Clone + Ord + Debug + Send + Sync, SourceNoncesRange: NoncesRange + Debug + Send + Sync, diff --git a/bridges/snowbridge/primitives/router/src/inbound/mod.rs b/bridges/snowbridge/primitives/router/src/inbound/mod.rs index a10884b45531..fbfc52d01c83 100644 --- a/bridges/snowbridge/primitives/router/src/inbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/inbound/mod.rs @@ -168,7 +168,8 @@ impl< ConvertAssetId, EthereumUniversalLocation, GlobalAssetHubLocation, - > where + > +where CreateAssetCall: Get, CreateAssetDeposit: Get, InboundQueuePalletInstance: Get, @@ -226,7 +227,8 @@ impl< ConvertAssetId, EthereumUniversalLocation, GlobalAssetHubLocation, - > where + > +where CreateAssetCall: Get, CreateAssetDeposit: Get, InboundQueuePalletInstance: Get, diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index d3b6c116dd7a..b23bce0e2185 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/mod.rs @@ -44,7 +44,8 @@ impl where + > +where UniversalLocation: Get, EthereumNetwork: Get, OutboundQueue: SendMessage, diff --git a/bridges/snowbridge/runtime/runtime-common/src/lib.rs b/bridges/snowbridge/runtime/runtime-common/src/lib.rs index aae45520ff4b..0b1a74b232a0 100644 --- a/bridges/snowbridge/runtime/runtime-common/src/lib.rs +++ b/bridges/snowbridge/runtime/runtime-common/src/lib.rs @@ -50,7 +50,8 @@ impl where + > +where Balance: BaseArithmetic + Unsigned + Copy + From + Into + Debug, AccountId: Clone + FullCodec, FeeAssetLocation: Get, diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 9dc41aa03d9b..21af35fe3de3 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -390,7 +390,8 @@ pub mod pallet { let maximum_channels = host_config .hrmp_max_message_num_per_candidate - .min(>::take()) as usize; + .min(>::take()) + as usize; // Note: this internally calls the `GetChannelInfo` implementation for this // pallet, which draws on the `RelevantMessagingState`. That in turn has diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs index d6cf819118e9..470b4d0f389e 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs @@ -265,7 +265,9 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = AssetHubRococo::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) + >( + test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest + ) }); // Sender's balance is reduced diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs index ddb82a954a87..ee0f297792f8 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs @@ -265,7 +265,9 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = AssetHubWestend::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) + >( + test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest + ) }); // Sender's balance is reduced diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs index 44e6b3934f0e..2619ca7591d0 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/teleport.rs @@ -107,7 +107,9 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = PeopleRococo::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) + >( + test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest + ) }); // Sender's balance is reduced diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs index 83888031723f..d9a2c23ac0c6 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/src/tests/teleport.rs @@ -107,7 +107,9 @@ fn limited_teleport_native_assets_from_system_para_to_relay_fails() { let delivery_fees = PeopleWestend::execute_with(|| { xcm_helpers::teleport_assets_delivery_fees::< ::XcmSender, - >(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest) + >( + test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest + ) }); // Sender's balance is reduced diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index af5d1ce17bf3..430c5bd18561 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -545,7 +545,8 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | + RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index bd7780339267..8565cd30f430 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -543,7 +543,8 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | + RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( diff --git a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs index 67b585ecfe86..c80222142304 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs @@ -1143,7 +1143,8 @@ pub fn create_and_manage_foreign_assets_for_local_consensus_parachain_assets_wor .with_balances(vec![( foreign_creator_as_account_id.clone(), existential_deposit + - asset_deposit + metadata_deposit_base + + asset_deposit + + metadata_deposit_base + metadata_deposit_per_byte_eta + buy_execution_fee_amount.into() + buy_execution_fee_amount.into(), diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs index 4ff388f4ba2b..d168d1b7a13c 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs @@ -301,12 +301,16 @@ fn relayed_incoming_message_works() { XcmOverBridgeHubRococoInstance, LocationToAccountId, WestendLocation, - >(SiblingParachainLocation::get(), BridgedUniversalLocation::get(), |locations, fee| { - bridge_hub_test_utils::open_bridge_with_storage::< - Runtime, - XcmOverBridgeHubRococoInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) - }) + >( + SiblingParachainLocation::get(), + BridgedUniversalLocation::get(), + |locations, fee| { + bridge_hub_test_utils::open_bridge_with_storage::< + Runtime, + XcmOverBridgeHubRococoInstance, + >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + }, + ) .1 }, construct_and_apply_extrinsic, @@ -331,12 +335,16 @@ fn free_relay_extrinsic_works() { XcmOverBridgeHubRococoInstance, LocationToAccountId, WestendLocation, - >(SiblingParachainLocation::get(), BridgedUniversalLocation::get(), |locations, fee| { - bridge_hub_test_utils::open_bridge_with_storage::< - Runtime, - XcmOverBridgeHubRococoInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) - }) + >( + SiblingParachainLocation::get(), + BridgedUniversalLocation::get(), + |locations, fee| { + bridge_hub_test_utils::open_bridge_with_storage::< + Runtime, + XcmOverBridgeHubRococoInstance, + >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + }, + ) .1 }, construct_and_apply_extrinsic, diff --git a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs index 663558f5fd5c..9c5d6269dc0e 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs @@ -506,11 +506,12 @@ pub fn message_dispatch_routing_works< // 2. this message is sent from other global consensus with destination of this Runtime // sibling parachain (HRMP) - let bridging_message = test_data::simulate_message_exporter_on_bridged_chain::< - BridgedNetwork, - NetworkWithParentCount, - AlwaysLatest, - >((RuntimeNetwork::get(), [Parachain(sibling_parachain_id)].into())); + let bridging_message = + test_data::simulate_message_exporter_on_bridged_chain::< + BridgedNetwork, + NetworkWithParentCount, + AlwaysLatest, + >((RuntimeNetwork::get(), [Parachain(sibling_parachain_id)].into())); // 2.1. WITHOUT opened hrmp channel -> RoutingError let result = diff --git a/cumulus/primitives/utility/src/lib.rs b/cumulus/primitives/utility/src/lib.rs index 6bd14d136a62..8530f5b87487 100644 --- a/cumulus/primitives/utility/src/lib.rs +++ b/cumulus/primitives/utility/src/lib.rs @@ -385,7 +385,8 @@ impl< FungiblesAssetMatcher, OnUnbalanced, AccountId, - > where + > +where Fungibles::Balance: Into, { fn new() -> Self { @@ -545,7 +546,8 @@ impl< FungiblesAssetMatcher, OnUnbalanced, AccountId, - > where + > +where Fungibles::Balance: Into, { fn drop(&mut self) { diff --git a/polkadot/node/core/approval-voting-parallel/src/lib.rs b/polkadot/node/core/approval-voting-parallel/src/lib.rs index 18c73cfba1fc..1a7ef756bdfc 100644 --- a/polkadot/node/core/approval-voting-parallel/src/lib.rs +++ b/polkadot/node/core/approval-voting-parallel/src/lib.rs @@ -878,7 +878,8 @@ pub struct ApprovalVotingToApprovalDistribution> - overseer::SubsystemSender for ApprovalVotingToApprovalDistribution + overseer::SubsystemSender + for ApprovalVotingToApprovalDistribution { #[allow(clippy::type_complexity, clippy::type_repetition_in_bounds)] fn send_message<'life0, 'async_trait>( diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs index 2a1861c72341..835098293050 100644 --- a/polkadot/node/core/approval-voting/src/lib.rs +++ b/polkadot/node/core/approval-voting/src/lib.rs @@ -2465,7 +2465,12 @@ fn schedule_wakeup_action( last_assignment_tick.map(|l| l + APPROVAL_DELAY).filter(|t| t > &tick_now), next_no_show, ) - .map(|tick| Action::ScheduleWakeup { block_hash, block_number, candidate_hash, tick }) + .map(|tick| Action::ScheduleWakeup { + block_hash, + block_number, + candidate_hash, + tick, + }) }, RequiredTranches::Pending { considered, next_no_show, clock_drift, .. } => { // select the minimum of `next_no_show`, or the tick of the next non-empty tranche diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs index 56e5ead0808f..db5ffd441c0d 100644 --- a/polkadot/node/core/approval-voting/src/tests.rs +++ b/polkadot/node/core/approval-voting/src/tests.rs @@ -254,7 +254,8 @@ where _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, _assignment: &polkadot_node_primitives::approval::v2::AssignmentCertV2, _backing_groups: Vec, - ) -> Result { + ) -> Result + { self.1(validator_index) } } diff --git a/polkadot/node/core/pvf/common/src/worker/security/change_root.rs b/polkadot/node/core/pvf/common/src/worker/security/change_root.rs index 9ec66906819f..fcfaf6541c29 100644 --- a/polkadot/node/core/pvf/common/src/worker/security/change_root.rs +++ b/polkadot/node/core/pvf/common/src/worker/security/change_root.rs @@ -124,7 +124,8 @@ fn try_restrict(worker_info: &WorkerInfo) -> Result<()> { libc::MS_BIND | libc::MS_REC | libc::MS_NOEXEC | libc::MS_NODEV | libc::MS_NOSUID | - libc::MS_NOATIME | additional_flags, + libc::MS_NOATIME | + additional_flags, ptr::null(), // ignored when MS_BIND is used ) < 0 { diff --git a/polkadot/node/network/approval-distribution/src/tests.rs b/polkadot/node/network/approval-distribution/src/tests.rs index 068559dea762..063e71f2f528 100644 --- a/polkadot/node/network/approval-distribution/src/tests.rs +++ b/polkadot/node/network/approval-distribution/src/tests.rs @@ -535,7 +535,8 @@ impl AssignmentCriteria for MockAssignmentCriteria { _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, _assignment: &polkadot_node_primitives::approval::v2::AssignmentCertV2, _backing_groups: Vec, - ) -> Result { + ) -> Result + { self.tranche } } diff --git a/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs b/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs index 8ea229dbf021..6b34538b6266 100644 --- a/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs +++ b/polkadot/node/network/availability-recovery/src/task/strategy/chunks.rs @@ -107,9 +107,10 @@ impl FetchChunks { state: &mut State, common_params: &RecoveryParams, ) -> Result { - let recovery_duration = common_params - .metrics - .time_erasure_recovery(RecoveryStrategy::::strategy_type(self)); + let recovery_duration = + common_params + .metrics + .time_erasure_recovery(RecoveryStrategy::::strategy_type(self)); // Send request to reconstruct available data from chunks. let (avilable_data_tx, available_data_rx) = oneshot::channel(); diff --git a/polkadot/node/subsystem-bench/src/lib/statement/mod.rs b/polkadot/node/subsystem-bench/src/lib/statement/mod.rs index e2d50f285687..dd7095d3b00c 100644 --- a/polkadot/node/subsystem-bench/src/lib/statement/mod.rs +++ b/polkadot/node/subsystem-bench/src/lib/statement/mod.rs @@ -114,14 +114,14 @@ fn build_overseer( state.pvd.clone(), state.own_backing_group.clone(), ); - let (statement_req_receiver, statement_req_cfg) = IncomingRequest::get_config_receiver::< - Block, - sc_network::NetworkWorker, - >(&ReqProtocolNames::new(GENESIS_HASH, None)); - let (candidate_req_receiver, candidate_req_cfg) = IncomingRequest::get_config_receiver::< - Block, - sc_network::NetworkWorker, - >(&ReqProtocolNames::new(GENESIS_HASH, None)); + let (statement_req_receiver, statement_req_cfg) = + IncomingRequest::get_config_receiver::>( + &ReqProtocolNames::new(GENESIS_HASH, None), + ); + let (candidate_req_receiver, candidate_req_cfg) = + IncomingRequest::get_config_receiver::>( + &ReqProtocolNames::new(GENESIS_HASH, None), + ); let keystore = make_keystore(); let subsystem = StatementDistributionSubsystem::new( keystore.clone(), diff --git a/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index 7938223df23b..a8af8b7996f9 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -665,7 +665,8 @@ where fn number( &self, hash: Block::Hash, - ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> { + ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> + { self.client.number(hash) } diff --git a/polkadot/runtime/parachains/src/hrmp.rs b/polkadot/runtime/parachains/src/hrmp.rs index b149404b41b8..220543f00ec3 100644 --- a/polkadot/runtime/parachains/src/hrmp.rs +++ b/polkadot/runtime/parachains/src/hrmp.rs @@ -945,7 +945,7 @@ impl Pallet { outgoing_paras.len() as u32 )) .saturating_add(::WeightInfo::force_process_hrmp_close( - outgoing_paras.len() as u32 + outgoing_paras.len() as u32, )) } diff --git a/polkadot/runtime/parachains/src/paras_inherent/mod.rs b/polkadot/runtime/parachains/src/paras_inherent/mod.rs index 84d8299cd29c..70c4ba72d58e 100644 --- a/polkadot/runtime/parachains/src/paras_inherent/mod.rs +++ b/polkadot/runtime/parachains/src/paras_inherent/mod.rs @@ -1216,18 +1216,19 @@ fn filter_backed_statements_from_disabled_validators< // Get relay parent block number of the candidate. We need this to get the group index // assigned to this core at this block number - let relay_parent_block_number = - match allowed_relay_parents.acquire_info(bc.descriptor().relay_parent(), None) { - Some((_, block_num)) => block_num, - None => { - log::debug!( - target: LOG_TARGET, - "Relay parent {:?} for candidate is not in the allowed relay parents. Dropping the candidate.", - bc.descriptor().relay_parent() - ); - return false - }, - }; + let relay_parent_block_number = match allowed_relay_parents + .acquire_info(bc.descriptor().relay_parent(), None) + { + Some((_, block_num)) => block_num, + None => { + log::debug!( + target: LOG_TARGET, + "Relay parent {:?} for candidate is not in the allowed relay parents. Dropping the candidate.", + bc.descriptor().relay_parent() + ); + return false + }, + }; // Get the group index for the core let group_idx = match scheduler::Pallet::::group_assigned_to_core( diff --git a/polkadot/runtime/parachains/src/ump_tests.rs b/polkadot/runtime/parachains/src/ump_tests.rs index d914bf8b6661..91571859ecf0 100644 --- a/polkadot/runtime/parachains/src/ump_tests.rs +++ b/polkadot/runtime/parachains/src/ump_tests.rs @@ -462,10 +462,11 @@ fn verify_relay_dispatch_queue_size_is_externally_accessible() { fn assert_queue_size(para: ParaId, count: u32, size: u32) { #[allow(deprecated)] - let raw_queue_size = sp_io::storage::get(&well_known_keys::relay_dispatch_queue_size(para)).expect( - "enqueuing a message should create the dispatch queue\ + let raw_queue_size = sp_io::storage::get(&well_known_keys::relay_dispatch_queue_size(para)) + .expect( + "enqueuing a message should create the dispatch queue\ and it should be accessible via the well known keys", - ); + ); let (c, s) = <(u32, u32)>::decode(&mut &raw_queue_size[..]) .expect("the dispatch queue size should be decodable into (u32, u32)"); assert_eq!((c, s), (count, size)); diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index b9f04b440b5f..576e297df31c 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1110,7 +1110,8 @@ impl InstanceFilter for ProxyType { matches!( c, RuntimeCall::Staking(..) | - RuntimeCall::Session(..) | RuntimeCall::Utility(..) | + RuntimeCall::Session(..) | + RuntimeCall::Utility(..) | RuntimeCall::FastUnstake(..) | RuntimeCall::VoterList(..) | RuntimeCall::NominationPools(..) diff --git a/polkadot/statement-table/src/generic.rs b/polkadot/statement-table/src/generic.rs index 1e90338a0f18..e3c470fcdeec 100644 --- a/polkadot/statement-table/src/generic.rs +++ b/polkadot/statement-table/src/generic.rs @@ -245,7 +245,8 @@ impl CandidateData { pub fn attested( &self, validity_threshold: usize, - ) -> Option> { + ) -> Option> + { let valid_votes = self.validity_votes.len(); if valid_votes < validity_threshold { return None @@ -321,7 +322,8 @@ impl Table { digest: &Ctx::Digest, context: &Ctx, minimum_backing_votes: u32, - ) -> Option> { + ) -> Option> + { self.candidate_votes.get(digest).and_then(|data| { let v_threshold = context.get_group_size(&data.group_id).map_or(usize::MAX, |len| { effective_minimum_backing_votes(len, minimum_backing_votes) diff --git a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs index 99f17693093e..7cb230f6e006 100644 --- a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs +++ b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs @@ -152,7 +152,7 @@ impl pallet_xcm::Config for Runtime { // We turn off sending for these tests type SendXcmOrigin = EnsureXcmOrigin; type XcmRouter = super::super::network::ParachainXcmRouter; // Provided by xcm-simulator - // Anyone can execute XCM programs + // Anyone can execute XCM programs type ExecuteXcmOrigin = EnsureXcmOrigin; // We execute any type of program type XcmExecuteFilter = Everything; diff --git a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs index 987bb3f9ab66..a31e664d8216 100644 --- a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs +++ b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs @@ -125,7 +125,7 @@ impl pallet_xcm::Config for Runtime { // No one can call `send` type SendXcmOrigin = EnsureXcmOrigin; type XcmRouter = super::super::network::RelayChainXcmRouter; // Provided by xcm-simulator - // Anyone can execute XCM programs + // Anyone can execute XCM programs type ExecuteXcmOrigin = EnsureXcmOrigin; // We execute any type of program type XcmExecuteFilter = Everything; diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs index 4a12bb7f47c6..210b8f656377 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs @@ -72,7 +72,7 @@ pub fn generate_holding_assets(max_assets: u32) -> Assets { let fungibles_amount: u128 = 100; let holding_fungibles = max_assets / 2; let holding_non_fungibles = max_assets - holding_fungibles - 1; // -1 because of adding `Here` asset - // add count of `holding_fungibles` + // add count of `holding_fungibles` (0..holding_fungibles) .map(|i| { Asset { diff --git a/polkadot/xcm/xcm-builder/src/asset_conversion.rs b/polkadot/xcm/xcm-builder/src/asset_conversion.rs index 16ae05c20795..6d090b04886c 100644 --- a/polkadot/xcm/xcm-builder/src/asset_conversion.rs +++ b/polkadot/xcm/xcm-builder/src/asset_conversion.rs @@ -137,7 +137,13 @@ impl< ConvertClassId: MaybeEquivalence, ConvertInstanceId: MaybeEquivalence, > MatchesNonFungibles - for MatchedConvertedConcreteId + for MatchedConvertedConcreteId< + ClassId, + InstanceId, + MatchClassId, + ConvertClassId, + ConvertInstanceId, + > { fn matches_nonfungibles(a: &Asset) -> result::Result<(ClassId, InstanceId), MatchError> { let (instance, class) = match (&a.fun, &a.id) { diff --git a/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs b/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs index b111a05a4f1f..006c28954bce 100644 --- a/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs @@ -270,7 +270,14 @@ impl< CheckAsset: AssetChecking, CheckingAccount: Get>, > TransactAsset - for NonFungiblesAdapter + for NonFungiblesAdapter< + Assets, + Matcher, + AccountIdConverter, + AccountId, + CheckAsset, + CheckingAccount, + > { fn can_check_in(origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { NonFungiblesMutateAdapter::< diff --git a/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs b/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs index 889a50a2bab9..2d14b4e571c6 100644 --- a/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs +++ b/polkadot/xcm/xcm-runtime-apis/tests/fee_estimation.rs @@ -197,7 +197,7 @@ fn fee_estimation_for_teleport() { fn dry_run_reserve_asset_transfer() { sp_tracing::init_for_tests(); let who = 1; // AccountId = u64. - // Native token used for fees. + // Native token used for fees. let balances = vec![(who, DeliveryFees::get() + ExistentialDeposit::get())]; // Relay token is the one we want to transfer. let assets = vec![(1, who, 100)]; // id, account_id, balance. diff --git a/substrate/client/cli/src/params/node_key_params.rs b/substrate/client/cli/src/params/node_key_params.rs index cdd637888114..70671bff8c05 100644 --- a/substrate/client/cli/src/params/node_key_params.rs +++ b/substrate/client/cli/src/params/node_key_params.rs @@ -116,8 +116,8 @@ impl NodeKeyParams { .clone() .unwrap_or_else(|| net_config_dir.join(NODE_KEY_ED25519_FILE)); if !self.unsafe_force_node_key_generation && - role.is_authority() && !is_dev && - !key_path.exists() + role.is_authority() && + !is_dev && !key_path.exists() { return Err(Error::NetworkKeyNotFound(key_path)) } diff --git a/substrate/client/consensus/grandpa/src/aux_schema.rs b/substrate/client/consensus/grandpa/src/aux_schema.rs index 8ec882591be9..c42310dcd72c 100644 --- a/substrate/client/consensus/grandpa/src/aux_schema.rs +++ b/substrate/client/consensus/grandpa/src/aux_schema.rs @@ -743,7 +743,9 @@ mod test { substrate_test_runtime_client::runtime::Block, _, _, - >(&client, H256::random(), 0, || unreachable!()) + >( + &client, H256::random(), 0, || unreachable!() + ) .unwrap(); assert_eq!( diff --git a/substrate/client/network/src/protocol/notifications/behaviour.rs b/substrate/client/network/src/protocol/notifications/behaviour.rs index 5a4559db0d8d..a562546145c8 100644 --- a/substrate/client/network/src/protocol/notifications/behaviour.rs +++ b/substrate/client/network/src/protocol/notifications/behaviour.rs @@ -2398,7 +2398,8 @@ mod tests { } fn development_notifs( - ) -> (Notifications, ProtocolController, Box) { + ) -> (Notifications, ProtocolController, Box) + { let (protocol_handle_pair, notif_service) = crate::protocol::notifications::service::notification_service("/proto/1".into()); let (to_notifications, from_controller) = diff --git a/substrate/client/network/sync/src/engine.rs b/substrate/client/network/sync/src/engine.rs index 96c1750b3116..dceea9954c6e 100644 --- a/substrate/client/network/sync/src/engine.rs +++ b/substrate/client/network/sync/src/engine.rs @@ -812,7 +812,8 @@ where } if !self.default_peers_set_no_slot_connected_peers.remove(&peer_id) && - info.inbound && info.info.roles.is_full() + info.inbound && + info.info.roles.is_full() { match self.num_in_peers.checked_sub(1) { Some(value) => { diff --git a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs index 073ee34a79f3..fa10fde388f9 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs @@ -343,7 +343,8 @@ where fn number( &self, hash: Block::Hash, - ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> { + ) -> sc_client_api::blockchain::Result::Header as HeaderT>::Number>> + { self.client.number(hash) } diff --git a/substrate/frame/bags-list/src/list/tests.rs b/substrate/frame/bags-list/src/list/tests.rs index e5fff76d75c7..fc4c4fbd088b 100644 --- a/substrate/frame/bags-list/src/list/tests.rs +++ b/substrate/frame/bags-list/src/list/tests.rs @@ -778,7 +778,7 @@ mod bags { assert_eq!(bag_1000.iter().count(), 3); bag_1000.insert_node_unchecked(node(4, None, None, bag_1000.bag_upper)); // panics in debug assert_eq!(bag_1000.iter().count(), 3); // in release we expect it to silently ignore the - // request. + // request. }); } diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index 2243859458be..a4984b34f6db 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/substrate/frame/balances/src/tests/currency_tests.rs @@ -1017,7 +1017,7 @@ fn slash_consumed_slash_full_works() { ExtBuilder::default().existential_deposit(100).build_and_execute_with(|| { Balances::make_free_balance_be(&1, 1_000); assert_ok!(System::inc_consumers(&1)); // <-- Reference counter added here is enough for all tests - // Slashed completed in full + // Slashed completed in full assert_eq!(Balances::slash(&1, 900), (NegativeImbalance::new(900), 0)); // Account is still alive assert!(System::account_exists(&1)); @@ -1029,7 +1029,7 @@ fn slash_consumed_slash_over_works() { ExtBuilder::default().existential_deposit(100).build_and_execute_with(|| { Balances::make_free_balance_be(&1, 1_000); assert_ok!(System::inc_consumers(&1)); // <-- Reference counter added here is enough for all tests - // Slashed completed in full + // Slashed completed in full assert_eq!(Balances::slash(&1, 1_000), (NegativeImbalance::new(900), 100)); // Account is still alive assert!(System::account_exists(&1)); @@ -1041,7 +1041,7 @@ fn slash_consumed_slash_partial_works() { ExtBuilder::default().existential_deposit(100).build_and_execute_with(|| { Balances::make_free_balance_be(&1, 1_000); assert_ok!(System::inc_consumers(&1)); // <-- Reference counter added here is enough for all tests - // Slashed completed in full + // Slashed completed in full assert_eq!(Balances::slash(&1, 800), (NegativeImbalance::new(800), 0)); // Account is still alive assert!(System::account_exists(&1)); diff --git a/substrate/frame/bounties/src/lib.rs b/substrate/frame/bounties/src/lib.rs index 7b89a6e3e76f..e30d6fa2d143 100644 --- a/substrate/frame/bounties/src/lib.rs +++ b/substrate/frame/bounties/src/lib.rs @@ -459,12 +459,12 @@ pub mod pallet { Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { let bounty = maybe_bounty.as_mut().ok_or(Error::::InvalidIndex)?; - let slash_curator = |curator: &T::AccountId, - curator_deposit: &mut BalanceOf| { - let imbalance = T::Currency::slash_reserved(curator, *curator_deposit).0; - T::OnSlash::on_unbalanced(imbalance); - *curator_deposit = Zero::zero(); - }; + let slash_curator = + |curator: &T::AccountId, curator_deposit: &mut BalanceOf| { + let imbalance = T::Currency::slash_reserved(curator, *curator_deposit).0; + T::OnSlash::on_unbalanced(imbalance); + *curator_deposit = Zero::zero(); + }; match bounty.status { BountyStatus::Proposed | BountyStatus::Approved | BountyStatus::Funded => { diff --git a/substrate/frame/child-bounties/src/lib.rs b/substrate/frame/child-bounties/src/lib.rs index 911fd4c4c49f..660a30ca5d26 100644 --- a/substrate/frame/child-bounties/src/lib.rs +++ b/substrate/frame/child-bounties/src/lib.rs @@ -473,12 +473,13 @@ pub mod pallet { let child_bounty = maybe_child_bounty.as_mut().ok_or(BountiesError::::InvalidIndex)?; - let slash_curator = |curator: &T::AccountId, - curator_deposit: &mut BalanceOf| { - let imbalance = T::Currency::slash_reserved(curator, *curator_deposit).0; - T::OnSlash::on_unbalanced(imbalance); - *curator_deposit = Zero::zero(); - }; + let slash_curator = + |curator: &T::AccountId, curator_deposit: &mut BalanceOf| { + let imbalance = + T::Currency::slash_reserved(curator, *curator_deposit).0; + T::OnSlash::on_unbalanced(imbalance); + *curator_deposit = Zero::zero(); + }; match child_bounty.status { ChildBountyStatus::Added => { diff --git a/substrate/frame/examples/offchain-worker/src/tests.rs b/substrate/frame/examples/offchain-worker/src/tests.rs index b665cbbb62ae..741adbe6d26a 100644 --- a/substrate/frame/examples/offchain-worker/src/tests.rs +++ b/substrate/frame/examples/offchain-worker/src/tests.rs @@ -266,11 +266,12 @@ fn should_submit_unsigned_transaction_on_chain_for_any_account() { { assert_eq!(body, price_payload); - let signature_valid = - ::Public, - frame_system::pallet_prelude::BlockNumberFor, - > as SignedPayload>::verify::(&price_payload, signature); + let signature_valid = ::Public, + frame_system::pallet_prelude::BlockNumberFor, + > as SignedPayload>::verify::( + &price_payload, signature + ); assert!(signature_valid); } @@ -320,11 +321,12 @@ fn should_submit_unsigned_transaction_on_chain_for_all_accounts() { { assert_eq!(body, price_payload); - let signature_valid = - ::Public, - frame_system::pallet_prelude::BlockNumberFor, - > as SignedPayload>::verify::(&price_payload, signature); + let signature_valid = ::Public, + frame_system::pallet_prelude::BlockNumberFor, + > as SignedPayload>::verify::( + &price_payload, signature + ); assert!(signature_valid); } diff --git a/substrate/frame/nis/src/lib.rs b/substrate/frame/nis/src/lib.rs index 016daa4cb78b..87e2276e768d 100644 --- a/substrate/frame/nis/src/lib.rs +++ b/substrate/frame/nis/src/lib.rs @@ -756,15 +756,13 @@ pub mod pallet { .map(|_| ()) // We ignore this error as it just means the amount we're trying to deposit is // dust and the beneficiary account doesn't exist. - .or_else( - |e| { - if e == TokenError::CannotCreate.into() { - Ok(()) - } else { - Err(e) - } - }, - )?; + .or_else(|e| { + if e == TokenError::CannotCreate.into() { + Ok(()) + } else { + Err(e) + } + })?; summary.receipts_on_hold.saturating_reduce(on_hold); } T::Currency::release(&HoldReason::NftReceipt.into(), &who, amount, Exact)?; diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs index 1039b288b2ae..e83f28b472cd 100644 --- a/substrate/frame/referenda/src/types.rs +++ b/substrate/frame/referenda/src/types.rs @@ -258,7 +258,8 @@ impl< Tally: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone, AccountId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone, ScheduleAddress: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone, - > ReferendumInfo + > + ReferendumInfo { /// Take the Decision Deposit from `self`, if there is one. Returns an `Err` if `self` is not /// in a valid state for the Decision Deposit to be refunded. diff --git a/substrate/frame/revive/proc-macro/src/lib.rs b/substrate/frame/revive/proc-macro/src/lib.rs index 95f4110a2d76..012b4bfab9a9 100644 --- a/substrate/frame/revive/proc-macro/src/lib.rs +++ b/substrate/frame/revive/proc-macro/src/lib.rs @@ -349,18 +349,19 @@ where let Some(ident) = path.path.get_ident() else { panic!("Type needs to be ident"); }; - let size = - if ident == "i8" || - ident == "i16" || ident == "i32" || - ident == "u8" || ident == "u16" || - ident == "u32" - { - 1 - } else if ident == "i64" || ident == "u64" { - 2 - } else { - panic!("Pass by value only supports primitives"); - }; + let size = if ident == "i8" || + ident == "i16" || + ident == "i32" || + ident == "u8" || + ident == "u16" || + ident == "u32" + { + 1 + } else if ident == "i64" || ident == "u64" { + 2 + } else { + panic!("Pass by value only supports primitives"); + }; registers_used += size; if registers_used > ALLOWED_REGISTERS { return quote! { diff --git a/substrate/frame/society/src/tests.rs b/substrate/frame/society/src/tests.rs index df8e844cdad9..2a13f99855b5 100644 --- a/substrate/frame/society/src/tests.rs +++ b/substrate/frame/society/src/tests.rs @@ -281,7 +281,7 @@ fn bidding_works() { // No more candidates satisfy the requirements assert_eq!(candidacies(), vec![]); assert_ok!(Society::defender_vote(Origin::signed(10), true)); // Keep defender around - // Next period + // Next period run_to_block(16); // Same members assert_eq!(members(), vec![10, 30, 40, 50]); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index ab2c00ca9ccc..dd178a95bec5 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7995,7 +7995,7 @@ mod ledger_recovery { assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // OK assert_eq!(Bonded::::get(&333), Some(444)); // OK assert!(Payee::::get(&333).is_some()); // OK - // however, ledger associated with its controller was killed. + // however, ledger associated with its controller was killed. assert!(Ledger::::get(&444).is_none()); // NOK // side effects on 444 - ledger, bonded, payee, lock should be completely removed. diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 346dff46f12e..68ced1bc0ed3 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -400,18 +400,19 @@ impl CallDef { } for (feeless_arg, arg) in feeless_check.inputs.iter().skip(1).zip(args.iter()) { - let feeless_arg_type = - if let syn::Pat::Type(syn::PatType { ty, .. }) = feeless_arg.clone() { - if let syn::Type::Reference(pat) = *ty { - pat.elem.clone() - } else { - let msg = "Invalid pallet::call, feeless_if closure argument must be a reference"; - return Err(syn::Error::new(ty.span(), msg)); - } + let feeless_arg_type = if let syn::Pat::Type(syn::PatType { ty, .. }) = + feeless_arg.clone() + { + if let syn::Type::Reference(pat) = *ty { + pat.elem.clone() } else { - let msg = "Invalid pallet::call, feeless_if closure argument must be a type ascription pattern"; - return Err(syn::Error::new(feeless_arg.span(), msg)); - }; + let msg = "Invalid pallet::call, feeless_if closure argument must be a reference"; + return Err(syn::Error::new(ty.span(), msg)); + } + } else { + let msg = "Invalid pallet::call, feeless_if closure argument must be a type ascription pattern"; + return Err(syn::Error::new(feeless_arg.span(), msg)); + }; if feeless_arg_type != arg.2 { let msg = diff --git a/substrate/frame/support/src/storage/types/double_map.rs b/substrate/frame/support/src/storage/types/double_map.rs index c70d9de54467..24aad3de0b33 100644 --- a/substrate/frame/support/src/storage/types/double_map.rs +++ b/substrate/frame/support/src/storage/types/double_map.rs @@ -129,7 +129,8 @@ impl OnEmpty, MaxValues, >, - > where + > +where Prefix: StorageInstance, Hasher1: crate::hash::StorageHasher, Hasher2: crate::hash::StorageHasher, diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index 8dbeecd8e860..a7465c87fb27 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -197,7 +197,8 @@ impl TryDecodeEntireS QueryKind, OnEmpty, MaxValues, - > where + > +where Prefix: CountedStorageMapInstance, Hasher: StorageHasher, Key: FullCodec, @@ -229,7 +230,8 @@ impl QueryKind, OnEmpty, MaxValues, - > where + > +where Prefix: StorageInstance, Hasher1: StorageHasher, Key1: FullCodec, diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index 7f1ce0556eab..3d6aa1d83749 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -2431,9 +2431,10 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // any storage version "enabled". assert!( ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) - .unwrap_err() == "On chain storage version set, while the pallet \ + .unwrap_err() == + "On chain storage version set, while the pallet \ doesn't have the `#[pallet::storage_version(VERSION)]` attribute." - .into() + .into() ); }); } diff --git a/substrate/frame/transaction-payment/src/tests.rs b/substrate/frame/transaction-payment/src/tests.rs index 35d5322a6f33..bac89967d6af 100644 --- a/substrate/frame/transaction-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/src/tests.rs @@ -273,8 +273,10 @@ fn signed_ext_length_fee_is_also_updated_per_congestion() { NextFeeMultiplier::::put(Multiplier::saturating_from_rational(3, 2)); let len = 10; - assert_ok!(ChargeTransactionPayment::::from(10) // tipped - .pre_dispatch(&1, CALL, &info_from_weight(Weight::from_parts(3, 0)), len)); + assert_ok!( + ChargeTransactionPayment::::from(10) // tipped + .pre_dispatch(&1, CALL, &info_from_weight(Weight::from_parts(3, 0)), len) + ); assert_eq!( Balances::free_balance(1), 100 // original diff --git a/substrate/frame/utility/src/lib.rs b/substrate/frame/utility/src/lib.rs index ed5544fe55ca..a4f66298f3fa 100644 --- a/substrate/frame/utility/src/lib.rs +++ b/substrate/frame/utility/src/lib.rs @@ -134,8 +134,8 @@ pub mod pallet { fn batched_calls_limit() -> u32 { let allocator_limit = sp_core::MAX_POSSIBLE_ALLOCATION; let call_size = ((core::mem::size_of::<::RuntimeCall>() as u32 + - CALL_ALIGN - 1) / CALL_ALIGN) * - CALL_ALIGN; + CALL_ALIGN - 1) / + CALL_ALIGN) * CALL_ALIGN; // The margin to take into account vec doubling capacity. let margin_factor = 3; diff --git a/substrate/frame/vesting/src/tests.rs b/substrate/frame/vesting/src/tests.rs index 004da0dfbfa1..57cb59f27a4d 100644 --- a/substrate/frame/vesting/src/tests.rs +++ b/substrate/frame/vesting/src/tests.rs @@ -182,7 +182,7 @@ fn unvested_balance_should_not_transfer() { ExtBuilder::default().existential_deposit(10).build().execute_with(|| { let user1_free_balance = Balances::free_balance(&1); assert_eq!(user1_free_balance, 100); // Account 1 has free balance - // Account 1 has only 5 units vested at block 1 (plus 50 unvested) + // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); // Account 1 cannot send more than vested amount... assert_noop!(Balances::transfer_allow_death(Some(1).into(), 2, 56), TokenError::Frozen); @@ -194,7 +194,7 @@ fn vested_balance_should_transfer() { ExtBuilder::default().existential_deposit(10).build().execute_with(|| { let user1_free_balance = Balances::free_balance(&1); assert_eq!(user1_free_balance, 100); // Account 1 has free balance - // Account 1 has only 5 units vested at block 1 (plus 50 unvested) + // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest(Some(1).into())); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 55)); @@ -232,7 +232,7 @@ fn vested_balance_should_transfer_using_vest_other() { ExtBuilder::default().existential_deposit(10).build().execute_with(|| { let user1_free_balance = Balances::free_balance(&1); assert_eq!(user1_free_balance, 100); // Account 1 has free balance - // Account 1 has only 5 units vested at block 1 (plus 50 unvested) + // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest_other(Some(2).into(), 1)); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 55)); @@ -286,7 +286,7 @@ fn extra_balance_should_transfer() { assert_eq!(Vesting::vesting_balance(&2), Some(200)); assert_ok!(Vesting::vest(Some(2).into())); assert_ok!(Balances::transfer_allow_death(Some(2).into(), 3, 100)); // Account 2 can send extra - // units gained + // units gained }); } @@ -296,7 +296,7 @@ fn liquid_funds_should_transfer_with_delayed_vesting() { let user12_free_balance = Balances::free_balance(&12); assert_eq!(user12_free_balance, 2560); // Account 12 has free balance - // Account 12 has liquid funds + // Account 12 has liquid funds assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - 256 * 5)); // Account 12 has delayed vesting diff --git a/substrate/utils/wasm-builder/src/wasm_project.rs b/substrate/utils/wasm-builder/src/wasm_project.rs index 3fc8dfbc7f35..dcd5e6f1ade9 100644 --- a/substrate/utils/wasm-builder/src/wasm_project.rs +++ b/substrate/utils/wasm-builder/src/wasm_project.rs @@ -601,9 +601,10 @@ fn project_enabled_features( // We don't want to enable the `std`/`default` feature for the wasm build and // we need to check if the feature is enabled by checking the env variable. *f != "std" && - *f != "default" && env::var(format!("CARGO_FEATURE_{feature_env}")) - .map(|v| v == "1") - .unwrap_or_default() + *f != "default" && + env::var(format!("CARGO_FEATURE_{feature_env}")) + .map(|v| v == "1") + .unwrap_or_default() }) .map(|d| d.0.clone()) .collect::>(); From 34393dbb23bb335f70981aa298b5ab1366984616 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 27 Sep 2024 14:18:56 +0200 Subject: [PATCH 24/28] Update substrate/client/rpc-spec-v2/Cargo.toml --- substrate/client/rpc-spec-v2/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index 644af61401c8..58dd8b830beb 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -60,4 +60,3 @@ assert_matches = { workspace = true } pretty_assertions = { workspace = true } sc-transaction-pool = { workspace = true, default-features = true } sc-utils = { workspace = true, default-features = true } -#tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } From 25051cdf30ca48a87782042ca1ae2e25a8c36cbf Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 1 Oct 2024 15:07:02 +0200 Subject: [PATCH 25/28] Update prdoc/pr_5741.prdoc Co-authored-by: James Wilson --- prdoc/pr_5741.prdoc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/prdoc/pr_5741.prdoc b/prdoc/pr_5741.prdoc index 3e676f924d56..5eafbc90ee85 100644 --- a/prdoc/pr_5741.prdoc +++ b/prdoc/pr_5741.prdoc @@ -6,9 +6,10 @@ title: make RPC endpoint `chainHead_v1_storage` faster doc: - audience: Node Operator description: | - The RPC endpoint `chainHead_v1_storage` has removed the static limit for the number - of storage queries instead it has removed all static limits and relies on backpressure. - This should improve the throughput for bigger storage queries significantly. + The RPC endpoint `chainHead_v1_storage` now relies solely on backpressure to + determine how quickly to serve back values instead of handing back a fixed number + of entries and then expecting the client to ask for more. This should improve the + throughput for bigger storage queries significantly. Benchmarks using subxt on localhost: - Iterate over 10 accounts on westend-dev -> ~2-3x faster From 9152f9e315cdec246d6a1db1983ecc0f1313d2aa Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 2 Oct 2024 17:14:20 +0200 Subject: [PATCH 26/28] remove needless trait bounds --- substrate/client/rpc-spec-v2/src/archive/archive_storage.rs | 6 +++--- substrate/client/rpc-spec-v2/src/common/storage.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs index 132b95623580..26e7c299de41 100644 --- a/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs +++ b/substrate/client/rpc-spec-v2/src/archive/archive_storage.rs @@ -55,9 +55,9 @@ impl ArchiveStorage { impl ArchiveStorage where - Block: BlockT + Send + 'static, - BE: Backend + Send + 'static, - Client: StorageProvider + Send + Sync + 'static, + Block: BlockT + 'static, + BE: Backend + 'static, + Client: StorageProvider + 'static, { /// Generate the response of the `archive_storage` method. pub fn handle_query( diff --git a/substrate/client/rpc-spec-v2/src/common/storage.rs b/substrate/client/rpc-spec-v2/src/common/storage.rs index 0328f12abbd0..2e24a8da8ca8 100644 --- a/substrate/client/rpc-spec-v2/src/common/storage.rs +++ b/substrate/client/rpc-spec-v2/src/common/storage.rs @@ -75,9 +75,9 @@ pub type QueryIterResult = Result<(Vec, Option), Strin impl Storage where - Block: BlockT + Send + 'static, - BE: Backend + Send + 'static, - Client: StorageProvider + Send + Sync + 'static, + Block: BlockT + 'static, + BE: Backend + 'static, + Client: StorageProvider + 'static, { /// Fetch the value from storage. pub fn query_value( From bb251b03cee1a8dcc1f08a85b9bce7c10111fdec Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 2 Oct 2024 17:32:40 +0200 Subject: [PATCH 27/28] remove more needless trait bounds --- substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs | 5 +---- substrate/client/service/src/builder.rs | 8 +------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs index 3b0b9bb63b50..a88e7f2a0b3a 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -50,7 +50,7 @@ use sp_api::CallApiAt; use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use sp_core::{traits::CallContext, Bytes}; use sp_rpc::list::ListOrValue; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_runtime::traits::Block as BlockT; use std::{marker::PhantomData, sync::Arc, time::Duration}; use tokio::sync::mpsc; @@ -190,9 +190,6 @@ where + CallApiAt + StorageProvider + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { fn chain_head_unstable_follow(&self, pending: PendingSubscriptionSink, with_runtime: bool) { let subscriptions = self.subscriptions.clone(); diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index fa32749bbe22..f27b7ec6fbad 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -86,7 +86,7 @@ use sp_consensus::block_validation::{ }; use sp_core::traits::{CodeExecutor, SpawnNamed}; use sp_keystore::KeystorePtr; -use sp_runtime::traits::{Block as BlockT, BlockIdTo, Header as HeaderT, NumberFor, Zero}; +use sp_runtime::traits::{Block as BlockT, BlockIdTo, NumberFor, Zero}; use std::{str::FromStr, sync::Arc, time::SystemTime}; /// Full client type. @@ -405,9 +405,6 @@ where TBl::Header: Unpin, TBackend: 'static + sc_client_api::backend::Backend + Send, TExPool: MaintainedTransactionPool::Hash> + 'static, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { let SpawnTasksParams { mut config, @@ -672,9 +669,6 @@ where TExPool: MaintainedTransactionPool::Hash> + 'static, TBl::Hash: Unpin, TBl::Header: Unpin, - <>::State as sc_client_api::StateBackend< - <::Header as HeaderT>::Hashing, - >>::RawIter: Send, { let system_info = sc_rpc::system::SystemInfo { chain_name: chain_spec.name().into(), From 49e013c9541b2d632faf9854330201f2367c04a5 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 2 Oct 2024 17:54:50 +0200 Subject: [PATCH 28/28] remove more needless trait bounds again --- .../client/rpc-spec-v2/src/chain_head/chain_head_storage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs index f7e9e004f8b5..936117e66f98 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_storage.rs @@ -51,8 +51,8 @@ impl ChainHeadStorage { impl ChainHeadStorage where - Block: BlockT + Send + 'static, - BE: Backend + Send + 'static, + Block: BlockT + 'static, + BE: Backend + 'static, Client: StorageProvider + Send + Sync + 'static, { /// Generate the block events for the `chainHead_storage` method.