Invariant tests: preserve_state is not respected when fail_on_revert = false

[lekhovitsky]

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 (3df3e0c 2024-03-04T00:19:46.775990000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.17;

import {Test} from "forge-std/Test.sol";

contract Handler is Test {
    uint256 public timestamp;

    constructor(uint256 initial) {
        timestamp = initial;
    }

    function skipTime(uint256 timedelta) external {
        timedelta = bound(timedelta, 1, 3600);
        vm.warp(block.timestamp + timedelta);
        timestamp = block.timestamp;
    }
}

contract Invariant is Test {
    Handler handler;

    error TimestampsAreDifferent(uint256 testTimestamp, uint256 handlerTimestamp);

    function setUp() public {
        handler = new Handler(block.timestamp);
    }

    function invariant_timestamps_are_equal() public {
        uint256 handlerTimestamp = handler.timestamp();
        if (handlerTimestamp != block.timestamp) revert TimestampsAreDifferent(block.timestamp, handlerTimestamp);
    }
}

Running the following test with preserve_state = true and fail_on_revert = false results in the falsely broken invariant:

Ran 1 test for contracts/test/invariant/Example.sol:Invariant
[FAIL. Reason: TimestampsAreDifferent(16400000 [1.64e7], 16400383 [1.64e7])]
        [Sequence]
                sender=0xEBE0eD5313c082e348796F85183130986450a216 addr=[contracts/test/invariant/Example.sol:Handler]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=skipTime(uint256) args=[383]
 invariant_timestamp() (runs: 1000, calls: 24976, reverts: 0)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 2.45s (2.45s CPU time)

[mds1]

This was expected behavior per #7219 (comment), cc @grandizzy, should this be changed?

[grandizzy]

This was expected behavior per #7219 (comment), cc @grandizzy, should this be changed?

Yes, I guess no harm to change, going to make a PR in couple