diff --git a/Cargo.lock b/Cargo.lock index 05715ad8dfb3..c83eeb050df9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4844,6 +4844,16 @@ dependencies = [ "syn 2.0.38", ] +[[package]] +name = "env_logger" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3" +dependencies = [ + "log", + "regex", +] + [[package]] name = "env_logger" version = "0.9.3" @@ -11905,6 +11915,7 @@ dependencies = [ "polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-util", "polkadot-primitives", + "quickcheck", "rand 0.8.5", "rand_chacha 0.3.1", "sc-network", @@ -13731,6 +13742,8 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6" dependencies = [ + "env_logger 0.8.4", + "log", "rand 0.8.5", ] diff --git a/polkadot/node/network/gossip-support/Cargo.toml b/polkadot/node/network/gossip-support/Cargo.toml index a9f68261addf..64a9743d1db2 100644 --- a/polkadot/node/network/gossip-support/Cargo.toml +++ b/polkadot/node/network/gossip-support/Cargo.toml @@ -35,3 +35,4 @@ polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } assert_matches = "1.4.0" async-trait = "0.1.57" lazy_static = "1.4.0" +quickcheck = "1.0.3" diff --git a/polkadot/node/network/gossip-support/src/lib.rs b/polkadot/node/network/gossip-support/src/lib.rs index 674c86e5ce27..0d1b04f2ba23 100644 --- a/polkadot/node/network/gossip-support/src/lib.rs +++ b/polkadot/node/network/gossip-support/src/lib.rs @@ -32,7 +32,7 @@ use std::{ use futures::{channel::oneshot, select, FutureExt as _}; use futures_timer::Delay; -use rand::{seq::SliceRandom as _, SeedableRng}; +use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; use sc_network::{config::parse_addr, Multiaddr}; @@ -607,7 +607,7 @@ async fn update_gossip_topology( .map(|(i, a)| (a.clone(), ValidatorIndex(i as _))) .collect(); - canonical_shuffling.shuffle(&mut rng); + fisher_yates_shuffle(&mut rng, &mut canonical_shuffling[..]); for (i, (_, validator_index)) in canonical_shuffling.iter().enumerate() { shuffled_indices[validator_index.0 as usize] = i; } @@ -627,6 +627,16 @@ async fn update_gossip_topology( Ok(()) } +// Durstenfeld algorithm for the Fisher-Yates shuffle +// https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#The_modern_algorithm +fn fisher_yates_shuffle(rng: &mut R, items: &mut [T]) { + for i in (1..items.len()).rev() { + // invariant: elements with index > i have been locked in place. + let index = rng.gen_range(0u32..(i as u32 + 1)); + items.swap(i, index as usize); + } +} + #[overseer::subsystem(GossipSupport, error = SubsystemError, prefix = self::overseer)] impl GossipSupport where diff --git a/polkadot/node/network/gossip-support/src/tests.rs b/polkadot/node/network/gossip-support/src/tests.rs index 2e909bb0a674..e5ee101c31d8 100644 --- a/polkadot/node/network/gossip-support/src/tests.rs +++ b/polkadot/node/network/gossip-support/src/tests.rs @@ -22,6 +22,8 @@ use assert_matches::assert_matches; use async_trait::async_trait; use futures::{executor, future, Future}; use lazy_static::lazy_static; +use quickcheck::quickcheck; +use rand::seq::SliceRandom as _; use sc_network::multiaddr::Protocol; use sp_authority_discovery::AuthorityPair as AuthorityDiscoveryPair; @@ -710,3 +712,23 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { assert_eq!(state.last_session_index, Some(1)); assert!(state.last_failure.is_none()); } + +// note: this test was added at a time where the default `rand::SliceRandom::shuffle` +// function was used to shuffle authorities for the topology and ensures backwards compatibility. +// +// in the same commit, an explicit fisher-yates implementation was added in place of the unspecified +// behavior of that function. If this test begins to fail at some point in the future, it can simply +// be removed as the desired behavior has been preserved. +quickcheck! { + fn rng_shuffle_equals_fisher_yates(x: Vec, seed_base: u8) -> bool { + let mut rng1: ChaCha20Rng = SeedableRng::from_seed([seed_base; 32]); + let mut rng2: ChaCha20Rng = SeedableRng::from_seed([seed_base; 32]); + + let mut data1 = x.clone(); + let mut data2 = x; + + data1.shuffle(&mut rng1); + crate::fisher_yates_shuffle(&mut rng2, &mut data2[..]); + data1 == data2 + } +}