From 41b6915ecb4b5691cdeeb585e26d46c4897ae151 Mon Sep 17 00:00:00 2001
From: jpserrat <35823283+jpserrat@users.noreply.github.com>
Date: Mon, 25 Nov 2024 12:54:04 -0300
Subject: [PATCH] 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 <tsvetomir@parity.io>
Co-authored-by: command-bot <>
---
.../src/collator_side/mod.rs | 2 +-
.../src/validator_side/mod.rs | 3 -
.../src/validator_side/tests/mod.rs | 60 -------------------
polkadot/node/subsystem-types/src/messages.rs | 15 ++---
.../src/node/collators/collator-protocol.md | 6 --
.../src/node/subsystems-and-jobs.md | 7 +--
.../src/types/overseer-protocol.md | 3 -
prdoc/pr_6628.prdoc | 12 ++++
8 files changed, 20 insertions(+), 88 deletions(-)
create mode 100644 prdoc/pr_6628.prdoc
diff --git a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs
index 504b0d71604..d77480272cb 100644
--- 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
index 86358f503d0..36ec959c340 100644
--- 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
index 7bc61dd4ebe..f2f23c188a6 100644
--- 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
index 28a3a1ab82a..b541f951921 100644
--- 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
index 432d9ab69ba..586a4169b5b 100644
--- 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
index a3ca7347eb6..a96f3fa3d4a 100644
--- 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
index 85415e42a11..cb862440727 100644
--- 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
new file mode 100644
index 00000000000..7ea0c496838
--- /dev/null
+++ 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
\ No newline at end of file
--
GitLab