Update ERC-4527: Move to Review #694
Conversation
eip-review-bot
changed the title
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
ERCS/erc-4527.md
Outdated
| @@ -45,7 +45,7 @@ In order to work with offline signers, the watch-only wallet should follow the f | |||
|
|
|||
| Since a single QR Code can only contain a limited amount of data, animated QR Codes should be utilized for data transmission. The `BlockchainCommons` have published a series of data transmission protocol called Uniform Resources (UR). It provides a basic method to encode data into animated QR Codes. This EIP will use UR and extend its current definition. | |||
There was a problem hiding this comment.
The
BlockchainCommonshave published a series of data transmission protocol called Uniform Resources (UR).
This is an external link. We generally don't permit links to external resources, except for the exceptions listed in EIP-1. You can see our rationale over here: https://ethereum.github.io/eipw/markdown-rel-links/
There are two options for referring to this external standard:
- Copy the relevant files into your assets directory, and link to them there. Unless I'm looking at the wrong repository, the UR specs are BSD licensed, so it would be fine to reproduce them here. Just include the license file alongside.
- Make a pull request adding Blockchain Commons as a permitted external source. The process for that is documented in EIP-5757.
There was a problem hiding this comment.
Hi @SamWilsn, I removed the links since they weren't necessary for this descriptive text.
ERCS/erc-4527.md
Outdated
| The following specification is written in Concise Data Definition Language `CDDL`. | ||
| UUIDs in this specification notated UUID are CBOR binary strings tagged with #6.37, per the IANA `CBOR Tags Registry`. | ||
| The following specification is written in Concise Data Definition Language (CDDL). | ||
| UUIDs in this specification notated UUID are CBOR binary strings tagged with #6.37, per the IANA CBOR Tags Registry. |
There was a problem hiding this comment.
Same kind of external link problem here. The IANA CBOR Tags Registry isn't a permitted external resource. I think you could get away here just removing this text.
|
Hi @SamWilsn I have updated the PR based on your comments, please check it. Thanks! |
| @@ -43,20 +43,20 @@ In order to work with offline signers, the watch-only wallet should follow the f | |||
|
|
|||
| ### Data Transmission Protocol | |||
|
|
|||
| Since a single QR Code can only contain a limited amount of data, animated QR Codes should be utilized for data transmission. The `BlockchainCommons` have published a series of data transmission protocol called Uniform Resources (UR). It provides a basic method to encode data into animated QR Codes. This EIP will use UR and extend its current definition. | |||
| Since a single QR Code can only contain a limited amount of data, animated QR Codes should be utilized for data transmission. The BlockchainCommons have published a series of data transmission protocol called Uniform Resources (UR). It provides a basic method to encode data into animated QR Codes. This EIP will use UR and extend its current definition. | |||
There was a problem hiding this comment.
This paragraph is still an external link (even though it lacks an explicit hyperlink.) ERC-4527 isn't implementable without also reading the Uniform Resources standard.
We require that such information fulfill any one of the following:
- It is included in the EIP or its assets directory (license permitting);
- It is hosted on a source permitted in EIP-1 (eg. IETF or W3C); or
- It has been assigned a DOI.
We have these requirements in place to ensure that external information doesn't disappear or change once the EIP is finalized. BlockchainCommons hasn't been approved as an external origin.
I'll draft up a pull request copying the relevant standards here.
There was a problem hiding this comment.
Huh, as it turns out, these are deeply interlinked and would be a pain in the ass to rehost here. I'll figure something else out.
There was a problem hiding this comment.
Hi @SamWilsn anything to do for moving the PR forward?
There was a problem hiding this comment.
Nothing on your end. I'll make sure this moves along on mine.
add new fields on the sign request and change the status to review