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

Changing SUICIDE variable #42

Merged
merged 4 commits into from
Dec 1, 2015
Merged

Changing SUICIDE variable #42

merged 4 commits into from
Dec 1, 2015

Conversation

Souptacular
Copy link
Contributor

Creating EIP-6 to propose renaming the SUICIDE variable in Ethereum.

@taoeffect
Copy link

Not exactly on topic but somewhat (and testing the waters):

I've heard (from a DEVCON1 presentation) that calling suicide (or selfdestruct as per this EIP) is inadvisable because it can result in financial loss in the event two txns are sent at the same time to a contract.

The first one kills the contract, and the second one sends money to a newly created address, causing it to be lost forever.

That seems like insane behavior if true. The speaker recommended that people therefore never do this and instead use variables to disable the contract.

So would it be reasonable to propose an EIP to remove this OP completely?

@Souptacular
Copy link
Contributor Author

@taoeffect I agree with you and I believe it was @chriseth who gave the talk you are referencing. I'm not sure what that would do to contracts that are already on the Ethereum blockchain that need to run suicide(). I assume the EVM would take this into account and there may be a deprecation strategy that the team has already came up with for other times when things need to be taken out of the language. I would be in support of removing it entirely.

@axic
Copy link
Member

axic commented Nov 28, 2015

If you go for changing the name, I would propose to go with HSC (Halt and Spontaneously Combust) from MIPS-X so that finally there would be a system implementing it :)

@taoeffect the wiki for this simple reason suggests that behaviour with the "deactivate" pattern, but I think it is more like a workaround than a solution.

The main purpose of the above OP would be to mark/schedule a contract for deletion (or for pushing to archive storage as opposed to "live" storage?), but we're lacking that cleaning process anyway.

@Souptacular
Copy link
Contributor Author

@rypdx released this code which acts as a good compromise between keping your contract running forever and suiciding it:
https://github.com/ryepdx/ethereum-deprecationsafe-proposal

@axic
Copy link
Member

axic commented Nov 28, 2015

@Souptacular I've did the same after seeing it in the wiki: https://github.com/axic/density/blob/master/modifiers/deactivate.sol

The section from the wiki for the record:

Does suicide() free up space in the blockchain?

It removes the contract bytecode and storage from the current block into the future, but since the blockchain stores every single block (i.e. all history), this will not actually free up space on full/achive nodes.

Create a contract that can be killed and return funds

First, a word of warning: Killing contracts sounds like a good idea, because “cleaning up” is always good, but as seen above, it does not really clean up. Furthermore, if Ether is sent to removed contracts, the Ether will be forever lost.

If you want to deactivate your contracts, rather disable them by changing some internal state which causes all functions to throw. This will make it impossible to use the contract and ether sent to the contract will be returned automatically.

Now to answering the question: Inside a constructor, msg.sender is the creator. Save it. Then suicide(creator); to kill and return funds.

@axic
Copy link
Member

axic commented Nov 28, 2015

On the other hand, if speaking for keeping the opcode, the behaviour (if/when archival is in place) could be specified as "mark for archival in 30 days from now" (where now is the block timestamp and replace 30 with whatever is sane).

@bobsummerwill
Copy link

+1 for a name change, whatever happens with the semantics.

Suicide is a serious issue. Just inappropriate and unnecessary to have this as an opcode name.

@ghost
Copy link

ghost commented Nov 28, 2015

Most of us won't be interacting with the "suicide" op code directly. We'll be using Solidity or Serpent. If the name is an issue it itself, then I think changing it in those languages should handle most of the problem there. But that's me, and I don't personally have a problem with it even as a keyword in those languages, so I'm probably not the best person to speak on that subject.

If the suicide semantics change (and I think there's a good argument for it, per @taoeffect), then my vote is for suicided contracts to return any ether sent to them, either indefinitely or for a trailing window. Alternatively, it might be worth implementing a deprecate op code that has those semantics. Then suicide itself could be deprecated without breaking any of the contracts already deployed.

@Souptacular
Copy link
Contributor Author

@ryepdx I agree. It seems like this is turning into a situation where:

  1. SUICIDE can be renamed to SELFDESTRUCT
  2. The semantics of the command should change, such as having any ether sent to a self-destructed contract return to sender rather than become unrecoverable. The 0x0000000000000000000000000000000000000000 address acts as a "coin burning" address just fine.

This would actually work great because the name change could coincide with changing the way the command operates which would cause less confusion.

@axic
Copy link
Member

axic commented Nov 28, 2015

@Souptacular if the semantics become "return any ether sent to it", then I would suggest using a keyword like DEACTIVATE as opposed to SELFDESTRUCT.

@ghost
Copy link

ghost commented Nov 28, 2015

@axic: I agree on this one, since SELFDESTRUCT implies removal from the blockchain entirely.

@Souptacular
Copy link
Contributor Author

@axic @ryepdx Good idea.

If we change the functionality of SUICIDE, we should change the name to match that function.

SELFDESTRUCT if the functionality stays the same

DEACTIVATE if we decide to change the functionality to return ether to sender (or tx.origin) address.

@vbuterin
Copy link
Contributor

In EIP 101, recipients will have to explicitly accept ether sent (if you use the msg.value parameter), so this functionality will exist by default. For ether sent using SEND, it might actually no longer be true that an address destroyed is inaccessible forever: you could just re-do the operation that was used to create the contract in the first place, and the new contract will have the same address (provided that it has the exact same init code). So I think that you'll get most of what you want even without SUICIDE/SELFDESTRUCT.

@Souptacular
Copy link
Contributor Author

@vbuterin That is good to hear that EIP 101 changes that functionality a bit. Although you can always remake the contract, being able to replicate the init code is going to be complex for some users and in that case it would be up to the contract creator to recreate the contract if someone unintentionally sent ether to it.

I am now questioning the need for suicide() altogether because currently it appears to have little benefit overall as indicated by the fact that many people are recommending to use "safe deactivation" style contracts like the ones @ryepdx and @axic created. @vbuterin do you think we could just remove suicide as an OPCODE or HLL code or would it cause complications?

Edited: Misread vbuterin's earlier comment so I added clarity.

@Smithgift
Copy link

+1 to changing the opcode name. SELFDESTRUCT is playful. SUICIDE is not.

In a world where the contract pays for its own continued existence, SELFDESTRUCT is a simple way to get the ETH out and tell the blockchain that the contract needs no longer exist at the same time. In an archiving world, some kind of ARCHIVE opcode would be polite, if nothing else.

@wanderer
Copy link
Member

@Souptacular the EIP looks fine and I'll merge this draft soon.

@chriseth
Copy link
Contributor

Implementation of the change in solidity: ethereum/solidity#256

@ethers
Copy link
Member

ethers commented Nov 30, 2015

@Souptacular Souptacular#1

wanderer added a commit that referenced this pull request Dec 1, 2015
@wanderer wanderer merged commit 05d50f5 into ethereum:master Dec 1, 2015
@axic axic mentioned this pull request Mar 1, 2019
bumblefudge pushed a commit to bumblefudge/EIPs that referenced this pull request Feb 16, 2024
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