chore(invariant): remove persist_state config, commit by default

[grandizzy]

Motivation

Closes #7268 Closes #4994

Solution

Remove preserve_state config and commit call by default - this will enable using vm.warp and vm.roll in invariants tests without any additional config (which is expected behavior) From test result it can be seen there is a slightly penalty from persisting cheatcode state (not something to worry about IMO).

Tests

Results were collected by performing different number of runs and depth on a test contract with:

• handler that advance block timestamp by 100 and increments block number on each call by using vm.warp and vm.roll (worst case scenario as persisting cheatcode states is now done by default)
• target contract with single selector that makes several state updates

Test contract

contract CommitState {
    uint256 state;
    mapping(address => uint256) public state1;
    mapping(address => mapping(address => bool)) public state2;

    function saveState(uint256 x, address y, address z) public {
        state = x;
        state1[y] = x;
        state2[y][z] = true;
    }
}

contract CommitStateHandler is Test {
    CommitState target;

    constructor() {
        target = new CommitState();
    }

    function fuzzTarget(uint256 x, address y, address z) public {
        vm.warp(block.timestamp + 100);
        vm.roll(block.number + 1);
        target.saveState(x, y, z);
    }
}

contract CommitStateTest is Test {
    function setUp() public {
        CommitStateHandler handler = new CommitStateHandler();
        targetContract(address(handler));
    }

    function invariant_commit_state() public view {}
}

Runs/Depth	time with Master	time with PR
100/1000	0m18.601s	0m19.587s
1000/1000	2m54.369s	3m12.411s
1000/5000	15m21.012s	15m55.039s
1/10000	0m10.528s	0m10.827s
10/10000	0m27.174s	0m29.075s
100/10000	3m18.966s	3m26.910s
10000/100	2m55.770s	3m1.623s

• time contains test contract compile time too

[grandizzy]

hey @pcaversaccio wanted to let you know that this change breaks sneakmate invariant invariantExecutingNotReadyProposal because time is now preserved between calls

Let's say there's a first run calling invariantExecutingNotReadyProposal https://github.com/pcaversaccio/snekmate/blob/e20185c1776256984d888d02dc243fe793119f2e/test/governance/TimelockController.t.sol#L4352-L4368 which calls TimelockControllerHandler.execute and properly reverts with "TimelockController: operation is not ready". However, TimelockControllerHandler.execute already does

        vm.warp(block.timestamp + minDelay);

before calling timelockController.execute (which reverts), see https://github.com/pcaversaccio/snekmate/blob/e20185c1776256984d888d02dc243fe793119f2e/test/governance/TimelockController.t.sol#L4424-L4433

Therefore 2nd time when invariantExecutingNotReadyProposal is called time has properly passed (advanced from prev call) and proposal will pass / call won't revert (see logs here snekmate.txt). I think it's safe to assume a fix is to just remove the warp call or make sure you're not advancing time so much for the proposal to become executable (like fuzz the timestamp to warp and bound it up to execution timestamp).

[pcaversaccio]

@grandizzy thanks for the heads-up; what's the time plan on the release on this breaking change?

[grandizzy]

@grandizzy thanks for the heads-up; what's the time plan on the release on this breaking change?

@pcaversaccio I am checking with other projects how this affects their usage, so probably 1 week or so, this was caught first as it is tested as external integration

[pcaversaccio]

Addressed via https://github.com/pcaversaccio/snekmate/pull/241 in snekmate.

[grandizzy]

ping @PaulRBerg @lucas-manuel do you forsee any issue (like the one above) in your projects? Thank you

[PaulRBerg]

tagging @smol-ninja and @andreivladbrg

[grandizzy]

btw, if you want to give it a quick check you can set preserve_state = true as this will be made default with this PR

[smol-ninja]

@grandizzy we are good with this PR. I have tested and there are no issues with the change.

[grandizzy]

tested ok ajna, morpho, maple, basin, llama, dss-conduit, weth-invariant-testing listed at https://github.com/perimetersec/public-fuzzing-campaigns-list?tab=readme-ov-file#foundry-invariant-testing