Eliminate side effects from failed transactions.

[destenson]

The recent exploitation in a bug in the DAO's contract showed a massive deficiency in Ethereum that I was not aware of. The Solidity documentation says that throw

        // `throw` terminates and reverts all changes to
        // the state and to Ether balances. It is often
        // a good idea to use this if functions are
        // called incorrectly. But watch out, this
        // will also consume all provided gas.

The DAO's contract code and other code that is vulnerable to the same bug are written to rely on that statement being true. We think that if we throw, the transaction will be voided & nulled (minus spent gas). Apparently this is not the case. Can you make it be please? Otherwise, Ethereum will never be taken seriously again.

[JasonCoombs]

That documentation is a recommendation for well-behaved code. There are other technical issues also which pose existential risks for Ethereum as a "contract-oriented" platform and for Solidity as a "contract-oriented programming language" intended to be taken seriously and used widely... See ethereum/solidity#598 (comment)

[taoteh1221]

@destenson I think the root issue here is that ALL transaction-related calls in higher level languages like Solidity in Ethereum should ALWAYS have a boolean == false return value if things like the stack limit reached, etc happen in the virtual machine...my understanding is that this is not the case right now. Therefore I don't believe 'throw' is the issue if I'm understanding this correctly (somebody please correct me if I am wrong), but rather the conditional statement that calls the 'throw' getting a reliable return value. If transaction-related calls always returned boolean == false reliably, then programmers of the higher level smart contract languages like Solidity do not need to do anything except look for a boolean == false value to 'throw'.

Example: Currently a programmer can get into a lot of trouble using 'send', without checking if the stack limit in the virtual machine is reached yet (I believe this is exactly what happened to TheDAO if I am not mistaken?). Checking if the stack limit has been reached has to be done explicitly in Solidity, with Solidity code. Surely not EVERYONE does or will EVER do that.

I created an issue for this in the Solidity repo earlier today: ethereum/solidity#663

[destenson]

@JasonCoombs Code will not always be well-behaved, I can guarantee you that. Isn't that the reason for a throw in the first place? To say "No way, something's wrong. We can't proceed. Undo everything!" At least that's what the documentation told me.

[destenson]

@taoteh1221, I am no expert, but I think the attacker intentionally caused it to end before the stack limit was reached, after about 28000 transactions in total. I did see your issue on Solidity as well. I think the exception-handling code needs to do a better job of rolling-back transactions, as should occur in the event of a stack overflow.

I think the root issue here is that a cryptocurrency, which is intended to be a modern and advanced bookkeeping system with computational capabilities deployed as smart contracts, failed to do the fundamental bookkeeping expected of it on a large and visible scale. If that's not fixed on the blockchain level eventually, Ethereum is done.

Ethereum 2.0 (or whatever may succeed it) will need provide bookkeeping functions to smart contract code instead of leaving it up to smart contract authors. I assumed from the documentation that this was already done behind the scenes by the compiler and enforced by the EVM.

Also, this is not purely a compiler issue, since the compiler doesn't have knowledge of code from deployed contracts that may or may not fail unexpectedly.

[taoteh1221]

@destenson Honestly I do not know the complete details of TheDAO attack. I was just looking over Christian Reitwiessner's blog post a few minutes ago on blog.ethereum.org from June 10th, and it actually looks like 'throw' is VERY dangerous to use, because it can stall a function in the middle of it running: https://blog.ethereum.org/2016/06/10/smart-contract-security/
Did TheDAO use 'throw' in the vulnerable code? This 'throw' function should probably be eliminated completely, the less proficient Solidity programmers out there will likely shoot themselves in the foot with this again and again. There has to be a better way to rollback a failed contract execution, like in the VM or something.

EDIT: Maybe they don't contract execution failure rollback in the VM because of the consequences of an endless loop in a contract call on the entire network. Not sure.

[destenson]

@taoteh1221, I think one cause is that the transaction that ends up throwing may occur in a different block from the one that originated it. In that case, a failed transaction can't successfully undo a block since it's already been accepted into the blockchain.

EDIT: As pointed out by vbuterin, I am mistaken about this. The exploit took advantage of the DAO contract not updating its internal state variables before calling a function that withdraws ETH and re-entered splitDAO(). Each time splitDAO() was entered it acted as if it were the first time. (I still don't quite understand why it ended when it did, though.)

[vbuterin]

I think one cause is that the transaction that ends up throwing may occur in a different block from the one that originated it.

This is false. The effect of a transaction happens fully synchronously, within the block that includes the transaction and before processing any future transactions.

[destenson]

@vbuterin, hmm, I understand the recursive nature of the exploit more than I do now when I wrote that. I apologize for that. Should functions on contracts be re-entrant by default? Was it designed to allow that so easily without intention? Or is that an unintended "feature"?

Also, I'm glad to know that transactions are intended to be fully synchronous. Some of the reports I read implied that was not the case. Why did the attack take so long? I would've expected it to be finished in a few blocks, not dragged out over the course of hundreds or thousands of blocks. Help me understand, theoretically, if someone emptied the contract's balance before the attacker did, would it have stopped the attack, or would Ethereum have prevented another transaction from empty the it since the Ether were already reserved by the attack?

[destenson]

I'm going to close this, because it seems that it was not a legitimate issue to begin with. I misunderstood the nature of the exploit & it doesn't look like this is the appropriate place to fix it. Thanks.

[destenson]

@vbuterin, I'm not sure where to ask this, but your comment provokes the question: if a transaction always occurs fully synchronously, is there a way for a contract to cause another transaction to occur in a future block, say the next one? Is a contract possible that could receive a payment, and return it on the next block, and not the same block?