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

Cardano URI Scheme #30

Merged
merged 4 commits into from
Jan 14, 2021
Merged

Conversation

SebastienGllmt
Copy link
Contributor

This is a specification of what we've already implemented in both the Yoroi Extension and in Yoroi Mobile

@rphair
Copy link
Collaborator

rphair commented Oct 20, 2020

Updated stake pool specific URI scheme CIP in response: 0ca5438

@gufmar
Copy link
Contributor

gufmar commented Oct 20, 2020

We should be wary of people who disguise “donate” links as actually opening up a phishing website that LOOKS like a wallet.

Could it be a security option to let users set a recognition word/phrase inside the wallet?
This is something phishing wallets can't know.

  1. one time setup: Please set something you know, such as your preferred color, your birth location, your first car...
  2. crucial wallet areas such as landing page for web+Cardano URIs clearly show this locally stored string

I know not perfect but an option

@SebastienGllmt
Copy link
Contributor Author

@gufmar the wallet checksum already plays this role for Yoroi

@@ -0,0 +1,77 @@
---
CIP: TODO
Copy link
Member

Choose a reason for hiding this comment

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

☝️ reminder

Copy link
Contributor

@crptmppt crptmppt Dec 9, 2020

Choose a reason for hiding this comment

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

proposing 12 13

Copy link
Contributor

Choose a reason for hiding this comment

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

PaymentUrls/PaymentUrls.md Outdated Show resolved Hide resolved

An alternative solution to the original problem described above is to use standard URL links in combination with a routing backend system. The routing system is used to redirect to the app's URI. The advantage of this scheme is that it allows to provide a fallback mechanism to handle the case when no application implementing the protocol is installed (for instance, by redirecting to the App Store or Google Play). This is the approach behind iOS Universal Links and Android App Links. In general, it provides a better user experience but requires a centralized system which makes it unsuitable for as a multi-app standard.

## Read More
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a link to existing Yoroi implementations as a reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our implementation is probably too deeply coupled with the rest of our code to serve as a good reference unfortunately

@SebastienGllmt SebastienGllmt merged commit 996539e into cardano-foundation:master Jan 14, 2021
@SebastienGllmt SebastienGllmt deleted the payment-urls branch January 14, 2021 00:53
@rphair
Copy link
Collaborator

rphair commented Jan 14, 2021

@SebastienGllmt it is hard to believe this CIP was put through without including the material in this other CIP as agreed: #25 (comment)

@crptmppt
Copy link
Contributor

@rphair the CIP is in 'Draft', there is much room to propose and add changes. As noted, Editors have limited bandwidth. Thanks for stepping up where needed, and I suggest you join the (new) CIP-Biweekly calls to be further involved - we're holding them publicly at https://www.crowdcast.io/cips-biweekly
(Will strive to publish upcoming agendas a week in advance)

@rphair
Copy link
Collaborator

rphair commented Jan 14, 2021

thank you @crptmppt for the confirmation & clarification about what it means to be a "draft" - that's very reassuring. I appreciate the invitation, have subscribed to that channel & will be happy to join your next meeting 😍

@SebastienGllmt
Copy link
Contributor Author

@rphair I'm kind of confused. There are two ways of merging our work:

  1. Now that this PR is merged, your submit a follow-up PR that modifies this document that extends it with your proposal
  2. Now that this PR is merged, your create a separate CIP that refers to this one and defines how you plan to extend it

My strong preference is on (1).

At any rate, both options required getting this PR to be merged first.

@rphair
Copy link
Collaborator

rphair commented Jan 16, 2021

@SebastienGllmt as we left it, according the agenda resulting from our last conversation, my single-pool link spec was to be included in your own spec (step 3) before the payment spec was merged in as an approved CIP (step 4):

  1. Simplify motivation
  2. Remove multi-pool notation
  3. Merge spec into Payment URI spec
  4. Merge payment spec as approved draft CIP
  5. Create new PR that adds the multi-pool notation and leave that as an open PR to the existing payment URI cip

I don't object to the change in plan, if that's what's it takes to keep the pool links included in this CIP going forward, but when observing step 4 happening before step 3 yesterday, after not hearing anything for 2 months, I think I was the one who was confused.

Also according to the original plan I would have only created one new PR: for adding the multi-pool spec into this CIP, since the single-pool spec would have been added before your own CIP pull request was merged. Now I will be submitting two new PRs: one immediately for single-pool, and another down the road for multi-pool as we originally planned.

Thanks for putting the key component through, and thanks again for allowing the URI scheme for this CIP to carry the stake pool links. I would only insist that my upcoming pull request include a link to the other PR (#25), which has now become a dead end, since it establishes a date on the original request as well as some helpful discussion.

@rphair
Copy link
Collaborator

rphair commented Jan 17, 2021

The proposed changes have now been refactored into this new PR: #61

crptmppt added a commit that referenced this pull request Sep 29, 2021
crptmppt added a commit that referenced this pull request Sep 29, 2021
* adding notes for CIP Editors meeting #30

* format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants