Add deprecate CALLCODE EIP #2488
Conversation
axic
force-pushed
the
deprecate-callcode
branch
from
fc2a5be to
213ea42
Compare
There was a problem hiding this comment.
Only change I would really want to see before merging is the addition of the Simple Summary to match the template. While I know some editors consider the Simple Summary optional, I personally consider it required in the face of ambiguity (template doesn't explicitly say either) since I think consistency is valuable and in almost all cases it does add some value and allows for tools to include a one-line description as subtext in a list.
|
|
||
| ## Abstract | ||
|
|
||
| Deprecate `CALLCODE` in a *somewhat* backwards compatible way, by making it always return failure. |
There was a problem hiding this comment.
| Deprecate `CALLCODE` in a *somewhat* backwards compatible way, by making it always return failure. | |
| Deprecate `CALLCODE` (`0xf2`) in a *somewhat* backwards compatible way, by making it always return failure. |
There was a problem hiding this comment.
I think this (and other comments) aligns with the new OPCODE referencing systeme that you recently proposed.
There was a problem hiding this comment.
Hmm, after leaving all of these suggestion comments, maybe the intent is that this style is only applied in the Specification section? If so, feel free to reject all of the suggestions that just add the parenthetical callcode.
There was a problem hiding this comment.
EIP-1 has this (from #1968):
So it is not super clear. I would be inclined to say mentioning it at the first reference in the Specification is a reasonable option, otherwise it would become annoying.
|
|
||
| ## Motivation | ||
|
|
||
| `CALLCODE` was part of the Frontier release of Ethereum. In the first few weeks/months it became clear |
There was a problem hiding this comment.
| `CALLCODE` was part of the Frontier release of Ethereum. In the first few weeks/months it became clear | |
| `CALLCODE` (`0xf2`) was part of the Frontier release of Ethereum. In the first few weeks/months it became clear |
|
|
||
| `CALLCODE` was part of the Frontier release of Ethereum. In the first few weeks/months it became clear | ||
| that it cannot accomplish its intended design goal. This was rectified with introducing `DELEGATECALL` | ||
| ([EIP-7](https://eips.ethereum.org/EIPS/eip-7)) in the Homestead update (early 2016). |
There was a problem hiding this comment.
| ([EIP-7](https://eips.ethereum.org/EIPS/eip-7)) in the Homestead update (early 2016). | |
| ([EIP-7](./eip-7)) in the Homestead update (early 2016). |
Relative paths will make the links work in GitHub, forks, branches, mirrors, etc.
There was a problem hiding this comment.
Absolute paths are the standard required, it has been the case for years.
There was a problem hiding this comment.
I have been told in the past to use relative paths (I think it was @Arachnid, quite a while back). Perhaps we should discuss this elsewhere, but is there somewhere I can read to catch-up on any history of discussion on this issue?
There was a problem hiding this comment.
Over the past 2 years there was an extensive amount of work, mostly by @fulldecent, to change every single link to the absolute. I cannot spend the time right now to dig up the issue numbers.
@Arachnid probably said that probably in 2016-2017. I also preferred the relative paths, but sentiment agreed with the absolute ones.
There was a problem hiding this comment.
I'd suggest this discussion is handled outside of this pull request.
| ## Motivation | ||
|
|
||
| `CALLCODE` was part of the Frontier release of Ethereum. In the first few weeks/months it became clear | ||
| that it cannot accomplish its intended design goal. This was rectified with introducing `DELEGATECALL` |
There was a problem hiding this comment.
| that it cannot accomplish its intended design goal. This was rectified with introducing `DELEGATECALL` | |
| that it cannot accomplish its intended design goal. This was rectified with introducing `DELEGATECALL` (`0xf4`) |
| that it cannot accomplish its intended design goal. This was rectified with introducing `DELEGATECALL` | ||
| ([EIP-7](https://eips.ethereum.org/EIPS/eip-7)) in the Homestead update (early 2016). | ||
|
|
||
| `CALLCODE` became never utilized, but it still puts a burden on EVM implementations. |
There was a problem hiding this comment.
| `CALLCODE` became never utilized, but it still puts a burden on EVM implementations. | |
| `CALLCODE` (`0xf2`) became never utilized, but it still puts a burden on EVM implementations. |
It has been removed as a requirement in #2186, but apparently the template was not updated. #2186 was triggered by countless complaints from EIP authors about the ambiguity the two overlapping sections cause and the wasted work it took writing them. |
|
Ah, we should definitely update the template to match with EIP-1. I'm a bit sad as I think the Simple Summary really did add value when it comes to formatted text. Kind of like how a forum may have a title and a short-description and the short description renders as subtext below the title in some contexts while only the title is rendered in other contexts. The abstract is almost always too long to render as a "long description", which means we now just have the title (usually like 5 words) and the abstract (a couple paragraphs) and nothing in-between. 😢 None the less, you are correct that EIP-1 says so, and it is just an issue with inconsistency between our own internal documentation. |
|
@MicahZoltu please consider a new review based on the these comments 😉 |
|
Also it is fine delaying this until when the TODO line is filled out. |
* Add deprecate CALLCODE EIP * Rename to EIP-2488 * Add discussion URL * Add requires field and security considerations * more motivation * more clarity of backwards compatibility
* Add deprecate CALLCODE EIP * Rename to EIP-2488 * Add discussion URL * Add requires field and security considerations * more motivation * more clarity of backwards compatibility
No description provided.