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

Limit fuzz runs to possible inputs #1901

Closed
2 tasks done
0xmikko opened this issue Jun 10, 2022 · 6 comments
Closed
2 tasks done

Limit fuzz runs to possible inputs #1901

0xmikko opened this issue Jun 10, 2022 · 6 comments
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test P-low Priority: low T-bug Type: bug

Comments

@0xmikko
Copy link

0xmikko commented Jun 10, 2022

Component

Forge

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

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (6130af7 2022-04-07T00:08:44.543037+00:00)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

The problem is connected with fuzzing testing.
We use bool parameter to cover possible test. So, some of our tests looks so:

function test_ACV1_2_05_remove_liquidity_works_as_expected(bool multicall)
        public;

So, as you can see there are only 2 options to check: true or false, btw, it passes through 256 test or the quantity is set in env var.

Please, check the max possible options to limit fuzzing.

@0xmikko 0xmikko added the T-bug Type: bug label Jun 10, 2022
@onbjerg onbjerg added this to Foundry Jun 10, 2022
@onbjerg onbjerg moved this to Todo in Foundry Jun 10, 2022
@onbjerg onbjerg changed the title Fuzzing number Limit fuzz runs to possible inputs Jun 10, 2022
@onbjerg onbjerg added A-testing Area: testing Cmd-forge-test Command: forge test C-forge Command: forge P-low Priority: low labels Jun 10, 2022
@mattsse
Copy link
Member

mattsse commented Jun 10, 2022

you're of course right

it should be possible to implement full coverage for arguments that allow it

@mds1
Copy link
Collaborator

mds1 commented Jun 10, 2022

it should be possible to implement full coverage for arguments that allow it

This is true in the case of bool here, as well as other cases like a single uint8 or with two bool inputs. But this doesn't scale well because you'd basically need to hardcode a set of signatures for which foundry says "don't use regular fuzz rules, instead enumerate all inputs if the user's number of fuzz runs allow it".

Ideas for approaches that might scale better:

  • Use a table test format: Table tests #858
  • Use a library like solidity-generators to generate your test arrays and loop over it as a concrete test instead of a fuzz test
  • Architect your tests differently, such as:
// This is the actual test foundry executes.
function test_ACV1_2_05_remove_liquidity_works_as_expected() public {
  _test_ACV1_2_05_remove_liquidity_works_as_expected(true);
  _test_ACV1_2_05_remove_liquidity_works_as_expected(false);
}

// Actual test logic and assertions here.
function _test_ACV1_2_05_remove_liquidity_works_as_expected(bool) internal {
    // --- snip ---
}

@OliverNChalk
Copy link
Contributor

Would it not be possible to check the number of possible combinations basis the byte-size of the function arguments? E.g. if you have a signature of:

func(bool,bool)

you have 2 bits of possible inputs, ergo 4 cases. Similarly, if you have:

func(uint8,uint8)

you have 16 bits = 65536 possible combinations. So in this case, 256 fuzz runs can only cover 8 bits of input information. Assuming most people won't fuzz more than 10_000 times in their CI runs, it would only be possible to exhaustively cover ~13 bits of input. The question would then be is it worth implementing a secondary strategy that generally won't cover more than 13-16 bits of input.

@onbjerg
Copy link
Member

onbjerg commented Jun 29, 2022

We just don't have a way currently to limit the fuzz runs based on input size. As to feasibility with our current setup I couldn't tell, we'd need to check the proptest docs :) But yes, the general idea you outlined applies

@onbjerg
Copy link
Member

onbjerg commented Aug 11, 2022

I've investigated more and it is not possible to do with proptest. We would essentially need to manually calculate the input space and forego using proptest all together if the number of possible inputs is less than the number of fuzz runs and just do a loop instead.

@mds1
Copy link
Collaborator

mds1 commented Feb 26, 2023

Going to close this since it would be difficult to implement with the current fuzz architecture, afaik it's not a feature of most other fuzzers, and there are workarounds described above to get similar behavior

@mds1 mds1 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 26, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Foundry Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test P-low Priority: low T-bug Type: bug
Projects
Archived in project
Development

No branches or pull requests

5 participants