Remove .send and .transfer.

[MicahZoltu]

Description

Once again, necessary gas cost adjustments in the EVM are being contested because people incorrectly have made assumptions that gas costs are fixed rather than variable. I think a lot of this stems from the fact that Solidity actively encourages this behavior through .send and .transfer methods. The only input into deciding the gas cost for any given operation is the operational cost of that instruction relative to other EVM instructions. As seen with Constantinople, and now Istanbul, the operational costs of various operations can change (both up and down) over time as EVM implementations gain/lose optimizations.

The advice that is constantly doled out telling people to use .transfer and send to protect from reentrancy has resulted in Constantinople being cancelled and Petersburg having some silly code to deal with the fact that the community has been giving bad advice to new Solidity engineers for years. Similar advice is now causing pushback against the proposed gas cost changes in Istanbul because people have hard-coded things like, if (gasleft() < 2300) I suspect largely because of the .transfer and .send methods.

I know that writing secure code is hard, and I'm a huge advocate for making writing secure code easier. However, .transfer and .send are likely going to eventually cause security issues (like almost happened with Constantinople) because it creates a false sense of security. Also, while we may not ever be able to drop SSTORE costs down to below 2300 in ETH 1.x because of legacy code that depends on it, ETH 2.0 or any other new platform running the EVM who doesn't have to support legacy contracts can set gas costs appropriately and not have to worry about breaking legacy code. However, as long as .transfer and .send exist people will continue using them and even new EVM based platforms will continue being in this bad place where gas costs are not calculated the way they should be.

TL;DR: Please remove .transfer and .send from Solidity (can deprecate for a couple years first) and advise people to follow development strategies that do not rely on on gas costs being fixed.

[jochem-brouwer]

I just discussed this with Micah over chat. I think a step in the right direction would be to explicilty note in the solidity docs that developers should be aware that .send or .transfer to a contract MIGHT fail in the future due to changed gas costs in a fork (if the contract is meant to only transfer to EOA's then I think it is fine to use this - but the developer SHOULD BE aware of this).

I also think that noting that "use transfer or send to protect against re-entrancy attacks" should be removed because I do not think it is part of the EVM design that the gas stipend (the 2300 gas) should only be used for logging and that it can be assumed that no SSTOREs get executed.

In general Solidity is currently the language most new developers use. This is hence also the gate to learning how to develop on Ethereum. Hence solidity has an important position to make sure that developers are aware of the not-so-obvious differences between code on Ethereum and "normal" code.

[chriseth]

Before we can remove send and transfer, we need a new function that forwards all gas. I don't think that .call.value(x)() should be the recommended way to transfer ether.

I second the suggested changes to the documentation.

We should also note (if that is not already part of the "withdraw pattern" documentation) that you should deal with the fact that a "forward all gas" call can always get you stuck because it consumes all gas.

[jochem-brouwer]

Good point on the consume all gas. Keep in mind that you will always keep 1/64 of the available gas which you had before a call due to the call depth limit eip.

[MicahZoltu]

We should also note (if that is not already part of the "withdraw pattern" documentation) that you should deal with the fact that a "forward all gas" call can always get you stuck because it consumes all gas.

I think it is more general to assert that:

calling external code (which includes ETH transfers) can always trigger a revert and there is nothing you can do to prevent this.

For the same reason as mentioned above, I think it is a bad idea for contracts to try to do any sort of conditional logic based on gas remaining or to use gas limiting as a means of controlling the negative impact a contract can have on operation, or ensuring that you "have enough gas leftover to finish". The two exceptions I may make is for a contract to always spend a percentage of gas on a child call. This allows the contract to continue to function even in the face of of changing gas prices, or to allow a user-specified "leftover", so gas price changes can be accommodated through UI updates.

Really, the withdraw pattern is the way to go and contracts should simply not call untrusted external code outside of those contexts.

[chriseth]

If we are saying "the withdraw pattern is the way to go", then maybe we should make that more explicit by disallowing send and transfer and replacing them by a new function called withdraw (bad naming, we should come up with a better one) that does the following:

withdraw(x) sends x wei to msg.sender (forwarding all gas) and terminates the execution (unless the send reverted in which case it will revert).

Terminating the execution is important because you should do state change before that anyway.

There is still the problem that the containing function might need return values.

[MicahZoltu]

🤔 Terminal functions in general are a neat idea. It would be even cooler though if the compiler instead were able to guarantee that no storage writes occurred after the call, and any external function calls were STATIC_CALL. That would allow you to do things like logging, or prep data for returning.

Of course, this doesn't protect the user from reentrancy since we cannot enforce at the language level that the caller continues to follow those rules. I believe we would need a new EVM instruction that would set a transaction level flag that says "no state writes from this point on".

[jochem-brouwer]

@MicahZoltu Basically that is what STATICCALL does - it does not allow any state writes a level deeper in the call frame. If a static called contract re-enters the current contract and tries to modify state there, it will fail.

I think solidity should start using STATICCALL for transfers and sends. The only downside is that STATICCALL is strict - you also cannot LOG events. - does not work because STATICCALL cannot transfer value as this obv. changes state as pointed out by @MicahZoltu

[MicahZoltu]

@jochem-brouwer Can you attach value to a STATICCALL? I would expect not since moving ETH around is a state change.

[jochem-brouwer]

@MicahZoltu yes of course, you are right, the value is always zero. It would hence not work when transfering ETH.

[rmeissner]

Is there any plan how to move this forward. With the current changes to the sload gas costs and future changes to event gas costs (that might be coming), it would be really nice to avoid that the 2300 gas limit is further used.

[chriseth]

If even the events get more costly, then maybe the EIP process should consider either removing or increasing the stipend.

[MicahZoltu]

Changing the EVM stipend is much harder than changing Solidity's bytecode generation because of backward compatibility. Even with the EVM having a stipend, the .transfer and .send recommendations are still bad ideas.

[chriseth]

I'm perfectly fine with introducing new functions, but we have to find good names. Because of try/catch, one would actually suffice. What about allowing .gas() for now?