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 504b0d716043943875de9635832b473872e5fd7e..d77480272cb4f80f7190a3f8362242037baefbcb 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 86358f503d04d58d55fb8ff1f4f2df7bbdd9c4e4..36ec959c34061a93b62b23aa70ce8810c14eb649 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 7bc61dd4ebec5de5ba21c2feb60284cd74fdbdad..f2f23c188a666a113d89582fc25e645da17d3add 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 28a3a1ab82ab20bdb92e023e56a93ca12b846391..b541f9519219384b2badf03502816ca77484af25 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 432d9ab69bab99297b51fb5af285e7636e8b90ae..586a4169b5bc5c92808e32d540dfcabb06b8c6bb 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 a3ca7347eb63efedb787c90a23c8e5b95138bcdb..a96f3fa3d4a000ce3596fa0e3d4630f05f09cb73 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 85415e42a11c0aa4cefc665592937aced77bd0ed..cb862440727b6e414695e7c2c7d13ffd99a85f2c 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 0000000000000000000000000000000000000000..7ea0c4968385642328be5a285fad89caec085bd6 --- /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