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

Remove the check that Merkle roots are not ⊥ for Orchard #530

Closed
wants to merge 5 commits into from

Conversation

daira
Copy link
Collaborator

@daira daira commented Jun 30, 2021

No description provided.

…ecking that each intermediate

Merkle root of the note commitment tree is not ⊥. Checking this rule would have imposed a
significant performance penalty, since intermediate roots do not otherwise need to be computed.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
…ace of MerkleHash^Orchard ∪ {⊥}

for the inputs and output, and map a ⊥ output from SinsemillaHash to 0.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
… any output of

MerkleCRH^Orchard is 0, and add a note in \crossref{merklepath} arguing that this is safe.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
…h}: the length of the input

to SinsemillaHash is 10 + 2·ℓ^Orchard_Merkle bits, not 6 + 2·ℓ^Orchard_Merkle bits.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira daira requested review from ebfull and str4d June 30, 2021 14:40
@daira
Copy link
Collaborator Author

daira commented Jun 30, 2021

I suggest reviewing this commit-by-commit with the rendered PDF open.

@daira daira marked this pull request as ready for review June 30, 2021 14:47
@daira daira changed the title Remove the check that intermediate Merkle roots are not ⊥ for Orchard Remove the check that Merkle roots are not ⊥ for Orchard Jun 30, 2021

\vspace{-1ex}
\consensusrule{The \merkleRoot of the \Orchard \noteCommitmentTree \MUSTNOT be $\bot$
in any (intermediate or output) \treestate created by a \block.}
Copy link
Collaborator Author

@daira daira Jun 30, 2021

Choose a reason for hiding this comment

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

Note that after this change, the Orchard Merkle root in a final treestate (not just an intermediate treestate) is allowed to be 0. There's no advantage in making a distinction between intermediate and final roots, even if the latter is cheaper to check.

Comment on lines +5931 to +5935
\item For \Sapling, Merkle \merkleHashes are specified to be encoded as bit sequences, but the
\merkleRoot $\rt{Sapling}$ is encoded for the \primaryInput of a \spendProof as an element
of $\GF{\ParamJ{q}}$, as specified in \crossref{cctsaplingspend}. The \spendCircuit allows
inputs to $\MerkleCRH{Sapling}$ at each \merkleNode to be \nonCanonicallyFieldElement encoded,
as specified in \crossref{cctmerklepath}.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just moving the existing note for formatting reasons.

Comment on lines +5937 to +5940
\item For \Orchard, Merkle \merkleHashes have type $\MerkleHashOrchard$ as defined in
\crossref{concreteextractorpallas}. Similarly to \Sapling, the \actionCircuit allows
inputs to $\MerkleCRH{Orchard}$ at each \merkleNode to be \nonCanonicallyFieldElement
encoded.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similarly, this is just moving the existing note into a list.

\item It must be infeasible to find a input of length $6 + 2 \mult \MerkleHashLength{Orchard}$
to $\SinsemillaHash$ that yields output $\bot$.
\item $\SinsemillaHash$ must be \collisionResistant.
\item It must be infeasible to find a input of length $10 + 2 \mult \MerkleHashLength{Orchard}$
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was an independent typo.

\end{securityrequirements}

\pnote{The prefix $l$ provides domain separation between inputs at different layers of the
\pnote{The prefix $\layerRepr$ provides domain separation between inputs at different layers of the
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similarly, an independent fix.

Comment on lines +10876 to +10877
\item There is no solution to $y^2 = 0^3 + 5$ in $\GF{\ParamP{q}}$, and so $\ExtractP(P)$
can only be $0$ when $P = \ZeroP$.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to say this in order to argue that MerkleCRHOrchard only returns 0 for the exceptional case that we proved would yield a discrete log.

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. I am not well-enough versed in the protocol spec to identify anything that might have been missed, but I have verified that all of the changes match my understanding.

@daira
Copy link
Collaborator Author

daira commented Jul 6, 2021

This was included in v2021.2.9.

@daira daira closed this Jul 6, 2021
@daira daira deleted the bottomless-merklecrh branch July 6, 2021 15:29
zkbot added a commit to zcash/zcash that referenced this pull request Jul 9, 2021
…uttycom

Update Orchard commitment tree hashes to use total MerkleCRH^Orchard.

See zcash/zips#530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants