Make ProtocolVersion a strongly typed struct (part 2)

paritytech · dmitry-markin · Aug 30, 2022 · Aug 12, 2022 · Aug 13, 2022 · Aug 18, 2022
commit a9091a0e66bd33a56634188eeb7763be9ccebb7e
diff --git a/node/network/approval-distribution/src/tests.rs b/node/network/approval-distribution/src/tests.rs
@@ -17,7 +17,7 @@
 use super::*;
 use assert_matches::assert_matches;
 use futures::{executor, future, Future};
-use polkadot_node_network_protocol::{our_view, view, ObservedRole};
+use polkadot_node_network_protocol::{our_view, peer_set::ValidationVersion, view, ObservedRole};
 use polkadot_node_primitives::approval::{
 	AssignmentCertKind, VRFOutput, VRFProof, RELAY_VRF_MODULO_CONTEXT,
 };
@@ -174,7 +174,7 @@ async fn setup_peer_with_view(
 		ApprovalDistributionMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected(
 			peer_id.clone(),
 			ObservedRole::Full,
-			1,
+			ValidationVersion::V1.into(),
 			None,
 		)),
 	)

diff --git a/node/network/bitfield-distribution/src/tests.rs b/node/network/bitfield-distribution/src/tests.rs
@@ -20,7 +20,8 @@ use bitvec::bitvec;
 use futures::executor;
 use maplit::hashmap;
 use polkadot_node_network_protocol::{
-	grid_topology::SessionBoundGridTopologyStorage, our_view, view, ObservedRole,
+	grid_topology::SessionBoundGridTopologyStorage, our_view, peer_set::ValidationVersion, view,
+	ObservedRole,
 };
 use polkadot_node_subsystem::{
 	jaeger,
@@ -568,7 +569,12 @@ fn changing_view() {
 			&mut ctx,
 			&mut state,
 			&Default::default(),
-			NetworkBridgeEvent::PeerConnected(peer_b.clone(), ObservedRole::Full, 1, None),
+			NetworkBridgeEvent::PeerConnected(
+				peer_b.clone(),
+				ObservedRole::Full,
+				ValidationVersion::V1.into(),
+				None
+			),
 			&mut rng,
 		));
 

diff --git a/node/network/bridge/src/lib.rs b/node/network/bridge/src/lib.rs
@@ -28,7 +28,8 @@ use parking_lot::Mutex;
 use sp_consensus::SyncOracle;
 
 use polkadot_node_network_protocol::{
-	peer_set::PeerSet, PeerId, ProtocolVersion, UnifiedReputationChange as Rep, View,
+	peer_set::{PeerSet, ProtocolVersion},
+	PeerId, UnifiedReputationChange as Rep, View,
 };
 
 /// Peer set info for network initialization.

diff --git a/node/network/bridge/src/network.rs b/node/network/bridge/src/network.rs
@@ -30,9 +30,9 @@ use sc_network_common::service::{
 };
 
 use polkadot_node_network_protocol::{
-	peer_set::{PeerSet, PeerSetProtocolNames},
+	peer_set::{PeerSet, PeerSetProtocolNames, ProtocolVersion},
 	request_response::{OutgoingRequest, Recipient, ReqProtocolNames, Requests},
-	PeerId, ProtocolVersion, UnifiedReputationChange as Rep,
+	PeerId, UnifiedReputationChange as Rep,
 };
 use polkadot_primitives::v2::{AuthorityDiscoveryId, Block, Hash};
 

diff --git a/node/network/bridge/src/rx/mod.rs b/node/network/bridge/src/rx/mod.rs
@@ -27,9 +27,11 @@ use sp_consensus::SyncOracle;
 
 use polkadot_node_network_protocol::{
 	self as net_protocol,
-	peer_set::{CollationVersion, PeerSet, PeerSetProtocolNames, PerPeerSet, ValidationVersion},
-	v1 as protocol_v1, ObservedRole, OurView, PeerId, ProtocolVersion,
-	UnifiedReputationChange as Rep, View,
+	peer_set::{
+		CollationVersion, PeerSet, PeerSetProtocolNames, PerPeerSet, ProtocolVersion,
+		ValidationVersion,
+	},
+	v1 as protocol_v1, ObservedRole, OurView, PeerId, UnifiedReputationChange as Rep, View,
 };
 
 use polkadot_node_subsystem::{
@@ -221,7 +223,7 @@ where
 					target: LOG_TARGET,
 					action = "PeerConnected",
 					peer_set = ?peer_set,
-					version,
+					version = u32::from(version),
 					peer = ?peer,
 					role = ?role
 				);
@@ -256,7 +258,7 @@ where
 								NetworkBridgeEvent::PeerConnected(
 									peer.clone(),
 									role,
-									1,
+									version,
 									maybe_authority,
 								),
 								NetworkBridgeEvent::PeerViewChange(peer.clone(), View::default()),
@@ -281,7 +283,7 @@ where
 								NetworkBridgeEvent::PeerConnected(
 									peer.clone(),
 									role,
-									1,
+									version,
 									maybe_authority,
 								),
 								NetworkBridgeEvent::PeerViewChange(peer.clone(), View::default()),
@@ -437,7 +439,9 @@ where
 
 				if !v_messages.is_empty() {
 					let (events, reports) =
-						if expected_versions[PeerSet::Validation] == Some(1) {
+						if expected_versions[PeerSet::Validation] ==
+							Some(ValidationVersion::V1.into())
+						{
 							handle_v1_peer_messages::<protocol_v1::ValidationProtocol, _>(
 								remote.clone(),
 								PeerSet::Validation,
@@ -468,7 +472,9 @@ where
 
 				if !c_messages.is_empty() {
 					let (events, reports) =
-						if expected_versions[PeerSet::Collation] == Some(1) {
+						if expected_versions[PeerSet::Collation] ==
+							Some(CollationVersion::V1.into())
+						{
 							handle_v1_peer_messages::<protocol_v1::CollationProtocol, _>(
 								remote.clone(),
 								PeerSet::Collation,

diff --git a/node/network/bridge/src/rx/tests.rs b/node/network/bridge/src/rx/tests.rs
@@ -673,7 +673,12 @@ fn peer_view_updates_sent_via_overseer() {
 		// bridge will inform about all connected peers.
 		{
 			assert_sends_validation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -716,7 +721,12 @@ fn peer_messages_sent_via_overseer() {
 		// bridge will inform about all connected peers.
 		{
 			assert_sends_validation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -786,7 +796,12 @@ fn peer_disconnect_from_just_one_peerset() {
 		// bridge will inform about all connected peers.
 		{
 			assert_sends_validation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -800,7 +815,12 @@ fn peer_disconnect_from_just_one_peerset() {
 
 		{
 			assert_sends_collation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -869,7 +889,12 @@ fn relays_collation_protocol_messages() {
 		// bridge will inform about all connected peers.
 		{
 			assert_sends_validation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer_a.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer_a.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -883,7 +908,12 @@ fn relays_collation_protocol_messages() {
 
 		{
 			assert_sends_collation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer_b.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer_b.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -962,7 +992,12 @@ fn different_views_on_different_peer_sets() {
 		// bridge will inform about all connected peers.
 		{
 			assert_sends_validation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;
@@ -976,7 +1011,12 @@ fn different_views_on_different_peer_sets() {
 
 		{
 			assert_sends_collation_event_to_all(
-				NetworkBridgeEvent::PeerConnected(peer.clone(), ObservedRole::Full, 1, None),
+				NetworkBridgeEvent::PeerConnected(
+					peer.clone(),
+					ObservedRole::Full,
+					ValidationVersion::V1.into(),
+					None,
+				),
 				&mut virtual_overseer,
 			)
 			.await;

diff --git a/node/network/bridge/src/tx/mod.rs b/node/network/bridge/src/tx/mod.rs
@@ -18,7 +18,7 @@
 use super::*;
 
 use polkadot_node_network_protocol::{
-	peer_set::{PeerSet, PeerSetProtocolNames},
+	peer_set::{CollationVersion, PeerSet, PeerSetProtocolNames, ValidationVersion},
 	request_response::ReqProtocolNames,
 	v1 as protocol_v1, PeerId, Versioned,
 };
@@ -164,6 +164,7 @@ where
 				peer_set = ?peer_set,
 			);
 
+			// [`NetworkService`] keeps track of the protocols by their main name.
 			let protocol = peerset_protocol_names.get_main_name(peer_set);
 			network_service.disconnect_peer(peer, protocol);
 		},
@@ -338,7 +339,15 @@ fn send_validation_message_v1(
 	message: WireMessage<protocol_v1::ValidationProtocol>,
 	metrics: &Metrics,
 ) {
-	send_message(net, peers, PeerSet::Validation, 1, protocol_names, message, metrics);
+	send_message(
+		net,
+		peers,
+		PeerSet::Validation,
+		ValidationVersion::V1.into(),
+		protocol_names,
+		message,
+		metrics,
+	);
 }
 
 fn send_collation_message_v1(
@@ -348,5 +357,13 @@ fn send_collation_message_v1(
 	message: WireMessage<protocol_v1::CollationProtocol>,
 	metrics: &Metrics,
 ) {
-	send_message(net, peers, PeerSet::Collation, 1, protocol_names, message, metrics);
+	send_message(
+		net,
+		peers,
+		PeerSet::Collation,
+		CollationVersion::V1.into(),
+		protocol_names,
+		message,
+		metrics,
+	);
 }
diff --git a/node/network/collator-protocol/src/collator_side/tests.rs b/node/network/collator-protocol/src/collator_side/tests.rs
@@ -31,6 +31,7 @@ use sp_runtime::traits::AppVerify;
 
 use polkadot_node_network_protocol::{
 	our_view,
+	peer_set::CollationVersion,
 	request_response::{IncomingRequest, ReqProtocolNames},
 	view,
 };
@@ -399,7 +400,7 @@ async fn connect_peer(
 		CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected(
 			peer.clone(),
 			polkadot_node_network_protocol::ObservedRole::Authority,
-			1,
+			CollationVersion::V1.into(),
 			authority_id.map(|v| HashSet::from([v])),
 		)),
 	)

diff --git a/node/network/collator-protocol/src/validator_side/tests.rs b/node/network/collator-protocol/src/validator_side/tests.rs
@@ -24,6 +24,7 @@ use std::{iter, sync::Arc, time::Duration};
 
 use polkadot_node_network_protocol::{
 	our_view,
+	peer_set::CollationVersion,
 	request_response::{Requests, ResponseSender},
 	ObservedRole,
 };
@@ -306,7 +307,7 @@ async fn connect_and_declare_collator(
 		CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected(
 			peer.clone(),
 			ObservedRole::Full,
-			1,
+			CollationVersion::V1.into(),
 			None,
 		)),
 	)
@@ -458,7 +459,7 @@ fn collator_authentication_verification_works() {
 			CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected(
 				peer_b,
 				ObservedRole::Full,
-				1,
+				CollationVersion::V1.into(),
 				None,
 			)),
 		)
@@ -946,7 +947,7 @@ fn disconnect_if_no_declare() {
 			CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected(
 				peer_b.clone(),
 				ObservedRole::Full,
-				1,
+				CollationVersion::V1.into(),
 				None,
 			)),
 		)
@@ -984,7 +985,7 @@ fn disconnect_if_wrong_declare() {
 			CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected(
 				peer_b.clone(),
 				ObservedRole::Full,
-				1,
+				CollationVersion::V1.into(),
 				None,
 			)),
 		)

diff --git a/node/network/protocol/src/peer_set.rs b/node/network/protocol/src/peer_set.rs
@@ -30,7 +30,7 @@ use strum::{EnumIter, IntoEnumIterator};
 const LEGACY_VALIDATION_PROTOCOL_V1: &str = "/polkadot/validation/1";
 const LEGACY_COLLATION_PROTOCOL_V1: &str = "/polkadot/collation/1";
 
-/// The legacy prtocol version. Is always 1.
+/// The legacy protocol version. Is always 1 for both validation & collation.
 const LEGACY_PROTOCOL_VERSION_V1: u32 = 1;
 
 /// Max notification size is currently constant.
@@ -196,6 +196,12 @@ pub fn peer_sets_info(
 #[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash)]
 pub struct ProtocolVersion(u32);
 
+impl From<ProtocolVersion> for u32 {
+	fn from(version: ProtocolVersion) -> u32 {
+		version.0
+	}
+}
+
 /// Supported validation protocol versions. Only versions defined here must be used in the codebase.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EnumIter)]
 pub enum ValidationVersion {