From ec1b613e11789173f2b76b14439bfe03645f5ff4 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 8 Feb 2023 15:59:20 +0200 Subject: [PATCH 01/35] rpc/chainhead: Test unpin for noncanonical prunned blocks Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/tests.rs | 108 +++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index 4084075f0b321..0ff890435f717 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1020,3 +1020,111 @@ async fn follow_prune_best_block() { }); assert_eq!(event, expected); } + +#[tokio::test] +async fn follow_unpin_pruned_block() { + let builder = TestClientBuilder::new(); + let backend = builder.backend(); + let mut client = Arc::new(builder.build()); + + let api = ChainHead::new( + client.clone(), + backend, + Arc::new(TaskExecutor::default()), + CHAIN_GENESIS, + MAX_PINNED_BLOCKS, + ) + .into_rpc(); + + // Block tree before the subscription: + // + // finalized -> block 1 -> block 2 + // ^^^ finalized + // -> block 1 -> block 3 + // + + let block_1 = client.new_block(Default::default()).unwrap().build().unwrap().block; + client.import(BlockOrigin::Own, block_1.clone()).await.unwrap(); + + let block_2 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_2_hash = block_2.header.hash(); + client.import(BlockOrigin::Own, block_2.clone()).await.unwrap(); + + // Block 3 with parent Block 1 is not the best imported. + let mut block_builder = client + .new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false) + .unwrap(); + // This push is required as otherwise block 3 has the same hash as block 2 and won't get + // imported + block_builder + .push_transfer(Transfer { + from: AccountKeyring::Alice.into(), + to: AccountKeyring::Ferdie.into(), + amount: 41, + nonce: 0, + }) + .unwrap(); + let block_3 = block_builder.build().unwrap().block; + let block_3_hash = block_3.header.hash(); + client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); + + // Block 3 is not pruned, pruning happens at height (N - 1). + client.finalize_block(block_2_hash, None).unwrap(); + + let mut sub = api.subscribe("chainHead_unstable_follow", [false]).await.unwrap(); + let sub_id = sub.subscription_id(); + let sub_id = serde_json::to_string(&sub_id).unwrap(); + + // Initialized must always be reported first. + let event: FollowEvent = get_next_event(&mut sub).await; + println!("Event: {:?}", event); + let expected = FollowEvent::Initialized(Initialized { + finalized_block_hash: format!("{:?}", block_2_hash), + finalized_block_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + // Block tree: + // + // finalized -> block 1 -> block 2 -> block 4 + // ^^^ finalized + // -> block 1 -> block 3 + // + // Mark block 4 as finalized to force block 3 to get pruned. + + let block_4 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_4_hash = block_4.header.hash(); + client.import(BlockOrigin::Own, block_4.clone()).await.unwrap(); + + client.finalize_block(block_4_hash, None).unwrap(); + + // Check block 4. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_4_hash), + parent_block_hash: format!("{:?}", block_2_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::BestBlockChanged(BestBlockChanged { + best_block_hash: format!("{:?}", block_4_hash), + }); + assert_eq!(event, expected); + + // Block 3 must be pruned. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::Finalized(Finalized { + finalized_block_hashes: vec![ + format!("{:?}", block_4_hash), + ], + pruned_block_hashes: vec![format!("{:?}", block_3_hash),], + }); + assert_eq!(event, expected); + + // Pruned hash can be unpinned. + let hash = format!("{:?}", block_3_hash); + let _res: () = api.call("chainHead_unstable_unpin", [&sub_id, &hash]).await.unwrap(); +} From 1236d5955b16c49688835f917e4a79f796a478e2 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 13 Feb 2023 16:52:52 +0200 Subject: [PATCH 02/35] rpc/tests: Ensure fork is not reported by the Finalized event Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/tests.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index 0ff890435f717..2a4753a048127 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1065,15 +1065,12 @@ async fn follow_unpin_pruned_block() { }) .unwrap(); let block_3 = block_builder.build().unwrap().block; - let block_3_hash = block_3.header.hash(); client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); // Block 3 is not pruned, pruning happens at height (N - 1). client.finalize_block(block_2_hash, None).unwrap(); let mut sub = api.subscribe("chainHead_unstable_follow", [false]).await.unwrap(); - let sub_id = sub.subscription_id(); - let sub_id = serde_json::to_string(&sub_id).unwrap(); // Initialized must always be reported first. let event: FollowEvent = get_next_event(&mut sub).await; @@ -1085,10 +1082,10 @@ async fn follow_unpin_pruned_block() { }); assert_eq!(event, expected); - // Block tree: + // Block tree: // // finalized -> block 1 -> block 2 -> block 4 - // ^^^ finalized + // ^^^ finalized // -> block 1 -> block 3 // // Mark block 4 as finalized to force block 3 to get pruned. @@ -1114,17 +1111,11 @@ async fn follow_unpin_pruned_block() { }); assert_eq!(event, expected); - // Block 3 must be pruned. + // Block 3 must not be reported as pruned. let event: FollowEvent = get_next_event(&mut sub).await; let expected = FollowEvent::Finalized(Finalized { - finalized_block_hashes: vec![ - format!("{:?}", block_4_hash), - ], - pruned_block_hashes: vec![format!("{:?}", block_3_hash),], + finalized_block_hashes: vec![format!("{:?}", block_4_hash)], + pruned_block_hashes: vec![], }); assert_eq!(event, expected); - - // Pruned hash can be unpinned. - let hash = format!("{:?}", block_3_hash); - let _res: () = api.call("chainHead_unstable_unpin", [&sub_id, &hash]).await.unwrap(); } From 477029f921f5ca60036a70e6b7f131f94f25b2fe Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 13 Feb 2023 17:13:18 +0200 Subject: [PATCH 03/35] rpc/chainhead: Detect pruned forks to ignore from events Signed-off-by: Alexandru Vasile --- Cargo.lock | 1 + client/rpc-spec-v2/Cargo.toml | 1 + .../rpc-spec-v2/src/chain_head/chain_head.rs | 133 +++++++++++++----- 3 files changed, 102 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3e5ab65c0ce20..a4a7b9af1c84c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8874,6 +8874,7 @@ dependencies = [ "futures", "futures-util", "hex", + "itertools", "jsonrpsee", "log", "parity-scale-codec", diff --git a/client/rpc-spec-v2/Cargo.toml b/client/rpc-spec-v2/Cargo.toml index 43fb189081bae..bdb9ca941df9f 100644 --- a/client/rpc-spec-v2/Cargo.toml +++ b/client/rpc-spec-v2/Cargo.toml @@ -34,6 +34,7 @@ tokio-stream = { version = "0.1", features = ["sync"] } array-bytes = "4.1" log = "0.4.17" futures-util = { version = "0.3.19", default-features = false } +itertools = "0.10.3" [dev-dependencies] serde_json = "1.0" diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 2a9cabaf2b310..e94cb0cbf02b9 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -37,12 +37,14 @@ use futures::{ stream::{self, Stream, StreamExt}, }; use futures_util::future::Either; +use itertools::Itertools; use jsonrpsee::{ core::{async_trait, RpcResult}, types::{SubscriptionEmptyError, SubscriptionId, SubscriptionResult}, SubscriptionSink, }; use log::{debug, error}; +use parking_lot::RwLock; use sc_client_api::{ Backend, BlockBackend, BlockImportNotification, BlockchainEvents, CallExecutor, ChildInfo, ExecutorProvider, FinalityNotification, StorageKey, StorageProvider, @@ -50,15 +52,15 @@ use sc_client_api::{ use serde::Serialize; use sp_api::CallApiAt; use sp_blockchain::{ - Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, + Backend as BlockChainBackend, Error as BlockChainError, HashAndNumber, HeaderBackend, + HeaderMetadata, }; use sp_core::{hexdisplay::HexDisplay, storage::well_known_keys, Bytes}; use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, Header}, }; -use std::{marker::PhantomData, sync::Arc}; - +use std::{collections::HashSet, marker::PhantomData, sync::Arc}; /// An API for chain head RPC calls. pub struct ChainHead { /// Substrate client. @@ -126,16 +128,15 @@ impl ChainHead { /// /// This includes the "Initialized" event followed by the in-memory /// blocks via "NewBlock" and the "BestBlockChanged". -fn generate_initial_events( +fn generate_initial_events( client: &Arc, - backend: &Arc, handle: &SubscriptionHandle, runtime_updates: bool, + initial_blocks: Vec<(Block::Hash, Block::Hash)>, ) -> Result>, SubscriptionManagementError> where Block: BlockT + 'static, Block::Header: Unpin, - BE: Backend + 'static, Client: HeaderBackend + CallApiAt + 'static, { // The initialized event is the first one sent. @@ -155,7 +156,6 @@ where runtime_updates, }); - let initial_blocks = get_initial_blocks(&backend, finalized_block_hash); let mut in_memory_blocks = Vec::with_capacity(initial_blocks.len() + 1); in_memory_blocks.push(initialized_event); @@ -250,33 +250,71 @@ where } } +/// Internal representation of the initial blocks that should be +/// reported or ignored by the chainHead. +#[derive(Clone, Debug)] +struct InitialBlocks { + /// Children of the latest finalized block, for which the `NewBlock` + /// event must be generated. + /// + /// It is a tuple of (block hash, parent hash). + in_memory: Vec<(Block::Hash, Block::Hash)>, + /// Blocks that should not be reported as pruned by the `Finalized` event. + /// + /// The substrate database will perform the pruning of height N at + /// the finalization N + 1. We could have the following block tree + /// when the user subscribes to the `unfollow` method: + /// [A] - [A1] - [A2] - [A3] + /// ^^ finalized + /// - [A1] - [B1] + /// + /// When the A3 block is finalized, B1 is reported as pruned, however + /// B1 was never reported as `NewBlock` (and as such was never pinned). + /// This is because the `NewBlock` events are generated for children of + /// the finalized hash. + pruned_forks: HashSet, +} + /// Get the in-memory blocks of the client, starting from the provided finalized hash. -/// -/// Returns a tuple of block hash with parent hash. -fn get_initial_blocks( +fn get_initial_blocks_with_forks( backend: &Arc, - parent_hash: Block::Hash, -) -> Vec<(Block::Hash, Block::Hash)> + finalized: HashAndNumber, +) -> Result, SubscriptionManagementError> where Block: BlockT + 'static, BE: Backend + 'static, { - let mut result = Vec::new(); - let mut next_hash = Vec::new(); - next_hash.push(parent_hash); - - while let Some(parent_hash) = next_hash.pop() { - let Ok(blocks) = backend.blockchain().children(parent_hash) else { - continue - }; - - for child_hash in blocks { - result.push((child_hash, parent_hash)); - next_hash.push(child_hash); + let blockchain = backend.blockchain(); + let leaves = blockchain + .leaves() + .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; + let finalized_number = finalized.number; + let finalized = finalized.hash; + let mut pruned_forks = HashSet::new(); + let mut in_memory = Vec::new(); + for leaf in leaves { + let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf) + .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; + + // When the common block number is smaller than the current finalized + // block, the tree route is a fork that should be pruned. + let common = tree_route.common_block(); + let number = common.number; + let blocks = tree_route.enacted().iter().map(|block| block.hash); + if number < finalized_number { + pruned_forks.extend(blocks); + } else if number == finalized_number { + // Ensure a `NewBlock` event is generated for all children of the + // finalized block. Describe the tree route as (child_node, parent_node) + let parents = std::iter::once(finalized).chain(blocks.clone()); + in_memory.extend(blocks.zip(parents)); } } - result + // Keep unique blocks. + let in_memory: Vec<_> = in_memory.into_iter().unique().collect(); + + Ok(InitialBlocks { in_memory, pruned_forks }) } /// Submit the events from the provided stream to the RPC client @@ -374,6 +412,7 @@ fn handle_finalized_blocks( client: &Arc, handle: &SubscriptionHandle, notification: FinalityNotification, + pruned_forks: &Arc>>, ) -> Result<(FollowEvent, Option>), SubscriptionManagementError> where Block: BlockT + 'static, @@ -389,12 +428,30 @@ where let mut finalized_block_hashes = notification.tree_route.iter().cloned().collect::>(); finalized_block_hashes.push(last_finalized); - let pruned_block_hashes: Vec<_> = notification.stale_heads.iter().cloned().collect(); + // Report all pruned blocks from the notification that are not + // part of the `pruned_forks`. + let pruned_block_hashes: Vec<_> = { + let mut to_ignore = pruned_forks.write(); + + notification + .stale_heads + .iter() + .cloned() + .filter_map(|hash| { + if !to_ignore.contains(&hash) { + return Some(hash) + } - let finalized_event = FollowEvent::Finalized(Finalized { - finalized_block_hashes, - pruned_block_hashes: pruned_block_hashes.clone(), - }); + to_ignore.remove(&hash); + None + }) + .collect() + }; + + let finalized_event = + FollowEvent::Finalized(Finalized { finalized_block_hashes, pruned_block_hashes }); + + let pruned_block_hashes: Vec<_> = notification.stale_heads.iter().cloned().collect(); let mut best_block_cache = handle.best_block_write(); match *best_block_cache { @@ -485,6 +542,17 @@ where }; debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription accepted", sub_id); + // Get all block hashes from the node's memory. + let info = self.client.info(); + let finalized = HashAndNumber { hash: info.finalized_hash, number: info.finalized_number }; + let Ok(InitialBlocks { in_memory, pruned_forks}) = get_initial_blocks_with_forks(&self.backend, finalized) else { + // Note: This could be warp sync error. + debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to get all in-memory blocks", sub_id); + let _ = sink.send(&FollowEvent::::Stop); + return Ok(()) + }; + let pruned_forks = Arc::new(RwLock::new(pruned_forks)); + let client = self.client.clone(); let handle = sub_handle.clone(); let subscription_id = sub_id.clone(); @@ -513,7 +581,7 @@ where .client .finality_notification_stream() .map(move |notification| { - match handle_finalized_blocks(&client, &handle, notification) { + match handle_finalized_blocks(&client, &handle, notification, &pruned_forks) { Ok((finalized_event, None)) => stream::iter(vec![finalized_event]), Ok((finalized_event, Some(best_block))) => stream::iter(vec![best_block, finalized_event]), @@ -529,9 +597,8 @@ where let merged = tokio_stream::StreamExt::merge(stream_import, stream_finalized); let subscriptions = self.subscriptions.clone(); let client = self.client.clone(); - let backend = self.backend.clone(); let fut = async move { - let Ok(initial_events) = generate_initial_events(&client, &backend, &sub_handle, runtime_updates) else { + let Ok(initial_events) = generate_initial_events(&client, &sub_handle, runtime_updates, in_memory) else { // Stop the subscription if we exceeded the maximum number of blocks pinned. debug!(target: "rpc-spec-v2", "[follow][id={:?}] Exceeded max pinned blocks from initial events", sub_id); let _ = sink.send(&FollowEvent::::Stop); From e3531a12f3b8e5cb6afe8754e4e08487d079e665 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 13 Feb 2023 17:17:42 +0200 Subject: [PATCH 04/35] rpc/tests: Check unpin can be called on pruned hashes Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/tests.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index 2a4753a048127..0c89f373ff8b4 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1019,10 +1019,16 @@ async fn follow_prune_best_block() { pruned_block_hashes: vec![format!("{:?}", block_2_hash)], }); assert_eq!(event, expected); + + // Pruned hash can be unpinned. + let sub_id = sub.subscription_id(); + let sub_id = serde_json::to_string(&sub_id).unwrap(); + let hash = format!("{:?}", block_2_hash); + let _res: () = api.call("chainHead_unstable_unpin", [&sub_id, &hash]).await.unwrap(); } #[tokio::test] -async fn follow_unpin_pruned_block() { +async fn follow_forks_pruned_block() { let builder = TestClientBuilder::new(); let backend = builder.backend(); let mut client = Arc::new(builder.build()); From 5288cd94ed3489b9420e0eeed5674940bc7de3f6 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 13 Feb 2023 18:57:06 +0200 Subject: [PATCH 05/35] Fix clippy Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index e94cb0cbf02b9..7c7554a452e8e 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -437,13 +437,13 @@ where .stale_heads .iter() .cloned() - .filter_map(|hash| { + .filter(|hash| { if !to_ignore.contains(&hash) { - return Some(hash) + return true } to_ignore.remove(&hash); - None + false }) .collect() }; From 904840d498bbfedba87fa9f9a8a7cedc1f95e118 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 16 Feb 2023 13:45:36 +0200 Subject: [PATCH 06/35] rpc/chain_head: Handle race with memory blocks and notifications Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 226 ++++++++++-------- 1 file changed, 131 insertions(+), 95 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 7c7554a452e8e..ab3fb189e6535 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -44,16 +44,13 @@ use jsonrpsee::{ SubscriptionSink, }; use log::{debug, error}; -use parking_lot::RwLock; use sc_client_api::{ Backend, BlockBackend, BlockImportNotification, BlockchainEvents, CallExecutor, ChildInfo, ExecutorProvider, FinalityNotification, StorageKey, StorageProvider, }; -use serde::Serialize; use sp_api::CallApiAt; use sp_blockchain::{ - Backend as BlockChainBackend, Error as BlockChainError, HashAndNumber, HeaderBackend, - HeaderMetadata, + Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, Info, }; use sp_core::{hexdisplay::HexDisplay, storage::well_known_keys, Bytes}; use sp_runtime::{ @@ -128,19 +125,26 @@ impl ChainHead { /// /// This includes the "Initialized" event followed by the in-memory /// blocks via "NewBlock" and the "BestBlockChanged". -fn generate_initial_events( +fn generate_initial_events( client: &Arc, + backend: &Arc, handle: &SubscriptionHandle, runtime_updates: bool, - initial_blocks: Vec<(Block::Hash, Block::Hash)>, -) -> Result>, SubscriptionManagementError> + info: &Info, +) -> Result<(Vec>, HashSet), SubscriptionManagementError> where Block: BlockT + 'static, Block::Header: Unpin, + BE: Backend + 'static, Client: HeaderBackend + CallApiAt + 'static, { + let Ok(ret) = get_initial_blocks_with_forks(&backend, &info) else { + return Err(SubscriptionManagementError::Custom("Failed to fetch initial blocks".into())); + }; + let initial_blocks = ret.in_memory; + // The initialized event is the first one sent. - let finalized_block_hash = client.info().finalized_hash; + let finalized_block_hash = info.finalized_hash; handle.pin_block(finalized_block_hash)?; let finalized_block_runtime = generate_runtime_event( @@ -180,13 +184,17 @@ where } // Generate a new best block event. - let best_block_hash = client.info().best_hash; + let best_block_hash = info.best_hash; if best_block_hash != finalized_block_hash { let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); + + let mut best_block_cache = handle.best_block_write(); + *best_block_cache = Some(best_block_hash); + in_memory_blocks.push(best_block); }; - Ok(in_memory_blocks) + Ok((in_memory_blocks, ret.pruned_forks)) } /// Parse hex-encoded string parameter as raw bytes. @@ -278,7 +286,7 @@ struct InitialBlocks { /// Get the in-memory blocks of the client, starting from the provided finalized hash. fn get_initial_blocks_with_forks( backend: &Arc, - finalized: HashAndNumber, + info: &Info, ) -> Result, SubscriptionManagementError> where Block: BlockT + 'static, @@ -288,8 +296,8 @@ where let leaves = blockchain .leaves() .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; - let finalized_number = finalized.number; - let finalized = finalized.hash; + let finalized_number = info.finalized_number; + let finalized = info.finalized_hash; let mut pruned_forks = HashSet::new(); let mut in_memory = Vec::new(); for leaf in leaves { @@ -319,13 +327,22 @@ where /// Submit the events from the provided stream to the RPC client /// for as long as the `rx_stop` event was not called. -async fn submit_events( +async fn submit_events( sink: &mut SubscriptionSink, mut stream: EventStream, + client: &Arc, + handle: &SubscriptionHandle, + runtime_updates: bool, + mut to_ignore: HashSet, + start_info: &Info, rx_stop: oneshot::Receiver<()>, ) where - EventStream: Stream + Unpin, - T: Serialize, + EventStream: Stream> + Unpin, + Block: BlockT + 'static, + Client: CallApiAt + + HeaderBackend + + HeaderMetadata + + 'static, { let mut stream_item = stream.next(); let mut stop_event = rx_stop; @@ -333,18 +350,37 @@ async fn submit_events( while let Either::Left((Some(event), next_stop_event)) = futures_util::future::select(stream_item, stop_event).await { - match sink.send(&event) { - Ok(true) => { - stream_item = stream.next(); - stop_event = next_stop_event; - }, - // Client disconnected. - Ok(false) => return, - Err(_) => { - // Failed to submit event. + let events = match event { + NotificationType::InitialEvents(events) => Ok(events), + NotificationType::NewBlock(notification) => + handle_import_blocks(client, handle, runtime_updates, notification), + NotificationType::Finalized(notification) => + handle_finalized_blocks(client, handle, notification, &mut to_ignore, &start_info), + }; + + let events = match events { + Ok(events) => events, + Err(err) => { + debug!(target: "rpc-spec-v2", "Failed to handle stream notification {:?}", err); break }, + }; + + for event in events { + match sink.send(&event) { + // Submitted successfully. + Ok(true) => continue, + // Client disconnected. + Ok(false) => return, + Err(_) => { + // Failed to submit event. + break + }, + } } + + stream_item = stream.next(); + stop_event = next_stop_event; } let _ = sink.send(&FollowEvent::::Stop); @@ -357,12 +393,17 @@ fn handle_import_blocks( handle: &SubscriptionHandle, runtime_updates: bool, notification: BlockImportNotification, -) -> Result<(FollowEvent, Option>), SubscriptionManagementError> +) -> Result>, SubscriptionManagementError> where Block: BlockT + 'static, Client: CallApiAt + 'static, { - handle.pin_block(notification.hash)?; + let is_new_pin = handle.pin_block(notification.hash)?; + // TODO: Also check number >= init info + if !is_new_pin { + // The block was already pinned by the initial block event in `generate_initial_events`. + return Ok(vec![]) + } let new_runtime = generate_runtime_event( &client, @@ -380,7 +421,7 @@ where }); if !notification.is_new_best { - return Ok((new_block, None)) + return Ok(vec![new_block]) } // If this is the new best block, then we need to generate two events. @@ -394,14 +435,14 @@ where // Note: This handles the race with the finalized branch. if block_cache != notification.hash { *best_block_cache = Some(notification.hash); - Ok((new_block, Some(best_block_event))) + Ok(vec![new_block, best_block_event]) } else { - Ok((new_block, None)) + Ok(vec![new_block]) } }, None => { *best_block_cache = Some(notification.hash); - Ok((new_block, Some(best_block_event))) + Ok(vec![new_block, best_block_event]) }, } } @@ -412,13 +453,20 @@ fn handle_finalized_blocks( client: &Arc, handle: &SubscriptionHandle, notification: FinalityNotification, - pruned_forks: &Arc>>, -) -> Result<(FollowEvent, Option>), SubscriptionManagementError> + to_ignore: &mut HashSet, + start_info: &Info, +) -> Result>, SubscriptionManagementError> where Block: BlockT + 'static, Client: HeaderBackend + HeaderMetadata + 'static, { let last_finalized = notification.hash; + let finalized_number = client.number(last_finalized.clone()).unwrap().unwrap(); + // Check if the finalized hash has been reported by the initial events. + if finalized_number < start_info.finalized_number { + return Ok(vec![]) + } + // We might not receive all new blocks reports, also pin the block here. handle.pin_block(last_finalized)?; @@ -431,8 +479,6 @@ where // Report all pruned blocks from the notification that are not // part of the `pruned_forks`. let pruned_block_hashes: Vec<_> = { - let mut to_ignore = pruned_forks.write(); - notification .stale_heads .iter() @@ -459,7 +505,7 @@ where // Check if the current best block is also reported as pruned. let reported_pruned = pruned_block_hashes.iter().find(|&&hash| hash == block_cache); if reported_pruned.is_none() { - return Ok((finalized_event, None)) + return Ok(vec![finalized_event]) } // The best block is reported as pruned. Therefore, we need to signal a new @@ -468,12 +514,10 @@ where if best_block_hash == block_cache { // The client doest not have any new information about the best block. // The information from `.info()` is updated from the DB as the last - // step of the finalization and it should be up to date. Also, the - // displaced nodes (list of nodes reported) should be reported with - // an offset of 32 blocks for substrate. + // step of the finalization and it should be up to date. // If the info is outdated, there is nothing the RPC can do for now. error!(target: "rpc-spec-v2", "Client does not contain different best block"); - Ok((finalized_event, None)) + Ok(vec![finalized_event]) } else { let ancestor = sp_blockchain::lowest_common_ancestor( &**client, @@ -497,13 +541,23 @@ where *best_block_cache = Some(best_block_hash); let best_block_event = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); - Ok((finalized_event, Some(best_block_event))) + Ok(vec![best_block_event, finalized_event]) } }, - None => Ok((finalized_event, None)), + None => Ok(vec![finalized_event]), } } +/// Internal representation of a block notification. +enum NotificationType { + /// The initial events generated from the node's memory. + InitialEvents(Vec>), + /// The new block notification obtained from `import_notification_stream`. + NewBlock(BlockImportNotification), + /// The finalized block notification obtained from `finality_notification_stream`. + Finalized(FinalityNotification), +} + #[async_trait] impl ChainHeadApiServer for ChainHead where @@ -542,73 +596,55 @@ where }; debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription accepted", sub_id); - // Get all block hashes from the node's memory. - let info = self.client.info(); - let finalized = HashAndNumber { hash: info.finalized_hash, number: info.finalized_number }; - let Ok(InitialBlocks { in_memory, pruned_forks}) = get_initial_blocks_with_forks(&self.backend, finalized) else { - // Note: This could be warp sync error. - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to get all in-memory blocks", sub_id); - let _ = sink.send(&FollowEvent::::Stop); - return Ok(()) - }; - let pruned_forks = Arc::new(RwLock::new(pruned_forks)); - - let client = self.client.clone(); - let handle = sub_handle.clone(); - let subscription_id = sub_id.clone(); - + // Register for the new block and finalized notifications. let stream_import = self .client .import_notification_stream() - .map(move |notification| { - match handle_import_blocks(&client, &handle, runtime_updates, notification) { - Ok((new_block, None)) => stream::iter(vec![new_block]), - Ok((new_block, Some(best_block))) => stream::iter(vec![new_block, best_block]), - Err(_) => { - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to handle block import notification.", subscription_id); - handle.stop(); - stream::iter(vec![]) - }, - } - }) - .flatten(); - - let client = self.client.clone(); - let handle = sub_handle.clone(); - let subscription_id = sub_id.clone(); + .map(|notification| NotificationType::NewBlock(notification)); let stream_finalized = self .client .finality_notification_stream() - .map(move |notification| { - match handle_finalized_blocks(&client, &handle, notification, &pruned_forks) { - Ok((finalized_event, None)) => stream::iter(vec![finalized_event]), - Ok((finalized_event, Some(best_block))) => - stream::iter(vec![best_block, finalized_event]), - Err(_) => { - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to import finalized blocks", subscription_id); - handle.stop(); - stream::iter(vec![]) - }, - } - }) - .flatten(); + .map(|notification| NotificationType::Finalized(notification)); let merged = tokio_stream::StreamExt::merge(stream_import, stream_finalized); + let subscriptions = self.subscriptions.clone(); + let backend = self.backend.clone(); let client = self.client.clone(); let fut = async move { - let Ok(initial_events) = generate_initial_events(&client, &sub_handle, runtime_updates, in_memory) else { - // Stop the subscription if we exceeded the maximum number of blocks pinned. - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Exceeded max pinned blocks from initial events", sub_id); - let _ = sink.send(&FollowEvent::::Stop); - return + let start_info = client.info(); + + let (initial_events, pruned_forks) = match generate_initial_events( + &client, + &backend, + &sub_handle, + runtime_updates, + &start_info, + ) { + Ok(blocks) => blocks, + Err(err) => { + debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to generate the initial events {:?}", sub_id, err); + let _ = sink.send(&FollowEvent::::Stop); + subscriptions.remove_subscription(&sub_id); + return + }, }; - let stream = stream::iter(initial_events).chain(merged); - - submit_events(&mut sink, stream.boxed(), rx_stop).await; - // The client disconnected or called the unsubscribe method. + let initial = NotificationType::InitialEvents(initial_events); + let stream = stream::once(futures::future::ready(initial)).chain(merged); + + submit_events( + &mut sink, + stream.boxed(), + &client, + &sub_handle, + runtime_updates, + pruned_forks, + &start_info, + rx_stop, + ) + .await; subscriptions.remove_subscription(&sub_id); debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription removed", sub_id); }; From 8dddb1e95fd8ae96cc27a97c1f8b8d6df6128b59 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 16 Feb 2023 14:55:09 +0200 Subject: [PATCH 07/35] rpc/chain_head: Add data config for the `follow` future Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 127 ++++++++++-------- 1 file changed, 69 insertions(+), 58 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index ab3fb189e6535..b08fac258c7e7 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -125,12 +125,8 @@ impl ChainHead { /// /// This includes the "Initialized" event followed by the in-memory /// blocks via "NewBlock" and the "BestBlockChanged". -fn generate_initial_events( - client: &Arc, - backend: &Arc, - handle: &SubscriptionHandle, - runtime_updates: bool, - info: &Info, +fn generate_initial_events( + data: &FollowData, ) -> Result<(Vec>, HashSet), SubscriptionManagementError> where Block: BlockT + 'static, @@ -138,21 +134,23 @@ where BE: Backend + 'static, Client: HeaderBackend + CallApiAt + 'static, { - let Ok(ret) = get_initial_blocks_with_forks(&backend, &info) else { + let client = &data.client; + let backend = &data.backend; + let start_info = &data.start_info; + let handle = &data.sub_handle; + let runtime_updates = data.runtime_updates; + + let Ok(ret) = get_initial_blocks_with_forks(backend, start_info) else { return Err(SubscriptionManagementError::Custom("Failed to fetch initial blocks".into())); }; - let initial_blocks = ret.in_memory; + let initial_blocks = ret.finalized_block_descendants; // The initialized event is the first one sent. - let finalized_block_hash = info.finalized_hash; + let finalized_block_hash = start_info.finalized_hash; handle.pin_block(finalized_block_hash)?; - let finalized_block_runtime = generate_runtime_event( - &client, - runtime_updates, - &BlockId::Hash(finalized_block_hash), - None, - ); + let finalized_block_runtime = + generate_runtime_event(client, runtime_updates, &BlockId::Hash(finalized_block_hash), None); let initialized_event = FollowEvent::Initialized(Initialized { finalized_block_hash, @@ -167,8 +165,8 @@ where handle.pin_block(child)?; let new_runtime = generate_runtime_event( - &client, - runtime_updates, + client, + data.runtime_updates, &BlockId::Hash(child), Some(&BlockId::Hash(parent)), ); @@ -184,7 +182,7 @@ where } // Generate a new best block event. - let best_block_hash = info.best_hash; + let best_block_hash = start_info.best_hash; if best_block_hash != finalized_block_hash { let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); @@ -266,12 +264,12 @@ struct InitialBlocks { /// event must be generated. /// /// It is a tuple of (block hash, parent hash). - in_memory: Vec<(Block::Hash, Block::Hash)>, + finalized_block_descendants: Vec<(Block::Hash, Block::Hash)>, /// Blocks that should not be reported as pruned by the `Finalized` event. /// /// The substrate database will perform the pruning of height N at /// the finalization N + 1. We could have the following block tree - /// when the user subscribes to the `unfollow` method: + /// when the user subscribes to the `follow` method: /// [A] - [A1] - [A2] - [A3] /// ^^ finalized /// - [A1] - [B1] @@ -299,7 +297,7 @@ where let finalized_number = info.finalized_number; let finalized = info.finalized_hash; let mut pruned_forks = HashSet::new(); - let mut in_memory = Vec::new(); + let mut finalized_block_descendants = Vec::new(); for leaf in leaves { let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf) .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; @@ -315,27 +313,24 @@ where // Ensure a `NewBlock` event is generated for all children of the // finalized block. Describe the tree route as (child_node, parent_node) let parents = std::iter::once(finalized).chain(blocks.clone()); - in_memory.extend(blocks.zip(parents)); + finalized_block_descendants.extend(blocks.zip(parents)); } } // Keep unique blocks. - let in_memory: Vec<_> = in_memory.into_iter().unique().collect(); + let finalized_block_descendants: Vec<_> = + finalized_block_descendants.into_iter().unique().collect(); - Ok(InitialBlocks { in_memory, pruned_forks }) + Ok(InitialBlocks { finalized_block_descendants, pruned_forks }) } /// Submit the events from the provided stream to the RPC client /// for as long as the `rx_stop` event was not called. -async fn submit_events( - sink: &mut SubscriptionSink, +async fn submit_events( + data: FollowData, mut stream: EventStream, - client: &Arc, - handle: &SubscriptionHandle, - runtime_updates: bool, mut to_ignore: HashSet, - start_info: &Info, - rx_stop: oneshot::Receiver<()>, + sub_id: String, ) where EventStream: Stream> + Unpin, Block: BlockT + 'static, @@ -345,17 +340,27 @@ async fn submit_events( + 'static, { let mut stream_item = stream.next(); - let mut stop_event = rx_stop; + let mut stop_event = data.rx_stop; + let mut sink = data.sink; while let Either::Left((Some(event), next_stop_event)) = futures_util::future::select(stream_item, stop_event).await { let events = match event { NotificationType::InitialEvents(events) => Ok(events), - NotificationType::NewBlock(notification) => - handle_import_blocks(client, handle, runtime_updates, notification), - NotificationType::Finalized(notification) => - handle_finalized_blocks(client, handle, notification, &mut to_ignore, &start_info), + NotificationType::NewBlock(notification) => handle_import_blocks( + &data.client, + &data.sub_handle, + data.runtime_updates, + notification, + ), + NotificationType::Finalized(notification) => handle_finalized_blocks( + &data.client, + &data.sub_handle, + notification, + &mut to_ignore, + &data.start_info, + ), }; let events = match events { @@ -384,6 +389,8 @@ async fn submit_events( } let _ = sink.send(&FollowEvent::::Stop); + data.subscriptions.remove_subscription(&sub_id); + debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription removed", sub_id); } /// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for @@ -558,6 +565,18 @@ enum NotificationType { Finalized(FinalityNotification), } +/// Data for the `follow` method to generate the block events. +struct FollowData { + client: Arc, + backend: Arc, + subscriptions: Arc>, + sub_handle: SubscriptionHandle, + rx_stop: oneshot::Receiver<()>, + sink: SubscriptionSink, + runtime_updates: bool, + start_info: Info, +} + #[async_trait] impl ChainHeadApiServer for ChainHead where @@ -614,19 +633,23 @@ where let client = self.client.clone(); let fut = async move { let start_info = client.info(); - - let (initial_events, pruned_forks) = match generate_initial_events( - &client, - &backend, - &sub_handle, + let mut follow_data = FollowData { + client, + backend, + subscriptions, + sub_handle, + rx_stop, + sink, runtime_updates, - &start_info, - ) { + start_info, + }; + + let (initial_events, pruned_forks) = match generate_initial_events(&follow_data) { Ok(blocks) => blocks, Err(err) => { debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to generate the initial events {:?}", sub_id, err); - let _ = sink.send(&FollowEvent::::Stop); - subscriptions.remove_subscription(&sub_id); + let _ = follow_data.sink.send(&FollowEvent::::Stop); + follow_data.subscriptions.remove_subscription(&sub_id); return }, }; @@ -634,19 +657,7 @@ where let initial = NotificationType::InitialEvents(initial_events); let stream = stream::once(futures::future::ready(initial)).chain(merged); - submit_events( - &mut sink, - stream.boxed(), - &client, - &sub_handle, - runtime_updates, - pruned_forks, - &start_info, - rx_stop, - ) - .await; - subscriptions.remove_subscription(&sub_id); - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription removed", sub_id); + submit_events(follow_data, stream.boxed(), pruned_forks, sub_id).await; }; self.executor.spawn("substrate-rpc-subscription", Some("rpc"), fut.boxed()); From b9e8f6f06a9194961058ac4747b8cd93d1ecd2f9 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 16 Feb 2023 15:59:50 +0200 Subject: [PATCH 08/35] rpc/chain_head: Address feedback Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 49 +++++++++---------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index b08fac258c7e7..5f9ba49bd20b8 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -58,6 +58,9 @@ use sp_runtime::{ traits::{Block as BlockT, Header}, }; use std::{collections::HashSet, marker::PhantomData, sync::Arc}; + +const LOG_TARGET: &str = "rpc-spec-v2"; + /// An API for chain head RPC calls. pub struct ChainHead { /// Substrate client. @@ -140,9 +143,7 @@ where let handle = &data.sub_handle; let runtime_updates = data.runtime_updates; - let Ok(ret) = get_initial_blocks_with_forks(backend, start_info) else { - return Err(SubscriptionManagementError::Custom("Failed to fetch initial blocks".into())); - }; + let ret = get_initial_blocks_with_forks(backend, start_info)?; let initial_blocks = ret.finalized_block_descendants; // The initialized event is the first one sent. @@ -267,7 +268,7 @@ struct InitialBlocks { finalized_block_descendants: Vec<(Block::Hash, Block::Hash)>, /// Blocks that should not be reported as pruned by the `Finalized` event. /// - /// The substrate database will perform the pruning of height N at + /// Substrate database will perform the pruning of height N at /// the finalization N + 1. We could have the following block tree /// when the user subscribes to the `follow` method: /// [A] - [A1] - [A2] - [A3] @@ -294,7 +295,6 @@ where let leaves = blockchain .leaves() .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; - let finalized_number = info.finalized_number; let finalized = info.finalized_hash; let mut pruned_forks = HashSet::new(); let mut finalized_block_descendants = Vec::new(); @@ -302,14 +302,10 @@ where let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf) .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; - // When the common block number is smaller than the current finalized - // block, the tree route is a fork that should be pruned. - let common = tree_route.common_block(); - let number = common.number; let blocks = tree_route.enacted().iter().map(|block| block.hash); - if number < finalized_number { + if !tree_route.retracted().is_empty() { pruned_forks.extend(blocks); - } else if number == finalized_number { + } else { // Ensure a `NewBlock` event is generated for all children of the // finalized block. Describe the tree route as (child_node, parent_node) let parents = std::iter::once(finalized).chain(blocks.clone()); @@ -366,7 +362,7 @@ async fn submit_events( let events = match events { Ok(events) => events, Err(err) => { - debug!(target: "rpc-spec-v2", "Failed to handle stream notification {:?}", err); + debug!(target: LOG_TARGET, "Failed to handle stream notification {:?}", err); break }, }; @@ -390,7 +386,7 @@ async fn submit_events( let _ = sink.send(&FollowEvent::::Stop); data.subscriptions.remove_subscription(&sub_id); - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription removed", sub_id); + debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", sub_id); } /// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for @@ -489,15 +485,8 @@ where notification .stale_heads .iter() + .filter(|hash| !to_ignore.remove(&hash)) .cloned() - .filter(|hash| { - if !to_ignore.contains(&hash) { - return true - } - - to_ignore.remove(&hash); - false - }) .collect() }; @@ -523,7 +512,7 @@ where // The information from `.info()` is updated from the DB as the last // step of the finalization and it should be up to date. // If the info is outdated, there is nothing the RPC can do for now. - error!(target: "rpc-spec-v2", "Client does not contain different best block"); + error!(target: LOG_TARGET, "Client does not contain different best block"); Ok(vec![finalized_event]) } else { let ancestor = sp_blockchain::lowest_common_ancestor( @@ -609,11 +598,11 @@ where let Some((rx_stop, sub_handle)) = self.subscriptions.insert_subscription(sub_id.clone(), runtime_updates, self.max_pinned_blocks) else { // Inserting the subscription can only fail if the JsonRPSee // generated a duplicate subscription ID. - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription already accepted", sub_id); + debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription already accepted", sub_id); let _ = sink.send(&FollowEvent::::Stop); return Ok(()) }; - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Subscription accepted", sub_id); + debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription accepted", sub_id); // Register for the new block and finalized notifications. let stream_import = self @@ -647,7 +636,10 @@ where let (initial_events, pruned_forks) = match generate_initial_events(&follow_data) { Ok(blocks) => blocks, Err(err) => { - debug!(target: "rpc-spec-v2", "[follow][id={:?}] Failed to generate the initial events {:?}", sub_id, err); + debug!( + target: LOG_TARGET, + "[follow][id={:?}] Failed to generate the initial events {:?}", sub_id, err + ); let _ = follow_data.sink.send(&FollowEvent::::Stop); follow_data.subscriptions.remove_subscription(&sub_id); return @@ -695,7 +687,12 @@ where }, Ok(None) => { // The block's body was pruned. This subscription ID has become invalid. - debug!(target: "rpc-spec-v2", "[body][id={:?}] Stopping subscription because hash={:?} was pruned", follow_subscription, hash); + debug!( + target: LOG_TARGET, + "[body][id={:?}] Stopping subscription because hash={:?} was pruned", + follow_subscription, + hash + ); handle.stop(); ChainHeadEvent::::Disjoint }, From a796034c741d13af510b81a2691139836cc5e30b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 16 Feb 2023 16:15:56 +0200 Subject: [PATCH 09/35] rpc/chain_head: Move best block cache on the data config Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 21 ++++++++++++------- .../src/chain_head/subscription.rs | 12 +---------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 5f9ba49bd20b8..3464c3c0a2c26 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -129,7 +129,7 @@ impl ChainHead { /// This includes the "Initialized" event followed by the in-memory /// blocks via "NewBlock" and the "BestBlockChanged". fn generate_initial_events( - data: &FollowData, + data: &mut FollowData, ) -> Result<(Vec>, HashSet), SubscriptionManagementError> where Block: BlockT + 'static, @@ -186,10 +186,7 @@ where let best_block_hash = start_info.best_hash; if best_block_hash != finalized_block_hash { let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); - - let mut best_block_cache = handle.best_block_write(); - *best_block_cache = Some(best_block_hash); - + data.best_block_cache = Some(best_block_hash); in_memory_blocks.push(best_block); }; @@ -338,6 +335,7 @@ async fn submit_events( let mut stream_item = stream.next(); let mut stop_event = data.rx_stop; let mut sink = data.sink; + let mut best_block_cache = data.best_block_cache; while let Either::Left((Some(event), next_stop_event)) = futures_util::future::select(stream_item, stop_event).await @@ -349,6 +347,7 @@ async fn submit_events( &data.sub_handle, data.runtime_updates, notification, + &mut best_block_cache, ), NotificationType::Finalized(notification) => handle_finalized_blocks( &data.client, @@ -356,6 +355,7 @@ async fn submit_events( notification, &mut to_ignore, &data.start_info, + &mut best_block_cache, ), }; @@ -396,6 +396,7 @@ fn handle_import_blocks( handle: &SubscriptionHandle, runtime_updates: bool, notification: BlockImportNotification, + best_block_cache: &mut Option, ) -> Result>, SubscriptionManagementError> where Block: BlockT + 'static, @@ -431,7 +432,6 @@ where let best_block_event = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash: notification.hash }); - let mut best_block_cache = handle.best_block_write(); match *best_block_cache { Some(block_cache) => { // The RPC layer has not reported this block as best before. @@ -458,6 +458,7 @@ fn handle_finalized_blocks( notification: FinalityNotification, to_ignore: &mut HashSet, start_info: &Info, + best_block_cache: &mut Option, ) -> Result>, SubscriptionManagementError> where Block: BlockT + 'static, @@ -495,7 +496,6 @@ where let pruned_block_hashes: Vec<_> = notification.stale_heads.iter().cloned().collect(); - let mut best_block_cache = handle.best_block_write(); match *best_block_cache { Some(block_cache) => { // Check if the current best block is also reported as pruned. @@ -563,7 +563,11 @@ struct FollowData { rx_stop: oneshot::Receiver<()>, sink: SubscriptionSink, runtime_updates: bool, + /// The starting point from which the blocks were generated from the + /// node's memory. start_info: Info, + /// The best reported block by this subscription. + best_block_cache: Option, } #[async_trait] @@ -631,9 +635,10 @@ where sink, runtime_updates, start_info, + best_block_cache: None, }; - let (initial_events, pruned_forks) = match generate_initial_events(&follow_data) { + let (initial_events, pruned_forks) = match generate_initial_events(&mut follow_data) { Ok(blocks) => blocks, Err(err) => { debug!( diff --git a/client/rpc-spec-v2/src/chain_head/subscription.rs b/client/rpc-spec-v2/src/chain_head/subscription.rs index 033db45ca755c..feeb26224ebb3 100644 --- a/client/rpc-spec-v2/src/chain_head/subscription.rs +++ b/client/rpc-spec-v2/src/chain_head/subscription.rs @@ -19,7 +19,7 @@ //! Subscription management for tracking subscription IDs to pinned blocks. use futures::channel::oneshot; -use parking_lot::{RwLock, RwLockWriteGuard}; +use parking_lot::RwLock; use sp_runtime::traits::Block as BlockT; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, @@ -53,10 +53,6 @@ struct SubscriptionInner { #[derive(Clone)] pub struct SubscriptionHandle { inner: Arc>>, - /// The best reported block by this subscription. - /// Have this as a separate variable to easily share - /// the write guard with the RPC layer. - best_block: Arc>>, } impl SubscriptionHandle { @@ -69,7 +65,6 @@ impl SubscriptionHandle { blocks: HashSet::new(), max_pinned_blocks, })), - best_block: Arc::new(RwLock::new(None)), } } @@ -125,11 +120,6 @@ impl SubscriptionHandle { let inner = self.inner.read(); inner.runtime_updates } - - /// Get the write guard of the best reported block. - pub fn best_block_write(&self) -> RwLockWriteGuard<'_, Option> { - self.best_block.write() - } } /// Manage block pinning / unpinning for subscription IDs. From 1d0b4d25dfe3f2e0af8a3f2eb9bad19c19c16d47 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 16 Feb 2023 18:35:54 +0200 Subject: [PATCH 10/35] rpc/chain_head: Send new events from the finalized stream Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 151 ++++++++++++++---- .../src/chain_head/subscription.rs | 9 ++ 2 files changed, 125 insertions(+), 35 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 3464c3c0a2c26..3149c0f86117c 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -55,7 +55,8 @@ use sp_blockchain::{ use sp_core::{hexdisplay::HexDisplay, storage::well_known_keys, Bytes}; use sp_runtime::{ generic::BlockId, - traits::{Block as BlockT, Header}, + traits::{Block as BlockT, Header, One}, + Saturating, }; use std::{collections::HashSet, marker::PhantomData, sync::Arc}; @@ -356,6 +357,7 @@ async fn submit_events( &mut to_ignore, &data.start_info, &mut best_block_cache, + data.runtime_updates, ), }; @@ -389,67 +391,93 @@ async fn submit_events( debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", sub_id); } -/// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for -/// every notification. -fn handle_import_blocks( +/// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for the +/// given block hash. +fn generate_import_event( client: &Arc, - handle: &SubscriptionHandle, runtime_updates: bool, - notification: BlockImportNotification, + block_hash: Block::Hash, + parent_block_hash: Block::Hash, + is_best_block: bool, best_block_cache: &mut Option, -) -> Result>, SubscriptionManagementError> +) -> Vec> where Block: BlockT + 'static, Client: CallApiAt + 'static, { - let is_new_pin = handle.pin_block(notification.hash)?; - // TODO: Also check number >= init info - if !is_new_pin { - // The block was already pinned by the initial block event in `generate_initial_events`. - return Ok(vec![]) - } - let new_runtime = generate_runtime_event( &client, runtime_updates, - &BlockId::Hash(notification.hash), - Some(&BlockId::Hash(*notification.header.parent_hash())), + &BlockId::Hash(block_hash), + Some(&BlockId::Hash(parent_block_hash)), ); // Note: `Block::Hash` will serialize to hexadecimal encoded string. let new_block = FollowEvent::NewBlock(NewBlock { - block_hash: notification.hash, - parent_block_hash: *notification.header.parent_hash(), + block_hash, + parent_block_hash, new_runtime, runtime_updates, }); - if !notification.is_new_best { - return Ok(vec![new_block]) + if !is_best_block { + return vec![new_block] } // If this is the new best block, then we need to generate two events. let best_block_event = - FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash: notification.hash }); + FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash: block_hash }); match *best_block_cache { Some(block_cache) => { // The RPC layer has not reported this block as best before. // Note: This handles the race with the finalized branch. - if block_cache != notification.hash { - *best_block_cache = Some(notification.hash); - Ok(vec![new_block, best_block_event]) + if block_cache != block_hash { + *best_block_cache = Some(block_hash); + vec![new_block, best_block_event] } else { - Ok(vec![new_block]) + vec![new_block] } }, None => { - *best_block_cache = Some(notification.hash); - Ok(vec![new_block, best_block_event]) + *best_block_cache = Some(block_hash); + vec![new_block, best_block_event] }, } } +/// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for +/// every notification. +fn handle_import_blocks( + client: &Arc, + handle: &SubscriptionHandle, + runtime_updates: bool, + notification: BlockImportNotification, + best_block_cache: &mut Option, +) -> Result>, SubscriptionManagementError> +where + Block: BlockT + 'static, + Client: CallApiAt + 'static, +{ + println!("New Block event: {:?}", notification.hash); + let is_new_pin = handle.pin_block(notification.hash)?; + // TODO: Also check number >= init info + if !is_new_pin { + // The block was already pinned by the initial block event in `generate_initial_events`, + // or by the finalized event in `handle_finalized_blocks`. + return Ok(vec![]) + } + + Ok(generate_import_event( + client, + runtime_updates, + notification.hash, + *notification.header.parent_hash(), + notification.is_new_best, + best_block_cache, + )) +} + /// Generate the "Finalized" event and potentially the "BestBlockChanged" for /// every notification. fn handle_finalized_blocks( @@ -459,20 +487,27 @@ fn handle_finalized_blocks( to_ignore: &mut HashSet, start_info: &Info, best_block_cache: &mut Option, + runtime_updates: bool, ) -> Result>, SubscriptionManagementError> where Block: BlockT + 'static, - Client: HeaderBackend + HeaderMetadata + 'static, + Client: HeaderBackend + + HeaderMetadata + + CallApiAt + + 'static, { + println!("Finalized block event: {:?}", notification.hash); let last_finalized = notification.hash; - let finalized_number = client.number(last_finalized.clone()).unwrap().unwrap(); + let Some(finalized_number) = client.number(last_finalized)? else { + return Err(SubscriptionManagementError::Custom("Block number not present in the database".into())) + }; + // Check if the finalized hash has been reported by the initial events. if finalized_number < start_info.finalized_number { return Ok(vec![]) } - // We might not receive all new blocks reports, also pin the block here. - handle.pin_block(last_finalized)?; + let mut events = Vec::new(); // The tree route contains the exclusive path from the last finalized block // to the block reported by the notification. Ensure the finalized block is @@ -480,6 +515,46 @@ where let mut finalized_block_hashes = notification.tree_route.iter().cloned().collect::>(); finalized_block_hashes.push(last_finalized); + // Find the parent hash of the first element from the vector such that + // we can group (finalized_hash, parent_hash) for the event generation. + let first_parent_hash = { + let hash = finalized_block_hashes.get(0).expect("Finalized hash was included; qed"); + let Some(number) = client.number(*hash)? else { + return Err(SubscriptionManagementError::Custom("Block number not present in the database".into())) + }; + let Some(parent) = client.hash(number.saturating_sub(One::one()))? else { + return Err(SubscriptionManagementError::Custom("Block hash not present in the database".into())) + }; + // The parent of the finalized block hash was not reported by the `chianHead`. + if !handle.contains_block(&parent) { + return Err(SubscriptionManagementError::Custom( + "Parent of the finalized is missing".into(), + )) + } + + parent + }; + + let parents = std::iter::once(&first_parent_hash).chain(finalized_block_hashes.iter()); + for (hash, parent) in finalized_block_hashes.iter().zip(parents) { + // We have reported this block before. + if !handle.pin_block(*hash)? { + continue + } + + // This is the first time we see this block. We must generate the + // `NewBlock` event. Additionally, for the last event, we must + // also generate the `BestBlock` event. + events.extend(generate_import_event( + client, + runtime_updates, + *hash, + *parent, + *hash == last_finalized, + best_block_cache, + )); + } + // Report all pruned blocks from the notification that are not // part of the `pruned_forks`. let pruned_block_hashes: Vec<_> = { @@ -501,7 +576,8 @@ where // Check if the current best block is also reported as pruned. let reported_pruned = pruned_block_hashes.iter().find(|&&hash| hash == block_cache); if reported_pruned.is_none() { - return Ok(vec![finalized_event]) + events.push(finalized_event); + return Ok(events) } // The best block is reported as pruned. Therefore, we need to signal a new @@ -513,7 +589,8 @@ where // step of the finalization and it should be up to date. // If the info is outdated, there is nothing the RPC can do for now. error!(target: LOG_TARGET, "Client does not contain different best block"); - Ok(vec![finalized_event]) + events.push(finalized_event); + Ok(events) } else { let ancestor = sp_blockchain::lowest_common_ancestor( &**client, @@ -537,10 +614,14 @@ where *best_block_cache = Some(best_block_hash); let best_block_event = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); - Ok(vec![best_block_event, finalized_event]) + events.extend([best_block_event, finalized_event]); + Ok(events) } }, - None => Ok(vec![finalized_event]), + None => { + events.push(finalized_event); + Ok(events) + }, } } diff --git a/client/rpc-spec-v2/src/chain_head/subscription.rs b/client/rpc-spec-v2/src/chain_head/subscription.rs index feeb26224ebb3..d9514ed4a15e7 100644 --- a/client/rpc-spec-v2/src/chain_head/subscription.rs +++ b/client/rpc-spec-v2/src/chain_head/subscription.rs @@ -20,6 +20,7 @@ use futures::channel::oneshot; use parking_lot::RwLock; +use sp_blockchain::Error; use sp_runtime::traits::Block as BlockT; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, @@ -33,10 +34,18 @@ pub enum SubscriptionManagementError { /// the subscription has exceeded the maximum number /// of blocks pinned. ExceededLimits, + /// Error originated from the blockchain (client or backend). + Blockchain(Error), /// Custom error. Custom(String), } +impl From for SubscriptionManagementError { + fn from(err: Error) -> Self { + SubscriptionManagementError::Blockchain(err) + } +} + /// Inner subscription data structure. struct SubscriptionInner { /// The `runtime_updates` parameter flag of the subscription. From 3bb66e4eda7c316e741c65dc7ff6286e9101c5f1 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 16 Feb 2023 19:41:24 +0200 Subject: [PATCH 11/35] rpc/chian_head: Report all pruned blocks Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 3149c0f86117c..669384ee477b7 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -290,15 +290,12 @@ where BE: Backend + 'static, { let blockchain = backend.blockchain(); - let leaves = blockchain - .leaves() - .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; + let leaves = blockchain.leaves()?; let finalized = info.finalized_hash; let mut pruned_forks = HashSet::new(); let mut finalized_block_descendants = Vec::new(); for leaf in leaves { - let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf) - .map_err(|err| SubscriptionManagementError::Custom(err.to_string()))?; + let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf)?; let blocks = tree_route.enacted().iter().map(|block| block.hash); if !tree_route.retracted().is_empty() { @@ -332,6 +329,7 @@ async fn submit_events( + HeaderBackend + HeaderMetadata + 'static, + BE: Backend + 'static, { let mut stream_item = stream.next(); let mut stop_event = data.rx_stop; @@ -358,6 +356,7 @@ async fn submit_events( &data.start_info, &mut best_block_cache, data.runtime_updates, + &data.backend, ), }; @@ -480,7 +479,7 @@ where /// Generate the "Finalized" event and potentially the "BestBlockChanged" for /// every notification. -fn handle_finalized_blocks( +fn handle_finalized_blocks( client: &Arc, handle: &SubscriptionHandle, notification: FinalityNotification, @@ -488,8 +487,10 @@ fn handle_finalized_blocks( start_info: &Info, best_block_cache: &mut Option, runtime_updates: bool, + backend: &Arc, ) -> Result>, SubscriptionManagementError> where + BE: Backend + 'static, Block: BlockT + 'static, Client: HeaderBackend + HeaderMetadata @@ -556,14 +557,24 @@ where } // Report all pruned blocks from the notification that are not - // part of the `pruned_forks`. + // part of the fork we need to ignore. let pruned_block_hashes: Vec<_> = { - notification - .stale_heads - .iter() - .filter(|hash| !to_ignore.remove(&hash)) - .cloned() - .collect() + let blockchain = backend.blockchain(); + let mut pruned = Vec::new(); + for stale_head in notification.stale_heads.iter() { + // Find the path from the canonical chain to the stale head. + let tree_route = sp_blockchain::tree_route(blockchain, last_finalized, *stale_head)?; + + // Collect only blocks that are not part of the canonical chain. + pruned.extend( + tree_route + .enacted() + .iter() + .filter_map(|block| (!to_ignore.remove(&block.hash)).then(|| block.hash)), + ) + } + + pruned }; let finalized_event = From 3847c74371b63ba8ab0527840ba50d4798149792 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 13:46:25 +0200 Subject: [PATCH 12/35] rpc/chain_head: Move `chainHead_follow` logic on dedicated file Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 606 ++++++++++++++++++ client/rpc-spec-v2/src/chain_head/mod.rs | 1 + .../src/chain_head/subscription.rs | 6 + 3 files changed, 613 insertions(+) create mode 100644 client/rpc-spec-v2/src/chain_head/chain_head_follow.rs diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs new file mode 100644 index 0000000000000..a4740d566e361 --- /dev/null +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -0,0 +1,606 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! Implementation of the `chainHead_follow` method. + +use crate::chain_head::{ + chain_head::LOG_TARGET, + event::{ + BestBlockChanged, Finalized, FollowEvent, Initialized, NewBlock, RuntimeEvent, + RuntimeVersionEvent, + }, + subscription::{SubscriptionHandle, SubscriptionManagement, SubscriptionManagementError}, +}; +use futures::{ + channel::oneshot, + stream::{self, Stream, StreamExt}, +}; +use futures_util::future::Either; +use itertools::Itertools; +use jsonrpsee::SubscriptionSink; +use log::{debug, error}; +use sc_client_api::{ + Backend, BlockBackend, BlockImportNotification, BlockchainEvents, ExecutorProvider, + FinalityNotification, StorageProvider, +}; +use sp_api::CallApiAt; +use sp_blockchain::{ + Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, Info, +}; +use sp_runtime::{ + generic::BlockId, + traits::{Block as BlockT, Header, One}, + Saturating, +}; +use std::{collections::HashSet, sync::Arc}; + +/// Generates the events of the `chainHead_follow` method. +pub struct ChainHeadFollow { + /// Substrate client. + client: Arc, + /// Backend of the chain. + backend: Arc, + /// Keep track of the pinned blocks for each subscription. + subscriptions: Arc>, + /// Subscription handle. + sub_handle: SubscriptionHandle, + /// Subscription was started with the runtime updates flag. + runtime_updates: bool, + /// Subscription ID. + sub_id: String, + /// The best reported block by this subscription. + best_block_cache: Option, +} + +impl ChainHeadFollow { + /// Create a new [`ChainHeadFollow`]. + pub fn new( + client: Arc, + backend: Arc, + subscriptions: Arc>, + sub_handle: SubscriptionHandle, + runtime_updates: bool, + sub_id: String, + ) -> Self { + Self { + client, + backend, + subscriptions, + sub_handle, + runtime_updates, + sub_id, + best_block_cache: None, + } + } +} + +/// A block notification. +enum NotificationType { + /// The initial events generated from the node's memory. + InitialEvents(Vec>), + /// The new block notification obtained from `import_notification_stream`. + NewBlock(BlockImportNotification), + /// The finalized block notification obtained from `finality_notification_stream`. + Finalized(FinalityNotification), +} + +/// The initial blocks that should be reported or ignored by the chainHead. +#[derive(Clone, Debug)] +struct InitialBlocks { + /// Children of the latest finalized block, for which the `NewBlock` + /// event must be generated. + /// + /// It is a tuple of (block hash, parent hash). + finalized_block_descendants: Vec<(Block::Hash, Block::Hash)>, + /// Blocks that should not be reported as pruned by the `Finalized` event. + /// + /// Substrate database will perform the pruning of height N at + /// the finalization N + 1. We could have the following block tree + /// when the user subscribes to the `follow` method: + /// [A] - [A1] - [A2] - [A3] + /// ^^ finalized + /// - [A1] - [B1] + /// + /// When the A3 block is finalized, B1 is reported as pruned, however + /// B1 was never reported as `NewBlock` (and as such was never pinned). + /// This is because the `NewBlock` events are generated for children of + /// the finalized hash. + pruned_forks: HashSet, +} + +impl ChainHeadFollow +where + Block: BlockT + 'static, + Block::Header: Unpin, + BE: Backend + 'static, + Client: BlockBackend + + ExecutorProvider + + HeaderBackend + + HeaderMetadata + + BlockchainEvents + + CallApiAt + + StorageProvider + + 'static, +{ + /// Conditionally generate the runtime event of the given block. + fn generate_runtime_event( + &self, + block: &BlockId, + parent: Option<&BlockId>, + ) -> Option + where + Block: BlockT + 'static, + Client: CallApiAt + 'static, + { + // No runtime versions should be reported. + if !self.runtime_updates { + return None + } + + let block_rt = match self.client.runtime_version_at(block) { + Ok(rt) => rt, + Err(err) => return Some(err.into()), + }; + + let parent = match parent { + Some(parent) => parent, + // Nothing to compare against, always report. + None => return Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt })), + }; + + let parent_rt = match self.client.runtime_version_at(parent) { + Ok(rt) => rt, + Err(err) => return Some(err.into()), + }; + + // Report the runtime version change. + if block_rt != parent_rt { + Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt })) + } else { + None + } + } + + /// Get the in-memory blocks of the client, starting from the provided finalized hash. + fn get_init_blocks_with_forks( + &self, + info: &Info, + ) -> Result, SubscriptionManagementError> { + let blockchain = self.backend.blockchain(); + let leaves = blockchain.leaves()?; + let finalized = info.finalized_hash; + let mut pruned_forks = HashSet::new(); + let mut finalized_block_descendants = Vec::new(); + for leaf in leaves { + let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf)?; + + let blocks = tree_route.enacted().iter().map(|block| block.hash); + if !tree_route.retracted().is_empty() { + pruned_forks.extend(blocks); + } else { + // Ensure a `NewBlock` event is generated for all children of the + // finalized block. Describe the tree route as (child_node, parent_node) + // Note: the order of elements matters here. + let parents = std::iter::once(finalized).chain(blocks.clone()); + finalized_block_descendants.extend(blocks.zip(parents)); + } + } + + // Keep unique blocks. + let finalized_block_descendants: Vec<_> = + finalized_block_descendants.into_iter().unique().collect(); + + Ok(InitialBlocks { finalized_block_descendants, pruned_forks }) + } + + /// Generate the initial events reported by the RPC `follow` method. + /// + /// Returns the initial events that should be reported directly, together with pruned + /// block hashes that should be ignored by the `Finalized` event. + fn generate_init_events( + &mut self, + info: &Info, + ) -> Result<(Vec>, HashSet), SubscriptionManagementError> + { + let init = self.get_init_blocks_with_forks(info)?; + + let initial_blocks = init.finalized_block_descendants; + + // The initialized event is the first one sent. + let finalized_block_hash = info.finalized_hash; + self.sub_handle.pin_block(finalized_block_hash)?; + + let finalized_block_runtime = + self.generate_runtime_event(&BlockId::Hash(finalized_block_hash), None); + + let initialized_event = FollowEvent::Initialized(Initialized { + finalized_block_hash, + finalized_block_runtime, + runtime_updates: self.runtime_updates, + }); + + let mut in_memory_blocks = Vec::with_capacity(initial_blocks.len() + 1); + + in_memory_blocks.push(initialized_event); + for (child, parent) in initial_blocks.into_iter() { + self.sub_handle.pin_block(child)?; + + let new_runtime = + self.generate_runtime_event(&BlockId::Hash(child), Some(&BlockId::Hash(parent))); + + let event = FollowEvent::NewBlock(NewBlock { + block_hash: child, + parent_block_hash: parent, + new_runtime, + runtime_updates: self.runtime_updates, + }); + + in_memory_blocks.push(event); + } + + // Generate a new best block event. + let best_block_hash = info.best_hash; + if best_block_hash != finalized_block_hash { + let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); + self.best_block_cache = Some(best_block_hash); + in_memory_blocks.push(best_block); + }; + + Ok((in_memory_blocks, init.pruned_forks)) + } + + /// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for the + /// given block hash. + fn generate_import_events( + &mut self, + block_hash: Block::Hash, + parent_block_hash: Block::Hash, + is_best_block: bool, + ) -> Vec> { + let new_runtime = self.generate_runtime_event( + &BlockId::Hash(block_hash), + Some(&BlockId::Hash(parent_block_hash)), + ); + + let new_block = FollowEvent::NewBlock(NewBlock { + block_hash, + parent_block_hash, + new_runtime, + runtime_updates: self.runtime_updates, + }); + + if !is_best_block { + return vec![new_block] + } + + // If this is the new best block, then we need to generate two events. + let best_block_event = + FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash: block_hash }); + + match self.best_block_cache { + Some(block_cache) => { + // The RPC layer has not reported this block as best before. + // Note: This handles the race with the finalized branch. + if block_cache != block_hash { + self.best_block_cache = Some(block_hash); + vec![new_block, best_block_event] + } else { + vec![new_block] + } + }, + None => { + self.best_block_cache = Some(block_hash); + vec![new_block, best_block_event] + }, + } + } + + /// Handle the import of new blocks by generating the appropriate events. + fn handle_import_blocks( + &mut self, + notification: BlockImportNotification, + info: &Info, + ) -> Result>, SubscriptionManagementError> + where + Block: BlockT + 'static, + Client: CallApiAt + 'static, + { + // The block was already pinned by the initial block events or by the finalized event. + if !self.sub_handle.pin_block(notification.hash)? { + return Ok(Default::default()) + } + + // Ensure we are only reporting blocks after the starting point. + let Some(block_number) = self.client.number(notification.hash)? else { + return Err(SubscriptionManagementError::BlockNumberAbsent) + }; + if block_number < info.finalized_number { + return Ok(Default::default()) + } + + Ok(self.generate_import_events( + notification.hash, + *notification.header.parent_hash(), + notification.is_new_best, + )) + } + + /// Generates new block events from the given finalized hashes. + /// + /// It may be possible that the `Finalized` event fired before the `NewBlock` + /// event. In that case, for each finalized hash that was not reported yet + /// generate the `NewBlock` event. For the final finalized hash we must also + /// generate one `BestBlock` event. + fn generate_finalized_events( + &mut self, + finalized_block_hashes: &[Block::Hash], + ) -> Result>, SubscriptionManagementError> { + let mut events = Vec::new(); + + // Nothing to be done if no finalized hashes are provided. + let Some(first_hash) = finalized_block_hashes.get(0) else { + return Ok(Default::default()) + }; + + // Find the parent hash. + let Some(first_number) = self.client.number(*first_hash)? else { + return Err(SubscriptionManagementError::BlockNumberAbsent) + }; + let Some(parent) = self.client.hash(first_number.saturating_sub(One::one()))? else { + return Err(SubscriptionManagementError::BlockHashAbsent) + }; + // The parent of the finalized block hash was not reported by the `chianHead`. + // There may be a gap in the notifications and we cannot guarantee that + // every new block has a parent reported by an event. + if !self.sub_handle.contains_block(&parent) { + return Err(SubscriptionManagementError::BlockNotPinned) + } + + let last_finalized = finalized_block_hashes + .last() + .expect("At least one finalized hash inserted; qed"); + let parents = std::iter::once(&parent).chain(finalized_block_hashes.iter()); + for (hash, parent) in finalized_block_hashes.iter().zip(parents) { + // This block is already reported by the import notification. + if !self.sub_handle.pin_block(*hash)? { + continue + } + + // This is the first time we see this block. Generate the `NewBlock` event; if this is + // the last block, also generate the `BestBlock` event. + events.extend(self.generate_import_events(*hash, *parent, hash == last_finalized)) + } + + Ok(events) + } + + /// Get all pruned block hashes from the provided stale heads. + /// + /// The result does not include hashes from `to_ignore`. + fn get_pruned_hashes( + &self, + stale_heads: &[Block::Hash], + last_finalized: Block::Hash, + to_ignore: &mut HashSet, + ) -> Result, SubscriptionManagementError> { + let blockchain = self.backend.blockchain(); + let mut pruned = Vec::new(); + + for stale_head in stale_heads { + let tree_route = sp_blockchain::tree_route(blockchain, last_finalized, *stale_head)?; + + // Collect only blocks that are not part of the canonical chain. + pruned.extend(tree_route.enacted().iter().filter_map(|block| { + if !to_ignore.remove(&block.hash) { + Some(block.hash) + } else { + None + } + })) + } + + Ok(pruned) + } + + /// Handle the finalization notification by generating the "Finalized" event. + /// + /// If the block of the notification was not reported yet, this method also + /// generates the events similar to `handle_import_blocks`. + fn handle_finalized_blocks( + &mut self, + notification: FinalityNotification, + to_ignore: &mut HashSet, + info: &Info, + ) -> Result>, SubscriptionManagementError> { + println!("Finalized block event: {:?}", notification.hash); + let last_finalized = notification.hash; + + // Ensure we are only reporting blocks after the starting point. + let Some(block_number) = self.client.number(last_finalized)? else { + return Err(SubscriptionManagementError::BlockNumberAbsent) + }; + if block_number < info.finalized_number { + return Ok(vec![]) + } + + // The tree route contains the exclusive path from the last finalized block to the block + // reported by the notification. Ensure the finalized block is also reported. + let mut finalized_block_hashes = + notification.tree_route.iter().cloned().collect::>(); + finalized_block_hashes.push(last_finalized); + + // If the finalized hashes were not reported yet, generate the `NewBlock` events. + let mut events = self.generate_finalized_events(&finalized_block_hashes)?; + + // Report all pruned blocks from the notification that are not + // part of the fork we need to ignore. + let pruned_block_hashes = + self.get_pruned_hashes(¬ification.stale_heads, last_finalized, to_ignore)?; + + let finalized_event = FollowEvent::Finalized(Finalized { + finalized_block_hashes, + pruned_block_hashes: pruned_block_hashes.clone(), + }); + + match self.best_block_cache { + Some(block_cache) => { + // Check if the current best block is also reported as pruned. + let reported_pruned = pruned_block_hashes.iter().find(|&&hash| hash == block_cache); + if reported_pruned.is_none() { + events.push(finalized_event); + return Ok(events) + } + + // The best block is reported as pruned. Therefore, we need to signal a new + // best block event before submitting the finalized event. + let best_block_hash = self.client.info().best_hash; + if best_block_hash == block_cache { + // The client doest not have any new information about the best block. + // The information from `.info()` is updated from the DB as the last + // step of the finalization and it should be up to date. + // If the info is outdated, there is nothing the RPC can do for now. + error!(target: LOG_TARGET, "Client does not contain different best block"); + events.push(finalized_event); + Ok(events) + } else { + let ancestor = sp_blockchain::lowest_common_ancestor( + &*self.client, + last_finalized, + best_block_hash, + )?; + + // The client's best block must be a descendent of the last finalized block. + // In other words, the lowest common ancestor must be the last finalized block. + if ancestor.hash != last_finalized { + return Err(SubscriptionManagementError::Custom( + "The finalized block is not an ancestor of the best block".into(), + )) + } + + // The RPC needs to also submit a new best block changed before the + // finalized event. + self.best_block_cache = Some(best_block_hash); + let best_block_event = + FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); + events.extend([best_block_event, finalized_event]); + Ok(events) + } + }, + None => { + events.push(finalized_event); + Ok(events) + }, + } + } + + /// Submit the events from the provided stream to the RPC client + /// for as long as the `rx_stop` event was not called. + async fn submit_events( + &mut self, + info: &Info, + mut stream: EventStream, + mut to_ignore: HashSet, + mut sink: SubscriptionSink, + rx_stop: oneshot::Receiver<()>, + ) where + EventStream: Stream> + Unpin, + { + let mut stream_item = stream.next(); + let mut stop_event = rx_stop; + + while let Either::Left((Some(event), next_stop_event)) = + futures_util::future::select(stream_item, stop_event).await + { + let events = match event { + NotificationType::InitialEvents(events) => Ok(events), + NotificationType::NewBlock(notification) => + self.handle_import_blocks(notification, &info), + NotificationType::Finalized(notification) => + self.handle_finalized_blocks(notification, &mut to_ignore, &info), + }; + + let events = match events { + Ok(events) => events, + Err(err) => { + debug!(target: LOG_TARGET, "Failed to handle stream notification {:?}", err); + break + }, + }; + + for event in events { + match sink.send(&event) { + // Submitted successfully. + Ok(true) => continue, + // Client disconnected. + Ok(false) => return, + Err(_) => { + // Failed to submit event. + break + }, + } + } + + stream_item = stream.next(); + stop_event = next_stop_event; + } + + let _ = sink.send(&FollowEvent::::Stop); + self.subscriptions.remove_subscription(&self.sub_id); + debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", self.sub_id); + } + + /// Generate the block events for the `chainHead_follow` method. + pub async fn generate_events( + &mut self, + mut sink: SubscriptionSink, + rx_stop: oneshot::Receiver<()>, + ) { + // Register for the new block and finalized notifications. + let stream_import = self + .client + .import_notification_stream() + .map(|notification| NotificationType::NewBlock(notification)); + + let stream_finalized = self + .client + .finality_notification_stream() + .map(|notification| NotificationType::Finalized(notification)); + + let info = self.client.info(); + let (initial_events, pruned_forks) = match self.generate_init_events(&info) { + Ok(blocks) => blocks, + Err(err) => { + debug!( + target: LOG_TARGET, + "[follow][id={:?}] Failed to generate the initial events {:?}", + self.sub_id, + err + ); + let _ = sink.send(&FollowEvent::::Stop); + self.subscriptions.remove_subscription(&self.sub_id); + return + }, + }; + + let initial = NotificationType::InitialEvents(initial_events); + let merged = tokio_stream::StreamExt::merge(stream_import, stream_finalized); + let stream = stream::once(futures::future::ready(initial)).chain(merged); + + self.submit_events(&info, stream.boxed(), pruned_forks, sink, rx_stop).await; + } +} diff --git a/client/rpc-spec-v2/src/chain_head/mod.rs b/client/rpc-spec-v2/src/chain_head/mod.rs index a25933b40f07d..abe716b8e3cb3 100644 --- a/client/rpc-spec-v2/src/chain_head/mod.rs +++ b/client/rpc-spec-v2/src/chain_head/mod.rs @@ -30,6 +30,7 @@ pub mod chain_head; pub mod error; pub mod event; +mod chain_head_follow; mod subscription; pub use api::ChainHeadApiServer; diff --git a/client/rpc-spec-v2/src/chain_head/subscription.rs b/client/rpc-spec-v2/src/chain_head/subscription.rs index d9514ed4a15e7..3322b304d1736 100644 --- a/client/rpc-spec-v2/src/chain_head/subscription.rs +++ b/client/rpc-spec-v2/src/chain_head/subscription.rs @@ -36,6 +36,12 @@ pub enum SubscriptionManagementError { ExceededLimits, /// Error originated from the blockchain (client or backend). Blockchain(Error), + /// The database does not contain a block number. + BlockNumberAbsent, + /// The database does not contain a block hash. + BlockHashAbsent, + /// Block was expected to be pinned. + BlockNotPinned, /// Custom error. Custom(String), } From 64876fa3b365a440f39a9abfad02fa87ef8ce1bc Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 13:47:14 +0200 Subject: [PATCH 13/35] rpc/chain_head: Delegate follow logic to `chain_head_follow` Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 593 +----------------- 1 file changed, 15 insertions(+), 578 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 669384ee477b7..122618288afc6 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -21,46 +21,32 @@ use crate::{ chain_head::{ api::ChainHeadApiServer, + chain_head_follow::ChainHeadFollow, error::Error as ChainHeadRpcError, - event::{ - BestBlockChanged, ChainHeadEvent, ChainHeadResult, ErrorEvent, Finalized, FollowEvent, - Initialized, NetworkConfig, NewBlock, RuntimeEvent, RuntimeVersionEvent, - }, - subscription::{SubscriptionHandle, SubscriptionManagement, SubscriptionManagementError}, + event::{ChainHeadEvent, ChainHeadResult, ErrorEvent, FollowEvent, NetworkConfig}, + subscription::SubscriptionManagement, }, SubscriptionTaskExecutor, }; use codec::Encode; -use futures::{ - channel::oneshot, - future::FutureExt, - stream::{self, Stream, StreamExt}, -}; -use futures_util::future::Either; -use itertools::Itertools; +use futures::future::FutureExt; use jsonrpsee::{ core::{async_trait, RpcResult}, types::{SubscriptionEmptyError, SubscriptionId, SubscriptionResult}, SubscriptionSink, }; -use log::{debug, error}; +use log::debug; use sc_client_api::{ - Backend, BlockBackend, BlockImportNotification, BlockchainEvents, CallExecutor, ChildInfo, - ExecutorProvider, FinalityNotification, StorageKey, StorageProvider, + Backend, BlockBackend, BlockchainEvents, CallExecutor, ChildInfo, ExecutorProvider, StorageKey, + StorageProvider, }; use sp_api::CallApiAt; -use sp_blockchain::{ - Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, Info, -}; +use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use sp_core::{hexdisplay::HexDisplay, storage::well_known_keys, Bytes}; -use sp_runtime::{ - generic::BlockId, - traits::{Block as BlockT, Header, One}, - Saturating, -}; -use std::{collections::HashSet, marker::PhantomData, sync::Arc}; +use sp_runtime::traits::Block as BlockT; +use std::{marker::PhantomData, sync::Arc}; -const LOG_TARGET: &str = "rpc-spec-v2"; +pub(crate) const LOG_TARGET: &str = "rpc-spec-v2"; /// An API for chain head RPC calls. pub struct ChainHead { @@ -125,75 +111,6 @@ impl ChainHead { } } -/// Generate the initial events reported by the RPC `follow` method. -/// -/// This includes the "Initialized" event followed by the in-memory -/// blocks via "NewBlock" and the "BestBlockChanged". -fn generate_initial_events( - data: &mut FollowData, -) -> Result<(Vec>, HashSet), SubscriptionManagementError> -where - Block: BlockT + 'static, - Block::Header: Unpin, - BE: Backend + 'static, - Client: HeaderBackend + CallApiAt + 'static, -{ - let client = &data.client; - let backend = &data.backend; - let start_info = &data.start_info; - let handle = &data.sub_handle; - let runtime_updates = data.runtime_updates; - - let ret = get_initial_blocks_with_forks(backend, start_info)?; - let initial_blocks = ret.finalized_block_descendants; - - // The initialized event is the first one sent. - let finalized_block_hash = start_info.finalized_hash; - handle.pin_block(finalized_block_hash)?; - - let finalized_block_runtime = - generate_runtime_event(client, runtime_updates, &BlockId::Hash(finalized_block_hash), None); - - let initialized_event = FollowEvent::Initialized(Initialized { - finalized_block_hash, - finalized_block_runtime, - runtime_updates, - }); - - let mut in_memory_blocks = Vec::with_capacity(initial_blocks.len() + 1); - - in_memory_blocks.push(initialized_event); - for (child, parent) in initial_blocks.into_iter() { - handle.pin_block(child)?; - - let new_runtime = generate_runtime_event( - client, - data.runtime_updates, - &BlockId::Hash(child), - Some(&BlockId::Hash(parent)), - ); - - let event = FollowEvent::NewBlock(NewBlock { - block_hash: child, - parent_block_hash: parent, - new_runtime, - runtime_updates, - }); - - in_memory_blocks.push(event); - } - - // Generate a new best block event. - let best_block_hash = start_info.best_hash; - if best_block_hash != finalized_block_hash { - let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); - data.best_block_cache = Some(best_block_hash); - in_memory_blocks.push(best_block); - }; - - Ok((in_memory_blocks, ret.pruned_forks)) -} - /// Parse hex-encoded string parameter as raw bytes. /// /// If the parsing fails, the subscription is rejected. @@ -215,453 +132,6 @@ fn parse_hex_param( } } -/// Conditionally generate the runtime event of the given block. -fn generate_runtime_event( - client: &Arc, - runtime_updates: bool, - block: &BlockId, - parent: Option<&BlockId>, -) -> Option -where - Block: BlockT + 'static, - Client: CallApiAt + 'static, -{ - // No runtime versions should be reported. - if !runtime_updates { - return None - } - - let block_rt = match client.runtime_version_at(block) { - Ok(rt) => rt, - Err(err) => return Some(err.into()), - }; - - let parent = match parent { - Some(parent) => parent, - // Nothing to compare against, always report. - None => return Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt })), - }; - - let parent_rt = match client.runtime_version_at(parent) { - Ok(rt) => rt, - Err(err) => return Some(err.into()), - }; - - // Report the runtime version change. - if block_rt != parent_rt { - Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt })) - } else { - None - } -} - -/// Internal representation of the initial blocks that should be -/// reported or ignored by the chainHead. -#[derive(Clone, Debug)] -struct InitialBlocks { - /// Children of the latest finalized block, for which the `NewBlock` - /// event must be generated. - /// - /// It is a tuple of (block hash, parent hash). - finalized_block_descendants: Vec<(Block::Hash, Block::Hash)>, - /// Blocks that should not be reported as pruned by the `Finalized` event. - /// - /// Substrate database will perform the pruning of height N at - /// the finalization N + 1. We could have the following block tree - /// when the user subscribes to the `follow` method: - /// [A] - [A1] - [A2] - [A3] - /// ^^ finalized - /// - [A1] - [B1] - /// - /// When the A3 block is finalized, B1 is reported as pruned, however - /// B1 was never reported as `NewBlock` (and as such was never pinned). - /// This is because the `NewBlock` events are generated for children of - /// the finalized hash. - pruned_forks: HashSet, -} - -/// Get the in-memory blocks of the client, starting from the provided finalized hash. -fn get_initial_blocks_with_forks( - backend: &Arc, - info: &Info, -) -> Result, SubscriptionManagementError> -where - Block: BlockT + 'static, - BE: Backend + 'static, -{ - let blockchain = backend.blockchain(); - let leaves = blockchain.leaves()?; - let finalized = info.finalized_hash; - let mut pruned_forks = HashSet::new(); - let mut finalized_block_descendants = Vec::new(); - for leaf in leaves { - let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf)?; - - let blocks = tree_route.enacted().iter().map(|block| block.hash); - if !tree_route.retracted().is_empty() { - pruned_forks.extend(blocks); - } else { - // Ensure a `NewBlock` event is generated for all children of the - // finalized block. Describe the tree route as (child_node, parent_node) - let parents = std::iter::once(finalized).chain(blocks.clone()); - finalized_block_descendants.extend(blocks.zip(parents)); - } - } - - // Keep unique blocks. - let finalized_block_descendants: Vec<_> = - finalized_block_descendants.into_iter().unique().collect(); - - Ok(InitialBlocks { finalized_block_descendants, pruned_forks }) -} - -/// Submit the events from the provided stream to the RPC client -/// for as long as the `rx_stop` event was not called. -async fn submit_events( - data: FollowData, - mut stream: EventStream, - mut to_ignore: HashSet, - sub_id: String, -) where - EventStream: Stream> + Unpin, - Block: BlockT + 'static, - Client: CallApiAt - + HeaderBackend - + HeaderMetadata - + 'static, - BE: Backend + 'static, -{ - let mut stream_item = stream.next(); - let mut stop_event = data.rx_stop; - let mut sink = data.sink; - let mut best_block_cache = data.best_block_cache; - - while let Either::Left((Some(event), next_stop_event)) = - futures_util::future::select(stream_item, stop_event).await - { - let events = match event { - NotificationType::InitialEvents(events) => Ok(events), - NotificationType::NewBlock(notification) => handle_import_blocks( - &data.client, - &data.sub_handle, - data.runtime_updates, - notification, - &mut best_block_cache, - ), - NotificationType::Finalized(notification) => handle_finalized_blocks( - &data.client, - &data.sub_handle, - notification, - &mut to_ignore, - &data.start_info, - &mut best_block_cache, - data.runtime_updates, - &data.backend, - ), - }; - - let events = match events { - Ok(events) => events, - Err(err) => { - debug!(target: LOG_TARGET, "Failed to handle stream notification {:?}", err); - break - }, - }; - - for event in events { - match sink.send(&event) { - // Submitted successfully. - Ok(true) => continue, - // Client disconnected. - Ok(false) => return, - Err(_) => { - // Failed to submit event. - break - }, - } - } - - stream_item = stream.next(); - stop_event = next_stop_event; - } - - let _ = sink.send(&FollowEvent::::Stop); - data.subscriptions.remove_subscription(&sub_id); - debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", sub_id); -} - -/// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for the -/// given block hash. -fn generate_import_event( - client: &Arc, - runtime_updates: bool, - block_hash: Block::Hash, - parent_block_hash: Block::Hash, - is_best_block: bool, - best_block_cache: &mut Option, -) -> Vec> -where - Block: BlockT + 'static, - Client: CallApiAt + 'static, -{ - let new_runtime = generate_runtime_event( - &client, - runtime_updates, - &BlockId::Hash(block_hash), - Some(&BlockId::Hash(parent_block_hash)), - ); - - // Note: `Block::Hash` will serialize to hexadecimal encoded string. - let new_block = FollowEvent::NewBlock(NewBlock { - block_hash, - parent_block_hash, - new_runtime, - runtime_updates, - }); - - if !is_best_block { - return vec![new_block] - } - - // If this is the new best block, then we need to generate two events. - let best_block_event = - FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash: block_hash }); - - match *best_block_cache { - Some(block_cache) => { - // The RPC layer has not reported this block as best before. - // Note: This handles the race with the finalized branch. - if block_cache != block_hash { - *best_block_cache = Some(block_hash); - vec![new_block, best_block_event] - } else { - vec![new_block] - } - }, - None => { - *best_block_cache = Some(block_hash); - vec![new_block, best_block_event] - }, - } -} - -/// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for -/// every notification. -fn handle_import_blocks( - client: &Arc, - handle: &SubscriptionHandle, - runtime_updates: bool, - notification: BlockImportNotification, - best_block_cache: &mut Option, -) -> Result>, SubscriptionManagementError> -where - Block: BlockT + 'static, - Client: CallApiAt + 'static, -{ - println!("New Block event: {:?}", notification.hash); - let is_new_pin = handle.pin_block(notification.hash)?; - // TODO: Also check number >= init info - if !is_new_pin { - // The block was already pinned by the initial block event in `generate_initial_events`, - // or by the finalized event in `handle_finalized_blocks`. - return Ok(vec![]) - } - - Ok(generate_import_event( - client, - runtime_updates, - notification.hash, - *notification.header.parent_hash(), - notification.is_new_best, - best_block_cache, - )) -} - -/// Generate the "Finalized" event and potentially the "BestBlockChanged" for -/// every notification. -fn handle_finalized_blocks( - client: &Arc, - handle: &SubscriptionHandle, - notification: FinalityNotification, - to_ignore: &mut HashSet, - start_info: &Info, - best_block_cache: &mut Option, - runtime_updates: bool, - backend: &Arc, -) -> Result>, SubscriptionManagementError> -where - BE: Backend + 'static, - Block: BlockT + 'static, - Client: HeaderBackend - + HeaderMetadata - + CallApiAt - + 'static, -{ - println!("Finalized block event: {:?}", notification.hash); - let last_finalized = notification.hash; - let Some(finalized_number) = client.number(last_finalized)? else { - return Err(SubscriptionManagementError::Custom("Block number not present in the database".into())) - }; - - // Check if the finalized hash has been reported by the initial events. - if finalized_number < start_info.finalized_number { - return Ok(vec![]) - } - - let mut events = Vec::new(); - - // The tree route contains the exclusive path from the last finalized block - // to the block reported by the notification. Ensure the finalized block is - // properly reported to that path. - let mut finalized_block_hashes = notification.tree_route.iter().cloned().collect::>(); - finalized_block_hashes.push(last_finalized); - - // Find the parent hash of the first element from the vector such that - // we can group (finalized_hash, parent_hash) for the event generation. - let first_parent_hash = { - let hash = finalized_block_hashes.get(0).expect("Finalized hash was included; qed"); - let Some(number) = client.number(*hash)? else { - return Err(SubscriptionManagementError::Custom("Block number not present in the database".into())) - }; - let Some(parent) = client.hash(number.saturating_sub(One::one()))? else { - return Err(SubscriptionManagementError::Custom("Block hash not present in the database".into())) - }; - // The parent of the finalized block hash was not reported by the `chianHead`. - if !handle.contains_block(&parent) { - return Err(SubscriptionManagementError::Custom( - "Parent of the finalized is missing".into(), - )) - } - - parent - }; - - let parents = std::iter::once(&first_parent_hash).chain(finalized_block_hashes.iter()); - for (hash, parent) in finalized_block_hashes.iter().zip(parents) { - // We have reported this block before. - if !handle.pin_block(*hash)? { - continue - } - - // This is the first time we see this block. We must generate the - // `NewBlock` event. Additionally, for the last event, we must - // also generate the `BestBlock` event. - events.extend(generate_import_event( - client, - runtime_updates, - *hash, - *parent, - *hash == last_finalized, - best_block_cache, - )); - } - - // Report all pruned blocks from the notification that are not - // part of the fork we need to ignore. - let pruned_block_hashes: Vec<_> = { - let blockchain = backend.blockchain(); - let mut pruned = Vec::new(); - for stale_head in notification.stale_heads.iter() { - // Find the path from the canonical chain to the stale head. - let tree_route = sp_blockchain::tree_route(blockchain, last_finalized, *stale_head)?; - - // Collect only blocks that are not part of the canonical chain. - pruned.extend( - tree_route - .enacted() - .iter() - .filter_map(|block| (!to_ignore.remove(&block.hash)).then(|| block.hash)), - ) - } - - pruned - }; - - let finalized_event = - FollowEvent::Finalized(Finalized { finalized_block_hashes, pruned_block_hashes }); - - let pruned_block_hashes: Vec<_> = notification.stale_heads.iter().cloned().collect(); - - match *best_block_cache { - Some(block_cache) => { - // Check if the current best block is also reported as pruned. - let reported_pruned = pruned_block_hashes.iter().find(|&&hash| hash == block_cache); - if reported_pruned.is_none() { - events.push(finalized_event); - return Ok(events) - } - - // The best block is reported as pruned. Therefore, we need to signal a new - // best block event before submitting the finalized event. - let best_block_hash = client.info().best_hash; - if best_block_hash == block_cache { - // The client doest not have any new information about the best block. - // The information from `.info()` is updated from the DB as the last - // step of the finalization and it should be up to date. - // If the info is outdated, there is nothing the RPC can do for now. - error!(target: LOG_TARGET, "Client does not contain different best block"); - events.push(finalized_event); - Ok(events) - } else { - let ancestor = sp_blockchain::lowest_common_ancestor( - &**client, - last_finalized, - best_block_hash, - ) - .map_err(|_| { - SubscriptionManagementError::Custom("Could not find common ancestor".into()) - })?; - - // The client's best block must be a descendent of the last finalized block. - // In other words, the lowest common ancestor must be the last finalized block. - if ancestor.hash != last_finalized { - return Err(SubscriptionManagementError::Custom( - "The finalized block is not an ancestor of the best block".into(), - )) - } - - // The RPC needs to also submit a new best block changed before the - // finalized event. - *best_block_cache = Some(best_block_hash); - let best_block_event = - FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); - events.extend([best_block_event, finalized_event]); - Ok(events) - } - }, - None => { - events.push(finalized_event); - Ok(events) - }, - } -} - -/// Internal representation of a block notification. -enum NotificationType { - /// The initial events generated from the node's memory. - InitialEvents(Vec>), - /// The new block notification obtained from `import_notification_stream`. - NewBlock(BlockImportNotification), - /// The finalized block notification obtained from `finality_notification_stream`. - Finalized(FinalityNotification), -} - -/// Data for the `follow` method to generate the block events. -struct FollowData { - client: Arc, - backend: Arc, - subscriptions: Arc>, - sub_handle: SubscriptionHandle, - rx_stop: oneshot::Receiver<()>, - sink: SubscriptionSink, - runtime_updates: bool, - /// The starting point from which the blocks were generated from the - /// node's memory. - start_info: Info, - /// The best reported block by this subscription. - best_block_cache: Option, -} - #[async_trait] impl ChainHeadApiServer for ChainHead where @@ -700,53 +170,20 @@ where }; debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription accepted", sub_id); - // Register for the new block and finalized notifications. - let stream_import = self - .client - .import_notification_stream() - .map(|notification| NotificationType::NewBlock(notification)); - - let stream_finalized = self - .client - .finality_notification_stream() - .map(|notification| NotificationType::Finalized(notification)); - - let merged = tokio_stream::StreamExt::merge(stream_import, stream_finalized); - let subscriptions = self.subscriptions.clone(); let backend = self.backend.clone(); let client = self.client.clone(); let fut = async move { - let start_info = client.info(); - let mut follow_data = FollowData { + let mut chain_head_follow = ChainHeadFollow::new( client, backend, subscriptions, sub_handle, - rx_stop, - sink, runtime_updates, - start_info, - best_block_cache: None, - }; - - let (initial_events, pruned_forks) = match generate_initial_events(&mut follow_data) { - Ok(blocks) => blocks, - Err(err) => { - debug!( - target: LOG_TARGET, - "[follow][id={:?}] Failed to generate the initial events {:?}", sub_id, err - ); - let _ = follow_data.sink.send(&FollowEvent::::Stop); - follow_data.subscriptions.remove_subscription(&sub_id); - return - }, - }; - - let initial = NotificationType::InitialEvents(initial_events); - let stream = stream::once(futures::future::ready(initial)).chain(merged); + sub_id, + ); - submit_events(follow_data, stream.boxed(), pruned_forks, sub_id).await; + chain_head_follow.generate_events(sink, rx_stop).await }; self.executor.spawn("substrate-rpc-subscription", Some("rpc"), fut.boxed()); From f84c4cea1cb9e3101ce05e97a459dda5d737f0ea Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 13:50:16 +0200 Subject: [PATCH 14/35] rpc/chain_head: Remove subscriptions on drop Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index a4740d566e361..ef8d525b62f38 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -560,8 +560,6 @@ where } let _ = sink.send(&FollowEvent::::Stop); - self.subscriptions.remove_subscription(&self.sub_id); - debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", self.sub_id); } /// Generate the block events for the `chainHead_follow` method. @@ -592,7 +590,6 @@ where err ); let _ = sink.send(&FollowEvent::::Stop); - self.subscriptions.remove_subscription(&self.sub_id); return }, }; @@ -604,3 +601,10 @@ where self.submit_events(&info, stream.boxed(), pruned_forks, sink, rx_stop).await; } } + +impl Drop for ChainHeadFollow { + fn drop(&mut self) { + self.subscriptions.remove_subscription(&self.sub_id); + debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", self.sub_id); + } +} From 908d1f27f597cab2797d1baa080acdd70066cd2d Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 14:04:06 +0200 Subject: [PATCH 15/35] rpc/tests: Ignore pruned blocks for a longer fork Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/tests.rs | 67 ++++++++++++++-------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index 0c89f373ff8b4..4a499d5d6a530 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1044,23 +1044,26 @@ async fn follow_forks_pruned_block() { // Block tree before the subscription: // - // finalized -> block 1 -> block 2 - // ^^^ finalized - // -> block 1 -> block 3 + // finalized -> block 1 -> block 2 -> block 3 + // ^^^ finalized + // -> block 1 -> block 4 -> block 5 // let block_1 = client.new_block(Default::default()).unwrap().build().unwrap().block; client.import(BlockOrigin::Own, block_1.clone()).await.unwrap(); let block_2 = client.new_block(Default::default()).unwrap().build().unwrap().block; - let block_2_hash = block_2.header.hash(); client.import(BlockOrigin::Own, block_2.clone()).await.unwrap(); - // Block 3 with parent Block 1 is not the best imported. + let block_3 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_3_hash = block_3.header.hash(); + client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); + + // Block 4 with parent Block 1 is not the best imported. let mut block_builder = client .new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false) .unwrap(); - // This push is required as otherwise block 3 has the same hash as block 2 and won't get + // This push is required as otherwise block 4 has the same hash as block 2 and won't get // imported block_builder .push_transfer(Transfer { @@ -1070,11 +1073,25 @@ async fn follow_forks_pruned_block() { nonce: 0, }) .unwrap(); - let block_3 = block_builder.build().unwrap().block; - client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); + let block_4 = block_builder.build().unwrap().block; + client.import(BlockOrigin::Own, block_4.clone()).await.unwrap(); + + let mut block_builder = client + .new_block_at(&BlockId::Hash(block_4.header.hash()), Default::default(), false) + .unwrap(); + block_builder + .push_transfer(Transfer { + from: AccountKeyring::Bob.into(), + to: AccountKeyring::Ferdie.into(), + amount: 41, + nonce: 0, + }) + .unwrap(); + let block_5 = block_builder.build().unwrap().block; + client.import(BlockOrigin::Own, block_5.clone()).await.unwrap(); - // Block 3 is not pruned, pruning happens at height (N - 1). - client.finalize_block(block_2_hash, None).unwrap(); + // Block 4 and 5 are not pruned, pruning happens at height (N - 1). + client.finalize_block(block_3_hash, None).unwrap(); let mut sub = api.subscribe("chainHead_unstable_follow", [false]).await.unwrap(); @@ -1082,7 +1099,7 @@ async fn follow_forks_pruned_block() { let event: FollowEvent = get_next_event(&mut sub).await; println!("Event: {:?}", event); let expected = FollowEvent::Initialized(Initialized { - finalized_block_hash: format!("{:?}", block_2_hash), + finalized_block_hash: format!("{:?}", block_3_hash), finalized_block_runtime: None, runtime_updates: false, }); @@ -1090,37 +1107,37 @@ async fn follow_forks_pruned_block() { // Block tree: // - // finalized -> block 1 -> block 2 -> block 4 - // ^^^ finalized - // -> block 1 -> block 3 + // finalized -> block 1 -> block 2 -> block 3 -> block 6 + // ^^^ finalized + // -> block 1 -> block 4 -> block 5 // - // Mark block 4 as finalized to force block 3 to get pruned. + // Mark block 6 as finalized to force block 4 and 5 to get pruned. - let block_4 = client.new_block(Default::default()).unwrap().build().unwrap().block; - let block_4_hash = block_4.header.hash(); - client.import(BlockOrigin::Own, block_4.clone()).await.unwrap(); + let block_6 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_6_hash = block_6.header.hash(); + client.import(BlockOrigin::Own, block_6.clone()).await.unwrap(); - client.finalize_block(block_4_hash, None).unwrap(); + client.finalize_block(block_6_hash, None).unwrap(); - // Check block 4. + // Check block 6. let event: FollowEvent = get_next_event(&mut sub).await; let expected = FollowEvent::NewBlock(NewBlock { - block_hash: format!("{:?}", block_4_hash), - parent_block_hash: format!("{:?}", block_2_hash), + block_hash: format!("{:?}", block_6_hash), + parent_block_hash: format!("{:?}", block_3_hash), new_runtime: None, runtime_updates: false, }); assert_eq!(event, expected); let event: FollowEvent = get_next_event(&mut sub).await; let expected = FollowEvent::BestBlockChanged(BestBlockChanged { - best_block_hash: format!("{:?}", block_4_hash), + best_block_hash: format!("{:?}", block_6_hash), }); assert_eq!(event, expected); - // Block 3 must not be reported as pruned. + // Block 4 and 5 must not be reported as pruned. let event: FollowEvent = get_next_event(&mut sub).await; let expected = FollowEvent::Finalized(Finalized { - finalized_block_hashes: vec![format!("{:?}", block_4_hash)], + finalized_block_hashes: vec![format!("{:?}", block_6_hash)], pruned_block_hashes: vec![], }); assert_eq!(event, expected); From 01e98b0e88c0a77615f4e35a16ce122c18eb5ba5 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 14:41:42 +0200 Subject: [PATCH 16/35] rpc/tests: Check all pruned blocks are reported, not just stale heads Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/tests.rs | 192 +++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index 4a499d5d6a530..ebbad582bded3 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1142,3 +1142,195 @@ async fn follow_forks_pruned_block() { }); assert_eq!(event, expected); } + +#[tokio::test] +async fn follow_report_multiple_pruned_block() { + let builder = TestClientBuilder::new(); + let backend = builder.backend(); + let mut client = Arc::new(builder.build()); + + let api = ChainHead::new( + client.clone(), + backend, + Arc::new(TaskExecutor::default()), + CHAIN_GENESIS, + MAX_PINNED_BLOCKS, + ) + .into_rpc(); + + // Block tree: + // + // finalized -> block 1 -> block 2 -> block 3 + // ^^^ finalized after subscription + // -> block 1 -> block 4 -> block 5 + + let finalized_hash = client.info().finalized_hash; + + let block_1 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_1_hash = block_1.header.hash(); + client.import(BlockOrigin::Own, block_1.clone()).await.unwrap(); + + let block_2 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_2_hash = block_2.header.hash(); + client.import(BlockOrigin::Own, block_2.clone()).await.unwrap(); + + let block_3 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_3_hash = block_3.header.hash(); + client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); + + // Block 4 with parent Block 1 is not the best imported. + let mut block_builder = client + .new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false) + .unwrap(); + // This push is required as otherwise block 4 has the same hash as block 2 and won't get + // imported + block_builder + .push_transfer(Transfer { + from: AccountKeyring::Alice.into(), + to: AccountKeyring::Ferdie.into(), + amount: 41, + nonce: 0, + }) + .unwrap(); + let block_4 = block_builder.build().unwrap().block; + let block_4_hash = block_4.header.hash(); + client.import(BlockOrigin::Own, block_4.clone()).await.unwrap(); + + let mut block_builder = client + .new_block_at(&BlockId::Hash(block_4.header.hash()), Default::default(), false) + .unwrap(); + block_builder + .push_transfer(Transfer { + from: AccountKeyring::Bob.into(), + to: AccountKeyring::Ferdie.into(), + amount: 41, + nonce: 0, + }) + .unwrap(); + let block_5 = block_builder.build().unwrap().block; + let block_5_hash = block_5.header.hash(); + client.import(BlockOrigin::Own, block_5.clone()).await.unwrap(); + + println!("Fin {:?}", finalized_hash); + println!("block_1_hash {:?}", block_1_hash); + println!("block_2_hash {:?}", block_2_hash); + println!("block_3_hash {:?}", block_3_hash); + println!("block_4_hash {:?}", block_4_hash); + println!("block_5_hash {:?}", block_5_hash); + + let mut sub = api.subscribe("chainHead_unstable_follow", [false]).await.unwrap(); + + // Initialized must always be reported first. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::Initialized(Initialized { + finalized_block_hash: format!("{:?}", finalized_hash), + finalized_block_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_1_hash), + parent_block_hash: format!("{:?}", finalized_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_2_hash), + parent_block_hash: format!("{:?}", block_1_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_3_hash), + parent_block_hash: format!("{:?}", block_2_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + // The fork must also be reported. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_4_hash), + parent_block_hash: format!("{:?}", block_1_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_5_hash), + parent_block_hash: format!("{:?}", block_4_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + + // The best block of the chain must also be reported. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::BestBlockChanged(BestBlockChanged { + best_block_hash: format!("{:?}", block_3_hash), + }); + assert_eq!(event, expected); + + // Block 4 and 5 are not pruned, pruning happens at height (N - 1). + client.finalize_block(block_3_hash, None).unwrap(); + + // Finalizing block 3 directly will also result in block 1 and 2 being finalized. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::Finalized(Finalized { + finalized_block_hashes: vec![ + format!("{:?}", block_1_hash), + format!("{:?}", block_2_hash), + format!("{:?}", block_3_hash), + ], + pruned_block_hashes: vec![], + }); + assert_eq!(event, expected); + + // Block tree: + // + // finalized -> block 1 -> block 2 -> block 3 -> block 6 + // ^^^ finalized + // -> block 1 -> block 4 -> block 5 + // + // Mark block 6 as finalized to force block 4 and 5 to get pruned. + + let block_6 = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_6_hash = block_6.header.hash(); + client.import(BlockOrigin::Own, block_6.clone()).await.unwrap(); + + client.finalize_block(block_6_hash, None).unwrap(); + + // Check block 6. + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::NewBlock(NewBlock { + block_hash: format!("{:?}", block_6_hash), + parent_block_hash: format!("{:?}", block_3_hash), + new_runtime: None, + runtime_updates: false, + }); + assert_eq!(event, expected); + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::BestBlockChanged(BestBlockChanged { + best_block_hash: format!("{:?}", block_6_hash), + }); + assert_eq!(event, expected); + + // Block 4 and 5 be reported as pruned, not just the stale head (block 5). + let event: FollowEvent = get_next_event(&mut sub).await; + let expected = FollowEvent::Finalized(Finalized { + finalized_block_hashes: vec![format!("{:?}", block_6_hash)], + pruned_block_hashes: vec![format!("{:?}", block_4_hash), format!("{:?}", block_5_hash)], + }); + assert_eq!(event, expected); +} From eceeb9235b3d82dc7d9f0d1ba267b574f2399153 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 15:33:27 +0200 Subject: [PATCH 17/35] rpc/tests: Remove println debug and fix indentation Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 19 +++++++++---------- client/rpc-spec-v2/src/chain_head/tests.rs | 9 --------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index ef8d525b62f38..79c5ec0d13be1 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -327,8 +327,8 @@ where // Ensure we are only reporting blocks after the starting point. let Some(block_number) = self.client.number(notification.hash)? else { - return Err(SubscriptionManagementError::BlockNumberAbsent) - }; + return Err(SubscriptionManagementError::BlockNumberAbsent) + }; if block_number < info.finalized_number { return Ok(Default::default()) } @@ -354,15 +354,15 @@ where // Nothing to be done if no finalized hashes are provided. let Some(first_hash) = finalized_block_hashes.get(0) else { - return Ok(Default::default()) - }; + return Ok(Default::default()) + }; // Find the parent hash. let Some(first_number) = self.client.number(*first_hash)? else { - return Err(SubscriptionManagementError::BlockNumberAbsent) - }; + return Err(SubscriptionManagementError::BlockNumberAbsent) + }; let Some(parent) = self.client.hash(first_number.saturating_sub(One::one()))? else { - return Err(SubscriptionManagementError::BlockHashAbsent) + return Err(SubscriptionManagementError::BlockHashAbsent) }; // The parent of the finalized block hash was not reported by the `chianHead`. // There may be a gap in the notifications and we cannot guarantee that @@ -427,13 +427,12 @@ where to_ignore: &mut HashSet, info: &Info, ) -> Result>, SubscriptionManagementError> { - println!("Finalized block event: {:?}", notification.hash); let last_finalized = notification.hash; // Ensure we are only reporting blocks after the starting point. let Some(block_number) = self.client.number(last_finalized)? else { - return Err(SubscriptionManagementError::BlockNumberAbsent) - }; + return Err(SubscriptionManagementError::BlockNumberAbsent) + }; if block_number < info.finalized_number { return Ok(vec![]) } diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index ebbad582bded3..fcc95ed88abfa 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1097,7 +1097,6 @@ async fn follow_forks_pruned_block() { // Initialized must always be reported first. let event: FollowEvent = get_next_event(&mut sub).await; - println!("Event: {:?}", event); let expected = FollowEvent::Initialized(Initialized { finalized_block_hash: format!("{:?}", block_3_hash), finalized_block_runtime: None, @@ -1210,14 +1209,6 @@ async fn follow_report_multiple_pruned_block() { let block_5 = block_builder.build().unwrap().block; let block_5_hash = block_5.header.hash(); client.import(BlockOrigin::Own, block_5.clone()).await.unwrap(); - - println!("Fin {:?}", finalized_hash); - println!("block_1_hash {:?}", block_1_hash); - println!("block_2_hash {:?}", block_2_hash); - println!("block_3_hash {:?}", block_3_hash); - println!("block_4_hash {:?}", block_4_hash); - println!("block_5_hash {:?}", block_5_hash); - let mut sub = api.subscribe("chainHead_unstable_follow", [false]).await.unwrap(); // Initialized must always be reported first. From b05f5fc39a3e77a3d43cb9bd21eb3e62f7f44526 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 15:39:15 +0200 Subject: [PATCH 18/35] rpc/chain_head: Remove unnecessary trait bounds Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 79c5ec0d13be1..e832fe44ac156 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -35,8 +35,7 @@ use itertools::Itertools; use jsonrpsee::SubscriptionSink; use log::{debug, error}; use sc_client_api::{ - Backend, BlockBackend, BlockImportNotification, BlockchainEvents, ExecutorProvider, - FinalityNotification, StorageProvider, + Backend, BlockBackend, BlockImportNotification, BlockchainEvents, FinalityNotification, }; use sp_api::CallApiAt; use sp_blockchain::{ @@ -126,15 +125,12 @@ struct InitialBlocks { impl ChainHeadFollow where Block: BlockT + 'static, - Block::Header: Unpin, BE: Backend + 'static, Client: BlockBackend - + ExecutorProvider + HeaderBackend + HeaderMetadata + BlockchainEvents + CallApiAt - + StorageProvider + 'static, { /// Conditionally generate the runtime event of the given block. @@ -142,11 +138,7 @@ where &self, block: &BlockId, parent: Option<&BlockId>, - ) -> Option - where - Block: BlockT + 'static, - Client: CallApiAt + 'static, - { + ) -> Option { // No runtime versions should be reported. if !self.runtime_updates { return None @@ -315,11 +307,7 @@ where &mut self, notification: BlockImportNotification, info: &Info, - ) -> Result>, SubscriptionManagementError> - where - Block: BlockT + 'static, - Client: CallApiAt + 'static, - { + ) -> Result>, SubscriptionManagementError> { // The block was already pinned by the initial block events or by the finalized event. if !self.sub_handle.pin_block(notification.hash)? { return Ok(Default::default()) From 425d6e7a8b60421bcece12add1941fe58524cf52 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Feb 2023 19:59:39 +0200 Subject: [PATCH 19/35] rpc/chain_head: Add debug log for pruned forks Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index e832fe44ac156..dd2b4699935f7 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -397,6 +397,10 @@ where if !to_ignore.remove(&block.hash) { Some(block.hash) } else { + debug!( + target: LOG_TARGET, + "Excluding pruned forks from the finalized notification" + ); None } })) From 002acbad934481d5c0e3a6dbea1cecb4d8c830ad Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Feb 2023 16:07:49 +0200 Subject: [PATCH 20/35] Revert "rpc/chain_head: Add debug log for pruned forks" This reverts commit 425d6e7a8b60421bcece12add1941fe58524cf52. --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index dd2b4699935f7..e832fe44ac156 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -397,10 +397,6 @@ where if !to_ignore.remove(&block.hash) { Some(block.hash) } else { - debug!( - target: LOG_TARGET, - "Excluding pruned forks from the finalized notification" - ); None } })) From 4140b5be256b0145569eb142e788f04307a13a2d Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Feb 2023 11:59:24 +0000 Subject: [PATCH 21/35] Adjust blockID for testing Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/tests.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/tests.rs b/client/rpc-spec-v2/src/chain_head/tests.rs index 7969489c327ec..0886efa94a756 100644 --- a/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/client/rpc-spec-v2/src/chain_head/tests.rs @@ -1056,9 +1056,8 @@ async fn follow_forks_pruned_block() { client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); // Block 4 with parent Block 1 is not the best imported. - let mut block_builder = client - .new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false) - .unwrap(); + let mut block_builder = + client.new_block_at(block_1.header.hash(), Default::default(), false).unwrap(); // This push is required as otherwise block 4 has the same hash as block 2 and won't get // imported block_builder @@ -1072,9 +1071,8 @@ async fn follow_forks_pruned_block() { let block_4 = block_builder.build().unwrap().block; client.import(BlockOrigin::Own, block_4.clone()).await.unwrap(); - let mut block_builder = client - .new_block_at(&BlockId::Hash(block_4.header.hash()), Default::default(), false) - .unwrap(); + let mut block_builder = + client.new_block_at(block_4.header.hash(), Default::default(), false).unwrap(); block_builder .push_transfer(Transfer { from: AccountKeyring::Bob.into(), @@ -1174,9 +1172,8 @@ async fn follow_report_multiple_pruned_block() { client.import(BlockOrigin::Own, block_3.clone()).await.unwrap(); // Block 4 with parent Block 1 is not the best imported. - let mut block_builder = client - .new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false) - .unwrap(); + let mut block_builder = + client.new_block_at(block_1.header.hash(), Default::default(), false).unwrap(); // This push is required as otherwise block 4 has the same hash as block 2 and won't get // imported block_builder @@ -1191,9 +1188,8 @@ async fn follow_report_multiple_pruned_block() { let block_4_hash = block_4.header.hash(); client.import(BlockOrigin::Own, block_4.clone()).await.unwrap(); - let mut block_builder = client - .new_block_at(&BlockId::Hash(block_4.header.hash()), Default::default(), false) - .unwrap(); + let mut block_builder = + client.new_block_at(block_4.header.hash(), Default::default(), false).unwrap(); block_builder .push_transfer(Transfer { from: AccountKeyring::Bob.into(), From aca8f3bf17c74b071380bdaee90203ebf14d5abe Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 1 Mar 2023 19:51:06 +0200 Subject: [PATCH 22/35] Update client/rpc-spec-v2/src/chain_head/chain_head_follow.rs Co-authored-by: Davide Galassi --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 8dc3073222e23..0cb9b5bab8c08 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// Copyright (C) Parity Technologies (UK) Ltd. // SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 // This program is free software: you can redistribute it and/or modify From 6429e05c4257e604b2ce6c36ece665b6f824ff52 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 19:52:27 +0200 Subject: [PATCH 23/35] rpc/chain_head: Rename `ChainHeadFollow` to `ChainHeadFollower` Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head.rs | 4 ++-- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 7d603388265e4..6293212684442 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -21,7 +21,7 @@ use crate::{ chain_head::{ api::ChainHeadApiServer, - chain_head_follow::ChainHeadFollow, + chain_head_follow::ChainHeadFollower, error::Error as ChainHeadRpcError, event::{ChainHeadEvent, ChainHeadResult, ErrorEvent, FollowEvent, NetworkConfig}, subscription::SubscriptionManagement, @@ -174,7 +174,7 @@ where let backend = self.backend.clone(); let client = self.client.clone(); let fut = async move { - let mut chain_head_follow = ChainHeadFollow::new( + let mut chain_head_follow = ChainHeadFollower::new( client, backend, subscriptions, diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 0cb9b5bab8c08..bd5cef26b80a6 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -48,7 +48,7 @@ use sp_runtime::{ use std::{collections::HashSet, sync::Arc}; /// Generates the events of the `chainHead_follow` method. -pub struct ChainHeadFollow { +pub struct ChainHeadFollower { /// Substrate client. client: Arc, /// Backend of the chain. @@ -65,8 +65,8 @@ pub struct ChainHeadFollow { best_block_cache: Option, } -impl ChainHeadFollow { - /// Create a new [`ChainHeadFollow`]. +impl ChainHeadFollower { + /// Create a new [`ChainHeadFollower`]. pub fn new( client: Arc, backend: Arc, @@ -121,7 +121,7 @@ struct InitialBlocks { pruned_forks: HashSet, } -impl ChainHeadFollow +impl ChainHeadFollower where Block: BlockT + 'static, BE: Backend + 'static, @@ -583,7 +583,7 @@ where } } -impl Drop for ChainHeadFollow { +impl Drop for ChainHeadFollower { fn drop(&mut self) { self.subscriptions.remove_subscription(&self.sub_id); debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", self.sub_id); From d3dd4a432dd0ec3abda1de5338cc72f3a9fe6f9d Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 19:59:51 +0200 Subject: [PATCH 24/35] rpc/chain_head: Remove subscriptions manually Signed-off-by: Alexandru Vasile --- .../rpc-spec-v2/src/chain_head/chain_head.rs | 8 ++++--- .../src/chain_head/chain_head_follow.rs | 22 ++----------------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index 6293212684442..c63e874c1bc15 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -177,13 +177,15 @@ where let mut chain_head_follow = ChainHeadFollower::new( client, backend, - subscriptions, sub_handle, runtime_updates, - sub_id, + sub_id.clone(), ); - chain_head_follow.generate_events(sink, rx_stop).await + chain_head_follow.generate_events(sink, rx_stop).await; + + subscriptions.remove_subscription(&sub_id); + debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", sub_id); }; self.executor.spawn("substrate-rpc-subscription", Some("rpc"), fut.boxed()); diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index bd5cef26b80a6..2e55bb77140f3 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -24,7 +24,7 @@ use crate::chain_head::{ BestBlockChanged, Finalized, FollowEvent, Initialized, NewBlock, RuntimeEvent, RuntimeVersionEvent, }, - subscription::{SubscriptionHandle, SubscriptionManagement, SubscriptionManagementError}, + subscription::{SubscriptionHandle, SubscriptionManagementError}, }; use futures::{ channel::oneshot, @@ -53,8 +53,6 @@ pub struct ChainHeadFollower { client: Arc, /// Backend of the chain. backend: Arc, - /// Keep track of the pinned blocks for each subscription. - subscriptions: Arc>, /// Subscription handle. sub_handle: SubscriptionHandle, /// Subscription was started with the runtime updates flag. @@ -70,20 +68,11 @@ impl ChainHeadFollower { pub fn new( client: Arc, backend: Arc, - subscriptions: Arc>, sub_handle: SubscriptionHandle, runtime_updates: bool, sub_id: String, ) -> Self { - Self { - client, - backend, - subscriptions, - sub_handle, - runtime_updates, - sub_id, - best_block_cache: None, - } + Self { client, backend, sub_handle, runtime_updates, sub_id, best_block_cache: None } } } @@ -582,10 +571,3 @@ where self.submit_events(&info, stream.boxed(), pruned_forks, sink, rx_stop).await; } } - -impl Drop for ChainHeadFollower { - fn drop(&mut self) { - self.subscriptions.remove_subscription(&self.sub_id); - debug!(target: LOG_TARGET, "[follow][id={:?}] Subscription removed", self.sub_id); - } -} From 3786aa89f4a646b938d114918e74ea4376440246 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 20:05:09 +0200 Subject: [PATCH 25/35] rpc/chain_head: Improve log messages by adding subID and errors Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 2e55bb77140f3..fa5333f91f558 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -444,7 +444,11 @@ where // The information from `.info()` is updated from the DB as the last // step of the finalization and it should be up to date. // If the info is outdated, there is nothing the RPC can do for now. - error!(target: LOG_TARGET, "Client does not contain different best block"); + error!( + target: LOG_TARGET, + "[follow][id={:?}] Client does not contain different best block", + self.sub_id, + ); events.push(finalized_event); Ok(events) } else { @@ -507,7 +511,12 @@ where let events = match events { Ok(events) => events, Err(err) => { - debug!(target: LOG_TARGET, "Failed to handle stream notification {:?}", err); + debug!( + target: LOG_TARGET, + "[follow][id={:?}] Failed to handle stream notification {:?}", + self.sub_id, + err + ); break }, }; @@ -518,8 +527,12 @@ where Ok(true) => continue, // Client disconnected. Ok(false) => return, - Err(_) => { + Err(err) => { // Failed to submit event. + debug!( + target: LOG_TARGET, + "[follow][id={:?}] Failed to send event {:?}", self.sub_id, err + ); break }, } From c24cdc66f6a5a92f21a413c635a34cfd82bba097 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 20:25:57 +0200 Subject: [PATCH 26/35] rpc/chain_head: Ensure `follow` stops sending events on first error Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index fa5333f91f558..479ed6231e426 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -517,32 +517,42 @@ where self.sub_id, err ); - break + let _ = sink.send(&FollowEvent::::Stop); + return }, }; for event in events { - match sink.send(&event) { - // Submitted successfully. - Ok(true) => continue, - // Client disconnected. - Ok(false) => return, - Err(err) => { - // Failed to submit event. - debug!( - target: LOG_TARGET, - "[follow][id={:?}] Failed to send event {:?}", self.sub_id, err - ); - break - }, + let result = sink.send(&event); + + // Migration note: the new version of jsonrpsee returns Result<(), DisconnectError> + // The logic from `Err(err)` should be moved when building the new + // `SubscriptionMessage`. + + // For now, jsonrpsee returns: + // Ok(true): message sent + // Ok(false): client disconnected or subscription closed + // Err(err): serder serialization error of the event + if let Err(err) = result { + // Failed to submit event. + debug!( + target: LOG_TARGET, + "[follow][id={:?}] Failed to send event {:?}", self.sub_id, err + ); + + let _ = sink.send(&FollowEvent::::Stop); + return + } + + if let Ok(false) = result { + // Client disconnected or subscription was closed. + return } } stream_item = stream.next(); stop_event = next_stop_event; } - - let _ = sink.send(&FollowEvent::::Stop); } /// Generate the block events for the `chainHead_follow` method. From bf85692565f125a9e8d6a2ca15e9723dc9645e04 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 20:27:30 +0200 Subject: [PATCH 27/35] rpc/chain_head: Use default constructor Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 479ed6231e426..af92a5a331f65 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -405,7 +405,7 @@ where return Err(SubscriptionManagementError::BlockNumberAbsent) }; if block_number < info.finalized_number { - return Ok(vec![]) + return Ok(Default::default()) } // The tree route contains the exclusive path from the last finalized block to the block From 5ba5cd22a17a5bb97b038006874ea3460593840f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 21:00:33 +0200 Subject: [PATCH 28/35] rpc/chain_head: Add `StartupPoint` structure Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 55 +++++++++++++------ 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index af92a5a331f65..d1dcc7ea509c8 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -42,7 +42,7 @@ use sp_blockchain::{ Backend as BlockChainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata, Info, }; use sp_runtime::{ - traits::{Block as BlockT, Header, One}, + traits::{Block as BlockT, Header as HeaderT, One}, Saturating, }; use std::{collections::HashSet, sync::Arc}; @@ -110,6 +110,26 @@ struct InitialBlocks { pruned_forks: HashSet, } +/// The startup point from which chainHead started to generate events. +struct StartupPoint { + /// Best block hash. + pub best_hash: Block::Hash, + /// The head of the finalized chain. + pub finalized_hash: Block::Hash, + /// Last finalized block number. + pub finalized_number: <::Header as HeaderT>::Number, +} + +impl From> for StartupPoint { + fn from(info: Info) -> Self { + StartupPoint:: { + best_hash: info.best_hash, + finalized_hash: info.finalized_hash, + finalized_number: info.finalized_number, + } + } +} + impl ChainHeadFollower where Block: BlockT + 'static, @@ -159,11 +179,11 @@ where /// Get the in-memory blocks of the client, starting from the provided finalized hash. fn get_init_blocks_with_forks( &self, - info: &Info, + startup_point: &StartupPoint, ) -> Result, SubscriptionManagementError> { let blockchain = self.backend.blockchain(); let leaves = blockchain.leaves()?; - let finalized = info.finalized_hash; + let finalized = startup_point.finalized_hash; let mut pruned_forks = HashSet::new(); let mut finalized_block_descendants = Vec::new(); for leaf in leaves { @@ -194,15 +214,15 @@ where /// block hashes that should be ignored by the `Finalized` event. fn generate_init_events( &mut self, - info: &Info, + startup_point: &StartupPoint, ) -> Result<(Vec>, HashSet), SubscriptionManagementError> { - let init = self.get_init_blocks_with_forks(info)?; + let init = self.get_init_blocks_with_forks(startup_point)?; let initial_blocks = init.finalized_block_descendants; // The initialized event is the first one sent. - let finalized_block_hash = info.finalized_hash; + let finalized_block_hash = startup_point.finalized_hash; self.sub_handle.pin_block(finalized_block_hash)?; let finalized_block_runtime = self.generate_runtime_event(finalized_block_hash, None); @@ -232,7 +252,7 @@ where } // Generate a new best block event. - let best_block_hash = info.best_hash; + let best_block_hash = startup_point.best_hash; if best_block_hash != finalized_block_hash { let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); self.best_block_cache = Some(best_block_hash); @@ -289,7 +309,7 @@ where fn handle_import_blocks( &mut self, notification: BlockImportNotification, - info: &Info, + startup_point: &StartupPoint, ) -> Result>, SubscriptionManagementError> { // The block was already pinned by the initial block events or by the finalized event. if !self.sub_handle.pin_block(notification.hash)? { @@ -300,7 +320,7 @@ where let Some(block_number) = self.client.number(notification.hash)? else { return Err(SubscriptionManagementError::BlockNumberAbsent) }; - if block_number < info.finalized_number { + if block_number < startup_point.finalized_number { return Ok(Default::default()) } @@ -396,7 +416,7 @@ where &mut self, notification: FinalityNotification, to_ignore: &mut HashSet, - info: &Info, + startup_point: &StartupPoint, ) -> Result>, SubscriptionManagementError> { let last_finalized = notification.hash; @@ -404,7 +424,7 @@ where let Some(block_number) = self.client.number(last_finalized)? else { return Err(SubscriptionManagementError::BlockNumberAbsent) }; - if block_number < info.finalized_number { + if block_number < startup_point.finalized_number { return Ok(Default::default()) } @@ -486,7 +506,7 @@ where /// for as long as the `rx_stop` event was not called. async fn submit_events( &mut self, - info: &Info, + startup_point: &StartupPoint, mut stream: EventStream, mut to_ignore: HashSet, mut sink: SubscriptionSink, @@ -503,9 +523,9 @@ where let events = match event { NotificationType::InitialEvents(events) => Ok(events), NotificationType::NewBlock(notification) => - self.handle_import_blocks(notification, &info), + self.handle_import_blocks(notification, &startup_point), NotificationType::Finalized(notification) => - self.handle_finalized_blocks(notification, &mut to_ignore, &info), + self.handle_finalized_blocks(notification, &mut to_ignore, &startup_point), }; let events = match events { @@ -572,8 +592,8 @@ where .finality_notification_stream() .map(|notification| NotificationType::Finalized(notification)); - let info = self.client.info(); - let (initial_events, pruned_forks) = match self.generate_init_events(&info) { + let startup_point = StartupPoint::from(self.client.info()); + let (initial_events, pruned_forks) = match self.generate_init_events(&startup_point) { Ok(blocks) => blocks, Err(err) => { debug!( @@ -591,6 +611,7 @@ where let merged = tokio_stream::StreamExt::merge(stream_import, stream_finalized); let stream = stream::once(futures::future::ready(initial)).chain(merged); - self.submit_events(&info, stream.boxed(), pruned_forks, sink, rx_stop).await; + self.submit_events(&startup_point, stream.boxed(), pruned_forks, sink, rx_stop) + .await; } } From f0a40f0b73441c6a5d88c9bb0a14f8a884dc7701 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 22:12:37 +0200 Subject: [PATCH 29/35] rpc/chain_head: Rename `in_memory_blocks` Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index d1dcc7ea509c8..526ff340cfb2e 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -233,9 +233,9 @@ where runtime_updates: self.runtime_updates, }); - let mut in_memory_blocks = Vec::with_capacity(initial_blocks.len() + 1); + let mut finalized_block_descendants = Vec::with_capacity(initial_blocks.len() + 1); - in_memory_blocks.push(initialized_event); + finalized_block_descendants.push(initialized_event); for (child, parent) in initial_blocks.into_iter() { self.sub_handle.pin_block(child)?; @@ -248,7 +248,7 @@ where runtime_updates: self.runtime_updates, }); - in_memory_blocks.push(event); + finalized_block_descendants.push(event); } // Generate a new best block event. @@ -256,10 +256,10 @@ where if best_block_hash != finalized_block_hash { let best_block = FollowEvent::BestBlockChanged(BestBlockChanged { best_block_hash }); self.best_block_cache = Some(best_block_hash); - in_memory_blocks.push(best_block); + finalized_block_descendants.push(best_block); }; - Ok((in_memory_blocks, init.pruned_forks)) + Ok((finalized_block_descendants, init.pruned_forks)) } /// Generate the "NewBlock" event and potentially the "BestBlockChanged" event for the From 47853cb4cb1582aa2de3b0cd8d8c7427c9973d68 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 1 Mar 2023 22:13:42 +0200 Subject: [PATCH 30/35] rpc/chain_head: Fix comment typo Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 526ff340cfb2e..e97a970303822 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -355,7 +355,7 @@ where let Some(parent) = self.client.hash(first_number.saturating_sub(One::one()))? else { return Err(SubscriptionManagementError::BlockHashAbsent) }; - // The parent of the finalized block hash was not reported by the `chianHead`. + // The parent of the finalized block hash was not reported by the `chainHead`. // There may be a gap in the notifications and we cannot guarantee that // every new block has a parent reported by an event. if !self.sub_handle.contains_block(&parent) { @@ -408,7 +408,7 @@ where Ok(pruned) } - /// Handle the finalization notification by generating the "Finalized" event. + /// Handle the finalization notification by generating the `Finalized` event. /// /// If the block of the notification was not reported yet, this method also /// generates the events similar to `handle_import_blocks`. From 708aa0e1dc42d2afd43be8d09840bbac08ec68de Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 2 Mar 2023 13:04:43 +0200 Subject: [PATCH 31/35] rpc/chain_head: Keep unique blocks and remove itertools Signed-off-by: Alexandru Vasile --- Cargo.lock | 1 - client/rpc-spec-v2/Cargo.toml | 1 - .../rpc-spec-v2/src/chain_head/chain_head_follow.rs | 13 +++++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dfc0d63749b00..299f639219437 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8941,7 +8941,6 @@ dependencies = [ "futures", "futures-util", "hex", - "itertools", "jsonrpsee", "log", "parity-scale-codec", diff --git a/client/rpc-spec-v2/Cargo.toml b/client/rpc-spec-v2/Cargo.toml index bdb9ca941df9f..43fb189081bae 100644 --- a/client/rpc-spec-v2/Cargo.toml +++ b/client/rpc-spec-v2/Cargo.toml @@ -34,7 +34,6 @@ tokio-stream = { version = "0.1", features = ["sync"] } array-bytes = "4.1" log = "0.4.17" futures-util = { version = "0.3.19", default-features = false } -itertools = "0.10.3" [dev-dependencies] serde_json = "1.0" diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index e97a970303822..6e7bf4d91eafc 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -31,7 +31,6 @@ use futures::{ stream::{self, Stream, StreamExt}, }; use futures_util::future::Either; -use itertools::Itertools; use jsonrpsee::SubscriptionSink; use log::{debug, error}; use sc_client_api::{ @@ -186,6 +185,7 @@ where let finalized = startup_point.finalized_hash; let mut pruned_forks = HashSet::new(); let mut finalized_block_descendants = Vec::new(); + let mut unique_descendants = HashSet::new(); for leaf in leaves { let tree_route = sp_blockchain::tree_route(blockchain, finalized, leaf)?; @@ -197,14 +197,15 @@ where // finalized block. Describe the tree route as (child_node, parent_node) // Note: the order of elements matters here. let parents = std::iter::once(finalized).chain(blocks.clone()); - finalized_block_descendants.extend(blocks.zip(parents)); + + for pair in blocks.zip(parents) { + if unique_descendants.insert(pair) { + finalized_block_descendants.push(pair); + } + } } } - // Keep unique blocks. - let finalized_block_descendants: Vec<_> = - finalized_block_descendants.into_iter().unique().collect(); - Ok(InitialBlocks { finalized_block_descendants, pruned_forks }) } From 49245dc665419723595d1d54ca3b4cbfd70500c5 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 2 Mar 2023 15:04:31 +0200 Subject: [PATCH 32/35] rpc/chain_head: Make sure `bestBlocks` events are generated in order Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 6e7bf4d91eafc..af5232aba5208 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -373,9 +373,42 @@ where continue } + // Generate only the `NewBlock` event for this block. + if hash != last_finalized { + events.extend(self.generate_import_events(*hash, *parent, false)); + continue + } + + // Determine if a `BestBlock` event should be generated for the last finalized block. + let is_best_block = match self.best_block_cache { + Some(best_block_hash) => { + // If the best reported block is a children of the last finalized, + // then do not report a `BestBlock` event older than what was previously + // reported. + let ancestor = sp_blockchain::lowest_common_ancestor( + &*self.client, + *last_finalized, + best_block_hash, + )?; + + // A best block on this branch was already reported. + if ancestor.hash == *last_finalized { + false + } else { + self.best_block_cache = Some(*hash); + true + } + }, + // This is the first best block event that we generate. + None => { + self.best_block_cache = Some(*hash); + true + }, + }; + // This is the first time we see this block. Generate the `NewBlock` event; if this is // the last block, also generate the `BestBlock` event. - events.extend(self.generate_import_events(*hash, *parent, hash == last_finalized)) + events.extend(self.generate_import_events(*hash, *parent, is_best_block)) } Ok(events) From 095a39d09a8e6ceca6ed19f728bd902343981703 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 2 Mar 2023 15:21:26 +0200 Subject: [PATCH 33/35] rpc/chain_head: Maintain order of reported blocks Signed-off-by: Alexandru Vasile --- .../src/chain_head/chain_head_follow.rs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index af5232aba5208..5d91c24f0f00e 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -379,36 +379,35 @@ where continue } - // Determine if a `BestBlock` event should be generated for the last finalized block. - let is_best_block = match self.best_block_cache { + match self.best_block_cache { Some(best_block_hash) => { // If the best reported block is a children of the last finalized, - // then do not report a `BestBlock` event older than what was previously - // reported. + // then we had a gap in notification. let ancestor = sp_blockchain::lowest_common_ancestor( &*self.client, *last_finalized, best_block_hash, )?; - // A best block on this branch was already reported. + // A descendent of the finalized block was already reported + // before the `NewBlock` event containing the finalized block + // is reported. if ancestor.hash == *last_finalized { - false - } else { - self.best_block_cache = Some(*hash); - true + return Err(SubscriptionManagementError::Custom( + "A descendent of the finalized block was already reported".into(), + )) } + self.best_block_cache = Some(*hash); }, // This is the first best block event that we generate. None => { self.best_block_cache = Some(*hash); - true }, }; // This is the first time we see this block. Generate the `NewBlock` event; if this is // the last block, also generate the `BestBlock` event. - events.extend(self.generate_import_events(*hash, *parent, is_best_block)) + events.extend(self.generate_import_events(*hash, *parent, true)) } Ok(events) From ce35d024e46b99e563f422c3668061b4d9e4bb6a Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 2 Mar 2023 16:51:37 +0200 Subject: [PATCH 34/35] rpc/chain_head: Parent of finalized block could be unpinned Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/chain_head_follow.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 5d91c24f0f00e..9173b7340b7e5 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -356,12 +356,6 @@ where let Some(parent) = self.client.hash(first_number.saturating_sub(One::one()))? else { return Err(SubscriptionManagementError::BlockHashAbsent) }; - // The parent of the finalized block hash was not reported by the `chainHead`. - // There may be a gap in the notifications and we cannot guarantee that - // every new block has a parent reported by an event. - if !self.sub_handle.contains_block(&parent) { - return Err(SubscriptionManagementError::BlockNotPinned) - } let last_finalized = finalized_block_hashes .last() From 08d23445a9df8d22b90a435b85cbbea511fddb54 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 2 Mar 2023 16:56:57 +0200 Subject: [PATCH 35/35] rpc/chain_head: Fix warning Signed-off-by: Alexandru Vasile --- client/rpc-spec-v2/src/chain_head/subscription.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/rpc-spec-v2/src/chain_head/subscription.rs b/client/rpc-spec-v2/src/chain_head/subscription.rs index b1ea2e9730117..77d57e747ebc1 100644 --- a/client/rpc-spec-v2/src/chain_head/subscription.rs +++ b/client/rpc-spec-v2/src/chain_head/subscription.rs @@ -40,8 +40,6 @@ pub enum SubscriptionManagementError { BlockNumberAbsent, /// The database does not contain a block hash. BlockHashAbsent, - /// Block was expected to be pinned. - BlockNotPinned, /// Custom error. Custom(String), }