Skip to content

Commit

Permalink
sim-lib: only use start delay if explicitly specified
Browse files Browse the repository at this point in the history
As is, we'll run with a 0 second start delay for:
- Random activities
- Defined activities with no `start_secs` specified

This very likely isn't the intent for defined activities (people should
set `start_secs=0` if they want their payments to fire immediately),
and doesn't work for random activity because we just fire all payments
on start.

Instead, we switch over to using our payment wait as the start delay in
the absence of this field being set for defined activities and always
for random activities.
  • Loading branch information
carlaKC committed Jul 8, 2024
1 parent e369d61 commit 3e63dd3
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 28 deletions.
9 changes: 4 additions & 5 deletions sim-lib/src/defined_activity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use tokio::time::Duration;
#[derive(Clone)]
pub struct DefinedPaymentActivity {
destination: NodeInfo,
start: Duration,
start: Option<Duration>,
count: Option<u64>,
wait: ValueOrRange<u16>,
amount: ValueOrRange<u64>,
Expand All @@ -16,7 +16,7 @@ pub struct DefinedPaymentActivity {
impl DefinedPaymentActivity {
pub fn new(
destination: NodeInfo,
start: Duration,
start: Option<Duration>,
count: Option<u64>,
wait: ValueOrRange<u16>,
amount: ValueOrRange<u64>,
Expand Down Expand Up @@ -48,7 +48,7 @@ impl DestinationGenerator for DefinedPaymentActivity {
}

impl PaymentGenerator for DefinedPaymentActivity {
fn payment_start(&self) -> Duration {
fn payment_start(&self) -> Option<Duration> {
self.start
}

Expand Down Expand Up @@ -77,7 +77,6 @@ impl PaymentGenerator for DefinedPaymentActivity {
#[cfg(test)]
mod tests {
use super::DefinedPaymentActivity;
use super::*;
use crate::test_utils::{create_nodes, get_random_keypair};
use crate::{DestinationGenerator, PaymentGenerationError, PaymentGenerator};

Expand All @@ -91,7 +90,7 @@ mod tests {

let generator = DefinedPaymentActivity::new(
node.clone(),
Duration::from_secs(0),
None,
None,
crate::ValueOrRange::Value(60),
crate::ValueOrRange::Value(payment_amt),
Expand Down
129 changes: 108 additions & 21 deletions sim-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ pub struct ActivityParser {
#[serde(with = "serializers::serde_node_id")]
pub destination: NodeId,
/// The time in the simulation to start the payment.
#[serde(default)]
pub start_secs: u16,
pub start_secs: Option<u16>,
/// The number of payments to send over the course of the simulation.
#[serde(default)]
pub count: Option<u64>,
Expand All @@ -204,7 +203,7 @@ pub struct ActivityDefinition {
/// The destination of the payment.
pub destination: NodeInfo,
/// The time in the simulation to start the payment.
pub start_secs: u16,
pub start_secs: Option<u16>,
/// The number of payments to send over the course of the simulation.
pub count: Option<u64>,
/// The interval of the event, as in every how many seconds the payment is performed.
Expand Down Expand Up @@ -316,7 +315,7 @@ pub struct PaymentGenerationError(String);

pub trait PaymentGenerator: Display + Send {
/// Returns the time that the payments should start
fn payment_start(&self) -> Duration;
fn payment_start(&self) -> Option<Duration>;

/// Returns the number of payments that should be made
fn payment_count(&self) -> Option<u64>;
Expand Down Expand Up @@ -773,7 +772,9 @@ impl Simulation {
for description in self.activity.iter() {
let activity_generator = DefinedPaymentActivity::new(
description.destination.clone(),
Duration::from_secs(description.start_secs.into()),
description
.start_secs
.map(|start| Duration::from_secs(start.into())),
description.count,
description.interval_secs,
description.amount_msat,
Expand Down Expand Up @@ -1037,22 +1038,8 @@ async fn produce_events<N: DestinationGenerator + ?Sized, A: PaymentGenerator +
}
}

let wait: Duration = if current_count == 0 {
let start = node_generator.payment_start();
if start != Duration::from_secs(0) {
log::debug!(
"Using a start delay. The first payment for {source} will be at {:?}.",
start
);
}
start
} else {
log::debug!(
"Next payment for {source} in {:?}.",
node_generator.next_payment_wait()
);
node_generator.next_payment_wait()
};
// On our first run, add a one-time start delay if set for our payment.
let wait = get_payment_delay(current_count, &source, node_generator.as_ref());

select! {
biased;
Expand Down Expand Up @@ -1094,6 +1081,30 @@ async fn produce_events<N: DestinationGenerator + ?Sized, A: PaymentGenerator +
}
}

/// Gets the wait time for the next payment. If this is the first payment being generated, and a specific start delay
/// was set we return a once-off delay. Otherwise, the interval between payments is returned.
fn get_payment_delay<A: PaymentGenerator + ?Sized>(
call_count: u64,
source: &NodeInfo,
node_generator: &A,
) -> Duration {
if call_count != 0 {
let wait = node_generator.next_payment_wait();
log::debug!("Next payment for {source} in {:?}.", wait,);
wait
} else if let Some(start) = node_generator.payment_start() {
log::debug!(
"First payment for {source} will be after a start delay of {:?}.",
start
);
start
} else {
let wait = node_generator.next_payment_wait();
log::debug!("First payment for {source} in {:?}.", wait);
wait
}
}

async fn consume_simulation_results(
logger: Arc<Mutex<PaymentResultLogger>>,
mut receiver: Receiver<(Payment, PaymentResult)>,
Expand Down Expand Up @@ -1327,3 +1338,79 @@ async fn track_payment_result(

Ok(())
}

#[cfg(test)]
mod tests {
use mockall::mock;

use crate::{get_payment_delay, test_utils, PaymentGenerationError, PaymentGenerator};
use std::fmt;
use std::time::Duration;

mock! {
pub Generator {}

impl fmt::Display for Generator {
fn fmt<'a>(&self, f: &mut fmt::Formatter<'a>) -> fmt::Result;
}

impl PaymentGenerator for Generator {
fn payment_start(&self) -> Option<Duration>;
fn payment_count(&self) -> Option<u64>;
fn next_payment_wait(&self) -> Duration;
fn payment_amount(&self, destination_capacity: Option<u64>) -> Result<u64, PaymentGenerationError>;
}
}

#[test]
fn test_no_payment_delay() {
let node = test_utils::create_nodes(1, 100_000)
.first()
.unwrap()
.0
.clone();

// Setup mocked generator to have no start time and send payments every 5 seconds.
let mut mock_generator = MockGenerator::new();
mock_generator.expect_payment_start().return_once(|| None);
let payment_interval = Duration::from_secs(5);
mock_generator
.expect_next_payment_wait()
.returning(move || payment_interval);

assert_eq!(
get_payment_delay(0, &node, &mock_generator),
payment_interval
);
assert_eq!(
get_payment_delay(1, &node, &mock_generator),
payment_interval
);
}

#[test]
fn test_payment_delay() {
let node = test_utils::create_nodes(1, 100_000)
.first()
.unwrap()
.0
.clone();

// Setup mocked generator to have a start delay and payment interval with different values.
let mut mock_generator = MockGenerator::new();
let start_delay = Duration::from_secs(10);
mock_generator
.expect_payment_start()
.return_once(move || Some(start_delay));
let payment_interval = Duration::from_secs(5);
mock_generator
.expect_next_payment_wait()
.returning(move || payment_interval);

assert_eq!(get_payment_delay(0, &node, &mock_generator), start_delay);
assert_eq!(
get_payment_delay(1, &node, &mock_generator),
payment_interval
);
}
}
4 changes: 2 additions & 2 deletions sim-lib/src/random_activity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ fn events_per_month(source_capacity_msat: u64, multiplier: f64, expected_payment

impl PaymentGenerator for RandomPaymentActivity {
/// Returns the time that the payments should start. This will always be 0 for the RandomPaymentActivity type.
fn payment_start(&self) -> Duration {
Duration::from_secs(0)
fn payment_start(&self) -> Option<Duration> {
None
}

/// Returns the number of payments that should be made. This will always be None for the RandomPaymentActivity type.
Expand Down

0 comments on commit 3e63dd3

Please sign in to comment.