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

Orchard note commitment tree test vectors #2384

Merged
merged 7 commits into from
Jun 25, 2021
Merged

Conversation

dconnolly
Copy link
Contributor

@dconnolly dconnolly commented Jun 24, 2021

Motivation

We need to check conformance between our implementation of Orchard note and value commitments, note commitment trees, and anything else that uses the Sinsemilla hash.

Specifications

Fixed a bug about not encoding the j in S(j) as a 10 bit in 4 little endian bytes, we just were just encoding it with 2 bytes.

image

https://zips.z.cash/protocol/protocol.pdf#concretesinsemillahash

Solution

Bring in several test vectors for Orchard note commitment tree empty roots, and Sinsemilla test vectors, from the zcash-hackworks library, and cross-checked against existing implementations in the ECC orchard crate.

Review

@conradoplg @upbqdn (if they have time)

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour

Follow Up Work

Partially addresses #2079,
related to #1287

@dconnolly dconnolly changed the title Orchard test vectors Orchard note commitment tree test vectors Jun 24, 2021
@dconnolly dconnolly added this to the 2021 Sprint 12 milestone Jun 24, 2021
@dconnolly dconnolly requested review from conradoplg, jvff and upbqdn June 24, 2021 08:22
@dconnolly dconnolly added A-dependencies Area: Dependency file updates A-docs Area: Documentation A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks P-Medium labels Jun 24, 2021
@teor2345 teor2345 removed the request for review from jvff June 25, 2021 05:19
@teor2345
Copy link
Contributor

(I've removed jvff as a reviewer, so he can focus on network stuff.)

Copy link
Collaborator

@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.

Looks good! 👍

There is just a small documentation fix

zebra-chain/src/orchard/sinsemilla.rs Outdated Show resolved Hide resolved
@dconnolly dconnolly enabled auto-merge (squash) June 25, 2021 16:10
@dconnolly dconnolly requested a review from conradoplg June 25, 2021 16:11
@dconnolly dconnolly merged commit bb974fd into main Jun 25, 2021
@dconnolly dconnolly deleted the orchard-test-vectors branch June 25, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency file updates A-docs Area: Documentation A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants