-
Notifications
You must be signed in to change notification settings - Fork 342
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
feat: add boundLog to enable log-uniform sampling #584
Conversation
For large ranges I agree logarithmic distribution is preferable
This is a pretty big weakness, as it means you are purely fuzzing with random values and not using any of the foundry strategies, such as edge biasing or the dictionary. Purely random fuzzing is the least valuable strategy. Also, is this actually the ideal solution, compared to updating random number generation in foundry to support this? If there is a way we can improve this upstream that is typically preferable to avoid forge-std bloat, so just want to make sure we think through the solution space |
cc @grandizzy @klkvr for thoughts |
@mds1 I think it makes sense to have such directly in foundry and have strategies generate values in bounded range. What I can see here as a potential issue is that we will need config to specify limits per fuzzed param so could lead to more complex config, e.g. for a fuzzed test like uint256[] public fixtureAmount = [5 * 1e18, 15 * 1e18, 25 * 1e18, 35 * 1e18];
uint8[2] public boundAmount = [5, 10]; (ref https://book.getfoundry.sh/forge/fuzz-testing#fuzz-test-fixtures) |
Yea the UX for this was always the trickiest, in the past we also considered just using the inline config which could be a good approach: /// forge-config: default.fuzz.range.x = [5, type(uint256).max];
/// forge-config: default.fuzz.distribution.x = logarithmic;
/// forge-config: default.fuzz.range.y = [1, 10];
/// forge-config: default.fuzz.distribution.x = uniform;
function testFoo(uint256 x, uint256 y) public {
// --- snip ---
} How would you specify distribution type with fixtures? Though, I am hesitant to expose distribution |
Yeah, wasn't thinking to use fixtures for distribution but maybe having a similar way to current fixtures for all configuration of named parameter in Solidity struct Uint256ParamConfig {
uint256 min;
uint256 max;
uint256[] fixtures;
uint256[] excluded;
}
Uint256ParamConfig public configAmount; (basically including fixtures in same config and also superseeding assume and bound cheatcodes per named param). Though probably inline config is better UX here considering that we need specific struct per type...
Yep, agree, maybe we should start an issue in foundry to track / implement these? |
That sounds good to me, would you mind creating that? :) |
Hey I am working on this issue and trying to solve it using the struct
and I am fetching the from another function that is checking the unit type of the value. Is it important to keep the exact same struct to solve the issue? |
Closing in favor of foundry-rs/foundry#9154 |
Using
bound
to generate random inputs is heavily skewed towards the upper end of the range [see below]. To explore a more balanced distribution of values, users should have the option to sample in log space instead.This PR adds
boundLog(uint256 x, uint8 min, uint8 max)
, which generates a random number between [2**min
,2**(max+1)-1
] that is uniformly distributed in log space (ie. data generated follows a log-uniform distribution)Note: contrarily to
bound
,boundLog
WILL NOT returnx
when the inputx
is within the provided [2**min
,2**(max+1)-1
] bounds.bound
vsboundLog
sampling between1
and2^32
,N=10**6