Skip to content

Commit

Permalink
fix(votekeeper): Compute threshold directly in VoteKeeper to accoun…
Browse files Browse the repository at this point in the history
…t for `Skip` threshold (#73)

* fix(votekeeper): Compute threshold directly in `VoteKeeper` to account for `Skip` threshold

* test: Re-enable VoteCount tests

* test: Re-enable RoundVotes tests

* test: Remove duplicate tests

* test: Re-enable VoteKeeper tests

* fix(votekeeper): Do not count validator weights twice if they prevoted and precommited when checking for Skip threshold
  • Loading branch information
romac authored Nov 21, 2023
1 parent 061b643 commit d868668
Show file tree
Hide file tree
Showing 9 changed files with 425 additions and 421 deletions.
2 changes: 1 addition & 1 deletion Code/common/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ where
fn round(&self) -> Round;

/// Get a reference to the value being voted for.
fn value(&self) -> Option<&<Ctx::Value as Value>::Id>;
fn value(&self) -> &Option<<Ctx::Value as Value>::Id>;

/// Take ownership of the value being voted for.
fn take_value(self) -> Option<<Ctx::Value as Value>::Id>;
Expand Down
12 changes: 8 additions & 4 deletions Code/driver/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use malachite_round::state::State as RoundState;
use malachite_vote::keeper::Message as VoteMessage;
use malachite_vote::keeper::VoteKeeper;
use malachite_vote::Threshold;
use malachite_vote::ThresholdParams;

use crate::env::Env as DriverEnv;
use crate::event::Event;
Expand Down Expand Up @@ -55,7 +56,10 @@ where
validator_set: Ctx::ValidatorSet,
address: Ctx::Address,
) -> Self {
let votes = VoteKeeper::new(validator_set.total_voting_power());
let votes = VoteKeeper::new(
validator_set.total_voting_power(),
ThresholdParams::default(), // TODO: Make this configurable
);

Self {
ctx,
Expand Down Expand Up @@ -235,9 +239,9 @@ where

let round = signed_vote.vote.round();

let Some(vote_msg) = self
.votes
.apply_vote(signed_vote.vote, validator.voting_power())
let Some(vote_msg) =
self.votes
.apply_vote(signed_vote.vote, validator.voting_power(), self.round)
else {
return Ok(None);
};
Expand Down
4 changes: 2 additions & 2 deletions Code/test/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ impl malachite_common::Vote<TestContext> for Vote {
self.round
}

fn value(&self) -> Option<&ValueId> {
self.value.as_ref()
fn value(&self) -> &Option<ValueId> {
&self.value
}

fn take_value(self) -> Option<ValueId> {
Expand Down
76 changes: 31 additions & 45 deletions Code/test/tests/round_votes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use malachite_common::VoteType;
use malachite_vote::round_votes::RoundVotes;
use malachite_vote::Threshold;

use malachite_test::{Address, ValueId};

Expand All @@ -13,47 +12,41 @@ const ADDRESS6: Address = Address::new([46; 20]);

#[test]
fn add_votes_nil() {
let total = 3;
let mut round_votes: RoundVotes<_, ValueId> = RoundVotes::new();

let mut round_votes: RoundVotes<_, ValueId> = RoundVotes::new(total, Default::default());
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS1, None, 1);
assert_eq!(w, 1);

// add a vote for nil. nothing changes.
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS1, None, 1);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS2, None, 1);
assert_eq!(w, 2);

// add it again, nothing changes.
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS2, None, 1);
assert_eq!(thresh, Threshold::Unreached);

// add it again, get Nil
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS3, None, 1);
assert_eq!(thresh, Threshold::Nil);
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS3, None, 1);
assert_eq!(w, 3);
}

#[test]
fn add_votes_single_value() {
let v = ValueId::new(1);
let val = Some(v);
let total = 4;
let weight = 1;

let mut round_votes: RoundVotes<_, ValueId> = RoundVotes::new(total, Default::default());
let mut round_votes: RoundVotes<_, ValueId> = RoundVotes::new();

// add a vote. nothing changes.
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS1, val, weight);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS1, val, weight);
assert_eq!(w, 1);

// add it again, nothing changes.
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS2, val, weight);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS2, val, weight);
assert_eq!(w, 2);

// add a vote for nil, get Thresh::Any
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS3, None, weight);
assert_eq!(thresh, Threshold::Any);
// add a vote for nil, get w::Any
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS3, None, weight);
assert_eq!(w, 1);

// add vote for value, get Thresh::Value
let thresh = round_votes.add_vote(VoteType::Prevote, ADDRESS4, val, weight);
assert_eq!(thresh, Threshold::Value(v));
// add vote for value, get w::Value
let w = round_votes.add_vote(VoteType::Prevote, ADDRESS4, val, weight);
assert_eq!(w, 3);
}

#[test]
Expand All @@ -62,31 +55,24 @@ fn add_votes_multi_values() {
let v2 = ValueId::new(2);
let val1 = Some(v1);
let val2 = Some(v2);
let total = 15;

let mut round_votes: RoundVotes<_, ValueId> = RoundVotes::new(total, Default::default());
let mut round_votes: RoundVotes<_, ValueId> = RoundVotes::new();

// add a vote for v1. nothing changes.
let thresh = round_votes.add_vote(VoteType::Precommit, ADDRESS1, val1, 1);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Precommit, ADDRESS1, val1, 1);
assert_eq!(w, 1);

// add a vote for v2. nothing changes.
let thresh = round_votes.add_vote(VoteType::Precommit, ADDRESS2, val2, 1);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Precommit, ADDRESS2, val2, 1);
assert_eq!(w, 1);

// add a vote for nil. nothing changes.
let thresh = round_votes.add_vote(VoteType::Precommit, ADDRESS3, None, 1);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Precommit, ADDRESS3, None, 1);
assert_eq!(w, 1);

// add a vote for v1. nothing changes
let thresh = round_votes.add_vote(VoteType::Precommit, ADDRESS4, val1, 1);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Precommit, ADDRESS4, val1, 1);
assert_eq!(w, 2);

// add a vote for v2. nothing changes
let thresh = round_votes.add_vote(VoteType::Precommit, ADDRESS5, val2, 1);
assert_eq!(thresh, Threshold::Unreached);
let w = round_votes.add_vote(VoteType::Precommit, ADDRESS5, val2, 1);
assert_eq!(w, 2);

// add a big vote for v2. get Value(v2)
let thresh = round_votes.add_vote(VoteType::Precommit, ADDRESS6, val2, 10);
assert_eq!(thresh, Threshold::Value(v2));
let w = round_votes.add_vote(VoteType::Precommit, ADDRESS6, val2, 10);
assert_eq!(w, 12);
}
155 changes: 0 additions & 155 deletions Code/test/tests/vote_count.rs

This file was deleted.

Loading

0 comments on commit d868668

Please sign in to comment.