-
Notifications
You must be signed in to change notification settings - Fork 591
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: Minimal Upgradeable Proxies #604
Add ERC: Minimal Upgradeable Proxies #604
Conversation
✅ All reviewers have approved. |
ERCS/erc-9999.md
Outdated
@@ -0,0 +1,397 @@ | |||
--- | |||
eip: 9999 |
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.
eip: 9999 | |
eip: 7760 |
Assigning next sequential EIP/ERC/RIP number.
Please also update the filename.
ERCS/erc-9999.md
Outdated
title: Minimal Upgradeable Proxies | ||
description: Minimal upgradeable proxies with immutable arguments and support for onchain implementation queries | ||
author: Atarpara (@Atarpara), JT Riley (@jtriley-eth), xiaobaiskill (@xiaobaiskill), Vectorized (@Vectorized) | ||
discussions-to: https://ethereum-magicians.org/t/minimal-upgradeable-proxies/20868 |
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.
discussions-to: https://ethereum-magicians.org/t/minimal-upgradeable-proxies/20868 | |
discussions-to: https://ethereum-magicians.org/t/erc-7760-minimal-upgradeable-proxies/20868 |
Updated topic title.
The discussions topic can just contain a link to the PR. There isn't a need to replicate the ERC text (as this may differ).
ERCS/erc-7760.md
Outdated
|
||
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119 and RFC 8174. | ||
|
||
### Overview |
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.
### Overview | |
### General specifications |
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.
Editorially I think this ERC draft is ready to submit as a Draft status. Whether this is mathematically "minimal", and/or whether the reference implementation is free of bugs or security flaws is up for technical peer reviews, hence the approve.
ERCS/erc-7760.md
Outdated
|
||
### Minimal ERC-1967 transparent upgradeable proxy | ||
|
||
The transparent upgradeable proxy MUST be deployed by a factory that is responsible for authenticating upgrades. |
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.
"MUST be deployed by a factory" seems vaguely defined. What do you try to say here? If a proxy was deployed by EOA via a regular contract call, would that EOA be considered a 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.
Ok, i’ll change to RECOMMENDED. The rationale is that a factory will be very helpful in preparing the exact bytecode needed.
An EOA can always deploy the same bytecode in place of the factory, and it will still work properly.
|
||
Emitting the ERC-1967 events during initialization is OPTIONAL. Indexers MUST NOT expect the initialization code to emit the ERC-1967 events. | ||
|
||
### Minimal ERC-1967 transparent upgradeable proxy |
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.
### Minimal ERC-1967 transparent upgradeable proxy | |
### Basic form of minimal ERC-1967 transparent upgradeable proxy |
ERCS/erc-7760.md
Outdated
|
||
As it is beneficial to install the factory at a vanity address with leading zero bytes so that the proxy's bytecode can be optimized to be shorter, variants for 14-byte factory address are provided. | ||
|
||
#### Minimal ERC-1967 transparent upgradeable proxy (20-byte factory address regular variant) |
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.
#### Minimal ERC-1967 transparent upgradeable proxy (20-byte factory address regular variant) | |
#### 20-byte factory address variant of minimal ERC-1967 transparent upgradeable proxy |
ERCS/erc-7760.md
Outdated
where `________________________________________` is the 20-byte factory address. | ||
|
||
|
||
#### Minimal ERC-1967 transparent upgradeable proxy (14-byte factory address regular variant) |
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.
#### Minimal ERC-1967 transparent upgradeable proxy (14-byte factory address regular variant) | |
#### 14-byte factory address variant of minimal ERC-1967 transparent upgradeable proxy |
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.
Here and below, I think the subsection names of specifications are important as one of the key proposal in this ERC is to propose different forms of minimal proxies. Current naming of these sections are a bit confusing I think, suggest revisit for better readability
ERCS/erc-7760.md
Outdated
|
||
### Transparent upgradeable proxy factory security considerations | ||
|
||
The transparent upgradeable proxy factory MUST implement proper access control to allow proxies to be upgraded by only authorized accounts. |
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.
MUST is all caps creates a feel like this is still specification because of RFC 2119. It'd suggest you change the tone
The transparent upgradeable proxy factory MUST implement proper access control to allow proxies to be upgraded by only authorized accounts. | |
To ensure security, the transparent upgradeable proxy factory must implement proper access control to allow proxies to be upgraded by only authorized accounts. |
ERCS/erc-7760.md
Outdated
|
||
### Onchain querying of implementation for I-variants | ||
|
||
The bytecode of the proxies before any optional immutable arguments MUST be verified with the following steps: |
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.
Should these requirements be in the specification section?
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.
Will move to specification section.
|
||
## Motivation | ||
|
||
Having standardized minimal bytecode for upgradeable proxies enables the following: |
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.
Can you provide details about how these bytecodes are generated so people can verify the correctness and minimal-ness?
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.
To the best of our knowledge, the transparent proxy can have 1 opcode shaved off it for its upgrade logic. The rest of the proxies are as minimal as possible, given the constraints of avoiding PUSH0
and optimizing first for runtime gas.
But since it has already been used in the wild, including the version that has been already used is more beneficial than including the theoretically more minimal version. The purpose of this standard is to aid block explorer auto verification. Support for existing proxies outweighs the 2-3 runtime gas saved.
Also, users who want to squeeze out every single bit of runtime gas savings and bytecode size will use the UUPS variant.
Historical context: ERC-1167 was not minimal at the time of finalization. The 0age proxy is more minimal in both runtime gas and bytecode size. But ERC-1167 was chosen as the base for ERC-6551's proxy due to wider support for auto-verification.
With all this in consideration, the transparent variant is minimal-enough.
…orized/ERCs into minimal-upgradeable-proxies
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.
All Reviewers Have Approved; Performing Automatic Merge...
https://ethereum-magicians.org/t/minimal-upgradeable-proxies/20868