Skip to content

Commit

Permalink
ignore disabled statements on responses
Browse files Browse the repository at this point in the history
  • Loading branch information
@ordian
ordian committed Oct 12, 2023
1 parent 3b7fd1a commit 9f6736d
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 23 deletions.
3 changes: 3 additions & 0 deletions polkadot/node/network/statement-distribution/src/v2/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2622,6 +2622,8 @@ pub(crate) async fn handle_response<Context>(
Some(g) => g,
};

let disabled_mask = relay_parent_state.statement_store.disabled_bitmask(group);

let res = response.validate_response(
&mut state.request_manager,
group,
Expand All @@ -2636,6 +2638,7 @@ pub(crate) async fn handle_response<Context>(

Some(g_index) == expected_group
},
disabled_mask,
);

for (peer, rep) in res.reputation_changes {
Expand Down
40 changes: 36 additions & 4 deletions polkadot/node/network/statement-distribution/src/v2/requests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@
//! (which requires state not owned by the request manager).

use super::{
BENEFIT_VALID_RESPONSE, BENEFIT_VALID_STATEMENT, COST_IMPROPERLY_DECODED_RESPONSE,
COST_INVALID_RESPONSE, COST_INVALID_SIGNATURE, COST_UNREQUESTED_RESPONSE_STATEMENT,
REQUEST_RETRY_DELAY,
BENEFIT_VALID_RESPONSE, BENEFIT_VALID_STATEMENT, COST_DISABLED_VALIDATOR,
COST_IMPROPERLY_DECODED_RESPONSE, COST_INVALID_RESPONSE, COST_INVALID_SIGNATURE,
COST_UNREQUESTED_RESPONSE_STATEMENT, REQUEST_RETRY_DELAY,
};
use crate::LOG_TARGET;

use bitvec::prelude::{BitVec, Lsb0};
use polkadot_node_network_protocol::{
request_response::{
outgoing::{Recipient as RequestRecipient, RequestError},
Expand Down Expand Up @@ -510,6 +511,7 @@ impl UnhandledResponse {
/// * If the response is partially valid, misbehavior by the responding peer.
/// * If there are other peers which have advertised the same candidate for different
/// relay-parents or para-ids, misbehavior reports for those peers will also be generated.
/// * Statements from disabled validators are filtered out and reported as misbehavior.
///
/// Finally, in the case that the response is either valid or partially valid,
/// this will clean up all remaining requests for the candidate in the manager.
Expand All @@ -524,6 +526,7 @@ impl UnhandledResponse {
session: SessionIndex,
validator_key_lookup: impl Fn(ValidatorIndex) -> Option<ValidatorId>,
allowed_para_lookup: impl Fn(ParaId, GroupIndex) -> bool,
disabled_mask: BitVec<u8, Lsb0>,
) -> ResponseValidationOutput {
let UnhandledResponse {
response: TaggedResponse { identifier, requested_peer, props, response },
Expand Down Expand Up @@ -600,6 +603,7 @@ impl UnhandledResponse {
session,
validator_key_lookup,
allowed_para_lookup,
disabled_mask,
);

if let CandidateRequestStatus::Complete { .. } = output.request_status {
Expand All @@ -619,10 +623,11 @@ fn validate_complete_response(
session: SessionIndex,
validator_key_lookup: impl Fn(ValidatorIndex) -> Option<ValidatorId>,
allowed_para_lookup: impl Fn(ParaId, GroupIndex) -> bool,
mut disabled_mask: BitVec<u8, Lsb0>,
) -> ResponseValidationOutput {
let RequestProperties { backing_threshold, mut unwanted_mask } = props;

// sanity check bitmask size. this is based entirely on
// sanity check bitmasks size. this is based entirely on
// local logic here.
if !unwanted_mask.has_len(group.len()) {
gum::error!(
Expand All @@ -635,6 +640,16 @@ fn validate_complete_response(
unwanted_mask.seconded_in_group.resize(group.len(), true);
unwanted_mask.validated_in_group.resize(group.len(), true);
}
if disabled_mask.len() != group.len() {
gum::error!(
target: LOG_TARGET,
group_len = group.len(),
"Logic bug: group size != disabled bitmask len"
);

// resize and attempt to continue.
disabled_mask.resize(group.len(), false);
}

let invalid_candidate_output = || ResponseValidationOutput {
request_status: CandidateRequestStatus::Incomplete,
Expand Down Expand Up @@ -712,6 +727,11 @@ fn validate_complete_response(
rep_changes.push((requested_peer, COST_UNREQUESTED_RESPONSE_STATEMENT));
continue
}

if disabled_mask[i] {
rep_changes.push((requested_peer, COST_DISABLED_VALIDATOR));
continue
}
},
CompactStatement::Valid(_) => {
if unwanted_mask.validated_in_group[i] {
Expand All @@ -723,6 +743,11 @@ fn validate_complete_response(
rep_changes.push((requested_peer, COST_UNREQUESTED_RESPONSE_STATEMENT));
continue
}

if disabled_mask[i] {
rep_changes.push((requested_peer, COST_DISABLED_VALIDATOR));
continue
}
},
}

Expand Down Expand Up @@ -988,6 +1013,7 @@ mod tests {
let group = &[ValidatorIndex(0), ValidatorIndex(1), ValidatorIndex(2)];

let unwanted_mask = StatementFilter::blank(group_size);
let disabled_mask = unwanted_mask.seconded_in_group.clone();
let request_properties = RequestProperties { unwanted_mask, backing_threshold: None };

// Get requests.
Expand Down Expand Up @@ -1031,6 +1057,7 @@ mod tests {
0,
validator_key_lookup,
allowed_para_lookup,
disabled_mask.clone(),
);
assert_eq!(
output,
Expand Down Expand Up @@ -1069,6 +1096,7 @@ mod tests {
0,
validator_key_lookup,
allowed_para_lookup,
disabled_mask,
);
assert_eq!(
output,
Expand Down Expand Up @@ -1108,6 +1136,7 @@ mod tests {
let group = &[ValidatorIndex(0), ValidatorIndex(1), ValidatorIndex(2)];

let unwanted_mask = StatementFilter::blank(group_size);
let disabled_mask = unwanted_mask.seconded_in_group.clone();
let request_properties = RequestProperties { unwanted_mask, backing_threshold: None };
let peer_advertised =
|_identifier: &CandidateIdentifier, _peer: &_| Some(StatementFilter::full(group_size));
Expand Down Expand Up @@ -1148,6 +1177,7 @@ mod tests {
0,
validator_key_lookup,
allowed_para_lookup,
disabled_mask,
);
assert_eq!(
output,
Expand Down Expand Up @@ -1188,6 +1218,7 @@ mod tests {
let group = &[ValidatorIndex(0), ValidatorIndex(1), ValidatorIndex(2)];

let unwanted_mask = StatementFilter::blank(group_size);
let disabled_mask = unwanted_mask.seconded_in_group.clone();
let request_properties = RequestProperties { unwanted_mask, backing_threshold: None };
let peer_advertised =
|_identifier: &CandidateIdentifier, _peer: &_| Some(StatementFilter::full(group_size));
Expand Down Expand Up @@ -1226,6 +1257,7 @@ mod tests {
0,
validator_key_lookup,
allowed_para_lookup,
disabled_mask,
);
assert_eq!(
output,
Expand Down
121 changes: 102 additions & 19 deletions polkadot/node/network/statement-distribution/src/v2/tests/requests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1111,20 +1111,22 @@ fn peer_reported_for_providing_statement_from_disabled_validator() {
};

let relay_parent = Hash::repeat_byte(1);
let peer_a = PeerId::random();
let peer_disabled = PeerId::random();
let peer_b = PeerId::random();

test_harness(config, |state, mut overseer| async move {
let local_validator = state.local.clone().unwrap();
let local_para = ParaId::from(local_validator.group_index.0);

let other_group_validators = state.group_validators(local_validator.group_index, true);
let v_a = other_group_validators[0];
let index_disabled = other_group_validators[0];
let index_b = other_group_validators[1];

let disabled_validators = vec![v_a];
let disabled_validators = vec![index_disabled];
let test_leaf =
state.make_dummy_leaf_with_disabled_validators(relay_parent, disabled_validators);

let (candidate, _) = make_candidate(
let (candidate, pvd) = make_candidate(
relay_parent,
1,
local_para,
Expand All @@ -1134,15 +1136,23 @@ fn peer_reported_for_providing_statement_from_disabled_validator() {
);
let candidate_hash = candidate.hash();

// peer A is in group, has relay parent in view.
// peer A is in group, has relay parent in view and disabled.
// peer B is in group, has relay parent in view.
{
connect_peer(
&mut overseer,
peer_a.clone(),
Some(vec![state.discovery_id(v_a)].into_iter().collect()),
peer_disabled.clone(),
Some(vec![state.discovery_id(index_disabled)].into_iter().collect()),
)
.await;
send_peer_view_change(&mut overseer, peer_a.clone(), view![relay_parent]).await;
connect_peer(
&mut overseer,
peer_b.clone(),
Some(vec![state.discovery_id(index_b)].into_iter().collect()),
)
.await;
send_peer_view_change(&mut overseer, peer_disabled.clone(), view![relay_parent]).await;
send_peer_view_change(&mut overseer, peer_b.clone(), view![relay_parent]).await;
}

activate_leaf(&mut overseer, &test_leaf, &state, true).await;
Expand All @@ -1155,31 +1165,104 @@ fn peer_reported_for_providing_statement_from_disabled_validator() {
)
.await;

let seconded_disabled = state
.sign_statement(
index_disabled,
CompactStatement::Seconded(candidate_hash),
&SigningContext { parent_hash: relay_parent, session_index: 1 },
)
.as_unchecked()
.clone();

let seconded_b = state
.sign_statement(
index_b,
CompactStatement::Seconded(candidate_hash),
&SigningContext { parent_hash: relay_parent, session_index: 1 },
)
.as_unchecked()
.clone();
{
let a_seconded_disabled = state
.sign_statement(
v_a,
CompactStatement::Seconded(candidate_hash),
&SigningContext { parent_hash: relay_parent, session_index: 1 },
)
.as_unchecked()
.clone();
send_peer_message(
&mut overseer,
peer_disabled.clone(),
protocol_v2::StatementDistributionMessage::Statement(
relay_parent,
seconded_disabled.clone(),
),
)
.await;

assert_matches!(
overseer.recv().await,
AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(p, r)))
if p == peer_disabled && r == COST_DISABLED_VALIDATOR.into() => { }
);
}

{
send_peer_message(
&mut overseer,
peer_a.clone(),
peer_b.clone(),
protocol_v2::StatementDistributionMessage::Statement(
relay_parent,
a_seconded_disabled,
seconded_b.clone(),
),
)
.await;

assert_matches!(
overseer.recv().await,
AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(p, r)))
if p == peer_a && r == COST_DISABLED_VALIDATOR.into() => { }
if p == peer_b && r == BENEFIT_VALID_STATEMENT_FIRST.into() => { }
);
}

// Send a request to peer and mock its response with a statement from disabled validator.
{
let statements = vec![seconded_disabled];

handle_sent_request(
&mut overseer,
peer_b,
candidate_hash,
StatementFilter::blank(group_size),
candidate.clone(),
pvd.clone(),
statements,
)
.await;

assert_matches!(
overseer.recv().await,
AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(p, r)))
if p == peer_b && r == COST_DISABLED_VALIDATOR.into() => { }
);

assert_matches!(
overseer.recv().await,
AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(p, r)))
if p == peer_b && r == BENEFIT_VALID_RESPONSE.into() => { }
);

assert_matches!(
overseer.recv().await,
AllMessages:: NetworkBridgeTx(
NetworkBridgeTxMessage::SendValidationMessage(
peers,
Versioned::V2(
protocol_v2::ValidationProtocol::StatementDistribution(
protocol_v2::StatementDistributionMessage::Statement(hash, statement),
),
),
)
) => {
assert_eq!(peers, vec![peer_disabled]);
assert_eq!(hash, relay_parent);
assert_eq!(statement, seconded_b);
}
);
answer_expected_hypothetical_depth_request(&mut overseer, vec![], None, false).await;
}

overseer
Expand Down

0 comments on commit 9f6736d

Please sign in to comment.