diff --git a/light-client/src/callback.rs b/light-client/src/callback.rs index a6db2d1fd..9d4c59ae7 100644 --- a/light-client/src/callback.rs +++ b/light-client/src/callback.rs @@ -20,7 +20,7 @@ impl Callback { } /// Call the underlying closure on `result`. - pub fn call(self, result: A) -> () { + pub fn call(self, result: A) { (self.inner)(result); } } diff --git a/light-client/src/operations/voting_power.rs b/light-client/src/operations/voting_power.rs index e291cb1c6..492d2e44f 100644 --- a/light-client/src/operations/voting_power.rs +++ b/light-client/src/operations/voting_power.rs @@ -1,19 +1,76 @@ use crate::{ bail, predicates::errors::VerificationError, - types::{SignedHeader, ValidatorSet}, + types::{Commit, SignedHeader, TrustThreshold, ValidatorSet}, }; -use anomaly::BoxError; +use serde::{Deserialize, Serialize}; +use std::collections::HashSet; +use std::fmt; + +use tendermint::block::CommitSig; +use tendermint::lite::types::TrustThreshold as _; use tendermint::lite::types::ValidatorSet as _; +use tendermint::vote::{SignedVote, Vote}; + +#[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct VotingPowerTally { + pub total: u64, + pub tallied: u64, + pub trust_threshold: TrustThreshold, +} + +impl fmt::Display for VotingPowerTally { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "VotingPower(total={} tallied={} trust_threshold={})", + self.total, self.tallied, self.trust_threshold + ) + } +} pub trait VotingPowerCalculator: Send { fn total_power_of(&self, validators: &ValidatorSet) -> u64; + + fn check_enough_trust( + &self, + untrusted_header: &SignedHeader, + untrusted_validators: &ValidatorSet, + trust_threshold: TrustThreshold, + ) -> Result<(), VerificationError> { + let voting_power = + self.voting_power_in(untrusted_header, untrusted_validators, trust_threshold)?; + + if trust_threshold.is_enough_power(voting_power.tallied, voting_power.total) { + Ok(()) + } else { + Err(VerificationError::NotEnoughTrust(voting_power)) + } + } + + fn check_signers_overlap( + &self, + untrusted_header: &SignedHeader, + untrusted_validators: &ValidatorSet, + ) -> Result<(), VerificationError> { + let trust_threshold = TrustThreshold::TWO_THIRDS; + let voting_power = + self.voting_power_in(untrusted_header, untrusted_validators, trust_threshold)?; + + if trust_threshold.is_enough_power(voting_power.tallied, voting_power.total) { + Ok(()) + } else { + Err(VerificationError::InsufficientSignersOverlap(voting_power)) + } + } + fn voting_power_in( &self, signed_header: &SignedHeader, - validators: &ValidatorSet, - ) -> Result; + validator_set: &ValidatorSet, + trust_threshold: TrustThreshold, + ) -> Result; } #[derive(Copy, Clone, Debug)] @@ -27,36 +84,105 @@ impl VotingPowerCalculator for ProdVotingPowerCalculator { fn voting_power_in( &self, signed_header: &SignedHeader, - validators: &ValidatorSet, - ) -> Result { - // 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; - - 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, + validator_set: &ValidatorSet, + trust_threshold: TrustThreshold, + ) -> Result { + let signatures = &signed_header.commit.signatures; + + let mut tallied_voting_power = 0_u64; + let mut seen_validators = HashSet::new(); + + // Get non-absent votes from the signatures + let non_absent_votes = signatures.iter().enumerate().flat_map(|(idx, signature)| { + if let Some(vote) = non_absent_vote(signature, idx as u64, &signed_header.commit) { + Some((signature, vote)) + } else { + None + } + }); + + for (signature, vote) in non_absent_votes { + // Ensure we only count a validator's power once + if seen_validators.contains(&vote.validator_address) { + bail!(VerificationError::DuplicateValidator( + vote.validator_address + )); + } else { + seen_validators.insert(vote.validator_address); + } + + let validator = match validator_set.validator(vote.validator_address) { + Some(validator) => validator, + None => continue, // Cannot find matching validator, so we skip the vote }; - // 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()) { - bail!(VerificationError::ImplementationSpecific(format!( - "Couldn't verify signature {:?} with validator {:?} on sign_bytes {:?}", - vote.signature(), - val, + // Check vote is valid + let sign_bytes = signed_vote.sign_bytes(); + if !validator.verify_signature(&sign_bytes, signed_vote.signature()) { + bail!(VerificationError::InvalidSignature { + signature: signed_vote.signature().to_vec(), + validator, sign_bytes, - ))); + }); + } + + // If the vote is neither absent nor nil, tally its power + if signature.is_commit() { + tallied_voting_power += validator.power(); + } else { + // It's OK. We include stray signatures (~votes for nil) + // to measure validator availability. } - signed_power += val.power(); + // TODO: Break out of the loop when we have enough voting power. + // See https://github.com/informalsystems/tendermint-rs/issues/235 } - Ok(signed_power) + let voting_power = VotingPowerTally { + total: self.total_power_of(validator_set), + tallied: tallied_voting_power, + trust_threshold, + }; + + Ok(voting_power) } } + +fn non_absent_vote(commit_sig: &CommitSig, validator_index: u64, commit: &Commit) -> Option { + let (validator_address, timestamp, signature, block_id) = match commit_sig { + CommitSig::BlockIDFlagAbsent { .. } => return None, + CommitSig::BlockIDFlagCommit { + validator_address, + timestamp, + signature, + } => ( + *validator_address, + *timestamp, + signature.clone(), + Some(commit.block_id.clone()), + ), + CommitSig::BlockIDFlagNil { + validator_address, + timestamp, + signature, + } => (*validator_address, *timestamp, 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, + }) +} diff --git a/light-client/src/predicates.rs b/light-client/src/predicates.rs index 949769c0c..0da9227a7 100644 --- a/light-client/src/predicates.rs +++ b/light-client/src/predicates.rs @@ -144,30 +144,6 @@ pub trait VerificationPredicates: Send { Ok(()) } - fn has_sufficient_voting_power( - &self, - signed_header: &SignedHeader, - validators: &ValidatorSet, - trust_threshold: &TrustThreshold, - calculator: &dyn VotingPowerCalculator, - ) -> Result<(), VerificationError> { - // FIXME: Do not discard underlying error - let total_power = calculator.total_power_of(validators); - let voting_power = calculator - .voting_power_in(signed_header, validators) - .map_err(|e| VerificationError::ImplementationSpecific(e.to_string()))?; - - ensure!( - voting_power * trust_threshold.denominator > total_power * trust_threshold.numerator, - VerificationError::InsufficientVotingPower { - total_power, - voting_power, - } - ); - - Ok(()) - } - fn has_sufficient_validators_overlap( &self, untrusted_sh: &SignedHeader, @@ -175,21 +151,7 @@ pub trait VerificationPredicates: Send { trust_threshold: &TrustThreshold, calculator: &dyn VotingPowerCalculator, ) -> Result<(), VerificationError> { - // FIXME: Do not discard underlying error - let total_power = calculator.total_power_of(trusted_validators); - let voting_power = calculator - .voting_power_in(untrusted_sh, trusted_validators) - .map_err(|e| VerificationError::ImplementationSpecific(e.to_string()))?; - - ensure!( - voting_power * trust_threshold.denominator > total_power * trust_threshold.numerator, - VerificationError::InsufficientValidatorsOverlap { - total_power, - signed_power: voting_power, - trust_threshold: *trust_threshold, - } - ); - + calculator.check_enough_trust(untrusted_sh, trusted_validators, *trust_threshold)?; Ok(()) } @@ -199,20 +161,7 @@ pub trait VerificationPredicates: Send { untrusted_validators: &ValidatorSet, calculator: &dyn VotingPowerCalculator, ) -> Result<(), VerificationError> { - // FIXME: Do not discard underlying error - let total_power = calculator.total_power_of(untrusted_validators); - let signed_power = calculator - .voting_power_in(untrusted_sh, untrusted_validators) - .map_err(|e| VerificationError::ImplementationSpecific(e.to_string()))?; - - ensure!( - signed_power * 3 > total_power * 2, - VerificationError::InsufficientCommitPower { - total_power, - signed_power, - } - ); - + calculator.check_signers_overlap(untrusted_sh, untrusted_validators)?; Ok(()) } diff --git a/light-client/src/predicates/errors.rs b/light-client/src/predicates/errors.rs index 64272d531..da2a9d345 100644 --- a/light-client/src/predicates/errors.rs +++ b/light-client/src/predicates/errors.rs @@ -3,7 +3,8 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::errors::ErrorExt; -use crate::types::{Hash, Height, Time, TrustThreshold}; +use crate::operations::voting_power::VotingPowerTally; +use crate::types::{Hash, Height, Time, Validator, ValidatorAddress}; /// The various errors which can be raised by the verifier component, /// when validating or verifying a light block. @@ -15,21 +16,29 @@ pub enum VerificationError { #[error("implementation specific: {0}")] ImplementationSpecific(String), - #[error( - "insufficient validators overlap: total_power={total_power} signed_power={signed_power} trust_threshold={trust_threshold}" - )] - InsufficientValidatorsOverlap { - total_power: u64, - signed_power: u64, - trust_threshold: TrustThreshold, + #[error("not enough trust because insufficient validators overlap: {0}")] + NotEnoughTrust(VotingPowerTally), + + #[error("insufficient signers overlap: {0}")] + InsufficientSignersOverlap(VotingPowerTally), + + // #[error( + // "validators and signatures count do no match: {validators_count} != {signatures_count}" + // )] + // ValidatorsAndSignaturesCountMismatch { + // validators_count: usize, + // signatures_count: usize, + // }, + #[error("duplicate validator with address {0}")] + DuplicateValidator(ValidatorAddress), + + #[error("Couldn't verify signature `{signature:?}` with validator `{validator:?}` on sign_bytes `{sign_bytes:?}`")] + InvalidSignature { + signature: Vec, + validator: Validator, + sign_bytes: Vec, }, - #[error("insufficient voting power: total_power={total_power} voting_power={voting_power}")] - InsufficientVotingPower { total_power: u64, voting_power: u64 }, - - #[error("invalid commit power: total_power={total_power} signed_power={signed_power}")] - InsufficientCommitPower { total_power: u64, signed_power: u64 }, - #[error("invalid commit: {0}")] InvalidCommit(String), @@ -76,7 +85,7 @@ impl ErrorExt for VerificationError { /// Whether this error means that the light block /// cannot be trusted w.r.t. the latest trusted state. fn not_enough_trust(&self) -> bool { - if let Self::InsufficientValidatorsOverlap { .. } = self { + if let Self::NotEnoughTrust { .. } = self { true } else { false diff --git a/light-client/src/supervisor.rs b/light-client/src/supervisor.rs index ba4482004..e68ff88b2 100644 --- a/light-client/src/supervisor.rs +++ b/light-client/src/supervisor.rs @@ -32,9 +32,9 @@ pub enum Event { /// The supervisor has terminated Terminated, /// The verification has succeded - VerificationSuccessed(LightBlock), + VerificationSuccess(Box), /// The verification has failed - VerificationFailed(Error), + VerificationFailure(Error), } /// An light client `Instance` packages a `LightClient` together with its `State`. @@ -213,9 +213,7 @@ impl Supervisor { } /// Report the given light block as evidence of a fork. - fn report_evidence(&mut self, _light_block: &LightBlock) { - () - } + fn report_evidence(&mut self, _light_block: &LightBlock) {} /// Perform fork detection with the given block and trusted state. #[pre(self.peers.primary().is_some())] @@ -314,8 +312,8 @@ impl Handle { let callback = Callback::new(move |result| { // We need to create an event here let event = match result { - Ok(header) => Event::VerificationSuccessed(header), - Err(err) => Event::VerificationFailed(err), + Ok(header) => Event::VerificationSuccess(Box::new(header)), + Err(err) => Event::VerificationFailure(err), }; sender.send(event).unwrap(); @@ -325,8 +323,8 @@ impl Handle { self.sender.send(event).unwrap(); match receiver.recv().unwrap() { - Event::VerificationSuccessed(header) => Ok(header), - Event::VerificationFailed(err) => Err(err), + Event::VerificationSuccess(header) => Ok(*header), + Event::VerificationFailure(err) => Err(err), _ => todo!(), } } diff --git a/light-client/src/types.rs b/light-client/src/types.rs index 09eef2e9c..32a083dbd 100644 --- a/light-client/src/types.rs +++ b/light-client/src/types.rs @@ -4,11 +4,13 @@ use derive_more::Display; use serde::{Deserialize, Serialize}; use tendermint::{ + account::Id as TMAccountId, block::{ header::Header as TMHeader, signed_header::SignedHeader as TMSignedHeader, Commit as TMCommit, }, lite::TrustThresholdFraction, + validator::Info as TMValidatorInfo, validator::Set as TMValidatorSet, }; @@ -30,6 +32,12 @@ pub type Header = TMHeader; /// Set of validators pub type ValidatorSet = TMValidatorSet; +/// Info about a single validator +pub type Validator = TMValidatorInfo; + +/// Validator address +pub type ValidatorAddress = TMAccountId; + /// A commit contains the justification (ie. a set of signatures) /// that a block was consensus, as committed by a set previous block of validators. pub type Commit = TMCommit; diff --git a/light-client/tests/light_client.rs b/light-client/tests/light_client.rs index a68b23a0e..87e5e2e6c 100644 --- a/light-client/tests/light_client.rs +++ b/light-client/tests/light_client.rs @@ -104,7 +104,11 @@ fn run_test_case(tc: TestCase) { latest_trusted = Trusted::new(new_state.signed_header, new_state.next_validators); } - Err(_) => { + Err(e) => { + dbg!(e); + // if !expects_err { + // dbg!(e); + // } assert!(expects_err); } } @@ -240,9 +244,10 @@ fn run_bisection_test(tc: TestBisection) { assert!(!expects_err); } Err(e) => { - if !expects_err { - dbg!(e); - } + dbg!(e); + // if !expects_err { + // dbg!(e); + // } assert!(expects_err); } } diff --git a/tendermint/src/block/commit_sig.rs b/tendermint/src/block/commit_sig.rs index c8b83e9c9..7b342b24a 100644 --- a/tendermint/src/block/commit_sig.rs +++ b/tendermint/src/block/commit_sig.rs @@ -11,8 +11,8 @@ use std::convert::TryFrom; #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] #[serde(try_from = "RawCommitSig", into = "RawCommitSig")] pub enum CommitSig { + // TODO: https://github.com/informalsystems/tendermint-rs/issues/260 - CommitSig validator address missing in Absent vote /// no vote was received from a validator. - // Todo: https://github.com/informalsystems/tendermint-rs/issues/260 - CommitSig validator address missing in Absent vote BlockIDFlagAbsent, /// voted for the Commit.BlockID. BlockIDFlagCommit { @@ -34,6 +34,42 @@ pub enum CommitSig { }, } +impl CommitSig { + /// Get the address of this validator if a vote was received. + pub fn validator_address(&self) -> Option { + match self { + Self::BlockIDFlagCommit { + validator_address, .. + } => Some(*validator_address), + Self::BlockIDFlagNil { + validator_address, .. + } => Some(*validator_address), + _ => None, + } + } + + /// 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 for CommitSig { diff --git a/tendermint/src/lite/types.rs b/tendermint/src/lite/types.rs index ca927e0b3..14a4509c9 100644 --- a/tendermint/src/lite/types.rs +++ b/tendermint/src/lite/types.rs @@ -97,6 +97,12 @@ pub struct TrustThresholdFraction { } impl TrustThresholdFraction { + /// Constant for a trust threshold of 2/3. + pub const TWO_THIRDS: Self = Self { + numerator: 2, + denominator: 3, + }; + /// Instantiate a TrustThresholdFraction if the given denominator and /// numerator are valid. ///