Unverified Commit e721a5aa authored by Bernhard Schuster's avatar Bernhard Schuster Committed by GitHub
Browse files

remove `AllSubsystems` and `AllSubsystemsGen` types (#3874)



* introduce the OverseerConnector, use it

* introduce is_relay_chain to RelayChainSelection

* Update node/service/src/lib.rs
Co-authored-by: Andronik Ordian's avatarAndronik Ordian <write@reusable.software>

* avoid the deferred setting of `is_relay_chain` in `RelayChainSelection`

* positive assertion is not mandated, only the negative one, to avoid a stall

* cleanup: overseer residue

* spellcheck

* fixin

* groundwork to obsolete Overseer::new and AllSubsystemsGen proc-macro

* Now all malus & tests can be ported to the builder pattern.

Obsoletes `Overseer::new`, `AllSubsystemsGen` derive macro, `AllSubsystems`.

* spellcheck

* adjust tests, minor fixes

* remove derive macro AllSubsystemsGen

* add forgotten file dummy.rs

* remove residue

* good news everyone!

* spellcheck

* address review comments

* fixup imports

* make it conditional

* fixup docs

* reduce import

* chore: fmt

* chore: fmt

* chore: spellcheck / nlprules

* fixup malus variant-a

* fmt

* fix

* fixins

* pfmt

* fixins

* chore: fmt

* remove expanded overseer generation

* tracing version

* Update node/network/statement-distribution/src/lib.rs
Co-authored-by: asynchronous rob's avatarRobert Habermeier <rphmeier@gmail.com>

* use future::ready instead

* silence warning

* chore: fmt
Co-authored-by: Andronik Ordian's avatarAndronik Ordian <write@reusable.software>
Co-authored-by: asynchronous rob's avatarRobert Habermeier <rphmeier@gmail.com>
parent 3338f9c7
Pipeline #159831 passed with stages
in 43 minutes and 42 seconds
......@@ -6553,7 +6553,6 @@ dependencies = [
"polkadot-node-network-protocol",
"polkadot-node-primitives",
"polkadot-node-subsystem-types",
"polkadot-overseer-all-subsystems-gen",
"polkadot-overseer-gen",
"polkadot-primitives",
"sc-client-api",
......@@ -6562,16 +6561,6 @@ dependencies = [
"tracing",
]
[[package]]
name = "polkadot-overseer-all-subsystems-gen"
version = "0.9.9"
dependencies = [
"proc-macro2",
"quote",
"syn",
"trybuild",
]
[[package]]
name = "polkadot-overseer-gen"
version = "0.9.9"
......
......@@ -73,7 +73,6 @@ members = [
"node/overseer",
"node/overseer/overseer-gen",
"node/overseer/overseer-gen/proc-macro",
"node/overseer/all-subsystems-gen",
"node/malus",
"node/primitives",
"node/service",
......
......@@ -29,7 +29,7 @@ pub use service::RuntimeApiCollection;
pub use service::{self, Block, CoreApi, IdentifyVariant, ProvideRuntimeApi, TFullClient};
#[cfg(feature = "malus")]
pub use service::create_default_subsystems;
pub use service::overseer::prepared_overseer_builder;
#[cfg(feature = "cli")]
pub use cli::*;
......
......@@ -150,7 +150,7 @@ impl Artifacts {
/// Inform the table about the artifact with the given ID. The state will be set to "preparing".
///
/// This function must be used only for brand new artifacts and should never be used for
/// This function must be used only for brand-new artifacts and should never be used for
/// replacing existing ones.
pub fn insert_preparing(&mut self, artifact_id: ArtifactId) {
// See the precondition.
......@@ -159,7 +159,7 @@ impl Artifacts {
/// Insert an artifact with the given ID as "prepared".
///
/// This function must be used only for brand new artifacts and should never be used for
/// This function must be used only for brand-new artifacts and should never be used for
/// replacing existing ones.
#[cfg(test)]
pub fn insert_prepared(&mut self, artifact_id: ArtifactId, last_time_needed: SystemTime) {
......
......@@ -24,10 +24,10 @@
use color_eyre::eyre;
use polkadot_cli::{
create_default_subsystems,
prepared_overseer_builder,
service::{
AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
OverseerGen, OverseerGenArgs, ParachainHost, ProvideRuntimeApi, SpawnNamed,
AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, OverseerGen,
OverseerGenArgs, ParachainHost, ProvideRuntimeApi, SpawnNamed,
},
Cli,
};
......@@ -37,7 +37,7 @@ use polkadot_cli::{
use polkadot_node_core_candidate_validation::CandidateValidationSubsystem;
use polkadot_node_subsystem::{
messages::{AllMessages, CandidateValidationMessage},
overseer::{self, OverseerConnector, OverseerHandle},
overseer::{self, Overseer, OverseerConnector, OverseerHandle},
FromOverseer,
};
......@@ -94,15 +94,10 @@ impl OverseerGen for BehaveMaleficient {
RuntimeClient::Api: ParachainHost<Block> + BabeApi<Block> + AuthorityDiscoveryApi<Block>,
Spawner: 'static + SpawnNamed + Clone + Unpin,
{
let spawner = args.spawner.clone();
let leaves = args.leaves.clone();
let runtime_client = args.runtime_client.clone();
let registry = args.registry.clone();
let candidate_validation_config = args.candidate_validation_config.clone();
// modify the subsystem(s) as needed:
let all_subsystems = create_default_subsystems(args)?.replace_candidate_validation(
// create the filtered subsystem
|orig: CandidateValidationSubsystem| {
prepared_overseer_builder(args)?
.replace_candidate_validation(|orig: CandidateValidationSubsystem| {
InterceptedSubsystem::new(
CandidateValidationSubsystem::with_config(
candidate_validation_config,
......@@ -111,10 +106,8 @@ impl OverseerGen for BehaveMaleficient {
),
Skippy::default(),
)
},
);
Overseer::new(leaves, all_subsystems, registry, runtime_client, spawner, connector)
})
.build_with_connector(connector)
.map_err(|e| e.into())
}
}
......
......@@ -56,7 +56,7 @@ pub struct SessionInfo {
/// validators.
pub validator_groups: Vec<Vec<AuthorityDiscoveryId>>,
/// Information about ourself:
/// Information about ourselves:
pub our_index: ValidatorIndex,
/// Remember to which group we belong, so we won't start fetching chunks for candidates with
......
......@@ -18,7 +18,7 @@
//! futures will still get polled, but will not count towards length. So length will only count
//! futures, which are still considered live.
//!
//! Usecase: If futures take longer than we would like them too, we maybe able to request the data
//! Usecase: If futures take longer than we would like them too, we may be able to request the data
//! from somewhere else as well. We don't really want to cancel the old future, because maybe it
//! was almost done, thus we would have wasted time with our impatience. By simply making them
//! not count towards length, we can make sure to have enough "live" requests ongoing, while at the
......
......@@ -29,7 +29,7 @@ use crate::{sender, LOG_TARGET};
pub enum Error {
/// Fatal errors of dispute distribution.
Fatal(Fatal),
/// Non fatal errors of dispute distribution.
/// Non-fatal errors of dispute distribution.
NonFatal(NonFatal),
}
......
......@@ -39,7 +39,7 @@ pub type FatalResult<T> = std::result::Result<T, Fatal>;
pub enum Error {
/// Fatal errors of dispute distribution.
Fatal(Fatal),
/// Non fatal errors of dispute distribution.
/// Non-fatal errors of dispute distribution.
NonFatal(NonFatal),
}
......
......@@ -105,7 +105,7 @@ const MAX_LARGE_STATEMENTS_PER_SENDER: usize = 20;
/// The statement distribution subsystem.
pub struct StatementDistribution {
/// Pointer to a keystore, which is required for determining this nodes validator index.
/// Pointer to a keystore, which is required for determining this node's validator index.
keystore: SyncCryptoStorePtr,
/// Receiver for incoming large statement requests.
req_receiver: Option<IncomingRequestReceiver<request_v1::StatementFetchingRequest>>,
......
......@@ -16,7 +16,6 @@ polkadot-node-subsystem-types = { path = "../subsystem-types" }
polkadot-node-metrics = { path = "../metrics" }
polkadot-primitives = { path = "../../primitives" }
polkadot-overseer-gen = { path = "./overseer-gen" }
polkadot-overseer-all-subsystems-gen = { path = "./all-subsystems-gen" }
tracing = "0.1.28"
lru = "0.6"
parity-util-mem = { version = ">= 0.10.1", default-features = false }
......
[package]
name = "polkadot-overseer-all-subsystems-gen"
version = "0.9.9"
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2018"
description = "Small proc macro to create mocking level iface type helpers"
[lib]
proc-macro = true
[dependencies]
syn = { version = "1.0.77", features = ["full", "extra-traits"] }
quote = "1.0.9"
proc-macro2 = "1.0.24"
[dev-dependencies]
trybuild = "1.0.45"
// 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 <http://www.gnu.org/licenses/>.
use std::collections::HashSet;
use proc_macro2::TokenStream;
use quote::quote;
use syn::{parse2, Error, GenericParam, Ident, Result, Type};
#[proc_macro_derive(AllSubsystemsGen)]
pub fn subsystems_gen(item: proc_macro::TokenStream) -> proc_macro::TokenStream {
let item: TokenStream = item.into();
impl_subsystems_gen(item).unwrap_or_else(|err| err.to_compile_error()).into()
}
fn impl_subsystems_gen(item: TokenStream) -> Result<proc_macro2::TokenStream> {
let span = proc_macro2::Span::call_site();
let ds = parse2::<syn::ItemStruct>(item.clone())?;
match ds.fields {
syn::Fields::Named(named) => {
#[derive(Clone)]
struct NameTyTup {
field: Ident,
ty: Type,
}
let mut orig_generics = ds.generics;
// remove default types
orig_generics.params = orig_generics
.params
.into_iter()
.map(|mut generic| {
match generic {
GenericParam::Type(ref mut param) => {
param.eq_token = None;
param.default = None;
},
_ => {},
}
generic
})
.collect();
// prepare a hashmap of generic type to member that uses it
let generic_types = orig_generics
.params
.iter()
.filter_map(|generic| {
if let GenericParam::Type(param) = generic {
Some(param.ident.clone())
} else {
None
}
})
.collect::<HashSet<Ident>>();
let strukt_ty = ds.ident;
if generic_types.is_empty() {
return Err(Error::new(
strukt_ty.span(),
"struct must have at least one generic parameter.",
))
}
// collect all fields that exist, and all fields that are replaceable
let mut replacable_items = Vec::<NameTyTup>::with_capacity(64);
let mut all_fields = replacable_items.clone();
let mut duplicate_generic_detection = HashSet::<Ident>::with_capacity(64);
for field in named.named {
let field_ident = field
.ident
.clone()
.ok_or_else(|| Error::new(span, "Member field must have a name."))?;
let ty = field.ty.clone();
let ntt = NameTyTup { field: field_ident, ty };
replacable_items.push(ntt.clone());
// assure every generic is used exactly once
let ty_ident = match field.ty {
Type::Path(path) => path.path.get_ident().cloned().ok_or_else(|| {
Error::new(
proc_macro2::Span::call_site(),
"Expected an identifier, but got a path.",
)
}),
_ => return Err(Error::new(proc_macro2::Span::call_site(), "Must be path.")),
}?;
if generic_types.contains(&ty_ident) {
if let Some(previous) = duplicate_generic_detection.replace(ty_ident) {
return Err(Error::new(previous.span(), "Generic type parameters may only be used for exactly one field, but is used more than once."));
}
}
all_fields.push(ntt);
}
let msg = "Generated by #[derive(AllSubsystemsGen)] derive proc-macro.";
let mut additive = TokenStream::new();
// generate an impl of `fn replace_#name`
for NameTyTup { field: replacable_item, ty: replacable_item_ty } in replacable_items {
let keeper = &all_fields
.iter()
.filter(|ntt| ntt.field != replacable_item)
.map(|ntt| ntt.field.clone())
.collect::<Vec<_>>();
let strukt_ty = strukt_ty.clone();
let fname = Ident::new(&format!("replace_{}", replacable_item), span);
// adjust the generics such that the appropriate member type is replaced
let mut modified_generics = orig_generics.clone();
modified_generics.params = modified_generics
.params
.into_iter()
.map(|mut generic| {
match generic {
GenericParam::Type(ref mut param) => {
param.eq_token = None;
param.default = None;
if match &replacable_item_ty {
Type::Path(path) => path
.path
.get_ident()
.filter(|&ident| ident == &param.ident)
.is_some(),
_ => false,
} {
param.ident = Ident::new("NEW", span);
}
},
_ => {},
}
generic
})
.collect();
additive.extend(quote! {
impl #orig_generics #strukt_ty #orig_generics {
#[doc = #msg]
pub fn #fname < NEW, F > (self, gen_replacement_fn: F) -> #strukt_ty #modified_generics
where
F: FnOnce(#replacable_item_ty) -> NEW,
{
let Self {
// To be replaced field:
#replacable_item,
// Fields to keep:
#(
#keeper,
)*
} = self;
// Some cases require that parts of the original are copied
// over, since they include a one time initialization.
let replacement = gen_replacement_fn(#replacable_item);
#strukt_ty :: #modified_generics {
#replacable_item: replacement,
#(
#keeper,
)*
}
}
}
});
}
Ok(additive)
},
syn::Fields::Unit =>
Err(Error::new(span, "Must be a struct with named fields. Not an unit struct.")),
syn::Fields::Unnamed(_) => Err(Error::new(
span,
"Must be a struct with named fields. Not an unnamed fields struct.",
)),
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn basic() {
let item = quote! {
pub struct AllSubsystems<A,B,CD> {
pub a: A,
pub beee: B,
pub dj: CD,
}
};
let output = impl_subsystems_gen(item).expect("Simple example always works. qed");
println!("//generated:");
println!("{}", output);
}
#[test]
fn ui() {
let t = trybuild::TestCases::new();
t.compile_fail("tests/ui/err-*.rs");
t.pass("tests/ui/ok-*.rs");
}
}
#![allow(dead_code)]
use polkadot_overseer_all_subsystems_gen::AllSubsystemsGen;
#[derive(Clone, AllSubsystemsGen)]
enum AllSubsystems<A,B> {
A(A),
B(B),
}
fn main() {
let all = AllSubsystems::<u8,u16>::A(0u8);
}
error: expected `struct`
--> $DIR/err-01-enum.rs:6:1
|
6 | enum AllSubsystems<A,B> {
| ^^^^
#![allow(dead_code)]
use polkadot_overseer_all_subsystems_gen::AllSubsystemsGen;
#[derive(Clone, AllSubsystemsGen)]
struct AllSubsystems<X> {
a: X,
b: X,
}
fn main() {
let all = AllSubsystems::<u16> {
a: 0_u16,
b: 1_u16,
};
let _all = all.replace_a(|_| 77u8);
}
error: Generic type parameters may only be used for exactly one field, but is used more than once.
--> $DIR/err-01-generic-used-twice.rs:6:5
|
6 | a: X,
| ^
error[E0599]: no method named `replace_a` found for struct `AllSubsystems` in the current scope
--> $DIR/err-01-generic-used-twice.rs:15:17
|
5 | struct AllSubsystems<X> {
| ----------------------- method `replace_a` not found for this
...
15 | let _all = all.replace_a(|_| 77u8);
| ^^^^^^^^^ method not found in `AllSubsystems<u16>`
#![allow(dead_code)]
use polkadot_overseer_all_subsystems_gen::AllSubsystemsGen;
#[derive(Clone, AllSubsystemsGen)]
struct AllSubsystems {
a: f32,
b: u16,
}
fn main() {
let all = AllSubsystems {
a: 0_f32,
b: 1_u16,
};
let _all = all.replace_a(|_| 77u8);
}
error: struct must have at least one generic parameter.
--> $DIR/err-01-no-generic.rs:6:8
|
6 | struct AllSubsystems {
| ^^^^^^^^^^^^^
error[E0599]: no method named `replace_a` found for struct `AllSubsystems` in the current scope
--> $DIR/err-01-no-generic.rs:16:17
|
6 | struct AllSubsystems {
| -------------------- method `replace_a` not found for this
...
16 | let _all = all.replace_a(|_| 77u8);
| ^^^^^^^^^ method not found in `AllSubsystems`
error: Generic type parameters may only be used once have at least one generic parameter.
--> $DIR/err-01-no-generics.rs:7:5
|
7 | a: X,
| ^
error[E0599]: no method named `replace_a` found for struct `AllSubsystems<u16>` in the current scope
--> $DIR/err-01-no-generics.rs:16:17
|
6 | struct AllSubsystems<X> {
| ----------------------- method `replace_a` not found for this
...
16 | let _all = all.replace_a(|_| 77u8);
| ^^^^^^^^^ method not found in `AllSubsystems<u16>`
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment