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

Unexpected Inlining of block.number with via-ir in Foundry Tests #6677

Closed
2 tasks done
0xLightt opened this issue Dec 28, 2023 · 1 comment
Closed
2 tasks done

Unexpected Inlining of block.number with via-ir in Foundry Tests #6677

0xLightt opened this issue Dec 28, 2023 · 1 comment
Labels
T-bug Type: bug

Comments

@0xLightt
Copy link

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 (67ab870 2023-12-27T00:16:03.636506444Z)

What command(s) is the bug in?

forge test

Operating System

Linux

Describe the bug

Description:

In my Foundry tests, I've discovered a peculiar issue concerning the handling of block.number. Normally, block.number should be dynamic, changing as the blockchain progresses, and this is indeed the case in tests when using vm.roll to simulate block progression. However, when block.number is assigned to a variable (like uint256 block1 = block.number;), unexpected behavior arises due to compiler optimizations. Instead of the variable retaining the block number at the time of assignment, it gets updated when vm.roll is used to change the block number. This is not in line with the expected behavior of Solidity in a live Ethereum environment, where a variable assigned with block.number should remain constant, reflecting the block number at the moment of assignment. This optimization-induced behavior can lead to misleading tests.

Was only able to reproduce this issue with via-ir enabled!

Code Snippet to Reproduce:

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

contract BlockNumberTest is Test {
    function test_blockNumber() public {
        uint256 block1 = block.number;
        assertEq(block1, 1);
        assertEq(block1, block.number);

        vm.roll(2);

        assertEq(block1, 2);
        assertEq(block1, block.number);
    }
}

Here is a passing test run that should fail (CI only fails due to failing to create coverage report): https://github.com/0xLightt/inlined-block-number/actions/runs/7342836645/job/19992591620

Expected Behavior:
The test should fail because block1 is expected to hold the value of block.number at the time of its assignment. When vm.roll(2) is called, block.number should change, but block1 should retain its original value.

Actual Behavior:
The test passes, indicating that block1 and block.number are treated as the same variable, likely due to compiler optimizations inlining block.number. This is inconsistent with how Solidity behaves in a live blockchain environment.

Impact:
This nuanced issue in Foundry tests, though uncommon, is noteworthy as it can cause unexpected outcomes, especially when comparing specific block numbers in detailed test scenarios.

Suggested Solutions/Improvements:
Considering that the issue seems to be confined to the test execution environment in Foundry, one possible solution could be the introduction of a cheat code. This cheat code could be designed to fetch the actual block.number value without being affected by the optimization or inlining process.

Request for Community Input:
I would appreciate any insights from other users or maintainers who might have encountered similar issues. Suggestions for temporary workarounds are also welcome until a more permanent solution is implemented. My workaround involved ceasing to record previous block numbers by directly fetching block.number. Instead, I assigned the same value of the expected block number values in the test passed in vm.roll.

@0xLightt 0xLightt added the T-bug Type: bug label Dec 28, 2023
@Evalir
Copy link
Member

Evalir commented Jan 3, 2024

This was already fixed through https://github.com/foundry-rs/foundry/pull/6630—please see the associated issue. Thanks for reporting!

@Evalir Evalir closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
No open projects
Archived in project
Development

No branches or pull requests

2 participants