Mocking SafeERC20 / calls to `functionCall()`

[flux627]

From what I understand from the Doppleganger code, it mocks function calls by hashing msg.data and creating a hashmap of the result to the mocked function. How do I predictably do this for library methods, and specifically, the SafeERC20 library? At first I tried loading the artifact mocking like this (using Typescript + Hardhat):

const erc20Artifact: Artifact = await hre.artifacts.readArtifact("SafeERC20");
const erc20: MockContract = await hre.waffle.deployMockContract(deployer, erc20Artifact.abi);
await erc20.mock.safeApprove.withArgs(someAddress, Zero).returns()

However after getting the following error,

TypeError: Cannot read property 'withArgs' of undefined

I decided to check the artifact file for SafeERC20. It has no ABI:

{
  "_format": "hh-sol-artifact-1",
  "contractName": "SafeERC20",
  "sourceName": "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol",
  "abi": [],
  "bytecode": "0x602d6037600b82828239805160001a607314602a57634e487b7160e01b600052600060045260246000fd5b30600052607381538281f3fe73000000000000000000000000000000000000000030146080604052600080fdfea164736f6c6343000806000a",
  "deployedBytecode": "0x73000000000000000000000000000000000000000030146080604052600080fdfea164736f6c6343000806000a",
  "linkReferences": {},
  "deployedLinkReferences": {}
}

So then I thought, ok, I probably need to mock the underlying methods the library is calling. When investigating the underlying methods for SafeERC20, I discovered that it's encoding the data manually and calling functions using the low-level functionCall() method (see here).

it's not clear to me how to correspond these manual calls to the needed data hash, nor is it clear how to use the current Waffle API to deploy the mock contract to capture calls to the correct function calls. I'm not sure if this is a lack of my own understanding or if it's a current limitation of the mock contract + Waffle.

Any advice as to how I should proceed is appreciated, also if there is another approach to mocking / unit testing that avoids this problem completely.

[chanhosuh]

await erc20.mock.safeApprove.withArgs(someAddress, Zero).returns()

should be

await erc20.mock.approve.withArgs(someAddress, Zero).returns()

same is true for the other SafeERC20 calls like safeTransfer etc.

Think of it this way: inside your smart contract, when you do something like using SafeERC20 for IERC20, you have now "wrapped" your IERC20 interface with SafeERC20. Your smart contract code now uses your wrapper function (the "safe" variant), which then calls the unwrapped function. The unwrapped function is what you need to mock since that's what is being called externally.

[chanhosuh]

As a funny aside, this dispatching does wreak havoc if you use revertsWithReason instead of returns, since the wrapper will intercept the revert reason and include its own "SafeERC20: low-level call failed".

[JaredBorders]

I am having a similar issue and I tried what @chanhosuh said to do but it still is not working... @flux627 did this solution work for you?

If you could spare a minute, please take a quick look at this swapInto() function and then it's respective test here.

The error I see: 1) SynthSwapShould execute swapInto(): TypeError: Cannot read property 'withArgs' of undefined at Context.<anonymous> (test/SynthSwap.ts:42:43) (...)

[chanhosuh]

The gist of my explanation is that you should just do the naive thing and forget you are using SafeERC20. It's irrelevant for the purpose of mocking what library you use to call an external contract, since there's no way it can "know" you're using a library to wrap the calls. The mock needs to have the ABI for the external contract, which means you use a regular IERC20 interface for the artifact and you mock the calls using that ABI.

In particularawait hre.artifacts.readArtifact("SafeERC20"); is wrong, etc. Just forget all about SafeERC20 and everything is fine.

[chanhosuh]

As a funny aside, this dispatching does wreak havoc if you use revertsWithReason instead of returns, since the wrapper will intercept the revert reason and include its own "SafeERC20: low-level call failed".

Actually, thankfully they fixed this in a newer version of Waffle maybe a few months back. So now the revert reason bubbles up. FYI, so as to not mislead.

[jcmonte]

@chanhosuh Is there a working example of this anywhere that we can look at?

Running into similar issues using low-level solidity calls. Even after trying to mock the underlying contract I'm greeted with "Mock on the method is not initialized".

In my case I'm just testing a function that forwards transaction data to be called later within the function.

[chanhosuh]

@JChiaramonte7 you're really asking a different question than this issue is about, but I'll answer it, because it's an interesting issue that has also bit me in the past :D

The Waffle mock contract (cf Doppelganger.sol) uses storage to maintain the initialized mock data. You're doing a delegatecall to the mock contract, which means you are now trying to exercise the Doppelganger code with the storage of the caller (your SynthSwap contract). So it's going to fail :'(

In my case, I just ended up creating my own pair of mock contracts for the caller and callee. I had them both inherit a common storage layout. There are pros and cons of that approach (including whether you could end up borking the code you are testing somehow), but in my case, it made sense and didn't introduce additional complications.

[jcmonte]

Ah good catch. That's exactly what's happening. I'm glad you ran into this too.

And thanks! I'll take a shot at your recommendation.

[simon-something]

To add to the original issue of @flux627 and @JaredBorders, one should note that safeApprove add a second call, to ierc20.allowance (this is a second mock which must be initialised and can be easily missed, while the hardhat trace will not provide useful info)

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/98c3a79b5765d58ef27856b8211c70a4907c63be/contracts/token/ERC20/utils/SafeERC20.sol#L55