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

Binary Agreement test updated to the proptest framework #336

Merged
merged 5 commits into from
Nov 20, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 109 additions & 77 deletions tests/binary_agreement.rs
Original file line number Diff line number Diff line change
@@ -1,108 +1,140 @@
#![deny(unused_must_use)]
//! Tests of the Binary Agreement protocol. Only one proposer instance
//! is tested. Each of the nodes in the simulated network run only one instance
//! of Binary Agreement. This way we only test correctness of the protocol and not
//! message dispatch between multiple proposers.
//! Tests of the Binary Agreement protocol
//!
//! Each of the nodes in the simulated network runs one instance of Binary Agreement. This suffices
//! to test correctness of the protocol.
//!
//! There are three properties that are tested:
//!
//! - Agreement: If any correct node outputs the bit b, then every correct node outputs b.
//! - Agreement: If any correct node outputs the bit `b`, then every correct node outputs `b`.
//!
//! - Termination: If all correct nodes receive input, then every correct node outputs a bit.
//!
//! - Validity: If any correct node outputs b, then at least one correct node received b as input.
//!
//! TODO: Implement adversaries and send BVAL messages at different times.
//! - Validity: If any correct node outputs `b`, then at least one correct node received `b` as
//! input.

extern crate env_logger;
extern crate failure;
extern crate hbbft;
extern crate log;
extern crate integer_sqrt;
extern crate proptest;
extern crate rand;
extern crate rand_derive;
extern crate serde_derive;
extern crate threshold_crypto as crypto;
extern crate threshold_crypto;

mod network;
pub mod net;
Copy link
Collaborator

Choose a reason for hiding this comment

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

That shouldn't be pub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are lots of "method is never used" lints otherwise in the net module.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right; some of them are already marked as #[allow(unused)]—maybe we should apply that attribute to the whole net module instead (either inside it, or at the use statements)? That would make the reason clearer than the pub.

Copy link
Contributor

Choose a reason for hiding this comment

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

One option would also be to make the testing framework a proper crate and just import that; I've done this on another project.

It's a bit more work, but it also does not require importing a million different crates on each test cases - right now, whenever I add a dependency to the net testing framework, I also need to add one in every test case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right, I guess that would be the proper solution. 👍
(Something for another PR, though.)


use std::iter::once;
use std::sync::Arc;
use std::time;

use log::info;
use rand::Rng;
use proptest::arbitrary::any;
use proptest::{prelude::ProptestConfig, prop_compose, proptest, proptest_helper};
use rand::{Rng, SeedableRng};

use hbbft::binary_agreement::BinaryAgreement;
use hbbft::NetworkInfo;
use hbbft::DistAlgorithm;

use network::{Adversary, MessageScheduler, NodeId, SilentAdversary, TestNetwork, TestNode};
use net::adversary::ReorderingAdversary;
use net::proptest::{gen_seed, NetworkDimension, TestRng, TestRngSeed};
use net::{NetBuilder, NewNodeInfo, VirtualNet};

fn test_binary_agreement<A: Adversary<BinaryAgreement<NodeId, u8>>>(
mut network: TestNetwork<A, BinaryAgreement<NodeId, u8>>,
/// Test configuration for Binary Agreement tests.
#[derive(Debug)]
struct TestConfig {
/// The desired network dimension.
dimension: NetworkDimension,
/// Random number generator to be passed to subsystems.
seed: TestRngSeed,
/// Input to Binary Agreement instances that has the following meaning:
///
/// - `Some(b)`: all instances receive `b` as input.
///
/// - `None`: each instance receives a random `bool` as input.
input: Option<bool>,
) {
let ids: Vec<NodeId> = network.nodes.keys().cloned().collect();
for id in ids {
network.input(id, input.unwrap_or_else(rand::random));
}
}

// Handle messages in random order until all nodes have output the proposed value.
while !network.nodes.values().all(TestNode::terminated) {
network.step();
prop_compose! {
/// Strategy to generate a test configuration.
fn arb_config()
(
dimension in NetworkDimension::range(1, 50),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just nothing that we need to be doubly sure 1 is a workable network size for the test (it's not for net_dynamic_hb, for example, since a node gets removed). Should you need to change it, beware that the 1-50 range here refers to the random initial size, not the minimum.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say we definitely want BA to work with a size of 1.

Copy link
Contributor Author

@vkomenda vkomenda Nov 19, 2018

Choose a reason for hiding this comment

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

If proptest reduces it to 1 in case of failure anyway, maybe it's worth starting from above 1.

seed in gen_seed(),
input in any::<Option<bool>>(),
) -> TestConfig
{
TestConfig { dimension, seed, input }
}
// Verify that all instances output the same value.
let mut expected = input;
for node in network.nodes.values() {
if let Some(b) = expected {
assert!(once(&b).eq(node.outputs()));
} else {
assert_eq!(1, node.outputs().len());
expected = Some(node.outputs()[0]);
}
}

/// Proptest wrapper for `binary_agreement` that runs the test function on generated configurations.
proptest!{
#![proptest_config(ProptestConfig {
cases: 1, .. ProptestConfig::default()
})]
#[test]
#[cfg_attr(feature = "cargo-clippy", allow(unnecessary_operation))]
fn run_binary_agreement(cfg in arb_config()) {
binary_agreement(cfg)
}
assert!(expected.iter().eq(network.observer.outputs()));
}

fn test_binary_agreement_different_sizes<A, F>(new_adversary: F)
where
A: Adversary<BinaryAgreement<NodeId, u8>>,
F: Fn(usize, usize) -> A,
{
// This returns an error in all but the first test.
let _ = env_logger::try_init();
type NodeId = u16;
type Algo = BinaryAgreement<NodeId, u8>;

let mut rng = rand::thread_rng();
let sizes = (1..6)
.chain(once(rng.gen_range(6, 20)))
.chain(once(rng.gen_range(30, 50)));
for size in sizes {
let num_faulty_nodes = (size - 1) / 3;
let num_good_nodes = size - num_faulty_nodes;
for &input in &[None, Some(false), Some(true)] {
info!(
"Test start: {} good nodes and {} faulty nodes, input: {:?}",
num_good_nodes, num_faulty_nodes, input
);
let adversary = |_| new_adversary(num_good_nodes, num_faulty_nodes);
let new_ba = |netinfo: Arc<NetworkInfo<NodeId>>| {
BinaryAgreement::new(netinfo, 0).expect("Binary Agreement instance")
};
let network = TestNetwork::new(num_good_nodes, num_faulty_nodes, adversary, new_ba);
test_binary_agreement(network, input);
info!(
"Test success: {} good nodes and {} faulty nodes, input: {:?}",
num_good_nodes, num_faulty_nodes, input
);
impl VirtualNet<Algo> {
fn test_binary_agreement<R>(&mut self, input: Option<bool>, mut rng: R)
afck marked this conversation as resolved.
Show resolved Hide resolved
where
R: Rng + 'static,
{
let ids: Vec<NodeId> = self.nodes().map(|n| *n.id()).collect();
for id in ids {
let _ = self.send_input(id, input.unwrap_or_else(|| rng.gen::<bool>()));
}
}
}

#[test]
fn test_binary_agreement_random_silent() {
let new_adversary = |_: usize, _: usize| SilentAdversary::new(MessageScheduler::Random);
test_binary_agreement_different_sizes(new_adversary);
// Handle messages in random order until all nodes have output the proposed value.
while !self.nodes().all(|node| node.algorithm().terminated()) {
let _ = self.crank_expect();
}
// Verify that all instances output the same value.
let mut expected = input;
for node in self.nodes() {
if let Some(b) = expected {
assert!(once(&b).eq(node.outputs()));
} else {
assert_eq!(1, node.outputs().len());
expected = Some(node.outputs()[0]);
}
}
// TODO: As soon as observers are added to the test framework, compare the expected output
// against the output of observers.
}
}

#[test]
fn test_binary_agreement_first_silent() {
let new_adversary = |_: usize, _: usize| SilentAdversary::new(MessageScheduler::First);
test_binary_agreement_different_sizes(new_adversary);
/// Tests Binary Agreement on a given configuration.
#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))]
fn binary_agreement(cfg: TestConfig) {
let mut rng: TestRng = TestRng::from_seed(cfg.seed);
let size = cfg.dimension.size();
let num_faulty_nodes = cfg.dimension.faulty();
let num_good_nodes = size - num_faulty_nodes;
println!(
"Test start: {} good nodes and {} faulty nodes, input: {:?}",
num_good_nodes, num_faulty_nodes, cfg.input
);
// Create a network with `size` validators and one observer.
let mut net: VirtualNet<Algo> = NetBuilder::new(0..size as u16)
.num_faulty(num_faulty_nodes as usize)
.message_limit(10_000 * size as usize)
.time_limit(time::Duration::from_secs(30 * size as u64))
.rng(rng.gen::<TestRng>())
.adversary(ReorderingAdversary::new(rng.gen::<TestRng>()))
.using(move |node_info: NewNodeInfo<_>| {
BinaryAgreement::new(Arc::new(node_info.netinfo), 0)
.expect("Failed to create a BinaryAgreement instance.")
}).build()
.expect("Could not construct test network.");
net.test_binary_agreement(cfg.input, rng.gen::<TestRng>());
println!(
"Test success: {} good nodes and {} faulty nodes, input: {:?}",
num_good_nodes, num_faulty_nodes, cfg.input
);
}