Navigation Menu

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.

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

Implement the Arbitrary fuzzing trait for contract value-types #715

Merged
merged 4 commits into from Mar 23, 2023

Conversation

brson
Copy link
Contributor

@brson brson commented Mar 2, 2023

What

Implement the Arbitrary trait for Symbol, BitSet, Static, and Status.

Why

This is a prerequisite for more complete fuzzing support in soroban-sdk: stellar/rs-soroban-sdk#878
This patch is only needed if that patch is to be accepted.

This patch also tweaks and reexports the call_with_suppressed_panic_hook function,
which is used by the sdk patch to provide users a helper function for catching panics in fuzz tests.

A more complete description is in the above link.

Known limitations

See sdk PR.

@brson
Copy link
Contributor Author

brson commented Mar 7, 2023

Rebased.

@dmkozh
Copy link
Contributor

dmkozh commented Mar 7, 2023

Just a heads up: we are in the middle of a significant env/sdk rewrite that is definitely going to affect this and SDK PRs to some degree. Due to that I would be inclined to wait until the rewrite is landed (should happen in a few days). Thanks for the work and sorry for slow review.

@brson brson force-pushed the arbitrary branch 2 times, most recently from 05704f4 to 71f73d7 Compare March 16, 2023 19:30
@brson
Copy link
Contributor Author

brson commented Mar 16, 2023

Rebased.

soroban-env-common/src/arbitrary.rs Outdated Show resolved Hide resolved
soroban-env-common/src/arbitrary.rs Outdated Show resolved Hide resolved
@brson
Copy link
Contributor Author

brson commented Mar 18, 2023

Is the Status types still exposed by the sdk? If not this patch is no longer needed at all.

@brson
Copy link
Contributor Author

brson commented Mar 19, 2023

I removed the implementation of Arbitrary for SymbolSmall. The only Arbitrary implementation left now in this crate is for Status. I'm not clear on whether contract authors can still access this type or not. It may be that none of the non-object contract types need to implement Arbitrary.

@dmkozh
Copy link
Contributor

dmkozh commented Mar 20, 2023

Is the Status types still exposed by the sdk?

Yeah, it's still exposed, so probably we still need this.

I removed the implementation of Arbitrary for SymbolSmall. The only Arbitrary implementation left now in this crate is for Status. I'm not clear on whether contract authors can still access this type or not. It may be that none of the non-object contract types need to implement Arbitrary.

SymbolSmall is accessible via soroban-env-common, but it isn't too useful as SDK exposes a higher level Symbol type that can be either small or object-based symbol, so I think it would make more sense to implement Arbitrary for the SDK Symbol instead.

@dmkozh
Copy link
Contributor

dmkozh commented Mar 21, 2023

Let's wait a bit with merging this to get it into the next release (the current release happens right now and there is also the SDK change which we won't have time to merge into it)

@dmkozh dmkozh merged commit 3c4963e into stellar:main Mar 23, 2023
6 checks passed
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.

None yet

2 participants