Minimal implementation of backward verification for IBC relayer (#709)

* Use custom error type instead of anomaly::BoxError

* prototype backward verification

* backward tests

* re-enabled model based tests

* fix backward algorithm

* optimize backward verification

* disable backward opti

* Fix wrong assertion

* Compute last_block_id hash when generating a light chain

* Add test for light chain correctness

* Add property-based tests for backward verification

* Comment out `bad` test

* Add more tests

* Remove println statement

* Use prop_assert!

* Formatting

* Remove hacky backward verification test

* Add negative tests for backward verification

* Feature-guard backward verification behind "backward-verif" flag

* Rename LightClient::verify_bisection to LightClient::verify_forward

* Update changelog

* Update doc comments

* Formatting

* Fixup after rebase

* Add integration test for backward verification

* Remove `backward-verif` feature in favor of `unstable`

* Cleanup + couple comments

* Check that root state for backward verif is within trusting period

* Add doc comment

* Fix mock clock time in testgen-based tests

* Remove unused import

* Revert "Use custom error type instead of anomaly::BoxError"

This reverts commit 8a5820b.

* Formatting

.gitignore

@@ -14,3 +14,6 @@ Cargo.lock
 
 # RPC probe results
 /rpc-probe/probe-results/
+
+# Proptest regressions dumps
+**/*.proptest-regressions

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## Unreleased
 
+## FEATURES
+
+* `[light-client]` Add basic support for backward verification, behind a `unstable` feature flag. ([#361])
+ Note: This feature is currently unstable and should not be relied on by downstream dependencies.
+
 ## IMPROVEMENTS:
 
 * `[all]` Update all crates to use the latest version of the following dependencies: ([#764])
@@ -16,9 +21,10 @@
 * `[light-client]` The `sled`-backed lightstore is now feature-guarded under
  the `lightstore-sled` feature, which is enabled by default for now. ([#428])
 
-[#769]: https://github.com/informalsystems/tendermint-rs/issues/769
-[#764]: https://github.com/informalsystems/tendermint-rs/issues/764
+[#361]: https://github.com/informalsystems/tendermint-rs/issues/361
 [#428]: https://github.com/informalsystems/tendermint-rs/issues/428
+[#764]: https://github.com/informalsystems/tendermint-rs/issues/764
+[#769]: https://github.com/informalsystems/tendermint-rs/issues/769
 
 ## v0.17.1
 

light-client/Cargo.toml

@@ -33,6 +33,7 @@ default = ["rpc-client", "lightstore-sled"]
 rpc-client = ["tokio", "tendermint-rpc/http-client"]
 secp256k1 = ["tendermint/secp256k1", "tendermint-rpc/secp256k1"]
 lightstore-sled = ["sled"]
+unstable = []
 
 [dependencies]
 tendermint = { version = "0.17.1", path = "../tendermint" }
@@ -58,3 +59,4 @@ serde_json = "1.0.51"
 gumdrop = "0.8.0"
 rand = "0.7.3"
 tempdir = "0.3.7"
+proptest = "0.10.1"

light-client/src/builder/light_client.rs

@@ -208,6 +208,7 @@ impl LightClientBuilder<HasTrustedState> {
  self.clock,
  self.scheduler,
  self.verifier,
+ self.hasher,
  self.io,
  );
 

light-client/src/errors.rs

@@ -11,7 +11,7 @@ use crate::{
  components::io::IoError,
  light_client::Options,
  predicates::errors::VerificationError,
- types::{Height, LightBlock, PeerId, Status},
+ types::{Hash, Height, LightBlock, PeerId, Status},
 };
 
 /// An error raised by this library
@@ -78,6 +78,19 @@ pub enum ErrorKind {
  #[error("invalid light block: {0}")]
  InvalidLightBlock(#[source] VerificationError),
 
+ /// Hash mismatch between two adjacent headers
+ #[error("hash mismatch between two adjacent headers: {h1} != {h2}")]
+ InvalidAdjacentHeaders {
+ /// Hash #1
+ h1: Hash,
+ /// Hash #2
+ h2: Hash,
+ },
+
+ /// Missing last_block_id field for header at given height
+ #[error("missing last_block_id for header at height {0}")]
+ MissingLastBlockId(Height),
+
  /// Internal channel disconnected
  #[error("internal channel disconnected")]
  ChannelDisconnected,

light-client/src/light_client.rs

@@ -2,23 +2,23 @@
 //!
 //! [1]: https://github.com/informalsystems/tendermint-rs/blob/master/docs/spec/lightclient/verification/verification.md
 
+use std::{fmt, time::Duration};
+
 use contracts::*;
 use derive_more::Display;
 use serde::{Deserialize, Serialize};
-use std::{fmt, time::Duration};
 
-use crate::components::{clock::Clock, io::*, scheduler::*, verifier::*};
-use crate::contracts::*;
 use crate::{
  bail,
+ components::{clock::Clock, io::*, scheduler::*, verifier::*},
+ contracts::*,
  errors::{Error, ErrorKind},
+ operations::Hasher,
  state::State,
  types::{Height, LightBlock, PeerId, Status, TrustThreshold},
 };
 
 /// Verification parameters
-///
-/// TODO: Find a better name than `Options`
 #[derive(Copy, Clone, Debug, PartialEq, Display, Serialize, Deserialize)]
 #[display(fmt = "{:?}", self)]
 pub struct Options {
@@ -53,10 +53,15 @@ pub struct LightClient {
  pub peer: PeerId,
  /// Options for this light client
  pub options: Options,
+
  clock: Box<dyn Clock>,
  scheduler: Box<dyn Scheduler>,
  verifier: Box<dyn Verifier>,
  io: Box<dyn Io>,
+
+ // Only used in verify_backwards when "unstable" feature is enabled
+ #[allow(dead_code)]
+ hasher: Box<dyn Hasher>,
 }
 
 impl fmt::Debug for LightClient {
@@ -76,6 +81,7 @@ impl LightClient {
  clock: impl Clock + 'static,
  scheduler: impl Scheduler + 'static,
  verifier: impl Verifier + 'static,
+ hasher: impl Hasher + 'static,
  io: impl Io + 'static,
  ) -> Self {
  Self {
@@ -84,6 +90,7 @@ impl LightClient {
  clock: Box::new(clock),
  scheduler: Box::new(scheduler),
  verifier: Box::new(verifier),
+ hasher: Box::new(hasher),
  io: Box::new(io),
  }
  }
@@ -95,6 +102,7 @@ impl LightClient {
  clock: Box<dyn Clock>,
  scheduler: Box<dyn Scheduler>,
  verifier: Box<dyn Verifier>,
+ hasher: Box<dyn Hasher>,
  io: Box<dyn Io>,
  ) -> Self {
  Self {
@@ -103,6 +111,7 @@ impl LightClient {
  clock,
  scheduler,
  verifier,
+ hasher,
  io,
  }
  }
@@ -127,8 +136,10 @@ impl LightClient {
  /// communicates with other nodes.
  /// - The Verifier component checks whether a header is valid and checks if a new light block
  /// should be trusted based on a previously verified light block.
- /// - The Scheduler component decides which height to try to verify next, in case the current
- /// block pass verification but cannot be trusted yet.
+ /// - When doing _forward_ verification, the Scheduler component decides which height to try to
+ /// verify next, in case the current block pass verification but cannot be trusted yet.
+ /// - When doing _backward_ verification, the Hasher component is used to determine
+ /// whether the `last_block_id` hash of a block matches the hash of the block right below it.
  ///
  /// ## Implements
  /// - [LCV-DIST-SAFE.1]
@@ -158,12 +169,33 @@ impl LightClient {
  target_height: Height,
  state: &mut State,
  ) -> Result<LightBlock, Error> {
- // Let's first look in the store to see whether we have already successfully verified this
- // block.
+ // Let's first look in the store to see whether
+ // we have already successfully verified this block.
  if let Some(light_block) = state.light_store.get_trusted_or_verified(target_height) {
  return Ok(light_block);
  }
 
+ // Get the highest trusted state
+ let highest = state
+ .light_store
+ .highest_trusted_or_verified()
+ .ok_or(ErrorKind::NoInitialTrustedState)?;
+
+ if target_height >= highest.height() {
+ // Perform forward verification with bisection
+ self.verify_forward(target_height, state)
+ } else {
+ // Perform sequential backward verification
+ self.verify_backward(target_height, state)
+ }
+ }
+
+ /// Perform forward verification with bisection.
+ fn verify_forward(
+ &self,
+ target_height: Height,
+ state: &mut State,
+ ) -> Result<LightBlock, Error> {
  let mut current_height = target_height;
 
  loop {
@@ -239,6 +271,108 @@ impl LightClient {
  }
  }
 
+ /// Stub for when "unstable" feature is disabled.
+ #[doc(hidden)]
+ #[cfg(not(feature = "unstable"))]
+ fn verify_backward(
+ &self,
+ target_height: Height,
+ state: &mut State,
+ ) -> Result<LightBlock, Error> {
+ let trusted_state = state
+ .light_store
+ .highest_trusted_or_verified()
+ .ok_or(ErrorKind::NoInitialTrustedState)?;
+
+ Err(ErrorKind::TargetLowerThanTrustedState {
+ target_height,
+ trusted_height: trusted_state.height(),
+ }
+ .into())
+ }
+
+ /// Perform sequential backward verification.
+ ///
+ /// Backward verification is implemented by taking a sliding window
+ /// of length two between the trusted state and the target block and
+ /// checking whether the last_block_id hash of the higher block
+ /// matches the computed hash of the lower block.
+ ///
+ /// ## Performance
+ /// The algorithm implemented is very inefficient in case the target
+ /// block is much lower than the highest trusted state.
+ /// For a trusted state at height `T`, and a target block at height `H`,
+ /// it will fetch and check hashes of `T - H` blocks.
+ ///
+ /// ## Stability
+ /// This feature is only available if the `unstable` flag of is enabled.
+ /// If the flag is disabled, then any attempt to verify a block whose
+ /// height is lower than the highest trusted state will result in a
+ /// `TargetLowerThanTrustedState` error.
+ #[cfg(feature = "unstable")]
+ fn verify_backward(
+ &self,
+ target_height: Height,
+ state: &mut State,
+ ) -> Result<LightBlock, Error> {
+ use std::convert::TryFrom;
+
+ let root = state
+ .light_store
+ .highest_trusted_or_verified()
+ .ok_or(ErrorKind::NoInitialTrustedState)?;
+
+ assert!(root.height() >= target_height);
+
+ // Check invariant [LCV-INV-TP.1]
+ if !is_within_trust_period(&root, self.options.trusting_period, self.clock.now()) {
+ bail!(ErrorKind::TrustedStateOutsideTrustingPeriod {
+ trusted_state: Box::new(root),
+ options: self.options,
+ });
+ }
+
+ // Compute a range of `Height`s from `trusted_height - 1` to `target_height`, inclusive.
+ let range = (target_height.value()..root.height().value()).rev();
+ let heights = range.map(|h| Height::try_from(h).unwrap());
+
+ let mut latest = root;
+
+ for height in heights {
+ let (current, _status) = self.get_or_fetch_block(height, state)?;
+
+ let latest_last_block_id = latest
+ .signed_header
+ .header
+ .last_block_id
+ .ok_or_else(|| ErrorKind::MissingLastBlockId(latest.height()))?;
+
+ let current_hash = self.hasher.hash_header(&current.signed_header.header);
+
+ if current_hash != latest_last_block_id.hash {
+ bail!(ErrorKind::InvalidAdjacentHeaders {
+ h1: current_hash,
+ h2: latest_last_block_id.hash
+ });
+ }
+
+ // `latest` and `current` are linked together by `last_block_id`,
+ // therefore it is not relevant which we verified first.
+ // For consistency, we say that `latest` was verifed using
+ // `current` so that the trace is always pointing down the chain.
+ state.light_store.insert(current.clone(), Status::Trusted);
+ state.light_store.insert(latest.clone(), Status::Trusted);
+ state.trace_block(latest.height(), current.height());
+
+ latest = current;
+ }
+
+ // We reached the target height.
+ assert_eq!(latest.height(), target_height);
+
+ Ok(latest)
+ }
+
  /// Look in the light store for a block from the given peer at the given height,
  /// which has not previously failed verification (ie. its status is not `Failed`).
  ///

light-client/src/store.rs

@@ -76,10 +76,10 @@ pub trait LightStore: Debug + Send + Sync {
 
  /// Get the light block of lowest height with the trusted or verified status.
  fn lowest_trusted_or_verified(&self) -> Option<LightBlock> {
- let latest_trusted = self.lowest(Status::Trusted);
- let latest_verified = self.lowest(Status::Verified);
+ let lowest_trusted = self.lowest(Status::Trusted);
+ let lowest_verified = self.lowest(Status::Verified);
 
- std_ext::option::select(latest_trusted, latest_verified, |t, v| {
+ std_ext::option::select(lowest_trusted, lowest_verified, |t, v| {
  std_ext::cmp::min_by_key(t, v, |lb| lb.height())
  })
  }

light-client/src/tests.rs

@@ -42,7 +42,7 @@ pub struct Initial {
 }
 
 #[derive(Deserialize, Clone, Debug)]
-pub struct TestBisection<LB> {
+pub struct LightClientTest<LB> {
  pub description: String,
  pub trust_options: TrustOptions,
  pub primary: Provider<LB>,

light-client/src/utils/std_ext.rs

@@ -2,7 +2,7 @@ pub mod cmp {
 
  /// Stable version of `std::cmp::max_by_key`.
  pub fn max_by_key<A, B: Ord>(a: A, b: A, key: impl Fn(&A) -> B) -> A {
- if key(&a) >= key(&b) {
+ if key(&a) > key(&b) {
  a
  } else {
  b