Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

light-client: integration test #431

Merged
merged 12 commits into from
Jul 10, 2020
26 changes: 25 additions & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
# - https://github.community/t/support-for-yaml-anchors/16128/15
# - https://github.community/t/reusing-sharing-inheriting-steps-between-jobs-declarations/16851/13
# - https://github.community/t/using-matrix-variable-in-docker-image-name/17296
test-integration-stable:
test-integration-tendermint-stable:
runs-on: ubuntu-latest
services:
tendermint:
Expand All @@ -85,6 +85,30 @@ jobs:
command: test
args: -p tendermint --test integration --no-fail-fast -- --ignored

# TODO(shonfeder): remove duplication once GitHub addresses one of these
# - https://github.community/t/support-for-yaml-anchors/16128/15
# - https://github.community/t/reusing-sharing-inheriting-steps-between-jobs-declarations/16851/13
# - https://github.community/t/using-matrix-variable-in-docker-image-name/17296
test-integration-light-client-stable:
runs-on: ubuntu-latest
services:
tendermint:
image: tendermint/tendermint:v0.33.5
ports:
- 26656:26656
- 26657:26657
- 26660:26660
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
override: true
- uses: actions-rs/cargo@v1
xla marked this conversation as resolved.
Show resolved Hide resolved
with:
command: test
args: -p tendermint-light-client --test integration --no-fail-fast -- --ignored

test-integration-latest:
runs-on: ubuntu-latest
services:
Expand Down
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ Light Client:
- Expose latest_trusted from Supervisor Handle ([#394])
- Turn `Handle` into a trait for ease of integration and testability ([#401])
- Improve `Supervisor` ergonomics according to [ADR-007] ([#403])
- Add integration test ([#431])

[#394]: https://github.com/informalsystems/tendermint-rs/pull/394
[#401]: https://github.com/informalsystems/tendermint-rs/pull/401
[#403]: https://github.com/informalsystems/tendermint-rs/pull/403
[#431]: https://github.com/informalsystems/tendermint-rs/pull/431
[ADR-007]: https://github.com/informalsystems/tendermint-rs/blob/master/docs/architecture/adr-007-light-client-supervisor-ergonomics.md

## [0.14.1] (2020-06-23)
Expand Down
3 changes: 1 addition & 2 deletions light-client/examples/light_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use tendermint_light_client::{
peer_list::PeerList,
state::State,
store::{sled::SledStore, LightStore},
types::{Height, PeerId, Status, Time, TrustThreshold},
types::{Height, PeerId, Status, TrustThreshold},
};

#[derive(Debug, Options)]
Expand Down Expand Up @@ -121,7 +121,6 @@ fn make_instance(
},
trusting_period: Duration::from_secs(36000),
clock_drift: Duration::from_secs(1),
now: Time::now(),
};

let verifier = ProdVerifier::default();
Expand Down
19 changes: 16 additions & 3 deletions light-client/src/components/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
CommitValidator, Hasher, ProdCommitValidator, ProdHasher, ProdVotingPowerCalculator,
VotingPowerCalculator,
},
types::LightBlock,
types::{LightBlock, Time},
};
use preds::{errors::VerificationError, ProdPredicates, VerificationPredicates};

Expand Down Expand Up @@ -44,7 +44,13 @@ impl From<Result<(), VerificationError>> for Verdict {
/// - [TMBC-VAL-COMMIT.1]
pub trait Verifier: Send {
/// Perform the verification.
fn verify(&self, untrusted: &LightBlock, trusted: &LightBlock, options: &Options) -> Verdict;
fn verify(
&self,
untrusted: &LightBlock,
trusted: &LightBlock,
options: &Options,
now: Time,
) -> Verdict;
}

/// Production implementation of the verifier.
Expand Down Expand Up @@ -91,7 +97,13 @@ impl Default for ProdVerifier {
}

impl Verifier for ProdVerifier {
fn verify(&self, untrusted: &LightBlock, trusted: &LightBlock, options: &Options) -> Verdict {
fn verify(
&self,
untrusted: &LightBlock,
trusted: &LightBlock,
options: &Options,
now: Time,
) -> Verdict {
preds::verify(
&*self.predicates,
&*self.voting_power_calculator,
Expand All @@ -100,6 +112,7 @@ impl Verifier for ProdVerifier {
&trusted,
&untrusted,
options,
now,
)
.into()
}
Expand Down
1 change: 1 addition & 0 deletions light-client/src/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub fn trusted_store_contains_block_at_target_height(
target_height: Height,
) -> bool {
light_store.get(target_height, Status::Verified).is_some()
|| light_store.get(target_height, Status::Trusted).is_some()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if there are more places like this. reminded me a little of #419

Copy link
Member Author

@romac romac Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I thought I had caught them all in #419 but evidently not. Will go over the codebase again to make sure.

}

pub fn is_within_trust_period(
Expand Down
4 changes: 3 additions & 1 deletion light-client/src/evidence.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::{components::io::IoError, types::PeerId};

use tendermint::{abci::transaction::Hash, evidence::Evidence};
use tendermint::abci::transaction::Hash;
use tendermint_rpc as rpc;

use contracts::{contract_trait, pre};
use std::collections::HashMap;

pub use tendermint::evidence::Evidence;

/// Interface for reporting evidence to full nodes, typically via the RPC client.
#[contract_trait]
pub trait EvidenceReporter: Send {
Expand Down
25 changes: 8 additions & 17 deletions light-client/src/light_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
bail,
errors::{Error, ErrorKind},
state::State,
types::{Height, LightBlock, PeerId, Status, Time, TrustThreshold},
types::{Height, LightBlock, PeerId, Status, TrustThreshold},
};

/// Verification parameters
Expand All @@ -26,23 +26,16 @@ pub struct Options {
/// and trusted validator set is sufficient for a commit to be
/// accepted going forward.
pub trust_threshold: TrustThreshold,

/// How long a validator set is trusted for (must be shorter than the chain's
/// unbonding period)
pub trusting_period: Duration,

/// Correction parameter dealing with only approximately synchronized clocks.
/// The local clock should always be ahead of timestamps from the blockchain; this
/// is the maximum amount that the local clock may drift behind a timestamp from the
/// blockchain.
pub clock_drift: Duration,
/// The current time
pub now: Time,
Copy link
Member

@liamsi liamsi Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually using these "Options" in #430 too. I was wondering how now made sense as an option. Much clearer when it isn't 👍

}

impl Options {
/// Override the stored current time with the given one.
pub fn with_now(self, now: Time) -> Self {
Self { now, ..self }
}
}

/// The light client implements a read operation of a header from the blockchain,
Expand Down Expand Up @@ -158,13 +151,11 @@ impl LightClient {
return Ok(light_block);
}

// Override the `now` fields in the given verification options with the current time,
// as per the given `clock`.
let options = self.options.with_now(self.clock.now());

let mut current_height = target_height;

loop {
let now = self.clock.now();

// Get the latest trusted state
let trusted_state = state
.light_store
Expand All @@ -179,10 +170,10 @@ impl LightClient {
}

// Check invariant [LCV-INV-TP.1]
if !is_within_trust_period(&trusted_state, options.trusting_period, options.now) {
if !is_within_trust_period(&trusted_state, self.options.trusting_period, now) {
bail!(ErrorKind::TrustedStateOutsideTrustingPeriod {
trusted_state: Box::new(trusted_state),
options,
options: self.options,
status: Status::Verified
});
}
Expand All @@ -201,7 +192,7 @@ impl LightClient {
// Validate and verify the current block
let verdict = self
.verifier
.verify(&current_block, &trusted_state, &options);
.verify(&current_block, &trusted_state, &self.options, now);

match verdict {
Verdict::Success => {
Expand Down
4 changes: 3 additions & 1 deletion light-client/src/predicates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ pub trait VerificationPredicates: Send {
/// check that their (next) validator sets hashes match.
/// - Otherwise, ensure that the untrusted block has a greater height than
/// the trusted block.
#[allow(clippy::too_many_arguments)]
xla marked this conversation as resolved.
Show resolved Hide resolved
pub fn verify(
vp: &dyn VerificationPredicates,
voting_power_calculator: &dyn VotingPowerCalculator,
Expand All @@ -210,13 +211,14 @@ pub fn verify(
trusted: &LightBlock,
untrusted: &LightBlock,
options: &Options,
now: Time,
) -> Result<(), VerificationError> {
// Ensure the latest trusted header hasn't expired
vp.is_within_trust_period(
&trusted.signed_header.header,
options.trusting_period,
options.clock_drift,
options.now,
now,
)?;

// Ensure the header validator hashes match the given validators
Expand Down
125 changes: 125 additions & 0 deletions light-client/tests/integration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
//! Light Client integration tests.
//!
//! These are all ignored by default, since they test against running
//! `tendermint node --proxy_app=kvstore`. They can be run using:
//!
//! ```
//! cargo test -- --ignored
//! ```

use tendermint_light_client::{
components::{
clock::SystemClock,
io::{AtHeight, Io, IoError, ProdIo},
scheduler,
verifier::ProdVerifier,
},
evidence::{Evidence, EvidenceReporter},
fork_detector::ProdForkDetector,
light_client::{self, LightClient},
peer_list::PeerList,
state::State,
store::{memory::MemoryStore, LightStore},
supervisor::{Handle, Instance, Supervisor},
types::{PeerId, Status, TrustThreshold},
};
xla marked this conversation as resolved.
Show resolved Hide resolved

use tendermint::abci::transaction::Hash as TransactionHash;

use std::collections::HashMap;
use std::time::Duration;

fn make_instance(peer_id: PeerId, options: light_client::Options, io: ProdIo) -> Instance {
xla marked this conversation as resolved.
Show resolved Hide resolved
let trusted_state = io
.fetch_light_block(peer_id, AtHeight::Highest)
.expect("could not request latest light block");

let mut light_store = MemoryStore::new();
light_store.insert(trusted_state, Status::Trusted);

let state = State {
light_store: Box::new(light_store),
verification_trace: HashMap::new(),
};

let verifier = ProdVerifier::default();
let clock = SystemClock;
let scheduler = scheduler::basic_bisecting_schedule;

let light_client = LightClient::new(peer_id, options, clock, scheduler, verifier, io);

Instance::new(light_client, state)
}

struct TestEvidenceReporter;

#[contracts::contract_trait]
impl EvidenceReporter for TestEvidenceReporter {
fn report(&self, evidence: Evidence, peer: PeerId) -> Result<TransactionHash, IoError> {
panic!(
"unexpected fork detected for peer {} with evidence: {:?}",
peer, evidence
);
}
}

#[test]
#[ignore]
fn sync() {
let primary: PeerId = "BADFADAD0BEFEEDC0C0ADEADBEEFC0FFEEFACADE".parse().unwrap();
let witness: PeerId = "CEFEEDBADFADAD0C0CEEFACADE0ADEADBEEFC0FF".parse().unwrap();

let node_address: tendermint::net::Address = "tcp://127.0.0.1:26657".parse().unwrap();

// Because our CI infrastructure can only spawn a single Tendermint node at the moment,
// we run this test against this very node as both the primary and witness.
// In a production environment, one should make sure that the primary and witness are
// different nodes, and check that the configured peer IDs match the ones returned
// by the nodes.
let mut peer_map = HashMap::new();
peer_map.insert(primary, node_address.clone());
peer_map.insert(witness, node_address);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the same RPC end-point is used twice. Do you think it makes sense add a comment that in a realistic scenario these would be different and that you'd check if the peerId matches?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 4128382.


let io = ProdIo::new(peer_map, Some(Duration::from_secs(2)));

let options = light_client::Options {
trust_threshold: TrustThreshold {
numerator: 1,
denominator: 3,
},
trusting_period: Duration::from_secs(60 * 60), // 60 minutes
clock_drift: Duration::from_secs(5 * 60), // 5 minutes
};

let primary_instance = make_instance(primary, options, io.clone());
let witness_instance = make_instance(witness, options, io);

let peer_list = PeerList::builder()
.primary(primary, primary_instance)
.witness(witness, witness_instance)
.build();

let mut supervisor =
Supervisor::new(peer_list, ProdForkDetector::default(), TestEvidenceReporter);

let handle = supervisor.handle();
std::thread::spawn(|| supervisor.run());

let max_iterations: usize = 20;

for i in 1..=max_iterations {
println!("[info ] - iteration {}/{}", i, max_iterations);

match handle.verify_to_highest() {
Ok(light_block) => {
println!("[info ] synced to block {}", light_block.height());
}
Err(err) => {
println!("[error] sync failed: {}", err);
panic!("failed to sync to highest: {}", err);
}
}

std::thread::sleep(Duration::from_millis(800));
}
}
4 changes: 1 addition & 3 deletions light-client/tests/light_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ fn verify_single(
trust_threshold,
trusting_period,
clock_drift,
now: now.into(),
};

let result = verifier.verify(&input, &trusted_state, &options);
let result = verifier.verify(&input, &trusted_state, &options, now.into());

match result {
Verdict::Success => Ok(input),
Expand Down Expand Up @@ -143,7 +142,6 @@ fn run_bisection_test(tc: TestBisection<LightBlock>) -> BisectionTestResult {
trust_threshold,
trusting_period: trusting_period.into(),
clock_drift,
now,
};

let provider = tc.primary;
Expand Down
1 change: 0 additions & 1 deletion light-client/tests/supervisor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ fn make_instance(peer_id: PeerId, trust_options: TrustOptions, io: MockIo, now:
trust_threshold: trust_options.trust_level,
trusting_period: trust_options.period.into(),
clock_drift: Duration::from_secs(10),
now,
};

let verifier = ProdVerifier::default();
Expand Down