From 393a2eff8b1ba8256257697626f38ed0b290b6a0 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 9 Jun 2020 12:29:25 +0200 Subject: [PATCH] Light Client: Follow-up (#284) * Introduce clock_drift verification parameter, as per the spec * Re-enable contract in IO component * Document the scheduler component * Cleanup fork detector * Document verifier * Fix tests and example * Add more documentation * Add more spec tags for traceability * Rename light_client crate to tendermint_light_client * WIP: Enable precondition on verify_to_target * Improve documentation * Point to light_client module for main documentation * Update #[deny] flags * Include trust threshold in InsufficientValidatorsOverlap error * Improve performance of LightStore::latest * Fix verifier predicates logic * Remove Verdict::and_then * Formatting * Disable test unsupported on CI * Address review comments * Rename scheduler::basic_schedule to scheduler::basic_bisecting_schedule --- light-client/Cargo.toml | 2 +- light-client/examples/light_client.rs | 24 +--- light-client/src/components.rs | 2 + light-client/src/components/fork_detector.rs | 12 +- light-client/src/components/io.rs | 17 ++- light-client/src/components/scheduler.rs | 75 ++++++++--- light-client/src/components/verifier.rs | 111 ++++++--------- light-client/src/contracts.rs | 20 +-- light-client/src/errors.rs | 2 + light-client/src/lib.rs | 16 ++- light-client/src/light_client.rs | 17 ++- light-client/src/macros.rs | 2 + light-client/src/predicates.rs | 135 ++++++++++--------- light-client/src/predicates/errors.rs | 12 +- light-client/src/prelude.rs | 3 +- light-client/src/state.rs | 2 + light-client/src/store.rs | 6 + light-client/src/store/sled.rs | 3 +- light-client/src/store/sled/utils.rs | 31 ++++- light-client/src/tests.rs | 2 + light-client/src/types.rs | 2 + light-client/tests/light_client.rs | 41 +++--- tendermint/src/lite/types.rs | 8 +- 23 files changed, 321 insertions(+), 224 deletions(-) diff --git a/light-client/Cargo.toml b/light-client/Cargo.toml index 691c65986..08faad28f 100644 --- a/light-client/Cargo.toml +++ b/light-client/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "light-client" +name = "tendermint-light-client" version = "0.1.0" authors = ["Romain Ruetschi "] edition = "2018" diff --git a/light-client/examples/light_client.rs b/light-client/examples/light_client.rs index 8beca6fb9..8a9dc4961 100644 --- a/light-client/examples/light_client.rs +++ b/light-client/examples/light_client.rs @@ -1,5 +1,5 @@ use gumdrop::Options; -use light_client::prelude::Height; +use tendermint_light_client::prelude::Height; use std::collections::HashMap; use std::path::PathBuf; @@ -58,8 +58,8 @@ fn main() { } fn sync_cmd(opts: SyncOpts) { - use light_client::components::scheduler; - use light_client::prelude::*; + use tendermint_light_client::components::scheduler; + use tendermint_light_client::prelude::*; let primary_addr = opts.address; let primary: PeerId = "BADFADAD0BEFEEDC0C0ADEADBEEFC0FFEEFACADE".parse().unwrap(); @@ -102,24 +102,14 @@ fn sync_cmd(opts: SyncOpts) { denominator: 3, }, trusting_period: Duration::from_secs(36000), + clock_drift: Duration::from_secs(1), now: Time::now(), }; - let predicates = ProdPredicates; - let voting_power_calculator = ProdVotingPowerCalculator; - let commit_validator = ProdCommitValidator; - let header_hasher = ProdHeaderHasher; - - let verifier = ProdVerifier::new( - predicates, - voting_power_calculator, - commit_validator, - header_hasher, - ); - + let verifier = ProdVerifier::default(); let clock = SystemClock; - let scheduler = scheduler::schedule; - let fork_detector = RealForkDetector::new(header_hasher); + let scheduler = scheduler::basic_bisecting_schedule; + let fork_detector = ProdForkDetector::default(); let mut light_client = LightClient::new( state, diff --git a/light-client/src/components.rs b/light-client/src/components.rs index c7e352643..a3eb766dc 100644 --- a/light-client/src/components.rs +++ b/light-client/src/components.rs @@ -1,3 +1,5 @@ +//! Components used by the Light Client. + pub mod clock; pub mod fork_detector; pub mod io; diff --git a/light-client/src/components/fork_detector.rs b/light-client/src/components/fork_detector.rs index 07ef624d4..a264cd967 100644 --- a/light-client/src/components/fork_detector.rs +++ b/light-client/src/components/fork_detector.rs @@ -14,11 +14,11 @@ pub trait ForkDetector { fn detect(&self, light_blocks: Vec) -> ForkDetection; } -pub struct RealForkDetector { +pub struct ProdForkDetector { header_hasher: Box, } -impl RealForkDetector { +impl ProdForkDetector { pub fn new(header_hasher: impl HeaderHasher + 'static) -> Self { Self { header_hasher: Box::new(header_hasher), @@ -26,7 +26,13 @@ impl RealForkDetector { } } -impl ForkDetector for RealForkDetector { +impl Default for ProdForkDetector { + fn default() -> Self { + Self::new(ProdHeaderHasher) + } +} + +impl ForkDetector for ProdForkDetector { fn detect(&self, mut light_blocks: Vec) -> ForkDetection { if light_blocks.is_empty() { return ForkDetection::NotDetected; diff --git a/light-client/src/components/io.rs b/light-client/src/components/io.rs index 284a750c7..cbfb78cff 100644 --- a/light-client/src/components/io.rs +++ b/light-client/src/components/io.rs @@ -1,4 +1,6 @@ -use contracts::pre; +use std::collections::HashMap; + +use contracts::{contract_trait, post, pre}; use serde::{Deserialize, Serialize}; use tendermint::{block, rpc}; use thiserror::Error; @@ -7,7 +9,6 @@ use tendermint::block::signed_header::SignedHeader as TMSignedHeader; use tendermint::validator::Set as TMValidatorSet; use crate::prelude::*; -use std::collections::HashMap; pub const LATEST_HEIGHT: Height = 0; @@ -19,15 +20,17 @@ pub enum IoError { } /// Interface for fetching light blocks from a full node, typically via the RPC client. -// TODO: Enable contracts on the trait once the provider field is available. -// #[contract_trait] +#[contract_trait] pub trait Io { /// Fetch a light block at the given height from the peer with the given peer ID. - // #[post(ret.map(|lb| lb.provider == peer).unwrap_or(true))] + /// + /// ## Postcondition + /// - The provider of the returned light block matches the given peer [LCV-IO-POST-PROVIDER] + #[post(ret.as_ref().map(|lb| lb.provider == peer).unwrap_or(true))] fn fetch_light_block(&mut self, peer: PeerId, height: Height) -> Result; } -// #[contract_trait] +#[contract_trait] impl Io for F where F: FnMut(PeerId, Height) -> Result, @@ -44,7 +47,7 @@ pub struct ProdIo { peer_map: HashMap, } -// #[contract_trait] +#[contract_trait] impl Io for ProdIo { fn fetch_light_block(&mut self, peer: PeerId, height: Height) -> Result { let signed_header = self.fetch_signed_header(peer, height)?; diff --git a/light-client/src/components/scheduler.rs b/light-client/src/components/scheduler.rs index b3d9a34a0..982d915e8 100644 --- a/light-client/src/components/scheduler.rs +++ b/light-client/src/components/scheduler.rs @@ -1,14 +1,25 @@ use crate::prelude::*; use contracts::*; +/// The scheduler decides what block to verify next given the current and target heights. +/// +/// The scheduler is given access to the light store, in order to optionally +/// improve performance by picking a next block that has already been fetched. #[contract_trait] pub trait Scheduler { + /// Decides what block to verify next. + /// + /// ## Precondition + /// - The light store contains at least one verified block. [LCV-SCHEDULE-PRE.1] + /// + /// ## Postcondition + /// - The resulting height must be valid according to `valid_schedule`. [LCV-SCHEDULE-POST.1] #[pre(light_store.latest(VerifiedStatus::Verified).is_some())] - #[post(valid_schedule(ret, target_height, next_height, light_store))] + #[post(valid_schedule(ret, target_height, current_height, light_store))] fn schedule( &self, light_store: &dyn LightStore, - next_height: Height, + current_height: Height, target_height: Height, ) -> Height; } @@ -21,18 +32,26 @@ where fn schedule( &self, light_store: &dyn LightStore, - next_height: Height, + current_height: Height, target_height: Height, ) -> Height { - self(light_store, next_height, target_height) + self(light_store, current_height, target_height) } } +/// Basic bisecting scheduler which picks the appropriate midpoint without +/// optimizing for performance using the blocks available in the light store. +/// +/// ## Precondition +/// - The light store contains at least one verified block. [LCV-SCHEDULE-PRE.1] +/// +/// ## Postcondition +/// - The resulting height must be valid according to `valid_schedule`. [LCV-SCHEDULE-POST.1] #[pre(light_store.latest(VerifiedStatus::Verified).is_some())] -#[post(valid_schedule(ret, target_height, next_height, light_store))] -pub fn schedule( +#[post(valid_schedule(ret, target_height, current_height, light_store))] +pub fn basic_bisecting_schedule( light_store: &dyn LightStore, - next_height: Height, + current_height: Height, target_height: Height, ) -> Height { let latest_trusted_height = light_store @@ -40,21 +59,43 @@ pub fn schedule( .map(|lb| lb.height()) .unwrap(); - if latest_trusted_height == next_height && latest_trusted_height < target_height { + if latest_trusted_height == current_height && latest_trusted_height < target_height { target_height - } else if latest_trusted_height < next_height && latest_trusted_height < target_height { - midpoint(latest_trusted_height, next_height) + } else if latest_trusted_height < current_height && latest_trusted_height < target_height { + midpoint(latest_trusted_height, current_height) } else if latest_trusted_height == target_height { target_height } else { - midpoint(next_height, target_height) + midpoint(current_height, target_height) } } -fn valid_schedule( +/// Checks whether the given `scheduled_height` is a valid schedule according to the +/// following specification. +/// +/// - i) If `latest_verified_height == current_height` and `latest_verified_height < target_height` +/// then `current_height < scheduled_height <= target_height`. +/// +/// - ii) If `latest_verified_height < current_height` and `latest_verified_height < target_height` +/// then `latest_verified_height < scheduled_height < current_height`. +/// +/// - iii) If `latest_verified_height = target_height` then `scheduled_height == target_height`. +/// +/// ## Note +/// +/// - Case i. captures the case where the light block at height `current_height` has been verified, +/// and we can choose a height closer to the `target_height`. As we get the `light_store` as parameter, +/// the choice of the next height can depend on the `light_store`, e.g., we can pick a height +/// for which we have already downloaded a light block. +/// - In Case ii. the header at `current_height` could not be verified, and we need to pick a lesser height. +/// - In Case iii. is a special case when we have verified the `target_height`. +/// +/// ## Implements +/// - [LCV-SCHEDULE-POST.1] +pub fn valid_schedule( scheduled_height: Height, target_height: Height, - next_height: Height, + current_height: Height, light_store: &dyn LightStore, ) -> bool { let latest_trusted_height = light_store @@ -62,10 +103,10 @@ fn valid_schedule( .map(|lb| lb.height()) .unwrap(); - if latest_trusted_height == next_height && latest_trusted_height < target_height { - next_height < scheduled_height && scheduled_height <= target_height - } else if latest_trusted_height < next_height && latest_trusted_height < target_height { - latest_trusted_height < scheduled_height && scheduled_height < next_height + if latest_trusted_height == current_height && latest_trusted_height < target_height { + current_height < scheduled_height && scheduled_height <= target_height + } else if latest_trusted_height < current_height && latest_trusted_height < target_height { + latest_trusted_height < scheduled_height && scheduled_height < current_height } else if latest_trusted_height == target_height { scheduled_height == target_height } else { diff --git a/light-client/src/components/verifier.rs b/light-client/src/components/verifier.rs index cf782f7c7..26ee1b659 100644 --- a/light-client/src/components/verifier.rs +++ b/light-client/src/components/verifier.rs @@ -1,21 +1,19 @@ +use crate::predicates as preds; use crate::prelude::*; +/// Represents the result of the verification performed by the +/// verifier component. #[derive(Debug)] pub enum Verdict { + /// Verification succeeded, the block is valid. Success, + /// The minimum voting power threshold is not reached, + /// the block cannot be trusted yet. NotEnoughTrust(VerificationError), + /// Verification failed, the block is invalid. Invalid(VerificationError), } -impl Verdict { - pub fn and_then(self, other: impl Fn() -> Verdict) -> Self { - match self { - Verdict::Success => other(), - _ => self, - } - } -} - impl From> for Verdict { fn from(result: Result<(), VerificationError>) -> Self { match result { @@ -26,35 +24,29 @@ impl From> for Verdict { } } +/// The verifier checks: +/// +/// a) whether a given untrusted light block is valid, and +/// b) whether a given untrusted light block should be trusted +/// based on a previously verified block. +/// +/// ## Implements +/// - [TMBC-VAL-CONTAINS-CORR.1] +/// - [TMBC-VAL-COMMIT.1] pub trait Verifier { - fn verify( - &self, - light_block: &LightBlock, - trusted_state: &TrustedState, - options: &Options, - ) -> Verdict { - self.validate_light_block(light_block, trusted_state, options) - .and_then(|| self.verify_overlap(light_block, trusted_state, options)) - .and_then(|| self.has_sufficient_voting_power(light_block, options)) - } - - fn validate_light_block( - &self, - light_block: &LightBlock, - trusted_state: &TrustedState, - options: &Options, - ) -> Verdict; - - fn verify_overlap( - &self, - light_block: &LightBlock, - trusted_state: &TrustedState, - options: &Options, - ) -> Verdict; - - fn has_sufficient_voting_power(&self, light_block: &LightBlock, options: &Options) -> Verdict; + /// Perform the verification. + fn verify(&self, untrusted: &LightBlock, trusted: &LightBlock, options: &Options) -> Verdict; } +/// Production implementation of the verifier. +/// +/// For testing purposes, this implementation is parametrized by: +/// - A set of predicates used to validate a light block +/// - A voting power calculator +/// - A commit validator +/// - A header hasher +/// +/// For regular use, one can construct a standard implementation with `ProdVerifier::default()`. pub struct ProdVerifier { predicates: Box, voting_power_calculator: Box, @@ -78,45 +70,26 @@ impl ProdVerifier { } } -impl Verifier for ProdVerifier { - fn validate_light_block( - &self, - light_block: &LightBlock, - trusted_state: &TrustedState, - options: &Options, - ) -> Verdict { - crate::predicates::validate_light_block( - &*self.predicates, - &self.commit_validator, - &self.header_hasher, - &trusted_state, - &light_block, - options, +impl Default for ProdVerifier { + fn default() -> Self { + Self::new( + ProdPredicates, + ProdVotingPowerCalculator, + ProdCommitValidator, + ProdHeaderHasher, ) - .into() - } - - fn verify_overlap( - &self, - light_block: &LightBlock, - trusted_state: &TrustedState, - options: &Options, - ) -> Verdict { - crate::predicates::verify_overlap( - &*self.predicates, - &self.voting_power_calculator, - &trusted_state, - &light_block, - options, - ) - .into() } +} - fn has_sufficient_voting_power(&self, light_block: &LightBlock, options: &Options) -> Verdict { - crate::predicates::has_sufficient_voting_power( +impl Verifier for ProdVerifier { + fn verify(&self, untrusted: &LightBlock, trusted: &TrustedState, options: &Options) -> Verdict { + preds::verify( &*self.predicates, &self.voting_power_calculator, - &light_block, + &self.commit_validator, + &self.header_hasher, + &trusted, + &untrusted, options, ) .into() diff --git a/light-client/src/contracts.rs b/light-client/src/contracts.rs index 6521e850a..eb9a9f476 100644 --- a/light-client/src/contracts.rs +++ b/light-client/src/contracts.rs @@ -1,3 +1,5 @@ +//! Predicates used in components contracts. + use crate::prelude::*; pub fn trusted_store_contains_block_at_target_height( @@ -18,15 +20,15 @@ pub fn is_within_trust_period( header_time > now - trusting_period } -// pub fn trusted_state_contains_block_within_trusting_period( -// light_store: &dyn LightStore, -// trusting_period: Duration, -// now: Time, -// ) -> bool { -// light_store -// .all(VerifiedStatus::Verified) -// .any(|lb| is_within_trust_period(&lb, trusting_period, now)) -// } +pub fn light_store_contains_block_within_trusting_period( + light_store: &dyn LightStore, + trusting_period: Duration, + now: Time, +) -> bool { + light_store + .all(VerifiedStatus::Verified) + .any(|lb| is_within_trust_period(&lb, trusting_period, now)) +} // pub fn target_height_greater_than_all_blocks_in_trusted_store( // light_store: &dyn LightStore, diff --git a/light-client/src/errors.rs b/light-client/src/errors.rs index c1426e889..162fc274c 100644 --- a/light-client/src/errors.rs +++ b/light-client/src/errors.rs @@ -1,3 +1,5 @@ +//! Toplevel errors raised by the light client. + use anomaly::{BoxError, Context}; use serde::{Deserialize, Serialize}; use thiserror::Error; diff --git a/light-client/src/lib.rs b/light-client/src/lib.rs index 8240d6897..1eb3a2bf8 100644 --- a/light-client/src/lib.rs +++ b/light-client/src/lib.rs @@ -1,9 +1,17 @@ -#![deny(rust_2018_idioms, nonstandard_style)] -#![warn( - unreachable_pub, - // missing_docs, +#![forbid(unsafe_code)] +#![deny( + // warnings, + // missing_docs, + trivial_casts, + trivial_numeric_casts, + unused_import_braces, + unused_qualifications, + rust_2018_idioms, + nonstandard_style, )] +//! See the `light_client` module for the main documentation. + pub mod components; pub mod contracts; pub mod errors; diff --git a/light-client/src/light_client.rs b/light-client/src/light_client.rs index 89a9ff46d..d86c225d5 100644 --- a/light-client/src/light_client.rs +++ b/light-client/src/light_client.rs @@ -1,3 +1,7 @@ +//! Light client implementation as per the [Core Verification specification][1]. +//! +//! [1]: https://github.com/informalsystems/tendermint-rs/blob/master/docs/spec/lightclient/verification.md + use contracts::*; use derive_more::Display; use serde::{Deserialize, Serialize}; @@ -18,6 +22,8 @@ pub struct Options { pub trust_threshold: TrustThreshold, /// The trusting period pub trusting_period: Duration, + /// Correction parameter dealing with only approximately synchronized clocks. + pub clock_drift: Duration, /// The current time pub now: Time, } @@ -98,7 +104,7 @@ impl LightClient { /// - [LCV-DIST-SAFE.1] /// - [LCV-DIST-LIFE.1] /// - [LCV-PRE-TP.1] - /// - [LCV-POST-TP.1] + /// - [LCV-POST-LS.1] /// - [LCV-INV-TP.1] /// /// ## Precondition @@ -106,13 +112,20 @@ impl LightClient { /// /// ## Postcondition /// - The light store contains a light block that corresponds - /// to a block of the blockchain of height `target_height` [LCV-POST-TP.1] + /// to a block of the blockchain of height `target_height` [LCV-POST-LS.1] /// /// ## Error conditions /// - If the precondition is violated [LVC-PRE-TP.1] /// - If the core verification loop invariant is violated [LCV-INV-TP.1] /// - If verification of a light block fails /// - If it cannot fetch a block from the blockchain + // #[pre( + // light_store_contains_block_within_trusting_period( + // self.state.light_store.as_ref(), + // self.options.trusting_period, + // self.clock.now(), + // ) + // )] #[post( ret.is_ok() ==> trusted_store_contains_block_at_target_height( self.state.light_store.as_ref(), diff --git a/light-client/src/macros.rs b/light-client/src/macros.rs index 0cc4c9472..a5d5aa94f 100644 --- a/light-client/src/macros.rs +++ b/light-client/src/macros.rs @@ -1,3 +1,5 @@ +//! Small macros used internally. + /// Bail out of the current function with the given error kind. #[macro_export] macro_rules! bail { diff --git a/light-client/src/predicates.rs b/light-client/src/predicates.rs index 9e343bd39..6c5660868 100644 --- a/light-client/src/predicates.rs +++ b/light-client/src/predicates.rs @@ -1,14 +1,23 @@ +//! Predicates for light block validation and verification. + use crate::prelude::*; use tendermint::lite::ValidatorSet as _; pub mod errors; +/// Production predicates, using the default implementation +/// of the `VerificationPredicates` trait. #[derive(Copy, Clone, Debug)] pub struct ProdPredicates; - impl VerificationPredicates for ProdPredicates {} +/// Defines the various predicates used to validate and verify light blocks. +/// +/// A default, spec abiding implementation is provided for each method. +/// +/// This enables test implementations to only override a single method rather than +/// have to re-define every predicate. pub trait VerificationPredicates { fn validator_sets_match(&self, light_block: &LightBlock) -> Result<(), VerificationError> { ensure!( @@ -71,23 +80,23 @@ pub trait VerificationPredicates { &self, header: &Header, trusting_period: Duration, + clock_drift: Duration, now: Time, ) -> Result<(), VerificationError> { - let expires_at = header.time + trusting_period; - ensure!( - header.time < now && expires_at > now, - VerificationError::NotWithinTrustPeriod { - at: expires_at, - now, + header.time < now + clock_drift, + VerificationError::HeaderFromTheFuture { + header_time: header.time, + now } ); + let expires_at = header.time + trusting_period; ensure!( - header.time <= now, - VerificationError::HeaderFromTheFuture { - header_time: header.time, - now + expires_at > now, + VerificationError::NotWithinTrustPeriod { + at: expires_at, + now, } ); @@ -170,6 +179,7 @@ pub trait VerificationPredicates { VerificationError::InsufficientValidatorsOverlap { total_power, signed_power: voting_power, + trust_threshold: *trust_threshold, } ); @@ -182,6 +192,7 @@ pub trait VerificationPredicates { 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) @@ -216,94 +227,86 @@ pub trait VerificationPredicates { } } -pub fn validate_light_block( +/// Validate the given light block. +/// +/// - Ensure the latest trusted header hasn't expired +/// - Ensure the header validator hashes match the given validators +/// - Ensure the header next validator hashes match the given next validators +/// - Additional implementation specific validation via `commit_validator` +/// - Check that the untrusted block is more recent than the trusted state +/// - If the untrusted block is the very next block after the trusted block, +/// check that their (next) validator sets hashes match. +/// - Otherwise, ensure that the untrusted block has a greater height than +/// the trusted block. +pub fn verify( vp: &dyn VerificationPredicates, + voting_power_calculator: &dyn VotingPowerCalculator, commit_validator: &dyn CommitValidator, header_hasher: &dyn HeaderHasher, - trusted_state: &TrustedState, - light_block: &LightBlock, + trusted: &LightBlock, + untrusted: &LightBlock, options: &Options, ) -> Result<(), VerificationError> { // Ensure the latest trusted header hasn't expired vp.is_within_trust_period( - &trusted_state.signed_header.header, + &trusted.signed_header.header, options.trusting_period, + options.clock_drift, options.now, )?; // Ensure the header validator hashes match the given validators - vp.validator_sets_match(&light_block)?; + vp.validator_sets_match(&untrusted)?; // Ensure the header next validator hashes match the given next validators - vp.next_validators_match(&light_block)?; + vp.next_validators_match(&untrusted)?; // Ensure the header matches the commit - vp.header_matches_commit(&light_block.signed_header, header_hasher)?; + vp.header_matches_commit(&untrusted.signed_header, header_hasher)?; // Additional implementation specific validation vp.valid_commit( - &light_block.signed_header, - &light_block.validators, + &untrusted.signed_header, + &untrusted.validators, commit_validator, )?; + // Check that the untrusted block is more recent than the trusted state vp.is_monotonic_bft_time( - &light_block.signed_header.header, - &trusted_state.signed_header.header, + &untrusted.signed_header.header, + &trusted.signed_header.header, )?; - let trusted_state_next_height = trusted_state - .height() - .checked_add(1) - .expect("height overflow"); + let trusted_next_height = trusted.height().checked_add(1).expect("height overflow"); - if light_block.height() == trusted_state_next_height { - vp.valid_next_validator_set(&light_block, trusted_state)?; + if untrusted.height() == trusted_next_height { + // If the untrusted block is the very next block after the trusted block, + // check that their (next) validator sets hashes match. + vp.valid_next_validator_set(&untrusted, trusted)?; } else { + // Otherwise, ensure that the untrusted block has a greater height than + // the trusted block. vp.is_monotonic_height( - &light_block.signed_header.header, - &trusted_state.signed_header.header, + &untrusted.signed_header.header, + &trusted.signed_header.header, )?; - } - Ok(()) -} - -pub fn verify_overlap( - vp: &dyn VerificationPredicates, - voting_power_calculator: &dyn VotingPowerCalculator, - trusted_state: &TrustedState, - light_block: &LightBlock, - options: &Options, -) -> Result<(), VerificationError> { - let untrusted_sh = &light_block.signed_header; - let untrusted_vals = &light_block.validators; + // Check there is enough overlap between the validator sets of + // the trusted and untrusted blocks. + vp.has_sufficient_validators_overlap( + &untrusted.signed_header, + &trusted.next_validators, + &options.trust_threshold, + voting_power_calculator, + )?; + } - vp.has_sufficient_validators_overlap( - &untrusted_sh, - &trusted_state.next_validators, - &options.trust_threshold, + // Verify that more than 2/3 of the validators correctly committed the block. + vp.has_sufficient_signers_overlap( + &untrusted.signed_header, + &untrusted.validators, voting_power_calculator, )?; - vp.has_sufficient_signers_overlap(&untrusted_sh, &untrusted_vals, voting_power_calculator)?; - Ok(()) } - -pub fn has_sufficient_voting_power( - vp: &dyn VerificationPredicates, - voting_power_calculator: &dyn VotingPowerCalculator, - light_block: &LightBlock, - options: &Options, -) -> Result<(), VerificationError> { - let untrusted_sh = &light_block.signed_header; - let untrusted_vals = &light_block.validators; - - vp.has_sufficient_voting_power( - &untrusted_sh, - &untrusted_vals, - &options.trust_threshold, - voting_power_calculator, - ) -} diff --git a/light-client/src/predicates/errors.rs b/light-client/src/predicates/errors.rs index aeec939ee..52231979a 100644 --- a/light-client/src/predicates/errors.rs +++ b/light-client/src/predicates/errors.rs @@ -4,6 +4,8 @@ use thiserror::Error; use crate::prelude::*; +/// The various errors which can be raised by the verifier component, +/// when validating or verifying a light block. #[derive(Debug, Clone, Error, PartialEq, Serialize, Deserialize)] pub enum VerificationError { #[error("header from the future: header_time={header_time} now={now}")] @@ -11,9 +13,13 @@ pub enum VerificationError { #[error("implementation specific: {0}")] ImplementationSpecific(String), #[error( - "insufficient validators overlap: total_power={total_power} signed_power={signed_power}" + "insufficient validators overlap: total_power={total_power} signed_power={signed_power} trust_threshold={trust_threshold}" )] - InsufficientValidatorsOverlap { total_power: u64, signed_power: u64 }, + InsufficientValidatorsOverlap { + total_power: u64, + signed_power: u64, + trust_threshold: TrustThreshold, + }, #[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}")] @@ -53,6 +59,8 @@ impl VerificationError { Context::new(self, Some(source.into())) } + /// Determines whether this error means that the light block is outright invalid, + /// or just cannot be trusted w.r.t. the latest trusted state. pub fn not_enough_trust(&self) -> bool { if let Self::InsufficientValidatorsOverlap { .. } = self { true diff --git a/light-client/src/prelude.rs b/light-client/src/prelude.rs index f1453c2d8..bb8cd1b77 100644 --- a/light-client/src/prelude.rs +++ b/light-client/src/prelude.rs @@ -1,5 +1,4 @@ -//! This prelude re-exports all the types which are commonly used -//! both within the light client codebase, and potentially by its users. +//! Re-exports all the types which are commonly used within the light client codebase. pub use std::time::{Duration, SystemTime}; diff --git a/light-client/src/state.rs b/light-client/src/state.rs index 1309ae5f8..fb1f2bac6 100644 --- a/light-client/src/state.rs +++ b/light-client/src/state.rs @@ -1,3 +1,5 @@ +//! State maintained by the light client. + use crate::prelude::*; use contracts::*; diff --git a/light-client/src/store.rs b/light-client/src/store.rs index 0feb3eda6..97b18c227 100644 --- a/light-client/src/store.rs +++ b/light-client/src/store.rs @@ -1,3 +1,9 @@ +//! Interface and implementations of the light block store. +//! +//! See the `memory` and `sled` modules for: +//! - a transient, in-memory implementation for testing purposes +//! - a persistent, on-disk, sled-backed implementation for production + use crate::prelude::*; use serde::{Deserialize, Serialize}; diff --git a/light-client/src/store/sled.rs b/light-client/src/store/sled.rs index 727c2372f..559332597 100644 --- a/light-client/src/store/sled.rs +++ b/light-client/src/store/sled.rs @@ -65,8 +65,7 @@ impl LightStore for SledStore { } fn latest(&self, status: VerifiedStatus) -> Option { - // FIXME: This is very inefficient since it iterates over all the blocks in the store with the given status. - self.all(status).max_by_key(|lb| lb.height()) + self.db(status).iter(&self.db).next_back() } fn all(&self, status: VerifiedStatus) -> Box> { diff --git a/light-client/src/store/sled/utils.rs b/light-client/src/store/sled/utils.rs index b73f249e3..a1ab3b99a 100644 --- a/light-client/src/store/sled/utils.rs +++ b/light-client/src/store/sled/utils.rs @@ -117,10 +117,39 @@ where Ok(()) } - pub fn iter(&self, db: &sled::Db) -> impl Iterator { + pub fn iter(&self, db: &sled::Db) -> impl DoubleEndedIterator { db.iter() .flatten() .map(|(_, v)| serde_cbor::from_slice(&v)) .flatten() } } + +// TODO: The test below is currently disabled because it fails on CI as we don't have +// access to `/tmp`. Need to figure out how to specify a proper temp dir. + +// #[cfg(test)] +// mod tests { +// use super::*; +// use crate::types::Height; + +// #[test] +// fn iter_next_back_returns_highest_height() { +// const DB_PATH: &str = "/tmp/tendermint_light_client_sled_test/"; +// std::fs::remove_dir_all(DB_PATH).unwrap(); +// let db = sled::open(DB_PATH).unwrap(); +// let kv: KeyValueDb = key_value("light_store/verified"); + +// kv.insert(&db, &1, &1).unwrap(); +// kv.insert(&db, &589473798493, &589473798493).unwrap(); +// kv.insert(&db, &12342425, &12342425).unwrap(); +// kv.insert(&db, &4, &4).unwrap(); + +// let mut iter = kv.iter(&db); +// assert_eq!(iter.next_back(), Some(589473798493)); +// assert_eq!(iter.next_back(), Some(12342425)); +// assert_eq!(iter.next_back(), Some(4)); +// assert_eq!(iter.next_back(), Some(1)); +// assert_eq!(iter.next_back(), None); +// } +// } diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs index 683b1b48b..7dd335c23 100644 --- a/light-client/src/tests.rs +++ b/light-client/src/tests.rs @@ -1,3 +1,5 @@ +//! Utilities and datatypes for use in tests. + use crate::prelude::*; use serde::Deserialize; diff --git a/light-client/src/types.rs b/light-client/src/types.rs index 96562aa1e..09eef2e9c 100644 --- a/light-client/src/types.rs +++ b/light-client/src/types.rs @@ -1,3 +1,5 @@ +//! Defines or just re-exports the main datatypes used by the light client. + use derive_more::Display; use serde::{Deserialize, Serialize}; diff --git a/light-client/tests/light_client.rs b/light-client/tests/light_client.rs index d09a3cbee..6eb195592 100644 --- a/light-client/tests/light_client.rs +++ b/light-client/tests/light_client.rs @@ -1,11 +1,13 @@ -use light_client::prelude::*; -use light_client::tests::{Trusted, *}; +use tendermint_light_client::components::scheduler; +use tendermint_light_client::prelude::*; +use tendermint_light_client::tests::{Trusted, *}; use std::collections::HashMap; use std::convert::TryInto; use std::fs; use std::path::{Path, PathBuf}; +use contracts::contract_trait; use tendermint::rpc; // Link to the commit that generated below JSON test files: @@ -21,14 +23,10 @@ fn verify_single( input: LightBlock, trust_threshold: TrustThreshold, trusting_period: Duration, + clock_drift: Duration, now: SystemTime, ) -> Result { - let verifier = ProdVerifier::new( - ProdPredicates, - ProdVotingPowerCalculator, - ProdCommitValidator, - ProdHeaderHasher, - ); + let verifier = ProdVerifier::default(); let trusted_state = LightBlock::new( trusted_state.signed_header, @@ -40,13 +38,11 @@ fn verify_single( let options = Options { trust_threshold, trusting_period, + clock_drift, now: now.into(), }; - let result = verifier - .validate_light_block(&input, &trusted_state, &options) - .and_then(|| verifier.verify_overlap(&input, &trusted_state, &options)) - .and_then(|| verifier.has_sufficient_voting_power(&input, &options)); + let result = verifier.verify(&input, &trusted_state, &options); match result { Verdict::Success => Ok(input), @@ -65,6 +61,9 @@ fn run_test_case(tc: TestCase) { None => false, }; + // FIXME: What should this be, and where should it be configured? + let clock_drift = Duration::from_secs(1); + let trusting_period: Duration = tc.initial.trusting_period.into(); let tm_now = tc.initial.now; let now = tm_now.to_system_time().unwrap(); @@ -77,6 +76,7 @@ fn run_test_case(tc: TestCase) { input.clone(), TrustThreshold::default(), trusting_period.into(), + clock_drift, now, ) { Ok(new_state) => { @@ -115,6 +115,7 @@ impl MockIo { } } +#[contract_trait] impl Io for MockIo { fn fetch_light_block(&mut self, _peer: PeerId, height: Height) -> Result { self.light_blocks @@ -152,13 +153,16 @@ fn run_bisection_test(tc: TestBisection) { let trusting_period = tc.trust_options.period; let now = tc.now; + // FIXME: What should this be, and where should it be configured? + let clock_drift = Duration::from_secs(1); + let clock = MockClock { now }; - let scheduler = light_client::components::scheduler::schedule; - let fork_detector = RealForkDetector::new(ProdHeaderHasher); + let fork_detector = ProdForkDetector::new(ProdHeaderHasher); let options = Options { trust_threshold, trusting_period: trusting_period.into(), + clock_drift, now, }; @@ -187,18 +191,13 @@ fn run_bisection_test(tc: TestBisection) { verification_trace: HashMap::new(), }; - let verifier = ProdVerifier::new( - ProdPredicates, - ProdVotingPowerCalculator, - ProdCommitValidator, - ProdHeaderHasher, - ); + let verifier = ProdVerifier::default(); let mut light_client = LightClient::new( state, options, clock, - scheduler, + scheduler::basic_bisecting_schedule, verifier, fork_detector, io.clone(), diff --git a/tendermint/src/lite/types.rs b/tendermint/src/lite/types.rs index ada586c81..ca927e0b3 100644 --- a/tendermint/src/lite/types.rs +++ b/tendermint/src/lite/types.rs @@ -1,7 +1,7 @@ //! All traits that are necessary and need to be implemented to use the main //! verification logic in [`super::verifier`] for a light client. -use std::fmt::Debug; +use std::fmt::{self, Debug, Display}; use std::time::SystemTime; use crate::serializers; @@ -130,6 +130,12 @@ impl Default for TrustThresholdFraction { } } +impl Display for TrustThresholdFraction { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}/{}", self.numerator, self.denominator) + } +} + /// Requester can be used to request [`SignedHeader`]s and [`ValidatorSet`]s for a /// given height, e.g., by talking to a tendermint fullnode through RPC. #[async_trait]