Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 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
31 changes: 31 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,37 @@ pub fn process_commit_validation_result<H, N>(
}
}

/// Historical votes seen in a round.
#[derive(Debug, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode))]
pub struct HistoricalVotes<H, N, S, Id> {
seen: Vec<SignedMessage<H, N, S, Id>>,
prevote_idx: Option<usize>,
precommit_idx: Option<usize>,
}

impl<H, N, S, Id> HistoricalVotes<H, N, S, Id> {
pub fn new() -> Self {
HistoricalVotes {
seen: Vec::new(),
prevote_idx: None,
precommit_idx: None,
}
}

pub fn seen(&self) -> &Vec<SignedMessage<H, N, S, Id>> {
&self.seen
}

pub fn prevote_idx(&self) -> Option<usize> {
self.prevote_idx
}

pub fn precommit_idx(&self) -> Option<usize> {
self.precommit_idx
}
}

#[cfg(test)]
mod tests {
use super::threshold;
Expand Down
59 changes: 55 additions & 4 deletions src/round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::bitfield::{Shared as BitfieldContext, Bitfield};
use crate::vote_graph::VoteGraph;
use crate::voter_set::VoterSet;

use super::{Equivocation, Prevote, Precommit, Chain, BlockNumberOps};
use super::{Equivocation, Prevote, Precommit, Chain, BlockNumberOps, HistoricalVotes, Message, SignedMessage};

#[derive(Hash, Eq, PartialEq)]
struct Address;
Expand Down Expand Up @@ -131,6 +131,7 @@ impl<Id: Hash + Eq + Clone, Vote: Clone + Eq, Signature: Clone + Eq> VoteTracker
Entry::Vacant(vacant) => {
self.current_weight += weight;
let multiplicity = vacant.insert(VoteMultiplicity::Single(vote, signature));

return AddVoteResult {
multiplicity: Some(multiplicity),
duplicated: false,
Expand Down Expand Up @@ -223,6 +224,7 @@ pub struct Round<Id: Hash + Eq, H: Hash + Eq, N, Signature> {
graph: VoteGraph<H, N, VoteWeight>, // DAG of blocks which have been voted on.
prevote: VoteTracker<Id, Prevote<H, N>, Signature>, // tracks prevotes that have been counted
precommit: VoteTracker<Id, Precommit<H, N>, Signature>, // tracks precommits
historical_votes: HistoricalVotes<H, N, Signature, Id>,
round_number: u64,
voters: VoterSet<Id>,
total_weight: u64,
Expand Down Expand Up @@ -274,6 +276,7 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
graph: VoteGraph::new(base_hash, base_number),
prevote: VoteTracker::new(),
precommit: VoteTracker::new(),
historical_votes: HistoricalVotes::new(),
bitfield_context: BitfieldContext::new(n_validators),
prevote_ghost: None,
precommit_ghost: None,
Expand Down Expand Up @@ -309,7 +312,7 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
let weight = info.weight();

let equivocation = {
let multiplicity = match self.prevote.add_vote(signer.clone(), vote, signature, weight) {
let multiplicity = match self.prevote.add_vote(signer.clone(), vote.clone(), signature.clone(), weight) {
AddVoteResult { multiplicity: Some(m), .. } => m,
AddVoteResult { duplicated, .. } => {
import_result.duplicated = duplicated;
Expand All @@ -332,13 +335,23 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
chain,
)?;

// Push the vote into HistoricalVotes.
let message = Message::Prevote(vote.clone());
let signed_message = SignedMessage { id: signer.clone(), signature, message };
self.historical_votes.seen.push(signed_message);

None
}
VoteMultiplicity::Equivocated(ref first, ref second) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't we need to store equivocated votes as well?

@rphmeier rphmeier May 13, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At the very least, the first equivocation of a type from a given voter

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed.

// mark the equivocator as such. no need to "undo" the first vote.
self.bitfield_context.equivocated_prevote(info)
.expect("info is instantiated from same voter set as bitfield; qed");

// Push the vote into HistoricalVotes.
let message = Message::Prevote(vote);
let signed_message = SignedMessage { id: signer.clone(), signature, message };
self.historical_votes.seen.push(signed_message);

Some(Equivocation {
round_number,
identity: signer,
Expand Down Expand Up @@ -386,7 +399,7 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
let weight = info.weight();

let equivocation = {
let multiplicity = match self.precommit.add_vote(signer.clone(), vote, signature, weight) {
let multiplicity = match self.precommit.add_vote(signer.clone(), vote.clone(), signature.clone(), weight) {
AddVoteResult { multiplicity: Some(m), .. } => m,
AddVoteResult { duplicated, .. } => {
import_result.duplicated = duplicated;
Expand All @@ -409,13 +422,22 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
chain,
)?;

let message = Message::Precommit(vote.clone());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this clone necessary?

let signed_message = SignedMessage { id: signer.clone(), signature, message };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unnecessary clone?

self.historical_votes.seen.push(signed_message);

None
}
VoteMultiplicity::Equivocated(ref first, ref second) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above.

// mark the equivocator as such. no need to "undo" the first vote.
self.bitfield_context.equivocated_precommit(info)
.expect("info is instantiated from same voter set as bitfield; qed");

// Push the vote into HistoricalVotes.
let message = Message::Precommit(vote);
let signed_message = SignedMessage { id: signer.clone(), signature, message };
self.historical_votes.seen.push(signed_message);

Some(Equivocation {
round_number,
identity: signer,
Expand Down Expand Up @@ -472,7 +494,7 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
type Item = (V, S);

fn next(&mut self) -> Option<(V, S)> {
match *self.multiplicity {
match self.multiplicity {
VoteMultiplicity::Single(ref v, ref s) => {
if self.yielded == 0 {
self.yielded += 1;
Expand Down Expand Up @@ -674,6 +696,35 @@ impl<Id, H, N, Signature> Round<Id, H, N, Signature> where
pub fn precommits(&self) -> Vec<(Id, Precommit<H, N>, Signature)> {
self.precommit.votes()
}

/// Return all votes (prevotes and precommits) by importing order.
pub fn historical_votes(&self) -> &HistoricalVotes<H, N, Signature, Id> {
&self.historical_votes
}

/// Set the number of prevotes and precommits received at the moment of prevoting.
/// It should be called inmediatly after prevoting.
pub fn set_prevoted_index(&mut self) {
self.historical_votes.prevote_idx = Some(self.historical_votes.seen.len())
}

/// Set the number of prevotes and precommits received at the moment of precommiting.
/// It should be called inmediatly after precommiting.
pub fn set_precommited_index(&mut self) {
self.historical_votes.precommit_idx = Some(self.historical_votes.seen.len())
}

/// Get the number of prevotes and precommits received at the moment of prevoting.
/// Returns None if the prevote wasn't realized.
pub fn prevoted_index(&self) -> Option<usize> {
self.historical_votes.prevote_idx
}

/// Get the number of prevotes and precommits received at the moment of precommiting.
/// Returns None if the precommit wasn't realized.
pub fn precommited_index(&self) -> Option<usize> {
self.historical_votes.precommit_idx
}
}

#[cfg(test)]
Expand Down
6 changes: 2 additions & 4 deletions src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use tokio::timer::Delay;
use parking_lot::Mutex;
use futures::prelude::*;
use futures::sync::mpsc::{self, UnboundedReceiver, UnboundedSender};
use super::{Chain, Commit, Error, Equivocation, Message, Prevote, Precommit, PrimaryPropose, SignedMessage};
use super::{Chain, Commit, Error, Equivocation, Message, Prevote, Precommit, PrimaryPropose, SignedMessage, HistoricalVotes};

pub const GENESIS_HASH: &str = "genesis";
const NULL_HASH: &str = "NULL";
Expand Down Expand Up @@ -53,8 +53,6 @@ impl DummyChain {
}

pub fn push_blocks(&mut self, mut parent: &'static str, blocks: &[&'static str]) {
use std::cmp::Ord;

if blocks.is_empty() { return }

let base_number = self.inner.get(parent).unwrap().number + 1;
Expand Down Expand Up @@ -218,7 +216,7 @@ impl crate::voter::Environment<&'static str, u32> for Environment {
_round: u64,
_state: RoundState<&'static str, u32>,
_base: (&'static str, u32),
_votes: Vec<SignedMessage<&'static str, u32, Signature, Id>>,
_votes: &HistoricalVotes<&'static str, u32, Self::Signature, Self::Id>,
) -> Result<(), Error> {
Ok(())
}
Expand Down
8 changes: 4 additions & 4 deletions src/voter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use std::sync::Arc;
use crate::round::State as RoundState;
use crate::{
Chain, Commit, CompactCommit, Equivocation, Message, Prevote, Precommit, PrimaryPropose,
SignedMessage, BlockNumberOps, validate_commit, CommitValidationResult
SignedMessage, BlockNumberOps, validate_commit, CommitValidationResult, HistoricalVotes,
};
use crate::voter_set::VoterSet;
use past_rounds::PastRounds;
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<H, N, Self::Signature, Self::Id>,
) -> Result<(), Self::Error>;

/// Called when a block should be finalized.
Expand Down Expand Up @@ -662,7 +662,7 @@ impl<H, N, E: Environment<H, N>, GlobalIn, GlobalOut> Voter<H, N, E, GlobalIn, G
self.best_round.round_number(),
self.best_round.round_state(),
self.best_round.dag_base(),
self.best_round.votes(),
self.best_round.historical_votes(),
)?;

let old_round_number = self.best_round.round_number();
Expand Down Expand Up @@ -690,7 +690,7 @@ impl<H, N, E: Environment<H, N>, GlobalIn, GlobalOut> Voter<H, N, E, GlobalIn, G
prospective_round.round_number(),
prospective_round.round_state(),
prospective_round.dag_base(),
prospective_round.votes(),
prospective_round.historical_votes(),
)?;

self.best_round = VotingRound::new(
Expand Down
26 changes: 7 additions & 19 deletions src/voter/voting_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::round::{Round, State as RoundState};
use crate::{
Commit, Message, Prevote, Precommit, PrimaryPropose, SignedMessage,
SignedPrecommit, BlockNumberOps, validate_commit, ImportResult,
HistoricalVotes,
};
use crate::voter_set::VoterSet;
use super::{Environment, Buffered};
Expand Down Expand Up @@ -255,25 +256,10 @@ impl<H, N, E: Environment<H, N>> VotingRound<H, N, E> where
self.best_finalized.as_ref()
}

/// Return all imported votes for the round (prevotes and precommits).
pub(super) fn votes(&self) -> Vec<SignedMessage<H, N, E::Signature, E::Id>> {
let prevotes = self.votes.prevotes().into_iter().map(|(id, prevote, signature)| {
SignedMessage {
id,
signature,
message: Message::Prevote(prevote),
}
});

let precommits = self.votes.precommits().into_iter().map(|(id, precommit, signature)| {
SignedMessage {
id,
signature,
message: Message::Precommit(precommit),
}
});

prevotes.chain(precommits).collect()
/// Return all votes for the round (prevotes and precommits),
/// sorted by imported order and indicating the indices where we voted.
pub(super) fn historical_votes(&self) -> &HistoricalVotes<H, N, E::Signature, E::Id> {
self.votes.historical_votes()
}

fn process_incoming(&mut self) -> Result<(), E::Error> {
Expand Down Expand Up @@ -370,6 +356,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_prevoted_index();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this going to race? Is it guaranteed that the next vote we process will be our own?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nevermind, I understand the semantics of this are to mark what we had seen so far when we prevoted/precommited (regardless of whether we'll eval our own vote before/after other new votes).

self.outgoing.push(Message::Prevote(prevote));
}
}
Expand Down Expand Up @@ -422,6 +409,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_precommited_index();
self.outgoing.push(Message::Precommit(precommit));
}
self.state = Some(State::Precommitted);
Expand Down