Followup on 149 and 151

tendermint-lite/src/main.rs

@@ -61,14 +61,14 @@ fn main() {
             latest_peer_height,
         );
 
-        let now = &SystemTime::now();
+        let now = SystemTime::now();
         let trusted_state = store.get(0).expect("can not read trusted state");
 
         let new_states = lite::verify_bisection(
             trusted_state.clone(),
             latest_peer_height,
             TrustThresholdFraction::default(),
-            &trusting_period,
+            trusting_period,
             now,
             &req,
         )

tendermint/src/lite/error.rs

@@ -2,7 +2,7 @@
 
 use crate::Hash;
 use anomaly::{BoxError, Context};
-use std::time::{SystemTime, SystemTimeError};
+use std::time::SystemTime;
 use thiserror::Error;
 
 /// The main error type verification methods will return.
@@ -17,8 +17,8 @@ pub enum Kind {
     Expired { at: SystemTime, now: SystemTime },
 
     /// Trusted header is from the future.
-    #[error("duration error {:?}", _0)]
-    DurationOutOfRange(#[from] SystemTimeError),
+    #[error("trusted header time is too far in the future")]
+    DurationOutOfRange,
 
     /// Header height smaller than expected.
     #[error("expected height >= {expected} (got: {got})")]

tendermint/src/lite/types.rs

@@ -7,6 +7,8 @@ use crate::lite::error::{Error, Kind};
 use failure::_core::fmt::Debug;
 use std::time::SystemTime;
 
+use anomaly::fail;
+
 pub type Height = u64;
 
 /// Header contains meta data about the block -
@@ -311,9 +313,10 @@ pub(super) mod mocks {
         fn validate(&self, _vals: &Self::ValidatorSet) -> Result<(), Error> {
             // some implementation specific checks:
             if self.vals.is_empty() || self.hash.algorithm() != Algorithm::Sha256 {
-                return Err(Kind::ImplementationSpecific
-                    .context("validator set is empty, or, invalid hash algo".to_string())
-                    .into());
+                fail!(
+                    Kind::ImplementationSpecific,
+                    "validator set is empty, or, invalid hash algo"
+                );
             }
             Ok(())
         }
@@ -342,9 +345,7 @@ pub(super) mod mocks {
                 return Ok(sh.to_owned());
             }
             println!("couldn't get sh for: {}", &h);
-            Err(Kind::RequestFailed
-                .context(format!("couldn't get sh for: {}", &h))
-                .into())
+            fail!(Kind::RequestFailed, "couldn't get sh for: {}", &h);
         }
 
         fn validator_set(&self, h: u64) -> Result<MockValSet, Error> {
@@ -353,9 +354,7 @@ pub(super) mod mocks {
                 return Ok(vs.to_owned());
             }
             println!("couldn't get vals for: {}", &h);
-            Err(Kind::RequestFailed
-                .context(format!("couldn't get vals for: {}", &h))
-                .into())
+            fail!(Kind::RequestFailed, "couldn't get vals for: {}", &h);
         }
     }
 

tendermint/src/lite/verifier.rs

@@ -15,30 +15,38 @@ use crate::lite::error::{Error, Kind};
 use crate::lite::{
     Commit, Header, Height, Requester, SignedHeader, TrustThreshold, TrustedState, ValidatorSet,
 };
+use anomaly::ensure;
+use std::ops::Add;
 
 /// Returns an error if the header has expired according to the given
 /// trusting_period and current time. If so, the verifier must be reset subjectively.
 fn is_within_trust_period<H>(
     last_header: &H,
-    trusting_period: &Duration,
-    now: &SystemTime,
+    trusting_period: Duration,
+    now: SystemTime,
 ) -> Result<(), Error>
 where
     H: Header,
 {
-    match now.duration_since(last_header.bft_time().into()) {
-        Ok(passed) => {
-            if passed > *trusting_period {
-                return Err(Kind::Expired {
-                    at: last_header.bft_time().into() + *trusting_period,
-                    now: *now,
-                }
-                .into());
-            }
-            Ok(())
+    let header_time: SystemTime = last_header.bft_time().into();
+    let expires_at = header_time.add(trusting_period);
+    // Ensure now > expires_at.
+    if expires_at <= now {
+        return Err(Kind::Expired {
+            at: expires_at,
+            now,
         }
-        Err(e) => Err(Kind::DurationOutOfRange(e).into()),
+        .into());
     }
+    // Also make sure the header is not after now.
+    ensure!(
+        header_time <= now,
+        Kind::DurationOutOfRange,
+        "header time: ({:?}) > now: ({:?})",
+        header_time,
+        now
+    );
+    Ok(())
 }
 
 /// Validate the validators, next validators, against the signed header.
@@ -219,8 +227,8 @@ pub fn verify_single<H, C, L>(
     untrusted_vals: &C::ValidatorSet,
     untrusted_next_vals: &C::ValidatorSet,
     trust_threshold: L,
-    trusting_period: &Duration,
-    now: &SystemTime,
+    trusting_period: Duration,
+    now: SystemTime,
 ) -> Result<TrustedState<C, H>, Error>
 where
     H: Header,
@@ -269,8 +277,8 @@ pub fn verify_bisection<C, H, L, R>(
     trusted_state: TrustedState<C, H>,
     untrusted_height: Height,
     trust_threshold: L,
-    trusting_period: &Duration,
-    now: &SystemTime,
+    trusting_period: Duration,
+    now: SystemTime,
     req: &R,
 ) -> Result<Vec<TrustedState<C, H>>, Error>
 where
@@ -459,7 +467,7 @@ mod tests {
         req
     }
 
-    // make a state with the given vals and commit and ensure we get the error.
+    // make a state with the given vals and commit and ensure we get the expected error kind.
     fn assert_single_err(
         ts: &TrustedState<MockCommit, MockHeader>,
         vals: Vec<usize>,
@@ -475,7 +483,7 @@ mod tests {
             TrustThresholdFraction::default(),
         );
         assert!(result.is_err());
-        assert_eq!(format!("{}", result.unwrap_err()), format!("{}", err));
+        assert_eq!(result.unwrap_err().to_string(), err.to_string());
     }
 
     // make a state with the given vals and commit and ensure we get no error.
@@ -682,20 +690,20 @@ mod tests {
 
         // less than the period, OK
         let header = MockHeader::new(4, header_time, fixed_hash(), fixed_hash());
-        assert!(is_within_trust_period(&header, &period, &now).is_ok());
+        assert!(is_within_trust_period(&header, period, now).is_ok());
 
-        // equal to the period, OK
+        // equal to the period, not OK
         let now = header_time + period;
-        assert!(is_within_trust_period(&header, &period, &now).is_ok());
+        assert!(is_within_trust_period(&header, period, now).is_err());
 
         // greater than the period, not OK
         let now = header_time + period + Duration::new(1, 0);
-        assert!(is_within_trust_period(&header, &period, &now).is_err());
+        assert!(is_within_trust_period(&header, period, now).is_err());
 
         // bft time in header is later than now, not OK:
         let now = SystemTime::UNIX_EPOCH;
         let later_than_now = now + Duration::new(60, 0);
         let future_header = MockHeader::new(4, later_than_now, fixed_hash(), fixed_hash());
-        assert!(is_within_trust_period(&future_header, &period, &now).is_err());
+        assert!(is_within_trust_period(&future_header, period, now).is_err());
     }
 }

tendermint/src/lite_impl/signed_header.rs

@@ -3,6 +3,7 @@
 use crate::lite::error::{Error, Kind};
 use crate::validator::Set;
 use crate::{block, hash, lite, vote};
+use anomaly::fail;
 
 impl lite::Commit for block::signed_header::SignedHeader {
     type ValidatorSet = Set;
@@ -35,14 +36,13 @@ impl lite::Commit for block::signed_header::SignedHeader {
             let sign_bytes = vote.sign_bytes();
 
             if !val.verify_signature(&sign_bytes, vote.signature()) {
-                return Err(Kind::ImplementationSpecific
-                    .context(format!(
-                        "Couldn't verify signature {:?} with validator {:?} on sign_bytes {:?}",
-                        vote.signature(),
-                        val,
-                        sign_bytes,
-                    ))
-                    .into());
+                fail!(
+                    Kind::ImplementationSpecific,
+                    "Couldn't verify signature {:?} with validator {:?} on sign_bytes {:?}",
+                    vote.signature(),
+                    val,
+                    sign_bytes,
+                );
             }
             signed_power += val.power();
         }
@@ -52,13 +52,12 @@ impl lite::Commit for block::signed_header::SignedHeader {
 
     fn validate(&self, vals: &Self::ValidatorSet) -> Result<(), Error> {
         if self.commit.precommits.len() != vals.validators().len() {
-            return Err(lite::error::Kind::ImplementationSpecific
-                .context(format!(
-                    "pre-commit length: {} doesn't match validator length: {}",
-                    self.commit.precommits.len(),
-                    vals.validators().len()
-                ))
-                .into());
+            fail!(
+                lite::error::Kind::ImplementationSpecific,
+                "pre-commit length: {} doesn't match validator length: {}",
+                self.commit.precommits.len(),
+                vals.validators().len()
+            );
         }
         // TODO: compare to the go code for more implementation related checks and clarify if this:
         // https://github.com/interchainio/tendermint-rs/pull/143/commits/0a30022fa47e909e6c7b20417dd178c8a3b84838#r374958528

tendermint/tests/lite.rs

@@ -104,8 +104,8 @@ fn run_test_cases(cases: TestCases) {
                 &untrusted_vals,
                 &untrusted_next_vals,
                 TrustThresholdFraction::default(),
-                &trusting_period,
-                &now,
+                trusting_period,
+                now,
             ) {
                 Ok(new_state) => {
                     let expected_state = TrustedState::new(