Add InventoryHash and MetaAddr proptests (#1985)

* Make proptest dependencies consistent between chain and network

* Implement Arbitrary for InventoryHash and use it in tests

* Impl Arbitrary for MetaAddr and use it in tests

Also test some extreme times in MetaAddr sanitization.

zebra-network/Cargo.toml

@@ -37,6 +37,6 @@ zebra-chain = { path = "../zebra-chain" }
 
 [dev-dependencies]
 proptest = "0.10"
-proptest-derive = "0.3.0"
+proptest-derive = "0.3"
 
 zebra-test = { path = "../zebra-test/" }

zebra-network/src/meta_addr.rs

@@ -18,6 +18,11 @@ use crate::protocol::{external::MAX_PROTOCOL_MESSAGE_LEN, types::PeerServices};
 
 use PeerAddrState::*;
 
+#[cfg(any(test, feature = "proptest-impl"))]
+use proptest_derive::Arbitrary;
+#[cfg(any(test, feature = "proptest-impl"))]
+mod arbitrary;
+
 #[cfg(test)]
 mod tests;
 
@@ -28,6 +33,7 @@ mod tests;
 /// [`AddressBook::maybe_connected_peers`] and
 /// [`AddressBook::reconnection_peers`].
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
 pub enum PeerAddrState {
     /// The peer has sent us a valid message.
     ///
@@ -196,6 +202,8 @@ impl MetaAddr {
         let last_seen = Utc.timestamp(ts - ts.rem_euclid(interval), 0);
         MetaAddr {
             addr: self.addr,
+            // services are sanitized during parsing, so we don't need to make
+            // any changes here
             services: self.services,
             last_seen,
             // the state isn't sent to the remote peer, but sanitize it anyway

zebra-network/src/meta_addr/arbitrary.rs

@@ -0,0 +1,31 @@
+use proptest::{arbitrary::any, arbitrary::Arbitrary, prelude::*};
+
+use super::{MetaAddr, PeerAddrState, PeerServices};
+
+use std::{net::SocketAddr, time::SystemTime};
+
+impl Arbitrary for MetaAddr {
+    type Parameters = ();
+
+    fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
+        (
+            any::<SocketAddr>(),
+            any::<PeerServices>(),
+            any::<SystemTime>(),
+            any::<PeerAddrState>(),
+        )
+            .prop_map(
+                |(addr, services, last_seen, last_connection_state)| MetaAddr {
+                    addr,
+                    services,
+                    // TODO: implement constraints on last_seen as part of the
+                    // last_connection_state refactor in #1849
+                    last_seen: last_seen.into(),
+                    last_connection_state,
+                },
+            )
+            .boxed()
+    }
+
+    type Strategy = BoxedStrategy<Self>;
+}

zebra-network/src/meta_addr/tests.rs

@@ -1,2 +1,4 @@
+mod check;
 mod preallocate;
 mod prop;
+mod vectors;

zebra-network/src/meta_addr/tests/check.rs

@@ -0,0 +1,45 @@
+//! Shared test checks for MetaAddr
+
+use super::super::MetaAddr;
+
+use crate::constants::TIMESTAMP_TRUNCATION_SECONDS;
+
+/// Make sure that the sanitize function reduces time and state metadata
+/// leaks.
+pub(crate) fn sanitize_avoids_leaks(entry: &MetaAddr) {
+    let sanitized = entry.sanitize();
+
+    // We want the sanitized timestamp to:
+    // - be a multiple of the truncation interval,
+    // - have a zero nanoseconds component, and
+    // - be within the truncation interval of the original timestamp.
+    assert_eq!(
+        sanitized.get_last_seen().timestamp() % TIMESTAMP_TRUNCATION_SECONDS,
+        0
+    );
+    assert_eq!(sanitized.get_last_seen().timestamp_subsec_nanos(), 0);
+    // handle underflow and overflow by skipping the check
+    // the other check will ensure correctness
+    let lowest_time = entry
+        .get_last_seen()
+        .timestamp()
+        .checked_sub(TIMESTAMP_TRUNCATION_SECONDS);
+    let highest_time = entry
+        .get_last_seen()
+        .timestamp()
+        .checked_add(TIMESTAMP_TRUNCATION_SECONDS);
+    if let Some(lowest_time) = lowest_time {
+        assert!(sanitized.get_last_seen().timestamp() > lowest_time);
+    }
+    if let Some(highest_time) = highest_time {
+        assert!(sanitized.get_last_seen().timestamp() < highest_time);
+    }
+
+    // Sanitize to the the default state, even though it's not serialized
+    assert_eq!(sanitized.last_connection_state, Default::default());
+    // We want the other fields to be unmodified
+    assert_eq!(sanitized.addr, entry.addr);
+    // Services are sanitized during parsing, so we don't need to make
+    // any changes in sanitize()
+    assert_eq!(sanitized.services, entry.services);
+}

zebra-network/src/meta_addr/tests/preallocate.rs

@@ -1,61 +1,51 @@
 //! Tests for trusted preallocation during deserialization.
 
-use super::super::{MetaAddr, PeerAddrState, PeerServices, META_ADDR_SIZE};
+use super::super::{MetaAddr, META_ADDR_SIZE};
 
 use zebra_chain::serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN};
 
-use chrono::{TimeZone, Utc};
+use proptest::prelude::*;
 use std::convert::TryInto;
 
-/// Confirm that each MetaAddr takes exactly META_ADDR_SIZE bytes when serialized.
+proptest! {
-/// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound.
+    /// Confirm that each MetaAddr takes exactly META_ADDR_SIZE bytes when serialized.
-#[test]
+    /// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound.
-fn meta_addr_size_is_correct() {
+    #[test]
-    let addr = MetaAddr {
+    fn meta_addr_size_is_correct(addr in MetaAddr::arbitrary()) {
-        addr: ([192, 168, 0, 0], 8333).into(),
+        let serialized = addr
-        services: PeerServices::default(),
+            .zcash_serialize_to_vec()
-        last_seen: Utc.timestamp(1_573_680_222, 0),
+            .expect("Serialization to vec must succeed");
-        last_connection_state: PeerAddrState::Responded,
+        assert!(serialized.len() == META_ADDR_SIZE)
-    };
-    let serialized = addr
-        .zcash_serialize_to_vec()
-        .expect("Serialization to vec must succeed");
-    assert!(serialized.len() == META_ADDR_SIZE)
-}
-
-/// Verifies that...
-/// 1. The smallest disallowed vector of `MetaAddrs`s is too large to fit in a legal Zcash message
-/// 2. The largest allowed vector is small enough to fit in a legal Zcash message
-#[test]
-fn meta_addr_max_allocation_is_correct() {
-    let addr = MetaAddr {
-        addr: ([192, 168, 0, 0], 8333).into(),
-        services: PeerServices::default(),
-        last_seen: Utc.timestamp(1_573_680_222, 0),
-        last_connection_state: PeerAddrState::Responded,
-    };
-    let max_allocation: usize = MetaAddr::max_allocation().try_into().unwrap();
-    let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1);
-    for _ in 0..(MetaAddr::max_allocation() + 1) {
-        smallest_disallowed_vec.push(addr);
     }
-    let smallest_disallowed_serialized = smallest_disallowed_vec
-        .zcash_serialize_to_vec()
-        .expect("Serialization to vec must succeed");
-    // Check that our smallest_disallowed_vec is only one item larger than the limit
-    assert!(((smallest_disallowed_vec.len() - 1) as u64) == MetaAddr::max_allocation());
-    // Check that our smallest_disallowed_vec is too big to send in a valid Zcash message
-    assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN);
 
-    // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency)
+    /// Verifies that...
-    smallest_disallowed_vec.pop();
+    /// 1. The smallest disallowed vector of `MetaAddrs`s is too large to fit in a legal Zcash message
-    let largest_allowed_vec = smallest_disallowed_vec;
+    /// 2. The largest allowed vector is small enough to fit in a legal Zcash message
-    let largest_allowed_serialized = largest_allowed_vec
+    #[test]
-        .zcash_serialize_to_vec()
+    fn meta_addr_max_allocation_is_correct(addr in MetaAddr::arbitrary()) {
-        .expect("Serialization to vec must succeed");
+        let max_allocation: usize = MetaAddr::max_allocation().try_into().unwrap();
+        let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1);
+        for _ in 0..(MetaAddr::max_allocation() + 1) {
+            smallest_disallowed_vec.push(addr);
+        }
+        let smallest_disallowed_serialized = smallest_disallowed_vec
+            .zcash_serialize_to_vec()
+            .expect("Serialization to vec must succeed");
+        // Check that our smallest_disallowed_vec is only one item larger than the limit
+        assert!(((smallest_disallowed_vec.len() - 1) as u64) == MetaAddr::max_allocation());
+        // Check that our smallest_disallowed_vec is too big to send in a valid Zcash message
+        assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN);
 
-    // Check that our largest_allowed_vec contains the maximum number of MetaAddrs
+        // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency)
-    assert!((largest_allowed_vec.len() as u64) == MetaAddr::max_allocation());
+        smallest_disallowed_vec.pop();
-    // Check that our largest_allowed_vec is small enough to fit in a Zcash message.
+        let largest_allowed_vec = smallest_disallowed_vec;
-    assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN);
+        let largest_allowed_serialized = largest_allowed_vec
+            .zcash_serialize_to_vec()
+            .expect("Serialization to vec must succeed");
+
+        // Check that our largest_allowed_vec contains the maximum number of MetaAddrs
+        assert!((largest_allowed_vec.len() as u64) == MetaAddr::max_allocation());
+        // Check that our largest_allowed_vec is small enough to fit in a Zcash message.
+        assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN);
+    }
 }

zebra-network/src/meta_addr/tests/prop.rs

@@ -1,29 +1,16 @@
-use super::super::*;
+//! Randomised property tests for MetaAddr.
 
-// XXX remove this test and replace it with a proptest instance.
+use super::{super::MetaAddr, check};
-#[test]
-fn sanitize_truncates_timestamps() {
-    zebra_test::init();
 
-    let services = PeerServices::default();
+use proptest::prelude::*;
-    let addr = "127.0.0.1:8233".parse().unwrap();
 
-    let entry = MetaAddr {
+proptest! {
-        services,
+    /// Make sure that the sanitize function reduces time and state metadata
-        addr,
+    /// leaks.
-        last_seen: Utc.timestamp(1_573_680_222, 0),
+    #[test]
-        last_connection_state: Responded,
+    fn sanitized_fields(entry in MetaAddr::arbitrary()) {
+        zebra_test::init();
+
+        check::sanitize_avoids_leaks(&entry);
     }
-    .sanitize();
-
-    // We want the sanitized timestamp to be a multiple of the truncation interval.
-    assert_eq!(
-        entry.get_last_seen().timestamp() % crate::constants::TIMESTAMP_TRUNCATION_SECONDS,
-        0
-    );
-    // We want the state to be the default
-    assert_eq!(entry.last_connection_state, Default::default());
-    // We want the other fields to be unmodified
-    assert_eq!(entry.addr, addr);
-    assert_eq!(entry.services, services);
 }

zebra-network/src/meta_addr/tests/vectors.rs

@@ -0,0 +1,28 @@
+//! Test vectors for MetaAddr.
+
+use super::{super::MetaAddr, check};
+
+use chrono::{MAX_DATETIME, MIN_DATETIME};
+
+/// Make sure that the sanitize function handles minimum and maximum times.
+#[test]
+fn sanitize_extremes() {
+    zebra_test::init();
+
+    let min_time_entry = MetaAddr {
+        addr: "127.0.0.1:8233".parse().unwrap(),
+        services: Default::default(),
+        last_seen: MIN_DATETIME,
+        last_connection_state: Default::default(),
+    };
+
+    let max_time_entry = MetaAddr {
+        addr: "127.0.0.1:8233".parse().unwrap(),
+        services: Default::default(),
+        last_seen: MAX_DATETIME,
+        last_connection_state: Default::default(),
+    };
+
+    check::sanitize_avoids_leaks(&min_time_entry);
+    check::sanitize_avoids_leaks(&max_time_entry);
+}

zebra-network/src/protocol/external.rs

@@ -7,6 +7,8 @@ mod message;
 /// Newtype wrappers for primitive types.
 pub mod types;
 
+#[cfg(any(test, feature = "proptest-impl"))]
+mod arbitrary;
 #[cfg(test)]
 mod tests;
 

zebra-network/src/protocol/external/arbitrary.rs

@@ -0,0 +1,54 @@
+use proptest::{arbitrary::any, arbitrary::Arbitrary, prelude::*};
+
+use super::InventoryHash;
+
+use zebra_chain::{block, transaction};
+
+impl InventoryHash {
+    /// Generate a proptest strategy for Inv Errors
+    pub fn error_strategy() -> BoxedStrategy<Self> {
+        Just(InventoryHash::Error).boxed()
+    }
+
+    /// Generate a proptest strategy for Inv Tx hashes
+    pub fn tx_strategy() -> BoxedStrategy<Self> {
+        // using any::<transaction::Hash> causes a trait impl error
+        // when building the zebra-network crate separately
+        (any::<[u8; 32]>())
+            .prop_map(transaction::Hash)
+            .prop_map(InventoryHash::Tx)
+            .boxed()
+    }
+
+    /// Generate a proptest strategy for Inv Block hashes
+    pub fn block_strategy() -> BoxedStrategy<Self> {
+        (any::<[u8; 32]>())
+            .prop_map(block::Hash)
+            .prop_map(InventoryHash::Block)
+            .boxed()
+    }
+
+    /// Generate a proptest strategy for Inv FilteredBlock hashes
+    pub fn filtered_block_strategy() -> BoxedStrategy<Self> {
+        (any::<[u8; 32]>())
+            .prop_map(block::Hash)
+            .prop_map(InventoryHash::FilteredBlock)
+            .boxed()
+    }
+}
+
+impl Arbitrary for InventoryHash {
+    type Parameters = ();
+
+    fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
+        prop_oneof![
+            Self::error_strategy(),
+            Self::tx_strategy(),
+            Self::block_strategy(),
+            Self::filtered_block_strategy(),
+        ]
+        .boxed()
+    }
+
+    type Strategy = BoxedStrategy<Self>;
+}

zebra-network/src/protocol/external/tests/preallocate.rs

@@ -2,73 +2,50 @@
 
 use super::super::inv::{InventoryHash, INV_HASH_SIZE};
 
-use zebra_chain::{
+use zebra_chain::serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN};
-    block,
-    serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN},
-    transaction,
-};
 
+use proptest::prelude::*;
 use std::convert::TryInto;
 
-/// Confirm that each InventoryHash takes exactly INV_HASH_SIZE bytes when serialized.
+proptest! {
-/// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound.
+    /// Confirm that each InventoryHash takes exactly INV_HASH_SIZE bytes when serialized.
-#[test]
+    /// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound.
-fn inv_hash_size_is_correct() {
+    #[test]
-    let block_hash = block::Hash([1u8; 32]);
+    fn inv_hash_size_is_correct(inv in InventoryHash::arbitrary()) {
-    let tx_hash = transaction::Hash([1u8; 32]);
+        let serialized_inv = inv
-    let inv_block = InventoryHash::Block(block_hash);
+            .zcash_serialize_to_vec()
-    let serialized_inv_block = inv_block
+            .expect("Serialization to vec must succeed");
-        .zcash_serialize_to_vec()
+        assert!(serialized_inv.len() == INV_HASH_SIZE);
-        .expect("Serialization to vec must succeed");
-    assert!(serialized_inv_block.len() == INV_HASH_SIZE);
-
-    let inv_filtered_block = InventoryHash::FilteredBlock(block_hash);
-    let serialized_inv_filtered = inv_filtered_block
-        .zcash_serialize_to_vec()
-        .expect("Serialization to vec must succeed");
-    assert!(serialized_inv_filtered.len() == INV_HASH_SIZE);
-
-    let inv_tx = InventoryHash::Tx(tx_hash);
-    let serialized_inv_tx = inv_tx
-        .zcash_serialize_to_vec()
-        .expect("Serialization to vec must succeed");
-    assert!(serialized_inv_tx.len() == INV_HASH_SIZE);
-
-    let inv_err = InventoryHash::Error;
-    let serializd_inv_err = inv_err
-        .zcash_serialize_to_vec()
-        .expect("Serialization to vec must succeed");
-    assert!(serializd_inv_err.len() == INV_HASH_SIZE)
-}
-
-/// Verifies that...
-/// 1. The smallest disallowed vector of `InventoryHash`s is too large to fit in a legal Zcash message
-/// 2. The largest allowed vector is small enough to fit in a legal Zcash message
-#[test]
-fn inv_hash_max_allocation_is_correct() {
-    let inv = InventoryHash::Error;
-    let max_allocation: usize = InventoryHash::max_allocation().try_into().unwrap();
-    let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1);
-    for _ in 0..(InventoryHash::max_allocation() + 1) {
-        smallest_disallowed_vec.push(inv);
     }
-    let smallest_disallowed_serialized = smallest_disallowed_vec
-        .zcash_serialize_to_vec()
-        .expect("Serialization to vec must succeed");
-    // Check that our smallest_disallowed_vec is only one item larger than the limit
-    assert!(((smallest_disallowed_vec.len() - 1) as u64) == InventoryHash::max_allocation());
-    // Check that our smallest_disallowed_vec is too big to fit in a Zcash message.
-    assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN);
 
-    // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency)
+    /// Verifies that...
-    smallest_disallowed_vec.pop();
+    /// 1. The smallest disallowed vector of `InventoryHash`s is too large to fit in a legal Zcash message
-    let largest_allowed_vec = smallest_disallowed_vec;
+    /// 2. The largest allowed vector is small enough to fit in a legal Zcash message
-    let largest_allowed_serialized = largest_allowed_vec
+    #[test]
-        .zcash_serialize_to_vec()
+    fn inv_hash_max_allocation_is_correct(inv in InventoryHash::arbitrary()) {
-        .expect("Serialization to vec must succeed");
+        let max_allocation: usize = InventoryHash::max_allocation().try_into().unwrap();
+        let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1);
+        for _ in 0..(InventoryHash::max_allocation() + 1) {
+            smallest_disallowed_vec.push(inv);
+        }
+        let smallest_disallowed_serialized = smallest_disallowed_vec
+            .zcash_serialize_to_vec()
+            .expect("Serialization to vec must succeed");
+        // Check that our smallest_disallowed_vec is only one item larger than the limit
+        assert!(((smallest_disallowed_vec.len() - 1) as u64) == InventoryHash::max_allocation());
+        // Check that our smallest_disallowed_vec is too big to fit in a Zcash message.
+        assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN);
 
-    // Check that our largest_allowed_vec contains the maximum number of InventoryHashes
+        // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency)
-    assert!((largest_allowed_vec.len() as u64) == InventoryHash::max_allocation());
+        smallest_disallowed_vec.pop();
-    // Check that our largest_allowed_vec is small enough to fit in a Zcash message.
+        let largest_allowed_vec = smallest_disallowed_vec;
-    assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN);
+        let largest_allowed_serialized = largest_allowed_vec
+            .zcash_serialize_to_vec()
+            .expect("Serialization to vec must succeed");
+
+        // Check that our largest_allowed_vec contains the maximum number of InventoryHashes
+        assert!((largest_allowed_vec.len() as u64) == InventoryHash::max_allocation());
+        // Check that our largest_allowed_vec is small enough to fit in a Zcash message.
+        assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN);
+    }
 }

zebra-network/src/protocol/external/types.rs

@@ -78,6 +78,7 @@ bitflags! {
     /// Note that bits 24-31 are reserved for temporary experiments; other
     /// service bits should be allocated via the ZIP process.
     #[derive(Default)]
+    #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
     pub struct PeerServices: u64 {
         /// NODE_NETWORK means that the node is a full node capable of serving
         /// blocks, as opposed to a light client that makes network requests but