Feat: Ability to set deploy caller

[Philogy]

Problem Currently it is not possible to change the msg.sender (op: caller) of the create transaction for Huff contracts, when using HuffConfig / HuffDeployer. This is necessary when testing contracts which contain caller dependent logic in the constructor.

Changes made This PR adds a .with_deployer(address) method to the HuffConfig contract which sets the address to be set as the caller for any upcoming contract deployments from that config contract. The default address is the config contract itself, this is done mainly so that the existing behavior is mimicked by default, the change is therefore backwards compatible. The caller is set via foundry's vm.prank cheatcode.

Testing Tests have been added for the included changes.

Potential Improvements The HuffDeployer library could be extended with more utility methods to allow for direct setting of the caller without having to interact with HuffConfig.

[devtooligan]

thanks for taking the time to make this contribution. i believe this has been addressed with pr #22 issue #21

[Philogy]

I saw that PR but it was using vm.broadcast so I thought it was addressing something else, I overlooked the fact that there was an existing issue. Although I don't see any tests being part of #22, does vm.broadcast really fix the issue? I thought that cheatcode was mainly for creating deploy scripts

[refcell]

This is an interesting issue.

With the current setup, the msg.sender is set as the caller as described in forge-std/src/Vm.sol:

// Has the next call (at this call depth only) create a transaction with the address provided as the sender that can later be signed and sent onchain

Thus, broadcast works as intended. The msg.sender is correctly set. ie, the following test construction passes:

function runTestConstructorCaller(address deployer) public {
    HuffConfig config = HuffDeployer.config();
    IRememberCreator rememberer = IRememberCreator(
        config
            .set_broadcast(true)
            .deploy("test/contracts/RememberCreator")
    );
    assertEq(rememberer.CREATOR(), msg.sender);
}

// @dev fuzzed test too slow, random examples and address(0) chosen
function testConstructorCaller() public {
    runTestConstructorCaller(address(uint160(uint256(keccak256("random addr 1")))));
    runTestConstructorCaller(address(uint160(uint256(keccak256("random addr 2")))));
    runTestConstructorCaller(address(0));
    runTestConstructorCaller(address(uint160(0x1000)));
}

BUT

The following test construction will fail!

function runTestConstructorCaller(address deployer) public {
    HuffConfig config = HuffDeployer.config();
    IRememberCreator rememberer = IRememberCreator(
        config
            .set_broadcast(true)
            .deploy("test/contracts/RememberCreator")
    );
    assertEq(rememberer.CREATOR(), address(this));
}

// @dev fuzzed test too slow, random examples and address(0) chosen
function testConstructorCaller() public {
    runTestConstructorCaller(address(uint160(uint256(keccak256("random addr 1")))));
    runTestConstructorCaller(address(uint160(uint256(keccak256("random addr 2")))));
    runTestConstructorCaller(address(0));
    runTestConstructorCaller(address(uint160(0x1000)));
}

The runTestConstructorCaller will perform correctly since the msg.sender is address(this) 🤔

But testConstructorCaller will fail since the broadcast uses the msg.sender which isn't address(this), it's from the previous call context.

Personally, I think it wouldn't hurt to expose functionality in HuffConfig to override the sender at the lowest level to avoid potential footguns as shown in the second test construction.

[devtooligan]

@abigger87 wait is your example missing a vm.prank or how does that deployer address get used?

[refcell]

Na deployer address isn't used. Can also be:

function runTestConstructorCaller() public {
    HuffConfig config = HuffDeployer.config();
    IRememberCreator rememberer = IRememberCreator(
        config
            .set_broadcast(true)
            .deploy("test/contracts/RememberCreator")
    );
    assertEq(rememberer.CREATOR(), address(this));
}

function testConstructorCaller() public {
    runTestConstructorCaller();
}

[devtooligan]

oh i see that deployer wasnt intended to do anything.

But regardless, i see your point there are def cases where it may be counter intuitive. Agreed a solution like the one proposed in this pr could help and definitely wont hurt anything.

👍

[alexroan]

This is a great addition IMO.

Controlling msg.sender with pranks for deployments is a common test pattern. I'd love to see this merged.

One workaround, for now, is to use address(config), where config is the instance of HuffConfig that is deploying the contract.

[MerlinEgalite]

This would be amazing and would fix an issue I encountered in huffmate where ownable tests were failing because the config contract address would change each time you use a function from the HuffDeployer lib.

https://github.com/pentagonxyz/huffmate/pull/88