From f2a7aca24edd8ef211d2b39e6eddb4aad0f3e36a Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 20 Nov 2023 13:15:05 +0100 Subject: [PATCH 1/5] zombienet: polkadot-image fixed for cumulus tests --- .gitlab/pipeline/zombienet/cumulus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/pipeline/zombienet/cumulus.yml b/.gitlab/pipeline/zombienet/cumulus.yml index 3cac67c2966e..3e4df000b7f7 100644 --- a/.gitlab/pipeline/zombienet/cumulus.yml +++ b/.gitlab/pipeline/zombienet/cumulus.yml @@ -28,7 +28,7 @@ - job: build-push-image-test-parachain artifacts: true variables: - POLKADOT_IMAGE: "docker.io/paritypr/polkadot-debug:master" + POLKADOT_IMAGE: "docker.io/paritypr/polkadot-debug:${DOCKER_IMAGES_VERSION}" GH_DIR: "https://github.com/paritytech/cumulus/tree/${CI_COMMIT_SHORT_SHA}/zombienet/tests" LOCAL_DIR: "/builds/parity/mirrors/polkadot-sdk/cumulus/zombienet/tests" COL_IMAGE: "docker.io/paritypr/test-parachain:${DOCKER_IMAGES_VERSION}" From 58005bc98412dddfc890b8895aec6f8a245b0adf Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 20 Nov 2023 17:43:32 +0100 Subject: [PATCH 2/5] Add back chain-api subsystem --- Cargo.lock | 4 + .../relay-chain-minimal-node/Cargo.toml | 4 + .../src/blockchain_rpc_client.rs | 91 ++++++++++++++++++- .../src/collator_overseer.rs | 3 +- 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b2d655e6f41c..4f698c9609f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4183,6 +4183,7 @@ dependencies = [ "polkadot-core-primitives", "polkadot-network-bridge", "polkadot-node-collation-generation", + "polkadot-node-core-chain-api", "polkadot-node-core-prospective-parachains", "polkadot-node-core-runtime-api", "polkadot-node-network-protocol", @@ -4190,16 +4191,19 @@ dependencies = [ "polkadot-overseer", "polkadot-primitives", "sc-authority-discovery", + "sc-client-api", "sc-network", "sc-network-common", "sc-service", "sc-tracing", "sc-utils", "sp-api", + "sp-blockchain", "sp-consensus", "sp-consensus-babe", "sp-runtime", "substrate-prometheus-endpoint", + "tokio", "tracing", ] diff --git a/cumulus/client/relay-chain-minimal-node/Cargo.toml b/cumulus/client/relay-chain-minimal-node/Cargo.toml index 53173fb41189..ce76fc5cd6d2 100644 --- a/cumulus/client/relay-chain-minimal-node/Cargo.toml +++ b/cumulus/client/relay-chain-minimal-node/Cargo.toml @@ -19,6 +19,7 @@ polkadot-collator-protocol = { path = "../../../polkadot/node/network/collator-p polkadot-network-bridge = { path = "../../../polkadot/node/network/bridge" } polkadot-node-collation-generation = { path = "../../../polkadot/node/collation-generation" } polkadot-node-core-runtime-api = { path = "../../../polkadot/node/core/runtime-api" } +polkadot-node-core-chain-api = { path = "../../../polkadot/node/core/chain-api" } polkadot-node-core-prospective-parachains = { path = "../../../polkadot/node/core/prospective-parachains" } # substrate deps @@ -26,6 +27,7 @@ sc-authority-discovery = { path = "../../../substrate/client/authority-discovery sc-network = { path = "../../../substrate/client/network" } sc-network-common = { path = "../../../substrate/client/network/common" } sc-service = { path = "../../../substrate/client/service" } +sc-client-api = { path = "../../../substrate/client/api" } substrate-prometheus-endpoint = { path = "../../../substrate/utils/prometheus" } sc-tracing = { path = "../../../substrate/client/tracing" } sc-utils = { path = "../../../substrate/client/utils" } @@ -33,6 +35,8 @@ sp-api = { path = "../../../substrate/primitives/api" } sp-consensus-babe = { path = "../../../substrate/primitives/consensus/babe" } sp-consensus = { path = "../../../substrate/primitives/consensus/common" } sp-runtime = { path = "../../../substrate/primitives/runtime" } +sp-blockchain = { path = "../../../substrate/primitives/blockchain" } +tokio = { version = "1.32.0", features = ["macros"] } # cumulus deps cumulus-relay-chain-interface = { path = "../relay-chain-interface" } diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index a473b3bced02..6070744073ba 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -27,7 +27,10 @@ use polkadot_primitives::{ vstaging::NodeFeatures, }; use sc_authority_discovery::{AuthorityDiscovery, Error as AuthorityDiscoveryError}; -use sp_api::{ApiError, RuntimeApiInfo}; +use sc_client_api::AuxStore; +use sp_api::{ApiError, BlockT, HeaderT, NumberFor, RuntimeApiInfo}; +use sp_blockchain::{BlockStatus, HeaderBackend, Info}; +use std::future::Future; #[derive(Clone)] pub struct BlockChainRpcClient { @@ -403,3 +406,89 @@ impl BlockChainRpcClient { Ok(self.rpc_client.get_finalized_heads_stream()?.boxed()) } } + +// Implementation required by Availability-Distribution subsystem +// but never called in our case. +impl AuxStore for BlockChainRpcClient { + fn insert_aux< + 'a, + 'b: 'a, + 'c: 'a, + I: IntoIterator, + D: IntoIterator, + >( + &self, + _insert: I, + _delete: D, + ) -> sp_blockchain::Result<()> { + unimplemented!("Not supported on the RPC collator") + } + + fn get_aux(&self, _key: &[u8]) -> sp_blockchain::Result>> { + unimplemented!("Not supported on the RPC collator") + } +} + +fn block_local(fut: impl Future) -> T { + let tokio_handle = tokio::runtime::Handle::current(); + tokio::task::block_in_place(|| tokio_handle.block_on(fut)) +} + +impl HeaderBackend for BlockChainRpcClient { + fn header( + &self, + hash: ::Hash, + ) -> sp_blockchain::Result::Header>> { + Ok(block_local(self.rpc_client.chain_get_header(Some(hash)))?) + } + + fn info(&self) -> Info { + let best_header = block_local(self.rpc_client.chain_get_header(None)) + .expect("Unable to get header from relay chain.") + .unwrap(); + let genesis_hash = block_local(self.rpc_client.chain_get_head(Some(0))) + .expect("Unable to get header from relay chain."); + let finalized_head = block_local(self.rpc_client.chain_get_finalized_head()) + .expect("Unable to get finalized head from relay chain."); + let finalized_header = block_local(self.rpc_client.chain_get_header(Some(finalized_head))) + .expect("Unable to get finalized header from relay chain.") + .unwrap(); + Info { + best_hash: best_header.hash(), + best_number: best_header.number, + genesis_hash, + finalized_hash: finalized_head, + finalized_number: finalized_header.number, + finalized_state: None, + number_leaves: 1, + block_gap: None, + } + } + + fn status( + &self, + hash: ::Hash, + ) -> sp_blockchain::Result { + if self.header(hash)?.is_some() { + Ok(BlockStatus::InChain) + } else { + Ok(BlockStatus::Unknown) + } + } + + fn number( + &self, + hash: ::Hash, + ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { + let result = block_local(self.rpc_client.chain_get_header(Some(hash)))? + .map(|maybe_header| maybe_header.number); + Ok(result) + } + + fn hash( + &self, + number: NumberFor, + ) -> sp_blockchain::Result::Hash>> { + Ok(block_local(self.rpc_client.chain_get_block_hash(number.into()))?) + } +} diff --git a/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs b/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs index 945344f85e97..a785a9f6f79c 100644 --- a/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs +++ b/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs @@ -24,6 +24,7 @@ use polkadot_network_bridge::{ NetworkBridgeTx as NetworkBridgeTxSubsystem, }; use polkadot_node_collation_generation::CollationGenerationSubsystem; +use polkadot_node_core_chain_api::ChainApiSubsystem; use polkadot_node_core_prospective_parachains::ProspectiveParachainsSubsystem; use polkadot_node_core_runtime_api::RuntimeApiSubsystem; use polkadot_node_network_protocol::{ @@ -112,7 +113,7 @@ fn build_overseer( .candidate_backing(DummySubsystem) .candidate_validation(DummySubsystem) .pvf_checker(DummySubsystem) - .chain_api(DummySubsystem) + .chain_api(ChainApiSubsystem::new(runtime_client.clone(), Metrics::register(registry)?)) .collation_generation(CollationGenerationSubsystem::new(Metrics::register(registry)?)) .collator_protocol({ let side = ProtocolSide::Collator { From 3e55996dfd39b158db0be84302b12a03db4c12b3 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 21 Nov 2023 09:45:23 +0100 Subject: [PATCH 3/5] Introduce ChainApiBackend trait --- Cargo.lock | 2 + .../src/blockchain_rpc_client.rs | 128 +++++++++--------- polkadot/node/core/chain-api/Cargo.toml | 5 +- polkadot/node/core/chain-api/src/lib.rs | 82 ++++++----- polkadot/node/core/chain-api/src/tests.rs | 21 +-- polkadot/node/overseer/src/lib.rs | 4 +- polkadot/node/subsystem-types/Cargo.toml | 1 + polkadot/node/subsystem-types/src/lib.rs | 2 +- .../subsystem-types/src/runtime_client.rs | 57 +++++++- 9 files changed, 183 insertions(+), 119 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4f698c9609f3..b86e735ffe80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12447,6 +12447,7 @@ dependencies = [ "polkadot-node-primitives", "polkadot-node-subsystem", "polkadot-node-subsystem-test-helpers", + "polkadot-node-subsystem-types", "polkadot-primitives", "sc-client-api", "sc-consensus-babe", @@ -12854,6 +12855,7 @@ dependencies = [ "smallvec", "sp-api", "sp-authority-discovery", + "sp-blockchain", "sp-consensus-babe", "substrate-prometheus-endpoint", "thiserror", diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index 6070744073ba..8758b6d87d93 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -20,7 +20,7 @@ use cumulus_relay_chain_interface::{RelayChainError, RelayChainResult}; use cumulus_relay_chain_rpc_interface::RelayChainRpcClient; use futures::{Stream, StreamExt}; use polkadot_core_primitives::{Block, BlockNumber, Hash, Header}; -use polkadot_overseer::RuntimeApiSubsystemClient; +use polkadot_overseer::{ChainApiBackend, RuntimeApiSubsystemClient}; use polkadot_primitives::{ async_backing::{AsyncBackingParams, BackingState}, slashing, @@ -29,8 +29,7 @@ use polkadot_primitives::{ use sc_authority_discovery::{AuthorityDiscovery, Error as AuthorityDiscoveryError}; use sc_client_api::AuxStore; use sp_api::{ApiError, BlockT, HeaderT, NumberFor, RuntimeApiInfo}; -use sp_blockchain::{BlockStatus, HeaderBackend, Info}; -use std::future::Future; +use sp_blockchain::Info; #[derive(Clone)] pub struct BlockChainRpcClient { @@ -57,6 +56,65 @@ impl BlockChainRpcClient { } } +#[async_trait::async_trait] +impl ChainApiBackend for BlockChainRpcClient { + async fn header( + &self, + hash: ::Hash, + ) -> sp_blockchain::Result::Header>> { + Ok(self.rpc_client.chain_get_header(Some(hash)).await?) + } + + async fn info(&self) -> sp_blockchain::Result> { + let (best_header_opt, genesis_hash, finalized_head) = futures::try_join!( + self.rpc_client.chain_get_header(None), + self.rpc_client.chain_get_head(Some(0)), + self.rpc_client.chain_get_finalized_head() + )?; + let best_header = best_header_opt.ok_or_else(|| { + RelayChainError::GenericError( + "Unable to retrieve best header from relay chain.".to_string(), + ) + })?; + + let finalized_header = + self.rpc_client.chain_get_header(Some(finalized_head)).await?.ok_or_else(|| { + RelayChainError::GenericError( + "Unable to retrieve finalized header from relay chain.".to_string(), + ) + })?; + Ok(Info { + best_hash: best_header.hash(), + best_number: best_header.number, + genesis_hash, + finalized_hash: finalized_head, + finalized_number: finalized_header.number, + finalized_state: Some((finalized_header.hash(), finalized_header.number)), + number_leaves: 1, + block_gap: None, + }) + } + + async fn number( + &self, + hash: ::Hash, + ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { + let result = self + .rpc_client + .chain_get_header(Some(hash)) + .await? + .map(|maybe_header| maybe_header.number); + Ok(result) + } + + async fn hash( + &self, + number: NumberFor, + ) -> sp_blockchain::Result::Hash>> { + Ok(self.rpc_client.chain_get_block_hash(number.into()).await?) + } +} + #[async_trait::async_trait] impl RuntimeApiSubsystemClient for BlockChainRpcClient { async fn validators( @@ -428,67 +486,3 @@ impl AuxStore for BlockChainRpcClient { unimplemented!("Not supported on the RPC collator") } } - -fn block_local(fut: impl Future) -> T { - let tokio_handle = tokio::runtime::Handle::current(); - tokio::task::block_in_place(|| tokio_handle.block_on(fut)) -} - -impl HeaderBackend for BlockChainRpcClient { - fn header( - &self, - hash: ::Hash, - ) -> sp_blockchain::Result::Header>> { - Ok(block_local(self.rpc_client.chain_get_header(Some(hash)))?) - } - - fn info(&self) -> Info { - let best_header = block_local(self.rpc_client.chain_get_header(None)) - .expect("Unable to get header from relay chain.") - .unwrap(); - let genesis_hash = block_local(self.rpc_client.chain_get_head(Some(0))) - .expect("Unable to get header from relay chain."); - let finalized_head = block_local(self.rpc_client.chain_get_finalized_head()) - .expect("Unable to get finalized head from relay chain."); - let finalized_header = block_local(self.rpc_client.chain_get_header(Some(finalized_head))) - .expect("Unable to get finalized header from relay chain.") - .unwrap(); - Info { - best_hash: best_header.hash(), - best_number: best_header.number, - genesis_hash, - finalized_hash: finalized_head, - finalized_number: finalized_header.number, - finalized_state: None, - number_leaves: 1, - block_gap: None, - } - } - - fn status( - &self, - hash: ::Hash, - ) -> sp_blockchain::Result { - if self.header(hash)?.is_some() { - Ok(BlockStatus::InChain) - } else { - Ok(BlockStatus::Unknown) - } - } - - fn number( - &self, - hash: ::Hash, - ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { - let result = block_local(self.rpc_client.chain_get_header(Some(hash)))? - .map(|maybe_header| maybe_header.number); - Ok(result) - } - - fn hash( - &self, - number: NumberFor, - ) -> sp_blockchain::Result::Hash>> { - Ok(block_local(self.rpc_client.chain_get_block_hash(number.into()))?) - } -} diff --git a/polkadot/node/core/chain-api/Cargo.toml b/polkadot/node/core/chain-api/Cargo.toml index 154fa20e75d0..fa824e78ffee 100644 --- a/polkadot/node/core/chain-api/Cargo.toml +++ b/polkadot/node/core/chain-api/Cargo.toml @@ -9,10 +9,9 @@ description = "The Chain API subsystem provides access to chain related utility [dependencies] futures = "0.3.21" gum = { package = "tracing-gum", path = "../../gum" } -sp-blockchain = { path = "../../../../substrate/primitives/blockchain" } -polkadot-primitives = { path = "../../../primitives" } polkadot-node-metrics = { path = "../../metrics" } polkadot-node-subsystem = { path = "../../subsystem" } +polkadot-node-subsystem-types = { path = "../../subsystem-types" } sc-client-api = { path = "../../../../substrate/client/api" } sc-consensus-babe = { path = "../../../../substrate/client/consensus/babe" } @@ -21,5 +20,7 @@ futures = { version = "0.3.21", features = ["thread-pool"] } maplit = "1.0.2" parity-scale-codec = "3.6.1" polkadot-node-primitives = { path = "../../primitives" } +polkadot-primitives = { path = "../../../primitives" } polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } sp-core = { path = "../../../../substrate/primitives/core" } +sp-blockchain = { path = "../../../../substrate/primitives/blockchain" } diff --git a/polkadot/node/core/chain-api/src/lib.rs b/polkadot/node/core/chain-api/src/lib.rs index 9b25481d7186..0635d7e1386b 100644 --- a/polkadot/node/core/chain-api/src/lib.rs +++ b/polkadot/node/core/chain-api/src/lib.rs @@ -35,13 +35,13 @@ use std::sync::Arc; use futures::prelude::*; use sc_client_api::AuxStore; -use sp_blockchain::HeaderBackend; +use futures::stream::StreamExt; use polkadot_node_subsystem::{ messages::ChainApiMessage, overseer, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, SubsystemResult, }; -use polkadot_primitives::Block; +use polkadot_node_subsystem_types::ChainApiBackend; mod metrics; use self::metrics::Metrics; @@ -67,7 +67,7 @@ impl ChainApiSubsystem { #[overseer::subsystem(ChainApi, error = SubsystemError, prefix = self::overseer)] impl ChainApiSubsystem where - Client: HeaderBackend + AuxStore + 'static, + Client: ChainApiBackend + AuxStore + 'static, { fn start(self, ctx: Context) -> SpawnedSubsystem { let future = run::(ctx, self) @@ -83,7 +83,7 @@ async fn run( subsystem: ChainApiSubsystem, ) -> SubsystemResult<()> where - Client: HeaderBackend + AuxStore, + Client: ChainApiBackend + AuxStore, { loop { match ctx.recv().await? { @@ -93,13 +93,15 @@ where FromOrchestra::Communication { msg } => match msg { ChainApiMessage::BlockNumber(hash, response_channel) => { let _timer = subsystem.metrics.time_block_number(); - let result = subsystem.client.number(hash).map_err(|e| e.to_string().into()); + let result = + subsystem.client.number(hash).await.map_err(|e| e.to_string().into()); subsystem.metrics.on_request(result.is_ok()); let _ = response_channel.send(result); }, ChainApiMessage::BlockHeader(hash, response_channel) => { let _timer = subsystem.metrics.time_block_header(); - let result = subsystem.client.header(hash).map_err(|e| e.to_string().into()); + let result = + subsystem.client.header(hash).await.map_err(|e| e.to_string().into()); subsystem.metrics.on_request(result.is_ok()); let _ = response_channel.send(result); }, @@ -113,46 +115,54 @@ where ChainApiMessage::FinalizedBlockHash(number, response_channel) => { let _timer = subsystem.metrics.time_finalized_block_hash(); // Note: we don't verify it's finalized - let result = subsystem.client.hash(number).map_err(|e| e.to_string().into()); + let result = + subsystem.client.hash(number).await.map_err(|e| e.to_string().into()); subsystem.metrics.on_request(result.is_ok()); let _ = response_channel.send(result); }, ChainApiMessage::FinalizedBlockNumber(response_channel) => { let _timer = subsystem.metrics.time_finalized_block_number(); - let result = subsystem.client.info().finalized_number; - // always succeeds - subsystem.metrics.on_request(true); - let _ = response_channel.send(Ok(result)); + let result = subsystem + .client + .info() + .await + .map_err(|e| e.to_string().into()) + .map(|info| info.finalized_number); + subsystem.metrics.on_request(result.is_ok()); + let _ = response_channel.send(result); }, ChainApiMessage::Ancestors { hash, k, response_channel } => { let _timer = subsystem.metrics.time_ancestors(); gum::trace!(target: LOG_TARGET, hash=%hash, k=k, "ChainApiMessage::Ancestors"); - let mut hash = hash; - - let next_parent = core::iter::from_fn(|| { - let maybe_header = subsystem.client.header(hash); - match maybe_header { - // propagate the error - Err(e) => { - let e = e.to_string().into(); - Some(Err(e)) - }, - // fewer than `k` ancestors are available - Ok(None) => None, - Ok(Some(header)) => { - // stop at the genesis header. - if header.number == 0 { - None - } else { - hash = header.parent_hash; - Some(Ok(hash)) - } - }, - } - }); - - let result = next_parent.take(k).collect::, _>>(); + let initial_hash = hash; + let closure_client = subsystem.client.clone(); + + let next_parent_stream = futures::stream::unfold( + (initial_hash, closure_client), + |(hash, client)| async move { + let maybe_header = client.header(hash).await; + match maybe_header { + // propagate the error + Err(e) => { + let e = e.to_string().into(); + Some((Err(e), (hash, client))) + }, + // fewer than `k` ancestors are available + Ok(None) => None, + Ok(Some(header)) => { + // stop at the genesis header. + if header.number == 0 { + None + } else { + Some((Ok(header.parent_hash), (header.parent_hash, client))) + } + }, + } + }, + ); + + let result = next_parent_stream.take(k).try_collect().await; subsystem.metrics.on_request(result.is_ok()); let _ = response_channel.send(result); }, diff --git a/polkadot/node/core/chain-api/src/tests.rs b/polkadot/node/core/chain-api/src/tests.rs index 331a4f9ba820..eae8f6fa4ac5 100644 --- a/polkadot/node/core/chain-api/src/tests.rs +++ b/polkadot/node/core/chain-api/src/tests.rs @@ -22,7 +22,8 @@ use std::collections::BTreeMap; use polkadot_node_primitives::BlockWeight; use polkadot_node_subsystem_test_helpers::{make_subsystem_context, TestSubsystemContextHandle}; -use polkadot_primitives::{BlockNumber, Hash, Header}; +use polkadot_node_subsystem_types::ChainApiBackend; +use polkadot_primitives::{Block, BlockNumber, Hash, Header}; use sp_blockchain::Info as BlockInfo; use sp_core::testing::TaskExecutor; @@ -110,7 +111,7 @@ fn last_key_value(map: &BTreeMap) -> (K, V) { map.iter().last().map(|(k, v)| (k.clone(), v.clone())).unwrap() } -impl HeaderBackend for TestClient { +impl sp_blockchain::HeaderBackend for TestClient { fn info(&self) -> BlockInfo { let genesis_hash = self.blocks.iter().next().map(|(h, _)| *h).unwrap(); let (best_hash, best_number) = last_key_value(&self.blocks); @@ -191,8 +192,8 @@ fn request_block_number() { async move { let zero = Hash::zero(); let test_cases = [ - (TWO, client.number(TWO).unwrap()), - (zero, client.number(zero).unwrap()), // not here + (TWO, client.number(TWO).await.unwrap()), + (zero, client.number(zero).await.unwrap()), // not here ]; for (hash, expected) in &test_cases { let (tx, rx) = oneshot::channel(); @@ -217,8 +218,10 @@ fn request_block_header() { test_harness(|client, mut sender| { async move { const NOT_HERE: Hash = Hash::repeat_byte(0x5); - let test_cases = - [(TWO, client.header(TWO).unwrap()), (NOT_HERE, client.header(NOT_HERE).unwrap())]; + let test_cases = [ + (TWO, client.header(TWO).await.unwrap()), + (NOT_HERE, client.header(NOT_HERE).await.unwrap()), + ]; for (hash, expected) in &test_cases { let (tx, rx) = oneshot::channel(); @@ -270,8 +273,8 @@ fn request_finalized_hash() { test_harness(|client, mut sender| { async move { let test_cases = [ - (1, client.hash(1).unwrap()), // not here - (2, client.hash(2).unwrap()), + (1, client.hash(1).await.unwrap()), // not here + (2, client.hash(2).await.unwrap()), ]; for (number, expected) in &test_cases { let (tx, rx) = oneshot::channel(); @@ -297,7 +300,7 @@ fn request_last_finalized_number() { async move { let (tx, rx) = oneshot::channel(); - let expected = client.info().finalized_number; + let expected = client.info().await.unwrap().finalized_number; sender .send(FromOrchestra::Communication { msg: ChainApiMessage::FinalizedBlockNumber(tx), diff --git a/polkadot/node/overseer/src/lib.rs b/polkadot/node/overseer/src/lib.rs index 5207bb830d8c..da99546a44f7 100644 --- a/polkadot/node/overseer/src/lib.rs +++ b/polkadot/node/overseer/src/lib.rs @@ -87,8 +87,8 @@ use polkadot_node_subsystem_types::messages::{ pub use polkadot_node_subsystem_types::{ errors::{SubsystemError, SubsystemResult}, - jaeger, ActivatedLeaf, ActiveLeavesUpdate, OverseerSignal, RuntimeApiSubsystemClient, - UnpinHandle, + jaeger, ActivatedLeaf, ActiveLeavesUpdate, ChainApiBackend, OverseerSignal, + RuntimeApiSubsystemClient, UnpinHandle, }; pub mod metrics; diff --git a/polkadot/node/subsystem-types/Cargo.toml b/polkadot/node/subsystem-types/Cargo.toml index 9fd3775da591..8e345cf222c6 100644 --- a/polkadot/node/subsystem-types/Cargo.toml +++ b/polkadot/node/subsystem-types/Cargo.toml @@ -17,6 +17,7 @@ polkadot-node-jaeger = { path = "../jaeger" } orchestra = { version = "0.3.3", default-features = false, features=["futures_channel"] } sc-network = { path = "../../../substrate/client/network" } sp-api = { path = "../../../substrate/primitives/api" } +sp-blockchain = { path = "../../../substrate/primitives/blockchain" } sp-consensus-babe = { path = "../../../substrate/primitives/consensus/babe" } sp-authority-discovery = { path = "../../../substrate/primitives/authority-discovery" } sc-client-api = { path = "../../../substrate/client/api" } diff --git a/polkadot/node/subsystem-types/src/lib.rs b/polkadot/node/subsystem-types/src/lib.rs index e3d6e4decf20..cd39aa03e567 100644 --- a/polkadot/node/subsystem-types/src/lib.rs +++ b/polkadot/node/subsystem-types/src/lib.rs @@ -40,7 +40,7 @@ pub mod errors; pub mod messages; mod runtime_client; -pub use runtime_client::{DefaultSubsystemClient, RuntimeApiSubsystemClient}; +pub use runtime_client::{ChainApiBackend, DefaultSubsystemClient, RuntimeApiSubsystemClient}; pub use jaeger::*; pub use polkadot_node_jaeger as jaeger; diff --git a/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index 8369fd215f4f..36e3365cf08c 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -18,17 +18,70 @@ use async_trait::async_trait; use polkadot_primitives::{ async_backing, runtime_api::ParachainHost, slashing, vstaging, Block, BlockNumber, CandidateCommitments, CandidateEvent, CandidateHash, CommittedCandidateReceipt, CoreState, - DisputeState, ExecutorParams, GroupRotationInfo, Hash, Id, InboundDownwardMessage, + DisputeState, ExecutorParams, GroupRotationInfo, Hash, Header, Id, InboundDownwardMessage, InboundHrmpMessage, OccupiedCoreAssumption, PersistedValidationData, PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, }; +use sc_client_api::HeaderBackend; use sc_transaction_pool_api::OffchainTransactionPoolFactory; -use sp_api::{ApiError, ApiExt, ProvideRuntimeApi}; +use sp_api::{ApiError, ApiExt, HeaderT, NumberFor, ProvideRuntimeApi}; use sp_authority_discovery::AuthorityDiscoveryApi; +use sp_blockchain::Info; use sp_consensus_babe::{BabeApi, Epoch}; use std::{collections::BTreeMap, sync::Arc}; +/// Offers header utilities. +/// +/// This is a async wrapper trait for ['HeaderBackend'] to be used with the +/// `ChainApiSubsystem`. +// This trait was introduced to suit the needs of collators. Depending on their operating mode, they +// might not have a client of the relay chain that can supply a synchronous HeaderBackend +// implementation. +#[async_trait] +pub trait ChainApiBackend: Send + Sync { + /// Get block header. Returns `None` if block is not found. + async fn header(&self, hash: Hash) -> sp_blockchain::Result>; + /// Get blockchain info. + async fn info(&self) -> sp_blockchain::Result>; + /// Get block number by hash. Returns `None` if the header is not in the chain. + async fn number( + &self, + hash: Hash, + ) -> sp_blockchain::Result::Number>>; + /// Get block hash by number. Returns `None` if the header is not in the chain. + async fn hash(&self, number: NumberFor) -> sp_blockchain::Result>; +} + +#[async_trait] +impl ChainApiBackend for T +where + T: HeaderBackend, +{ + /// Get block header. Returns `None` if block is not found. + async fn header(&self, hash: Hash) -> sp_blockchain::Result> { + HeaderBackend::header(self, hash) + } + + /// Get blockchain info. + async fn info(&self) -> sp_blockchain::Result> { + Ok(HeaderBackend::info(self)) + } + + /// Get block number by hash. Returns `None` if the header is not in the chain. + async fn number( + &self, + hash: Hash, + ) -> sp_blockchain::Result::Number>> { + HeaderBackend::number(self, hash) + } + + /// Get block hash by number. Returns `None` if the header is not in the chain. + async fn hash(&self, number: NumberFor) -> sp_blockchain::Result> { + HeaderBackend::hash(self, number) + } +} + /// Exposes all runtime calls that are used by the runtime API subsystem. #[async_trait] pub trait RuntimeApiSubsystemClient { From 0de345a72462d81b27a571ca86e1833131389cdb Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 22 Nov 2023 12:33:47 +0100 Subject: [PATCH 4/5] cleanup --- .../relay-chain-minimal-node/src/blockchain_rpc_client.rs | 2 +- polkadot/node/core/chain-api/src/lib.rs | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index 8758b6d87d93..d2fc58a3a45f 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -465,7 +465,7 @@ impl BlockChainRpcClient { } } -// Implementation required by Availability-Distribution subsystem +// Implementation required by ChainApiSubsystem // but never called in our case. impl AuxStore for BlockChainRpcClient { fn insert_aux< diff --git a/polkadot/node/core/chain-api/src/lib.rs b/polkadot/node/core/chain-api/src/lib.rs index 0635d7e1386b..7fd5166310fe 100644 --- a/polkadot/node/core/chain-api/src/lib.rs +++ b/polkadot/node/core/chain-api/src/lib.rs @@ -135,11 +135,8 @@ where let _timer = subsystem.metrics.time_ancestors(); gum::trace!(target: LOG_TARGET, hash=%hash, k=k, "ChainApiMessage::Ancestors"); - let initial_hash = hash; - let closure_client = subsystem.client.clone(); - let next_parent_stream = futures::stream::unfold( - (initial_hash, closure_client), + (hash, subsystem.client.clone()), |(hash, client)| async move { let maybe_header = client.header(hash).await; match maybe_header { From 9934ed59293dc18f146d92bc6ddff092dd15571c Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 22 Nov 2023 15:36:53 +0100 Subject: [PATCH 5/5] Update cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> --- .../relay-chain-minimal-node/src/blockchain_rpc_client.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index d2fc58a3a45f..c40ca5c858ba 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -99,12 +99,11 @@ impl ChainApiBackend for BlockChainRpcClient { &self, hash: ::Hash, ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { - let result = self + Ok(self .rpc_client .chain_get_header(Some(hash)) .await? - .map(|maybe_header| maybe_header.number); - Ok(result) + .map(|maybe_header| maybe_header.number)) } async fn hash(