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

ZIPs 32 and 316: Refine how IVK components are derived, and other cleanups #564

Merged
merged 19 commits into from
Dec 8, 2021

Conversation

daira
Copy link
Collaborator

@daira daira commented Oct 12, 2021

Co-authored-by: str4d jack@electriccoin.co
Co-authored-by: Kris Nuttycombe kris@electriccoin.co
Co-authored-by: Deirdre Connolly deirdre@zfnd.org
Signed-off-by: Daira Hopwood daira@jacaranda.org

@daira daira requested review from nuttycom, dconnolly and str4d and removed request for str4d October 12, 2021 22:28
@daira daira added this to the Core Sprint 2021-40 milestone Oct 12, 2021
@r3ld3v r3ld3v added the S-committed Status: Planned work in a sprint label Oct 13, 2021
zip-0032.rst Outdated Show resolved Hide resolved
zip-0032.rst Outdated Show resolved Hide resolved
zip-0032.rst Outdated Show resolved Hide resolved
zip-0316.rst Outdated Show resolved Hide resolved
zip-0316.rst Outdated Show resolved Hide resolved
zip-0316.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

Some small fixes/comments to add

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

zip-0316.rst Outdated Show resolved Hide resolved
@dconnolly
Copy link
Contributor

dconnolly commented Oct 27, 2021

An alternative idea from @str4d:

Amend orchard key tree to derive ask, nk, 2 rIVKs, one for recipient, one for change.

Both halves use the same nk and ak in their viewing keys

Would have to change how ovk and d_k are derived

prf_expand only depends on ak and nk, not rIVK

Only need adding an extra bit of data to the key tree

@str4d correct me

@str4d
Copy link
Collaborator

str4d commented Nov 19, 2021

What we decided on was the following:

  • Add a new domain separator for PRF_Expand
  • Derive a "change FVK" from the "normal FVK" via the same process as how we derive dk and ovk for Orchard.
  • From this "change FVK", derive "change ivk", "change ovk" and the "default change address" using the normal process.

Then when spending notes:

  • Send change to the default change address (we never use diversifiers for change, regardless of whether the notes we spent came from diversified addresses).
  • Make payment outputs recoverable with the normal ovk, and change outputs recoverable with the change ovk.

This has the following effects:

  • The normal FVK provides full visibility into payments and change.
  • The normal ivk provides visibility only into incoming payments.
  • The normal ovk (if used) provides visibility only into outgoing payments.
  • The UFVK and UIVK formats are unaltered (so we can undo a bunch of the changes in this PR).
    • There's little-to-no rationale for exposing the change ivk or ovk to someone (it would provide a very weird view), but if it were necessary, just provide two UIVKs.

@str4d
Copy link
Collaborator

str4d commented Nov 22, 2021

I've reverted all refinements to ZIP 32, in line with the above decision. There were refinements to ZIP 316 that I left in place (specifically the fix to how transparent IVKs were defined, which we want to retain).

zip-0316.rst Outdated Show resolved Hide resolved
str4d and others added 4 commits December 8, 2021 00:27
The hardened change path approach is being dropped. ZIP 316 will include
separate amendments (to be made later) that derive change addresses
within each protocol's key tree, instead of at the spend authorization
level.
The largest valid integer for any BIP 32 path element with a defined
hardening state (in this case, non-hardened) is 2^32 - 1 (being the
31-bit integer with all bits set to 1). The range of valid diversifier
indices for transparent-including UAs is defined as end-inclusive in
the ZIP, but used the end-exclusive bound 2^32.
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
…ncy.

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

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

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

dconnolly and others added 2 commits December 8, 2021 23:32
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
Co-authored-by: Kris Nuttycombe <kris.nuttycombe@gmail.com>
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira daira force-pushed the zip-ivk-changes branch 2 times, most recently from 6e9f5d0 to 983a67a Compare December 8, 2021 23:43
daira added 2 commits December 8, 2021 23:46
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira daira changed the title ZIPs 32 and 316: refine how UIVK components are derived for Transparent P2PKH ZIPs 32 and 316: Refine how IVK components are derived, and other cleanups Dec 8, 2021
@daira daira merged commit 227db1e into zcash:main Dec 8, 2021
@daira daira deleted the zip-ivk-changes branch December 8, 2021 23:49
@r3ld3v r3ld3v removed the S-committed Status: Planned work in a sprint label Dec 20, 2021
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.

5 participants