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

block.timestamp bugs when using skip #6180

Closed
2 tasks done
77abe77 opened this issue Nov 1, 2023 · 6 comments · Fixed by #6630
Closed
2 tasks done

block.timestamp bugs when using skip #6180

77abe77 opened this issue Nov 1, 2023 · 6 comments · Fixed by #6630
Labels
T-bug Type: bug

Comments

@77abe77
Copy link

77abe77 commented Nov 1, 2023

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 (619f3c5 2023-10-22T00:19:08.867786000Z)

What command(s) is the bug in?

forge test -vvvv --mc "ForgeBug"

Operating System

None

Describe the bug

Two bugs, easy to reproduce:

Consists of 2 contracts ForgeBug and ForgeBug1

Common bug in both contracts:

Actual Behavior:

duration (end - start) is always 0. Implying that start is getting erroneously mutated along the way.

Expected Behavior:

duration (end - start) should be non zero and that start should be caching the first value of block.timestamp

contract ForgeBug is Test {
    function test_timebug() external {
        uint start = block.timestamp;
        console2.log("start: %d", start);

        for (uint i; i < 4; i++) {
            skip(15);
            console2.log("skip 15 sec: %d", block.timestamp);
        }

        uint end = block.timestamp;
        console2.log("start: %d", start);
        console2.log("end: %d", end);
        console2.log("duration: %d", end - start);
    }

}

ForgeBug Contract Additional Bug

Actual Behavior:

Logs:
  start: 1
  skip 15 sec: 16
  skip 15 sec: 16
  skip 15 sec: 16
  skip 15 sec: 16
  start: 16
  end: 16
  duration: 0

Expected Behavior:

Logs:
  start: 1
  skip 15 sec: 16
  skip 15 sec: 31
  skip 15 sec: 46
  skip 15 sec: 61
  start: 61
  end: 61
  duration: 0

ForgeTest1 has an unrolled for loop version of the test included and causes correct behavior (minus duration bug)

contract ForgeBug1 is Test {
    function test_timebug1() external {
        uint start = block.timestamp;
        console2.log("start: %d", start);

        skip(15);
        console2.log("skip 15 sec: %d", block.timestamp);
        skip(15);
        console2.log("skip 15 sec: %d", block.timestamp);
        skip(15);
        console2.log("skip 15 sec: %d", block.timestamp);
        skip(15);
        console2.log("skip 15 sec: %d", block.timestamp);

        uint end = block.timestamp;
        console2.log("start: %d", start);
        console2.log("end: %d", end);
        console2.log("duration: %d", end - start);
    }

    function test_timebug2() external {
        uint start = block.timestamp;
        console2.log("start: %d", start);

        for (uint i; i < 4; i++) {
            skip(15);
            console2.log("skip 15 sec: %d", block.timestamp);
        }

        uint end = block.timestamp;
        console2.log("start: %d", start);
        console2.log("end: %d", end);
    }

}

Logs:

[PASS] test_timebug1() (gas: 12642)
Logs:
  start: 1
  skip 15 sec: 16
  skip 15 sec: 31
  skip 15 sec: 46
  skip 15 sec: 61
  start: 61
  end: 61
  duration: 0
  
  
[PASS] test_timebug2() (gas: 12066)
Logs:
  start: 1
  skip 15 sec: 16
  skip 15 sec: 31
  skip 15 sec: 46
  skip 15 sec: 61
  start: 61
  end: 61
@77abe77 77abe77 added the T-bug Type: bug label Nov 1, 2023
@gakonst gakonst added this to Foundry Nov 1, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Nov 1, 2023
@mattsse
Copy link
Member

mattsse commented Nov 1, 2023

unable to reproduce:

Running 1 test for test/Timestamp.t.sol:ForgeBug
[PASS] test_timebug() (gas: 13628)
Logs:
  start: 1
  skip 15 sec: 16
  skip 15 sec: 31
  skip 15 sec: 46
  skip 15 sec: 61
  start: 1
  end: 61
  duration: 60

@77abe77
Copy link
Author

77abe77 commented Nov 1, 2023

@mattsse what forge version are you running?

@mds1
Copy link
Collaborator

mds1 commented Nov 1, 2023

@77abe77 are you compiling with via-ir? If so see the convo and links in #4934 (comment)

@77abe77
Copy link
Author

77abe77 commented Nov 1, 2023

@mds1 good catch, yes I am! will take a look

@brockelmore
Copy link
Member

@mds1 - maybe we should add vm.blockTimestamp() and vm.blockHeight() cheatcodes (as non-pure fns)

Effectively this prevents the optimizer from doing this. Alternatively we could add a vm.blackbox(...) that just echos the input data but overloading it would be a pain. So forge specific timestamps/block heights may make sense? Ergonomics of this aren't great, but at least we can point people to it 🤷

@mds1
Copy link
Collaborator

mds1 commented Nov 10, 2023

vm.blockTimestamp() and vm.blockNumber() are nice workarounds, I like that idea. Agreed it’s a bit clunky, but the native cheat is still simpler than users having to implement a workaround themselves, so supportive 👍

@github-project-automation github-project-automation bot moved this from Todo to Done in Foundry Dec 21, 2023
pcaversaccio added a commit to pcaversaccio/snekmate that referenced this issue Jan 3, 2025
### 🕓 Changelog

This commit refactors `TimelockController.t.sol` to use
`getBlockTimestamp()` instead of `block.timestamp`, as `block.timestamp`
is assumed to be constant across a transaction and will be optimised out
by the compiler. For more details, see
[issue](foundry-rs/foundry#6180).
Additionally, `solhint` has been upgraded to the latest version,
[`v5.0.4`](https://github.com/protofire/solhint/releases/tag/v5.0.4),
and all submodules have been updated to their most recent commits.

---------

Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
@DaniPopes DaniPopes closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants