foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.28k stars 1.75k forks source link

Cheatcode `expectRevert` doesn't catch error messages from libraries #4405

Closed pedrommaiaa closed 1 month ago

pedrommaiaa commented 1 year ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.2.0 (b44b045 2023-02-21T00:22:41.521901Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

Helper Contract:

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

import {ERC20} from "solmate/tokens/ERC20.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";

contract Helper {
    ERC20 public token;

    constructor(ERC20 _token) { token = _token; }

    function doSafeTransfer(address to, uint256 amount) external {
        SafeTransferLib.safeTransfer(token, to, amount); 
    }
}

In the example bellow testRevertTransfer is expected to be successful as it was supposed to catch the error message from the library but in reality it doesn't.

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

import {Test} from "forge-std/Test.sol";
import {Helper} from "../src/Helper.sol";
import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";

contract LibraryTest is Test {
    Helper public helper;
    MockERC20 public token;

    function setUp() public {
        token = new MockERC20("MockToken", "TST", 18);
        helper = new Helper(token);
    }

    // This doesn't work.
    function testRevertTransfer() external {
        vm.expectRevert(bytes("TRANSFER_FAILED"));
        SafeTransferLib.safeTransfer(token, address(0xBEEF), 1e18);
    }

    // This works fine.
    function testRevertHelperTransfer() external {
        vm.expectRevert(bytes("TRANSFER_FAILED"));
        helper.doSafeTransfer(address(0xBEEF), 1e18);
    }
}

Repo with reproducible error: https://github.com/pedrommaiaa/Foundry-Library-Error Issue also opened in the forge-std repo: https://github.com/foundry-rs/forge-std/issues/306

ssadler commented 1 year ago

As a workaround, you can define a wrapper for your pure function and call it using this in your test:


contract Tests {
  function testThing() public {
    vm.expectRevert("...");
    this.wrapPure();
  }
  function wrapPure() public { MyLib.wrapPure(); }
}
ssadler commented 1 year ago

I have an example where it's not catching a revert but it's not a library, instead it's an abstract contract.

The wrapper trick above works also in this case.

highskore commented 1 year ago

I have an example where it's not catching a revert but it's not a library, instead it's an abstract contract.

The wrapper trick above works also in this case.

Just faced a similar issue with an abstract contract and I managed to solve it using this method

grandizzy commented 1 month ago

expectRevert cheatcode expects an revert from the next external call, so in the original issue

    // This doesn't work.
    function testRevertTransfer() external {
        vm.expectRevert(bytes("TRANSFER_FAILED"));
        SafeTransferLib.safeTransfer(token, address(0xBEEF), 1e18);
    }

fails because SafeTransferLib is inlined in contract. To check behavior you can change the signature of SafeTransferLib.safeTransfer (lib/solmate/src/utils/SafeTransferLib.sol) to public instead internal and test is going to pass.

Going to close this one and track it with https://github.com/foundry-rs/foundry/issues/5367 as there is a broader discussion about. thank you!