Optimize Create2.computeAddress

[ToweringTopaz]

This is a more efficient method of computing a create2 address, utilizing the scratch space and (temporarily) the free memory pointer slot. See this discussion with solc devs regarding the memory-safety of this method.

For consistency and simplicity, also refactored the deploy function to use a named return variable.

PR Checklist

• Tests
• Documentation
• Changelog entry

[Amxx]

Hello @ToweringTopaz and thank you for your PR.

IMO, this kind of optimization negatively affect readability and auditability of the code. I'm not sure how significant the gas savings are. We would need number on that.

[Amxx]

I ran both versions of the code with similar input.

Going from solidity to assembly saves 122 gas.

I tried setting the free memory ptr to a large value before running the code, to see if memory depth would affect the cost of the solidity version and would cause a more significant different. That was not the case.

[Amxx]

It that change optimizing anything? If not, I would revert it and focus on the computeAddress function

[ToweringTopaz]

Hello @ToweringTopaz and thank you for your PR.

IMO, this kind of optimization negatively affect readability and auditability of the code. I'm not sure how significant the gas savings are. We would need number on that.

In addition to the savings on execution, this reduces deployment size by 360 bytes for a simple contract. That's a lot of bytecode, saving 77,000 gas on deployment.

[Amxx]

In addition to the savings on execution, this reduces deployment size by 360 bytes for a simple contract. That's a lot of bytecode, saving 77,000 gas on deployment.

Were the optimizations turned on when you tested that ? I'm getting "just" 13k gas difference. That might depend on the number of use if the function is inlined.

[ToweringTopaz]

In addition to the savings on execution, this reduces deployment size by 360 bytes for a simple contract. That's a lot of bytecode, saving 77,000 gas on deployment.

Were the optimizations turned on when you tested that ? I'm getting "just" 13k gas difference. That might depend on the number of use if the function is inlined.

Optimization was off. Another factor that might apply is if abi.encodePacked is used elsewhere in the same contract.

Would it be helpful to add comments to the assembly code, or is it self-explanatory enough?

[Amxx]

I think it's self explanatory.

How would abi.encodePacked been used elsewhere affect the produced code. Can you measure that?

[ToweringTopaz]

I think it's self explanatory.

How would abi.encodePacked been used elsewhere affect the produced code. Can you measure that?

If abi.encodePacked(bytes1,address,bytes32,bytes32) is used elsewhere, the new method only saves 27 bytes of bytecode with optimizations turned off and 60 with optimizations on, via-IR, 200 runs. This is the extreme "minimum benefit" case, so in all cases the new method provides a cost savings.

contract Create2Test1 {

    function matchingEncodePacked(bytes1 a, address b, bytes32 c, bytes32 d) external pure returns (bytes memory) {
        return abi.encodePacked(a,b,c,d);
    }

    function test1() external view returns (address addr) {
        return Create2.computeAddressOld(bytes32(block.timestamp), bytes32(block.timestamp), msg.sender);
    }
}
contract Create2Test2 {
    function matchingEncodePacked(bytes1 a, address b, bytes32 c, bytes32 d) external pure returns (bytes memory) {
        return abi.encodePacked(a,b,c,d);
    }

    function test2() external view returns (address addr) {
        return Create2.computeAddressNew(bytes32(block.timestamp), bytes32(block.timestamp), msg.sender);
    }
}

[Amxx]

If abi.encodePacked(bytes1,address,bytes32,bytes32) is used elsewhere

that is deployment cost saving, right ?

I guess its because the corresponding logic would be included anyway (like a private function).

[ToweringTopaz]

If abi.encodePacked(bytes1,address,bytes32,bytes32) is used elsewhere

that is deployment cost saving, right ?

Right. Of course this is the most extreme example offering the least savings. Not many other things pack (bytes1,address,bytes32,bytes32) in this manner, in practice.

[frangio]

I am not comfortable with overwriting the free memory pointer in this way. The Gitter discussion is not a strong enough guarantee. The reality is that this use of memory is outside of what is assumed by Solidity in memory-safe blocks, according to the documentation (source).

In this PR I haven't seen how much gas is saved on execution. Is most of the gas saved due to avoiding memory expansion or is it due to an optimized code path? It would be okay and memory-safe if instead of doing this we simply write on free memory without updating the free memory pointer. We would be expanding memory slightly but it may not be that bad in context.

If we move forward with my suggestion, the assembly block also needs a comment explaining what it does.

[ToweringTopaz]

I am not comfortable with overwriting the free memory pointer in this way. The Gitter discussion is not a strong enough guarantee. The reality is that this use of memory is outside of what is assumed by Solidity in memory-safe blocks, according to the documentation (source).

In this PR I haven't seen how much gas is saved on execution. Is most of the gas saved due to avoiding memory expansion or is it due to an optimized code path? It would be okay and memory-safe if instead of doing this we simply write on free memory without updating the free memory pointer. We would be expanding memory slightly but it may not be that bad in context.

@Amxx calculated gas savings of 122 using the unmodified method. Using free memory reduces this a bit due to ADD opcodes and 3 words of memory expansion in some cases, but it's still an optimized code path.

[Amxx]

I've added a "visual explanation" of how the assembly write to memory

[frangio]

LGTM.

I want to add some detail. I consider this optimization to be almost too small to matter and something that the compiler should be taking care of. However, the assembly required is simple enough, and evidently the compiler is not taking care of it, so I agree with merging.