Ack, thanks for pointing that out! Should be fixed now.
I tried Option I in Parity Ethereum. Looks really simple to me because it applies universally to genesis (if not considering testnets). Some jsontests need to be regenerated, but we need to regenerate them anyway even if we decide to pull.
Proposal: Take some time to do some deep chain analysis to find out if there are any susceptible contracts that are actually in use, if not move forward with EIP as-is and update documentation to no longer advise users to rely on the stipend.
When critquing this proposal, pleases be clear whether your concern is with our ability to accurately find any such contract or that even if we don’t find such a contract we should still not move forward with this path.
1 Like
- This looks fine
- This should be a new opcode
- This is not in line with the idea of gas usage (gas usage is proportional to computation time) which is, if I recall correctly, the entire idea why 1283 was proposed
- This is only on the solidity side? Solidity explicitly forwards 2300 gas on transfers and calls. I don’t see how this can be changed
- I don’t like this one because now you have to forward much more gas than which is actually used.
I like 1) the most. This is simple and mitigates this attack.
In general, we should discuss what hard forks can change and what not. Since via solidity most developers think that if send or transfer is used re-entrancy attacks are mitigated, we cannot change this in a hard fork later because it is almost certain that there will be vulnerable contracts to this attack (because most contracts are written in solidity and we have a lot of those contracts so the chance of hitting at least a vulnerable one is really high). However, if we think about other forks in the past (such as changing gas usage of extcodesize) this might look straightforward but it could change the behavior of contracts which depend gasleft() and explicitly assume that this opcode uses a certain amount of gas. The question is now: what if all contracts except one handle this EIP fine, but the single contract is now broken? Can we now fork or not? This is not ethical but it is worth discussing.
Another, much more general idea is to include an opcode which writes a number to some special storage where this certain contract can opt-in for EIPs. Let’s say a contract is vulnerable for the reentrancy attack. Only the owner of the contract can call this function. The owner simply decides not to opt-in for this EIP while other contracts can. Now the contract is not vulnerable.
1 Like
Does the 2300 gas stipend have a purpose beyond preventing reentrancy?
Afaik, these calls were never supposed to change state. Making this explicit by introducing a new call context keeps expected behaviour consistent without shackling SSTORE. Couldn’t the gas stipend then be removed to allow for more logging?
@Flash I think that the 2300 gas stipend was indeed introduced to prevent against reentrancy per this stack exchange post and this discussion.
Introducing a new call context where everything is allowed except SSTORE might be interesting but it is starting to get very bloated with all the different call types (call, delegatecall, callcode, staticcall, this new one). Technically sending value is also a special kind of SSTORE since it alters the balance of an address. Logging (events) instead are not the same since these can never be SLOADed (or equivalent for the balance: BALANCE opcode).
What do you mean by “removing gas stipend”?
1 Like
-
Is good as a solution that does not require changes to the existing contracts after the release and allows for EIP-1283 to be delivered and remain important. It works nicely because it just blocks something that would not be allowed on Byzantium anyway (so no correctly built contract would expect this not to fail).
-
It may cause things that were expected not to fail in the valid contracts to fail after the change is introduced and we would have to specify which operations are allowed explicitly - LOG, PUSH?, DUP?, CALLCODE?, etc. - very likely that we would end up blocking SSTORE only in the end.
3,4,5 - all seriously decrease net gas metering impact
1 Like
I mean removing the then redundant 2300 gas cap on send and transfer in the case that we went with 2.
I wouldn’t call that bloated, it’s a useful call type. But I’ll happily leave that decision to others.
This would be a change in solidity? We can’t change contracts which are on the chain already, so we cannot simply change that any contract which forwards 2300 gas now suddenly forwards less.
In solidity, I think the reasonable change would be to assume that CALL always costs at least 700 gas and then hence only forward 700 gas. This can only lead to problems if the fallback function is so deep in the contract (too much solidity functions are around) such that the 700 gas is already used before it reaches the fallback function. Of course this can be optimized via a check that if CALLDATALENGTH==0 it immediately jumps to the fallback but this adds a slight amount of overhead gas before you get into any function block.
I think this is a bad idea both because we can’t guarantee we’ll identify all vulnerable contracts, and because we shouldn’t violate this invariant retrospectively.
FWIW - delegatecall is a different call type, but results in a regular call context (it executes as normal). Likewise for callcode. We wouldn’t need a new opcode for this, either.
I wasn’t aware Solidity does this. The call stipend is an EVM feature; Solidity could equally set the gas to 0 with the same effect.
In my opinion, the EIP-1283 does not need to be remediated, but rather removed all together. I do understand its intent (making inter-frame communications cheaper), and I proposed an alternative which is cheaper, slightly more powerful, and without the semantic changes to the existing opcodes:
Looking at the history EIP-1283, we see that it has been EIP-1087, formulated in a way which was harder to implement in Parity than in Geth. Then, it was reformulated into EIP-1283, to make it more abstract and less implementation-dependent. However, the semantic complexity was still there, and that caused a consensus issue on Ropsten, which delayed Constantinople once. And now it delays Constantinople once again, due to unintended consequences. Both of these incident can be traced to two things this EIP is doing that were not done before:
- Reducing refund counter (prior to that, refund counter was only increased). That was basis of the Ropsten bug
- Reducing the gas cost on an action. In all other hard forks, the gas cost of operations were only increased.
There is another potential impact of this EIP, which has not been appreciated yet. It will make harder for State Rent group to figure out alternatives to the State Rent, because the edge cases introduced by this EIP will make analysis harder.
My suggestion is to simply exclude this EIP from Constantinople. If the inter-frame communication is still in demand, it can be introduced by a more specialised change, rather than piggy-backing on the existing resource (contract storage). But honestly, I think this particular cost reduction is not a very important feature, and app developers can definitely live without it, while we are concentrating our efforts on Ethereum 1x and scalability improvements instead.
7 Likes
I agree with @Arachnid. Besides doing a deep chain analysis which will take very long to “look” correct it will be extremely hard to prove that there are a low amount of contracts susceptible. Keep in mind that simply looking for “drain” attacks are just part of the scope. It is also possible to do funky things to the contract by writing to some storage slots which messes up the logic of the contract. I think this not doable, but I have no experience with static analysis so that might just be my inexperience.
No this does not work, because the idea behind transfer and send is that you are also allowed to send Ether to contracts which have a payable fallback. These hence execute (given a solidity-compiled contract) the code to select a function: if none is found, it checks if there is a (payable) fallback. (Hence forwarding 0 gas will immediately result in an out of gas error).
Besides all this I agree with @AlexeyAkhunov to remove it altogether. I think in most cases writing to the same storage slot in a contract twice should be removed at compile time. At least in transactions which only happen during a single call this can be removed at compile-time. There might be some cases that this is harder if there are calls to other contracts (which call back into the current contract), but I think with some tricks this might also be possible. Hence the only case where this EIP is applicable is where a contract calls another one, which calls back into the current one. In all other cases more than single-writes to the same storage slot can be removed at compile time.
1 Like
IF we can gain the necessary confidence that this bug isn’t exploitable against any active contracts in the wild, why is maintaining this invariant necessary? Not only was the invariant only implied, not explicitly stated, but if no one is depending on it what do we gain by maintaining it?
2 Likes
Unfortunately, it cannot be removed at compile time, IMO. In order to do that, there would need to be an alternative resource in EVM that allows retaining information between the call frames. Currently, only storage is retained between the frames, since fresh memory is given to each frame, and the stacks are segregated too.
2 Likes
Yes, this is why I noted that you cannot remove it if there is a call to another contract which calls back into the current one. But you are right, it is more specific: it there are multiple call frames on the current contract then it is not possible to remove at compile time. However, I think there might be some kind of trick via the returnvalue of a call frame.
I’m generally against asserting that we will never decrease gas costs. In order for gas accounting to function appropriately we need the power/authority to be able to tune them up/down as hardware/software changes with time. While I can appreciate that having this ability makes things harder, I believe it is necessary for keeping Ethereum competitive.
IIUC, it sounds like you are sort of arguing that Ethereum 1.x is in maintenance mode and we basically shouldn’t be doing any active development on it that isn’t necessary for Ethereum 2.0? Any changes to the EVM should be proposed targeting Ethereum 2.0 instead?
I am not saying we should not decrease costs of operations. I am pointing out that there was complexity in EIP which was not on the plain sight. And it should have been given more weight when discussing the alternatives.
Yes. It has already been proposed by Vitalik at DevCon3, October 2017, in his “Modest Proposal for 2.0”. He suggested to keep Ethereum 1.0 “safe and conservative”. Of course, Ethereum 1x initiative kind of goes opposite to that, but only because we believe that there are SOME changes necessary to keep Ethereum 1.0 alive.
1 Like
You misunderstand; the EVM itself enforces the gas stipend. If you make a call to another contract that sends value with 0 gas, the recipient contract gets 2300 gas regardless.
That’s the whole point of the EIP - to make the use of temporary storage for cross-contract calls practical. Examples include mutexes and setting temporary token allowances.
It was explicitly designed in as an invariant, because it’s useful to be able to reason straightforwardly about ‘value sends’ while still providing the target contract with the opportunity to execute a little code. I don’t think we should throw that out, even in the (hypothetical and very unlikely) situation that we can without breaking anything already deployed.
2 Likes
This is how I understood it but I wasn’t aware that this was done by the EVM. Is there an issue with removing this gas stipend in the case that send and transfer use the new call context you described for 2.?
I agree that an invariant shouldn’t be changed.
Removing the stipend would break existing code that relies on being able to log an event when it’s sent funds, if it’s called by an existing contract.
1 Like