Light client: bisection, store, requester (#100)

* Add bisection to light client module:

 - implement `can_trust` according to the latest spec CanTrustBisection (wip)
 - add a `Requester` and a `Store` trait
 - sue refs in parameters (as they are used in several places and we don't want to Copy)

* Group params h1 & ha_next_vals into a type `TrustedState`

 - reduce params to clippy threshold (7); still a lot ...
 - rename vars for better readability

* Add VerifyHeader logic from spec:

 - rename expired to is_within_trust_period to be closer to the spec
 - use TrusteState trait in check_support params
 - use TrusteState in lite tests

* Review: doc improvements and minor improvements on naming

* Review: doc improvements

* More doc improvements

* Minor doc improvement

* MockHeader and test_is_within_trust_period (#104)

* MockHeader and test_is_within_trust_period

* remove Validator trait; make ValidatorSet associated to Commit (#105)

* remove Validator trait; make ValidatorSet associated to Commit

* remove stale comment

* Fix clippy errors (#108)

* Review comments: (#107)

* Review comments:

 - update some comments / documentation
 - verify vals (not next vals)
 - store header and vals in trusted state

* One offs & renaming related to trusted state

* Rename & remove redundant store of trusted state: c
 - an_trust -> can_trust_bisection
 - remove a redundant check for trust period
 - remove redundant adding state to the store (added TODO)

* Bucky/some light follow up (#109)

* use ? instead of return Err(err)

* remove unused errors

* move a validation and add TODOs about them

* check_support returns early for sequential case

* drop let _ =

* add comment to voting_power_in about signers

Co-authored-by: Ethan Buchman <ethan@coinculture.info>

tendermint/src/block/commit.rs

@@ -32,7 +32,7 @@ impl Precommits {
 
     /// Convert this collection of precommits into a vector
     pub fn into_vec(self) -> Vec<Option<Vote>> {
-        self.0.clone()
+        self.0
     }
 
     /// Iterate over the precommits in the collection

tendermint/src/block/signed_header.rs

@@ -1,12 +1,7 @@
 //! SignedHeader contains commit and and block header.
 //! It is what the rpc endpoint /commit returns and hence can be used by a
 //! light client.
-use crate::{
-    block, hash, lite,
-    lite::types::Validator,
-    lite::{Error, ValidatorSet},
-    vote::SignedVote,
-};
+use crate::{block, hash, lite, lite::Error, validator::Set, vote::SignedVote};
 use serde::{Deserialize, Serialize};
 
 /// Signed block headers
@@ -54,14 +49,16 @@ impl SignedHeader {
 }
 
 impl lite::Commit for SignedHeader {
+    type ValidatorSet = Set;
+
     fn header_hash(&self) -> hash::Hash {
         self.commit.block_id.hash
     }
+    fn votes_len(&self) -> usize {
+        self.commit.precommits.len()
+    }
 
-    fn voting_power_in<V>(&self, validators: &V) -> Result<u64, Error>
-    where
-        V: ValidatorSet,
-    {
+    fn voting_power_in(&self, validators: &Set) -> Result<u64, 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 = 0u64;
@@ -93,8 +90,4 @@ impl lite::Commit for SignedHeader {
 
         Ok(signed_power)
     }
-
-    fn votes_len(&self) -> usize {
-        self.commit.precommits.len()
-    }
 }

tendermint/src/consensus/state.rs

@@ -49,18 +49,14 @@ impl fmt::Display for State {
 
 impl Ord for State {
     fn cmp(&self, other: &State) -> Ordering {
-        if self.height < other.height {
-            Ordering::Less
-        } else if self.height == other.height {
-            if self.round < other.round {
-                Ordering::Less
-            } else if self.round == other.round {
-                self.step.cmp(&other.step)
-            } else {
-                Ordering::Greater
-            }
-        } else {
-            Ordering::Greater
+        match self.height.cmp(&other.height) {
+            Ordering::Greater => Ordering::Greater,
+            Ordering::Less => Ordering::Less,
+            Ordering::Equal => match self.round.cmp(&other.round) {
+                Ordering::Greater => Ordering::Greater,
+                Ordering::Less => Ordering::Less,
+                Ordering::Equal => self.step.cmp(&other.step),
+            },
         }
     }
 }

tendermint/src/lite/types.rs

@@ -1,30 +1,26 @@
 //! All traits that are necessary and need to be implemented to use the main
 //! verification logic in `super::verifier` for a light client.
 
-// TODO can we abstract this away and use a generic identifier instead ?
-// Ie. something that just implements Eq ?
-// (Ismail): a really easy solution would be have a trait that expects an
-// as_bytes(&self) -> &[u8] method. It's unlikely that a hash won't be
-// representable as bytes, or an Id (that is basically also a hash)
-// but this feels a a bit like cheating
-use crate::account::Id;
-
 use crate::block::Height;
 use crate::Hash;
 
 use failure::_core::fmt::Debug;
 use std::time::SystemTime;
 
 /// TrustedState stores the latest state trusted by a lite client,
-/// including the last header and the validator set to use to verify
-/// the next header.
-pub struct TrustedState<H, V>
-where
-    H: Header,
-    V: ValidatorSet,
-{
-    pub last_header: H, // height H-1
-    pub validators: V,  // height H
+/// including the last header (at height h-1) and the validator set
+/// (at height h) to use to verify the next header.
+pub trait TrustedState {
+    type LastHeader: SignedHeader;
+    type ValidatorSet: ValidatorSet;
+
+    /// Initialize the TrustedState with the given signed header and validator set.
+    /// Note that if the height of the passed in header is h-1, the passed in validator set
+    /// must have been requested for height h.
+    fn new(last_header: &Self::LastHeader, vals: &Self::ValidatorSet) -> Self;
+
+    fn last_header(&self) -> &Self::LastHeader; // height H-1
+    fn validators(&self) -> &Self::ValidatorSet; // height H
 }
 
 /// SignedHeader bundles a Header and a Commit for convenience.
@@ -61,48 +57,38 @@ pub trait Header: Debug {
 /// It also provides a lookup method to fetch a validator by
 /// its identifier.
 pub trait ValidatorSet {
-    type Validator: Validator;
-
     /// Hash of the validator set.
     fn hash(&self) -> Hash;
 
     /// Total voting power of the set
     fn total_power(&self) -> u64;
 
-    /// Fetch validator via their ID (ie. their address).
-    fn validator(&self, val_id: Id) -> Option<Self::Validator>;
-
     /// Return the number of validators in this validator set.
     fn len(&self) -> usize;
 
     /// Returns true iff the validator set is empty.
     fn is_empty(&self) -> bool;
 }
 
-/// Validator has a voting power and can verify
-/// its own signatures. Note it must have implicit access
-/// to its public key material to verify signatures.
-pub trait Validator {
-    fn power(&self) -> u64;
-    fn verify_signature(&self, sign_bytes: &[u8], signature: &[u8]) -> bool;
-}
-
 /// Commit is proof a Header is valid.
 /// It has an underlying Vote type with the relevant vote data
 /// for verification.
 pub trait Commit {
+    type ValidatorSet: ValidatorSet;
+
     /// Hash of the header this commit is for.
     fn header_hash(&self) -> Hash;
 
     /// Compute the voting power of the validators that correctly signed the commit,
-    /// have according to their voting power in the passed in validator set.
+    /// according to their voting power in the passed in validator set.
     /// Will return an error in case an invalid signature was included.
     ///
-    /// This method corresponds to the (pure) auxiliary function int the spec:
+    /// This method corresponds to the (pure) auxiliary function in the spec:
     /// `votingpower_in(signers(h.Commit),h.Header.V)`.
-    fn voting_power_in<V>(&self, vals: &V) -> Result<u64, Error>
-    where
-        V: ValidatorSet;
+    /// Note this expects the Commit to be able to compute `signers(h.Commit)`,
+    /// ie. the identity of the validators that signed it, so they
+    /// can be cross-referenced with the given `vals`.
+    fn voting_power_in(&self, vals: &Self::ValidatorSet) -> Result<u64, Error>;
 
     /// Return the number of votes included in this commit
     /// (including nil/empty votes).
@@ -122,18 +108,54 @@ pub trait TrustThreshold {
     }
 }
 
-#[derive(Debug)]
+/// Requester can be used to request `SignedHeaders` and `ValidatorSet`s for a
+/// given height, e.g., by talking to a tendermint fullnode through RPC.
+pub trait Requester {
+    // TODO(Liamsi): consider putting this trait and the Store into a separate module / file...
+    type SignedHeader: SignedHeader;
+    type ValidatorSet: ValidatorSet;
+
+    /// Request the signed header at height h.
+    fn signed_header<H>(&self, h: H) -> Result<Self::SignedHeader, Error>
+    where
+        H: Into<Height>;
+
+    /// Request the validator set at height h.
+    fn validator_set<H>(&self, h: H) -> Result<Self::ValidatorSet, Error>
+    where
+        H: Into<Height>;
+}
+
+/// This store can be used to store all the headers that have passed basic verification
+/// and that are within the light client's trust period.
+pub trait Store {
+    type TrustedState: TrustedState;
+
+    /// Add this state (header at height h, validators at height h+1) as trusted to the store.
+    fn add(&mut self, trusted: &Self::TrustedState) -> Result<(), Error>;
+
+    /// Retrieve the trusted state at height h if it exists.
+    /// If it does not exist return an error.
+    fn get(&self, h: Height) -> Result<&Self::TrustedState, Error>;
+
+    /// Retrieve the trusted signed header with the largest height h' with h' <= h, if it exists.
+    /// If it does not exist return an error.
+    fn get_smaller_or_equal(&self, h: Height) -> Result<Self::TrustedState, Error>;
+}
+
+#[derive(Debug, PartialEq)]
 pub enum Error {
     Expired,
     DurationOutOfRange,
-    NonSequentialHeight,
-    NonIncreasingHeight,
+
+    InvalidSignature, // TODO: deduplicate with ErrorKind::SignatureInvalid
 
     InvalidValidatorSet,
     InvalidNextValidatorSet,
     InvalidCommitValue, // commit is not for the header we expected
     InvalidCommitLength,
-    InvalidSignature,
 
-    InsufficientVotingPower,
+    InsufficientVotingPower, // TODO(Liamsi): change to same name as spec if this changes (curently ErrTooMuchChange)
+
+    RequestFailed,
 }