From 800d8e5a8bc93288abf3a307b9889fdde5a60ef8 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 10 Aug 2023 11:55:32 +0300 Subject: [PATCH 01/28] Add `disabled_validators` in staging runtime api --- polkadot/node/core/runtime-api/src/cache.rs | 18 ++++++++++++++++++ polkadot/node/core/runtime-api/src/lib.rs | 10 ++++++++++ polkadot/node/subsystem-types/src/messages.rs | 6 ++++++ .../node/subsystem-types/src/runtime_client.rs | 7 +++++++ polkadot/node/subsystem-util/src/lib.rs | 1 + polkadot/primitives/src/runtime_api.rs | 6 ++++++ .../src/runtime_api_impl/vstaging.rs | 11 ++++++++++- polkadot/runtime/rococo/src/lib.rs | 9 ++++++++- 8 files changed, 66 insertions(+), 2 deletions(-) diff --git a/polkadot/node/core/runtime-api/src/cache.rs b/polkadot/node/core/runtime-api/src/cache.rs index cd22f37ddee43..896ef3f8e0d8e 100644 --- a/polkadot/node/core/runtime-api/src/cache.rs +++ b/polkadot/node/core/runtime-api/src/cache.rs @@ -65,6 +65,7 @@ pub(crate) struct RequestResultCache { LruMap>, key_ownership_proof: LruMap<(Hash, ValidatorId), Option>, + disabled_validators: LruMap>, staging_para_backing_state: LruMap<(Hash, ParaId), Option>, staging_async_backing_params: LruMap, @@ -97,6 +98,7 @@ impl Default for RequestResultCache { disputes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), unapplied_slashes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), key_ownership_proof: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), + disabled_validators: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), staging_para_backing_state: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), staging_async_backing_params: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), @@ -434,6 +436,21 @@ impl RequestResultCache { None } + pub(crate) fn disabled_validators( + &mut self, + relay_parent: &Hash, + ) -> Option<&Vec> { + self.disabled_validators.get(relay_parent).map(|v| &*v) + } + + pub(crate) fn cache_disabled_validators( + &mut self, + relay_parent: Hash, + disabled_validators: Vec, + ) { + self.disabled_validators.insert(relay_parent, disabled_validators); + } + pub(crate) fn staging_para_backing_state( &mut self, key: (Hash, ParaId), @@ -509,6 +526,7 @@ pub(crate) enum RequestResult { vstaging::slashing::OpaqueKeyOwnershipProof, Option<()>, ), + DisabledValidators(Hash, Vec), StagingParaBackingState(Hash, ParaId, Option), StagingAsyncBackingParams(Hash, vstaging::AsyncBackingParams), diff --git a/polkadot/node/core/runtime-api/src/lib.rs b/polkadot/node/core/runtime-api/src/lib.rs index 78531d41272b5..04f5bb3fd7068 100644 --- a/polkadot/node/core/runtime-api/src/lib.rs +++ b/polkadot/node/core/runtime-api/src/lib.rs @@ -163,6 +163,8 @@ where .requests_cache .cache_key_ownership_proof((relay_parent, validator_id), key_ownership_proof), SubmitReportDisputeLost(_, _, _, _) => {}, + DisabledValidators(relay_parent, disabled_validators) => + self.requests_cache.cache_disabled_validators(relay_parent, disabled_validators), StagingParaBackingState(relay_parent, para_id, constraints) => self .requests_cache @@ -294,6 +296,8 @@ where Request::SubmitReportDisputeLost(dispute_proof, key_ownership_proof, sender) }, ), + Request::DisabledValidators(sender) => query!(disabled_validators(), sender) + .map(|sender| Request::DisabledValidators(sender)), Request::StagingParaBackingState(para, sender) => query!(staging_para_backing_state(para), sender) @@ -551,6 +555,12 @@ where ver = Request::SUBMIT_REPORT_DISPUTE_LOST_RUNTIME_REQUIREMENT, sender ), + Request::DisabledValidators(sender) => query!( + DisabledValidators, + disabled_validators(), + ver = Request::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, + sender + ), Request::StagingParaBackingState(para, sender) => { query!( diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 8adc39eed56db..0d85034103d0e 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -691,6 +691,9 @@ pub enum RuntimeApiRequest { slashing::OpaqueKeyOwnershipProof, RuntimeApiSender>, ), + /// Returns all disabled validators at a given block height. + /// `V6` + DisabledValidators(RuntimeApiSender>), /// Get the backing state of the given para. /// This is a staging API that will not be available on production runtimes. @@ -719,6 +722,9 @@ impl RuntimeApiRequest { /// `SubmitReportDisputeLost` pub const SUBMIT_REPORT_DISPUTE_LOST_RUNTIME_REQUIREMENT: u32 = 5; + /// `DisabledValidators` + pub const DISABLED_VALIDATORS_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/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index 312cc4eec6ce9..77ef358b9b0ef 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -232,6 +232,9 @@ pub trait RuntimeApiSubsystemClient { session_index: SessionIndex, ) -> Result, ApiError>; + /// Gets the disabled validators at a specific block height + async fn disabled_validators(&self, at: Hash) -> Result, ApiError>; + // === Asynchronous backing API === /// Returns candidate's acceptance limitations for asynchronous backing for a relay parent. @@ -473,6 +476,10 @@ where runtime_api.submit_report_dispute_lost(at, dispute_proof, key_ownership_proof) } + async fn disabled_validators(&self, at: Hash) -> Result, ApiError> { + self.client.runtime_api().disabled_validators(at) + } + async fn staging_para_backing_state( &self, at: Hash, diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index daee4a8350e5a..71d2cd335850f 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -226,6 +226,7 @@ specialize_requests! { fn request_unapplied_slashes() -> Vec<(SessionIndex, CandidateHash, slashing::PendingSlashes)>; UnappliedSlashes; fn request_key_ownership_proof(validator_id: ValidatorId) -> Option; KeyOwnershipProof; fn request_submit_report_dispute_lost(dp: slashing::DisputeProof, okop: slashing::OpaqueKeyOwnershipProof) -> Option<()>; SubmitReportDisputeLost; + fn request_disabled_validators() -> Vec; DisabledValidators; fn request_staging_async_backing_params() -> vstaging_primitives::AsyncBackingParams; StagingAsyncBackingParams; } diff --git a/polkadot/primitives/src/runtime_api.rs b/polkadot/primitives/src/runtime_api.rs index 483256fe20f36..463bb789b1edd 100644 --- a/polkadot/primitives/src/runtime_api.rs +++ b/polkadot/primitives/src/runtime_api.rs @@ -240,6 +240,12 @@ sp_api::decl_runtime_apis! { key_ownership_proof: vstaging::slashing::OpaqueKeyOwnershipProof, ) -> Option<()>; + /* Staging APIs */ + + /// Returns a sorted Vec with the `ValidatorIndex` of all disabled validators. + #[api_version(6)] + fn disabled_validators() -> Vec; + /***** Asynchronous backing *****/ /// Returns the state of parachain backing for a given para. diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index 5406428377d0d..af1f3b17b7c5e 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -23,7 +23,7 @@ use primitives::{ AsyncBackingParams, BackingState, CandidatePendingAvailability, Constraints, InboundHrmpLimitations, OutboundHrmpChannelLimitations, }, - Id as ParaId, + Id as ParaId, ValidatorIndex, }; use sp_std::prelude::*; @@ -118,3 +118,12 @@ pub fn backing_state( pub fn async_backing_params() -> AsyncBackingParams { >::config().async_backing_params } + +/// Implementation for `DisabledValidators` +pub fn disabled_validators() -> Vec { + >::disabled_validators() + .iter() + .cloned() + .map(|v| ValidatorIndex(v)) + .collect() +} diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 6894bd7bbf442..940db21c66a8f 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/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, }; @@ -1714,6 +1716,7 @@ sp_api::impl_runtime_apis! { } } + #[api_version(6)] impl primitives::runtime_api::ParachainHost for Runtime { fn validators() -> Vec { parachains_runtime_api_impl::validators::() @@ -1844,6 +1847,10 @@ sp_api::impl_runtime_apis! { key_ownership_proof, ) } + + fn disabled_validators() -> Vec { + parachains_staging_runtime_api_impl::disabled_validators::() + } } #[api_version(3)] From 936216d206821df7f9701c0181f99f8e646a10cc Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 4 Sep 2023 14:13:32 +0300 Subject: [PATCH 02/28] Cumulus changes --- .../relay-chain-minimal-node/src/blockchain_rpc_client.rs | 7 +++++++ .../client/relay-chain-rpc-interface/src/rpc_client.rs | 8 ++++++++ .../integration-tests/emulated/common/src/lib.rs | 1 + 3 files changed, 16 insertions(+) diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index fc4d803002cb2..9e0ba05b2a837 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -338,6 +338,13 @@ impl RuntimeApiSubsystemClient for BlockChainRpcClient { .await?) } + async fn disabled_validators( + &self, + at: Hash, + ) -> Result, ApiError> { + Ok(self.rpc_client.parachain_host_disabled_validators(at).await?) + } + async fn staging_async_backing_params(&self, at: Hash) -> Result { Ok(self.rpc_client.parachain_host_staging_async_backing_params(at).await?) } diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs index b079294b78421..b73b718e3e83d 100644 --- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs +++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs @@ -588,6 +588,14 @@ impl RelayChainRpcClient { .await } + pub async fn parachain_host_disabled_validators( + &self, + at: RelayHash, + ) -> Result, RelayChainError> { + self.call_remote_runtime_function("ParachainHost_disabled_validators", at, None::<()>) + .await + } + #[allow(missing_docs)] pub async fn parachain_host_staging_async_backing_params( &self, diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index f6d589700360c..f0f7aee1fb4fd 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -19,6 +19,7 @@ pub use impls::{RococoWococoMessageHandler, WococoRococoMessageHandler}; pub use parachains_common::{AccountId, Balance}; pub use paste; use polkadot_parachain::primitives::HrmpChannelId; +use polkadot_primitives::runtime_api::runtime_decl_for_parachain_host::ParachainHostV6; pub use polkadot_runtime_parachains::inclusion::{AggregateMessageOrigin, UmpQueueId}; pub use sp_core::{sr25519, storage::Storage, Get}; use sp_tracing; From 7165ef9fe7d75793d428ff6f635aba916ef16901 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 31 Aug 2023 12:24:52 +0300 Subject: [PATCH 03/28] Handling of disabled validators in backing subsystem --- polkadot/node/core/backing/src/lib.rs | 72 ++++++++++++++----- polkadot/node/core/backing/src/tests/mod.rs | 11 +++ .../src/tests/prospective_parachains.rs | 10 +++ polkadot/node/subsystem-util/src/lib.rs | 19 ++++- 4 files changed, 93 insertions(+), 19 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 58763e6d80cc1..1cf905c06faec 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -379,6 +379,20 @@ struct TableContext { validator: Option, groups: HashMap>, validators: Vec, + disabled_validators: Vec, +} + +impl TableContext { + pub fn validator_is_disabled(&self, validator_idx: &ValidatorIndex) -> bool { + self.disabled_validators + .iter() + .find(|disabled_val_idx| *disabled_val_idx == validator_idx) + .is_some() + } + + pub fn local_validator_is_disabled(&self) -> Option { + self.validator.as_ref().map(|v| v.disabled()) + } } impl TableContextTrait for TableContext { @@ -983,7 +997,7 @@ async fn construct_per_relay_parent_state( let parent = relay_parent; - let (validators, groups, session_index, cores) = futures::try_join!( + let (validators, groups, session_index, cores, disabled_validators) = 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, @@ -991,6 +1005,10 @@ async fn construct_per_relay_parent_state( RuntimeApiRequest::AvailabilityCores(tx) },) .await, + request_from_runtime(parent, ctx.sender(), |tx| { + RuntimeApiRequest::DisabledValidators(tx) + },) + .await, ) .map_err(Error::JoinMultiple)?; @@ -998,22 +1016,27 @@ async fn construct_per_relay_parent_state( 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 disabled_validators = try_runtime_api!(disabled_validators); let signing_context = SigningContext { parent_hash: parent, session_index }; - let validator = - match Validator::construct(&validators, signing_context.clone(), keystore.clone()) { - Ok(v) => Some(v), - Err(util::Error::NotAValidator) => None, - Err(e) => { - gum::warn!( - target: LOG_TARGET, - err = ?e, - "Cannot participate in candidate backing", - ); + let validator = match Validator::construct( + &validators, + &disabled_validators, + signing_context.clone(), + keystore.clone(), + ) { + Ok(v) => Some(v), + Err(util::Error::NotAValidator) => None, + Err(e) => { + gum::warn!( + target: LOG_TARGET, + err = ?e, + "Cannot participate in candidate backing", + ); - return Ok(None) - }, - }; + return Ok(None) + }, + }; let mut groups = HashMap::new(); let n_cores = cores.len(); @@ -1043,7 +1066,7 @@ async fn construct_per_relay_parent_state( } } - let table_context = TableContext { groups, validators, validator }; + let table_context = TableContext { validator, groups, validators, disabled_validators }; let table_config = TableConfig { allow_multiple_seconded: match mode { ProspectiveParachainsMode::Enabled { .. } => true, @@ -1552,7 +1575,17 @@ async fn import_statement( let stmt = primitive_statement_to_table(statement); - Ok(rp_state.table.import_statement(&rp_state.table_context, stmt)) + // Don't import statement if the sender is disabled + if rp_state.table_context.validator_is_disabled(&stmt.sender) { + gum::warn!( + target: LOG_TARGET, + sender_validator_idx = ?stmt.sender, + "Not importing statement because the sender is disabled" + ); + return Ok(None) + } else { + Ok(rp_state.table.import_statement(&rp_state.table_context, stmt)) + } } /// Handles a summary received from [`import_statement`] and dispatches `Backed` notifications and @@ -1943,6 +1976,13 @@ async fn handle_second_message( return Ok(()) } + // Just return if the local validator is disabled. If we are here the local node should be a + // validator but defensively use `unwrap_or(false)`) to continue processin in this case. + if rp_state.table_context.local_validator_is_disabled().unwrap_or(false) { + gum::warn!(target: LOG_TARGET, "Local validator is disabled. Don't validate and second"); + return Ok(()) + } + // If the message is a `CandidateBackingMessage::Second`, sign and dispatch a // Seconded statement only if we have not signed a Valid statement for the requested candidate. // diff --git a/polkadot/node/core/backing/src/tests/mod.rs b/polkadot/node/core/backing/src/tests/mod.rs index 054337669c077..b7221fa2210f2 100644 --- a/polkadot/node/core/backing/src/tests/mod.rs +++ b/polkadot/node/core/backing/src/tests/mod.rs @@ -289,6 +289,16 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS tx.send(Ok(test_state.availability_cores.clone())).unwrap(); } ); + + // Check that subsystem job issues a request for the disabled validators. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) + ) if parent == test_state.relay_parent => { + tx.send(Ok(Vec::new())).unwrap(); + } + ); } // Test that a `CandidateBackingMessage::Second` issues validation work @@ -1418,6 +1428,7 @@ fn candidate_backing_reorders_votes() { let table_context = TableContext { validator: None, + disabled_validators: Vec::new(), groups: validator_groups, validators: validator_public.clone(), }; diff --git a/polkadot/node/core/backing/src/tests/prospective_parachains.rs b/polkadot/node/core/backing/src/tests/prospective_parachains.rs index 7c2773c8e3b6b..57d67fea79f58 100644 --- a/polkadot/node/core/backing/src/tests/prospective_parachains.rs +++ b/polkadot/node/core/backing/src/tests/prospective_parachains.rs @@ -183,6 +183,16 @@ async fn activate_leaf( tx.send(Ok(test_state.availability_cores.clone())).unwrap(); } ); + + // Check that the subsystem job issues a request for the disabled validators. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) + ) if parent == hash => { + tx.send(Ok(Vec::new())).unwrap(); + } + ); } } diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 71d2cd335850f..10654b8aec8a7 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -379,6 +379,7 @@ pub struct Validator { signing_context: SigningContext, key: ValidatorId, index: ValidatorIndex, + disabled: bool, } impl Validator { @@ -391,16 +392,19 @@ impl Validator { // Note: request_validators and request_session_index_for_child do not and cannot // run concurrently: they both have a mutable handle to the same sender. // However, each of them returns a oneshot::Receiver, and those are resolved concurrently. - let (validators, session_index) = futures::try_join!( + let (validators, session_index, disabled_validators) = futures::try_join!( request_validators(parent, sender).await, request_session_index_for_child(parent, sender).await, + request_disabled_validators(parent, sender).await, )?; let signing_context = SigningContext { session_index: session_index?, parent_hash: parent }; let validators = validators?; - Self::construct(&validators, signing_context, keystore) + let disabled_validators = disabled_validators?; + + Self::construct(&validators, &disabled_validators, signing_context, keystore) } /// Construct a validator instance without performing runtime fetches. @@ -408,13 +412,17 @@ impl Validator { /// This can be useful if external code also needs the same data. pub fn construct( validators: &[ValidatorId], + disabled_validators: &[ValidatorIndex], signing_context: SigningContext, keystore: KeystorePtr, ) -> Result { let (key, index) = signing_key_and_index(validators, &keystore).ok_or(Error::NotAValidator)?; - Ok(Validator { signing_context, key, index }) + let disabled = + disabled_validators.iter().find(|d: &&ValidatorIndex| **d == index).is_some(); + + Ok(Validator { signing_context, key, index, disabled }) } /// Get this validator's id. @@ -427,6 +435,11 @@ impl Validator { self.index } + /// Get the enabled/disabled state of this validator + pub fn disabled(&self) -> bool { + self.disabled + } + /// Get the current signing context. pub fn signing_context(&self) -> &SigningContext { &self.signing_context From 24b5df5d53bdc935547ce373e4cd6e0311c1c98f Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 31 Aug 2023 15:26:57 +0300 Subject: [PATCH 04/28] Tests --- polkadot/node/core/backing/src/tests/mod.rs | 54 ++++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/polkadot/node/core/backing/src/tests/mod.rs b/polkadot/node/core/backing/src/tests/mod.rs index b7221fa2210f2..efd55174e7094 100644 --- a/polkadot/node/core/backing/src/tests/mod.rs +++ b/polkadot/node/core/backing/src/tests/mod.rs @@ -41,7 +41,7 @@ use sp_keyring::Sr25519Keyring; use sp_keystore::Keystore; use sp_tracing as _; use statement_table::v2::Misbehavior; -use std::collections::HashMap; +use std::{collections::HashMap, time::Duration}; mod prospective_parachains; @@ -80,6 +80,7 @@ struct TestState { head_data: HashMap, signing_context: SigningContext, relay_parent: Hash, + disabled_validators: Vec, } impl TestState { @@ -150,6 +151,7 @@ impl Default for TestState { validation_data, signing_context, relay_parent, + disabled_validators: Vec::new(), } } } @@ -296,7 +298,7 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS AllMessages::RuntimeApi( RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) ) if parent == test_state.relay_parent => { - tx.send(Ok(Vec::new())).unwrap(); + tx.send(Ok(test_state.disabled_validators.clone())).unwrap(); } ); } @@ -2003,3 +2005,51 @@ fn new_leaf_view_doesnt_clobber_old() { virtual_overseer }); } + +// Test that a disabled local validator doesn't do any work on `CandidateBackingMessage::Second` +#[test] +fn disabled_validator_doesnt_distribute_statement() { + let mut test_state = TestState::default(); + test_state.disabled_validators.push(ValidatorIndex(0)); + + test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move { + test_startup(&mut virtual_overseer, &test_state).await; + + let pov = PoV { block_data: BlockData(vec![42, 43, 44]) }; + let pvd = dummy_pvd(); + let validation_code = ValidationCode(vec![1, 2, 3]); + + let expected_head_data = test_state.head_data.get(&test_state.chain_ids[0]).unwrap(); + + let pov_hash = pov.hash(); + let candidate = TestCandidateBuilder { + para_id: test_state.chain_ids[0], + relay_parent: test_state.relay_parent, + pov_hash, + head_data: expected_head_data.clone(), + erasure_root: make_erasure_root(&test_state, pov.clone(), pvd.clone()), + persisted_validation_data_hash: pvd.hash(), + validation_code: validation_code.0.clone(), + } + .build(); + + let second = CandidateBackingMessage::Second( + test_state.relay_parent, + candidate.to_plain(), + pvd.clone(), + pov.clone(), + ); + + virtual_overseer.send(FromOrchestra::Communication { msg: second }).await; + + // Ensure backing subsystem is not doing any work + assert_matches!(virtual_overseer.recv().timeout(Duration::from_secs(1)).await, None); + + virtual_overseer + .send(FromOrchestra::Signal(OverseerSignal::ActiveLeaves( + ActiveLeavesUpdate::stop_work(test_state.relay_parent), + ))) + .await; + virtual_overseer + }); +} From e4dee17aeea8ee66dad4b7f5ae7ad2a82b187da0 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 4 Sep 2023 15:46:15 +0300 Subject: [PATCH 05/28] Fix compilation errors --- .../parachains/integration-tests/emulated/common/src/lib.rs | 1 + polkadot/runtime/rococo/src/lib.rs | 4 ++++ polkadot/runtime/westend/src/lib.rs | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index 49751136f7ba0..e5ec3c245778d 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -24,6 +24,7 @@ use constants::{ bridge_hub_polkadot, bridge_hub_rococo, collectives, kusama, penpal, polkadot, rococo, westend, }; use impls::{RococoWococoMessageHandler, WococoRococoMessageHandler}; +use polkadot_primitives::runtime_api::runtime_decl_for_parachain_host::ParachainHostV6; // Substrate use frame_support::traits::OnInitialize; diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 8284d6e0d3608..71ddf09855210 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1845,6 +1845,10 @@ sp_api::impl_runtime_apis! { ) } + fn minimum_backing_votes() -> u32 { + parachains_staging_runtime_api_impl::minimum_backing_votes::() + } + fn disabled_validators() -> Vec { parachains_staging_runtime_api_impl::disabled_validators::() } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 73aa4980151ea..a368699fa1de2 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1695,6 +1695,10 @@ sp_api::impl_runtime_apis! { fn minimum_backing_votes() -> u32 { parachains_staging_runtime_api_impl::minimum_backing_votes::() } + + fn disabled_validators() -> Vec { + parachains_staging_runtime_api_impl::disabled_validators::() + } } impl beefy_primitives::BeefyApi for Runtime { From 8db76f5aa3ffd9c238b20f203f9d2cf2eb1a0342 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 5 Sep 2023 14:20:56 +0300 Subject: [PATCH 06/28] spelling --- polkadot/node/core/backing/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 6997df8594b00..ec2369f6c7d86 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1992,7 +1992,7 @@ async fn handle_second_message( } // Just return if the local validator is disabled. If we are here the local node should be a - // validator but defensively use `unwrap_or(false)`) to continue processin in this case. + // validator but defensively use `unwrap_or(false)`) to continue processing in this case. if rp_state.table_context.local_validator_is_disabled().unwrap_or(false) { gum::warn!(target: LOG_TARGET, "Local validator is disabled. Don't validate and second"); return Ok(()) From e2846b221d553c716242da88c68db8128932f16a Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 5 Sep 2023 14:39:50 +0300 Subject: [PATCH 07/28] clippy --- polkadot/node/subsystem-util/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 10654b8aec8a7..1b5c13d8098df 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -420,7 +420,7 @@ impl Validator { signing_key_and_index(validators, &keystore).ok_or(Error::NotAValidator)?; let disabled = - disabled_validators.iter().find(|d: &&ValidatorIndex| **d == index).is_some(); + disabled_validators.iter().any(|d: &ValidatorIndex| *d == index); Ok(Validator { signing_context, key, index, disabled }) } From 8604b69d040d7fe9afff07ff4ba108edb4150340 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 5 Sep 2023 14:48:58 +0300 Subject: [PATCH 08/28] fmt --- polkadot/node/subsystem-util/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 1b5c13d8098df..f2102182a4f42 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -419,8 +419,7 @@ impl Validator { let (key, index) = signing_key_and_index(validators, &keystore).ok_or(Error::NotAValidator)?; - let disabled = - disabled_validators.iter().any(|d: &ValidatorIndex| *d == index); + let disabled = disabled_validators.iter().any(|d: &ValidatorIndex| *d == index); Ok(Validator { signing_context, key, index, disabled }) } From 78afa7686d39a0081272dbda26ab6d763915f067 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 5 Sep 2023 15:02:32 +0300 Subject: [PATCH 09/28] clippy --- polkadot/node/core/backing/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index ec2369f6c7d86..334a6965e7396 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -390,8 +390,7 @@ impl TableContext { pub fn validator_is_disabled(&self, validator_idx: &ValidatorIndex) -> bool { self.disabled_validators .iter() - .find(|disabled_val_idx| *disabled_val_idx == validator_idx) - .is_some() + .any(|disabled_val_idx| *disabled_val_idx == *validator_idx) } pub fn local_validator_is_disabled(&self) -> Option { From 9e39e58fd4814995b0c8c28779d9cf29d1b37feb Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 5 Sep 2023 16:51:47 +0300 Subject: [PATCH 10/28] Fix tests --- polkadot/node/core/runtime-api/src/tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/polkadot/node/core/runtime-api/src/tests.rs b/polkadot/node/core/runtime-api/src/tests.rs index bb7c296896117..fa316d190450b 100644 --- a/polkadot/node/core/runtime-api/src/tests.rs +++ b/polkadot/node/core/runtime-api/src/tests.rs @@ -250,6 +250,10 @@ impl RuntimeApiSubsystemClient for MockSubsystemClient { Ok(self.authorities.clone()) } + async fn disabled_validators(&self, _: Hash) -> Result, ApiError> { + todo!("Not required for tests") + } + async fn staging_async_backing_params( &self, _: Hash, From 91d1a8d8f8d98023e2c0337bf21abbcbe3a8daab Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 11 Sep 2023 16:06:55 +0300 Subject: [PATCH 11/28] Demote log level of a trace --- polkadot/node/core/backing/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 334a6965e7396..d12a10da610b0 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1588,7 +1588,7 @@ async fn import_statement( // Don't import statement if the sender is disabled if rp_state.table_context.validator_is_disabled(&stmt.sender) { - gum::warn!( + gum::debug!( target: LOG_TARGET, sender_validator_idx = ?stmt.sender, "Not importing statement because the sender is disabled" From 3d7efaba170df72e35f45064bfd7ebe7f59940b8 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Wed, 13 Sep 2023 14:04:14 +0300 Subject: [PATCH 12/28] Code review feedback --- polkadot/node/core/backing/src/lib.rs | 45 ++++++++++++++++----------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index d12a10da610b0..029e7fba577e7 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -387,12 +387,14 @@ struct TableContext { } impl TableContext { + // Returns `true` if the provided `ValidatorIndex` is in the disabled validators list pub fn validator_is_disabled(&self, validator_idx: &ValidatorIndex) -> bool { self.disabled_validators .iter() .any(|disabled_val_idx| *disabled_val_idx == *validator_idx) } + // Returns `true` if the local validator is in the disabled validators list pub fn local_validator_is_disabled(&self) -> Option { self.validator.as_ref().map(|v| v.disabled()) } @@ -1586,17 +1588,7 @@ async fn import_statement( let stmt = primitive_statement_to_table(statement); - // Don't import statement if the sender is disabled - if rp_state.table_context.validator_is_disabled(&stmt.sender) { - gum::debug!( - target: LOG_TARGET, - sender_validator_idx = ?stmt.sender, - "Not importing statement because the sender is disabled" - ); - return Ok(None) - } else { - Ok(rp_state.table.import_statement(&rp_state.table_context, stmt)) - } + Ok(rp_state.table.import_statement(&rp_state.table_context, stmt)) } /// Handles a summary received from [`import_statement`] and dispatches `Backed` notifications and @@ -1760,6 +1752,12 @@ async fn kick_off_validation_work( background_validation_tx: &mpsc::Sender<(Hash, ValidatedCandidateCommand)>, attesting: AttestingData, ) -> Result<(), Error> { + // Do nothing if the local validator is disabled + if rp_state.table_context.local_validator_is_disabled() == Some(true) { + gum::debug!(target: LOG_TARGET, "We are disabled - don't kick off validation"); + return Ok(()) + } + let candidate_hash = attesting.candidate.hash(); if rp_state.issued_statements.contains(&candidate_hash) { return Ok(()) @@ -1817,6 +1815,16 @@ async fn maybe_validate_and_import( }, }; + // Don't import statement if the sender is disabled + if rp_state.table_context.validator_is_disabled(&statement.validator_index()) { + gum::debug!( + target: LOG_TARGET, + sender_validator_idx = ?statement.validator_index(), + "Not importing statement because the sender is disabled" + ); + return Ok(()) + } + let res = import_statement(ctx, rp_state, &mut state.per_candidate, &statement).await; // if we get an Error::RejectedByProspectiveParachains, @@ -1978,6 +1986,13 @@ async fn handle_second_message( Some(r) => r, }; + // Just return if the local validator is disabled. If we are here the local node should be a + // validator but defensively use `unwrap_or(false)`) to continue processing in this case. + if rp_state.table_context.local_validator_is_disabled().unwrap_or(false) { + gum::warn!(target: LOG_TARGET, "Local validator is disabled. Don't validate and second"); + return Ok(()) + } + // Sanity check that candidate is from our assignment. if Some(candidate.descriptor().para_id) != rp_state.assignment { gum::debug!( @@ -1990,13 +2005,6 @@ async fn handle_second_message( return Ok(()) } - // Just return if the local validator is disabled. If we are here the local node should be a - // validator but defensively use `unwrap_or(false)`) to continue processing in this case. - if rp_state.table_context.local_validator_is_disabled().unwrap_or(false) { - gum::warn!(target: LOG_TARGET, "Local validator is disabled. Don't validate and second"); - return Ok(()) - } - // If the message is a `CandidateBackingMessage::Second`, sign and dispatch a // Seconded statement only if we have not signed a Valid statement for the requested candidate. // @@ -2031,6 +2039,7 @@ async fn handle_statement_message( ) -> Result<(), Error> { let _timer = metrics.time_process_statement(); + // Validator disabling is handled in `maybe_validate_and_import` match maybe_validate_and_import(ctx, state, relay_parent, statement).await { Err(Error::ValidationFailed(_)) => Ok(()), Err(e) => Err(e), From e68fa6aef2c6c268313dfaf79131984f0caedd0b Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Wed, 13 Sep 2023 15:30:57 +0300 Subject: [PATCH 13/28] tests --- polkadot/node/core/backing/src/tests/mod.rs | 256 +++++++++++++++++++- 1 file changed, 255 insertions(+), 1 deletion(-) diff --git a/polkadot/node/core/backing/src/tests/mod.rs b/polkadot/node/core/backing/src/tests/mod.rs index f43b0409fa133..bd73a10bcd240 100644 --- a/polkadot/node/core/backing/src/tests/mod.rs +++ b/polkadot/node/core/backing/src/tests/mod.rs @@ -2272,7 +2272,7 @@ fn new_leaf_view_doesnt_clobber_old() { // Test that a disabled local validator doesn't do any work on `CandidateBackingMessage::Second` #[test] -fn disabled_validator_doesnt_distribute_statement() { +fn disabled_validator_doesnt_distribute_statement_on_receiving_second() { let mut test_state = TestState::default(); test_state.disabled_validators.push(ValidatorIndex(0)); @@ -2317,3 +2317,257 @@ fn disabled_validator_doesnt_distribute_statement() { virtual_overseer }); } + +// Test that a disabled local validator doesn't do any work on `CandidateBackingMessage::Statement` +#[test] +fn disabled_validator_doesnt_distribute_statement_on_receiving_statement() { + let mut test_state = TestState::default(); + test_state.disabled_validators.push(ValidatorIndex(0)); + + test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move { + test_startup(&mut virtual_overseer, &test_state).await; + + let pov = PoV { block_data: BlockData(vec![42, 43, 44]) }; + let pvd = dummy_pvd(); + let validation_code = ValidationCode(vec![1, 2, 3]); + + let expected_head_data = test_state.head_data.get(&test_state.chain_ids[0]).unwrap(); + + let pov_hash = pov.hash(); + let candidate = TestCandidateBuilder { + para_id: test_state.chain_ids[0], + relay_parent: test_state.relay_parent, + pov_hash, + head_data: expected_head_data.clone(), + erasure_root: make_erasure_root(&test_state, pov.clone(), pvd.clone()), + persisted_validation_data_hash: pvd.hash(), + validation_code: validation_code.0.clone(), + } + .build(); + + let public2 = Keystore::sr25519_generate_new( + &*test_state.keystore, + ValidatorId::ID, + Some(&test_state.validators[2].to_seed()), + ) + .expect("Insert key into keystore"); + + let signed = SignedFullStatementWithPVD::sign( + &test_state.keystore, + StatementWithPVD::Seconded(candidate.clone(), pvd.clone()), + &test_state.signing_context, + ValidatorIndex(2), + &public2.into(), + ) + .ok() + .flatten() + .expect("should be signed"); + + let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed.clone()); + + virtual_overseer.send(FromOrchestra::Communication { msg: statement }).await; + + // Ensure backing subsystem is not doing any work + assert_matches!(virtual_overseer.recv().timeout(Duration::from_secs(1)).await, None); + + virtual_overseer + .send(FromOrchestra::Signal(OverseerSignal::ActiveLeaves( + ActiveLeavesUpdate::stop_work(test_state.relay_parent), + ))) + .await; + virtual_overseer + }); +} + +// Test that a validator doesn't do any work on receiving a `CandidateBackingMessage::Statement` +// from a disabled validator +#[test] +fn validator_ignores_statements_from_disabled_validators() { + let mut test_state = TestState::default(); + test_state.disabled_validators.push(ValidatorIndex(2)); + + test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move { + test_startup(&mut virtual_overseer, &test_state).await; + + let pov = PoV { block_data: BlockData(vec![42, 43, 44]) }; + let pvd = dummy_pvd(); + let validation_code = ValidationCode(vec![1, 2, 3]); + + let expected_head_data = test_state.head_data.get(&test_state.chain_ids[0]).unwrap(); + + let pov_hash = pov.hash(); + let candidate = TestCandidateBuilder { + para_id: test_state.chain_ids[0], + relay_parent: test_state.relay_parent, + pov_hash, + head_data: expected_head_data.clone(), + erasure_root: make_erasure_root(&test_state, pov.clone(), pvd.clone()), + persisted_validation_data_hash: pvd.hash(), + validation_code: validation_code.0.clone(), + } + .build(); + let candidate_commitments_hash = candidate.commitments.hash(); + + let public2 = Keystore::sr25519_generate_new( + &*test_state.keystore, + ValidatorId::ID, + Some(&test_state.validators[2].to_seed()), + ) + .expect("Insert key into keystore"); + + let signed_2 = SignedFullStatementWithPVD::sign( + &test_state.keystore, + StatementWithPVD::Seconded(candidate.clone(), pvd.clone()), + &test_state.signing_context, + ValidatorIndex(2), + &public2.into(), + ) + .ok() + .flatten() + .expect("should be signed"); + + let statement_2 = + CandidateBackingMessage::Statement(test_state.relay_parent, signed_2.clone()); + + virtual_overseer.send(FromOrchestra::Communication { msg: statement_2 }).await; + + // Ensure backing subsystem is not doing any work + assert_matches!(virtual_overseer.recv().timeout(Duration::from_secs(1)).await, None); + + // Now send a statement from a honest validator and make sure it gets processed + let public3 = Keystore::sr25519_generate_new( + &*test_state.keystore, + ValidatorId::ID, + Some(&test_state.validators[3].to_seed()), + ) + .expect("Insert key into keystore"); + + let signed_3 = SignedFullStatementWithPVD::sign( + &test_state.keystore, + StatementWithPVD::Seconded(candidate.clone(), pvd.clone()), + &test_state.signing_context, + ValidatorIndex(3), + &public3.into(), + ) + .ok() + .flatten() + .expect("should be signed"); + + let statement_3 = + CandidateBackingMessage::Statement(test_state.relay_parent, signed_3.clone()); + + virtual_overseer.send(FromOrchestra::Communication { msg: statement_3 }).await; + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeByHash(hash, tx)) + ) if hash == validation_code.hash() => { + tx.send(Ok(Some(validation_code.clone()))).unwrap(); + } + ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(tx)) + ) => { + tx.send(Ok(1u32.into())).unwrap(); + } + ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionExecutorParams(sess_idx, tx)) + ) if sess_idx == 1 => { + tx.send(Ok(Some(ExecutorParams::default()))).unwrap(); + } + ); + + // Sending a `Statement::Seconded` for our assignment will start + // validation process. The first thing requested is the PoV. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::AvailabilityDistribution( + AvailabilityDistributionMessage::FetchPoV { + relay_parent, + tx, + .. + } + ) if relay_parent == test_state.relay_parent => { + tx.send(pov.clone()).unwrap(); + } + ); + + // The next step is the actual request to Validation subsystem + // to validate the `Seconded` candidate. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::CandidateValidation( + CandidateValidationMessage::ValidateFromExhaustive( + _pvd, + _validation_code, + candidate_receipt, + _pov, + _, + timeout, + tx, + ), + ) if _pvd == pvd && + _validation_code == validation_code && + *_pov == pov && &candidate_receipt.descriptor == candidate.descriptor() && + timeout == PvfExecTimeoutKind::Backing && + candidate_commitments_hash == candidate_receipt.commitments_hash => + { + tx.send(Ok( + ValidationResult::Valid(CandidateCommitments { + head_data: expected_head_data.clone(), + upward_messages: Default::default(), + horizontal_messages: Default::default(), + new_validation_code: None, + processed_downward_messages: 0, + hrmp_watermark: 0, + }, test_state.validation_data.clone()), + )).unwrap(); + } + ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::AvailabilityStore( + AvailabilityStoreMessage::StoreAvailableData { candidate_hash, tx, .. } + ) if candidate_hash == candidate.hash() => { + tx.send(Ok(())).unwrap(); + } + ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::StatementDistribution( + StatementDistributionMessage::Share(hash, _stmt) + ) => { + assert_eq!(test_state.relay_parent, hash); + } + ); + + assert_matches!( + virtual_overseer.recv().await, + AllMessages::Provisioner( + ProvisionerMessage::ProvisionableData( + _, + ProvisionableData::BackedCandidate(candidate_receipt) + ) + ) => { + assert_eq!(candidate_receipt, candidate.to_plain()); + } + ); + + virtual_overseer + .send(FromOrchestra::Signal(OverseerSignal::ActiveLeaves( + ActiveLeavesUpdate::stop_work(test_state.relay_parent), + ))) + .await; + virtual_overseer + }); +} From 70dc6b62d8c98691ea5055702ebb334d510cc4d0 Mon Sep 17 00:00:00 2001 From: ordian Date: Wed, 27 Sep 2023 18:07:56 +0200 Subject: [PATCH 14/28] implement disabled_validators correctly --- .../subsystem-types/src/runtime_client.rs | 1 - .../src/runtime_api_impl/vstaging.rs | 34 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index 1c56bebb70602..f7adcf9862b5d 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -240,7 +240,6 @@ pub trait RuntimeApiSubsystemClient { session_index: SessionIndex, ) -> Result; - // === v7: Asynchronous backing API === /// Returns candidate's acceptance limitations for asynchronous backing for a relay parent. diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index 5c9f2a8d64003..dba4fc7b3cc60 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -16,18 +16,28 @@ //! Put implementations of functions from staging APIs here. -use primitives::{ - ValidatorIndex, -}; -use sp_std::prelude::*; +use crate::shared; +use primitives::ValidatorIndex; +use sp_std::prelude::Vec; +use sp_std::collections::btree_map::BTreeMap; /// Implementation for `DisabledValidators` -pub fn disabled_validators() -> Vec { - // >::disabled_validators() - // .iter() - // .cloned() - // .map(|v| ValidatorIndex(v)) - // .collect() - // TODO - Vec::new() +pub fn disabled_validators() -> Vec +where + T: pallet_session::Config + shared::Config, +{ + let shuffled_indices = >::active_validator_indices(); + // mapping from raw validator index to `ValidatorIndex` + // this computation is the same within a session, but should be cheap + let reverse_index = shuffled_indices + .iter() + .enumerate() + .map(|(i, v)| (v.0, ValidatorIndex(i as u32))) + .collect::>(); + + // we might have disabled validators who are not parachain validators + >::disabled_validators() + .iter() + .filter_map(|v| reverse_index.get(v).cloned()) + .collect() } From bca3c83b34d88b403de471b878ebc8c1077de2df Mon Sep 17 00:00:00 2001 From: ordian Date: Wed, 27 Sep 2023 18:09:38 +0200 Subject: [PATCH 15/28] add a CAVEAT comment --- polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index dba4fc7b3cc60..4214f25cee544 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -22,6 +22,8 @@ use sp_std::prelude::Vec; use sp_std::collections::btree_map::BTreeMap; /// Implementation for `DisabledValidators` +// CAVEAT: this should only be called on the node side +// as it might produce incorrect results on session boundaries pub fn disabled_validators() -> Vec where T: pallet_session::Config + shared::Config, From f717d0b74ac362a68cc251655ce6bdf39f76d51d Mon Sep 17 00:00:00 2001 From: ordian Date: Wed, 27 Sep 2023 18:13:25 +0200 Subject: [PATCH 16/28] cargo fmt --- polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index 4214f25cee544..24a076f3a4431 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -18,8 +18,7 @@ use crate::shared; use primitives::ValidatorIndex; -use sp_std::prelude::Vec; -use sp_std::collections::btree_map::BTreeMap; +use sp_std::{collections::btree_map::BTreeMap, prelude::Vec}; /// Implementation for `DisabledValidators` // CAVEAT: this should only be called on the node side From 2a5af8963d6f8a142644e98158de96ceac4822f9 Mon Sep 17 00:00:00 2001 From: ordian Date: Wed, 27 Sep 2023 18:32:39 +0200 Subject: [PATCH 17/28] Clarify docs --- polkadot/primitives/src/runtime_api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/primitives/src/runtime_api.rs b/polkadot/primitives/src/runtime_api.rs index ac5a3b054de24..5ec897c8cbb40 100644 --- a/polkadot/primitives/src/runtime_api.rs +++ b/polkadot/primitives/src/runtime_api.rs @@ -261,7 +261,7 @@ sp_api::decl_runtime_apis! { /***** Added in v8 *****/ - /// Returns a sorted Vec with the `ValidatorIndex` of all disabled validators. + /// Returns a list of all disabled validators at the given block. #[api_version(8)] fn disabled_validators() -> Vec; } From cf671f4450666ede2f6d3fcb42913835cba4a3a5 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 17 Oct 2023 11:46:28 +0300 Subject: [PATCH 18/28] Update polkadot/node/core/backing/src/lib.rs Co-authored-by: ordian --- polkadot/node/core/backing/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 029e7fba577e7..7d77d6a81a02f 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1987,7 +1987,7 @@ async fn handle_second_message( }; // Just return if the local validator is disabled. If we are here the local node should be a - // validator but defensively use `unwrap_or(false)`) to continue processing in this case. + // validator but defensively use `unwrap_or(false)` to continue processing in this case. if rp_state.table_context.local_validator_is_disabled().unwrap_or(false) { gum::warn!(target: LOG_TARGET, "Local validator is disabled. Don't validate and second"); return Ok(()) From 88079815988816b0236365988ea81e4e2ede8bb9 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 17 Oct 2023 22:16:46 +0300 Subject: [PATCH 19/28] Don't kikoff validation work if we are not a validator --- polkadot/node/core/backing/src/lib.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 7d77d6a81a02f..ed57b1d330b6b 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1752,10 +1752,17 @@ async fn kick_off_validation_work( background_validation_tx: &mpsc::Sender<(Hash, ValidatedCandidateCommand)>, attesting: AttestingData, ) -> Result<(), Error> { - // Do nothing if the local validator is disabled - if rp_state.table_context.local_validator_is_disabled() == Some(true) { - gum::debug!(target: LOG_TARGET, "We are disabled - don't kick off validation"); - return Ok(()) + // Do nothing if the local validator is disabled or not a validator at all + match rp_state.table_context.local_validator_is_disabled() { + Some(true) => { + gum::debug!(target: LOG_TARGET, "We are disabled - don't kick off validation"); + return Ok(()) + }, + Some(false) => {}, // we are not disabled - move on + None => { + gum::debug!(target: LOG_TARGET, "We are not a validator - don't kick off validation"); + return Ok(()) + }, } let candidate_hash = attesting.candidate.hash(); From 617e477dc48ccea41e3a196121761b8eb14c3690 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 19 Oct 2023 10:34:34 +0300 Subject: [PATCH 20/28] Extract `has_required_runtime` from provisioner to subsystem-util --- polkadot/node/core/provisioner/src/lib.rs | 59 ++--------------------- polkadot/node/subsystem-util/src/lib.rs | 58 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 56 deletions(-) diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs index f81e5550b15da..a8475cffa1990 100644 --- a/polkadot/node/core/provisioner/src/lib.rs +++ b/polkadot/node/core/provisioner/src/lib.rs @@ -29,13 +29,13 @@ use polkadot_node_subsystem::{ jaeger, messages::{ CandidateBackingMessage, ChainApiMessage, ProspectiveParachainsMessage, ProvisionableData, - ProvisionerInherentData, ProvisionerMessage, RuntimeApiMessage, RuntimeApiRequest, + ProvisionerInherentData, ProvisionerMessage, RuntimeApiRequest, }, overseer, ActivatedLeaf, ActiveLeavesUpdate, FromOrchestra, LeafStatus, OverseerSignal, - PerLeafSpan, RuntimeApiError, SpawnedSubsystem, SubsystemError, + PerLeafSpan, SpawnedSubsystem, SubsystemError, }; use polkadot_node_subsystem_util::{ - request_availability_cores, request_persisted_validation_data, + has_required_runtime, request_availability_cores, request_persisted_validation_data, runtime::{prospective_parachains_mode, ProspectiveParachainsMode}, TimeoutExt, }; @@ -862,56 +862,3 @@ fn bitfields_indicate_availability( 3 * availability.count_ones() >= 2 * availability.len() } - -// If we have to be absolutely precise here, this method gets the version of the `ParachainHost` -// api. For brevity we'll just call it 'runtime version'. -async fn has_required_runtime( - sender: &mut impl overseer::ProvisionerSenderTrait, - relay_parent: Hash, - required_runtime_version: u32, -) -> bool { - gum::trace!(target: LOG_TARGET, ?relay_parent, "Fetching ParachainHost runtime api version"); - - let (tx, rx) = oneshot::channel(); - sender - .send_message(RuntimeApiMessage::Request(relay_parent, RuntimeApiRequest::Version(tx))) - .await; - - match rx.await { - Result::Ok(Ok(runtime_version)) => { - gum::trace!( - target: LOG_TARGET, - ?relay_parent, - ?runtime_version, - ?required_runtime_version, - "Fetched ParachainHost runtime api version" - ); - runtime_version >= required_runtime_version - }, - Result::Ok(Err(RuntimeApiError::Execution { source: error, .. })) => { - gum::trace!( - target: LOG_TARGET, - ?relay_parent, - ?error, - "Execution error while fetching ParachainHost runtime api version" - ); - false - }, - Result::Ok(Err(RuntimeApiError::NotSupported { .. })) => { - gum::trace!( - target: LOG_TARGET, - ?relay_parent, - "NotSupported error while fetching ParachainHost runtime api version" - ); - false - }, - Result::Err(_) => { - gum::trace!( - target: LOG_TARGET, - ?relay_parent, - "Cancelled error while fetching ParachainHost runtime api version" - ); - false - }, - } -} diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 1db62cc7190cc..190f6e07be696 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -92,6 +92,8 @@ mod determine_new_blocks; #[cfg(test)] mod tests; +const LOG_TARGET: &'static str = "parachain::subsystem-util"; + /// Duration a job will wait after sending a stop signal before hard-aborting. pub const JOB_GRACEFUL_STOP_DURATION: Duration = Duration::from_secs(1); /// Capacity of channels to and from individual jobs @@ -157,6 +159,62 @@ where rx } +/// Verifies if `ParachainHost` runtime api is at least at version `required_runtime_version`. This +/// method is used to determine if a given runtime call is supported by the runtime. +pub async fn has_required_runtime( + sender: &mut Sender, + relay_parent: Hash, + required_runtime_version: u32, +) -> bool +where + Sender: SubsystemSender, +{ + gum::trace!(target: LOG_TARGET, ?relay_parent, "Fetching ParachainHost runtime api version"); + + let (tx, rx) = oneshot::channel(); + sender + .send_message(RuntimeApiMessage::Request(relay_parent, RuntimeApiRequest::Version(tx))) + .await; + + match rx.await { + Result::Ok(Ok(runtime_version)) => { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + ?runtime_version, + ?required_runtime_version, + "Fetched ParachainHost runtime api version" + ); + runtime_version >= required_runtime_version + }, + Result::Ok(Err(RuntimeApiError::Execution { source: error, .. })) => { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + ?error, + "Execution error while fetching ParachainHost runtime api version" + ); + false + }, + Result::Ok(Err(RuntimeApiError::NotSupported { .. })) => { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + "NotSupported error while fetching ParachainHost runtime api version" + ); + false + }, + Result::Err(_) => { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + "Cancelled error while fetching ParachainHost runtime api version" + ); + false + }, + } +} + /// Construct specialized request functions for the runtime. /// /// These would otherwise get pretty repetitive. From d7f43151578d0838caf4ee56c52b99f3b2643435 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 19 Oct 2023 10:39:44 +0300 Subject: [PATCH 21/28] Handle old runtimes --- polkadot/node/core/backing/src/lib.rs | 33 ++++++++++++++++++++----- polkadot/node/subsystem-util/src/lib.rs | 18 +++++++++++--- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index ed57b1d330b6b..23d4ef1d09649 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -118,6 +118,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; +use util::has_required_runtime; mod error; @@ -1007,7 +1008,7 @@ async fn construct_per_relay_parent_state( let parent = relay_parent; - let (session_index, validators, groups, cores, disabled_validators) = futures::try_join!( + let (session_index, validators, groups, cores) = futures::try_join!( request_session_index_for_child(parent, ctx.sender()).await, request_validators(parent, ctx.sender()).await, request_validator_groups(parent, ctx.sender()).await, @@ -1015,10 +1016,6 @@ async fn construct_per_relay_parent_state( RuntimeApiRequest::AvailabilityCores(tx) },) .await, - request_from_runtime(parent, ctx.sender(), |tx| { - RuntimeApiRequest::DisabledValidators(tx) - },) - .await, ) .map_err(Error::JoinMultiple)?; @@ -1028,7 +1025,31 @@ async fn construct_per_relay_parent_state( let cores = try_runtime_api!(cores); let minimum_backing_votes = try_runtime_api!(request_min_backing_votes(parent, session_index, ctx.sender()).await); - let disabled_validators = try_runtime_api!(disabled_validators); + + // TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 + // Once runtime ver `DISABLED_VALIDATORS_RUNTIME_REQUIREMENT` is released remove this `if` + // statement, add `request_from_runtime` call to the `try_join!` call above and use + // `try_runtime_api!` to get `disabled_validators` + let disabled_validators = if has_required_runtime( + ctx.sender(), + parent, + RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, + ) + .await + { + let disabled_validators = request_from_runtime(parent, ctx.sender(), |tx| { + RuntimeApiRequest::DisabledValidators(tx) + }) + .await + .await + .map_err(Error::JoinMultiple)?; + + let disabled_validators = try_runtime_api!(disabled_validators); + disabled_validators + } else { + gum::debug!(target: LOG_TARGET, "Runtime doesn't support `DisabledValidators` - continuing with an empty disabled validators set"); + vec![] + }; let signing_context = SigningContext { parent_hash: parent, session_index }; let validator = match Validator::construct( diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 190f6e07be696..d1358747ff14d 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -449,17 +449,29 @@ impl Validator { // Note: request_validators and request_session_index_for_child do not and cannot // run concurrently: they both have a mutable handle to the same sender. // However, each of them returns a oneshot::Receiver, and those are resolved concurrently. - let (validators, session_index, disabled_validators) = futures::try_join!( + let (validators, session_index) = futures::try_join!( request_validators(parent, sender).await, request_session_index_for_child(parent, sender).await, - request_disabled_validators(parent, sender).await, )?; let signing_context = SigningContext { session_index: session_index?, parent_hash: parent }; let validators = validators?; - let disabled_validators = disabled_validators?; + // TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 + // When `DisabledValidators` is released remove this `if`` and add a + // `request_disabled_validators` call in the `try_join!` call above + let disabled_validators = if has_required_runtime( + sender, + parent, + RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, + ) + .await + { + request_disabled_validators(parent, sender).await.await?? + } else { + vec![] + }; Self::construct(&validators, &disabled_validators, signing_context, keystore) } From 718f04b7950af8bccdc18e7af7fcf04624beac98 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 19 Oct 2023 11:53:01 +0300 Subject: [PATCH 22/28] Fix tests --- polkadot/node/core/backing/src/tests/mod.rs | 28 +++++++++++++------ .../src/tests/prospective_parachains.rs | 28 +++++++++++++------ 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/polkadot/node/core/backing/src/tests/mod.rs b/polkadot/node/core/backing/src/tests/mod.rs index 6d0cbc5353e46..26b0c679325bc 100644 --- a/polkadot/node/core/backing/src/tests/mod.rs +++ b/polkadot/node/core/backing/src/tests/mod.rs @@ -285,24 +285,34 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS } ); - // Check that subsystem job issues a request for the disabled validators. + // Check if subsystem job issues a request for the minimum backing votes. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + parent, + RuntimeApiRequest::MinimumBackingVotes(session_index, tx), + )) if parent == test_state.relay_parent && session_index == test_state.signing_context.session_index => { + tx.send(Ok(test_state.minimum_backing_votes)).unwrap(); + } + ); + + // Check that subsystem job issues a request for the runtime version. assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) + RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) ) if parent == test_state.relay_parent => { - tx.send(Ok(test_state.disabled_validators.clone())).unwrap(); + tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT)).unwrap(); } ); - // Check if subsystem job issues a request for the minimum backing votes. + // Check that subsystem job issues a request for the disabled validators. assert_matches!( virtual_overseer.recv().await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - parent, - RuntimeApiRequest::MinimumBackingVotes(session_index, tx), - )) if parent == test_state.relay_parent && session_index == test_state.signing_context.session_index => { - tx.send(Ok(test_state.minimum_backing_votes)).unwrap(); + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) + ) if parent == test_state.relay_parent => { + tx.send(Ok(test_state.disabled_validators.clone())).unwrap(); } ); } diff --git a/polkadot/node/core/backing/src/tests/prospective_parachains.rs b/polkadot/node/core/backing/src/tests/prospective_parachains.rs index 0606e03c19d4e..661b74c2f969a 100644 --- a/polkadot/node/core/backing/src/tests/prospective_parachains.rs +++ b/polkadot/node/core/backing/src/tests/prospective_parachains.rs @@ -185,24 +185,34 @@ async fn activate_leaf( } ); - // Check that the subsystem job issues a request for the disabled validators. + // Check if subsystem job issues a request for the minimum backing votes. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + parent, + RuntimeApiRequest::MinimumBackingVotes(session_index, tx), + )) if parent == hash && session_index == test_state.signing_context.session_index => { + tx.send(Ok(test_state.minimum_backing_votes)).unwrap(); + } + ); + + // Check that subsystem job issues a request for the runtime version. assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) + RuntimeApiMessage::Request(parent, RuntimeApiRequest::Version(tx)) ) if parent == hash => { - tx.send(Ok(Vec::new())).unwrap(); + tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT)).unwrap(); } ); - // Check if subsystem job issues a request for the minimum backing votes. + // Check that the subsystem job issues a request for the disabled validators. assert_matches!( virtual_overseer.recv().await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - parent, - RuntimeApiRequest::MinimumBackingVotes(session_index, tx), - )) if parent == hash && session_index == test_state.signing_context.session_index => { - tx.send(Ok(test_state.minimum_backing_votes)).unwrap(); + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx)) + ) if parent == hash => { + tx.send(Ok(Vec::new())).unwrap(); } ); } From 275ce02a3edceb5fcc5b5937f7516e53625810f5 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 19 Oct 2023 22:50:59 +0300 Subject: [PATCH 23/28] Use the specialized runtime request function for disabled validators --- polkadot/node/core/backing/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 23d4ef1d09649..6867605064973 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -118,7 +118,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; -use util::has_required_runtime; +use util::{has_required_runtime, request_disabled_validators}; mod error; @@ -1037,12 +1037,10 @@ async fn construct_per_relay_parent_state( ) .await { - let disabled_validators = request_from_runtime(parent, ctx.sender(), |tx| { - RuntimeApiRequest::DisabledValidators(tx) - }) - .await - .await - .map_err(Error::JoinMultiple)?; + let disabled_validators = request_disabled_validators(parent, ctx.sender()) + .await + .await + .map_err(Error::JoinMultiple)?; let disabled_validators = try_runtime_api!(disabled_validators); disabled_validators From f2b17f1192a7e268626118d4dd6d7f3a10255959 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Thu, 19 Oct 2023 22:51:13 +0300 Subject: [PATCH 24/28] Fix log level --- polkadot/node/core/backing/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 6867605064973..0053e32456c34 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1774,7 +1774,7 @@ async fn kick_off_validation_work( // Do nothing if the local validator is disabled or not a validator at all match rp_state.table_context.local_validator_is_disabled() { Some(true) => { - gum::debug!(target: LOG_TARGET, "We are disabled - don't kick off validation"); + gum::info!(target: LOG_TARGET, "We are disabled - don't kick off validation"); return Ok(()) }, Some(false) => {}, // we are not disabled - move on From 3c890594a06566a735fad4e47eaac14a2ce61fae Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Fri, 20 Oct 2023 14:24:40 +0300 Subject: [PATCH 25/28] Extract `get_disabled_validators_with_fallback` in a separate module so that it can be reused in other subsystems --- polkadot/node/core/backing/src/lib.rs | 29 +++------- polkadot/node/subsystem-util/src/lib.rs | 3 + polkadot/node/subsystem-util/src/vstaging.rs | 58 ++++++++++++++++++++ 3 files changed, 68 insertions(+), 22 deletions(-) create mode 100644 polkadot/node/subsystem-util/src/vstaging.rs diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 0053e32456c34..9fc03f2efdd5b 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -118,7 +118,7 @@ use statement_table::{ }, Config as TableConfig, Context as TableContextTrait, Table, }; -use util::{has_required_runtime, request_disabled_validators}; +use util::vstaging::get_disabled_validators_with_fallback; mod error; @@ -1027,27 +1027,12 @@ async fn construct_per_relay_parent_state( try_runtime_api!(request_min_backing_votes(parent, session_index, ctx.sender()).await); // TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 - // Once runtime ver `DISABLED_VALIDATORS_RUNTIME_REQUIREMENT` is released remove this `if` - // statement, add `request_from_runtime` call to the `try_join!` call above and use - // `try_runtime_api!` to get `disabled_validators` - let disabled_validators = if has_required_runtime( - ctx.sender(), - parent, - RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, - ) - .await - { - let disabled_validators = request_disabled_validators(parent, ctx.sender()) - .await - .await - .map_err(Error::JoinMultiple)?; - - let disabled_validators = try_runtime_api!(disabled_validators); - disabled_validators - } else { - gum::debug!(target: LOG_TARGET, "Runtime doesn't support `DisabledValidators` - continuing with an empty disabled validators set"); - vec![] - }; + // Once runtime ver `DISABLED_VALIDATORS_RUNTIME_REQUIREMENT` is released remove this call to + // `get_disabled_validators_with_fallback`, add `request_disabled_validators` call to the + // `try_join!` above and use `try_runtime_api!` to get `disabled_validators` + let disabled_validators = get_disabled_validators_with_fallback(ctx.sender(), parent) + .await + .map_err(Error::UtilError)?; let signing_context = SigningContext { parent_hash: parent, session_index }; let validator = match Validator::construct( diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index d1358747ff14d..a8b3db3bbf30e 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -79,6 +79,9 @@ pub mod inclusion_emulator; /// Convenient and efficient runtime info access. pub mod runtime; +/// Helpers for working with unreleased runtime calls +pub mod vstaging; + /// Nested message sending /// /// Useful for having mostly synchronous code, with submodules spawning short lived asynchronous diff --git a/polkadot/node/subsystem-util/src/vstaging.rs b/polkadot/node/subsystem-util/src/vstaging.rs new file mode 100644 index 0000000000000..03641ac6e8ed3 --- /dev/null +++ b/polkadot/node/subsystem-util/src/vstaging.rs @@ -0,0 +1,58 @@ +// 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 . + +//! Contains helpers for staging runtime calls. +//! +//! This module is intended to contain common boiler plate code handling unreleased runtime API +//! calls. + +use polkadot_node_subsystem_types::messages::{RuntimeApiMessage, RuntimeApiRequest}; +use polkadot_overseer::SubsystemSender; +use polkadot_primitives::{Hash, ValidatorIndex}; + +use crate::{has_required_runtime, request_disabled_validators, Error}; + +const LOG_TARGET: &'static str = "parachain::subsystem-util-vstaging"; + +/// Returns disabled validators list if the runtime supports it. Otherwise logs a debug messages and +/// returns an empty vec. +/// Once runtime ver `DISABLED_VALIDATORS_RUNTIME_REQUIREMENT` is released remove this function and +/// replace all usages with `request_disabled_validators` +/// TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 +pub async fn get_disabled_validators_with_fallback>( + sender: &mut Sender, + relay_parent: Hash, +) -> Result, Error> { + let disabled_validators = if has_required_runtime( + sender, + relay_parent, + RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, + ) + .await + { + let r = request_disabled_validators(relay_parent, sender) + .await + .await + .map_err(Error::Oneshot)??; + + r + } else { + gum::debug!(target: LOG_TARGET, "Runtime doesn't support `DisabledValidators` - continuing with an empty disabled validators set"); + vec![] + }; + + Ok(disabled_validators) +} From 15b5069eb4e38c249d8320b69659381063aeb983 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Fri, 20 Oct 2023 14:26:17 +0300 Subject: [PATCH 26/28] Small style fix --- polkadot/node/subsystem-util/src/vstaging.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/polkadot/node/subsystem-util/src/vstaging.rs b/polkadot/node/subsystem-util/src/vstaging.rs index 03641ac6e8ed3..b1ba20112dfa7 100644 --- a/polkadot/node/subsystem-util/src/vstaging.rs +++ b/polkadot/node/subsystem-util/src/vstaging.rs @@ -43,12 +43,10 @@ pub async fn get_disabled_validators_with_fallback Date: Fri, 20 Oct 2023 14:34:33 +0300 Subject: [PATCH 27/28] Use `get_disabled_validators_with_fallback` in `Validator` from subsystem-util too --- polkadot/node/subsystem-util/src/lib.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index a8b3db3bbf30e..1fe52767df61c 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -55,6 +55,7 @@ use sp_core::ByteArray; use sp_keystore::{Error as KeystoreError, KeystorePtr}; use std::time::Duration; use thiserror::Error; +use vstaging::get_disabled_validators_with_fallback; pub use metered; pub use polkadot_node_network_protocol::MIN_GOSSIP_PEERS; @@ -462,19 +463,9 @@ impl Validator { let validators = validators?; // TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 - // When `DisabledValidators` is released remove this `if`` and add a - // `request_disabled_validators` call in the `try_join!` call above - let disabled_validators = if has_required_runtime( - sender, - parent, - RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, - ) - .await - { - request_disabled_validators(parent, sender).await.await?? - } else { - vec![] - }; + // When `DisabledValidators` is released remove this and add a + // `request_disabled_validators` call here + let disabled_validators = get_disabled_validators_with_fallback(sender, parent).await?; Self::construct(&validators, &disabled_validators, signing_context, keystore) } From 1a6648a5077e506ef0a673d2e556380f7e4abdf1 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Fri, 20 Oct 2023 15:16:00 +0300 Subject: [PATCH 28/28] Fix comments --- polkadot/node/subsystem-util/src/vstaging.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/subsystem-util/src/vstaging.rs b/polkadot/node/subsystem-util/src/vstaging.rs index b1ba20112dfa7..3efd3b61f93cd 100644 --- a/polkadot/node/subsystem-util/src/vstaging.rs +++ b/polkadot/node/subsystem-util/src/vstaging.rs @@ -27,11 +27,11 @@ use crate::{has_required_runtime, request_disabled_validators, Error}; const LOG_TARGET: &'static str = "parachain::subsystem-util-vstaging"; +// TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 /// Returns disabled validators list if the runtime supports it. Otherwise logs a debug messages and /// returns an empty vec. /// Once runtime ver `DISABLED_VALIDATORS_RUNTIME_REQUIREMENT` is released remove this function and /// replace all usages with `request_disabled_validators` -/// TODO: https://github.com/paritytech/polkadot-sdk/issues/1940 pub async fn get_disabled_validators_with_fallback>( sender: &mut Sender, relay_parent: Hash,