From 45492e39e6c9a833445a781345343b53eec1f428 Mon Sep 17 00:00:00 2001 From: Stephane Gurgenidze <59443568+sw10pa@users.noreply.github.com> Date: Wed, 19 Mar 2025 11:34:53 +0400 Subject: [PATCH] runtime-api: remove redundant version checks (#7610) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue - [[#1940] Cleanup boilerplate code when DisabledValidators runtime api call is released](https://github.com/paritytech/polkadot-sdk/issues/1940) - Includes other runtime APIs, such as `MinimumBackingVotes` and `NodeFeatures`, that are already supported by Polkadot. ## Description This PR removes unnecessary runtime API version checks from `subsystem-util/src/runtime` for APIs supported by Polkadot (the most recent network to upgrade). Specifically, it applies to the `DisabledValidators`, `MinimumBackingVotes` and `NodeFeatures` APIs. --------- Co-authored-by: Bastian Köcher <git@kchr.de> --- polkadot/node/collation-generation/src/lib.rs | 14 +-- .../node/collation-generation/src/tests.rs | 2 +- polkadot/node/core/av-store/src/lib.rs | 2 +- polkadot/node/core/av-store/src/tests.rs | 21 ++-- polkadot/node/core/backing/src/lib.rs | 30 ++--- .../core/dispute-coordinator/src/tests.rs | 16 --- polkadot/node/core/provisioner/src/error.rs | 3 + polkadot/node/core/provisioner/src/lib.rs | 12 +- .../availability-distribution/src/error.rs | 6 +- .../src/requester/session_cache.rs | 10 +- .../src/tests/state.rs | 2 +- .../network/availability-recovery/src/lib.rs | 2 +- .../availability-recovery/src/tests.rs | 2 +- .../network/collator-protocol/src/error.rs | 3 + .../src/validator_side/mod.rs | 8 +- .../statement-distribution/src/error.rs | 8 +- .../statement-distribution/src/v2/mod.rs | 28 +++-- .../src/v2/tests/mod.rs | 5 +- .../src/lib/availability/test_state.rs | 2 +- .../subsystem-util/src/availability_chunks.rs | 70 ++++++------ polkadot/node/subsystem-util/src/lib.rs | 19 ++-- .../node/subsystem-util/src/runtime/mod.rs | 105 ++---------------- prdoc/pr_7610.prdoc | 30 +++++ 23 files changed, 171 insertions(+), 229 deletions(-) create mode 100644 prdoc/pr_7610.prdoc diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index 3c8a216f5f3..b4b3e009a6b 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -44,9 +44,9 @@ use polkadot_node_subsystem::{ SubsystemContext, SubsystemError, SubsystemResult, SubsystemSender, }; use polkadot_node_subsystem_util::{ - request_claim_queue, request_persisted_validation_data, request_session_index_for_child, - request_validation_code_hash, request_validators, - runtime::{request_node_features, ClaimQueueSnapshot}, + request_claim_queue, request_node_features, request_persisted_validation_data, + request_session_index_for_child, request_validation_code_hash, request_validators, + runtime::ClaimQueueSnapshot, }; use polkadot_primitives::{ collator_signature_payload, @@ -56,8 +56,7 @@ use polkadot_primitives::{ ClaimQueueOffset, CommittedCandidateReceiptV2, TransposedClaimQueue, }, CandidateCommitments, CandidateDescriptor, CollatorPair, CoreIndex, Hash, Id as ParaId, - NodeFeatures, OccupiedCoreAssumption, PersistedValidationData, SessionIndex, - ValidationCodeHash, + OccupiedCoreAssumption, PersistedValidationData, SessionIndex, ValidationCodeHash, }; use schnellru::{ByLength, LruMap}; use sp_core::crypto::Pair; @@ -510,9 +509,8 @@ impl SessionInfoCache { let n_validators = request_validators(relay_parent, &mut sender.clone()).await.await??.len(); - let node_features = request_node_features(relay_parent, session_index, sender) - .await? - .unwrap_or(NodeFeatures::EMPTY); + let node_features = + request_node_features(relay_parent, session_index, sender).await.await??; let info = PerSessionInfo { v2_receipts: node_features diff --git a/polkadot/node/collation-generation/src/tests.rs b/polkadot/node/collation-generation/src/tests.rs index dc1d7b3489c..99a2307260e 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -29,7 +29,7 @@ use polkadot_node_subsystem_util::TimeoutExt; use polkadot_primitives::{ node_features, vstaging::{CandidateDescriptorVersion, CoreSelector, UMPSignal, UMP_SEPARATOR}, - CollatorPair, PersistedValidationData, + CollatorPair, NodeFeatures, PersistedValidationData, }; use polkadot_primitives_test_helpers::dummy_head_data; use rstest::rstest; diff --git a/polkadot/node/core/av-store/src/lib.rs b/polkadot/node/core/av-store/src/lib.rs index 75a0a4b08ed..ac763e71c9c 100644 --- a/polkadot/node/core/av-store/src/lib.rs +++ b/polkadot/node/core/av-store/src/lib.rs @@ -1329,7 +1329,7 @@ fn store_available_data( }) .collect(); - let chunk_indices = availability_chunk_indices(Some(&node_features), n_validators, core_index)?; + let chunk_indices = availability_chunk_indices(&node_features, n_validators, core_index)?; for (validator_index, chunk_index) in chunk_indices.into_iter().enumerate() { write_chunk( &mut tx, diff --git a/polkadot/node/core/av-store/src/tests.rs b/polkadot/node/core/av-store/src/tests.rs index d1da058f9e4..a9d9e5ac60b 100644 --- a/polkadot/node/core/av-store/src/tests.rs +++ b/polkadot/node/core/av-store/src/tests.rs @@ -510,12 +510,8 @@ fn store_pov_and_queries_work() { let query_all_chunks_res = query_all_chunks( &mut virtual_overseer, - availability_chunk_indices( - Some(&node_features), - n_validators as usize, - core_index, - ) - .unwrap(), + availability_chunk_indices(&node_features, n_validators as usize, core_index) + .unwrap(), candidate_hash, ) .await; @@ -596,12 +592,8 @@ fn store_pov_and_queries_work() { let query_all_chunks_res = query_all_chunks( &mut virtual_overseer, - availability_chunk_indices( - Some(&node_features), - n_validators as usize, - core_index, - ) - .unwrap(), + availability_chunk_indices(&node_features, n_validators as usize, core_index) + .unwrap(), candidate_hash, ) .await; @@ -618,7 +610,7 @@ fn store_pov_and_queries_work() { .await .unwrap(); let expected_chunk_index = availability_chunk_index( - Some(&node_features), + &node_features, n_validators as usize, core_index, ValidatorIndex(validator_index), @@ -723,7 +715,8 @@ fn query_all_chunks_works() { } let chunk_indices = - availability_chunk_indices(None, n_validators as usize, CoreIndex(0)).unwrap(); + availability_chunk_indices(&NodeFeatures::EMPTY, n_validators as usize, CoreIndex(0)) + .unwrap(); assert_eq!( query_all_chunks(&mut virtual_overseer, chunk_indices.clone(), candidate_hash_1) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 8b54a8b5907..f5bc346131d 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -99,9 +99,10 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::{ self as util, backing_implicit_view::View as ImplicitView, - request_claim_queue, request_disabled_validators, request_session_executor_params, - request_session_index_for_child, request_validator_groups, request_validators, - runtime::{self, request_min_backing_votes, ClaimQueueSnapshot}, + request_claim_queue, request_disabled_validators, request_min_backing_votes, + request_node_features, request_session_executor_params, request_session_index_for_child, + request_validator_groups, request_validators, + runtime::{self, ClaimQueueSnapshot}, Validator, }; use polkadot_parachain_primitives::primitives::IsSystem; @@ -125,7 +126,6 @@ use polkadot_statement_table::{ Context as TableContextTrait, Table, }; use sp_keystore::KeystorePtr; -use util::runtime::request_node_features; mod error; @@ -260,7 +260,7 @@ struct PerSessionCache { /// Cache for storing validators list, retrieved from the runtime. validators_cache: LruMap<SessionIndex, Arc<Vec<ValidatorId>>>, /// Cache for storing node features, retrieved from the runtime. - node_features_cache: LruMap<SessionIndex, Option<NodeFeatures>>, + node_features_cache: LruMap<SessionIndex, NodeFeatures>, /// Cache for storing executor parameters, retrieved from the runtime. executor_params_cache: LruMap<SessionIndex, Arc<ExecutorParams>>, /// Cache for storing the minimum backing votes threshold, retrieved from the runtime. @@ -322,15 +322,20 @@ impl PerSessionCache { session_index: SessionIndex, parent: Hash, sender: &mut impl overseer::SubsystemSender<RuntimeApiMessage>, - ) -> Result<Option<NodeFeatures>, Error> { + ) -> Result<NodeFeatures, RuntimeApiError> { // Try to get the node features from the cache. if let Some(node_features) = self.node_features_cache.get(&session_index) { return Ok(node_features.clone()); } // Fetch the node features from the runtime since it was not in the cache. - let node_features: Option<NodeFeatures> = - request_node_features(parent, session_index, sender).await?; + let node_features = request_node_features(parent, session_index, sender) + .await + .await + .map_err(|err| RuntimeApiError::Execution { + runtime_api_name: "NodeFeatures", + source: Arc::new(err), + })??; // Cache the fetched node features for future use. self.node_features_cache.insert(session_index, node_features.clone()); @@ -388,11 +393,12 @@ impl PerSessionCache { // Fetch the value from the runtime since it was not in the cache. let minimum_backing_votes = request_min_backing_votes(parent, session_index, sender) + .await .await .map_err(|err| RuntimeApiError::Execution { runtime_api_name: "MinimumBackingVotes", source: Arc::new(err), - })?; + })??; // Cache the fetched value for future use. self.minimum_backing_votes_cache.insert(session_index, minimum_backing_votes); @@ -1152,10 +1158,8 @@ async fn construct_per_relay_parent_state<Context>( let validators = per_session_cache.validators(session_index, parent, ctx.sender()).await; let validators = try_runtime_api!(validators); - let node_features = per_session_cache - .node_features(session_index, parent, ctx.sender()) - .await? - .unwrap_or(NodeFeatures::EMPTY); + let node_features = per_session_cache.node_features(session_index, parent, ctx.sender()).await; + let node_features = try_runtime_api!(node_features); let inject_core_index = node_features .get(FeatureIndex::ElasticScalingMVP as usize) diff --git a/polkadot/node/core/dispute-coordinator/src/tests.rs b/polkadot/node/core/dispute-coordinator/src/tests.rs index 9383f71804e..658ce7cd98c 100644 --- a/polkadot/node/core/dispute-coordinator/src/tests.rs +++ b/polkadot/node/core/dispute-coordinator/src/tests.rs @@ -413,13 +413,6 @@ impl TestState { )) => { tx.send(Ok(Vec::new())).unwrap(); }, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - _new_leaf, - RuntimeApiRequest::Version(tx), - )) => { - tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT)) - .unwrap(); - }, AllMessages::RuntimeApi(RuntimeApiMessage::Request( _new_leaf, RuntimeApiRequest::DisabledValidators(tx), @@ -4387,15 +4380,6 @@ async fn handle_disabled_validators_queries( virtual_overseer: &mut VirtualOverseer, disabled_validators: Vec<ValidatorIndex>, ) { - assert_matches!( - virtual_overseer.recv().await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - _new_leaf, - RuntimeApiRequest::Version(tx), - )) => { - tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT)).unwrap(); - } - ); assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( diff --git a/polkadot/node/core/provisioner/src/error.rs b/polkadot/node/core/provisioner/src/error.rs index aae3234c3cc..08804c65f35 100644 --- a/polkadot/node/core/provisioner/src/error.rs +++ b/polkadot/node/core/provisioner/src/error.rs @@ -47,6 +47,9 @@ pub enum Error { #[error("failed to get session index")] CanceledSessionIndex(#[source] oneshot::Canceled), + #[error("failed to get node features")] + CanceledNodeFeatures(#[source] oneshot::Canceled), + #[error("failed to get backed candidates")] CanceledBackedCandidates(#[source] oneshot::Canceled), diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs index 4aced8eaefd..b36988c9f1d 100644 --- a/polkadot/node/core/provisioner/src/lib.rs +++ b/polkadot/node/core/provisioner/src/lib.rs @@ -35,14 +35,13 @@ use polkadot_node_subsystem::{ SubsystemError, }; use polkadot_node_subsystem_util::{ - request_availability_cores, request_session_index_for_child, runtime::request_node_features, - TimeoutExt, + request_availability_cores, request_node_features, request_session_index_for_child, TimeoutExt, }; use polkadot_primitives::{ node_features::FeatureIndex, vstaging::{BackedCandidate, CoreState}, - CandidateHash, CoreIndex, Hash, Id as ParaId, NodeFeatures, SessionIndex, - SignedAvailabilityBitfield, ValidatorIndex, + CandidateHash, CoreIndex, Hash, Id as ParaId, SessionIndex, SignedAvailabilityBitfield, + ValidatorIndex, }; use std::collections::{BTreeMap, HashMap}; @@ -203,8 +202,9 @@ async fn handle_active_leaves_update( .map_err(Error::CanceledSessionIndex)??; if per_session.get(&session_index).is_none() { let elastic_scaling_mvp = request_node_features(leaf.hash, session_index, sender) - .await? - .unwrap_or(NodeFeatures::EMPTY) + .await + .await + .map_err(Error::CanceledNodeFeatures)?? .get(FeatureIndex::ElasticScalingMVP as usize) .map(|b| *b) .unwrap_or(false); diff --git a/polkadot/node/network/availability-distribution/src/error.rs b/polkadot/node/network/availability-distribution/src/error.rs index 72a809dd114..852e7bbcbbb 100644 --- a/polkadot/node/network/availability-distribution/src/error.rs +++ b/polkadot/node/network/availability-distribution/src/error.rs @@ -23,7 +23,7 @@ use polkadot_primitives::SessionIndex; use futures::channel::oneshot; -use polkadot_node_subsystem::{ChainApiError, SubsystemError}; +use polkadot_node_subsystem::{ChainApiError, RuntimeApiError, SubsystemError}; use polkadot_node_subsystem_util::runtime; use crate::LOG_TARGET; @@ -55,6 +55,9 @@ pub enum Error { #[error("Retrieving response from Chain API unexpectedly failed with error: {0}")] ChainApi(#[from] ChainApiError), + #[error("Failed to get node features from the runtime")] + FailedNodeFeatures(#[source] RuntimeApiError), + // av-store will drop the sender on any error that happens. #[error("Response channel to obtain chunk failed")] QueryChunkResponseChannel(#[source] oneshot::Canceled), @@ -108,6 +111,7 @@ pub fn log_error( JfyiError::NoSuchCachedSession { .. } | JfyiError::QueryAvailableDataResponseChannel(_) | JfyiError::QueryChunkResponseChannel(_) | + JfyiError::FailedNodeFeatures(_) | JfyiError::ErasureCoding(_) => gum::warn!(target: LOG_TARGET, error = %jfyi, ctx), JfyiError::FetchPoV(_) | JfyiError::SendResponse | diff --git a/polkadot/node/network/availability-distribution/src/requester/session_cache.rs b/polkadot/node/network/availability-distribution/src/requester/session_cache.rs index a762c262dba..2fa5f006996 100644 --- a/polkadot/node/network/availability-distribution/src/requester/session_cache.rs +++ b/polkadot/node/network/availability-distribution/src/requester/session_cache.rs @@ -20,7 +20,7 @@ use rand::{seq::SliceRandom, thread_rng}; use schnellru::{ByLength, LruMap}; use polkadot_node_subsystem::overseer; -use polkadot_node_subsystem_util::runtime::{request_node_features, RuntimeInfo}; +use polkadot_node_subsystem_util::{request_node_features, runtime::RuntimeInfo}; use polkadot_primitives::{ AuthorityDiscoveryId, GroupIndex, Hash, NodeFeatures, SessionIndex, ValidatorIndex, }; @@ -66,7 +66,7 @@ pub struct SessionInfo { pub our_group: Option<GroupIndex>, /// Node features. - pub node_features: Option<NodeFeatures>, + pub node_features: NodeFeatures, } /// Report of bad validators. @@ -175,8 +175,10 @@ impl SessionCache { .get_session_info_by_index(ctx.sender(), relay_parent, session_index) .await?; - let node_features = - request_node_features(relay_parent, session_index, ctx.sender()).await?; + let node_features = request_node_features(relay_parent, session_index, ctx.sender()) + .await + .await? + .map_err(Error::FailedNodeFeatures)?; let discovery_keys = info.session_info.discovery_keys.clone(); let mut validator_groups = info.session_info.validator_groups.clone(); diff --git a/polkadot/node/network/availability-distribution/src/tests/state.rs b/polkadot/node/network/availability-distribution/src/tests/state.rs index c6dd17a344e..681e7891af3 100644 --- a/polkadot/node/network/availability-distribution/src/tests/state.rs +++ b/polkadot/node/network/availability-distribution/src/tests/state.rs @@ -109,7 +109,7 @@ impl TestState { let session_info = make_session_info(); let our_chunk_index = availability_chunk_index( - Some(&node_features), + &node_features, session_info.validators.len(), CoreIndex(1), ValidatorIndex(0), diff --git a/polkadot/node/network/availability-recovery/src/lib.rs b/polkadot/node/network/availability-recovery/src/lib.rs index eb54d9657d8..da499af5e48 100644 --- a/polkadot/node/network/availability-recovery/src/lib.rs +++ b/polkadot/node/network/availability-recovery/src/lib.rs @@ -485,7 +485,7 @@ async fn handle_recover<Context>( ) && chunk_mapping_enabled { let chunk_indices = - availability_chunk_indices(Some(node_features), n_validators, core_index)?; + availability_chunk_indices(node_features, n_validators, core_index)?; let chunk_indices: VecDeque<_> = chunk_indices .iter() diff --git a/polkadot/node/network/availability-recovery/src/tests.rs b/polkadot/node/network/availability-recovery/src/tests.rs index 9a46d542078..d9a3c13d00d 100644 --- a/polkadot/node/network/availability-recovery/src/tests.rs +++ b/polkadot/node/network/availability-recovery/src/tests.rs @@ -715,7 +715,7 @@ fn map_chunks( core_index: CoreIndex, ) -> IndexedVec<ValidatorIndex, ErasureChunk> { let chunk_indices = - availability_chunk_indices(Some(node_features), n_validators, core_index).unwrap(); + availability_chunk_indices(node_features, n_validators, core_index).unwrap(); (0..n_validators) .map(|val_idx| chunks[chunk_indices[val_idx].0 as usize].clone()) diff --git a/polkadot/node/network/collator-protocol/src/error.rs b/polkadot/node/network/collator-protocol/src/error.rs index 97fd4076bb8..b8cf686defe 100644 --- a/polkadot/node/network/collator-protocol/src/error.rs +++ b/polkadot/node/network/collator-protocol/src/error.rs @@ -71,6 +71,9 @@ pub enum Error { #[error("Response receiver for claim queue request cancelled")] CancelledClaimQueue(oneshot::Canceled), + #[error("Response receiver for node features request cancelled")] + CancelledNodeFeatures(oneshot::Canceled), + #[error("No state for the relay parent")] RelayParentStateNotFound, } diff --git a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs index 93a8c31168c..bb8c8cde643 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -49,8 +49,7 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::{ backing_implicit_view::View as ImplicitView, reputation::{ReputationAggregator, REPUTATION_CHANGE_INTERVAL}, - request_claim_queue, request_session_index_for_child, - runtime::request_node_features, + request_claim_queue, request_node_features, request_session_index_for_child, }; use polkadot_primitives::{ node_features, @@ -1283,8 +1282,9 @@ where .map_err(Error::CancelledSessionIndex)??; let v2_receipts = request_node_features(*leaf, session_index, sender) - .await? - .unwrap_or_default() + .await + .await + .map_err(Error::CancelledNodeFeatures)?? .get(node_features::FeatureIndex::CandidateReceiptV2 as usize) .map(|b| *b) .unwrap_or(false); diff --git a/polkadot/node/network/statement-distribution/src/error.rs b/polkadot/node/network/statement-distribution/src/error.rs index cff9afbf866..fb906669ce1 100644 --- a/polkadot/node/network/statement-distribution/src/error.rs +++ b/polkadot/node/network/statement-distribution/src/error.rs @@ -73,7 +73,7 @@ pub enum Error { FetchSessionInfo(RuntimeApiError), #[error("Fetching disabled validators failed {0:?}")] - FetchDisabledValidators(runtime::Error), + FetchDisabledValidators(RuntimeApiError), #[error("Fetching validator groups failed {0:?}")] FetchValidatorGroups(RuntimeApiError), @@ -81,6 +81,12 @@ pub enum Error { #[error("Fetching claim queue failed {0:?}")] FetchClaimQueue(RuntimeApiError), + #[error("Fetching minimum backing votes failed {0:?}")] + FetchMinimumBackingVotes(RuntimeApiError), + + #[error("Fetching node features failed {0:?}")] + FetchNodeFeatures(RuntimeApiError), + #[error("Attempted to share statement when not a validator or not assigned")] InvalidShare, diff --git a/polkadot/node/network/statement-distribution/src/v2/mod.rs b/polkadot/node/network/statement-distribution/src/v2/mod.rs index 3034ca7caf8..e6214978b92 100644 --- a/polkadot/node/network/statement-distribution/src/v2/mod.rs +++ b/polkadot/node/network/statement-distribution/src/v2/mod.rs @@ -43,16 +43,15 @@ use polkadot_node_subsystem::{ overseer, ActivatedLeaf, }; use polkadot_node_subsystem_util::{ - backing_implicit_view::View as ImplicitView, - reputation::ReputationAggregator, - runtime::{request_min_backing_votes, request_node_features, ClaimQueueSnapshot}, + backing_implicit_view::View as ImplicitView, reputation::ReputationAggregator, + request_min_backing_votes, request_node_features, runtime::ClaimQueueSnapshot, }; use polkadot_primitives::{ node_features::FeatureIndex, vstaging::{transpose_claim_queue, CandidateDescriptorVersion, TransposedClaimQueue}, AuthorityDiscoveryId, CandidateHash, CompactStatement, CoreIndex, GroupIndex, - GroupRotationInfo, Hash, Id as ParaId, IndexedVec, NodeFeatures, SessionIndex, SessionInfo, - SignedStatement, SigningContext, UncheckedSignedStatement, ValidatorId, ValidatorIndex, + GroupRotationInfo, Hash, Id as ParaId, IndexedVec, SessionIndex, SessionInfo, SignedStatement, + SigningContext, UncheckedSignedStatement, ValidatorId, ValidatorIndex, }; use sp_keystore::KeystorePtr; @@ -586,11 +585,13 @@ pub(crate) async fn handle_active_leaves_update<Context>( for new_relay_parent in new_relay_parents.iter().cloned() { let disabled_validators: HashSet<_> = - polkadot_node_subsystem_util::runtime::get_disabled_validators_with_fallback( - ctx.sender(), + polkadot_node_subsystem_util::request_disabled_validators( new_relay_parent, + ctx.sender(), ) .await + .await + .map_err(JfyiError::RuntimeApiUnavailable)? .map_err(JfyiError::FetchDisabledValidators)? .into_iter() .collect(); @@ -629,15 +630,22 @@ pub(crate) async fn handle_active_leaves_update<Context>( }; let minimum_backing_votes = - request_min_backing_votes(new_relay_parent, session_index, ctx.sender()).await?; + request_min_backing_votes(new_relay_parent, session_index, ctx.sender()) + .await + .await + .map_err(JfyiError::RuntimeApiUnavailable)? + .map_err(JfyiError::FetchMinimumBackingVotes)?; let node_features = - request_node_features(new_relay_parent, session_index, ctx.sender()).await?; + request_node_features(new_relay_parent, session_index, ctx.sender()) + .await + .await + .map_err(JfyiError::RuntimeApiUnavailable)? + .map_err(JfyiError::FetchNodeFeatures)?; let mut per_session_state = PerSessionState::new( session_info, &state.keystore, minimum_backing_votes, node_features - .unwrap_or(NodeFeatures::EMPTY) .get(FeatureIndex::CandidateReceiptV2 as usize) .map(|b| *b) .unwrap_or(false), diff --git a/polkadot/node/network/statement-distribution/src/v2/tests/mod.rs b/polkadot/node/network/statement-distribution/src/v2/tests/mod.rs index da3715e3873..619d9d46336 100644 --- a/polkadot/node/network/statement-distribution/src/v2/tests/mod.rs +++ b/polkadot/node/network/statement-distribution/src/v2/tests/mod.rs @@ -34,8 +34,9 @@ use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_node_subsystem_util::TimeoutExt; use polkadot_primitives::{ vstaging::CommittedCandidateReceiptV2 as CommittedCandidateReceipt, AssignmentPair, Block, - BlockNumber, GroupRotationInfo, HeadData, Header, IndexedVec, PersistedValidationData, - SessionIndex, SessionInfo, ValidatorPair, DEFAULT_SCHEDULING_LOOKAHEAD, + BlockNumber, GroupRotationInfo, HeadData, Header, IndexedVec, NodeFeatures, + PersistedValidationData, SessionIndex, SessionInfo, ValidatorPair, + DEFAULT_SCHEDULING_LOOKAHEAD, }; use sc_keystore::LocalKeystore; use sc_network::ProtocolName; diff --git a/polkadot/node/subsystem-bench/src/lib/availability/test_state.rs b/polkadot/node/subsystem-bench/src/lib/availability/test_state.rs index 764572ffe19..450896c47c8 100644 --- a/polkadot/node/subsystem-bench/src/lib/availability/test_state.rs +++ b/polkadot/node/subsystem-bench/src/lib/availability/test_state.rs @@ -118,7 +118,7 @@ impl TestState { test_state.chunk_indices = (0..config.n_cores) .map(|core_index| { availability_chunk_indices( - Some(&default_node_features()), + &default_node_features(), config.n_validators, CoreIndex(core_index as u32), ) diff --git a/polkadot/node/subsystem-util/src/availability_chunks.rs b/polkadot/node/subsystem-util/src/availability_chunks.rs index 651dd3633cf..f44fcd2a7af 100644 --- a/polkadot/node/subsystem-util/src/availability_chunks.rs +++ b/polkadot/node/subsystem-util/src/availability_chunks.rs @@ -22,21 +22,20 @@ use polkadot_primitives::{node_features, ChunkIndex, CoreIndex, NodeFeatures, Va /// Any modification to the output of the function needs to be coordinated via the runtime. /// It's best to use minimal/no external dependencies. pub fn availability_chunk_index( - maybe_node_features: Option<&NodeFeatures>, + node_features: &NodeFeatures, n_validators: usize, core_index: CoreIndex, validator_index: ValidatorIndex, ) -> Result<ChunkIndex, polkadot_erasure_coding::Error> { - if let Some(features) = maybe_node_features { - if let Some(&true) = features - .get(usize::from(node_features::FeatureIndex::AvailabilityChunkMapping as u8)) - .as_deref() - { - let systematic_threshold = systematic_recovery_threshold(n_validators)? as u32; - let core_start_pos = core_index.0 * systematic_threshold; - - return Ok(ChunkIndex((core_start_pos + validator_index.0) % n_validators as u32)) - } + if node_features + .get(usize::from(node_features::FeatureIndex::AvailabilityChunkMapping as u8)) + .map(|bitref| *bitref) + .unwrap_or_default() + { + let systematic_threshold = systematic_recovery_threshold(n_validators)? as u32; + let core_start_pos = core_index.0 * systematic_threshold; + + return Ok(ChunkIndex((core_start_pos + validator_index.0) % n_validators as u32)) } Ok(validator_index.into()) @@ -48,26 +47,25 @@ pub fn availability_chunk_index( /// Any modification to the output of the function needs to be coordinated via the /// runtime. It's best to use minimal/no external dependencies. pub fn availability_chunk_indices( - maybe_node_features: Option<&NodeFeatures>, + node_features: &NodeFeatures, n_validators: usize, core_index: CoreIndex, ) -> Result<Vec<ChunkIndex>, polkadot_erasure_coding::Error> { let identity = (0..n_validators).map(|index| ChunkIndex(index as u32)); - if let Some(features) = maybe_node_features { - if let Some(&true) = features - .get(usize::from(node_features::FeatureIndex::AvailabilityChunkMapping as u8)) - .as_deref() - { - let systematic_threshold = systematic_recovery_threshold(n_validators)? as u32; - let core_start_pos = core_index.0 * systematic_threshold; - - return Ok(identity - .into_iter() - .cycle() - .skip(core_start_pos as usize) - .take(n_validators) - .collect()) - } + if node_features + .get(usize::from(node_features::FeatureIndex::AvailabilityChunkMapping as u8)) + .map(|bitref| *bitref) + .unwrap_or_default() + { + let systematic_threshold = systematic_recovery_threshold(n_validators)? as u32; + let core_start_pos = core_index.0 * systematic_threshold; + + return Ok(identity + .into_iter() + .cycle() + .skip(core_start_pos as usize) + .take(n_validators) + .collect()) } Ok(identity.collect()) @@ -102,9 +100,7 @@ mod tests { // If the mapping feature is not enabled, it should always be the identity vector. { - for node_features in - [None, Some(NodeFeatures::EMPTY), Some(node_features_with_other_bits_enabled())] - { + for node_features in [NodeFeatures::EMPTY, node_features_with_other_bits_enabled()] { for core_index in 0..n_cores { let indices = availability_chunk_indices( node_features.as_ref(), @@ -141,7 +137,7 @@ mod tests { for core_index in 0..n_cores { let indices = availability_chunk_indices( - Some(&node_features), + &node_features, n_validators as usize, CoreIndex(core_index), ) @@ -151,7 +147,7 @@ mod tests { assert_eq!( indices[validator_index as usize], availability_chunk_index( - Some(&node_features), + &node_features, n_validators as usize, CoreIndex(core_index), ValidatorIndex(validator_index) @@ -184,7 +180,7 @@ mod tests { let node_features = node_features_with_mapping_enabled(); assert_eq!( - availability_chunk_indices(Some(&node_features), n_validators, CoreIndex(0)) + availability_chunk_indices(&node_features, n_validators, CoreIndex(0)) .unwrap() .into_iter() .map(|i| i.0) @@ -192,7 +188,7 @@ mod tests { vec![0, 1, 2, 3, 4, 5, 6] ); assert_eq!( - availability_chunk_indices(Some(&node_features), n_validators, CoreIndex(1)) + availability_chunk_indices(&node_features, n_validators, CoreIndex(1)) .unwrap() .into_iter() .map(|i| i.0) @@ -200,7 +196,7 @@ mod tests { vec![2, 3, 4, 5, 6, 0, 1] ); assert_eq!( - availability_chunk_indices(Some(&node_features), n_validators, CoreIndex(2)) + availability_chunk_indices(&node_features, n_validators, CoreIndex(2)) .unwrap() .into_iter() .map(|i| i.0) @@ -208,7 +204,7 @@ mod tests { vec![4, 5, 6, 0, 1, 2, 3] ); assert_eq!( - availability_chunk_indices(Some(&node_features), n_validators, CoreIndex(3)) + availability_chunk_indices(&node_features, n_validators, CoreIndex(3)) .unwrap() .into_iter() .map(|i| i.0) @@ -216,7 +212,7 @@ mod tests { vec![6, 0, 1, 2, 3, 4, 5] ); assert_eq!( - availability_chunk_indices(Some(&node_features), n_validators, CoreIndex(4)) + availability_chunk_indices(&node_features, n_validators, CoreIndex(4)) .unwrap() .into_iter() .map(|i| i.0) diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 6b069ee8611..69f4a7c3523 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -48,12 +48,11 @@ use polkadot_primitives::{ ScrapedOnChainVotes, }, AsyncBackingParams, AuthorityDiscoveryId, CandidateHash, CoreIndex, EncodeAs, ExecutorParams, - GroupIndex, GroupRotationInfo, Hash, Id as ParaId, OccupiedCoreAssumption, + GroupIndex, GroupRotationInfo, Hash, Id as ParaId, NodeFeatures, OccupiedCoreAssumption, PersistedValidationData, SessionIndex, SessionInfo, Signed, SigningContext, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, }; pub use rand; -use runtime::get_disabled_validators_with_fallback; use sp_application_crypto::AppCrypto; use sp_core::ByteArray; use sp_keystore::{Error as KeystoreError, KeystorePtr}; @@ -315,6 +314,8 @@ specialize_requests! { fn request_claim_queue() -> BTreeMap<CoreIndex, VecDeque<ParaId>>; ClaimQueue; fn request_para_backing_state(para_id: ParaId) -> Option<BackingState>; ParaBackingState; fn request_backing_constraints(para_id: ParaId) -> Option<Constraints>; BackingConstraints; + fn request_min_backing_votes(session_index: SessionIndex) -> u32; MinimumBackingVotes; + fn request_node_features(session_index: SessionIndex) -> NodeFeatures; NodeFeatures; } @@ -476,11 +477,12 @@ impl Validator { where S: SubsystemSender<RuntimeApiMessage>, { - // Note: request_validators and request_session_index_for_child do not and cannot - // run concurrently: they both have a mutable handle to the same sender. + // Note: request_validators, request_disabled_validators and request_session_index_for_child + // do not and cannot run concurrently: they both have a mutable handle to the same sender. // However, each of them returns a oneshot::Receiver, and those are resolved concurrently. - let (validators, session_index) = futures::try_join!( + let (validators, disabled_validators, session_index) = futures::try_join!( request_validators(parent, sender).await, + request_disabled_validators(parent, sender).await, request_session_index_for_child(parent, sender).await, )?; @@ -488,12 +490,7 @@ impl Validator { let validators = validators?; - // TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 - // When `DisabledValidators` is released remove this and add a - // `request_disabled_validators` call here - let disabled_validators = get_disabled_validators_with_fallback(sender, parent) - .await - .map_err(|e| Error::try_from(e).expect("the conversion is infallible; qed"))?; + let disabled_validators = disabled_validators?; Self::construct(&validators, &disabled_validators, signing_context, keystore) } diff --git a/polkadot/node/subsystem-util/src/runtime/mod.rs b/polkadot/node/subsystem-util/src/runtime/mod.rs index 46a009ae5c8..1fbae0e0987 100644 --- a/polkadot/node/subsystem-util/src/runtime/mod.rs +++ b/polkadot/node/subsystem-util/src/runtime/mod.rs @@ -36,15 +36,15 @@ use polkadot_primitives::{ CandidateHash, CoreIndex, EncodeAs, ExecutorParams, GroupIndex, GroupRotationInfo, Hash, Id as ParaId, IndexedVec, NodeFeatures, SessionIndex, SessionInfo, Signed, SigningContext, UncheckedSigned, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, - DEFAULT_SCHEDULING_LOOKAHEAD, LEGACY_MIN_BACKING_VOTES, + DEFAULT_SCHEDULING_LOOKAHEAD, }; use std::collections::{BTreeMap, VecDeque}; use crate::{ - has_required_runtime, request_availability_cores, request_candidate_events, - request_claim_queue, request_disabled_validators, request_from_runtime, - request_key_ownership_proof, request_on_chain_votes, request_session_executor_params, + request_availability_cores, request_candidate_events, request_claim_queue, + request_disabled_validators, request_from_runtime, request_key_ownership_proof, + request_node_features, request_on_chain_votes, request_session_executor_params, request_session_index_for_child, request_session_info, request_submit_report_dispute_lost, request_unapplied_slashes, request_validation_code_by_hash, request_validator_groups, }; @@ -202,7 +202,7 @@ impl RuntimeInfo { Some(result) => Ok(result), None => { let disabled_validators = - get_disabled_validators_with_fallback(sender, relay_parent).await?; + request_disabled_validators(relay_parent, sender).await.await??; self.disabled_validators_cache.insert(relay_parent, disabled_validators.clone()); Ok(disabled_validators) }, @@ -235,9 +235,8 @@ impl RuntimeInfo { let validator_info = self.get_validator_info(&session_info)?; - let node_features = request_node_features(parent, session_index, sender) - .await? - .unwrap_or(NodeFeatures::EMPTY); + let node_features = + request_node_features(parent, session_index, sender).await.await??; let last_set_index = node_features.iter_ones().last().unwrap_or_default(); if last_set_index >= FeatureIndex::FirstUnassigned as usize { gum::warn!(target: LOG_TARGET, "Runtime requires feature bit {} that node doesn't support, please upgrade node version", last_set_index); @@ -468,64 +467,6 @@ where .await } -/// Request the min backing votes value. -/// Prior to runtime API version 6, just return a hardcoded constant. -pub async fn request_min_backing_votes( - parent: Hash, - session_index: SessionIndex, - sender: &mut impl overseer::SubsystemSender<RuntimeApiMessage>, -) -> Result<u32> { - let min_backing_votes_res = recv_runtime( - request_from_runtime(parent, sender, |tx| { - RuntimeApiRequest::MinimumBackingVotes(session_index, tx) - }) - .await, - ) - .await; - - if let Err(Error::RuntimeRequest(RuntimeApiError::NotSupported { .. })) = min_backing_votes_res - { - gum::trace!( - target: LOG_TARGET, - ?parent, - "Querying the backing threshold from the runtime is not supported by the current Runtime API", - ); - - Ok(LEGACY_MIN_BACKING_VOTES) - } else { - min_backing_votes_res - } -} - -/// Request the node features enabled in the runtime. -/// Pass in the session index for caching purposes, as it should only change on session boundaries. -/// Prior to runtime API version 9, just return `None`. -pub async fn request_node_features( - parent: Hash, - session_index: SessionIndex, - sender: &mut impl overseer::SubsystemSender<RuntimeApiMessage>, -) -> Result<Option<NodeFeatures>> { - let res = recv_runtime( - request_from_runtime(parent, sender, |tx| { - RuntimeApiRequest::NodeFeatures(session_index, tx) - }) - .await, - ) - .await; - - if let Err(Error::RuntimeRequest(RuntimeApiError::NotSupported { .. })) = res { - gum::trace!( - target: LOG_TARGET, - ?parent, - "Querying the node features from the runtime is not supported by the current Runtime API", - ); - - Ok(None) - } else { - res.map(Some) - } -} - /// A snapshot of the runtime claim queue at an arbitrary relay chain block. #[derive(Default)] pub struct ClaimQueueSnapshot(pub BTreeMap<CoreIndex, VecDeque<ParaId>>); @@ -568,34 +509,6 @@ impl ClaimQueueSnapshot { } } -// TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 -/// Returns disabled validators list if the runtime supports it. Otherwise logs a debug messages and -/// returns an empty vec. -/// Once runtime ver `DISABLED_VALIDATORS_RUNTIME_REQUIREMENT` is released remove this function and -/// replace all usages with `request_disabled_validators` -pub async fn get_disabled_validators_with_fallback<Sender: SubsystemSender<RuntimeApiMessage>>( - sender: &mut Sender, - relay_parent: Hash, -) -> Result<Vec<ValidatorIndex>> { - let disabled_validators = if has_required_runtime( - sender, - relay_parent, - RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, - ) - .await - { - request_disabled_validators(relay_parent, sender) - .await - .await - .map_err(Error::RuntimeRequestCanceled)?? - } else { - gum::debug!(target: LOG_TARGET, "Runtime doesn't support `DisabledValidators` - continuing with an empty disabled validators set"); - vec![] - }; - - Ok(disabled_validators) -} - /// Fetch the claim queue and wrap it into a helpful `ClaimQueueSnapshot` pub async fn fetch_claim_queue( sender: &mut impl SubsystemSender<RuntimeApiMessage>, @@ -609,8 +522,8 @@ pub async fn fetch_claim_queue( Ok(cq.into()) } -/// Checks if the runtime supports `request_claim_queue` and attempts to fetch the claim queue. -/// Returns `ClaimQueueSnapshot` or `None` if claim queue API is not supported by runtime. +/// Returns the lookahead from the scheduler params if the runtime supports it, +/// or default value if scheduling lookahead API is not supported by runtime. pub async fn fetch_scheduling_lookahead( parent: Hash, session_index: SessionIndex, diff --git a/prdoc/pr_7610.prdoc b/prdoc/pr_7610.prdoc new file mode 100644 index 00000000000..e20ec54e5ff --- /dev/null +++ b/prdoc/pr_7610.prdoc @@ -0,0 +1,30 @@ +title: "runtime-api: remove redundant version checks" + +doc: + - audience: Node Dev + description: | + This PR removes unnecessary runtime API version checks for APIs supported + by Polkadot (the most recent network to upgrade). Specifically, it applies + to the `DisabledValidators`, `MinimumBackingVotes` and `NodeFeatures` APIs. + +crates: + - name: polkadot-node-subsystem-util + bump: major + - name: polkadot-statement-distribution + bump: major + - name: polkadot-availability-distribution + bump: patch + - name: polkadot-availability-recovery + bump: patch + - name: polkadot-collator-protocol + bump: patch + - name: polkadot-node-collation-generation + bump: patch + - name: polkadot-node-core-av-store + bump: patch + - name: polkadot-node-core-backing + bump: patch + - name: polkadot-node-core-dispute-coordinator + bump: patch + - name: polkadot-node-core-provisioner + bump: patch -- GitLab