From 55a30ac81bf32a72e0b79ca2e7bb612344a5c43d Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 10:12:02 +0000 Subject: [PATCH 01/21] Check authority status on active leaves update Signed-off-by: Andrei Sandu --- node/core/chain-selection/src/lib.rs | 67 +++++++++++++++++++++++----- node/service/src/overseer.rs | 6 ++- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/node/core/chain-selection/src/lib.rs b/node/core/chain-selection/src/lib.rs index ee968496da1e..eb4c1d21e446 100644 --- a/node/core/chain-selection/src/lib.rs +++ b/node/core/chain-selection/src/lib.rs @@ -22,7 +22,9 @@ use polkadot_node_subsystem::{ messages::{ChainApiMessage, ChainSelectionMessage}, overseer, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError, }; +use polkadot_node_subsystem_util::Validator; use polkadot_primitives::v1::{BlockNumber, ConsensusLog, Hash, Header}; +use sp_keystore::SyncCryptoStorePtr; use futures::{channel::oneshot, future::Either, prelude::*}; use kvdb::KeyValueDB; @@ -307,13 +309,23 @@ pub struct Config { pub struct ChainSelectionSubsystem { config: Config, db: Arc, + keystore: SyncCryptoStorePtr, +} + +/// A structure to pass arguments to the subsystem main loop. +struct ChainSelectionSubsystemArgs<'a, Context, B> { + ctx: &'a mut Context, + backend: &'a mut B, + stagnant_check_interval: &'a StagnantCheckInterval, + clock: &'a (dyn Clock + Sync), + keystore: SyncCryptoStorePtr, } impl ChainSelectionSubsystem { /// Create a new instance of the subsystem with the given config /// and key-value store. - pub fn new(config: Config, db: Arc) -> Self { - ChainSelectionSubsystem { config, db } + pub fn new(config: Config, db: Arc, keystore: SyncCryptoStorePtr) -> Self { + ChainSelectionSubsystem { config, db, keystore } } } @@ -329,9 +341,15 @@ where ); SpawnedSubsystem { - future: run(ctx, backend, self.config.stagnant_check_interval, Box::new(SystemClock)) - .map(Ok) - .boxed(), + future: run( + ctx, + backend, + self.config.stagnant_check_interval, + Box::new(SystemClock), + self.keystore, + ) + .map(Ok) + .boxed(), name: "chain-selection-subsystem", } } @@ -342,13 +360,21 @@ async fn run( mut backend: B, stagnant_check_interval: StagnantCheckInterval, clock: Box, + keystore: SyncCryptoStorePtr, ) where Context: SubsystemContext, Context: overseer::SubsystemContext, B: Backend, { loop { - let res = run_until_error(&mut ctx, &mut backend, &stagnant_check_interval, &*clock).await; + let res = run_until_error(ChainSelectionSubsystemArgs { + ctx: &mut ctx, + backend: &mut backend, + stagnant_check_interval: &stagnant_check_interval, + clock: &*clock, + keystore, + }) + .await; match res { Err(e) => { e.trace(); @@ -368,18 +394,20 @@ async fn run( // // A return value of `Ok` indicates that an exit should be made, while non-fatal errors // lead to another call to this function. -async fn run_until_error( - ctx: &mut Context, - backend: &mut B, - stagnant_check_interval: &StagnantCheckInterval, - clock: &(dyn Clock + Sync), +async fn run_until_error<'a, Context, B>( + args: ChainSelectionSubsystemArgs<'a, Context, B>, ) -> Result<(), Error> where Context: SubsystemContext, Context: overseer::SubsystemContext, B: Backend, { + let ChainSelectionSubsystemArgs { ctx, backend, stagnant_check_interval, clock, keystore } = + args; + let mut stagnant_check_stream = stagnant_check_interval.timeout_stream(); + let mut is_validator = false; + let mut sender = ctx.sender().clone(); loop { futures::select! { msg = ctx.recv().fuse() => { @@ -398,6 +426,23 @@ where ).await?; backend.write(write_ops)?; + + if is_validator != Validator::new(leaf.hash, keystore.clone(), &mut sender).await.is_ok() { + is_validator = !is_validator; + tracing::info!( + "👮 Authority status changed to `{}` at block #{}({})", + is_validator, + leaf.number, + leaf.hash + ); + } else { + tracing::info!( + "🚔 Authority status is `{}` at block #{}({})", + is_validator, + leaf.number, + leaf.hash + ); + } } } FromOverseer::Signal(OverseerSignal::BlockFinalized(h, n)) => { diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index bbdc7692fc52..2d654e19ce54 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -281,7 +281,11 @@ where authority_discovery_service.clone(), Metrics::register(registry)?, )) - .chain_selection(ChainSelectionSubsystem::new(chain_selection_config, parachains_db)) + .chain_selection(ChainSelectionSubsystem::new( + chain_selection_config, + parachains_db, + keystore.clone(), + )) .leaves(Vec::from_iter( leaves .into_iter() From ffea18fe3ca12b27e6471ad9f44592799ec90956 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 10:12:28 +0000 Subject: [PATCH 02/21] cargo changes Signed-off-by: Andrei Sandu --- Cargo.lock | 1 + node/core/chain-selection/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 86e93fda792c..b4a0662695ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6386,6 +6386,7 @@ dependencies = [ "polkadot-node-subsystem-util", "polkadot-primitives", "sp-core", + "sp-keystore", "thiserror", "tracing", ] diff --git a/node/core/chain-selection/Cargo.toml b/node/core/chain-selection/Cargo.toml index 814f732dae61..ea963552a6ca 100644 --- a/node/core/chain-selection/Cargo.toml +++ b/node/core/chain-selection/Cargo.toml @@ -16,6 +16,7 @@ polkadot-node-subsystem-util = { path = "../../subsystem-util" } kvdb = "0.10.0" thiserror = "1.0.30" parity-scale-codec = "2" +sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" } [dev-dependencies] polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } From 5bd56bb367ec0b01ecca04498b3974ab67bf3189 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 10:49:15 +0000 Subject: [PATCH 03/21] Fix tests Signed-off-by: Andrei Sandu --- Cargo.lock | 1 + node/core/chain-selection/Cargo.toml | 1 + node/core/chain-selection/src/tests.rs | 3 +++ 3 files changed, 5 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index b4a0662695ed..550265cd2a11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6385,6 +6385,7 @@ dependencies = [ "polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-util", "polkadot-primitives", + "sc-keystore", "sp-core", "sp-keystore", "thiserror", diff --git a/node/core/chain-selection/Cargo.toml b/node/core/chain-selection/Cargo.toml index ea963552a6ca..189cd187f45f 100644 --- a/node/core/chain-selection/Cargo.toml +++ b/node/core/chain-selection/Cargo.toml @@ -24,3 +24,4 @@ sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } parking_lot = "0.11" assert_matches = "1" kvdb-memorydb = "0.10.0" +sc-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/chain-selection/src/tests.rs b/node/core/chain-selection/src/tests.rs index 396e11520733..fc22366bb5b3 100644 --- a/node/core/chain-selection/src/tests.rs +++ b/node/core/chain-selection/src/tests.rs @@ -33,6 +33,7 @@ use assert_matches::assert_matches; use futures::channel::oneshot; use parity_scale_codec::Encode; use parking_lot::Mutex; +use sc_keystore::LocalKeystore; use sp_core::testing::TaskExecutor; use polkadot_node_subsystem::{ @@ -234,6 +235,7 @@ fn test_harness>( ) { let pool = TaskExecutor::new(); let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool); + let keystore = Arc::new(LocalKeystore::in_memory()) as SyncCryptoStorePtr; let backend = TestBackend::default(); let clock = TestClock::new(0); @@ -242,6 +244,7 @@ fn test_harness>( backend.clone(), StagnantCheckInterval::new(TEST_STAGNANT_INTERVAL), Box::new(clock.clone()), + keystore, ); let test_fut = test(backend, clock, virtual_overseer); From 1234ab1326a05f799d921f763d6761a3d13da6c9 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 14:33:53 +0000 Subject: [PATCH 04/21] Add metric for authority status Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 22 +++++++- node/network/gossip-support/src/metrics.rs | 61 ++++++++++++++++++++++ node/service/src/overseer.rs | 1 + 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 node/network/gossip-support/src/metrics.rs diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index c01c816dafcd..3a4629099df4 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -57,6 +57,10 @@ use polkadot_primitives::v1::{AuthorityDiscoveryId, Hash, SessionIndex}; #[cfg(test)] mod tests; +mod metrics; + +use metrics::Metrics; + const LOG_TARGET: &str = "parachain::gossip-support"; // How much time should we wait to reissue a connection request // since the last authority discovery resolution failure. @@ -104,6 +108,9 @@ pub struct GossipSupport { connected_authorities_by_peer_id: HashMap>, /// Authority discovery service. authority_discovery: AD, + + /// Subsystem metrics. + metrics: Metrics, } impl GossipSupport @@ -111,7 +118,10 @@ where AD: AuthorityDiscovery, { /// Create a new instance of the [`GossipSupport`] subsystem. - pub fn new(keystore: SyncCryptoStorePtr, authority_discovery: AD) -> Self { + pub fn new(keystore: SyncCryptoStorePtr, authority_discovery: AD, metrics: Metrics) -> Self { + // Initialize the `polkadot_node_is_authority` metric. + metrics.on_is_not_authority(); + Self { keystore, last_session_index: None, @@ -121,6 +131,7 @@ where connected_authorities: HashMap::new(), connected_authorities_by_peer_id: HashMap::new(), authority_discovery, + metrics, } } @@ -212,7 +223,14 @@ where } let all_authorities = determine_relevant_authorities(ctx, relay_parent).await?; - let our_index = ensure_i_am_an_authority(&self.keystore, &all_authorities).await?; + let our_index = ensure_i_am_an_authority(&self.keystore, &all_authorities) + .await + .map_err(|e| { + self.metrics.on_is_not_authority(); + e + })?; + self.metrics.on_is_authority(); + let other_authorities = { let mut authorities = all_authorities.clone(); authorities.swap_remove(our_index); diff --git a/node/network/gossip-support/src/metrics.rs b/node/network/gossip-support/src/metrics.rs new file mode 100644 index 000000000000..8442db960175 --- /dev/null +++ b/node/network/gossip-support/src/metrics.rs @@ -0,0 +1,61 @@ +// Copyright 2021 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 . + +use polkadot_node_subsystem_util::{ + metrics, + metrics::{ + prometheus, + prometheus::{Gauge, PrometheusError, Registry, U64}, + }, +}; + +/// Dispute Distribution metrics. +#[derive(Clone, Default)] +pub struct Metrics(Option); + +#[derive(Clone)] +struct MetricsInner { + /// Tracks authority status. + is_authority: Gauge, +} + +impl Metrics { + /// Set the authority flag. + pub fn on_is_authority(&self) { + if let Some(metrics) = &self.0 { + metrics.is_authority.set(1); + } + } + + /// Unset the authority flag. + pub fn on_is_not_authority(&self) { + if let Some(metrics) = &self.0 { + metrics.is_authority.set(0); + } + } +} + +impl metrics::Metrics for Metrics { + fn try_register(registry: &Registry) -> Result { + let metrics = MetricsInner { + is_authority: prometheus::register( + Gauge::new("polkadot_node_is_authority", "Tracks the node authority status.")?, + registry, + )?, + }; + Ok(Metrics(Some(metrics))) + } +} diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index 2d654e19ce54..6d56f64b9def 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -264,6 +264,7 @@ where .gossip_support(GossipSupportSubsystem::new( keystore.clone(), authority_discovery_service.clone(), + Metrics::register(registry)?, )) .dispute_coordinator(if disputes_enabled { DisputeCoordinatorSubsystem::new( From a5d3b6ea966fdada056acafd4a82d0b3dabce20a Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 14:34:44 +0000 Subject: [PATCH 05/21] Revert "Fix tests" This reverts commit 5bd56bb367ec0b01ecca04498b3974ab67bf3189. --- Cargo.lock | 1 - node/core/chain-selection/Cargo.toml | 1 - node/core/chain-selection/src/tests.rs | 3 --- 3 files changed, 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 550265cd2a11..b4a0662695ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6385,7 +6385,6 @@ dependencies = [ "polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-util", "polkadot-primitives", - "sc-keystore", "sp-core", "sp-keystore", "thiserror", diff --git a/node/core/chain-selection/Cargo.toml b/node/core/chain-selection/Cargo.toml index 189cd187f45f..ea963552a6ca 100644 --- a/node/core/chain-selection/Cargo.toml +++ b/node/core/chain-selection/Cargo.toml @@ -24,4 +24,3 @@ sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } parking_lot = "0.11" assert_matches = "1" kvdb-memorydb = "0.10.0" -sc-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/chain-selection/src/tests.rs b/node/core/chain-selection/src/tests.rs index fc22366bb5b3..396e11520733 100644 --- a/node/core/chain-selection/src/tests.rs +++ b/node/core/chain-selection/src/tests.rs @@ -33,7 +33,6 @@ use assert_matches::assert_matches; use futures::channel::oneshot; use parity_scale_codec::Encode; use parking_lot::Mutex; -use sc_keystore::LocalKeystore; use sp_core::testing::TaskExecutor; use polkadot_node_subsystem::{ @@ -235,7 +234,6 @@ fn test_harness>( ) { let pool = TaskExecutor::new(); let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool); - let keystore = Arc::new(LocalKeystore::in_memory()) as SyncCryptoStorePtr; let backend = TestBackend::default(); let clock = TestClock::new(0); @@ -244,7 +242,6 @@ fn test_harness>( backend.clone(), StagnantCheckInterval::new(TEST_STAGNANT_INTERVAL), Box::new(clock.clone()), - keystore, ); let test_fut = test(backend, clock, virtual_overseer); From cf824f3625a4ae327d8efa20e1ec4f70e2e43122 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 14:35:03 +0000 Subject: [PATCH 06/21] Revert "cargo changes" This reverts commit ffea18fe3ca12b27e6471ad9f44592799ec90956. --- Cargo.lock | 1 - node/core/chain-selection/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b4a0662695ed..86e93fda792c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6386,7 +6386,6 @@ dependencies = [ "polkadot-node-subsystem-util", "polkadot-primitives", "sp-core", - "sp-keystore", "thiserror", "tracing", ] diff --git a/node/core/chain-selection/Cargo.toml b/node/core/chain-selection/Cargo.toml index ea963552a6ca..814f732dae61 100644 --- a/node/core/chain-selection/Cargo.toml +++ b/node/core/chain-selection/Cargo.toml @@ -16,7 +16,6 @@ polkadot-node-subsystem-util = { path = "../../subsystem-util" } kvdb = "0.10.0" thiserror = "1.0.30" parity-scale-codec = "2" -sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" } [dev-dependencies] polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } From 625cc2f0c78a556e16429285198bd99889efe578 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 14:35:14 +0000 Subject: [PATCH 07/21] Revert "Check authority status on active leaves update" This reverts commit 55a30ac81bf32a72e0b79ca2e7bb612344a5c43d. --- node/core/chain-selection/src/lib.rs | 67 +++++----------------------- node/service/src/overseer.rs | 6 +-- 2 files changed, 12 insertions(+), 61 deletions(-) diff --git a/node/core/chain-selection/src/lib.rs b/node/core/chain-selection/src/lib.rs index eb4c1d21e446..ee968496da1e 100644 --- a/node/core/chain-selection/src/lib.rs +++ b/node/core/chain-selection/src/lib.rs @@ -22,9 +22,7 @@ use polkadot_node_subsystem::{ messages::{ChainApiMessage, ChainSelectionMessage}, overseer, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError, }; -use polkadot_node_subsystem_util::Validator; use polkadot_primitives::v1::{BlockNumber, ConsensusLog, Hash, Header}; -use sp_keystore::SyncCryptoStorePtr; use futures::{channel::oneshot, future::Either, prelude::*}; use kvdb::KeyValueDB; @@ -309,23 +307,13 @@ pub struct Config { pub struct ChainSelectionSubsystem { config: Config, db: Arc, - keystore: SyncCryptoStorePtr, -} - -/// A structure to pass arguments to the subsystem main loop. -struct ChainSelectionSubsystemArgs<'a, Context, B> { - ctx: &'a mut Context, - backend: &'a mut B, - stagnant_check_interval: &'a StagnantCheckInterval, - clock: &'a (dyn Clock + Sync), - keystore: SyncCryptoStorePtr, } impl ChainSelectionSubsystem { /// Create a new instance of the subsystem with the given config /// and key-value store. - pub fn new(config: Config, db: Arc, keystore: SyncCryptoStorePtr) -> Self { - ChainSelectionSubsystem { config, db, keystore } + pub fn new(config: Config, db: Arc) -> Self { + ChainSelectionSubsystem { config, db } } } @@ -341,15 +329,9 @@ where ); SpawnedSubsystem { - future: run( - ctx, - backend, - self.config.stagnant_check_interval, - Box::new(SystemClock), - self.keystore, - ) - .map(Ok) - .boxed(), + future: run(ctx, backend, self.config.stagnant_check_interval, Box::new(SystemClock)) + .map(Ok) + .boxed(), name: "chain-selection-subsystem", } } @@ -360,21 +342,13 @@ async fn run( mut backend: B, stagnant_check_interval: StagnantCheckInterval, clock: Box, - keystore: SyncCryptoStorePtr, ) where Context: SubsystemContext, Context: overseer::SubsystemContext, B: Backend, { loop { - let res = run_until_error(ChainSelectionSubsystemArgs { - ctx: &mut ctx, - backend: &mut backend, - stagnant_check_interval: &stagnant_check_interval, - clock: &*clock, - keystore, - }) - .await; + let res = run_until_error(&mut ctx, &mut backend, &stagnant_check_interval, &*clock).await; match res { Err(e) => { e.trace(); @@ -394,20 +368,18 @@ async fn run( // // A return value of `Ok` indicates that an exit should be made, while non-fatal errors // lead to another call to this function. -async fn run_until_error<'a, Context, B>( - args: ChainSelectionSubsystemArgs<'a, Context, B>, +async fn run_until_error( + ctx: &mut Context, + backend: &mut B, + stagnant_check_interval: &StagnantCheckInterval, + clock: &(dyn Clock + Sync), ) -> Result<(), Error> where Context: SubsystemContext, Context: overseer::SubsystemContext, B: Backend, { - let ChainSelectionSubsystemArgs { ctx, backend, stagnant_check_interval, clock, keystore } = - args; - let mut stagnant_check_stream = stagnant_check_interval.timeout_stream(); - let mut is_validator = false; - let mut sender = ctx.sender().clone(); loop { futures::select! { msg = ctx.recv().fuse() => { @@ -426,23 +398,6 @@ where ).await?; backend.write(write_ops)?; - - if is_validator != Validator::new(leaf.hash, keystore.clone(), &mut sender).await.is_ok() { - is_validator = !is_validator; - tracing::info!( - "👮 Authority status changed to `{}` at block #{}({})", - is_validator, - leaf.number, - leaf.hash - ); - } else { - tracing::info!( - "🚔 Authority status is `{}` at block #{}({})", - is_validator, - leaf.number, - leaf.hash - ); - } } } FromOverseer::Signal(OverseerSignal::BlockFinalized(h, n)) => { diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index 6d56f64b9def..58be883fa199 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -282,11 +282,7 @@ where authority_discovery_service.clone(), Metrics::register(registry)?, )) - .chain_selection(ChainSelectionSubsystem::new( - chain_selection_config, - parachains_db, - keystore.clone(), - )) + .chain_selection(ChainSelectionSubsystem::new(chain_selection_config, parachains_db)) .leaves(Vec::from_iter( leaves .into_iter() From d8304fdbd230dabaa0547395112358d819971639 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 15:09:52 +0000 Subject: [PATCH 08/21] Test fixups Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/metrics.rs | 6 ++++++ node/network/gossip-support/src/tests.rs | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/node/network/gossip-support/src/metrics.rs b/node/network/gossip-support/src/metrics.rs index 8442db960175..72601985795f 100644 --- a/node/network/gossip-support/src/metrics.rs +++ b/node/network/gossip-support/src/metrics.rs @@ -33,6 +33,12 @@ struct MetricsInner { } impl Metrics { + #[cfg(test)] + /// Dummy constructor for testing. + pub fn new_dummy() -> Self { + Self(None) + } + /// Set the authority flag. pub fn on_is_authority(&self) { if let Some(metrics) = &self.0 { diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index a3267a1daa11..0ebd729bbda3 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -126,7 +126,11 @@ async fn get_other_authorities_addrs_map() -> HashMap GossipSupport { - GossipSupport::new(make_ferdie_keystore(), MOCK_AUTHORITY_DISCOVERY.clone()) + GossipSupport::new( + make_ferdie_keystore(), + MOCK_AUTHORITY_DISCOVERY.clone(), + Metrics::new_dummy(), + ) } fn test_harness, AD: AuthorityDiscovery>( From a44d8bf6b6333d59c9b3bc45d9e1a1790fda4cbd Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 15:27:00 +0000 Subject: [PATCH 09/21] fix Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/network/gossip-support/src/metrics.rs b/node/network/gossip-support/src/metrics.rs index 72601985795f..97924fdad994 100644 --- a/node/network/gossip-support/src/metrics.rs +++ b/node/network/gossip-support/src/metrics.rs @@ -33,8 +33,8 @@ struct MetricsInner { } impl Metrics { - #[cfg(test)] /// Dummy constructor for testing. + #[cfg(test)] pub fn new_dummy() -> Self { Self(None) } From 404bae80ed1688d6a2113d877415a6ea5141ddaa Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 20:05:17 +0000 Subject: [PATCH 10/21] update Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index 3a4629099df4..862b914a4f63 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -220,16 +220,16 @@ where "New session detected", ); self.last_session_index = Some(session_index); + + // Update the authority status metric on every new session. + match util::Validator::new(leaf, self.keystore.clone(), ctx.sender()).await { + Ok(_) => self.metrics.on_is_authority(), + Err(_) => self.metrics.on_is_not_authority(), + } } let all_authorities = determine_relevant_authorities(ctx, relay_parent).await?; - let our_index = ensure_i_am_an_authority(&self.keystore, &all_authorities) - .await - .map_err(|e| { - self.metrics.on_is_not_authority(); - e - })?; - self.metrics.on_is_authority(); + let our_index = ensure_i_am_an_authority(&self.keystore, &all_authorities).await?; let other_authorities = { let mut authorities = all_authorities.clone(); From d30205f40fa3cc58e194ced7245a1439f852275d Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Jan 2022 20:07:54 +0000 Subject: [PATCH 11/21] undo damage Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index 862b914a4f63..de6e4afd5a28 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -230,7 +230,6 @@ where let all_authorities = determine_relevant_authorities(ctx, relay_parent).await?; let our_index = ensure_i_am_an_authority(&self.keystore, &all_authorities).await?; - let other_authorities = { let mut authorities = all_authorities.clone(); authorities.swap_remove(our_index); From b0a3428e6d710237df9a9dbb606e813d8d45c760 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 09:29:13 +0000 Subject: [PATCH 12/21] dont update status on runtime errors Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index de6e4afd5a28..936e0ff9e7f6 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -224,7 +224,9 @@ where // Update the authority status metric on every new session. match util::Validator::new(leaf, self.keystore.clone(), ctx.sender()).await { Ok(_) => self.metrics.on_is_authority(), - Err(_) => self.metrics.on_is_not_authority(), + Err(util::Error::NotAValidator) => self.metrics.on_is_not_authority(), + // Don't update on runtime errors. + Err(_) => {}, } } From 65aa234619ee875045b2c2b83ae8c5230a9046e8 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 11:35:53 +0000 Subject: [PATCH 13/21] Fix tests Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 6 ++-- node/network/gossip-support/src/metrics.rs | 4 +-- node/network/gossip-support/src/tests.rs | 35 ++++++++++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index 936e0ff9e7f6..d6815dffa2ac 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -120,7 +120,7 @@ where /// Create a new instance of the [`GossipSupport`] subsystem. pub fn new(keystore: SyncCryptoStorePtr, authority_discovery: AD, metrics: Metrics) -> Self { // Initialize the `polkadot_node_is_authority` metric. - metrics.on_is_not_authority(); + metrics.on_role_is_not_authority(); Self { keystore, @@ -223,8 +223,8 @@ where // Update the authority status metric on every new session. match util::Validator::new(leaf, self.keystore.clone(), ctx.sender()).await { - Ok(_) => self.metrics.on_is_authority(), - Err(util::Error::NotAValidator) => self.metrics.on_is_not_authority(), + Ok(_) => self.metrics.on_role_is_authority(), + Err(util::Error::NotAValidator) => self.metrics.on_role_is_not_authority(), // Don't update on runtime errors. Err(_) => {}, } diff --git a/node/network/gossip-support/src/metrics.rs b/node/network/gossip-support/src/metrics.rs index 97924fdad994..907983fd99e6 100644 --- a/node/network/gossip-support/src/metrics.rs +++ b/node/network/gossip-support/src/metrics.rs @@ -40,14 +40,14 @@ impl Metrics { } /// Set the authority flag. - pub fn on_is_authority(&self) { + pub fn on_role_is_authority(&self) { if let Some(metrics) = &self.0 { metrics.is_authority.set(1); } } /// Unset the authority flag. - pub fn on_is_not_authority(&self) { + pub fn on_role_is_not_authority(&self) { if let Some(metrics) = &self.0 { metrics.is_authority.set(0); } diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index 0ebd729bbda3..b6cae89b50ea 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -218,6 +218,32 @@ async fn test_neighbors(overseer: &mut VirtualOverseer) { ); } +// Helper function to test the `Validator::new()` call from inside ActiveLeavesUpdate +// handler. +async fn check_validator_status(overseer: &mut VirtualOverseer, hash: Hash) { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + relay_parent, + RuntimeApiRequest::Validators(sender), + )) => { + assert_eq!(relay_parent, hash); + sender.send(Ok(Vec::new())).unwrap(); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + relay_parent, + RuntimeApiRequest::SessionIndexForChild(tx), + )) => { + assert_eq!(relay_parent, hash); + tx.send(Ok(2)).unwrap(); + } + ); +} + #[test] fn issues_a_connection_request_on_new_session() { let hash = Hash::repeat_byte(0xAA); @@ -234,6 +260,9 @@ fn issues_a_connection_request_on_new_session() { tx.send(Ok(1)).unwrap(); } ); + + check_validator_status(overseer, hash).await; + assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( @@ -300,6 +329,9 @@ fn issues_a_connection_request_on_new_session() { tx.send(Ok(2)).unwrap(); } ); + + check_validator_status(overseer, hash).await; + assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( @@ -382,6 +414,9 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { tx.send(Ok(1)).unwrap(); } ); + + check_validator_status(overseer, hash).await; + assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( From 8a29350969780213caf3239fbc03045fdf2930ee Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 11:40:42 +0000 Subject: [PATCH 14/21] fix inconsistency Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index b6cae89b50ea..0bb6ebd831e3 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -236,10 +236,10 @@ async fn check_validator_status(overseer: &mut VirtualOverseer, hash: Hash) { overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( relay_parent, - RuntimeApiRequest::SessionIndexForChild(tx), + RuntimeApiRequest::SessionIndexForChild(sender), )) => { assert_eq!(relay_parent, hash); - tx.send(Ok(2)).unwrap(); + sender.send(Ok(2)).unwrap(); } ); } From d05bfc921564a010e69444cbef71cf72d729a077 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 15:49:04 +0000 Subject: [PATCH 15/21] Review feedback Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 64 +++++++++++++++++---- node/network/gossip-support/src/metrics.rs | 35 ++++++++++-- node/network/gossip-support/src/tests.rs | 65 +++++++++++----------- primitives/src/v2/mod.rs | 3 +- 4 files changed, 117 insertions(+), 50 deletions(-) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index d6815dffa2ac..e035353f8f82 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -49,7 +49,7 @@ use polkadot_node_subsystem::{ RuntimeApiRequest, }, overseer, ActiveLeavesUpdate, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, - SubsystemError, + SubsystemError, SubsystemSender, }; use polkadot_node_subsystem_util as util; use polkadot_primitives::v1::{AuthorityDiscoveryId, Hash, SessionIndex}; @@ -119,8 +119,9 @@ where { /// Create a new instance of the [`GossipSupport`] subsystem. pub fn new(keystore: SyncCryptoStorePtr, authority_discovery: AD, metrics: Metrics) -> Self { - // Initialize the `polkadot_node_is_authority` metric. - metrics.on_role_is_not_authority(); + // Initialize metrics to `0`. + metrics.on_is_not_authority(); + metrics.on_is_not_parachain_validator(); Self { keystore, @@ -220,14 +221,6 @@ where "New session detected", ); self.last_session_index = Some(session_index); - - // Update the authority status metric on every new session. - match util::Validator::new(leaf, self.keystore.clone(), ctx.sender()).await { - Ok(_) => self.metrics.on_role_is_authority(), - Err(util::Error::NotAValidator) => self.metrics.on_role_is_not_authority(), - // Don't update on runtime errors. - Err(_) => {}, - } } let all_authorities = determine_relevant_authorities(ctx, relay_parent).await?; @@ -242,10 +235,59 @@ where if is_new_session { update_gossip_topology(ctx, our_index, all_authorities, relay_parent).await?; + self.update_authority_status_metrics(leaf, ctx.sender()).await?; } } } + Ok(()) + } + async fn update_authority_status_metrics( + &mut self, + leaf: Hash, + sender: &mut impl SubsystemSender, + ) -> Result<(), util::Error> { + if let Some(session_info) = util::request_session_info( + leaf, + self.last_session_index + .expect("Last session index is always set on every session index change"), + sender, + ) + .await + .await?? + { + let maybe_index = match ensure_i_am_an_authority( + &self.keystore, + &session_info.discovery_keys, + ) + .await + { + Ok(index) => { + self.metrics.on_is_authority(); + Some(index) + }, + Err(util::Error::NotAValidator) => { + self.metrics.on_is_not_authority(); + None + }, + // Don't update on runtime errors. + Err(_) => None, + }; + + if let Some(validator_index) = maybe_index { + // The subset of authorities participating in parachain consensus. + let parachain_validators_this_session = session_info.validators; + + // First `maxValidators` entries are the parachain validators. We'll check + // if our index is in this set to avoid searching for the keys. + // https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148 + if validator_index < parachain_validators_this_session.len() { + self.metrics.on_is_parachain_validator(); + } else { + self.metrics.on_is_not_parachain_validator(); + } + } + } Ok(()) } diff --git a/node/network/gossip-support/src/metrics.rs b/node/network/gossip-support/src/metrics.rs index 907983fd99e6..67aa258921d8 100644 --- a/node/network/gossip-support/src/metrics.rs +++ b/node/network/gossip-support/src/metrics.rs @@ -28,8 +28,10 @@ pub struct Metrics(Option); #[derive(Clone)] struct MetricsInner { - /// Tracks authority status. + /// Tracks authority status for producing relay chain blocks. is_authority: Gauge, + /// Tracks authority status for parachain approval checking. + is_parachain_validator: Gauge, } impl Metrics { @@ -39,26 +41,47 @@ impl Metrics { Self(None) } - /// Set the authority flag. - pub fn on_role_is_authority(&self) { + /// Set the `relaychain validator` metric. + pub fn on_is_authority(&self) { if let Some(metrics) = &self.0 { metrics.is_authority.set(1); } } - /// Unset the authority flag. - pub fn on_role_is_not_authority(&self) { + /// Unset the `relaychain validator` metric. + pub fn on_is_not_authority(&self) { if let Some(metrics) = &self.0 { metrics.is_authority.set(0); } } + + /// Set the `parachain validator` metric. + pub fn on_is_parachain_validator(&self) { + if let Some(metrics) = &self.0 { + metrics.is_parachain_validator.set(1); + } + } + + /// Unset the `parachain validator` metric. + pub fn on_is_not_parachain_validator(&self) { + if let Some(metrics) = &self.0 { + metrics.is_parachain_validator.set(0); + } + } } impl metrics::Metrics for Metrics { fn try_register(registry: &Registry) -> Result { let metrics = MetricsInner { is_authority: prometheus::register( - Gauge::new("polkadot_node_is_authority", "Tracks the node authority status.")?, + Gauge::new("polkadot_node_is_authority", "Tracks the node authority status across sessions. \ + An authority is any node that is a potential block producer in a session.")?, + registry, + )?, + is_parachain_validator: prometheus::register( + Gauge::new("polkadot_node_is_parachain_validator", + "Tracks the node parachain validator status across sessions. Parachain validators are a \ + subset of authorities that perform approval checking of all parachain candidates in a session.")?, registry, )?, }; diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index 0bb6ebd831e3..d2c3b888e7a0 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -218,32 +218,6 @@ async fn test_neighbors(overseer: &mut VirtualOverseer) { ); } -// Helper function to test the `Validator::new()` call from inside ActiveLeavesUpdate -// handler. -async fn check_validator_status(overseer: &mut VirtualOverseer, hash: Hash) { - assert_matches!( - overseer_recv(overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - relay_parent, - RuntimeApiRequest::Validators(sender), - )) => { - assert_eq!(relay_parent, hash); - sender.send(Ok(Vec::new())).unwrap(); - } - ); - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - relay_parent, - RuntimeApiRequest::SessionIndexForChild(sender), - )) => { - assert_eq!(relay_parent, hash); - sender.send(Ok(2)).unwrap(); - } - ); -} - #[test] fn issues_a_connection_request_on_new_session() { let hash = Hash::repeat_byte(0xAA); @@ -261,8 +235,6 @@ fn issues_a_connection_request_on_new_session() { } ); - check_validator_status(overseer, hash).await; - assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( @@ -287,6 +259,17 @@ fn issues_a_connection_request_on_new_session() { test_neighbors(overseer).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + relay_parent, + RuntimeApiRequest::SessionInfo(1, sender), + )) => { + assert_eq!(relay_parent, hash); + sender.send(Ok(Some(polkadot_primitives::v2::SessionInfo::default()))).unwrap(); + } + ); + virtual_overseer }); @@ -330,8 +313,6 @@ fn issues_a_connection_request_on_new_session() { } ); - check_validator_status(overseer, hash).await; - assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( @@ -356,6 +337,17 @@ fn issues_a_connection_request_on_new_session() { test_neighbors(overseer).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + relay_parent, + RuntimeApiRequest::SessionInfo(2, sender), + )) => { + assert_eq!(relay_parent, hash); + sender.send(Ok(Some(polkadot_primitives::v2::SessionInfo::default()))).unwrap(); + } + ); + virtual_overseer }); assert_eq!(state.last_session_index, Some(2)); @@ -415,8 +407,6 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { } ); - check_validator_status(overseer, hash).await; - assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( @@ -445,6 +435,17 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { test_neighbors(overseer).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + relay_parent, + RuntimeApiRequest::SessionInfo(1, sender), + )) => { + assert_eq!(relay_parent, hash); + sender.send(Ok(Some(polkadot_primitives::v2::SessionInfo::default()))).unwrap(); + } + ); + virtual_overseer }) }; diff --git a/primitives/src/v2/mod.rs b/primitives/src/v2/mod.rs index 065d5cc3c057..e07b355feb9c 100644 --- a/primitives/src/v2/mod.rs +++ b/primitives/src/v2/mod.rs @@ -28,7 +28,8 @@ use parity_util_mem::MallocSizeOf; /// Information about validator sets of a session. #[derive(Clone, Encode, Decode, RuntimeDebug, TypeInfo)] -#[cfg_attr(feature = "std", derive(PartialEq, MallocSizeOf))] +#[cfg_attr(feature = "std", derive(PartialEq, MallocSizeOf, Default))] +#[cfg_attr(all(feature = "std", test), derive(Default))] pub struct SessionInfo { /****** New in v2 *******/ /// All the validators actively participating in parachain consensus. From 2d75603bee1be0e276802a8fc6dd2c3f42032c70 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 16:06:39 +0000 Subject: [PATCH 16/21] Dont derive primitive Default Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/tests.rs | 22 +++++++++++++++++++--- primitives/src/v2/mod.rs | 3 +-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index d2c3b888e7a0..b34fa2106e20 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -58,6 +58,22 @@ lazy_static! { Sr25519Keyring::Charlie.public().into(), Sr25519Keyring::Eve.public().into(), ]; + static ref SESSION_INFO: polkadot_primitives::v2::SessionInfo = + polkadot_primitives::v2::SessionInfo { + active_validator_indices: Vec::new(), + random_seed: [0u8; 32], + dispute_period: 10, + validators: Vec::new(), + discovery_keys: Vec::new(), + assignment_keys: Vec::new(), + validator_groups: Vec::new(), + n_cores: 1, + zeroth_delay_tranche_width: 0, + relay_vrf_modulo_samples: 0, + n_delay_tranches: 0, + no_show_slots: 0, + needed_approvals: 0, + }; } type VirtualOverseer = test_helpers::TestSubsystemContextHandle; @@ -266,7 +282,7 @@ fn issues_a_connection_request_on_new_session() { RuntimeApiRequest::SessionInfo(1, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(polkadot_primitives::v2::SessionInfo::default()))).unwrap(); + sender.send(Ok(Some(SESSION_INFO.clone()))).unwrap(); } ); @@ -344,7 +360,7 @@ fn issues_a_connection_request_on_new_session() { RuntimeApiRequest::SessionInfo(2, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(polkadot_primitives::v2::SessionInfo::default()))).unwrap(); + sender.send(Ok(Some(SESSION_INFO.clone()))).unwrap(); } ); @@ -442,7 +458,7 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { RuntimeApiRequest::SessionInfo(1, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(polkadot_primitives::v2::SessionInfo::default()))).unwrap(); + sender.send(Ok(Some(SESSION_INFO.clone()))).unwrap(); } ); diff --git a/primitives/src/v2/mod.rs b/primitives/src/v2/mod.rs index e07b355feb9c..065d5cc3c057 100644 --- a/primitives/src/v2/mod.rs +++ b/primitives/src/v2/mod.rs @@ -28,8 +28,7 @@ use parity_util_mem::MallocSizeOf; /// Information about validator sets of a session. #[derive(Clone, Encode, Decode, RuntimeDebug, TypeInfo)] -#[cfg_attr(feature = "std", derive(PartialEq, MallocSizeOf, Default))] -#[cfg_attr(all(feature = "std", test), derive(Default))] +#[cfg_attr(feature = "std", derive(PartialEq, MallocSizeOf))] pub struct SessionInfo { /****** New in v2 *******/ /// All the validators actively participating in parachain consensus. From cac69c609c5dccb6601c449437ebb791196d800d Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 16:15:58 +0000 Subject: [PATCH 17/21] add dummy_session_info helper Signed-off-by: Andrei Sandu --- Cargo.lock | 1 + node/network/gossip-support/Cargo.toml | 1 + node/network/gossip-support/src/tests.rs | 23 ++++------------------- primitives/test-helpers/src/lib.rs | 21 +++++++++++++++++++++ 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 86e93fda792c..3b8cb761317e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6167,6 +6167,7 @@ dependencies = [ "polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-util", "polkadot-primitives", + "polkadot-primitives-test-helpers", "rand 0.8.4", "rand_chacha 0.3.1", "sc-network", diff --git a/node/network/gossip-support/Cargo.toml b/node/network/gossip-support/Cargo.toml index e9f41edca58b..3179a29ae15e 100644 --- a/node/network/gossip-support/Cargo.toml +++ b/node/network/gossip-support/Cargo.toml @@ -31,3 +31,4 @@ polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } assert_matches = "1.4.0" async-trait = "0.1.52" lazy_static = "1.4.0" +polkadot-primitives-test-helpers = { path = "../../../primitives/test-helpers" } \ No newline at end of file diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index b34fa2106e20..d97c8edf3ad9 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -34,6 +34,7 @@ use polkadot_node_subsystem::{ }; use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_node_subsystem_util::TimeoutExt as _; +use polkadot_primitives_test_helpers::dummy_session_info; use test_helpers::mock::make_ferdie_keystore; use super::*; @@ -58,22 +59,6 @@ lazy_static! { Sr25519Keyring::Charlie.public().into(), Sr25519Keyring::Eve.public().into(), ]; - static ref SESSION_INFO: polkadot_primitives::v2::SessionInfo = - polkadot_primitives::v2::SessionInfo { - active_validator_indices: Vec::new(), - random_seed: [0u8; 32], - dispute_period: 10, - validators: Vec::new(), - discovery_keys: Vec::new(), - assignment_keys: Vec::new(), - validator_groups: Vec::new(), - n_cores: 1, - zeroth_delay_tranche_width: 0, - relay_vrf_modulo_samples: 0, - n_delay_tranches: 0, - no_show_slots: 0, - needed_approvals: 0, - }; } type VirtualOverseer = test_helpers::TestSubsystemContextHandle; @@ -282,7 +267,7 @@ fn issues_a_connection_request_on_new_session() { RuntimeApiRequest::SessionInfo(1, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(SESSION_INFO.clone()))).unwrap(); + sender.send(Ok(Some(dummy_session_info()))).unwrap(); } ); @@ -360,7 +345,7 @@ fn issues_a_connection_request_on_new_session() { RuntimeApiRequest::SessionInfo(2, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(SESSION_INFO.clone()))).unwrap(); + sender.send(Ok(Some(dummy_session_info()))).unwrap(); } ); @@ -458,7 +443,7 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { RuntimeApiRequest::SessionInfo(1, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(SESSION_INFO.clone()))).unwrap(); + sender.send(Ok(Some(dummy_session_info()))).unwrap(); } ); diff --git a/primitives/test-helpers/src/lib.rs b/primitives/test-helpers/src/lib.rs index ccf103d79e58..a60505c36948 100644 --- a/primitives/test-helpers/src/lib.rs +++ b/primitives/test-helpers/src/lib.rs @@ -26,6 +26,9 @@ use polkadot_primitives::v1::{ CommittedCandidateReceipt, Hash, HeadData, Id as ParaId, ValidationCode, ValidationCodeHash, ValidatorId, }; + +use polkadot_primitives::v2::SessionInfo; + use sp_application_crypto::sr25519; use sp_keyring::Sr25519Keyring; use sp_runtime::generic::Digest; @@ -224,3 +227,21 @@ impl TestCandidateBuilder { CandidateReceipt { descriptor, commitments_hash: self.commitments_hash } } } + +pub fn dummy_session_info() -> SessionInfo { + SessionInfo { + active_validator_indices: Vec::new(), + random_seed: [0u8; 32], + dispute_period: 10, + validators: Vec::new(), + discovery_keys: Vec::new(), + assignment_keys: Vec::new(), + validator_groups: Vec::new(), + n_cores: 0, + zeroth_delay_tranche_width: 0, + relay_vrf_modulo_samples: 0, + n_delay_tranches: 0, + no_show_slots: 0, + needed_approvals: 0, + } +} From a0930ec51be04ddd14885de1d023414004fe2808 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 16:35:45 +0000 Subject: [PATCH 18/21] unset parachain validator status if no longer authority Signed-off-by: Andrei Sandu --- node/network/gossip-support/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index e035353f8f82..a12fbf82d236 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -268,6 +268,7 @@ where }, Err(util::Error::NotAValidator) => { self.metrics.on_is_not_authority(); + self.metrics.on_is_not_parachain_validator(); None }, // Don't update on runtime errors. From c953489d216efaf037dda042c5660de8d4cd7f2c Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 16:39:57 +0000 Subject: [PATCH 19/21] update Signed-off-by: Andrei Sandu --- Cargo.lock | 1 - node/network/gossip-support/Cargo.toml | 1 - node/network/gossip-support/src/tests.rs | 7 +++---- primitives/test-helpers/src/lib.rs | 18 ------------------ 4 files changed, 3 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b8cb761317e..86e93fda792c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6167,7 +6167,6 @@ dependencies = [ "polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-util", "polkadot-primitives", - "polkadot-primitives-test-helpers", "rand 0.8.4", "rand_chacha 0.3.1", "sc-network", diff --git a/node/network/gossip-support/Cargo.toml b/node/network/gossip-support/Cargo.toml index 3179a29ae15e..e9f41edca58b 100644 --- a/node/network/gossip-support/Cargo.toml +++ b/node/network/gossip-support/Cargo.toml @@ -31,4 +31,3 @@ polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } assert_matches = "1.4.0" async-trait = "0.1.52" lazy_static = "1.4.0" -polkadot-primitives-test-helpers = { path = "../../../primitives/test-helpers" } \ No newline at end of file diff --git a/node/network/gossip-support/src/tests.rs b/node/network/gossip-support/src/tests.rs index d97c8edf3ad9..5009742e8631 100644 --- a/node/network/gossip-support/src/tests.rs +++ b/node/network/gossip-support/src/tests.rs @@ -34,7 +34,6 @@ use polkadot_node_subsystem::{ }; use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_node_subsystem_util::TimeoutExt as _; -use polkadot_primitives_test_helpers::dummy_session_info; use test_helpers::mock::make_ferdie_keystore; use super::*; @@ -267,7 +266,7 @@ fn issues_a_connection_request_on_new_session() { RuntimeApiRequest::SessionInfo(1, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(dummy_session_info()))).unwrap(); + sender.send(Ok(None)).unwrap(); } ); @@ -345,7 +344,7 @@ fn issues_a_connection_request_on_new_session() { RuntimeApiRequest::SessionInfo(2, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(dummy_session_info()))).unwrap(); + sender.send(Ok(None)).unwrap(); } ); @@ -443,7 +442,7 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { RuntimeApiRequest::SessionInfo(1, sender), )) => { assert_eq!(relay_parent, hash); - sender.send(Ok(Some(dummy_session_info()))).unwrap(); + sender.send(Ok(None)).unwrap(); } ); diff --git a/primitives/test-helpers/src/lib.rs b/primitives/test-helpers/src/lib.rs index a60505c36948..f0bf6112ca9a 100644 --- a/primitives/test-helpers/src/lib.rs +++ b/primitives/test-helpers/src/lib.rs @@ -227,21 +227,3 @@ impl TestCandidateBuilder { CandidateReceipt { descriptor, commitments_hash: self.commitments_hash } } } - -pub fn dummy_session_info() -> SessionInfo { - SessionInfo { - active_validator_indices: Vec::new(), - random_seed: [0u8; 32], - dispute_period: 10, - validators: Vec::new(), - discovery_keys: Vec::new(), - assignment_keys: Vec::new(), - validator_groups: Vec::new(), - n_cores: 0, - zeroth_delay_tranche_width: 0, - relay_vrf_modulo_samples: 0, - n_delay_tranches: 0, - no_show_slots: 0, - needed_approvals: 0, - } -} From 4633e8c0564bed210cc5960227718af85a5a7bca Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 16:41:52 +0000 Subject: [PATCH 20/21] damn Signed-off-by: Andrei Sandu --- primitives/test-helpers/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/primitives/test-helpers/src/lib.rs b/primitives/test-helpers/src/lib.rs index f0bf6112ca9a..09da521b6219 100644 --- a/primitives/test-helpers/src/lib.rs +++ b/primitives/test-helpers/src/lib.rs @@ -27,8 +27,6 @@ use polkadot_primitives::v1::{ ValidatorId, }; -use polkadot_primitives::v2::SessionInfo; - use sp_application_crypto::sr25519; use sp_keyring::Sr25519Keyring; use sp_runtime::generic::Digest; From 541ad6da73d87d31c1ef89fa0814a05afbc49ac6 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Jan 2022 16:42:28 +0000 Subject: [PATCH 21/21] :facepalm: Signed-off-by: Andrei Sandu --- primitives/test-helpers/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/primitives/test-helpers/src/lib.rs b/primitives/test-helpers/src/lib.rs index 09da521b6219..ccf103d79e58 100644 --- a/primitives/test-helpers/src/lib.rs +++ b/primitives/test-helpers/src/lib.rs @@ -26,7 +26,6 @@ use polkadot_primitives::v1::{ CommittedCandidateReceipt, Hash, HeadData, Id as ParaId, ValidationCode, ValidationCodeHash, ValidatorId, }; - use sp_application_crypto::sr25519; use sp_keyring::Sr25519Keyring; use sp_runtime::generic::Digest;