From f59ea03479103ccc0e0e25bc03e2b73373d13e11 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 10 Jul 2024 09:14:05 -0400 Subject: [PATCH 1/4] multi: add cfg struct to simulation Nodes and activity are not included here, because we want to be able to create our config struct separately from parsing of nodes and activity. This reasoning will become more apparent in future commits when we add two different running modes for the simulator (regular operation and simulated network mode). --- sim-cli/src/main.rs | 13 +++++--- sim-lib/src/lib.rs | 74 ++++++++++++++++++++++++++++----------------- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/sim-cli/src/main.rs b/sim-cli/src/main.rs index 3021023b..70f32237 100644 --- a/sim-cli/src/main.rs +++ b/sim-cli/src/main.rs @@ -1,4 +1,5 @@ use bitcoin::secp256k1::PublicKey; +use sim_lib::SimulationCfg; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; @@ -205,13 +206,15 @@ async fn main() -> anyhow::Result<()> { }; let sim = Simulation::new( + SimulationCfg::new( + cli.total_time, + cli.expected_pmt_amt, + cli.capacity_multiplier, + write_results, + cli.fix_seed, + ), clients, validated_activities, - cli.total_time, - cli.expected_pmt_amt, - cli.capacity_multiplier, - write_results, - cli.fix_seed, ); let sim2 = sim.clone(); diff --git a/sim-lib/src/lib.rs b/sim-lib/src/lib.rs index 2d20568b..92bfd5ff 100644 --- a/sim-lib/src/lib.rs +++ b/sim-lib/src/lib.rs @@ -474,15 +474,9 @@ impl MutRng { } } +/// Contains the configuration options for our simulation. #[derive(Clone)] -pub struct Simulation { - /// The lightning node that is being simulated. - nodes: HashMap>>, - /// The activity that are to be executed on the node. - activity: Vec, - /// High level triggers used to manage simulation tasks and shutdown. - shutdown_trigger: Trigger, - shutdown_listener: Listener, +pub struct SimulationCfg { /// Total simulation time. The simulation will run forever if undefined. total_time: Option, /// The expected payment size for the network. @@ -496,6 +490,37 @@ pub struct Simulation { seeded_rng: MutRng, } +impl SimulationCfg { + pub fn new( + total_time: Option, + expected_payment_msat: u64, + activity_multiplier: f64, + write_results: Option, + seed: Option, + ) -> Self { + Self { + total_time: total_time.map(|x| Duration::from_secs(x as u64)), + expected_payment_msat, + activity_multiplier, + write_results, + seeded_rng: MutRng::new(seed), + } + } +} + +#[derive(Clone)] +pub struct Simulation { + /// Config for the simulation itself. + cfg: SimulationCfg, + /// The lightning node that is being simulated. + nodes: HashMap>>, + /// The activity that are to be executed on the node. + activity: Vec, + /// High level triggers used to manage simulation tasks and shutdown. + shutdown_trigger: Trigger, + shutdown_listener: Listener, +} + #[derive(Clone)] pub struct WriteResults { /// Data directory where CSV result files are written. @@ -516,25 +541,17 @@ struct ExecutorKit { impl Simulation { pub fn new( + cfg: SimulationCfg, nodes: HashMap>>, activity: Vec, - total_time: Option, - expected_payment_msat: u64, - activity_multiplier: f64, - write_results: Option, - seed: Option, ) -> Self { let (shutdown_trigger, shutdown_listener) = triggered::trigger(); Self { + cfg, nodes, activity, shutdown_trigger, shutdown_listener, - total_time: total_time.map(|x| Duration::from_secs(x as u64)), - expected_payment_msat, - activity_multiplier, - write_results, - seeded_rng: MutRng::new(seed), } } @@ -620,7 +637,7 @@ impl Simulation { } pub async fn run(&self) -> Result<(), SimulationError> { - if let Some(total_time) = self.total_time { + if let Some(total_time) = self.cfg.total_time { log::info!("Running the simulation for {}s.", total_time.as_secs()); } else { log::info!("Running the simulation forever."); @@ -706,7 +723,7 @@ impl Simulation { }); // Start a task that will shutdown the simulation if the total_time is met. - if let Some(total_time) = self.total_time { + if let Some(total_time) = self.cfg.total_time { let t = self.shutdown_trigger.clone(); let l = self.shutdown_listener.clone(); @@ -786,7 +803,7 @@ impl Simulation { }); // csr: consume simulation results - let csr_write_results = self.write_results.clone(); + let csr_write_results = self.cfg.write_results.clone(); tasks.spawn(async move { log::debug!("Starting simulation results consumer."); if let Err(e) = consume_simulation_results( @@ -853,9 +870,10 @@ impl Simulation { for (pk, node) in self.nodes.iter() { let chan_capacity = node.lock().await.list_channels().await?.iter().sum::(); - if let Err(e) = - RandomPaymentActivity::validate_capacity(chan_capacity, self.expected_payment_msat) - { + if let Err(e) = RandomPaymentActivity::validate_capacity( + chan_capacity, + self.cfg.expected_payment_msat, + ) { log::warn!("Node: {} not eligible for activity generation: {e}.", *pk); continue; } @@ -870,7 +888,7 @@ impl Simulation { let network_generator = Arc::new(Mutex::new( NetworkGraphView::new( active_nodes.values().cloned().collect(), - self.seeded_rng.clone(), + self.cfg.seeded_rng.clone(), ) .map_err(SimulationError::RandomActivityError)?, )); @@ -887,9 +905,9 @@ impl Simulation { payment_generator: Box::new( RandomPaymentActivity::new( *capacity, - self.expected_payment_msat, - self.activity_multiplier, - self.seeded_rng.clone(), + self.cfg.expected_payment_msat, + self.cfg.activity_multiplier, + self.cfg.seeded_rng.clone(), ) .map_err(SimulationError::RandomActivityError)?, ), From 9bae9f2d6254647cb929f2aa363832e8529d90a1 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 10 Jul 2024 14:07:00 -0400 Subject: [PATCH 2/4] sim-lib/refactor: rename MutRng to RngSend Preparation for further refactoring where we'll remove the mutex. --- sim-lib/src/lib.rs | 28 ++++++++++++++-------------- sim-lib/src/random_activity.rs | 20 ++++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/sim-lib/src/lib.rs b/sim-lib/src/lib.rs index 92bfd5ff..6d642777 100644 --- a/sim-lib/src/lib.rs +++ b/sim-lib/src/lib.rs @@ -446,22 +446,22 @@ enum SimulationOutput { SendPaymentFailure(Payment, PaymentResult), } -/// MutRngType is a convenient type alias for any random number generator (RNG) type that +/// RngSendType is a convenient type alias for any random number generator (RNG) type that /// allows shared and exclusive access. This is necessary because a single RNG /// is to be shared across multiple `DestinationGenerator`s and `PaymentGenerator`s /// for deterministic outcomes. /// /// **Note**: `StdMutex`, i.e. (`std::sync::Mutex`), is used here to avoid making the traits /// `DestinationGenerator` and `PaymentGenerator` async. -type MutRngType = Arc>; +type RngSendType = Arc>; -/// Newtype for `MutRngType` to encapsulate and hide implementation details for -/// creating new `MutRngType` types. Provides convenient API for the same purpose. +/// Newtype for `RngSendType` to encapsulate and hide implementation details for +/// creating new `RngSendType` types. Provides convenient API for the same purpose. #[derive(Clone)] -struct MutRng(MutRngType); +struct RngSend(RngSendType); -impl MutRng { - /// Creates a new MutRng given an optional `u64` argument. If `seed_opt` is `Some`, +impl RngSend { + /// Creates a new RngSend given an optional `u64` argument. If `seed_opt` is `Some`, /// random activity generation in the simulator occurs near-deterministically. /// If it is `None`, activity generation is truly random, and based on a /// non-deterministic source of entropy. @@ -487,7 +487,7 @@ pub struct SimulationCfg { /// Configurations for printing results to CSV. Results are not written if this option is None. write_results: Option, /// Random number generator created from fixed seed. - seeded_rng: MutRng, + seeded_rng: RngSend, } impl SimulationCfg { @@ -503,7 +503,7 @@ impl SimulationCfg { expected_payment_msat, activity_multiplier, write_results, - seeded_rng: MutRng::new(seed), + seeded_rng: RngSend::new(seed), } } } @@ -1414,7 +1414,7 @@ async fn track_payment_result( #[cfg(test)] mod tests { - use crate::{get_payment_delay, test_utils, MutRng, PaymentGenerationError, PaymentGenerator}; + use crate::{get_payment_delay, test_utils, PaymentGenerationError, PaymentGenerator, RngSend}; use mockall::mock; use std::fmt; use std::time::Duration; @@ -1424,8 +1424,8 @@ mod tests { let seeds = vec![u64::MIN, u64::MAX]; for seed in seeds { - let mut_rng_1 = MutRng::new(Some(seed)); - let mut_rng_2 = MutRng::new(Some(seed)); + let mut_rng_1 = RngSend::new(Some(seed)); + let mut_rng_2 = RngSend::new(Some(seed)); let mut rng_1 = mut_rng_1.0.lock().unwrap(); let mut rng_2 = mut_rng_2.0.lock().unwrap(); @@ -1436,8 +1436,8 @@ mod tests { #[test] fn create_unseeded_mut_rng() { - let mut_rng_1 = MutRng::new(None); - let mut_rng_2 = MutRng::new(None); + let mut_rng_1 = RngSend::new(None); + let mut_rng_2 = RngSend::new(None); let mut rng_1 = mut_rng_1.0.lock().unwrap(); let mut rng_2 = mut_rng_2.0.lock().unwrap(); diff --git a/sim-lib/src/random_activity.rs b/sim-lib/src/random_activity.rs index b22139d9..65597523 100644 --- a/sim-lib/src/random_activity.rs +++ b/sim-lib/src/random_activity.rs @@ -7,8 +7,8 @@ use rand_distr::{Distribution, Exp, LogNormal, WeightedIndex}; use std::time::Duration; use crate::{ - DestinationGenerationError, DestinationGenerator, MutRng, NodeInfo, PaymentGenerationError, - PaymentGenerator, + DestinationGenerationError, DestinationGenerator, NodeInfo, PaymentGenerationError, + PaymentGenerator, RngSend, }; const HOURS_PER_MONTH: u64 = 30 * 24; @@ -30,14 +30,14 @@ pub enum RandomActivityError { pub struct NetworkGraphView { node_picker: WeightedIndex, nodes: Vec<(NodeInfo, u64)>, - rng: MutRng, + rng: RngSend, } impl NetworkGraphView { /// Creates a network view for the map of node public keys to capacity (in millisatoshis) provided. Returns an error /// if any node's capacity is zero (the node cannot receive), or there are not at least two nodes (one node can't /// send to itself). - pub fn new(nodes: Vec<(NodeInfo, u64)>, rng: MutRng) -> Result { + pub fn new(nodes: Vec<(NodeInfo, u64)>, rng: RngSend) -> Result { if nodes.len() < 2 { return Err(RandomActivityError::ValueError( "at least two nodes required for activity generation".to_string(), @@ -116,7 +116,7 @@ pub struct RandomPaymentActivity { expected_payment_amt: u64, source_capacity: u64, event_dist: Exp, - rng: MutRng, + rng: RngSend, } impl RandomPaymentActivity { @@ -127,7 +127,7 @@ impl RandomPaymentActivity { source_capacity_msat: u64, expected_payment_amt: u64, multiplier: f64, - rng: MutRng, + rng: RngSend, ) -> Result { if source_capacity_msat == 0 { return Err(RandomActivityError::ValueError( @@ -308,7 +308,7 @@ mod tests { #[test] fn test_new() { // Check that we need, at least, two nodes - let rng = MutRng::new(Some(u64::MAX)); + let rng = RngSend::new(Some(u64::MAX)); for i in 0..2 { assert!(matches!( NetworkGraphView::new(create_nodes(i, 42 * (i as u64 + 1)), rng.clone()), @@ -362,7 +362,7 @@ mod tests { nodes.extend(create_nodes(big_node_count, big_node_capacity)); let big_node = nodes.last().unwrap().0.pubkey; - let rng = MutRng::new(Some(u64::MAX)); + let rng = RngSend::new(Some(u64::MAX)); let view = NetworkGraphView::new(nodes, rng).unwrap(); for _ in 0..10 { @@ -380,7 +380,7 @@ mod tests { // For the payment activity generator to fail during construction either the provided capacity must fail validation or the exponential // distribution must fail building given the inputs. The former will be thoroughly tested in its own unit test, but we'll test some basic cases // here. Mainly, if the `capacity < expected_payment_amnt / 2`, the generator will fail building - let rng = MutRng::new(Some(u64::MAX)); + let rng = RngSend::new(Some(u64::MAX)); let expected_payment = get_random_int(1, 100); assert!(RandomPaymentActivity::new( 2 * expected_payment, @@ -453,7 +453,7 @@ mod tests { // All of them will yield a sigma squared smaller than 0, which we have a sanity check for. let expected_payment = get_random_int(1, 100); let source_capacity = 2 * expected_payment; - let rng = MutRng::new(Some(u64::MAX)); + let rng = RngSend::new(Some(u64::MAX)); let pag = RandomPaymentActivity::new(source_capacity, expected_payment, 1.0, rng).unwrap(); From 9a76bd68b19e58bb5f297bf196504615524c8411 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 10 Jul 2024 14:07:54 -0400 Subject: [PATCH 3/4] sim-lib: use multiple RNGs to decrease variance of fixed seed runs As-is, we are sharing a single RNG for all of our random operations - destination selection for each payment, and selection of payment parameters per-sending node. All of these operations may complete in in different orders every time we run (be it because a task gets access to a mutex in a different order, or a payment is faster or slower to dispatch on the actual underlying node). This leads to large variance in our supposedly deterministic run, because the values we get from our "fixed" seed are different based on this order of operations. To address this, we use separate RNGs per-task, which reduces the variety in order of operations impact on the values that each task gets from its RNG. We can't do this for network_generator (because we use one for all of our tasks), so we take the approach of passing a single RNG into both our Payment and Network generator. This is admittedly a little clunky API-wise, IMO a great improvement in the feature. Testing this change against the original approach with: - 50x runs of the simulator - Sum up total revenue for a target node per run - Look at the coefficient of variance (mean / std dev) for each data set Results: - Old approach: CoV = 0.5 - New approach: CoV = 40 This increase indicates a large *decrease* in the variance in the revenue of a single node across runs (which is what we want, more deterministic outcomes with a fixed seed). --- sim-lib/src/defined_activity.rs | 18 ++++-- sim-lib/src/lib.rs | 97 +++++++++++++++++--------------- sim-lib/src/random_activity.rs | 99 ++++++++++----------------------- 3 files changed, 92 insertions(+), 122 deletions(-) diff --git a/sim-lib/src/defined_activity.rs b/sim-lib/src/defined_activity.rs index ea137609..e24a7ba0 100644 --- a/sim-lib/src/defined_activity.rs +++ b/sim-lib/src/defined_activity.rs @@ -1,6 +1,6 @@ use crate::{ DestinationGenerationError, DestinationGenerator, NodeInfo, PaymentGenerationError, - PaymentGenerator, ValueOrRange, + PaymentGenerator, RngSend, ValueOrRange, }; use std::fmt; use tokio::time::Duration; @@ -45,6 +45,7 @@ impl fmt::Display for DefinedPaymentActivity { impl DestinationGenerator for DefinedPaymentActivity { fn choose_destination( &self, + _: &mut RngSend, _: bitcoin::secp256k1::PublicKey, ) -> Result<(NodeInfo, Option), DestinationGenerationError> { Ok((self.destination.clone(), None)) @@ -60,12 +61,13 @@ impl PaymentGenerator for DefinedPaymentActivity { self.count } - fn next_payment_wait(&self) -> Result { + fn next_payment_wait(&self, _: &mut RngSend) -> Result { Ok(Duration::from_secs(self.wait.value() as u64)) } fn payment_amount( &self, + _: &mut RngSend, destination_capacity: Option, ) -> Result { if destination_capacity.is_some() { @@ -82,7 +84,7 @@ impl PaymentGenerator for DefinedPaymentActivity { mod tests { use super::DefinedPaymentActivity; use crate::test_utils::{create_nodes, get_random_keypair}; - use crate::{DestinationGenerator, PaymentGenerationError, PaymentGenerator}; + use crate::{DestinationGenerator, PaymentGenerationError, PaymentGenerator, RngSend}; #[test] fn test_defined_activity_generator() { @@ -100,13 +102,17 @@ mod tests { crate::ValueOrRange::Value(payment_amt), ); - let (dest, dest_capacity) = generator.choose_destination(source.1).unwrap(); + let mut rng = RngSend::new(None); + let (dest, dest_capacity) = generator.choose_destination(&mut rng, source.1).unwrap(); assert_eq!(node.pubkey, dest.pubkey); assert!(dest_capacity.is_none()); - assert_eq!(payment_amt, generator.payment_amount(None).unwrap()); + assert_eq!( + payment_amt, + generator.payment_amount(&mut rng, None).unwrap() + ); assert!(matches!( - generator.payment_amount(Some(10)), + generator.payment_amount(&mut rng, Some(10)), Err(PaymentGenerationError(..)) )); } diff --git a/sim-lib/src/lib.rs b/sim-lib/src/lib.rs index 6d642777..79953684 100644 --- a/sim-lib/src/lib.rs +++ b/sim-lib/src/lib.rs @@ -13,7 +13,6 @@ use std::collections::HashSet; use std::fmt::{Display, Formatter}; use std::marker::Send; use std::path::PathBuf; -use std::sync::Mutex as StdMutex; use std::time::{SystemTimeError, UNIX_EPOCH}; use std::{collections::HashMap, sync::Arc, time::SystemTime}; use thiserror::Error; @@ -239,6 +238,8 @@ pub enum SimulationError { PaymentGenerationError(PaymentGenerationError), #[error("Destination Generation Error: {0}")] DestinationGenerationError(DestinationGenerationError), + #[error("Fixed Seed Error: {0}")] + FixedSeedError(String), } #[derive(Debug, Error)] @@ -317,6 +318,7 @@ pub trait DestinationGenerator: Send { /// capacity (if available). fn choose_destination( &self, + rng: &mut RngSend, source: PublicKey, ) -> Result<(NodeInfo, Option), DestinationGenerationError>; } @@ -333,11 +335,15 @@ pub trait PaymentGenerator: Display + Send { fn payment_count(&self) -> Option; /// Returns the number of seconds that a node should wait until firing its next payment. - fn next_payment_wait(&self) -> Result; + fn next_payment_wait( + &self, + rng: &mut RngSend, + ) -> Result; /// Returns a payment amount based, with a destination capacity optionally provided to inform the amount picked. fn payment_amount( &self, + rng: &mut RngSend, destination_capacity: Option, ) -> Result; } @@ -446,30 +452,22 @@ enum SimulationOutput { SendPaymentFailure(Payment, PaymentResult), } -/// RngSendType is a convenient type alias for any random number generator (RNG) type that -/// allows shared and exclusive access. This is necessary because a single RNG -/// is to be shared across multiple `DestinationGenerator`s and `PaymentGenerator`s -/// for deterministic outcomes. -/// -/// **Note**: `StdMutex`, i.e. (`std::sync::Mutex`), is used here to avoid making the traits -/// `DestinationGenerator` and `PaymentGenerator` async. -type RngSendType = Arc>; +/// RngSendType is a convenient type alias for any random number generator (RNG) type that is also Send. +type RngSendType = Box; -/// Newtype for `RngSendType` to encapsulate and hide implementation details for -/// creating new `RngSendType` types. Provides convenient API for the same purpose. -#[derive(Clone)] -struct RngSend(RngSendType); +/// Newtype for `RngSendType` to encapsulate and hide implementation details for creating new `RngSendType` types. +/// Provides convenient API for the same purpose. +pub struct RngSend(RngSendType); impl RngSend { - /// Creates a new RngSend given an optional `u64` argument. If `seed_opt` is `Some`, - /// random activity generation in the simulator occurs near-deterministically. - /// If it is `None`, activity generation is truly random, and based on a - /// non-deterministic source of entropy. + /// Creates a new RngSend given an optional `u64` argument. If `seed_opt` is `Some`, random activity generation + /// in the simulator occurs near-deterministically. If it is `None`, activity generation is truly random, and + /// based on a non-deterministic source of entropy. pub fn new(seed_opt: Option) -> Self { if let Some(seed) = seed_opt { - Self(Arc::new(StdMutex::new(ChaCha8Rng::seed_from_u64(seed)))) + RngSend(Box::new(ChaCha8Rng::seed_from_u64(seed))) } else { - Self(Arc::new(StdMutex::new(StdRng::from_entropy()))) + RngSend(Box::new(StdRng::from_entropy())) } } } @@ -487,7 +485,7 @@ pub struct SimulationCfg { /// Configurations for printing results to CSV. Results are not written if this option is None. write_results: Option, /// Random number generator created from fixed seed. - seeded_rng: RngSend, + seed: Option, } impl SimulationCfg { @@ -503,7 +501,7 @@ impl SimulationCfg { expected_payment_msat, activity_multiplier, write_results, - seeded_rng: RngSend::new(seed), + seed, } } } @@ -886,11 +884,8 @@ impl Simulation { } let network_generator = Arc::new(Mutex::new( - NetworkGraphView::new( - active_nodes.values().cloned().collect(), - self.cfg.seeded_rng.clone(), - ) - .map_err(SimulationError::RandomActivityError)?, + NetworkGraphView::new(active_nodes.values().cloned().collect()) + .map_err(SimulationError::RandomActivityError)?, )); log::info!( @@ -907,7 +902,6 @@ impl Simulation { *capacity, self.cfg.expected_payment_msat, self.cfg.activity_multiplier, - self.cfg.seeded_rng.clone(), ) .map_err(SimulationError::RandomActivityError)?, ), @@ -982,6 +976,8 @@ impl Simulation { let pe_shutdown = self.shutdown_trigger.clone(); let pe_listener = self.shutdown_listener.clone(); let pe_sender = sender.clone(); + let pe_seed = self.cfg.seed; + tasks.spawn(async move { let source = executor.source_info.clone(); @@ -996,6 +992,7 @@ impl Simulation { executor.network_generator, executor.payment_generator, pe_sender, + pe_seed, pe_listener, ) .await @@ -1091,8 +1088,14 @@ async fn produce_events>, node_generator: Box, sender: Sender, + seed: Option, listener: Listener, ) -> Result<(), SimulationError> { + // We create one RNG per payment activity generator so that when we have a fixed seed, each generator will get the + // same set of values across runs (with a shared RNG, the order in which tasks access the shared RNG would change + // the value that each generator gets). + let mut rng = RngSend::new(seed); + let mut current_count = 0; loop { if let Some(c) = node_generator.payment_count() { @@ -1104,7 +1107,7 @@ async fn produce_events { - let (destination, capacity) = network_generator.lock().await.choose_destination(source.pubkey).map_err(SimulationError::DestinationGenerationError)?; + let (destination, capacity) = network_generator.lock().await. + choose_destination(&mut rng, source.pubkey).map_err(SimulationError::DestinationGenerationError)?; // Only proceed with a payment if the amount is non-zero, otherwise skip this round. If we can't get // a payment amount something has gone wrong (because we should have validated that we can always // generate amounts), so we exit. - let amount = match node_generator.payment_amount(capacity) { + let amount = match node_generator.payment_amount(&mut rng,capacity) { Ok(amt) => { if amt == 0 { log::debug!("Skipping zero amount payment for {source} -> {destination}."); @@ -1152,6 +1156,7 @@ fn get_payment_delay( call_count: u64, source: &NodeInfo, node_generator: &A, + rng: &mut RngSend, ) -> Result { // Note: we can't check if let Some() && call_count (syntax not allowed) so we add an additional branch in here. // The alternative is to call payment_start twice (which is _technically_ fine because it always returns the same @@ -1159,7 +1164,7 @@ fn get_payment_delay( // don't have to make any assumptions about the underlying operation of payment_start. if call_count != 0 { let wait = node_generator - .next_payment_wait() + .next_payment_wait(rng) .map_err(SimulationError::PaymentGenerationError)?; log::debug!("Next payment for {source} in {:?}.", wait); Ok(wait) @@ -1171,7 +1176,7 @@ fn get_payment_delay( Ok(start) } else { let wait = node_generator - .next_payment_wait() + .next_payment_wait(rng) .map_err(SimulationError::PaymentGenerationError)?; log::debug!("First payment for {source} in {:?}.", wait); Ok(wait) @@ -1427,8 +1432,8 @@ mod tests { let mut_rng_1 = RngSend::new(Some(seed)); let mut_rng_2 = RngSend::new(Some(seed)); - let mut rng_1 = mut_rng_1.0.lock().unwrap(); - let mut rng_2 = mut_rng_2.0.lock().unwrap(); + let mut rng_1 = mut_rng_1.0; + let mut rng_2 = mut_rng_2.0; assert_eq!(rng_1.next_u64(), rng_2.next_u64()) } @@ -1439,8 +1444,8 @@ mod tests { let mut_rng_1 = RngSend::new(None); let mut_rng_2 = RngSend::new(None); - let mut rng_1 = mut_rng_1.0.lock().unwrap(); - let mut rng_2 = mut_rng_2.0.lock().unwrap(); + let mut rng_1 = mut_rng_1.0; + let mut rng_2 = mut_rng_2.0; assert_ne!(rng_1.next_u64(), rng_2.next_u64()) } @@ -1455,8 +1460,8 @@ mod tests { impl PaymentGenerator for Generator { fn payment_start(&self) -> Option; fn payment_count(&self) -> Option; - fn next_payment_wait(&self) -> Result; - fn payment_amount(&self, destination_capacity: Option) -> Result; + fn next_payment_wait(&self, rng: &mut RngSend) -> Result; + fn payment_amount(&self,rng: &mut RngSend, destination_capacity: Option) -> Result; } } @@ -1474,14 +1479,15 @@ mod tests { let payment_interval = Duration::from_secs(5); mock_generator .expect_next_payment_wait() - .returning(move || Ok(payment_interval)); + .returning(move |_| Ok(payment_interval)); + let mut rng = RngSend::new(None); assert_eq!( - get_payment_delay(0, &node, &mock_generator).unwrap(), + get_payment_delay(0, &node, &mock_generator, &mut rng).unwrap(), payment_interval ); assert_eq!( - get_payment_delay(1, &node, &mock_generator).unwrap(), + get_payment_delay(1, &node, &mock_generator, &mut rng).unwrap(), payment_interval ); } @@ -1503,14 +1509,15 @@ mod tests { let payment_interval = Duration::from_secs(5); mock_generator .expect_next_payment_wait() - .returning(move || Ok(payment_interval)); + .returning(move |_| Ok(payment_interval)); + let mut rng = RngSend::new(None); assert_eq!( - get_payment_delay(0, &node, &mock_generator).unwrap(), + get_payment_delay(0, &node, &mock_generator, &mut rng).unwrap(), start_delay ); assert_eq!( - get_payment_delay(1, &node, &mock_generator).unwrap(), + get_payment_delay(1, &node, &mock_generator, &mut rng).unwrap(), payment_interval ); } diff --git a/sim-lib/src/random_activity.rs b/sim-lib/src/random_activity.rs index 65597523..8112ca80 100644 --- a/sim-lib/src/random_activity.rs +++ b/sim-lib/src/random_activity.rs @@ -30,14 +30,13 @@ pub enum RandomActivityError { pub struct NetworkGraphView { node_picker: WeightedIndex, nodes: Vec<(NodeInfo, u64)>, - rng: RngSend, } impl NetworkGraphView { /// Creates a network view for the map of node public keys to capacity (in millisatoshis) provided. Returns an error /// if any node's capacity is zero (the node cannot receive), or there are not at least two nodes (one node can't /// send to itself). - pub fn new(nodes: Vec<(NodeInfo, u64)>, rng: RngSend) -> Result { + pub fn new(nodes: Vec<(NodeInfo, u64)>) -> Result { if nodes.len() < 2 { return Err(RandomActivityError::ValueError( "at least two nodes required for activity generation".to_string(), @@ -57,11 +56,7 @@ impl NetworkGraphView { let node_picker = WeightedIndex::new(nodes.iter().map(|(_, v)| *v).collect::>()) .map_err(|e| RandomActivityError::ValueError(e.to_string()))?; - Ok(NetworkGraphView { - node_picker, - nodes, - rng, - }) + Ok(NetworkGraphView { node_picker, nodes }) } } @@ -71,19 +66,15 @@ impl DestinationGenerator for NetworkGraphView { /// very small graphs, or those with one node significantly more capitalized than others). fn choose_destination( &self, + rng: &mut RngSend, source: PublicKey, ) -> Result<(NodeInfo, Option), DestinationGenerationError> { - let mut rng = self - .rng - .0 - .lock() - .map_err(|e| DestinationGenerationError(e.to_string()))?; // While it's very unlikely that we can't pick a destination that is not our source, it's possible that there's // a bug in our selection, so we track attempts to select a non-source node so that we can warn if this takes // improbably long. let mut i = 1; loop { - let index = self.node_picker.sample(&mut *rng); + let index = self.node_picker.sample(&mut rng.0); // Unwrapping is safe given `NetworkGraphView` has the same amount of elements for `nodes` and `node_picker` let (node_info, capacity) = self.nodes.get(index).unwrap(); @@ -116,7 +107,6 @@ pub struct RandomPaymentActivity { expected_payment_amt: u64, source_capacity: u64, event_dist: Exp, - rng: RngSend, } impl RandomPaymentActivity { @@ -127,7 +117,6 @@ impl RandomPaymentActivity { source_capacity_msat: u64, expected_payment_amt: u64, multiplier: f64, - rng: RngSend, ) -> Result { if source_capacity_msat == 0 { return Err(RandomActivityError::ValueError( @@ -163,7 +152,6 @@ impl RandomPaymentActivity { expected_payment_amt, source_capacity: source_capacity_msat, event_dist, - rng, }) } @@ -224,13 +212,8 @@ impl PaymentGenerator for RandomPaymentActivity { } /// Returns the amount of time until the next payment should be scheduled for the node. - fn next_payment_wait(&self) -> Result { - let mut rng = self - .rng - .0 - .lock() - .map_err(|e| PaymentGenerationError(e.to_string()))?; - let duration_in_secs = self.event_dist.sample(&mut *rng) as u64; + fn next_payment_wait(&self, rng: &mut RngSend) -> Result { + let duration_in_secs = self.event_dist.sample(&mut rng.0) as u64; Ok(Duration::from_secs(duration_in_secs)) } @@ -244,6 +227,7 @@ impl PaymentGenerator for RandomPaymentActivity { /// channel capacity. fn payment_amount( &self, + rng: &mut RngSend, destination_capacity: Option, ) -> Result { let destination_capacity = destination_capacity.ok_or(PaymentGenerationError( @@ -267,12 +251,7 @@ impl PaymentGenerator for RandomPaymentActivity { let log_normal = LogNormal::new(mu, sigma_square.sqrt()) .map_err(|e| PaymentGenerationError(e.to_string()))?; - let mut rng = self - .rng - .0 - .lock() - .map_err(|e| PaymentGenerationError(e.to_string()))?; - let payment_amount = log_normal.sample(&mut *rng) as u64; + let payment_amount = log_normal.sample(&mut rng.0) as u64; Ok(payment_amount) } @@ -308,10 +287,9 @@ mod tests { #[test] fn test_new() { // Check that we need, at least, two nodes - let rng = RngSend::new(Some(u64::MAX)); for i in 0..2 { assert!(matches!( - NetworkGraphView::new(create_nodes(i, 42 * (i as u64 + 1)), rng.clone()), + NetworkGraphView::new(create_nodes(i, 42 * (i as u64 + 1))), Err(RandomActivityError::ValueError { .. }) )); } @@ -321,18 +299,18 @@ mod tests { let mut nodes = create_nodes(1, 0); nodes.extend(create_nodes(1, 21)); assert!(matches!( - NetworkGraphView::new(nodes, rng.clone()), + NetworkGraphView::new(nodes), Err(RandomActivityError::InsufficientCapacity { .. }) )); // All of them are 0 assert!(matches!( - NetworkGraphView::new(create_nodes(2, 0), rng.clone()), + NetworkGraphView::new(create_nodes(2, 0)), Err(RandomActivityError::InsufficientCapacity { .. }) )); // Otherwise we should be good - assert!(NetworkGraphView::new(create_nodes(2, 42), rng).is_ok()); + assert!(NetworkGraphView::new(create_nodes(2, 42)).is_ok()); } #[test] @@ -362,11 +340,11 @@ mod tests { nodes.extend(create_nodes(big_node_count, big_node_capacity)); let big_node = nodes.last().unwrap().0.pubkey; - let rng = RngSend::new(Some(u64::MAX)); - let view = NetworkGraphView::new(nodes, rng).unwrap(); + let view = NetworkGraphView::new(nodes).unwrap(); for _ in 0..10 { - view.choose_destination(big_node).unwrap(); + view.choose_destination(&mut RngSend::new(None), big_node) + .unwrap(); } } } @@ -380,47 +358,27 @@ mod tests { // For the payment activity generator to fail during construction either the provided capacity must fail validation or the exponential // distribution must fail building given the inputs. The former will be thoroughly tested in its own unit test, but we'll test some basic cases // here. Mainly, if the `capacity < expected_payment_amnt / 2`, the generator will fail building - let rng = RngSend::new(Some(u64::MAX)); let expected_payment = get_random_int(1, 100); - assert!(RandomPaymentActivity::new( - 2 * expected_payment, - expected_payment, - 1.0, - rng.clone() - ) - .is_ok()); + assert!( + RandomPaymentActivity::new(2 * expected_payment, expected_payment, 1.0,).is_ok() + ); assert!(matches!( - RandomPaymentActivity::new( - 2 * expected_payment, - expected_payment + 1, - 1.0, - rng.clone() - ), + RandomPaymentActivity::new(2 * expected_payment, expected_payment + 1, 1.0,), Err(RandomActivityError::InsufficientCapacity { .. }) )); // Respecting the internal exponential distribution creation, neither of the parameters can be zero. Otherwise we may try to create an exponential // function with lambda = NaN, which will error out, or with lambda = Inf, which does not make sense for our use-case assert!(matches!( - RandomPaymentActivity::new( - 0, - get_random_int(1, 10), - get_random_int(1, 10) as f64, - rng.clone() - ), + RandomPaymentActivity::new(0, get_random_int(1, 10), get_random_int(1, 10) as f64,), Err(RandomActivityError::ValueError { .. }) )); assert!(matches!( - RandomPaymentActivity::new( - get_random_int(1, 10), - 0, - get_random_int(1, 10) as f64, - rng.clone() - ), + RandomPaymentActivity::new(get_random_int(1, 10), 0, get_random_int(1, 10) as f64,), Err(RandomActivityError::ValueError { .. }) )); assert!(matches!( - RandomPaymentActivity::new(get_random_int(1, 10), get_random_int(1, 10), 0.0, rng), + RandomPaymentActivity::new(get_random_int(1, 10), get_random_int(1, 10), 0.0), Err(RandomActivityError::ValueError { .. }) )); } @@ -453,30 +411,29 @@ mod tests { // All of them will yield a sigma squared smaller than 0, which we have a sanity check for. let expected_payment = get_random_int(1, 100); let source_capacity = 2 * expected_payment; - let rng = RngSend::new(Some(u64::MAX)); - let pag = - RandomPaymentActivity::new(source_capacity, expected_payment, 1.0, rng).unwrap(); + let mut rng = RngSend::new(Some(u64::MAX)); + let pag = RandomPaymentActivity::new(source_capacity, expected_payment, 1.0).unwrap(); // Wrong cases for i in 0..source_capacity { assert!(matches!( - pag.payment_amount(Some(i)), + pag.payment_amount(&mut rng, Some(i)), Err(PaymentGenerationError(..)) )) } // All other cases will work. We are not going to exhaustively test for the rest up to u64::MAX, let just pick a bunch for i in source_capacity + 1..100 * source_capacity { - assert!(pag.payment_amount(Some(i)).is_ok()) + assert!(pag.payment_amount(&mut rng, Some(i)).is_ok()) } // We can even try really high numbers to make sure they are not troublesome for i in u64::MAX - 10000..u64::MAX { - assert!(pag.payment_amount(Some(i)).is_ok()) + assert!(pag.payment_amount(&mut rng, Some(i)).is_ok()) } assert!(matches!( - pag.payment_amount(None), + pag.payment_amount(&mut rng, None), Err(PaymentGenerationError(..)) )); } From 082a6db1b7035930bb46c20afe4559679c330d3c Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 10 Jul 2024 14:00:59 -0400 Subject: [PATCH 4/4] sim-lib: make rng unique but deterministic per sending node To get some variety in our simulator, we update our RNG that's based on the same set seed to also use some data from nodes public keys. This will be the same across runs, but provides different values for each node. --- sim-lib/src/defined_activity.rs | 2 +- sim-lib/src/lib.rs | 40 ++++++++++++++++++++++++--------- sim-lib/src/random_activity.rs | 4 ++-- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/sim-lib/src/defined_activity.rs b/sim-lib/src/defined_activity.rs index e24a7ba0..a3c144a7 100644 --- a/sim-lib/src/defined_activity.rs +++ b/sim-lib/src/defined_activity.rs @@ -102,7 +102,7 @@ mod tests { crate::ValueOrRange::Value(payment_amt), ); - let mut rng = RngSend::new(None); + let mut rng = RngSend::new(None, 0); let (dest, dest_capacity) = generator.choose_destination(&mut rng, source.1).unwrap(); assert_eq!(node.pubkey, dest.pubkey); assert!(dest_capacity.is_none()); diff --git a/sim-lib/src/lib.rs b/sim-lib/src/lib.rs index 79953684..7aa101a5 100644 --- a/sim-lib/src/lib.rs +++ b/sim-lib/src/lib.rs @@ -462,10 +462,11 @@ pub struct RngSend(RngSendType); impl RngSend { /// Creates a new RngSend given an optional `u64` argument. If `seed_opt` is `Some`, random activity generation /// in the simulator occurs near-deterministically. If it is `None`, activity generation is truly random, and - /// based on a non-deterministic source of entropy. - pub fn new(seed_opt: Option) -> Self { + /// based on a non-deterministic source of entropy. We also pass in "salt" for each RngSend that is created so + /// that individual callers with the same seed can still produce unique RNGs. + pub fn new(seed_opt: Option, salt: u64) -> Self { if let Some(seed) = seed_opt { - RngSend(Box::new(ChaCha8Rng::seed_from_u64(seed))) + RngSend(Box::new(ChaCha8Rng::seed_from_u64(seed + salt))) } else { RngSend(Box::new(StdRng::from_entropy())) } @@ -1091,10 +1092,19 @@ async fn produce_events, listener: Listener, ) -> Result<(), SimulationError> { + // Pubkey is only exposed via to_string API, so we grab the last 15 digits to get an identifier for this + // node to feed into our RNG creation below. + let pk_str = source.pubkey.to_string(); + let pk = u64::from_str_radix(&pk_str[pk_str.len() - 15..], 16) + .map_err(|e| SimulationError::FixedSeedError(e.to_string()))?; + // We create one RNG per payment activity generator so that when we have a fixed seed, each generator will get the // same set of values across runs (with a shared RNG, the order in which tasks access the shared RNG would change - // the value that each generator gets). - let mut rng = RngSend::new(seed); + // the value that each generator gets). We "salt" the set seed with a value based on the node's public key so that + // each generator will start with a different seed, but it will be the same across runs. It is not critical is the + // value is the same for two nodes (and it's very unlikely). We can't use an index here because order of iteration + // through hashmaps is random. + let mut rng = RngSend::new(seed, pk); let mut current_count = 0; loop { @@ -1429,8 +1439,8 @@ mod tests { let seeds = vec![u64::MIN, u64::MAX]; for seed in seeds { - let mut_rng_1 = RngSend::new(Some(seed)); - let mut_rng_2 = RngSend::new(Some(seed)); + let mut_rng_1 = RngSend::new(Some(seed), 0); + let mut_rng_2 = RngSend::new(Some(seed), 0); let mut rng_1 = mut_rng_1.0; let mut rng_2 = mut_rng_2.0; @@ -1439,10 +1449,18 @@ mod tests { } } + #[test] + fn create_salted_rng() { + let mut rng_1 = RngSend::new(Some(1234), 0); + let mut rng_2 = RngSend::new(Some(1234), 1); + + assert!(rng_1.0.next_u64() != rng_2.0.next_u64()) + } + #[test] fn create_unseeded_mut_rng() { - let mut_rng_1 = RngSend::new(None); - let mut_rng_2 = RngSend::new(None); + let mut_rng_1 = RngSend::new(None, 0); + let mut_rng_2 = RngSend::new(None, 0); let mut rng_1 = mut_rng_1.0; let mut rng_2 = mut_rng_2.0; @@ -1481,7 +1499,7 @@ mod tests { .expect_next_payment_wait() .returning(move |_| Ok(payment_interval)); - let mut rng = RngSend::new(None); + let mut rng = RngSend::new(None, 0); assert_eq!( get_payment_delay(0, &node, &mock_generator, &mut rng).unwrap(), payment_interval @@ -1511,7 +1529,7 @@ mod tests { .expect_next_payment_wait() .returning(move |_| Ok(payment_interval)); - let mut rng = RngSend::new(None); + let mut rng = RngSend::new(None, 0); assert_eq!( get_payment_delay(0, &node, &mock_generator, &mut rng).unwrap(), start_delay diff --git a/sim-lib/src/random_activity.rs b/sim-lib/src/random_activity.rs index 8112ca80..c3f44b74 100644 --- a/sim-lib/src/random_activity.rs +++ b/sim-lib/src/random_activity.rs @@ -343,7 +343,7 @@ mod tests { let view = NetworkGraphView::new(nodes).unwrap(); for _ in 0..10 { - view.choose_destination(&mut RngSend::new(None), big_node) + view.choose_destination(&mut RngSend::new(None, 0), big_node) .unwrap(); } } @@ -411,7 +411,7 @@ mod tests { // All of them will yield a sigma squared smaller than 0, which we have a sanity check for. let expected_payment = get_random_int(1, 100); let source_capacity = 2 * expected_payment; - let mut rng = RngSend::new(Some(u64::MAX)); + let mut rng = RngSend::new(Some(u64::MAX), 0); let pag = RandomPaymentActivity::new(source_capacity, expected_payment, 1.0).unwrap(); // Wrong cases