Replace Implementation+etch Pattern with CREATE2

[saucepoint]

Related Issue

Closes #59

By using CREATE2 natively, we reduce the testing complexity. Prior to this change, testing a hook required a wrapper contract ('Implementation') that overrides validateHookAddress as a no-op. Within tests, users need to define both contracts, and etch the 'Implementation' bytecode to a desired address

This pattern is noticeably verbose, with little reusable patterns that leads to copy + paste boilerplate

Description of changes

• Created a helper library test/utils/HookMiner.sol, which finds a salt to deploy a hook with a specific flag pattern

• Updated the tests to deploy hooks with CREATE2 instead of the implementation and vm.etch pattern

Example:

Before, where TWAMMImplementation is a boilerplate wrapper contract around the actual TWAMM

        TWAMM twamm = TWAMM(
            address(uint160(Hooks.BEFORE_INITIALIZE_FLAG | Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_MODIFY_POSITION_FLAG))
        );
        TWAMMImplementation impl = new TWAMMImplementation(manager, 10_000, twamm);
        (, bytes32[] memory writes) = vm.accesses(address(impl));
        vm.etch(address(twamm), address(impl).code);
        // for each storage key that was written during the hook implementation, copy the value over
        unchecked {
            for (uint256 i = 0; i < writes.length; i++) {
                bytes32 slot = writes[i];
                vm.store(address(twamm), slot, vm.load(address(impl), slot));
            }
        }

After:

        // Find a salt that produces a hook address with the desired `flags`
        uint160 flags =
            uint160(Hooks.BEFORE_INITIALIZE_FLAG | Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_MODIFY_POSITION_FLAG);
        (address hookAddress, bytes32 salt) =
            HookMiner.find(address(this), flags, 0, type(TWAMM).creationCode, abi.encode(manager, 10_000));

        // Deploy hook to the precomputed address using the salt
        twamm = new TWAMM{salt: salt}(manager, 10_000);

[ewilz]

confused about this check. Is there a scenario where this would be false and we'd want to halt the test? Doesn't the check on line 32 in HookMiner take care of this? Feels extra, or like we don't trust our hook miner...in which case it could maybe have its own tests 🤔

[saucepoint]

It shouldnt fail and we can trust the miner. It's really a safety check since there's a minor footgun with the deployer argument in HookMiner.find(address deployer, ...)

In tests, deployer can be address(this) or the pranking address from vm.prank. There's a probability that the mined salt creates a valid address with the wrong deployer. i.e. HookMiner.find(alice, ...) but forgetting to prank on new FullRange. This would cause a mismatch between hookAddress and address(fullRange)

The hook would still deploy but a dev might use hookAddress in a way that causes false positive bugs/errors.

Happy to remove it or change it to assertEq (to imply a safety check)

[saucepoint]

after thinking about it some more, decided to remove it entirely. while it is good to encourage safety checks, i dont think its something that had to be showcased in the codebase

[ewilz]

lgtm other than the nits. impressed with the lack of latency on the mining!?

[saucepoint]

replaced by #122

mining addresses in tests has been reported to be annoyingly slow