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 d064e987cd28..69afda8ef087 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -696,7 +696,7 @@ async fn advertise_collation( .should_advertise_to(candidate_hash, peer_ids, &peer); match should_advertise { - ShouldAdvertiseTo::Yes => {} + ShouldAdvertiseTo::Yes => {}, ShouldAdvertiseTo::NotAuthority => { gum::trace!( target: LOG_TARGET, @@ -706,7 +706,7 @@ async fn advertise_collation( "Not advertising collation: not relevant to peer" ); continue - } + }, ShouldAdvertiseTo::AlreadyAdvertised => { gum::debug!( target: LOG_TARGET, @@ -716,7 +716,7 @@ async fn advertise_collation( "Not advertising collation: already advertised" ); continue - } + }, } gum::debug!( diff --git a/polkadot/node/network/statement-distribution/src/vstaging/tests/requests.rs b/polkadot/node/network/statement-distribution/src/vstaging/tests/requests.rs index 5eef5809b4d4..ee35623c060b 100644 --- a/polkadot/node/network/statement-distribution/src/vstaging/tests/requests.rs +++ b/polkadot/node/network/statement-distribution/src/vstaging/tests/requests.rs @@ -1323,6 +1323,208 @@ fn local_node_sanity_checks_incoming_requests() { }); } +#[test] +fn local_node_checks_that_peer_can_request_before_responding() { + let config = TestConfig { + validator_count: 20, + group_size: 3, + local_validator: true, + async_backing_params: None, + }; + + let relay_parent = Hash::repeat_byte(1); + let peer_a = PeerId::random(); + let peer_b = PeerId::random(); + + test_harness(config, |mut state, mut overseer| async move { + let local_validator = state.local.clone().unwrap(); + let local_para = ParaId::from(local_validator.group_index.0); + + let test_leaf = state.make_dummy_leaf(relay_parent); + + let (candidate, pvd) = make_candidate( + relay_parent, + 1, + local_para, + test_leaf.para_data(local_para).head_data.clone(), + vec![4, 5, 6].into(), + Hash::repeat_byte(42).into(), + ); + let candidate_hash = candidate.hash(); + + // Peers A and B are in group and have relay parent in view. + let other_group_validators = state.group_validators(local_validator.group_index, true); + + connect_peer( + &mut overseer, + peer_a.clone(), + Some(vec![state.discovery_id(other_group_validators[0])].into_iter().collect()), + ) + .await; + + connect_peer( + &mut overseer, + peer_b.clone(), + Some(vec![state.discovery_id(other_group_validators[1])].into_iter().collect()), + ) + .await; + let peer_b_index = other_group_validators[1]; + + send_peer_view_change(&mut overseer, peer_a.clone(), view![relay_parent]).await; + send_peer_view_change(&mut overseer, peer_b.clone(), view![relay_parent]).await; + + // Finish setup + activate_leaf(&mut overseer, &test_leaf, &state, true).await; + + answer_expected_hypothetical_depth_request( + &mut overseer, + vec![], + Some(relay_parent), + false, + ) + .await; + + let mask = StatementFilter::blank(state.config.group_size); + + // Confirm candidate. + let signed = state.sign_statement( + local_validator.validator_index, + CompactStatement::Seconded(candidate_hash), + &SigningContext { session_index: 1, parent_hash: relay_parent }, + ); + let full_signed = signed + .clone() + .convert_to_superpayload(StatementWithPVD::Seconded(candidate.clone(), pvd.clone())) + .unwrap(); + + overseer + .send(FromOrchestra::Communication { + msg: StatementDistributionMessage::Share(relay_parent, full_signed), + }) + .await; + + assert_matches!( + overseer.recv().await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::VStaging(protocol_vstaging::ValidationProtocol::StatementDistribution( + protocol_vstaging::StatementDistributionMessage::Statement( + r, + s, + ) + )) + )) => { + assert_eq!(peers, vec![peer_a.clone(), peer_b.clone()]); + assert_eq!(r, relay_parent); + assert_eq!(s.unchecked_payload(), &CompactStatement::Seconded(candidate_hash)); + assert_eq!(s.unchecked_validator_index(), local_validator.validator_index); + } + ); + + answer_expected_hypothetical_depth_request(&mut overseer, vec![], None, false).await; + + // Local node should respond to requests from peers in the same group + // which appear to not have already seen the candidate + { + // Peer requests candidate and local responds + let response = state + .send_request( + peer_a, + request_vstaging::AttestedCandidateRequest { + candidate_hash: candidate.hash(), + mask: mask.clone(), + }, + ) + .await + .await; + + let expected_statements = vec![signed.into_unchecked()]; + assert_matches!(response, full_response => { + // Response is the same for vstaging. + let request_vstaging::AttestedCandidateResponse { candidate_receipt, persisted_validation_data, statements } = + request_vstaging::AttestedCandidateResponse::decode( + &mut full_response.result.expect("We should have a proper answer").as_ref(), + ).expect("Decoding should work"); + assert_eq!(candidate_receipt, candidate); + assert_eq!(persisted_validation_data, pvd); + assert_eq!(statements, expected_statements); + }); + } + + // Local node should reject requests if the requester appears to know + // the candidate (has sent them a Seconded statement) + { + let statement = state + .sign_statement( + peer_b_index, + CompactStatement::Seconded(candidate_hash), + &SigningContext { parent_hash: relay_parent, session_index: 1 }, + ) + .as_unchecked() + .clone(); + + send_peer_message( + &mut overseer, + peer_b.clone(), + protocol_vstaging::StatementDistributionMessage::Statement(relay_parent, statement), + ) + .await; + + assert_matches!( + overseer.recv().await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(p, r))) + if p == peer_b && r == BENEFIT_VALID_STATEMENT_FIRST.into() => { } + ); + + let response = state + .send_request( + peer_b, + request_vstaging::AttestedCandidateRequest { + candidate_hash: candidate.hash(), + mask: mask.clone(), + }, + ) + .await + .await; + + // Peer already knows about this candidate. Should reject. + assert_matches!( + response, + RawOutgoingResponse { + result, + reputation_changes, + sent_feedback + } => { + assert_matches!(result, Err(())); + assert_eq!(reputation_changes, vec![COST_UNEXPECTED_REQUEST.into()]); + assert_matches!(sent_feedback, None); + } + ); + + // Handling leftover statement distribution message + assert_matches!( + overseer.recv().await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::VStaging(protocol_vstaging::ValidationProtocol::StatementDistribution( + protocol_vstaging::StatementDistributionMessage::Statement( + r, + s, + ) + )) + )) => { + assert_eq!(peers, vec![peer_a.clone()]); + assert_eq!(r, relay_parent); + assert_eq!(s.unchecked_payload(), &CompactStatement::Seconded(candidate_hash)); + assert_eq!(s.unchecked_validator_index(), peer_b_index); + } + ); + } + + overseer + }); +} + #[test] fn local_node_respects_statement_mask() { let validator_count = 6;