Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add fuzzing support with docs; RawVal comparison proptests (#957)
### What Preliminary support for fuzzing Soroban contracts with [cargo-fuzz](https://github.com/rust-fuzz/cargo-fuzz/). This patch is primarily concerned with introducing a pattern for implementing the [Arbitrary](https://docs.rs/arbitrary/latest/arbitrary/) trait, by which fuzz tests generate semi-random Rust values. This is a new revision of previous pr #878. Little has changed functionally since that pr. This patch additionally uses the Arbitrary impls with [`proptest-arbitrary-interop` crate](https://github.com/graydon/proptest-arbitrary-interop) to add proptests that help ensure that: RawVals and ScVals can be converted between each other and their their comparison functions are equivalent, which closes stellar/rs-soroban-env#743. This patch introduces the SorobanArbitrary trait which looks like this: ```rust pub trait SorobanArbitrary: TryFromVal<Env, Self::Prototype> + IntoVal<Env, RawVal> + TryFromVal<Env, RawVal> { type Prototype: for<'a> Arbitrary<'a>; } ``` Basically every type relevant to soroban contracts implements (or should) this trait, including i32, u32, i64, u64, i128, u128, (), bool, I256Val, U256Val, Error, Bytes, BytesN, Vec, Map, Address, Symbol, TimepointVal, DurationVal. The `#[contracttype]` macro automatically derives an implementation, along with a type that implements `SorobanArbitrary::Prototype`. In use the trait looks like ```rust use soroban_sdk::{Address, Env, Vec}; use soroban_sdk::contracttype; use soroban_sdk::arbitrary::{Arbitrary, SorobanArbitrary}; use std::vec::Vec as RustVec; #[derive(Arbitrary, Debug)] struct TestInput { deposit_amount: i128, claim_address: <Address as SorobanArbitrary>::Prototype, time_bound: <TimeBound as SorobanArbitrary>::Prototype, } #[contracttype] pub struct TimeBound { pub kind: TimeBoundKind, pub timestamp: u64, } #[contracttype] pub enum TimeBoundKind { Before, After, } fuzz_target!(|input: TestInput| { let env = Env::default(); let claim_address: Address = input.claim_address.into_val(&env); let time_bound: TimeBound = input.time_bound.into_val(&env). // fuzz the program based on the input }); ``` A more complete example is at https://github.com/brson/rs-soroban-sdk/blob/val-fuzz/soroban-sdk/fuzz/fuzz_targets/fuzz_rawval_cmp.rs ### Why This patch assumes it is desirable to fuzz Soroban contracts with cargo-fuzz. Soroban reference types can only be constructed with an `Env`, but the `Arbitrary` trait constructs values from nothing but bytes. The `SorobanArbitrary` trait provides a pattern to bridge this gap, expecting fuzz tests to construct `SorobanArbitrary::Prototype` types, and then convert them to their final type with `FromVal`/`IntoVal`. There are a lot of docs here and hopefully they explain what's going on well enough. ### fuzz_catch_panic This patch also introduces a helper function, `fuzz_catch_panic`, which is built off of the `call_with_suppressed_panic_hook` function in soroban-env-host. The `fuzz_target!` macro overrides the Rust panic hook to abort on panic, assuming all panics are bugs, but Soroban contracts fail by panicking, and I have found it desirable to fuzz failing contracts. `fuzz_catch_panic` temporarily prevents the fuzzer from aborting on panic. ### Known limitations The introduction of SorobanArbitrary requires a bunch of extra documentation to explain why Soroban contracts can't just use the stock Arbitrary trait. As an alternative to this approach, we could instead expect users to construct XDR types, not SorobanArbitrary::Prototype types, and convert those to RawVals. I have been assuming that contract authors should rather concern themselves with contract types, and not the serialization types; and presently there are a number of XDR types that have values which can't be converted to contract types. The SorobanArbitrary::Prototype approach does allow more flexibility in the generation of contract types, e.g. we can generate some adversarial types like invalid object references and bad tags. Contracts must use `IntoVal` to create the final types, but these traits are under-constrained for this purpose and always require type hints: ```rust fuzz_target!(|input: <Address as SorobanArbitrary>::Prototype| { let env = Env::default(); let address: Address = input.into_val(&env); // fuzz the program based on the input }); ``` This is quite unfortunate because it means the real type must be named twice. This patch introduces a new private module `arbitrary_extra` which simply defines a bunch of new conversions like ```rust impl TryFromVal<Env, u32> for u32 { type Error = ConversionError; fn try_from_val(_env: &Env, v: &u32) -> Result<Self, Self::Error> { Ok(*v) } } ``` These conversions are required by `SorobanArbitrary`, which is only defined when `cfg(feature = "testutils")`; the `arbitrary_extra` module defines these conversions for all cfgs to ensure type inference is always the same, but the impls are probably useless outside of the SorobanArbitrary prototype pattern. Crates that use `#[contracttype]` need to define a "testutils" feature if they want to use Arbitrary. This patch doesn't generate "adversarial" values, which might include: - RawVals with bad tags - objects that have invalid references - objects that are reused multiple times - deeply nested vecs and maps - vecs and maps that contain heterogenous element types The arbitrary module has unit tests, and these help especially ensure the macros for custom types are correct, but the tests are not exhaustive. --------- Co-authored-by: Leigh McCulloch <[email protected]> Co-authored-by: Graydon Hoare <[email protected]>
- Loading branch information