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

gasleft() are inconsistent between tests, appear to depend on test name #6164

Closed
2 tasks done
yi-sun opened this issue Oct 29, 2023 · 5 comments
Closed
2 tasks done
Labels
T-bug Type: bug

Comments

@yi-sun
Copy link

yi-sun commented Oct 29, 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 (dee4181 2023-10-29T00:18:36.507223145Z)

What command(s) is the bug in?

No response

Operating System

Linux

Describe the bug

The behavior of gasleft() is not consistent across tests. In particular, it appears to depend on the name of the test. To reproduce, consider the following 3 test files:

Test.t.sol:

pragma solidity 0.8.19;

import "forge-std/console.sol";
import "forge-std/Test.sol";

contract GasTest is Test {
    function setUp() public { }

    function test_x() public {
        console.log(gasleft());
    }

    function test_x2() public {
        console.log(gasleft());
    }
}

Test2.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import "forge-std/console.sol";
import "forge-std/Test.sol";

contract GasTest2 is Test {
    function setUp() public { }

    function test_x() public {
        console.log(gasleft());
    }
}

Test3.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import "forge-std/console.sol";
import "forge-std/Test.sol";

contract GasTest3 is Test {
    function setUp() public { }

    function test_x3() public {
        console.log(gasleft());
    }
}

Running forge test --mt test_x -vv yields:

Running 2 tests for test/query/Test.t.sol:GasTest
[PASS] test_x() (gas: 3220)
Logs:
  9223372036854754427

[PASS] test_x2() (gas: 3264)
Logs:
  9223372036854754383

Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 522.66µs

Running 1 test for test/query/Test3.t.sol:GasTest3
[PASS] test_x3() (gas: 3132)
Logs:
  9223372036854754515

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 196.88µs

Running 1 test for test/query/Test2.t.sol:GasTest2
[PASS] test_x() (gas: 3220)
Logs:
  9223372036854754427

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 408.53µs
 
Ran 3 test suites: 4 tests passed, 0 failed, 0 skipped (4 total tests)
@yi-sun yi-sun added the T-bug Type: bug label Oct 29, 2023
@gakonst gakonst added this to Foundry Oct 29, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Oct 29, 2023
@yi-sun yi-sun changed the title gasleft() does not return consistent values between tests gasleft() are inconsistent between tests, appear to depend on test name Oct 29, 2023
@emo-eth
Copy link
Contributor

emo-eth commented Oct 30, 2023

This is a quirk of function dispatch in smart contracts – by the time an external function is called, Solidity's done a fair bit of work to match the function selector to a valid function. Solc doesn't (yet) have constant-gas/constant-time function dispatch, so calls that satisfy earlier function signature checks will consume less gas before entering the function (Seaport 1.6 has an "efficient" fulfillBasic endpoint with a different function selector that saves a couple hundred gas – otherwise it was dead last in the function dispatch checks).

You have some potential workarounds by using the pauseGasMetering and resumeGasMetering cheatcodes – but there are some issues with the cheatcodes at present (see #5491, #5564).

You may be able to pause gas metering in the setUp function, then resume it during tests; that should bypass metering during the function dispatch for the tests themselves. Hopefully, you won't run into the cheatcodes' quirks/bugs.

@mds1
Copy link
Collaborator

mds1 commented Oct 31, 2023

Closing per @jameswenzel's comment which is correct. If you share more about what you're trying to do we may be able to provide guidance on how to do it to avoid bias from function dispatch gas

@mds1 mds1 closed this as completed Oct 31, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Foundry Oct 31, 2023
@yi-sun
Copy link
Author

yi-sun commented Oct 31, 2023

Thanks @jameswenzel ! Was not aware function dispatch could cause different gas costs.

I'm trying to create positive and negative tests for a function which should revert or not based on the value of gasleft() at a certain middle point of its execution. Any guidance on the best practice for this in Foundry would be much appreciated!

@mds1
Copy link
Collaborator

mds1 commented Nov 1, 2023

Is this a correct interpretation of what you're trying to test:

contract Foo {
  function foo() public {
    // Do some stuff.
   
    // Revert if not enough gas left.
    if (gasleft() < 20_000) revert InsufficientGas();

    // Do more stuff.
  }
}

If so, some options:

  • Check gasleft() in your test, and if it's too high before entering foo, do some arbitrary increments or SSTOREs in a loop to burn gas as needed.
  • Use a low-level call so you can specify how much gas to forward
  • Change the gasleft() read to a virtual function and override it in tests, such as below (I don't love having to structure source code to accomodate tests but it does work)
  • Forge does have a vm.expectCallMinGas which may help you. I don't think it's documented in the book but here's the PR that implemented it feat: Add new expectCall cheatcode variants #4413.
contract Foo {
  function _gasleft() internal virtual returns (uint256) {
    return gasleft(); // virtual to be overridden in tests
  }

  function foo() public {
    // Do some stuff.
   
    // Revert if not enough gas left.
    if (gasleft() < 20_000) revert InsufficientGas();

    // Do more stuff.
  }
}

contract FooHarness is Foo {
  function _gasleft() internal override returns (uint256) {
    return 10; // or add a setter method to read this from storage, etc.
  }
}

@yi-sun
Copy link
Author

yi-sun commented Nov 1, 2023

I think vm.expectCallMinGas is exactly what I’m looking for — thanks for the detailed explanations!

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

No branches or pull requests

3 participants