-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
EIP-4758: Deactivate selfdestruct #4758
Conversation
All tests passed; auto-merging...(pass) eip-4758.md
|
Co-authored-by: Bolton Bailey <bolton.bailey@gmail.com>
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.
Blockers:
- Remove external links
- Fix header.
- Redirect discussion to Ethereum Magicians.
- Rename Summary section to Abstract.
- Add missing sections:
## Security Considerations
All EIPs must contain a section that discusses the security implications/considerations relevant to the proposed change. Include information that might be important for security discussions, surfaces risks and can be used throughout the life cycle of the proposal. E.g. include security-relevant design decisions, concerns, important discussions, implementation-specific guidance and pitfalls, an outline of threats and risks and how they are being addressed. EIP submissions missing the "Security Considerations" section will be rejected. An EIP cannot proceed to status "Final" without a Security Considerations discussion deemed sufficient by the reviewers.
## Copyright
Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).
EIPS/eip-4758.md
Outdated
|
||
## Rationale | ||
|
||
Getting rid of the `SELFDESTRUCT` opcode has been considered in the past (for arguments see [here](https://hackmd.io/@vbuterin/selfdestruct)), and there are currently no strong reasons to use it. Disabling it will be a requirement for statelessness. |
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.
Please include the rationale inline here, not in an external link. You don't need to provide a complete argument/essay, just the key talking points about alternative options like having it become a no-op, or having it become a hard error. A couple sentences is probably fine in this case.
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.
This still needs to be addressed.
|
||
## Specification | ||
|
||
* The `SELFDESTRUCT` opcode is renamed to `SENDALL`, and now only immediately moves all ETH in the account to the target; it no longer destroys code or storage or alters the nonce |
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 forgot to mention that SENDALL
still terminates execution.
EIPS/eip-4758.md
Outdated
|
||
## Rationale | ||
|
||
Getting rid of the `SELFDESTRUCT` opcode has been considered in the past (for arguments see [here](https://hackmd.io/@vbuterin/selfdestruct)), and there are currently no strong reasons to use it. Disabling it will be a requirement for statelessness. |
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.
This still needs to be addressed.
@chfast Since this is getting merged as a draft, I recommend moving your comment to the discussions-to link so it doesn't get lost. |
No, thanks. Here is good. |
In my experience, comments in closed PRs almost always get forgotten/missed/ignored. While you are welcome to not port your comment over to the discussions-to link, I would be sad to see such a valuable suggestion get lost to time. |
## Specification | ||
|
||
* The `SELFDESTRUCT` opcode is renamed to `SENDALL`, and now only immediately moves all ETH in the account to the target; it no longer destroys code or storage or alters the nonce | ||
* All refunds related to `SELFDESTRUCT` are 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.
They were already removed in London. Is there a reason to mention this here?
* Deactivate selfdestruct initial commit * Add discussions-to * Remove erroneous brackets in category * Update EIPS/eip-4758.md Co-authored-by: Bolton Bailey <bolton.bailey@gmail.com> * Move discussion to Ethereum Magicians * Fix header, add security considerations and other fixes * Fix typo * Remove external link Co-authored-by: Bolton Bailey <bolton.bailey@gmail.com>
1. Any use where `SELFDESTRUCT` is used to burn non-ETH token balances, such as ERC20, inside a contract. We do not know of any such use (since it can easily be done by sending to a burn address this seems an unlikely way to use `SELFDESTRUCT`) | ||
2. Where `CREATE2` is used to redeploy a contract in the same place. There are two ways in which this can fail: | ||
- The destruction prevents the contract from being used outside of a certain context. For example, the contract allows anyone to withdraw funds, but `SELFDESTRUCT` is used at the end of an operation to prevent others from doing this. This type of operation can easily be modified to not depend on `SELFDESTRUCT`. | ||
- The `SELFDESTRUCT` operation is used in order to make a contract upgradable. This is not supported anymore and delegates should be used. |
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.
This should say: this will break all the previously deployed code that depends on it.
the work required for implementing a Core EIP will be much greater than for an ERC and the EIP will need sufficient interest from the Ethereum client teams. |
This EIP deactivates the
SELFDESTRUCT
opcode, and instead renames it toSENDALL
, with the only renaming functionality being to move all funds to the caller.