bug: mockCall not working as expected

[mds1]

The tests in CheatCodes.sol passed, however the test below fails. Tagging @onbjerg here since you implemented this feature originally in #403

pragma solidity 0.8.10;

import "ds-test/test.sol";

interface Vm {
  function mockCall(address,bytes calldata,bytes calldata) external;
}

contract MyContract {
  function outer() public returns (uint256) {
    return inner();
  }

  function inner() public returns (uint256) {
    return 5;
  }
}

contract MyContractTest is DSTest {
  MyContract c;
  Vm vm = Vm(address(bytes20(uint160(uint256(keccak256('hevm cheat code'))))));

  function testMock() public {
    c = new MyContract();
    assertEq(c.inner(), 5);
    assertEq(c.outer(), 5);

    uint256 _newVal = 10;
    vm.mockCall(address(c), abi.encodeWithSelector(c.inner.selector), abi.encode(_newVal));

    assertEq(c.inner(), _newVal); // passes
    assertEq(c.outer(), _newVal); // fails
  }
}

[onbjerg]

Maybe the documentation was a bit confusing - outer is not making a call in the EVM sense of the word, it is just jumping to a new location in the code of the contract. mockCall only works for calls between accounts. I think my usage of the word "internal call" (by which I mean a contract calling another contract) might have lead to that confusion?

[mds1]

Ah that makes sense. Changing return inner() to return this.inner() does result in the test passing.

However as a user I'd expect (and personally would like) the above usage to pass, even though we're jumping to inner() and not calling. Having inner() return different values based on whether it was a call vs. jump seems undesirable

[onbjerg]

Hmm, this is going to be pretty difficult. We can get the locations of different calls in the bytecode using functionDebugData from the Solidity compiler JSON output, but the names are a bit mangled (something like @<actual name>_<some id>) and we would also have to hook into jump instructions which I am not sure we are equipped to right now (think we would have to hook here?)

I definitely agree that it makes sense, though.

Just noting here for the future (I don't have much bandwidth currently):

• We need to parse functionDebugData from the Solidity compiler and build a mapping of locations to symbols
• The mockCall (and expectCall?) cheatcodes need to determine whether we are dealing with an actual call or a jump and store that information accordingly
• We need to hook into CheatcodeStackExecutor::pre_validate and check if we are dealing with a jump instruction and if we are dealing with a known location we want to mock, and then we need to somehow return our mocked data by skipping the code section at the jump destination and just returning the mocked data - not sure how we are going to do this since that seems to be in Machine which is nested inside Runtime which is instantiated in the Sputnik executor internals

I'm also not sure if some of these functions are inlined if they are internal(?)

[onbjerg]

Very briefly skimming it looks like it would be a lot easier if we used revm, but I'm not sure what the drawbacks are

[gakonst]

We intend to switch over to Revm eventually, is the problem that hooking so deeply into sputnik can be hard, whereas revm's inspectors are cleaner?

[onbjerg]

Yeah, it's pretty much impossible for us (as far as I can tell) to hook into the actual stepping of the EVM itself with Sputnik, but revm inspectors solve this by giving us full control

[mds1]

Going to close this feature request, and we can always re-open if needed. IME I have not had the need to do this recently, so closing

[avniculae]

Going to close this feature request, and we can always re-open if needed. IME I have not had the need to do this recently, so closing

@mds1 Would you mind sharing how can one achieve stateless unit tests with this mockCall limitation and without implementing additional mocked contracts?

[mds1]

@avniculae Can you expand on what you’re trying to do? I don’t understand exactly what you’re looking for :)

[avniculae]

@mds1 Building on your example above, I was wondering how to best test the outer() function given this mockCall limitation? Think about a scenario where the inner() function would actually represent an internal function of a library, which heavily depends on state; in this scenario, I would like to just be able to mock all calls to the inner() function, be them internal or external. The alternative that I see is to build mock contracts where you can easily set the state of the contracts (ie values of variables) such that you can know in advance the effects and result of inner(); however, I don't find this scalable in complex projects.

[mds1]

It depends on the specifics of your use case. Some ideas include:

• Make an external call to the library that way you can mock the response
• Use vm.store or forge-std's stdStorage library to update storage slots
• Use vm.etch to place arbitrary code at the library's address

[avniculae]

Thanks @mds1, this is helpful! The vm.etch solution is interesting -- I'm thinking one can get the code of the contract and replace the relevant portion of the bytes with bytes corresponding to the mocked function? Have you seen something like this before?

[mds1]

Yep exactly, if the contract is one local to your project it's easy to make a patched version. If it's someone else's, e.g. uniswap, typically you'd clone their repo, patch the code, compile, and copy out the resulting bytecode into your project. I don't know of any examples offhand to link to, but if you search the foundry issues or telegram channels you might find some

[Thomas-Smets]

@avniculae We did something similar to fuzz the state of Liquidity Positions of Uniswap V3.
Basically we created Extension contracts of the original uniswap-v3-core repo's with setters for state.
And next we used vm.etch to 'deploy' the Extended Uniswap contracts where we can fuzz the full state.

Might help you as an example:
https://github.com/arcadia-finance/accounts-v2/tree/main/test/utils/fixtures/uniswap-v3

Only for constants in the contract this is annoying, since you can't simply override them.
We had to do some manipulation of the bytecode tho before using vm.etch, to replace constants in the original contracts.

[frank-lim-partior]

FYI for anyone looking trying to use this, mocking inner() works for outer2 and outer3 as of today.

pragma solidity ^0.8.13;

interface WithInner {
    function inner() external view returns(uint256);
}

contract MyContract {

    function inner() public view returns (uint256) {
        return 1;
    }
    
    /// @dev returns real value, vm.mockCall does not work
    function outer1() public view returns (uint256) {
        return inner();
    }

    /// @dev returns vm.mockCall value
    function outer2() public view returns (uint256) {
        return this.inner();
    }

    /// @dev returns vm.mockCall value
    function outer3() public view returns (uint256) {
        return WithInner(address(this)).inner();
    }
}

[clauBv23]

Ending here after going over the expectCall foundry's doc and seeing this note:

ℹ️ Internal calls
This cheatcode does not currently work on internal calls. See issue #432.

While this issue doesn't seem to be specifically about expectCall,
I'm wondering if there's a workaround for expecting internal calls? Or are there any plans to add this feature in the future?