🐛 No Way to Avoid Salt Being Re-Hashed

[Philogy]

Describe the issue:

When supplying a salt in e.g. deployCreate2(bytes32, bytes) the salt is re-hashed in the _guard function. This is super annoying as it:

1. Makes CreateX incompatible with common salt minters such as create2crunch (although the cross-chain protection byte requires a minor modification)
2. Causes a 2x slow-down in vanity address mining (makes address derivation require 2x keccak hashes rather than just 1 with normal create2)

Maybe I'm just being stupid and missing the obvious method that lets you do a CREATE2 deployment while being able to directly choose the salt but I can't see it.

Code example to reproduce the issue:

Relevant code:

    function _guard(bytes32 salt) internal view returns (bytes32 guardedSalt) {
        (SenderBytes senderBytes, RedeployProtectionFlag redeployProtectionFlag) = _parseSalt({salt: salt});

        if (senderBytes == SenderBytes.MsgSender && redeployProtectionFlag == RedeployProtectionFlag.True) {
            // Configures a permissioned deploy protection as well as a cross-chain redeploy protection.
-           guardedSalt = keccak256(abi.encode(msg.sender, block.chainid, salt));
        } else if (senderBytes == SenderBytes.MsgSender && redeployProtectionFlag == RedeployProtectionFlag.False) {
            // Configures solely a permissioned deploy protection.
-           guardedSalt = _efficientHash({a: bytes32(uint256(uint160(msg.sender))), b: salt});
        } else if (senderBytes == SenderBytes.MsgSender) {
            // Reverts if the 21st byte is greater than `0x01` in order to enforce developer explicitness.
            revert InvalidSalt({emitter: _SELF});
        } else if (senderBytes == SenderBytes.ZeroAddress && redeployProtectionFlag == RedeployProtectionFlag.True) {
            // Configures solely a cross-chain redeploy protection. In order to prevent a pseudo-randomly
            // generated cross-chain redeploy protection, we enforce the zero address check for the first 20 bytes.
-           guardedSalt = _efficientHash({a: bytes32(block.chainid), b: salt});
        } else if (
            senderBytes == SenderBytes.ZeroAddress && redeployProtectionFlag == RedeployProtectionFlag.Unspecified
        ) {
            // Reverts if the 21st byte is greater than `0x01` in order to enforce developer explicitness.
            revert InvalidSalt({emitter: _SELF});
        } else {
            // For the non-pseudo-random cases, the salt value `salt` is hashed to prevent the safeguard mechanisms
            // from being bypassed. Otherwise, the salt value `salt` is not modified.
-           guardedSalt = (salt != _generateSalt()) ? keccak256(abi.encode(salt)) : salt;
        }
    }

Relevant log output:

No response

[pcaversaccio]

@Philogy this is a feature and not a bug. That's our design to implement safely a permissioned deploy protection and cross-chain redeploy protection (see here). The only way to not double hash the salt is by generating it pseudo-randomly by CreateX. We built on purpose our own cruncher createXcrunch to accommodate for these features, where you not only can mine for leading zeros, or containing zeros, but also pattern-matching addresses. If you don't like it, fair, I can offer my predecessor of CreateX, called Create2Deployer (that one is added as a predeploy in the OP stack fyi).

[Philogy]

I understand. I'm just wondering why you couldn't have had all these features without having to rehash the salt, what additional security does it provide?

[pcaversaccio]

I understand. I'm just wondering why you couldn't have had all these features without having to rehash the salt, what additional security does it provide?

I'm not sure I fully understand your question. We need to have a bytes32 salt, depending on whether you want different protections (as elaborated in the section I linked above), you need to transform the salt and the additional attributes msg.sender and/or block.chainid into another (pseudorandom) bytes32 salt value. And that's why we use keccak256.

For the non-pseudo-random cases here, the salt value salt is hashed as part of the _guard function to prevent the safeguard mechanisms from being bypassed. The reason is that if someone deploys on chain A with a preconfigured mechanism, anyone can calculate the inferred guardedSalt and reuse it on chain B otherwise (if we wouldn't hash it).

[Philogy]

Yeah specifically in the last case, not sure I fully understand the purpose of doing keccak256(abi.encode(salt)). What safety feature are you hoping people won't bypass?

The pre-image is still public and anyone else can still redeploy on other chains using the un-hashed salt by running it through the function themselves, I just don't understand this part, what's the value of the extra hash?

[pcaversaccio]

Yeah specifically in the last case, not sure I fully understand the purpose of doing keccak256(abi.encode(salt)). What safety feature are you hoping people won't bypass?

So let's make an example:

• On chain A I deploy a contract using a permissioned deploy protection. This will give me the final salt:

guardedSalt = _efficientHash({a: bytes32(uint256(uint160(msg.sender))), b: salt});

• Someone else sees this deployment, and can calculate the guardedSalt (or simply copy it from a transaction debugger). This person deploys on chain B the same initcode with the guardedSalt as input. Since the msg.sender doesn't match in the if else statement in the function _guard, the code would lead him to the final else statement. If we didn't hash here the salt, he would be able to deploy the same initcode from his address even though the original deployment configured a permissioned deploy protection (i.e. it's only me who can deploy it). Does this make sense?