Resurrect EIP-1328 #4416

ligi · 2021-11-03T10:42:59Z

EIP-1328 is heavily used in WalletConnect - and very unlikely to change - but as I understand the process we now need to move it to Draft then to Review then to Last Call then to Final. Starting this process now - cc @pedrouid

eth-bot · 2021-11-03T10:43:44Z

All tests passed; auto-merging...

(pass) eip-1328.md

classification
`updateEIP`

passed!

pedrouid · 2021-11-03T11:02:52Z

Seconded! We should also update the spec with parameters for v1.0 and v2.0

axic · 2021-11-03T12:01:28Z

I think you can bring it to Review directly given it is mostly ready.

ligi · 2021-11-03T12:02:57Z

@axic thanks - but according to this state-transition diagram I cannot:

axic

Please update this to the current EIP-1 standards:

Add description: preamble and remove Simple Summary
Optionally add Motivation
Remove References
Add Backwards Compatibility
Add Security Considerations
Make sure at least one author has a GitHub handle and not an email

axic · 2021-11-03T12:03:59Z

@axic thanks - but according to this state-transition diagram I cannot:

Good catch. I think this is somewhat inconsistent, given if you have caught the bot at the time of making the change, you wouldn't need to do two steps.

@MicahZoltu @lightclient do you want to enforce having two PRs here to first move to Draft then to Review? 😉

ligi · 2021-11-03T12:17:55Z

I actually caught it the first time: #4020 (comment)
Unfortunately I did not catch the second attempt ..-)

Do we really need to add empty sections for backward compatibility and security considerations when they do not apply?

axic · 2021-11-03T12:47:36Z

Do we really need to add empty sections for backward compatibility and security considerations when they do not apply?

It seems so.

MicahZoltu · 2021-11-04T07:06:33Z

I believe we discussed this and agreed that if the author wants to go from stagnant to Review that is fine.

The Backward Compatibility and Security Consideration sections are required. At the least they should have a one-line blurb that says "There are no known backward compatibility issues introduced by this EIP" or "There are no known security issues presented by the introduction of this EIP."

lightclient · 2022-01-11T16:40:53Z

Bumping as this is heavily used in WalletConnect and it would be great to move to final

can also be for desktop - maybe we should remove the whole part - because now there is also "copy" - no real value added anyway IMHO

ligi · 2022-02-04T21:47:19Z

I did most things @axic pointed out changed from EIP-1 in style - just left

Add Backwards Compatibility
Add Security Considerations

Also @pedrouid can you add the 2.0 changes you mentioned?

Also I think the rationale needs some rework - sounds weird and not that helpful.

ligi · 2022-02-04T21:48:25Z

EIPS/eip-1328.md

 ## Abstract

-This standard defines how the data to connect some application and a wallet can be encoded with a URI. This URI can then be shown either as a QR code or for mobile to mobile as a link.
+This standard defines how the data to connect some application and a wallet can be encoded with a URI. This URI can then be shown either as a QR code or as a link.


@pedrouid do we perhaps want to remove "This URI can then be shown either as a QR code or as a link." - think it adds no value and restricts us without a need. e.g. how does the "copy" fall in place there.

I feel like this is already addressed when "link" is mentioned since it's a common pattern to include copy to clipboard for website links as well

pedrouid · 2022-02-05T14:04:53Z

Thanks @ligi for resurrecting this EIP. I will look into this by Tuesday to include the missing sections and parameters for v2.0

pedrouid · 2022-02-09T12:07:54Z

Sorry for the delay.... I have created a PR to Ligi's PR to include the v2.0 parameters: ligi#2

Missing sections

ligi · 2022-02-09T15:12:27Z

Incorporated most things requested - but have one open discussion point with @pedrouid - marking as draft until this is resolved.

MicahZoltu · 2022-02-11T12:07:09Z

EIPS/eip-1328.md

 eip: 1328
 title: WalletConnect Standard URI Format
-author: ligi <ligi@ligi.de>, Pedro Gomes <pedrouid@protonmail.com>
+author: ligi <@ligi>, Pedro Gomes <@pedrouid>
 type: Standards Track
 category: ERC
-status: Stagnant
+status: Draft
 created: 2018-08-15
 discussions-to: https://ethereum-magicians.org/t/wallet-connect-eip/850
+description: Standard for WalletConnect URIs


Suggested change

eip: 1328

title: WalletConnect Standard URI Format

author: ligi <ligi@ligi.de>, Pedro Gomes <pedrouid@protonmail.com>

author: ligi <@ligi>, Pedro Gomes <@pedrouid>

type: Standards Track

category: ERC

status: Stagnant

status: Draft

created: 2018-08-15

discussions-to: https://ethereum-magicians.org/t/wallet-connect-eip/850

description: Standard for WalletConnect URIs

eip: 1328

title: WalletConnect Standard URI Format

description: Standard for WalletConnect URIs

author: ligi (@ligi), Pedro Gomes (@pedrouid)

discussions-to: https://ethereum-magicians.org/t/wallet-connect-eip/850

status: Draft

type: Standards Track

category: ERC

created: 2018-08-15

Fixes field ordering to align with https://raw.githubusercontent.com/ethereum/EIPs/master/eip-template.md.

Also fixes () for GitHub username, <> is for email.

pedrouid · 2022-04-07T16:08:27Z

Thanks @ligi for making sure this EIP is resurrected and merged

I've provided with the final changes regarding parameters for WalletConnect v2.0 protocol

ligi#4

Update eip-1328.md

ligi · 2022-04-11T09:07:51Z

@pedrouid thanks for the final changes for WC2.0!
@MicahZoltu thanks for your suggestions - unfortunately I saw them after merging @pedrouid 's and then got this when trying to merge yours:

So I changed it myself and gave you credits in the commit msg - sorry for not seeing it earlier

Think this one should be mergeable now.

Changes applied.

MicahZoltu · 2022-05-06T05:25:58Z

@SamWilsn @lightclient @axic This has been sitting waiting for a review for quite a while. Any chance one of you have some time to take a look?

SamWilsn · 2022-05-13T18:05:13Z

EIPS/eip-1328.md

@@ -1,21 +1,18 @@
 ---
 eip: 1328
 title: WalletConnect Standard URI Format


Suggested change

title: WalletConnect Standard URI Format

title: WalletConnect URI Format

SamWilsn · 2022-05-13T18:05:50Z

EIPS/eip-1328.md

@@ -1,21 +1,18 @@
 ---
 eip: 1328
 title: WalletConnect Standard URI Format
-author: ligi <ligi@ligi.de>, Pedro Gomes <pedrouid@protonmail.com>
+description: Standard for WalletConnect URIs


Suggested change

description: Standard for WalletConnect URIs

description: Define URIs for initiating connections between applications and wallets

SamWilsn · 2022-05-13T18:08:23Z

EIPS/eip-1328.md

 ## Rationale

 The need for this ERC stems from the discussion to move away from JSON format used in the alpha version of the WalletConnect protocol which makes for very inneficient parsing of the intent of the QR code, making it easier to create better QR code parsers APIs for Wallets to implement. Also by using a URI instead of a JSON inside the QR-Code the Android Intent system can be leveraged.


This "Rationale" section reads more like it should belong in the "Motivation" section.

"Motivation" explains why the EIP as a whole needs to exist, while "Rationale" should explain individual choices made in the EIP itself.

SamWilsn · 2022-07-18T05:21:12Z

Opened ligi#5 with my suggestions.

Apply my suggestions to the PR

ligi · 2022-07-18T05:22:10Z

merged

SamWilsn

I think we're good to go!

SamWilsn · 2022-07-18T05:28:52Z

@ligi is this good to go to review?

ligi · 2022-07-18T05:33:20Z

yes!

@MicahZoltu

* Resurrect EIP-1328 * Update to current EIP-1 * Remove the specific use case can also be for desktop - maybe we should remove the whole part - because now there is also "copy" - no real value added anyway IMHO * Update eip-1328.md * Update eip-1328.md * Update EIPS/eip-1328.md * Update EIPS/eip-1328.md * Update EIPS/eip-1328.md * Update EIPS/eip-1328.md * Update eip-1328.md * Apply suggestions by @MicahZoltu * Apply my suggestions to the PR Co-authored-by: Pedro Gomes <pedrogomes94@gmail.com> Co-authored-by: Sam Wilson <sam.wilson@mesh.xyz>

@MicahZoltu

* Resurrect EIP-1328 * Update to current EIP-1 * Remove the specific use case can also be for desktop - maybe we should remove the whole part - because now there is also "copy" - no real value added anyway IMHO * Update eip-1328.md * Update eip-1328.md * Update EIPS/eip-1328.md * Update EIPS/eip-1328.md * Update EIPS/eip-1328.md * Update EIPS/eip-1328.md * Update eip-1328.md * Apply suggestions by @MicahZoltu * Apply my suggestions to the PR Co-authored-by: Pedro Gomes <pedrogomes94@gmail.com> Co-authored-by: Sam Wilson <sam.wilson@mesh.xyz>

axic previously requested changes Nov 3, 2021

View reviewed changes

axic added the waiting: response from author label Nov 11, 2021

github-actions bot added the stale label Jan 10, 2022

github-actions bot removed the stale label Jan 11, 2022

alita-moore added automerge and removed automerge labels Jan 18, 2022

poojaranjan mentioned this pull request Feb 3, 2022

EIPIP Meeting 49 Agenda ethcatherders/EIPIP#107

Closed

ligi added 3 commits February 4, 2022 22:36

Resurrect EIP-1328

c918220

Update to current EIP-1

498904f

Remove the specific use case

a5e188c

can also be for desktop - maybe we should remove the whole part - because now there is also "copy" - no real value added anyway IMHO

ligi force-pushed the resurrect_1328 branch from 43c0fe3 to a5e188c Compare February 4, 2022 21:43

eth-bot added the statusChange label Feb 4, 2022

ligi commented Feb 4, 2022

View reviewed changes

poojaranjan mentioned this pull request Feb 9, 2022

EIP editor apprenticeship meeting 12 ethcatherders/EIPIP#106

Closed

Update eip-1328.md

458faf7

pedrouid and others added 3 commits February 9, 2022 13:12

Update eip-1328.md

72e45a7

Update EIPS/eip-1328.md

b71e351

Update EIPS/eip-1328.md

6433ca0

ligi added 3 commits February 9, 2022 14:47

Update EIPS/eip-1328.md

0bb6cf8

Update EIPS/eip-1328.md

109f8c4

Merge pull request #3 from pedrouid/patch-10

9a41f1a

Missing sections

ligi marked this pull request as draft February 9, 2022 15:11

MicahZoltu reviewed Feb 11, 2022

View reviewed changes

Update eip-1328.md

844fd5f

ligi added 2 commits April 11, 2022 10:55

Merge pull request #4 from pedrouid/patch-11

9306edb

Update eip-1328.md

Apply suggestions by @MicahZoltu

0e728c3

ligi marked this pull request as ready for review April 11, 2022 09:07

SamWilsn requested changes May 13, 2022

View reviewed changes

github-actions bot added the stale label Jul 12, 2022

Apply my suggestions to the PR

7980502

Merge pull request #5 from SamWilsn/resurrect_resurrect_1328

9559a47

Apply my suggestions to the PR

github-actions bot removed the stale label Jul 18, 2022

SamWilsn approved these changes Jul 18, 2022

View reviewed changes

eth-bot added updateEIP and removed statusChange labels Jul 18, 2022

kodiakhq bot merged commit c0e1c15 into ethereum:master Jul 18, 2022

Brandylee24 approved these changes May 29, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resurrect EIP-1328 #4416

Resurrect EIP-1328 #4416

ligi commented Nov 3, 2021

eth-bot commented Nov 3, 2021 •

edited

Loading

pedrouid commented Nov 3, 2021

axic commented Nov 3, 2021

ligi commented Nov 3, 2021

axic left a comment •

edited

Loading

axic commented Nov 3, 2021 •

edited

Loading

ligi commented Nov 3, 2021

axic commented Nov 3, 2021

MicahZoltu commented Nov 4, 2021

lightclient commented Jan 11, 2022

ligi commented Feb 4, 2022 •

edited

Loading

ligi Feb 4, 2022

pedrouid Feb 9, 2022

pedrouid commented Feb 5, 2022

pedrouid commented Feb 9, 2022

ligi commented Feb 9, 2022

MicahZoltu Feb 11, 2022

pedrouid commented Apr 7, 2022

ligi commented Apr 11, 2022

MicahZoltu commented May 6, 2022

SamWilsn May 13, 2022

SamWilsn May 13, 2022

SamWilsn May 13, 2022

SamWilsn commented Jul 18, 2022

ligi commented Jul 18, 2022

SamWilsn left a comment

SamWilsn commented Jul 18, 2022

ligi commented Jul 18, 2022

	title: WalletConnect Standard URI Format
	title: WalletConnect URI Format

	description: Standard for WalletConnect URIs
	description: Define URIs for initiating connections between applications and wallets

		## Rationale

		The need for this ERC stems from the discussion to move away from JSON format used in the alpha version of the WalletConnect protocol which makes for very inneficient parsing of the intent of the QR code, making it easier to create better QR code parsers APIs for Wallets to implement. Also by using a URI instead of a JSON inside the QR-Code the Android Intent system can be leveraged.

Resurrect EIP-1328 #4416

Resurrect EIP-1328 #4416

Conversation

ligi commented Nov 3, 2021

eth-bot commented Nov 3, 2021 • edited Loading

All tests passed; auto-merging...

(pass) eip-1328.md

pedrouid commented Nov 3, 2021

axic commented Nov 3, 2021

ligi commented Nov 3, 2021

axic left a comment • edited Loading

Choose a reason for hiding this comment

axic commented Nov 3, 2021 • edited Loading

ligi commented Nov 3, 2021

axic commented Nov 3, 2021

MicahZoltu commented Nov 4, 2021

lightclient commented Jan 11, 2022

ligi commented Feb 4, 2022 • edited Loading

ligi Feb 4, 2022

Choose a reason for hiding this comment

pedrouid Feb 9, 2022

Choose a reason for hiding this comment

pedrouid commented Feb 5, 2022

pedrouid commented Feb 9, 2022

ligi commented Feb 9, 2022

MicahZoltu Feb 11, 2022

Choose a reason for hiding this comment

pedrouid commented Apr 7, 2022

ligi commented Apr 11, 2022

MicahZoltu commented May 6, 2022

SamWilsn May 13, 2022

Choose a reason for hiding this comment

SamWilsn May 13, 2022

Choose a reason for hiding this comment

SamWilsn May 13, 2022

Choose a reason for hiding this comment

SamWilsn commented Jul 18, 2022

ligi commented Jul 18, 2022

SamWilsn left a comment

Choose a reason for hiding this comment

SamWilsn commented Jul 18, 2022

ligi commented Jul 18, 2022

eth-bot commented Nov 3, 2021 •

edited

Loading

axic left a comment •

edited

Loading

axic commented Nov 3, 2021 •

edited

Loading

ligi commented Feb 4, 2022 •

edited

Loading