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

Part of ZIP 212: validate Sapling and Orchard output of coinbase transactions #2362

Closed
dconnolly opened this issue Jun 21, 2021 · 6 comments · Fixed by #3029
Closed

Part of ZIP 212: validate Sapling and Orchard output of coinbase transactions #2362

dconnolly opened this issue Jun 21, 2021 · 6 comments · Fixed by #3029
Assignees
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-4 Canopy Network Upgrade: Canopy specific tasks

Comments

@dconnolly
Copy link
Contributor

Motivation

For NU4 (Canopy), ZIP-212 introduced a new consensus rule related to the format of Sapling Output note plaintexts. While we aren't sending/receiving money yet in Zebra, as per ZIP-213 we still need to decrypt and validate Sapling coinbase Outputs to the all-zero outgoing viewing key:

image

image

image

ZIP-212 is an update to the coinbase rules specified in ZIP-213.

Specifications

image

https://zips.z.cash/zip-0212

We need to implement "Decryption using a Full Viewing Key (Sapling and Orchard)":

image
image

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

Solution

We must move up our implementation of decrypting Sapling (and Orchard) notes for outgoing viewing keys, and use that in shielded coinbase checking, by decrypting the note ciphertexts of Sapling Outputs / Orchard Actions, and checking that the lead byte of the note plaintext resulting from decrypting to the all-zero 32 bytes outgoing viewing key is 0x02.

This relies on:

#2041
#269

This will require finishing / checking the Sapling and Orchard key derivation is complete according to the spec, and then use that as part of semantic transaction validation to check the shielded coinbase outputs can be decrypted under the all-zeros outgoing viewing key. The decryption implementation in zebra-chain should error / fail to represent a note plaintext with an incorrect leading byte (structural validation).

Alternatives

Not validate this consensus rule, and not be a fully validating consensus node.

Related Work

#2041
#269

@dconnolly dconnolly added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-4 Canopy Network Upgrade: Canopy specific tasks P-Medium labels Jun 21, 2021
@teor2345 teor2345 changed the title Part of ZIP 212: validate Sapling (and Orchard?) output of coinbase transactions Part of ZIP 212: validate Sapling and Orchard output of coinbase transactions Jun 21, 2021
@teor2345
Copy link
Contributor

teor2345 commented Jun 21, 2021

@dconnolly @conradoplg I think ZIP-212 is out of date here.

The consensus rules from the spec include Orchard:

[Heartwood onward] All Sapling and Orchard outputs in coinbase transactions MUST decrypt to a note plaintext , i.e. the procedure in § 4.19.3 ‘Decryption using a Full Viewing Key (Sapling and Orchard)’ on p. 65 does not return ⊥, using a sequence of 32 zero bytes as the outgoing viewing key.

[Canopy onward] Any Sapling or Orchard output of a coinbase transaction decrypted to a note plaintext according to the preceding rule MUST have note plaintext lead byte equal to 0x02. (This applies even during the “grace period” specified in [ZIP-212].)

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

@dconnolly
Copy link
Contributor Author

dconnolly commented Oct 13, 2021

Here's the zcash_note_encryption crate, using it for ZIP-212 coinbase note decryption compliance is as easy as calling a single function (after serializing/constructing the types), but I think for wallet/Zebra client, we'd like to port / freshly implement in zebra-chain with/against our own types:

https://github.com/zcash/librustzcash/blob/master/components/zcash_note_encryption/src/lib.rs

@mpguerra
Copy link
Contributor

mpguerra commented Nov 2, 2021

@dconnolly how does #2041 relate to this issue? Do we need to do it before this one? or is it already covered as part of this issue?

We should either add #2041 to sprint 22, remove the dependency on #2041 for this issue or close #2041 altogether depending on the answer.

@dconnolly
Copy link
Contributor Author

dconnolly commented Nov 8, 2021

@dconnolly how does #2041 relate to this issue? Do we need to do it before this one? or is it already covered as part of this issue?

We should either add #2041 to sprint 22, remove the dependency on #2041 for this issue or close #2041 altogether depending on the answer.

Per #3029, this #2041 is not strictly a blocker to #2362 because we do not need to derive the OutgoingCipherKey, as it is only a static value (all zeroes) to validate that consensus rule.

We need to implement #2041 for Orchard and the same for Sapling (but not Sprout) for client work, when we need to derive a non-static key to decrypt notes.

@teor2345
Copy link
Contributor

teor2345 commented Nov 8, 2021

We need to implement #2041 for Orchard and the same for Sapling (but not Sprout) to client work, when we need to derive a non-static key to decrypt notes.

I've removed #2041 from the sprint and left it in the wallet epic.

@dconnolly
Copy link
Contributor Author

I am removing #269 as a blocker to this issue alone, as we are able to validate this part of ZIP-212 using the function from zcash_note_encryption using the static all-zeroes OutgoingCipherKey alone, and moving it to the wallet epic.

Everything else in ZIP-212 will require finishing the derivation of the OutgoingCipherKey and finishing the Sapling/Orchard note encryption/decryption to work natively with our types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-4 Canopy Network Upgrade: Canopy specific tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants