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

Invariant test fuzzed functions always use msg.value == 0 #8449

Closed
2 tasks done
Melvillian opened this issue Jul 16, 2024 · 4 comments
Closed
2 tasks done

Invariant test fuzzed functions always use msg.value == 0 #8449

Melvillian opened this issue Jul 16, 2024 · 4 comments
Labels
T-bug Type: bug

Comments

@Melvillian
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 (ea7817c 2024-07-16T00:18:48.894654000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

tl;dr: It looks like both from personal testing with console.log on my invariant tested contracts, and from reading the forge Rust code, that for invariant tests the fuzzed function calls always use a value of 0 for msg.value. Is this true? And if so, is there a way to change this via some config value? My code under test needs non-zero values for msg.value, otherwise nothing interesting will happen in my tests.

Context:
I am writing a simple kickstarter clone app here: https://github.com/Melvillian/crowdfund-project. If you clone that repo and run forge test --match-test invariant -vvvvv, you will find in the console.log'ed output that the msg.value parameter of every call to the TestProject.contribute function is equal to 0. This is a problem because for TestProject.contribute to work it needs to have a non-zero value for msg.value.

From perusing the forge code, it looks like passing 0 for msg.value is the intended result, based on this line here. Note, since there are several functions that look like they could be the one that actually calls the function that fuzzes the contract, I'm not sure if that is the correct line for where the msg.value == 0 is set, so correct me if I'm wrong.

@Melvillian Melvillian added the T-bug Type: bug label Jul 16, 2024
@grandizzy
Copy link
Collaborator

grandizzy commented Jul 16, 2024

hey @Melvillian you can use a handler based approach for this, see https://book.getfoundry.sh/forge/invariant-testing#handler-based-testing Basically you fuzz the handler selector with value and then call your desired target. I will create a simple sample on how to do that and post here

@grandizzy
Copy link
Collaborator

@Melvillian

click here for a sample on how to accomplish this
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {Project} from "../src/Project.sol";
import {TestProject} from "../test/TestProject.sol";
import {ProjectFactory} from "../src/ProjectFactory.sol";
import {ERC721TokenReceiver} from "solmate/tokens/ERC721.sol";

contract ProjectTestHandler is Test {
    TestProject public proj;

    constructor(TestProject _proj) {
        proj = _proj;
    }

    function contribute(uint256 amount) public {
        // Keep amount contributed between 0.1 and 1 ether
        amount = bound(amount, 0.1 * 1e18, 1 * 1e18);
        proj.contribute{value: amount}();
    }
}

contract ProjectTest is Test, ERC721TokenReceiver {
    TestProject public proj;
    string public constant TOKEN_NAME = "Test Token";
    string public constant TOKEN_SYMBOL = "TST";

    function setUp() public {
        proj = new TestProject(TOKEN_NAME, TOKEN_SYMBOL, 10 ether, address(this));

        ProjectTestHandler handler = new ProjectTestHandler(proj);
        targetContract(address(handler));

        // Fund test handler with ETH
        vm.deal(address(handler), 100000 ether);
    }

    function invariant_send_msg_value() public {
        assertEq(
            proj.summedBalances(address(this)) / 1 ether, proj.balanceOf(address(this)) + proj.nftsOwed(address(this))
        );
    }

    fallback() external payable {}
}

If you run forge test --mt invariant_send_msg_value -vvvv you will see traces like

  [42133] ProjectTestHandler::contribute(3403)
    ├─ [0] console::log("Bound result", 900000000000003404 [9e17]) [staticcall]
    │   └─ ← [Stop] 
    ├─ [26561] TestProject::contribute{value: 900000000000003404}()
    │   ├─ [0] console::log("msg.value", 900000000000003404 [9e17]) [staticcall]
    │   │   └─ ← [Stop] 
    │   ├─ emit Contributed(contributor: ProjectTestHandler: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], amount: 900000000000003404 [9e17], newNfts: 1)
    │   └─ ← [Stop] 
    └─ ← [Return] 

or

  [21424] ProjectTestHandler::contribute(3167343262926237667357408146846597089315997 [3.167e42])
    ├─ [0] console::log("Bound result", 254706333018384733 [2.547e17]) [staticcall]
    │   └─ ← [Stop] 
    ├─ [5915] TestProject::contribute{value: 254706333018384733}()
    │   ├─ [0] console::log("msg.value", 254706333018384733 [2.547e17]) [staticcall]
    │   │   └─ ← [Stop] 
    │   └─ ← [Revert] ProjectAlreadyFunded()
    └─ ← [Revert] ProjectAlreadyFunded()

What's nice about this approach is that in handler's contribute function you can use cheatcodes to control what you send, like bound (to make sure value sent in certain range), or vm.assume, vm.prank to control the msg.sender, deal to make sure account properly funded, etc.

Be aware that test won't fail when revert like ProjectAlreadyFunded above happens but only if you specify

[invariant]
fail_on_revert = true

in your foundry.toml file.

Lmk if this clarifies and good to close the issue. thank you

@Melvillian
Copy link
Author

Melvillian commented Jul 16, 2024

Thank you for the detailed and functional fix @grandizzy! I made some slight tweaks to your suggested code and pasted my working code below for future devs who're tripped up by this problem. The main insight you gave me is that: yes, the msg.value always defaults to 0 in Invariant tests, but that's because you're always able to randomize them by using a Handler contract that implements Test and wrap the function you want to test in the Handler's own fuzzed function.

Click me for the code I used to setup fuzzed `msg.value` values on my tested code
// SPDX-License-Identifier: UNLICENSED
    pragma solidity ^0.8.24;
    
    import {Project} from "../src/Project.sol";
    import {Test, console} from "forge-std/Test.sol";
    import {ERC721TokenReceiver} from "solmate/tokens/ERC721.sol";
    
    /**
     * @title TestProjectHandler
     * @dev A contract that adds some ghost variables to allow for invariant testing
     */
    contract TestProjectHandler is Test {
        // this value does not decrease when withdrawEth is called, which allows us
        // to efficiently calculate the number of nfts a user is due even after they
        // are claimed
        mapping(address => uint256) public summedBalances;
        Project public project;
    
        constructor(Project _proj) {
            project = _proj;
        }
    
        function contribute(uint256 amount) public payable {
            amount = bound(amount, project.MIN_CONTRIBUTION(), 10 ether);
            project.contribute{value: amount}();
            summedBalances[address(this)] += amount;
        }
    
        function claimNfts() public {
            project.claimNfts();
        }
    }

    contract ProjectTest is Test, ERC721TokenReceiver {
        TestProjectHandler public projHandler;
        Project public proj;
        string public constant TOKEN_NAME = "Test Token";
        string public constant TOKEN_SYMBOL = "TST";
    
        function setUp() public {
            proj = new Project(TOKEN_NAME, TOKEN_SYMBOL, 10 ether, address(this));
            projHandler = new TestProjectHandler(proj);
    
            targetContract(address(projHandler));
    
            // Fund test handler with ETH
            vm.deal(address(projHandler), 100000 ether);
    
            FuzzSelector memory fuzzSelector = FuzzSelector({addr: address(projHandler), selectors: new bytes4[](1)});
            fuzzSelector.selectors[0] = TestProjectHandler.claimNfts.selector;
            targetSelector(fuzzSelector);
    
            fuzzSelector = FuzzSelector({addr: address(projHandler), selectors: new bytes4[](1)});
            fuzzSelector.selectors[0] = TestProjectHandler.contribute.selector;
            targetSelector(fuzzSelector);
        }
    
        function invariant_num_nfts_claimable_and_held_by_user_always_matches_nfts_minted() public {
            assertEq(
                projHandler.summedBalances(address(projHandler)) / 1 ether,
                proj.balanceOf(address(projHandler)) + proj.nftsOwed(address(projHandler))
            );
        }
    }

@Melvillian
Copy link
Author

Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
Development

No branches or pull requests

2 participants