Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ethexe/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,6 @@ gear-workspace-hack.workspace = true
libp2p-swarm-test = { version = "0.6.0", default-features = false, features = ["tokio"] }
tokio = { workspace = true, features = ["full", "test-util"] }
tracing-subscriber.workspace = true
proptest = { workspace = true }
ethexe-common = { workspace = true, features = ["mock"] }
ethexe-db = { workspace = true, features = ["mock"] }
6 changes: 3 additions & 3 deletions ethexe/network/src/injected.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,10 @@ impl NetworkBehaviour for Behaviour {
mod tests {
use super::*;
use crate::{
utils::tests::init_logger,
utils::tests::{arb_value, init_logger},
validator::discovery::{SignedValidatorIdentity, ValidatorAddresses, ValidatorIdentity},
};
use ethexe_common::{injected::InjectedTransaction, mock::Mock};
use ethexe_common::injected::InjectedTransaction;
use gsigner::secp256k1::{Secp256k1SignerExt, Signer};
use libp2p::{
Swarm, Transport,
Expand All @@ -420,7 +420,7 @@ mod tests {
let signer = Signer::memory();
let pub_key = signer.generate().unwrap();

let tx = InjectedTransaction::mock(());
let tx = arb_value::<InjectedTransaction>(());
let tx = signer.signed_message(pub_key, tx, None).unwrap();

AddressedInjectedTransaction { recipient, tx }
Expand Down
6 changes: 3 additions & 3 deletions ethexe/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -845,11 +845,11 @@ mod tests {
use super::*;
use crate::{
db_sync::{ExternalDataProvider, tests::fill_data_provider},
utils::tests::init_logger,
utils::tests::{arb_value, init_logger},
};
use assert_matches::assert_matches;
use async_trait::async_trait;
use ethexe_common::{BlockHeader, ProtocolTimelines, db::*, gear::CodeState, mock::*};
use ethexe_common::{BlockHeader, ProtocolTimelines, db::*, gear::CodeState};
use ethexe_db::Database;
use gprimitives::{ActorId, CodeId, H256};
use gsigner::secp256k1::Signer;
Expand Down Expand Up @@ -970,7 +970,7 @@ mod tests {

db.set_config(DBConfig {
timelines: TIMELINES,
..DBConfig::mock(())
..arb_value::<DBConfig>(())
});

let key = signer.generate().unwrap();
Expand Down
15 changes: 15 additions & 0 deletions ethexe/network/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,11 @@ pub(crate) mod tests {
utils::{ConnectionMap, ExponentialBackoffInterval},
};
use libp2p::swarm::ConnectionId;
use proptest::{
arbitrary::Arbitrary,
strategy::{Strategy, ValueTree},
test_runner::TestRunner,
};
use std::{collections::HashSet, future, time::Duration};
use tokio::time;
use tracing_subscriber::EnvFilter;
Expand All @@ -423,6 +428,16 @@ pub(crate) mod tests {
.try_init();
}

pub fn arb_value<T>(args: impl Into<T::Parameters>) -> T
where
T: Arbitrary + 'static,
{
T::arbitrary_with(args.into())
.new_tree(&mut TestRunner::default())
Comment thread
ark0f marked this conversation as resolved.
.expect("strategy must produce a value")
.current()
}

#[test]
fn connection_map_key_cleared() {
let mut map = ConnectionMap::without_limits();
Expand Down
160 changes: 145 additions & 15 deletions ethexe/network/src/validator/topic.rs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, remove or rewrite proptests. The only property we want to test is relationship between era index in snapshot and message itself

Original file line number Diff line number Diff line change
Expand Up @@ -326,16 +326,18 @@ impl ValidatorTopic {
#[cfg(test)]
mod tests {
use super::*;
use crate::utils::tests::arb_value;
use assert_matches::assert_matches;
use ethexe_common::{
Announce,
ecdsa::SignedData,
gear_core::{message::ReplyCode, rpc::ReplyInfo},
injected::Promise,
mock::Mock,
network::{SignedValidatorMessage, ValidatorMessage},
};
use gsigner::secp256k1::{Secp256k1SignerExt, Signer};
use gsigner::secp256k1::{PrivateKey, Secp256k1SignerExt, Signer};
use nonempty::{NonEmpty, nonempty};
use proptest::{prelude::*, test_runner::Config as ProptestConfig};

const CHAIN_HEAD_ERA: u64 = 10;

Expand All @@ -357,24 +359,25 @@ mod tests {
)
}

fn new_validator_message(era_index: u64) -> VerifiedValidatorMessage {
let signer = Signer::memory();
let pub_key = signer.generate().unwrap();

signer
.signed_data(
pub_key,
ValidatorMessage {
era_index,
payload: Announce::mock(()),
},
None,
)
fn validator_message_from_private_key(
private_key: PrivateKey,
era_index: u64,
payload: Announce,
) -> VerifiedValidatorMessage {
SignedData::create(&private_key, ValidatorMessage { era_index, payload })
.map(SignedValidatorMessage::from)
.unwrap()
.into_verified()
}

fn new_validator_message(era_index: u64) -> VerifiedValidatorMessage {
validator_message_from_private_key(
PrivateKey::random(),
era_index,
arb_value::<Announce>(()),
)
}

fn signed_promise() -> SignedPromise {
let signer = Signer::memory();
let pub_key = signer.generate().unwrap();
Expand All @@ -390,6 +393,133 @@ mod tests {
signer.signed_message(pub_key, promise, None).unwrap()
}

fn private_key_strategy() -> impl Strategy<Value = PrivateKey> {
any::<[u8; 32]>().prop_filter_map("valid secp256k1 private key", |seed| {
PrivateKey::from_seed(seed).ok()
})
}

fn distinct_private_keys() -> impl Strategy<Value = (PrivateKey, PrivateKey)> {
(private_key_strategy(), private_key_strategy()).prop_filter(
"distinct validator addresses",
|(message_key, next_key)| {
message_key.public_key().to_address() != next_key.public_key().to_address()
},
)
}

proptest! {
#![proptest_config(ProptestConfig::with_cases(64))]

#[test]
fn proptest_current_era_messages_from_current_validators_are_accepted(
private_key in private_key_strategy(),
payload in any::<Announce>(),
) {
let message = validator_message_from_private_key(private_key, CHAIN_HEAD_ERA, payload);
let expected = message.clone();
let mut alice = new_topic(nonempty![expected.address()]);

let (acceptance, verified_msg) =
alice.verify_validator_message(PeerId::random(), message);

prop_assert!(matches!(acceptance, MessageAcceptance::Accept));
prop_assert_eq!(verified_msg, Some(expected));
prop_assert_eq!(alice.cached_messages.len(), 0);
prop_assert_eq!(alice.next_message(), None);
}

#[test]
fn proptest_next_era_messages_from_next_validators_are_cached_and_released_once(
private_key in private_key_strategy(),
payload in any::<Announce>(),
) {
let message =
validator_message_from_private_key(private_key, CHAIN_HEAD_ERA + 1, payload);
let expected = message.clone();
let snapshot = ValidatorListSnapshot {
current_era_index: CHAIN_HEAD_ERA,
current_validators: nonempty![Address::default()].into(),
next_validators: Some(nonempty![expected.address()].into()),
};
let mut alice = ValidatorTopic::new(peer_score::Handle::new_test(), Arc::new(snapshot));

let (acceptance, verified_msg) =
alice.verify_validator_message(PeerId::random(), message);

prop_assert!(matches!(acceptance, MessageAcceptance::Ignore));
prop_assert_eq!(verified_msg, None);
prop_assert_eq!(alice.cached_messages.len(), 1);

alice.on_new_snapshot(new_snapshot(CHAIN_HEAD_ERA + 1, nonempty![expected.address()]));

prop_assert_eq!(alice.cached_messages.len(), 0);
prop_assert_eq!(alice.next_message(), Some(expected));
prop_assert_eq!(alice.next_message(), None);
}

#[test]
fn proptest_too_old_messages_are_rejected(
private_key in private_key_strategy(),
payload in any::<Announce>(),
era_index in 0u64..(CHAIN_HEAD_ERA - 1),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The property test proptest_too_old_messages_are_rejected uses the range 0u64..(CHAIN_HEAD_ERA - 1), which excludes CHAIN_HEAD_ERA - 1. Meanwhile, proptest_current_era_messages_from_current_validators_are_accepted only tests CHAIN_HEAD_ERA. This leaves the boundary case CHAIN_HEAD_ERA - 1 untested by property tests. Depending on the protocol rules, this era should either be accepted or rejected; the ranges should be adjusted to cover it and ensure no gaps in verification.

) {
let message = validator_message_from_private_key(private_key, era_index, payload);
let mut alice = new_topic(nonempty![message.address()]);

let (acceptance, verified_msg) =
alice.verify_validator_message(PeerId::random(), message);

prop_assert!(matches!(acceptance, MessageAcceptance::Reject));
prop_assert_eq!(verified_msg, None);
prop_assert_eq!(alice.cached_messages.len(), 0);
prop_assert_eq!(alice.next_message(), None);
}

#[test]
fn proptest_too_new_messages_are_rejected(
private_key in private_key_strategy(),
payload in any::<Announce>(),
era_index in (CHAIN_HEAD_ERA + 2)..(CHAIN_HEAD_ERA + 102),
) {
let message = validator_message_from_private_key(private_key, era_index, payload);
let mut alice = new_topic(nonempty![message.address()]);

let (acceptance, verified_msg) =
alice.verify_validator_message(PeerId::random(), message);

prop_assert!(matches!(acceptance, MessageAcceptance::Reject));
prop_assert_eq!(verified_msg, None);
prop_assert_eq!(alice.cached_messages.len(), 0);
prop_assert_eq!(alice.next_message(), None);
}

#[test]
fn proptest_next_era_messages_from_non_next_validators_are_rejected(
keys in distinct_private_keys(),
payload in any::<Announce>(),
) {
let (message_key, next_key) = keys;
let message =
validator_message_from_private_key(message_key, CHAIN_HEAD_ERA + 1, payload);
let next_address = next_key.public_key().to_address();
let snapshot = ValidatorListSnapshot {
current_era_index: CHAIN_HEAD_ERA,
current_validators: nonempty![next_address].into(),
next_validators: Some(nonempty![next_address].into()),
};
let mut alice = ValidatorTopic::new(peer_score::Handle::new_test(), Arc::new(snapshot));

let (acceptance, verified_msg) =
alice.verify_validator_message(PeerId::random(), message);

prop_assert!(matches!(acceptance, MessageAcceptance::Reject));
prop_assert_eq!(verified_msg, None);
prop_assert_eq!(alice.cached_messages.len(), 0);
prop_assert_eq!(alice.next_message(), None);
}
}

#[test]
fn too_old_era() {
let bob_message = new_validator_message(CHAIN_HEAD_ERA - 2);
Expand Down
12 changes: 6 additions & 6 deletions utils/gear-workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ errno = { version = "0.3" }
gimli = { version = "0.28" }
hyper-rustls = { version = "0.27", default-features = false, features = ["aws-lc-rs", "http1", "http2", "logging", "ring", "tls12", "webpki-tokio"] }
hyper-util = { version = "0.1", default-features = false, features = ["client-proxy"] }
itertools-594e8ee84c453af0 = { package = "itertools", version = "0.13", default-features = false, features = ["use_std"] }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
Comment thread
ark0f marked this conversation as resolved.
libc = { version = "0.2", default-features = false, features = ["extra_traits"] }
miniz_oxide = { version = "0.8", default-features = false, features = ["simd", "with-alloc"] }
mio = { version = "1", features = ["net", "os-ext"] }
Expand All @@ -803,7 +803,7 @@ errno = { version = "0.3" }
gimli = { version = "0.28" }
hyper-rustls = { version = "0.27", default-features = false, features = ["aws-lc-rs", "http1", "http2", "logging", "ring", "tls12", "webpki-tokio"] }
hyper-util = { version = "0.1", default-features = false, features = ["client-proxy"] }
itertools-594e8ee84c453af0 = { package = "itertools", version = "0.13", default-features = false, features = ["use_std"] }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
libc = { version = "0.2", default-features = false, features = ["extra_traits"] }
miniz_oxide = { version = "0.8", default-features = false, features = ["simd", "with-alloc"] }
mio = { version = "1", features = ["net", "os-ext"] }
Expand All @@ -828,7 +828,7 @@ errno = { version = "0.3" }
gimli = { version = "0.28" }
hyper-rustls = { version = "0.27", default-features = false, features = ["aws-lc-rs", "http1", "http2", "logging", "ring", "tls12", "webpki-tokio"] }
hyper-util = { version = "0.1", default-features = false, features = ["client-proxy"] }
itertools-594e8ee84c453af0 = { package = "itertools", version = "0.13", default-features = false, features = ["use_std"] }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
libc = { version = "0.2", default-features = false, features = ["extra_traits"] }
miniz_oxide = { version = "0.8", default-features = false, features = ["simd", "with-alloc"] }
mio = { version = "1", features = ["net", "os-ext"] }
Expand All @@ -851,7 +851,7 @@ errno = { version = "0.3" }
gimli = { version = "0.28" }
hyper-rustls = { version = "0.27", default-features = false, features = ["aws-lc-rs", "http1", "http2", "logging", "ring", "tls12", "webpki-tokio"] }
hyper-util = { version = "0.1", default-features = false, features = ["client-proxy"] }
itertools-594e8ee84c453af0 = { package = "itertools", version = "0.13", default-features = false, features = ["use_std"] }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
libc = { version = "0.2", default-features = false, features = ["extra_traits"] }
miniz_oxide = { version = "0.8", default-features = false, features = ["simd", "with-alloc"] }
mio = { version = "1", features = ["net", "os-ext"] }
Expand All @@ -875,7 +875,7 @@ errno = { version = "0.3" }
gimli = { version = "0.28" }
hyper-rustls = { version = "0.27", default-features = false, features = ["aws-lc-rs", "http1", "http2", "logging", "ring", "tls12", "webpki-tokio"] }
hyper-util = { version = "0.1", default-features = false, features = ["client-proxy"] }
itertools-594e8ee84c453af0 = { package = "itertools", version = "0.13", default-features = false, features = ["use_std"] }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
libc = { version = "0.2", default-features = false, features = ["extra_traits"] }
miniz_oxide = { version = "0.8", default-features = false, features = ["simd", "with-alloc"] }
nom = { version = "7" }
Expand All @@ -897,7 +897,7 @@ errno = { version = "0.3" }
gimli = { version = "0.28" }
hyper-rustls = { version = "0.27", default-features = false, features = ["aws-lc-rs", "http1", "http2", "logging", "ring", "tls12", "webpki-tokio"] }
hyper-util = { version = "0.1", default-features = false, features = ["client-proxy"] }
itertools-594e8ee84c453af0 = { package = "itertools", version = "0.13", default-features = false, features = ["use_std"] }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
libc = { version = "0.2", default-features = false, features = ["extra_traits"] }
miniz_oxide = { version = "0.8", default-features = false, features = ["simd", "with-alloc"] }
nom = { version = "7" }
Expand Down
Loading