Skip to content
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

EIP-1: disallow links to external domains #3358

Closed
wants to merge 1 commit into from

Conversation

fulldecent
Copy link
Contributor

@fulldecent fulldecent commented Mar 10, 2021

This PR updates EIP-1 to reflect current policy.

I think this is a poor policy and should not be implemented for any academic publication such as the EIPs.

Reference:


I protest this "do not link" requirement on two bases:

  1. There are currently 345 EIPS listed at https://eips.ethereum.org/all and all 345 of them do link to an external domain
  2. I have never seen an academic publication which discourages linking to authoritative sources of information

fulldecent added a commit to fulldecent/EIPs that referenced this pull request Mar 10, 2021
@MicahZoltu
Copy link
Contributor

There are currently 345 EIPS listed at https://eips.ethereum.org/all and all 345 of them do link to an external domain

Most of the rules we have for EIPs are not followed by older EIPs. 😄 The goal is to have quality increase going forward, but we don't currently have the staffing necessary to cleanup older stuff. In fact, we have a whole suite of tests against the repository that are disabled right now because so many old EIPs fail to pass even basic automated testing.

I have never seen an academic publication which discourages linking to authoritative sources of information

EIPs are technical standards, not academic documents. RFCs are a much closer analogy to EIPs than academic publications.

@MicahZoltu
Copy link
Contributor

Going to wait to see if any other editors comment on this before merging. Normally I give about a month for other editors to comment before merging changes to EIP-1 or the template, since in most cases no other editor will comment. 😢

@Souptacular
Copy link
Contributor

Could we write something like "It is highly discouraged to link to outside sources" rather than disallowing it completely? I see instances where outside source linking is important when showing complex implementation details/examples that may need to reside in another repo or even to provide context to why a certain ERC is an improvement to an ecosystem issue that needs to be highlighted by going to an external source rather than tediously outlining what another product/technology does.

@fulldecent
Copy link
Contributor Author

RFCs were cited above as a comparable reference publication that would should aspire to.

To download all the RFCs, load https://www.rfc-editor.org/in-notes/tar/RFC-all.tar.gz

You can remove any link to rfc-editor.org on those pages with

LC_ALL=C sed -I '' 's|https://www.rfc-editor.org||g' *

And now is a count of how many RFCs do or do not include links to external websites:

~/tmp/rfcs/RFC-all
❯ egrep -L 'https?://.*' * | wc -l
    2913

~/tmp/rfcs/RFC-all
❯ egrep -l 'https?://.*' * | wc -l
    5875

Before you check man grep please guess which -L -l is which.

@fulldecent
Copy link
Contributor Author

If there was another argument in favor of removing external links from new EIPs, please reference that discussion here.


And if there are no more remaining arguments FOR, I recommend this PR be closed due to the two strong arguments AGAINST at original post.

@lightclient
Copy link
Member

I'm okay with allowing links to high quality sources, but they should be expected to be reasonably permanent. AFAIK, RFCs don't usually link to things in their body. They use a citation format which may link to things externally (by URL or by name).

@MicahZoltu
Copy link
Contributor

MicahZoltu commented Mar 12, 2021

Sadly, we (EIP editors) don't have the manpower to figure out which links are "high quality" and which are "low quality". We also don't have the manpower to identify which are likely to be around for a while and which aren't.

As I mentioned on the EIPIP call, I'm already cutting quality and quantity of reviews due to availability constraints, and evaluating external links is a pretty big time sink.

I see instances where outside source linking is important when showing complex implementation details/examples that may need to reside in another repo or even to provide context to why a certain ERC is an improvement to an ecosystem issue that needs to be highlighted by going to an external source rather than tediously outlining what another product/technology does.

As I have argued elsewhere, I don't think this content should be in the EIP. I consider the EIP to be the technical specifications that is linked to by arguments, not that the EIP links to external arguments.

EIPs that are short and to the point are far more likely to get actually read and implemented than EIPs that include a 10 page dissertation on why a change in a good idea.

@MicahZoltu
Copy link
Contributor

MicahZoltu commented Mar 12, 2021

I'm okay with changing the wording to be a tad less absolute, though it is incredibly rare that I think an exception is reasonable. ETH2 specs repository is the one exception I have tentatively agreed to in recent history.

@lightclient
Copy link
Member

It just feels like EIPs are sitting in this inbetween place where they're supposed to be technical specs, but they also need to have a rationale section where the design decisions are explained but only if they can be explained in a few paragraphs without external links. All the while, they need to be choreographed through a hard fork process which is fully separate from the specification process, but requires certain spec stages to move forward in the hard fork coordination.

I won't pretend I know what the right solution is, but I think we're trying to solve everything at once and result is inconsistent.

@Arachnid
Copy link
Contributor

Arachnid commented Mar 14, 2021

I do think the current prohibition is too broad. Let's take a look at 1155, which I think is a very well written EIP.

It links to:

  1. https://json-schema.org/
  2. RFC 2119
  3. The reference implementation on GitHub
  4. Other example implementations
  5. Several articles and discussions supporting the need for the standard

I think it's reasonable to argue that 5, and to some degree 4 are unnecessary. But linking to the reference implementation is a definite benefit to readers, and prohibiting #1 and #2 is equivalent to asserting that no EIP should build on any outside standardisation work - which I think is the opposite of what we want to achieve here.

Also, under current rules all the recent hardfork meta EIPs are in violation, as they link to the eth1.0-specs GitHub repository.

@MicahZoltu
Copy link
Contributor

@Arachnid Why not inline the reference implementation or include it in ../assets/eip-XXX? An external link will eventually break. Also, the vast majority of "reference implementations" people link to are just like the go-ethereum repository, which is not very useful. If the reference implementation is actually self contained (like IMO it should be), then you can inline it or add it as an asset without much difficulty.

I'm not nearly as against linking to other standards organizations like IETF, RFC, etc. The vast majority of external links people submit however are to the following, none of which are good external links IMO:

  1. Personal repositories.
  2. Company repositories.
  3. Academic papers hosted on some random website.
  4. Draft specifications.
  5. Blogs.
  6. Twitter.

Also, under current rules all the recent hardfork meta EIPs are in violation, as they link to the eth1.0-specs GitHub repository.

Hard fork coordination is no longer handled in this repository. A weird exception was made for Berlin since the process change in the middle of Berlin coordination, but from here on out there will be no hardfork coordination EIPs (unless @axic wins our debate).

It is worth noting that we do not have the manpower to go back and fix old EIPs that don't conform to new patterns/processes, so all of the old hard fork meta EIPs still exist.

@Arachnid
Copy link
Contributor

Why not inline the reference implementation or include it in ../assets/eip-XXX? An external link will eventually break.

The reference implementation would ideally contain a set of unit tests etc. Putting all of that in an asset directory seems unwieldy, and unlikely to be effectively executable.

What if we allowed links to specific commit/tree hashes, for stability?

I'm not nearly as against linking to other standards organizations like IETF, RFC, etc.

Then we need to at least codify an exception for these. And bear in mind that not all standards are going to be from the IETF - the Json Schema standard example I gave above is a perfectly legitimate standard.

@MicahZoltu
Copy link
Contributor

Then we need to at least codify an exception for these.

This is part of the reason I have never actually codified my practice, it allowed it to be a little bit fuzzy and since I am the reviewer for probably over 90% of EIPs (with @lightclient picking up the remaining), it is pretty easy for me to just agree with myself on what the rules are. 😁 (yes, I'm aware that this is a dysfunctional system and I welcome a challenge to my unelected power in the form of anyone else taking up some of the load).

Semi-related to the above, EIP editors are incredibly understaffed and any suggested process that increases the amount of time required to review an EIP and/or enforce rules is a non-starter unless it adds tremendous value. "No external links" is an incredibly easy rule to enforce and takes me all of 10 seconds to add a comment when I see it being violated. If there is an expectation that editors review each individual link, that increases the burden on the already over-worked editors that will just result in burn out and then nothing getting reviewed.

In other words, if we had a team of editors who could take the time to carefully review all links then I would be more in favor of coming up with a complex set of requirements for external links. I We definitely don't have the time available at the moment to review every third party repository that is linked in the Reference Implementation section.

@MicahZoltu
Copy link
Contributor

To be fair to the opponents, there is the even lower effort option of allowing people to link to whatever they want in an EIP. IMO, this will result in a very significant decrease in average EIP quality since people love to link to twitter, random GitHub accounts, draft specifications, academic papers hosted on some university user account, etc. If we take the option of "anything goes", we will end up with a whole lot of terrible content in EIPs that currently is getting curated out.

@fulldecent
Copy link
Contributor Author

The way to reduce burden on editors in this project (and the role of people accepting contributions in every project) is to:

  1. Have clear, prescriptive, reasonable guidelines and project scoping.
  2. Empower other people to primarily police item 1.

Disallowing all external links is obviously a non-starter. As example, ERC-721 has a reference implementation (0xcert made it) and as an author it is surely my prerogative to direct people to use an implementation where I assert (and pay bug bounties) to ensure implementors won't get hacked.

ERC-721 also makes copious reference to discussions/tweets/meetings (sponsored by EF, thank you) demonstrating that consensus was achieved on this standard. This was important to me and without it there would not have been a single unified NFT standard that everyone uses. There are good arguments that such notes discussions need not be referenced in the EIP, which I would have respected did they exist.


The solution is to raise the bar.

The first step was to make a RSS feed and Last Call. That means I see every EIP /before/ it publishes, and I can give people a hard time if shitty/low effort work is coming out.

We should take your notes above, get them into a wording that is helpful and put the spirit into EIP-1. By putting it into there, somebody like me can walk into any EIP and say "please remove that link to Twitter, because EIP-1 says so, there are well-thought-out reasons for this rule which you can see".

By the time you, dear editors, see it, it will have already been cut down.


Proposed text follows. Please advise if I should update this PR to use the below text.

Guidelines on linking to external website

Editors pay attention to which websites your EIP links to. To ensure a smooth review process, you should too.

Do link to:

  • Authoritative standards and specifications which are required to properly implement your EIP, such as RFC 2119 (the RFC that defines the words "SHALL" and "MUST").
  • Complete, usable and fully test-cased implementations of the standard (no work-in-progress, no alpha/beta code, no failing test cases). For ERCs, the best implementation will be something you deployed to Mainnet where you will personally lose money if something is wrong with it.
  • Other normative specifications that would be duplicative or unwieldily to inline to the EIP text.

Do not link to:

  • Interfaces files. For ERCs, best practice is currently to inline Solidity or Vyper interfaces directly into the ERC with full NatSpec markup.
  • Motivating discussions (other than the discussions-to: link).
  • EIPs other than in "final" or "living" status, because such targets may change. (Rare exceptions may apply to this rule.)

@@ -207,6 +207,10 @@ EIPs may also have a `superseded-by` header indicating that an EIP has been rend

References to other EIPs should follow the format `EIP-N` where `N` is the EIP number you are referring to. Each EIP that is referenced in an EIP **MUST** be accompanied by a relative markdown link the first time it is referenced, and **MAY** be accompanied by a link on subsequent references. The link **MUST** always be done via relative paths so that the links work in this GitHub repository, forks of this repository, the main EIPs site, mirrors of the main EIP site, etc. For example, you would link to this EIP with `[EIP-1](./eip-1.md)`.

## Linking to outside sources

EIPs **MUST NOT** link to external domains.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EIPs **MUST NOT** link to external domains.
EIPs should link only to credible external domains at addresses that are expected to continue working for the foreseeable future. Ephemeral sources such as social media, chat logs, and comments should be disallowed. Discretion is left up to EIP Editors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be okay with something along these lines.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experience, "discretion left up to editors" is read as "please argue with the editors on why Peepeth is not an ephemeral source". As I mentioned above, my primary concern is that any "fuzziness" is going to eat precious editor time. Whether it is the editors reviewing every external link, or it is editors resolving disputes about what is "credible" or what is "ephemeral" doesn't really change the time sink much, just changes the mechanism.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we allow some links we either need to build a comprehensive list of valid sources and then maintain it or we need to allow editors to decide if the link is credible. I don't have a strong opinion between the two, only that I don't want to maintain a list right now.

Copy link
Contributor

@MicahZoltu MicahZoltu Mar 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't want to maintain a list. 😁 I think I would prefer something like:

Suggested change
EIPs **MUST NOT** link to external domains.
EIPs should avoid any external links. Some exceptions have been made by editors in the past such as linking to the `eth2.0-specs` repository or linking to an RFC at ietf.org in certain situations, but such exceptions are relatively rare and avoiding external links is the best strategy for getting your EIP merged.

The idea here is that it makes it clear to the reader that they probably won't get an exception and it intentionally doesn't give them specific things to argue that their link meets requirements of (like non-ephemeral and credible).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also fine to me.

@Arachnid
Copy link
Contributor

What should an EIP that needs to reference an RFC, or something like JSON-Schema do in this circumstance?

@axic
Copy link
Member

axic commented Mar 16, 2021

I am sorry for not having read every single comment here, while they do seem to be insightful, they are quite a bit to read. I will put two questions out here anyway:

  1. What is the main motivation behind this?
  2. Why does EIP editors have to enact and enforce this rule, and why cannot it be the fault of those who agree to the quality of the EIP/ERC when it is moved to Final, i.e. anyone not raising concerns at the latest during Last Call. Of course providing recommendations would be useful, but I am not convinced we should police the use of external sources.

@MicahZoltu
Copy link
Contributor

MicahZoltu commented Mar 17, 2021

1. What is the main motivation behind this?

@axic To maintain quality and consistency in quality across all EIPs.

2. Why does EIP editors have to enact and enforce this rule, and why cannot it be the fault of those who agree to the quality of the EIP/ERC when it is moved to Final, i.e. anyone not raising concerns at the latest during Last Call. Of course providing recommendations would be useful, but I am not convinced we should police the use of external sources.

If the editors don't stop people from publishing low quality EIPs, we will end up with a large volume of low quality EIPs. If we don't care about that, then the editor job becomes way easier (maybe even unnecessary). If we do care about maintaining consistent quality across all EIPs, we either need to pick quality guidelines that are incredibly easy to review/enforce (current process), or we need to onboard more active editors so we can take the time to debate with every EIP author who wants to color outside the guidelines.

@MicahZoltu
Copy link
Contributor

What should an EIP that needs to reference an RFC, or something like JSON-Schema do in this circumstance?

For RFCs linked to ietf.org, just submit the EIP and I'll likely comment that external links are recommended against but merge anyway. For json-schema, I'm a bit more hesitant. This is mainly because I see it as a slippery slope. I think https://json-schema.org/ is operated by ietf and has been around for a while (circa 2012), but I had to go look that up and I don't want to do that research for every link someone proposes (in fact, this is currently my primary argument against allowing any external links outside of a whitelist).

@MicahZoltu
Copy link
Contributor

  • Complete, usable and fully test-cased implementations of the standard (no work-in-progress, no alpha/beta code, no failing test cases). For ERCs, the best implementation will be something you deployed to Mainnet where you will personally lose money if something is wrong with it.

I'm not a fan of this one because the life expectancy of a link to a GitHub repository is generally pretty low. Repositories rename, organizations rename, users rename, etc.

  • Other normative specifications that would be duplicative or unwieldily to inline to the EIP text.

I'm not a fan of this one because it opens the floor up to too much debate from users who believe their link is a "normative specification" and as editors we don't have the time to review all of these claims.

@github-actions
Copy link

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added stale and removed stale labels May 27, 2021
@github-actions
Copy link

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jul 26, 2021
@github-actions
Copy link

github-actions bot commented Aug 2, 2021

This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

@github-actions github-actions bot closed this Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants