From 838c089b10addab1486bfdfd4d9325ed57284dd3 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 15 Nov 2022 11:06:26 +0200 Subject: [PATCH 1/2] Expose `ApiError::UnknownBlock` to the runtime api caller --- node/collation-generation/src/lib.rs | 1 + .../src/disputes/prioritized_selection/mod.rs | 2 + node/core/provisioner/src/error.rs | 3 + node/core/provisioner/src/lib.rs | 8 +++ node/core/pvf-checker/src/lib.rs | 2 +- node/core/pvf-checker/src/runtime_api.rs | 2 + node/core/runtime-api/src/lib.rs | 62 +++++++++++++------ node/subsystem-types/src/errors.rs | 10 +++ node/subsystem-types/src/lib.rs | 2 +- node/subsystem-types/src/runtime_client.rs | 3 +- 10 files changed, 72 insertions(+), 23 deletions(-) diff --git a/node/collation-generation/src/lib.rs b/node/collation-generation/src/lib.rs index b3d728f70a48..5dfd10f73e17 100644 --- a/node/collation-generation/src/lib.rs +++ b/node/collation-generation/src/lib.rs @@ -431,6 +431,7 @@ async fn obtain_current_validation_code_hash( } }, Err(e @ RuntimeApiError::Execution { .. }) => Err(e.into()), + Err(e @ RuntimeApiError::UnknownBlock { .. }) => Err(e.into()), } } diff --git a/node/core/provisioner/src/disputes/prioritized_selection/mod.rs b/node/core/provisioner/src/disputes/prioritized_selection/mod.rs index 2b5e6b177abf..7bf106a37a1c 100644 --- a/node/core/provisioner/src/disputes/prioritized_selection/mod.rs +++ b/node/core/provisioner/src/disputes/prioritized_selection/mod.rs @@ -477,6 +477,8 @@ where GetOnchainDisputesError::Execution(e, relay_parent), RuntimeApiError::NotSupported { .. } => GetOnchainDisputesError::NotSupported(e, relay_parent), + RuntimeApiError::UnknownBlock { .. } => + GetOnchainDisputesError::UnknownBlock(e, relay_parent), }) }) .map(|v| v.into_iter().map(|e| ((e.0, e.1), e.2)).collect()) diff --git a/node/core/provisioner/src/error.rs b/node/core/provisioner/src/error.rs index de520fc1fe04..7f28ff318e99 100644 --- a/node/core/provisioner/src/error.rs +++ b/node/core/provisioner/src/error.rs @@ -90,6 +90,9 @@ pub enum GetOnchainDisputesError { #[error("runtime doesn't support RuntimeApiRequest::Disputes for parent {1}")] NotSupported(#[source] RuntimeApiError, Hash), + + #[error("runtime api is called on unknown block {1}")] + UnknownBlock(#[source] RuntimeApiError, Hash), } pub fn log_error(result: Result<()>) -> std::result::Result<(), FatalError> { diff --git a/node/core/provisioner/src/lib.rs b/node/core/provisioner/src/lib.rs index a8f19854073e..d5188c3349bf 100644 --- a/node/core/provisioner/src/lib.rs +++ b/node/core/provisioner/src/lib.rs @@ -739,6 +739,14 @@ async fn has_required_runtime( ); false }, + Result::Ok(Err(RuntimeApiError::UnknownBlock { .. })) => { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + "UnknownBlock error while fetching ParachainHost runtime api version" + ); + false + }, Result::Err(_) => { gum::trace!( target: LOG_TARGET, diff --git a/node/core/pvf-checker/src/lib.rs b/node/core/pvf-checker/src/lib.rs index 7e961dcf4b88..79e3882ea9ee 100644 --- a/node/core/pvf-checker/src/lib.rs +++ b/node/core/pvf-checker/src/lib.rs @@ -518,7 +518,7 @@ async fn sign_and_submit_pvf_check_statement( target: LOG_TARGET, ?relay_parent, ?validation_code_hash, - "error occured during submitting a vote: {:?}", + "error occurred during submitting a vote: {:?}", e, ); }, diff --git a/node/core/pvf-checker/src/runtime_api.rs b/node/core/pvf-checker/src/runtime_api.rs index b42f5943af56..9206bc14f23e 100644 --- a/node/core/pvf-checker/src/runtime_api.rs +++ b/node/core/pvf-checker/src/runtime_api.rs @@ -70,6 +70,7 @@ pub(crate) enum RuntimeRequestError { NotSupported, ApiError, CommunicationError, + UnknownBlock, } pub(crate) async fn runtime_api_request( @@ -102,6 +103,7 @@ pub(crate) async fn runtime_api_request( RuntimeRequestError::ApiError }, NotSupported { .. } => RuntimeRequestError::NotSupported, + UnknownBlock { .. } => RuntimeRequestError::UnknownBlock, } }) }) diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index 3d016305bc64..8fda342071d0 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -27,7 +27,7 @@ use polkadot_node_subsystem::{ messages::{RuntimeApiMessage, RuntimeApiRequest as Request}, overseer, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, SubsystemResult, }; -use polkadot_node_subsystem_types::RuntimeApiSubsystemClient; +use polkadot_node_subsystem_types::{ApiError, RuntimeApiSubsystemClient}; use polkadot_primitives::Hash; use cache::{RequestResult, RequestResultCache}; @@ -355,33 +355,55 @@ where let sender = $sender; let version: u32 = $version; // enforce type for the version expression let runtime_version = client.api_version_parachain_host(relay_parent).await - .unwrap_or_else(|e| { - gum::warn!( - target: LOG_TARGET, - "cannot query the runtime API version: {}", - e, - ); - Some(0) + .map_err(|e| { + if let ApiError::UnknownBlock(s) = e { + // We want a specific indication for this error. It generally indicates that the runtime api is + // called for a block which is already pruned. Legitimate reasons for this may be block restart. + // Separate value in `RuntimeApiError` enum allows the caller to handle it properly. + gum::warn!( + target: LOG_TARGET, + "cannot query the runtime API version due to UnknownBlock error: {}", + s, + ); + RuntimeApiError::UnknownBlock{runtime_api_name: stringify!($api_name)} + } else { + gum::warn!( + target: LOG_TARGET, + "cannot query the runtime API version due to error: {}", + e, + ); + RuntimeApiError::Execution{runtime_api_name: stringify!($api_name), source: std::sync::Arc::new(e)} + } }) - .unwrap_or_else(|| { - gum::warn!( - target: LOG_TARGET, - "no runtime version is reported" - ); - 0 + .map(|v| { + match v { + Some(v) => v, + None => { + gum::warn!( + target: LOG_TARGET, + "no runtime version is reported" + ); + 0 + } + } }); - let res = if runtime_version >= version { - client.$api_name(relay_parent $(, $param.clone() )*).await + let res = match runtime_version { + Ok(runtime_version) if runtime_version >= version => { + client.$api_name(relay_parent $(, $param.clone() )*).await .map_err(|e| RuntimeApiError::Execution { runtime_api_name: stringify!($api_name), source: std::sync::Arc::new(e), }) - } else { - Err(RuntimeApiError::NotSupported { - runtime_api_name: stringify!($api_name), - }) + }, + Ok(_) => { + Err(RuntimeApiError::NotSupported { + runtime_api_name: stringify!($api_name), + }) + } + Err(e) => Err(e) }; + metrics.on_request(res.is_ok()); let _ = sender.send(res.clone()); diff --git a/node/subsystem-types/src/errors.rs b/node/subsystem-types/src/errors.rs index 48829e7fc779..b633a59cc88c 100644 --- a/node/subsystem-types/src/errors.rs +++ b/node/subsystem-types/src/errors.rs @@ -39,6 +39,16 @@ pub enum RuntimeApiError { /// The runtime API being called runtime_api_name: &'static str, }, + + /// The runtime API request can't be executed because the block at which the execution must + /// happen doesn't exist. This usually happens when the block is already pruned. + #[error( + "The runtime API {runtime_api_name} was called for an unknown (already pruned?) block" + )] + UnknownBlock { + /// The runtime API being called + runtime_api_name: &'static str, + }, } /// A description of an error causing the chain API request to be unservable. diff --git a/node/subsystem-types/src/lib.rs b/node/subsystem-types/src/lib.rs index 576507367095..fa4dd553a3e7 100644 --- a/node/subsystem-types/src/lib.rs +++ b/node/subsystem-types/src/lib.rs @@ -31,7 +31,7 @@ pub mod errors; pub mod messages; mod runtime_client; -pub use runtime_client::RuntimeApiSubsystemClient; +pub use runtime_client::{ApiError, RuntimeApiSubsystemClient}; pub use jaeger::*; pub use polkadot_node_jaeger as jaeger; diff --git a/node/subsystem-types/src/runtime_client.rs b/node/subsystem-types/src/runtime_client.rs index 9a55462b8852..81962c5db486 100644 --- a/node/subsystem-types/src/runtime_client.rs +++ b/node/subsystem-types/src/runtime_client.rs @@ -22,11 +22,12 @@ use polkadot_primitives::{ PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, }; -use sp_api::{ApiError, ApiExt, ProvideRuntimeApi}; +use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_authority_discovery::AuthorityDiscoveryApi; use sp_consensus_babe::{BabeApi, Epoch}; use std::collections::BTreeMap; +pub use sp_api::ApiError; /// Exposes all runtime calls that are used by the runtime API subsystem. #[async_trait] pub trait RuntimeApiSubsystemClient { From ededd9f3e1399a2bdab53e73f57841a5c495044c Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 26 Jan 2023 11:14:19 +0200 Subject: [PATCH 2/2] Fix prioritized selection in dispute-coordinator --- .../src/disputes/prioritized_selection/mod.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/node/core/provisioner/src/disputes/prioritized_selection/mod.rs b/node/core/provisioner/src/disputes/prioritized_selection/mod.rs index 7bf106a37a1c..d05517d9eb75 100644 --- a/node/core/provisioner/src/disputes/prioritized_selection/mod.rs +++ b/node/core/provisioner/src/disputes/prioritized_selection/mod.rs @@ -128,6 +128,15 @@ where ); HashMap::new() }, + Err(GetOnchainDisputesError::UnknownBlock(runtime_api_err, parent_hash)) => { + gum::warn!( + target: LOG_TARGET, + ?runtime_api_err, + ?parent_hash, + "Trying to fetch onchain disputes for unknown block.", + ); + HashMap::new() + }, }; let recent_disputes = request_disputes(sender).await;