Add a reentrancy protection to EIP 1283#1701
Add a reentrancy protection to EIP 1283#1701forshtat wants to merge 4 commits intoethereum:masterfrom
Conversation
EIPS/eip-1283.md
Outdated
| * If *original value* does not equal *current value* (this storage | ||
| slot is dirty), 200 gas is deducted. Apply both of the following | ||
| clauses. | ||
| clauses. If *gasleft* is less then or equal 2300, fail the transaction |
There was a problem hiding this comment.
I would recommend to apply the gasleft check in the beginning, before any of the branches here are applied.
Co-Authored-By: alex-forshtat-tbk <[email protected]>
Co-Authored-By: alex-forshtat-tbk <[email protected]>
|
IMO this would have been better off as a stand-alone EIP, so fork definitions in node impl-ns don't have to invent branching Constantinoples. As described here. |
|
What do you think of replacing 200 gas cost with 2300 gas cost and 2100 gas refund? |
I think that as refund is limited to half of the transaction gas cost, this would in some cases limit the benefits of this EIP. Why do you think this is a better solution? |
|
Recommend changing 2300 -> 1600. As noted in the ChainSecurity article, the |
2300 is a magic number inherited from The former covers cases where The former makes an existing uncodified invariant explicit, whereas the latter guards against one pattern only. |
|
1283 is final and describes something implemented on test networks and in clients. If you'd like to propose an alternative version, please open a new EIP for that. |
6 participants
UPD: As per @Arachnid 's request, moved into a separate EIP: #1706
An attack is described in:
https://medium.com/chainsecurity/constantinople-enables-new-reentrancy-attack-ace4088297d9
This is easily mitigated if SSTORE is not allowed in low gasleft state, without breaking the backward compatibility and the original intention of this EIP.