Skip to content

Commit

Permalink
Rewrite VotingPowerCalculator::voting_power_in.
Browse files Browse the repository at this point in the history
- Only count `Commit` signatures towards the voting power.
- Rely less (but still) on the validator address.
- Do not rely on `signed_votes` method to get the votes.
  • Loading branch information
romac committed Jun 8, 2020
1 parent 8b7a2aa commit 74786f4
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 16 deletions.
103 changes: 87 additions & 16 deletions light-client/src/operations/voting_power.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use crate::prelude::*;

use anomaly::BoxError;

use tendermint::block::CommitSig;
use tendermint::lite::types::ValidatorSet as _;
use tendermint::vote::{SignedVote, Vote};

pub trait VotingPowerCalculator {
fn total_power_of(&self, validators: &ValidatorSet) -> u64;
Expand Down Expand Up @@ -50,36 +53,104 @@ impl VotingPowerCalculator for ProdVotingPowerCalculator {
fn voting_power_in(
&self,
signed_header: &SignedHeader,
validators: &ValidatorSet,
validator_set: &ValidatorSet,
) -> Result<u64, BoxError> {
let signatures = &signed_header.commit.signatures;
let validators = validator_set.validators();

// ensure!(
// validators.len() == signatures.len(),
// // TODO: Raise error
// );

// NOTE: We don't know the validators that committed this block,
// so we have to check for each vote if its validator is already known.
let mut signed_power = 0_u64;
let voting_power_needed = self.total_power_of(validator_set) * 2 / 3;
let mut tallied_voting_power = 0_u64;

for (idx, signature) in signatures.into_iter().enumerate() {
if signature.is_absent() {
continue; // OK, some signatures can be absent.
}

// The vals and commit have a 1-to-1 correspondance (see check above).
// This means we don't need the validator address or to do any lookup.
let val = validators[idx];

for vote in &signed_header.signed_votes() {
// Only count if this vote is from a known validator.
// TODO: we still need to check that we didn't see a vote from this validator twice ...
let val_id = vote.validator_id();
let val = match validators.validator(val_id) {
Some(v) => v,
None => continue,
};
let vote = vote_from_non_absent_signature(signature, idx as u64, &signed_header.commit)
.unwrap(); // SAFETY: Safe because of `is_absent()` check above.

// check vote is valid from validator
let sign_bytes = vote.sign_bytes();
let signed_vote = SignedVote::new(
(&vote).into(),
signed_header.header.chain_id.as_str(),
vote.validator_address,
vote.signature,
);

if !val.verify_signature(&sign_bytes, vote.signature()) {
// Check vote is valid from validator
let sign_bytes = signed_vote.sign_bytes();
if !val.verify_signature(&sign_bytes, signed_vote.signature()) {
bail!(VerificationError::ImplementationSpecific(format!(
"Couldn't verify signature {:?} with validator {:?} on sign_bytes {:?}",
vote.signature(),
signed_vote.signature(),
val,
sign_bytes,
)));
}

signed_power += val.power();
if signature.is_commit() {
tallied_voting_power += val.power();
} else {
// It's OK. We include stray signatures (~votes for nil) to measure
// validator availability.
}

if tallied_voting_power >= voting_power_needed {
break;
}
}

Ok(signed_power)
Ok(tallied_voting_power)
}
}

fn vote_from_non_absent_signature(
commit_sig: &CommitSig,
validator_index: u64,
commit: &Commit,
) -> Option<Vote> {
let (validator_address, timestamp, signature, block_id) = match commit_sig {
CommitSig::BlockIDFlagAbsent { .. } => return None,
CommitSig::BlockIDFlagCommit {
validator_address,
timestamp,
signature,
} => (
validator_address.clone(),
timestamp.clone(),
signature.clone(),
Some(commit.block_id.clone()),
),
CommitSig::BlockIDFlagNil {
validator_address,
timestamp,
signature,
} => (
validator_address.clone(),
timestamp.clone(),
signature.clone(),
None,
),
};

Some(Vote {
vote_type: tendermint::vote::Type::Precommit,
height: commit.height,
round: commit.round,
block_id,
timestamp,
validator_address,
validator_index,
signature,
})
}
23 changes: 23 additions & 0 deletions tendermint/src/block/commit_sig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,29 @@ pub enum CommitSig {
},
}

impl CommitSig {
/// Whether this signature is absent (no vote was received from validator)
pub fn is_absent(&self) -> bool {
self == &Self::BlockIDFlagAbsent
}

/// Whether this signature is a commit (validator voted for the Commit.BlockId)
pub fn is_commit(&self) -> bool {
match self {
Self::BlockIDFlagCommit { .. } => true,
_ => false,
}
}

/// Whether this signature is nil (validator voted for nil)
pub fn is_nil(&self) -> bool {
match self {
Self::BlockIDFlagNil { .. } => true,
_ => false,
}
}
}

// Todo: https://github.com/informalsystems/tendermint-rs/issues/259 - CommitSig Timestamp can be zero time
// Todo: https://github.com/informalsystems/tendermint-rs/issues/260 - CommitSig validator address missing in Absent vote
impl TryFrom<RawCommitSig> for CommitSig {
Expand Down

0 comments on commit 74786f4

Please sign in to comment.