Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Testutils is nondeterministic #810

Closed
graydon opened this issue May 16, 2023 · 3 comments
Closed

Testutils is nondeterministic #810

graydon opened this issue May 16, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@graydon
Copy link
Contributor

graydon commented May 16, 2023

There are some testutils methods that access thread_rng and are therefore nondeterministic. In general in core we prefer all tests to have fairly rigid determinism and, if they want "quasi nondeterminism", to use a special-purpose seed that's slow-changing (eg. once a day) to make debugging sessions even on that quasi-nondeterminism stable.

But in general most of the paths I'm seeing under testutils would be fine with a constant-seeded PRNG, so I would generally prefer that.
@graydon graydon added the bug Something isn't working label May 16, 2023
@graydon graydon mentioned this issue May 19, 2023
Merged
@graydon
Copy link
Contributor Author

graydon commented Jun 12, 2023

(Might be worth checking if these cases can use the newly-added host PRNG)

@anupsdf
Copy link
Contributor

anupsdf commented Jun 22, 2023

Setting target milestone to Post testnet after discussing with @graydon

graydon added a commit that referenced this issue Oct 20, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
fe8fd71
@graydon graydon mentioned this issue Oct 20, 2023
Merged
graydon added a commit that referenced this issue Oct 20, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
9ef86c7
graydon added a commit that referenced this issue Oct 20, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
341d27b
graydon added a commit that referenced this issue Oct 20, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
c0fc57d
graydon added a commit that referenced this issue Oct 20, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
9e06a8d
graydon added a commit that referenced this issue Oct 21, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
d1b2a26
graydon added a commit that referenced this issue Oct 21, 2023
@graydon
Eliminate uses of HashMap and thread_rng (fix #810)
98edfa3
github-merge-queue bot pushed a commit that referenced this issue Oct 23, 2023
@graydon
Cackle gate (#1124)
62da7a1 
This adds a transitive-API-usage-checker called
[`cackle`](https://davidlattimore.github.io/making-supply-chain-attacks-harder.html)
to the soroban build. It is currently (with a few exceptions) wired to
prohibit most uses of the filesystem, network, time, the environment,
threading, random numbers or hashtables. It's configured by cackle.toml
and there's a new Makefile and github action target for it.

I also took the opportunity to eliminate all uses of HashMap and
thread_rng (see #810).

Thanks to @SirTyson for suggesting this tool! It was a little tricky to
get working right but once working it seems good.
@brson brson mentioned this issue Oct 24, 2023
Merged
github-merge-queue bot pushed a commit to stellar/rs-soroban-sdk that referenced this issue Oct 24, 2023
@brson @leighmcculloch
Use deterministic randomness in arbitrary tests (#1120)
9850278 
### What

Use a seeded StdRng instead of thread_rng in arbitrary tests.

### Why

Per stellar/rs-soroban-env#810 randomized test
cases should be deterministic.

### Known limitations

This is different from the approach in soroban-env-host from
stellar/rs-soroban-env#1124, where a test prng
is attached to the env / host.

There are two more uses of thread_rng in this crate, but I don't know
enough about that code to know how they should be removed, i.e. if they
should follow a pattern similar to soroban-env-host. I am willing to
replace all uses of thread_rng if given guidance about the preferred way
to do it.

The implementation of StdRng is allowed to change in the future, so it's
possible the exact numbers generated could change. I could import
rand_chacha instead if desired.

Co-authored-by: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com>
@graydon
Copy link
Contributor Author

graydon commented Oct 25, 2023

This was ultimately fixed in #1124

@graydon graydon closed this as completed Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@graydon @anupsdf