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

vm.resumeGasMetering can cause tests with expected reverts to fail with an OutOfGas error #5564

Closed
2 tasks done
emo-eth opened this issue Aug 8, 2023 · 1 comment · Fixed by #8736
Closed
2 tasks done
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge Cmd-forge-test Command: forge test T-bug Type: bug
Milestone

Comments

@emo-eth
Copy link
Contributor

emo-eth commented Aug 8, 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 (6672134 2023-08-08T00:21:59.824374000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

After calling vm.pauseGasMetering, calling vm.resumeGasMetering within a reverting external call causes tests to fail with an OutOfGas error.

There seem to broadly be inconsistencies and unexpected behavior with the GasMetering functions; see #5491 as possibly related.

A reproduction, which eliminates self-external-calls as the issue.

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

contract RevertingExternalContract {
    error MyError();

    Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));

    function externalReverts() external {
        vm.resumeGasMetering();
        revert MyError();
    }
}

contract MeteringTest is Test {
    error MyError();

    RevertingExternalContract externalContract;

    function setUp() public {
        externalContract = new RevertingExternalContract();
    }

    function testSelfNormalRevert() public {
        vm.expectRevert(MeteringTest.MyError.selector);
        this.selfReverts();
    }

    function testSelfMeteringRevert() public {
        vm.pauseGasMetering();
        vm.expectRevert(MeteringTest.MyError.selector);
        this.selfReverts();
    }

    function testExternalNormalRevert() public {
        vm.expectRevert(RevertingExternalContract.MyError.selector);
        externalContract.externalReverts();
    }

    function testExternalMeteringRevert() public {
        vm.pauseGasMetering();
        vm.expectRevert(RevertingExternalContract.MyError.selector);
        externalContract.externalReverts();
    }

    function testExternalMeteringManualCheck() public {
        vm.pauseGasMetering();
        (bool success,) = address(this).call(abi.encodeWithSelector(this.selfReverts.selector));
        assertEq(success, false);
    }

    function selfReverts() external {
        vm.resumeGasMetering();
        revert MyError();
    }
}

Output of running these tests:

[PASS] testSelfNormalRevert() (gas: 3953)
Test result: FAILED. 2 passed; 3 failed; 0 skipped; finished in 402.42µs
Ran 1 test suites: 2 tests passed, 3 failed, 0 skipped (5 total tests)

Failing tests:
Encountered 3 failing tests in test/Metering.t.sol:MeteringTest
[FAIL. Reason: EvmError: OutOfGas] testExternalMeteringManualCheck() (gas: 9223372036854754743)
[FAIL. Reason: EvmError: OutOfGas] testExternalMeteringRevert() (gas: 9223372036854754743)
[FAIL. Reason: EvmError: OutOfGas] testSelfMeteringRevert() (gas: 9223372036854754743)
@zerosnacks
Copy link
Member

zerosnacks commented Jun 28, 2024

Unable to reproduce the OutOfGas error but am able to reproduce the huge gas consumption described in #5491

Ran 5 tests for test/Metering.t.sol:MeteringTest
[PASS] testExternalMeteringManualCheck() (gas: 18302628885633699948)
[PASS] testExternalMeteringRevert() (gas: 18302628885633699399)
[PASS] testExternalNormalRevert() (gas: 8627)
[PASS] testSelfMeteringRevert() (gas: 18302628885633699354)
[PASS] testSelfNormalRevert() (gas: 4005)
Suite result: ok. 5 passed; 0 failed; 0 skipped; finished in 3.72ms (611.34µs CPU time)

With the --isolate flag

Ran 5 tests for test/Metering.t.sol:MeteringTest
[PASS] testExternalMeteringManualCheck() (gas: 6248)
[PASS] testExternalMeteringRevert() (gas: 3200)
[PASS] testExternalNormalRevert() (gas: 32191)
[PASS] testSelfMeteringRevert() (gas: 3156)
[PASS] testSelfNormalRevert() (gas: 27569)

Leaving the open as it is clear this kind of gas consumption could lead to and OutOfGas error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge Cmd-forge-test Command: forge test T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants