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

CIP-0013 | Generalise for further protocols #559

Merged
merged 15 commits into from
Aug 1, 2023

Conversation

rphair
Copy link
Collaborator

@rphair rphair commented Jul 24, 2023

This change was considered a couple years ago (but held off due to not much demand at the time) in this forum thread (now added into Discussions: history) and now deemed necessary for #546.

It's a substantial rewrite but preserves the formatting & language for the Payment links and the additions for Stake Pool links which I added afterwards.

Notably it indicates that further extensions to the protocol (//keyword where the keyword is called an "authority" by URI convention) should be defined in further CIPs. Although that may not be strictly adhered to by developers, I think that is a reasonable thing to require in this CIP and for other CIPs which depend in any way on a URI scheme.

(formatted new version in branch)

@rphair rphair added the Update Adds content or significantly reworks an existing proposal label Jul 24, 2023
@rphair
Copy link
Collaborator Author

rphair commented Jul 24, 2023

@Crypto2099 this should be ready for your own review as well. Next stop for me is a detailed review of #546 to ensure that these two CIPs will work well together. 😎

Copy link
Collaborator

@Crypto2099 Crypto2099 left a comment

Choose a reason for hiding this comment

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

Overall I think this is a step in the right direction for CIP-13. It provides additional clarity and understanding for how to expand the protocol in the future as well as better defining the existing feature set that is part of the core CIP-13 itself.

Rationale:
- Use `cardano:` over `ada:` as other projects that implement this standard tend to take the project name over the currency name (this makes sense if we consider this protocol as a generic way for interacting with the blockchain through wallets - as opposed to a simple payment system)
- Many wallets support multiple currencies. Following the same standard will ensure higher adoption of our protocol.

Examples:
```
<a href="web+cardano:Ae2tdPwUPEZ76BjmWDTS7poTekAvNqBjgfthF92pSLSDVpRVnLP7meaFhVd">Donate</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

<a href="web+cardano:Ae2tdPwUPEZ76BjmWDTS7poTekAvNqBjgfthF92pSLSDVpRVnLP7meaFhVd">Donate</a>

Is there a reason why this URI schema has persisted to this point lacking the two forward slashes? (//)

Since we are looking to standardize this and prepare for future expansion, would it be safe to require that web+cardano:// always be present?

Further, should we consider a "pay to" authority rather than defaulting to hoping that it is a valid address?

Copy link
Collaborator Author

@rphair rphair Jul 25, 2023

Choose a reason for hiding this comment

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

The URI spec only has a // when the authority field is present; this from the Wiki page (which is quoting the RFC) & itself an ABNF grammar... either the // and the authority or present together or neither of them are present, so // is definitely not a requirement for all URIs:

URI = scheme ":" ["//" authority] path ["?" query] ["#" fragment]

To disambiguate your middle question: for our URI spec, web+cardano: always has to be present but // does not.

We are locked into supporting the payment links without an authority because for a long time that was the only implementation of these URIs that was getting professional consideration (someone suggested stake pool links in 2018 but nobody ever moved on it; timeline & links are in the PRs at the Discussion links).

As far as I know Yoroi in particular has supported payment links with only a path-query string that whole time: about 5 years now. If I had to tell them they needed to commit to honouring an authority prefix (let's call it //payment) I would therefore expect about 5 years to get a response.

For some of the reasons in #559 (comment), I'd be open to a new CIP with a //payment schema supporting a more developed syntax, but in the meantime those 2 interpretations (payment vs. authority) of everything after the : can never collide: because one never has // and the others always do.

The "hope" that a payment address is valid would have been considered by editors when this CIP was originally debated from original authors @SebastienGllmt and @v-almonacid for that part of the scheme. I believe it's not in the CIP because it's not the URI's "job"... how a wallet or perhaps a dApp would handle a URI with an invalid payment address would be up to the designers.

One application for a further //payment protocol (with a separate CIP) might be to use higher-level addresses e.g. AdaHandles. In fact it could create a beautiful range of possibility (e.g. multiple and/or proportional payments): all of which are beyond the scope of this CIP which is basically becoming a switch to get to newly defined protocols for everything not considered 3 to 5 years ago.

Choose a reason for hiding this comment

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

We are locked into supporting the payment links without an authority because for a long time that was the only implementation of these URIs that was getting professional consideration (someone suggested stake pool links in 2018 but nobody ever moved on it; timeline & links are in the PRs at the Discussion links).

As far as I know Yoroi in particular has supported payment links with a bare path-query string that whole time: about 5 years now. If I had to tell them they needed to commit to honouring an authority prefix (let's call it //payment) I would therefore expect about 5 years to get a response.

Personally I think it's not too late to change the URI scheme to add an authority for payments, if that's what you think is best (I do). And while it is difficult to say whether there is a lot of people using payments URIs right now with Yoroi, I don't think it's a big deal to add this breaking change. But it'd be better of course if you try to get feedback from wallet providers first.

how a wallet or perhaps a dApp would handle a URI with an invalid payment address would be up to the designers

Exactly


### Rationale for stake pool links

#### How do URI delegation portfolio links supplement use of JSON files for the same purpose?

URIs facilitate the "social element" of delegated staking and pool promotion through a socially familiar, easily accessible, and less centralised convention for sharing stake pool references and potential delegation portfolios without having to construct or host a JSON file.

The processing of a JSON file delivered by a web server will depend highly on a user's platform and might not even be seen by the wallet application at all. With a properly associated `web+cardano:` protocol, developers and users have another option available in case JSON files are not delivered properly to the wallet application.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we better and more cleanly handle the varied methods of delegation via a link that would "point" to a well-defined JSON/CBOR schema metadata file that can be fetched? This would greatly simplify the URIs themselves and the associated HTML where they are embedded in addition to improving troubleshooting and diagnostics.

Copy link
Collaborator Author

@rphair rphair Jul 25, 2023

Choose a reason for hiding this comment

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

In the months it took the stake pool links to be put through as an addition to this CIP, the counter of a JSON file query was the main objection. Rather than look at the stake pool approach on its own merit, most CIP editors at the time and IOG developers who jumped in said that JSON would be "better" and for a while the discussion just ended there: let alone being open to a cooperative approach. //stake was eventually merged when I managed to show that a rigorously defined URI syntax had fewer practical limitations than the JSON proponents had all assumed.

That is at least an indicator that a hybrid URI-to-JSON stake portfolio protocol would be unpopular. In fact, over 2 years after those discussions, CIP-0017 remains unimplemented as far as I know

The motivation for having the one-line, simple syntax available is for the sake of web designers, SPOs, and marketers & is already in the currently defined CIP (the reasons they would find JSON impractical are detailed in the associated PRs). Other resource locators like encapsulated JSON URLs might be useful generally... though still no commercial interest I know of in supporting more flexible delegation. If & when the time comes, we can suggest to those implementors that they create a new authority (e.g. //stakelink) with a separate CIP for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

... some dev will come along someday wanting to put urlencoded CBOR in a URL: so rather than create a precedent to support this in a constantly modified (or deprecated) CIP-0013 we can simply tell them, "Create a new authority keyword for it & define your support for it there" ... whether it relates to payments, delegation and/or anything else 🤓

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose my primary objection is to the use of potentially unicode pool tickers as the "keys" in the URL arguments, I feel like this creates a cumbersome barrier to implementation for the wallets. However, I also understand this was introduced most likely in response to URL max-length limitations that could quickly be exceeded by 3+ pool hex IDs.

The parser that I've written to handle logical parsing of CIP-13 URIs does manage to handle it but has not been rigorously tested against things like URL-encoded UTF-8 tickers so will need some additional work there for matching.

https://github.com/Crypto2099/POO/blob/main/packages/lib/cardano-poo-ts/src/CIP13Parser.ts

Copy link
Collaborator Author

@rphair rphair Jul 25, 2023

Choose a reason for hiding this comment

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

The problem with not using the stake pools as keys is that the syntax expands greatly, and then fails for multi pool links, when using them as values.

After months of debate, those against using the stake pools as keys couldn't come up with an alternative syntax which was concise and unambiguous:

I don't know if any alternative syntax you might be thinking of would have this same problem, but the main problem with the last suggestion (having a poolticker keyword with the hex pool ID as the value) was lack of support for multi-pool delegation links, since URI query variables can't have more than one value:

So as difficult as it might be for parsers and wallets to implement this (as I remember pointed out by @SebastienGllmt in the earlier threads, though I can't find this comment now), I don't see a way around it unless someone wants to spell out an alternative syntax & attempt another disqualification of the CIP-13 stake pool link protocol as it's now been defined for the last 2 years... keeping in mind that the main problem would be the support of multiple pool values rather than the length of the URI string.

@rphair
Copy link
Collaborator Author

rphair commented Jul 25, 2023

A suggestion that will help to focus and expedite the review of this particular PR: the changes here are only to refine the language, description, and grammar to support and encourage further protocols in addition to the payment (authority == NULL) and weighted stake pool set (authority == stake) already defined in the existing CIP version.

Neither the basics of the URI scheme nor those 2 protocols themselves are changing even one bit in this PR. These individual protocols are not subject to re-review, nor do they have to be validated again from first principles, because some other parts of the CIP have been reworded for inclusiveness.

Therefore I would suggest any modifications to those particular protocols be submitted as separate PRs so they can be considered separately. This PR is only about the expansion into new protocols like //claim as submitted in #546.

@v-almonacid
Copy link

Good work @rphair 👍

Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

I like this update, it looks to me like it opens up the standard for further expansion and adoption without compromising backwards compatibility.

@rphair rphair merged commit 4720165 into cardano-foundation:master Aug 1, 2023
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Nov 17, 2023
* first draft, will proofread for GitHub flavoured markdown

* remove accidentally duplicated line

* URI scheme is no longer "basic" but all encompassing

* ugly typo at top, need to correct immediately

* clarify that other URI standards should have their own CIPs

* doc reorganisation means "next" link not appropriate anymore

* better clarification of terms used in grammar

* ABNF section improperly broken between protocols + badly formatted

* makes more sense with *all* stake pool details in one place

* former "grammar" only section now includes *interpretation*

* final corrections for initial draft

* consolidated + simplified language for payment links vs BIP-21

* moved payment Rationale out of Spec, simplified section anchors

* typo

* current discussion has become significant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants