-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Update EIP-4527: Move to draft #6013
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
Conversation
A critical exception has occurred: |
@axic, @SamWilsn, @Pandapip1 would you please review this change? And I am not sure whether it is ok to move stagnant to review. I checked the EIP-1, from my understanding it is ok to do it. |
EIPS/eip-4527.md
Outdated
@@ -4,7 +4,7 @@ title: QR Code data transmission protocol for the offline wallets | |||
description: QR Code data transmission protocol between wallets and offline signers. | |||
author: Aaron Chen (@aaronisme), Sora Lee (@soralit), ligi (@ligi), Dan Miller (@danjm), AndreasGassmann (@andreasgassmann), xardass (@xardass), Lixin Liu (@BitcoinLixin) | |||
discussions-to: https://ethereum-magicians.org/t/add-qr-code-scanning-between-software-wallet-cold-signer-hardware-wallet/6568 | |||
status: Stagnant | |||
status: Review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be moved back to draft, since it never made it to review before it was stagnated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pandapip1 Hi, I have moved it back to draft, please check it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GitHub links should be removed (if unnecessary) or their contents moved to the EIP's assets folder and updated to link there.
EIPS/eip-4527.md
Outdated
@@ -4,17 +4,20 @@ title: QR Code data transmission protocol for the offline wallets | |||
description: QR Code data transmission protocol between wallets and offline signers. | |||
author: Aaron Chen (@aaronisme), Sora Lee (@soralit), ligi (@ligi), Dan Miller (@danjm), AndreasGassmann (@andreasgassmann), xardass (@xardass), Lixin Liu (@BitcoinLixin) | |||
discussions-to: https://ethereum-magicians.org/t/add-qr-code-scanning-between-software-wallet-cold-signer-hardware-wallet/6568 | |||
status: Stagnant | |||
status: Draft |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
status: Draft | |
status: Draft |
This is a small typo, but it blocks this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Pandapip1 I have updated the file, would you please check it again?
EIPS/eip-4527.md
Outdated
This reference implementation is in Javascript and available at: https://github.com/KeystoneHQ/keystone-airgaped-base/tree/master/packages/ur-registry-eth | ||
|
||
Metamask has adopted it for its integration with QR-based Signers. https://github.com/MetaMask/metamask-extension/pull/12065 | ||
|
||
BlockchainCommons Uniform Resources: https://github.com/BlockchainCommons/Research/blob/master/papers/bcr-2020-005-ur.md | ||
|
||
BlockchainCommons UR Types: https://github.com/BlockchainCommons/Research/blob/master/papers/bcr-2020-006-urtypes.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These links must be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
EIPS/eip-4527.md
Outdated
BlockchainCommons Uniform Resources: https://github.com/BlockchainCommons/Research/blob/master/papers/bcr-2020-005-ur.md | ||
|
||
BlockchainCommons UR Types: https://github.com/BlockchainCommons/Research/blob/master/papers/bcr-2020-006-urtypes.md | ||
|
||
Here is a video that shows how it works: https://www.youtube.com/watch?v=1eM53TYG1YA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
EIPS/eip-4527.md
Outdated
Currently, there is no existing protocol to define data transmissions via QR Codes so there are no backward compatibility issues that needs to be addressed now. | ||
|
||
## Test Cases | ||
The reference implementation contains the [test cases](https://github.com/KeystoneHQ/keystone-airgaped-base/tree/master/packages/ur-registry-eth/__tests__). | ||
|
||
The reference implementation contains the `test cases`: https://github.com/KeystoneHQ/keystone-airgaped-base/tree/master/packages/ur-registry-eth/__tests__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
@Pandapip1 I have fixed the comments as requested, Please check thanks |
EIPS/eip-4527.md
Outdated
Metamask has adopted it for its integration with QR-based Signers. https://github.com/MetaMask/metamask-extension/pull/12065 | ||
|
||
Here is a video that shows how it works: https://www.youtube.com/watch?v=1eM53TYG1YA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metamask has adopted it for its integration with QR-based Signers. https://github.com/MetaMask/metamask-extension/pull/12065 | |
Here is a video that shows how it works: https://www.youtube.com/watch?v=1eM53TYG1YA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
@Pandapip1 I have updated the file as requested |
Update some mirror change to the EIP and move it into review