-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Call for Input: Remove External Implementations from ERC-20 #287
Comments
Responding to the points @Dexaran raised on their pull request:
The standard was written against solidity 0.4, and is specified in terms of that version. If solidity released a version 1.0 with a different calling convention, we wouldn't update ERC-20 to use that calling convention because that would invalidate all existing ERC-20 tokens. By that logic, I don't believe that the age of the reference implementation is a good argument for removing it.
EIP-1 only requires that the EIP is CC0, and doesn't really specify whether assets are part of the EIP or are separate works. I usually operate under the policy that assets can be any license that doesn't impose too much on implementers. We have a discussion thread open to come up with an official policy. All that to say that if we wanted to move an MIT-licensed reference implementation into the EIPs repository, I wouldn't object. Further, since we do allow linking to non-CC0 resources (eg. IETF RFCs, WHATWG specs, etc.), I don't believe the license is a sufficient argument for removing these links.
This is, at least somewhat, convincing. We should maintain credible neutrality as much as possible, and having a link to a particular external implementation directly in an EIP somewhat violates that principle, especially since we cannot update EIPs as new implementations are created. That said, I think we need to balance this credible neutrality against our promise of immutability. ERC-20 is Final, and shouldn't be changed. Our policy on external links didn't exist when ERC-20 was written. Obviously if it were written today, ERC-20 wouldn't have those links. I am going to have to oppose this decision, though I'm open to being convinced otherwise. |
I vote no (but still don't have a vote). Final ERCs should be final. Other ERCs have links/references to specific implementations e.g. https://eips.ethereum.org/EIPS/eip-721#implementations If this PR is merged then the same should be done for every ERC rather than singling one out. |
This comment was marked as off-topic.
This comment was marked as off-topic.
My understanding is that this was just proposed for ERC-20 which Dexaran has a very public gripe about. |
This comment was marked as off-topic.
This comment was marked as off-topic.
"The standard" is just a set of abstract rules described in text. Any particular implementation is not standard. So I see no reason to mislead potential implementers by providing them with an insanely outdated code. The EIP repo is supposed to help people to build useful things. And the presence of this links doesn't help in any sensible way and is potentially misleading from my point of view. I would also argue that solidity version bumps must be allowed even for "final" EIPs but this is not the topic of this PR.
You also don't allow new EIPs to have links.
It looks like a serious endorsement (which again goes directly against EIP-5069) for any person who is not deeply involved in the EIP process. Other users certainly interpret it as an endorsement. |
I do have a lot of concerns about EIP-20, most importantly the fact that it caused a loss of $200M for the end users. As the result I work quite often with the proposal text so I spotted a detail that must be improved in my opinion. |
I vote +1, for a variety of reasons:
FWIW I do agree that all external reference implementations should be removed or inlined. I dismiss the following counterarguments:
Note: @Dexaran's gripe with ERC-20 is very much justified. Nobody should be arguing that the problem doesn't exist. If money is being lost, then there's a problem and the responsibility lies somewhere. I would argue ERC-223 is not the best way to solve the problem, because the responsibility lies with wallets not providing a good UX when submitting transactions, not with the token contracts themselves. |
+1 , the implementation references and details can happen in discussions-to link, don't need to be part of the standard |
Removing the links is also not consistent with EIPs remaining final. I'd argue that removing the links is more disruptive than keeping them. Both links are qualified with a commit hash.
Both the linked OpenZeppelin and Consensys implementations are MIT. As everyone should be well aware by now (:wink:), I fully support MIT licensed reference implementations. We can take this particular argument over here. Hell, we could move both of those implementations into the ERCs repository, and be done with this.
The OZ implementation clocks in at 205 lines (plus 65 for
If we don't just move the implementations into the assets directory (which right now is my preferred solution), I'd be okay adding a
The whole concept of "normative" is mentioned twice in EIP-1: here and here. The former resets the last call period, and the latter explicitly states:
Those links were not against the rules at the time, which is a justification we've used recently, and so are not an errata. I'd hardly call removing example code "add[ing] non-normative clarifications". |
This is your most convincing argument IMO. However, if I get permission from the authors, would that change your mind? |
😸
Should authors have extra authority over anyone else once an EIP goes to final? I don't really think so. If we are going to do something with these links, would everyone be happy with moving these files into the |
The precedent was that if the editors and the people responsible for an action disagree with whether the action was 'wrong' then EIP-1 takes precedence. If the editors and the people responsible for an action agree the action should be reverted, and the community seems to be in favor of removing the links... why wouldn't we? |
From @Pandapip1:
|
Here's my attempt at a solution that satisfies everyone: ethereum/ERCs#112 The reference implementations are moved into the ERCs repository, under the MIT license. |
i think once something becomes "commodity" reference implementations can be dropped, imo (although not a strongly held one) reference implementations should/could exist in discussions-to or we can have reference implementations or spec links section If reference implementations have uncovered vulnerabilities, a final EIP should be allowed to mark them deprecated and refer to a fixed one. If the EIP itself is flawed, it should be marked deprecated/superceeded by |
Having a permissively licensed reference implementation that the proposal itself can refer to is pretty nice, IMO. For example, when the specification section describes an algorithm in English, having the code to refer to is useful as a developer (eg. EIP-2315). What benefit do we get by removing them?
I am not against a |
Would everyone mind restating their opinion on this? I fear I've made it more confusing by adding my pull request. The options (as I see them) are:
I think we should move the ERC-20 reference implementations into the ERCs repository (see ethereum/ERCs#112.) |
As an associate I don't have a vote. I would rather option 4: that implementations and security sections for every ERC link to a wiki page on Eth Magicians, so they can be maintained by the community. If this PR is merged then the same should be done for every ERC rather than singling one out. |
|
So to summarize my Keeper ruling, we have:
This is a difficult one to judge, because there is no clear prevailing opinion. Deciding between doing something and doing nothing is a pretty easy call, however. Only @xinbenlv was of the opinion that we should leave the links unchanged. The rough consensus is pretty clear: do something with the links. I am not choosing "remove the links" because only @g11tech has expressed clear support in removing the links, while I am extremely opposed to changing the meaning of a Final proposal. If @lightclient was more strongly in favour of removing the links, I would've gone with this option. So that leaves moving the reference implementations into our repository. |
Call for Input
Do we merge ethereum/EIPs#7886?
ethereum/EIPs#7886 is merged, removing links to external implementations.
Background
See ethereum/EIPs#7886.
The text was updated successfully, but these errors were encountered: