foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.3k stars 1.75k forks source link

Using enum as paramter in fuzz test fails #871

Closed david-k closed 2 years ago

david-k commented 2 years ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.1.0 (39b6e39 2022-03-08T00:05:45.429338818+00:00)

What command(s) is the bug in?

No response

Operating System

Linux

Describe the bug

When using an enum as a parameter in a fuzz test the test will very likely fail because the provided value is invalid for the enum:

contract MyTest is DSTest {
    enum Enum { A, B }

    // Always fails
    function test_enum(Enum e) public {

    }
}

My understanding is that this happens because enums are represented as unit8 in the EVM, and when fuzzing we naturally want to sample values from the full range of uint8. However, in the case of enums this causes the test to immediately fail.

I don't know if it is possible to somehow provide additional information to the test runner so that it knows that only a subset of values should be considered.

You can of course easily work around this issue, but being able to use enums would still be nice.

gakonst commented 2 years ago

Ah, this seems hard to get around. The best way to do this would be:

function test_enum(uint8 _e) public {
    vm.assume(_e < 2);
    Enum e = Enum(_e);
}

I don't think we can solve this in some other way, unless the compiler gives us information about how many enum variants there are? cc @mattsse

onbjerg commented 2 years ago

Read over the Solidity docs and it does not seem like the JSON output has any option to return info on enums

david-k commented 2 years ago

Yeah, I figured that this isn't easy to do. Fortunately, one can easily work around this with some modulo operations or vm.assume(). (Thanks btw for mentioning vm.assume(), didn't know about that one!)

mattsse commented 2 years ago

Ah, this seems hard to get around. The best way to do this would be:

function test_enum(uint8 _e) public {
    vm.assume(_e < 2);
    Enum e = Enum(_e);
}

I don't think we can solve this in some other way, unless the compiler gives us information about how many enum variants there are? cc @mattsse

This is not included in the output and only limited to "internalType": "enum MyTest.Enum",

the module solution is pretty neat though.

vote close.

0xalpharush commented 2 years ago

I'd appreciate reconsidering how the fuzzer treats enums as it requires a lot of boilerplate to craft more complex types e.g. structs and cast unsigned integers to an enum type.

If that's not realistic, it may be worth displaying suggestions to users when an enum is in a function signature and it's reverting. It wasn't readily apparent that the failure is due to invalid calldata.

onbjerg commented 2 years ago

We need solc to give us more information on enum types so we can generate inputs that are not outside of the bounds of the number of variants of the enum

gakonst commented 2 years ago

@0xalpharush we agree that this would be nice, as Bjerg writes above, per our last investigation, we don't have that information to provide more context to the user.. If you have any suggestions on how to do that, we'd obviously be very happy to include, as this is indeed an important shortcoming (cc @mds1 per previous convos around the fuzzer)

0xalpharush commented 2 years ago

I'm not sure if Foundry has access to the panic code. For enum conversions that fail, the panic code is 0x21. Would it be possible to brute force a valid size by using inputs ranging from 1 to 256 (the max enum size) and cap the upper bound when a 0x21 panic is hit? Alternatively, this can be outsourced to a static analysis tool like slither and Foundry can read that max enum size.

gakonst commented 2 years ago

This makes sense - I think we have access to the panic code given we handle it in Forge-std. Will investigate, PRs welcome of course

hrkrshnn commented 2 years ago

@onbjerg where do you want the extra information? In the --abi or --ast-compact-json? Can you open an issue in the solc repo?


As a side note, type(Enum).max works. So something like vm.assume(rand <= uint8(type(Enum).max)) is probably the best right now.

frangio commented 2 years ago

Alternatively, this can be outsourced to a static analysis tool like slither and Foundry can read that max enum size.

This isn't necessary if Foundry has access to the AST. But I'd say that enum information should be included in the ABI.

onbjerg commented 2 years ago

We don't process the AST in forge test at all and I'd be hesitant to do so currently since it's not documented anywhere as far as I can tell, so implementing e.g. https://github.com/gakonst/ethers-rs/pull/1567 was a major pain (and is likely to break at some point). vm.assume is OK for now but having the number of variants for enums in the ABI would be nice (like we have struct descriptions)

frangio commented 2 years ago

I maintain documentation for the AST at https://solidity-ast.netlify.app, but dealing with different Solidity versions is difficult so to keep forge test AST agnostic is probably a good idea.

JacobHomanics commented 7 months ago

Revisiting this issue as I encounter issues with my code when trying to write fuzz tests wherein a struct containing an enum throws an EvmError when the function runs.

Screenshot 2024-03-27 at 5 12 38 PM Screenshot 2024-03-27 at 5 13 23 PM
0xmoebius commented 3 months ago

Revisiting this issue as I encounter issues with my code when trying to write fuzz tests wherein a struct containing an enum throws an EvmError when the function runs.

Screenshot 2024-03-27 at 5 12 38 PM Screenshot 2024-03-27 at 5 13 23 PM

having the same issue. can't fuzz a struct w/ an enum inside because the conversion fails with any fuzzed value bigger than enum.max