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.

Sign up for GitHub

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

Jump to bottom

ZIP-321: Payment Request URIs #395

Merged
merged 27 commits into from
Sep 24, 2020
Merged

ZIP-321: Payment Request URIs #395

merged 27 commits into from
Sep 24, 2020

Conversation

nuttycom
Copy link
Contributor

@nuttycom nuttycom commented Sep 4, 2020

Closes #347

@nuttycom nuttycom self-assigned this Sep 4, 2020
@nuttycom nuttycom requested a review from daira September 4, 2020 16:59
@nuttycom
Copy link
Contributor Author

nuttycom commented Sep 4, 2020

@ChileBob this may be of interest to you.

@nuttycom nuttycom added this to the Core Sprint 2020-35 milestone Sep 4, 2020
@nuttycom
Copy link
Contributor Author

nuttycom commented Sep 4, 2020

@adityapk00 I'd like to draw your attention to this (standardization of zcash payment URIs) as well.

@ChileBob
Copy link

ChileBob commented Sep 5, 2020

A request, could a few fully formatted URIs be provided as examples?

pacu
pacu reviewed Sep 8, 2020
View reviewed changes
Copy link
Contributor

@pacu pacu left a comment

Choose a reason for hiding this comment

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

+1 to @ChileBob's request on some samples.

daira
daira requested changes Sep 15, 2020
View reviewed changes
Copy link
Collaborator

@daira daira left a comment

Choose a reason for hiding this comment

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

Suggested some more minor changes to the syntax, and addition of an Examples section.

zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Show resolved Hide resolved
nuttycom and others added 2 commits September 15, 2020 11:48
@nuttycom @daira
Apply suggestions from code review
4f8c148 
Replace `-` with `.` for parameter index delimiter and clarify specification of amount and address values.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>
@nuttycom
Fix missing memoparam
f9be8c6
daira
daira reviewed Sep 17, 2020
View reviewed changes
Copy link
Collaborator

@daira daira left a comment

Choose a reason for hiding this comment

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

minor comments

zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
zip-0321.rst Outdated Show resolved Hide resolved
nuttycom
nuttycom commented Sep 17, 2020
View reviewed changes
zip-0321.rst Show resolved Hide resolved
@nuttycom @daira
Apply suggestions from code review
4331563 
Co-authored-by: Daira Hopwood <daira@jacaranda.org>
daira
daira reviewed Sep 17, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 17, 2020
View reviewed changes
zip-0321.rst Show resolved Hide resolved
nuttycom and others added 2 commits September 17, 2020 17:46
@nuttycom
Require leading zero for decimal amounts.
78b4fad
@nuttycom @daira
Update zip-0321.rst
2a08b20 
Co-authored-by: Daira Hopwood <daira@jacaranda.org>
nuttycom
nuttycom commented Sep 19, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 21, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
pacu
pacu reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
@nuttycom @daira
Mark ZIP as Proposed
27ac9af 
Co-authored-by: Daira Hopwood <daira@jacaranda.org>
nuttycom
nuttycom commented Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira and others added 3 commits September 24, 2020 16:44
@daira @nuttycom
Clarifications
57cd724 
Co-authored-by: Kris Nuttycombe <kris.nuttycombe@gmail.com>
@daira
Clarifications
ae5ebe4 
Cosmetics.
@nuttycom @daira
Add multiple-payments as a feature to be considered under the grace p…
05daae9 
…eriod clause.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
daira
daira reviewed Sep 24, 2020
View reviewed changes
zip-0321.rst Outdated Show resolved Hide resolved
@daira
Line wrapping
70e5729
@daira
ZIP 321: rST fixes; generate HTML and index.
a78a129 
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira daira merged commit a78a129 into zcash:master Sep 24, 2020
@github-pages github-pages bot temporarily deployed to github-pages September 24, 2020 17:56 Inactive
@nuttycom nuttycom deleted the zip-0321 branch September 24, 2020 18:33
nathan-at-least
nathan-at-least reviewed Oct 5, 2020
View reviewed changes
Copy link
Contributor

@nathan-at-least nathan-at-least left a comment

Choose a reason for hiding this comment

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

Clarification about zcashaddress grammar.

zip-0321.rst
.. code-block:: EBNF

zcashurn = "zcash:" ( zcashaddress [ "?" zcashparams ] / "?" zcashparams )
zcashaddress = 1*base58
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this line mean a zcash address begins with a 1 prefix? If so, that's incorrect.

Copy link
Contributor Author

@nuttycom nuttycom Oct 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

No, this means that the zcashaddress production is one or more base58 characters. See https://github.com/zcash/zips/pull/395/files#diff-a386636de36a003f2f6c54ee53696106R127. Also you'll note that this production has changed in a later amendment to the ZIP, and some more qualifications on addresses were added in 8c4e0b5#diff-a386636de36a003f2f6c54ee53696106R152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathan-at-least nathan-at-least nathan-at-least left review comments

@daira daira daira approved these changes

@pacu pacu pacu approved these changes

@braddmiller braddmiller Awaiting requested review from braddmiller

@gmale gmale Awaiting requested review from gmale

Assignees

@nuttycom nuttycom

Labels
None yet
Projects
None yet
Milestone
Core Sprint 2020-37
Development

Successfully merging this pull request may close these issues.

[ZIP 321] Specify payment request URIs
6 participants
@nuttycom @ChileBob @pacu @daira @nathan-at-least @r3ld3v