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.

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

Answer queries #65

Merged
merged 22 commits into from
May 14, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ impl<Vote: Eq, Signature: Eq> VoteMultiplicity<Vote, Signature> {
struct VoteTracker<Id: Hash + Eq, Vote, Signature> {
votes: HashMap<Id, VoteMultiplicity<Vote, Signature>>,
current_weight: u64,
voted_at: Option<usize>,
}

/// Result of adding a vote.
Expand All @@ -111,6 +112,7 @@ impl<Id: Hash + Eq + Clone, Vote: Clone + Eq, Signature: Clone + Eq> VoteTracker
VoteTracker {
votes: HashMap::new(),
current_weight: 0,
voted_at: None,
}
}

Expand Down Expand Up @@ -674,6 +676,30 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
pub fn precommits(&self) -> Vec<(Id, Precommit<H, N>, Signature)> {
self.precommit.votes()
}

/// Set the length of prevotes received at the moment of prevoting.
pub fn set_prevote_idx(&mut self) -> bool {
self.prevote.voted_at = Some(self.prevote.votes().len());
println!("set prevote idx to {:?}", self.prevote.voted_at);
Copy link
Contributor

Choose a reason for hiding this comment

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

stray println

true
}

/// Set the length of precommits received at the moment of precommiting.
pub fn set_precommit_idx(&mut self) -> bool {
self.precommit.voted_at = Some(self.precommit.votes().len());
println!("set precommit idx to {:?}", self.precommit.voted_at);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

true
}

/// Get the length of prevotes received at the moment of prevoting.
pub fn prevote_idx(&self) -> Option<usize> {
self.prevote.voted_at
}

/// Get the length of precommits received at the moment of precommiting.
pub fn precommit_idx(&self) -> Option<usize> {
self.precommit.voted_at
}
}

#[cfg(test)]
Expand Down
4 changes: 2 additions & 2 deletions src/voter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::{
};
use crate::voter_set::VoterSet;
use past_rounds::PastRounds;
use voting_round::{VotingRound, State as VotingRoundState};
use voting_round::{VotingRound, State as VotingRoundState, HistoricalVotes};

mod past_rounds;
mod voting_round;
Expand Down Expand Up @@ -101,7 +101,7 @@ pub trait Environment<H: Eq, N: BlockNumberOps>: Chain<H, N> {
round: u64,
state: RoundState<H, N>,
base: (H, N),
votes: Vec<SignedMessage<H, N, Self::Signature, Self::Id>>,
votes: HistoricalVotes<SignedMessage<H, N, Self::Signature, Self::Id>>,
) -> Result<(), Self::Error>;

/// Called when a block should be finalized.
Expand Down
28 changes: 26 additions & 2 deletions src/voter/voting_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ impl Voting {
}
}

pub struct HistoricalVotes<M> {
seen: Vec<M>,
prevote_idx: Option<usize>,
precommit_idx: Option<usize>,
}

impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
H: Hash + Clone + Eq + Ord + ::std::fmt::Debug,
N: Copy + BlockNumberOps + ::std::fmt::Debug,
Expand Down Expand Up @@ -201,6 +207,16 @@ impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
self.votes.voters()
}

/// Get the best block finalized in this round.
pub(super) fn set_precommit_idx(&mut self, idx: usize) -> bool {
self.votes.set_precommit_idx(idx)
}

/// Get the best block finalized in this round.
pub(super) fn set_prevote_idx(&mut self, idx: usize) -> bool {
self.votes.set_prevote_idx(idx)
}

/// Get the best block finalized in this round.
pub(super) fn finalized(&self) -> Option<&(H, N)> {
self.votes.finalized()
Expand Down Expand Up @@ -256,7 +272,7 @@ impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
}

/// Return all imported votes for the round (prevotes and precommits).
pub(super) fn votes(&self) -> Vec<SignedMessage<H, N, E::Signature, E::Id>> {
pub(super) fn votes(&self) -> HistoricalVotes<SignedMessage<H, N, E::Signature, E::Id>> {
let prevotes = self.votes.prevotes().into_iter().map(|(id, prevote, signature)| {
SignedMessage {
id,
Expand All @@ -273,7 +289,13 @@ impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
}
});

prevotes.chain(precommits).collect()
let prevotes_len = prevotes.len();

HistoricalVotes {
seen: prevotes.chain(precommits).collect(),
prevote_idx: self.votes.prevote_idx(),
precommit_idx: self.votes.precommit_idx().map(|idx| idx + prevotes_len),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this logic is right. There is no guarantee that we see all prevotes before seeing any precommits. Since the estimate logic depends on both prevotes and precommits, we need an interleaved list where the indices are those we've voted at.

Copy link
Author

@marcio-diaz marcio-diaz May 9, 2019

Choose a reason for hiding this comment

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

Ok, I'll fix it. I thought it shouldn't invalidate the votes. But it's ugly and not what was asked anyway.

}
}

fn process_incoming(&mut self) -> Result<(), E::Error> {
Expand Down Expand Up @@ -370,6 +392,7 @@ impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
if let Some(prevote) = self.construct_prevote(last_round_state)? {
debug!(target: "afg", "Casting prevote for round {}", self.votes.number());
self.env.prevoted(self.round_number(), prevote.clone())?;
self.votes.set_prevote_idx();
self.outgoing.push(Message::Prevote(prevote));
}
}
Expand Down Expand Up @@ -422,6 +445,7 @@ impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
debug!(target: "afg", "Casting precommit for round {}", self.votes.number());
let precommit = self.construct_precommit();
self.env.precommitted(self.round_number(), precommit.clone())?;
self.votes.set_precommit_idx();
self.outgoing.push(Message::Precommit(precommit));
}
self.state = Some(State::Precommitted);
Expand Down