remove ReportCollator message (#6628)

Closes #6415 # Description Remove unused message `ReportCollator` and test related to this message on the collator protocol validator side. cc: @tdimitrov --------- Co-authored-by: Tsvetomir Dimitrov <[email protected]> Co-authored-by: command-bot <>
paritytech · Nov 25, 2024 · 41b6915 · 41b6915
1 parent c422d8b
commit 41b6915
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 88 deletions.
diff --git a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs
@@ -899,7 +899,7 @@ async fn process_msg<Context>(
 				);
 			}
 		},
-		msg @ (ReportCollator(..) | Invalid(..) | Seconded(..)) => {
+		msg @ (Invalid(..) | Seconded(..)) => {
 			gum::warn!(
 				target: LOG_TARGET,
 				"{:?} message is not expected on the collator side of the protocol",

diff --git a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs
@@ -1462,9 +1462,6 @@ async fn process_msg<Context>(
 				"DistributeCollation message is not expected on the validator side of the protocol",
 			);
 		},
-		ReportCollator(id) => {
-			report_collator(&mut state.reputation, ctx.sender(), &state.peer_data, id).await;
-		},
 		NetworkBridgeUpdate(event) => {
 			if let Err(e) = handle_network_msg(ctx, state, keystore, event).await {
 				gum::warn!(

diff --git a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs
@@ -638,66 +638,6 @@ fn act_on_advertisement_v2() {
 	});
 }
 
-// Test that other subsystems may modify collators' reputations.
-#[test]
-fn collator_reporting_works() {
-	let test_state = TestState::default();
-
-	test_harness(ReputationAggregator::new(|_| true), |test_harness| async move {
-		let TestHarness { mut virtual_overseer, .. } = test_harness;
-
-		overseer_send(
-			&mut virtual_overseer,
-			CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange(
-				our_view![test_state.relay_parent],
-			)),
-		)
-		.await;
-
-		respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent)
-			.await;
-
-		let peer_b = PeerId::random();
-		let peer_c = PeerId::random();
-
-		connect_and_declare_collator(
-			&mut virtual_overseer,
-			peer_b,
-			test_state.collators[0].clone(),
-			test_state.chain_ids[0],
-			CollationVersion::V1,
-		)
-		.await;
-
-		connect_and_declare_collator(
-			&mut virtual_overseer,
-			peer_c,
-			test_state.collators[1].clone(),
-			test_state.chain_ids[0],
-			CollationVersion::V1,
-		)
-		.await;
-
-		overseer_send(
-			&mut virtual_overseer,
-			CollatorProtocolMessage::ReportCollator(test_state.collators[0].public()),
-		)
-		.await;
-
-		assert_matches!(
-			overseer_recv(&mut virtual_overseer).await,
-			AllMessages::NetworkBridgeTx(
-				NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(peer, rep)),
-			) => {
-				assert_eq!(peer, peer_b);
-				assert_eq!(rep.value, COST_REPORT_BAD.cost_or_benefit());
-			}
-		);
-
-		virtual_overseer
-	});
-}
-
 // Test that we verify the signatures on `Declare` and `AdvertiseCollation` messages.
 #[test]
 fn collator_authentication_verification_works() {

diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs
@@ -48,12 +48,12 @@ use polkadot_primitives::{
 		CommittedCandidateReceiptV2 as CommittedCandidateReceipt, CoreState,
 	},
 	ApprovalVotingParams, AuthorityDiscoveryId, BlockNumber, CandidateCommitments, CandidateHash,
-	CandidateIndex, CollatorId, CoreIndex, DisputeState, ExecutorParams, GroupIndex,
-	GroupRotationInfo, Hash, HeadData, Header as BlockHeader, Id as ParaId, InboundDownwardMessage,
-	InboundHrmpMessage, MultiDisputeStatementSet, NodeFeatures, OccupiedCoreAssumption,
-	PersistedValidationData, PvfCheckStatement, PvfExecKind as RuntimePvfExecKind, SessionIndex,
-	SessionInfo, SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode,
-	ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature,
+	CandidateIndex, CoreIndex, DisputeState, ExecutorParams, GroupIndex, GroupRotationInfo, Hash,
+	HeadData, Header as BlockHeader, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage,
+	MultiDisputeStatementSet, NodeFeatures, OccupiedCoreAssumption, PersistedValidationData,
+	PvfCheckStatement, PvfExecKind as RuntimePvfExecKind, SessionIndex, SessionInfo,
+	SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash,
+	ValidatorId, ValidatorIndex, ValidatorSignature,
 };
 use polkadot_statement_table::v2::Misbehavior;
 use std::{
@@ -250,9 +250,6 @@ pub enum CollatorProtocolMessage {
 		/// The core index where the candidate should be backed.
 		core_index: CoreIndex,
 	},
-	/// Report a collator as having provided an invalid collation. This should lead to disconnect
-	/// and blacklist of the collator.
-	ReportCollator(CollatorId),
 	/// Get a network bridge update.
 	#[from]
 	NetworkBridgeUpdate(NetworkBridgeEvent<net_protocol::CollatorProtocolMessage>),

diff --git a/polkadot/roadmap/implementers-guide/src/node/collators/collator-protocol.md b/polkadot/roadmap/implementers-guide/src/node/collators/collator-protocol.md
@@ -151,12 +151,6 @@ time per relay parent. This reduces the bandwidth requirements and as we can sec
 the others are probably not required anyway. If the request times out, we need to note the collator as being unreliable
 and reduce its priority relative to other collators.
 
-As a validator, once the collation has been fetched some other subsystem will inspect and do deeper validation of the
-collation. The subsystem will report to this subsystem with a [`CollatorProtocolMessage`][CPM]`::ReportCollator`. In
-that case, if we are connected directly to the collator, we apply a cost to the `PeerId` associated with the collator
-and potentially disconnect or blacklist it. If the collation is seconded, we notify the collator and apply a benefit to
-the `PeerId` associated with the collator.
-
 ### Interaction with [Candidate Backing][CB]
 
 As collators advertise the availability, a validator will simply second the first valid parablock candidate per relay

diff --git a/polkadot/roadmap/implementers-guide/src/node/subsystems-and-jobs.md b/polkadot/roadmap/implementers-guide/src/node/subsystems-and-jobs.md
@@ -129,7 +129,6 @@ digraph {
 
     cand_sel    -> coll_prot    [arrowhead = "diamond", label = "FetchCollation"]
     cand_sel    -> cand_back    [arrowhead = "onormal", label = "Second"]
-    cand_sel    -> coll_prot    [arrowhead = "onormal", label = "ReportCollator"]
 
     cand_val    -> runt_api     [arrowhead = "diamond", label = "Request::PersistedValidationData"]
     cand_val    -> runt_api     [arrowhead = "diamond", label = "Request::ValidationCode"]
@@ -231,7 +230,7 @@ sequenceDiagram
 
     VS ->> CandidateSelection: Collation
 
-    Note over CandidateSelection: Lots of other machinery in play here,<br/>but there are only three outcomes from the<br/>perspective of the `CollatorProtocol`:
+    Note over CandidateSelection: Lots of other machinery in play here,<br/>but there are only two outcomes from the<br/>perspective of the `CollatorProtocol`:
 
     alt happy path
         CandidateSelection -->> VS: FetchCollation
@@ -242,10 +241,6 @@ sequenceDiagram
         NB ->> VS: Collation
         Deactivate VS
 
-    else collation invalid or unexpected
-        CandidateSelection ->> VS: ReportCollator
-        VS ->> NB: ReportPeer
-
     else CandidateSelection already selected a different candidate
         Note over CandidateSelection: silently drop
     end

diff --git a/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md b/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md
@@ -436,9 +436,6 @@ enum CollatorProtocolMessage {
     DistributeCollation(CandidateReceipt, PoV, Option<oneshot::Sender<CollationSecondedSignal>>),
     /// Fetch a collation under the given relay-parent for the given ParaId.
     FetchCollation(Hash, ParaId, ResponseChannel<(CandidateReceipt, PoV)>),
-    /// Report a collator as having provided an invalid collation. This should lead to disconnect
-    /// and blacklist of the collator.
-    ReportCollator(CollatorId),
     /// Note a collator as having provided a good collation.
     NoteGoodCollation(CollatorId, SignedFullStatement),
     /// Notify a collator that its collation was seconded.

diff --git a/prdoc/pr_6628.prdoc b/prdoc/pr_6628.prdoc
@@ -0,0 +1,12 @@
+title: "Remove ReportCollator message"
+
+doc:
+  - audience: Node Dev
+    description: |
+      Remove unused message ReportCollator and test related to this message on the collator protocol validator side.
+
+crates:
+  - name: polkadot-node-subsystem-types
+    bump: patch
+  - name: polkadot-collator-protocol
+    bump: major