Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace implementation+etch pattern with Create2Deployer #59

Closed
saucepoint opened this issue Sep 11, 2023 · 3 comments
Closed

Replace implementation+etch pattern with Create2Deployer #59

saucepoint opened this issue Sep 11, 2023 · 3 comments

Comments

@saucepoint
Copy link
Collaborator

saucepoint commented Sep 11, 2023

Component

General design optimization (improving efficiency, cleanliness, or developer experience)

Describe the suggested feature and problem it solves.

Arachnid's create2 deployer proxy is now available on anvil and forge test by default

This allows us to deterministically mine + deploy arbitrary contracts without the need for a factory or Implementation wrapper that overrides validateHookAddress.

A minor change but it reduces repo clutter+boilerplate by adding consistent abstractions around hook development and testing

Describe the desired implementation.

Currently you need to define an Implementation which overrides the flag checks, and then etch the bytecode to a correctly-flagged address:

       LimitOrder limitOrder = LimitOrder(address(uint160(Hooks.AFTER_INITIALIZE_FLAG | Hooks.AFTER_SWAP_FLAG)));
        vm.record();
        LimitOrderImplementation impl = new LimitOrderImplementation(manager, limitOrder);
        (, bytes32[] memory writes) = vm.accesses(address(impl));
        vm.etch(address(limitOrder), 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(limitOrder), slot, vm.load(address(impl), slot));
            }
        }

Abstracting the create2 deployer proxy, we get a much cleaner devex:

        // Deploy the hook to an address with the correct flags
        uint160 flags = uint160(Hooks.AFTER_INITIALIZE_FLAG | Hooks.AFTER_SWAP_FLAG);
        bytes memory hookBytecode = abi.encodePacked(type(LimitOrder).creationCode, abi.encode(address(manager)));
        (, uint256 salt) = HookDeployer.mineSalt(CREATE2_DEPLOYER, flags, hookBytecode);

        LimitOrder limitOrder = new LimitOrder{salt: salt}(address(manager));

Describe alternatives.

N/A

Additional context.

Already works and is available here:

https://github.com/saucepoint/v4-template/blob/main/test/Counter.t.sol#L30-L37

and

https://github.com/saucepoint/v4-template/blob/main/test/utils/HookDeployer.sol


I'm happy to PR and update all the tests to the new pattern!

@snreynolds
Copy link
Member

Cool! I think I like this overall. Some thoughts:

  • Mining addresses onchain is a bit wonky but for testing infra I think its fine. Overall I do like the cleanliness of this pattern over the implementation wrappers and overriding and I think it would be nice for this repo to expose the tools for other hooks to use this testing pattern.
  • Can we make this cross chain compatible? Maybe we need a config for the right create2deployers on various chains?
  • I think it would be nice to have you PR any of the testing infra like the hook deployer contract into this repo, are you open to that?

tagging @ewilz to see if she has any more thoughts?

@saucepoint
Copy link
Collaborator Author

Wrote a script to check all the ~major chains. Looks like most chains do have Arachnid's deployer proxy at 0x4e59b44847b379578588920cA78FbF26c0B4956C (+ shipped w/ anvil now).

You are correct that this issue is more about testing patterns and infra. Naming the utility library HookDeployer might be a misnomer that I'll change for the PR!

Chain Matching Bytecode
Ethereum
Goerli
Sepolia
Arbitrum One
Arbitrum Goerli
Arbitrum Nova
OP Mainnet
Optimism Goerli
Polygon
Polygon Mumbai
Polygon zkEVM
Polygon zkEVM Testnet
Base
Base Goerli
Avalanche
Avalanche Fuji
Boba Network
BNB Smart Chain
Binance Smart Chain Testnet
Canto
Celo
Alfajores
Cannoli
Fantom
Fantom Testnet
Gnosis
Gnosis Chiado
Linea Mainnet
Linea Goerli Testnet
Mantle
Mantle Testnet
Metis
Metis Goerli
opBNB
opBNB Testnet
zkSync Era
zkSync Era Testnet
Zora
Zora Goerli Testnet

@saucepoint
Copy link
Collaborator Author

given hook flags are now 14 bits, mining is too slow for tests

reference PR for deployCodeTo

#122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants