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

extcodesize check bypass for mockCall #1987

Closed
jcmonte opened this issue Jun 15, 2022 · 6 comments · Fixed by #2576
Closed

extcodesize check bypass for mockCall #1987

jcmonte opened this issue Jun 15, 2022 · 6 comments · Fixed by #2576
Assignees
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge Cmd-forge-test Command: forge test T-feature Type: feature

Comments

@jcmonte
Copy link

jcmonte commented Jun 15, 2022

Component

Forge

Describe the feature you would like

Adding an extcodesize check bypass when mocking functions. Functions that have no return data are still subject to an extcodesize check (see ethereum/solidity#12204). This makes it challenging when working with external integrations in a non forked environment (mocked calls still revert because extcodesize == 0).

Currently, my local solution is to fill junk bytes at a target address before mocking:

function mockCall(address where, bytes memory data) internal {
        cheats.etch(where, "0xdeadbeef");
        cheats.mockCall(where, data, abi.encode());
}

It would be nice to have this feature integrated into foundry that way mockCall covers all possible scenarios. Just not sure if my solution is ideal because it leaves junk bytecode at addresses.

Additional context

No response

@jcmonte jcmonte added the T-feature Type: feature label Jun 15, 2022
@onbjerg onbjerg added this to Foundry Jun 15, 2022
@onbjerg onbjerg moved this to Todo in Foundry Jun 15, 2022
@mds1
Copy link
Collaborator

mds1 commented Jun 16, 2022

Check out all the discussion in foundry-rs/forge-std#46 around how foundry should handle mocking, including questions about what should be upstreamed to foundry and what should be part of forge-std. The discussion has stalled, but would love your input there, as this is one of things covered.

@onbjerg
Copy link
Member

onbjerg commented Jun 18, 2022

I'm not entirely sure how we can solve this specific issue in Forge, I'd love some input. Here are some different approaches and the issues I see with them:

  • We could write junk data to the address ourselves and reset it after - however, this makes the contract completely unuseable for anything than the mocked function, which is obviously not ideal.
  • We could try to bypass extcodesize checks smartly, but this requires us to basically look ahead and do some "fake" execution every time we hit an extcodesizecheck to see if we're about to call a mocked function.

It seems like your scenario is using mockCall on an empty address which is why you're having this issue, but a lot of people also use mockCall on an address that has a bytecode, where they only mock one or two functions that should still work as intended.

For your use case specifically, can I suggest doing the vm.etch in setUp? That should clean up your test a bit, I assume.

@onbjerg onbjerg added Cmd-forge-test Command: forge test C-forge Command: forge A-cheatcodes Area: cheatcodes labels Jun 18, 2022
@jcmonte
Copy link
Author

jcmonte commented Jun 21, 2022

Thanks @mds1, must have missed it the first time around, but I'll take a look at the discussion there!

And @onbjerg thanks for response. My suggestions would either be:

  • Improve error messaging to show a call has failed due to an extcodesize check. (currently just reverts without any message)
  • Write junk data to an address when mocking if we detect no code at an address. Really there shouldn't be a scenario where mocking fails.

For your use case specifically, can I suggest doing the vm.etch in setUp? That should clean up your test a bit, I assume.

Yeah this is a good call, we will do this for now.

@sendra
Copy link

sendra commented Aug 3, 2022

How is the state of this issue? Encountered the same problem when trying to mock a function that does not return.
Was able to solve it with the solution used by @JChiaramonte7 adding vm.etch(where, '0xdeadbeef'); on setUp, but would be nice to have this option already included on mockCall for the cases where there are no return params

@onbjerg
Copy link
Member

onbjerg commented Aug 3, 2022

Hasn't been a priority because there is so much else to look at. I think the solution is pretty simple, so I'll take a look at it now since it was bumped

@onbjerg onbjerg self-assigned this Aug 3, 2022
@onbjerg onbjerg moved this from Todo to In Progress in Foundry Aug 3, 2022
@onbjerg
Copy link
Member

onbjerg commented Aug 3, 2022

PR is up, should be good for the nightly tonight :)

Repository owner moved this from In Progress to Done in Foundry Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge Cmd-forge-test Command: forge test T-feature Type: feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants