foundry-rs/forge-std

Forge Standard Library is a collection of helpful contracts for use with forge and foundry. It leverages forge's cheatcodes to make writing tests easier and faster, while improving the UX of cheatcodes. For more in-depth usage examples checkout the tests.

forge-std's Issues

Not able to install forge-std

Feature request: console.table

In JavaScript, console.table is a very useful utility that pretty prints the input data.

Example:

console.table(["apples", "oranges", "bananas"]);

You would get something like this in the console:

Many times I find myself adding empty spaces to my Solidity console.log statements just so that I can see the outputs aligned on the vertical axis.

It would be nice to have a helper function that creates a table with ASCII characters (e.g. -) and auto-indents the outputs for us.

feature: syntactic sugar for reading storage variables

Currently, in order to access a value from storage in a test you need to:

1. get its storage slot, which is a uint256
2. convert the slot variable to bytes32
3. load the data from the address' storage
4. convert back to the variable's type

// get slot
uint256 varSlot = stdstore
            .target(address(contractName))
            .sig(contractName.storageVar.selector)
            .depth(0)
            .find();
// convert to bytes32 and load from slot, then convert to desired variable type (uint256 in this case)
uint256 storageVar = uint256(vm.load(address(collatz), bytes32(delay_slot)));

Would be great to have some syntactic sugar to make this process more straightforward as well as hopefully getting rid of the back and forth with type conversions. Perhaps ...find().load() or something along those lines?

Update the CI to run against older solc versions too

Context

@mds1 suggested we update the CI to run against a few different solc versions, so that changes made to the Test contract do not break compatibility with older Solidity versions, which has happened twice.

Motivation

I wanted to update the CI to run tests against all the supported major versions, but the tests are not backward-compatible.

The following changes are required to make it work, but I do not think this approach would be sustainable in the long term.

You can jump to the Solution instead.

Latest

The tests are compiling and passing.

0.8.0

• Adjust pragmas in StdAssertions.t.sol, StdError.t.sol, StdMath.t.sol
• Explicitly cast uint192 to uint256 when passing a value as a argument to a fuction that takes uint256

0.7.0

• Run tests with --no-match-contract StdErrorsTest because most of the std errors either did not exist or reverted without data
• Divide testGetPercentDelta_Int and testGetPercentDelta_Uint into simplier tests and use testFail for the ones that check for a revert when dividing by 0 instead of using stdError.divisionError

0.6.0

• Specify visiblity for all constructors
• Make external test functions public and change the location of some parameters from calldata to memory (or leave the test functions external and change the location of some parameters from memory to calldata)
• Use constants instead of type(<TYPE>).<MIN/MAX>
• {value: <VALUE>} is not supported, meanwhile .value(<VALUE>) is deprecated for pragma >=0.7.0 (Make a helper function with assembly and use that instead? Don't like that solution.)
• Add getter functions to libraries for returning constants
• <STORAGE_VARIABLE>.slot in assembly is not supported; hardcode the slot number or use a pointer

Solution

Check only if the Test contract compiles for versions 0.8.0, 0.7.0, 0.6.0, but do not run tests against those versions. Run test against the latest common version, as before.

feat: `noPrecompiles` modifier or function

Ref foundry-rs/foundry#1228

The fuzzer will pass precompile addresses to tests. These are valid inputs so we don't want to strip them from the fuzzer, but in some tests they can cause problems. The scope of this issue is to add a modifier OR a function (I'm not yet sure which we should prefer, open to discussion here, as well a name other than noPrecompiles) that does something like this:

// --- forge-std ---
// Use this version to infer the chain ID from the test environment.
function noPrecompiles(address who) internal {
  noPrecompiles(who, block.chainid);
}

// Use this version to specify which chain's precompiles to use.
function noPrecompiles(address who, uint256 chainId) internal {
  if (chainid == 1 || chainid == 31337) {
    // Mainnet precompiles: https://www.evm.codes/precompiled
    vm.assume(who > address(9));
  } else if (chainid == 10 || chainid == 69 || chainid == 420) {
    // Optimism precompiles: https://github.com/ethereum-optimism/optimism/blob/master/packages/contracts/contracts/libraries/constants/Lib_PredeployAddresses.sol
    vm.assume(who < 0x4200000000000000000000000000000000000000 && who > 0x4200000000000000000000000000000000000013);
  }
  // etc. for other chains
}

// --- myTest.t.sol ---
function testSomeAddress(address a, address b) public {
  noPrecomples(a); // Precompiles for `b` are allowed.
}

I think including mainnet, optimism, and arbitrum precompiles is a good start for this PR

Meta: Unbundling the cheats from the assertions (leg 1)

Description

The way the Test contract is currently architected is not in line with the principle of separation of concerns.

The most problematic thing IMHO is the fact that the cheats and assertions live under the same roof. This makes it difficult for Forge Std to be composed with other testing assertion contracts, such as PRBTest.

Of course, I'm a bit biased because of PRBTest, but even if we nevermind my intentions, there are many reasons for why people may wish to write their own testing assertion contracts. If anything, maybe they simply don't like either DSTest or PRBTest. Or perhaps they're working with user-defined value types, which need certain overloads for assertEq. Or perhaps they're using a fork of the EVM on which DSTest doesn't compile or doesn't work well. Bottom line is: Forge Std is not as modular as it could be.

Additionally, separating the cheats from the assertions would also tidy up the contracts a bit, since there would be fewer lines of code in each contract.

Solution

Create a Cheats contract and move all cheats there. Test would then inherit from Cheats.

Doing this would make Forge Std more modular. Users would be able to selectively use just the cheats or just the assertions.

deployCode with value

forge-std has this nice little function called deployCode that makes it really easy to deploy contracts compiled with old versions, very useful in tests.
Though, it has an issue. We cannot pass value in order to send ETH upon contract construction.
So we may change the create low level function argument, actually the first one that is sending value.

Now the issue is that in order to be able to send ETH to this deployCode function, it needs to be external, and we cannot call external function from the parent test contract because it inherits Test.sol.

Is there an easy solution here, or would it be worth creating a new cheatcode to do that ?

first key in a mapping with type uint as key does allow to access zero key

mapping(uint=> address) ownerOf

trying to get storage slot of key 0 doesnt work, complains about index out of bounds

uint256 slotOfNewOwner = stdstore
            .target(address(nft))
            .sig(nft.ownerOf.selector)
            .with_key(0)
            .find();

instead accessing key 0 the following way works:

uint256 slotOfNewOwner = stdstore
            .target(address(nft))
            .sig(nft.ownerOf.selector)
            .with_key(1)
            .find();

mockCall doesn't work when using it with expectRevert.

Source:

  function swapFromLocalAssetIfNeeded(address _asset, uint256 _amount) internal returns (uint256, address) {
    AppStorage storage s = LibConnextStorage.connextStorage();
    console.log(".....point 1");
    // Get the token id
    (, bytes32 id) = s.tokenRegistry.getTokenId(_asset);

    console.log(".....point 2");
    // If the adopted asset is the local asset, no need to swap
    address adopted = s.canonicalToAdopted[id];
    if (adopted == _asset) {
      return (_amount, _asset);
    }
    console.log(".....point 3");
    // NOTE: Normally, this would be checked via the `whenSwapNotPaused` modifier (as in the
    // StableSwapFacet). However, when entering an internal swap, the best place to check
    // is in these swap functions where swaps can be stopped. You can check here for only
    // swap pauses (general pauses are checked before this function is called)
    if (s._paused == PausedFunctions.Swap) revert AssetLogic__swapFromLocalAssetIfNeeded_swapPaused();
    console.log(".....point 4");
    // Swap the asset to the proper local asset
    return _swapAsset(id, _asset, adopted, _amount);
  }

Test

  function test_AssetLogic_swapFromLocalAssetIfNeeded_revertIfSwappingPaused() public {
    s.canonicalToAdopted[canonicalTokenId] = adopted;
    s._paused = PausedFunctions.Swap;
    vm.mockCall(
      _tokenRegistry,
      abi.encodeWithSelector(ITokenRegistry.getTokenId.selector),
      abi.encode(canonicalDomain, canonicalTokenId)
    );
    vm.expectRevert(AssetLogic.AssetLogic__swapFromLocalAssetIfNeeded_swapPaused.selector);
    AssetLogic.swapFromLocalAssetIfNeeded(asset, 10000);
  }

Result logs

This screenshot of logs just shows mockCalls didn't work because expectRevert. How to resolve this case?

meta: usefulness of a Strings/Bytes utils in forge-std?

Forge's ffi cheatcode accepts an array of strings. This generally works well for passing shell commands, but becomes troublesome when those commands have to include data or arguments represented by solidity variables.

Other than OpenZeppelin's Strings library, which only handles uints, I am not aware of any other common String conversion libraries. Most solutions are home grown.

If FFI grows in popularity, it might be helpful to expose a library that helps with the conversion back and forth between strings consumable by non-evm Web3 libraries and EVM variables. For example, if you want to pass a bytes memory array to FFI, you need to implement your own function like this (copied from the differential testing tutorial):

library Strings2 {

    ///@dev converts bytes array to its ASCII hex string representation
    /// TODO: Definitely more efficient way to do this by processing multiple (16?) bytes at once
    /// but really a helper function for the tests, efficiency not key.
    function toHexString(bytes memory input) public pure returns (string memory) {
        require(input.length < type(uint256).max / 2 - 1);
        bytes16 symbols = "0123456789abcdef";
        bytes memory hex_buffer = new bytes(2 * input.length + 2);
        hex_buffer[0] = "0";
        hex_buffer[1] = "x";

        uint pos = 2;
        uint256 length = input.length;
        for (uint i = 0; i < length; ++i) {
            uint _byte = uint8(input[i]);
            hex_buffer[pos++] = symbols[_byte >> 4];
            hex_buffer[pos++] = symbols[_byte & 0xf];
        }
        return string(hex_buffer);
    }
}

There are likely other examples as well. Opening up here for discussion to see if others think it's worth including a library, and if so what features we would want to support.

Meta: Unbundling Test from Script

Description

Commit f18682b made by @devanonon added two new utility functions in the Script contract:

1. computeCreateAddress
2. addressFromLast20Bytes

Which are of course very useful in the context of deploying contracts (the main feature of Solidity Scripting).

But now this makes it confusing that the Test contract inherits from Script. Why should it? They seem to be two separate concerns. If a user just so happens to define a function called run in their tests, that would be treated as a script entrypoint by Forge.

If the goal was to add the vm property to Test, we could simply redeclare the VM_ADDRESS and the vm variables in Test. Redundancy is not intrinsically bad.

Actually, even before commit f18682b, I was a bit confused by the fact that the Test contract inherits fromScript. Even in the Foundry Book, there's an implicit distinction made between Writing Tests and Solidity Scripting. So even if this proposal isn't accepted, at the very least we should add a warning in the Foundry Book that contracts inheriting from Test also inherit from Script.

Solution

1. Remove the inheritance from Script in Test.
2. Redeclare VM_ADDRESS and vm in Test.
3. Rewrite the methods in Script to have public visibility.
4. Rewrite the tests in Script.t.sol to instantiate an internal test contract script.

Caveats

1. If this proposal passes, a child contract couldn't inherit from both at the same time, but if I understand the goals of Test and Script correctly, users are not expected to do this anyway.
2. As I understand it, Foundry excludes all contracts with the IS_SCRIPT flag set to true from the gas reports. But does it also exclude IS_TEST contracts? If yes, then this isn't a problem. But if not, why? If it does that for scripts it should also it for pure tests contracts. Happy to open an issue in Foundry to track this.

CapWords naming sytle

Is there a reason why the names of the contracts and libraries don't begin with a capital letter?

E.g. stdCheats --> StdCheats

Update Readme

We should update the readme to reflect the fact that Test is the most common usage and document its uses

Add `coinbase` cheat to `Vm`

See foundry-rs/foundry#1493

Feature request: cheatcode to expect ERC-20 token balance change

In Waffle, there is a chai matcher for testing whether a contract call changes the ERC-20 token balance of an account:

changeTokenBalance

Which can be used like this:

await expect(() => token.transfer(walletTo.address, 200))
  .to.changeTokenBalance(token, walletTo, 200);

await expect(() => token.transferFrom(wallet.address, walletTo.address, 200))
  .to.changeTokenBalance(token, walletTo, 200);

It would be nice to have an equivalent VM cheatcode for this.

Meta: Potential benefits of switching the pragma to ">=0.8.0 <0.9.0"

I know that there's a keen interest in keeping Forge Std compatible with Solidity v0.6 and Solidity v0.7, but I also think that there would be quite a few benefits in upgrading to Solidity v0.8, so I made this post to shed some light on them.

Even if we don't upgrade the pragma now, it might be worth it keep these advantages in the back of our minds (maybe we could implement them on a "solidity-v8" branch).

1. Provide a speed bump to Solidity v0.8 users (probably the majority of users at the time of writing this) via:
   • unchecked arithmetic
   • Reverting with custom errors instead of revert reason strings
2. Simplify and even delete some functions, e.g. getCode could be replaced by <address>.code.
3. Free functions (introduced in v0.7.1) (see what how I used them in PRBTest).
4. Provide type safety via user-defined value types (introduced v0.8.13), e.g. in assertApproxEqRel.
5. type(uint256).max to get the min and max values permitted in a given type.
6. Make it possible to implement the change proposed by @mds1 in #78 (because interfaces and libraries can inherit in Solidity v0.8)
7. Make it possible to upstream assertions to PRBTest (see discussion here).

And potentially several other enhancements could be made to the syntax in StdStorage.

`forge snapshot` hangs with `--fork-url` arg

Forge version: forge 0.2.0 (9b2d95d 2022-06-23T00:13:03.193579407Z)

When running forge snapshot with the --fork-url argument to account for tests using mainnet forking, the output hangs without finishing or the command terminating.

This is the output from the most recent run with RUST_LOG=ethers=trace set which overflows the terminal ending with the 19 tests from WitchV2.t.sol and never proceeding.
forge-issue.txt

feat: upstream Solmate's `bound`

This method is useful for wrapping inputs of a fuzz test into a certain range, and t11s has given approval to upstream this. Scope:

• Add bound
• Add the associates tests
  • Add bound tests that cover type(uint256).max as the upper bound input, since this is an edge case handled separately within bound
  • Change the testFail tests to the expectRevert style

I don't plan on working on this over the next few days, so this is open for anyone to take 🙂

refactor: add an abstract `Script` contract

With solidity scripting and deploys on the way (foundry-rs/foundry#1208), a a series of new cheatcodes for sending transactions will be added to the Vm interface. To access these, you'd currently need to have your scripts inherit from Test which is a bit awkward because (1) this is not a test file, and (2) even if you just renamed Test, you still inherit lots of cheatcodes and forge-std methods which can be dangerous for live transactions, and therefore are not desirable to have present in a script.

As a result, I think we should refactor to have abstract contract Script and abstract contract Test is Script, this way scripts can just inherit from Script, and tests can continue to inherit from Test with no breaking changes.

This abstract Script contract will contain:

• A minimal Vm interface with just the new broadcast cheatcodes, as well as "view" cheatcodes that don't change state. I may be missing some, but I think this includes: load, addr, getNonce, records, accesses, getCode, and label.
• The stdMath library.
• The stdStorage library, but with the checked_write methods disabled.

Libraries don't get inherited by default, so we may need to split the last one into stdStorageRead and stdStorage is stdStorageRead (assuming libraries can inherit? I'm not actually sure). Alternatively perhaps we make it into a contract and use a storage var to conditionally disable checked_write based on that, though this would be a breaking change.

meta: how to improve mocking in forge-std?

Forge has two cheatcodes for mocking:

interface Vm {
    // Mocks a call to an address, returning specified data.
    // Calldata can either be strict or a partial match, e.g. if you only
    // pass a Solidity selector to the expected calldata, then the entire Solidity
    // function will be mocked.
    function mockCall(address, bytes calldata, bytes calldata) external;
    // Clears all mocked calls
    function clearMockedCalls() external;
}

This results in the following mocking UX native to forge:

• Mocking responses at an address that already has code is straightforward using mockCall
• Mocking responses at an address that does not have code is a bit clunky, as it requires etching dummy code to that address
• Mocking reverts and out of gas scenarios is not possible natively
• If you call a method at a mocked address and it doesn't exist, the revert reason is empty instead of something like "function selector not found on target address", which can be confusing. (This is a general forge issue, not mock specific).

This is ok, but not great. Given that, what should forge-std do to improve mocking UX, and what should be upstreamed to forge?

For context, various mocking contracts exist to help with the above scenarios (note that I have not carefully reviewed each, these are just my quick impressions):

• Gnosis' MockContract.sol is very flexible, and allows you to mock reverts and out of gas errors
• Waffle's Doppelganger.sol is similar. It has less code (so presumably faster to compile) but does not support mocking out of gas errors.
• @maurelian's UniMock seems to be the smallest, and could probably be easily extended to mock an out of gas error.
• @cleanunicorn's MockPrivder seems like another good option.

My initial reaction is:

1. Improve forge so it can provide better revert reasons if the revert is due to a selector not being found.
2. Improve forge's mockCall so it can support mocking reverts with specified data. (Maybe support mocking out of gas errors too? Not sure how common needing to mock OOG is, just mentioning it since gnosis's mock supports it).
3. With the above, we can leave forge's mockCall as the default for when you need to mock responses on contracts that already exist.
4. Then add one of the above mock contracts to forge-std with deployMock() and deployMock(address where) helper methods. This would be used in cases where you need a mock at an address that doesn't have any code.

I think this should should cover all popular use cases of mocking, and uncommon use cases probably don't need to be supported in forge-std. But let me know if there's common scenarios I missed!

Tagging @bbonanno @maurelian and @cleanunicorn since I think you all may have some thoughts/feedback on the above 🙂

feature: support writing to packed slots

For local development, I currently use uint256 everywhere so I can use the forge-std helpers to easily set slot values. However, eventually I want to start packing variables without these helpers and the tests breaking.

I think the easiest way to do this is to leverage the solc storage layout to get the info we need. From @brockelmore:

ye so this actually gets interesting now that i have a dynamic mapping cuz we can actually represent json directly in solidity (kek). we could add a cheatcode to get the storage layout into memory then you could pass in the layout with relative ease for a slot and know youre doing it correctly. in the short term if this is something u want and can easily provide the positioning + left and right padding we can support it

something like .packed(left, right)

where each is number of bits on the left and right

which eventually would ideally be grabbable in code via cheatcode

Since storage layout is not included by default, we either need to throw an error if the storage layout isn't present, or recompile the specified contract with that flag on the fly

For contracts that aren't local (e.g. if testing against a forked network), I think we'd be limited to only setting vars in packed slots if the contract is verified on Etherscan so we can pull the source and compile with storage layout enabled

Installation failed.

System: MacOS X 10.15
Foundry forge version: forge 0.2.0 (a63e56d 2022-04-10T00:12:58.599340+00:00)
Use the command line that Readme.md mentioned: forge install foundry-rs/forge-std
Error result:

Confusing Licenses

When a user runs forge init, they get a bunch of files from this repository. Presumably, they may end up leaving a bunch of code from this repo in their repo, which means the license matters (especially for the testing code).

There are two licenses in the root, which do not seem like very compatible licenses with each other, both same copyright holder: https://github.com/foundry-rs/forge-std/blob/master/LICENSE-APACHE and https://github.com/foundry-rs/forge-std/blob/master/LICENSE-MIT

In addition to that, the testing contracts from dapp.tools are probably the most important parts for people to re-use. In fact, I guess they probably pretty much have to re-use them to be able to use many of Foundry's more advanced features. It does appear that in this repo, files do have individual licenses, for example: https://github.com/foundry-rs/forge-std/blob/master/src/Test.sol#L1

When I do run forge init, I also get some files that have UNLICENSED (do forge init in an empty directory, then git grep UNLICENSE). This gets even more confusing, because that's not one of the listed licenses in the created repo.

This may make it a bit more difficult for some people to use these testing tools on their projects, without the needed license clarity.

Previous issue (told to move it here): foundry-rs/foundry#2311

"restartPrank" Helper

Component

Forge

Describe the feature you would like

I think the need to start a global prank in the setUp function but deactivate it in certain tests is a pretty common scenario.

To deactivate the global prank, I have to run two commands:

vm.stopPrank();
vm.startPrank(otherUser);

Lest forge fails with the following error:

[FAIL. Reason: You have an active prank already.]

It would be nice if there was a cheatcode that could atomically perform these two commands: a restartPrank cheatcode that accepts an address as an argument to be passed to startPrank.

Additional context

No response

Forge CI

should setup a ci - @abigger87 wanna look into this

multiple contracts deployed via deployContract cannot interact with each other

Edit: see this repo with the example - https://github.com/jgeary/one-off-forge-issue

Let's say theres an old protocol (0.4.24) and a new protocol I'm building (0.8.10). The old protocol consists of ContractA and ContractB. ContractB needs to make some function calls to ContractA. The new protocol is just simply ContractC which interacts through an interface with ContractB. Now I want to write an integration test for this system.

What I want to do is use deployCode twice, once for ContractA and again for ContractB, then create a new instance of ContractC natively, then interact with them all and make some assertions.

It seems like this is impossible, and it specifically breaks down when ContractB attempts to make a call to ContractA. This causes a revert with no reason. See example below.

AB.sol:

pragma solidity 0.4.24;

contract Ownable {
    address public owner;

    constructor() public {
        owner = msg.sender;
    }

    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }

    function renounceOwnership() public onlyOwner {
        owner = address(0);
    }

    function transferOwnership(address _newOwner) public onlyOwner {
        _transferOwnership(_newOwner);
    }

    function _transferOwnership(address _newOwner) internal {
        require(_newOwner != address(0));
        owner = _newOwner;
    }
}

contract A is Ownable {}

contract B {
    A a;

    constructor(A _a) {
        a = _a;
    }

    function step1() external {
        require(
            true,
            "flip this to false to test that we're getting to this point"
        );
        require(
            a.owner() == address(this),
            "a.owner() reverts so the condition is never evaluated and this error message is not shown"
        );
        require(
            false,
            "this error message is also not shown because above line reverts"
        );
        a.transferOwnership(msg.sender);
    }

    function step2() external {
        require(a.owner() == msg.sender);
    }
}

C.sol:

pragma solidity 0.8.10;

interface IA {
    function transferOwnership(address _newOwner) external;

    function renounceOwnership() external;

    function owner() external returns (address);
}

interface IB {
    function step1() external;

    function step2() external;
}

contract C {
    address a;
    address b;

    constructor(address _a, address _b) {
        a = _a;
        b = _b;
    }

    function step1() external {
        IB(b).step1();
    }

    function step2() external {
        IB(b).step2();
        IA(a).renounceOwnership();
    }
}

Integration.t.sol:

pragma solidity 0.8.10;

import "ds-test/test.sol";
import {stdCheats} from "forge-std/stdlib.sol";
import "../C.sol";

contract IntegrationTest is DSTest, stdCheats {
    C internal c;

    function setUp() public {
        address a = deployCode("AB.sol:A");
        address b = deployCode("AB.sol:B", abi.encodePacked(a));

        IA(a).transferOwnership(b);
        c = new C(a, b);
    }

    function test() public {
        c.step1();
        c.step2();
    }
}

Output:

[FAIL. Reason: ] test() (gas: 4571)
Traces:
  [530621] IntegrationTest::setUp()
    ├─ [0] VM::getCode("AB.sol:A")
    │   └─ ← ()
    ├─ → new A@0xce71…c246
    │   └─ ← 384 bytes of code
    ├─ [0] VM::getCode("AB.sol:B")
    │   └─ ← ()
    ├─ → new B@0x185a…1aea
    │   └─ ← 761 bytes of code
    ├─ [891] A::transferOwnership(0x185a4dc360ce69bdccee33b3784b0282f7961aea)
    │   └─ ← ()
    ├─ → new C@0xefc5…b132
    │   └─ ← 408 bytes of code
    └─ ← ()
  [4571] IntegrationTest::test() // this is red
    ├─ [3842] C::step1() // red
    │   ├─ [3201] B::step1() // red
    │   │   └─ ← ()
    │   └─ ← ()
    └─ ← ()

Failed tests:
[FAIL. Reason: ] test() (gas: 4571)

feat: check for successful deploy in `deployCode`

create (and create2, though it's not used) return the zero address if a deploy fails. The deployCode helpers should have a check such as:

require(addr != address(0), "Deployment failed. Ensure the artifact path and file contents are correct.")

Meta: Unbundling the cheats from the assertions (leg 2)

Description

This is the 2nd leg of my proposal to unbundle the cheats from the assertions. Here I'll assume that you have read the 1st leg.

The fact that the custom cheats are added in the same namespace as the testing assertions has two inconveniences.

1. The API is inconsistent. Users need to remember that certain cheats are available only via the vm variable, while others are to be found in the default namespace of Test. For example, vm.startPrank and changePrank.
2. Namespace pollution. Users may prefer to implement their own versions of, say, the bound function. This is what happened in #92.

Solution

Instead of inheriting from Cheats in Test, we create an instance of Cheats in Test:

contract Test {
    Cheats internal cheats;
}

The vm would have to be re-instantiated in Test, but IMO that's a fair price to pay.

This way, users would know that all custom cheats offered by Forge Std are available only via cheats, and the namespace would be tidier.

Meta: suggestion to rename the "Test" contract

Description

There's a few inconvenience caused by the current name Test:

1. There's no indication that this is a test supposed to be run in Foundry/ Forge. If all you knew was that you have to inherit from a Test contract, you wouldn't have the slightest clue that you're supposed to run it via forge test.
2. Making references to it is difficult from outside the repo, since you always have to add additional context, i.e. "the Test contract from forge-std" or "forge-std's Test contract".
3. Namespace pollution. Since Test is such a generic and simple name, users may want to use it themselves.

Solution

Rename Test to one of the following options:

1. ForgeTest
2. FoundryTest
3. StdTest

Besides addressing all the inconveniences mentioned above, being explicit in the test contract name would double as a marketing gimmick. Just consider all the Solidity code snippets that skip on the import statements.

Backward Compatibility

We can implement this without introducing a breaking change by keeping the Test.sol file, importing the new ForgeTest.sol file in there, and defining a Test contract that inherits from ForgeTest:

import "./ForgeTest.sol";

/// @dev DEPRECATED: Use `ForgeTest` instead
contract Test is ForgeTest {}

Bug: Reverting test with mocked ERC20 token function call

Noticed an interesting bug. I have a test where I am mocking ERC20 token. For some reason the test always reverts at the end even though the transfer call is successful. The same thing happens for any function call from IERC20 interface.

On the other hand, mocking IERC721 and IERC1155 functions works just fine.

pragma solidity 0.8.4;

import "forge-std/Test.sol";
import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";

contract FailingTest is Test {

    function testERC20Transfer_failing() external {
       IERC20 token = IERC20(address(0xa66e7));

        vm.etch(address(token), bytes("0x01"));
        vm.mockCall(
            address(token),
            abi.encodeWithSelector(token.transfer.selector),
            abi.encodePacked(true)
        );

        token.transfer(address(0xB0B), 101e18);
    }

}

Trace:

[FAIL. Reason: Revert] testERC20Transfer_failing() (gas: 5295)
Traces:
  [5295] FailingTest::testERC20Transfer_failing() 
    ├─ [0] VM::etch(0x00000000000000000000000000000000000a66e7, 0x30783031) 
    │   └─ ← ()
    ├─ [0] VM::mockCall(0x00000000000000000000000000000000000a66e7, 0xa9059cbb, 0x01) 
    │   └─ ← ()
    ├─ [0] 0x0000…66e7::a9059cbb(0000000000000000000000000000000000000000000000000000000000000b0b00000000000000000000000000000000000000000000000579a814e10a740000) 
    │   └─ ← 0x01
    └─ ← ()            <-(reverts here)

✅ Improve test coverage

Issue

I've found tests to be less reliable when modifying code after a long time, because some functions are not covered enough. This requires manual checking to make sure everything works.

Open for anyone to take, or I'll do it later.

remove import console in Script.sol

Hi guys,

in the Script.sol, console and console2 were imported:

import "./console.sol";
import "./console2.sol";

but then if I want use console.log in the contract, I need use alias like:

import {console as clg} "forge-std/console.sol"

otherwise there would be DeclarationError (because Test.sol import Script).

I don't know why the console imported in Script.sol, and Test.sol not use that.

So I wonder if it's possible to be more autonomous, Thanks for your time

Feature request: Pin specific version/branch of dependency

Issue

Currently, when installing a specific version/branch of a dependency, this version is lost when running forge update.
Example:

forge install rari-capital/solmate@v7
forge update

=> checks out to master

Example Scenario

I currently am depending on solmate@v7's LibString.sol and have multiple of my own dependencies in my projects, which I frequently update. Every time I want to update, I currently run:

forge update
forge remove solmate
forge install rari-capital/solmate@v7

I realize that I also could just update the packages individually. Still, I would expect forge not to switch branches when updating.

Storage slots written during setUp() don't persist

Take the test contract:

contract OptionSettlementTest is DSTest, NFTreceiver {
    Vm public constant VM = Vm(HEVM_ADDRESS);
    OptionSettlementEngine public engine;

    // Tokens
    address public constant WETH_A = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    address public constant DAI_A = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
    address public constant USDC_A = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;

    // Admin
    address public constant FEE_TO = 0x36273803306a3C22bc848f8Db761e974697ece0d;

    // Users
    address public constant ALICE = address(0xA);
    address public constant BOB = address(0xB);
    address public constant CAROL = address(0xC);
    address public constant DAVE = address(0xD);
    address public constant EVE = address(0xE);

    // Token interfaces
    IERC20 public constant DAI = IERC20(DAI_A);
    IERC20 public constant WETH = IERC20(WETH_A);
    IERC20 public constant USDC = IERC20(USDC_A);

    // Test option
    uint256 public testOptionId;

    using stdStorage for StdStorage;
    StdStorage public stdstore;

    function writeTokenBalance(
        address who,
        address token,
        uint256 amt
    ) internal {
        stdstore
            .target(token)
            .sig(IERC20(token).balanceOf.selector)
            .with_key(who)
            .checked_write(amt);
    }

    function setUp() public {
        engine = new OptionSettlementEngine();

        IOptionSettlementEngine.Option memory option = IOptionSettlementEngine
            .Option({
                underlyingAsset: WETH_A,
                exerciseAsset: DAI_A,
                settlementSeed: 1234567,
                underlyingAmount: 1 ether,
                exerciseAmount: 3000 ether,
                exerciseTimestamp: uint40(block.timestamp),
                expiryTimestamp: (uint40(block.timestamp) + 604800)
            });
        testOptionId = engine.newChain(option);

        // // Now we have 1B DAI and 1B USDC
        writeTokenBalance(address(this), DAI_A, 1000000000 * 1e18);
        writeTokenBalance(address(this), USDC_A, 1000000000 * 1e6);
        // // And 10 M WETH
        writeTokenBalance(address(this), WETH_A, 10000000 * 1e18);

        WETH.approve(address(engine), type(uint256).max);
        DAI.approve(address(engine), type(uint256).max);
        USDC.approve(address(engine), type(uint256).max);

        // pre-load balances and approvals
        address[6] memory recipients = [
            address(engine),
            ALICE,
            BOB,
            CAROL,
            DAVE,
            EVE
        ];
        for (uint256 i = 0; i == 5; i++) {
            address recipient = recipients[i];
            writeTokenBalance(recipient, DAI_A, 1000000000 * 1e18);
            writeTokenBalance(recipient, USDC_A, 1000000000 * 1e6);
            writeTokenBalance(recipient, WETH_A, 10000000 * 1e18);

            VM.startPrank(recipient);
            WETH.approve(address(engine), type(uint256).max);
            DAI.approve(address(engine), type(uint256).max);
            USDC.approve(address(engine), type(uint256).max);
            engine.setApprovalForAll(address(this), true);
            VM.stopPrank();
        }
    }

    function testFeeBps() public {
        assertEq(engine.feeBps(), 5);
    }

    function testFeeTo() public {
        assertEq(engine.feeTo(), FEE_TO);
    }

    function testSetFeeTo() public {
        assertEq(engine.feeTo(), FEE_TO);
        VM.expectRevert();
        engine.setFeeTo(ALICE);
    }

    function testBalances() public {
        assertEq(DAI.balanceOf(ALICE), 1000000000 * 1e18);
    }
}

testBalances() unexpectedly fails unless a writeTokenBalance(alice, DAI_A, 1000000000 * 1e18) is added inside that function.

Tip for ERC20s that are proxies

The aave token and other ERC-20s are implemented with a proxy contract. Having the tip function or a similar cheat code also work for these tokens would be helpful 🙏

✨ Option to halt tests on failure

I asked on Discord about this and got some feedback; opening an issue to discuss if we should implement it.

Motivation

This is how tests currently work:

function testExample() public {
    assertTrue(x); // okay
    assertTrue(y); // fail
    assertTrue(z); // okay
}

// [FAIL] testExample()

z was evaluated even though y did not meet the requirements.

But say you're testing an interaction with complex contracts with lots of external calls and traces:

function testExample() public {
    c1.withdraw();
    assertEq(myBalance(), 100e18); // fail
    c2.deposit();
    assertEq(c2.availableFunds(), 100e18); // fail
    c2.borrow(); // revert
    assertApproxEqRel(dBalance(), 2500e18, 0.98e18); // not reached
}

// [FAIL. Reason: Collateral cannot be something something.] testExample()

• This would produce many traces
• Trigger multiple errors and eventually cause an unrelated revert in an external contract
• Show it as the reason why the test failed, which is confusing
• Not stop deployment scripts if a condition is not met
• Take time to fail when testing against a forked network (until everything is executed)

Solution

If this sounds good, should it be done in Forge Std or Foundry?

Foundry

Allow halt_tests, halt_on_failure or similar, in foundry.toml.

Forge Std

Add stdConfig mapping - like a mini version of foundry.toml but for Forge Std.

Then, users could access it their setUp stdConfig["halt_tests"] = false;.

Other things I'd considered:

• halts modifier, but got feedback that it's better to be a switch on the project level

This is how we would do it: dapphub/ds-test#40

Example

function testExample() public {
    c1.withdraw();
    assertEq(balance(), 100e18); // fail => halt
    c2.deposit(); // not reached
    assertEq(c2.availableFunds(), 100e18); // not reached
    c2.borrow(); // not reached
    assertApproxEqRel(dBalance(), 2500e18, 0.98e18); // not reached
}

[FAIL. Reason: Halted!] testExample()
Logs:
  Error: a == b not satisfied [uint]
    Expected: 100000000000000000000
      Actual: 50000000

Feature request: cheatcode to expect ETH balance change

In Waffle, there is a chai matcher for testing whether a contract call changes the ETH balance of an account:

changeEtherBalance

Which can be used like this:

await expect(() => wallet.sendTransaction({to: walletTo.address, value: 200}))
  .to.changeEtherBalance(walletTo, 200);

await expect(await wallet.sendTransaction({to: walletTo.address, value: 200}))
  .to.changeEtherBalance(walletTo, 200);

It would be nice to have an equivalent VM cheatcode for this.

Support for storage find in Vyper-compiled contracts

Seems like you guys are using the same trick we are to scan storage for arbitrary variables. One issue I ran into when adding this to spells-mainnet for Maker is that Vyper uses an inverted format for mappings as you can see here. Might be worthwhile to support Vyper contracts with find as it only doubles the search space.

✨ ffi(string)

Motivation

Improve the developer experience of the ffi cheatcode by allowing typing the command as the argument.

Solution

/// SUPPORT THE CURRENT WAY

string memory cmds = new string[](2);
cmd[0] = "cat";
cmd[1] = "1337.txt";

ffi(cmds);


/// IMPROVE DX

ffi("cat 1337.txt");


/// EASY TO CONCAT

ffi(string.concat("cat ", toString(1337), ".txt");

// or
string memory leet = toString(1337);
ffi(string.concat("cat ", leet, ".txt");

_{toString from OZ}

I'll implement this if it's a good idea.

Edit

Not sure how great because of dealing with spaces.

E.g. Can use double space to indicate that there's a space, as in

echo "Gm,  world"

But if there is a string var, it would require calling a function to correct it first.

Will close for now.

Add forge-std and ds-test as NPM modules for Hardhat backwards compatibility

Component

Forge

Describe the feature you would like

Issue described in detail here.

Many of use would like to use Foundry + Hardhat, which still has tons of use cases (integration testing, complex deployment scripts, etc) and should really not be treated as a direct competitor. However, there are some small inconveniences that we have found when trying to do this.

As shown in the issue, it can almost be tidied up into one source of truth for all dependencies by mapping the paths the Forge uses to node_modules. But since ds-test and forge-std do not have NPM modules, you still need to maintain a ./lib directory and remappings.txt.

The simplest fix would probably be to wrap these dependencies in NPM modules. This probably has use-cases outside just the one described above, but would help immensely for all the Hardhat folks that want to give Foundry a go.

Thanks! 🙏

Additional context

No response

Meta: Upstreaming the assertions to PRBTest

This discussion is contingent on bumping the pragma to >=0.8.0 <0.9.0.

Building up on the arguments I have made in Why Choose PRBTest Over DSTest?:

1. Switching to PRBTest would free Forge Std of the path dependence problem that plagues DSTest (that is, all DappHub projects depend upon DSTest, so updating it is painful to orchestrate). PRBTest is a brand new project, and I have designed it from the get go to never suffer from path dependence.
2. Letting PRBTest handle all testing assertions would take a burden off the shoulders of Forge Std. Forge Std could instead focus on its juiciest features, such as StdStorage, and could continue "moving fast and breaking things" knowing that unless PRBTest is explicitly version bumped, the assertion syntax will remain the same.
3. Forge Std wouldn't have to maintain the Vm interface anymore.
4. Forge Std wouldn't have to maintain the tests in StdAssertions.t.sol anymore.
5. PRBTest has a more permissive license than DSTest. MIT versus GPL v3.

Finally, on a personal level, Foundry has become my primary development framework. It's what I recommend to anyone looking to jump into Ethereum development. I've also recently become a frequent contributor to Forge Std and the Foundry Book. Upstreaming the assertions to PRBTest would make us both more agile.

`deployCodeTo` function

It would be nice to have a deployCodeTo(address target, string memory what, bytes memory args) function that deploys the what contract to the target address. I think this could be possible by using CREATE to deploy the contract and then finding storage slots that were set during the contract deployment and then using vm.etch to set the deployed bytecode to the target address and then vm.store to set each storage slot that was changed.

Optimism has "predeploys" and this functionality would be very useful for setting up the predeploys at the correct addresses. This could also be useful for forked networks to see how changing a contract's code would impact execution when the contract is not upgradable.

Some additional thoughts:
An edge case to take into account is if during the constructor of the contract deployment, it calls an initialize function on another contract with address(self) which then sets the value in storage. Would be complex to handle the case that deploys a contract and passes in address(self) as an immutable value.

Unused local variable "bytes memory b"

After running the steps mentioned in the Working on an Existing Project chapter in the Foundry Book, I got the following warnings emitted in the console:

Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
   --> /Users/paulrberg/workspace/pad/forge-std/src/test/StdStorage.t.sol:252:5:
    |
252 |     constructor() public {
    |     ^ (Relevant source part starts here and spans across multiple lines).



Warning: Unused local variable.
   --> /Users/paulrberg/workspace/pad/forge-std/src/test/StdError.t.sol:106:9:
    |
106 |         bytes memory b = someBytes;
    |         ^^^^^^^^^^^^^^

As per #19, it looks like the first one is expected. But it looks like the second one may be not.

Is the b variable purposefully left unused, or is this an issue that should be resolved?

`stdStorage` unable to find `bool` slot within the map struct variable

Hey guys,

Seems like stdStorage unable to find bool slot within a map struct variable. I debug and found out that the rdata not return hex"1337" if address.staticcall to the bool slot. The whole bytes32 just become 0000000000000000000000000000000000000000000000000000000000000000 as you can see below:

You can reproduce with this contract: https://github.com/spicysquid168/solidity-playground/blob/main/src/TryStdStorageBool.sol
Run by forge script ./src/TryStdStorageBool.sol -vvv

Thanks,

#51 breaks backwards compatibility before `0.8.0`

Description

Recently #51 was merged and it included an unchecked in the stdMath.abs function, which requires to bump the Solidity version for the forge-std package to be pragma solidity >=0.8.0 <0.9.0; instead of pragma solidity >=0.6.0 <0.9.0;

I'm myself running into an issue here since I wanted to test a 0.6.0 contract and this was broken. You either will need to remove the unchecked or bump the version.

Meta: New category StdUtils

Description

When I was first told about the bound function, I had immediately assumed that it's some flavor of assume. So I had thought that I can use it like this:

vm.bound

Of course, I had then learned what its purpose is and how it should be used.

Now, looking at its source code, I can see that there is neither a call to vm, nor a call to any other cheat defined in Test. So it doesn't seem to me like this is a "cheat" - it's more of like a utility function.

Solution

1st Proposal

Add a STD-UTILS delineator at the bottom of the Test contract:

/*//////////////////////////////////////////////////////////////////////////
                                STD-UTILS
//////////////////////////////////////////////////////////////////////////*

Move the bound function underneath it, and write a new test file StdUtils.t.sol in which to move the following tests:

• testBound
• testBoundUint256Max
• testCannotBoundMaxLessThanMin

2nd Proposal

Create a new StdUtils contract and move bound there. Test would then inherit from StdUtils.

[FAIL. Reason: stdStorage find(StdStorage): Slot(s) not found.] (potential bug)

Hello I opened a discussion here: foundry-rs/foundry#2188 but was recommended to open an issue as there may be a bug.

I created a mock contract to simulate Property Management. Please see below:

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

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/utils/Counters.sol";

contract PropertyManagement {

    address payable private propertyManager;
    address payable private tenant;

    using Counters for Counters.Counter;
    Counters.Counter private _propertyId; 
    Counters.Counter private _agreementId; 

    struct Property {
        uint256 propertyId;
        uint256 rent;
        uint8 bedrooms;
        uint8 bathrooms;
        address _tenant;
        bool vacant;
    }

    struct Agreement {
        Property property;
        uint256 agreementId;
        uint256 duration;
        address _tenant;
        address _propertyManager;
    }

    mapping(uint => Property) public propertyById;
    mapping(uint => Agreement) public agreementById;

    constructor(address _propertymanager) {
        propertyManager = payable(_propertymanager);
    }

    receive() external payable {}

    modifier onlyPropertyManager {
        require(msg.sender == propertyManager, "You're not the property manager.");
        _;
    }

    function createProperty(uint256 _rent, uint8 _bedrooms, uint8 _bathrooms) public onlyPropertyManager {
        _propertyId.increment();
        uint256 setPropertyId = _propertyId.current();
        Property storage property = propertyById[setPropertyId];
        property.propertyId = setPropertyId;
        property.rent = _rent;
        property.bedrooms = _bedrooms;
        property.bathrooms = _bathrooms;
        property.vacant = true;
    }

    function createAgreement(uint256 _pId, uint256 _duration, address _tenant, uint256 _amount) public payable onlyPropertyManager {
        Property memory property = propertyById[_pId];
        require(property.rent == _amount, "Please provide correct amount.");
        require(_amount <= msg.sender.balance, "You dont have enough money.");

        _agreementId.increment();
        uint256 setAgreementId = _agreementId.current();
        Agreement storage agreement = agreementById[setAgreementId];
        agreement.property = property;
        agreement.agreementId = setAgreementId;
        agreement.duration = _duration;
        agreement._tenant = _tenant;
        agreement._propertyManager = msg.sender;

        payable(address(this)).transfer(_amount);
        Property storage rental = propertyById[_pId];
        rental.vacant = false;
        rental._tenant = _tenant;
    }

}

Two of my tests are below:

function testCheckTenant() public {
        vm.prank(address(1));
        propertyManagement.createProperty(1000, 3, 3);
        vm.prank(address(1));
        propertyManagement.createAgreement{value: 1000}(1, 1, tenant, 1000);
        address slot = stdstore
            .target(address(propertyManagement))
            .sig(propertyManagement.agreementById.selector)
            .with_key(1)
            .depth(2)
            .read_address();
            
        emit log_address(slot);
        //assertEq(address(uint160(uint(vm.load(address(propertyManagement), slot)))), tenant);

    }

function testCreateProperty() public {
        vm.prank(address(1));
        propertyManagement.createProperty(1000, 4, 3);
        uint256 slot = stdstore
            .target(address(propertyManagement))
            .sig(propertyManagement.propertyById.selector)
            .with_key(1)
            .depth(2)
            .find();

        
        assertEq(uint(vm.load(address(propertyManagement), bytes32(slot))),4);
    }

I am unable to access slots past slot 0 and 1 for testCreateProperty() and have trouble accessing any of my needed slots on testCheckTenant(). I was told that Forge may have difficulties with packed structs like my Property struct, but also having issues with the Agreement struct - maybe in part due to the fact that I store the Property struct in my Agreement struct.

I did notice when I was doing my test yesterday that when i switched the bedrooms field in Property from uint8 to uint256 that it did work so that does seem to be part of the problem. However, it is not only the Property struct it's also the Agreement struct that I cannot access.

Appreciate the guidance on this and interested to know if this is a bug with forge-std. Thanks!

Feature request: adding console.log(string,bytes32)

I often find myself needing to log bytes32 and currently this only works with console.logBytes32.
Is there a possibility of extending console to add in console.log(bytes32) or console.log(string,bytes32)?

Meta: missing contributor guide

Description

It's not clear what commands should contributors run in order to:

1. Build the contracts
2. Test the contracts

It would be very useful to provide some basic guidance on this, especially since this code base uses a broad Solidity pragma (>=0.6.0 <0.9.0) and some contracts use very different pragmas (e.g. STDError which uses >=0.8.10 <0.9.0).

This is in part why contributors like @devanonon couldn't see any problem with using type().max in commit 914702ae - there's no guidance on how to backtest new features against older compiler versions.

Solution

Add a "Contributing" section in the README or a CONTRIBUTING.md file.

Side note: perhaps it would also be worth it to define a foundry.toml configuration file with the defaults that should be used when compiling and testing?

Add tests for `assertApproxEqRel`, `assertApproxEqAbs`

See assertApproxEqRel and assertApproxEqAbs functions in Test.sol.

Create a new file in src/test/.