Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

core/authority-discovery: Enable authorities to discover each other #3452

Merged
merged 23 commits into from
Sep 6, 2019
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f72e1cb
node/runtime: Add authority-discovery as session handler
mxinden Aug 21, 2019
d34eb4f
core/network: Make network worker return Dht events on poll
mxinden Aug 21, 2019
94a238a
*: Add scaffolding and integration for core/authority-discovery module
mxinden Aug 21, 2019
7de7db7
core/authority-discovery: Implement module logic itself
mxinden Aug 21, 2019
ed294de
core/network: Finish NetworkWoker if NetworkService stream finished
mxinden Aug 22, 2019
fbc3a5b
core/authority-discovery: Ensure being woken up on next interval tick
mxinden Aug 22, 2019
9de2655
core/authority-discovery: Adjust interval to be proactive on
mxinden Aug 27, 2019
5d1042d
core/authority-discovery: Implement unit tests
mxinden Aug 27, 2019
4421bad
core/authority-discovery: Publish and query on different intervals
mxinden Aug 29, 2019
76db412
*: Remove abstract type AuthorityId replaced by newtype Vec<u8>
mxinden Aug 30, 2019
2d025e0
Merge remote-tracking branch 'paritytech/master' into authority-disco…
mxinden Aug 30, 2019
56da02c
{core,srml}/authority-discovery: Adjust tests to merge
mxinden Aug 30, 2019
1a5f974
node/runtime: Bump runtime spec and impl version
mxinden Aug 30, 2019
32214d9
*: Instantiate authority discovery within node/cli
mxinden Sep 2, 2019
2f5c938
Merge remote-tracking branch 'paritytech/master' into authority-disco…
mxinden Sep 2, 2019
30bd78e
core/authority-discovery: Remove patch version in Cargo.toml
mxinden Sep 3, 2019
1a376bf
core/authority-discovery: Add doc comments to Error enum
mxinden Sep 3, 2019
c5d5a90
authority-discovery: Encode address as Protobuf bytes instead of string
mxinden Sep 3, 2019
d162540
*: Have AuthorityApi.{sign,verify} borrow its inputs
mxinden Sep 4, 2019
024687e
core/authority-discovery: Handle tokio timer errors
mxinden Sep 4, 2019
9c8534c
Merge remote-tracking branch 'paritytech/master' into authority-disco…
mxinden Sep 4, 2019
2e954eb
srml/authority-discovery: Address unit test failures
mxinden Sep 4, 2019
c39c50e
core/authority-discovery: Address unit test failures
mxinden Sep 5, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ members = [
"core/utils/fork-tree",
"core/utils/wasm-builder",
"core/utils/wasm-builder-runner",
"core/authority-discovery",
"srml/support",
"srml/support/procedural",
"srml/support/procedural/tools",
Expand Down
32 changes: 32 additions & 0 deletions core/authority-discovery/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[package]
name = "substrate-authority-discovery"
version = "2.0.0"
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2018"
build = "build.rs"

[build-dependencies]
prost-build = "0.5"

[dependencies]
network = { package = "substrate-network", path = "../../core/network" }
sr-primitives = { path = "../../core/sr-primitives" }
primitives = { package = "substrate-primitives", path = "../primitives" }
client = { package = "substrate-client", path = "../../core/client" }
authority-discovery-primitives = { package = "substrate-authority-discovery-primitives", path = "./primitives", default-features = false }
codec = { package = "parity-scale-codec", default-features = false, version = "1.0.3" }
futures = "0.1.17"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for sticking with this outdated version of futures? The current version in the 0.1 family is 0.1.28.

As an aside, I think it would be great if dependencies were sorted alphabetically.

Copy link
Contributor Author

@mxinden mxinden Sep 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason to using 0.1.17 other than other packages using the same:

grep -ri --include Cargo.toml "futures = "
node-template/Cargo.toml:futures = "0.1"
core/client/Cargo.toml:futures = { version = "0.1", optional = true }
core/network/Cargo.toml:futures = "0.1.17"
core/authority-discovery/Cargo.toml:futures = "0.1.17"
core/service/test/Cargo.toml:futures = "0.1"
core/service/Cargo.toml:futures = "0.1.17"
core/finality-grandpa/Cargo.toml:futures = "0.1"
core/cli/Cargo.toml:futures = "0.1.17"
core/rpc/Cargo.toml:futures = "0.1.17"
core/consensus/rhd/Cargo.toml:futures = "0.1.17"
node/rpc-client/Cargo.toml:futures = "0.1.26"
node/cli/Cargo.toml:futures = "0.1"
Cargo.toml:futures = "0.1"

Shouldn't we update them all at once?

More general question: Why are we ever specifying the semver patch version?


As an aside, I think it would be great if dependencies were sorted alphabetically.

I will do that.

Copy link
Contributor

@tomaka tomaka Sep 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More general question: Why are we ever specifying the semver patch version?

Considering that this is below 1.0, the general rule is that the patch version here is the equivalent of a minor version post-1.0.

If you use a feature that was added in futures 0.1.12 and you only depend on futures 0.1.11 for example, your code might compile thanks to your Cargo.lock but you still have things wrong.

This is too hard to enforce manually however, and there's no tooling to do that automatically.

tokio-timer = "0.2"
keystore = { package = "substrate-keystore", path = "../../core/keystore" }
libp2p = { version = "0.12.0", default-features = false, features = ["secp256k1", "libp2p-websocket"] }
serde_json = "1.0"
log = "0.4"
derive_more = "0.14.0"
prost = "0.5"
bytes = "0.4"

[dev-dependencies]
test-client = { package = "substrate-test-runtime-client", path = "../../core/test-runtime/client" }
peerset = { package = "substrate-peerset", path = "../../core/peerset" }
parking_lot = { version = "0.9.0" }
tokio = { version = "0.1.11"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to my previous comment, tokio < 0.2 is already at 0.1.22.

3 changes: 3 additions & 0 deletions core/authority-discovery/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
prost_build::compile_protos(&["src/schema/dht.proto"], &["src/schema"]).unwrap();
}
20 changes: 10 additions & 10 deletions core/authority-discovery/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,31 @@
#![cfg_attr(not(feature = "std"), no_std)]

use client::decl_runtime_apis;
use codec::Codec;
use rstd::vec::Vec;

#[derive(codec::Encode, codec::Decode, Eq, PartialEq, Clone)]
#[cfg_attr(feature = "std", derive(Debug, Hash))]
pub struct Signature(pub Vec<u8>);
#[derive(codec::Encode, codec::Decode, Eq, PartialEq, Clone)]
#[cfg_attr(feature = "std", derive(Debug, Hash))]
pub struct AuthorityId(pub Vec<u8>);

decl_runtime_apis! {
/// The authority discovery api.
///
/// This api is used by the `core/authority-discovery` module to retrieve our
/// own authority identifier, to retrieve identifiers of the current authority
/// set, as well as sign and verify Kademlia Dht external address payloads
/// from and to other authorities.
pub trait AuthorityDiscoveryApi<AuthorityId: Codec> {
/// Returns own authority identifier iff it is part of the current authority
/// set, otherwise this function returns None. The restriction might be
/// softened in the future in case a consumer needs to learn own authority
/// identifier.
fn authority_id() -> Option<AuthorityId>;

pub trait AuthorityDiscoveryApi {
/// Retrieve authority identifiers of the current authority set.
fn authorities() -> Vec<AuthorityId>;

/// Sign the given payload with the private key corresponding to the given authority id.
fn sign(payload: Vec<u8>, authority_id: AuthorityId) -> Option<Vec<u8>>;
fn sign(payload: Vec<u8>) -> Option<(Signature, AuthorityId)>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably due to decl_runtime_apis! that this consumes the Vec<u8> instead of just borrowing a &[u8], right?


/// Verify the given signature for the given payload with the given
/// authority identifier.
fn verify(payload: Vec<u8>, signature: Vec<u8>, authority_id: AuthorityId) -> bool;
fn verify(payload: Vec<u8>, signature: Signature, authority_id: AuthorityId) -> bool;
}
}
36 changes: 36 additions & 0 deletions core/authority-discovery/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2019 Parity Technologies (UK) Ltd.
// This file is part of Substrate.

// Substrate 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.

// Substrate 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 Substrate. If not, see <http://www.gnu.org/licenses/>.

//! Authority discovery errors.

/// AuthorityDiscovery Result.
pub type Result<T> = std::result::Result<T, Error>;

#[derive(Debug, derive_more::Display, derive_more::From)]
pub enum Error {
mxinden marked this conversation as resolved.
Show resolved Hide resolved
VerifyingDhtPayload,
HashingAuthorityId(libp2p::core::multiaddr::multihash::EncodeError),
CallingRuntime(client::error::Error),
SigningDhtPayload,
/// From the Dht we only get the hashed authority id. In order to retrieve the actual authority id and to ensure it
/// is actually an authority, we match the hash against the hash of the authority id of all other authorities. This
/// error is the result of the above failing.
MatchingHashedAuthorityIdWithAuthorityId,
SettingPeersetPriorityGroup(String),
Encoding(prost::EncodeError),
Decoding(prost::DecodeError),
ParsingMultiaddress(libp2p::core::multiaddr::Error),
}
Loading