Skip to content

Conversation

carlaKC
Copy link
Contributor

@carlaKC carlaKC commented Aug 18, 2025

Pulling this out of #301 to speed up the tests so we don't have to wait ~1min for each run, this change gets full test suite back down to ~5s.

@carlaKC carlaKC requested review from f3r10 and removed request for f3r10 August 18, 2025 19:27
.unwrap()
.iter()
.cloned()
.take(20)
Copy link
Collaborator

Choose a reason for hiding this comment

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

it looks like only 19 payments are generated. And the correct expected payment is
let expected_payment_list = vec![ pk1, pk2, pk1, pk1, pk1, pk3, pk3, pk3, pk4, pk3, pk2, pk1, pk4, pk2, pk2, pk2, pk2, pk1, pk3, ];

Speed up this test by using our sim clock that can speed up the
simulation. The downside of this clock is that we may stop one payment
over/under if we try to match exactly to the number of payments that
we expect in a given period of time.

Instead, we generate our set of expected payment and then run the
simulation for much longer than we need to get this expected list, so
we should always get at least this subset. We then just assert that the
payments we get exactly match this first set.
@carlaKC
Copy link
Contributor Author

carlaKC commented Aug 22, 2025

I think that we have a race here - this passes on my machine and is failing here.

Pushed a change to make this 12 payments, so that we definitely hit the target count - diff was trivial so didn't use a fixup.

DId a quick flake test here with:
for i in {1..100}; do echo "Test run $i"; cargo test test_deterministic_payments_events_random || break; done

And I run into an issue where the second last (index 10) payment is either pk2 or pk3. The only test change that I can see breaking this is the clock speedup, which makes me think we may have a rounding issue somewhere in the determinism change that needs tie-breaking?

@f3r10
Copy link
Collaborator

f3r10 commented Sep 3, 2025

I found the issue.
The problem is with the rounding of the wait_time.
There are times (after 1000 times, probably) that instead of getting 1.543701036s, the wait_time is 1.495485804s
Rounding 1.543701036s is 2, and rounding 1.495485804s is 1. Becuase of this reason the determined order fails.

I think one possible solution would be to round to the first decimal place.

@f3r10
Copy link
Collaborator

f3r10 commented Sep 4, 2025

the solution that I found was to round the wait_time to one decimal.

fn round_duration_to_one_decimal(duration: Duration) -> f64 {
    let millis = duration.as_millis();
    let rounded_millis = (millis + 50) / 100 * 100;
    rounded_millis as f64 / 1000.0
}
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants