Skip to content

Conversation

aaronisme
Copy link
Contributor

Update some mirror change to the EIP and move it into review

@aaronisme aaronisme requested a review from eth-bot as a code owner November 21, 2022 04:42
@github-actions github-actions bot added c-status Changes a proposal's status s-review This EIP is in Review t-erc labels Nov 21, 2022
@eth-bot
Copy link
Collaborator

eth-bot commented Nov 21, 2022

A critical exception has occurred:
Message: pr 6013 is already merged; quitting
(cc @alita-moore, @mryalamanchi)

@aaronisme
Copy link
Contributor Author

@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.

@Pandapip1 Pandapip1 changed the title update the eip detail and move to review Update EIP-4527: Move to review Nov 21, 2022
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
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

Copy link
Contributor Author

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.

@Pandapip1 Pandapip1 changed the title Update EIP-4527: Move to review Update EIP-4527: Move to draft Nov 21, 2022
@github-actions github-actions bot added s-draft This EIP is a Draft and removed s-review This EIP is in Review labels Nov 22, 2022
Copy link
Member

@Pandapip1 Pandapip1 left a 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
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
status: Draft
status: Draft

This is a small typo, but it blocks this PR.

Copy link
Contributor Author

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
Comment on lines 231 to 237
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
Copy link
Member

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

Copy link
Contributor Author

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
Copy link
Member

Choose a reason for hiding this comment

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

This one too

Copy link
Contributor Author

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__
Copy link
Member

Choose a reason for hiding this comment

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

Must be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@aaronisme
Copy link
Contributor Author

@Pandapip1 I have fixed the comments as requested, Please check thanks

EIPS/eip-4527.md Outdated
Comment on lines 233 to 235
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
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
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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@aaronisme
Copy link
Contributor Author

@Pandapip1 I have updated the file as requested

@eth-bot eth-bot enabled auto-merge (squash) November 27, 2022 22:58
@eth-bot eth-bot merged commit 3826404 into ethereum:master Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-status Changes a proposal's status s-draft This EIP is a Draft t-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants