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

Disallow some cheatcodes for precompiles #1159

Closed
onbjerg opened this issue Mar 31, 2022 · 3 comments · Fixed by #4905
Closed

Disallow some cheatcodes for precompiles #1159

onbjerg opened this issue Mar 31, 2022 · 3 comments · Fixed by #4905
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge Cmd-forge-test Command: forge test D-average Difficulty: average P-normal Priority: normal T-feature Type: feature
Milestone

Comments

@onbjerg
Copy link
Member

onbjerg commented Mar 31, 2022

I think we should disallow vm.etch for addresses 0 < n < 10, and throw an error about using an address >10.

I prefer breaking these early and getting "correct" behavior vs hacking something on Revm.

(This should be fine for transfers etc on the <10 addresses and seems only a vm.etch issue?)

Originally posted by @gakonst in #1146 (comment)

Worth considering disallowing other cheatcodes for those addresses. It's not realistic to prank that address or mockCall with it, and you also don't want to save or load slots from there, for example.

As for what addresses to block off, to be safe we might also want to:

  • Disallow a larger range, such as 0 < n < 99 to account for precompiles being added or other EVM chains that have additional precompiles
  • Prevent etching at addresses that other chains have predeployed contracts at. For example Optimism has predeployed contracts at 0x420...000 < n < 0x420...013, not sure about other networks offhand

Originally posted by @mds1 in #1146 (comment)

This is a breaking change

@onbjerg onbjerg added T-feature Type: feature Cmd-forge-test Command: forge test C-forge Command: forge A-cheatcodes Area: cheatcodes P-normal Priority: normal D-average Difficulty: average labels Mar 31, 2022
@onbjerg onbjerg mentioned this issue Mar 31, 2022
2 tasks
@onbjerg onbjerg moved this to Todo in Foundry Apr 17, 2022
@onbjerg onbjerg added this to Foundry Apr 17, 2022
@onbjerg onbjerg added this to the v1.0.0 milestone Jul 1, 2022
@varunsrin
Copy link

One observation is that vm.prank from such addresses is only problematic if you are calling a function that expects the caller to be an EOA. For example if it has code like payable(msg.sender).transfer(). So for some tests, I can actually use cheat codes like prank from these and it's fine. If we stop pranking from them entirely, then would it be preventing people from testing things they might actually want to test?

An alternative might be to issue a warning when this is seen, instead of disallowing.

Also, I've noticed a few other addresses have been problematic in my tests that might be worth adding to this list:

        address(0xCe71065D4017F316EC606Fe4422e11eB2c47c246), // FuzzerDict
        address(0x4e59b44847b379578588920cA78FbF26c0B4956C), // CREATE2 Factory (?)
        address(0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84), // address(this)
        address(0x185a4dc360CE69bDCceE33b3784B0282f7961aea), // ???
        address(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D) // ???

@mds1
Copy link
Collaborator

mds1 commented Aug 28, 2022

For reference:

  • 0x185a4dc360CE69bDCceE33b3784B0282f7961aea is the default sender account:

    foundry/evm/src/lib.rs

    Lines 33 to 42 in 307576d

    /// Stores the caller address to be used as _sender_ account for:
    /// - deploying Test contracts
    /// - deploying Script contracts
    ///
    /// The address was derived from `address(uint160(uint256(keccak256("foundry default caller"))))`
    /// and is equal to 0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38.
    pub const CALLER: Address = H160([
    0x18, 0x04, 0xc8, 0xAB, 0x1F, 0x12, 0xE6, 0xbb, 0xF3, 0x89, 0x4D, 0x40, 0x83, 0xF3, 0x3E, 0x07,
    0x30, 0x9D, 0x1F, 0x38,
    ]);
  • 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D is the cheatcode address:
    /// The cheatcode handler address (0x7109709ECfa91a80626fF3989D68f67F5b1DD12D).
    ///
    /// This is the same address as the one used in DappTools's HEVM.
    /// `address(bytes20(uint160(uint256(keccak256('hevm cheat code')))))`
    pub const CHEATCODE_ADDRESS: Address = H160([
    0x71, 0x09, 0x70, 0x9E, 0xcf, 0xa9, 0x1a, 0x80, 0x62, 0x6f, 0xf3, 0x98, 0x9d, 0x68, 0xf6, 0x7f,
    0x5b, 0x1d, 0xd1, 0x2d,
    ]);

Also, the last 4 addresses in the above comment should probably all have default labels set if they don't already.

@Evalir
Copy link
Member

Evalir commented May 8, 2023

Taking this one on @mds1 — was wondering, do we wanna just warn or should we just disallow?

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 D-average Difficulty: average P-normal Priority: normal T-feature Type: feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants