fix: do not flood dictionary with data dependent on fuzz inputs

[klkvr]

Motivation

Currently data collected during individual fuzz/invariant run (e.g. logs, storage writes) persists in fuzz dictionary resulting in large dictionary sizes due to it being flooded by data dependent on fuzz inputs. In #7551 case it resulted in dictionary accumulating log topics which directly contained fuzz inputs thus decreasing a chance of picking actually useful value from push byte as a fuzz input.

Solution

For fuzz tests we now don't call collect_state_from_call at all making calldata choice completely stateless.

For invariant tests I've refactored EvmFuzzState to keep track of all newly added keys and an option to remove them via revert fn. Thus, on invariant runs we accumulate and persist dictionary between separate calls, but it is getting cleared after each invariant sequence run.

Setters now also don't let dictionary size to grow beyond configured limit which didn't really work before

Results

This does not directly resolv #7551 because in this exact case fuzz-test has a structure with many branches and a chance to reach a branch which should've caused failure is relatively low because most of the runs are getting caught earlier.

For the sake of experiment let's remove some of the branches to increase chance of hitting this exact failure:

function test_initialize(PoolKey memory key0, uint160 sqrtPriceX96) public {
    vm.assume(key0.currency0 < key0.currency1);
    vm.assume(!key0.fee.isDynamicFee());
    key0.tickSpacing = int24(bound(key0.tickSpacing, manager.MIN_TICK_SPACING(), manager.MAX_TICK_SPACING()));

    // Assumptions tested in Pool.t.sol
    sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_RATIO, TickMath.MAX_SQRT_RATIO - 1));

    // tested in Hooks.t.sol
    key0.hooks = IHooks(Constants.ADDRESS_ZERO);

    if (
        (key0.fee & SwapFeeLibrary.DYNAMIC_FEE_FLAG == 0) && (key0.fee & SwapFeeLibrary.STATIC_FEE_MASK >= 1000000)
    ) {
        vm.expectRevert(abi.encodeWithSelector(SwapFeeLibrary.FeeTooLarge.selector));
        manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);
    } else {
        vm.expectEmit(true, true, true, true);
        emit Initialize(key0.toId(), key0.currency0, key0.currency1, key0.fee, key0.tickSpacing, key0.hooks);
        manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);

        (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId());
        assertEq(slot0.sqrtPriceX96, sqrtPriceX96);
        assertEq(slot0.protocolFee, 0);
    }
}

On latest nightly, forge is not able to catch a revert after 100.000 runs.
On this PR's version, revert is catched on 9261 run.

This benchmark is dependent on exact test and seed and this is kind of an edge case as this test suite results in 7000 push bytes being pre-collected after setUp, but I believe that cleaning up fuzz dictionary is reasonable anyway.

[grandizzy]

this rang a bell re #7462 which I cannot reproduce with this PR anymore (reproducible with master and UniStaker repo by reverting commit uniswapfoundation/UniStaker@1573451 that workarounds this issue)

[grandizzy]

I used PoC #3607 (comment) to collect and visualize some stats on test_initialize (revert caught locally after 25098 runs), it can be seen that with PR there are less unique values (as dictionary not flooded) and number of times a fuzzed value is picked better distributed than without PR, attaching here, hope it makes sense

Without PR

With PR

[Evalir]

hey @grandizzy thanks! an aside: how are you getting these metrics? I think you dropped the info in another issue or PR, but we should document further

[grandizzy]

hey @grandizzy thanks! an aside: how are you getting these metrics? I think you dropped the info in another issue or PR, but we should document further

hey @Evalir , tagged you in the metrics issue #3607 (comment)

[mattsse]

all of this makes sense to me, but would appreciate another look @DaniPopes

[mattsse]

great, one less type alias 🙏

[DaniPopes]

lgtm

[Evalir]

gg