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

ZIP 320: choose TEX Address alternative #798

Merged
merged 4 commits into from
Mar 18, 2024
Merged

Conversation

daira
Copy link
Collaborator

@daira daira commented Mar 13, 2024

fixes #795

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
@daira daira added this to the ZIP 320 Support milestone Mar 13, 2024
@daira daira added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 13, 2024
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

One blocking comment, plus suggestions.

zip-0320.rst Show resolved Hide resolved
zip-0320.rst Show resolved Hide resolved
zip-0320.rst Outdated Show resolved Hide resolved
In order to show accurate transaction history to a user, wallets SHOULD
remember when a particular transaction output was sent to a TEX Address, so
that they can show that form rather than its P2PKH form. It is acceptable that
this information may be lost on recovery from seed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't the place for it, but a standard for how to format a (potentially-zero-valued) internal shielded change memo in the deshielding transaction could help mitigate this toe-stub potential.

zip-0320.rst Outdated Show resolved Hide resolved
zip-0320.rst Show resolved Hide resolved
zip-0320.rst Show resolved Hide resolved
zip-0320.rst Outdated Show resolved Hide resolved
zip-0320.rst Show resolved Hide resolved
Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

ACK e5a24b4

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.

LGTM!

A non-blocking comment would be to remove the UA alternative context from the Rationale section into its own section. It kind of comes out of the blue when I read through the text.

Copy link
Contributor

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

LGTM

@daira
Copy link
Collaborator Author

daira commented Mar 18, 2024

A non-blocking comment would be to remove the UA alternative context from the Rationale section into its own section. It kind of comes out of the blue when I read through the text.

I think it's too short now to justify another section.

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
@daira daira merged commit 3b706de into zcash:main Mar 18, 2024
@daira daira deleted the zip-320-choose-tex branch March 18, 2024 17:50
@daira daira removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2024
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.

ZIP 320: Update for decision to use TEX addresses
4 participants