Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

early rewards #233

Merged
merged 5 commits into from
Jun 22, 2022
Merged

early rewards #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e9fd51e
early rewards
pifragile Jun 17, 2022
7e2ad6e
remove clone
pifragile Jun 22, 2022
390cde2
refactor tests
pifragile Jun 22, 2022
6442479
refactor phase check
pifragile Jun 22, 2022
765c13d
Merge branch 'master' into early-rewards
brenzi Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 78 additions & 5 deletions ceremonies/meetup-validation/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ pub fn get_participant_judgements(
participant_attestations: &Attestations,
attestation_threshold_fn: fn(usize) -> usize,
) -> Result<ParticipantJudgements, MeetupValidationError> {
let mut participant_judgements =
ParticipantJudgements { legit: participants.clone(), excluded: vec![] };
let mut participant_judgements = ParticipantJudgements {
legit: participants.clone(),
excluded: vec![],
early_rewards_possible: false,
};
participant_judgements.exclude_participants(get_excluded_participants_no_vote(
&participant_judgements.legit,
participant_votes,
)?);

let (n_confirmed, _num_votes) =
let (n_confirmed, _num_votes, vote_is_unanimous) =
find_majority_vote(&participant_judgements.legit, participant_votes)?;

participant_judgements.exclude_participants(get_excluded_participants_wrong_vote(
Expand All @@ -38,15 +41,83 @@ pub fn get_participant_judgements(
n_confirmed,
)?);

// this check has to be made before we exclude participants based on attestations
let early_rewards_possible = early_rewards_possible(
participant_judgements.legit.clone(),
participant_attestations.clone(),
participants.len(),
n_confirmed,
vote_is_unanimous,
);

participant_judgements.exclude_participants(get_excluded_participants_num_attestations(
&participant_judgements.legit,
participant_attestations.clone(),
attestation_threshold_fn,
)?);

participant_judgements.early_rewards_possible = early_rewards_possible;
Ok(participant_judgements)
}

fn vote_yields_majority(num_participants: usize, n_confirmed: u32) -> bool {
n_confirmed as f64 > (num_participants as f64) / 2.0
}

fn num_attestations_matches_vote(
legit_participants: &Participants,
participant_attestations: &Attestations,
n_confirmed: u32,
) -> bool {
participant_attestations
.into_iter()
.enumerate()
.map(|(i, v)| !(legit_participants.contains(&i)) || v.len() == (n_confirmed - 1) as usize)
.all(|item| item)
}
pifragile marked this conversation as resolved.
Show resolved Hide resolved

fn attestation_graph_is_fully_connected(
legit_participants: Participants,
participant_attestations: Attestations,
) -> bool {
for (i, mut attestations) in participant_attestations.into_iter().enumerate() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens, if i attested people that fell out of the legit participant set? I believe your logic with sorting and testing equality would not see this as fully connected. but it should

Anyway, sorting twice within a for loop doesn't strike me as efficient. Wouldn't a nested for loop through legit participants be more efficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is made before any participants can fall out of the set, because the attestation graph needs to be fully connected for all participants in order for early payout to be possible, right?

about the efficiency. my implementation:
O(n) (loop over attestations) * O(n^2) (sorting, worstcase)

nested loops:
O(n) (loop over attestations) * O(n) (loop over each elem in attestations) * O(n) (check if legit_participant contains elem)

so in asymptotic complexity they are the same.
i agree that in the first implementation we have some overhead because it is 2*O(n^2), but i would argue that for those small input sizes it shouldnt really matter.

but you decide, should i change it to the nested loop?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah, its bounded, so I guess it's fine. Do we already have a worst case benchmark for this claim function? that will tell us the price

// only consider participants present in the meetup
if !legit_participants.contains(&i) {
continue
}
attestations.sort();
let mut expected_attestations = legit_participants.clone();
// remove self
expected_attestations.retain(|&p| p != i);
expected_attestations.sort();

if attestations != expected_attestations {
return false
}
}
true
}

fn early_rewards_possible(
legit_participants: Participants,
participant_attestations: Attestations,
num_total_participants: usize,
n_confirmed: u32,
vote_is_unanimous: bool,
) -> bool {
if vote_is_unanimous &&
vote_yields_majority(num_total_participants, n_confirmed) &&
num_attestations_matches_vote(
&legit_participants,
&participant_attestations,
n_confirmed,
) && attestation_graph_is_fully_connected(legit_participants, participant_attestations)
{
return true
}
false
}

fn get_excluded_participants_no_vote(
participants: &Vec<ParticipantIndex>,
participant_votes: &Vec<u32>,
Expand Down Expand Up @@ -160,7 +231,7 @@ fn get_excluded_participants_num_attestations(
fn find_majority_vote(
participants: &Participants,
participant_votes: &Vec<u32>,
) -> Result<(u32, u32), MeetupValidationError> {
) -> Result<(u32, u32, bool), MeetupValidationError> {
let mut n_vote_candidates: Vec<(u32, u32)> = vec![];
for i in participants {
let this_vote = participant_votes.get_or_err(*i)?;
Expand All @@ -179,7 +250,8 @@ fn find_majority_vote(
return Err(MeetupValidationError::NoDependableVote)
}
let (n_confirmed, vote_count) = n_vote_candidates.get_or_err(0)?;
Ok((*n_confirmed, *vote_count))
let vote_is_unanimous = n_vote_candidates.len() == 1;
Ok((*n_confirmed, *vote_count, vote_is_unanimous))
}

fn filter_attestations(
Expand Down Expand Up @@ -320,6 +392,7 @@ pub struct ExcludedParticipant {
pub struct ParticipantJudgements {
pub legit: Vec<usize>,
pub excluded: Vec<ExcludedParticipant>,
pub early_rewards_possible: bool,
}

impl ParticipantJudgements {
Expand Down
180 changes: 178 additions & 2 deletions ceremonies/meetup-validation/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ fn get_excluded_participants_num_attestations_works() {
fn find_majority_vote_works() {
let participants: Participants = vec![0, 1, 2, 3, 4];
let participant_votes: Vec<u32> = vec![1, 1, 2, 3, 1];
assert_eq!(find_majority_vote(&participants, &participant_votes), Ok((1u32, 3u32)));
assert_eq!(find_majority_vote(&participants, &participant_votes), Ok((1u32, 3u32, false)));
}

#[test]
fn find_majority_vote_works_with_unanimous_vote() {
let participants: Participants = vec![0, 1, 2, 3, 4];
let participant_votes: Vec<u32> = vec![1, 1, 1, 1, 1];
assert_eq!(find_majority_vote(&participants, &participant_votes), Ok((1u32, 5u32, true)));
}

#[test]
Expand All @@ -109,7 +116,11 @@ fn get_participant_judgements_works_case_0() {
vec![0, 1, 2],
vec![3, 3, 3],
vec![vec![1, 2], vec![0, 2], vec![0, 1]],
ParticipantJudgements { legit: vec![0, 1, 2], excluded: vec![] },
ParticipantJudgements {
legit: vec![0, 1, 2],
excluded: vec![],
early_rewards_possible: true,
},
);
}

Expand All @@ -123,6 +134,7 @@ fn get_participant_judgements_works_case_1() {
ParticipantJudgements {
legit: vec![1, 2, 3],
excluded: vec![ExcludedParticipant { index: 0, reason: ExclusionReason::NoVote }],
early_rewards_possible: true,
},
);
}
Expand All @@ -137,6 +149,7 @@ fn get_participant_judgements_works_case_2() {
ParticipantJudgements {
legit: vec![1, 2, 3],
excluded: vec![ExcludedParticipant { index: 0, reason: ExclusionReason::WrongVote }],
early_rewards_possible: false,
},
);
}
Expand All @@ -156,6 +169,7 @@ fn get_participant_judgements_works_case_3() {
index: 0,
reason: ExclusionReason::TooFewOutgoingAttestations,
}],
early_rewards_possible: false,
},
);
}
Expand All @@ -175,6 +189,7 @@ fn get_participant_judgements_works_case_4() {
index: 0,
reason: ExclusionReason::TooFewIncomingAttestations,
}],
early_rewards_possible: false,
},
);
}
Expand All @@ -198,3 +213,164 @@ fn validate_participant_judgements(
participant_judgements
);
}

#[test]
fn vote_yields_majority_works() {
assert_eq!(vote_yields_majority(5, 3), true);
assert_eq!(vote_yields_majority(5, 2), false);
assert_eq!(vote_yields_majority(4, 2), false);
}

#[test]
fn num_attestations_matches_vote_works() {
assert_eq!(
num_attestations_matches_vote(
&vec![0, 1, 2, 3],
&vec![
vec![1, 2, 3],
vec![0, 2, 3],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
4
),
true
);

assert_eq!(
num_attestations_matches_vote(
&vec![0, 1, 2, 3],
&vec![vec![1, 2, 3], vec![0, 2], vec![0, 1, 3], vec![0, 1, 2], vec![0, 1, 2, 3, 4, 5]],
4
),
false
);
}

#[test]
fn attestation_graph_is_fully_connected_works() {
assert_eq!(
attestation_graph_is_fully_connected(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 3],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
),
true
);

assert_eq!(
attestation_graph_is_fully_connected(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 4],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
),
false
);
}

#[test]
fn early_rewards_possible_works() {
assert_eq!(
early_rewards_possible(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 3],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
5,
4,
true
),
true
);
}
#[test]
fn early_rewards_possible_works_unanimous_vote() {
assert_eq!(
early_rewards_possible(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 3],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
5,
4,
false
),
false
);
}
#[test]
fn early_rewards_possible_works_vote_is_not_majority() {
assert_eq!(
early_rewards_possible(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 3],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
5,
2,
true
),
false
);
}
#[test]
fn early_rewards_possible_works_attesttations_do_not_match_vote() {
assert_eq!(
early_rewards_possible(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 3, 4],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
5,
4,
true
),
false
);
}
#[test]
fn early_rewards_possible_works_attestation_graph_is_not_fully_connected() {
assert_eq!(
early_rewards_possible(
vec![0, 1, 2, 3],
vec![
vec![1, 2, 3],
vec![0, 2, 4],
vec![0, 1, 3],
vec![0, 1, 2],
vec![0, 1, 2, 3, 4, 5]
],
5,
4,
true
),
false
);
}
pifragile marked this conversation as resolved.
Show resolved Hide resolved