Unverified Commit 06040c81 authored by Lldenaurois's avatar Lldenaurois Committed by GitHub
Browse files

Follow-up PR: Count no-shows (#3309)

* node/approval-voting: test for invalid validator index in assignments

This commit adds a unit test to show that, currently, validator indexes
greater than n_validators (or the length of the approvals bitvector) are
counted in n_assignments. In the subsequent commit we will correct this
behavior.

* node/approval-voting: ignore invalid validator indexes in n_assignments

This commit ignores any validator assignments whose index is beyond
n_validators. Without this check, an improperly crafted assignment would
be counted towards the approval.

It still remains that n_assignments and count_no_shows inspect the
number of validators and approvals, respectively. Ideally we would
add greater safety around ensuring these two values cannot differ.
parent f223297b
Pipeline #143174 passed with stages
in 35 minutes and 23 seconds
......@@ -385,7 +385,10 @@ pub fn tranches_to_approve(
return None;
}
let n_assignments = assignments.len();
// Count the number of valid validator assignments.
let n_assignments = assignments.iter()
.filter(|(v_index, _)| v_index.0 < n_validators as u32)
.count();
// count no-shows. An assignment is a no-show if there is no corresponding approval vote
// after a fixed duration.
......@@ -982,6 +985,59 @@ mod tests {
);
}
#[test]
fn validator_indexes_out_of_range_are_ignored_in_assignments() {
let block_tick = 20;
let no_show_duration = 10;
let needed_approvals = 3;
let mut candidate: CandidateEntry = approval_db::v1::CandidateEntry {
candidate: Default::default(),
session: 0,
block_assignments: Default::default(),
approvals: bitvec![BitOrderLsb0, u8; 0; 3],
}.into();
for i in 0..3 {
candidate.mark_approval(ValidatorIndex(i));
}
let approval_entry = approval_db::v1::ApprovalEntry {
tranches: vec![
// Assignments with invalid validator indexes.
approval_db::v1::TrancheEntry {
tranche: 1,
assignments: (2..5).map(|i| (ValidatorIndex(i), 1.into())).collect(),
},
],
assignments: bitvec![BitOrderLsb0, u8; 1; 3],
our_assignment: None,
our_approval_sig: None,
backing_group: GroupIndex(0),
approved: false,
}.into();
let approvals = bitvec![BitOrderLsb0, u8; 0; 3];
let tranche_now = 10;
assert_eq!(
tranches_to_approve(
&approval_entry,
&approvals,
tranche_now,
block_tick,
no_show_duration,
needed_approvals,
),
RequiredTranches::Pending {
considered: 10,
next_no_show: None,
maximum_broadcast: DelayTranche::max_value(),
clock_drift: 0,
},
);
}
#[test]
fn filled_tranche_iterator_yields_sequential_tranches() {
const PREFIX: u32 = 10;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment