From e110cd7ae8fcaf261185cd8df8950b0ef7b1a2f0 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 28 Oct 2019 11:06:16 +0100 Subject: [PATCH] *: Disable authority discovery module (#3914) The authority discovery module enables authorities to be discoverable and discover other authorities to improve interconnection among them. In order to achieve this the module needs to know when the authority set changes, thus when a session changes. One has to register a module as a *session handler* in order for it to be notified of changing sessions. The order and number of these *session handlers* **MUST** correspond to the order and number of the *session keys*. Commit 027d887 added the authority discovery to the `SessionHandlers`. Given that the authority discovery module piggybacks on the Babe session keys the commit violated the above constraint. This commit reverts most of 027d887, leaving `core/authority-discovery` and `srml/authority-discovery` untouched. --- substrate/Cargo.lock | 6 ---- substrate/core/service/Cargo.toml | 2 -- substrate/node/cli/Cargo.toml | 2 -- substrate/node/cli/src/chain_spec.rs | 9 ++--- substrate/node/cli/src/service.rs | 12 ++----- substrate/node/runtime/Cargo.toml | 4 --- substrate/node/runtime/src/lib.rs | 47 ++++----------------------- substrate/node/testing/src/genesis.rs | 1 - 8 files changed, 12 insertions(+), 71 deletions(-) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index af472eee31..3343942cc1 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -2392,7 +2392,6 @@ dependencies = [ "serde 1.0.101 (registry+https://github.com/rust-lang/crates.io-index)", "sr-io 2.0.0", "sr-primitives 2.0.0", - "srml-authority-discovery 0.1.0", "srml-balances 2.0.0", "srml-contracts 2.0.0", "srml-finality-tracker 2.0.0", @@ -2403,7 +2402,6 @@ dependencies = [ "srml-timestamp 2.0.0", "srml-transaction-payment 2.0.0", "structopt 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", - "substrate-authority-discovery 2.0.0", "substrate-basic-authorship 2.0.0", "substrate-chain-spec 2.0.0", "substrate-cli 2.0.0", @@ -2510,7 +2508,6 @@ dependencies = [ "sr-staking-primitives 2.0.0", "sr-std 2.0.0", "sr-version 2.0.0", - "srml-authority-discovery 0.1.0", "srml-authorship 0.1.0", "srml-babe 2.0.0", "srml-balances 2.0.0", @@ -2539,7 +2536,6 @@ dependencies = [ "srml-transaction-payment 2.0.0", "srml-treasury 2.0.0", "srml-utility 2.0.0", - "substrate-authority-discovery-primitives 2.0.0", "substrate-client 2.0.0", "substrate-consensus-babe-primitives 2.0.0", "substrate-keyring 2.0.0", @@ -5587,8 +5583,6 @@ dependencies = [ "sr-io 2.0.0", "sr-primitives 2.0.0", "substrate-application-crypto 2.0.0", - "substrate-authority-discovery 2.0.0", - "substrate-authority-discovery-primitives 2.0.0", "substrate-chain-spec 2.0.0", "substrate-client 2.0.0", "substrate-client-db 2.0.0", diff --git a/substrate/core/service/Cargo.toml b/substrate/core/service/Cargo.toml index 04371087ef..0d6f279362 100644 --- a/substrate/core/service/Cargo.toml +++ b/substrate/core/service/Cargo.toml @@ -32,14 +32,12 @@ client = { package = "substrate-client", path = "../../core/client" } client_db = { package = "substrate-client-db", path = "../../core/client/db", features = ["kvdb-rocksdb"] } codec = { package = "parity-scale-codec", version = "1.0.0" } substrate-executor = { path = "../../core/executor" } -substrate-authority-discovery = { path = "../../core/authority-discovery"} transaction_pool = { package = "substrate-transaction-pool", path = "../../core/transaction-pool" } rpc-servers = { package = "substrate-rpc-servers", path = "../../core/rpc-servers" } rpc = { package = "substrate-rpc", path = "../../core/rpc" } tel = { package = "substrate-telemetry", path = "../../core/telemetry" } offchain = { package = "substrate-offchain", path = "../../core/offchain" } parity-multiaddr = { package = "parity-multiaddr", version = "0.5.0" } -authority-discovery-primitives = { package = "substrate-authority-discovery-primitives", path = "../authority-discovery/primitives", default-features = false } [dev-dependencies] substrate-test-runtime-client = { path = "../test-runtime/client" } diff --git a/substrate/node/cli/Cargo.toml b/substrate/node/cli/Cargo.toml index 5fa92360d6..7e011d9de8 100644 --- a/substrate/node/cli/Cargo.toml +++ b/substrate/node/cli/Cargo.toml @@ -48,8 +48,6 @@ balances = { package = "srml-balances", path = "../../srml/balances" } transaction-payment = { package = "srml-transaction-payment", path = "../../srml/transaction-payment" } support = { package = "srml-support", path = "../../srml/support", default-features = false } im_online = { package = "srml-im-online", path = "../../srml/im-online", default-features = false } -sr-authority-discovery = { package = "srml-authority-discovery", path = "../../srml/authority-discovery", default-features = false } -authority-discovery = { package = "substrate-authority-discovery", path = "../../core/authority-discovery"} serde = { version = "1.0.101", features = [ "derive" ] } client_db = { package = "substrate-client-db", path = "../../core/client/db", features = ["kvdb-rocksdb"] } offchain = { package = "substrate-offchain", path = "../../core/offchain" } diff --git a/substrate/node/cli/src/chain_spec.rs b/substrate/node/cli/src/chain_spec.rs index 360421bda1..80c74455e3 100644 --- a/substrate/node/cli/src/chain_spec.rs +++ b/substrate/node/cli/src/chain_spec.rs @@ -20,9 +20,9 @@ use chain_spec::ChainSpecExtension; use primitives::{Pair, Public, crypto::UncheckedInto, sr25519}; use serde::{Serialize, Deserialize}; use node_runtime::{ - AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, ContractsConfig, CouncilConfig, DemocracyConfig, - ElectionsConfig, GrandpaConfig, ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, StakerStatus, - StakingConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig, WASM_BINARY, + BabeConfig, BalancesConfig, ContractsConfig, CouncilConfig, DemocracyConfig, ElectionsConfig, GrandpaConfig, + ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, StakerStatus, StakingConfig, SudoConfig, SystemConfig, + TechnicalCommitteeConfig, WASM_BINARY, }; use node_runtime::Block; use node_runtime::constants::{time::*, currency::*}; @@ -265,9 +265,6 @@ pub fn testnet_genesis( im_online: Some(ImOnlineConfig { keys: vec![], }), - authority_discovery: Some(AuthorityDiscoveryConfig{ - keys: vec![], - }), grandpa: Some(GrandpaConfig { authorities: vec![], }), diff --git a/substrate/node/cli/src/service.rs b/substrate/node/cli/src/service.rs index 78b978b72b..ef1fd8ad9f 100644 --- a/substrate/node/cli/src/service.rs +++ b/substrate/node/cli/src/service.rs @@ -130,8 +130,8 @@ macro_rules! new_full { // back-pressure. Authority discovery is triggering one event per authority within the current authority set. // This estimates the authority set size to be somewhere below 10 000 thereby setting the channel buffer size to // 10 000. - let (dht_event_tx, dht_event_rx) = - mpsc::channel::(10000); + let (dht_event_tx, _dht_event_rx) = + mpsc::channel::(10_000); let service = builder.with_network_protocol(|_| Ok(crate::service::NodeProtocol::new()))? .with_finality_proof_provider(|client, backend| @@ -169,14 +169,6 @@ macro_rules! new_full { let babe = babe::start_babe(babe_config)?; service.spawn_essential_task(babe); - - let authority_discovery = authority_discovery::AuthorityDiscovery::new( - service.client(), - service.network(), - dht_event_rx, - ); - - service.spawn_task(authority_discovery); } let config = grandpa::Config { diff --git a/substrate/node/runtime/Cargo.toml b/substrate/node/runtime/Cargo.toml index 9169f26dc3..94a7683dc5 100644 --- a/substrate/node/runtime/Cargo.toml +++ b/substrate/node/runtime/Cargo.toml @@ -12,7 +12,6 @@ rustc-hex = { version = "2.0", optional = true } safe-mix = { version = "1.0", default-features = false } serde = { version = "1.0.101", optional = true } -authority-discovery-primitives = { package = "substrate-authority-discovery-primitives", path = "../../core/authority-discovery/primitives", default-features = false } babe-primitives = { package = "substrate-consensus-babe-primitives", path = "../../core/consensus/babe/primitives", default-features = false } client = { package = "substrate-client", path = "../../core/client", default-features = false } node-primitives = { path = "../primitives", default-features = false } @@ -25,7 +24,6 @@ substrate-keyring = { path = "../../core/keyring", optional = true } substrate-session = { path = "../../core/session", default-features = false } version = { package = "sr-version", path = "../../core/sr-version", default-features = false } -authority-discovery = { package = "srml-authority-discovery", path = "../../srml/authority-discovery", default-features = false } authorship = { package = "srml-authorship", path = "../../srml/authorship", default-features = false } babe = { package = "srml-babe", path = "../../srml/babe", default-features = false } balances = { package = "srml-balances", path = "../../srml/balances", default-features = false } @@ -64,8 +62,6 @@ runtime_io = { package = "sr-io", path = "../../core/sr-io" } [features] default = ["std"] std = [ - "authority-discovery-primitives/std", - "authority-discovery/std", "authorship/std", "babe-primitives/std", "babe/std", diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index d81ed2ebbd..b56e015d77 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -29,7 +29,7 @@ use node_primitives::{ AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, Moment, Signature, }; -use babe_primitives::{AuthorityId as BabeId, AuthoritySignature as BabeSignature}; +use babe_primitives::AuthorityId as BabeId; use grandpa::fg_primitives; use client::{ block_builder::api::{self as block_builder_api, InherentData, CheckInherentsResult}, @@ -50,8 +50,6 @@ use version::NativeVersion; use primitives::OpaqueMetadata; use grandpa::{AuthorityId as GrandpaId, AuthorityWeight as GrandpaWeight}; use im_online::sr25519::{AuthorityId as ImOnlineId}; -use authority_discovery_primitives::{AuthorityId as EncodedAuthorityId, Signature as EncodedSignature}; -use codec::{Encode, Decode}; use system::offchain::TransactionSubmitter; #[cfg(any(feature = "std", test))] @@ -83,8 +81,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 188, - impl_version: 188, + spec_version: 189, + impl_version: 189, apis: RUNTIME_API_VERSIONS, }; @@ -211,7 +209,10 @@ impl authorship::Trait for Runtime { type EventHandler = Staking; } -type SessionHandlers = (Grandpa, Babe, ImOnline, AuthorityDiscovery); +// !!!!!!!!!!!!! +// WARNING!!!!!! SEE NOTE BELOW BEFORE TOUCHING THIS CODE +// !!!!!!!!!!!!! +type SessionHandlers = (Grandpa, Babe, ImOnline); impl_opaque_keys! { pub struct SessionKeys { @@ -439,10 +440,6 @@ impl offences::Trait for Runtime { type OnOffenceHandler = Staking; } -impl authority_discovery::Trait for Runtime { - type AuthorityId = BabeId; -} - impl grandpa::Trait for Runtime { type Event = Event; } @@ -531,7 +528,6 @@ construct_runtime!( Contracts: contracts, Sudo: sudo, ImOnline: im_online::{Module, Call, Storage, Event, ValidateUnsigned, Config}, - AuthorityDiscovery: authority_discovery::{Module, Call, Config}, Offences: offences::{Module, Call, Storage, Event}, RandomnessCollectiveFlip: randomness_collective_flip::{Module, Call, Storage}, Nicks: nicks::{Module, Call, Storage, Event}, @@ -646,35 +642,6 @@ impl_runtime_apis! { } } - impl authority_discovery_primitives::AuthorityDiscoveryApi for Runtime { - fn authorities() -> Vec { - AuthorityDiscovery::authorities().into_iter() - .map(|id| id.encode()) - .map(EncodedAuthorityId) - .collect() - } - - fn sign(payload: &Vec) -> Option<(EncodedSignature, EncodedAuthorityId)> { - AuthorityDiscovery::sign(payload).map(|(sig, id)| { - (EncodedSignature(sig.encode()), EncodedAuthorityId(id.encode())) - }) - } - - fn verify(payload: &Vec, signature: &EncodedSignature, authority_id: &EncodedAuthorityId) -> bool { - let signature = match BabeSignature::decode(&mut &signature.0[..]) { - Ok(s) => s, - _ => return false, - }; - - let authority_id = match BabeId::decode(&mut &authority_id.0[..]) { - Ok(id) => id, - _ => return false, - }; - - AuthorityDiscovery::verify(payload, signature, authority_id) - } - } - impl system_rpc_runtime_api::AccountNonceApi for Runtime { fn account_nonce(account: AccountId) -> Index { System::account_nonce(account) diff --git a/substrate/node/testing/src/genesis.rs b/substrate/node/testing/src/genesis.rs index 5220d1774b..b6ee28cf0f 100644 --- a/substrate/node/testing/src/genesis.rs +++ b/substrate/node/testing/src/genesis.rs @@ -89,7 +89,6 @@ pub fn config(support_changes_trie: bool, code: Option<&[u8]>) -> GenesisConfig authorities: vec![], }), im_online: Some(Default::default()), - authority_discovery: Some(Default::default()), democracy: Some(Default::default()), collective_Instance1: Some(Default::default()), collective_Instance2: Some(Default::default()),