-
Notifications
You must be signed in to change notification settings - Fork 827
Add ERC: Permissionless CREATE2 Factory #1052
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
Add ERC: Permissionless CREATE2 Factory #1052
Conversation
|
The commit 558f3d6 (as a parent of e8f0ce2) contains errors. |
ERCS/erc-7955.md
Outdated
| --- | ||
| eip: 7955 | ||
| title: Permissionless CREATE2 Factory | ||
| description: A permissionless method for the cross-chain deployment of a universal CREATE2 factory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your description is mostly just restating your title in more words. I think the only new information introduced here is that it's "cross-chain". You should use your description to elaborate on the ideas introduced in your title, not just restating them.
ERCS/erc-7955.md
Outdated
|
|
||
| #### 1. Nick's Method | ||
|
|
||
| Use Nick's method to randomly generate a signature for a transaction **without** [EIP-155](./eip-155.md) replay protection that deploys the CREATE2 factory. Nick's method ensures that there is no known private key for an account that deploys the CREATE2 factory, meaning that the resulting contract will have a deterministic address and code on all chains. This strategy is used by the _Deterministic Deployment Proxy_ (`Arachnid/deterministic-deployment-proxy`), one of the most widely used CREATE2 factory contracts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deterministic Deployment Proxy (
Arachnid/deterministic-deployment-proxy)
This is an external link and must be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean the reference to the GitHub repository? This was intended to be rendered as:
<i>Deterministic Deployment Proxy</i> (<code>Arachnid/deterministic-deployment-proxy</code>)And should just be plain text with no link. If this is also forbidden, I can remove it (and it also appears in a few other places when referencing safe-global/safe-singleton-factory for example).
ERCS/erc-7955.md
Outdated
|
|
||
| - It does not work on chains that only accept EIP-155 replay-protected transactions. | ||
| - It is sensitive to changes in gas parameters on the target chain since the gas price and limit in the deployment transaction is sealed, and a new one cannot be signed without a private key. | ||
| - Reverts due to alternative gas schedules make the CREATE2 factory no longer deployable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tense here is off, but I'm not sure how you intend it to be read. Perhaps:
| - Reverts due to alternative gas schedules make the CREATE2 factory no longer deployable. | |
| - Reverts due to alternative gas schedules making the CREATE2 factory no longer deployable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant "reverts" as a noun (so "the reverts ... make"), maybe just some missing punctuation:
| - Reverts due to alternative gas schedules make the CREATE2 factory no longer deployable. | |
| - Reverts, due to alternative gas schedules, make the CREATE2 factory no longer deployable. |
Does that make it clearer?
ERCS/erc-7955.md
Outdated
|
|
||
| #### 2. Secret Private Key | ||
|
|
||
| Keep a carefully guarded secret key and use it to sign transactions to deploy CREATE2 factory contracts. The resulting contract will have a deterministic address and code on all chains where the first transaction of the deployer account is a CREATE2 factory deployment, which can be verified post-deployment to ensure trustlessness. Additionally, this method does not have the same gas sensitivity downsides as Nick's method, as the private key can sign a creation transaction with appropriate gas parameters at the time of execution. This is the strategy used by the _Safe Singleton Factory_ (`safe-global/safe-singleton-factory`) and _CreateX_ (`pcaversaccio/createx`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar external links. Please remove them.
ERCS/erc-7955.md
Outdated
| The _bootstrap contract_ MAY implement additional features such as: | ||
|
|
||
| - Abort early if either _CREATE2 factory contract_ is already deployed or the _deployer_ is not correctly delegated to. | ||
| - This can help mitigate gas griefing from the previously described front-running issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, nice catch. The issue is described below and not above. Will fix.
ERCS/erc-7955.md
Outdated
|
|
||
| One issue with this method is that because the `DEPLOYER_PRIVATE_KEY` is public, anyone can sign alternative delegations or transactions and front-run a legitimate CREATE2 factory deployment. We consider this to not be a serious issue however, as: | ||
|
|
||
| 1. Doing so does not prevent future deployments - meaning that an attacker can only delay the deployment of the CREATE2 factory with a sustained attack at a gas cost to the attacker. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe the account paying for the gas of a 7702 transaction has to be the same account as the one being delegated. You never have to put funds into DEPLOYER_ADDRESS to execute the code contained within it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe the account paying for the gas of a 7702 transaction has to be the same account as the one being delegated.
Correct.
You never have to put funds into DEPLOYER_ADDRESS to execute the code contained within it.
Also correct. In fact, you should never send funds to DEPLOYER_ADDRESS as anyone can take them.
The front-running issue stems from the fact that an attacker can submit an alternate EIP-7702 transaction (from another address they control) in order to increase the DEPLOYER_ADDRESS's nonce, thereby invaliding the EIP-7702 delegation signature from DEPLOYER_PRIVATE_KEY in a legitimate deployment transaction that is included afterwards.
Just fixes a small inconsistency where we were using `||` and `++` for byte concatenation. Prefer the former.
- Remove duplicate linking to EIP-7702 (only first one should be linked) - Remove trailing whitespace - Fix spelling error
Requires should be _just_ numbers without the `EIP-` or `ERC-` prefix. This causes issues with the generated HTLM and proofer.
Fixed some section titles that weren't titlecased correctly.
In order to not depend on EIP-3855 `PUSH0` opcode, we refactor the factory code to use `RETURNDATASIZE` which is guaranteed to be `0` **except** for when `CREATE2` reverts. With a little bit of moving code around, we also managed to change make the code one byte smaller and fit into a 32-byte word.
The ERC lint does not like additional top level sections, so move the "custom" forwards compatibility section into a subsection of the backwards compatibility
We remove all non-relative links, as the ERC linter does not allow them. This is slightly unfortunate, since they did provide additional context to the ERC, but 🤷. Note that we format GitHub repositories with italicized title and code for them to stand out: ``` _Title_ (`org/repo`) ```
a16dcd3 to
e6cb880
Compare
* Edits to ERC This PR implements the suggested edits to the ERC, and adds a new section explaining the rationale why Nick's method cannot be used for generating the EIP-7702 authorization signature (since it was a question on Ethereum magicians). * Some small follow-up edits * Another small edit * Clarify reverts cause. Co-authored-by: Shebin John <[email protected]> --------- Co-authored-by: Shebin John <[email protected]>
The description included a forbidden word, remove it.
|
First of all, thank you for the detailed review @SamWilsn :). I think I addressed all of your comments, let me know if there is still something left. TBH, writing a good description is hard 😅 (its like naming things in software development). Anyway, open to more feedback if you have any. |
| 0x0005: DUP3 # Stack: [32; salt; 0; 32] | Push 32 to the stack | ||
| 0x0006: CALLDATASIZE # Stack: [msg.data.len; 32; salt; ...] | Followed by the calldata length | ||
| 0x0007: SUB # Stack: [code.len; salt; 0; 32] | Compute `msg.data.length - 32`, which is the length of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it acceptable for msg.data.length - 32 to potentially overflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would cause the creation transaction to revert with "out of gas" (you would be copying ~2**256 bytes into memory), so it is acceptable IMO.
I saw your implementation in https://ethereum-magicians.org/t/eip-tbd-multi-chain-deterministic-deployment-factory/24998 and think that saturating subtraction is another way to do it. Although, it may cause "silent failures" where incorrectly encoded CREATE2 deployments lead to successful transactions deployments of contracts with empty code; so it violates the "law of least surprise" for me: I would be least surprised if incorrect encoding of parameters causes a revert. There is an argument to be made to make the reverts more explicit i.e. using the REVERT opcode and not rely on the fact that you will run out of gas when trying to copy 1000...000 bytes of calldata into memory, especially given the later will use up all transaction gas, and the former will not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'll give this a think, reverting if sub 32 is probably a better idea.
No description provided.