From 5feadf44b0f252a7a2535f73e6c8dbf094554d6d Mon Sep 17 00:00:00 2001 From: alindima Date: Mon, 21 Aug 2023 14:08:46 +0300 Subject: [PATCH 1/5] move min backing votes const to runtime also cache it per-session in the backing subsystem Signed-off-by: alindima --- node/core/backing/src/lib.rs | 55 ++++++++++++++----- node/core/backing/src/tests/mod.rs | 39 +++++++++---- .../src/tests/prospective_parachains.rs | 40 ++++++++++---- node/core/runtime-api/src/cache.rs | 15 +++++ node/core/runtime-api/src/lib.rs | 8 +++ node/service/src/fake_runtime_api.rs | 4 ++ node/subsystem-types/src/messages.rs | 2 + node/subsystem-types/src/runtime_client.rs | 7 +++ node/subsystem-util/src/runtime/mod.rs | 42 ++++++++++++-- primitives/src/runtime_api.rs | 3 + runtime/kusama/src/lib.rs | 4 ++ runtime/parachains/src/configuration.rs | 4 ++ .../src/configuration/migration/v8.rs | 1 + runtime/parachains/src/inclusion/mod.rs | 18 ++---- runtime/parachains/src/runtime_api_impl/v5.rs | 6 ++ runtime/polkadot/src/lib.rs | 4 ++ runtime/rococo/src/lib.rs | 4 ++ runtime/test-runtime/src/lib.rs | 4 ++ runtime/westend/src/lib.rs | 4 ++ statement-table/src/generic.rs | 28 +++++----- 20 files changed, 226 insertions(+), 66 deletions(-) diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 58763e6d80cc..fa11183a5cc0 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -80,8 +80,8 @@ use futures::{ use error::{Error, FatalResult}; use polkadot_node_primitives::{ - minimum_votes, AvailableData, InvalidCandidate, PoV, SignedFullStatementWithPVD, - StatementWithPVD, ValidationResult, + AvailableData, InvalidCandidate, PoV, SignedFullStatementWithPVD, StatementWithPVD, + ValidationResult, }; use polkadot_node_subsystem::{ messages::{ @@ -96,8 +96,7 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::{ self as util, backing_implicit_view::{FetchError as ImplicitViewFetchError, View as ImplicitView}, - request_from_runtime, request_session_index_for_child, request_validator_groups, - request_validators, + request_from_runtime, request_validator_groups, request_validators, runtime::{prospective_parachains_mode, ProspectiveParachainsMode}, Validator, }; @@ -116,6 +115,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; +use util::runtime::RuntimeInfo; mod error; @@ -219,6 +219,8 @@ struct PerRelayParentState { awaiting_validation: HashSet, /// Data needed for retrying in case of `ValidatedCandidateCommand::AttestNoPoV`. fallbacks: HashMap, + /// The minimum backing votes threshold. + minimum_backing_votes: u32, } struct PerCandidateState { @@ -275,6 +277,8 @@ struct State { background_validation_tx: mpsc::Sender<(Hash, ValidatedCandidateCommand)>, /// The handle to the keystore used for signing. keystore: KeystorePtr, + /// The minimum backing votes threshold. + runtime_info: RuntimeInfo, } impl State { @@ -289,6 +293,7 @@ impl State { per_candidate: HashMap::new(), background_validation_tx, keystore, + runtime_info: RuntimeInfo::new(None), } } } @@ -400,8 +405,8 @@ impl TableContextTrait for TableContext { self.groups.get(group).map_or(false, |g| g.iter().any(|a| a == authority)) } - fn requisite_votes(&self, group: &ParaId) -> usize { - self.groups.get(group).map_or(usize::MAX, |g| minimum_votes(g.len())) + fn get_group_size(&self, group: &ParaId) -> Option { + self.groups.get(group).map(|g| g.len()) } } @@ -943,7 +948,14 @@ async fn handle_active_leaves_update( // construct a `PerRelayParent` from the runtime API // and insert it. - let per = construct_per_relay_parent_state(ctx, maybe_new, &state.keystore, mode).await?; + let per = construct_per_relay_parent_state( + ctx, + maybe_new, + &state.keystore, + &mut state.runtime_info, + mode, + ) + .await?; if let Some(per) = per { state.per_relay_parent.insert(maybe_new, per); @@ -959,6 +971,7 @@ async fn construct_per_relay_parent_state( ctx: &mut Context, relay_parent: Hash, keystore: &KeystorePtr, + runtime_info: &mut RuntimeInfo, mode: ProspectiveParachainsMode, ) -> Result, Error> { macro_rules! try_runtime_api { @@ -983,10 +996,14 @@ async fn construct_per_relay_parent_state( let parent = relay_parent; - let (validators, groups, session_index, cores) = futures::try_join!( + let session_index = + try_runtime_api!(runtime_info.get_session_index_for_child(ctx.sender(), parent).await); + let minimum_backing_votes = + runtime_info.get_min_backing_votes(ctx.sender(), session_index, parent).await; + + let (validators, groups, cores) = futures::try_join!( request_validators(parent, ctx.sender()).await, request_validator_groups(parent, ctx.sender()).await, - request_session_index_for_child(parent, ctx.sender()).await, request_from_runtime(parent, ctx.sender(), |tx| { RuntimeApiRequest::AvailabilityCores(tx) },) @@ -996,8 +1013,8 @@ async fn construct_per_relay_parent_state( let validators: Vec<_> = try_runtime_api!(validators); let (validator_groups, group_rotation_info) = try_runtime_api!(groups); - let session_index = try_runtime_api!(session_index); let cores = try_runtime_api!(cores); + let minimum_backing_votes = try_runtime_api!(minimum_backing_votes); let signing_context = SigningContext { parent_hash: parent, session_index }; let validator = @@ -1061,6 +1078,7 @@ async fn construct_per_relay_parent_state( issued_statements: HashSet::new(), awaiting_validation: HashSet::new(), fallbacks: HashMap::new(), + minimum_backing_votes, })) } @@ -1563,10 +1581,13 @@ async fn post_import_statement_actions( rp_state: &mut PerRelayParentState, summary: Option<&TableSummary>, ) -> Result<(), Error> { - if let Some(attested) = summary - .as_ref() - .and_then(|s| rp_state.table.attested_candidate(&s.candidate, &rp_state.table_context)) - { + if let Some(attested) = summary.as_ref().and_then(|s| { + rp_state.table.attested_candidate( + &s.candidate, + &rp_state.table_context, + rp_state.minimum_backing_votes, + ) + }) { let candidate_hash = attested.candidate.hash(); // `HashSet::insert` returns true if the thing wasn't in there already. @@ -2009,7 +2030,11 @@ fn handle_get_backed_candidates_message( }; rp_state .table - .attested_candidate(&candidate_hash, &rp_state.table_context) + .attested_candidate( + &candidate_hash, + &rp_state.table_context, + rp_state.minimum_backing_votes, + ) .and_then(|attested| table_attested_to_backed(attested, &rp_state.table_context)) }) .collect(); diff --git a/node/core/backing/src/tests/mod.rs b/node/core/backing/src/tests/mod.rs index 054337669c07..5bb8326d409c 100644 --- a/node/core/backing/src/tests/mod.rs +++ b/node/core/backing/src/tests/mod.rs @@ -80,6 +80,7 @@ struct TestState { head_data: HashMap, signing_context: SigningContext, relay_parent: Hash, + minimum_backing_votes: u32, } impl TestState { @@ -150,6 +151,7 @@ impl Default for TestState { validation_data, signing_context, relay_parent, + minimum_backing_votes: 2, } } } @@ -250,33 +252,50 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS } ); - // Check that subsystem job issues a request for a validator set. + // Check that subsystem job issues a request for the session index for child. assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::Validators(tx)) + RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx)) ) if parent == test_state.relay_parent => { - tx.send(Ok(test_state.validator_public.clone())).unwrap(); + tx.send(Ok(test_state.signing_context.session_index)).unwrap(); } ); - // Check that subsystem job issues a request for the validator groups. + // Check if subsystem job issues a request for the minimum backing votes. + // This may or may not happen, depending if the minimum backing votes is already cached in the + // RuntimeInfo. + let next_message = { + let msg = virtual_overseer.recv().await; + match msg { + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + parent, + RuntimeApiRequest::MinimumBackingVotes(tx), + )) if parent == test_state.relay_parent => { + tx.send(Ok(test_state.minimum_backing_votes)).unwrap(); + virtual_overseer.recv().await + }, + _ => msg, + } + }; + + // Check that subsystem job issues a request for a validator set. assert_matches!( - virtual_overseer.recv().await, + next_message, AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::ValidatorGroups(tx)) + RuntimeApiMessage::Request(parent, RuntimeApiRequest::Validators(tx)) ) if parent == test_state.relay_parent => { - tx.send(Ok(test_state.validator_groups.clone())).unwrap(); + tx.send(Ok(test_state.validator_public.clone())).unwrap(); } ); - // Check that subsystem job issues a request for the session index for child. + // Check that subsystem job issues a request for the validator groups. assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx)) + RuntimeApiMessage::Request(parent, RuntimeApiRequest::ValidatorGroups(tx)) ) if parent == test_state.relay_parent => { - tx.send(Ok(test_state.signing_context.session_index)).unwrap(); + tx.send(Ok(test_state.validator_groups.clone())).unwrap(); } ); diff --git a/node/core/backing/src/tests/prospective_parachains.rs b/node/core/backing/src/tests/prospective_parachains.rs index 7c2773c8e3b6..93a8e94b98de 100644 --- a/node/core/backing/src/tests/prospective_parachains.rs +++ b/node/core/backing/src/tests/prospective_parachains.rs @@ -138,13 +138,41 @@ async fn activate_leaf( } for (hash, number) in ancestry_iter.take(requested_len) { - // Check that subsystem job issues a request for a validator set. let msg = match next_overseer_message.take() { Some(msg) => msg, None => virtual_overseer.recv().await, }; + + // Check that subsystem job issues a request for the session index for child. assert_matches!( msg, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx)) + ) if parent == hash => { + tx.send(Ok(test_state.signing_context.session_index)).unwrap(); + } + ); + + // Check if subsystem job issues a request for the minimum backing votes. + // This may or may not happen, depending if the minimum backing votes is already cached in + // the `RuntimeInfo`. + let next_message = { + let msg = virtual_overseer.recv().await; + match msg { + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + parent, + RuntimeApiRequest::MinimumBackingVotes(tx), + )) if parent == hash => { + tx.send(Ok(test_state.minimum_backing_votes)).unwrap(); + virtual_overseer.recv().await + }, + _ => msg, + } + }; + + // Check that subsystem job issues a request for a validator set. + assert_matches!( + next_message, AllMessages::RuntimeApi( RuntimeApiMessage::Request(parent, RuntimeApiRequest::Validators(tx)) ) if parent == hash => { @@ -164,16 +192,6 @@ async fn activate_leaf( } ); - // Check that subsystem job issues a request for the session index for child. - assert_matches!( - virtual_overseer.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx)) - ) if parent == hash => { - tx.send(Ok(test_state.signing_context.session_index)).unwrap(); - } - ); - // Check that subsystem job issues a request for the availability cores. assert_matches!( virtual_overseer.recv().await, diff --git a/node/core/runtime-api/src/cache.rs b/node/core/runtime-api/src/cache.rs index 26aaf3fb6ec8..07c8095fd34e 100644 --- a/node/core/runtime-api/src/cache.rs +++ b/node/core/runtime-api/src/cache.rs @@ -40,6 +40,7 @@ const DEFAULT_CACHE_CAP: NonZeroUsize = match NonZeroUsize::new(128) { pub(crate) struct RequestResultCache { authorities: LruCache>, validators: LruCache>, + minimum_backing_votes: LruCache, validator_groups: LruCache>, GroupRotationInfo)>, availability_cores: LruCache>, persisted_validation_data: @@ -78,6 +79,7 @@ impl Default for RequestResultCache { Self { authorities: LruCache::new(DEFAULT_CACHE_CAP), validators: LruCache::new(DEFAULT_CACHE_CAP), + minimum_backing_votes: LruCache::new(DEFAULT_CACHE_CAP), validator_groups: LruCache::new(DEFAULT_CACHE_CAP), availability_cores: LruCache::new(DEFAULT_CACHE_CAP), persisted_validation_data: LruCache::new(DEFAULT_CACHE_CAP), @@ -131,6 +133,18 @@ impl RequestResultCache { self.validators.put(relay_parent, validators); } + pub(crate) fn minimum_backing_votes(&mut self, relay_parent: &Hash) -> Option { + self.minimum_backing_votes.get(relay_parent).copied() + } + + pub(crate) fn cache_minimum_backing_votes( + &mut self, + relay_parent: Hash, + minimum_backing_votes: u32, + ) { + self.minimum_backing_votes.put(relay_parent, minimum_backing_votes); + } + pub(crate) fn validator_groups( &mut self, relay_parent: &Hash, @@ -472,6 +486,7 @@ pub(crate) enum RequestResult { // The structure of each variant is (relay_parent, [params,]*, result) Authorities(Hash, Vec), Validators(Hash, Vec), + MinimumBackingVotes(Hash, u32), ValidatorGroups(Hash, (Vec>, GroupRotationInfo)), AvailabilityCores(Hash, Vec), PersistedValidationData(Hash, ParaId, OccupiedCoreAssumption, Option), diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index 78531d41272b..37e63ff673cd 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -101,6 +101,9 @@ where self.requests_cache.cache_authorities(relay_parent, authorities), Validators(relay_parent, validators) => self.requests_cache.cache_validators(relay_parent, validators), + MinimumBackingVotes(relay_parent, minimum_backing_votes) => self + .requests_cache + .cache_minimum_backing_votes(relay_parent, minimum_backing_votes), ValidatorGroups(relay_parent, groups) => self.requests_cache.cache_validator_groups(relay_parent, groups), AvailabilityCores(relay_parent, cores) => @@ -301,6 +304,8 @@ where Request::StagingAsyncBackingParams(sender) => query!(staging_async_backing_params(), sender) .map(|sender| Request::StagingAsyncBackingParams(sender)), + Request::MinimumBackingVotes(sender) => query!(minimum_backing_votes(), sender) + .map(|sender| Request::MinimumBackingVotes(sender)), } } @@ -450,6 +455,9 @@ where Request::Authorities(sender) => query!(Authorities, authorities(), ver = 1, sender), Request::Validators(sender) => query!(Validators, validators(), ver = 1, sender), + Request::MinimumBackingVotes(sender) => + query!(MinimumBackingVotes, minimum_backing_votes(), ver = 1, sender), + Request::ValidatorGroups(sender) => { query!(ValidatorGroups, validator_groups(), ver = 1, sender) }, diff --git a/node/service/src/fake_runtime_api.rs b/node/service/src/fake_runtime_api.rs index d9553afa024b..f8eec2143352 100644 --- a/node/service/src/fake_runtime_api.rs +++ b/node/service/src/fake_runtime_api.rs @@ -121,6 +121,10 @@ sp_api::impl_runtime_apis! { unimplemented!() } + fn minimum_backing_votes() -> u32 { + unimplemented!() + } + fn validator_groups() -> (Vec>, GroupRotationInfo) { unimplemented!() } diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index 8adc39eed56d..aafec7c8bbee 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -606,6 +606,8 @@ pub enum RuntimeApiRequest { Authorities(RuntimeApiSender>), /// Get the current validator set. Validators(RuntimeApiSender>), + /// Get the minimum required backing votes. + MinimumBackingVotes(RuntimeApiSender), /// Get the validator groups and group rotation info. ValidatorGroups(RuntimeApiSender<(Vec>, GroupRotationInfo)>), /// Get information on all availability cores. diff --git a/node/subsystem-types/src/runtime_client.rs b/node/subsystem-types/src/runtime_client.rs index 312cc4eec6ce..72ea2113e4bc 100644 --- a/node/subsystem-types/src/runtime_client.rs +++ b/node/subsystem-types/src/runtime_client.rs @@ -40,6 +40,9 @@ pub trait RuntimeApiSubsystemClient { /// Get the current validators. async fn validators(&self, at: Hash) -> Result, ApiError>; + /// Get the minimum number of backing votes. + async fn minimum_backing_votes(&self, at: Hash) -> Result; + /// Returns the validator groups and rotation info localized based on the hypothetical child /// of a block whose state this is invoked on. Note that `now` in the `GroupRotationInfo` /// should be the successor of the number of the block. @@ -275,6 +278,10 @@ where self.client.runtime_api().validators(at) } + async fn minimum_backing_votes(&self, at: Hash) -> Result { + self.client.runtime_api().minimum_backing_votes(at) + } + async fn validator_groups( &self, at: Hash, diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index 1f5641e3ea95..534449ca7507 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -26,7 +26,9 @@ use sp_core::crypto::ByteArray; use sp_keystore::{Keystore, KeystorePtr}; use polkadot_node_subsystem::{ - errors::RuntimeApiError, messages::RuntimeApiMessage, overseer, SubsystemSender, + errors::RuntimeApiError, + messages::{RuntimeApiMessage, RuntimeApiRequest}, + overseer, SubsystemSender, }; use polkadot_primitives::{ vstaging, CandidateEvent, CandidateHash, CoreState, EncodeAs, GroupIndex, GroupRotationInfo, @@ -36,9 +38,9 @@ use polkadot_primitives::{ }; use crate::{ - request_availability_cores, request_candidate_events, request_key_ownership_proof, - request_on_chain_votes, request_session_index_for_child, request_session_info, - request_staging_async_backing_params, request_submit_report_dispute_lost, + request_availability_cores, request_candidate_events, request_from_runtime, + request_key_ownership_proof, request_on_chain_votes, request_session_index_for_child, + request_session_info, request_staging_async_backing_params, request_submit_report_dispute_lost, request_unapplied_slashes, request_validation_code_by_hash, request_validator_groups, }; @@ -74,6 +76,9 @@ pub struct RuntimeInfo { /// Look up cached sessions by `SessionIndex`. session_info_cache: LruCache, + /// Look up minimum validator backing votes threshold by `SessionIndex`. + min_backing_votes: LruCache, + /// Key store for determining whether we are a validator and what `ValidatorIndex` we have. keystore: Option, } @@ -120,6 +125,7 @@ impl RuntimeInfo { .max(NonZeroUsize::new(10).expect("10 is larger than 0; qed")), ), session_info_cache: LruCache::new(cfg.session_cache_lru_size), + min_backing_votes: LruCache::new(cfg.session_cache_lru_size), keystore: cfg.keystore, } } @@ -159,6 +165,34 @@ impl RuntimeInfo { self.get_session_info_by_index(sender, relay_parent, session_index).await } + /// Get minimum_backing_votes by relay parent hash. + pub async fn get_min_backing_votes<'a, Sender>( + &mut self, + sender: &mut Sender, + session_index: SessionIndex, + relay_parent: Hash, + ) -> Result + where + Sender: SubsystemSender, + { + if !self.min_backing_votes.contains(&session_index) { + let min_votes = recv_runtime( + request_from_runtime(relay_parent, sender, |tx| { + RuntimeApiRequest::MinimumBackingVotes(tx) + }) + .await, + ) + .await?; + + self.min_backing_votes.put(session_index, min_votes); + } + + Ok(*self + .min_backing_votes + .get(&session_index) + .expect("We just put the value there. qed.")) + } + /// Get `ExtendedSessionInfo` by session index. /// /// `request_session_info` still requires the parent to be passed in, so we take the parent diff --git a/primitives/src/runtime_api.rs b/primitives/src/runtime_api.rs index 483256fe20f3..0c338e85ef2e 100644 --- a/primitives/src/runtime_api.rs +++ b/primitives/src/runtime_api.rs @@ -240,6 +240,9 @@ sp_api::decl_runtime_apis! { key_ownership_proof: vstaging::slashing::OpaqueKeyOwnershipProof, ) -> Option<()>; + /// Get the minimum number of backing votes for a parachain candidate. + fn minimum_backing_votes() -> u32; + /***** Asynchronous backing *****/ /// Returns the state of parachain backing for a given para. diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index c7077e38a653..41fce1721c76 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1757,6 +1757,10 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } + fn minimum_backing_votes() -> u32 { + parachains_runtime_api_impl::minimum_backing_votes::() + } + fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index 03d1ae420495..f3ee2dc80865 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -242,6 +242,9 @@ pub struct HostConfiguration { /// /// This value should be greater than [`paras_availability_period`]. pub minimum_validation_upgrade_delay: BlockNumber, + /// The minimum number of valid backing statements required to consider a parachain candidate + /// backable. + pub minimum_backing_votes: u32, } impl> Default for HostConfiguration { @@ -292,6 +295,7 @@ impl> Default for HostConfiguration Default for ProcessedCandidates { } } -/// Number of backing votes we need for a valid backing. -/// -/// WARNING: This check has to be kept in sync with the node side checks. -pub fn minimum_backing_votes(n_validators: usize) -> usize { - // For considerations on this value see: - // https://github.com/paritytech/polkadot/pull/1656#issuecomment-999734650 - // and - // https://github.com/paritytech/polkadot/issues/4386 - sp_std::cmp::min(n_validators, 2) -} - /// Reads the footprint of queues for a specific origin type. pub trait QueueFootprinter { type Origin; @@ -622,6 +611,7 @@ impl Pallet { return Ok(ProcessedCandidates::default()) } + let minimum_backing_votes = configuration::Pallet::::config().minimum_backing_votes; let validators = shared::Pallet::::active_validator_keys(); // Collect candidate receipts with backers. @@ -738,7 +728,11 @@ impl Pallet { match maybe_amount_validated { Ok(amount_validated) => ensure!( - amount_validated >= minimum_backing_votes(group_vals.len()), + amount_validated >= + sp_std::cmp::min( + group_vals.len(), + minimum_backing_votes as usize + ), Error::::InsufficientBacking, ), Err(()) => { diff --git a/runtime/parachains/src/runtime_api_impl/v5.rs b/runtime/parachains/src/runtime_api_impl/v5.rs index cd1579689733..1f88833e2749 100644 --- a/runtime/parachains/src/runtime_api_impl/v5.rs +++ b/runtime/parachains/src/runtime_api_impl/v5.rs @@ -38,6 +38,12 @@ pub fn validators() -> Vec { >::active_validator_keys() } +/// Implementation of the `minimum_backing_votes` function of the runtime API. +pub fn minimum_backing_votes() -> u32 { + let config = >::config(); + config.minimum_backing_votes +} + /// Implementation for the `validator_groups` function of the runtime API. pub fn validator_groups( ) -> (Vec>, GroupRotationInfo>) { diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index ad10de445ab3..88b1d593aaab 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1695,6 +1695,10 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } + fn minimum_backing_votes() -> u32 { + parachains_runtime_api_impl::minimum_backing_votes::() + } + fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index fb2a56c8100c..997f65454f04 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -1720,6 +1720,10 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } + fn minimum_backing_votes() -> u32 { + parachains_runtime_api_impl::minimum_backing_votes::() + } + fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index b2397299430d..154ee6001f63 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -826,6 +826,10 @@ sp_api::impl_runtime_apis! { runtime_impl::validators::() } + fn minimum_backing_votes() -> u32 { + runtime_impl::minimum_backing_votes::() + } + fn validator_groups() -> (Vec>, GroupRotationInfo) { runtime_impl::validator_groups::() } diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 3ade28c51fba..67594220a13a 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1432,6 +1432,10 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } + fn minimum_backing_votes() -> u32 { + parachains_runtime_api_impl::minimum_backing_votes::() + } + fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } diff --git a/statement-table/src/generic.rs b/statement-table/src/generic.rs index a427aae42fb9..a40b39381bf0 100644 --- a/statement-table/src/generic.rs +++ b/statement-table/src/generic.rs @@ -57,8 +57,8 @@ pub trait Context { /// Members are meant to submit candidates and vote on validity. fn is_member_of(&self, authority: &Self::AuthorityId, group: &Self::GroupId) -> bool; - /// requisite number of votes for validity from a group. - fn requisite_votes(&self, group: &Self::GroupId) -> usize; + /// Get a validator group size. + fn get_group_size(&self, group: &Self::GroupId) -> Option; } /// Table configuration. @@ -319,9 +319,12 @@ impl Table { &self, digest: &Ctx::Digest, context: &Ctx, + minimum_backing_votes: u32, ) -> Option> { self.candidate_votes.get(digest).and_then(|data| { - let v_threshold = context.requisite_votes(&data.group_id); + let v_threshold = context + .get_group_size(&data.group_id) + .map_or(usize::MAX, |len| std::cmp::min(minimum_backing_votes as usize, len)); data.attested(v_threshold) }) } @@ -636,16 +639,13 @@ mod tests { self.authorities.get(authority).map(|v| v == group).unwrap_or(false) } - fn requisite_votes(&self, id: &GroupId) -> usize { - let mut total_validity = 0; - - for validity in self.authorities.values() { - if validity == id { - total_validity += 1 - } + fn get_group_size(&self, group: &Self::GroupId) -> Option { + let count = self.authorities.values().filter(|g| *g == group).count(); + if count == 0 { + None + } else { + Some(count) } - - total_validity / 2 + 1 } } @@ -910,7 +910,7 @@ mod tests { table.import_statement(&context, statement); assert!(!table.detected_misbehavior.contains_key(&AuthorityId(1))); - assert!(table.attested_candidate(&candidate_digest, &context).is_none()); + assert!(table.attested_candidate(&candidate_digest, &context, 2).is_none()); let vote = SignedStatement { statement: Statement::Valid(candidate_digest), @@ -920,7 +920,7 @@ mod tests { table.import_statement(&context, vote); assert!(!table.detected_misbehavior.contains_key(&AuthorityId(2))); - assert!(table.attested_candidate(&candidate_digest, &context).is_some()); + assert!(table.attested_candidate(&candidate_digest, &context, 2).is_some()); } #[test] From eb864e086cfb8e82d763f35da0c4e265b096cef9 Mon Sep 17 00:00:00 2001 From: alindima Date: Tue, 22 Aug 2023 11:33:41 +0300 Subject: [PATCH 2/5] add runtime migration --- node/core/backing/src/lib.rs | 1 + node/core/runtime-api/src/tests.rs | 4 + primitives/src/runtime_api.rs | 1 + runtime/kusama/src/lib.rs | 1 + runtime/parachains/src/configuration.rs | 17 +- .../parachains/src/configuration/migration.rs | 1 + .../src/configuration/migration/v8.rs | 105 +++++- .../src/configuration/migration/v9.rs | 321 ++++++++++++++++++ runtime/parachains/src/configuration/tests.rs | 6 + runtime/parachains/src/inclusion/tests.rs | 6 +- .../parachains/src/paras_inherent/tests.rs | 239 ++++++------- runtime/polkadot/src/lib.rs | 2 + runtime/rococo/src/lib.rs | 1 + runtime/westend/src/lib.rs | 1 + 14 files changed, 583 insertions(+), 123 deletions(-) create mode 100644 runtime/parachains/src/configuration/migration/v9.rs diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index fa11183a5cc0..9af0d6046b96 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -1000,6 +1000,7 @@ async fn construct_per_relay_parent_state( try_runtime_api!(runtime_info.get_session_index_for_child(ctx.sender(), parent).await); let minimum_backing_votes = runtime_info.get_min_backing_votes(ctx.sender(), session_index, parent).await; + // TODO: if this does not exist, fall back to the hardcoded 2 value. let (validators, groups, cores) = futures::try_join!( request_validators(parent, ctx.sender()).await, diff --git a/node/core/runtime-api/src/tests.rs b/node/core/runtime-api/src/tests.rs index c3f8108312be..f448fa942198 100644 --- a/node/core/runtime-api/src/tests.rs +++ b/node/core/runtime-api/src/tests.rs @@ -264,6 +264,10 @@ impl RuntimeApiSubsystemClient for MockSubsystemClient { ) -> Result, ApiError> { todo!("Not required for tests") } + + async fn minimum_backing_votes(&self, _: Hash) -> Result { + todo!("Not required for tests") + } } #[test] diff --git a/primitives/src/runtime_api.rs b/primitives/src/runtime_api.rs index 0c338e85ef2e..d7eadc43ed69 100644 --- a/primitives/src/runtime_api.rs +++ b/primitives/src/runtime_api.rs @@ -241,6 +241,7 @@ sp_api::decl_runtime_apis! { ) -> Option<()>; /// Get the minimum number of backing votes for a parachain candidate. + /// TODO: bump api version here? fn minimum_backing_votes() -> u32; /***** Asynchronous backing *****/ diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 41fce1721c76..4ec57758f2f1 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1606,6 +1606,7 @@ pub mod migrations { frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, + parachains_configuration::migration::v9::MigrateToV9, ); } diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index f3ee2dc80865..6d6fb53a447b 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -295,7 +295,7 @@ impl> Default for HostConfiguration (remove UMP dispatch queue) /// v6-v7: /// v7-v8: - const STORAGE_VERSION: StorageVersion = StorageVersion::new(8); + /// v8-v9: + const STORAGE_VERSION: StorageVersion = StorageVersion::new(9); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -1154,6 +1155,18 @@ pub mod pallet { config.on_demand_ttl = new; }) } + /// Set the minimum backing votes threshold. + #[pallet::call_index(52)] + #[pallet::weight(( + T::WeightInfo::set_config_with_u32(), + DispatchClass::Operational + ))] + pub fn set_minimum_backing_votes(origin: OriginFor, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::schedule_config_update(|config| { + config.minimum_backing_votes = new; + }) + } } #[pallet::hooks] diff --git a/runtime/parachains/src/configuration/migration.rs b/runtime/parachains/src/configuration/migration.rs index 4499b116462b..26f8a85b496d 100644 --- a/runtime/parachains/src/configuration/migration.rs +++ b/runtime/parachains/src/configuration/migration.rs @@ -19,3 +19,4 @@ pub mod v6; pub mod v7; pub mod v8; +pub mod v9; diff --git a/runtime/parachains/src/configuration/migration/v8.rs b/runtime/parachains/src/configuration/migration/v8.rs index 2fe23f37fa60..5c5b34821835 100644 --- a/runtime/parachains/src/configuration/migration/v8.rs +++ b/runtime/parachains/src/configuration/migration/v8.rs @@ -23,14 +23,114 @@ use frame_support::{ weights::Weight, }; use frame_system::pallet_prelude::BlockNumberFor; -use primitives::SessionIndex; +use primitives::{ + vstaging::AsyncBackingParams, Balance, ExecutorParams, SessionIndex, + ON_DEMAND_DEFAULT_QUEUE_MAX_SIZE, +}; use sp_runtime::Perbill; use sp_std::vec::Vec; use frame_support::traits::OnRuntimeUpgrade; use super::v7::V7HostConfiguration; -type V8HostConfiguration = configuration::HostConfiguration; +/// All configuration of the runtime with respect to paras. +#[derive(Clone, Encode, Decode, Debug)] +pub struct V8HostConfiguration { + pub max_code_size: u32, + pub max_head_data_size: u32, + pub max_upward_queue_count: u32, + pub max_upward_queue_size: u32, + pub max_upward_message_size: u32, + pub max_upward_message_num_per_candidate: u32, + pub hrmp_max_message_num_per_candidate: u32, + pub validation_upgrade_cooldown: BlockNumber, + pub validation_upgrade_delay: BlockNumber, + pub async_backing_params: AsyncBackingParams, + pub max_pov_size: u32, + pub max_downward_message_size: u32, + pub hrmp_max_parachain_outbound_channels: u32, + pub hrmp_sender_deposit: Balance, + pub hrmp_recipient_deposit: Balance, + pub hrmp_channel_max_capacity: u32, + pub hrmp_channel_max_total_size: u32, + pub hrmp_max_parachain_inbound_channels: u32, + pub hrmp_channel_max_message_size: u32, + pub executor_params: ExecutorParams, + pub code_retention_period: BlockNumber, + pub on_demand_cores: u32, + pub on_demand_retries: u32, + pub on_demand_queue_max_size: u32, + pub on_demand_target_queue_utilization: Perbill, + pub on_demand_fee_variability: Perbill, + pub on_demand_base_fee: Balance, + pub on_demand_ttl: BlockNumber, + pub group_rotation_frequency: BlockNumber, + pub paras_availability_period: BlockNumber, + pub scheduling_lookahead: u32, + pub max_validators_per_core: Option, + pub max_validators: Option, + pub dispute_period: SessionIndex, + pub dispute_post_conclusion_acceptance_period: BlockNumber, + pub no_show_slots: u32, + pub n_delay_tranches: u32, + pub zeroth_delay_tranche_width: u32, + pub needed_approvals: u32, + pub relay_vrf_modulo_samples: u32, + pub pvf_voting_ttl: SessionIndex, + pub minimum_validation_upgrade_delay: BlockNumber, +} + +impl> Default for V8HostConfiguration { + fn default() -> Self { + Self { + async_backing_params: AsyncBackingParams { + max_candidate_depth: 0, + allowed_ancestry_len: 0, + }, + group_rotation_frequency: 1u32.into(), + paras_availability_period: 1u32.into(), + no_show_slots: 1u32.into(), + validation_upgrade_cooldown: Default::default(), + validation_upgrade_delay: 2u32.into(), + code_retention_period: Default::default(), + max_code_size: Default::default(), + max_pov_size: Default::default(), + max_head_data_size: Default::default(), + on_demand_cores: Default::default(), + on_demand_retries: Default::default(), + scheduling_lookahead: 1, + max_validators_per_core: Default::default(), + max_validators: None, + dispute_period: 6, + dispute_post_conclusion_acceptance_period: 100.into(), + n_delay_tranches: Default::default(), + zeroth_delay_tranche_width: Default::default(), + needed_approvals: Default::default(), + relay_vrf_modulo_samples: Default::default(), + max_upward_queue_count: Default::default(), + max_upward_queue_size: Default::default(), + max_downward_message_size: Default::default(), + max_upward_message_size: Default::default(), + max_upward_message_num_per_candidate: Default::default(), + hrmp_sender_deposit: Default::default(), + hrmp_recipient_deposit: Default::default(), + hrmp_channel_max_capacity: Default::default(), + hrmp_channel_max_total_size: Default::default(), + hrmp_max_parachain_inbound_channels: Default::default(), + hrmp_channel_max_message_size: Default::default(), + hrmp_max_parachain_outbound_channels: Default::default(), + hrmp_max_message_num_per_candidate: Default::default(), + pvf_voting_ttl: 2u32.into(), + minimum_validation_upgrade_delay: 2.into(), + executor_params: Default::default(), + on_demand_queue_max_size: ON_DEMAND_DEFAULT_QUEUE_MAX_SIZE, + on_demand_base_fee: 10_000_000u128, + on_demand_fee_variability: Perbill::from_percent(3), + on_demand_target_queue_utilization: Perbill::from_percent(25), + on_demand_ttl: 5u32.into(), + } + } +} mod v7 { use super::*; @@ -150,7 +250,6 @@ on_demand_base_fee : 10_000_000u128, on_demand_fee_variability : Perbill::from_percent(3), on_demand_target_queue_utilization : Perbill::from_percent(25), on_demand_ttl : 5u32.into(), -minimum_backing_votes : 2 } }; diff --git a/runtime/parachains/src/configuration/migration/v9.rs b/runtime/parachains/src/configuration/migration/v9.rs new file mode 100644 index 000000000000..56d89959258a --- /dev/null +++ b/runtime/parachains/src/configuration/migration/v9.rs @@ -0,0 +1,321 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot 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. + +// Polkadot 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 Polkadot. If not, see . + +//! A module that is responsible for migration of storage. + +use crate::configuration::{self, Config, Pallet}; +use frame_support::{ + pallet_prelude::*, + traits::{Defensive, StorageVersion}, + weights::Weight, +}; +use frame_system::pallet_prelude::BlockNumberFor; +use primitives::SessionIndex; +use sp_runtime::Perbill; +use sp_std::vec::Vec; + +use frame_support::traits::OnRuntimeUpgrade; + +use super::v8::V8HostConfiguration; +type V9HostConfiguration = configuration::HostConfiguration; + +mod v8 { + use super::*; + + #[frame_support::storage_alias] + pub(crate) type ActiveConfig = + StorageValue, V8HostConfiguration>, OptionQuery>; + + #[frame_support::storage_alias] + pub(crate) type PendingConfigs = StorageValue< + Pallet, + Vec<(SessionIndex, V8HostConfiguration>)>, + OptionQuery, + >; +} + +mod v9 { + use super::*; + + #[frame_support::storage_alias] + pub(crate) type ActiveConfig = + StorageValue, V9HostConfiguration>, OptionQuery>; + + #[frame_support::storage_alias] + pub(crate) type PendingConfigs = StorageValue< + Pallet, + Vec<(SessionIndex, V9HostConfiguration>)>, + OptionQuery, + >; +} + +pub struct MigrateToV9(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for MigrateToV9 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + log::trace!(target: crate::configuration::LOG_TARGET, "Running pre_upgrade() for HostConfiguration MigrateToV9"); + Ok(Vec::new()) + } + + fn on_runtime_upgrade() -> Weight { + log::info!(target: configuration::LOG_TARGET, "HostConfiguration MigrateToV9 started"); + if StorageVersion::get::>() == 8 { + let weight_consumed = migrate_to_v9::(); + + log::info!(target: configuration::LOG_TARGET, "HostConfiguration MigrateToV9 executed successfully"); + StorageVersion::new(9).put::>(); + + weight_consumed + } else { + log::warn!(target: configuration::LOG_TARGET, "HostConfiguration MigrateToV9 should be removed."); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + log::trace!(target: crate::configuration::LOG_TARGET, "Running post_upgrade() for HostConfiguration MigrateToV9"); + ensure!( + StorageVersion::get::>() >= 9, + "Storage version should be >= 9 after the migration" + ); + + Ok(()) + } +} + +fn migrate_to_v9() -> Weight { + // Unusual formatting is justified: + // - make it easier to verify that fields assign what they supposed to assign. + // - this code is transient and will be removed after all migrations are done. + // - this code is important enough to optimize for legibility sacrificing consistency. + #[rustfmt::skip] + let translate = + |pre: V8HostConfiguration>| -> + V9HostConfiguration> + { + V9HostConfiguration { +max_code_size : pre.max_code_size, +max_head_data_size : pre.max_head_data_size, +max_upward_queue_count : pre.max_upward_queue_count, +max_upward_queue_size : pre.max_upward_queue_size, +max_upward_message_size : pre.max_upward_message_size, +max_upward_message_num_per_candidate : pre.max_upward_message_num_per_candidate, +hrmp_max_message_num_per_candidate : pre.hrmp_max_message_num_per_candidate, +validation_upgrade_cooldown : pre.validation_upgrade_cooldown, +validation_upgrade_delay : pre.validation_upgrade_delay, +max_pov_size : pre.max_pov_size, +max_downward_message_size : pre.max_downward_message_size, +hrmp_sender_deposit : pre.hrmp_sender_deposit, +hrmp_recipient_deposit : pre.hrmp_recipient_deposit, +hrmp_channel_max_capacity : pre.hrmp_channel_max_capacity, +hrmp_channel_max_total_size : pre.hrmp_channel_max_total_size, +hrmp_max_parachain_inbound_channels : pre.hrmp_max_parachain_inbound_channels, +hrmp_max_parachain_outbound_channels : pre.hrmp_max_parachain_outbound_channels, +hrmp_channel_max_message_size : pre.hrmp_channel_max_message_size, +code_retention_period : pre.code_retention_period, +on_demand_cores : pre.on_demand_cores, +on_demand_retries : pre.on_demand_retries, +group_rotation_frequency : pre.group_rotation_frequency, +paras_availability_period : pre.paras_availability_period, +scheduling_lookahead : pre.scheduling_lookahead, +max_validators_per_core : pre.max_validators_per_core, +max_validators : pre.max_validators, +dispute_period : pre.dispute_period, +dispute_post_conclusion_acceptance_period: pre.dispute_post_conclusion_acceptance_period, +no_show_slots : pre.no_show_slots, +n_delay_tranches : pre.n_delay_tranches, +zeroth_delay_tranche_width : pre.zeroth_delay_tranche_width, +needed_approvals : pre.needed_approvals, +relay_vrf_modulo_samples : pre.relay_vrf_modulo_samples, +pvf_voting_ttl : pre.pvf_voting_ttl, +minimum_validation_upgrade_delay : pre.minimum_validation_upgrade_delay, +async_backing_params : pre.async_backing_params, +executor_params : pre.executor_params, +on_demand_queue_max_size : 10_000u32, +on_demand_base_fee : 10_000_000u128, +on_demand_fee_variability : Perbill::from_percent(3), +on_demand_target_queue_utilization : Perbill::from_percent(25), +on_demand_ttl : 5u32.into(), +minimum_backing_votes : 2 + } + }; + + let v8 = v8::ActiveConfig::::get() + .defensive_proof("Could not decode old config") + .unwrap_or_default(); + let v9 = translate(v8); + v9::ActiveConfig::::set(Some(v9)); + + // Allowed to be empty. + let pending_v8 = v8::PendingConfigs::::get().unwrap_or_default(); + let mut pending_v9 = Vec::new(); + + for (session, v8) in pending_v8.into_iter() { + let v9 = translate(v8); + pending_v9.push((session, v9)); + } + v9::PendingConfigs::::set(Some(pending_v9.clone())); + + let num_configs = (pending_v9.len() + 1) as u64; + T::DbWeight::get().reads_writes(num_configs, num_configs) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{new_test_ext, Test}; + + #[test] + fn v9_deserialized_from_actual_data() { + // Example how to get new `raw_config`: + // We'll obtain the raw_config at a specified a block + // Steps: + // 1. Go to Polkadot.js -> Developer -> Chain state -> Storage: https://polkadot.js.org/apps/#/chainstate + // 2. Set these parameters: + // 2.1. selected state query: configuration; activeConfig(): + // PolkadotRuntimeParachainsConfigurationHostConfiguration + // 2.2. blockhash to query at: + // 0xf89d3ab5312c5f70d396dc59612f0aa65806c798346f9db4b35278baed2e0e53 (the hash of + // the block) + // 2.3. Note the value of encoded storage key -> + // 0x06de3d8a54d27e44a9d5ce189618f22db4b49d95320d9021994c850f25b8e385 for the + // referenced block. + // 2.4. You'll also need the decoded values to update the test. + // 3. Go to Polkadot.js -> Developer -> Chain state -> Raw storage + // 3.1 Enter the encoded storage key and you get the raw config. + + // This exceeds the maximal line width length, but that's fine, since this is not code and + // doesn't need to be read and also leaving it as one line allows to easily copy it. + let raw_config = + hex_literal::hex![" + 0000300000800000080000000000100000c8000005000000050000000200000002000000000000000000000000005000000010000400000000000000000000000000000000000000000000000000000000000000000000000800000000200000040000000000100000b004000000000000000000001027000080b2e60e80c3c901809698000000000000000000000000000500000014000000040000000100000001010000000006000000640000000200000019000000000000000300000002000000020000000500000002000000" + ]; + + let v9 = + V9HostConfiguration::::decode(&mut &raw_config[..]).unwrap(); + + // We check only a sample of the values here. If we missed any fields or messed up data + // types that would skew all the fields coming after. + assert_eq!(v9.max_code_size, 3_145_728); + assert_eq!(v9.validation_upgrade_cooldown, 2); + assert_eq!(v9.max_pov_size, 5_242_880); + assert_eq!(v9.hrmp_channel_max_message_size, 1_048_576); + assert_eq!(v9.n_delay_tranches, 25); + assert_eq!(v9.minimum_validation_upgrade_delay, 5); + assert_eq!(v9.group_rotation_frequency, 20); + assert_eq!(v9.on_demand_cores, 0); + assert_eq!(v9.on_demand_base_fee, 10_000_000); + assert_eq!(v9.minimum_backing_votes, 2); + } + + #[test] + fn test_migrate_to_v9() { + // Host configuration has lots of fields. However, in this migration we only add one + // field. The most important part to check are a couple of the last fields. We also pick + // extra fields to check arbitrarily, e.g. depending on their position (i.e. the middle) and + // also their type. + // + // We specify only the picked fields and the rest should be provided by the `Default` + // implementation. That implementation is copied over between the two types and should work + // fine. + let v8 = V8HostConfiguration:: { + needed_approvals: 69, + paras_availability_period: 55, + hrmp_recipient_deposit: 1337, + max_pov_size: 1111, + minimum_validation_upgrade_delay: 20, + ..Default::default() + }; + + let mut pending_configs = Vec::new(); + pending_configs.push((100, v8.clone())); + pending_configs.push((300, v8.clone())); + + new_test_ext(Default::default()).execute_with(|| { + // Implant the v8 version in the state. + v8::ActiveConfig::::set(Some(v8)); + v8::PendingConfigs::::set(Some(pending_configs)); + + migrate_to_v9::(); + + let v9 = v9::ActiveConfig::::get().unwrap(); + let mut configs_to_check = v9::PendingConfigs::::get().unwrap(); + configs_to_check.push((0, v9.clone())); + + for (_, v8) in configs_to_check { + #[rustfmt::skip] + { + assert_eq!(v8.max_code_size , v9.max_code_size); + assert_eq!(v8.max_head_data_size , v9.max_head_data_size); + assert_eq!(v8.max_upward_queue_count , v9.max_upward_queue_count); + assert_eq!(v8.max_upward_queue_size , v9.max_upward_queue_size); + assert_eq!(v8.max_upward_message_size , v9.max_upward_message_size); + assert_eq!(v8.max_upward_message_num_per_candidate , v9.max_upward_message_num_per_candidate); + assert_eq!(v8.hrmp_max_message_num_per_candidate , v9.hrmp_max_message_num_per_candidate); + assert_eq!(v8.validation_upgrade_cooldown , v9.validation_upgrade_cooldown); + assert_eq!(v8.validation_upgrade_delay , v9.validation_upgrade_delay); + assert_eq!(v8.max_pov_size , v9.max_pov_size); + assert_eq!(v8.max_downward_message_size , v9.max_downward_message_size); + assert_eq!(v8.hrmp_max_parachain_outbound_channels , v9.hrmp_max_parachain_outbound_channels); + assert_eq!(v8.hrmp_sender_deposit , v9.hrmp_sender_deposit); + assert_eq!(v8.hrmp_recipient_deposit , v9.hrmp_recipient_deposit); + assert_eq!(v8.hrmp_channel_max_capacity , v9.hrmp_channel_max_capacity); + assert_eq!(v8.hrmp_channel_max_total_size , v9.hrmp_channel_max_total_size); + assert_eq!(v8.hrmp_max_parachain_inbound_channels , v9.hrmp_max_parachain_inbound_channels); + assert_eq!(v8.hrmp_channel_max_message_size , v9.hrmp_channel_max_message_size); + assert_eq!(v8.code_retention_period , v9.code_retention_period); + assert_eq!(v8.on_demand_cores , v9.on_demand_cores); + assert_eq!(v8.on_demand_retries , v9.on_demand_retries); + assert_eq!(v8.group_rotation_frequency , v9.group_rotation_frequency); + assert_eq!(v8.paras_availability_period , v9.paras_availability_period); + assert_eq!(v8.scheduling_lookahead , v9.scheduling_lookahead); + assert_eq!(v8.max_validators_per_core , v9.max_validators_per_core); + assert_eq!(v8.max_validators , v9.max_validators); + assert_eq!(v8.dispute_period , v9.dispute_period); + assert_eq!(v8.no_show_slots , v9.no_show_slots); + assert_eq!(v8.n_delay_tranches , v9.n_delay_tranches); + assert_eq!(v8.zeroth_delay_tranche_width , v9.zeroth_delay_tranche_width); + assert_eq!(v8.needed_approvals , v9.needed_approvals); + assert_eq!(v8.relay_vrf_modulo_samples , v9.relay_vrf_modulo_samples); + assert_eq!(v8.pvf_voting_ttl , v9.pvf_voting_ttl); + assert_eq!(v8.minimum_validation_upgrade_delay , v9.minimum_validation_upgrade_delay); + assert_eq!(v8.async_backing_params.allowed_ancestry_len, v9.async_backing_params.allowed_ancestry_len); + assert_eq!(v8.async_backing_params.max_candidate_depth , v9.async_backing_params.max_candidate_depth); + assert_eq!(v8.executor_params , v9.executor_params); + assert_eq!(v8.minimum_backing_votes , v9.minimum_backing_votes); + }; // ; makes this a statement. `rustfmt::skip` cannot be put on an expression. + } + }); + } + + // Test that migration doesn't panic in case there're no pending configurations upgrades in + // pallet's storage. + #[test] + fn test_migrate_to_v9_no_pending() { + let v8 = V8HostConfiguration::::default(); + + new_test_ext(Default::default()).execute_with(|| { + // Implant the v8 version in the state. + v8::ActiveConfig::::set(Some(v8)); + // Ensure there're no pending configs. + v8::PendingConfigs::::set(None); + + // Shouldn't fail. + migrate_to_v9::(); + }); + } +} diff --git a/runtime/parachains/src/configuration/tests.rs b/runtime/parachains/src/configuration/tests.rs index 83de7db932b4..690bb43153f8 100644 --- a/runtime/parachains/src/configuration/tests.rs +++ b/runtime/parachains/src/configuration/tests.rs @@ -317,6 +317,7 @@ fn setting_pending_config_members() { on_demand_fee_variability: Perbill::from_percent(3), on_demand_target_queue_utilization: Perbill::from_percent(25), on_demand_ttl: 5u32, + minimum_backing_votes: 5, }; Configuration::set_validation_upgrade_cooldown( @@ -467,6 +468,11 @@ fn setting_pending_config_members() { .unwrap(); Configuration::set_pvf_voting_ttl(RuntimeOrigin::root(), new_config.pvf_voting_ttl) .unwrap(); + Configuration::set_minimum_backing_votes( + RuntimeOrigin::root(), + new_config.minimum_backing_votes, + ) + .unwrap(); assert_eq!(PendingConfigs::::get(), vec![(shared::SESSION_DELAY, new_config)],); }) diff --git a/runtime/parachains/src/inclusion/tests.rs b/runtime/parachains/src/inclusion/tests.rs index 7c22ac36a802..8df9befd5fa7 100644 --- a/runtime/parachains/src/inclusion/tests.rs +++ b/runtime/parachains/src/inclusion/tests.rs @@ -85,6 +85,10 @@ fn default_allowed_relay_parent_tracker() -> AllowedRelayParentsTracker usize { + std::cmp::min(group_len, configuration::Pallet::::config().minimum_backing_votes as usize) +} + #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) enum BackingKind { #[allow(unused)] @@ -124,7 +128,7 @@ pub(crate) fn back_candidate( let signing = match kind { BackingKind::Unanimous => group.len(), - BackingKind::Threshold => threshold, + BackingKind::Threshold => threshold as usize, BackingKind::Lacking => threshold.saturating_sub(1), }; diff --git a/runtime/parachains/src/paras_inherent/tests.rs b/runtime/parachains/src/paras_inherent/tests.rs index 1e5271909664..ab515cb37565 100644 --- a/runtime/parachains/src/paras_inherent/tests.rs +++ b/runtime/parachains/src/paras_inherent/tests.rs @@ -948,8 +948,11 @@ fn default_header() -> primitives::Header { mod sanitizers { use super::*; - use crate::inclusion::tests::{ - back_candidate, collator_sign_candidate, BackingKind, TestCandidateBuilder, + use crate::{ + inclusion::tests::{ + back_candidate, collator_sign_candidate, BackingKind, TestCandidateBuilder, + }, + mock::{new_test_ext, MockGenesisConfig}, }; use bitvec::order::Lsb0; use primitives::{ @@ -1207,131 +1210,133 @@ mod sanitizers { #[test] fn candidates() { - const RELAY_PARENT_NUM: u32 = 3; - - let header = default_header(); - let relay_parent = header.hash(); - let session_index = SessionIndex::from(0_u32); - - let keystore = LocalKeystore::in_memory(); - let keystore = Arc::new(keystore) as KeystorePtr; - let signing_context = SigningContext { parent_hash: relay_parent, session_index }; - - let validators = vec![ - keyring::Sr25519Keyring::Alice, - keyring::Sr25519Keyring::Bob, - keyring::Sr25519Keyring::Charlie, - keyring::Sr25519Keyring::Dave, - ]; - for validator in validators.iter() { - Keystore::sr25519_generate_new( - &*keystore, - PARACHAIN_KEY_TYPE_ID, - Some(&validator.to_seed()), - ) - .unwrap(); - } - - let has_concluded_invalid = - |_idx: usize, _backed_candidate: &BackedCandidate| -> bool { false }; - - let entry_ttl = 10_000; - let scheduled = (0_usize..2) - .into_iter() - .map(|idx| { - let core_idx = CoreIndex::from(idx as u32); - let ca = CoreAssignment { - paras_entry: ParasEntry::new( - Assignment::new(ParaId::from(1_u32 + idx as u32)), - entry_ttl, - ), - core: core_idx, - }; - ca - }) - .collect::>(); - - let group_validators = |group_index: GroupIndex| { - match group_index { - group_index if group_index == GroupIndex::from(0) => Some(vec![0, 1]), - group_index if group_index == GroupIndex::from(1) => Some(vec![2, 3]), - _ => panic!("Group index out of bounds for 2 parachains and 1 parathread core"), + new_test_ext(MockGenesisConfig::default()).execute_with(|| { + const RELAY_PARENT_NUM: u32 = 3; + + let header = default_header(); + let relay_parent = header.hash(); + let session_index = SessionIndex::from(0_u32); + + let keystore = LocalKeystore::in_memory(); + let keystore = Arc::new(keystore) as KeystorePtr; + let signing_context = SigningContext { parent_hash: relay_parent, session_index }; + + let validators = vec![ + keyring::Sr25519Keyring::Alice, + keyring::Sr25519Keyring::Bob, + keyring::Sr25519Keyring::Charlie, + keyring::Sr25519Keyring::Dave, + ]; + for validator in validators.iter() { + Keystore::sr25519_generate_new( + &*keystore, + PARACHAIN_KEY_TYPE_ID, + Some(&validator.to_seed()), + ) + .unwrap(); } - .map(|m| m.into_iter().map(ValidatorIndex).collect::>()) - }; - - let backed_candidates = (0_usize..2) - .into_iter() - .map(|idx0| { - let idx1 = idx0 + 1; - let mut candidate = TestCandidateBuilder { - para_id: ParaId::from(idx1), - relay_parent, - pov_hash: Hash::repeat_byte(idx1 as u8), - persisted_validation_data_hash: [42u8; 32].into(), - hrmp_watermark: RELAY_PARENT_NUM, - ..Default::default() - } - .build(); - - collator_sign_candidate(Sr25519Keyring::One, &mut candidate); - let backed = back_candidate( - candidate, - &validators, - group_validators(GroupIndex::from(idx0 as u32)).unwrap().as_ref(), - &keystore, - &signing_context, - BackingKind::Threshold, - ); - backed - }) - .collect::>(); - - // happy path - assert_eq!( - sanitize_backed_candidates::( - backed_candidates.clone(), - has_concluded_invalid, - &scheduled - ), - backed_candidates - ); - - // nothing is scheduled, so no paraids match, thus all backed candidates are skipped - { - let scheduled = &Vec::new(); - assert!(sanitize_backed_candidates::( - backed_candidates.clone(), - has_concluded_invalid, - &scheduled - ) - .is_empty()); - } + let has_concluded_invalid = + |_idx: usize, _backed_candidate: &BackedCandidate| -> bool { false }; - // candidates that have concluded as invalid are filtered out - { - // mark every second one as concluded invalid - let set = { - let mut set = std::collections::HashSet::new(); - for (idx, backed_candidate) in backed_candidates.iter().enumerate() { - if idx & 0x01 == 0 { - set.insert(backed_candidate.hash()); - } + let entry_ttl = 10_000; + let scheduled = (0_usize..2) + .into_iter() + .map(|idx| { + let core_idx = CoreIndex::from(idx as u32); + let ca = CoreAssignment { + paras_entry: ParasEntry::new( + Assignment::new(ParaId::from(1_u32 + idx as u32)), + entry_ttl, + ), + core: core_idx, + }; + ca + }) + .collect::>(); + + let group_validators = |group_index: GroupIndex| { + match group_index { + group_index if group_index == GroupIndex::from(0) => Some(vec![0, 1]), + group_index if group_index == GroupIndex::from(1) => Some(vec![2, 3]), + _ => panic!("Group index out of bounds for 2 parachains and 1 parathread core"), } - set + .map(|m| m.into_iter().map(ValidatorIndex).collect::>()) }; - let has_concluded_invalid = - |_idx: usize, candidate: &BackedCandidate| set.contains(&candidate.hash()); + + let backed_candidates = (0_usize..2) + .into_iter() + .map(|idx0| { + let idx1 = idx0 + 1; + let mut candidate = TestCandidateBuilder { + para_id: ParaId::from(idx1), + relay_parent, + pov_hash: Hash::repeat_byte(idx1 as u8), + persisted_validation_data_hash: [42u8; 32].into(), + hrmp_watermark: RELAY_PARENT_NUM, + ..Default::default() + } + .build(); + + collator_sign_candidate(Sr25519Keyring::One, &mut candidate); + + let backed = back_candidate( + candidate, + &validators, + group_validators(GroupIndex::from(idx0 as u32)).unwrap().as_ref(), + &keystore, + &signing_context, + BackingKind::Threshold, + ); + backed + }) + .collect::>(); + + // happy path assert_eq!( sanitize_backed_candidates::( backed_candidates.clone(), has_concluded_invalid, &scheduled - ) - .len(), - backed_candidates.len() / 2 + ), + backed_candidates ); - } + + // nothing is scheduled, so no paraids match, thus all backed candidates are skipped + { + let scheduled = &Vec::new(); + assert!(sanitize_backed_candidates::( + backed_candidates.clone(), + has_concluded_invalid, + &scheduled + ) + .is_empty()); + } + + // candidates that have concluded as invalid are filtered out + { + // mark every second one as concluded invalid + let set = { + let mut set = std::collections::HashSet::new(); + for (idx, backed_candidate) in backed_candidates.iter().enumerate() { + if idx & 0x01 == 0 { + set.insert(backed_candidate.hash()); + } + } + set + }; + let has_concluded_invalid = + |_idx: usize, candidate: &BackedCandidate| set.contains(&candidate.hash()); + assert_eq!( + sanitize_backed_candidates::( + backed_candidates.clone(), + has_concluded_invalid, + &scheduled + ) + .len(), + backed_candidates.len() / 2 + ); + } + }); } } diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 88b1d593aaab..cb53b33ba959 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1525,6 +1525,8 @@ pub mod migrations { frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, + + parachains_configuration::migration::v9::MigrateToV9, ); } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 997f65454f04..cb799325471c 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -1553,6 +1553,7 @@ pub mod migrations { assigned_slots::migration::v1::VersionCheckedMigrateToV1, parachains_scheduler::migration::v1::MigrateToV1, parachains_configuration::migration::v8::MigrateToV8, + parachains_configuration::migration::v9::MigrateToV9, ); } diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 67594220a13a..4a217da8695b 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1291,6 +1291,7 @@ pub mod migrations { assigned_slots::migration::v1::VersionCheckedMigrateToV1, parachains_scheduler::migration::v1::MigrateToV1, parachains_configuration::migration::v8::MigrateToV8, + parachains_configuration::migration::v9::MigrateToV9, ); } From 09c9f4e4040e4ba7a04b1a9a45d624fee97b6ad7 Mon Sep 17 00:00:00 2001 From: alindima Date: Tue, 22 Aug 2023 16:18:24 +0300 Subject: [PATCH 3/5] introduce api versioning for min_backing votes also enable it for rococo/versi for testing --- node/core/backing/src/lib.rs | 20 +++++++++++++++---- node/core/backing/src/tests/mod.rs | 10 ++++++++++ .../src/tests/prospective_parachains.rs | 10 ++++++++++ node/core/runtime-api/src/lib.rs | 9 ++++++--- node/primitives/src/lib.rs | 2 ++ node/service/src/fake_runtime_api.rs | 4 ---- node/subsystem-types/src/messages.rs | 3 +++ primitives/src/runtime_api.rs | 5 ++++- runtime/kusama/src/lib.rs | 4 ---- runtime/parachains/src/runtime_api_impl/v5.rs | 6 ------ .../src/runtime_api_impl/vstaging.rs | 6 ++++++ runtime/polkadot/src/lib.rs | 4 ---- runtime/rococo/src/lib.rs | 13 +++++++----- runtime/test-runtime/src/lib.rs | 4 ---- runtime/westend/src/lib.rs | 4 ---- 15 files changed, 65 insertions(+), 39 deletions(-) diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 9af0d6046b96..fee4d0df2567 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -115,7 +115,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; -use util::runtime::RuntimeInfo; +use util::{request_runtime_api_version, runtime::RuntimeInfo}; mod error; @@ -127,6 +127,9 @@ mod tests; const LOG_TARGET: &str = "parachain::candidate-backing"; +/// Used prior to runtime API version 6. +const LEGACY_MIN_BACKING_VOTES: u32 = 2; + /// PoV data to validate. enum PoVData { /// Already available (from candidate selection). @@ -998,9 +1001,19 @@ async fn construct_per_relay_parent_state( let session_index = try_runtime_api!(runtime_info.get_session_index_for_child(ctx.sender(), parent).await); + let runtime_api_version = try_runtime_api!(request_runtime_api_version(parent, ctx.sender()) + .await + .await + .map_err(Error::RuntimeApiUnavailable)?); + let minimum_backing_votes = - runtime_info.get_min_backing_votes(ctx.sender(), session_index, parent).await; - // TODO: if this does not exist, fall back to the hardcoded 2 value. + if runtime_api_version >= RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT { + try_runtime_api!( + runtime_info.get_min_backing_votes(ctx.sender(), session_index, parent).await + ) + } else { + LEGACY_MIN_BACKING_VOTES + }; let (validators, groups, cores) = futures::try_join!( request_validators(parent, ctx.sender()).await, @@ -1015,7 +1028,6 @@ async fn construct_per_relay_parent_state( let validators: Vec<_> = try_runtime_api!(validators); let (validator_groups, group_rotation_info) = try_runtime_api!(groups); let cores = try_runtime_api!(cores); - let minimum_backing_votes = try_runtime_api!(minimum_backing_votes); let signing_context = SigningContext { parent_hash: parent, session_index }; let validator = diff --git a/node/core/backing/src/tests/mod.rs b/node/core/backing/src/tests/mod.rs index 5bb8326d409c..e23eb51983d5 100644 --- a/node/core/backing/src/tests/mod.rs +++ b/node/core/backing/src/tests/mod.rs @@ -262,6 +262,16 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS } ); + // Check that subsystem job issues a request for the runtime API version. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) + ) if parent == test_state.relay_parent => { + tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); + } + ); + // Check if subsystem job issues a request for the minimum backing votes. // This may or may not happen, depending if the minimum backing votes is already cached in the // RuntimeInfo. diff --git a/node/core/backing/src/tests/prospective_parachains.rs b/node/core/backing/src/tests/prospective_parachains.rs index 93a8e94b98de..a08c4b1fb070 100644 --- a/node/core/backing/src/tests/prospective_parachains.rs +++ b/node/core/backing/src/tests/prospective_parachains.rs @@ -153,6 +153,16 @@ async fn activate_leaf( } ); + // Check that subsystem job issues a request for the runtime API version. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) + ) if parent == hash => { + tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); + } + ); + // Check if subsystem job issues a request for the minimum backing votes. // This may or may not happen, depending if the minimum backing votes is already cached in // the `RuntimeInfo`. diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index 37e63ff673cd..c33e057d4362 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -455,9 +455,6 @@ where Request::Authorities(sender) => query!(Authorities, authorities(), ver = 1, sender), Request::Validators(sender) => query!(Validators, validators(), ver = 1, sender), - Request::MinimumBackingVotes(sender) => - query!(MinimumBackingVotes, minimum_backing_votes(), ver = 1, sender), - Request::ValidatorGroups(sender) => { query!(ValidatorGroups, validator_groups(), ver = 1, sender) }, @@ -559,6 +556,12 @@ where ver = Request::SUBMIT_REPORT_DISPUTE_LOST_RUNTIME_REQUIREMENT, sender ), + Request::MinimumBackingVotes(sender) => query!( + MinimumBackingVotes, + minimum_backing_votes(), + ver = Request::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT, + sender + ), Request::StagingParaBackingState(para, sender) => { query!( diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 392781783319..f44179096ab4 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -659,6 +659,8 @@ pub fn maybe_compress_pov(pov: PoV) -> PoV { /// How many votes we need to consider a candidate backed. /// /// WARNING: This has to be kept in sync with the runtime check in the inclusion module. +/// The min threshold has been added to the HostConfiguration and after the runtime API for querying +/// it, `minimum_backing_votes`, is moved from vstaging to production, we can remove this. pub fn minimum_votes(n_validators: usize) -> usize { std::cmp::min(2, n_validators) } diff --git a/node/service/src/fake_runtime_api.rs b/node/service/src/fake_runtime_api.rs index f8eec2143352..d9553afa024b 100644 --- a/node/service/src/fake_runtime_api.rs +++ b/node/service/src/fake_runtime_api.rs @@ -121,10 +121,6 @@ sp_api::impl_runtime_apis! { unimplemented!() } - fn minimum_backing_votes() -> u32 { - unimplemented!() - } - fn validator_groups() -> (Vec>, GroupRotationInfo) { unimplemented!() } diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index aafec7c8bbee..13759a12d395 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -721,6 +721,9 @@ impl RuntimeApiRequest { /// `SubmitReportDisputeLost` pub const SUBMIT_REPORT_DISPUTE_LOST_RUNTIME_REQUIREMENT: u32 = 5; + /// `MinimumBackingVotes` + pub const MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT: u32 = 6; + /// Minimum version for backing state, required for async backing. /// /// 99 for now, should be adjusted to VSTAGING/actual runtime version once released. diff --git a/primitives/src/runtime_api.rs b/primitives/src/runtime_api.rs index d7eadc43ed69..86aca6c9bc6f 100644 --- a/primitives/src/runtime_api.rs +++ b/primitives/src/runtime_api.rs @@ -240,8 +240,11 @@ sp_api::decl_runtime_apis! { key_ownership_proof: vstaging::slashing::OpaqueKeyOwnershipProof, ) -> Option<()>; + /***** Staging *****/ + /// Get the minimum number of backing votes for a parachain candidate. - /// TODO: bump api version here? + /// This is a staging method! Do not use on production runtimes! + #[api_version(6)] fn minimum_backing_votes() -> u32; /***** Asynchronous backing *****/ diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 9555eb176d07..448f018384d8 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1894,10 +1894,6 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } - fn minimum_backing_votes() -> u32 { - parachains_runtime_api_impl::minimum_backing_votes::() - } - fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } diff --git a/runtime/parachains/src/runtime_api_impl/v5.rs b/runtime/parachains/src/runtime_api_impl/v5.rs index 1f88833e2749..cd1579689733 100644 --- a/runtime/parachains/src/runtime_api_impl/v5.rs +++ b/runtime/parachains/src/runtime_api_impl/v5.rs @@ -38,12 +38,6 @@ pub fn validators() -> Vec { >::active_validator_keys() } -/// Implementation of the `minimum_backing_votes` function of the runtime API. -pub fn minimum_backing_votes() -> u32 { - let config = >::config(); - config.minimum_backing_votes -} - /// Implementation for the `validator_groups` function of the runtime API. pub fn validator_groups( ) -> (Vec>, GroupRotationInfo>) { diff --git a/runtime/parachains/src/runtime_api_impl/vstaging.rs b/runtime/parachains/src/runtime_api_impl/vstaging.rs index 5406428377d0..7446e769d845 100644 --- a/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -118,3 +118,9 @@ pub fn backing_state( pub fn async_backing_params() -> AsyncBackingParams { >::config().async_backing_params } + +/// Implementation of the `minimum_backing_votes` function of the runtime API. +pub fn minimum_backing_votes() -> u32 { + let config = >::config(); + config.minimum_backing_votes +} diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 5b9d374ae3b7..ea7639ee3a48 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1697,10 +1697,6 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } - fn minimum_backing_votes() -> u32 { - parachains_runtime_api_impl::minimum_backing_votes::() - } - fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index ec45fb1a289e..f02936d16b07 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -46,7 +46,9 @@ use runtime_parachains::{ inclusion::{AggregateMessageOrigin, UmpQueueId}, initializer as parachains_initializer, origin as parachains_origin, paras as parachains_paras, paras_inherent as parachains_paras_inherent, - runtime_api_impl::v5 as parachains_runtime_api_impl, + runtime_api_impl::{ + v5 as parachains_runtime_api_impl, vstaging as parachains_staging_runtime_api_impl, + }, scheduler as parachains_scheduler, session_info as parachains_session_info, shared as parachains_shared, }; @@ -1715,15 +1717,12 @@ sp_api::impl_runtime_apis! { } } + #[api_version(6)] impl primitives::runtime_api::ParachainHost for Runtime { fn validators() -> Vec { parachains_runtime_api_impl::validators::() } - fn minimum_backing_votes() -> u32 { - parachains_runtime_api_impl::minimum_backing_votes::() - } - fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } @@ -1849,6 +1848,10 @@ sp_api::impl_runtime_apis! { key_ownership_proof, ) } + + fn minimum_backing_votes() -> u32 { + parachains_staging_runtime_api_impl::minimum_backing_votes::() + } } #[api_version(3)] diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 154ee6001f63..b2397299430d 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -826,10 +826,6 @@ sp_api::impl_runtime_apis! { runtime_impl::validators::() } - fn minimum_backing_votes() -> u32 { - runtime_impl::minimum_backing_votes::() - } - fn validator_groups() -> (Vec>, GroupRotationInfo) { runtime_impl::validator_groups::() } diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 66c5a9d08044..232045ab9f78 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1567,10 +1567,6 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::validators::() } - fn minimum_backing_votes() -> u32 { - parachains_runtime_api_impl::minimum_backing_votes::() - } - fn validator_groups() -> (Vec>, GroupRotationInfo) { parachains_runtime_api_impl::validator_groups::() } From fe51ef80eac284ee7ae7b1ce3b4cfd0166eddd97 Mon Sep 17 00:00:00 2001 From: alindima Date: Wed, 23 Aug 2023 11:25:24 +0300 Subject: [PATCH 4/5] also add min_backing_votes runtime calls to statement-distribution this dependency has been recently introduced by async backing --- node/core/backing/src/lib.rs | 25 ++++----------- .../network/statement-distribution/Cargo.toml | 2 +- .../src/vstaging/grid.rs | 2 +- .../src/vstaging/groups.rs | 12 ++++--- .../src/vstaging/mod.rs | 32 ++++++++++++++----- .../src/vstaging/tests/mod.rs | 26 ++++++++++++--- node/primitives/src/lib.rs | 9 ------ node/subsystem-types/src/runtime_client.rs | 15 +++++---- node/subsystem-util/src/lib.rs | 32 ++++++++++++++++++- node/subsystem-util/src/runtime/error.rs | 2 +- node/subsystem-util/src/runtime/mod.rs | 6 ++-- 11 files changed, 106 insertions(+), 57 deletions(-) diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index fee4d0df2567..96f3d4c4fde2 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -115,7 +115,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; -use util::{request_runtime_api_version, runtime::RuntimeInfo}; +use util::{request_min_backing_votes, runtime::RuntimeInfo}; mod error; @@ -127,9 +127,6 @@ mod tests; const LOG_TARGET: &str = "parachain::candidate-backing"; -/// Used prior to runtime API version 6. -const LEGACY_MIN_BACKING_VOTES: u32 = 2; - /// PoV data to validate. enum PoVData { /// Already available (from candidate selection). @@ -280,7 +277,7 @@ struct State { background_validation_tx: mpsc::Sender<(Hash, ValidatedCandidateCommand)>, /// The handle to the keystore used for signing. keystore: KeystorePtr, - /// The minimum backing votes threshold. + /// Runtime info cached per-session. runtime_info: RuntimeInfo, } @@ -1001,19 +998,11 @@ async fn construct_per_relay_parent_state( let session_index = try_runtime_api!(runtime_info.get_session_index_for_child(ctx.sender(), parent).await); - let runtime_api_version = try_runtime_api!(request_runtime_api_version(parent, ctx.sender()) - .await - .await - .map_err(Error::RuntimeApiUnavailable)?); - - let minimum_backing_votes = - if runtime_api_version >= RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT { - try_runtime_api!( - runtime_info.get_min_backing_votes(ctx.sender(), session_index, parent).await - ) - } else { - LEGACY_MIN_BACKING_VOTES - }; + + let minimum_backing_votes = request_min_backing_votes(parent, ctx.sender(), |sender| { + runtime_info.get_min_backing_votes(sender, session_index, parent) + }) + .await?; let (validators, groups, cores) = futures::try_join!( request_validators(parent, ctx.sender()).await, diff --git a/node/network/statement-distribution/Cargo.toml b/node/network/statement-distribution/Cargo.toml index b9d52bda5706..d1622002dfc1 100644 --- a/node/network/statement-distribution/Cargo.toml +++ b/node/network/statement-distribution/Cargo.toml @@ -16,6 +16,7 @@ sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "maste polkadot-node-subsystem = {path = "../../subsystem" } polkadot-node-primitives = { path = "../../primitives" } polkadot-node-subsystem-util = { path = "../../subsystem-util" } +polkadot-node-subsystem-types = { path = "../../subsystem-types" } polkadot-node-network-protocol = { path = "../../network/protocol" } arrayvec = "0.7.4" indexmap = "1.9.1" @@ -39,4 +40,3 @@ sc-network = { git = "https://github.com/paritytech/substrate", branch = "master futures-timer = "3.0.2" polkadot-primitives-test-helpers = { path = "../../../primitives/test-helpers" } rand_chacha = "0.3" -polkadot-node-subsystem-types = { path = "../../subsystem-types" } diff --git a/node/network/statement-distribution/src/vstaging/grid.rs b/node/network/statement-distribution/src/vstaging/grid.rs index c6c73f8bae52..4be7bb00296d 100644 --- a/node/network/statement-distribution/src/vstaging/grid.rs +++ b/node/network/statement-distribution/src/vstaging/grid.rs @@ -1074,7 +1074,7 @@ mod tests { fn dummy_groups(group_size: usize) -> Groups { let groups = vec![(0..(group_size as u32)).map(ValidatorIndex).collect()].into(); - Groups::new(groups) + Groups::new(groups, 2) } #[test] diff --git a/node/network/statement-distribution/src/vstaging/groups.rs b/node/network/statement-distribution/src/vstaging/groups.rs index 86321b30f220..5ef6beb13060 100644 --- a/node/network/statement-distribution/src/vstaging/groups.rs +++ b/node/network/statement-distribution/src/vstaging/groups.rs @@ -16,7 +16,6 @@ //! A utility for tracking groups and their members within a session. -use polkadot_node_primitives::minimum_votes; use polkadot_primitives::vstaging::{GroupIndex, IndexedVec, ValidatorIndex}; use std::collections::HashMap; @@ -27,12 +26,16 @@ use std::collections::HashMap; pub struct Groups { groups: IndexedVec>, by_validator_index: HashMap, + backing_threshold: u32, } impl Groups { /// Create a new [`Groups`] tracker with the groups and discovery keys /// from the session. - pub fn new(groups: IndexedVec>) -> Self { + pub fn new( + groups: IndexedVec>, + backing_threshold: u32, + ) -> Self { let mut by_validator_index = HashMap::new(); for (i, group) in groups.iter().enumerate() { @@ -42,7 +45,7 @@ impl Groups { } } - Groups { groups, by_validator_index } + Groups { groups, by_validator_index, backing_threshold } } /// Access all the underlying groups. @@ -60,7 +63,8 @@ impl Groups { &self, group_index: GroupIndex, ) -> Option<(usize, usize)> { - self.get(group_index).map(|g| (g.len(), minimum_votes(g.len()))) + self.get(group_index) + .map(|g| (g.len(), std::cmp::min(g.len(), self.backing_threshold as usize))) } /// Get the group index for a validator by index. diff --git a/node/network/statement-distribution/src/vstaging/mod.rs b/node/network/statement-distribution/src/vstaging/mod.rs index 03af4ce81598..2767534f8b14 100644 --- a/node/network/statement-distribution/src/vstaging/mod.rs +++ b/node/network/statement-distribution/src/vstaging/mod.rs @@ -40,9 +40,12 @@ use polkadot_node_subsystem::{ }, overseer, ActivatedLeaf, }; +use polkadot_node_subsystem_types::messages::RuntimeApiRequest; use polkadot_node_subsystem_util::{ - backing_implicit_view::View as ImplicitView, reputation::ReputationAggregator, - runtime::ProspectiveParachainsMode, + backing_implicit_view::View as ImplicitView, + reputation::ReputationAggregator, + request_from_runtime, request_min_backing_votes, + runtime::{recv_runtime, ProspectiveParachainsMode}, }; use polkadot_primitives::vstaging::{ AuthorityDiscoveryId, CandidateHash, CompactStatement, CoreIndex, CoreState, GroupIndex, @@ -163,8 +166,8 @@ struct PerSessionState { } impl PerSessionState { - fn new(session_info: SessionInfo, keystore: &KeystorePtr) -> Self { - let groups = Groups::new(session_info.validator_groups.clone()); + fn new(session_info: SessionInfo, keystore: &KeystorePtr, backing_threshold: u32) -> Self { + let groups = Groups::new(session_info.validator_groups.clone(), backing_threshold); let mut authority_lookup = HashMap::new(); for (i, ad) in session_info.discovery_keys.iter().cloned().enumerate() { authority_lookup.insert(ad, ValidatorIndex(i as _)); @@ -504,9 +507,22 @@ pub(crate) async fn handle_active_leaves_update( Some(s) => s, }; - state - .per_session - .insert(session_index, PerSessionState::new(session_info, &state.keystore)); + let minimum_backing_votes = + request_min_backing_votes(new_relay_parent, ctx.sender(), |sender| async { + recv_runtime( + request_from_runtime(new_relay_parent, sender, |tx| { + RuntimeApiRequest::MinimumBackingVotes(tx) + }) + .await, + ) + .await + }) + .await?; + + state.per_session.insert( + session_index, + PerSessionState::new(session_info, &state.keystore, minimum_backing_votes), + ); } let per_session = state @@ -2502,7 +2518,7 @@ pub(crate) async fn dispatch_requests(ctx: &mut Context, state: &mut St Some(RequestProperties { unwanted_mask, backing_threshold: if require_backing { - Some(polkadot_node_primitives::minimum_votes(group.len())) + Some(per_session.groups.get_size_and_backing_threshold(group_index)?.1) } else { None }, diff --git a/node/network/statement-distribution/src/vstaging/tests/mod.rs b/node/network/statement-distribution/src/vstaging/tests/mod.rs index f5f4c8443257..fa73f4a9a2b0 100644 --- a/node/network/statement-distribution/src/vstaging/tests/mod.rs +++ b/node/network/statement-distribution/src/vstaging/tests/mod.rs @@ -356,7 +356,7 @@ async fn activate_leaf( virtual_overseer: &mut VirtualOverseer, leaf: &TestLeaf, test_state: &TestState, - expect_session_info_request: bool, + is_new_session: bool, ) { let activated = ActivatedLeaf { hash: leaf.hash, @@ -371,14 +371,14 @@ async fn activate_leaf( )))) .await; - handle_leaf_activation(virtual_overseer, leaf, test_state, expect_session_info_request).await; + handle_leaf_activation(virtual_overseer, leaf, test_state, is_new_session).await; } async fn handle_leaf_activation( virtual_overseer: &mut VirtualOverseer, leaf: &TestLeaf, test_state: &TestState, - expect_session_info_request: bool, + is_new_session: bool, ) { let TestLeaf { number, hash, parent_hash, para_data, session, availability_cores } = leaf; @@ -447,7 +447,7 @@ async fn handle_leaf_activation( } ); - if expect_session_info_request { + if is_new_session { assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi( @@ -455,6 +455,24 @@ async fn handle_leaf_activation( tx.send(Ok(Some(test_state.session_info.clone()))).unwrap(); } ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx))) if parent == *hash => { + tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); + } + ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + parent, + RuntimeApiRequest::MinimumBackingVotes(tx), + )) if parent == *hash => { + tx.send(Ok(2)).unwrap(); + } + ); } } diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index f44179096ab4..fc0002aaefbc 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -655,12 +655,3 @@ pub fn maybe_compress_pov(pov: PoV) -> PoV { let pov = PoV { block_data: BlockData(raw) }; pov } - -/// How many votes we need to consider a candidate backed. -/// -/// WARNING: This has to be kept in sync with the runtime check in the inclusion module. -/// The min threshold has been added to the HostConfiguration and after the runtime API for querying -/// it, `minimum_backing_votes`, is moved from vstaging to production, we can remove this. -pub fn minimum_votes(n_validators: usize) -> usize { - std::cmp::min(2, n_validators) -} diff --git a/node/subsystem-types/src/runtime_client.rs b/node/subsystem-types/src/runtime_client.rs index 72ea2113e4bc..c6f46e806f04 100644 --- a/node/subsystem-types/src/runtime_client.rs +++ b/node/subsystem-types/src/runtime_client.rs @@ -40,9 +40,6 @@ pub trait RuntimeApiSubsystemClient { /// Get the current validators. async fn validators(&self, at: Hash) -> Result, ApiError>; - /// Get the minimum number of backing votes. - async fn minimum_backing_votes(&self, at: Hash) -> Result; - /// Returns the validator groups and rotation info localized based on the hypothetical child /// of a block whose state this is invoked on. Note that `now` in the `GroupRotationInfo` /// should be the successor of the number of the block. @@ -235,6 +232,10 @@ pub trait RuntimeApiSubsystemClient { session_index: SessionIndex, ) -> Result, ApiError>; + // === STAGING v6 === + /// Get the minimum number of backing votes. + async fn minimum_backing_votes(&self, at: Hash) -> Result; + // === Asynchronous backing API === /// Returns candidate's acceptance limitations for asynchronous backing for a relay parent. @@ -278,10 +279,6 @@ where self.client.runtime_api().validators(at) } - async fn minimum_backing_votes(&self, at: Hash) -> Result { - self.client.runtime_api().minimum_backing_votes(at) - } - async fn validator_groups( &self, at: Hash, @@ -480,6 +477,10 @@ where runtime_api.submit_report_dispute_lost(at, dispute_proof, key_ownership_proof) } + async fn minimum_backing_votes(&self, at: Hash) -> Result { + self.client.runtime_api().minimum_backing_votes(at) + } + async fn staging_para_backing_state( &self, at: Hash, diff --git a/node/subsystem-util/src/lib.rs b/node/subsystem-util/src/lib.rs index daee4a8350e5..dad813fb5ba6 100644 --- a/node/subsystem-util/src/lib.rs +++ b/node/subsystem-util/src/lib.rs @@ -39,7 +39,10 @@ pub use overseer::{ pub use polkadot_node_metrics::{metrics, Metronome}; -use futures::channel::{mpsc, oneshot}; +use futures::{ + channel::{mpsc, oneshot}, + Future, +}; use parity_scale_codec::Encode; use polkadot_primitives::{ @@ -96,6 +99,8 @@ mod tests; pub const JOB_GRACEFUL_STOP_DURATION: Duration = Duration::from_secs(1); /// Capacity of channels to and from individual jobs pub const JOB_CHANNEL_CAPACITY: usize = 64; +/// Used prior to runtime API version 6. +const LEGACY_MIN_BACKING_VOTES: u32 = 2; /// Utility errors #[derive(Debug, Error)] @@ -230,6 +235,31 @@ specialize_requests! { fn request_staging_async_backing_params() -> vstaging_primitives::AsyncBackingParams; StagingAsyncBackingParams; } +/// Request the min backing votes value. +/// Prior to runtime API version 6, just return a hardcoded constant. +pub async fn request_min_backing_votes<'a, S, Func, Fut>( + parent: Hash, + sender: &'a mut S, + get_min_backing_votes: Func, +) -> Result +where + S: overseer::SubsystemSender, + Func: FnOnce(&'a mut S) -> Fut, + Fut: Future>, +{ + let runtime_api_version = request_runtime_api_version(parent, sender) + .await + .await + .map_err(runtime::Error::RuntimeRequestCanceled)? + .map_err(runtime::Error::RuntimeRequest)?; + + if runtime_api_version >= RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT { + get_min_backing_votes(sender).await + } else { + Ok(LEGACY_MIN_BACKING_VOTES) + } +} + /// Requests executor parameters from the runtime effective at given relay-parent. First obtains /// session index at the relay-parent, relying on the fact that it should be cached by the runtime /// API caching layer even if the block itself has already been pruned. Then requests executor diff --git a/node/subsystem-util/src/runtime/error.rs b/node/subsystem-util/src/runtime/error.rs index db3eacd68514..203f7f64d83f 100644 --- a/node/subsystem-util/src/runtime/error.rs +++ b/node/subsystem-util/src/runtime/error.rs @@ -43,7 +43,7 @@ pub enum Error { pub type Result = std::result::Result; /// Receive a response from a runtime request and convert errors. -pub(crate) async fn recv_runtime( +pub async fn recv_runtime( r: oneshot::Receiver>, ) -> Result { let result = r diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index 534449ca7507..9d4dcc5fff2f 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -47,8 +47,8 @@ use crate::{ /// Errors that can happen on runtime fetches. mod error; -use error::{recv_runtime, Result}; -pub use error::{Error, FatalError, JfyiError}; +use error::Result; +pub use error::{recv_runtime, Error, FatalError, JfyiError}; const LOG_TARGET: &'static str = "parachain::runtime-info"; @@ -166,7 +166,7 @@ impl RuntimeInfo { } /// Get minimum_backing_votes by relay parent hash. - pub async fn get_min_backing_votes<'a, Sender>( + pub async fn get_min_backing_votes( &mut self, sender: &mut Sender, session_index: SessionIndex, From c846d6d477163903dc5ca0ff2b1d625f0799d5d7 Mon Sep 17 00:00:00 2001 From: alindima Date: Wed, 23 Aug 2023 11:52:01 +0300 Subject: [PATCH 5/5] remove explicit version runtime API call this is not needed, as the RuntimeAPISubsystem already takes care of versioning and will return NotSupported if the version is not right. --- node/core/backing/src/lib.rs | 11 ++++--- node/core/backing/src/tests/mod.rs | 16 ++++----- .../src/tests/prospective_parachains.rs | 10 ------ .../src/vstaging/mod.rs | 17 ++++++---- .../src/vstaging/tests/mod.rs | 8 ----- node/subsystem-util/src/lib.rs | 32 +----------------- node/subsystem-util/src/runtime/mod.rs | 33 ++++++++++++++++++- 7 files changed, 57 insertions(+), 70 deletions(-) diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 96f3d4c4fde2..2dc614876fe6 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -115,7 +115,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; -use util::{request_min_backing_votes, runtime::RuntimeInfo}; +use util::runtime::{request_min_backing_votes, RuntimeInfo}; mod error; @@ -999,10 +999,11 @@ async fn construct_per_relay_parent_state( let session_index = try_runtime_api!(runtime_info.get_session_index_for_child(ctx.sender(), parent).await); - let minimum_backing_votes = request_min_backing_votes(parent, ctx.sender(), |sender| { - runtime_info.get_min_backing_votes(sender, session_index, parent) - }) - .await?; + let minimum_backing_votes = + request_min_backing_votes(parent, ctx.sender(), |parent, sender| { + runtime_info.get_min_backing_votes(sender, session_index, parent) + }) + .await?; let (validators, groups, cores) = futures::try_join!( request_validators(parent, ctx.sender()).await, diff --git a/node/core/backing/src/tests/mod.rs b/node/core/backing/src/tests/mod.rs index e23eb51983d5..fc083ede3af6 100644 --- a/node/core/backing/src/tests/mod.rs +++ b/node/core/backing/src/tests/mod.rs @@ -263,14 +263,14 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS ); // Check that subsystem job issues a request for the runtime API version. - assert_matches!( - virtual_overseer.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) - ) if parent == test_state.relay_parent => { - tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); - } - ); + // assert_matches!( + // virtual_overseer.recv().await, + // AllMessages::RuntimeApi( + // RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) + // ) if parent == test_state.relay_parent => { + // tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); + // } + // ); // Check if subsystem job issues a request for the minimum backing votes. // This may or may not happen, depending if the minimum backing votes is already cached in the diff --git a/node/core/backing/src/tests/prospective_parachains.rs b/node/core/backing/src/tests/prospective_parachains.rs index a08c4b1fb070..93a8e94b98de 100644 --- a/node/core/backing/src/tests/prospective_parachains.rs +++ b/node/core/backing/src/tests/prospective_parachains.rs @@ -153,16 +153,6 @@ async fn activate_leaf( } ); - // Check that subsystem job issues a request for the runtime API version. - assert_matches!( - virtual_overseer.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) - ) if parent == hash => { - tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); - } - ); - // Check if subsystem job issues a request for the minimum backing votes. // This may or may not happen, depending if the minimum backing votes is already cached in // the `RuntimeInfo`. diff --git a/node/network/statement-distribution/src/vstaging/mod.rs b/node/network/statement-distribution/src/vstaging/mod.rs index 2767534f8b14..264abaf2af3b 100644 --- a/node/network/statement-distribution/src/vstaging/mod.rs +++ b/node/network/statement-distribution/src/vstaging/mod.rs @@ -44,8 +44,8 @@ use polkadot_node_subsystem_types::messages::RuntimeApiRequest; use polkadot_node_subsystem_util::{ backing_implicit_view::View as ImplicitView, reputation::ReputationAggregator, - request_from_runtime, request_min_backing_votes, - runtime::{recv_runtime, ProspectiveParachainsMode}, + request_from_runtime, + runtime::{recv_runtime, request_min_backing_votes, ProspectiveParachainsMode}, }; use polkadot_primitives::vstaging::{ AuthorityDiscoveryId, CandidateHash, CompactStatement, CoreIndex, CoreState, GroupIndex, @@ -507,17 +507,20 @@ pub(crate) async fn handle_active_leaves_update( Some(s) => s, }; - let minimum_backing_votes = - request_min_backing_votes(new_relay_parent, ctx.sender(), |sender| async { + let minimum_backing_votes = request_min_backing_votes( + new_relay_parent, + ctx.sender(), + |parent, sender| async move { recv_runtime( - request_from_runtime(new_relay_parent, sender, |tx| { + request_from_runtime(parent, sender, |tx| { RuntimeApiRequest::MinimumBackingVotes(tx) }) .await, ) .await - }) - .await?; + }, + ) + .await?; state.per_session.insert( session_index, diff --git a/node/network/statement-distribution/src/vstaging/tests/mod.rs b/node/network/statement-distribution/src/vstaging/tests/mod.rs index fa73f4a9a2b0..83922f408527 100644 --- a/node/network/statement-distribution/src/vstaging/tests/mod.rs +++ b/node/network/statement-distribution/src/vstaging/tests/mod.rs @@ -456,14 +456,6 @@ async fn handle_leaf_activation( } ); - assert_matches!( - virtual_overseer.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx))) if parent == *hash => { - tx.send(Ok(RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT)).unwrap(); - } - ); - assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( diff --git a/node/subsystem-util/src/lib.rs b/node/subsystem-util/src/lib.rs index dad813fb5ba6..daee4a8350e5 100644 --- a/node/subsystem-util/src/lib.rs +++ b/node/subsystem-util/src/lib.rs @@ -39,10 +39,7 @@ pub use overseer::{ pub use polkadot_node_metrics::{metrics, Metronome}; -use futures::{ - channel::{mpsc, oneshot}, - Future, -}; +use futures::channel::{mpsc, oneshot}; use parity_scale_codec::Encode; use polkadot_primitives::{ @@ -99,8 +96,6 @@ mod tests; pub const JOB_GRACEFUL_STOP_DURATION: Duration = Duration::from_secs(1); /// Capacity of channels to and from individual jobs pub const JOB_CHANNEL_CAPACITY: usize = 64; -/// Used prior to runtime API version 6. -const LEGACY_MIN_BACKING_VOTES: u32 = 2; /// Utility errors #[derive(Debug, Error)] @@ -235,31 +230,6 @@ specialize_requests! { fn request_staging_async_backing_params() -> vstaging_primitives::AsyncBackingParams; StagingAsyncBackingParams; } -/// Request the min backing votes value. -/// Prior to runtime API version 6, just return a hardcoded constant. -pub async fn request_min_backing_votes<'a, S, Func, Fut>( - parent: Hash, - sender: &'a mut S, - get_min_backing_votes: Func, -) -> Result -where - S: overseer::SubsystemSender, - Func: FnOnce(&'a mut S) -> Fut, - Fut: Future>, -{ - let runtime_api_version = request_runtime_api_version(parent, sender) - .await - .await - .map_err(runtime::Error::RuntimeRequestCanceled)? - .map_err(runtime::Error::RuntimeRequest)?; - - if runtime_api_version >= RuntimeApiRequest::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT { - get_min_backing_votes(sender).await - } else { - Ok(LEGACY_MIN_BACKING_VOTES) - } -} - /// Requests executor parameters from the runtime effective at given relay-parent. First obtains /// session index at the relay-parent, relying on the fact that it should be cached by the runtime /// API caching layer even if the block itself has already been pruned. Then requests executor diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index 9d4dcc5fff2f..ddee697e78c1 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -16,7 +16,7 @@ //! Convenient interface to runtime information. -use std::num::NonZeroUsize; +use std::{future::Future, num::NonZeroUsize}; use lru::LruCache; @@ -52,6 +52,9 @@ pub use error::{recv_runtime, Error, FatalError, JfyiError}; const LOG_TARGET: &'static str = "parachain::runtime-info"; +/// Used prior to runtime API version 6. +const LEGACY_MIN_BACKING_VOTES: u32 = 2; + /// Configuration for construction a `RuntimeInfo`. pub struct Config { /// Needed for retrieval of `ValidatorInfo` @@ -490,3 +493,31 @@ where }) } } + +/// Request the min backing votes value. +/// Prior to runtime API version 6, just return a hardcoded constant. +pub async fn request_min_backing_votes<'a, S, Func, Fut>( + parent: Hash, + sender: &'a mut S, + get_min_backing_votes: Func, +) -> Result +where + S: overseer::SubsystemSender, + Func: FnOnce(Hash, &'a mut S) -> Fut, + Fut: Future>, +{ + let min_backing_votes_res = get_min_backing_votes(parent, sender).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 + } +}