From 238a2ae46e393455798e7b1439bd7b49611cff0f Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 08:58:36 +0200 Subject: [PATCH 01/46] Move cache populating task to separate module --- light-base/src/json_rpc_service/background.rs | 94 +------------ .../background/legacy_state_sub.rs | 123 ++++++++++++++++++ 2 files changed, 125 insertions(+), 92 deletions(-) create mode 100644 light-base/src/json_rpc_service/background/legacy_state_sub.rs diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 3ae5493af9..74f1487316 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -48,6 +48,7 @@ use smoldot::{ mod chain_head; mod getters; +mod legacy_state_sub; mod state_chain; mod transactions; @@ -281,98 +282,7 @@ pub(super) fn start( ); } - // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime - // service. - // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache - // TODO: extract to separate function - me.platform - .clone() - .spawn_task(format!("{}-cache-populate", me.log_target).into(), { - future::Abortable::new( - async move { - loop { - let mut cache = me.cache.lock().await; - - // Subscribe to new runtime service blocks in order to push them in the - // cache as soon as they are available. - // The buffer size should be large enough so that, if the CPU is busy, it - // doesn't become full before the execution of this task resumes. - // The maximum number of pinned block is ignored, as this maximum is a way to - // avoid malicious behaviors. This code is by definition not considered - // malicious. - let mut subscribe_all = me - .runtime_service - .subscribe_all( - "json-rpc-blocks-cache", - 32, - NonZeroUsize::new(usize::max_value()).unwrap(), - ) - .await; - - cache.subscription_id = Some(subscribe_all.new_blocks.id()); - cache.recent_pinned_blocks.clear(); - debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); - - let finalized_block_hash = header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - ); - cache.recent_pinned_blocks.put( - finalized_block_hash, - subscribe_all.finalized_block_scale_encoded_header, - ); - - for block in subscribe_all.non_finalized_blocks_ancestry_order { - if cache.recent_pinned_blocks.len() - == cache.recent_pinned_blocks.cap().get() - { - let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); - subscribe_all.new_blocks.unpin_block(&hash).await; - } - - let hash = - header::hash_from_scale_encoded_header(&block.scale_encoded_header); - cache - .recent_pinned_blocks - .put(hash, block.scale_encoded_header); - } - - drop(cache); - - loop { - let notification = subscribe_all.new_blocks.next().await; - match notification { - Some(runtime_service::Notification::Block(block)) => { - let mut cache = me.cache.lock().await; - - if cache.recent_pinned_blocks.len() - == cache.recent_pinned_blocks.cap().get() - { - let (hash, _) = - cache.recent_pinned_blocks.pop_lru().unwrap(); - subscribe_all.new_blocks.unpin_block(&hash).await; - } - - let hash = header::hash_from_scale_encoded_header( - &block.scale_encoded_header, - ); - cache - .recent_pinned_blocks - .put(hash, block.scale_encoded_header); - } - Some(runtime_service::Notification::Finalized { .. }) - | Some(runtime_service::Notification::BestBlockChanged { - .. - }) => {} - None => break, - } - } - } - }, - background_abort_registrations.next().unwrap(), - ) - .map(|_: Result<(), _>| ()) - .boxed() - }); + legacy_state_sub::start_task(me, background_abort_registrations.next().unwrap()); debug_assert!(background_abort_registrations.next().is_none()); } diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs new file mode 100644 index 0000000000..51ab02738f --- /dev/null +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -0,0 +1,123 @@ +// Smoldot +// Copyright (C) 2023 Pierre Krieger +// 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 . + +use core::num::NonZeroUsize; + +use alloc::{format, sync::Arc}; +use futures_util::{future, stream::AbortRegistration, FutureExt}; +use smoldot::header; + +use crate::{platform::PlatformRef, runtime_service}; + +use super::Background; + +// Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. +pub(super) fn start_task( + me: Arc>, + abort_registration: AbortRegistration, +) { + // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache + // TODO: extract to separate function + me.platform + .clone() + .spawn_task(format!("{}-cache-populate", me.log_target).into(), { + future::Abortable::new( + async move { + loop { + let mut cache = me.cache.lock().await; + + // Subscribe to new runtime service blocks in order to push them in the + // cache as soon as they are available. + // The buffer size should be large enough so that, if the CPU is busy, it + // doesn't become full before the execution of this task resumes. + // The maximum number of pinned block is ignored, as this maximum is a way to + // avoid malicious behaviors. This code is by definition not considered + // malicious. + let mut subscribe_all = me + .runtime_service + .subscribe_all( + "json-rpc-blocks-cache", + 32, + NonZeroUsize::new(usize::max_value()).unwrap(), + ) + .await; + + cache.subscription_id = Some(subscribe_all.new_blocks.id()); + cache.recent_pinned_blocks.clear(); + debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); + + let finalized_block_hash = header::hash_from_scale_encoded_header( + &subscribe_all.finalized_block_scale_encoded_header, + ); + cache.recent_pinned_blocks.put( + finalized_block_hash, + subscribe_all.finalized_block_scale_encoded_header, + ); + + for block in subscribe_all.non_finalized_blocks_ancestry_order { + if cache.recent_pinned_blocks.len() + == cache.recent_pinned_blocks.cap().get() + { + let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); + subscribe_all.new_blocks.unpin_block(&hash).await; + } + + let hash = + header::hash_from_scale_encoded_header(&block.scale_encoded_header); + cache + .recent_pinned_blocks + .put(hash, block.scale_encoded_header); + } + + drop(cache); + + loop { + let notification = subscribe_all.new_blocks.next().await; + match notification { + Some(runtime_service::Notification::Block(block)) => { + let mut cache = me.cache.lock().await; + + if cache.recent_pinned_blocks.len() + == cache.recent_pinned_blocks.cap().get() + { + let (hash, _) = + cache.recent_pinned_blocks.pop_lru().unwrap(); + subscribe_all.new_blocks.unpin_block(&hash).await; + } + + let hash = header::hash_from_scale_encoded_header( + &block.scale_encoded_header, + ); + cache + .recent_pinned_blocks + .put(hash, block.scale_encoded_header); + } + Some(runtime_service::Notification::Finalized { .. }) + | Some(runtime_service::Notification::BestBlockChanged { + .. + }) => {} + None => break, + } + } + } + }, + abort_registration, + ) + .map(|_: Result<(), _>| ()) + .boxed() + }); +} From 95357d2f025475cc94a450aab5359cb5468b364b Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 09:02:19 +0200 Subject: [PATCH 02/46] Move main loop to separate function --- .../background/legacy_state_sub.rs | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 51ab02738f..d48a2ae243 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -85,34 +85,10 @@ pub(super) fn start_task( drop(cache); - loop { - let notification = subscribe_all.new_blocks.next().await; - match notification { - Some(runtime_service::Notification::Block(block)) => { - let mut cache = me.cache.lock().await; - - if cache.recent_pinned_blocks.len() - == cache.recent_pinned_blocks.cap().get() - { - let (hash, _) = - cache.recent_pinned_blocks.pop_lru().unwrap(); - subscribe_all.new_blocks.unpin_block(&hash).await; - } - - let hash = header::hash_from_scale_encoded_header( - &block.scale_encoded_header, - ); - cache - .recent_pinned_blocks - .put(hash, block.scale_encoded_header); - } - Some(runtime_service::Notification::Finalized { .. }) - | Some(runtime_service::Notification::BestBlockChanged { - .. - }) => {} - None => break, - } - } + run(Task { + background: me, + new_blocks: subscribe_all.new_blocks, + }); } }, abort_registration, @@ -121,3 +97,32 @@ pub(super) fn start_task( .boxed() }); } + +struct Task { + background: Arc>, + new_blocks: runtime_service::Subscription, +} + +async fn run(mut task: Task) { + loop { + let notification = task.new_blocks.next().await; + match notification { + Some(runtime_service::Notification::Block(block)) => { + let mut cache = task.background.cache.lock().await; + + if cache.recent_pinned_blocks.len() == cache.recent_pinned_blocks.cap().get() { + let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); + task.new_blocks.unpin_block(&hash).await; + } + + let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); + cache + .recent_pinned_blocks + .put(hash, block.scale_encoded_header); + } + Some(runtime_service::Notification::Finalized { .. }) + | Some(runtime_service::Notification::BestBlockChanged { .. }) => {} + None => break, + } + } +} From 5e7a60bcb148a663c51d1977b580f42ef7afd4b4 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 09:06:13 +0200 Subject: [PATCH 03/46] Pass individual components to the task rather than the Background --- light-base/src/json_rpc_service/background.rs | 14 ++++-- .../background/legacy_state_sub.rs | 45 ++++++++++--------- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 74f1487316..7d000605e3 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -87,7 +87,7 @@ struct Background { /// Various information caches about blocks, to potentially reduce the number of network /// requests to perform. - cache: Mutex, + cache: Arc>, /// Hash of the genesis block. /// Keeping the genesis block is important, as the genesis block hash is included in @@ -190,7 +190,7 @@ pub(super) fn start( sync_service: config.sync_service.clone(), runtime_service: config.runtime_service.clone(), transactions_service: config.transactions_service.clone(), - cache: Mutex::new(Cache { + cache: Arc::new(Mutex::new(Cache { recent_pinned_blocks: lru::LruCache::with_hasher( NonZeroUsize::new(32).unwrap(), Default::default(), @@ -204,7 +204,7 @@ pub(super) fn start( NonZeroUsize::new(2).unwrap(), util::SipHasherBuild::new(rand::random()), ), - }), + })), genesis_block_hash: config.genesis_block_hash, printed_legacy_json_rpc_warning: atomic::AtomicBool::new(false), chain_head_follow_tasks: Mutex::new(hashbrown::HashMap::with_hasher(Default::default())), @@ -282,7 +282,13 @@ pub(super) fn start( ); } - legacy_state_sub::start_task(me, background_abort_registrations.next().unwrap()); + legacy_state_sub::start_task( + me.cache.clone(), + me.platform.clone(), + me.log_target.clone(), + me.runtime_service.clone(), + background_abort_registrations.next().unwrap(), + ); debug_assert!(background_abort_registrations.next().is_none()); } diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index d48a2ae243..e9d35efed8 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -17,28 +17,32 @@ use core::num::NonZeroUsize; -use alloc::{format, sync::Arc}; +use alloc::{format, string::String, sync::Arc}; +use async_lock::Mutex; use futures_util::{future, stream::AbortRegistration, FutureExt}; use smoldot::header; use crate::{platform::PlatformRef, runtime_service}; -use super::Background; +use super::Cache; // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. pub(super) fn start_task( - me: Arc>, + cache: Arc>, + platform: TPlat, + log_target: String, + runtime_service: Arc>, abort_registration: AbortRegistration, ) { // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache // TODO: extract to separate function - me.platform + platform .clone() - .spawn_task(format!("{}-cache-populate", me.log_target).into(), { + .spawn_task(format!("{}-cache-populate", log_target).into(), { future::Abortable::new( async move { loop { - let mut cache = me.cache.lock().await; + let mut cache_lock = cache.lock().await; // Subscribe to new runtime service blocks in order to push them in the // cache as soon as they are available. @@ -47,8 +51,7 @@ pub(super) fn start_task( // The maximum number of pinned block is ignored, as this maximum is a way to // avoid malicious behaviors. This code is by definition not considered // malicious. - let mut subscribe_all = me - .runtime_service + let subscribe_all = runtime_service .subscribe_all( "json-rpc-blocks-cache", 32, @@ -56,39 +59,39 @@ pub(super) fn start_task( ) .await; - cache.subscription_id = Some(subscribe_all.new_blocks.id()); - cache.recent_pinned_blocks.clear(); - debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); + cache_lock.subscription_id = Some(subscribe_all.new_blocks.id()); + cache_lock.recent_pinned_blocks.clear(); + debug_assert!(cache_lock.recent_pinned_blocks.cap().get() >= 1); let finalized_block_hash = header::hash_from_scale_encoded_header( &subscribe_all.finalized_block_scale_encoded_header, ); - cache.recent_pinned_blocks.put( + cache_lock.recent_pinned_blocks.put( finalized_block_hash, subscribe_all.finalized_block_scale_encoded_header, ); for block in subscribe_all.non_finalized_blocks_ancestry_order { - if cache.recent_pinned_blocks.len() - == cache.recent_pinned_blocks.cap().get() + if cache_lock.recent_pinned_blocks.len() + == cache_lock.recent_pinned_blocks.cap().get() { - let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); + let (hash, _) = cache_lock.recent_pinned_blocks.pop_lru().unwrap(); subscribe_all.new_blocks.unpin_block(&hash).await; } let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - cache + cache_lock .recent_pinned_blocks .put(hash, block.scale_encoded_header); } - drop(cache); + drop(cache_lock); run(Task { - background: me, + cache: cache.clone(), new_blocks: subscribe_all.new_blocks, - }); + }).await; } }, abort_registration, @@ -99,7 +102,7 @@ pub(super) fn start_task( } struct Task { - background: Arc>, + cache: Arc>, new_blocks: runtime_service::Subscription, } @@ -108,7 +111,7 @@ async fn run(mut task: Task) { let notification = task.new_blocks.next().await; match notification { Some(runtime_service::Notification::Block(block)) => { - let mut cache = task.background.cache.lock().await; + let mut cache = task.cache.lock().await; if cache.recent_pinned_blocks.len() == cache.recent_pinned_blocks.cap().get() { let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); From 62a65708fcf073553ff179633caa64bd060e5ed3 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 13:13:33 +0200 Subject: [PATCH 04/46] Move `chain_subscribeAllHeads` to new task --- light-base/src/json_rpc_service/background.rs | 22 +++- .../background/legacy_state_sub.rs | 100 ++++++++++++--- .../background/state_chain.rs | 118 ------------------ 3 files changed, 105 insertions(+), 135 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 7d000605e3..0c1930634f 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -85,6 +85,10 @@ struct Background { /// See [`StartConfig::transactions_service`]. transactions_service: Arc>, + /// Channel where to send requests that concern the legacy JSON-RPC API that are handled by + /// a dedicated task. + to_legacy: Mutex>, + /// Various information caches about blocks, to potentially reduce the number of network /// requests to perform. cache: Arc>, @@ -176,6 +180,8 @@ pub(super) fn start( max_parallel_requests: NonZeroU32, background_abort_registrations: Vec, ) { + let (to_legacy_tx, to_legacy_rx) = async_channel::bounded(8); + let me = Arc::new(Background { log_target, platform: config.platform, @@ -190,6 +196,7 @@ pub(super) fn start( sync_service: config.sync_service.clone(), runtime_service: config.runtime_service.clone(), transactions_service: config.transactions_service.clone(), + to_legacy: Mutex::new(to_legacy_tx), cache: Arc::new(Mutex::new(Cache { recent_pinned_blocks: lru::LruCache::with_hasher( NonZeroUsize::new(32).unwrap(), @@ -237,7 +244,17 @@ pub(super) fn start( subscription_start, } => { requests_processing_task = task; - tx.send(either::Right(subscription_start)).await.unwrap(); + match subscription_start.request() { + methods::MethodCall::chain_subscribeAllHeads {} => { + me.to_legacy + .lock() + .await + .send(subscription_start) + .await + .unwrap(); + } + _ => tx.send(either::Right(subscription_start)).await.unwrap(), + } } service::Event::SubscriptionDestroyed { task, @@ -287,6 +304,7 @@ pub(super) fn start( me.platform.clone(), me.log_target.clone(), me.runtime_service.clone(), + to_legacy_rx, background_abort_registrations.next().unwrap(), ); @@ -678,7 +696,7 @@ impl Background { self.submit_and_watch_transaction(request).await } methods::MethodCall::chain_subscribeAllHeads {} => { - self.chain_subscribe_all_heads(request).await; + unreachable!() } methods::MethodCall::chain_subscribeFinalizedHeads {} => { self.chain_subscribe_finalized_heads(request).await; diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index e9d35efed8..11538b229f 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -17,10 +17,15 @@ use core::num::NonZeroUsize; -use alloc::{format, string::String, sync::Arc}; +use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc}; use async_lock::Mutex; -use futures_util::{future, stream::AbortRegistration, FutureExt}; -use smoldot::header; +use futures_lite::{FutureExt as _, StreamExt as _}; +use futures_util::{future, stream::AbortRegistration, FutureExt as _}; +use smoldot::{ + header, + informant::HashDisplay, + json_rpc::{methods, service}, +}; use crate::{platform::PlatformRef, runtime_service}; @@ -32,13 +37,14 @@ pub(super) fn start_task( platform: TPlat, log_target: String, runtime_service: Arc>, + requests_rx: async_channel::Receiver, abort_registration: AbortRegistration, ) { // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache // TODO: extract to separate function - platform - .clone() - .spawn_task(format!("{}-cache-populate", log_target).into(), { + platform.clone().spawn_task( + format!("{}-cache-populate", log_target).into(), + Box::pin({ future::Abortable::new( async move { loop { @@ -90,27 +96,48 @@ pub(super) fn start_task( run(Task { cache: cache.clone(), + log_target: log_target.clone(), + block_number_bytes: runtime_service.block_number_bytes(), new_blocks: subscribe_all.new_blocks, - }).await; + requests_rx, + // TODO: all the subscriptions are dropped if the task returns + all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), + }) + .await; + + panic!() // TODO: not implemented correctly } }, abort_registration, ) .map(|_: Result<(), _>| ()) - .boxed() - }); + }), + ); } struct Task { cache: Arc>, + log_target: String, + block_number_bytes: usize, new_blocks: runtime_service::Subscription, + requests_rx: async_channel::Receiver, + // TODO: shrink_to_fit? + all_heads_subscriptions: hashbrown::HashMap, } async fn run(mut task: Task) { loop { - let notification = task.new_blocks.next().await; - match notification { - Some(runtime_service::Notification::Block(block)) => { + match task + .new_blocks + .next() + .map(either::Left) + .or(task.requests_rx.next().map(either::Right)) + .await + { + either::Left(Some(runtime_service::Notification::Block(block))) => { let mut cache = task.cache.lock().await; if cache.recent_pinned_blocks.len() == cache.recent_pinned_blocks.cap().get() { @@ -118,14 +145,57 @@ async fn run(mut task: Task) { task.new_blocks.unpin_block(&hash).await; } + let json_rpc_header = match methods::Header::from_scale_encoded_header( + &block.scale_encoded_header, + task.block_number_bytes, + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeAllHeads` subscription has skipped \ + block due to undecodable header. Hash: {}. Error: {}", + HashDisplay(&header::hash_from_scale_encoded_header( + &block.scale_encoded_header + )), + error, + ); + continue; + } + }; + let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); cache .recent_pinned_blocks .put(hash, block.scale_encoded_header); + + drop(cache); + + for (subscription_id, subscription) in &mut task.all_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_allHead { + subscription: subscription_id.as_str().into(), + result: json_rpc_header.clone(), + }) + .await; + } + } + either::Left(Some(runtime_service::Notification::Finalized { .. })) + | either::Left(Some(runtime_service::Notification::BestBlockChanged { .. })) => {} + + either::Right(Some(request)) => match request.request() { + methods::MethodCall::chain_subscribeNewHeads {} => { + let subscription = request.accept(); + let subscription_id = subscription.subscription_id().to_owned(); + task.all_heads_subscriptions + .insert(subscription_id, subscription); + } + _ => unreachable!(), // TODO: stronger typing to avoid this? + }, + + either::Left(None) | either::Right(None) => { + break; } - Some(runtime_service::Notification::Finalized { .. }) - | Some(runtime_service::Notification::BestBlockChanged { .. }) => {} - None => break, } } } diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index cf64b14ec8..0ba7f1499a 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -327,124 +327,6 @@ impl Background { } } - /// Handles a call to [`methods::MethodCall::chain_subscribeAllHeads`]. - pub(super) async fn chain_subscribe_all_heads( - self: &Arc, - request: service::SubscriptionStartProcess, - ) { - let methods::MethodCall::chain_subscribeAllHeads {} = request.request() else { - unreachable!() - }; - - self.platform - .spawn_task(format!("{}-subscribe-all-heads", self.log_target).into(), { - let log_target = self.log_target.clone(); - let sync_service = self.sync_service.clone(); - let runtime_service = self.runtime_service.clone(); - - async move { - let mut subscription = request.accept(); - let subscription_id = subscription.subscription_id().to_owned(); - - 'main_sub_loop: loop { - let mut new_blocks = { - // The buffer size should be large enough so that, if the CPU is busy, it - // doesn't become full before the execution of the runtime service resumes. - // The maximum number of pinned block is ignored, as this maximum is a way - // to avoid malicious behaviors. This code is by definition not considered - // malicious. - let subscribe_all = runtime_service - .subscribe_all( - "chain_subscribeAllHeads", - 64, - NonZeroUsize::new(usize::max_value()).unwrap(), - ) - .await; - - // The existing finalized and already-known blocks aren't reported to the - // user, but we need to unpin them on to the runtime service. - subscribe_all - .new_blocks - .unpin_block(&header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - )) - .await; - for block in subscribe_all.non_finalized_blocks_ancestry_order { - subscribe_all - .new_blocks - .unpin_block(&header::hash_from_scale_encoded_header( - &block.scale_encoded_header, - )) - .await; - } - - subscribe_all.new_blocks - }; - - loop { - let message = { - let next_new_block = pin::pin!(new_blocks.next()); - let next_message = pin::pin!(subscription.wait_until_stale()); - match future::select(next_new_block, next_message).await { - future::Either::Left((v, _)) => either::Left(v), - future::Either::Right((v, _)) => either::Right(v), - } - }; - - match message { - either::Left(None) => { - // Break from the inner loop in order to recreate the channel. - break; - } - either::Left(Some(runtime_service::Notification::Block(block))) => { - new_blocks - .unpin_block(&header::hash_from_scale_encoded_header( - &block.scale_encoded_header, - )) - .await; - - let header = match methods::Header::from_scale_encoded_header( - &block.scale_encoded_header, - sync_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &log_target, - "`chain_subscribeAllHeads` subscription has skipped \ - block due to undecodable header. Hash: {}. Error: {}", - HashDisplay(&header::hash_from_scale_encoded_header( - &block.scale_encoded_header - )), - error, - ); - continue; - } - }; - - subscription - .send_notification(methods::ServerToClient::chain_allHead { - subscription: (&subscription_id).into(), - result: header, - }) - .await; - } - either::Left(Some( - runtime_service::Notification::BestBlockChanged { .. }, - )) - | either::Left(Some(runtime_service::Notification::Finalized { - .. - })) => {} - either::Right(()) => { - break 'main_sub_loop; - } - } - } - } - } - }); - } - /// Handles a call to [`methods::MethodCall::chain_subscribeFinalizedHeads`]. pub(super) async fn chain_subscribe_finalized_heads( self: &Arc, From c4a1d5994079dfb865191551eebc8aaf3376c2e0 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 13:17:37 +0200 Subject: [PATCH 05/46] Move chain_subscribeNewHeads to new task --- light-base/src/json_rpc_service/background.rs | 5 +- .../background/legacy_state_sub.rs | 38 +++++++-- .../background/state_chain.rs | 83 +------------------ 3 files changed, 38 insertions(+), 88 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 0c1930634f..211d52c9da 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -245,7 +245,8 @@ pub(super) fn start( } => { requests_processing_task = task; match subscription_start.request() { - methods::MethodCall::chain_subscribeAllHeads {} => { + methods::MethodCall::chain_subscribeAllHeads {} + | methods::MethodCall::chain_subscribeNewHeads {} => { me.to_legacy .lock() .await @@ -702,7 +703,7 @@ impl Background { self.chain_subscribe_finalized_heads(request).await; } methods::MethodCall::chain_subscribeNewHeads {} => { - self.chain_subscribe_new_heads(request).await; + unreachable!() } methods::MethodCall::state_subscribeRuntimeVersion {} => { self.state_subscribe_runtime_version(request).await; diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 11538b229f..a0e842a738 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -105,6 +105,10 @@ pub(super) fn start_task( 8, Default::default(), ), + new_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), }) .await; @@ -126,6 +130,8 @@ struct Task { requests_rx: async_channel::Receiver, // TODO: shrink_to_fit? all_heads_subscriptions: hashbrown::HashMap, + // TODO: shrink_to_fit? + new_heads_subscriptions: hashbrown::HashMap, } async fn run(mut task: Task) { @@ -153,8 +159,8 @@ async fn run(mut task: Task) { Err(error) => { log::warn!( target: &task.log_target, - "`chain_subscribeAllHeads` subscription has skipped \ - block due to undecodable header. Hash: {}. Error: {}", + "`chain_subscribeAllHeads` or `chain_subscribeNewHeads` subscription \ + has skipped block due to undecodable header. Hash: {}. Error: {}", HashDisplay(&header::hash_from_scale_encoded_header( &block.scale_encoded_header )), @@ -179,17 +185,39 @@ async fn run(mut task: Task) { }) .await; } + + if block.is_new_best { + for (subscription_id, subscription) in &mut task.new_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_newHead { + subscription: subscription_id.as_str().into(), + result: json_rpc_header.clone(), + }) + .await; + } + } + } + either::Left(Some(runtime_service::Notification::Finalized { .. })) => {} + either::Left(Some(runtime_service::Notification::BestBlockChanged { + hash, .. + })) => { + // TODO: report a chain_newHead subscription } - either::Left(Some(runtime_service::Notification::Finalized { .. })) - | either::Left(Some(runtime_service::Notification::BestBlockChanged { .. })) => {} either::Right(Some(request)) => match request.request() { - methods::MethodCall::chain_subscribeNewHeads {} => { + methods::MethodCall::chain_subscribeAllHeads {} => { let subscription = request.accept(); let subscription_id = subscription.subscription_id().to_owned(); task.all_heads_subscriptions .insert(subscription_id, subscription); } + methods::MethodCall::chain_subscribeNewHeads {} => { + let subscription = request.accept(); + let subscription_id = subscription.subscription_id().to_owned(); + // TODO: must immediately send the current best block + task.new_heads_subscriptions + .insert(subscription_id, subscription); + } _ => unreachable!(), // TODO: stronger typing to avoid this? }, diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 0ba7f1499a..ad3ccfd637 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -19,16 +19,11 @@ use super::{Background, GetKeysPagedCacheKey, PlatformRef}; -use crate::{runtime_service, sync_service}; +use crate::sync_service; use alloc::{borrow::ToOwned as _, format, string::ToString as _, sync::Arc, vec, vec::Vec}; use async_lock::MutexGuard; -use core::{ - iter, - num::{NonZeroU32, NonZeroUsize}, - pin, - time::Duration, -}; +use core::{iter, num::NonZeroU32, pin, time::Duration}; use futures_util::{future, stream, FutureExt as _, StreamExt as _}; use smoldot::{ header, @@ -405,80 +400,6 @@ impl Background { ); } - /// Handles a call to [`methods::MethodCall::chain_subscribeNewHeads`]. - pub(super) async fn chain_subscribe_new_heads( - self: &Arc, - request: service::SubscriptionStartProcess, - ) { - let methods::MethodCall::chain_subscribeNewHeads {} = request.request() else { - unreachable!() - }; - - let mut blocks_list = { - let (block_header, blocks_subscription) = - sub_utils::subscribe_best(&self.runtime_service).await; - stream::once(future::ready(block_header)).chain(blocks_subscription) - }; - - self.platform - .spawn_task(format!("{}-subscribe-new-heads", self.log_target).into(), { - let log_target = self.log_target.clone(); - let sync_service = self.sync_service.clone(); - - async move { - let mut subscription = request.accept(); - let subscription_id = subscription.subscription_id().to_owned(); - - loop { - let event = { - let unsubscribed = pin::pin!(subscription.wait_until_stale()); - match future::select(blocks_list.next(), unsubscribed).await { - future::Either::Left((ev, _)) => either::Left(ev), - future::Either::Right((ev, _)) => either::Right(ev), - } - }; - - match event { - either::Left(None) => { - // Stream returned by `subscribe_best` is always unlimited. - unreachable!() - } - either::Left(Some(header)) => { - let header = match methods::Header::from_scale_encoded_header( - &header, - sync_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &log_target, - "`chain_subscribeNewHeads` subscription has skipped block \ - due to undecodable header. Hash: {}. Error: {}", - HashDisplay(&header::hash_from_scale_encoded_header( - &header - )), - error, - ); - continue; - } - }; - - subscription - .send_notification(methods::ServerToClient::chain_newHead { - subscription: (&subscription_id).into(), - result: header, - }) - .await; - } - either::Right(()) => { - break; - } - } - } - } - }); - } - /// Handles a call to [`methods::MethodCall::payment_queryInfo`]. pub(super) async fn payment_query_info(self: &Arc, request: service::RequestProcess) { let methods::MethodCall::payment_queryInfo { From fc8553402daaefbaefedc7155e9658b5d8cc5b95 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 13:33:39 +0200 Subject: [PATCH 06/46] Add a `Message` enum for messages --- light-base/src/json_rpc_service/background.rs | 6 ++++-- .../json_rpc_service/background/legacy_state_sub.rs | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 211d52c9da..b1b3e90984 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -87,7 +87,7 @@ struct Background { /// Channel where to send requests that concern the legacy JSON-RPC API that are handled by /// a dedicated task. - to_legacy: Mutex>, + to_legacy: Mutex>, /// Various information caches about blocks, to potentially reduce the number of network /// requests to perform. @@ -250,7 +250,9 @@ pub(super) fn start( me.to_legacy .lock() .await - .send(subscription_start) + .send(legacy_state_sub::Message::SubscriptionStart( + subscription_start, + )) .await .unwrap(); } diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index a0e842a738..e555c9d4b0 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -31,13 +31,17 @@ use crate::{platform::PlatformRef, runtime_service}; use super::Cache; +pub(super) enum Message { + SubscriptionStart(service::SubscriptionStartProcess), +} + // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. pub(super) fn start_task( cache: Arc>, platform: TPlat, log_target: String, runtime_service: Arc>, - requests_rx: async_channel::Receiver, + requests_rx: async_channel::Receiver, abort_registration: AbortRegistration, ) { // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache @@ -127,7 +131,7 @@ struct Task { log_target: String, block_number_bytes: usize, new_blocks: runtime_service::Subscription, - requests_rx: async_channel::Receiver, + requests_rx: async_channel::Receiver, // TODO: shrink_to_fit? all_heads_subscriptions: hashbrown::HashMap, // TODO: shrink_to_fit? @@ -204,7 +208,7 @@ async fn run(mut task: Task) { // TODO: report a chain_newHead subscription } - either::Right(Some(request)) => match request.request() { + either::Right(Some(Message::SubscriptionStart(request))) => match request.request() { methods::MethodCall::chain_subscribeAllHeads {} => { let subscription = request.accept(); let subscription_id = subscription.subscription_id().to_owned(); From b7f70c0237f73448a5962f2a9d1ef6a3eda33f47 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 13:34:59 +0200 Subject: [PATCH 07/46] Notify when subscription is destroyed --- light-base/src/json_rpc_service/background.rs | 8 ++++++++ .../src/json_rpc_service/background/legacy_state_sub.rs | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index b1b3e90984..a3bda75547 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -269,6 +269,14 @@ pub(super) fn start( .lock() .await .remove(&subscription_id); + me.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::SubscriptionDestroyed { + subscription_id, + }) + .await + .unwrap(); } service::Event::SerializedRequestsIoClosed => { break; diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index e555c9d4b0..abdb6b14b2 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -33,6 +33,7 @@ use super::Cache; pub(super) enum Message { SubscriptionStart(service::SubscriptionStartProcess), + SubscriptionDestroyed { subscription_id: String }, } // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. @@ -225,6 +226,12 @@ async fn run(mut task: Task) { _ => unreachable!(), // TODO: stronger typing to avoid this? }, + either::Right(Some(Message::SubscriptionDestroyed { subscription_id })) => { + task.all_heads_subscriptions.remove(&subscription_id); + task.new_heads_subscriptions.remove(&subscription_id); + // TODO: shrink_to_fit? + } + either::Left(None) | either::Right(None) => { break; } From a0cd7b801698906fee05735e971c8a4ea3a6dc85 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 13:48:38 +0200 Subject: [PATCH 08/46] Make runtime_access ask the cache through a message --- light-base/src/json_rpc_service/background.rs | 192 +++++++++--------- .../background/legacy_state_sub.rs | 43 +++- .../background/state_chain.rs | 4 - 3 files changed, 130 insertions(+), 109 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index a3bda75547..01d6ad98ce 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -37,6 +37,7 @@ use core::{ sync::atomic, time::Duration, }; +use futures_channel::oneshot; use futures_util::{future, FutureExt as _}; use smoldot::{ executor::{host, runtime_host}, @@ -87,7 +88,7 @@ struct Background { /// Channel where to send requests that concern the legacy JSON-RPC API that are handled by /// a dedicated task. - to_legacy: Mutex>, + to_legacy: Mutex>>, /// Various information caches about blocks, to potentially reduce the number of network /// requests to perform. @@ -942,114 +943,109 @@ impl Background { self: &Arc, block_hash: &[u8; 32], ) -> Result, RuntimeCallError> { - let cache_lock = self.cache.lock().await; - // Try to find the block in the cache of recent blocks. Most of the time, the call target // should be in there. - let lock = if cache_lock.recent_pinned_blocks.contains(block_hash) { - // The runtime service has the block pinned, meaning that we can ask the runtime - // service to perform the call. - self.runtime_service - .pinned_block_runtime_access(cache_lock.subscription_id.unwrap(), block_hash) + if let Some(runtime_access) = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() .await - .ok() - } else { - None + .send(legacy_state_sub::Message::RecentBlockRuntimeAccess { + block_hash: *block_hash, + result_tx: tx, + }) + .await + .unwrap(); + rx.await.unwrap() + } { + return Ok(runtime_access); }; - Ok(if let Some(lock) = lock { - lock - } else { - // Second situation: the block is not in the cache of recent blocks. This isn't great. - drop::>(cache_lock); - - // The only solution is to download the runtime of the block in question from the network. + // Second situation: the block is not in the cache of recent blocks. This isn't great. + // The only solution is to download the runtime of the block in question from the network. - // TODO: considering caching the runtime code the same way as the state trie root hash + // TODO: considering caching the runtime code the same way as the state trie root hash - // In order to grab the runtime code and perform the call network request, we need - // to know the state trie root hash and the height of the block. - let (state_trie_root_hash, block_number) = self - .state_trie_root_hash(block_hash) - .await - .map_err(RuntimeCallError::FindStorageRootHashError)?; - - // Download the runtime of this block. This takes a long time as the runtime is rather - // big (around 1MiB in general). - let (storage_code, storage_heap_pages) = { - let entries = self - .sync_service - .clone() - .storage_query( - block_number, - block_hash, - &state_trie_root_hash, - [ - sync_service::StorageRequestItem { - key: b":code".to_vec(), - ty: sync_service::StorageRequestItemTy::Value, - }, - sync_service::StorageRequestItem { - key: b":heappages".to_vec(), - ty: sync_service::StorageRequestItemTy::Value, - }, - ] - .into_iter(), - 3, - Duration::from_secs(20), - NonZeroU32::new(1).unwrap(), - ) - .await - .map_err(runtime_service::RuntimeCallError::StorageQuery) - .map_err(RuntimeCallError::Call)?; - // TODO: not elegant - let heap_pages = entries - .iter() - .find_map(|entry| match entry { - sync_service::StorageResultItem::Value { key, value } - if key == b":heappages" => - { - Some(value.clone()) // TODO: overhead - } - _ => None, - }) - .unwrap(); - let code = entries - .iter() - .find_map(|entry| match entry { - sync_service::StorageResultItem::Value { key, value } - if key == b":code" => - { - Some(value.clone()) // TODO: overhead - } - _ => None, - }) - .unwrap(); - (code, heap_pages) - }; - - // Give the code and heap pages to the runtime service. The runtime service will - // try to find any similar runtime it might have, and if not will compile it. - let pinned_runtime_id = self - .runtime_service - .compile_and_pin_runtime(storage_code, storage_heap_pages) - .await; - - let precall = self - .runtime_service - .pinned_runtime_access( - pinned_runtime_id.clone(), - *block_hash, + // In order to grab the runtime code and perform the call network request, we need + // to know the state trie root hash and the height of the block. + let (state_trie_root_hash, block_number) = self + .state_trie_root_hash(block_hash) + .await + .map_err(RuntimeCallError::FindStorageRootHashError)?; + + // Download the runtime of this block. This takes a long time as the runtime is rather + // big (around 1MiB in general). + let (storage_code, storage_heap_pages) = { + let entries = self + .sync_service + .clone() + .storage_query( block_number, - state_trie_root_hash, + block_hash, + &state_trie_root_hash, + [ + sync_service::StorageRequestItem { + key: b":code".to_vec(), + ty: sync_service::StorageRequestItemTy::Value, + }, + sync_service::StorageRequestItem { + key: b":heappages".to_vec(), + ty: sync_service::StorageRequestItemTy::Value, + }, + ] + .into_iter(), + 3, + Duration::from_secs(20), + NonZeroU32::new(1).unwrap(), ) - .await; + .await + .map_err(runtime_service::RuntimeCallError::StorageQuery) + .map_err(RuntimeCallError::Call)?; + // TODO: not elegant + let heap_pages = entries + .iter() + .find_map(|entry| match entry { + sync_service::StorageResultItem::Value { key, value } + if key == b":heappages" => + { + Some(value.clone()) // TODO: overhead + } + _ => None, + }) + .unwrap(); + let code = entries + .iter() + .find_map(|entry| match entry { + sync_service::StorageResultItem::Value { key, value } if key == b":code" => { + Some(value.clone()) // TODO: overhead + } + _ => None, + }) + .unwrap(); + (code, heap_pages) + }; - // TODO: consider keeping pinned runtimes in a cache instead - self.runtime_service.unpin_runtime(pinned_runtime_id).await; + // Give the code and heap pages to the runtime service. The runtime service will + // try to find any similar runtime it might have, and if not will compile it. + let pinned_runtime_id = self + .runtime_service + .compile_and_pin_runtime(storage_code, storage_heap_pages) + .await; + + let precall = self + .runtime_service + .pinned_runtime_access( + pinned_runtime_id.clone(), + *block_hash, + block_number, + state_trie_root_hash, + ) + .await; - precall - }) + // TODO: consider keeping pinned runtimes in a cache instead + self.runtime_service.unpin_runtime(pinned_runtime_id).await; + + Ok(precall) } /// Performs a runtime call to a random block. diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index abdb6b14b2..04586f6220 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -19,6 +19,7 @@ use core::num::NonZeroUsize; use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc}; use async_lock::Mutex; +use futures_channel::oneshot; use futures_lite::{FutureExt as _, StreamExt as _}; use futures_util::{future, stream::AbortRegistration, FutureExt as _}; use smoldot::{ @@ -31,9 +32,15 @@ use crate::{platform::PlatformRef, runtime_service}; use super::Cache; -pub(super) enum Message { +pub(super) enum Message { SubscriptionStart(service::SubscriptionStartProcess), - SubscriptionDestroyed { subscription_id: String }, + SubscriptionDestroyed { + subscription_id: String, + }, + RecentBlockRuntimeAccess { + block_hash: [u8; 32], + result_tx: oneshot::Sender>>, + }, } // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. @@ -42,7 +49,7 @@ pub(super) fn start_task( platform: TPlat, log_target: String, runtime_service: Arc>, - requests_rx: async_channel::Receiver, + requests_rx: async_channel::Receiver>, abort_registration: AbortRegistration, ) { // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache @@ -102,7 +109,7 @@ pub(super) fn start_task( run(Task { cache: cache.clone(), log_target: log_target.clone(), - block_number_bytes: runtime_service.block_number_bytes(), + runtime_service, new_blocks: subscribe_all.new_blocks, requests_rx, // TODO: all the subscriptions are dropped if the task returns @@ -130,9 +137,9 @@ pub(super) fn start_task( struct Task { cache: Arc>, log_target: String, - block_number_bytes: usize, + runtime_service: Arc>, new_blocks: runtime_service::Subscription, - requests_rx: async_channel::Receiver, + requests_rx: async_channel::Receiver>, // TODO: shrink_to_fit? all_heads_subscriptions: hashbrown::HashMap, // TODO: shrink_to_fit? @@ -158,7 +165,7 @@ async fn run(mut task: Task) { let json_rpc_header = match methods::Header::from_scale_encoded_header( &block.scale_encoded_header, - task.block_number_bytes, + task.runtime_service.block_number_bytes(), ) { Ok(h) => h, Err(error) => { @@ -232,6 +239,28 @@ async fn run(mut task: Task) { // TODO: shrink_to_fit? } + either::Right(Some(Message::RecentBlockRuntimeAccess { + block_hash, + result_tx, + })) => { + let cache_lock = task.cache.lock().await; + let access = if cache_lock.recent_pinned_blocks.contains(&block_hash) { + // The runtime service has the block pinned, meaning that we can ask the runtime + // service to perform the call. + task.runtime_service + .pinned_block_runtime_access( + cache_lock.subscription_id.unwrap(), + &block_hash, + ) + .await + .ok() + } else { + None + }; + + let _ = result_tx.send(access); + } + either::Left(None) | either::Right(None) => { break; } diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index ad3ccfd637..227a1722bc 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -824,8 +824,6 @@ impl Background { &sub_utils::subscribe_best(&self.runtime_service).await.0, ); - let cache = self.cache.lock().await; - let at = at.as_ref().map(|h| h.0).unwrap_or(best_block); let mut out = methods::StorageChangeSet { @@ -833,8 +831,6 @@ impl Background { changes: Vec::new(), }; - drop(cache); - let fut = self.storage_query( keys.iter(), &at, From 7e6084e6d01b7d71896d3cdc1455894456d0122c Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 14:01:15 +0200 Subject: [PATCH 09/46] Query block number from cache --- .../background/legacy_state_sub.rs | 33 +++++++++++++++++++ .../background/state_chain.rs | 32 ++++++++---------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 04586f6220..95053e2009 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -41,6 +41,10 @@ pub(super) enum Message { block_hash: [u8; 32], result_tx: oneshot::Sender>>, }, + BlockNumber { + block_hash: [u8; 32], + result_tx: oneshot::Sender>, + }, } // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. @@ -261,6 +265,35 @@ async fn run(mut task: Task) { let _ = result_tx.send(access); } + either::Right(Some(Message::BlockNumber { + block_hash, + result_tx, + })) => { + let mut cache_lock = task.cache.lock().await; + let cache_lock = &mut *cache_lock; + + if let Some(future) = cache_lock + .block_state_root_hashes_numbers + .get_mut(&block_hash) + { + let _ = future.now_or_never(); + } + + let block_number = match ( + cache_lock + .recent_pinned_blocks + .get(&block_hash) + .map(|h| header::decode(h, task.runtime_service.block_number_bytes())), + cache_lock.block_state_root_hashes_numbers.get(&block_hash), + ) { + (Some(Ok(header)), _) => Some(header.number), + (_, Some(future::MaybeDone::Done(Ok((_, num))))) => Some(*num), + _ => None, + }; + + let _ = result_tx.send(block_number); + } + either::Left(None) | either::Right(None) => { break; } diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 227a1722bc..bd27c46dc6 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -17,13 +17,14 @@ //! All legacy JSON-RPC method handlers that relate to the chain or the storage. -use super::{Background, GetKeysPagedCacheKey, PlatformRef}; +use super::{legacy_state_sub, Background, GetKeysPagedCacheKey, PlatformRef}; use crate::sync_service; use alloc::{borrow::ToOwned as _, format, string::ToString as _, sync::Arc, vec, vec::Vec}; use async_lock::MutexGuard; use core::{iter, num::NonZeroU32, pin, time::Duration}; +use futures_channel::oneshot; use futures_util::{future, stream, FutureExt as _, StreamExt as _}; use smoldot::{ header, @@ -100,24 +101,17 @@ impl Background { // knowing it will lead to a better selection of peers, and thus increase the chances of // the requests succeeding. let block_number = { - let mut cache_lock = self.cache.lock().await; - let cache_lock = &mut *cache_lock; - - if let Some(future) = cache_lock.block_state_root_hashes_numbers.get_mut(&hash) { - let _ = future.now_or_never(); - } - - match ( - cache_lock - .recent_pinned_blocks - .get(&hash) - .map(|h| header::decode(h, self.sync_service.block_number_bytes())), - cache_lock.block_state_root_hashes_numbers.get(&hash), - ) { - (Some(Ok(header)), _) => Some(header.number), - (_, Some(future::MaybeDone::Done(Ok((_, num))))) => Some(*num), - _ => None, - } + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockNumber { + block_hash: hash, + result_tx: tx, + }) + .await + .unwrap(); + rx.await.unwrap() }; // Block bodies and justifications aren't stored locally. Ask the network. From 7e9a4c49798bc53c2bd4b6861010071d944b8bd0 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 14:54:00 +0200 Subject: [PATCH 10/46] Ask block header from cache through a message --- .../background/legacy_state_sub.rs | 21 +++++++- .../background/state_chain.rs | 48 +++++++++++-------- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 95053e2009..6f1f993021 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -17,7 +17,7 @@ use core::num::NonZeroUsize; -use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc}; +use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc, vec::Vec}; use async_lock::Mutex; use futures_channel::oneshot; use futures_lite::{FutureExt as _, StreamExt as _}; @@ -45,6 +45,10 @@ pub(super) enum Message { block_hash: [u8; 32], result_tx: oneshot::Sender>, }, + BlockHeader { + block_hash: [u8; 32], + result_tx: oneshot::Sender>>, + }, } // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. @@ -294,6 +298,21 @@ async fn run(mut task: Task) { let _ = result_tx.send(block_number); } + either::Right(Some(Message::BlockHeader { + block_hash, + result_tx, + })) => { + let mut cache_lock = task.cache.lock().await; + let header = if let Some(header) = cache_lock.recent_pinned_blocks.get(&block_hash) + { + Some(header.clone()) + } else { + None + }; + + let _ = result_tx.send(header); + } + either::Left(None) | either::Right(None) => { break; } diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index bd27c46dc6..98bf49c617 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -22,10 +22,9 @@ use super::{legacy_state_sub, Background, GetKeysPagedCacheKey, PlatformRef}; use crate::sync_service; use alloc::{borrow::ToOwned as _, format, string::ToString as _, sync::Arc, vec, vec::Vec}; -use async_lock::MutexGuard; use core::{iter, num::NonZeroU32, pin, time::Duration}; use futures_channel::oneshot; -use futures_util::{future, stream, FutureExt as _, StreamExt as _}; +use futures_util::{future, stream, StreamExt as _}; use smoldot::{ header, informant::HashDisplay, @@ -221,31 +220,42 @@ impl Background { // Try to look in the cache of recent blocks. If not found, ask the peer-to-peer network. // `header` is `Err` if and only if the network request failed. let scale_encoded_header = { - let mut cache_lock = self.cache.lock().await; - if let Some(header) = cache_lock.recent_pinned_blocks.get(&hash) { - Ok(header.clone()) + let from_cache = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockHeader { + block_hash: hash, + result_tx: tx, + }) + .await + .unwrap(); + rx.await.unwrap() + }; + + if let Some(header) = from_cache { + Ok(header) } else { // Header isn't known locally. We need to ask the network. // First, try to determine the block number by looking into the cache. // The request can be fulfilled no matter whether it is found, but knowing it will // lead to a better selection of peers, and thus increase the chances of the // requests succeeding. - let block_number = if let Some(future) = - cache_lock.block_state_root_hashes_numbers.get_mut(&hash) - { - let _ = future.now_or_never(); - - match future { - future::MaybeDone::Done(Ok((_, num))) => Some(*num), - _ => None, - } - } else { - None + let block_number = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockNumber { + block_hash: hash, + result_tx: tx, + }) + .await + .unwrap(); + rx.await.unwrap() }; - // Release the lock as we're going to start a long asynchronous operation. - drop::>(cache_lock); - // Actual network query. let result = if let Some(block_number) = block_number { self.sync_service From 532cb53f824ddd3b344c3301d2327efeca0dd567 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 17:14:11 +0200 Subject: [PATCH 11/46] Ask for the block state root and number by sending a message --- light-base/src/json_rpc_service/background.rs | 138 +++++------------- .../background/legacy_state_sub.rs | 131 ++++++++++++++++- .../background/state_chain.rs | 58 ++++++-- 3 files changed, 209 insertions(+), 118 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 01d6ad98ce..b445b3269f 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -315,6 +315,7 @@ pub(super) fn start( me.cache.clone(), me.platform.clone(), me.log_target.clone(), + me.sync_service.clone(), me.runtime_service.clone(), to_legacy_rx, background_abort_registrations.next().unwrap(), @@ -796,97 +797,6 @@ impl Background { } } - /// Obtain the state trie root hash and number of the given block, and make sure to put it - /// in cache. - async fn state_trie_root_hash( - &self, - hash: &[u8; 32], - ) -> Result<([u8; 32], u64), StateTrieRootHashError> { - let fetch = { - // Try to find an existing entry in cache, and if not create one. - let mut cache_lock = self.cache.lock().await; - - // Look in `recent_pinned_blocks`. - match cache_lock - .recent_pinned_blocks - .get(hash) - .map(|h| header::decode(h, self.sync_service.block_number_bytes())) - { - Some(Ok(header)) => return Ok((*header.state_root, header.number)), - Some(Err(err)) => return Err(StateTrieRootHashError::HeaderDecodeError(err)), // TODO: can this actually happen? unclear - None => {} - } - - // Look in `block_state_root_hashes`. - match cache_lock.block_state_root_hashes_numbers.get(hash) { - Some(future::MaybeDone::Done(Ok(val))) => return Ok(*val), - Some(future::MaybeDone::Future(f)) => f.clone(), - Some(future::MaybeDone::Gone) => unreachable!(), // We never use `Gone`. - Some(future::MaybeDone::Done(Err( - err @ StateTrieRootHashError::HeaderDecodeError(_), - ))) => { - // In case of a fatal error, return immediately. - return Err(err.clone()); - } - Some(future::MaybeDone::Done(Err(StateTrieRootHashError::NetworkQueryError))) - | None => { - // No existing cache entry. Create the future that will perform the fetch - // but do not actually start doing anything now. - let fetch = { - let sync_service = self.sync_service.clone(); - let hash = *hash; - async move { - // The sync service knows which peers are potentially aware of - // this block. - let result = sync_service - .clone() - .block_query_unknown_number( - hash, - protocol::BlocksRequestFields { - header: true, - body: false, - justifications: false, - }, - 4, - Duration::from_secs(8), - NonZeroU32::new(2).unwrap(), - ) - .await; - - if let Ok(block) = result { - // If successful, the `block_query` function guarantees that the - // header is present and valid. - let header = block.header.unwrap(); - debug_assert_eq!( - header::hash_from_scale_encoded_header(&header), - hash - ); - let decoded = - header::decode(&header, sync_service.block_number_bytes()) - .unwrap(); - Ok((*decoded.state_root, decoded.number)) - } else { - // TODO: better error details? - Err(StateTrieRootHashError::NetworkQueryError) - } - } - }; - - // Insert the future in the cache, so that any other call will use the same - // future. - let wrapped = fetch.boxed().shared(); - cache_lock - .block_state_root_hashes_numbers - .put(*hash, future::maybe_done(wrapped.clone())); - wrapped - } - } - }; - - // We await separately to be certain that the lock isn't held anymore. - fetch.await - } - async fn storage_query( &self, keys: impl Iterator + Clone> + Clone, @@ -895,10 +805,25 @@ impl Background { timeout_per_request: Duration, max_parallel: NonZeroU32, ) -> Result>>, StorageQueryError> { - let (state_trie_root_hash, block_number) = self - .state_trie_root_hash(hash) - .await - .map_err(StorageQueryError::FindStorageRootHashError)?; + let (state_trie_root_hash, block_number) = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockStateRootAndNumber { + block_hash: *hash, + result_tx: tx, + }) + .await + .unwrap(); + + match rx.await.unwrap() { + Ok(v) => v, + Err(err) => { + return Err(StorageQueryError::FindStorageRootHashError(err)); + } + } + }; let result = self .sync_service @@ -968,10 +893,25 @@ impl Background { // In order to grab the runtime code and perform the call network request, we need // to know the state trie root hash and the height of the block. - let (state_trie_root_hash, block_number) = self - .state_trie_root_hash(block_hash) - .await - .map_err(RuntimeCallError::FindStorageRootHashError)?; + let (state_trie_root_hash, block_number) = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockStateRootAndNumber { + block_hash: *block_hash, + result_tx: tx, + }) + .await + .unwrap(); + + match rx.await.unwrap() { + Ok(v) => v, + Err(err) => { + return Err(RuntimeCallError::FindStorageRootHashError(err)); + } + } + }; // Download the runtime of this block. This takes a long time as the runtime is rather // big (around 1MiB in general). diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 6f1f993021..ba277c75b1 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -15,7 +15,12 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use core::num::NonZeroUsize; +use core::{ + future::Future, + num::{NonZeroU32, NonZeroUsize}, + pin::Pin, + time::Duration, +}; use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc, vec::Vec}; use async_lock::Mutex; @@ -26,11 +31,12 @@ use smoldot::{ header, informant::HashDisplay, json_rpc::{methods, service}, + network::protocol, }; -use crate::{platform::PlatformRef, runtime_service}; +use crate::{platform::PlatformRef, runtime_service, sync_service}; -use super::Cache; +use super::{Cache, StateTrieRootHashError}; pub(super) enum Message { SubscriptionStart(service::SubscriptionStartProcess), @@ -41,6 +47,10 @@ pub(super) enum Message { block_hash: [u8; 32], result_tx: oneshot::Sender>>, }, + BlockStateRootAndNumber { + block_hash: [u8; 32], + result_tx: oneshot::Sender>, + }, BlockNumber { block_hash: [u8; 32], result_tx: oneshot::Sender>, @@ -56,6 +66,7 @@ pub(super) fn start_task( cache: Arc>, platform: TPlat, log_target: String, + sync_service: Arc>, runtime_service: Arc>, requests_rx: async_channel::Receiver>, abort_registration: AbortRegistration, @@ -117,6 +128,8 @@ pub(super) fn start_task( run(Task { cache: cache.clone(), log_target: log_target.clone(), + platform: platform.clone(), + sync_service, runtime_service, new_blocks: subscribe_all.new_blocks, requests_rx, @@ -145,6 +158,8 @@ pub(super) fn start_task( struct Task { cache: Arc>, log_target: String, + platform: TPlat, + sync_service: Arc>, runtime_service: Arc>, new_blocks: runtime_service::Subscription, requests_rx: async_channel::Receiver>, @@ -313,6 +328,116 @@ async fn run(mut task: Task) { let _ = result_tx.send(header); } + either::Right(Some(Message::BlockStateRootAndNumber { + block_hash, + result_tx, + })) => { + let fetch = { + // Try to find an existing entry in cache, and if not create one. + let mut cache_lock = task.cache.lock().await; + + // Look in `recent_pinned_blocks`. + match cache_lock + .recent_pinned_blocks + .get(&block_hash) + .map(|h| header::decode(h, task.runtime_service.block_number_bytes())) + { + Some(Ok(header)) => { + let _ = result_tx.send(Ok((*header.state_root, header.number))); + continue; + } + Some(Err(err)) => { + let _ = + result_tx.send(Err(StateTrieRootHashError::HeaderDecodeError(err))); + continue; + } // TODO: can this actually happen? unclear + None => {} + } + + // Look in `block_state_root_hashes`. + match cache_lock.block_state_root_hashes_numbers.get(&block_hash) { + Some(future::MaybeDone::Done(Ok(val))) => { + let _ = result_tx.send(Ok(*val)); + continue; + } + Some(future::MaybeDone::Future(f)) => f.clone(), + Some(future::MaybeDone::Gone) => unreachable!(), // We never use `Gone`. + Some(future::MaybeDone::Done(Err( + err @ StateTrieRootHashError::HeaderDecodeError(_), + ))) => { + // In case of a fatal error, return immediately. + let _ = result_tx.send(Err(err.clone())); + continue; + } + Some(future::MaybeDone::Done(Err( + StateTrieRootHashError::NetworkQueryError, + ))) + | None => { + // No existing cache entry. Create the future that will perform the fetch + // but do not actually start doing anything now. + let fetch = { + let sync_service = task.sync_service.clone(); + async move { + // The sync service knows which peers are potentially aware of + // this block. + let result = sync_service + .clone() + .block_query_unknown_number( + block_hash, + protocol::BlocksRequestFields { + header: true, + body: false, + justifications: false, + }, + 4, + Duration::from_secs(8), + NonZeroU32::new(2).unwrap(), + ) + .await; + + if let Ok(block) = result { + // If successful, the `block_query` function guarantees that the + // header is present and valid. + let header = block.header.unwrap(); + debug_assert_eq!( + header::hash_from_scale_encoded_header(&header), + block_hash + ); + let decoded = header::decode( + &header, + sync_service.block_number_bytes(), + ) + .unwrap(); + Ok((*decoded.state_root, decoded.number)) + } else { + // TODO: better error details? + Err(StateTrieRootHashError::NetworkQueryError) + } + } + }; + + // Insert the future in the cache, so that any other call will use the same + // future. + let wrapped = (Box::pin(fetch) + as Pin + Send>>) + .shared(); + cache_lock + .block_state_root_hashes_numbers + .put(block_hash, future::maybe_done(wrapped.clone())); + wrapped + } + } + }; + + // We await separately to be certain that the lock isn't held anymore. + // TODO: crappy design + task.platform + .spawn_task("dummy-adapter".into(), async move { + let outcome = fetch.await; + let _ = result_tx.send(outcome); + }); + } + either::Left(None) | either::Right(None) => { break; } diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 98bf49c617..b126f3b574 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -517,14 +517,27 @@ impl Background { // Obtain the state trie root and height of the requested block. // This is necessary to perform network storage queries. - let (state_root, block_number) = match self.state_trie_root_hash(&hash).await { - Ok(v) => v, - Err(err) => { - request.fail(json_rpc::parse::ErrorResponse::ServerError( - -32000, - &format!("Failed to fetch block information: {err}"), - )); - return; + let (state_root, block_number) = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockStateRootAndNumber { + block_hash: hash, + result_tx: tx, + }) + .await + .unwrap(); + + match rx.await.unwrap() { + Ok(v) => v, + Err(err) => { + request.fail(json_rpc::parse::ErrorResponse::ServerError( + -32000, + &format!("Failed to fetch block information: {err}"), + )); + return; + } } }; @@ -615,14 +628,27 @@ impl Background { // Obtain the state trie root and height of the requested block. // This is necessary to perform network storage queries. - let (state_root, block_number) = match self.state_trie_root_hash(&hash).await { - Ok(v) => v, - Err(err) => { - request.fail(json_rpc::parse::ErrorResponse::ServerError( - -32000, - &format!("Failed to fetch block information: {err}"), - )); - return; + let (state_root, block_number) = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::BlockStateRootAndNumber { + block_hash: hash, + result_tx: tx, + }) + .await + .unwrap(); + + match rx.await.unwrap() { + Ok(v) => v, + Err(err) => { + request.fail(json_rpc::parse::ErrorResponse::ServerError( + -32000, + &format!("Failed to fetch block information: {err}"), + )); + return; + } } }; From 173ca4fbcdce582495c5f58189f41e19d51ef3c0 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 18:12:57 +0200 Subject: [PATCH 12/46] Move `state_get_keys_paged` as a separate field --- light-base/src/json_rpc_service/background.rs | 21 ++++++++++--------- .../background/state_chain.rs | 6 ++---- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index b445b3269f..7637fe995d 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -94,6 +94,13 @@ struct Background { /// requests to perform. cache: Arc>, + /// When `state_getKeysPaged` is called and the response is truncated, the response is + /// inserted in this cache. The API user is likely to call `state_getKeysPaged` again with + /// the same parameters, in which case we hit the cache and avoid the networking requests. + /// The values are list of keys. + state_get_keys_paged_cache: + Mutex>, util::SipHasherBuild>>, + /// Hash of the genesis block. /// Keeping the genesis block is important, as the genesis block hash is included in /// transaction signatures, and must therefore be queried by upper-level UIs. @@ -157,12 +164,6 @@ struct Cache { >, fnv::FnvBuildHasher, >, - - /// When `state_getKeysPaged` is called and the response is truncated, the response is - /// inserted in this cache. The API user is likely to call `state_getKeysPaged` again with - /// the same parameters, in which case we hit the cache and avoid the networking requests. - /// The values are list of keys. - state_get_keys_paged: lru::LruCache>, util::SipHasherBuild>, } /// See [`Cache::state_get_keys_paged`]. @@ -208,11 +209,11 @@ pub(super) fn start( NonZeroUsize::new(32).unwrap(), Default::default(), ), - state_get_keys_paged: lru::LruCache::with_hasher( - NonZeroUsize::new(2).unwrap(), - util::SipHasherBuild::new(rand::random()), - ), })), + state_get_keys_paged_cache: Mutex::new(lru::LruCache::with_hasher( + NonZeroUsize::new(2).unwrap(), + util::SipHasherBuild::new(rand::random()), + )), genesis_block_hash: config.genesis_block_hash, printed_legacy_json_rpc_warning: atomic::AtomicBool::new(false), chain_head_follow_tasks: Mutex::new(hashbrown::HashMap::with_hasher(Default::default())), diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index b126f3b574..449eff25f1 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -605,10 +605,9 @@ impl Background { // same parameters, we store the untruncated responses in a cache. Check if we hit the // cache. if let Some(keys) = - self.cache + self.state_get_keys_paged_cache .lock() .await - .state_get_keys_paged .get(&GetKeysPagedCacheKey { hash, prefix: prefix.clone(), @@ -692,10 +691,9 @@ impl Background { // JSON-RPC client will call the function again with the exact same parameters. // Thus, store the results in a cache. if out.len() != keys.len() { - self.cache + self.state_get_keys_paged_cache .lock() .await - .state_get_keys_paged .push(GetKeysPagedCacheKey { hash, prefix }, keys); } From cf756d34b9555b67ec40896a9a17ea2c4a00f56e Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 18:17:13 +0200 Subject: [PATCH 13/46] The `Cache` is now scoped to `legacy_state_sub` --- light-base/src/json_rpc_service/background.rs | 59 --------- .../background/legacy_state_sub.rs | 118 ++++++++++++------ 2 files changed, 81 insertions(+), 96 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 7637fe995d..42334a9952 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -90,10 +90,6 @@ struct Background { /// a dedicated task. to_legacy: Mutex>>, - /// Various information caches about blocks, to potentially reduce the number of network - /// requests to perform. - cache: Arc>, - /// When `state_getKeysPaged` is called and the response is truncated, the response is /// inserted in this cache. The API user is likely to call `state_getKeysPaged` again with /// the same parameters, in which case we hit the cache and avoid the networking requests. @@ -123,49 +119,6 @@ struct Background { >, } -struct Cache { - /// When the runtime service reports a new block, it is kept pinned and inserted in this LRU - /// cache. When an entry in removed from the cache, it is unpinned. - /// - /// JSON-RPC clients are more likely to ask for information about recent blocks and perform - /// calls on them, hence a cache of recent blocks. - recent_pinned_blocks: lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, - - /// Subscription on the runtime service under which the blocks of - /// [`Cache::recent_pinned_blocks`] are pinned. - /// - /// Contains `None` only at initialization, in which case [`Cache::recent_pinned_blocks`] - /// is guaranteed to be empty. In other words, if a block is found in - /// [`Cache::recent_pinned_blocks`] then this field is guaranteed to be `Some`. - subscription_id: Option, - - /// State trie root hashes and numbers of blocks that were not in - /// [`Cache::recent_pinned_blocks`]. - /// - /// The state trie root hash can also be an `Err` if the network request failed or if the - /// header is of an invalid format. - /// - /// The state trie root hash and number are wrapped in a `Shared` future. When multiple - /// requests need the state trie root hash and number of the same block, they are only queried - /// once and the query is inserted in the cache while in progress. This way, the multiple - /// requests can all wait on that single future. - /// - /// Most of the time, the JSON-RPC client will query blocks that are found in - /// [`Cache::recent_pinned_blocks`], but occasionally it will query older blocks. When the - /// storage of an older block is queried, it is common for the JSON-RPC client to make several - /// storage requests to that same old block. In order to avoid having to retrieve the state - /// trie root hash multiple, we store these hashes in this LRU cache. - block_state_root_hashes_numbers: lru::LruCache< - [u8; 32], - future::MaybeDone< - future::Shared< - future::BoxFuture<'static, Result<([u8; 32], u64), StateTrieRootHashError>>, - >, - >, - fnv::FnvBuildHasher, - >, -} - /// See [`Cache::state_get_keys_paged`]. #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct GetKeysPagedCacheKey { @@ -199,17 +152,6 @@ pub(super) fn start( runtime_service: config.runtime_service.clone(), transactions_service: config.transactions_service.clone(), to_legacy: Mutex::new(to_legacy_tx), - cache: Arc::new(Mutex::new(Cache { - recent_pinned_blocks: lru::LruCache::with_hasher( - NonZeroUsize::new(32).unwrap(), - Default::default(), - ), - subscription_id: None, - block_state_root_hashes_numbers: lru::LruCache::with_hasher( - NonZeroUsize::new(32).unwrap(), - Default::default(), - ), - })), state_get_keys_paged_cache: Mutex::new(lru::LruCache::with_hasher( NonZeroUsize::new(2).unwrap(), util::SipHasherBuild::new(rand::random()), @@ -313,7 +255,6 @@ pub(super) fn start( } legacy_state_sub::start_task( - me.cache.clone(), me.platform.clone(), me.log_target.clone(), me.sync_service.clone(), diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index ba277c75b1..153458cc68 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -36,7 +36,7 @@ use smoldot::{ use crate::{platform::PlatformRef, runtime_service, sync_service}; -use super::{Cache, StateTrieRootHashError}; +use super::StateTrieRootHashError; pub(super) enum Message { SubscriptionStart(service::SubscriptionStartProcess), @@ -63,7 +63,6 @@ pub(super) enum Message { // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. pub(super) fn start_task( - cache: Arc>, platform: TPlat, log_target: String, sync_service: Arc>, @@ -79,7 +78,17 @@ pub(super) fn start_task( future::Abortable::new( async move { loop { - let mut cache_lock = cache.lock().await; + let mut cache = Cache { + recent_pinned_blocks: lru::LruCache::with_hasher( + NonZeroUsize::new(32).unwrap(), + Default::default(), + ), + subscription_id: None, + block_state_root_hashes_numbers: lru::LruCache::with_hasher( + NonZeroUsize::new(32).unwrap(), + Default::default(), + ), + }; // Subscribe to new runtime service blocks in order to push them in the // cache as soon as they are available. @@ -96,37 +105,35 @@ pub(super) fn start_task( ) .await; - cache_lock.subscription_id = Some(subscribe_all.new_blocks.id()); - cache_lock.recent_pinned_blocks.clear(); - debug_assert!(cache_lock.recent_pinned_blocks.cap().get() >= 1); + cache.subscription_id = Some(subscribe_all.new_blocks.id()); + cache.recent_pinned_blocks.clear(); + debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); let finalized_block_hash = header::hash_from_scale_encoded_header( &subscribe_all.finalized_block_scale_encoded_header, ); - cache_lock.recent_pinned_blocks.put( + cache.recent_pinned_blocks.put( finalized_block_hash, subscribe_all.finalized_block_scale_encoded_header, ); for block in subscribe_all.non_finalized_blocks_ancestry_order { - if cache_lock.recent_pinned_blocks.len() - == cache_lock.recent_pinned_blocks.cap().get() + if cache.recent_pinned_blocks.len() + == cache.recent_pinned_blocks.cap().get() { - let (hash, _) = cache_lock.recent_pinned_blocks.pop_lru().unwrap(); + let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); subscribe_all.new_blocks.unpin_block(&hash).await; } let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - cache_lock + cache .recent_pinned_blocks .put(hash, block.scale_encoded_header); } - drop(cache_lock); - run(Task { - cache: cache.clone(), + cache, log_target: log_target.clone(), platform: platform.clone(), sync_service, @@ -155,8 +162,51 @@ pub(super) fn start_task( ); } +struct Cache { + /// When the runtime service reports a new block, it is kept pinned and inserted in this LRU + /// cache. When an entry in removed from the cache, it is unpinned. + /// + /// JSON-RPC clients are more likely to ask for information about recent blocks and perform + /// calls on them, hence a cache of recent blocks. + recent_pinned_blocks: lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, + + /// Subscription on the runtime service under which the blocks of + /// [`Cache::recent_pinned_blocks`] are pinned. + /// + /// Contains `None` only at initialization, in which case [`Cache::recent_pinned_blocks`] + /// is guaranteed to be empty. In other words, if a block is found in + /// [`Cache::recent_pinned_blocks`] then this field is guaranteed to be `Some`. + subscription_id: Option, + + /// State trie root hashes and numbers of blocks that were not in + /// [`Cache::recent_pinned_blocks`]. + /// + /// The state trie root hash can also be an `Err` if the network request failed or if the + /// header is of an invalid format. + /// + /// The state trie root hash and number are wrapped in a `Shared` future. When multiple + /// requests need the state trie root hash and number of the same block, they are only queried + /// once and the query is inserted in the cache while in progress. This way, the multiple + /// requests can all wait on that single future. + /// + /// Most of the time, the JSON-RPC client will query blocks that are found in + /// [`Cache::recent_pinned_blocks`], but occasionally it will query older blocks. When the + /// storage of an older block is queried, it is common for the JSON-RPC client to make several + /// storage requests to that same old block. In order to avoid having to retrieve the state + /// trie root hash multiple, we store these hashes in this LRU cache. + block_state_root_hashes_numbers: lru::LruCache< + [u8; 32], + future::MaybeDone< + future::Shared< + future::BoxFuture<'static, Result<([u8; 32], u64), StateTrieRootHashError>>, + >, + >, + fnv::FnvBuildHasher, + >, +} + struct Task { - cache: Arc>, + cache: Cache, log_target: String, platform: TPlat, sync_service: Arc>, @@ -179,10 +229,10 @@ async fn run(mut task: Task) { .await { either::Left(Some(runtime_service::Notification::Block(block))) => { - let mut cache = task.cache.lock().await; - - if cache.recent_pinned_blocks.len() == cache.recent_pinned_blocks.cap().get() { - let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); + if task.cache.recent_pinned_blocks.len() + == task.cache.recent_pinned_blocks.cap().get() + { + let (hash, _) = task.cache.recent_pinned_blocks.pop_lru().unwrap(); task.new_blocks.unpin_block(&hash).await; } @@ -206,12 +256,10 @@ async fn run(mut task: Task) { }; let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - cache + task.cache .recent_pinned_blocks .put(hash, block.scale_encoded_header); - drop(cache); - for (subscription_id, subscription) in &mut task.all_heads_subscriptions { subscription .send_notification(methods::ServerToClient::chain_allHead { @@ -266,13 +314,12 @@ async fn run(mut task: Task) { block_hash, result_tx, })) => { - let cache_lock = task.cache.lock().await; - let access = if cache_lock.recent_pinned_blocks.contains(&block_hash) { + let access = if task.cache.recent_pinned_blocks.contains(&block_hash) { // The runtime service has the block pinned, meaning that we can ask the runtime // service to perform the call. task.runtime_service .pinned_block_runtime_access( - cache_lock.subscription_id.unwrap(), + task.cache.subscription_id.unwrap(), &block_hash, ) .await @@ -288,10 +335,8 @@ async fn run(mut task: Task) { block_hash, result_tx, })) => { - let mut cache_lock = task.cache.lock().await; - let cache_lock = &mut *cache_lock; - - if let Some(future) = cache_lock + if let Some(future) = task + .cache .block_state_root_hashes_numbers .get_mut(&block_hash) { @@ -299,11 +344,11 @@ async fn run(mut task: Task) { } let block_number = match ( - cache_lock + task.cache .recent_pinned_blocks .get(&block_hash) .map(|h| header::decode(h, task.runtime_service.block_number_bytes())), - cache_lock.block_state_root_hashes_numbers.get(&block_hash), + task.cache.block_state_root_hashes_numbers.get(&block_hash), ) { (Some(Ok(header)), _) => Some(header.number), (_, Some(future::MaybeDone::Done(Ok((_, num))))) => Some(*num), @@ -317,8 +362,7 @@ async fn run(mut task: Task) { block_hash, result_tx, })) => { - let mut cache_lock = task.cache.lock().await; - let header = if let Some(header) = cache_lock.recent_pinned_blocks.get(&block_hash) + let header = if let Some(header) = task.cache.recent_pinned_blocks.get(&block_hash) { Some(header.clone()) } else { @@ -334,10 +378,10 @@ async fn run(mut task: Task) { })) => { let fetch = { // Try to find an existing entry in cache, and if not create one. - let mut cache_lock = task.cache.lock().await; // Look in `recent_pinned_blocks`. - match cache_lock + match task + .cache .recent_pinned_blocks .get(&block_hash) .map(|h| header::decode(h, task.runtime_service.block_number_bytes())) @@ -355,7 +399,7 @@ async fn run(mut task: Task) { } // Look in `block_state_root_hashes`. - match cache_lock.block_state_root_hashes_numbers.get(&block_hash) { + match task.cache.block_state_root_hashes_numbers.get(&block_hash) { Some(future::MaybeDone::Done(Ok(val))) => { let _ = result_tx.send(Ok(*val)); continue; @@ -421,7 +465,7 @@ async fn run(mut task: Task) { let wrapped = (Box::pin(fetch) as Pin + Send>>) .shared(); - cache_lock + task.cache .block_state_root_hashes_numbers .put(block_hash, future::maybe_done(wrapped.clone())); wrapped From 9d6dd9eb70fdce9266194491b3746f9953eb7867 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 18:18:52 +0200 Subject: [PATCH 14/46] Remove background abort registration system --- light-base/src/json_rpc_service.rs | 40 +---- light-base/src/json_rpc_service/background.rs | 6 - .../background/legacy_state_sub.rs | 154 ++++++++---------- 3 files changed, 73 insertions(+), 127 deletions(-) diff --git a/light-base/src/json_rpc_service.rs b/light-base/src/json_rpc_service.rs index 5304b1a649..0c8591d2a2 100644 --- a/light-base/src/json_rpc_service.rs +++ b/light-base/src/json_rpc_service.rs @@ -43,9 +43,8 @@ use crate::{ network_service, platform::PlatformRef, runtime_service, sync_service, transactions_service, }; -use alloc::{format, string::String, sync::Arc, vec::Vec}; +use alloc::{format, string::String, sync::Arc}; use core::num::{NonZeroU32, NonZeroUsize}; -use futures_util::future; use smoldot::{ chain_spec, json_rpc::{self, service}, @@ -90,21 +89,6 @@ pub struct Config { pub fn service(config: Config) -> (Frontend, ServicePrototype) { let log_target = format!("json-rpc-{}", config.log_name); - // We are later going to spawn a bunch of tasks. Each task is associated with an "abort - // handle" that makes it possible to later abort it. We calculate here the number of handles - // that are necessary. - // This calculation must be in sync with the part of the code that spawns the tasks. Assertions - // are there in order to make sure that this is the case. - let num_handles = 1; // TODO: a bit ridiculous for this to be 1 - - let mut background_aborts = Vec::with_capacity(usize::try_from(num_handles).unwrap()); - let mut background_abort_registrations = Vec::with_capacity(background_aborts.capacity()); - for _ in 0..num_handles { - let (abort, reg) = future::AbortHandle::new_pair(); - background_aborts.push(abort); - background_abort_registrations.push(reg); - } - let (requests_processing_task, requests_responses_io) = service::client_main_task(service::Config { max_active_subscriptions: config.max_subscriptions, @@ -115,11 +99,9 @@ pub fn service(config: Config) -> (Frontend, ServicePrototype) { let frontend = Frontend { log_target: log_target.clone(), requests_responses_io: Arc::new(requests_responses_io), - background_aborts: Arc::from(background_aborts), }; let prototype = ServicePrototype { - background_abort_registrations, log_target, requests_processing_task, max_parallel_requests: config.max_parallel_requests, @@ -143,9 +125,6 @@ pub struct Frontend { /// Target to use when emitting logs. log_target: String, - - /// Handles to abort the background tasks. - background_aborts: Arc<[future::AbortHandle]>, } impl Frontend { @@ -221,18 +200,6 @@ impl Frontend { } } -impl Drop for Frontend { - fn drop(&mut self) { - // Call `abort()` if this was the last instance of the `Arc` (and thus the - // last instance of `Frontend`). - if let Some(background_aborts) = Arc::get_mut(&mut self.background_aborts) { - for background_abort in background_aborts { - background_abort.abort(); - } - } - } -} - /// Prototype for a JSON-RPC service. Must be initialized using [`ServicePrototype::start`]. pub struct ServicePrototype { /// Task processing the requests. @@ -245,10 +212,6 @@ pub struct ServicePrototype { /// Value obtained through [`Config::max_parallel_requests`]. max_parallel_requests: NonZeroU32, - - /// List of abort handles. When tasks are spawned, each handle is associated with a task, so - /// that they can all be aborted. See [`Frontend::background_aborts`]. - background_abort_registrations: Vec, } /// Configuration for a JSON-RPC service. @@ -310,7 +273,6 @@ impl ServicePrototype { config, self.requests_processing_task, self.max_parallel_requests, - self.background_abort_registrations, ) } } diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 42334a9952..a96e3e5425 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -133,7 +133,6 @@ pub(super) fn start( config: StartConfig<'_, TPlat>, mut requests_processing_task: service::ClientMainTask, max_parallel_requests: NonZeroU32, - background_abort_registrations: Vec, ) { let (to_legacy_tx, to_legacy_rx) = async_channel::bounded(8); @@ -161,8 +160,6 @@ pub(super) fn start( chain_head_follow_tasks: Mutex::new(hashbrown::HashMap::with_hasher(Default::default())), }); - let mut background_abort_registrations = background_abort_registrations.into_iter(); - let (tx, rx) = async_channel::bounded( usize::try_from(max_parallel_requests.get()).unwrap_or(usize::max_value()), ); @@ -260,10 +257,7 @@ pub(super) fn start( me.sync_service.clone(), me.runtime_service.clone(), to_legacy_rx, - background_abort_registrations.next().unwrap(), ); - - debug_assert!(background_abort_registrations.next().is_none()); } impl Background { diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 153458cc68..251e84eeb9 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -68,96 +68,86 @@ pub(super) fn start_task( sync_service: Arc>, runtime_service: Arc>, requests_rx: async_channel::Receiver>, - abort_registration: AbortRegistration, ) { // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache // TODO: extract to separate function platform.clone().spawn_task( format!("{}-cache-populate", log_target).into(), - Box::pin({ - future::Abortable::new( - async move { - loop { - let mut cache = Cache { - recent_pinned_blocks: lru::LruCache::with_hasher( - NonZeroUsize::new(32).unwrap(), - Default::default(), - ), - subscription_id: None, - block_state_root_hashes_numbers: lru::LruCache::with_hasher( - NonZeroUsize::new(32).unwrap(), - Default::default(), - ), - }; - - // Subscribe to new runtime service blocks in order to push them in the - // cache as soon as they are available. - // The buffer size should be large enough so that, if the CPU is busy, it - // doesn't become full before the execution of this task resumes. - // The maximum number of pinned block is ignored, as this maximum is a way to - // avoid malicious behaviors. This code is by definition not considered - // malicious. - let subscribe_all = runtime_service - .subscribe_all( - "json-rpc-blocks-cache", - 32, - NonZeroUsize::new(usize::max_value()).unwrap(), - ) - .await; - - cache.subscription_id = Some(subscribe_all.new_blocks.id()); - cache.recent_pinned_blocks.clear(); - debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); - - let finalized_block_hash = header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - ); - cache.recent_pinned_blocks.put( - finalized_block_hash, - subscribe_all.finalized_block_scale_encoded_header, - ); + Box::pin(async move { + loop { + let mut cache = Cache { + recent_pinned_blocks: lru::LruCache::with_hasher( + NonZeroUsize::new(32).unwrap(), + Default::default(), + ), + subscription_id: None, + block_state_root_hashes_numbers: lru::LruCache::with_hasher( + NonZeroUsize::new(32).unwrap(), + Default::default(), + ), + }; - for block in subscribe_all.non_finalized_blocks_ancestry_order { - if cache.recent_pinned_blocks.len() - == cache.recent_pinned_blocks.cap().get() - { - let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); - subscribe_all.new_blocks.unpin_block(&hash).await; - } - - let hash = - header::hash_from_scale_encoded_header(&block.scale_encoded_header); - cache - .recent_pinned_blocks - .put(hash, block.scale_encoded_header); - } + // Subscribe to new runtime service blocks in order to push them in the + // cache as soon as they are available. + // The buffer size should be large enough so that, if the CPU is busy, it + // doesn't become full before the execution of this task resumes. + // The maximum number of pinned block is ignored, as this maximum is a way to + // avoid malicious behaviors. This code is by definition not considered + // malicious. + let subscribe_all = runtime_service + .subscribe_all( + "json-rpc-blocks-cache", + 32, + NonZeroUsize::new(usize::max_value()).unwrap(), + ) + .await; + + cache.subscription_id = Some(subscribe_all.new_blocks.id()); + cache.recent_pinned_blocks.clear(); + debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); + + let finalized_block_hash = header::hash_from_scale_encoded_header( + &subscribe_all.finalized_block_scale_encoded_header, + ); + cache.recent_pinned_blocks.put( + finalized_block_hash, + subscribe_all.finalized_block_scale_encoded_header, + ); + + for block in subscribe_all.non_finalized_blocks_ancestry_order { + if cache.recent_pinned_blocks.len() == cache.recent_pinned_blocks.cap().get() { + let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); + subscribe_all.new_blocks.unpin_block(&hash).await; + } - run(Task { - cache, - log_target: log_target.clone(), - platform: platform.clone(), - sync_service, - runtime_service, - new_blocks: subscribe_all.new_blocks, - requests_rx, - // TODO: all the subscriptions are dropped if the task returns - all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, - Default::default(), - ), - new_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, - Default::default(), - ), - }) - .await; + let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); + cache + .recent_pinned_blocks + .put(hash, block.scale_encoded_header); + } - panic!() // TODO: not implemented correctly - } - }, - abort_registration, - ) - .map(|_: Result<(), _>| ()) + run(Task { + cache, + log_target: log_target.clone(), + platform: platform.clone(), + sync_service, + runtime_service, + new_blocks: subscribe_all.new_blocks, + requests_rx, + // TODO: all the subscriptions are dropped if the task returns + all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), + new_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), + }) + .await; + + panic!() // TODO: not implemented correctly + } }), ); } From c5fe69503706660a4273b70c26f70941a0a70fe4 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 18:20:47 +0200 Subject: [PATCH 15/46] Simplify `Frontend::queue_rpc_request` --- light-base/src/json_rpc_service.rs | 32 ++++++++++-------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/light-base/src/json_rpc_service.rs b/light-base/src/json_rpc_service.rs index 0c8591d2a2..811dabee0b 100644 --- a/light-base/src/json_rpc_service.rs +++ b/light-base/src/json_rpc_service.rs @@ -135,26 +135,6 @@ impl Frontend { /// isn't called often enough. Use [`HandleRpcError::into_json_rpc_error`] to build the /// JSON-RPC response to immediately send back to the user. pub fn queue_rpc_request(&self, json_rpc_request: String) -> Result<(), HandleRpcError> { - // If the request isn't even a valid JSON-RPC request, we can't even send back a response. - // We have no choice but to immediately refuse the request. - if let Err(error) = json_rpc::parse::parse_call(&json_rpc_request) { - log::warn!( - target: &self.log_target, - "Refused malformed JSON-RPC request: {}", error - ); - return Err(HandleRpcError::MalformedJsonRpc(error)); - } - - // Logging the request before it is queued. - log::debug!( - target: &self.log_target, - "PendingRequestsQueue <= {}", - crate::util::truncated_str( - json_rpc_request.chars().filter(|c| !c.is_control()), - 100, - ) - ); - match self .requests_responses_io .try_send_request(json_rpc_request) @@ -167,9 +147,17 @@ impl Frontend { json_rpc_request: request, }), Err(service::TrySendRequestError { - cause: service::TrySendRequestErrorCause::MalformedJson(err), + cause: service::TrySendRequestErrorCause::MalformedJson(error), .. - }) => Err(HandleRpcError::MalformedJsonRpc(err)), + }) => { + // If the request isn't even a valid JSON-RPC request, we can't even send back a + // response. We have no choice but to immediately refuse the request. + log::warn!( + target: &self.log_target, + "Refused malformed JSON-RPC request: {}", error + ); + Err(HandleRpcError::MalformedJsonRpc(error)) + } Err(service::TrySendRequestError { cause: service::TrySendRequestErrorCause::ClientMainTaskDestroyed, .. From ed5ebe4026f6581c5ccc45efd7290639059dc099 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 18:34:10 +0200 Subject: [PATCH 16/46] Remove TODOs and update CHANGELOG --- light-base/src/json_rpc_service/background/legacy_state_sub.rs | 2 -- wasm-node/CHANGELOG.md | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 251e84eeb9..e49dc4539b 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -69,8 +69,6 @@ pub(super) fn start_task( runtime_service: Arc>, requests_rx: async_channel::Receiver>, ) { - // TODO: this is actually racy, as a block subscription task could report a new block to a client, and then client can query it, before this block has been been added to the cache - // TODO: extract to separate function platform.clone().spawn_task( format!("{}-cache-populate", log_target).into(), Box::pin(async move { diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index e2cb91c881..6f6f29f6ec 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -5,6 +5,7 @@ ### Changed - The `chainHead_unstable_storage` JSON-RPC function now supports a `type` equal to `closest-descendant-merkle-value` and no longer supports `closest-ancestor-merkle-value`, in accordance with the latest changes in the JSON-RPC API specification. ([#824](https://github.com/smol-dot/smoldot/pull/824)) +- Blocks are now reported to `chain_subscribeAllHeads` and `chain_subscribeNewHeads` subscribers only after they have been put in the cache, preventing race conditions where JSON-RPC clients suffer from a cache miss if they ask information about these blocks too quickly. ## 1.0.11 - 2023-06-25 From aeae663704b0b1d28463efe763183d2ea3705934 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 20:07:00 +0200 Subject: [PATCH 17/46] Perform the re-subscription within the task --- .../background/legacy_state_sub.rs | 183 +++++++++++------- 1 file changed, 118 insertions(+), 65 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index e49dc4539b..ae090b6ca7 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -85,52 +85,13 @@ pub(super) fn start_task( ), }; - // Subscribe to new runtime service blocks in order to push them in the - // cache as soon as they are available. - // The buffer size should be large enough so that, if the CPU is busy, it - // doesn't become full before the execution of this task resumes. - // The maximum number of pinned block is ignored, as this maximum is a way to - // avoid malicious behaviors. This code is by definition not considered - // malicious. - let subscribe_all = runtime_service - .subscribe_all( - "json-rpc-blocks-cache", - 32, - NonZeroUsize::new(usize::max_value()).unwrap(), - ) - .await; - - cache.subscription_id = Some(subscribe_all.new_blocks.id()); - cache.recent_pinned_blocks.clear(); - debug_assert!(cache.recent_pinned_blocks.cap().get() >= 1); - - let finalized_block_hash = header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - ); - cache.recent_pinned_blocks.put( - finalized_block_hash, - subscribe_all.finalized_block_scale_encoded_header, - ); - - for block in subscribe_all.non_finalized_blocks_ancestry_order { - if cache.recent_pinned_blocks.len() == cache.recent_pinned_blocks.cap().get() { - let (hash, _) = cache.recent_pinned_blocks.pop_lru().unwrap(); - subscribe_all.new_blocks.unpin_block(&hash).await; - } - - let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - cache - .recent_pinned_blocks - .put(hash, block.scale_encoded_header); - } - run(Task { cache, log_target: log_target.clone(), platform: platform.clone(), sync_service, runtime_service, - new_blocks: subscribe_all.new_blocks, + subscription: Subscription::NotCreated, requests_rx, // TODO: all the subscriptions are dropped if the task returns all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( @@ -199,7 +160,7 @@ struct Task { platform: TPlat, sync_service: Arc>, runtime_service: Arc>, - new_blocks: runtime_service::Subscription, + subscription: Subscription, requests_rx: async_channel::Receiver>, // TODO: shrink_to_fit? all_heads_subscriptions: hashbrown::HashMap, @@ -207,21 +168,89 @@ struct Task { new_heads_subscriptions: hashbrown::HashMap, } +enum Subscription { + Active(runtime_service::Subscription), + Pending(Pin> + Send>>), + NotCreated, +} + async fn run(mut task: Task) { loop { - match task - .new_blocks - .next() - .map(either::Left) - .or(task.requests_rx.next().map(either::Right)) - .await - { - either::Left(Some(runtime_service::Notification::Block(block))) => { + enum WhatHappened<'a, TPlat: PlatformRef> { + SubscriptionNotification( + runtime_service::Notification, + &'a mut runtime_service::Subscription, + ), + SubscriptionDead, + SubscriptionReady(runtime_service::SubscribeAll), + Message(Message), + ForegroundDead, + } + + let event = { + let subscription_event = async { + match &mut task.subscription { + Subscription::NotCreated => WhatHappened::SubscriptionDead, + Subscription::Active(sub) => match sub.next().await { + Some(n) => WhatHappened::SubscriptionNotification(n, sub), + None => WhatHappened::SubscriptionDead, + }, + Subscription::Pending(pending) => { + WhatHappened::SubscriptionReady(pending.await) + } + } + }; + + let message = async { + match task.requests_rx.next().await { + Some(msg) => WhatHappened::Message(msg), + None => WhatHappened::ForegroundDead, + } + }; + + subscription_event.or(message).await + }; + + match event { + WhatHappened::SubscriptionReady(subscribe_all) => { + task.cache.subscription_id = Some(subscribe_all.new_blocks.id()); + task.cache.recent_pinned_blocks.clear(); + debug_assert!(task.cache.recent_pinned_blocks.cap().get() >= 1); + + let finalized_block_hash = header::hash_from_scale_encoded_header( + &subscribe_all.finalized_block_scale_encoded_header, + ); + task.cache.recent_pinned_blocks.put( + finalized_block_hash, + subscribe_all.finalized_block_scale_encoded_header, + ); + + for block in subscribe_all.non_finalized_blocks_ancestry_order { + if task.cache.recent_pinned_blocks.len() + == task.cache.recent_pinned_blocks.cap().get() + { + let (hash, _) = task.cache.recent_pinned_blocks.pop_lru().unwrap(); + subscribe_all.new_blocks.unpin_block(&hash).await; + } + + let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); + task.cache + .recent_pinned_blocks + .put(hash, block.scale_encoded_header); + } + + task.subscription = Subscription::Active(subscribe_all.new_blocks); + } + + WhatHappened::SubscriptionNotification( + runtime_service::Notification::Block(block), + subscription, + ) => { if task.cache.recent_pinned_blocks.len() == task.cache.recent_pinned_blocks.cap().get() { let (hash, _) = task.cache.recent_pinned_blocks.pop_lru().unwrap(); - task.new_blocks.unpin_block(&hash).await; + subscription.unpin_block(&hash).await; } let json_rpc_header = match methods::Header::from_scale_encoded_header( @@ -268,14 +297,18 @@ async fn run(mut task: Task) { } } } - either::Left(Some(runtime_service::Notification::Finalized { .. })) => {} - either::Left(Some(runtime_service::Notification::BestBlockChanged { - hash, .. - })) => { + WhatHappened::SubscriptionNotification( + runtime_service::Notification::Finalized { .. }, + _, + ) => {} + WhatHappened::SubscriptionNotification( + runtime_service::Notification::BestBlockChanged { hash, .. }, + _, + ) => { // TODO: report a chain_newHead subscription } - either::Right(Some(Message::SubscriptionStart(request))) => match request.request() { + WhatHappened::Message(Message::SubscriptionStart(request)) => match request.request() { methods::MethodCall::chain_subscribeAllHeads {} => { let subscription = request.accept(); let subscription_id = subscription.subscription_id().to_owned(); @@ -292,16 +325,16 @@ async fn run(mut task: Task) { _ => unreachable!(), // TODO: stronger typing to avoid this? }, - either::Right(Some(Message::SubscriptionDestroyed { subscription_id })) => { + WhatHappened::Message(Message::SubscriptionDestroyed { subscription_id }) => { task.all_heads_subscriptions.remove(&subscription_id); task.new_heads_subscriptions.remove(&subscription_id); // TODO: shrink_to_fit? } - either::Right(Some(Message::RecentBlockRuntimeAccess { + WhatHappened::Message(Message::RecentBlockRuntimeAccess { block_hash, result_tx, - })) => { + }) => { let access = if task.cache.recent_pinned_blocks.contains(&block_hash) { // The runtime service has the block pinned, meaning that we can ask the runtime // service to perform the call. @@ -319,10 +352,10 @@ async fn run(mut task: Task) { let _ = result_tx.send(access); } - either::Right(Some(Message::BlockNumber { + WhatHappened::Message(Message::BlockNumber { block_hash, result_tx, - })) => { + }) => { if let Some(future) = task .cache .block_state_root_hashes_numbers @@ -346,10 +379,10 @@ async fn run(mut task: Task) { let _ = result_tx.send(block_number); } - either::Right(Some(Message::BlockHeader { + WhatHappened::Message(Message::BlockHeader { block_hash, result_tx, - })) => { + }) => { let header = if let Some(header) = task.cache.recent_pinned_blocks.get(&block_hash) { Some(header.clone()) @@ -360,10 +393,10 @@ async fn run(mut task: Task) { let _ = result_tx.send(header); } - either::Right(Some(Message::BlockStateRootAndNumber { + WhatHappened::Message(Message::BlockStateRootAndNumber { block_hash, result_tx, - })) => { + }) => { let fetch = { // Try to find an existing entry in cache, and if not create one. @@ -470,9 +503,29 @@ async fn run(mut task: Task) { }); } - either::Left(None) | either::Right(None) => { + WhatHappened::ForegroundDead => { break; } + + WhatHappened::SubscriptionDead => { + // Subscribe to new runtime service blocks in order to push them in the + // cache as soon as they are available. + // The buffer size should be large enough so that, if the CPU is busy, it + // doesn't become full before the execution of this task resumes. + // The maximum number of pinned block is ignored, as this maximum is a way to + // avoid malicious behaviors. This code is by definition not considered + // malicious. + let runtime_service = task.runtime_service.clone(); + task.subscription = Subscription::Pending(Box::pin(async move { + runtime_service + .subscribe_all( + "json-rpc-blocks-cache", + 32, + NonZeroUsize::new(usize::max_value()).unwrap(), + ) + .await + })); + } } } } From af2ff4b6d6081a373dbbb8758b3712a800dea9ce Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 20:11:09 +0200 Subject: [PATCH 18/46] Inline the fields of `Cache` within `Task` --- .../background/legacy_state_sub.rs | 64 ++++++------------- 1 file changed, 20 insertions(+), 44 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index ae090b6ca7..d20c9b021e 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -73,7 +73,7 @@ pub(super) fn start_task( format!("{}-cache-populate", log_target).into(), Box::pin(async move { loop { - let mut cache = Cache { + run(Task { recent_pinned_blocks: lru::LruCache::with_hasher( NonZeroUsize::new(32).unwrap(), Default::default(), @@ -83,10 +83,6 @@ pub(super) fn start_task( NonZeroUsize::new(32).unwrap(), Default::default(), ), - }; - - run(Task { - cache, log_target: log_target.clone(), platform: platform.clone(), sync_service, @@ -111,7 +107,7 @@ pub(super) fn start_task( ); } -struct Cache { +struct Task { /// When the runtime service reports a new block, it is kept pinned and inserted in this LRU /// cache. When an entry in removed from the cache, it is unpinned. /// @@ -152,10 +148,7 @@ struct Cache { >, fnv::FnvBuildHasher, >, -} -struct Task { - cache: Cache, log_target: String, platform: TPlat, sync_service: Arc>, @@ -213,29 +206,26 @@ async fn run(mut task: Task) { match event { WhatHappened::SubscriptionReady(subscribe_all) => { - task.cache.subscription_id = Some(subscribe_all.new_blocks.id()); - task.cache.recent_pinned_blocks.clear(); - debug_assert!(task.cache.recent_pinned_blocks.cap().get() >= 1); + task.subscription_id = Some(subscribe_all.new_blocks.id()); + task.recent_pinned_blocks.clear(); + debug_assert!(task.recent_pinned_blocks.cap().get() >= 1); let finalized_block_hash = header::hash_from_scale_encoded_header( &subscribe_all.finalized_block_scale_encoded_header, ); - task.cache.recent_pinned_blocks.put( + task.recent_pinned_blocks.put( finalized_block_hash, subscribe_all.finalized_block_scale_encoded_header, ); for block in subscribe_all.non_finalized_blocks_ancestry_order { - if task.cache.recent_pinned_blocks.len() - == task.cache.recent_pinned_blocks.cap().get() - { - let (hash, _) = task.cache.recent_pinned_blocks.pop_lru().unwrap(); + if task.recent_pinned_blocks.len() == task.recent_pinned_blocks.cap().get() { + let (hash, _) = task.recent_pinned_blocks.pop_lru().unwrap(); subscribe_all.new_blocks.unpin_block(&hash).await; } let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - task.cache - .recent_pinned_blocks + task.recent_pinned_blocks .put(hash, block.scale_encoded_header); } @@ -246,10 +236,8 @@ async fn run(mut task: Task) { runtime_service::Notification::Block(block), subscription, ) => { - if task.cache.recent_pinned_blocks.len() - == task.cache.recent_pinned_blocks.cap().get() - { - let (hash, _) = task.cache.recent_pinned_blocks.pop_lru().unwrap(); + if task.recent_pinned_blocks.len() == task.recent_pinned_blocks.cap().get() { + let (hash, _) = task.recent_pinned_blocks.pop_lru().unwrap(); subscription.unpin_block(&hash).await; } @@ -273,8 +261,7 @@ async fn run(mut task: Task) { }; let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - task.cache - .recent_pinned_blocks + task.recent_pinned_blocks .put(hash, block.scale_encoded_header); for (subscription_id, subscription) in &mut task.all_heads_subscriptions { @@ -335,14 +322,11 @@ async fn run(mut task: Task) { block_hash, result_tx, }) => { - let access = if task.cache.recent_pinned_blocks.contains(&block_hash) { + let access = if task.recent_pinned_blocks.contains(&block_hash) { // The runtime service has the block pinned, meaning that we can ask the runtime // service to perform the call. task.runtime_service - .pinned_block_runtime_access( - task.cache.subscription_id.unwrap(), - &block_hash, - ) + .pinned_block_runtime_access(task.subscription_id.unwrap(), &block_hash) .await .ok() } else { @@ -356,20 +340,15 @@ async fn run(mut task: Task) { block_hash, result_tx, }) => { - if let Some(future) = task - .cache - .block_state_root_hashes_numbers - .get_mut(&block_hash) - { + if let Some(future) = task.block_state_root_hashes_numbers.get_mut(&block_hash) { let _ = future.now_or_never(); } let block_number = match ( - task.cache - .recent_pinned_blocks + task.recent_pinned_blocks .get(&block_hash) .map(|h| header::decode(h, task.runtime_service.block_number_bytes())), - task.cache.block_state_root_hashes_numbers.get(&block_hash), + task.block_state_root_hashes_numbers.get(&block_hash), ) { (Some(Ok(header)), _) => Some(header.number), (_, Some(future::MaybeDone::Done(Ok((_, num))))) => Some(*num), @@ -383,8 +362,7 @@ async fn run(mut task: Task) { block_hash, result_tx, }) => { - let header = if let Some(header) = task.cache.recent_pinned_blocks.get(&block_hash) - { + let header = if let Some(header) = task.recent_pinned_blocks.get(&block_hash) { Some(header.clone()) } else { None @@ -402,7 +380,6 @@ async fn run(mut task: Task) { // Look in `recent_pinned_blocks`. match task - .cache .recent_pinned_blocks .get(&block_hash) .map(|h| header::decode(h, task.runtime_service.block_number_bytes())) @@ -420,7 +397,7 @@ async fn run(mut task: Task) { } // Look in `block_state_root_hashes`. - match task.cache.block_state_root_hashes_numbers.get(&block_hash) { + match task.block_state_root_hashes_numbers.get(&block_hash) { Some(future::MaybeDone::Done(Ok(val))) => { let _ = result_tx.send(Ok(*val)); continue; @@ -486,8 +463,7 @@ async fn run(mut task: Task) { let wrapped = (Box::pin(fetch) as Pin + Send>>) .shared(); - task.cache - .block_state_root_hashes_numbers + task.block_state_root_hashes_numbers .put(block_hash, future::maybe_done(wrapped.clone())); wrapped } From 8ee9ec48f166b34d50e241d97b145a7f3e3ef09b Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 20:19:19 +0200 Subject: [PATCH 19/46] Move recent pinned blocks to Subscription::Active as it makes sense there --- .../background/legacy_state_sub.rs | 193 +++++++++++------- 1 file changed, 114 insertions(+), 79 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index d20c9b021e..55e4563c8b 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -74,11 +74,6 @@ pub(super) fn start_task( Box::pin(async move { loop { run(Task { - recent_pinned_blocks: lru::LruCache::with_hasher( - NonZeroUsize::new(32).unwrap(), - Default::default(), - ), - subscription_id: None, block_state_root_hashes_numbers: lru::LruCache::with_hasher( NonZeroUsize::new(32).unwrap(), Default::default(), @@ -108,21 +103,6 @@ pub(super) fn start_task( } struct Task { - /// When the runtime service reports a new block, it is kept pinned and inserted in this LRU - /// cache. When an entry in removed from the cache, it is unpinned. - /// - /// JSON-RPC clients are more likely to ask for information about recent blocks and perform - /// calls on them, hence a cache of recent blocks. - recent_pinned_blocks: lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, - - /// Subscription on the runtime service under which the blocks of - /// [`Cache::recent_pinned_blocks`] are pinned. - /// - /// Contains `None` only at initialization, in which case [`Cache::recent_pinned_blocks`] - /// is guaranteed to be empty. In other words, if a block is found in - /// [`Cache::recent_pinned_blocks`] then this field is guaranteed to be `Some`. - subscription_id: Option, - /// State trie root hashes and numbers of blocks that were not in /// [`Cache::recent_pinned_blocks`]. /// @@ -162,7 +142,17 @@ struct Task { } enum Subscription { - Active(runtime_service::Subscription), + Active { + /// Object representing the subscription. + subscription: runtime_service::Subscription, + + /// When the runtime service reports a new block, it is kept pinned and inserted in this + /// LRU cache. When an entry in removed from the cache, it is unpinned. + /// + /// JSON-RPC clients are more likely to ask for information about recent blocks and + /// perform calls on them, hence a cache of recent blocks. + recent_pinned_blocks: lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, + }, Pending(Pin> + Send>>), NotCreated, } @@ -170,10 +160,11 @@ enum Subscription { async fn run(mut task: Task) { loop { enum WhatHappened<'a, TPlat: PlatformRef> { - SubscriptionNotification( - runtime_service::Notification, - &'a mut runtime_service::Subscription, - ), + SubscriptionNotification { + notification: runtime_service::Notification, + subscription: &'a mut runtime_service::Subscription, + recent_pinned_blocks: &'a mut lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, + }, SubscriptionDead, SubscriptionReady(runtime_service::SubscribeAll), Message(Message), @@ -184,8 +175,15 @@ async fn run(mut task: Task) { let subscription_event = async { match &mut task.subscription { Subscription::NotCreated => WhatHappened::SubscriptionDead, - Subscription::Active(sub) => match sub.next().await { - Some(n) => WhatHappened::SubscriptionNotification(n, sub), + Subscription::Active { + subscription, + recent_pinned_blocks, + } => match subscription.next().await { + Some(notification) => WhatHappened::SubscriptionNotification { + notification, + subscription, + recent_pinned_blocks, + }, None => WhatHappened::SubscriptionDead, }, Subscription::Pending(pending) => { @@ -206,38 +204,40 @@ async fn run(mut task: Task) { match event { WhatHappened::SubscriptionReady(subscribe_all) => { - task.subscription_id = Some(subscribe_all.new_blocks.id()); - task.recent_pinned_blocks.clear(); - debug_assert!(task.recent_pinned_blocks.cap().get() >= 1); + let mut recent_pinned_blocks = + lru::LruCache::with_hasher(NonZeroUsize::new(32).unwrap(), Default::default()); let finalized_block_hash = header::hash_from_scale_encoded_header( &subscribe_all.finalized_block_scale_encoded_header, ); - task.recent_pinned_blocks.put( + recent_pinned_blocks.put( finalized_block_hash, subscribe_all.finalized_block_scale_encoded_header, ); for block in subscribe_all.non_finalized_blocks_ancestry_order { - if task.recent_pinned_blocks.len() == task.recent_pinned_blocks.cap().get() { - let (hash, _) = task.recent_pinned_blocks.pop_lru().unwrap(); + if recent_pinned_blocks.len() == recent_pinned_blocks.cap().get() { + let (hash, _) = recent_pinned_blocks.pop_lru().unwrap(); subscribe_all.new_blocks.unpin_block(&hash).await; } let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - task.recent_pinned_blocks - .put(hash, block.scale_encoded_header); + recent_pinned_blocks.put(hash, block.scale_encoded_header); } - task.subscription = Subscription::Active(subscribe_all.new_blocks); + task.subscription = Subscription::Active { + subscription: subscribe_all.new_blocks, + recent_pinned_blocks, + }; } - WhatHappened::SubscriptionNotification( - runtime_service::Notification::Block(block), + WhatHappened::SubscriptionNotification { + notification: runtime_service::Notification::Block(block), subscription, - ) => { - if task.recent_pinned_blocks.len() == task.recent_pinned_blocks.cap().get() { - let (hash, _) = task.recent_pinned_blocks.pop_lru().unwrap(); + recent_pinned_blocks, + } => { + if recent_pinned_blocks.len() == recent_pinned_blocks.cap().get() { + let (hash, _) = recent_pinned_blocks.pop_lru().unwrap(); subscription.unpin_block(&hash).await; } @@ -261,8 +261,7 @@ async fn run(mut task: Task) { }; let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - task.recent_pinned_blocks - .put(hash, block.scale_encoded_header); + recent_pinned_blocks.put(hash, block.scale_encoded_header); for (subscription_id, subscription) in &mut task.all_heads_subscriptions { subscription @@ -284,14 +283,14 @@ async fn run(mut task: Task) { } } } - WhatHappened::SubscriptionNotification( - runtime_service::Notification::Finalized { .. }, - _, - ) => {} - WhatHappened::SubscriptionNotification( - runtime_service::Notification::BestBlockChanged { hash, .. }, - _, - ) => { + WhatHappened::SubscriptionNotification { + notification: runtime_service::Notification::Finalized { .. }, + .. + } => {} + WhatHappened::SubscriptionNotification { + notification: runtime_service::Notification::BestBlockChanged { hash, .. }, + .. + } => { // TODO: report a chain_newHead subscription } @@ -322,11 +321,26 @@ async fn run(mut task: Task) { block_hash, result_tx, }) => { - let access = if task.recent_pinned_blocks.contains(&block_hash) { - // The runtime service has the block pinned, meaning that we can ask the runtime - // service to perform the call. + let subscription_id_with_block = if let Subscription::Active { + recent_pinned_blocks, + subscription, + .. + } = &task.subscription + { + if recent_pinned_blocks.contains(&block_hash) { + // The runtime service has the block pinned, meaning that we can ask the runtime + // service to perform the call. + Some(subscription.id()) + } else { + None + } + } else { + None + }; + + let access = if let Some(subscription_id) = subscription_id_with_block { task.runtime_service - .pinned_block_runtime_access(task.subscription_id.unwrap(), &block_hash) + .pinned_block_runtime_access(subscription_id, &block_hash) .await .ok() } else { @@ -344,15 +358,23 @@ async fn run(mut task: Task) { let _ = future.now_or_never(); } - let block_number = match ( - task.recent_pinned_blocks - .get(&block_hash) - .map(|h| header::decode(h, task.runtime_service.block_number_bytes())), - task.block_state_root_hashes_numbers.get(&block_hash), - ) { - (Some(Ok(header)), _) => Some(header.number), - (_, Some(future::MaybeDone::Done(Ok((_, num))))) => Some(*num), - _ => None, + let block_number = if let Subscription::Active { + recent_pinned_blocks, + .. + } = &mut task.subscription + { + match ( + recent_pinned_blocks + .get(&block_hash) + .map(|h| header::decode(h, task.runtime_service.block_number_bytes())), + task.block_state_root_hashes_numbers.get(&block_hash), + ) { + (Some(Ok(header)), _) => Some(header.number), + (_, Some(future::MaybeDone::Done(Ok((_, num))))) => Some(*num), + _ => None, + } + } else { + None }; let _ = result_tx.send(block_number); @@ -362,8 +384,16 @@ async fn run(mut task: Task) { block_hash, result_tx, }) => { - let header = if let Some(header) = task.recent_pinned_blocks.get(&block_hash) { - Some(header.clone()) + let header = if let Subscription::Active { + recent_pinned_blocks, + .. + } = &mut task.subscription + { + if let Some(header) = recent_pinned_blocks.get(&block_hash) { + Some(header.clone()) + } else { + None + } } else { None }; @@ -379,21 +409,26 @@ async fn run(mut task: Task) { // Try to find an existing entry in cache, and if not create one. // Look in `recent_pinned_blocks`. - match task - .recent_pinned_blocks - .get(&block_hash) - .map(|h| header::decode(h, task.runtime_service.block_number_bytes())) + if let Subscription::Active { + recent_pinned_blocks, + .. + } = &mut task.subscription { - Some(Ok(header)) => { - let _ = result_tx.send(Ok((*header.state_root, header.number))); - continue; + match recent_pinned_blocks + .get(&block_hash) + .map(|h| header::decode(h, task.runtime_service.block_number_bytes())) + { + Some(Ok(header)) => { + let _ = result_tx.send(Ok((*header.state_root, header.number))); + continue; + } + Some(Err(err)) => { + let _ = result_tx + .send(Err(StateTrieRootHashError::HeaderDecodeError(err))); + continue; + } // TODO: can this actually happen? unclear + None => {} } - Some(Err(err)) => { - let _ = - result_tx.send(Err(StateTrieRootHashError::HeaderDecodeError(err))); - continue; - } // TODO: can this actually happen? unclear - None => {} } // Look in `block_state_root_hashes`. From 0f655d0b5a05c6c043d7f39368512ed6d6a54feb Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 20:19:53 +0200 Subject: [PATCH 20/46] Simplify `start_task` --- .../background/legacy_state_sub.rs | 51 ++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 55e4563c8b..caa4b4c8c4 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -70,35 +70,28 @@ pub(super) fn start_task( requests_rx: async_channel::Receiver>, ) { platform.clone().spawn_task( - format!("{}-cache-populate", log_target).into(), - Box::pin(async move { - loop { - run(Task { - block_state_root_hashes_numbers: lru::LruCache::with_hasher( - NonZeroUsize::new(32).unwrap(), - Default::default(), - ), - log_target: log_target.clone(), - platform: platform.clone(), - sync_service, - runtime_service, - subscription: Subscription::NotCreated, - requests_rx, - // TODO: all the subscriptions are dropped if the task returns - all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, - Default::default(), - ), - new_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, - Default::default(), - ), - }) - .await; - - panic!() // TODO: not implemented correctly - } - }), + format!("{}-cache", log_target).into(), + Box::pin(run(Task { + block_state_root_hashes_numbers: lru::LruCache::with_hasher( + NonZeroUsize::new(32).unwrap(), + Default::default(), + ), + log_target: log_target.clone(), + platform: platform.clone(), + sync_service, + runtime_service, + subscription: Subscription::NotCreated, + requests_rx, + // TODO: all the subscriptions are dropped if the task returns + all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), + new_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), + })), ); } From 54bb53e0c9b9642ed37461bd70f6d570aaf51e43 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 20:45:11 +0200 Subject: [PATCH 21/46] Wrap recent blocks in a struct --- .../background/legacy_state_sub.rs | 50 +++++++++++++------ 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index caa4b4c8c4..fdbb6466f0 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -144,19 +144,24 @@ enum Subscription { /// /// JSON-RPC clients are more likely to ask for information about recent blocks and /// perform calls on them, hence a cache of recent blocks. - recent_pinned_blocks: lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, + recent_pinned_blocks: lru::LruCache<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, }, Pending(Pin> + Send>>), NotCreated, } +struct RecentBlock { + scale_encoded_header: Vec, +} + async fn run(mut task: Task) { loop { enum WhatHappened<'a, TPlat: PlatformRef> { SubscriptionNotification { notification: runtime_service::Notification, subscription: &'a mut runtime_service::Subscription, - recent_pinned_blocks: &'a mut lru::LruCache<[u8; 32], Vec, fnv::FnvBuildHasher>, + recent_pinned_blocks: + &'a mut lru::LruCache<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, }, SubscriptionDead, SubscriptionReady(runtime_service::SubscribeAll), @@ -205,7 +210,9 @@ async fn run(mut task: Task) { ); recent_pinned_blocks.put( finalized_block_hash, - subscribe_all.finalized_block_scale_encoded_header, + RecentBlock { + scale_encoded_header: subscribe_all.finalized_block_scale_encoded_header, + }, ); for block in subscribe_all.non_finalized_blocks_ancestry_order { @@ -215,7 +222,12 @@ async fn run(mut task: Task) { } let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - recent_pinned_blocks.put(hash, block.scale_encoded_header); + recent_pinned_blocks.put( + hash, + RecentBlock { + scale_encoded_header: block.scale_encoded_header, + }, + ); } task.subscription = Subscription::Active { @@ -254,7 +266,12 @@ async fn run(mut task: Task) { }; let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - recent_pinned_blocks.put(hash, block.scale_encoded_header); + recent_pinned_blocks.put( + hash, + RecentBlock { + scale_encoded_header: block.scale_encoded_header, + }, + ); for (subscription_id, subscription) in &mut task.all_heads_subscriptions { subscription @@ -357,9 +374,12 @@ async fn run(mut task: Task) { } = &mut task.subscription { match ( - recent_pinned_blocks - .get(&block_hash) - .map(|h| header::decode(h, task.runtime_service.block_number_bytes())), + recent_pinned_blocks.get(&block_hash).map(|b| { + header::decode( + &b.scale_encoded_header, + task.runtime_service.block_number_bytes(), + ) + }), task.block_state_root_hashes_numbers.get(&block_hash), ) { (Some(Ok(header)), _) => Some(header.number), @@ -382,8 +402,8 @@ async fn run(mut task: Task) { .. } = &mut task.subscription { - if let Some(header) = recent_pinned_blocks.get(&block_hash) { - Some(header.clone()) + if let Some(block) = recent_pinned_blocks.get(&block_hash) { + Some(block.scale_encoded_header.clone()) } else { None } @@ -407,10 +427,12 @@ async fn run(mut task: Task) { .. } = &mut task.subscription { - match recent_pinned_blocks - .get(&block_hash) - .map(|h| header::decode(h, task.runtime_service.block_number_bytes())) - { + match recent_pinned_blocks.get(&block_hash).map(|b| { + header::decode( + &b.scale_encoded_header, + task.runtime_service.block_number_bytes(), + ) + }) { Some(Ok(header)) => { let _ = result_tx.send(Ok((*header.state_root, header.number))); continue; From d9a9187335044d9a9696b1da10e34ea06b6d5f30 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 4 Jul 2023 20:52:23 +0200 Subject: [PATCH 22/46] Add `runtime_version` field to `RecentBlock` --- .../src/json_rpc_service/background/legacy_state_sub.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index fdbb6466f0..9624ae88b5 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -28,7 +28,7 @@ use futures_channel::oneshot; use futures_lite::{FutureExt as _, StreamExt as _}; use futures_util::{future, stream::AbortRegistration, FutureExt as _}; use smoldot::{ - header, + executor, header, informant::HashDisplay, json_rpc::{methods, service}, network::protocol, @@ -152,6 +152,7 @@ enum Subscription { struct RecentBlock { scale_encoded_header: Vec, + runtime_version: Arc>, } async fn run(mut task: Task) { @@ -212,6 +213,7 @@ async fn run(mut task: Task) { finalized_block_hash, RecentBlock { scale_encoded_header: subscribe_all.finalized_block_scale_encoded_header, + runtime_version: Arc::new(subscribe_all.finalized_block_runtime), }, ); @@ -226,6 +228,10 @@ async fn run(mut task: Task) { hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, + runtime_version: block + .new_runtime + .map(Arc::new) + .unwrap_or_else(|| todo!()), // TODO: }, ); } @@ -270,6 +276,7 @@ async fn run(mut task: Task) { hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, + runtime_version: block.new_runtime.map(Arc::new).unwrap_or_else(|| todo!()), // TODO: }, ); From 1c6bbe695c91f6f34178fe0f465786bb265dbabf Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 07:21:16 +0200 Subject: [PATCH 23/46] Fix pinning strategy --- .../background/legacy_state_sub.rs | 147 ++++++++++++++---- 1 file changed, 115 insertions(+), 32 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 9624ae88b5..75b5a751f5 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -17,6 +17,7 @@ use core::{ future::Future, + iter, num::{NonZeroU32, NonZeroUsize}, pin::Pin, time::Duration, @@ -140,11 +141,18 @@ enum Subscription { subscription: runtime_service::Subscription, /// When the runtime service reports a new block, it is kept pinned and inserted in this - /// LRU cache. When an entry in removed from the cache, it is unpinned. + /// list. + /// + /// Blocks are removed from this container and unpinned when they leave + /// [`Subscription::Active::finalized_and_pruned_lru`]. /// /// JSON-RPC clients are more likely to ask for information about recent blocks and /// perform calls on them, hence a cache of recent blocks. - recent_pinned_blocks: lru::LruCache<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, + pinned_blocks: hashbrown::HashMap<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, + + /// When a block is finalized or pruned, it is inserted into this LRU cache. The least + /// recently used blocks are removed and unpinned. + finalized_and_pruned_lru: lru::LruCache<[u8; 32], (), fnv::FnvBuildHasher>, }, Pending(Pin> + Send>>), NotCreated, @@ -161,8 +169,9 @@ async fn run(mut task: Task) { SubscriptionNotification { notification: runtime_service::Notification, subscription: &'a mut runtime_service::Subscription, - recent_pinned_blocks: - &'a mut lru::LruCache<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, + pinned_blocks: + &'a mut hashbrown::HashMap<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, + finalized_and_pruned_lru: &'a mut lru::LruCache<[u8; 32], (), fnv::FnvBuildHasher>, }, SubscriptionDead, SubscriptionReady(runtime_service::SubscribeAll), @@ -176,12 +185,14 @@ async fn run(mut task: Task) { Subscription::NotCreated => WhatHappened::SubscriptionDead, Subscription::Active { subscription, - recent_pinned_blocks, + pinned_blocks, + finalized_and_pruned_lru, } => match subscription.next().await { Some(notification) => WhatHappened::SubscriptionNotification { notification, subscription, - recent_pinned_blocks, + pinned_blocks, + finalized_and_pruned_lru, }, None => WhatHappened::SubscriptionDead, }, @@ -203,28 +214,28 @@ async fn run(mut task: Task) { match event { WhatHappened::SubscriptionReady(subscribe_all) => { - let mut recent_pinned_blocks = - lru::LruCache::with_hasher(NonZeroUsize::new(32).unwrap(), Default::default()); + let mut pinned_blocks = + hashbrown::HashMap::with_capacity_and_hasher(32, Default::default()); + let mut finalized_and_pruned_lru = lru::LruCache::with_hasher( + NonZeroUsize::new(32).unwrap(), + fnv::FnvBuildHasher::default(), + ); let finalized_block_hash = header::hash_from_scale_encoded_header( &subscribe_all.finalized_block_scale_encoded_header, ); - recent_pinned_blocks.put( + pinned_blocks.insert( finalized_block_hash, RecentBlock { scale_encoded_header: subscribe_all.finalized_block_scale_encoded_header, runtime_version: Arc::new(subscribe_all.finalized_block_runtime), }, ); + finalized_and_pruned_lru.put(finalized_block_hash, ()); for block in subscribe_all.non_finalized_blocks_ancestry_order { - if recent_pinned_blocks.len() == recent_pinned_blocks.cap().get() { - let (hash, _) = recent_pinned_blocks.pop_lru().unwrap(); - subscribe_all.new_blocks.unpin_block(&hash).await; - } - let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - recent_pinned_blocks.put( + pinned_blocks.insert( hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, @@ -238,20 +249,17 @@ async fn run(mut task: Task) { task.subscription = Subscription::Active { subscription: subscribe_all.new_blocks, - recent_pinned_blocks, + pinned_blocks, + finalized_and_pruned_lru, }; } WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::Block(block), subscription, - recent_pinned_blocks, + pinned_blocks, + .. } => { - if recent_pinned_blocks.len() == recent_pinned_blocks.cap().get() { - let (hash, _) = recent_pinned_blocks.pop_lru().unwrap(); - subscription.unpin_block(&hash).await; - } - let json_rpc_header = match methods::Header::from_scale_encoded_header( &block.scale_encoded_header, task.runtime_service.block_number_bytes(), @@ -272,7 +280,7 @@ async fn run(mut task: Task) { }; let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - recent_pinned_blocks.put( + pinned_blocks.insert( hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, @@ -300,15 +308,90 @@ async fn run(mut task: Task) { } } } + WhatHappened::SubscriptionNotification { - notification: runtime_service::Notification::Finalized { .. }, - .. - } => {} + notification: + runtime_service::Notification::Finalized { + hash, + pruned_blocks, + best_block_hash, + }, + pinned_blocks, + finalized_and_pruned_lru, + subscription, + } => { + for block_hash in pruned_blocks.into_iter().chain(iter::once(hash)) { + if finalized_and_pruned_lru.len() == finalized_and_pruned_lru.cap().get() { + let (hash_to_unpin, _) = finalized_and_pruned_lru.pop_lru().unwrap(); + subscription.unpin_block(&hash_to_unpin).await; + pinned_blocks.remove(&hash_to_unpin).unwrap(); + } + finalized_and_pruned_lru.put(block_hash, ()); + } + + // TODO: skip below if best block didn't change + let best_block_header = &pinned_blocks + .get(&best_block_hash) + .unwrap() + .scale_encoded_header; + let best_block_json_rpc_header = match methods::Header::from_scale_encoded_header( + &best_block_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeNewHeads` subscription has skipped block due to \ + undecodable header. Hash: {}. Error: {}", + HashDisplay(&best_block_hash), + error, + ); + continue; + } + }; + + for (subscription_id, subscription) in &mut task.new_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_newHead { + subscription: subscription_id.as_str().into(), + result: best_block_json_rpc_header.clone(), + }) + .await; + } + } + WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::BestBlockChanged { hash, .. }, + pinned_blocks, .. } => { - // TODO: report a chain_newHead subscription + let header = &pinned_blocks.get(&hash).unwrap().scale_encoded_header; + let json_rpc_header = match methods::Header::from_scale_encoded_header( + &header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeNewHeads` subscription has skipped block due to \ + undecodable header. Hash: {}. Error: {}", + HashDisplay(&hash), + error, + ); + continue; + } + }; + + for (subscription_id, subscription) in &mut task.new_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_newHead { + subscription: subscription_id.as_str().into(), + result: json_rpc_header.clone(), + }) + .await; + } } WhatHappened::Message(Message::SubscriptionStart(request)) => match request.request() { @@ -339,12 +422,12 @@ async fn run(mut task: Task) { result_tx, }) => { let subscription_id_with_block = if let Subscription::Active { - recent_pinned_blocks, + pinned_blocks: recent_pinned_blocks, subscription, .. } = &task.subscription { - if recent_pinned_blocks.contains(&block_hash) { + if recent_pinned_blocks.contains_key(&block_hash) { // The runtime service has the block pinned, meaning that we can ask the runtime // service to perform the call. Some(subscription.id()) @@ -376,7 +459,7 @@ async fn run(mut task: Task) { } let block_number = if let Subscription::Active { - recent_pinned_blocks, + pinned_blocks: recent_pinned_blocks, .. } = &mut task.subscription { @@ -405,7 +488,7 @@ async fn run(mut task: Task) { result_tx, }) => { let header = if let Subscription::Active { - recent_pinned_blocks, + pinned_blocks: recent_pinned_blocks, .. } = &mut task.subscription { @@ -430,7 +513,7 @@ async fn run(mut task: Task) { // Look in `recent_pinned_blocks`. if let Subscription::Active { - recent_pinned_blocks, + pinned_blocks: recent_pinned_blocks, .. } = &mut task.subscription { From d95f273cda51eecb49103c02e71c28788e60504b Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 07:27:14 +0200 Subject: [PATCH 24/46] Handle finalized subscriptions in new task --- light-base/src/json_rpc_service/background.rs | 5 +- .../background/legacy_state_sub.rs | 50 +++++++++++- .../background/state_chain.rs | 78 ------------------- 3 files changed, 51 insertions(+), 82 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index a96e3e5425..6cf7490eff 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -187,7 +187,8 @@ pub(super) fn start( requests_processing_task = task; match subscription_start.request() { methods::MethodCall::chain_subscribeAllHeads {} - | methods::MethodCall::chain_subscribeNewHeads {} => { + | methods::MethodCall::chain_subscribeNewHeads {} + | methods::MethodCall::chain_subscribeFinalizedHeads {} => { me.to_legacy .lock() .await @@ -648,7 +649,7 @@ impl Background { unreachable!() } methods::MethodCall::chain_subscribeFinalizedHeads {} => { - self.chain_subscribe_finalized_heads(request).await; + unreachable!() } methods::MethodCall::chain_subscribeNewHeads {} => { unreachable!() diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 75b5a751f5..51ac695469 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -92,6 +92,10 @@ pub(super) fn start_task( 8, Default::default(), ), + finalized_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), })), ); } @@ -133,6 +137,9 @@ struct Task { all_heads_subscriptions: hashbrown::HashMap, // TODO: shrink_to_fit? new_heads_subscriptions: hashbrown::HashMap, + // TODO: shrink_to_fit? + finalized_heads_subscriptions: + hashbrown::HashMap, } enum Subscription { @@ -312,7 +319,7 @@ async fn run(mut task: Task) { WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::Finalized { - hash, + hash: finalized_hash, pruned_blocks, best_block_hash, }, @@ -320,7 +327,38 @@ async fn run(mut task: Task) { finalized_and_pruned_lru, subscription, } => { - for block_hash in pruned_blocks.into_iter().chain(iter::once(hash)) { + let finalized_block_header = &pinned_blocks + .get(&finalized_hash) + .unwrap() + .scale_encoded_header; + let finalized_block_json_rpc_header = + match methods::Header::from_scale_encoded_header( + finalized_block_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeNewHeads` subscription has skipped block due to \ + undecodable header. Hash: {}. Error: {}", + HashDisplay(&best_block_hash), + error, + ); + continue; + } + }; + + for (subscription_id, subscription) in &mut task.finalized_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_finalizedHead { + subscription: subscription_id.as_str().into(), + result: finalized_block_json_rpc_header.clone(), + }) + .await; + } + + for block_hash in pruned_blocks.into_iter().chain(iter::once(finalized_hash)) { if finalized_and_pruned_lru.len() == finalized_and_pruned_lru.cap().get() { let (hash_to_unpin, _) = finalized_and_pruned_lru.pop_lru().unwrap(); subscription.unpin_block(&hash_to_unpin).await; @@ -408,12 +446,20 @@ async fn run(mut task: Task) { task.new_heads_subscriptions .insert(subscription_id, subscription); } + methods::MethodCall::chain_subscribeFinalizedHeads {} => { + let subscription = request.accept(); + let subscription_id = subscription.subscription_id().to_owned(); + // TODO: must immediately send the current finalized block + task.finalized_heads_subscriptions + .insert(subscription_id, subscription); + } _ => unreachable!(), // TODO: stronger typing to avoid this? }, WhatHappened::Message(Message::SubscriptionDestroyed { subscription_id }) => { task.all_heads_subscriptions.remove(&subscription_id); task.new_heads_subscriptions.remove(&subscription_id); + task.finalized_heads_subscriptions.remove(&subscription_id); // TODO: shrink_to_fit? } diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 449eff25f1..491b1bd2d4 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -326,84 +326,6 @@ impl Background { } } - /// Handles a call to [`methods::MethodCall::chain_subscribeFinalizedHeads`]. - pub(super) async fn chain_subscribe_finalized_heads( - self: &Arc, - request: service::SubscriptionStartProcess, - ) { - let methods::MethodCall::chain_subscribeFinalizedHeads {} = request.request() else { - unreachable!() - }; - - let mut blocks_list = { - let (finalized_block_header, finalized_blocks_subscription) = - sub_utils::subscribe_finalized(&self.runtime_service).await; - stream::once(future::ready(finalized_block_header)).chain(finalized_blocks_subscription) - }; - - self.platform.spawn_task( - format!("{}-subscribe-finalized-heads", self.log_target).into(), - { - let log_target = self.log_target.clone(); - let sync_service = self.sync_service.clone(); - - async move { - let mut subscription = request.accept(); - let subscription_id = subscription.subscription_id().to_owned(); - - loop { - let event = { - let unsubscribed = pin::pin!(subscription.wait_until_stale()); - match future::select(blocks_list.next(), unsubscribed).await { - future::Either::Left((ev, _)) => either::Left(ev), - future::Either::Right((ev, _)) => either::Right(ev), - } - }; - - match event { - either::Left(None) => { - // Stream returned by `subscribe_finalized` is always unlimited. - unreachable!() - } - either::Left(Some(header)) => { - let header = match methods::Header::from_scale_encoded_header( - &header, - sync_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &log_target, - "`chain_subscribeFinalizedHeads` subscription has skipped \ - block due to undecodable header. Hash: {}. Error: {}", - HashDisplay(&header::hash_from_scale_encoded_header( - &header - )), - error, - ); - continue; - } - }; - - subscription - .send_notification( - methods::ServerToClient::chain_finalizedHead { - subscription: (&subscription_id).into(), - result: header, - }, - ) - .await; - } - either::Right(()) => { - break; - } - } - } - } - }, - ); - } - /// Handles a call to [`methods::MethodCall::payment_queryInfo`]. pub(super) async fn payment_query_info(self: &Arc, request: service::RequestProcess) { let methods::MethodCall::payment_queryInfo { From fd965d934d1dc2b2a19e08efd4347694f95f90c7 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 07:28:15 +0200 Subject: [PATCH 25/46] Fix runtime_version todo!() --- .../background/legacy_state_sub.rs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 51ac695469..b625d53857 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -246,10 +246,14 @@ async fn run(mut task: Task) { hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, - runtime_version: block - .new_runtime - .map(Arc::new) - .unwrap_or_else(|| todo!()), // TODO: + runtime_version: match block.new_runtime { + Some(r) => Arc::new(r), + None => pinned_blocks + .get(&block.parent_hash) + .unwrap() + .runtime_version + .clone(), + }, }, ); } @@ -291,7 +295,14 @@ async fn run(mut task: Task) { hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, - runtime_version: block.new_runtime.map(Arc::new).unwrap_or_else(|| todo!()), // TODO: + runtime_version: match block.new_runtime { + Some(r) => Arc::new(r), + None => pinned_blocks + .get(&block.parent_hash) + .unwrap() + .runtime_version + .clone(), + }, }, ); From 25c72d1d39359b7d71cd09277edc5ea9eae740bf Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 07:31:52 +0200 Subject: [PATCH 26/46] Remove obsolete TODO --- light-base/src/json_rpc_service/background/legacy_state_sub.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index b625d53857..56f4312a8b 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -83,7 +83,6 @@ pub(super) fn start_task( runtime_service, subscription: Subscription::NotCreated, requests_rx, - // TODO: all the subscriptions are dropped if the task returns all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( 8, Default::default(), From 679318b72f7bd1f11bb796f3c3725c206c0c3416 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 07:42:49 +0200 Subject: [PATCH 27/46] Keep current best and finalized and report them immediately --- .../background/legacy_state_sub.rs | 180 ++++++++++++++---- 1 file changed, 144 insertions(+), 36 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 56f4312a8b..645fb6a0e0 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -146,6 +146,14 @@ enum Subscription { /// Object representing the subscription. subscription: runtime_service::Subscription, + /// Hash of the current best block. Guaranteed to be in + /// [`Subscription::Active::pinned_blocks`]. + current_best_block: [u8; 32], + + /// Hash of the current finalized block. Guaranteed to be in + /// [`Subscription::Active::pinned_blocks`]. + current_finalized_block: [u8; 32], + /// When the runtime service reports a new block, it is kept pinned and inserted in this /// list. /// @@ -178,6 +186,8 @@ async fn run(mut task: Task) { pinned_blocks: &'a mut hashbrown::HashMap<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, finalized_and_pruned_lru: &'a mut lru::LruCache<[u8; 32], (), fnv::FnvBuildHasher>, + current_best_block: &'a mut [u8; 32], + current_finalized_block: &'a mut [u8; 32], }, SubscriptionDead, SubscriptionReady(runtime_service::SubscribeAll), @@ -193,12 +203,16 @@ async fn run(mut task: Task) { subscription, pinned_blocks, finalized_and_pruned_lru, + current_best_block, + current_finalized_block, } => match subscription.next().await { Some(notification) => WhatHappened::SubscriptionNotification { notification, subscription, pinned_blocks, finalized_and_pruned_lru, + current_best_block, + current_finalized_block, }, None => WhatHappened::SubscriptionDead, }, @@ -239,6 +253,8 @@ async fn run(mut task: Task) { ); finalized_and_pruned_lru.put(finalized_block_hash, ()); + let mut current_best_block = finalized_block_hash; + for block in subscribe_all.non_finalized_blocks_ancestry_order { let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); pinned_blocks.insert( @@ -255,12 +271,18 @@ async fn run(mut task: Task) { }, }, ); + + if block.is_new_best { + current_best_block = hash; + } } task.subscription = Subscription::Active { subscription: subscribe_all.new_blocks, pinned_blocks, finalized_and_pruned_lru, + current_best_block, + current_finalized_block: finalized_block_hash, }; } @@ -331,12 +353,16 @@ async fn run(mut task: Task) { runtime_service::Notification::Finalized { hash: finalized_hash, pruned_blocks, - best_block_hash, + best_block_hash: new_best_block_hash, }, pinned_blocks, finalized_and_pruned_lru, subscription, + current_best_block, + current_finalized_block, } => { + *current_finalized_block = finalized_hash; + let finalized_block_header = &pinned_blocks .get(&finalized_hash) .unwrap() @@ -350,9 +376,9 @@ async fn run(mut task: Task) { Err(error) => { log::warn!( target: &task.log_target, - "`chain_subscribeNewHeads` subscription has skipped block due to \ - undecodable header. Hash: {}. Error: {}", - HashDisplay(&best_block_hash), + "`chain_subscribeFinalizedHeads` subscription has skipped block \ + due to undecodable header. Hash: {}. Error: {}", + HashDisplay(&new_best_block_hash), error, ); continue; @@ -368,6 +394,10 @@ async fn run(mut task: Task) { .await; } + // An important detail here is that the newly-finalized block is added to the list + // at the end, in order to guaranteed that it doesn't get removed. This is + // necessary in order to guarantee that the current finalized (and current best, + // if the best block is also the finalized block) remains pinned. for block_hash in pruned_blocks.into_iter().chain(iter::once(finalized_hash)) { if finalized_and_pruned_lru.len() == finalized_and_pruned_lru.cap().get() { let (hash_to_unpin, _) = finalized_and_pruned_lru.pop_lru().unwrap(); @@ -377,35 +407,39 @@ async fn run(mut task: Task) { finalized_and_pruned_lru.put(block_hash, ()); } - // TODO: skip below if best block didn't change - let best_block_header = &pinned_blocks - .get(&best_block_hash) - .unwrap() - .scale_encoded_header; - let best_block_json_rpc_header = match methods::Header::from_scale_encoded_header( - &best_block_header, - task.runtime_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &task.log_target, - "`chain_subscribeNewHeads` subscription has skipped block due to \ - undecodable header. Hash: {}. Error: {}", - HashDisplay(&best_block_hash), - error, - ); - continue; - } - }; + if *current_best_block != new_best_block_hash { + *current_best_block = new_best_block_hash; + + let best_block_header = &pinned_blocks + .get(&new_best_block_hash) + .unwrap() + .scale_encoded_header; + let best_block_json_rpc_header = + match methods::Header::from_scale_encoded_header( + &best_block_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeNewHeads` subscription has skipped block due to \ + undecodable header. Hash: {}. Error: {}", + HashDisplay(&new_best_block_hash), + error, + ); + continue; + } + }; - for (subscription_id, subscription) in &mut task.new_heads_subscriptions { - subscription - .send_notification(methods::ServerToClient::chain_newHead { - subscription: subscription_id.as_str().into(), - result: best_block_json_rpc_header.clone(), - }) - .await; + for (subscription_id, subscription) in &mut task.new_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_newHead { + subscription: subscription_id.as_str().into(), + result: best_block_json_rpc_header.clone(), + }) + .await; + } } } @@ -450,16 +484,90 @@ async fn run(mut task: Task) { .insert(subscription_id, subscription); } methods::MethodCall::chain_subscribeNewHeads {} => { - let subscription = request.accept(); + let mut subscription = request.accept(); let subscription_id = subscription.subscription_id().to_owned(); - // TODO: must immediately send the current best block + let to_send = if let Subscription::Active { + current_best_block, + pinned_blocks, + .. + } = &task.subscription + { + Some( + match methods::Header::from_scale_encoded_header( + &pinned_blocks + .get(current_best_block) + .unwrap() + .scale_encoded_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeNewHeads` subscription has skipped \ + block due to undecodable header. Hash: {}. Error: {}", + HashDisplay(current_best_block), + error, + ); + continue; + } + }, + ) + } else { + None + }; + if let Some(to_send) = to_send { + subscription + .send_notification(methods::ServerToClient::chain_newHead { + subscription: subscription_id.as_str().into(), + result: to_send, + }) + .await; + } task.new_heads_subscriptions .insert(subscription_id, subscription); } methods::MethodCall::chain_subscribeFinalizedHeads {} => { - let subscription = request.accept(); + let mut subscription = request.accept(); let subscription_id = subscription.subscription_id().to_owned(); - // TODO: must immediately send the current finalized block + let to_send = if let Subscription::Active { + current_finalized_block, + pinned_blocks, + .. + } = &task.subscription + { + Some( + match methods::Header::from_scale_encoded_header( + &pinned_blocks + .get(current_finalized_block) + .unwrap() + .scale_encoded_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeFinalizedHeads` subscription has skipped \ + block due to undecodable header. Hash: {}. Error: {}", + HashDisplay(current_finalized_block), + error, + ); + continue; + } + }, + ) + } else { + None + }; + if let Some(to_send) = to_send { + subscription + .send_notification(methods::ServerToClient::chain_finalizedHead { + subscription: subscription_id.as_str().into(), + result: to_send, + }) + .await; + } task.finalized_heads_subscriptions .insert(subscription_id, subscription); } From 7ec026a84a08bb95371d47901e225d1749161876 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 07:45:08 +0200 Subject: [PATCH 28/46] Fix unused imports and variables --- light-base/src/json_rpc_service/background.rs | 2 -- .../src/json_rpc_service/background/legacy_state_sub.rs | 4 +--- light-base/src/json_rpc_service/background/state_chain.rs | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 6cf7490eff..80c5b4aa0d 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -38,13 +38,11 @@ use core::{ time::Duration, }; use futures_channel::oneshot; -use futures_util::{future, FutureExt as _}; use smoldot::{ executor::{host, runtime_host}, header, json_rpc::{self, methods, service}, libp2p::{multiaddr, PeerId}, - network::protocol, }; mod chain_head; diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 645fb6a0e0..21baf5e7f2 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -24,10 +24,9 @@ use core::{ }; use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc, vec::Vec}; -use async_lock::Mutex; use futures_channel::oneshot; use futures_lite::{FutureExt as _, StreamExt as _}; -use futures_util::{future, stream::AbortRegistration, FutureExt as _}; +use futures_util::{future, FutureExt as _}; use smoldot::{ executor, header, informant::HashDisplay, @@ -288,7 +287,6 @@ async fn run(mut task: Task) { WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::Block(block), - subscription, pinned_blocks, .. } => { diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 491b1bd2d4..31af7bb70b 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -27,7 +27,6 @@ use futures_channel::oneshot; use futures_util::{future, stream, StreamExt as _}; use smoldot::{ header, - informant::HashDisplay, json_rpc::{self, methods, service}, network::protocol, }; From 6323c4466c40f006aa7a8284963e446cf97fc8d6 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 08:03:33 +0200 Subject: [PATCH 29/46] Ask the best block from the new task instead of using sub_utils --- .../background/legacy_state_sub.rs | 29 ++++ .../background/state_chain.rs | 159 +++++++++++++----- 2 files changed, 149 insertions(+), 39 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 21baf5e7f2..54911a3414 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -47,6 +47,9 @@ pub(super) enum Message { block_hash: [u8; 32], result_tx: oneshot::Sender>>, }, + CurrentBestBlockHash { + result_tx: oneshot::Sender<[u8; 32]>, + }, BlockStateRootAndNumber { block_hash: [u8; 32], result_tx: oneshot::Sender>, @@ -78,6 +81,7 @@ pub(super) fn start_task( ), log_target: log_target.clone(), platform: platform.clone(), + best_block_report: Vec::with_capacity(4), sync_service, runtime_service, subscription: Subscription::NotCreated, @@ -127,6 +131,8 @@ struct Task { log_target: String, platform: TPlat, + // TODO: shrink_to_fit? + best_block_report: Vec>, sync_service: Arc>, runtime_service: Arc>, subscription: Subscription, @@ -178,6 +184,16 @@ struct RecentBlock { async fn run(mut task: Task) { loop { + if let Subscription::Active { + current_best_block, .. + } = &task.subscription + { + while let Some(sender) = task.best_block_report.pop() { + let _ = sender.send(*current_best_block); + } + task.best_block_report.shrink_to_fit(); + } + enum WhatHappened<'a, TPlat: PlatformRef> { SubscriptionNotification { notification: runtime_service::Notification, @@ -612,6 +628,19 @@ async fn run(mut task: Task) { let _ = result_tx.send(access); } + WhatHappened::Message(Message::CurrentBestBlockHash { result_tx }) => { + match &task.subscription { + Subscription::Active { + current_best_block, .. + } => { + let _ = result_tx.send(*current_best_block); + } + Subscription::Pending(_) | Subscription::NotCreated => { + task.best_block_report.push(result_tx); + } + } + } + WhatHappened::Message(Message::BlockNumber { block_hash, result_tx, diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 31af7bb70b..ceeb39b1fa 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -40,9 +40,16 @@ impl Background { unreachable!() }; - let block_hash = header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ); + let block_hash = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + }; let result = self .runtime_call( @@ -89,9 +96,16 @@ impl Background { // `hash` equal to `None` means "the current best block". let hash = match hash { Some(h) => h.0, - None => header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ), + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } }; // Try to determine the block number by looking for the block in cache. @@ -185,9 +199,17 @@ impl Background { methods::HashHexString(self.genesis_block_hash), )), None => { - let best_block = header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ); + let best_block = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + }; + request.respond(methods::Response::chain_getBlockHash( methods::HashHexString(best_block), )); @@ -211,9 +233,16 @@ impl Background { // `hash` equal to `None` means "best block". let hash = match hash { Some(h) => h.0, - None => header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ), + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } }; // Try to look in the cache of recent blocks. If not found, ask the peer-to-peer network. @@ -337,9 +366,16 @@ impl Background { let block_hash = match block_hash { Some(h) => h.0, - None => header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ), + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } }; let result = self @@ -395,9 +431,14 @@ impl Background { let block_hash = if let Some(hash) = hash { hash.0 } else { - header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ) + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() }; let result = self @@ -431,9 +472,16 @@ impl Background { // `hash` equal to `None` means "best block". let hash = match hash { Some(h) => h.0, - None => header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ), + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } }; // Obtain the state trie root and height of the requested block. @@ -514,9 +562,16 @@ impl Background { // `hash` equal to `None` means "best block". let hash = match hash { Some(h) => h.0, - None => header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ), + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } }; // A prefix of `None` means "empty". @@ -636,9 +691,14 @@ impl Background { let block_hash = if let Some(hash) = hash { hash.0 } else { - header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ) + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() }; let result = self @@ -691,9 +751,16 @@ impl Background { let block_hash = match block_hash { Some(h) => h.0, - None => header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - ), + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } }; match self @@ -736,12 +803,19 @@ impl Background { unreachable!() }; - let hash = hash - .as_ref() - .map(|h| h.0) - .unwrap_or(header::hash_from_scale_encoded_header( - sub_utils::subscribe_best(&self.runtime_service).await.0, - )); + let hash = match hash { + Some(h) => h.0, + None => { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + } + }; let fut = self.storage_query( iter::once(&key.0), @@ -769,9 +843,16 @@ impl Background { unreachable!() }; - let best_block = header::hash_from_scale_encoded_header( - &sub_utils::subscribe_best(&self.runtime_service).await.0, - ); + let best_block = { + let (tx, rx) = oneshot::channel(); + self.to_legacy + .lock() + .await + .send(legacy_state_sub::Message::CurrentBestBlockHash { result_tx: tx }) + .await + .unwrap(); + rx.await.unwrap() + }; let at = at.as_ref().map(|h| h.0).unwrap_or(best_block); From 827445f933478173f1917be63b10793f741b9880 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 09:07:43 +0200 Subject: [PATCH 30/46] Move runtime subscription to new task --- light-base/src/json_rpc_service/background.rs | 3 +- .../background/legacy_state_sub.rs | 144 +++++++++++++++++- wasm-node/CHANGELOG.md | 1 + 3 files changed, 142 insertions(+), 6 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 80c5b4aa0d..e498c18a17 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -186,7 +186,8 @@ pub(super) fn start( match subscription_start.request() { methods::MethodCall::chain_subscribeAllHeads {} | methods::MethodCall::chain_subscribeNewHeads {} - | methods::MethodCall::chain_subscribeFinalizedHeads {} => { + | methods::MethodCall::chain_subscribeFinalizedHeads {} + | methods::MethodCall::state_subscribeRuntimeVersion {} => { me.to_legacy .lock() .await diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 54911a3414..76cd8d9452 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -98,6 +98,10 @@ pub(super) fn start_task( 8, Default::default(), ), + runtime_version_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 8, + Default::default(), + ), })), ); } @@ -144,6 +148,9 @@ struct Task { // TODO: shrink_to_fit? finalized_heads_subscriptions: hashbrown::HashMap, + // TODO: shrink_to_fit? + runtime_version_subscriptions: + hashbrown::HashMap, } enum Subscription { @@ -304,6 +311,7 @@ async fn run(mut task: Task) { WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::Block(block), pinned_blocks, + current_best_block, .. } => { let json_rpc_header = match methods::Header::from_scale_encoded_header( @@ -359,6 +367,28 @@ async fn run(mut task: Task) { }) .await; } + + let new_best_runtime = &pinned_blocks.get(&hash).unwrap().runtime_version; + if !Arc::ptr_eq( + new_best_runtime, + &pinned_blocks + .get(current_best_block) + .unwrap() + .runtime_version, + ) { + for (subscription_id, subscription) in + &mut task.runtime_version_subscriptions + { + subscription + .send_notification(methods::ServerToClient::state_runtimeVersion { + subscription: subscription_id.as_str().into(), + result: convert_runtime_version(new_best_runtime), + }) + .await; + } + } + + *current_best_block = hash; } } @@ -422,8 +452,6 @@ async fn run(mut task: Task) { } if *current_best_block != new_best_block_hash { - *current_best_block = new_best_block_hash; - let best_block_header = &pinned_blocks .get(&new_best_block_hash) .unwrap() @@ -454,15 +482,48 @@ async fn run(mut task: Task) { }) .await; } + + let new_best_runtime = &pinned_blocks + .get(&new_best_block_hash) + .unwrap() + .runtime_version; + if !Arc::ptr_eq( + new_best_runtime, + &pinned_blocks + .get(current_best_block) + .unwrap() + .runtime_version, + ) { + for (subscription_id, subscription) in + &mut task.runtime_version_subscriptions + { + subscription + .send_notification(methods::ServerToClient::state_runtimeVersion { + subscription: subscription_id.as_str().into(), + result: convert_runtime_version(new_best_runtime), + }) + .await; + } + } + + *current_best_block = new_best_block_hash; } } WhatHappened::SubscriptionNotification { - notification: runtime_service::Notification::BestBlockChanged { hash, .. }, + notification: + runtime_service::Notification::BestBlockChanged { + hash: new_best_hash, + .. + }, pinned_blocks, + current_best_block, .. } => { - let header = &pinned_blocks.get(&hash).unwrap().scale_encoded_header; + let header = &pinned_blocks + .get(&new_best_hash) + .unwrap() + .scale_encoded_header; let json_rpc_header = match methods::Header::from_scale_encoded_header( &header, task.runtime_service.block_number_bytes(), @@ -473,7 +534,7 @@ async fn run(mut task: Task) { target: &task.log_target, "`chain_subscribeNewHeads` subscription has skipped block due to \ undecodable header. Hash: {}. Error: {}", - HashDisplay(&hash), + HashDisplay(&new_best_hash), error, ); continue; @@ -488,6 +549,26 @@ async fn run(mut task: Task) { }) .await; } + + let new_best_runtime = &pinned_blocks.get(&new_best_hash).unwrap().runtime_version; + if !Arc::ptr_eq( + new_best_runtime, + &pinned_blocks + .get(current_best_block) + .unwrap() + .runtime_version, + ) { + for (subscription_id, subscription) in &mut task.runtime_version_subscriptions { + subscription + .send_notification(methods::ServerToClient::state_runtimeVersion { + subscription: subscription_id.as_str().into(), + result: convert_runtime_version(new_best_runtime), + }) + .await; + } + } + + *current_best_block = new_best_hash; } WhatHappened::Message(Message::SubscriptionStart(request)) => match request.request() { @@ -585,6 +666,35 @@ async fn run(mut task: Task) { task.finalized_heads_subscriptions .insert(subscription_id, subscription); } + methods::MethodCall::state_subscribeRuntimeVersion {} => { + let mut subscription = request.accept(); + let subscription_id = subscription.subscription_id().to_owned(); + let to_send = if let Subscription::Active { + current_best_block, + pinned_blocks, + .. + } = &task.subscription + { + Some(convert_runtime_version( + &pinned_blocks + .get(current_best_block) + .unwrap() + .runtime_version, + )) + } else { + None + }; + if let Some(to_send) = to_send { + subscription + .send_notification(methods::ServerToClient::state_runtimeVersion { + subscription: (&subscription_id).into(), + result: to_send, + }) + .await; + } + task.runtime_version_subscriptions + .insert(subscription_id, subscription); + } _ => unreachable!(), // TODO: stronger typing to avoid this? }, @@ -592,6 +702,7 @@ async fn run(mut task: Task) { task.all_heads_subscriptions.remove(&subscription_id); task.new_heads_subscriptions.remove(&subscription_id); task.finalized_heads_subscriptions.remove(&subscription_id); + task.runtime_version_subscriptions.remove(&subscription_id); // TODO: shrink_to_fit? } @@ -836,3 +947,26 @@ async fn run(mut task: Task) { } } } + +fn convert_runtime_version( + runtime: &Arc>, +) -> Option { + if let Ok(runtime_spec) = &**runtime { + let runtime_spec = runtime_spec.decode(); + Some(methods::RuntimeVersion { + spec_name: runtime_spec.spec_name.into(), + impl_name: runtime_spec.impl_name.into(), + authoring_version: u64::from(runtime_spec.authoring_version), + spec_version: u64::from(runtime_spec.spec_version), + impl_version: u64::from(runtime_spec.impl_version), + transaction_version: runtime_spec.transaction_version.map(u64::from), + state_version: runtime_spec.state_version.map(u8::from).map(u64::from), + apis: runtime_spec + .apis + .map(|api| (methods::HexString(api.name_hash.to_vec()), api.version)) + .collect(), + }) + } else { + None + } +} diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index 6f6f29f6ec..02274f9558 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -6,6 +6,7 @@ - The `chainHead_unstable_storage` JSON-RPC function now supports a `type` equal to `closest-descendant-merkle-value` and no longer supports `closest-ancestor-merkle-value`, in accordance with the latest changes in the JSON-RPC API specification. ([#824](https://github.com/smol-dot/smoldot/pull/824)) - Blocks are now reported to `chain_subscribeAllHeads` and `chain_subscribeNewHeads` subscribers only after they have been put in the cache, preventing race conditions where JSON-RPC clients suffer from a cache miss if they ask information about these blocks too quickly. +- Runtime updates are now always reported to `state_subscribeRuntimeVersion` subscribers immediately after the `chain_subscribeNewHeads` notification corresponding to the block containing the runtime update. They were previously reported in a pseudo-random order. ## 1.0.11 - 2023-06-25 From 61a35c012904c26c9aa0cf968e34c8fdcb517338 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 09:12:58 +0200 Subject: [PATCH 31/46] Remove unused function --- light-base/src/json_rpc_service/background.rs | 2 +- .../background/state_chain.rs | 93 ------------------- 2 files changed, 1 insertion(+), 94 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index e498c18a17..9043425fae 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -654,7 +654,7 @@ impl Background { unreachable!() } methods::MethodCall::state_subscribeRuntimeVersion {} => { - self.state_subscribe_runtime_version(request).await; + unreachable!() } methods::MethodCall::state_subscribeStorage { .. } => { self.state_subscribe_storage(request).await; diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index ceeb39b1fa..f585dc70c2 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -878,99 +878,6 @@ impl Background { request.respond(methods::Response::state_queryStorageAt(vec![out])); } - /// Handles a call to [`methods::MethodCall::state_subscribeRuntimeVersion`]. - pub(super) async fn state_subscribe_runtime_version( - self: &Arc, - request: service::SubscriptionStartProcess, - ) { - let methods::MethodCall::state_subscribeRuntimeVersion {} = request.request() else { - unreachable!() - }; - - let runtime_service = self.runtime_service.clone(); - - self.platform.spawn_task( - format!("{}-subscribe-runtime-version", self.log_target).into(), - async move { - let mut subscription = request.accept(); - let subscription_id = subscription.subscription_id().to_owned(); - - let (current_spec, spec_changes) = - sub_utils::subscribe_runtime_version(&runtime_service).await; - let mut spec_changes = - pin::pin!(stream::iter(iter::once(current_spec)).chain(spec_changes)); - - loop { - let event = { - let unsubscribed = pin::pin!(subscription.wait_until_stale()); - match future::select(spec_changes.next(), unsubscribed).await { - future::Either::Left((ev, _)) => either::Left(ev), - future::Either::Right((ev, _)) => either::Right(ev), - } - }; - - match event { - either::Left(None) => { - // Stream returned by `subscribe_runtime_version` is always unlimited. - unreachable!() - } - either::Left(Some(new_runtime)) => { - if let Ok(runtime_spec) = new_runtime { - let runtime_spec = runtime_spec.decode(); - subscription - .send_notification( - methods::ServerToClient::state_runtimeVersion { - subscription: (&subscription_id).into(), - result: Some(methods::RuntimeVersion { - spec_name: runtime_spec.spec_name.into(), - impl_name: runtime_spec.impl_name.into(), - authoring_version: u64::from( - runtime_spec.authoring_version, - ), - spec_version: u64::from(runtime_spec.spec_version), - impl_version: u64::from(runtime_spec.impl_version), - transaction_version: runtime_spec - .transaction_version - .map(u64::from), - state_version: runtime_spec - .state_version - .map(u8::from) - .map(u64::from), - apis: runtime_spec - .apis - .map(|api| { - ( - methods::HexString( - api.name_hash.to_vec(), - ), - api.version, - ) - }) - .collect(), - }), - }, - ) - .await; - } else { - subscription - .send_notification( - methods::ServerToClient::state_runtimeVersion { - subscription: (&subscription_id).into(), - result: None, - }, - ) - .await; - } - } - either::Right(()) => { - break; - } - } - } - }, - ); - } - /// Handles a call to [`methods::MethodCall::state_subscribeStorage`]. pub(super) async fn state_subscribe_storage( self: &Arc, From f6351e586791a87be3394e2d1b31f79aff8f6b62 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 11:28:22 +0200 Subject: [PATCH 32/46] Move `state_subscribeStorage` to new task --- light-base/src/json_rpc_service/background.rs | 6 +- .../background/legacy_state_sub.rs | 247 +++++++++++++++++- 2 files changed, 244 insertions(+), 9 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 9043425fae..511572c071 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -148,7 +148,7 @@ pub(super) fn start( sync_service: config.sync_service.clone(), runtime_service: config.runtime_service.clone(), transactions_service: config.transactions_service.clone(), - to_legacy: Mutex::new(to_legacy_tx), + to_legacy: Mutex::new(to_legacy_tx.clone()), state_get_keys_paged_cache: Mutex::new(lru::LruCache::with_hasher( NonZeroUsize::new(2).unwrap(), util::SipHasherBuild::new(rand::random()), @@ -187,7 +187,8 @@ pub(super) fn start( methods::MethodCall::chain_subscribeAllHeads {} | methods::MethodCall::chain_subscribeNewHeads {} | methods::MethodCall::chain_subscribeFinalizedHeads {} - | methods::MethodCall::state_subscribeRuntimeVersion {} => { + | methods::MethodCall::state_subscribeRuntimeVersion {} + | methods::MethodCall::state_subscribeStorage { .. } => { me.to_legacy .lock() .await @@ -256,6 +257,7 @@ pub(super) fn start( me.log_target.clone(), me.sync_service.clone(), me.runtime_service.clone(), + to_legacy_tx, to_legacy_rx, ); } diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 76cd8d9452..15ca4d3e5f 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -23,14 +23,17 @@ use core::{ time::Duration, }; -use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc, vec::Vec}; +use alloc::{ + borrow::ToOwned as _, boxed::Box, collections::BTreeSet, format, string::String, sync::Arc, + vec::Vec, +}; use futures_channel::oneshot; use futures_lite::{FutureExt as _, StreamExt as _}; use futures_util::{future, FutureExt as _}; use smoldot::{ executor, header, informant::HashDisplay, - json_rpc::{methods, service}, + json_rpc::{self, methods, service}, network::protocol, }; @@ -62,16 +65,24 @@ pub(super) enum Message { block_hash: [u8; 32], result_tx: oneshot::Sender>>, }, + StorageFetch { + block_hash: [u8; 32], + result: Result, sync_service::StorageQueryError>, + }, } // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. +// TODO: weird to pass both the sender and receiver pub(super) fn start_task( platform: TPlat, log_target: String, sync_service: Arc>, runtime_service: Arc>, + requests_tx: async_channel::Sender>, requests_rx: async_channel::Receiver>, ) { + let requests_tx = async_channel::Sender::downgrade(&requests_tx); + platform.clone().spawn_task( format!("{}-cache", log_target).into(), Box::pin(run(Task { @@ -85,23 +96,37 @@ pub(super) fn start_task( sync_service, runtime_service, subscription: Subscription::NotCreated, + requests_tx, requests_rx, all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, + 2, Default::default(), ), new_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, + 2, Default::default(), ), finalized_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, + 2, Default::default(), ), runtime_version_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( - 8, + 2, + Default::default(), + ), + storage_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( + 16, Default::default(), ), + storage_subscriptions_by_key: hashbrown::HashMap::with_capacity_and_hasher( + 16, + crate::util::SipHasherBuild::new([0; 16]), // TODO: proper seed + ), + stale_storage_subscriptions: hashbrown::HashSet::with_capacity_and_hasher( + 16, + Default::default(), + ), + storage_query_in_progress: false, })), ); } @@ -140,6 +165,8 @@ struct Task { sync_service: Arc>, runtime_service: Arc>, subscription: Subscription, + /// Sending side of [`Task::requests_rx`]. + requests_tx: async_channel::WeakSender>, requests_rx: async_channel::Receiver>, // TODO: shrink_to_fit? all_heads_subscriptions: hashbrown::HashMap, @@ -151,6 +178,22 @@ struct Task { // TODO: shrink_to_fit? runtime_version_subscriptions: hashbrown::HashMap, + // TODO: shrink_to_fit? + storage_subscriptions: + hashbrown::HashMap>), fnv::FnvBuildHasher>, + // TODO: shrink_to_fit? + storage_subscriptions_by_key: hashbrown::HashMap< + Vec, + hashbrown::HashSet, + crate::util::SipHasherBuild, + >, + /// List of storage subscriptions whose latest sent notification isn't about the current + /// best block. + stale_storage_subscriptions: hashbrown::HashSet, + + /// `true` if there exists a background task currently fetching storage items for storage + /// subscriptions. This task will send a [`Message::StorageFetch`] once it's finished. + storage_query_in_progress: bool, } enum Subscription { @@ -192,13 +235,81 @@ struct RecentBlock { async fn run(mut task: Task) { loop { if let Subscription::Active { - current_best_block, .. + pinned_blocks, + current_best_block, + .. } = &task.subscription { while let Some(sender) = task.best_block_report.pop() { let _ = sender.send(*current_best_block); } task.best_block_report.shrink_to_fit(); + + if !task.storage_query_in_progress && !task.stale_storage_subscriptions.is_empty() { + let (block_number, state_trie_root) = match header::decode( + &pinned_blocks + .get(current_best_block) + .unwrap() + .scale_encoded_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(header) => (header.number, *header.state_root), + Err(_) => { + // Can't decode the header of the current best block. + // All the subscriptions are marked as non-stale, since they are up-to-date + // with the current best block. + // TODO: print warning? + task.stale_storage_subscriptions.clear(); + continue; + } + }; + + task.storage_query_in_progress = true; + + let mut keys = + hashbrown::HashSet::with_hasher(crate::util::SipHasherBuild::new([0; 16])); // TODO: proper seed + keys.extend( + task.stale_storage_subscriptions + .iter() + .map(|s_id| &task.storage_subscriptions.get(s_id).unwrap().1) + .flat_map(|keys_list| keys_list.iter().cloned()), + ); + + task.platform.spawn_task( + format!("{}-storage-subscriptions-fetch", task.log_target).into(), + Box::pin({ + let block_hash = current_best_block.clone(); + let sync_service = task.sync_service.clone(); + // TODO: a bit overcomplicated because `WeakSender` doesn't implement `Clone`: https://github.com/smol-rs/async-channel/pull/62 + let requests_tx = async_channel::Sender::downgrade( + &task.requests_tx.upgrade().unwrap().clone(), + ); + async move { + let result = sync_service + .clone() + .storage_query( + block_number, + &block_hash, + &state_trie_root, + keys.into_iter() + .map(|key| sync_service::StorageRequestItem { + key, + ty: sync_service::StorageRequestItemTy::Value, + }), + 4, + Duration::from_secs(12), + NonZeroU32::new(2).unwrap(), + ) + .await; + if let Some(requests_tx) = requests_tx.upgrade() { + let _ = requests_tx + .send(Message::StorageFetch { block_hash, result }) + .await; + } + } + }), + ); + } } enum WhatHappened<'a, TPlat: PlatformRef> { @@ -299,6 +410,9 @@ async fn run(mut task: Task) { } } + task.stale_storage_subscriptions + .extend(task.storage_subscriptions.keys().cloned()); + task.subscription = Subscription::Active { subscription: subscribe_all.new_blocks, pinned_blocks, @@ -388,6 +502,9 @@ async fn run(mut task: Task) { } } + task.stale_storage_subscriptions + .extend(task.storage_subscriptions.keys().cloned()); + *current_best_block = hash; } } @@ -506,6 +623,9 @@ async fn run(mut task: Task) { } } + task.stale_storage_subscriptions + .extend(task.storage_subscriptions.keys().cloned()); + *current_best_block = new_best_block_hash; } } @@ -568,6 +688,9 @@ async fn run(mut task: Task) { } } + task.stale_storage_subscriptions + .extend(task.storage_subscriptions.keys().cloned()); + *current_best_block = new_best_hash; } @@ -695,6 +818,34 @@ async fn run(mut task: Task) { task.runtime_version_subscriptions .insert(subscription_id, subscription); } + methods::MethodCall::state_subscribeStorage { list } => { + // TODO: limit the size of `list` to avoid DoS attacks + if list.is_empty() { + // When the list of keys is empty, that means we want to subscribe to *all* + // storage changes. It is not possible to reasonably implement this in a + // light client. + request.fail(json_rpc::parse::ErrorResponse::ServerError( + -32000, + "Subscribing to all storage changes isn't supported", + )); + continue; + } + + let subscription = request.accept(); + let subscription_id = subscription.subscription_id().to_owned(); + task.stale_storage_subscriptions + .insert(subscription_id.clone()); + for key in &list { + task.storage_subscriptions_by_key + .entry(key.0.clone()) + .or_default() + .insert(subscription_id.clone()); + } + task.storage_subscriptions.insert( + subscription_id, + (subscription, list.into_iter().map(|l| l.0).collect()), + ); + } _ => unreachable!(), // TODO: stronger typing to avoid this? }, @@ -703,6 +854,18 @@ async fn run(mut task: Task) { task.new_heads_subscriptions.remove(&subscription_id); task.finalized_heads_subscriptions.remove(&subscription_id); task.runtime_version_subscriptions.remove(&subscription_id); + if let Some((_, keys)) = task.storage_subscriptions.remove(&subscription_id) { + for key in keys { + let hashbrown::hash_map::Entry::Occupied(mut entry) = task.storage_subscriptions_by_key.entry(key) + else { unreachable!() }; + let _was_in = entry.get_mut().remove(&subscription_id); + debug_assert!(_was_in); + if entry.get().is_empty() { + entry.remove(); + } + } + } + task.stale_storage_subscriptions.remove(&subscription_id); // TODO: shrink_to_fit? } @@ -921,6 +1084,76 @@ async fn run(mut task: Task) { }); } + WhatHappened::Message(Message::StorageFetch { + block_hash, + result: Ok(result), + }) => { + debug_assert!(task.storage_query_in_progress); + task.storage_query_in_progress = false; + + let is_up_to_date = match task.subscription { + Subscription::Active { + current_best_block, .. + } => current_best_block == block_hash, + Subscription::NotCreated | Subscription::Pending(_) => true, + }; + + let mut notifications_to_send = hashbrown::HashMap::< + String, + Vec<(methods::HexString, Option)>, + _, + >::with_capacity_and_hasher( + task.storage_subscriptions.len(), + fnv::FnvBuildHasher::default(), + ); + + for item in result { + let sync_service::StorageResultItem::Value { key, value } = item + else { unreachable!() }; + for subscription_id in task + .storage_subscriptions_by_key + .get(&key) + .into_iter() + .flat_map(|list| list.iter()) + { + notifications_to_send + .entry_ref(subscription_id) + .or_insert_with(Vec::new) + .push(( + methods::HexString(key.clone()), + value.clone().map(methods::HexString), + )); + } + } + + for (subscription_id, changes) in notifications_to_send { + if is_up_to_date { + task.stale_storage_subscriptions.remove(&subscription_id); + } + task.storage_subscriptions + .get_mut(&subscription_id) + .unwrap() + .0 + .send_notification(methods::ServerToClient::state_storage { + subscription: subscription_id.into(), + result: methods::StorageChangeSet { + block: methods::HashHexString(block_hash), + changes, + }, + }) + .await; + } + } + + WhatHappened::Message(Message::StorageFetch { + block_hash, + result: Err(_), + }) => { + debug_assert!(task.storage_query_in_progress); + task.storage_query_in_progress = false; + // TODO: add a delay or something? + } + WhatHappened::ForegroundDead => { break; } From 9b941ebb493155969f367574837ab64ac50fa87f Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 12:39:28 +0200 Subject: [PATCH 33/46] Update CHANGELOG --- wasm-node/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index 02274f9558..e2d040717c 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -7,6 +7,7 @@ - The `chainHead_unstable_storage` JSON-RPC function now supports a `type` equal to `closest-descendant-merkle-value` and no longer supports `closest-ancestor-merkle-value`, in accordance with the latest changes in the JSON-RPC API specification. ([#824](https://github.com/smol-dot/smoldot/pull/824)) - Blocks are now reported to `chain_subscribeAllHeads` and `chain_subscribeNewHeads` subscribers only after they have been put in the cache, preventing race conditions where JSON-RPC clients suffer from a cache miss if they ask information about these blocks too quickly. - Runtime updates are now always reported to `state_subscribeRuntimeVersion` subscribers immediately after the `chain_subscribeNewHeads` notification corresponding to the block containing the runtime update. They were previously reported in a pseudo-random order. +- All the storage subscriptions made using `state_subscribeStorage` are now queried together into a single networking request per block, instead of sending one networking query per storage key and per subscription. ## 1.0.11 - 2023-06-25 From 5658f95b94f40e88279ccd4b9127ea71cc8763d7 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 12:51:40 +0200 Subject: [PATCH 34/46] Small tweaks and restore logging incoming requests --- light-base/src/json_rpc_service.rs | 19 +++++++++++++++++-- light-base/src/json_rpc_service/background.rs | 8 -------- .../background/legacy_state_sub.rs | 1 - 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/light-base/src/json_rpc_service.rs b/light-base/src/json_rpc_service.rs index 811dabee0b..9eb908a44b 100644 --- a/light-base/src/json_rpc_service.rs +++ b/light-base/src/json_rpc_service.rs @@ -43,7 +43,11 @@ use crate::{ network_service, platform::PlatformRef, runtime_service, sync_service, transactions_service, }; -use alloc::{format, string::String, sync::Arc}; +use alloc::{ + format, + string::{String, ToString as _}, + sync::Arc, +}; use core::num::{NonZeroU32, NonZeroUsize}; use smoldot::{ chain_spec, @@ -135,11 +139,22 @@ impl Frontend { /// isn't called often enough. Use [`HandleRpcError::into_json_rpc_error`] to build the /// JSON-RPC response to immediately send back to the user. pub fn queue_rpc_request(&self, json_rpc_request: String) -> Result<(), HandleRpcError> { + let log_friendly_request = + crate::util::truncated_str(json_rpc_request.chars().filter(|c| !c.is_control()), 100) + .to_string(); + match self .requests_responses_io .try_send_request(json_rpc_request) { - Ok(()) => Ok(()), + Ok(()) => { + log::debug!( + target: &self.log_target, + "JSON-RPC => {}", + log_friendly_request + ); + Ok(()) + } Err(service::TrySendRequestError { cause: service::TrySendRequestErrorCause::TooManyPendingRequests, request, diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 511572c071..80ebb7f681 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -265,14 +265,6 @@ pub(super) fn start( impl Background { /// Pulls one request from the inner state machine, and processes it. async fn handle_request(self: &Arc, request: service::RequestProcess) { - // TODO: restore some form of logging - /*log::debug!(target: &self.log_target, "PendingRequestsQueue => {}", - crate::util::truncated_str( - json_rpc_request.chars().filter(|c| !c.is_control()), - 100, - ) - );*/ - // Print a warning for legacy JSON-RPC functions. match request.request() { methods::MethodCall::account_nextIndex { .. } diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 15ca4d3e5f..7b0f9182e5 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -160,7 +160,6 @@ struct Task { log_target: String, platform: TPlat, - // TODO: shrink_to_fit? best_block_report: Vec>, sync_service: Arc>, runtime_service: Arc>, From 210cefdf5a82ed7a194584656531ff23f2e99e33 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 12:54:29 +0200 Subject: [PATCH 35/46] Remove obsolete code --- light-base/src/json_rpc_service/background.rs | 18 +- .../background/state_chain.rs | 168 ------- .../background/state_chain/sub_utils.rs | 454 ------------------ 3 files changed, 5 insertions(+), 635 deletions(-) delete mode 100644 light-base/src/json_rpc_service/background/state_chain/sub_utils.rs diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 80ebb7f681..738ba73db6 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -638,21 +638,13 @@ impl Background { methods::MethodCall::author_submitAndWatchExtrinsic { .. } => { self.submit_and_watch_transaction(request).await } - methods::MethodCall::chain_subscribeAllHeads {} => { + methods::MethodCall::chain_subscribeAllHeads {} + | methods::MethodCall::chain_subscribeFinalizedHeads {} + | methods::MethodCall::chain_subscribeNewHeads {} + | methods::MethodCall::state_subscribeRuntimeVersion {} + | methods::MethodCall::state_subscribeStorage { .. } => { unreachable!() } - methods::MethodCall::chain_subscribeFinalizedHeads {} => { - unreachable!() - } - methods::MethodCall::chain_subscribeNewHeads {} => { - unreachable!() - } - methods::MethodCall::state_subscribeRuntimeVersion {} => { - unreachable!() - } - methods::MethodCall::state_subscribeStorage { .. } => { - self.state_subscribe_storage(request).await; - } methods::MethodCall::chainHead_unstable_body { .. } => { self.chain_head_unstable_body(request).await; diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index f585dc70c2..9cdcdb362c 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -31,8 +31,6 @@ use smoldot::{ network::protocol, }; -mod sub_utils; - impl Background { /// Handles a call to [`methods::MethodCall::system_accountNextIndex`]. pub(super) async fn account_next_index(self: &Arc, request: service::RequestProcess) { @@ -877,170 +875,4 @@ impl Background { request.respond(methods::Response::state_queryStorageAt(vec![out])); } - - /// Handles a call to [`methods::MethodCall::state_subscribeStorage`]. - pub(super) async fn state_subscribe_storage( - self: &Arc, - request: service::SubscriptionStartProcess, - ) { - let methods::MethodCall::state_subscribeStorage { list } = request.request() else { - unreachable!() - }; - - if list.is_empty() { - // When the list of keys is empty, that means we want to subscribe to *all* - // storage changes. It is not possible to reasonably implement this in a - // light client. - request.fail(json_rpc::parse::ErrorResponse::ServerError( - -32000, - "Subscribing to all storage changes isn't supported", - )); - return; - } - - // Build a stream of `methods::StorageChangeSet` items to send back to the user. - let storage_updates = { - let known_values = (0..list.len()).map(|_| None).collect::>(); - let runtime_service = self.runtime_service.clone(); - let sync_service = self.sync_service.clone(); - let log_target = self.log_target.clone(); - - stream::unfold( - (None, list, known_values), - move |(mut blocks_stream, list, mut known_values)| { - let sync_service = sync_service.clone(); - let runtime_service = runtime_service.clone(); - let log_target = log_target.clone(); - async move { - loop { - if blocks_stream.is_none() { - // TODO: why is this done against the runtime_service and not the sync_service? clarify - let (block_header, blocks_subscription) = - sub_utils::subscribe_best(&runtime_service).await; - blocks_stream = Some( - stream::once(future::ready(block_header)) - .chain(blocks_subscription), - ); - } - - let block = match blocks_stream.as_mut().unwrap().next().await { - Some(b) => b, - None => { - blocks_stream = None; - continue; - } - }; - - let block_hash = header::hash_from_scale_encoded_header(&block); - let (state_trie_root, block_number) = { - let decoded = - header::decode(&block, sync_service.block_number_bytes()) - .unwrap(); - (decoded.state_root, decoded.number) - }; - - let mut out = methods::StorageChangeSet { - block: methods::HashHexString(block_hash), - changes: Vec::new(), - }; - - for (key_index, key) in list.iter().enumerate() { - // TODO: parallelism? - match sync_service - .clone() - .storage_query( - block_number, - &block_hash, - state_trie_root, - iter::once(sync_service::StorageRequestItem { - key: key.0.clone(), - ty: sync_service::StorageRequestItemTy::Value, - }), - 4, - Duration::from_secs(12), - NonZeroU32::new(2).unwrap(), - ) - .await - { - Ok(mut values) => { - let Some(sync_service::StorageResultItem::Value { - value, - .. - }) = values.pop() - else { - unreachable!() - }; - match &mut known_values[key_index] { - Some(v) if *v == value => {} - v => { - *v = Some(value.clone()); - out.changes.push(( - key.clone(), - value.map(methods::HexString), - )); - } - } - } - Err(error) => { - log::log!( - target: &log_target, - if error.is_network_problem() { - log::Level::Debug - } else { - log::Level::Warn - }, - "state_subscribeStorage changes check failed: {}", - error - ); - } - } - } - - if !out.changes.is_empty() { - return Some((out, (blocks_stream, list, known_values))); - } - } - } - }, - ) - }; - - self.platform.spawn_task( - format!("{}-subscribe-storage", self.log_target).into(), - async move { - let mut subscription = request.accept(); - let subscription_id = subscription.subscription_id().to_owned(); - - let mut storage_updates = pin::pin!(storage_updates); - - loop { - let event = { - let unsubscribed = pin::pin!(subscription.wait_until_stale()); - match future::select(storage_updates.next(), unsubscribed).await { - future::Either::Left((ev, _)) => either::Left(ev), - future::Either::Right((ev, _)) => either::Right(ev), - } - }; - - match event { - either::Left(None) => { - // Stream created above is always unlimited. - unreachable!() - } - either::Left(Some(changes)) => { - subscription - .send_notification(methods::ServerToClient::state_storage { - subscription: (&subscription_id).into(), - result: changes, - }) - .await; - } - either::Right(()) => { - break; - } - } - } - }, - ) - } } diff --git a/light-base/src/json_rpc_service/background/state_chain/sub_utils.rs b/light-base/src/json_rpc_service/background/state_chain/sub_utils.rs deleted file mode 100644 index c50daa140a..0000000000 --- a/light-base/src/json_rpc_service/background/state_chain/sub_utils.rs +++ /dev/null @@ -1,454 +0,0 @@ -// Smoldot -// Copyright (C) 2019-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 . - -//! This module contains useful features built on top of the [`RuntimeService`] that are only used -//! by the JSON-RPC service. - -use crate::{ - platform::PlatformRef, - runtime_service::{Notification, RuntimeError, RuntimeService}, -}; - -use alloc::{sync::Arc, vec::Vec}; -use core::num::NonZeroUsize; -use futures_util::{future, stream, StreamExt as _}; -use smoldot::{executor, header}; - -/// Returns the current runtime version, plus an unlimited stream that produces one item every -/// time the specs of the runtime of the best block are changed. -/// -/// The future returned by this function waits until the runtime is available. This can take -/// a long time. -/// -/// The stream can generate an `Err` if the runtime in the best block is invalid. -/// -/// The stream is infinite. In other words it is guaranteed to never return `None`. -pub async fn subscribe_runtime_version( - runtime_service: &Arc>, -) -> ( - Result, - stream::BoxStream<'static, Result>, -) { - let mut master_stream = stream::unfold(runtime_service.clone(), |runtime_service| async move { - let subscribe_all = runtime_service - .subscribe_all("subscribe-runtime-version", 16, NonZeroUsize::new(24).unwrap()) - .await; - - // Map of runtimes by hash. Contains all non-finalized blocks, plus the current finalized - // block. - let mut headers = hashbrown::HashMap::< - [u8; 32], - Arc>, - fnv::FnvBuildHasher, - >::with_capacity_and_hasher(16, Default::default()); - - let current_finalized_hash = header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - ); - subscribe_all - .new_blocks - .unpin_block(¤t_finalized_hash) - .await; - - headers.insert( - current_finalized_hash, - Arc::new(subscribe_all.finalized_block_runtime), - ); - - let mut current_best = None; - for block in subscribe_all.non_finalized_blocks_ancestry_order { - let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - subscribe_all.new_blocks.unpin_block(&hash).await; - - if let Some(new_runtime) = block.new_runtime { - headers.insert(hash, Arc::new(new_runtime)); - } else { - let parent_runtime = headers - .get(&block.parent_hash) - .unwrap() - .clone(); - headers.insert(hash, parent_runtime); - } - - if block.is_new_best { - debug_assert!(current_best.is_none()); - current_best = Some(hash); - } - } - let current_best = current_best.unwrap_or(current_finalized_hash); - let current_best_runtime = (**headers.get(¤t_best).unwrap()).clone(); - - // Turns `subscribe_all.new_blocks` into a stream of headers. - let substream = stream::unfold( - ( - subscribe_all.new_blocks, - headers, - current_finalized_hash, - current_best, - ), - |( - mut new_blocks, - mut headers, - mut current_finalized_hash, - mut current_best, - )| async move { - loop { - match new_blocks.next().await? { - Notification::Block(block) => { - let hash = - header::hash_from_scale_encoded_header(&block.scale_encoded_header); - new_blocks.unpin_block(&hash).await; - - if let Some(new_runtime) = block.new_runtime { - headers.insert(hash, Arc::new(new_runtime)); - } else { - let parent_runtime = headers - .get(&block.parent_hash) - .unwrap() - .clone(); - headers.insert(hash, parent_runtime); - } - - if block.is_new_best { - let current_best_runtime = - headers.get(¤t_best).unwrap(); - let new_best_runtime = headers.get(&hash).unwrap(); - current_best = hash; - - if !Arc::ptr_eq(current_best_runtime, new_best_runtime) { - let runtime = (**new_best_runtime).clone(); - break Some(( - runtime, - ( - new_blocks, - headers, - current_finalized_hash, - current_best, - ), - )); - } - } - } - Notification::Finalized { - hash, - pruned_blocks, - best_block_hash, - } => { - let current_best_runtime = - headers.get(¤t_best).unwrap().clone(); - let new_best_runtime = - headers.get(&best_block_hash).unwrap().clone(); - - // Clean up the headers we won't need anymore. - for pruned_block in pruned_blocks { - let _was_in = headers.remove(&pruned_block); - debug_assert!(_was_in.is_some()); - } - - let _ = headers - .remove(¤t_finalized_hash) - .unwrap(); - current_finalized_hash = hash; - current_best = best_block_hash; - - if !Arc::ptr_eq(¤t_best_runtime, &new_best_runtime) { - let runtime = (*new_best_runtime).clone(); - break Some(( - runtime, - ( - new_blocks, - headers, - current_finalized_hash, - current_best, - ), - )); - } - } - Notification::BestBlockChanged { hash } => { - let current_best_runtime = - headers.get(¤t_best).unwrap().clone(); - let new_best_runtime = - headers.get(&hash).unwrap().clone(); - - current_best = hash; - - if !Arc::ptr_eq(¤t_best_runtime, &new_best_runtime) { - let runtime = (*new_best_runtime).clone(); - break Some(( - runtime, - ( - new_blocks, - headers, - current_finalized_hash, - current_best, - ), - )); - } - } - } - } - }, - ); - - // Prepend the current best block to the stream. - let substream = stream::once(future::ready(current_best_runtime)).chain(substream); - Some((substream, runtime_service)) - }) - .flatten() - .boxed(); - - // TODO: we don't dedup blocks; in other words the stream can produce the same block twice if the inner subscription drops - - // Now that we have a stream, extract the first element to be the first value. - let first_value = master_stream.next().await.unwrap(); - (first_value, master_stream) -} - -/// Returns the SCALE-encoded header of the current finalized block, plus an unlimited stream -/// that produces one item every time the finalized block is changed. -/// -/// This function only returns once the runtime of the current finalized block is known. This -/// might take a long time. -pub async fn subscribe_finalized( - runtime_service: &Arc>, -) -> (Vec, stream::BoxStream<'static, Vec>) { - let mut master_stream = stream::unfold(runtime_service.clone(), |runtime_service| async move { - let subscribe_all = runtime_service - .subscribe_all("subscribe-finalized", 16, NonZeroUsize::new(32).unwrap()) - .await; - - // Map of block headers by hash. Contains all non-finalized blocks headers. - let mut non_finalized_headers = - hashbrown::HashMap::<[u8; 32], Vec, fnv::FnvBuildHasher>::with_capacity_and_hasher( - 16, - Default::default(), - ); - - subscribe_all - .new_blocks - .unpin_block(&header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - )) - .await; - - for block in subscribe_all.non_finalized_blocks_ancestry_order { - let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - subscribe_all.new_blocks.unpin_block(&hash).await; - non_finalized_headers.insert(hash, block.scale_encoded_header); - } - - // Turns `subscribe_all.new_blocks` into a stream of headers. - let substream = stream::unfold( - (subscribe_all.new_blocks, non_finalized_headers), - |(mut new_blocks, mut non_finalized_headers)| async { - loop { - match new_blocks.next().await? { - Notification::Block(block) => { - let hash = - header::hash_from_scale_encoded_header(&block.scale_encoded_header); - new_blocks.unpin_block(&hash).await; - non_finalized_headers.insert(hash, block.scale_encoded_header); - } - Notification::Finalized { - hash, - pruned_blocks, - .. - } => { - // Clean up the headers we won't need anymore. - for pruned_block in pruned_blocks { - let _was_in = non_finalized_headers.remove(&pruned_block); - debug_assert!(_was_in.is_some()); - } - - let header = non_finalized_headers.remove(&hash).unwrap(); - break Some((header, (new_blocks, non_finalized_headers))); - } - Notification::BestBlockChanged { .. } => {} - } - } - }, - ); - - // Prepend the current finalized block to the stream. - let substream = stream::once(future::ready( - subscribe_all.finalized_block_scale_encoded_header, - )) - .chain(substream); - - Some((substream, runtime_service)) - }) - .flatten() - .boxed(); - - // TODO: we don't dedup blocks; in other words the stream can produce the same block twice if the inner subscription drops - - // Now that we have a stream, extract the first element to be the first value. - let first_value = master_stream.next().await.unwrap(); - (first_value, master_stream) -} - -/// Returns the SCALE-encoded header of the current best block, plus an unlimited stream that -/// produces one item every time the best block is changed. -/// -/// This function only returns once the runtime of the current best block is known. This might -/// take a long time. -pub async fn subscribe_best( - runtime_service: &Arc>, -) -> (Vec, stream::BoxStream<'static, Vec>) { - let mut master_stream = stream::unfold(runtime_service.clone(), |runtime_service| async move { - let subscribe_all = runtime_service - .subscribe_all("subscribe-best", 16, NonZeroUsize::new(32).unwrap()) - .await; - - // Map of block headers by hash. Contains all non-finalized blocks headers, plus the - // current finalized block header. - let mut headers = - hashbrown::HashMap::<[u8; 32], Vec, fnv::FnvBuildHasher>::with_capacity_and_hasher( - 16, - Default::default(), - ); - - let current_finalized_hash = header::hash_from_scale_encoded_header( - &subscribe_all.finalized_block_scale_encoded_header, - ); - - subscribe_all - .new_blocks - .unpin_block(¤t_finalized_hash) - .await; - - headers.insert( - current_finalized_hash, - subscribe_all.finalized_block_scale_encoded_header, - ); - - let mut current_best = None; - for block in subscribe_all.non_finalized_blocks_ancestry_order { - let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - subscribe_all.new_blocks.unpin_block(&hash).await; - headers.insert(hash, block.scale_encoded_header); - - if block.is_new_best { - debug_assert!(current_best.is_none()); - current_best = Some(hash); - } - } - let current_best = current_best.unwrap_or(current_finalized_hash); - let current_best_header = headers.get(¤t_best).unwrap().clone(); - - // Turns `subscribe_all.new_blocks` into a stream of headers. - let substream = stream::unfold( - ( - subscribe_all.new_blocks, - headers, - current_finalized_hash, - current_best, - ), - |( - mut new_blocks, - mut headers, - mut current_finalized_hash, - mut current_best, - )| async move { - loop { - match new_blocks.next().await? { - Notification::Block(block) => { - let hash = - header::hash_from_scale_encoded_header(&block.scale_encoded_header); - new_blocks.unpin_block(&hash).await; - headers.insert(hash, block.scale_encoded_header); - - if block.is_new_best { - current_best = hash; - let header = - headers.get(¤t_best).unwrap().clone(); - break Some(( - header, - ( - new_blocks, - headers, - current_finalized_hash, - current_best, - ), - )); - } - } - Notification::Finalized { - hash, - pruned_blocks, - best_block_hash, - } => { - // Clean up the headers we won't need anymore. - for pruned_block in pruned_blocks { - let _was_in = headers.remove(&pruned_block); - debug_assert!(_was_in.is_some()); - } - - let _ = headers - .remove(¤t_finalized_hash) - .unwrap(); - current_finalized_hash = hash; - - if best_block_hash != current_best { - current_best = best_block_hash; - let header = - headers.get(¤t_best).unwrap().clone(); - break Some(( - header, - ( - new_blocks, - headers, - current_finalized_hash, - current_best, - ), - )); - } - } - Notification::BestBlockChanged { hash } => { - if hash != current_best { - current_best = hash; - let header = - headers.get(¤t_best).unwrap().clone(); - break Some(( - header, - ( - new_blocks, - headers, - current_finalized_hash, - current_best, - ), - )); - } - } - } - } - }, - ); - - // Prepend the current best block to the stream. - let substream = stream::once(future::ready(current_best_header)).chain(substream); - Some((substream, runtime_service)) - }) - .flatten() - .boxed(); - - // TODO: we don't dedup blocks; in other words the stream can produce the same block twice if the inner subscription drops - - // Now that we have a stream, extract the first element to be the first value. - let first_value = master_stream.next().await.unwrap(); - (first_value, master_stream) -} From c071fa33a0315e220ab48506412327fe8edd1bd7 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 13:42:32 +0200 Subject: [PATCH 36/46] Add TODO --- light-base/src/json_rpc_service/background/legacy_state_sub.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 7b0f9182e5..dc8c52459a 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -228,6 +228,7 @@ enum Subscription { struct RecentBlock { scale_encoded_header: Vec, + // TODO: do we really need to keep the runtime version here, given that the block is still pinned in the runtime service? runtime_version: Arc>, } From 4cd5c7b6d7f04789c4e6bfc6536b0ff522a2d642 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 13:45:03 +0200 Subject: [PATCH 37/46] Change creation API to return a sender --- light-base/src/json_rpc_service/background.rs | 16 ++++++---------- .../background/legacy_state_sub.rs | 11 +++++------ 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 738ba73db6..1a534435a0 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -132,7 +132,12 @@ pub(super) fn start( mut requests_processing_task: service::ClientMainTask, max_parallel_requests: NonZeroU32, ) { - let (to_legacy_tx, to_legacy_rx) = async_channel::bounded(8); + let to_legacy_tx = legacy_state_sub::start_task( + config.platform.clone(), + log_target.clone(), + config.sync_service.clone(), + config.runtime_service.clone(), + ); let me = Arc::new(Background { log_target, @@ -251,15 +256,6 @@ pub(super) fn start( }, ); } - - legacy_state_sub::start_task( - me.platform.clone(), - me.log_target.clone(), - me.sync_service.clone(), - me.runtime_service.clone(), - to_legacy_tx, - to_legacy_rx, - ); } impl Background { diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index dc8c52459a..c0fdba745e 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -72,16 +72,13 @@ pub(super) enum Message { } // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. -// TODO: weird to pass both the sender and receiver pub(super) fn start_task( platform: TPlat, log_target: String, sync_service: Arc>, runtime_service: Arc>, - requests_tx: async_channel::Sender>, - requests_rx: async_channel::Receiver>, -) { - let requests_tx = async_channel::Sender::downgrade(&requests_tx); +) -> async_channel::Sender> { + let (requests_tx, requests_rx) = async_channel::bounded(8); platform.clone().spawn_task( format!("{}-cache", log_target).into(), @@ -96,7 +93,7 @@ pub(super) fn start_task( sync_service, runtime_service, subscription: Subscription::NotCreated, - requests_tx, + requests_tx: async_channel::Sender::downgrade(&requests_tx), requests_rx, all_heads_subscriptions: hashbrown::HashMap::with_capacity_and_hasher( 2, @@ -129,6 +126,8 @@ pub(super) fn start_task( storage_query_in_progress: false, })), ); + + requests_tx } struct Task { From 382efd23ceb33a0b386c140e9109e7e8c225fa61 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 13:51:54 +0200 Subject: [PATCH 38/46] Use a Config struct and pass proper seeds --- light-base/src/json_rpc_service/background.rs | 12 +++---- .../background/legacy_state_sub.rs | 36 ++++++++++--------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 1a534435a0..427b13230b 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -132,12 +132,12 @@ pub(super) fn start( mut requests_processing_task: service::ClientMainTask, max_parallel_requests: NonZeroU32, ) { - let to_legacy_tx = legacy_state_sub::start_task( - config.platform.clone(), - log_target.clone(), - config.sync_service.clone(), - config.runtime_service.clone(), - ); + let to_legacy_tx = legacy_state_sub::start_task(legacy_state_sub::Config { + platform: config.platform.clone(), + log_target: log_target.clone(), + sync_service: config.sync_service.clone(), + runtime_service: config.runtime_service.clone(), + }); let me = Arc::new(Background { log_target, diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index c0fdba745e..df3e7f3269 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -23,10 +23,7 @@ use core::{ time::Duration, }; -use alloc::{ - borrow::ToOwned as _, boxed::Box, collections::BTreeSet, format, string::String, sync::Arc, - vec::Vec, -}; +use alloc::{borrow::ToOwned as _, boxed::Box, format, string::String, sync::Arc, vec::Vec}; use futures_channel::oneshot; use futures_lite::{FutureExt as _, StreamExt as _}; use futures_util::{future, FutureExt as _}; @@ -71,27 +68,31 @@ pub(super) enum Message { }, } +pub(super) struct Config { + pub platform: TPlat, + pub log_target: String, + pub sync_service: Arc>, + pub runtime_service: Arc>, +} + // Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. pub(super) fn start_task( - platform: TPlat, - log_target: String, - sync_service: Arc>, - runtime_service: Arc>, + config: Config, ) -> async_channel::Sender> { let (requests_tx, requests_rx) = async_channel::bounded(8); - platform.clone().spawn_task( - format!("{}-cache", log_target).into(), + config.platform.clone().spawn_task( + format!("{}-cache", config.log_target).into(), Box::pin(run(Task { block_state_root_hashes_numbers: lru::LruCache::with_hasher( NonZeroUsize::new(32).unwrap(), Default::default(), ), - log_target: log_target.clone(), - platform: platform.clone(), + log_target: config.log_target.clone(), + platform: config.platform.clone(), best_block_report: Vec::with_capacity(4), - sync_service, - runtime_service, + sync_service: config.sync_service, + runtime_service: config.runtime_service, subscription: Subscription::NotCreated, requests_tx: async_channel::Sender::downgrade(&requests_tx), requests_rx, @@ -117,7 +118,7 @@ pub(super) fn start_task( ), storage_subscriptions_by_key: hashbrown::HashMap::with_capacity_and_hasher( 16, - crate::util::SipHasherBuild::new([0; 16]), // TODO: proper seed + crate::util::SipHasherBuild::new(rand::random()), ), stale_storage_subscriptions: hashbrown::HashSet::with_capacity_and_hasher( 16, @@ -265,8 +266,9 @@ async fn run(mut task: Task) { task.storage_query_in_progress = true; - let mut keys = - hashbrown::HashSet::with_hasher(crate::util::SipHasherBuild::new([0; 16])); // TODO: proper seed + let mut keys = hashbrown::HashSet::with_hasher(crate::util::SipHasherBuild::new( + rand::random(), + )); keys.extend( task.stale_storage_subscriptions .iter() From 41ea4cc157e4c3a3f3ff5993668478066fc499e8 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 14:38:21 +0200 Subject: [PATCH 39/46] Add lots of comments and tweaks --- .../background/legacy_state_sub.rs | 202 ++++++++++++++---- 1 file changed, 163 insertions(+), 39 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index df3e7f3269..56d68547a8 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -38,51 +38,96 @@ use crate::{platform::PlatformRef, runtime_service, sync_service}; use super::StateTrieRootHashError; +/// Message that can be passed to the task started with [`start_task`]. pub(super) enum Message { + /// JSON-RPC client has sent a subscription request. + /// + /// Only the legacy API subscription requests are supported. Any other will trigger a panic. SubscriptionStart(service::SubscriptionStartProcess), + + /// JSON-RPC client has unsubscribed from something. SubscriptionDestroyed { + /// Identifier of the subscription. Does not necessarily match any of the subscriptions + /// previously passed through [`Message::SubscriptionStart`]. subscription_id: String, }, + + /// The task must send back access to the runtime of the given block, or `None` if the block + /// isn't available in the cache. RecentBlockRuntimeAccess { + /// Hash of the block to query. block_hash: [u8; 32], + /// How to send back the result. result_tx: oneshot::Sender>>, }, + + /// The task must send back the current best block hash. + /// + /// Waits for the runtime service to be ready, which can potentially take a long time. CurrentBestBlockHash { + /// How to send back the result. result_tx: oneshot::Sender<[u8; 32]>, }, + + /// The task must send back the state root and number the given block. If the block isn't + /// available in the cache, a network request is performed. + // TODO: refactor this message and the ones below to be more consistent BlockStateRootAndNumber { + /// Hash of the block to query. block_hash: [u8; 32], + /// How to send back the result. result_tx: oneshot::Sender>, }, + + /// The task must send back the number of the given block, or `None` if the block isn't + /// available in the cache. BlockNumber { + /// Hash of the block to query. block_hash: [u8; 32], + /// How to send back the result. result_tx: oneshot::Sender>, }, + + /// The task must send back the header of the given block, or `None` if the block isn't + /// available in the cache. BlockHeader { + /// Hash of the block to query. block_hash: [u8; 32], + /// How to send back the result. result_tx: oneshot::Sender>>, }, + + /// Internal message. Do not use. StorageFetch { + /// Hash of the block the storage fetch targets. block_hash: [u8; 32], + /// Results provided by the [`sync_service`]. result: Result, sync_service::StorageQueryError>, }, } +/// Configuration to pass to [`start_task`]. pub(super) struct Config { + /// Access to the platform bindings. pub platform: TPlat, + /// Prefix used for all the logging in this module. pub log_target: String, + /// Sync service used to start networking requests. pub sync_service: Arc>, + /// Runtime service used to subscribe to notifications regarding blocks and report them to + /// the JSON-RPC client. pub runtime_service: Arc>, } -// Spawn one task dedicated to filling the `Cache` with new blocks from the runtime service. +/// Spawn a task dedicated to holding a cache and fulfilling the legacy API subscriptions that the +/// JSON-RPC client starts. pub(super) fn start_task( config: Config, ) -> async_channel::Sender> { let (requests_tx, requests_rx) = async_channel::bounded(8); config.platform.clone().spawn_task( - format!("{}-cache", config.log_target).into(), + format!("{}-legacy-state-subscriptions", config.log_target).into(), Box::pin(run(Task { block_state_root_hashes_numbers: lru::LruCache::with_hasher( NonZeroUsize::new(32).unwrap(), @@ -132,54 +177,50 @@ pub(super) fn start_task( } struct Task { - /// State trie root hashes and numbers of blocks that were not in - /// [`Cache::recent_pinned_blocks`]. - /// - /// The state trie root hash can also be an `Err` if the network request failed or if the - /// header is of an invalid format. - /// - /// The state trie root hash and number are wrapped in a `Shared` future. When multiple - /// requests need the state trie root hash and number of the same block, they are only queried - /// once and the query is inserted in the cache while in progress. This way, the multiple - /// requests can all wait on that single future. - /// - /// Most of the time, the JSON-RPC client will query blocks that are found in - /// [`Cache::recent_pinned_blocks`], but occasionally it will query older blocks. When the - /// storage of an older block is queried, it is common for the JSON-RPC client to make several - /// storage requests to that same old block. In order to avoid having to retrieve the state - /// trie root hash multiple, we store these hashes in this LRU cache. - block_state_root_hashes_numbers: lru::LruCache< - [u8; 32], - future::MaybeDone< - future::Shared< - future::BoxFuture<'static, Result<([u8; 32], u64), StateTrieRootHashError>>, - >, - >, - fnv::FnvBuildHasher, - >, - + /// See [`Config::log_target`]. log_target: String, + /// See [`Config::platform`]. platform: TPlat, - best_block_report: Vec>, + /// See [`Config::sync_service`]. sync_service: Arc>, + /// See [`Config::runtime_service`]. runtime_service: Arc>, + + /// State of the subscription towards the runtime service. subscription: Subscription, + /// Whenever the subscription becomes active and the best block becomes available, it must be + /// sent on these channels as soon as possible. + best_block_report: Vec>, + /// Sending side of [`Task::requests_rx`]. requests_tx: async_channel::WeakSender>, + /// How to receive messages from the API user. requests_rx: async_channel::Receiver>, + + /// List of all active `chain_subscribeAllHeads` subscriptions, indexed by the subscription ID. // TODO: shrink_to_fit? all_heads_subscriptions: hashbrown::HashMap, + /// List of all active `chain_subscribeNewHeads` subscriptions, indexed by the subscription ID. // TODO: shrink_to_fit? new_heads_subscriptions: hashbrown::HashMap, // TODO: shrink_to_fit? + /// List of all active `chain_subscribeFinalizedHeads` subscriptions, indexed by the + /// subscription ID. finalized_heads_subscriptions: hashbrown::HashMap, // TODO: shrink_to_fit? + /// List of all active `state_subscribeRuntimeVersion` subscriptions, indexed by the + /// subscription ID. runtime_version_subscriptions: hashbrown::HashMap, + + /// List of all active `state_subscribeStorage` subscriptions, indexed by the subscription ID. + /// The value is the subscription plus the list of keys requested by this subscription. // TODO: shrink_to_fit? storage_subscriptions: hashbrown::HashMap>), fnv::FnvBuildHasher>, + /// Identical to [`Task::storage_subscriptions`] by indexed by requested key. The inner + /// `HashSet`s are never empty. // TODO: shrink_to_fit? storage_subscriptions_by_key: hashbrown::HashMap< Vec, @@ -189,13 +230,40 @@ struct Task { /// List of storage subscriptions whose latest sent notification isn't about the current /// best block. stale_storage_subscriptions: hashbrown::HashSet, - /// `true` if there exists a background task currently fetching storage items for storage /// subscriptions. This task will send a [`Message::StorageFetch`] once it's finished. storage_query_in_progress: bool, + + /// State trie root hashes and numbers of blocks that were not in + /// [`Cache::recent_pinned_blocks`]. + /// + /// The state trie root hash can also be an `Err` if the network request failed or if the + /// header is of an invalid format. + /// + /// The state trie root hash and number are wrapped in a `Shared` future. When multiple + /// requests need the state trie root hash and number of the same block, they are only queried + /// once and the query is inserted in the cache while in progress. This way, the multiple + /// requests can all wait on that single future. + /// + /// Most of the time, the JSON-RPC client will query blocks that are found in + /// [`Cache::recent_pinned_blocks`], but occasionally it will query older blocks. When the + /// storage of an older block is queried, it is common for the JSON-RPC client to make several + /// storage requests to that same old block. In order to avoid having to retrieve the state + /// trie root hash multiple, we store these hashes in this LRU cache. + block_state_root_hashes_numbers: lru::LruCache< + [u8; 32], + future::MaybeDone< + future::Shared< + future::BoxFuture<'static, Result<([u8; 32], u64), StateTrieRootHashError>>, + >, + >, + fnv::FnvBuildHasher, + >, } +/// State of the subscription towards the runtime service. See [`Task::subscription`]. enum Subscription { + /// Subscription is active. Active { /// Object representing the subscription. subscription: runtime_service::Subscription, @@ -222,7 +290,13 @@ enum Subscription { /// recently used blocks are removed and unpinned. finalized_and_pruned_lru: lru::LruCache<[u8; 32], (), fnv::FnvBuildHasher>, }, + + /// Wiating for the runtime service to start the subscription. Can potentially take a long + /// time. Pending(Pin> + Send>>), + + /// Subscription not requested yet. Should transition to [`Subscription::Pending`] as soon + /// as possible. NotCreated, } @@ -232,20 +306,26 @@ struct RecentBlock { runtime_version: Arc>, } +/// Actually run the task. async fn run(mut task: Task) { loop { + // Perform some internal state updates if necessary. if let Subscription::Active { pinned_blocks, current_best_block, .. } = &task.subscription { + // Process the content of `best_block_report` while let Some(sender) = task.best_block_report.pop() { let _ = sender.send(*current_best_block); } task.best_block_report.shrink_to_fit(); + // Start a task that fetches the storage items of the stale storage subscriptions. if !task.storage_query_in_progress && !task.stale_storage_subscriptions.is_empty() { + // If the header of the current best block can't be decoded, we don't start + // the task. let (block_number, state_trie_root) = match header::decode( &pinned_blocks .get(current_best_block) @@ -264,8 +344,8 @@ async fn run(mut task: Task) { } }; - task.storage_query_in_progress = true; - + // Build the list of keys that must be requested by aggregating the keys requested + // by all stale storage subscriptions. let mut keys = hashbrown::HashSet::with_hasher(crate::util::SipHasherBuild::new( rand::random(), )); @@ -276,6 +356,17 @@ async fn run(mut task: Task) { .flat_map(|keys_list| keys_list.iter().cloned()), ); + // If the list of keys to query is empty, we mark all subscriptions as no longer + // stale and loop again. This is necessary in order to prevent infinite loops if + // the JSON-RPC client subscribes to an empty list of items. + if keys.is_empty() { + task.stale_storage_subscriptions.clear(); + continue; + } + + // Start the task in the background. + // The task will send a `Message::StorageFetch` once it is done. + task.storage_query_in_progress = true; task.platform.spawn_task( format!("{}-storage-subscriptions-fetch", task.log_target).into(), Box::pin({ @@ -329,7 +420,8 @@ async fn run(mut task: Task) { ForegroundDead, } - let event = { + // Asynchronously wait for something to happen. This can potentially take a long time. + let event: WhatHappened<'_, TPlat> = { let subscription_event = async { match &mut task.subscription { Subscription::NotCreated => WhatHappened::SubscriptionDead, @@ -366,8 +458,11 @@ async fn run(mut task: Task) { subscription_event.or(message).await }; + // Perform internal state updates depending on what happened. match event { + // Runtime service is now ready to give us blocks. WhatHappened::SubscriptionReady(subscribe_all) => { + // We must transition to `Subscription::Active`. let mut pinned_blocks = hashbrown::HashMap::with_capacity_and_hasher(32, Default::default()); let mut finalized_and_pruned_lru = lru::LruCache::with_hasher( @@ -411,9 +506,13 @@ async fn run(mut task: Task) { } } + // All storage subscriptions are unconditionally marked as stale, as the best + // block has most likely changed. task.stale_storage_subscriptions .extend(task.storage_subscriptions.keys().cloned()); + // TODO: must notify finalized heads and new heads subscriptions + task.subscription = Subscription::Active { subscription: subscribe_all.new_blocks, pinned_blocks, @@ -423,6 +522,7 @@ async fn run(mut task: Task) { }; } + // A new non-finalized block has appeared! WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::Block(block), pinned_blocks, @@ -449,7 +549,7 @@ async fn run(mut task: Task) { }; let hash = header::hash_from_scale_encoded_header(&block.scale_encoded_header); - pinned_blocks.insert( + let _was_in = pinned_blocks.insert( hash, RecentBlock { scale_encoded_header: block.scale_encoded_header, @@ -463,6 +563,7 @@ async fn run(mut task: Task) { }, }, ); + debug_assert!(_was_in.is_none()); for (subscription_id, subscription) in &mut task.all_heads_subscriptions { subscription @@ -510,6 +611,7 @@ async fn run(mut task: Task) { } } + // A block has been finalized. WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::Finalized { @@ -556,10 +658,14 @@ async fn run(mut task: Task) { .await; } + // Add the pruned and finalized blocks to the LRU cache. The least-recently used + // entries in the cache are unpinned and no longer tracked. + // // An important detail here is that the newly-finalized block is added to the list // at the end, in order to guaranteed that it doesn't get removed. This is // necessary in order to guarantee that the current finalized (and current best, - // if the best block is also the finalized block) remains pinned. + // if the best block is also the finalized block) remains pinned until at least + // a different block gets finalized. for block_hash in pruned_blocks.into_iter().chain(iter::once(finalized_hash)) { if finalized_and_pruned_lru.len() == finalized_and_pruned_lru.cap().get() { let (hash_to_unpin, _) = finalized_and_pruned_lru.pop_lru().unwrap(); @@ -631,6 +737,7 @@ async fn run(mut task: Task) { } } + // The current best block has now changed. WhatHappened::SubscriptionNotification { notification: runtime_service::Notification::BestBlockChanged { @@ -695,6 +802,7 @@ async fn run(mut task: Task) { *current_best_block = new_best_hash; } + // Request from the JSON-RPC client. WhatHappened::Message(Message::SubscriptionStart(request)) => match request.request() { methods::MethodCall::chain_subscribeAllHeads {} => { let subscription = request.accept(); @@ -847,10 +955,15 @@ async fn run(mut task: Task) { (subscription, list.into_iter().map(|l| l.0).collect()), ); } + + // Any other request. _ => unreachable!(), // TODO: stronger typing to avoid this? }, + // JSON-RPC client has unsubscribed. WhatHappened::Message(Message::SubscriptionDestroyed { subscription_id }) => { + // We don't know the type of the unsubscription, that's not a big deal. Just + // remove the entry from everywhere. task.all_heads_subscriptions.remove(&subscription_id); task.new_heads_subscriptions.remove(&subscription_id); task.finalized_heads_subscriptions.remove(&subscription_id); @@ -1085,6 +1198,8 @@ async fn run(mut task: Task) { }); } + // Background task dedicated to performing a storage query for the storage + // subscription has finished. WhatHappened::Message(Message::StorageFetch { block_hash, result: Ok(result), @@ -1092,6 +1207,8 @@ async fn run(mut task: Task) { debug_assert!(task.storage_query_in_progress); task.storage_query_in_progress = false; + // Determine whether another storage query targeting a more up-to-date block + // must be started afterwards. let is_up_to_date = match task.subscription { Subscription::Active { current_best_block, .. @@ -1099,6 +1216,10 @@ async fn run(mut task: Task) { Subscription::NotCreated | Subscription::Pending(_) => true, }; + // Because all the keys of all the subscriptions are merged into one network + // request, we must now attribute each item in the result back to its subscription. + // While this solution is a bit CPU-heavy, it is a more elegant solution than + // keeping track of subscription in the background task. let mut notifications_to_send = hashbrown::HashMap::< String, Vec<(methods::HexString, Option)>, @@ -1107,7 +1228,6 @@ async fn run(mut task: Task) { task.storage_subscriptions.len(), fnv::FnvBuildHasher::default(), ); - for item in result { let sync_service::StorageResultItem::Value { key, value } = item else { unreachable!() }; @@ -1127,6 +1247,8 @@ async fn run(mut task: Task) { } } + // Send the notifications and mark the subscriptions as no longer stale if + // relevant. for (subscription_id, changes) in notifications_to_send { if is_up_to_date { task.stale_storage_subscriptions.remove(&subscription_id); @@ -1146,6 +1268,8 @@ async fn run(mut task: Task) { } } + // Background task dedicated to performing a storage query for the storage + // subscription has finished but was unsuccessful. WhatHappened::Message(Message::StorageFetch { block_hash, result: Err(_), @@ -1155,13 +1279,13 @@ async fn run(mut task: Task) { // TODO: add a delay or something? } + // JSON-RPC service has been destroyed. Stop the task altogether. WhatHappened::ForegroundDead => { - break; + return; } + // The subscription towards the runtime service needs to be renewed. WhatHappened::SubscriptionDead => { - // Subscribe to new runtime service blocks in order to push them in the - // cache as soon as they are available. // The buffer size should be large enough so that, if the CPU is busy, it // doesn't become full before the execution of this task resumes. // The maximum number of pinned block is ignored, as this maximum is a way to From 8d56473bc5048ec1b37f2cdb18fb8eda09777d3d Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:22:03 +0200 Subject: [PATCH 40/46] Report finalized block separately --- .../background/legacy_state_sub.rs | 84 +++++++++++-------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 56d68547a8..87d54a53d3 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -276,6 +276,10 @@ enum Subscription { /// [`Subscription::Active::pinned_blocks`]. current_finalized_block: [u8; 32], + /// If `true`, the finalized heads subscriptions hasn't been updated about the new + /// current block yet. + finalized_heads_subscriptions_stale: bool, + /// When the runtime service reports a new block, it is kept pinned and inserted in this /// list. /// @@ -313,8 +317,10 @@ async fn run(mut task: Task) { if let Subscription::Active { pinned_blocks, current_best_block, + current_finalized_block, + finalized_heads_subscriptions_stale, .. - } = &task.subscription + } = &mut task.subscription { // Process the content of `best_block_report` while let Some(sender) = task.best_block_report.pop() { @@ -322,6 +328,43 @@ async fn run(mut task: Task) { } task.best_block_report.shrink_to_fit(); + // If the finalized heads subcriptions aren't up-to-date with the latest finalized + // block, report it to them. + if *finalized_heads_subscriptions_stale { + let finalized_block_header = &pinned_blocks + .get(current_finalized_block) + .unwrap() + .scale_encoded_header; + let finalized_block_json_rpc_header = + match methods::Header::from_scale_encoded_header( + finalized_block_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeFinalizedHeads` subscription has skipped block \ + due to undecodable header. Hash: {}. Error: {}", + HashDisplay(current_finalized_block), + error, + ); + continue; + } + }; + + for (subscription_id, subscription) in &mut task.finalized_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_finalizedHead { + subscription: subscription_id.as_str().into(), + result: finalized_block_json_rpc_header.clone(), + }) + .await; + } + + *finalized_heads_subscriptions_stale = false; + } + // Start a task that fetches the storage items of the stale storage subscriptions. if !task.storage_query_in_progress && !task.stale_storage_subscriptions.is_empty() { // If the header of the current best block can't be decoded, we don't start @@ -413,6 +456,7 @@ async fn run(mut task: Task) { finalized_and_pruned_lru: &'a mut lru::LruCache<[u8; 32], (), fnv::FnvBuildHasher>, current_best_block: &'a mut [u8; 32], current_finalized_block: &'a mut [u8; 32], + finalized_heads_subscriptions_stale: &'a mut bool, }, SubscriptionDead, SubscriptionReady(runtime_service::SubscribeAll), @@ -431,6 +475,7 @@ async fn run(mut task: Task) { finalized_and_pruned_lru, current_best_block, current_finalized_block, + finalized_heads_subscriptions_stale, } => match subscription.next().await { Some(notification) => WhatHappened::SubscriptionNotification { notification, @@ -439,6 +484,7 @@ async fn run(mut task: Task) { finalized_and_pruned_lru, current_best_block, current_finalized_block, + finalized_heads_subscriptions_stale, }, None => WhatHappened::SubscriptionDead, }, @@ -511,7 +557,7 @@ async fn run(mut task: Task) { task.stale_storage_subscriptions .extend(task.storage_subscriptions.keys().cloned()); - // TODO: must notify finalized heads and new heads subscriptions + // TODO: must notify new heads subscriptions task.subscription = Subscription::Active { subscription: subscribe_all.new_blocks, @@ -519,6 +565,7 @@ async fn run(mut task: Task) { finalized_and_pruned_lru, current_best_block, current_finalized_block: finalized_block_hash, + finalized_heads_subscriptions_stale: true, }; } @@ -624,39 +671,10 @@ async fn run(mut task: Task) { subscription, current_best_block, current_finalized_block, + finalized_heads_subscriptions_stale, } => { *current_finalized_block = finalized_hash; - - let finalized_block_header = &pinned_blocks - .get(&finalized_hash) - .unwrap() - .scale_encoded_header; - let finalized_block_json_rpc_header = - match methods::Header::from_scale_encoded_header( - finalized_block_header, - task.runtime_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &task.log_target, - "`chain_subscribeFinalizedHeads` subscription has skipped block \ - due to undecodable header. Hash: {}. Error: {}", - HashDisplay(&new_best_block_hash), - error, - ); - continue; - } - }; - - for (subscription_id, subscription) in &mut task.finalized_heads_subscriptions { - subscription - .send_notification(methods::ServerToClient::chain_finalizedHead { - subscription: subscription_id.as_str().into(), - result: finalized_block_json_rpc_header.clone(), - }) - .await; - } + *finalized_heads_subscriptions_stale = true; // Add the pruned and finalized blocks to the LRU cache. The least-recently used // entries in the cache are unpinned and no longer tracked. From 1c371aa121ae1c97ab6476dd8d6ce7138dc0c32d Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:28:03 +0200 Subject: [PATCH 41/46] Simplify notifying best block update --- .../background/legacy_state_sub.rs | 225 ++++++------------ 1 file changed, 76 insertions(+), 149 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 87d54a53d3..bcd061f52e 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -272,12 +272,18 @@ enum Subscription { /// [`Subscription::Active::pinned_blocks`]. current_best_block: [u8; 32], + /// If `Some`, the new heads and runtime version subscriptions haven't been updated about + /// the new current best block yet. Contains the previous best block that the + /// subscriptions are aware of. The previous best block is guaranteed to be in + /// [`Subscription::Active::pinned_blocks`]. + new_heads_and_runtime_subscriptions_stale: Option>, + /// Hash of the current finalized block. Guaranteed to be in /// [`Subscription::Active::pinned_blocks`]. current_finalized_block: [u8; 32], - /// If `true`, the finalized heads subscriptions hasn't been updated about the new - /// current block yet. + /// If `true`, the finalized heads subscriptions haven't been updated about the new + /// current finalized block yet. finalized_heads_subscriptions_stale: bool, /// When the runtime service reports a new block, it is kept pinned and inserted in this @@ -317,6 +323,7 @@ async fn run(mut task: Task) { if let Subscription::Active { pinned_blocks, current_best_block, + new_heads_and_runtime_subscriptions_stale, current_finalized_block, finalized_heads_subscriptions_stale, .. @@ -365,6 +372,63 @@ async fn run(mut task: Task) { *finalized_heads_subscriptions_stale = false; } + // If the new heads and runtime version subscriptions aren't up-to-date with the latest + // best block, report it to them. + if let Some(previous_best_block) = new_heads_and_runtime_subscriptions_stale.take() { + let best_block_header = &pinned_blocks + .get(current_best_block) + .unwrap() + .scale_encoded_header; + let best_block_json_rpc_header = match methods::Header::from_scale_encoded_header( + &best_block_header, + task.runtime_service.block_number_bytes(), + ) { + Ok(h) => h, + Err(error) => { + log::warn!( + target: &task.log_target, + "`chain_subscribeNewHeads` subscription has skipped block due to \ + undecodable header. Hash: {}. Error: {}", + HashDisplay(current_best_block), + error, + ); + continue; + } + }; + + for (subscription_id, subscription) in &mut task.new_heads_subscriptions { + subscription + .send_notification(methods::ServerToClient::chain_newHead { + subscription: subscription_id.as_str().into(), + result: best_block_json_rpc_header.clone(), + }) + .await; + } + + let new_best_runtime = &pinned_blocks + .get(current_best_block) + .unwrap() + .runtime_version; + if previous_best_block.map_or(true, |prev_best_block| { + !Arc::ptr_eq( + new_best_runtime, + &pinned_blocks.get(&prev_best_block).unwrap().runtime_version, + ) + }) { + for (subscription_id, subscription) in &mut task.runtime_version_subscriptions { + subscription + .send_notification(methods::ServerToClient::state_runtimeVersion { + subscription: subscription_id.as_str().into(), + result: convert_runtime_version(new_best_runtime), + }) + .await; + } + } + + task.stale_storage_subscriptions + .extend(task.storage_subscriptions.keys().cloned()); + } + // Start a task that fetches the storage items of the stale storage subscriptions. if !task.storage_query_in_progress && !task.stale_storage_subscriptions.is_empty() { // If the header of the current best block can't be decoded, we don't start @@ -455,6 +519,7 @@ async fn run(mut task: Task) { &'a mut hashbrown::HashMap<[u8; 32], RecentBlock, fnv::FnvBuildHasher>, finalized_and_pruned_lru: &'a mut lru::LruCache<[u8; 32], (), fnv::FnvBuildHasher>, current_best_block: &'a mut [u8; 32], + new_heads_and_runtime_subscriptions_stale: &'a mut Option>, current_finalized_block: &'a mut [u8; 32], finalized_heads_subscriptions_stale: &'a mut bool, }, @@ -474,6 +539,7 @@ async fn run(mut task: Task) { pinned_blocks, finalized_and_pruned_lru, current_best_block, + new_heads_and_runtime_subscriptions_stale, current_finalized_block, finalized_heads_subscriptions_stale, } => match subscription.next().await { @@ -483,6 +549,7 @@ async fn run(mut task: Task) { pinned_blocks, finalized_and_pruned_lru, current_best_block, + new_heads_and_runtime_subscriptions_stale, current_finalized_block, finalized_heads_subscriptions_stale, }, @@ -552,18 +619,12 @@ async fn run(mut task: Task) { } } - // All storage subscriptions are unconditionally marked as stale, as the best - // block has most likely changed. - task.stale_storage_subscriptions - .extend(task.storage_subscriptions.keys().cloned()); - - // TODO: must notify new heads subscriptions - task.subscription = Subscription::Active { subscription: subscribe_all.new_blocks, pinned_blocks, finalized_and_pruned_lru, current_best_block, + new_heads_and_runtime_subscriptions_stale: Some(None), current_finalized_block: finalized_block_hash, finalized_heads_subscriptions_stale: true, }; @@ -574,6 +635,7 @@ async fn run(mut task: Task) { notification: runtime_service::Notification::Block(block), pinned_blocks, current_best_block, + new_heads_and_runtime_subscriptions_stale, .. } => { let json_rpc_header = match methods::Header::from_scale_encoded_header( @@ -622,38 +684,7 @@ async fn run(mut task: Task) { } if block.is_new_best { - for (subscription_id, subscription) in &mut task.new_heads_subscriptions { - subscription - .send_notification(methods::ServerToClient::chain_newHead { - subscription: subscription_id.as_str().into(), - result: json_rpc_header.clone(), - }) - .await; - } - - let new_best_runtime = &pinned_blocks.get(&hash).unwrap().runtime_version; - if !Arc::ptr_eq( - new_best_runtime, - &pinned_blocks - .get(current_best_block) - .unwrap() - .runtime_version, - ) { - for (subscription_id, subscription) in - &mut task.runtime_version_subscriptions - { - subscription - .send_notification(methods::ServerToClient::state_runtimeVersion { - subscription: subscription_id.as_str().into(), - result: convert_runtime_version(new_best_runtime), - }) - .await; - } - } - - task.stale_storage_subscriptions - .extend(task.storage_subscriptions.keys().cloned()); - + *new_heads_and_runtime_subscriptions_stale = Some(Some(*current_best_block)); *current_best_block = hash; } } @@ -670,6 +701,7 @@ async fn run(mut task: Task) { finalized_and_pruned_lru, subscription, current_best_block, + new_heads_and_runtime_subscriptions_stale, current_finalized_block, finalized_heads_subscriptions_stale, } => { @@ -694,63 +726,7 @@ async fn run(mut task: Task) { } if *current_best_block != new_best_block_hash { - let best_block_header = &pinned_blocks - .get(&new_best_block_hash) - .unwrap() - .scale_encoded_header; - let best_block_json_rpc_header = - match methods::Header::from_scale_encoded_header( - &best_block_header, - task.runtime_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &task.log_target, - "`chain_subscribeNewHeads` subscription has skipped block due to \ - undecodable header. Hash: {}. Error: {}", - HashDisplay(&new_best_block_hash), - error, - ); - continue; - } - }; - - for (subscription_id, subscription) in &mut task.new_heads_subscriptions { - subscription - .send_notification(methods::ServerToClient::chain_newHead { - subscription: subscription_id.as_str().into(), - result: best_block_json_rpc_header.clone(), - }) - .await; - } - - let new_best_runtime = &pinned_blocks - .get(&new_best_block_hash) - .unwrap() - .runtime_version; - if !Arc::ptr_eq( - new_best_runtime, - &pinned_blocks - .get(current_best_block) - .unwrap() - .runtime_version, - ) { - for (subscription_id, subscription) in - &mut task.runtime_version_subscriptions - { - subscription - .send_notification(methods::ServerToClient::state_runtimeVersion { - subscription: subscription_id.as_str().into(), - result: convert_runtime_version(new_best_runtime), - }) - .await; - } - } - - task.stale_storage_subscriptions - .extend(task.storage_subscriptions.keys().cloned()); - + *new_heads_and_runtime_subscriptions_stale = Some(Some(*current_best_block)); *current_best_block = new_best_block_hash; } } @@ -764,59 +740,10 @@ async fn run(mut task: Task) { }, pinned_blocks, current_best_block, + new_heads_and_runtime_subscriptions_stale, .. } => { - let header = &pinned_blocks - .get(&new_best_hash) - .unwrap() - .scale_encoded_header; - let json_rpc_header = match methods::Header::from_scale_encoded_header( - &header, - task.runtime_service.block_number_bytes(), - ) { - Ok(h) => h, - Err(error) => { - log::warn!( - target: &task.log_target, - "`chain_subscribeNewHeads` subscription has skipped block due to \ - undecodable header. Hash: {}. Error: {}", - HashDisplay(&new_best_hash), - error, - ); - continue; - } - }; - - for (subscription_id, subscription) in &mut task.new_heads_subscriptions { - subscription - .send_notification(methods::ServerToClient::chain_newHead { - subscription: subscription_id.as_str().into(), - result: json_rpc_header.clone(), - }) - .await; - } - - let new_best_runtime = &pinned_blocks.get(&new_best_hash).unwrap().runtime_version; - if !Arc::ptr_eq( - new_best_runtime, - &pinned_blocks - .get(current_best_block) - .unwrap() - .runtime_version, - ) { - for (subscription_id, subscription) in &mut task.runtime_version_subscriptions { - subscription - .send_notification(methods::ServerToClient::state_runtimeVersion { - subscription: subscription_id.as_str().into(), - result: convert_runtime_version(new_best_runtime), - }) - .await; - } - } - - task.stale_storage_subscriptions - .extend(task.storage_subscriptions.keys().cloned()); - + *new_heads_and_runtime_subscriptions_stale = Some(Some(*current_best_block)); *current_best_block = new_best_hash; } From e510594585d8c09eee1ecf8d7b56454a31f45f16 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:29:06 +0200 Subject: [PATCH 42/46] Split the blocks more for readability --- .../background/legacy_state_sub.rs | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index bcd061f52e..7dd1a48adf 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -320,6 +320,8 @@ struct RecentBlock { async fn run(mut task: Task) { loop { // Perform some internal state updates if necessary. + + // Process the content of `best_block_report` if let Subscription::Active { pinned_blocks, current_best_block, @@ -327,16 +329,25 @@ async fn run(mut task: Task) { current_finalized_block, finalized_heads_subscriptions_stale, .. - } = &mut task.subscription + } = &task.subscription { - // Process the content of `best_block_report` while let Some(sender) = task.best_block_report.pop() { let _ = sender.send(*current_best_block); } task.best_block_report.shrink_to_fit(); + } - // If the finalized heads subcriptions aren't up-to-date with the latest finalized - // block, report it to them. + // If the finalized heads subcriptions aren't up-to-date with the latest finalized block, + // report it to them. + if let Subscription::Active { + pinned_blocks, + current_best_block, + new_heads_and_runtime_subscriptions_stale, + current_finalized_block, + finalized_heads_subscriptions_stale, + .. + } = &mut task.subscription + { if *finalized_heads_subscriptions_stale { let finalized_block_header = &pinned_blocks .get(current_finalized_block) @@ -371,9 +382,19 @@ async fn run(mut task: Task) { *finalized_heads_subscriptions_stale = false; } + } - // If the new heads and runtime version subscriptions aren't up-to-date with the latest - // best block, report it to them. + // If the new heads and runtime version subscriptions aren't up-to-date with the latest + // best block, report it to them. + if let Subscription::Active { + pinned_blocks, + current_best_block, + new_heads_and_runtime_subscriptions_stale, + current_finalized_block, + finalized_heads_subscriptions_stale, + .. + } = &mut task.subscription + { if let Some(previous_best_block) = new_heads_and_runtime_subscriptions_stale.take() { let best_block_header = &pinned_blocks .get(current_best_block) @@ -428,8 +449,18 @@ async fn run(mut task: Task) { task.stale_storage_subscriptions .extend(task.storage_subscriptions.keys().cloned()); } + } - // Start a task that fetches the storage items of the stale storage subscriptions. + // Start a task that fetches the storage items of the stale storage subscriptions. + if let Subscription::Active { + pinned_blocks, + current_best_block, + new_heads_and_runtime_subscriptions_stale, + current_finalized_block, + finalized_heads_subscriptions_stale, + .. + } = &task.subscription + { if !task.storage_query_in_progress && !task.stale_storage_subscriptions.is_empty() { // If the header of the current best block can't be decoded, we don't start // the task. From cb9ec78c25b69ed020a7883d0e434ff4da90f6e4 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:30:41 +0200 Subject: [PATCH 43/46] Fix all warnings --- .../background/legacy_state_sub.rs | 20 ++----------------- .../background/state_chain.rs | 5 ++--- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 7dd1a48adf..c007ab7313 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -323,12 +323,7 @@ async fn run(mut task: Task) { // Process the content of `best_block_report` if let Subscription::Active { - pinned_blocks, - current_best_block, - new_heads_and_runtime_subscriptions_stale, - current_finalized_block, - finalized_heads_subscriptions_stale, - .. + current_best_block, .. } = &task.subscription { while let Some(sender) = task.best_block_report.pop() { @@ -341,8 +336,6 @@ async fn run(mut task: Task) { // report it to them. if let Subscription::Active { pinned_blocks, - current_best_block, - new_heads_and_runtime_subscriptions_stale, current_finalized_block, finalized_heads_subscriptions_stale, .. @@ -390,8 +383,6 @@ async fn run(mut task: Task) { pinned_blocks, current_best_block, new_heads_and_runtime_subscriptions_stale, - current_finalized_block, - finalized_heads_subscriptions_stale, .. } = &mut task.subscription { @@ -455,9 +446,6 @@ async fn run(mut task: Task) { if let Subscription::Active { pinned_blocks, current_best_block, - new_heads_and_runtime_subscriptions_stale, - current_finalized_block, - finalized_heads_subscriptions_stale, .. } = &task.subscription { @@ -769,7 +757,6 @@ async fn run(mut task: Task) { hash: new_best_hash, .. }, - pinned_blocks, current_best_block, new_heads_and_runtime_subscriptions_stale, .. @@ -1246,10 +1233,7 @@ async fn run(mut task: Task) { // Background task dedicated to performing a storage query for the storage // subscription has finished but was unsuccessful. - WhatHappened::Message(Message::StorageFetch { - block_hash, - result: Err(_), - }) => { + WhatHappened::Message(Message::StorageFetch { result: Err(_), .. }) => { debug_assert!(task.storage_query_in_progress); task.storage_query_in_progress = false; // TODO: add a delay or something? diff --git a/light-base/src/json_rpc_service/background/state_chain.rs b/light-base/src/json_rpc_service/background/state_chain.rs index 9cdcdb362c..0950eded95 100644 --- a/light-base/src/json_rpc_service/background/state_chain.rs +++ b/light-base/src/json_rpc_service/background/state_chain.rs @@ -21,10 +21,9 @@ use super::{legacy_state_sub, Background, GetKeysPagedCacheKey, PlatformRef}; use crate::sync_service; -use alloc::{borrow::ToOwned as _, format, string::ToString as _, sync::Arc, vec, vec::Vec}; -use core::{iter, num::NonZeroU32, pin, time::Duration}; +use alloc::{format, string::ToString as _, sync::Arc, vec, vec::Vec}; +use core::{iter, num::NonZeroU32, time::Duration}; use futures_channel::oneshot; -use futures_util::{future, stream, StreamExt as _}; use smoldot::{ header, json_rpc::{self, methods, service}, From 66c460f651555feff4cc11ac8d7542d42e8b9fc7 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:32:13 +0200 Subject: [PATCH 44/46] Small tweak --- .../src/json_rpc_service/background/legacy_state_sub.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index c007ab7313..806ebaa375 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -665,8 +665,8 @@ async fn run(mut task: Task) { Err(error) => { log::warn!( target: &task.log_target, - "`chain_subscribeAllHeads` or `chain_subscribeNewHeads` subscription \ - has skipped block due to undecodable header. Hash: {}. Error: {}", + "`chain_subscribeAllHeads` subscription has skipped block due to \ + undecodable header. Hash: {}. Error: {}", HashDisplay(&header::hash_from_scale_encoded_header( &block.scale_encoded_header )), From 9934527c62469868bc0ab0d1c16bb5c2c44a2522 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:40:04 +0200 Subject: [PATCH 45/46] PR link --- wasm-node/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index e2d040717c..41c543a9d1 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -5,9 +5,9 @@ ### Changed - The `chainHead_unstable_storage` JSON-RPC function now supports a `type` equal to `closest-descendant-merkle-value` and no longer supports `closest-ancestor-merkle-value`, in accordance with the latest changes in the JSON-RPC API specification. ([#824](https://github.com/smol-dot/smoldot/pull/824)) -- Blocks are now reported to `chain_subscribeAllHeads` and `chain_subscribeNewHeads` subscribers only after they have been put in the cache, preventing race conditions where JSON-RPC clients suffer from a cache miss if they ask information about these blocks too quickly. -- Runtime updates are now always reported to `state_subscribeRuntimeVersion` subscribers immediately after the `chain_subscribeNewHeads` notification corresponding to the block containing the runtime update. They were previously reported in a pseudo-random order. -- All the storage subscriptions made using `state_subscribeStorage` are now queried together into a single networking request per block, instead of sending one networking query per storage key and per subscription. +- Blocks are now reported to `chain_subscribeAllHeads` and `chain_subscribeNewHeads` subscribers only after they have been put in the cache, preventing race conditions where JSON-RPC clients suffer from a cache miss if they ask information about these blocks too quickly. ([#854](https://github.com/smol-dot/smoldot/pull/854)) +- Runtime updates are now always reported to `state_subscribeRuntimeVersion` subscribers immediately after the `chain_subscribeNewHeads` notification corresponding to the block containing the runtime update. They were previously reported in a pseudo-random order. ([#854](https://github.com/smol-dot/smoldot/pull/854)) +- All the storage subscriptions made using `state_subscribeStorage` are now queried together into a single networking request per block, instead of sending one networking query per storage key and per subscription. ([#854](https://github.com/smol-dot/smoldot/pull/854)) ## 1.0.11 - 2023-06-25 From 9e360ab241c44ffcf7a1e562cd495b79589c913c Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 5 Jul 2023 15:57:32 +0200 Subject: [PATCH 46/46] Docfix and error moved --- light-base/src/json_rpc_service/background.rs | 16 +++----------- .../background/legacy_state_sub.rs | 21 ++++++++++++------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index 427b13230b..b97e95847d 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -40,7 +40,6 @@ use core::{ use futures_channel::oneshot; use smoldot::{ executor::{host, runtime_host}, - header, json_rpc::{self, methods, service}, libp2p::{multiaddr, PeerId}, }; @@ -117,7 +116,7 @@ struct Background { >, } -/// See [`Cache::state_get_keys_paged`]. +/// See [`Background::state_get_keys_paged_cache`]. #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct GetKeysPagedCacheKey { /// Value of the `hash` parameter of the call to `state_getKeysPaged`. @@ -1114,7 +1113,7 @@ impl Background { enum StorageQueryError { /// Error while finding the storage root hash of the requested block. #[display(fmt = "Failed to obtain block state trie root: {_0}")] - FindStorageRootHashError(StateTrieRootHashError), + FindStorageRootHashError(legacy_state_sub::StateTrieRootHashError), /// Error while retrieving the storage item from other nodes. #[display(fmt = "{_0}")] StorageRetrieval(sync_service::StorageQueryError), @@ -1125,7 +1124,7 @@ enum StorageQueryError { enum RuntimeCallError { /// Error while finding the storage root hash of the requested block. #[display(fmt = "Failed to obtain block state trie root: {_0}")] - FindStorageRootHashError(StateTrieRootHashError), + FindStorageRootHashError(legacy_state_sub::StateTrieRootHashError), #[display(fmt = "{_0}")] Call(runtime_service::RuntimeCallError), #[display(fmt = "{_0}")] @@ -1143,15 +1142,6 @@ enum RuntimeCallError { }, } -/// Error potentially returned by [`Background::state_trie_root_hash`]. -#[derive(Debug, derive_more::Display, Clone)] -enum StateTrieRootHashError { - /// Failed to decode block header. - HeaderDecodeError(header::Error), - /// Error while fetching block header from network. - NetworkQueryError, -} - #[derive(Debug)] struct RuntimeCallResult { return_value: Vec, diff --git a/light-base/src/json_rpc_service/background/legacy_state_sub.rs b/light-base/src/json_rpc_service/background/legacy_state_sub.rs index 806ebaa375..013f13e67a 100644 --- a/light-base/src/json_rpc_service/background/legacy_state_sub.rs +++ b/light-base/src/json_rpc_service/background/legacy_state_sub.rs @@ -36,8 +36,6 @@ use smoldot::{ use crate::{platform::PlatformRef, runtime_service, sync_service}; -use super::StateTrieRootHashError; - /// Message that can be passed to the task started with [`start_task`]. pub(super) enum Message { /// JSON-RPC client has sent a subscription request. @@ -119,6 +117,15 @@ pub(super) struct Config { pub runtime_service: Arc>, } +/// Error potentially returned by [`Message::BlockStateRootAndNumber`]. +#[derive(Debug, derive_more::Display, Clone)] +pub(super) enum StateTrieRootHashError { + /// Failed to decode block header. + HeaderDecodeError(header::Error), + /// Error while fetching block header from network. + NetworkQueryError, +} + /// Spawn a task dedicated to holding a cache and fulfilling the legacy API subscriptions that the /// JSON-RPC client starts. pub(super) fn start_task( @@ -235,7 +242,7 @@ struct Task { storage_query_in_progress: bool, /// State trie root hashes and numbers of blocks that were not in - /// [`Cache::recent_pinned_blocks`]. + /// [`Subscription::Active::pinned_blocks`]. /// /// The state trie root hash can also be an `Err` if the network request failed or if the /// header is of an invalid format. @@ -246,10 +253,10 @@ struct Task { /// requests can all wait on that single future. /// /// Most of the time, the JSON-RPC client will query blocks that are found in - /// [`Cache::recent_pinned_blocks`], but occasionally it will query older blocks. When the - /// storage of an older block is queried, it is common for the JSON-RPC client to make several - /// storage requests to that same old block. In order to avoid having to retrieve the state - /// trie root hash multiple, we store these hashes in this LRU cache. + /// [`Subscription::Active::pinned_blocks`], but occasionally it will query older blocks. When + /// the storage of an older block is queried, it is common for the JSON-RPC client to make + /// several storage requests to that same old block. In order to avoid having to retrieve the + /// state trie root hash multiple, we store these hashes in this LRU cache. block_state_root_hashes_numbers: lru::LruCache< [u8; 32], future::MaybeDone<