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

EIP-4758: Deactivate selfdestruct #4758

Merged
merged 8 commits into from
Mar 28, 2022

Conversation

dankrad
Copy link
Contributor

@dankrad dankrad commented Feb 3, 2022

This EIP deactivates the SELFDESTRUCT opcode, and instead renames it to SENDALL, with the only renaming functionality being to move all funds to the caller.

@eth-bot
Copy link
Collaborator

eth-bot commented Feb 3, 2022

All tests passed; auto-merging...

(pass) eip-4758.md

classification
updateEIP
  • passed!

@dankrad dankrad mentioned this pull request Feb 3, 2022
@dankrad dankrad changed the title Deactivate selfdestruct initial commit EIP-4758: Deactivate selfdestruct Feb 3, 2022
EIPS/eip-4758.md Outdated Show resolved Hide resolved
Co-authored-by: Bolton Bailey <[email protected]>
Copy link
Contributor

@MicahZoltu MicahZoltu left a 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 Show resolved Hide resolved
EIPS/eip-4758.md Outdated Show resolved Hide resolved
EIPS/eip-4758.md Outdated Show resolved Hide resolved
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.
Copy link
Contributor

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.

Copy link
Contributor

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.

EIPS/eip-4758.md Outdated Show resolved Hide resolved
EIPS/eip-4758.md Outdated Show resolved Hide resolved
EIPS/eip-4758.md Outdated Show resolved Hide resolved
EIPS/eip-4758.md Outdated Show resolved Hide resolved

## 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
Copy link
Member

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.
Copy link
Contributor

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.

@MicahZoltu
Copy link
Contributor

@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.

@MicahZoltu MicahZoltu dismissed lightclient’s stale review March 28, 2022 01:26

Feedback addressed.

@MicahZoltu MicahZoltu closed this Mar 28, 2022
@MicahZoltu MicahZoltu reopened this Mar 28, 2022
@eth-bot eth-bot enabled auto-merge (squash) March 28, 2022 01:31
@eth-bot eth-bot merged commit 7591860 into ethereum:master Mar 28, 2022
@ethereum ethereum deleted a comment Mar 28, 2022
@chfast
Copy link
Member

chfast commented Mar 28, 2022

@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.

@MicahZoltu
Copy link
Contributor

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
Copy link
Contributor

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?

PowerStream3604 pushed a commit to PowerStream3604/EIPs that referenced this pull request May 19, 2022
* Deactivate selfdestruct initial commit

* Add discussions-to

* Remove erroneous brackets in category

* Update EIPS/eip-4758.md

Co-authored-by: Bolton Bailey <[email protected]>

* Move discussion to Ethereum Magicians

* Fix header, add security considerations and other fixes

* Fix typo

* Remove external link

Co-authored-by: Bolton Bailey <[email protected]>
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.

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.

@Dstoney
Copy link

Dstoney commented Nov 28, 2022

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.

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 this pull request may close these issues.

9 participants