Commit 0ab8594c authored by Peter Goodspeed-Niklaus's avatar Peter Goodspeed-Niklaus
Browse files

remove randomness in favor of a simpler 1 of N procedure

This deserves a bit of explanation, as the motivating issue explicitly
requested randomness. In short, it's hard to get randomness to compile
for `wasm32-unknown-unknown` because that is explicitly intended to be
as deterministic as practical. Additionally, even though it would never
be used for consensus purposes, it still felt offputting to intentionally
introduce randomness into a node's operations. Except, it wasn't really
random, either: it was a deterministic PRNG varying only in its state,
and getting the state to work right for that target would have required
initializing from a constant.

Given that it was a deterministic sequence anyway, it seemed much simpler
and more explicit to simply select one of each N messages instead of
attempting any kind of realistic randomness.
parent f43d9646
Pipeline #118845 passed with stages
in 20 minutes and 24 seconds
......@@ -1936,21 +1936,10 @@ checksum = "7abc8dd8451921606d809ba32e95b6111925cd2906060d2dcc29c070220503eb"
dependencies = [
"cfg-if 0.1.10",
"libc",
"wasi 0.9.0+wasi-snapshot-preview1",
"wasi",
"wasm-bindgen",
]
[[package]]
name = "getrandom"
version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4060f4657be78b8e766215b02b18a2e862d83745545de804638e2b545e81aee6"
dependencies = [
"cfg-if 1.0.0",
"libc",
"wasi 0.10.0+wasi-snapshot-preview1",
]
[[package]]
name = "ghash"
version = "0.3.0"
......@@ -5337,8 +5326,6 @@ dependencies = [
"polkadot-node-subsystem",
"polkadot-node-subsystem-util",
"polkadot-primitives",
"rand 0.8.1",
"rand_chacha 0.3.0",
"sc-client-api",
"sp-core",
"streamunordered",
......@@ -6215,7 +6202,7 @@ version = "0.7.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03"
dependencies = [
"getrandom 0.1.14",
"getrandom",
"libc",
"rand_chacha 0.2.2",
"rand_core 0.5.1",
......@@ -6223,18 +6210,6 @@ dependencies = [
"rand_pcg 0.2.1",
]
[[package]]
name = "rand"
version = "0.8.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c24fcd450d3fa2b592732565aa4f17a27a61c65ece4726353e000939b0edee34"
dependencies = [
"libc",
"rand_chacha 0.3.0",
"rand_core 0.6.1",
"rand_hc 0.3.0",
]
[[package]]
name = "rand_chacha"
version = "0.1.1"
......@@ -6255,16 +6230,6 @@ dependencies = [
"rand_core 0.5.1",
]
[[package]]
name = "rand_chacha"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e12735cf05c9e10bf21534da50a147b924d555dc7a547c42e6bb2d5b6017ae0d"
dependencies = [
"ppv-lite86",
"rand_core 0.6.1",
]
[[package]]
name = "rand_core"
version = "0.3.1"
......@@ -6286,16 +6251,7 @@ version = "0.5.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "90bde5296fc891b0cef12a6d03ddccc162ce7b2aff54160af9338f8d40df6d19"
dependencies = [
"getrandom 0.1.14",
]
[[package]]
name = "rand_core"
version = "0.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c026d7df8b298d90ccbbc5190bd04d85e159eaf5576caeacf8741da93ccbd2e5"
dependencies = [
"getrandom 0.2.1",
"getrandom",
]
[[package]]
......@@ -6325,15 +6281,6 @@ dependencies = [
"rand_core 0.5.1",
]
[[package]]
name = "rand_hc"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3190ef7066a446f2e7f42e239d161e905420ccab01eb967c9eb27d21b2322a73"
dependencies = [
"rand_core 0.6.1",
]
[[package]]
name = "rand_isaac"
version = "0.1.1"
......@@ -6460,7 +6407,7 @@ version = "0.3.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "09b23093265f8d200fa7b4c2c76297f47e681c655f6f1285a8780d6a022f7431"
dependencies = [
"getrandom 0.1.14",
"getrandom",
"redox_syscall",
"rust-argon2",
]
......@@ -7725,7 +7672,7 @@ dependencies = [
"arrayref",
"arrayvec 0.5.2",
"curve25519-dalek 2.1.0",
"getrandom 0.1.14",
"getrandom",
"merlin",
"rand 0.7.3",
"rand_core 0.5.1",
......@@ -9928,12 +9875,6 @@ version = "0.9.0+wasi-snapshot-preview1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519"
[[package]]
name = "wasi"
version = "0.10.0+wasi-snapshot-preview1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f"
[[package]]
name = "wasm-bindgen"
version = "0.2.69"
......
......@@ -13,8 +13,6 @@ polkadot-node-primitives = { package = "polkadot-node-primitives", path = "../pr
polkadot-node-subsystem-util = { path = "../subsystem-util" }
polkadot-primitives = { path = "../../primitives" }
polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../subsystem" }
rand = "0.8.1"
rand_chacha = "0.3.0"
streamunordered = "0.5.1"
tracing = "0.1.22"
tracing-futures = "0.2.4"
......
......@@ -74,7 +74,6 @@ use futures::{
Future, FutureExt, SinkExt, StreamExt,
};
use futures_timer::Delay;
use rand_chacha::ChaCha8Rng as Rng;
use streamunordered::{StreamYield, StreamUnordered};
use polkadot_primitives::v1::{Block, BlockNumber, Hash};
......@@ -101,8 +100,8 @@ const CHANNEL_CAPACITY: usize = 1024;
const STOP_DELAY: u64 = 1;
// Target for logs.
const LOG_TARGET: &'static str = "overseer";
// Rate at which messages are timed.
const MESSAGE_TIMER_METRIC_CAPTURE_RATE: f64 = 0.005;
// 1 message out of each of this many is timed.
const MESSAGE_TIMER_METRIC_CAPTURE_RATE: u32 = 256;
/// A type of messages that are sent from [`Subsystem`] to [`Overseer`].
///
......@@ -327,29 +326,17 @@ pub struct OverseerSubsystemContext<M>{
rx: mpsc::Receiver<FromOverseer<M>>,
tx: mpsc::Sender<MaybeTimed<ToOverseer>>,
metrics: Metrics,
rng: Rng,
distribution: rand::distributions::Bernoulli,
capture_each: u32,
capture_state: u32,
}
impl<M> OverseerSubsystemContext<M> {
/// Create a new `OverseerSubsystemContext` with randomized initial RNG state.
/// Create a new `OverseerSubsystemContext`.
///
/// The internal RNG is used to determine which messages are timed.
///
/// `capture_rate` determines what fraction of messages are timed. Its value is clamped
/// to the range `0.0..=1.0`.
fn new(rx: mpsc::Receiver<FromOverseer<M>>, tx: mpsc::Sender<MaybeTimed<ToOverseer>>, metrics: Metrics, mut capture_rate: f64) -> Self {
use rand_chacha::rand_core::SeedableRng;
let rng = Rng::from_rng(rand::thread_rng()).expect("can fail only if the inner RNG is fallible; thread_rng is infallible; qed");
if capture_rate < 0.0 {
capture_rate = 0.0;
} else if capture_rate > 1.0 {
capture_rate = 1.0;
}
let distribution = rand::distributions::Bernoulli::new(capture_rate).expect("input is clamped to valid range; qed");
OverseerSubsystemContext { rx, tx, metrics, rng, distribution }
/// `capture_each` determines which messages are timed, i.e. a value of 256 means that every 256th
/// message is timed. A value of `0` has a special meaning: no messages are timed.
fn new(rx: mpsc::Receiver<FromOverseer<M>>, tx: mpsc::Sender<MaybeTimed<ToOverseer>>, metrics: Metrics, capture_each: u32) -> Self {
OverseerSubsystemContext { rx, tx, metrics, capture_each, capture_state: 0 }
}
/// Create a new `OverseserSubsystemContext` with no metering.
......@@ -357,19 +344,24 @@ impl<M> OverseerSubsystemContext<M> {
/// Intended for tests.
#[allow(unused)]
fn new_unmetered(rx: mpsc::Receiver<FromOverseer<M>>, tx: mpsc::Sender<MaybeTimed<ToOverseer>>) -> Self {
use rand_chacha::rand_core::SeedableRng;
let rng = Rng::seed_from_u64(0);
let metrics = Metrics::default();
let distribution = rand::distributions::Bernoulli::new(0.0).expect("input is within valid range; qed");
OverseerSubsystemContext::new(rx, tx, metrics, 0)
}
OverseerSubsystemContext { rx, tx, metrics, rng, distribution }
fn should_capture(&mut self) -> bool {
if self.capture_each == 0 {
false
} else {
self.capture_state += 1;
if self.capture_state >= self.capture_each {
self.capture_state = 0;
}
self.capture_state == 0
}
}
fn maybe_timed<T>(&mut self, t: T) -> MaybeTimed<T> {
use rand::distributions::Distribution;
let timer = if self.distribution.sample(&mut self.rng) {
let timer = if self.should_capture() {
self.metrics.time_message_hold()
} else {
None
......@@ -383,22 +375,24 @@ impl<M> OverseerSubsystemContext<M> {
///
/// This is somewhat more expensive than `self.maybe_timed` because it must clone some stuff.
fn make_maybe_timed<T>(&mut self) -> impl FnMut(T) -> MaybeTimed<T> {
use rand::{RngCore, SeedableRng};
// We don't want to simply clone this RNG because we don't want to duplicate its state.
// It's not ever going to be used for cryptographic purposes, but it's still better to
// keep good habits.
let mut child_seed = [0_u8; 32];
self.rng.fill_bytes(&mut child_seed);
let mut rng = Rng::from_seed(child_seed);
let metrics = self.metrics.clone();
let distribution = self.distribution.clone();
let capture_each = self.capture_each;
let mut capture_state = self.capture_state;
move |t| {
use rand::distributions::Distribution;
let mut should_capture = move || {
if capture_each == 0 {
false
} else {
capture_state += 1;
if capture_state >= capture_each {
capture_state = 0;
}
capture_state == 0
}
};
let timer = if distribution.sample(&mut rng) {
let timer = if should_capture() {
metrics.time_message_hold()
} else {
None
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment