✨ ERC1967Factory should salt by default

[RogerPodacter]

With current default behavior different users will create unrelated contracts on different networks with the same contract address.

This could create security issues by someone purposely "spoofing," but beyond this there is a norm that contracts at identical addresses on different networks are related. ERC1967Factory itself exploits this norm, which is what causes the issue! It should "pay it forward" by helping users not be dumb.

The default behavior of ERC1967Factory should be to not cause collisions unless the deploying address creates the situation (intentionally or otherwise). Currently collisions can "appear out of nowhere," which is confusing and deserving of a change of defaults.

Great factory otherwise though!!

[Vectorized]

Hashing the salt with the chainId can help prevent replays, but this will make it 2x more expensive to mine vanity addresses for deployments.

Hmm... I should have baked in the implementation and initial admin into the initcode. Back then, I was probably thinking if having the same initcode will make it easier for Etherscan to auto-verify, but in the end, they didn't get to my request.

A compromise is to set the first 20-bytes of the salt to an account.

[Vectorized]

Update: after thinking deeper, it seems like there's no easy generalized way.

Even if we include in the implementation and admin into the initcode or salt, a troublemaker can still craft a data that can spoil the initialization of the deployment.

If you need a watertight permissionless initial deployment, you have to write a disposable contract that can be permissionlessly deployed.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

import "solady/src/utils/ERC1967Factory.sol";
import "solady/src/utils/ERC1967FactoryConstants.sol";

contract ERC1967FactoryFixedDeployer {
    // Some hardcoded value based on `implementation`, `admin`, `data`.
    // We only hardcode the hash, and pass in the variables on-the-fly to verify, so that
    // the bytecode of this fixed deployer contract is smaller.
    bytes32 internal constant _FIXED_DEPLOYMENT_HASH =
        0x6d0303985e0805f1f9ff102005aff821d34ef3e95521cecc9d4d6d74ea309f04;

    function deployDeterministicAndCall(
        address implementation,
        address admin,
        bytes32 salt,
        bytes calldata data
    ) public payable returns (address proxy) {
        require(uint256(salt) >> 96 == uint256(uint160(address(this))));
        require(_FIXED_DEPLOYMENT_HASH == keccak256(abi.encode(implementation, admin, data)));
        proxy = ERC1967Factory(payable(ERC1967FactoryConstants.ADDRESS))
            .deployDeterministicAndCall{value: msg.value}(implementation, admin, salt, data);
    }
}

Then mine a salt to deploy the proxy via this one-use ERC1967FactoryFixedDeployer.

[RogerPodacter]

Interesting! I thought that the fact that deterministic deployment address can't be front ran (because they include the deployer's address) we could use the same mechanism to ensure addresses aren't duplicated.

Could the disposable contract be deployed and self-destructed in the same deployment tx on the factory? Maybe I'm misunderstanding.

At minimum though, I would say that two people calling the most basic deploy or deployAndCall on different chains shouldn't produce duplicate addresses. I.e., I think more confusion in practice is probably created by randos (like I) not using salts than attackers.

This is what led to this issue anyway; I was like "wait why is Etherscan telling me my contract has assets on Arbitrum... oh wait!"

Maybe deployAndCall could move from:

_deploy(implementation, admin, bytes32(0), false, data);

to:

_deploy(implementation, admin, keccak256(msg.sender, perSenderNonce), true, data);

Ah but then you have to store the nonces! Might be worth it, tbh. This thing is already cheap enough! People should be grateful!

ORRRRR The factory could have a different address on each chain. Like instead of starting with all zeros the first few digits could be the chainid of the chain (yes again a little more expensive, but people need to chill).

No great options, but I think the default behavior is also IMO not great!

Heavy lies the crown...

[Vectorized]

Could the disposable contract be deployed and self-destructed in the same deployment tx on the factory? Maybe I'm misunderstanding.

I'm thinking how can we make the disposable contract as lightweight as possible. Right now it costs 140k+ gas to deploy on L1. I want to make it lower. Maybe some huffoor can help.

Unfortunately, it cannot call the ERC1967Factory in the constructor, because any argument to the constructor will change the initcode, which changes the disposable contract's address.

Self-destructing won't refund much gas too.