diff --git a/primitives/npos-elections/fuzzer/src/phragmen_pjr.rs b/primitives/npos-elections/fuzzer/src/phragmen_pjr.rs index 9727d1406ad24..49794f21fb256 100644 --- a/primitives/npos-elections/fuzzer/src/phragmen_pjr.rs +++ b/primitives/npos-elections/fuzzer/src/phragmen_pjr.rs @@ -112,7 +112,7 @@ fn iteration(mut candidate_count: usize, mut voter_count: usize, seed: u64) { let threshold = standard_threshold(rounds, voters.iter().map(|voter| voter.budget())); assert!( - pjr_check_core(&candidates, &voters, threshold), + pjr_check_core(&candidates, &voters, threshold).is_ok(), "unbalanced sequential phragmen must satisfy PJR", ); } diff --git a/primitives/npos-elections/src/pjr.rs b/primitives/npos-elections/src/pjr.rs index 61e0b2deb79fc..6caed9059e875 100644 --- a/primitives/npos-elections/src/pjr.rs +++ b/primitives/npos-elections/src/pjr.rs @@ -74,7 +74,7 @@ pub fn pjr_check( supports: &Supports, all_candidates: Vec, all_voters: Vec<(AccountId, VoteWeight, Vec)>, -) -> bool { +) -> Result<(), AccountId> { let t = standard_threshold(supports.len(), all_voters.iter().map(|voter| voter.1 as ExtendedBalance)); t_pjr_check(supports, all_candidates, all_voters, t) } @@ -119,7 +119,7 @@ pub fn t_pjr_check( all_candidates: Vec, all_voters: Vec<(AccountId, VoteWeight, Vec)>, t: Threshold, -) -> bool { +) -> Result<(), AccountId> { // First order of business: derive `(candidates, voters)` from `supports`. let (candidates, voters) = prepare_pjr_input( supports, @@ -133,18 +133,99 @@ pub fn t_pjr_check( /// The internal implementation of the PJR check after having the data converted. /// /// [`pjr_check`] or [`t_pjr_check`] are typically easier to work with. +/// +/// This function returns an `AccountId` in the `Err` case. This is the counter_example: the ID of the +/// unelected candidate with the highest prescore, such that `pre_score(counter_example) >= t`. pub fn pjr_check_core( candidates: &[CandidatePtr], voters: &[Voter], t: Threshold, -) -> bool { +) -> Result<(), AccountId> { let unelected = candidates.iter().filter(|c| !c.borrow().elected); let maybe_max_pre_score = unelected.map(|c| (pre_score(Rc::clone(c), voters, t), c.borrow().who.clone())).max(); // if unelected is empty then the solution is indeed PJR. - maybe_max_pre_score.map_or(true, |(max_pre_score, _)| max_pre_score < t) + match maybe_max_pre_score { + Some((max_pre_score, counter_example)) if max_pre_score >= t => Err(counter_example), + _ => Ok(()), + } } +/// Validate a challenge to an election result. +/// +/// A challenge to an election result is valid if there exists some counter_example for which +/// `pre_score(counter_example) >= threshold`. Validating an existing counter_example is computationally +/// cheaper than re-running the PJR check. +/// +/// This function uses the standard threshold. +/// +/// Returns `true` if the challenge is valid: the proposed solution does not satisfy PJR. +/// Returns `false` if the challenge is invalid: the proposed solution does in fact satisfy PJR. +pub fn validate_pjr_challenge( + counter_example: AccountId, + supports: &Supports, + all_candidates: Vec, + all_voters: Vec<(AccountId, VoteWeight, Vec)>, +) -> bool { + let threshold = standard_threshold(supports.len(), all_voters.iter().map(|voter| voter.1 as ExtendedBalance)); + validate_t_pjr_challenge(counter_example, supports, all_candidates, all_voters, threshold) +} +/// Validate a challenge to an election result. +/// +/// A challenge to an election result is valid if there exists some counter_example for which +/// `pre_score(counter_example) >= threshold`. Validating an existing counter_example is computationally +/// cheaper than re-running the PJR check. +/// +/// This function uses a supplied threshold. +/// +/// Returns `true` if the challenge is valid: the proposed solution does not satisfy PJR. +/// Returns `false` if the challenge is invalid: the proposed solution does in fact satisfy PJR. +pub fn validate_t_pjr_challenge( + counter_example: AccountId, + supports: &Supports, + all_candidates: Vec, + all_voters: Vec<(AccountId, VoteWeight, Vec)>, + threshold: Threshold, +) -> bool { + let (candidates, voters) = prepare_pjr_input( + supports, + all_candidates, + all_voters, + ); + validate_pjr_challenge_core(counter_example, &candidates, &voters, threshold) +} + +/// Validate a challenge to an election result. +/// +/// A challenge to an election result is valid if there exists some counter_example for which +/// `pre_score(counter_example) >= threshold`. Validating an existing counter_example is computationally +/// cheaper than re-running the PJR check. +/// +/// Returns `true` if the challenge is valid: the proposed solution does not satisfy PJR. +/// Returns `false` if the challenge is invalid: the proposed solution does in fact satisfy PJR. +fn validate_pjr_challenge_core( + counter_example: AccountId, + candidates: &[CandidatePtr], + voters: &[Voter], + threshold: Threshold, +) -> bool { + // Performing a linear search of the candidate list is not great, for obvious reasons. However, + // the alternatives are worse: + // + // - we could pre-sort the candidates list in `prepare_pjr_input` (n log n) which would let us + // binary search for the appropriate one here (log n). Overall runtime is `n log n` which is + // worse than the current runtime of `n`. + // + // - we could probably pre-sort the candidates list in `n` in `prepare_pjr_input` using some + // unsafe code leveraging the existing `candidates_index`: allocate an uninitialized vector of + // appropriate length, then copy in all the elements. We'd really prefer to avoid unsafe code + // in the runtime, though. + let candidate = match candidates.iter().find(|candidate| candidate.borrow().who == counter_example) { + None => return false, + Some(candidate) => candidate.clone(), + }; + pre_score(candidate, &voters, threshold) >= threshold +} /// Convert the data types that the user runtime has into ones that can be used by this module. /// @@ -315,6 +396,15 @@ mod tests { voter } + fn assert_core_failure( + candidates: &[CandidatePtr], + voters: &[Voter], + t: Threshold, + ) { + let counter_example = pjr_check_core(candidates, voters, t).unwrap_err(); + assert!(validate_pjr_challenge_core(counter_example, candidates, voters, t)); + } + #[test] fn slack_works() { let voter = setup_voter(10, vec![(1, 10, true), (2, 20, true)]); @@ -388,9 +478,9 @@ mod tests { // fyi. this is not PJR, obviously because the votes of 3 can bump the stake a lot but they // are being ignored. - assert!(!pjr_check_core(&candidates, &voters, 1)); - assert!(!pjr_check_core(&candidates, &voters, 10)); - assert!(!pjr_check_core(&candidates, &voters, 20)); + assert_core_failure(&candidates, &voters, 1); + assert_core_failure(&candidates, &voters, 10); + assert_core_failure(&candidates, &voters, 20); } // These next tests ensure that the threshold phase change property holds for us, but that's not their real purpose. @@ -476,7 +566,7 @@ mod tests { let mut prev_threshold = 0; // find the binary range containing the threshold beyond which the PJR check succeeds - while !pjr_check_core(&candidates, &voters, threshold) { + while pjr_check_core(&candidates, &voters, threshold).is_err() { prev_threshold = threshold; threshold = threshold.checked_mul(2).expect("pjr check must fail before we run out of capacity in u128"); } @@ -488,7 +578,7 @@ mod tests { while high_bound - low_bound > 1 { // maintain the invariant that low_bound fails and high_bound passes let test = low_bound + ((high_bound - low_bound) / 2); - if pjr_check_core(&candidates, &voters, test) { + if pjr_check_core(&candidates, &voters, test).is_ok() { high_bound = test; } else { low_bound = test; @@ -502,12 +592,12 @@ mod tests { let mut unexpected_failures = Vec::new(); let mut unexpected_successes = Vec::new(); for t in 0..=low_bound { - if pjr_check_core(&candidates, &voters, t) { + if pjr_check_core(&candidates, &voters, t).is_ok() { unexpected_successes.push(t); } } for t in high_bound..(high_bound*2) { - if !pjr_check_core(&candidates, &voters, t) { + if pjr_check_core(&candidates, &voters, t).is_err() { unexpected_failures.push(t); } }