Name changes for GrandPa and Beefy notifications protocols (#10463)

* grandpa: update notif protocol name

* grandpa: add chain id prefix to protocol name

* grandpa: beautify protocol name handling

* grandpa: prepend genesis hash to protocol name

* chain-spec: add optional 'fork_id'

'fork_id' is used to uniquely identify forks of the same chain/network
'ChainSpec' trait provides default 'None' implementation, meaning this
chain hasn't been forked.

* grandpa: protocol_name mod instead of struct

* beefy: add genesis hash prefix to protocol name

* chainspec: add fork_id

* grandpa: simplify protocol name

* grandpa: contain protocol name building logic

* beefy: contain protocol name building logic

* grandpa: fix tests

* fix merge damage

* fix docs reference visibility

Signed-off-by: acatangiu <adrian@parity.io>

* Update client/finality-grandpa/src/lib.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update client/finality-grandpa/src/communication/mod.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update client/beefy/src/lib.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update client/beefy/src/lib.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* avoid using hash default, even for protocol names

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
This commit is contained in:
Adrian Catangiu
2022-01-05 19:01:44 +02:00
committed by GitHub
parent 223d929f86
commit c1865988df
18 changed files with 163 additions and 46 deletions
@@ -1664,6 +1664,7 @@ pub(super) struct PeerReport {
#[cfg(test)]
mod tests {
use super::{environment::SharedVoterSetState, *};
use crate::communication;
use sc_network::config::Role;
use sc_network_gossip::Validator as GossipValidatorT;
use sc_network_test::Block;
@@ -1679,6 +1680,7 @@ mod tests {
local_role: Role::Authority,
observer_enabled: true,
telemetry: None,
protocol_name: communication::grandpa_protocol_name::NAME.into(),
}
}
@@ -1840,13 +1842,13 @@ mod tests {
// messages from old rounds are expired.
for round_num in 1u64..last_kept_round {
let topic = crate::communication::round_topic::<Block>(round_num, 1);
let topic = communication::round_topic::<Block>(round_num, 1);
assert!(is_expired(topic, &[1, 2, 3]));
}
// messages from not-too-old rounds are not expired.
for round_num in last_kept_round..10 {
let topic = crate::communication::round_topic::<Block>(round_num, 1);
let topic = communication::round_topic::<Block>(round_num, 1);
assert!(!is_expired(topic, &[1, 2, 3]));
}
}
@@ -2262,7 +2264,7 @@ mod tests {
// we accept messages from rounds 9, 10 and 11
// therefore neither of those should be considered expired
for round in &[9, 10, 11] {
assert!(!is_expired(crate::communication::round_topic::<Block>(*round, 1), &[]))
assert!(!is_expired(communication::round_topic::<Block>(*round, 1), &[]))
}
}
@@ -2310,7 +2312,7 @@ mod tests {
if message_allowed(
peer,
MessageIntent::Broadcast,
&crate::communication::round_topic::<Block>(1, 0),
&communication::round_topic::<Block>(1, 0),
&[],
) {
allowed += 1;
@@ -2374,7 +2376,7 @@ mod tests {
assert!(!val.message_allowed()(
&light_peer,
MessageIntent::Broadcast,
&crate::communication::round_topic::<Block>(1, 0),
&communication::round_topic::<Block>(1, 0),
&[],
));
@@ -2388,7 +2390,7 @@ mod tests {
assert!(!val.message_allowed()(
&light_peer,
MessageIntent::Broadcast,
&crate::communication::round_topic::<Block>(1, 0),
&communication::round_topic::<Block>(1, 0),
&[],
));
@@ -2412,8 +2414,8 @@ mod tests {
auth_data: Vec::new(),
};
crate::communication::gossip::GossipMessage::<Block>::Commit(
crate::communication::gossip::FullCommitMessage {
communication::gossip::GossipMessage::<Block>::Commit(
communication::gossip::FullCommitMessage {
round: Round(2),
set_id: SetId(0),
message: commit,
@@ -2426,7 +2428,7 @@ mod tests {
assert!(val.message_allowed()(
&light_peer,
MessageIntent::Broadcast,
&crate::communication::global_topic::<Block>(0),
&communication::global_topic::<Block>(0),
&commit,
));
}
@@ -2466,8 +2468,8 @@ mod tests {
auth_data: Vec::new(),
};
crate::communication::gossip::GossipMessage::<Block>::Commit(
crate::communication::gossip::FullCommitMessage {
communication::gossip::GossipMessage::<Block>::Commit(
communication::gossip::FullCommitMessage {
round: Round(1),
set_id: SetId(1),
message: commit,
@@ -2485,7 +2487,7 @@ mod tests {
assert!(message_allowed(
&peer1,
MessageIntent::Broadcast,
&crate::communication::global_topic::<Block>(1),
&communication::global_topic::<Block>(1),
&commit,
));
@@ -2494,7 +2496,7 @@ mod tests {
assert!(!message_allowed(
&peer2,
MessageIntent::Broadcast,
&crate::communication::global_topic::<Block>(1),
&communication::global_topic::<Block>(1),
&commit,
));
}
@@ -2511,8 +2513,8 @@ mod tests {
auth_data: Vec::new(),
};
crate::communication::gossip::GossipMessage::<Block>::Commit(
crate::communication::gossip::FullCommitMessage {
communication::gossip::GossipMessage::<Block>::Commit(
communication::gossip::FullCommitMessage {
round: Round(round),
set_id: SetId(set_id),
message: commit,
@@ -2532,15 +2534,13 @@ mod tests {
// a commit message for round 1 that finalizes the same height as we
// have observed previously should not be expired
assert!(
!message_expired(crate::communication::global_topic::<Block>(1), &commit(1, 1, 2),)
);
assert!(!message_expired(communication::global_topic::<Block>(1), &commit(1, 1, 2),));
// it should be expired if it is for a lower block
assert!(message_expired(crate::communication::global_topic::<Block>(1), &commit(1, 1, 1)));
assert!(message_expired(communication::global_topic::<Block>(1), &commit(1, 1, 1)));
// or the same block height but from the previous round
assert!(message_expired(crate::communication::global_topic::<Block>(1), &commit(0, 1, 2)));
assert!(message_expired(communication::global_topic::<Block>(1), &commit(0, 1, 2)));
}
#[test]
@@ -67,9 +67,27 @@ mod periodic;
#[cfg(test)]
pub(crate) mod tests;
/// Name of the notifications protocol used by Grandpa. Must be registered towards the networking
/// in order for Grandpa to properly function.
pub const GRANDPA_PROTOCOL_NAME: &'static str = "/paritytech/grandpa/1";
pub mod grandpa_protocol_name {
use sc_chain_spec::ChainSpec;
pub(crate) const NAME: &'static str = "/grandpa/1";
/// Old names for the notifications protocol, used for backward compatibility.
pub(crate) const LEGACY_NAMES: [&'static str; 1] = ["/paritytech/grandpa/1"];
/// Name of the notifications protocol used by GRANDPA.
///
/// Must be registered towards the networking in order for GRANDPA to properly function.
pub fn standard_name<Hash: std::fmt::Display>(
genesis_hash: &Hash,
chain_spec: &Box<dyn ChainSpec>,
) -> std::borrow::Cow<'static, str> {
let chain_prefix = match chain_spec.fork_id() {
Some(fork_id) => format!("/{}/{}", genesis_hash, fork_id),
None => format!("/{}", genesis_hash),
};
format!("{}{}", chain_prefix, NAME).into()
}
}
// cost scalars for reporting peers.
mod cost {
@@ -220,13 +238,14 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
prometheus_registry: Option<&Registry>,
telemetry: Option<TelemetryHandle>,
) -> Self {
let protocol = config.protocol_name.clone();
let (validator, report_stream) =
GossipValidator::new(config, set_state.clone(), prometheus_registry, telemetry.clone());
let validator = Arc::new(validator);
let gossip_engine = Arc::new(Mutex::new(GossipEngine::new(
service.clone(),
GRANDPA_PROTOCOL_NAME,
protocol,
validator.clone(),
prometheus_registry,
)));
@@ -22,7 +22,7 @@ use super::{
gossip::{self, GossipValidator},
Round, SetId, VoterSet,
};
use crate::{communication::GRANDPA_PROTOCOL_NAME, environment::SharedVoterSetState};
use crate::{communication::grandpa_protocol_name, environment::SharedVoterSetState};
use futures::prelude::*;
use parity_scale_codec::Encode;
use sc_network::{config::Role, Event as NetworkEvent, ObservedRole, PeerId};
@@ -97,7 +97,7 @@ impl sc_network_gossip::ValidatorContext<Block> for TestNetwork {
<Self as sc_network_gossip::Network<Block>>::write_notification(
self,
who.clone(),
GRANDPA_PROTOCOL_NAME.into(),
grandpa_protocol_name::NAME.into(),
data,
);
}
@@ -148,6 +148,7 @@ fn config() -> crate::Config {
local_role: Role::Authority,
observer_enabled: true,
telemetry: None,
protocol_name: grandpa_protocol_name::NAME.into(),
}
}
@@ -286,7 +287,7 @@ fn good_commit_leads_to_relay() {
// Add the sending peer and send the commit
let _ = sender.unbounded_send(NetworkEvent::NotificationStreamOpened {
remote: sender_id.clone(),
protocol: GRANDPA_PROTOCOL_NAME.into(),
protocol: grandpa_protocol_name::NAME.into(),
negotiated_fallback: None,
role: ObservedRole::Full,
});
@@ -294,7 +295,7 @@ fn good_commit_leads_to_relay() {
let _ = sender.unbounded_send(NetworkEvent::NotificationsReceived {
remote: sender_id.clone(),
messages: vec![(
GRANDPA_PROTOCOL_NAME.into(),
grandpa_protocol_name::NAME.into(),
commit_to_send.clone().into(),
)],
});
@@ -303,7 +304,7 @@ fn good_commit_leads_to_relay() {
let receiver_id = sc_network::PeerId::random();
let _ = sender.unbounded_send(NetworkEvent::NotificationStreamOpened {
remote: receiver_id.clone(),
protocol: GRANDPA_PROTOCOL_NAME.into(),
protocol: grandpa_protocol_name::NAME.into(),
negotiated_fallback: None,
role: ObservedRole::Full,
});
@@ -321,7 +322,10 @@ fn good_commit_leads_to_relay() {
sender.unbounded_send(NetworkEvent::NotificationsReceived {
remote: receiver_id,
messages: vec![(GRANDPA_PROTOCOL_NAME.into(), msg.encode().into())],
messages: vec![(
grandpa_protocol_name::NAME.into(),
msg.encode().into(),
)],
})
};
@@ -433,14 +437,14 @@ fn bad_commit_leads_to_report() {
Event::EventStream(sender) => {
let _ = sender.unbounded_send(NetworkEvent::NotificationStreamOpened {
remote: sender_id.clone(),
protocol: GRANDPA_PROTOCOL_NAME.into(),
protocol: grandpa_protocol_name::NAME.into(),
negotiated_fallback: None,
role: ObservedRole::Full,
});
let _ = sender.unbounded_send(NetworkEvent::NotificationsReceived {
remote: sender_id.clone(),
messages: vec![(
GRANDPA_PROTOCOL_NAME.into(),
grandpa_protocol_name::NAME.into(),
commit_to_send.clone().into(),
)],
});