-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 EIP: Minimal Upgradable Proxy Contract #7229
Conversation
File
|
The commit 8184ce4 (as a parent of b87b9f0) contains errors. |
all errors are fixed. view please. |
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.
You could slim down your example code a bit. We try to keep reference implementations minimal.
The big overall comment here is that you aren't proposing a standard, you're proposing a specific implementation. We don't generally allow specific implementations to be published as EIPs.
EIPs shine when multiple different parties have to coordinate to implement a system. For example, ERC-20 is a great example of what an EIP should be because every wallet wants to work with every ERC-20 token, and every ERC-20 token wants to work with every wallet.
This EIP doesn't really coordinate anything. If someone wants to use your bytecode, they can just use it without having to coordinate with anyone else.
This isn't me saying your bytecode doesn't have value. It absolutely does, and you should promote it as much as possible. Just not in the EIPs repository.
I'm going to close this PR, but feel free to re-open it if you would like to rewrite it as a standard instead of an implementation.
type: Standards Track | ||
category: ERC | ||
created: 2023-06-24 | ||
--- |
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.
--- | |
requires: 3855 | |
--- |
@@ -0,0 +1,249 @@ | |||
--- | |||
eip: 7229 | |||
title: Minimal Upgradable Proxy Contract |
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 title doesn't differentiate itself enough form the other proxy contract standards. Maybe something like:
title: Minimal Upgradable Proxy Contract | |
title: Gas Optimized Proxy Contract |
--- | ||
eip: 7229 | ||
title: Minimal Upgradable Proxy Contract | ||
description: A lightweight contract upgrade pattern designed to save gas costs while providing the ability to upgrade 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.
Should probably mention custom assembly in here somewhere.
|
||
## Abstract | ||
|
||
This proposal introduces the Minimal Upgradable Contract, a lightweight alternative to the existing upgradable contract implementation provided by OpenZeppelin. The Minimal Upgradable Contract aims to significantly reduce gas consumption during deployment while maintaining upgradability features. |
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 abstract should be a terse technical summary of the proposal. The reader should be able to get a high level understanding of how the proposal accomplishes its goal (in this case, handrolled assembly.)
The exact bytecode of the standard minimal upgradable contract is this: | ||
`7fxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx73yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy81556009604c3d396009526010605560293960395ff3365f5f375f5f365f7f545af43d5f5f3e3d5f82603757fd5bf3`; In this bytecode, the 1st to 32nd byte (inclusive) needs to be replaced with a 32-byte slot, and the 34th to 53rd byte (inclusive) needs to be replaced with a 20-byte address. | ||
Please note that the placeholders `xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx` represent the 32-byte slot and `yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy` represents the 20-byte address. | ||
|
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.
You should probably mention what the "slot" and "address" mean. To someone very familiar with solidity/proxies, it's obvious that the address is the implementation contract, but it's good to be explicit.
@@ -0,0 +1,36 @@ | |||
// SPDX-License-Identifier: UNLICENSED |
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.
No UNLICENSED
.
@@ -0,0 +1,43 @@ | |||
// SPDX-License-Identifier: UNLICENSED |
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.
No UNLICENSED
@@ -0,0 +1,35 @@ | |||
// SPDX-License-Identifier: UNLICENSED |
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.
No UNLICENSED
.
@@ -0,0 +1,36 @@ | |||
// SPDX-License-Identifier: UNLICENSED |
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.
No UNLICENSED
@@ -0,0 +1,36 @@ | |||
// SPDX-License-Identifier: UNLICENSED |
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.
No UNLICENSED
.
Discussion at https://ethereum-magicians.org/t/eip-xxxx-minimal-upgradable-proxy/14754