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-0099? | Proof of Onboarding #546

Merged
merged 54 commits into from
Dec 12, 2023

Conversation

Crypto2099
Copy link
Collaborator

The POO (Proof of Onboarding) Protocol defines a new standard for Cardano CIP-13 URI formats as well as a new standard for a communication protocol and API between wallets and project servers to provide a reliable and standardized method to "airdrop" tokens onto users.

The primary use case scenario for this protocol is for projects attending live events where the users in question may not even have a crypto wallet yet, let alone any crypto coins in their wallet. How do we still onboard (and capture reliable metrics) these users to the ecosystem?

In partnership with $HOSKY Token, VEGAS Stake Pool, and VESPR wallet we have developed this specification to support the following user journey:

  1. Project generates a printed flyer or card that contains two (or more) QR codes.
  2. The first QR code takes the user to the app store on their device to download and install the wallet software (in this case VESPR wallet).
  3. The second QR code can be scanned from within the application (or via deep linking) and makes a call to the project's server w/ the wallet address of the user and a claim code that can be campaign-specific or one-time user.
  4. The project server is responsible for validating address and code for eligibility to receive whatever tokens the code may qualify them for.
  5. The project server issues one of several standardized responses to inform the wallet of the status of the request.
  6. User receives the tokens via a transaction from the project server.

@Crypto2099
Copy link
Collaborator Author

Crypto2099 commented Jul 6, 2023

@rphair rphair changed the title CIP-????: POO Protocol CIP-???? | Proof of Onboarding Jul 6, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

1 - A note about the title:

  1. It's a memorable acronym but people landing on this PR should more importantly know immediately what it's about, with as little distraction as possible.
  2. The term "protocol" is optional considering that other CIPs for protocols don't have it in the title... and not necessary here because "onboarding" implies a process (although the extra word does produce another catchy acronym).

In any case, if this new title "sticks" (so to speak) we can update the document to reflect it at some point.

2 - More urgently we should first consider decoupling this from CIP-0013 as much as possible, based on prior agreement. I would be open to retitling CIP-0013 as "URIs for payment and stake pool links" to restrict it to those two URI protocols alone. The history:

The decision to produce separate CIPs for further extensions to the Cardano URI scheme has mainly taken place on the Forum: https://forum.cardano.org/t/cip-generalized-cardano-urls/57464

... and a little bit on GitHub in response to an aborted request to include some URI protocols for an unpopular app-specific URI scheme: #130 #234

... but we've never formalised the decision to document additional URI protocols as new CIPs because there was nothing to merge yet. We knew that day was coming and I think your practical requirement will force us to confirm this.

If the consensus is to do that, some time after 18 July (after I have another deadline) I can work on a CIP-0013 update or deprecation depending upon how you, reviewers & editors think the URI Scheme should be maintained going forward... given that this is the first useful extension to it that's been proposed since we merged my own "stake pool link" updates. Then you'd be free to include the entire "onboarding" spec in a single new CIP document.

I do feel strongly that bulking up the ancient CIP-0013 with the somewhat heavyweight "onboarding" protocol is going to cause some confusion as well as discourage implementors from supporting the 2 URI protocols already documented there.

@rphair rphair self-assigned this Jul 6, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

also somehow we have to remove the illegal comment entries from the JSON; otherwise they'll be highlighted as errors on all the GitHub syntax checkers:

CIP-XXXX/README.md Outdated Show resolved Hide resolved
@Crypto2099
Copy link
Collaborator Author

1 - A note about the title:

  1. It's a memorable acronym but people landing on this PR should more importantly know immediately what it's about, with as little distraction as possible.
  2. The term "protocol" is optional considering that other CIPs for protocols don't have it in the title... and not necessary here because "onboarding" implies a process (although the extra word does produce another catchy acronym).

In any case, if this new title "sticks" (so to speak) we can update the document to reflect it at some point.

2 - More urgently we should first consider decoupling this from CIP-0013 as much as possible, based on prior agreement. I would be open to retitling CIP-0013 as "URIs for payment and stake pool links" to restrict it to those two URI protocols alone. The history:

The decision to produce separate CIPs for further extensions to the Cardano URI scheme has mainly taken place on the Forum: https://forum.cardano.org/t/cip-generalized-cardano-urls/57464

... and a little bit on GitHub in response to an aborted request to include some URI protocols for an unpopular app-specific URI scheme: #130 #234

... but we've never formalised the decision to document additional URI protocols as new CIPs because there was nothing to merge yet. We knew that day was coming and I think your practical requirement will force us to confirm this.

If the consensus is to do that, some time after 18 July (after I have another deadline) I can work on a CIP-0013 update or deprecation depending upon how you, reviewers & editors think the URI Scheme should be maintained going forward... given that this is the first useful extension to it that's been proposed since we merged my own "stake pool link" updates. Then you'd be free to include the entire "onboarding" spec in a single new CIP document.

I do feel strongly that bulking up the ancient CIP-0013 with the somewhat heavyweight "onboarding" protocol is going to cause some confusion as well as discourage implementors from supporting the 2 URI protocols already documented there.

Do you feel that the best solution in this case then is to bring all of the CIP-13 amendments into the scope of this single README file and so the URI itself is also part of this CIP? I think this makes a lot of sense and allows the versioning syntax of the URI to also be updated alongside this document.

@rphair
Copy link
Collaborator

rphair commented Jul 8, 2023

@Crypto2099 the way we were leaning from the Forum discussion + the last CIP meeting when this issue came up was to:

  1. modify CIP13 so the "scheme" (ABNF) part of it just breaks out the "protocol" string, then declares that each protocol has its own separate definition for query parameters and argument processing.
  2. shift the CIP13 content for the two already defined protocols ("payments" and "stake pool links") so they are each fully and separately pre-defined in CIP13 itself... to avoid deprecating CIP13 and then having to define 2 new CIPs for these already accepted protocols.
  3. declare in CIP13 that any new protocols besides these two can be defined in a new CIP which handles that newly declared protocol string (//claim in this/your case).

Therefore this PR would not modify CIP-13... although someone (me I think) would rewrite CIP-13 accordingly ASAP in a separate PR as we were thinking of doing 2 years ago.

The only difference between the plan above and what we talked about back then... with CIP-68 happening in between with its included definitions for certain initially defined asset labels... has made me think that this organisation would also suit CIP-13... and as we've seen in #504 I would like to ensure new protocol definitions, including this PR, don't require modifying CIP-13 each time. 😬

@Crypto2099
Copy link
Collaborator Author

@rphair okay yeah, so I think I was a bit vague, that was also my understanding from the links that you mentioned which I did review. So, I will move the modifications that are currently in the CIP-13 README into the POO (CIP-????) README and they will just live there and be modified and iterated along w/ the Proof of Onboarding CIP in the future. This makes sense and actually makes it much easier to follow along with progress and changes IMO.

@rphair
Copy link
Collaborator

rphair commented Jul 8, 2023

@Crypto2099 sounds fine & I've scheduled to work on that revision of CIP13 soon after I'm done with my other deadline on the 18th. There's a CIP editors' meeting scheduled for that day so I'll plan to talk there about the changes proposed to CIP13 itself when we introduce your new proposal from the agenda. 😎

@thaddeusdiamond
Copy link
Contributor

@Crypto2099 what do you think about including a few GET endpoints to check status? For instance, I could imagine a web architecture (e.g., React or React Native) that submits the initial POST to claim the tokens, then continually polls on an interval until the GET is met with a 200 and the tx hash of the completed claim. You could just check the wallet using something like Blockfrost, but it would often be nice to know the exact tx hash linked to the claim, especially if the user is claiming multiple of the same policy ID just with different codes.

@Crypto2099
Copy link
Collaborator Author

Crypto2099 commented Jul 18, 2023

@Crypto2099 what do you think about including a few GET endpoints to check status? For instance, I could imagine a web architecture (e.g., React or React Native) that submits the initial POST to claim the tokens, then continually polls on an interval until the GET is met with a 200 and the tx hash of the completed claim. You could just check the wallet using something like Blockfrost, but it would often be nice to know the exact tx hash linked to the claim, especially if the user is claiming multiple of the same policy ID just with different codes.

This should actually already be handled by the various 200 responses already defined within the protocol as it stands. So we move from 200 (initial receipt and acknowledgement) to 201 (pending delivery and queue order assuming same wallet + code) to 202 once completed along with the txn hash. Whether or not the wallet implements a polling would be up to their individual implementation.

…arding README.md and update name and other clarifications per feedback. Also fixed in-file JSON errors that were causing problems.
@Crypto2099
Copy link
Collaborator Author

I've started compiling some companion tools and helper functions which should help others to integrate Proof of Onboarding into their project and/or wallet here: https://github.com/Crypto2099/POO. Of maybe particular interest would be the CIP-13 Parser Lib found here: https://github.com/Crypto2099/POO/tree/main/packages/lib/cardano-poo-ts (@rphair @KtorZ)...

I think we need to work on the spec for staking and clean up the spec for payment addresses r.e. CIP-13 but I realize that's part of a larger conversation specific to CIP-13 itself :)

@rphair
Copy link
Collaborator

rphair commented Jul 19, 2023

(@Crypto2099) I think we need to work on the spec for staking and clean up the spec for payment addresses r.e. CIP-13 but I realize that's part of a larger conversation specific to CIP-13 itself :)

Thanks for giving me time to finish a Catalyst posting and rest a bit... I'll publish a PR draft for that by the end of the week.

@Crypto2099
Copy link
Collaborator Author

We have updated the documentation to remove reference (for the time being) to a Version 2 of the protocol as it is not currently needed or beneficial and could, in fact, hinder adoption. Examples and documentation have been updated to reflect this.

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Readers who start at the OP above should know this proposal doesn't depend upon QR or any other media to convey the protocol URIs: this protocol doesn't necessarily integrate or depend upon any wallet choice (contrary to how it might sound from the OP).

High level description of my individual edits & comments:

1 - Identifying a version number explicitly is still awaiting agreement on how to specify this, as in what we were discussing in #520

2 - CIP titles are instantiated into parsed documents from the preamble titles as H1 everywhere, so the section headers were demoted to H2 and then subheadings further levels down from there (apologies for the mess of commits; if I knew how many headers there were going to be, I would have done this in a single initial commit pushed into your branch 🤪).

3 - Some places where the whole Cardano URI Scheme is reproduced here can be simplified, since in #559 the grammar has "hooks" in it (the authority keyword and the query following & defined by that keyword) that can be referred to simply by CIPs which define a new branch of the URI scheme.

4 - Some narrative in Path to Active needs to be put in standard form... and the parts which confirm your design choices would work great in the Rationale.

The protocol mechanism itself (HTTP response mechanism) seems remarkably well thought out and I can't conceive at this time anything that would make it not work. Really well done to your team on this concept & documentation. 🤩

CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
@rphair rphair removed their assignment Jul 24, 2023
@rphair rphair requested review from KtorZ and Ryun1 July 24, 2023 17:39
@rphair rphair changed the title CIP-???? | Proof of Onboarding CIP-0099? | Proof of Onboarding Aug 1, 2023
@rphair
Copy link
Collaborator

rphair commented Aug 1, 2023

@Crypto2099 I trust you will be happy with the cognate CIP-0099. I have scribbled on your PR enough that I will let you do the honours in changing your own number. 😅

CIP-XXXX/README.md Outdated Show resolved Hide resolved
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.

Hey @Crypto2099,

A well defined proposal! see my small comments throughout.

CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved
Crypto2099 and others added 3 commits November 8, 2023 17:55
Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>
Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>
* Language edits, additions, and modifications to bring the document in line with community and editor feedback.
* Addition of explicit versioning and modification criteria
@Crypto2099
Copy link
Collaborator Author

@Ryun1 @rphair I believe all changes (including moving to its new forever home in the CIP-0099 directory) requested or inquired about have been addressed and made in this latest version. Respectfully requesting formal review and potentially submission for Final Review during the next CIP Editors meeting if possible.

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

OK, it seems like everything we have been looking for (all the particulars I was expecting & was double checking from @Ryun1) is there now, with some more corrections besides.

I understand that this CIP won't be "versioned" as such but will require a new CIP to make functional changes. In general I would hope our CIPs would be more flexible than that, but also see in this case how it would be horrendous to keep multiple behavioural descriptions for all these interdependent pieces in a single document.

The "paper wallet" reference definitely works much better where you have it now in the Rationale. In summary I think this is ready for a Review at the next meeting in the hope of promoting it soon after.

Comment on lines 418 to 432
### Implementation Plan

In order to encourage adoption of this standard the authors commit to execute a functional minimum viable
proof of concept demonstration in partnership between Adam Dean, HOSKY Token, and VESPR wallet. This real-world
implementation should give us important insight on whether the system requires additional modifications or changes.

From there we will do our best to engage with other teams and projects in the ecosystem to encourage and support
adoption of this standard on a larger scale.

## Implementation Actions

1. [X] VESPR Mobile Wallet supports the Proof of Onboarding Protocol and is freely available for the use of any and all projects.
2. [X] HOSKY Project has released an open source server-side implementation software that may be used as a proof of concept for any interested projects.
3. [X] Multiple projects at multiple, global events have successfully deployed Proof of Onboarding.
4. [ ] Onboard additional wallet providers, server/service providers, and redemption methods.
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this to fit in with other CIPs, even those with a similar amount of coordination between separate entities, this section should be stripped of its subjective statements and formatted as a single list of achievements. I'm not just saying this without consideration of the subject matter... and likewise the community would trust you & your partners not to proceed through the Implementation plan without the "insight" that you would have from the previous stages.

Whatever is in the background paragraphs preceding the Implementation checklist also reduces to checklist items when you remove the subjective stuff. Once the list of actors & what they are doing is all in one list — ordered as chronologically as you are able, in series-parallel combination if necessary — it will be easier for the community to follow objectively.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is the suggestion to combine both "implementation plan" along with "implementation actions" into a single checkbox list?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, but what I wrote above wasn't simply saying that. I think the items that mightn't fit into a checkbox list are implicit and/or can be assumed from good faith in our community over what the implementors will be doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Roger that, I will revise and get a new PR before Tuesday's meeting :)

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Looks good as well for Active status; green lights across the Path to Active board.

@rphair rphair merged commit 7754916 into cardano-foundation:master Dec 12, 2023
@brouwerQ
Copy link

brouwerQ commented Dec 13, 2023

@Crypto2099 I have a few suggestions for extending this CIP:

1. Add an optional field infoMessage in the response

Add an optional field infoMessage in the response, so the application can provide some extra information about the claim or a thank you message that the wallet will display.

2. Add an optional field claimId

Add an optional field claimId (integers as well as strings should be allowed) in the request and response to identify this claim that can be sent back with subsequent calls by the wallet. This field will ease the identification of different claims for the same URL in case of campaign specific claims. The address field could also be used for this, but a separate field adds greater flexibility and is a solution for wallets that don't use multiple addresses.

This field should be optional at the wallet and application server and should be handled as follows:

  • A supporting wallet uses a value of 0 for this field in a new claim request. null is intentionally not used here, because in certain server infrastructures it might be difficult to differentiate the value null for a field from the absence of that field.
  • If the wallet doesn't get a value (or the value 0) back for this field, it knows that claimId isn't used for this claim and everything works as before.
  • If the wallet does get a value back for this field, it will use this value for all subsequent requests for this claim.
  • If a supporting application server (that decides to use this field) receives a value of 0 for this field, it generates a new value for this claim and sends it back in the response. It uses this id to identify subsequent requests for this claim (care should be taken to check if the other fields like code and address have the same value if necessary). If a different request with the value 0 for claimId is received with the same identifying paramaters (code, address,...), it should treat it as a separate claim (which might throw a 409 alreadyclaimed if no more claims are allowed).
  • If a supporting application server doesn't get a value back for this field, everything works as before. The application server should however differentiate such requests from requests with the same identifying paramaters that does contain a value for claimId (including 0).

3. Allow the user to pay for the minUTxO value and/or tx fees

I think this CIP goes much further than just onboarding new users with paper QR codes and should also support the well used method of users providing the minUTxO value and/or tx fee costs. The path used in the url is claim, and self providing minUTxO value or tx costs is also considered a claim, so that's why this is a must have imho.

The first response after a claim can return different status code 2xx deposit_required with 3 extra required fields and 1 extra optional field in the response:

  • depositLovelaces - required, the amount of lovelaces to deposit.
  • depositAddress - required, the address to send the lovelaces to.
  • validUntil - required, slot number by when the deposit tx should be included in an adopted block. After signing the deposit tx, wallets should decide if a claim tx can still make it on time on chain before broadcasting the claim tx.
  • confirmations - optional, number of blocks that has to be adopted on chain after the block with the deposit tx after which the claim will be queued for execution

Because of the already present parameter lovelaces, the used wallet can easily calculate and display the total user cost for this claim. Also the above introduced field infoMessage can be used to provide some information of what will happen when the ADA is sent too late or the wrong mount of ADA is sent.

Application servers can handle multiple requests as follows:

  • claimId provided by wallet

    • value 0: generate new values for claimId, depositLovelaces and depositAddress and add to response

    • other value: look value up, send error message if unknown

      • valid deposit tx on chain

        • number of confirmations not reached yet: send new status code 2xx deposit_received
        • number of confirmations reached: send status code accepted (or queued or claimed)
      • invalid deposit tx on chain: send error message (new status code needed); the wallet should allow the user to do the claim again by using 0 again for claimId

      • no deposit tx on chain

        • coupled validUntil isn't reached yet: server should always return the same values for depositLovelaces and depositAddress as in the first response for this claim; it might increase the value of validUntil to extend the claim time if it seems fit (but never decrease it)
        • coupled validUntil is already reached: throw a 410 expired (maybe a different status code should be used to differentiate an expiration of the claim itself from an expiration of the deposit time); the wallet should allow the user to do the claim again by using 0 again for claimId
  • claimId not provided by wallet, identify claim with other parameters (code, address,...), send error message if unknown

    • valid deposit tx on chain:

      • number of confirmations not reached yet: send new status code 2xx deposit_received
      • number of confirmations reached: send status code accepted (or queued or claimed); after a certain time, this request might be handled as a totally new claim if mutiple claims for the same parameters are allowed
    • invalid deposit tx on chain: send error message (new status code needed); after a certain, preferably short, time, this request MUST be handled as a totally new claim to allow the user to make a correct claim again

    • no deposit tx on chain

      • coupled validUntil isn't reached yet: server should always return the same values for depositLovelaces and depositAddress as in the first response for this claim; it might increase the value of validUntil to extend the claim time if it seems fit (but never decrease it)
      • coupled validUntil is already reached: throw a expired error message; after a certain, preferably short, time, this request MUST be handled as a totally new claim to allow the user to make a correct claim again

4. Suggestion: CIP name change

I think this CIP goes much further than just onboarding new users and a different name as Proof of Onboarding should be used.

@brouwerQ
Copy link

@Crypto2099 Thoughts on my comment above?

Ryun1 added a commit to Ryun1/CIPs that referenced this pull request Mar 6, 2024
* CIP-????: POO Protocol

Initial Commit/PR

* Update with relevant links to the PR

* Fix YAML format

* Fix YAML format

* Rollback changes to the CIP-13 README.md and add to the Proof of Onboarding README.md and update name and other clarifications per feedback. Also fixed in-file JSON errors that were causing problems.

* Minor update to document progress on acceptance criteria and implementation actions taken in July 2023.

* Update examples to properly reflect that the API server should return the tokens array with hex-encoded asset names and not ASCII asset names.

* Remove Version 2 for now as it is unnecessary at this time, let's keep it simple for the sake of early adoption.

* Fix minor typo in an example

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level

* fix header level & add explicit section title

* fix header level

* fix header level

* fix header level

* fix header level

* Update CIP-XXXX/README.md

* fixed glitch from edit during CIP meeting

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>

* Changes to CIP-99:
* Language edits, additions, and modifications to bring the document in line with community and editor feedback.
* Addition of explicit versioning and modification criteria

* Updates to the implementation plan and acceptance criteria for CIP-99

* Update CIP-99 to active status

---------

Co-authored-by: Robert Phair <rphair@cosd.com>
Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>
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.

None yet

5 participants